Merge pull request #45897 from ncdc/gc-require-list-watch

Automatic merge from submit-queue (batch tested with PRs 46149, 45897, 46293, 46296, 46194)

GC: update required verbs for deletable resources, allow list of ignored resources to be customized

The garbage collector controller currently needs to list, watch, get,
patch, update, and delete resources. Update the criteria for
deletable resources to reflect this.

Also allow the list of resources the garbage collector controller should
ignore to be customizable, so downstream integrators can add their own
resources to the list, if necessary.

cc @caesarxuchao @deads2k @smarterclayton @mfojtik @liggitt @sttts @kubernetes/sig-api-machinery-pr-reviews

cmd/kube-controller-manager/app/core.go

@@ -178,7 +178,7 @@ func startGarbageCollectorController(ctx ControllerContext) (bool, error) {
 	if err != nil {
 		return true, fmt.Errorf("failed to get supported resources from server: %v", err)
 	}
-	deletableResources := discovery.FilteredBy(discovery.SupportsAllVerbs{Verbs: []string{"delete"}}, preferredResources)
+	deletableResources := discovery.FilteredBy(discovery.SupportsAllVerbs{Verbs: []string{"get", "list", "watch", "patch", "update", "delete"}}, preferredResources)
 	deletableGroupVersionResources, err := discovery.GroupVersionResources(deletableResources)
 	if err != nil {
 		return true, fmt.Errorf("Failed to parse resources from server: %v", err)
@@ -189,7 +189,20 @@ func startGarbageCollectorController(ctx ControllerContext) (bool, error) {
 	metaOnlyClientPool := dynamic.NewClientPool(config, restMapper, dynamic.LegacyAPIPathResolverFunc)
 	config.ContentConfig = dynamic.ContentConfig()
 	clientPool := dynamic.NewClientPool(config, restMapper, dynamic.LegacyAPIPathResolverFunc)
-	garbageCollector, err := garbagecollector.NewGarbageCollector(metaOnlyClientPool, clientPool, restMapper, deletableGroupVersionResources, ctx.InformerFactory)
+
+	ignoredResources := make(map[schema.GroupResource]struct{})
+	for _, r := range ctx.Options.GCIgnoredResources {
+		ignoredResources[schema.GroupResource{Group: r.Group, Resource: r.Resource}] = struct{}{}
+	}
+
+	garbageCollector, err := garbagecollector.NewGarbageCollector(
+		metaOnlyClientPool,
+		clientPool,
+		restMapper,
+		deletableGroupVersionResources,
+		ignoredResources,
+		ctx.InformerFactory,
+	)
 	if err != nil {
 		return true, fmt.Errorf("Failed to start the generic garbage collector: %v", err)
 	}

cmd/kube-controller-manager/app/options/BUILD

@@ -14,6 +14,7 @@ go_library(
     deps = [
         "//pkg/apis/componentconfig:go_default_library",
         "//pkg/client/leaderelection:go_default_library",
+        "//pkg/controller/garbagecollector:go_default_library",
         "//pkg/features:go_default_library",
         "//pkg/master/ports:go_default_library",
         "//vendor/github.com/spf13/pflag:go_default_library",

cmd/kube-controller-manager/app/options/options.go

@@ -29,6 +29,7 @@ import (
 	utilfeature "k8s.io/apiserver/pkg/util/feature"
 	"k8s.io/kubernetes/pkg/apis/componentconfig"
 	"k8s.io/kubernetes/pkg/client/leaderelection"
+	"k8s.io/kubernetes/pkg/controller/garbagecollector"
 	"k8s.io/kubernetes/pkg/master/ports"
 
 	// add the kubernetes feature gates
@@ -47,6 +48,11 @@ type CMServer struct {
 
 // NewCMServer creates a new CMServer with a default config.
 func NewCMServer() *CMServer {
+	gcIgnoredResources := make([]componentconfig.GroupResource, 0, len(garbagecollector.DefaultIgnoredResources()))
+	for r := range garbagecollector.DefaultIgnoredResources() {
+		gcIgnoredResources = append(gcIgnoredResources, componentconfig.GroupResource{Group: r.Group, Resource: r.Resource})
+	}
+
 	s := CMServer{
 		KubeControllerManagerConfiguration: componentconfig.KubeControllerManagerConfiguration{
 			Controllers:                                     []string{"*"},
@@ -103,6 +109,7 @@ func NewCMServer() *CMServer {
 			ControllerStartInterval:               metav1.Duration{Duration: 0 * time.Second},
 			EnableGarbageCollector:                true,
 			ConcurrentGCSyncs:                     20,
+			GCIgnoredResources:                    gcIgnoredResources,
 			ClusterSigningCertFile:                "/etc/kubernetes/ca/ca.pem",
 			ClusterSigningKeyFile:                 "/etc/kubernetes/ca/ca.key",
 			ReconcilerSyncLoopPeriod:              metav1.Duration{Duration: 60 * time.Second},

pkg/apis/componentconfig/types.go

@@ -691,6 +691,13 @@ type LeaderElectionConfiguration struct {
 	ResourceLock string
 }
 
+type GroupResource struct {
+	// group is the group portion of the GroupResource.
+	Group string
+	// resource is the resource portion of the GroupResource.
+	Resource string
+}
+
 type KubeControllerManagerConfiguration struct {
 	metav1.TypeMeta
 
@@ -877,6 +884,8 @@ type KubeControllerManagerConfiguration struct {
 	// concurrentGCSyncs is the number of garbage collector workers that are
 	// allowed to sync concurrently.
 	ConcurrentGCSyncs int32
+	// gcIgnoredResources is the list of GroupResources that garbage collection should ignore.
+	GCIgnoredResources []GroupResource
 	// nodeEvictionRate is the number of nodes per second on which pods are deleted in case of node failure when a zone is healthy
 	NodeEvictionRate float32
 	// secondaryNodeEvictionRate is the number of nodes per second on which pods are deleted in case of node failure when a zone is unhealty

pkg/apis/componentconfig/zz_generated.deepcopy.go

@@ -36,6 +36,7 @@ func init() {
 func RegisterDeepCopies(scheme *runtime.Scheme) error {
 	return scheme.AddGeneratedDeepCopyFuncs(
 		conversion.GeneratedDeepCopyFunc{Fn: DeepCopy_componentconfig_ClientConnectionConfiguration, InType: reflect.TypeOf(&ClientConnectionConfiguration{})},
+		conversion.GeneratedDeepCopyFunc{Fn: DeepCopy_componentconfig_GroupResource, InType: reflect.TypeOf(&GroupResource{})},
 		conversion.GeneratedDeepCopyFunc{Fn: DeepCopy_componentconfig_IPVar, InType: reflect.TypeOf(&IPVar{})},
 		conversion.GeneratedDeepCopyFunc{Fn: DeepCopy_componentconfig_KubeControllerManagerConfiguration, InType: reflect.TypeOf(&KubeControllerManagerConfiguration{})},
 		conversion.GeneratedDeepCopyFunc{Fn: DeepCopy_componentconfig_KubeProxyConfiguration, InType: reflect.TypeOf(&KubeProxyConfiguration{})},
@@ -66,6 +67,16 @@ func DeepCopy_componentconfig_ClientConnectionConfiguration(in interface{}, out
 	}
 }
 
+// DeepCopy_componentconfig_GroupResource is an autogenerated deepcopy function.
+func DeepCopy_componentconfig_GroupResource(in interface{}, out interface{}, c *conversion.Cloner) error {
+	{
+		in := in.(*GroupResource)
+		out := out.(*GroupResource)
+		*out = *in
+		return nil
+	}
+}
+
 // DeepCopy_componentconfig_IPVar is an autogenerated deepcopy function.
 func DeepCopy_componentconfig_IPVar(in interface{}, out interface{}, c *conversion.Cloner) error {
 	{
@@ -92,6 +103,11 @@ func DeepCopy_componentconfig_KubeControllerManagerConfiguration(in interface{},
 			*out = make([]string, len(*in))
 			copy(*out, *in)
 		}
+		if in.GCIgnoredResources != nil {
+			in, out := &in.GCIgnoredResources, &out.GCIgnoredResources
+			*out = make([]GroupResource, len(*in))
+			copy(*out, *in)
+		}
 		return nil
 	}
 }

pkg/controller/garbagecollector/garbagecollector.go

@@ -78,6 +78,7 @@ func NewGarbageCollector(
 	clientPool dynamic.ClientPool,
 	mapper meta.RESTMapper,
 	deletableResources map[schema.GroupVersionResource]struct{},
+	ignoredResources map[schema.GroupResource]struct{},
 	sharedInformers informers.SharedInformerFactory,
 ) (*GarbageCollector, error) {
 	attemptToDelete := workqueue.NewNamedRateLimitingQueue(workqueue.DefaultControllerRateLimiter(), "garbage_collector_attempt_to_delete")
@@ -103,6 +104,7 @@ func NewGarbageCollector(
 		attemptToOrphan:  attemptToOrphan,
 		absentOwnerCache: absentOwnerCache,
 		sharedInformers:  sharedInformers,
+		ignoredResources: ignoredResources,
 	}
 	if err := gb.monitorsForResources(deletableResources); err != nil {
 		return nil, err

pkg/controller/garbagecollector/garbagecollector_test.go

@@ -60,7 +60,7 @@ func TestNewGarbageCollector(t *testing.T) {
 
 	client := fake.NewSimpleClientset()
 	sharedInformers := informers.NewSharedInformerFactory(client, 0)
-	gc, err := NewGarbageCollector(metaOnlyClientPool, clientPool, api.Registry.RESTMapper(), podResource, sharedInformers)
+	gc, err := NewGarbageCollector(metaOnlyClientPool, clientPool, api.Registry.RESTMapper(), podResource, ignoredResources, sharedInformers)
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -131,7 +131,7 @@ func setupGC(t *testing.T, config *restclient.Config) garbageCollector {
 	podResource := map[schema.GroupVersionResource]struct{}{{Version: "v1", Resource: "pods"}: {}}
 	client := fake.NewSimpleClientset()
 	sharedInformers := informers.NewSharedInformerFactory(client, 0)
-	gc, err := NewGarbageCollector(metaOnlyClientPool, clientPool, api.Registry.RESTMapper(), podResource, sharedInformers)
+	gc, err := NewGarbageCollector(metaOnlyClientPool, clientPool, api.Registry.RESTMapper(), podResource, ignoredResources, sharedInformers)
 	if err != nil {
 		t.Fatal(err)
 	}

pkg/controller/garbagecollector/graph_builder.go

@@ -94,6 +94,7 @@ type GraphBuilder struct {
 	absentOwnerCache *UIDCache
 	sharedInformers  informers.SharedInformerFactory
 	stopCh           <-chan struct{}
+	ignoredResources map[schema.GroupResource]struct{}
 }
 
 func listWatcher(client *dynamic.Client, resource schema.GroupVersionResource) *cache.ListWatch {
@@ -193,7 +194,7 @@ func (gb *GraphBuilder) controllerFor(resource schema.GroupVersionResource, kind
 
 func (gb *GraphBuilder) monitorsForResources(resources map[schema.GroupVersionResource]struct{}) error {
 	for resource := range resources {
-		if _, ok := ignoredResources[resource]; ok {
+		if _, ok := gb.ignoredResources[resource.GroupResource()]; ok {
 			glog.V(5).Infof("ignore resource %#v", resource)
 			continue
 		}
@@ -231,16 +232,24 @@ func (gb *GraphBuilder) Run(stopCh <-chan struct{}) {
 	gb.stopCh = stopCh
 }
 
-var ignoredResources = map[schema.GroupVersionResource]struct{}{
-	{Group: "extensions", Version: "v1beta1", Resource: "replicationcontrollers"}:              {},
-	{Group: "", Version: "v1", Resource: "bindings"}:                                           {},
-	{Group: "", Version: "v1", Resource: "componentstatuses"}:                                  {},
-	{Group: "", Version: "v1", Resource: "events"}:                                             {},
-	{Group: "authentication.k8s.io", Version: "v1beta1", Resource: "tokenreviews"}:             {},
-	{Group: "authorization.k8s.io", Version: "v1beta1", Resource: "subjectaccessreviews"}:      {},
-	{Group: "authorization.k8s.io", Version: "v1beta1", Resource: "selfsubjectaccessreviews"}:  {},
-	{Group: "authorization.k8s.io", Version: "v1beta1", Resource: "localsubjectaccessreviews"}: {},
-	{Group: "apiregistration.k8s.io", Version: "v1beta1", Resource: "apiservices"}:             {},
+var ignoredResources = map[schema.GroupResource]struct{}{
+	{Group: "extensions", Resource: "replicationcontrollers"}:              {},
+	{Group: "", Resource: "bindings"}:                                      {},
+	{Group: "", Resource: "componentstatuses"}:                             {},
+	{Group: "", Resource: "events"}:                                        {},
+	{Group: "authentication.k8s.io", Resource: "tokenreviews"}:             {},
+	{Group: "authorization.k8s.io", Resource: "subjectaccessreviews"}:      {},
+	{Group: "authorization.k8s.io", Resource: "selfsubjectaccessreviews"}:  {},
+	{Group: "authorization.k8s.io", Resource: "localsubjectaccessreviews"}: {},
+	{Group: "apiregistration.k8s.io", Resource: "apiservices"}:             {},
+	{Group: "apiextensions.k8s.io", Resource: "customresourcedefinitions"}: {},
+}
+
+// DefaultIgnoredResources returns the default set of resources that the garbage collector controller
+// should ignore. This is exposed so downstream integrators can have access to the defaults, and add
+// to them as necessary when constructing the controller.
+func DefaultIgnoredResources() map[schema.GroupResource]struct{} {
+	return ignoredResources
 }
 
 func (gb *GraphBuilder) enqueueChanges(e *event) {

test/integration/garbagecollector/garbage_collector_test.go

@@ -148,7 +148,14 @@ func setup(t *testing.T, stop chan struct{}) (*httptest.Server, framework.CloseF
 	config.ContentConfig.NegotiatedSerializer = nil
 	clientPool := dynamic.NewClientPool(config, api.Registry.RESTMapper(), dynamic.LegacyAPIPathResolverFunc)
 	sharedInformers := informers.NewSharedInformerFactory(clientSet, 0)
-	gc, err := garbagecollector.NewGarbageCollector(metaOnlyClientPool, clientPool, api.Registry.RESTMapper(), deletableGroupVersionResources, sharedInformers)
+	gc, err := garbagecollector.NewGarbageCollector(
+		metaOnlyClientPool,
+		clientPool,
+		api.Registry.RESTMapper(),
+		deletableGroupVersionResources,
+		garbagecollector.DefaultIgnoredResources(),
+		sharedInformers,
+	)
 	if err != nil {
 		t.Fatalf("Failed to create garbage collector")
 	}