From 344fe56ed30c0b83ab0a01e3b1344ecea3925863 Mon Sep 17 00:00:00 2001 From: Di Xu Date: Mon, 6 Nov 2017 17:12:57 +0800 Subject: [PATCH] change DefaultGarbageCollectionPolicy to DeleteDependents for workload controllers --- pkg/registry/apps/statefulset/BUILD | 3 + pkg/registry/apps/statefulset/strategy.go | 18 ++- .../apps/statefulset/strategy_test.go | 51 +++++++- pkg/registry/batch/cronjob/strategy.go | 2 +- pkg/registry/batch/cronjob/strategy_test.go | 2 +- pkg/registry/batch/job/strategy.go | 2 +- pkg/registry/batch/job/strategy_test.go | 2 +- pkg/registry/core/event/strategy.go | 2 +- .../core/replicationcontroller/strategy.go | 2 +- pkg/registry/extensions/daemonset/BUILD | 1 + pkg/registry/extensions/daemonset/strategy.go | 16 ++- .../extensions/daemonset/strategy_test.go | 51 +++++++- pkg/registry/extensions/deployment/BUILD | 2 + .../extensions/deployment/strategy.go | 16 ++- .../extensions/deployment/strategy_test.go | 64 ++++++++++ pkg/registry/extensions/replicaset/BUILD | 2 + .../extensions/replicaset/strategy.go | 16 ++- .../extensions/replicaset/strategy_test.go | 55 +++++++++ .../pkg/registry/generic/registry/BUILD | 1 + .../pkg/registry/generic/registry/store.go | 39 +++--- .../registry/generic/registry/store_test.go | 93 +++++++++++++- .../apiserver/pkg/registry/rest/delete.go | 2 +- test/integration/replicaset/BUILD | 1 + .../integration/replicaset/replicaset_test.go | 113 ++++++++++++++++++ 24 files changed, 513 insertions(+), 43 deletions(-) diff --git a/pkg/registry/apps/statefulset/BUILD b/pkg/registry/apps/statefulset/BUILD index 0a227ada4c..deb73b96dc 100644 --- a/pkg/registry/apps/statefulset/BUILD +++ b/pkg/registry/apps/statefulset/BUILD @@ -19,11 +19,14 @@ go_library( "//pkg/api/pod:go_default_library", "//pkg/apis/apps:go_default_library", "//pkg/apis/apps/validation:go_default_library", + "//vendor/k8s.io/api/apps/v1beta1:go_default_library", + "//vendor/k8s.io/api/apps/v1beta2:go_default_library", "//vendor/k8s.io/apimachinery/pkg/api/equality:go_default_library", "//vendor/k8s.io/apimachinery/pkg/api/errors:go_default_library", "//vendor/k8s.io/apimachinery/pkg/apis/meta/internalversion:go_default_library", "//vendor/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library", "//vendor/k8s.io/apimachinery/pkg/runtime:go_default_library", + "//vendor/k8s.io/apimachinery/pkg/runtime/schema:go_default_library", "//vendor/k8s.io/apimachinery/pkg/util/validation/field:go_default_library", "//vendor/k8s.io/apimachinery/pkg/watch:go_default_library", "//vendor/k8s.io/apiserver/pkg/endpoints/request:go_default_library", diff --git a/pkg/registry/apps/statefulset/strategy.go b/pkg/registry/apps/statefulset/strategy.go index 9ec843a1c8..08ff765a34 100644 --- a/pkg/registry/apps/statefulset/strategy.go +++ b/pkg/registry/apps/statefulset/strategy.go @@ -17,8 +17,11 @@ limitations under the License. package statefulset import ( + appsv1beta1 "k8s.io/api/apps/v1beta1" + appsv1beta2 "k8s.io/api/apps/v1beta2" apiequality "k8s.io/apimachinery/pkg/api/equality" "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apimachinery/pkg/util/validation/field" genericapirequest "k8s.io/apiserver/pkg/endpoints/request" "k8s.io/apiserver/pkg/registry/rest" @@ -38,9 +41,18 @@ type statefulSetStrategy struct { // Strategy is the default logic that applies when creating and updating Replication StatefulSet objects. var Strategy = statefulSetStrategy{legacyscheme.Scheme, names.SimpleNameGenerator} -// DefaultGarbageCollectionPolicy returns Orphan because that was the default -// behavior before the server-side garbage collection was implemented. -func (statefulSetStrategy) DefaultGarbageCollectionPolicy() rest.GarbageCollectionPolicy { +// DefaultGarbageCollectionPolicy returns OrphanDependents by default. For apps/v1, returns DeleteDependents. +func (statefulSetStrategy) DefaultGarbageCollectionPolicy(ctx genericapirequest.Context) rest.GarbageCollectionPolicy { + if requestInfo, found := genericapirequest.RequestInfoFrom(ctx); found { + groupVersion := schema.GroupVersion{Group: requestInfo.APIGroup, Version: requestInfo.APIVersion} + switch groupVersion { + case appsv1beta1.SchemeGroupVersion, appsv1beta2.SchemeGroupVersion: + // for back compatibility + return rest.OrphanDependents + default: + return rest.DeleteDependents + } + } return rest.OrphanDependents } diff --git a/pkg/registry/apps/statefulset/strategy_test.go b/pkg/registry/apps/statefulset/strategy_test.go index 9e99e7e7e8..aba7b35bc6 100644 --- a/pkg/registry/apps/statefulset/strategy_test.go +++ b/pkg/registry/apps/statefulset/strategy_test.go @@ -91,12 +91,59 @@ func TestStatefulSetStrategy(t *testing.T) { if len(errs) == 0 { t.Errorf("expected a validation error since updates are disallowed on statefulsets.") } +} +func TestStatefulsetDefaultGarbageCollectionPolicy(t *testing.T) { // Make sure we correctly implement the interface. // Otherwise a typo could silently change the default. var gcds rest.GarbageCollectionDeleteStrategy = Strategy - if got, want := gcds.DefaultGarbageCollectionPolicy(), rest.OrphanDependents; got != want { - t.Errorf("DefaultGarbageCollectionPolicy() = %#v, want %#v", got, want) + tests := []struct { + requestInfo genericapirequest.RequestInfo + expectedGCPolicy rest.GarbageCollectionPolicy + isNilRequestInfo bool + }{ + { + genericapirequest.RequestInfo{ + APIGroup: "apps", + APIVersion: "v1beta1", + Resource: "statefulsets", + }, + rest.OrphanDependents, + false, + }, + { + genericapirequest.RequestInfo{ + APIGroup: "apps", + APIVersion: "v1beta2", + Resource: "statefulsets", + }, + rest.OrphanDependents, + false, + }, + { + genericapirequest.RequestInfo{ + APIGroup: "apps", + APIVersion: "v1", + Resource: "statefulsets", + }, + rest.DeleteDependents, + false, + }, + { + expectedGCPolicy: rest.OrphanDependents, + isNilRequestInfo: true, + }, + } + + for _, test := range tests { + context := genericapirequest.NewContext() + if !test.isNilRequestInfo { + context = genericapirequest.WithRequestInfo(context, &test.requestInfo) + } + if got, want := gcds.DefaultGarbageCollectionPolicy(context), test.expectedGCPolicy; got != want { + t.Errorf("%s/%s: DefaultGarbageCollectionPolicy() = %#v, want %#v", test.requestInfo.APIGroup, + test.requestInfo.APIVersion, got, want) + } } } diff --git a/pkg/registry/batch/cronjob/strategy.go b/pkg/registry/batch/cronjob/strategy.go index 556ec7a8df..3544ca10b1 100644 --- a/pkg/registry/batch/cronjob/strategy.go +++ b/pkg/registry/batch/cronjob/strategy.go @@ -39,7 +39,7 @@ var Strategy = cronJobStrategy{legacyscheme.Scheme, names.SimpleNameGenerator} // DefaultGarbageCollectionPolicy returns Orphan because that was the default // behavior before the server-side garbage collection was implemented. -func (cronJobStrategy) DefaultGarbageCollectionPolicy() rest.GarbageCollectionPolicy { +func (cronJobStrategy) DefaultGarbageCollectionPolicy(ctx genericapirequest.Context) rest.GarbageCollectionPolicy { return rest.OrphanDependents } diff --git a/pkg/registry/batch/cronjob/strategy_test.go b/pkg/registry/batch/cronjob/strategy_test.go index 0df11e384d..6cf9b60b58 100644 --- a/pkg/registry/batch/cronjob/strategy_test.go +++ b/pkg/registry/batch/cronjob/strategy_test.go @@ -96,7 +96,7 @@ func TestCronJobStrategy(t *testing.T) { // Make sure we correctly implement the interface. // Otherwise a typo could silently change the default. var gcds rest.GarbageCollectionDeleteStrategy = Strategy - if got, want := gcds.DefaultGarbageCollectionPolicy(), rest.OrphanDependents; got != want { + if got, want := gcds.DefaultGarbageCollectionPolicy(genericapirequest.NewContext()), rest.OrphanDependents; got != want { t.Errorf("DefaultGarbageCollectionPolicy() = %#v, want %#v", got, want) } } diff --git a/pkg/registry/batch/job/strategy.go b/pkg/registry/batch/job/strategy.go index dad7af68c5..aecf38a36d 100644 --- a/pkg/registry/batch/job/strategy.go +++ b/pkg/registry/batch/job/strategy.go @@ -47,7 +47,7 @@ var Strategy = jobStrategy{legacyscheme.Scheme, names.SimpleNameGenerator} // DefaultGarbageCollectionPolicy returns Orphan because that was the default // behavior before the server-side garbage collection was implemented. -func (jobStrategy) DefaultGarbageCollectionPolicy() rest.GarbageCollectionPolicy { +func (jobStrategy) DefaultGarbageCollectionPolicy(ctx genericapirequest.Context) rest.GarbageCollectionPolicy { return rest.OrphanDependents } diff --git a/pkg/registry/batch/job/strategy_test.go b/pkg/registry/batch/job/strategy_test.go index f55ded1552..f8cfc60919 100644 --- a/pkg/registry/batch/job/strategy_test.go +++ b/pkg/registry/batch/job/strategy_test.go @@ -105,7 +105,7 @@ func TestJobStrategy(t *testing.T) { // Make sure we correctly implement the interface. // Otherwise a typo could silently change the default. var gcds rest.GarbageCollectionDeleteStrategy = Strategy - if got, want := gcds.DefaultGarbageCollectionPolicy(), rest.OrphanDependents; got != want { + if got, want := gcds.DefaultGarbageCollectionPolicy(genericapirequest.NewContext()), rest.OrphanDependents; got != want { t.Errorf("DefaultGarbageCollectionPolicy() = %#v, want %#v", got, want) } } diff --git a/pkg/registry/core/event/strategy.go b/pkg/registry/core/event/strategy.go index 0777e387f7..0ca15dfa68 100644 --- a/pkg/registry/core/event/strategy.go +++ b/pkg/registry/core/event/strategy.go @@ -42,7 +42,7 @@ type eventStrategy struct { // Event objects via the REST API. var Strategy = eventStrategy{legacyscheme.Scheme, names.SimpleNameGenerator} -func (eventStrategy) DefaultGarbageCollectionPolicy() rest.GarbageCollectionPolicy { +func (eventStrategy) DefaultGarbageCollectionPolicy(ctx genericapirequest.Context) rest.GarbageCollectionPolicy { return rest.Unsupported } diff --git a/pkg/registry/core/replicationcontroller/strategy.go b/pkg/registry/core/replicationcontroller/strategy.go index 8d2cb79d54..ede6e67248 100644 --- a/pkg/registry/core/replicationcontroller/strategy.go +++ b/pkg/registry/core/replicationcontroller/strategy.go @@ -51,7 +51,7 @@ var Strategy = rcStrategy{legacyscheme.Scheme, names.SimpleNameGenerator} // DefaultGarbageCollectionPolicy returns Orphan because that was the default // behavior before the server-side garbage collection was implemented. -func (rcStrategy) DefaultGarbageCollectionPolicy() rest.GarbageCollectionPolicy { +func (rcStrategy) DefaultGarbageCollectionPolicy(ctx genericapirequest.Context) rest.GarbageCollectionPolicy { return rest.OrphanDependents } diff --git a/pkg/registry/extensions/daemonset/BUILD b/pkg/registry/extensions/daemonset/BUILD index d2afe48f1b..76dcae9bd9 100644 --- a/pkg/registry/extensions/daemonset/BUILD +++ b/pkg/registry/extensions/daemonset/BUILD @@ -18,6 +18,7 @@ go_library( "//pkg/api/pod:go_default_library", "//pkg/apis/extensions:go_default_library", "//pkg/apis/extensions/validation:go_default_library", + "//vendor/k8s.io/api/apps/v1beta2:go_default_library", "//vendor/k8s.io/api/extensions/v1beta1:go_default_library", "//vendor/k8s.io/apimachinery/pkg/api/equality:go_default_library", "//vendor/k8s.io/apimachinery/pkg/api/validation:go_default_library", diff --git a/pkg/registry/extensions/daemonset/strategy.go b/pkg/registry/extensions/daemonset/strategy.go index 759324fff7..04723fdcf2 100644 --- a/pkg/registry/extensions/daemonset/strategy.go +++ b/pkg/registry/extensions/daemonset/strategy.go @@ -17,6 +17,7 @@ limitations under the License. package daemonset import ( + appsv1beta2 "k8s.io/api/apps/v1beta2" extensionsv1beta1 "k8s.io/api/extensions/v1beta1" apiequality "k8s.io/apimachinery/pkg/api/equality" apivalidation "k8s.io/apimachinery/pkg/api/validation" @@ -41,9 +42,18 @@ type daemonSetStrategy struct { // Strategy is the default logic that applies when creating and updating DaemonSet objects. var Strategy = daemonSetStrategy{legacyscheme.Scheme, names.SimpleNameGenerator} -// DefaultGarbageCollectionPolicy returns Orphan because that was the default -// behavior before the server-side garbage collection was implemented. -func (daemonSetStrategy) DefaultGarbageCollectionPolicy() rest.GarbageCollectionPolicy { +// DefaultGarbageCollectionPolicy returns OrphanDependents by default. For apps/v1, returns DeleteDependents. +func (daemonSetStrategy) DefaultGarbageCollectionPolicy(ctx genericapirequest.Context) rest.GarbageCollectionPolicy { + if requestInfo, found := genericapirequest.RequestInfoFrom(ctx); found { + groupVersion := schema.GroupVersion{Group: requestInfo.APIGroup, Version: requestInfo.APIVersion} + switch groupVersion { + case extensionsv1beta1.SchemeGroupVersion, appsv1beta2.SchemeGroupVersion: + // for back compatibility + return rest.OrphanDependents + default: + return rest.DeleteDependents + } + } return rest.OrphanDependents } diff --git a/pkg/registry/extensions/daemonset/strategy_test.go b/pkg/registry/extensions/daemonset/strategy_test.go index 466b0f17bf..539a560cb8 100644 --- a/pkg/registry/extensions/daemonset/strategy_test.go +++ b/pkg/registry/extensions/daemonset/strategy_test.go @@ -35,12 +35,57 @@ const ( namespace = "test-namespace" ) -func TestDefaultGarbageCollectionPolicy(t *testing.T) { +func TestDaemonsetDefaultGarbageCollectionPolicy(t *testing.T) { // Make sure we correctly implement the interface. // Otherwise a typo could silently change the default. var gcds rest.GarbageCollectionDeleteStrategy = Strategy - if got, want := gcds.DefaultGarbageCollectionPolicy(), rest.OrphanDependents; got != want { - t.Errorf("DefaultGarbageCollectionPolicy() = %#v, want %#v", got, want) + tests := []struct { + requestInfo genericapirequest.RequestInfo + expectedGCPolicy rest.GarbageCollectionPolicy + isNilRequestInfo bool + }{ + { + genericapirequest.RequestInfo{ + APIGroup: "extensions", + APIVersion: "v1beta1", + Resource: "daemonsets", + }, + rest.OrphanDependents, + false, + }, + { + genericapirequest.RequestInfo{ + APIGroup: "apps", + APIVersion: "v1beta2", + Resource: "daemonsets", + }, + rest.OrphanDependents, + false, + }, + { + genericapirequest.RequestInfo{ + APIGroup: "apps", + APIVersion: "v1", + Resource: "daemonsets", + }, + rest.DeleteDependents, + false, + }, + { + expectedGCPolicy: rest.OrphanDependents, + isNilRequestInfo: true, + }, + } + + for _, test := range tests { + context := genericapirequest.NewContext() + if !test.isNilRequestInfo { + context = genericapirequest.WithRequestInfo(context, &test.requestInfo) + } + if got, want := gcds.DefaultGarbageCollectionPolicy(context), test.expectedGCPolicy; got != want { + t.Errorf("%s/%s: DefaultGarbageCollectionPolicy() = %#v, want %#v", test.requestInfo.APIGroup, + test.requestInfo.APIVersion, got, want) + } } } diff --git a/pkg/registry/extensions/deployment/BUILD b/pkg/registry/extensions/deployment/BUILD index daeb853697..850dda20a5 100644 --- a/pkg/registry/extensions/deployment/BUILD +++ b/pkg/registry/extensions/deployment/BUILD @@ -20,6 +20,7 @@ go_library( "//pkg/apis/extensions:go_default_library", "//pkg/apis/extensions/validation:go_default_library", "//vendor/k8s.io/api/apps/v1beta1:go_default_library", + "//vendor/k8s.io/api/apps/v1beta2:go_default_library", "//vendor/k8s.io/api/extensions/v1beta1:go_default_library", "//vendor/k8s.io/apimachinery/pkg/api/equality:go_default_library", "//vendor/k8s.io/apimachinery/pkg/api/validation:go_default_library", @@ -47,6 +48,7 @@ go_test( "//vendor/k8s.io/apimachinery/pkg/util/intstr:go_default_library", "//vendor/k8s.io/apimachinery/pkg/util/validation/field:go_default_library", "//vendor/k8s.io/apiserver/pkg/endpoints/request:go_default_library", + "//vendor/k8s.io/apiserver/pkg/registry/rest:go_default_library", ], ) diff --git a/pkg/registry/extensions/deployment/strategy.go b/pkg/registry/extensions/deployment/strategy.go index 6a2a437d3e..29fcd750aa 100644 --- a/pkg/registry/extensions/deployment/strategy.go +++ b/pkg/registry/extensions/deployment/strategy.go @@ -18,6 +18,7 @@ package deployment import ( appsv1beta1 "k8s.io/api/apps/v1beta1" + appsv1beta2 "k8s.io/api/apps/v1beta2" extensionsv1beta1 "k8s.io/api/extensions/v1beta1" apiequality "k8s.io/apimachinery/pkg/api/equality" apivalidation "k8s.io/apimachinery/pkg/api/validation" @@ -43,9 +44,18 @@ type deploymentStrategy struct { // objects via the REST API. var Strategy = deploymentStrategy{legacyscheme.Scheme, names.SimpleNameGenerator} -// DefaultGarbageCollectionPolicy returns Orphan because that's the default -// behavior before the server-side garbage collection is implemented. -func (deploymentStrategy) DefaultGarbageCollectionPolicy() rest.GarbageCollectionPolicy { +// DefaultGarbageCollectionPolicy returns OrphanDependents by default. For apps/v1, returns DeleteDependents. +func (deploymentStrategy) DefaultGarbageCollectionPolicy(ctx genericapirequest.Context) rest.GarbageCollectionPolicy { + if requestInfo, found := genericapirequest.RequestInfoFrom(ctx); found { + groupVersion := schema.GroupVersion{Group: requestInfo.APIGroup, Version: requestInfo.APIVersion} + switch groupVersion { + case extensionsv1beta1.SchemeGroupVersion, appsv1beta1.SchemeGroupVersion, appsv1beta2.SchemeGroupVersion: + // for back compatibility + return rest.OrphanDependents + default: + return rest.DeleteDependents + } + } return rest.OrphanDependents } diff --git a/pkg/registry/extensions/deployment/strategy_test.go b/pkg/registry/extensions/deployment/strategy_test.go index 77e65f8ce3..02a95479c5 100644 --- a/pkg/registry/extensions/deployment/strategy_test.go +++ b/pkg/registry/extensions/deployment/strategy_test.go @@ -25,6 +25,7 @@ import ( "k8s.io/apimachinery/pkg/util/intstr" "k8s.io/apimachinery/pkg/util/validation/field" genericapirequest "k8s.io/apiserver/pkg/endpoints/request" + "k8s.io/apiserver/pkg/registry/rest" api "k8s.io/kubernetes/pkg/apis/core" "k8s.io/kubernetes/pkg/apis/extensions" ) @@ -183,3 +184,66 @@ func newDeploymentWithSelectorLabels(selectorLabels map[string]string) *extensio }, } } + +func TestDeploymentDefaultGarbageCollectionPolicy(t *testing.T) { + // Make sure we correctly implement the interface. + // Otherwise a typo could silently change the default. + var gcds rest.GarbageCollectionDeleteStrategy = Strategy + tests := []struct { + requestInfo genericapirequest.RequestInfo + expectedGCPolicy rest.GarbageCollectionPolicy + isNilRequestInfo bool + }{ + { + genericapirequest.RequestInfo{ + APIGroup: "extensions", + APIVersion: "v1beta1", + Resource: "deployments", + }, + rest.OrphanDependents, + false, + }, + { + genericapirequest.RequestInfo{ + APIGroup: "apps", + APIVersion: "v1beta1", + Resource: "deployments", + }, + rest.OrphanDependents, + false, + }, + { + genericapirequest.RequestInfo{ + APIGroup: "apps", + APIVersion: "v1beta2", + Resource: "deployments", + }, + rest.OrphanDependents, + false, + }, + { + genericapirequest.RequestInfo{ + APIGroup: "apps", + APIVersion: "v1", + Resource: "deployments", + }, + rest.DeleteDependents, + false, + }, + { + expectedGCPolicy: rest.OrphanDependents, + isNilRequestInfo: true, + }, + } + + for _, test := range tests { + context := genericapirequest.NewContext() + if !test.isNilRequestInfo { + context = genericapirequest.WithRequestInfo(context, &test.requestInfo) + } + if got, want := gcds.DefaultGarbageCollectionPolicy(context), test.expectedGCPolicy; got != want { + t.Errorf("%s/%s: DefaultGarbageCollectionPolicy() = %#v, want %#v", test.requestInfo.APIGroup, + test.requestInfo.APIVersion, got, want) + } + } +} diff --git a/pkg/registry/extensions/replicaset/BUILD b/pkg/registry/extensions/replicaset/BUILD index 430ba736c0..fd4e7eaa36 100644 --- a/pkg/registry/extensions/replicaset/BUILD +++ b/pkg/registry/extensions/replicaset/BUILD @@ -19,6 +19,7 @@ go_library( "//pkg/api/pod:go_default_library", "//pkg/apis/extensions:go_default_library", "//pkg/apis/extensions/validation:go_default_library", + "//vendor/k8s.io/api/apps/v1beta2:go_default_library", "//vendor/k8s.io/api/extensions/v1beta1:go_default_library", "//vendor/k8s.io/apimachinery/pkg/api/equality:go_default_library", "//vendor/k8s.io/apimachinery/pkg/api/validation:go_default_library", @@ -49,6 +50,7 @@ go_test( "//vendor/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library", "//vendor/k8s.io/apimachinery/pkg/util/validation/field:go_default_library", "//vendor/k8s.io/apiserver/pkg/endpoints/request:go_default_library", + "//vendor/k8s.io/apiserver/pkg/registry/rest:go_default_library", ], ) diff --git a/pkg/registry/extensions/replicaset/strategy.go b/pkg/registry/extensions/replicaset/strategy.go index 03ac326630..fac1f2a5a0 100644 --- a/pkg/registry/extensions/replicaset/strategy.go +++ b/pkg/registry/extensions/replicaset/strategy.go @@ -22,6 +22,7 @@ import ( "fmt" "strconv" + appsv1beta2 "k8s.io/api/apps/v1beta2" extensionsv1beta1 "k8s.io/api/extensions/v1beta1" apiequality "k8s.io/apimachinery/pkg/api/equality" apivalidation "k8s.io/apimachinery/pkg/api/validation" @@ -50,9 +51,18 @@ type rsStrategy struct { // Strategy is the default logic that applies when creating and updating ReplicaSet objects. var Strategy = rsStrategy{legacyscheme.Scheme, names.SimpleNameGenerator} -// DefaultGarbageCollectionPolicy returns Orphan because that's the default -// behavior before the server-side garbage collection is implemented. -func (rsStrategy) DefaultGarbageCollectionPolicy() rest.GarbageCollectionPolicy { +// DefaultGarbageCollectionPolicy returns OrphanDependents by default. For apps/v1, returns DeleteDependents. +func (rsStrategy) DefaultGarbageCollectionPolicy(ctx genericapirequest.Context) rest.GarbageCollectionPolicy { + if requestInfo, found := genericapirequest.RequestInfoFrom(ctx); found { + groupVersion := schema.GroupVersion{Group: requestInfo.APIGroup, Version: requestInfo.APIVersion} + switch groupVersion { + case extensionsv1beta1.SchemeGroupVersion, appsv1beta2.SchemeGroupVersion: + // for back compatibility + return rest.OrphanDependents + default: + return rest.DeleteDependents + } + } return rest.OrphanDependents } diff --git a/pkg/registry/extensions/replicaset/strategy_test.go b/pkg/registry/extensions/replicaset/strategy_test.go index 2e87de59c0..c2a98e3957 100644 --- a/pkg/registry/extensions/replicaset/strategy_test.go +++ b/pkg/registry/extensions/replicaset/strategy_test.go @@ -23,6 +23,7 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/util/validation/field" genericapirequest "k8s.io/apiserver/pkg/endpoints/request" + "k8s.io/apiserver/pkg/registry/rest" api "k8s.io/kubernetes/pkg/apis/core" "k8s.io/kubernetes/pkg/apis/extensions" ) @@ -227,3 +228,57 @@ func newReplicaSetWithSelectorLabels(selectorLabels map[string]string) *extensio }, } } + +func TestReplicasetDefaultGarbageCollectionPolicy(t *testing.T) { + // Make sure we correctly implement the interface. + // Otherwise a typo could silently change the default. + var gcds rest.GarbageCollectionDeleteStrategy = Strategy + tests := []struct { + requestInfo genericapirequest.RequestInfo + expectedGCPolicy rest.GarbageCollectionPolicy + isNilRequestInfo bool + }{ + { + genericapirequest.RequestInfo{ + APIGroup: "extensions", + APIVersion: "v1beta1", + Resource: "replicasets", + }, + rest.OrphanDependents, + false, + }, + { + genericapirequest.RequestInfo{ + APIGroup: "apps", + APIVersion: "v1beta2", + Resource: "replicasets", + }, + rest.OrphanDependents, + false, + }, + { + genericapirequest.RequestInfo{ + APIGroup: "apps", + APIVersion: "v1", + Resource: "replicasets", + }, + rest.DeleteDependents, + false, + }, + { + expectedGCPolicy: rest.OrphanDependents, + isNilRequestInfo: true, + }, + } + + for _, test := range tests { + context := genericapirequest.NewContext() + if !test.isNilRequestInfo { + context = genericapirequest.WithRequestInfo(context, &test.requestInfo) + } + if got, want := gcds.DefaultGarbageCollectionPolicy(context), test.expectedGCPolicy; got != want { + t.Errorf("%s/%s: DefaultGarbageCollectionPolicy() = %#v, want %#v", test.requestInfo.APIGroup, + test.requestInfo.APIVersion, got, want) + } + } +} diff --git a/staging/src/k8s.io/apiserver/pkg/registry/generic/registry/BUILD b/staging/src/k8s.io/apiserver/pkg/registry/generic/registry/BUILD index c6eb9a421b..02c6becc73 100644 --- a/staging/src/k8s.io/apiserver/pkg/registry/generic/registry/BUILD +++ b/staging/src/k8s.io/apiserver/pkg/registry/generic/registry/BUILD @@ -24,6 +24,7 @@ go_test( "//vendor/k8s.io/apimachinery/pkg/fields:go_default_library", "//vendor/k8s.io/apimachinery/pkg/labels:go_default_library", "//vendor/k8s.io/apimachinery/pkg/runtime:go_default_library", + "//vendor/k8s.io/apimachinery/pkg/runtime/schema:go_default_library", "//vendor/k8s.io/apimachinery/pkg/runtime/serializer:go_default_library", "//vendor/k8s.io/apimachinery/pkg/selection:go_default_library", "//vendor/k8s.io/apimachinery/pkg/util/validation/field:go_default_library", diff --git a/staging/src/k8s.io/apiserver/pkg/registry/generic/registry/store.go b/staging/src/k8s.io/apiserver/pkg/registry/generic/registry/store.go index 8536d0de74..db77f4efea 100644 --- a/staging/src/k8s.io/apiserver/pkg/registry/generic/registry/store.go +++ b/staging/src/k8s.io/apiserver/pkg/registry/generic/registry/store.go @@ -699,12 +699,17 @@ var ( // priority, there are three factors affect whether to add/remove the // FinalizerOrphanDependents: options, existing finalizers of the object, // and e.DeleteStrategy.DefaultGarbageCollectionPolicy. -func shouldOrphanDependents(e *Store, accessor metav1.Object, options *metav1.DeleteOptions) bool { - if gcStrategy, ok := e.DeleteStrategy.(rest.GarbageCollectionDeleteStrategy); ok { - if gcStrategy.DefaultGarbageCollectionPolicy() == rest.Unsupported { - // return false to indicate that we should NOT orphan - return false - } +func shouldOrphanDependents(ctx genericapirequest.Context, e *Store, accessor metav1.Object, options *metav1.DeleteOptions) bool { + // Get default GC policy from this REST object type + gcStrategy, ok := e.DeleteStrategy.(rest.GarbageCollectionDeleteStrategy) + var defaultGCPolicy rest.GarbageCollectionPolicy + if ok { + defaultGCPolicy = gcStrategy.DefaultGarbageCollectionPolicy(ctx) + } + + if defaultGCPolicy == rest.Unsupported { + // return false to indicate that we should NOT orphan + return false } // An explicit policy was set at deletion time, that overrides everything @@ -733,10 +738,8 @@ func shouldOrphanDependents(e *Store, accessor metav1.Object, options *metav1.De } // Get default orphan policy from this REST object type if it exists - if gcStrategy, ok := e.DeleteStrategy.(rest.GarbageCollectionDeleteStrategy); ok { - if gcStrategy.DefaultGarbageCollectionPolicy() == rest.OrphanDependents { - return true - } + if defaultGCPolicy == rest.OrphanDependents { + return true } return false } @@ -746,9 +749,9 @@ func shouldOrphanDependents(e *Store, accessor metav1.Object, options *metav1.De // priority, there are three factors affect whether to add/remove the // FinalizerDeleteDependents: options, existing finalizers of the object, and // e.DeleteStrategy.DefaultGarbageCollectionPolicy. -func shouldDeleteDependents(e *Store, accessor metav1.Object, options *metav1.DeleteOptions) bool { - // Get default orphan policy from this REST object type - if gcStrategy, ok := e.DeleteStrategy.(rest.GarbageCollectionDeleteStrategy); ok && gcStrategy.DefaultGarbageCollectionPolicy() == rest.Unsupported { +func shouldDeleteDependents(ctx genericapirequest.Context, e *Store, accessor metav1.Object, options *metav1.DeleteOptions) bool { + // Get default GC policy from this REST object type + if gcStrategy, ok := e.DeleteStrategy.(rest.GarbageCollectionDeleteStrategy); ok && gcStrategy.DefaultGarbageCollectionPolicy(ctx) == rest.Unsupported { // return false to indicate that we should NOT delete in foreground return false } @@ -789,12 +792,12 @@ func shouldDeleteDependents(e *Store, accessor metav1.Object, options *metav1.De // The finalizers returned are intended to be handled by the garbage collector. // If garbage collection is disabled for the store, this function returns false // to ensure finalizers aren't set which will never be cleared. -func deletionFinalizersForGarbageCollection(e *Store, accessor metav1.Object, options *metav1.DeleteOptions) (bool, []string) { +func deletionFinalizersForGarbageCollection(ctx genericapirequest.Context, e *Store, accessor metav1.Object, options *metav1.DeleteOptions) (bool, []string) { if !e.EnableGarbageCollection { return false, []string{} } - shouldOrphan := shouldOrphanDependents(e, accessor, options) - shouldDeleteDependentInForeground := shouldDeleteDependents(e, accessor, options) + shouldOrphan := shouldOrphanDependents(ctx, e, accessor, options) + shouldDeleteDependentInForeground := shouldDeleteDependents(ctx, e, accessor, options) newFinalizers := []string{} // first remove both finalizers, add them back if needed. @@ -880,7 +883,7 @@ func (e *Store) updateForGracefulDeletionAndFinalizers(ctx genericapirequest.Con if err != nil { return nil, err } - needsUpdate, newFinalizers := deletionFinalizersForGarbageCollection(e, existingAccessor, options) + needsUpdate, newFinalizers := deletionFinalizersForGarbageCollection(ctx, e, existingAccessor, options) if needsUpdate { existingAccessor.SetFinalizers(newFinalizers) } @@ -972,7 +975,7 @@ func (e *Store) Delete(ctx genericapirequest.Context, name string, options *meta // Handle combinations of graceful deletion and finalization by issuing // the correct updates. - shouldUpdateFinalizers, _ := deletionFinalizersForGarbageCollection(e, accessor, options) + shouldUpdateFinalizers, _ := deletionFinalizersForGarbageCollection(ctx, e, accessor, options) // TODO: remove the check, because we support no-op updates now. if graceful || pendingFinalizers || shouldUpdateFinalizers { err, ignoreNotFound, deleteImmediately, out, lastExisting = e.updateForGracefulDeletionAndFinalizers(ctx, name, key, options, preconditions, obj) diff --git a/staging/src/k8s.io/apiserver/pkg/registry/generic/registry/store_test.go b/staging/src/k8s.io/apiserver/pkg/registry/generic/registry/store_test.go index 9e632113c5..14d8fa6a3d 100644 --- a/staging/src/k8s.io/apiserver/pkg/registry/generic/registry/store_test.go +++ b/staging/src/k8s.io/apiserver/pkg/registry/generic/registry/store_test.go @@ -35,6 +35,7 @@ import ( "k8s.io/apimachinery/pkg/fields" "k8s.io/apimachinery/pkg/labels" "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apimachinery/pkg/runtime/serializer" "k8s.io/apimachinery/pkg/selection" "k8s.io/apimachinery/pkg/util/validation/field" @@ -82,7 +83,7 @@ type testOrphanDeleteStrategy struct { *testRESTStrategy } -func (t *testOrphanDeleteStrategy) DefaultGarbageCollectionPolicy() rest.GarbageCollectionPolicy { +func (t *testOrphanDeleteStrategy) DefaultGarbageCollectionPolicy(ctx genericapirequest.Context) rest.GarbageCollectionPolicy { return rest.OrphanDependents } @@ -2005,3 +2006,93 @@ func denyCreateValidation(obj runtime.Object) error { func denyUpdateValidation(obj, old runtime.Object) error { return fmt.Errorf("admission denied") } + +type fakeStrategy struct { + runtime.ObjectTyper + names.NameGenerator +} + +func (fakeStrategy) DefaultGarbageCollectionPolicy(ctx genericapirequest.Context) rest.GarbageCollectionPolicy { + appsv1beta1 := schema.GroupVersion{Group: "apps", Version: "v1beta1"} + appsv1beta2 := schema.GroupVersion{Group: "apps", Version: "v1beta2"} + extensionsv1beta1 := schema.GroupVersion{Group: "extensions", Version: "v1beta1"} + if requestInfo, found := genericapirequest.RequestInfoFrom(ctx); found { + groupVersion := schema.GroupVersion{Group: requestInfo.APIGroup, Version: requestInfo.APIVersion} + switch groupVersion { + case appsv1beta1, appsv1beta2, extensionsv1beta1: + // for back compatibility + return rest.OrphanDependents + default: + return rest.DeleteDependents + } + } + return rest.OrphanDependents +} + +func TestDeletionFinalizersForGarbageCollection(t *testing.T) { + destroyFunc, registry := NewTestGenericStoreRegistry(t) + defer destroyFunc() + + registry.DeleteStrategy = fakeStrategy{} + registry.EnableGarbageCollection = true + + tests := []struct { + requestInfo genericapirequest.RequestInfo + desiredFinalizers []string + isNilRequestInfo bool + changed bool + }{ + { + genericapirequest.RequestInfo{ + APIGroup: "extensions", + APIVersion: "v1beta1", + }, + []string{metav1.FinalizerOrphanDependents}, + false, + true, + }, + { + genericapirequest.RequestInfo{ + APIGroup: "apps", + APIVersion: "v1beta1", + }, + []string{metav1.FinalizerOrphanDependents}, + false, + true, + }, + { + genericapirequest.RequestInfo{ + APIGroup: "apps", + APIVersion: "v1beta2", + }, + []string{metav1.FinalizerOrphanDependents}, + false, + true, + }, + { + genericapirequest.RequestInfo{ + APIGroup: "apps", + APIVersion: "v1", + }, + []string{}, + false, + false, + }, + } + + for _, test := range tests { + context := genericapirequest.NewContext() + if !test.isNilRequestInfo { + context = genericapirequest.WithRequestInfo(context, &test.requestInfo) + } + changed, finalizers := deletionFinalizersForGarbageCollection(context, registry, &example.ReplicaSet{}, &metav1.DeleteOptions{}) + if !changed { + if test.changed { + t.Errorf("%s/%s: no new finalizers are added", test.requestInfo.APIGroup, test.requestInfo.APIVersion) + } + } else if !reflect.DeepEqual(finalizers, test.desiredFinalizers) { + t.Errorf("%s/%s: want %#v, got %#v", test.requestInfo.APIGroup, test.requestInfo.APIVersion, + test.desiredFinalizers, finalizers) + } + } +} diff --git a/staging/src/k8s.io/apiserver/pkg/registry/rest/delete.go b/staging/src/k8s.io/apiserver/pkg/registry/rest/delete.go index d7782a279f..11a1474c84 100644 --- a/staging/src/k8s.io/apiserver/pkg/registry/rest/delete.go +++ b/staging/src/k8s.io/apiserver/pkg/registry/rest/delete.go @@ -48,7 +48,7 @@ const ( // orphan dependents by default. type GarbageCollectionDeleteStrategy interface { // DefaultGarbageCollectionPolicy returns the default garbage collection behavior. - DefaultGarbageCollectionPolicy() GarbageCollectionPolicy + DefaultGarbageCollectionPolicy(ctx genericapirequest.Context) GarbageCollectionPolicy } // RESTGracefulDeleteStrategy must be implemented by the registry that supports diff --git a/test/integration/replicaset/BUILD b/test/integration/replicaset/BUILD index ea194022e2..2784e1fa87 100644 --- a/test/integration/replicaset/BUILD +++ b/test/integration/replicaset/BUILD @@ -17,6 +17,7 @@ go_test( deps = [ "//pkg/api/v1/pod:go_default_library", "//pkg/controller/replicaset:go_default_library", + "//pkg/util/slice:go_default_library", "//test/integration/framework:go_default_library", "//vendor/k8s.io/api/core/v1:go_default_library", "//vendor/k8s.io/api/extensions/v1beta1:go_default_library", diff --git a/test/integration/replicaset/replicaset_test.go b/test/integration/replicaset/replicaset_test.go index 7ad3489c7c..fb42cb24c1 100644 --- a/test/integration/replicaset/replicaset_test.go +++ b/test/integration/replicaset/replicaset_test.go @@ -40,6 +40,7 @@ import ( "k8s.io/client-go/util/retry" podutil "k8s.io/kubernetes/pkg/api/v1/pod" "k8s.io/kubernetes/pkg/controller/replicaset" + "k8s.io/kubernetes/pkg/util/slice" "k8s.io/kubernetes/test/integration/framework" ) @@ -930,3 +931,115 @@ func TestFullyLabeledReplicas(t *testing.T) { t.Fatalf("Failed to verify only one pod is fully labeled: %v", err) } } + +func TestReplicaSetsExtensionsV1beta1DefaultGCPolicy(t *testing.T) { + s, closeFn, rm, informers, c := rmSetup(t) + defer closeFn() + ns := framework.CreateTestingNamespace("test-default-gc-extensions", s, t) + defer framework.DeleteTestingNamespace(ns, s, t) + stopCh := runControllerAndInformers(t, rm, informers, 0) + defer close(stopCh) + + rs := newRS("rs", ns.Name, 2) + fakeFinalizer := "kube.io/dummy-finalizer" + rs.Finalizers = []string{fakeFinalizer} + rss, _ := createRSsPods(t, c, []*v1beta1.ReplicaSet{rs}, []*v1.Pod{}) + rs = rss[0] + waitRSStable(t, c, rs) + + // Verify RS creates 2 pods + podClient := c.CoreV1().Pods(ns.Name) + pods := getPods(t, podClient, labelMap()) + if len(pods.Items) != 2 { + t.Fatalf("len(pods) = %d, want 2", len(pods.Items)) + } + + rsClient := c.ExtensionsV1beta1().ReplicaSets(ns.Name) + err := rsClient.Delete(rs.Name, nil) + if err != nil { + t.Fatalf("Failed to delete rs: %v", err) + } + + // Verify orphan finalizer has been added + if err := wait.PollImmediate(interval, timeout, func() (bool, error) { + newRS, err := rsClient.Get(rs.Name, metav1.GetOptions{}) + if err != nil { + return false, err + } + return slice.ContainsString(newRS.Finalizers, metav1.FinalizerOrphanDependents, nil), nil + }); err != nil { + t.Fatalf("Failed to verify orphan finalizer is added: %v", err) + } + + updateRS(t, rsClient, rs.Name, func(rs *v1beta1.ReplicaSet) { + var finalizers []string + // remove fakeFinalizer + for _, finalizer := range rs.Finalizers { + if finalizer != fakeFinalizer { + finalizers = append(finalizers, finalizer) + } + } + rs.Finalizers = finalizers + }) + + rsClient.Delete(rs.Name, nil) +} + +func TestReplicaSetsAppsV1DefaultGCPolicy(t *testing.T) { + s, closeFn, rm, informers, c := rmSetup(t) + defer closeFn() + ns := framework.CreateTestingNamespace("test-default-gc-extensions", s, t) + defer framework.DeleteTestingNamespace(ns, s, t) + stopCh := runControllerAndInformers(t, rm, informers, 0) + defer close(stopCh) + + rs := newRS("rs", ns.Name, 2) + fakeFinalizer := "kube.io/dummy-finalizer" + rs.Finalizers = []string{fakeFinalizer} + rss, _ := createRSsPods(t, c, []*v1beta1.ReplicaSet{rs}, []*v1.Pod{}) + rs = rss[0] + waitRSStable(t, c, rs) + + // Verify RS creates 2 pods + podClient := c.CoreV1().Pods(ns.Name) + pods := getPods(t, podClient, labelMap()) + if len(pods.Items) != 2 { + t.Fatalf("len(pods) = %d, want 2", len(pods.Items)) + } + + rsClient := c.AppsV1().ReplicaSets(ns.Name) + err := rsClient.Delete(rs.Name, nil) + if err != nil { + t.Fatalf("Failed to delete rs: %v", err) + } + + // Verify no new finalizer has been added + if err := wait.PollImmediate(interval, timeout, func() (bool, error) { + newRS, err := rsClient.Get(rs.Name, metav1.GetOptions{}) + if err != nil { + return false, err + } + if newRS.DeletionTimestamp == nil { + return false, nil + } + if got, want := newRS.Finalizers, []string{fakeFinalizer}; !reflect.DeepEqual(got, want) { + return false, fmt.Errorf("got finalizers: %+v; want: %+v", got, want) + } + return true, nil + }); err != nil { + t.Fatalf("Failed to verify the finalizer: %v", err) + } + + updateRS(t, c.ExtensionsV1beta1().ReplicaSets(ns.Name), rs.Name, func(rs *v1beta1.ReplicaSet) { + var finalizers []string + // remove fakeFinalizer + for _, finalizer := range rs.Finalizers { + if finalizer != fakeFinalizer { + finalizers = append(finalizers, finalizer) + } + } + rs.Finalizers = finalizers + }) + + rsClient.Delete(rs.Name, nil) +}