From 50db32cf8c3301d27bb71b9e59f9449c18c944ba Mon Sep 17 00:00:00 2001 From: Lukasz Szaszkiewicz Date: Fri, 5 Jun 2020 12:18:53 +0200 Subject: [PATCH] GC doesn't have to create monitors in the constructor --- cmd/kube-controller-manager/app/BUILD | 1 - cmd/kube-controller-manager/app/core.go | 5 ----- pkg/controller/garbagecollector/garbagecollector.go | 7 +------ .../garbagecollector/garbagecollector_test.go | 12 ++++-------- .../garbagecollector/garbage_collector_test.go | 2 -- 5 files changed, 5 insertions(+), 22 deletions(-) diff --git a/cmd/kube-controller-manager/app/BUILD b/cmd/kube-controller-manager/app/BUILD index c1527ba41eb..14962b7bda0 100644 --- a/cmd/kube-controller-manager/app/BUILD +++ b/cmd/kube-controller-manager/app/BUILD @@ -118,7 +118,6 @@ go_library( "//staging/src/k8s.io/apiserver/pkg/server/mux:go_default_library", "//staging/src/k8s.io/apiserver/pkg/util/feature:go_default_library", "//staging/src/k8s.io/client-go/discovery/cached:go_default_library", - "//staging/src/k8s.io/client-go/discovery/cached/memory:go_default_library", "//staging/src/k8s.io/client-go/dynamic:go_default_library", "//staging/src/k8s.io/client-go/informers:go_default_library", "//staging/src/k8s.io/client-go/informers/storage/v1:go_default_library", diff --git a/cmd/kube-controller-manager/app/core.go b/cmd/kube-controller-manager/app/core.go index 0849539ddb9..48641234ade 100644 --- a/cmd/kube-controller-manager/app/core.go +++ b/cmd/kube-controller-manager/app/core.go @@ -33,7 +33,6 @@ import ( v1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/runtime/schema" utilfeature "k8s.io/apiserver/pkg/util/feature" - cacheddiscovery "k8s.io/client-go/discovery/cached/memory" storagev1informer "k8s.io/client-go/informers/storage/v1" clientset "k8s.io/client-go/kubernetes" "k8s.io/client-go/metadata" @@ -500,7 +499,6 @@ func startGarbageCollectorController(ctx ControllerContext) (http.Handler, bool, } gcClientset := ctx.ClientBuilder.ClientOrDie("generic-garbage-collector") - discoveryClient := cacheddiscovery.NewMemCacheClient(gcClientset.Discovery()) config := ctx.ClientBuilder.ConfigOrDie("generic-garbage-collector") metadataClient, err := metadata.NewForConfig(config) @@ -508,8 +506,6 @@ func startGarbageCollectorController(ctx ControllerContext) (http.Handler, bool, return nil, true, err } - // Get an initial set of deletable resources to prime the garbage collector. - deletableResources := garbagecollector.GetDeletableResources(discoveryClient) ignoredResources := make(map[schema.GroupResource]struct{}) for _, r := range ctx.ComponentConfig.GarbageCollectorController.GCIgnoredResources { ignoredResources[schema.GroupResource{Group: r.Group, Resource: r.Resource}] = struct{}{} @@ -517,7 +513,6 @@ func startGarbageCollectorController(ctx ControllerContext) (http.Handler, bool, garbageCollector, err := garbagecollector.NewGarbageCollector( metadataClient, ctx.RESTMapper, - deletableResources, ignoredResources, ctx.ObjectOrMetadataInformerFactory, ctx.InformersStarted, diff --git a/pkg/controller/garbagecollector/garbagecollector.go b/pkg/controller/garbagecollector/garbagecollector.go index 606dae39c4c..a337e0f15e0 100644 --- a/pkg/controller/garbagecollector/garbagecollector.go +++ b/pkg/controller/garbagecollector/garbagecollector.go @@ -76,7 +76,6 @@ type GarbageCollector struct { func NewGarbageCollector( metadataClient metadata.Interface, mapper resettableRESTMapper, - deletableResources map[schema.GroupVersionResource]struct{}, ignoredResources map[schema.GroupResource]struct{}, sharedInformers controller.InformerFactory, informersStarted <-chan struct{}, @@ -91,7 +90,7 @@ func NewGarbageCollector( attemptToOrphan: attemptToOrphan, absentOwnerCache: absentOwnerCache, } - gb := &GraphBuilder{ + gc.dependencyGraphBuilder = &GraphBuilder{ metadataClient: metadataClient, informersStarted: informersStarted, restMapper: mapper, @@ -105,10 +104,6 @@ func NewGarbageCollector( sharedInformers: sharedInformers, ignoredResources: ignoredResources, } - if err := gb.syncMonitors(deletableResources); err != nil { - utilruntime.HandleError(fmt.Errorf("failed to sync all monitors: %v", err)) - } - gc.dependencyGraphBuilder = gb return gc, nil } diff --git a/pkg/controller/garbagecollector/garbagecollector_test.go b/pkg/controller/garbagecollector/garbagecollector_test.go index 9c792300e51..d07402021e5 100644 --- a/pkg/controller/garbagecollector/garbagecollector_test.go +++ b/pkg/controller/garbagecollector/garbagecollector_test.go @@ -81,12 +81,12 @@ func TestGarbageCollectorConstruction(t *testing.T) { // construction will not fail. alwaysStarted := make(chan struct{}) close(alwaysStarted) - gc, err := NewGarbageCollector(metadataClient, rm, twoResources, map[schema.GroupResource]struct{}{}, + gc, err := NewGarbageCollector(metadataClient, rm, map[schema.GroupResource]struct{}{}, controller.NewInformerFactory(sharedInformers, metadataInformers), alwaysStarted) if err != nil { t.Fatal(err) } - assert.Equal(t, 1, len(gc.dependencyGraphBuilder.monitors)) + assert.Equal(t, 0, len(gc.dependencyGraphBuilder.monitors)) // Make sure resource monitor syncing creates and stops resource monitors. tweakableRM.Add(schema.GroupVersionKind{Group: "tpr.io", Version: "v1", Kind: "unknown"}, nil) @@ -198,12 +198,11 @@ func setupGC(t *testing.T, config *restclient.Config) garbageCollector { t.Fatal(err) } - podResource := map[schema.GroupVersionResource]struct{}{{Version: "v1", Resource: "pods"}: {}} client := fake.NewSimpleClientset() sharedInformers := informers.NewSharedInformerFactory(client, 0) alwaysStarted := make(chan struct{}) close(alwaysStarted) - gc, err := NewGarbageCollector(metadataClient, &testRESTMapper{testrestmapper.TestOnlyStaticRESTMapper(legacyscheme.Scheme)}, podResource, ignoredResources, sharedInformers, alwaysStarted) + gc, err := NewGarbageCollector(metadataClient, &testRESTMapper{testrestmapper.TestOnlyStaticRESTMapper(legacyscheme.Scheme)}, ignoredResources, sharedInformers, alwaysStarted) if err != nil { t.Fatal(err) } @@ -827,13 +826,10 @@ func TestGarbageCollectorSync(t *testing.T) { t.Fatal(err) } - podResource := map[schema.GroupVersionResource]struct{}{ - {Group: "", Version: "v1", Resource: "pods"}: {}, - } sharedInformers := informers.NewSharedInformerFactory(client, 0) alwaysStarted := make(chan struct{}) close(alwaysStarted) - gc, err := NewGarbageCollector(metadataClient, rm, podResource, map[schema.GroupResource]struct{}{}, sharedInformers, alwaysStarted) + gc, err := NewGarbageCollector(metadataClient, rm, map[schema.GroupResource]struct{}{}, sharedInformers, alwaysStarted) if err != nil { t.Fatal(err) } diff --git a/test/integration/garbagecollector/garbage_collector_test.go b/test/integration/garbagecollector/garbage_collector_test.go index 88430363829..a06c3fd1e2f 100644 --- a/test/integration/garbagecollector/garbage_collector_test.go +++ b/test/integration/garbagecollector/garbage_collector_test.go @@ -232,7 +232,6 @@ func setupWithServer(t *testing.T, result *kubeapiservertesting.TestServer, work discoveryClient := cacheddiscovery.NewMemCacheClient(clientSet.Discovery()) restMapper := restmapper.NewDeferredDiscoveryRESTMapper(discoveryClient) restMapper.Reset() - deletableResources := garbagecollector.GetDeletableResources(discoveryClient) config := *result.ClientConfig metadataClient, err := metadata.NewForConfig(&config) if err != nil { @@ -249,7 +248,6 @@ func setupWithServer(t *testing.T, result *kubeapiservertesting.TestServer, work gc, err := garbagecollector.NewGarbageCollector( metadataClient, restMapper, - deletableResources, garbagecollector.DefaultIgnoredResources(), controller.NewInformerFactory(sharedInformers, metadataInformers), alwaysStarted,