Skip to content

Commit b7fde2e

Browse files
committed
update few errors
Signed-off-by: Rohit Patil <[email protected]>
1 parent afb7d63 commit b7fde2e

File tree

3 files changed

+104
-29
lines changed

3 files changed

+104
-29
lines changed

pkg/promptsets/core/health_check.go

Lines changed: 51 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -2,12 +2,32 @@ package core
22

33
import (
44
"fmt"
5+
"strings"
56

67
"github.com/containers/kubernetes-mcp-server/pkg/api"
7-
internalk8s "github.com/containers/kubernetes-mcp-server/pkg/kubernetes"
88
)
99

10-
func initHealthCheckPrompts(o internalk8s.Openshift) []api.ServerPrompt {
10+
const (
11+
// Health check configuration constants
12+
defaultRestartThreshold = 5
13+
eventLookbackMinutes = 30
14+
maxWarningEvents = 20
15+
)
16+
17+
// isVerboseEnabled checks if the verbose flag is enabled.
18+
// It accepts "true", "1", "yes", or "y" (case-insensitive) as truthy values.
19+
func isVerboseEnabled(value string) bool {
20+
switch strings.ToLower(value) {
21+
case "true", "1", "yes", "y":
22+
return true
23+
default:
24+
return false
25+
}
26+
}
27+
28+
// initHealthCheckPrompts creates prompts for cluster health diagnostics.
29+
// These prompts guide LLMs to systematically check cluster components using existing tools.
30+
func initHealthCheckPrompts() []api.ServerPrompt {
1131
return []api.ServerPrompt{
1232
{
1333
Name: "cluster_health_check",
@@ -25,7 +45,7 @@ func initHealthCheckPrompts(o internalk8s.Openshift) []api.ServerPrompt {
2545
},
2646
},
2747
GetMessages: func(arguments map[string]string) []api.PromptMessage {
28-
verbose := arguments["verbose"] == "true"
48+
verbose := isVerboseEnabled(arguments["verbose"])
2949
namespace := arguments["namespace"]
3050

3151
return buildHealthCheckPromptMessages(verbose, namespace)
@@ -34,13 +54,15 @@ func initHealthCheckPrompts(o internalk8s.Openshift) []api.ServerPrompt {
3454
}
3555
}
3656

57+
// buildHealthCheckPromptMessages constructs the prompt messages for cluster health checks.
58+
// It adapts the instructions based on verbose mode and namespace filtering.
3759
func buildHealthCheckPromptMessages(verbose bool, namespace string) []api.PromptMessage {
3860
scopeMsg := "across all namespaces"
3961
podListInstruction := "- Use pods_list to get all pods"
4062

4163
if namespace != "" {
4264
scopeMsg = fmt.Sprintf("in namespace '%s'", namespace)
43-
podListInstruction = fmt.Sprintf("- Use pods_list_in_namespace with namespace parameter set to '%s' to get all pods in namespace '%s'", namespace, namespace)
65+
podListInstruction = fmt.Sprintf("- Use pods_list_in_namespace with namespace '%s'", namespace)
4466
}
4567

4668
verboseMsg := ""
@@ -52,14 +74,17 @@ func buildHealthCheckPromptMessages(verbose bool, namespace string) []api.Prompt
5274
"- Event messages and timestamps"
5375
}
5476

77+
// Construct the event display range dynamically using maxWarningEvents
78+
eventDisplayRange := fmt.Sprintf("10-%d", maxWarningEvents)
79+
5580
userMessage := fmt.Sprintf(`Please perform a comprehensive health check on the Kubernetes cluster %s.
5681
5782
Follow these steps systematically:
5883
5984
## 1. Check Cluster-Level Components
6085
6186
### For OpenShift Clusters:
62-
- Use resources_list with kind=ClusterOperator to check cluster operator health
87+
- Use resources_list with apiVersion=config.openshift.io/v1 and kind=ClusterOperator to check cluster operator health
6388
- Look for operators with:
6489
* Degraded=True (CRITICAL)
6590
* Available=False (CRITICAL)
@@ -70,7 +95,7 @@ Follow these steps systematically:
7095
- Note the cluster type in your report
7196
7297
## 2. Check Node Health
73-
- Use resources_list with kind=Node to examine all nodes
98+
- Use resources_list with apiVersion=v1 and kind=Node to examine all nodes
7499
- Check each node for:
75100
* Ready condition != True (CRITICAL)
76101
* Unschedulable spec field = true (WARNING)
@@ -84,30 +109,30 @@ Follow these steps systematically:
84109
* Container state waiting with reason:
85110
- CrashLoopBackOff (CRITICAL)
86111
- ImagePullBackOff or ErrImagePull (CRITICAL)
87-
* RestartCount > 5 (WARNING - configurable threshold)
112+
* RestartCount > %d (WARNING - configurable threshold)
88113
- Group issues by type and count occurrences
89114
90115
## 4. Check Workload Controllers
91116
- Use resources_list for each workload type:
92-
* kind=Deployment (apps/v1)
93-
* kind=StatefulSet (apps/v1)
94-
* kind=DaemonSet (apps/v1)
117+
* apiVersion=apps/v1, kind=Deployment
118+
* apiVersion=apps/v1, kind=StatefulSet
119+
* apiVersion=apps/v1, kind=DaemonSet
95120
- For each controller, compare:
96121
* spec.replicas vs status.readyReplicas (Deployment/StatefulSet)
97122
* status.desiredNumberScheduled vs status.numberReady (DaemonSet)
98123
* Report mismatches as WARNINGs
99124
100125
## 5. Check Storage
101-
- Use resources_list with kind=PersistentVolumeClaim
126+
- Use resources_list with apiVersion=v1 and kind=PersistentVolumeClaim
102127
- Identify PVCs not in Bound phase (WARNING)
103128
- Note namespace and PVC name for each issue
104129
105130
## 6. Check Recent Events (Optional)
106131
- Use events_list to get cluster events
107132
- Filter for:
108133
* Type = Warning
109-
* Timestamp within last 30 minutes
110-
- Limit to 10-20 most recent warnings
134+
* Timestamp within last %d minutes
135+
- Limit to %s most recent warnings
111136
- Include event message and involved object%s
112137
113138
## Output Format
@@ -139,7 +164,7 @@ Scope: [all namespaces / specific namespace]
139164
[PVC status: total, bound, pending/other]
140165
141166
### Recent Events
142-
[Warning events from last 30 minutes]
167+
[Warning events from last %d minutes]
143168
144169
================================================
145170
Summary
@@ -162,7 +187,18 @@ Warnings: [count]
162187
- Be efficient: don't call the same tool multiple times unnecessarily
163188
- If a resource type doesn't exist (e.g., ClusterOperator on vanilla K8s), skip it gracefully
164189
- Provide clear, actionable insights in your summary
165-
- Use emojis for visual clarity: ✅ (healthy), ⚠️ (warning), ❌ (critical)`, scopeMsg, podListInstruction, verboseMsg)
190+
- Use emojis for visual clarity: ✅ (healthy), ⚠️ (warning), ❌ (critical)
191+
192+
### Common apiVersion Values
193+
194+
When using resources_list, specify the correct apiVersion for each resource type:
195+
- Core resources: apiVersion=v1 (Pod, Service, Node, PersistentVolumeClaim, ConfigMap, Secret, Namespace)
196+
- Apps: apiVersion=apps/v1 (Deployment, StatefulSet, DaemonSet, ReplicaSet)
197+
- Batch: apiVersion=batch/v1 (Job, CronJob)
198+
- RBAC: apiVersion=rbac.authorization.k8s.io/v1 (Role, RoleBinding, ClusterRole, ClusterRoleBinding)
199+
- Networking: apiVersion=networking.k8s.io/v1 (Ingress, NetworkPolicy)
200+
- OpenShift Config: apiVersion=config.openshift.io/v1 (ClusterOperator, ClusterVersion)
201+
- OpenShift Routes: apiVersion=route.openshift.io/v1 (Route)`, scopeMsg, podListInstruction, defaultRestartThreshold, eventLookbackMinutes, eventDisplayRange, verboseMsg, eventLookbackMinutes)
166202

167203
assistantMessage := `I'll perform a comprehensive cluster health check following the systematic approach outlined. Let me start by gathering information about the cluster components.`
168204

pkg/promptsets/core/health_check_test.go

Lines changed: 52 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -8,9 +8,39 @@ import (
88
"github.com/stretchr/testify/require"
99
)
1010

11+
func TestIsVerboseEnabled(t *testing.T) {
12+
tests := []struct {
13+
name string
14+
input string
15+
expected bool
16+
}{
17+
{"true lowercase", "true", true},
18+
{"true capitalized", "True", true},
19+
{"true uppercase", "TRUE", true},
20+
{"numeric 1", "1", true},
21+
{"yes lowercase", "yes", true},
22+
{"yes capitalized", "Yes", true},
23+
{"yes uppercase", "YES", true},
24+
{"y lowercase", "y", true},
25+
{"y uppercase", "Y", true},
26+
{"false", "false", false},
27+
{"0", "0", false},
28+
{"no", "no", false},
29+
{"empty string", "", false},
30+
{"random string", "random", false},
31+
}
32+
33+
for _, tt := range tests {
34+
t.Run(tt.name, func(t *testing.T) {
35+
result := isVerboseEnabled(tt.input)
36+
assert.Equal(t, tt.expected, result, "isVerboseEnabled(%q) should return %v", tt.input, tt.expected)
37+
})
38+
}
39+
}
40+
1141
func TestInitHealthCheckPrompts(t *testing.T) {
1242
// When
13-
prompts := initHealthCheckPrompts(nil)
43+
prompts := initHealthCheckPrompts()
1444

1545
// Then
1646
require.Len(t, prompts, 1)
@@ -58,7 +88,7 @@ func TestBuildHealthCheckPromptMessages(t *testing.T) {
5888
userContent := messages[0].Content
5989
assert.Contains(t, userContent, "in namespace 'test-namespace'")
6090
assert.NotContains(t, userContent, "across all namespaces")
61-
assert.Contains(t, userContent, "Use pods_list_in_namespace with namespace parameter set to 'test-namespace'")
91+
assert.Contains(t, userContent, "Use pods_list_in_namespace with namespace 'test-namespace'")
6292
assert.NotContains(t, userContent, "Use pods_list to get all pods")
6393
})
6494

@@ -156,24 +186,25 @@ func TestBuildHealthCheckPromptMessages(t *testing.T) {
156186
}
157187
})
158188

159-
t.Run("User message contains workload types", func(t *testing.T) {
189+
t.Run("User message contains workload types with apiVersions", func(t *testing.T) {
160190
// When
161191
messages := buildHealthCheckPromptMessages(false, "")
162192

163193
// Then
164194
userContent := messages[0].Content
165195

166-
workloadTypes := []string{
167-
"kind=Deployment",
168-
"kind=StatefulSet",
169-
"kind=DaemonSet",
170-
"kind=ClusterOperator",
171-
"kind=Node",
172-
"kind=PersistentVolumeClaim",
196+
// Check for apiVersion + kind pairs
197+
resourceSpecs := []string{
198+
"apiVersion=apps/v1, kind=Deployment",
199+
"apiVersion=apps/v1, kind=StatefulSet",
200+
"apiVersion=apps/v1, kind=DaemonSet",
201+
"apiVersion=config.openshift.io/v1 and kind=ClusterOperator",
202+
"apiVersion=v1 and kind=Node",
203+
"apiVersion=v1 and kind=PersistentVolumeClaim",
173204
}
174205

175-
for _, wl := range workloadTypes {
176-
assert.Contains(t, userContent, wl, "Missing workload type: %s", wl)
206+
for _, spec := range resourceSpecs {
207+
assert.Contains(t, userContent, spec, "Missing resource spec: %s", spec)
177208
}
178209
})
179210

@@ -218,7 +249,7 @@ func TestBuildHealthCheckPromptMessages(t *testing.T) {
218249

219250
func TestGetMessagesWithArguments(t *testing.T) {
220251
// Given
221-
prompts := initHealthCheckPrompts(nil)
252+
prompts := initHealthCheckPrompts()
222253
require.Len(t, prompts, 1)
223254

224255
getMessages := prompts[0].GetMessages
@@ -329,4 +360,12 @@ func TestHealthCheckPromptCompleteness(t *testing.T) {
329360
}
330361
assert.Greater(t, foundVerbs, 3, "Prompt should use clear imperative language")
331362
})
363+
364+
t.Run("Includes apiVersion reference section", func(t *testing.T) {
365+
assert.Contains(t, userContent, "Common apiVersion Values")
366+
assert.Contains(t, userContent, "apiVersion=config.openshift.io/v1")
367+
assert.Contains(t, userContent, "apiVersion=apps/v1")
368+
assert.Contains(t, userContent, "apiVersion=v1")
369+
assert.Contains(t, userContent, "ClusterOperator, ClusterVersion")
370+
})
332371
}

pkg/promptsets/core/promptset.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ func (t *PromptSet) GetPrompts(o internalk8s.Openshift) []api.ServerPrompt {
2525
prompts := make([]api.ServerPrompt, 0)
2626

2727
// Health check prompts
28-
prompts = append(prompts, initHealthCheckPrompts(o)...)
28+
prompts = append(prompts, initHealthCheckPrompts()...)
2929

3030
// Future: Add more prompts here
3131
// prompts = append(prompts, initTroubleshootingPrompts(o)...)

0 commit comments

Comments
 (0)