From b1a5756a4d713cbe34055da4af5351abb44b071d Mon Sep 17 00:00:00 2001 From: Mangirdas Judeikis Date: Thu, 21 May 2026 11:36:00 +0300 Subject: [PATCH 1/4] Fixup openapi controller eviction --- pkg/server/openapiv3/controller.go | 12 +++++ pkg/server/openapiv3/controller_test.go | 60 +++++++++++++++++++++++++ pkg/server/openapiv3_evictor.go | 58 ++++++++++++++++++++++++ pkg/server/server.go | 1 + 4 files changed, 131 insertions(+) create mode 100644 pkg/server/openapiv3/controller_test.go create mode 100644 pkg/server/openapiv3_evictor.go diff --git a/pkg/server/openapiv3/controller.go b/pkg/server/openapiv3/controller.go index 354701199b8..a6bac4ddcf3 100644 --- a/pkg/server/openapiv3/controller.go +++ b/pkg/server/openapiv3/controller.go @@ -267,3 +267,15 @@ func (c *Controller) GetCRDSpecs(clusterName logicalcluster.Name, name string) ( return specs, nil } + +// EvictCluster removes all cached OpenAPI v3 specs for clusterName. Normally +// the entry is cleaned per-CRD via the CRD informer's delete event handler, +// but if the LogicalCluster delete races ahead of those CRD deletes (or any +// CRD delete event is missed), the bucket — which holds 100KB–1MB of built +// spec per CRD version — would leak for the lifetime of the process. Call +// this from a LogicalCluster delete handler to bound retention. +func (c *Controller) EvictCluster(clusterName logicalcluster.Name) { + c.lock.Lock() + defer c.lock.Unlock() + delete(c.byClusterNameVersion, clusterName) +} diff --git a/pkg/server/openapiv3/controller_test.go b/pkg/server/openapiv3/controller_test.go new file mode 100644 index 00000000000..1ce8841145d --- /dev/null +++ b/pkg/server/openapiv3/controller_test.go @@ -0,0 +1,60 @@ +/* +Copyright 2026 The kcp Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package openapiv3 + +import ( + "sync" + "testing" + + "k8s.io/kube-openapi/pkg/cached" + "k8s.io/kube-openapi/pkg/spec3" + + "github.com/kcp-dev/logicalcluster/v3" +) + +// TestEvictCluster verifies EvictCluster drops the whole per-cluster +// bucket. This is the back-stop for missed CRD delete events: if a +// LogicalCluster is gone, every cached spec under it must go too. +func TestEvictCluster(t *testing.T) { + c := &Controller{ + byClusterNameVersion: map[logicalcluster.Name]map[string]map[string]cached.Value[*spec3.OpenAPI]{ + "target": {"crd-a": {"v1": cached.Static(&spec3.OpenAPI{}, "etag")}}, + "other": {"crd-b": {"v1": cached.Static(&spec3.OpenAPI{}, "etag")}}, + "another": {"crd-c": {"v1": cached.Static(&spec3.OpenAPI{}, "etag")}}, + }, + } + c.lock = sync.Mutex{} + + c.EvictCluster("target") + + if _, ok := c.byClusterNameVersion["target"]; ok { + t.Error("byClusterNameVersion[target] should be removed after EvictCluster") + } + if _, ok := c.byClusterNameVersion["other"]; !ok { + t.Error("byClusterNameVersion[other] must be preserved") + } + if _, ok := c.byClusterNameVersion["another"]; !ok { + t.Error("byClusterNameVersion[another] must be preserved") + } + + // Idempotent: evicting again must not panic and must leave others alone. + c.EvictCluster("target") + c.EvictCluster("never-existed") + if len(c.byClusterNameVersion) != 2 { + t.Errorf("expected 2 remaining buckets, got %d", len(c.byClusterNameVersion)) + } +} diff --git a/pkg/server/openapiv3_evictor.go b/pkg/server/openapiv3_evictor.go new file mode 100644 index 00000000000..5a3552493c1 --- /dev/null +++ b/pkg/server/openapiv3_evictor.go @@ -0,0 +1,58 @@ +/* +Copyright 2026 The kcp Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package server + +import ( + "k8s.io/client-go/tools/cache" + "k8s.io/klog/v2" + + "github.com/kcp-dev/logicalcluster/v3" + corev1alpha1 "github.com/kcp-dev/sdk/apis/core/v1alpha1" + corev1alpha1informers "github.com/kcp-dev/sdk/client/informers/externalversions/core/v1alpha1" + + "github.com/kcp-dev/kcp/pkg/server/openapiv3" +) + +// installOpenAPIV3Evictor registers a LogicalCluster delete handler that drops +// every cached OpenAPI v3 spec for the cluster. The controller's CRD informer +// handler scrubs entries per-CRD, but that path depends on each CRD delete +// event being observed and ordered before the LogicalCluster delete. When that +// ordering breaks the bucket leaks 100KB–1MB per CRD version forever. See +// https://github.com/kcp-dev/kcp/issues/4071. +func installOpenAPIV3Evictor(informer corev1alpha1informers.LogicalClusterClusterInformer, controller *openapiv3.Controller) { + _, _ = informer.Informer().AddEventHandler(cache.ResourceEventHandlerFuncs{ + DeleteFunc: func(obj any) { + lc, ok := obj.(*corev1alpha1.LogicalCluster) + if !ok { + tombstone, tok := obj.(cache.DeletedFinalStateUnknown) + if !tok { + return + } + lc, ok = tombstone.Obj.(*corev1alpha1.LogicalCluster) + if !ok { + return + } + } + name := logicalcluster.From(lc) + if name == "" { + return + } + klog.V(4).InfoS("evicting OpenAPI v3 cache", "logicalcluster", name) + controller.EvictCluster(name) + }, + }) +} diff --git a/pkg/server/server.go b/pkg/server/server.go index e8173ec011f..dd7f1e2883f 100644 --- a/pkg/server/server.go +++ b/pkg/server/server.go @@ -739,6 +739,7 @@ func (s *Server) Run(ctx context.Context) error { }); err != nil { return err } + installOpenAPIV3Evictor(s.KcpSharedInformerFactory.Core().V1alpha1().LogicalClusters(), s.openAPIv3Controller) go s.openAPIv3Controller.Run(ctx) return nil }); err != nil { From 21cb972017f89881922f752ba46ac4913e45e1e7 Mon Sep 17 00:00:00 2001 From: Mangirdas Judeikis Date: Thu, 21 May 2026 13:24:49 +0300 Subject: [PATCH 2/4] adjust context logger --- pkg/server/openapiv3_evictor.go | 7 +++++-- pkg/server/server.go | 2 +- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/pkg/server/openapiv3_evictor.go b/pkg/server/openapiv3_evictor.go index 5a3552493c1..a7f65c7daf9 100644 --- a/pkg/server/openapiv3_evictor.go +++ b/pkg/server/openapiv3_evictor.go @@ -17,6 +17,8 @@ limitations under the License. package server import ( + "context" + "k8s.io/client-go/tools/cache" "k8s.io/klog/v2" @@ -33,7 +35,8 @@ import ( // event being observed and ordered before the LogicalCluster delete. When that // ordering breaks the bucket leaks 100KB–1MB per CRD version forever. See // https://github.com/kcp-dev/kcp/issues/4071. -func installOpenAPIV3Evictor(informer corev1alpha1informers.LogicalClusterClusterInformer, controller *openapiv3.Controller) { +func installOpenAPIV3Evictor(ctx context.Context, informer corev1alpha1informers.LogicalClusterClusterInformer, controller *openapiv3.Controller) { + logger := klog.FromContext(ctx).WithName("openapiv3-evictor") _, _ = informer.Informer().AddEventHandler(cache.ResourceEventHandlerFuncs{ DeleteFunc: func(obj any) { lc, ok := obj.(*corev1alpha1.LogicalCluster) @@ -51,7 +54,7 @@ func installOpenAPIV3Evictor(informer corev1alpha1informers.LogicalClusterCluste if name == "" { return } - klog.V(4).InfoS("evicting OpenAPI v3 cache", "logicalcluster", name) + logger.V(4).Info("evicting OpenAPI v3 cache", "logicalcluster", name) controller.EvictCluster(name) }, }) diff --git a/pkg/server/server.go b/pkg/server/server.go index dd7f1e2883f..2ba73553f9f 100644 --- a/pkg/server/server.go +++ b/pkg/server/server.go @@ -739,7 +739,7 @@ func (s *Server) Run(ctx context.Context) error { }); err != nil { return err } - installOpenAPIV3Evictor(s.KcpSharedInformerFactory.Core().V1alpha1().LogicalClusters(), s.openAPIv3Controller) + installOpenAPIV3Evictor(ctx, s.KcpSharedInformerFactory.Core().V1alpha1().LogicalClusters(), s.openAPIv3Controller) go s.openAPIv3Controller.Run(ctx) return nil }); err != nil { From 92d568f43d0663a23035f4aee5a19d5cc6cddb79 Mon Sep 17 00:00:00 2001 From: "Nelo-T. Wallus" <10514301+ntnn@users.noreply.github.com> Date: Fri, 22 May 2026 09:29:31 +0200 Subject: [PATCH 3/4] Remove unnecessary mutex initialization Remove mutex initialization before EvictCluster call. --- pkg/server/openapiv3/controller_test.go | 1 - 1 file changed, 1 deletion(-) diff --git a/pkg/server/openapiv3/controller_test.go b/pkg/server/openapiv3/controller_test.go index 1ce8841145d..0181f371e99 100644 --- a/pkg/server/openapiv3/controller_test.go +++ b/pkg/server/openapiv3/controller_test.go @@ -37,7 +37,6 @@ func TestEvictCluster(t *testing.T) { "another": {"crd-c": {"v1": cached.Static(&spec3.OpenAPI{}, "etag")}}, }, } - c.lock = sync.Mutex{} c.EvictCluster("target") From b26d292f64276806ef914c09681307b806c393ee Mon Sep 17 00:00:00 2001 From: Mangirdas Judeikis Date: Mon, 25 May 2026 09:57:13 +0300 Subject: [PATCH 4/4] lint --- pkg/server/openapiv3/controller_test.go | 1 - 1 file changed, 1 deletion(-) diff --git a/pkg/server/openapiv3/controller_test.go b/pkg/server/openapiv3/controller_test.go index 0181f371e99..e80f733d6ce 100644 --- a/pkg/server/openapiv3/controller_test.go +++ b/pkg/server/openapiv3/controller_test.go @@ -17,7 +17,6 @@ limitations under the License. package openapiv3 import ( - "sync" "testing" "k8s.io/kube-openapi/pkg/cached"