Skip to content

Commit 25608da

Browse files
manusamanusa
authored
fix(kubernetes): remove unneeded CacheInvalidate() method (127)
fix(kubernetes): remove unneeded CacheInvalidate() method --- test(output): improve age regex --- test(kubernetes): remove unneeded CacheInvalidate() method (mutex lock) --- test(kubernetes): split TestPodsTop to avoid discovery client cache issues
1 parent 2957faa commit 25608da

File tree

5 files changed

+47
-45
lines changed

5 files changed

+47
-45
lines changed

‎pkg/kubernetes/kubernetes.go‎

Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -164,16 +164,6 @@ func (m *Manager) Derived(ctx context.Context) *Kubernetes {
164164
return derived
165165
}
166166

167-
// TODO: check test to see why cache isn't getting invalidated automatically https://github.com/manusa/kubernetes-mcp-server/pull/125#discussion_r2152194784
168-
func (k *Kubernetes) CacheInvalidate() {
169-
if k.manager.discoveryClient != nil {
170-
k.manager.discoveryClient.Invalidate()
171-
}
172-
if k.manager.deferredDiscoveryRESTMapper != nil {
173-
k.manager.deferredDiscoveryRESTMapper.Reset()
174-
}
175-
}
176-
177167
func (k *Kubernetes) NewHelm() *helm.Helm {
178168
// This is a derived Kubernetes, so it already has the Helm initialized
179169
return helm.NewHelm(k.manager)

‎pkg/mcp/namespaces_test.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,7 @@ func TestNamespacesListAsTable(t *testing.T) {
7272
"(?<kind>Namespace)\\s+" +
7373
"(?<name>ns-1)\\s+" +
7474
"(?<status>Active)\\s+" +
75-
"(?<age>\\d+(s|m))\\s+" +
75+
"(?<age>(\\d+m)?\\d+s)\\s+" +
7676
"(?<labels>kubernetes.io/metadata.name=ns-1)"
7777
if m, e := regexp.MatchString(expectedRow, out); !m || e != nil {
7878
t.Fatalf("Expected row '%s' not found in output:\n%s", expectedRow, out)
@@ -83,7 +83,7 @@ func TestNamespacesListAsTable(t *testing.T) {
8383
"(?<kind>Namespace)\\s+" +
8484
"(?<name>ns-2)\\s+" +
8585
"(?<status>Active)\\s+" +
86-
"(?<age>\\d+(s|m))\\s+" +
86+
"(?<age>(\\d+m)?\\d+s)\\s+" +
8787
"(?<labels>kubernetes.io/metadata.name=ns-2)"
8888
if m, e := regexp.MatchString(expectedRow, out); !m || e != nil {
8989
t.Fatalf("Expected row '%s' not found in output:\n%s", expectedRow, out)

‎pkg/mcp/pods_test.go‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -209,7 +209,7 @@ func TestPodsListAsTable(t *testing.T) {
209209
"(?<ready>0\\/1)\\s+" +
210210
"(?<status>Pending)\\s+" +
211211
"(?<restarts>0)\\s+" +
212-
"(?<age>\\d+(s|m))\\s+" +
212+
"(?<age>(\\d+m)?\\d+s)\\s+" +
213213
"(?<ip><none>)\\s+" +
214214
"(?<node><none>)\\s+" +
215215
"(?<nominated_node><none>)\\s+" +
@@ -227,7 +227,7 @@ func TestPodsListAsTable(t *testing.T) {
227227
"(?<ready>0\\/1)\\s+" +
228228
"(?<status>Pending)\\s+" +
229229
"(?<restarts>0)\\s+" +
230-
"(?<age>\\d+(s|m))\\s+" +
230+
"(?<age>(\\d+m)?\\d+s)\\s+" +
231231
"(?<ip><none>)\\s+" +
232232
"(?<node><none>)\\s+" +
233233
"(?<nominated_node><none>)\\s+" +
@@ -270,7 +270,7 @@ func TestPodsListAsTable(t *testing.T) {
270270
"(?<ready>0\\/1)\\s+" +
271271
"(?<status>Pending)\\s+" +
272272
"(?<restarts>0)\\s+" +
273-
"(?<age>\\d+(s|m))\\s+" +
273+
"(?<age>(\\d+m)?\\d+s)\\s+" +
274274
"(?<ip><none>)\\s+" +
275275
"(?<node><none>)\\s+" +
276276
"(?<nominated_node><none>)\\s+" +

‎pkg/mcp/pods_top_test.go‎

Lines changed: 40 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -7,23 +7,50 @@ import (
77
"testing"
88
)
99

10-
func TestPodsTop(t *testing.T) {
10+
func TestPodsTopMetricsUnavailable(t *testing.T) {
1111
testCase(t, func(c *mcpContext) {
1212
mockServer := NewMockServer()
1313
defer mockServer.Close()
1414
c.withKubeConfig(mockServer.config)
15-
metricsApiAvailable := false
1615
mockServer.Handle(http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) {
17-
println("Request received:", req.Method, req.URL.Path) // TODO: REMOVE LINE
1816
w.Header().Set("Content-Type", "application/json")
1917
// Request Performed by DiscoveryClient to Kube API (Get API Groups legacy -core-)
2018
if req.URL.Path == "/api" {
21-
if !metricsApiAvailable {
19+
_, _ = w.Write([]byte(`{"kind":"APIVersions","versions":[],"serverAddressByClientCIDRs":[{"clientCIDR":"0.0.0.0/0"}]}`))
20+
return
21+
}
22+
// Request Performed by DiscoveryClient to Kube API (Get API Groups)
23+
if req.URL.Path == "/apis" {
24+
_, _ = w.Write([]byte(`{"kind":"APIGroupList","apiVersion":"v1","groups":[]}`))
25+
return
26+
}
27+
}))
28+
podsTopMetricsApiUnavailable, err := c.callTool("pods_top", map[string]interface{}{})
29+
t.Run("pods_top with metrics API not available", func(t *testing.T) {
30+
if err != nil {
31+
t.Fatalf("call tool failed %v", err)
32+
}
33+
if !podsTopMetricsApiUnavailable.IsError {
34+
t.Errorf("call tool should have returned an error")
35+
}
36+
if podsTopMetricsApiUnavailable.Content[0].(mcp.TextContent).Text != "failed to get pods top: metrics API is not available" {
37+
t.Errorf("call tool returned unexpected content: %s", podsTopMetricsApiUnavailable.Content[0].(mcp.TextContent).Text)
38+
}
39+
})
40+
})
41+
}
2242

23-
_, _ = w.Write([]byte(`{"kind":"APIVersions","versions":[],"serverAddressByClientCIDRs":[{"clientCIDR":"0.0.0.0/0"}]}`))
24-
} else {
25-
_, _ = w.Write([]byte(`{"kind":"APIVersions","versions":["metrics.k8s.io/v1beta1"],"serverAddressByClientCIDRs":[{"clientCIDR":"0.0.0.0/0"}]}`))
26-
}
43+
func TestPodsTopMetricsAvailable(t *testing.T) {
44+
testCase(t, func(c *mcpContext) {
45+
mockServer := NewMockServer()
46+
defer mockServer.Close()
47+
c.withKubeConfig(mockServer.config)
48+
mockServer.Handle(http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) {
49+
println("Request received:", req.Method, req.URL.Path) // TODO: REMOVE LINE
50+
w.Header().Set("Content-Type", "application/json")
51+
// Request Performed by DiscoveryClient to Kube API (Get API Groups legacy -core-)
52+
if req.URL.Path == "/api" {
53+
_, _ = w.Write([]byte(`{"kind":"APIVersions","versions":["metrics.k8s.io/v1beta1"],"serverAddressByClientCIDRs":[{"clientCIDR":"0.0.0.0/0"}]}`))
2754
return
2855
}
2956
// Request Performed by DiscoveryClient to Kube API (Get API Groups)
@@ -32,12 +59,12 @@ func TestPodsTop(t *testing.T) {
3259
return
3360
}
3461
// Request Performed by DiscoveryClient to Kube API (Get API Resources)
35-
if metricsApiAvailable && req.URL.Path == "/apis/metrics.k8s.io/v1beta1" {
62+
if req.URL.Path == "/apis/metrics.k8s.io/v1beta1" {
3663
_, _ = w.Write([]byte(`{"kind":"APIResourceList","apiVersion":"v1","groupVersion":"metrics.k8s.io/v1beta1","resources":[{"name":"pods","singularName":"","namespaced":true,"kind":"PodMetrics","verbs":["get","list"]}]}`))
3764
return
3865
}
3966
// Pod Metrics from all namespaces
40-
if metricsApiAvailable && req.URL.Path == "/apis/metrics.k8s.io/v1beta1/pods" {
67+
if req.URL.Path == "/apis/metrics.k8s.io/v1beta1/pods" {
4168
if req.URL.Query().Get("labelSelector") == "app=pod-ns-5-42" {
4269
_, _ = w.Write([]byte(`{"kind":"PodMetricsList","apiVersion":"metrics.k8s.io/v1beta1","items":[` +
4370
`{"metadata":{"name":"pod-ns-5-42","namespace":"ns-5"},"containers":[{"name":"container-1","usage":{"cpu":"42m","memory":"42Mi"}}]}` +
@@ -52,42 +79,27 @@ func TestPodsTop(t *testing.T) {
5279
return
5380
}
5481
// Pod Metrics from configured namespace
55-
if metricsApiAvailable && req.URL.Path == "/apis/metrics.k8s.io/v1beta1/namespaces/default/pods" {
82+
if req.URL.Path == "/apis/metrics.k8s.io/v1beta1/namespaces/default/pods" {
5683
_, _ = w.Write([]byte(`{"kind":"PodMetricsList","apiVersion":"metrics.k8s.io/v1beta1","items":[` +
5784
`{"metadata":{"name":"pod-1","namespace":"default"},"containers":[{"name":"container-1","usage":{"cpu":"10m","memory":"20Mi"}},{"name":"container-2","usage":{"cpu":"30m","memory":"40Mi"}}]}` +
5885
`]}`))
5986
return
6087
}
6188
// Pod Metrics from ns-5 namespace
62-
if metricsApiAvailable && req.URL.Path == "/apis/metrics.k8s.io/v1beta1/namespaces/ns-5/pods" {
89+
if req.URL.Path == "/apis/metrics.k8s.io/v1beta1/namespaces/ns-5/pods" {
6390
_, _ = w.Write([]byte(`{"kind":"PodMetricsList","apiVersion":"metrics.k8s.io/v1beta1","items":[` +
6491
`{"metadata":{"name":"pod-ns-5-1","namespace":"ns-5"},"containers":[{"name":"container-1","usage":{"cpu":"10m","memory":"20Mi"}}]}` +
6592
`]}`))
6693
return
6794
}
6895
// Pod Metrics from ns-5 namespace with pod-ns-5-5 pod name
69-
if metricsApiAvailable && req.URL.Path == "/apis/metrics.k8s.io/v1beta1/namespaces/ns-5/pods/pod-ns-5-5" {
96+
if req.URL.Path == "/apis/metrics.k8s.io/v1beta1/namespaces/ns-5/pods/pod-ns-5-5" {
7097
_, _ = w.Write([]byte(`{"kind":"PodMetrics","apiVersion":"metrics.k8s.io/v1beta1",` +
7198
`"metadata":{"name":"pod-ns-5-5","namespace":"ns-5"},` +
7299
`"containers":[{"name":"container-1","usage":{"cpu":"13m","memory":"37Mi"}}]` +
73100
`}`))
74101
}
75102
}))
76-
podsTopMetricsApiUnavailable, err := c.callTool("pods_top", map[string]interface{}{})
77-
t.Run("pods_top with metrics API not available", func(t *testing.T) {
78-
if err != nil {
79-
t.Fatalf("call tool failed %v", err)
80-
}
81-
if !podsTopMetricsApiUnavailable.IsError {
82-
t.Errorf("call tool should have returned an error")
83-
}
84-
if podsTopMetricsApiUnavailable.Content[0].(mcp.TextContent).Text != "failed to get pods top: metrics API is not available" {
85-
t.Errorf("call tool returned unexpected content: %s", podsTopMetricsApiUnavailable.Content[0].(mcp.TextContent).Text)
86-
}
87-
})
88-
// Enable metrics API addon
89-
metricsApiAvailable = true
90-
c.mcpServer.k.Derived(t.Context()).CacheInvalidate() // Force discovery client to refresh
91103
podsTopDefaults, err := c.callTool("pods_top", map[string]interface{}{})
92104
t.Run("pods_top defaults returns pod metrics from all namespaces", func(t *testing.T) {
93105
if err != nil {

‎pkg/mcp/resources_test.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -178,7 +178,7 @@ func TestResourcesListAsTable(t *testing.T) {
178178
"(?<kind>ConfigMap)\\s+" +
179179
"(?<name>a-configmap-to-list-as-table)\\s+" +
180180
"(?<data>1)\\s+" +
181-
"(?<age>\\d+(s|m))\\s+" +
181+
"(?<age>(\\d+m)?\\d+s)\\s+" +
182182
"(?<labels>resource=config-map)"
183183
if m, e := regexp.MatchString(expectedRow, outConfigMapList); !m || e != nil {
184184
t.Fatalf("Expected row '%s' not found in output:\n%s", expectedRow, outConfigMapList)
@@ -216,7 +216,7 @@ func TestResourcesListAsTable(t *testing.T) {
216216
"(?<apiVersion>route.openshift.io/v1)\\s+" +
217217
"(?<kind>Route)\\s+" +
218218
"(?<name>an-openshift-route-to-list-as-table)\\s+" +
219-
"(?<age>\\d+(s|m))\\s+" +
219+
"(?<age>(\\d+m)?\\d+s)\\s+" +
220220
"(?<labels><none>)"
221221
if m, e := regexp.MatchString(expectedRow, outRouteList); !m || e != nil {
222222
t.Fatalf("Expected row '%s' not found in output:\n%s", expectedRow, outRouteList)

0 commit comments

Comments
 (0)