From acb4c00b3982ea2d2ef101852b2b4b83cf25370a Mon Sep 17 00:00:00 2001 From: Clayton Coleman Date: Mon, 12 Sep 2016 19:14:04 -0400 Subject: [PATCH] EnableGarbageCollection should be a struct member on RESTOptions Not a global. Now that we have RESTOptions this was an easy change. --- cmd/kube-apiserver/app/options/options.go | 5 ----- pkg/genericapiserver/config.go | 16 +++++++++------- .../options/server_run_options.go | 6 ++++++ pkg/master/master.go | 1 + pkg/registry/generic/options.go | 9 +++++---- pkg/registry/generic/registry/store.go | 12 ++++++------ pkg/registry/generic/registry/store_test.go | 9 +++------ .../garbagecollector/garbage_collector_test.go | 14 +------------- 8 files changed, 31 insertions(+), 41 deletions(-) diff --git a/cmd/kube-apiserver/app/options/options.go b/cmd/kube-apiserver/app/options/options.go index dc051ffeaa7..41dead7354d 100644 --- a/cmd/kube-apiserver/app/options/options.go +++ b/cmd/kube-apiserver/app/options/options.go @@ -24,7 +24,6 @@ import ( genericoptions "k8s.io/kubernetes/pkg/genericapiserver/options" kubeletclient "k8s.io/kubernetes/pkg/kubelet/client" "k8s.io/kubernetes/pkg/master/ports" - "k8s.io/kubernetes/pkg/registry/generic/registry" "github.com/spf13/pflag" ) @@ -123,8 +122,4 @@ func (s *APIServer) AddFlags(fs *pflag.FlagSet) { "If true, server will do its best to fix the update request to pass the validation, "+ "e.g., setting empty UID in update request to its existing value. This flag can be turned off "+ "after we fix all the clients that send malformed updates.") - - fs.BoolVar(®istry.EnableGarbageCollector, "enable-garbage-collector", true, ""+ - "Enables the generic garbage collector. MUST be synced with the corresponding flag "+ - "of the kube-controller-manager.") } diff --git a/pkg/genericapiserver/config.go b/pkg/genericapiserver/config.go index b367bd0c4a5..79053c4911c 100644 --- a/pkg/genericapiserver/config.go +++ b/pkg/genericapiserver/config.go @@ -72,13 +72,14 @@ type Config struct { // Allows api group versions or specific resources to be conditionally enabled/disabled. APIResourceConfigSource APIResourceConfigSource // allow downstream consumers to disable the index route - EnableIndex bool - EnableProfiling bool - EnableWatchCache bool - APIPrefix string - APIGroupPrefix string - CorsAllowedOriginList []string - Authenticator authenticator.Request + EnableIndex bool + EnableProfiling bool + EnableWatchCache bool + EnableGarbageCollection bool + APIPrefix string + APIGroupPrefix string + CorsAllowedOriginList []string + Authenticator authenticator.Request // TODO(roberthbailey): Remove once the server no longer supports http basic auth. SupportsBasicAuth bool Authorizer authorizer.Authorizer @@ -169,6 +170,7 @@ func NewConfig(options *options.ServerRunOptions) *Config { AuditLogMaxAge: options.AuditLogMaxAge, AuditLogMaxBackups: options.AuditLogMaxBackups, AuditLogMaxSize: options.AuditLogMaxSize, + EnableGarbageCollection: options.EnableGarbageCollection, EnableIndex: true, EnableLogsSupport: options.EnableLogsSupport, EnableProfiling: options.EnableProfiling, diff --git a/pkg/genericapiserver/options/server_run_options.go b/pkg/genericapiserver/options/server_run_options.go index 0456ed1b1d0..a0aaa79cf6b 100644 --- a/pkg/genericapiserver/options/server_run_options.go +++ b/pkg/genericapiserver/options/server_run_options.go @@ -83,6 +83,7 @@ type ServerRunOptions struct { AuditLogMaxAge int AuditLogMaxBackups int AuditLogMaxSize int + EnableGarbageCollection bool EnableLogsSupport bool EnableProfiling bool EnableSwaggerUI bool @@ -133,6 +134,7 @@ func NewServerRunOptions() *ServerRunOptions { DefaultStorageMediaType: "application/json", DefaultStorageVersions: registered.AllPreferredGroupVersions(), DeleteCollectionWorkers: 1, + EnableGarbageCollection: true, EnableLogsSupport: true, EnableProfiling: true, EnableWatchCache: true, @@ -306,6 +308,10 @@ func (s *ServerRunOptions) AddUniversalFlags(fs *pflag.FlagSet) { fs.IntVar(&s.AuditLogMaxSize, "audit-log-maxsize", s.AuditLogMaxSize, "The maximum size in megabytes of the audit log file before it gets rotated. Defaults to 100MB.") + fs.BoolVar(&s.EnableGarbageCollection, "enable-garbage-collector", s.EnableGarbageCollection, ""+ + "Enables the generic garbage collector. MUST be synced with the corresponding flag "+ + "of the kube-controller-manager.") + fs.BoolVar(&s.EnableProfiling, "profiling", s.EnableProfiling, "Enable profiling via web interface host:port/debug/pprof/") diff --git a/pkg/master/master.go b/pkg/master/master.go index 1475a732e8c..32504e0a177 100644 --- a/pkg/master/master.go +++ b/pkg/master/master.go @@ -781,6 +781,7 @@ func (m *Master) GetRESTOptionsOrDie(c *Config, resource unversioned.GroupResour StorageConfig: storageConfig, Decorator: m.StorageDecorator(), DeleteCollectionWorkers: m.deleteCollectionWorkers, + EnableGarbageCollection: c.Config.EnableGarbageCollection, ResourcePrefix: c.StorageFactory.ResourcePrefix(resource), } } diff --git a/pkg/registry/generic/options.go b/pkg/registry/generic/options.go index 74e95e71fb4..b856ca4f2e5 100644 --- a/pkg/registry/generic/options.go +++ b/pkg/registry/generic/options.go @@ -20,9 +20,10 @@ import "k8s.io/kubernetes/pkg/storage/storagebackend" // RESTOptions is set of configuration options to generic registries. type RESTOptions struct { - StorageConfig *storagebackend.Config - Decorator StorageDecorator - DeleteCollectionWorkers int + StorageConfig *storagebackend.Config + Decorator StorageDecorator - ResourcePrefix string + EnableGarbageCollection bool + DeleteCollectionWorkers int + ResourcePrefix string } diff --git a/pkg/registry/generic/registry/store.go b/pkg/registry/generic/registry/store.go index fd49e3ee116..e48f87efe3a 100644 --- a/pkg/registry/generic/registry/store.go +++ b/pkg/registry/generic/registry/store.go @@ -42,10 +42,6 @@ import ( "github.com/golang/glog" ) -// EnableGarbageCollector affects the handling of Update and Delete requests. It -// must be synced with the corresponding flag in kube-controller-manager. -var EnableGarbageCollector bool - // Store implements generic.Registry. // It's intended to be embeddable, so that you can implement any // non-generic functions if needed. @@ -93,6 +89,10 @@ type Store struct { // Called to cleanup storage clients. DestroyFunc func() + // EnableGarbageCollection affects the handling of Update and Delete requests. It + // must be synced with the corresponding flag in kube-controller-manager. + EnableGarbageCollection bool + // DeleteCollectionWorkers is the maximum number of workers in a single // DeleteCollection call. DeleteCollectionWorkers int @@ -272,7 +272,7 @@ func (e *Store) Create(ctx api.Context, obj runtime.Object) (runtime.Object, err // it further checks if the object's DeletionGracePeriodSeconds is 0. If so, it // returns true. func (e *Store) shouldDelete(ctx api.Context, key string, obj, existing runtime.Object) bool { - if !EnableGarbageCollector { + if !e.EnableGarbageCollection { return false } newMeta, err := api.ObjectMetaFor(obj) @@ -694,7 +694,7 @@ func (e *Store) Delete(ctx api.Context, name string, options *api.DeleteOptions) var ignoreNotFound bool var deleteImmediately bool = true var lastExisting, out runtime.Object - if !EnableGarbageCollector { + if !e.EnableGarbageCollection { // TODO: remove the check on graceful, because we support no-op updates now. if graceful { err, ignoreNotFound, deleteImmediately, out, lastExisting = e.updateForGracefulDeletion(ctx, name, key, options, preconditions, obj) diff --git a/pkg/registry/generic/registry/store_test.go b/pkg/registry/generic/registry/store_test.go index bdf1b6a8a31..5240ad039e7 100644 --- a/pkg/registry/generic/registry/store_test.go +++ b/pkg/registry/generic/registry/store_test.go @@ -618,9 +618,7 @@ func TestStoreDelete(t *testing.T) { } func TestGracefulStoreHandleFinalizers(t *testing.T) { - EnableGarbageCollector = true initialGeneration := int64(1) - defer func() { EnableGarbageCollector = false }() podWithFinalizer := &api.Pod{ ObjectMeta: api.ObjectMeta{Name: "foo", Finalizers: []string{"foo.com/x"}, Generation: initialGeneration}, Spec: api.PodSpec{NodeName: "machine"}, @@ -628,6 +626,7 @@ func TestGracefulStoreHandleFinalizers(t *testing.T) { testContext := api.WithNamespace(api.NewContext(), "test") destroyFunc, registry := NewTestGenericStoreRegistry(t) + registry.EnableGarbageCollection = true defaultDeleteStrategy := testRESTStrategy{api.Scheme, api.SimpleNameGenerator, true, false, true} registry.DeleteStrategy = testGracefulStrategy{defaultDeleteStrategy} defer destroyFunc() @@ -678,9 +677,7 @@ func TestGracefulStoreHandleFinalizers(t *testing.T) { } func TestNonGracefulStoreHandleFinalizers(t *testing.T) { - EnableGarbageCollector = true initialGeneration := int64(1) - defer func() { EnableGarbageCollector = false }() podWithFinalizer := &api.Pod{ ObjectMeta: api.ObjectMeta{Name: "foo", Finalizers: []string{"foo.com/x"}, Generation: initialGeneration}, Spec: api.PodSpec{NodeName: "machine"}, @@ -688,6 +685,7 @@ func TestNonGracefulStoreHandleFinalizers(t *testing.T) { testContext := api.WithNamespace(api.NewContext(), "test") destroyFunc, registry := NewTestGenericStoreRegistry(t) + registry.EnableGarbageCollection = true defer destroyFunc() // create pod _, err := registry.Create(testContext, podWithFinalizer) @@ -755,9 +753,7 @@ func TestNonGracefulStoreHandleFinalizers(t *testing.T) { } func TestStoreDeleteWithOrphanDependents(t *testing.T) { - EnableGarbageCollector = true initialGeneration := int64(1) - defer func() { EnableGarbageCollector = false }() podWithOrphanFinalizer := func(name string) *api.Pod { return &api.Pod{ ObjectMeta: api.ObjectMeta{Name: name, Finalizers: []string{"foo.com/x", api.FinalizerOrphan, "bar.com/y"}, Generation: initialGeneration}, @@ -984,6 +980,7 @@ func TestStoreDeleteWithOrphanDependents(t *testing.T) { testContext := api.WithNamespace(api.NewContext(), "test") destroyFunc, registry := NewTestGenericStoreRegistry(t) + registry.EnableGarbageCollection = true defer destroyFunc() for _, tc := range testcases { diff --git a/test/integration/garbagecollector/garbage_collector_test.go b/test/integration/garbagecollector/garbage_collector_test.go index 25a8205f90a..8691d549eae 100644 --- a/test/integration/garbagecollector/garbage_collector_test.go +++ b/test/integration/garbagecollector/garbage_collector_test.go @@ -38,7 +38,6 @@ import ( "k8s.io/kubernetes/pkg/client/typed/dynamic" "k8s.io/kubernetes/pkg/controller/garbagecollector" "k8s.io/kubernetes/pkg/controller/garbagecollector/metaonly" - "k8s.io/kubernetes/pkg/registry/generic/registry" "k8s.io/kubernetes/pkg/runtime/serializer" "k8s.io/kubernetes/pkg/types" "k8s.io/kubernetes/pkg/util/wait" @@ -121,6 +120,7 @@ func newOwnerRC(name, namespace string) *v1.ReplicationController { func setup(t *testing.T) (*httptest.Server, *garbagecollector.GarbageCollector, clientset.Interface) { masterConfig := framework.NewIntegrationTestMasterConfig() masterConfig.EnableCoreControllers = false + masterConfig.EnableGarbageCollection = true _, s := framework.RunAMaster(masterConfig) clientSet, err := clientset.NewForConfig(&restclient.Config{Host: s.URL}) @@ -153,9 +153,6 @@ func TestCascadingDeletion(t *testing.T) { ns := framework.CreateTestingNamespace("gc-cascading-deletion", s, t) defer framework.DeleteTestingNamespace(ns, s, t) - oldEnableGarbageCollector := registry.EnableGarbageCollector - registry.EnableGarbageCollector = true - defer func() { registry.EnableGarbageCollector = oldEnableGarbageCollector }() rcClient := clientSet.Core().ReplicationControllers(ns.Name) podClient := clientSet.Core().Pods(ns.Name) @@ -262,9 +259,6 @@ func TestCreateWithNonExistentOwner(t *testing.T) { ns := framework.CreateTestingNamespace("gc-non-existing-owner", s, t) defer framework.DeleteTestingNamespace(ns, s, t) - oldEnableGarbageCollector := registry.EnableGarbageCollector - registry.EnableGarbageCollector = true - defer func() { registry.EnableGarbageCollector = oldEnableGarbageCollector }() podClient := clientSet.Core().Pods(ns.Name) pod := newPod(garbageCollectedPodName, ns.Name, []v1.OwnerReference{{UID: "doesn't matter", Name: toBeDeletedRCName}}) @@ -367,9 +361,6 @@ func TestStressingCascadingDeletion(t *testing.T) { ns := framework.CreateTestingNamespace("gc-stressing-cascading-deletion", s, t) defer framework.DeleteTestingNamespace(ns, s, t) - oldEnableGarbageCollector := registry.EnableGarbageCollector - registry.EnableGarbageCollector = true - defer func() { registry.EnableGarbageCollector = oldEnableGarbageCollector }() stopCh := make(chan struct{}) go gc.Run(5, stopCh) defer close(stopCh) @@ -452,9 +443,6 @@ func TestOrphaning(t *testing.T) { ns := framework.CreateTestingNamespace("gc-orphaning", s, t) defer framework.DeleteTestingNamespace(ns, s, t) - oldEnableGarbageCollector := registry.EnableGarbageCollector - registry.EnableGarbageCollector = true - defer func() { registry.EnableGarbageCollector = oldEnableGarbageCollector }() podClient := clientSet.Core().Pods(ns.Name) rcClient := clientSet.Core().ReplicationControllers(ns.Name) // create the RC with the orphan finalizer set