diff --git a/pkg/registry/core/event/BUILD b/pkg/registry/core/event/BUILD index 91c0da283d..4a9fd1b4da 100644 --- a/pkg/registry/core/event/BUILD +++ b/pkg/registry/core/event/BUILD @@ -24,6 +24,7 @@ go_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/generic:go_default_library", + "//vendor/k8s.io/apiserver/pkg/registry/rest:go_default_library", "//vendor/k8s.io/apiserver/pkg/storage:go_default_library", "//vendor/k8s.io/apiserver/pkg/storage/names:go_default_library", ], diff --git a/pkg/registry/core/event/strategy.go b/pkg/registry/core/event/strategy.go index 77ea252b48..e70518af04 100644 --- a/pkg/registry/core/event/strategy.go +++ b/pkg/registry/core/event/strategy.go @@ -25,6 +25,7 @@ import ( "k8s.io/apimachinery/pkg/util/validation/field" genericapirequest "k8s.io/apiserver/pkg/endpoints/request" "k8s.io/apiserver/pkg/registry/generic" + "k8s.io/apiserver/pkg/registry/rest" "k8s.io/apiserver/pkg/storage" "k8s.io/apiserver/pkg/storage/names" "k8s.io/kubernetes/pkg/api" @@ -40,6 +41,10 @@ type eventStrategy struct { // Event objects via the REST API. var Strategy = eventStrategy{api.Scheme, names.SimpleNameGenerator} +func (eventStrategy) DefaultGarbageCollectionPolicy() rest.GarbageCollectionPolicy { + return rest.Unsupported +} + func (eventStrategy) NamespaceScoped() bool { return true } 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 15a507c663..75242efe18 100644 --- a/staging/src/k8s.io/apiserver/pkg/registry/generic/registry/BUILD +++ b/staging/src/k8s.io/apiserver/pkg/registry/generic/registry/BUILD @@ -68,6 +68,7 @@ go_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/runtime:go_default_library", + "//vendor/k8s.io/apimachinery/pkg/util/sets: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/staging/src/k8s.io/apiserver/pkg/registry/generic/registry/store.go b/staging/src/k8s.io/apiserver/pkg/registry/generic/registry/store.go index 2dcea903a0..ad8c552a71 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 @@ -34,6 +34,7 @@ import ( "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" utilruntime "k8s.io/apimachinery/pkg/util/runtime" + "k8s.io/apimachinery/pkg/util/sets" "k8s.io/apimachinery/pkg/util/validation/field" "k8s.io/apimachinery/pkg/watch" genericapirequest "k8s.io/apiserver/pkg/endpoints/request" @@ -628,120 +629,120 @@ var ( errEmptiedFinalizers = fmt.Errorf("emptied finalizers") ) -// shouldUpdateFinalizerOrphanDependents returns if the finalizers need to be +// shouldOrphanDependents returns true if the finalizer for orphaning should be set // updated for FinalizerOrphanDependents. In the order of highest to lowest // priority, there are three factors affect whether to add/remove the // FinalizerOrphanDependents: options, existing finalizers of the object, // and e.DeleteStrategy.DefaultGarbageCollectionPolicy. -func shouldUpdateFinalizerOrphanDependents(e *Store, accessor metav1.Object, options *metav1.DeleteOptions) (shouldUpdate bool, shouldOrphan bool) { - shouldOrphan = false - // Get default orphan policy from this REST object type +func shouldOrphanDependents(e *Store, accessor metav1.Object, options *metav1.DeleteOptions) bool { if gcStrategy, ok := e.DeleteStrategy.(rest.GarbageCollectionDeleteStrategy); ok { - if gcStrategy.DefaultGarbageCollectionPolicy() == rest.OrphanDependents { - shouldOrphan = true + if gcStrategy.DefaultGarbageCollectionPolicy() == rest.Unsupported { + // return false to indicate that we should NOT orphan + return false } } - // If a finalizer is set in the object, it overrides the default - hasOrphanFinalizer := false - finalizers := accessor.GetFinalizers() - for _, f := range finalizers { - // validation should make sure the two cases won't be true at the same - // time. - switch f { - case metav1.FinalizerOrphanDependents: - shouldOrphan = true - hasOrphanFinalizer = true - break - case metav1.FinalizerDeleteDependents: - shouldOrphan = false - break - } - } - - // If an explicit policy was set at deletion time, that overrides both + // An explicit policy was set at deletion time, that overrides everything if options != nil && options.OrphanDependents != nil { - shouldOrphan = *options.OrphanDependents + return *options.OrphanDependents } if options != nil && options.PropagationPolicy != nil { switch *options.PropagationPolicy { case metav1.DeletePropagationOrphan: - shouldOrphan = true + return true case metav1.DeletePropagationBackground, metav1.DeletePropagationForeground: - shouldOrphan = false + return false } } - shouldUpdate = shouldOrphan != hasOrphanFinalizer - return shouldUpdate, shouldOrphan + // If a finalizer is set in the object, it overrides the default + // validation should make sure the two cases won't be true at the same time. + finalizers := accessor.GetFinalizers() + for _, f := range finalizers { + switch f { + case metav1.FinalizerOrphanDependents: + return true + case metav1.FinalizerDeleteDependents: + return false + } + } + + // 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 + } + } + return false } -// shouldUpdateFinalizerDeleteDependents returns if the finalizers need to be +// shouldDeleteDependents returns true if the finalizer for foreground deletion should be set // updated for FinalizerDeleteDependents. In the order of highest to lowest // priority, there are three factors affect whether to add/remove the // FinalizerDeleteDependents: options, existing finalizers of the object, and // e.DeleteStrategy.DefaultGarbageCollectionPolicy. -func shouldUpdateFinalizerDeleteDependents(e *Store, accessor metav1.Object, options *metav1.DeleteOptions) (shouldUpdate bool, shouldDeleteDependentInForeground bool) { - // default to false - shouldDeleteDependentInForeground = false - - // If a finalizer is set in the object, it overrides the default - hasFinalizerDeleteDependents := false - finalizers := accessor.GetFinalizers() - for _, f := range finalizers { - // validation has made sure the two cases won't be true at the same - // time. - switch f { - case metav1.FinalizerDeleteDependents: - shouldDeleteDependentInForeground = true - hasFinalizerDeleteDependents = true - break - case metav1.FinalizerOrphanDependents: - shouldDeleteDependentInForeground = false - break - } +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 { + // return false to indicate that we should NOT delete in foreground + return false } // If an explicit policy was set at deletion time, that overrides both if options != nil && options.OrphanDependents != nil { - shouldDeleteDependentInForeground = false + return false } if options != nil && options.PropagationPolicy != nil { switch *options.PropagationPolicy { case metav1.DeletePropagationForeground: - shouldDeleteDependentInForeground = true + return true case metav1.DeletePropagationBackground, metav1.DeletePropagationOrphan: - shouldDeleteDependentInForeground = false + return false } } - shouldUpdate = shouldDeleteDependentInForeground != hasFinalizerDeleteDependents - return shouldUpdate, shouldDeleteDependentInForeground -} - -// shouldUpdateFinalizers returns if we need to update the finalizers of the -// object, and the desired list of finalizers. -func shouldUpdateFinalizers(e *Store, accessor metav1.Object, options *metav1.DeleteOptions) (shouldUpdate bool, newFinalizers []string) { - shouldUpdate1, shouldOrphan := shouldUpdateFinalizerOrphanDependents(e, accessor, options) - shouldUpdate2, shouldDeleteDependentInForeground := shouldUpdateFinalizerDeleteDependents(e, accessor, options) - oldFinalizers := accessor.GetFinalizers() - if !shouldUpdate1 && !shouldUpdate2 { - return false, oldFinalizers + // If a finalizer is set in the object, it overrides the default + // validation has made sure the two cases won't be true at the same time. + finalizers := accessor.GetFinalizers() + for _, f := range finalizers { + switch f { + case metav1.FinalizerDeleteDependents: + return true + case metav1.FinalizerOrphanDependents: + return false + } } + return false +} + +// deletionFinalizers returns the deletion finalizers we should set on the object and a bool indicate they did or +// did not change. +func deletionFinalizers(e *Store, accessor metav1.Object, options *metav1.DeleteOptions) (bool, []string) { + shouldOrphan := shouldOrphanDependents(e, accessor, options) + shouldDeleteDependentInForeground := shouldDeleteDependents(e, accessor, options) + newFinalizers := []string{} + // first remove both finalizers, add them back if needed. - for _, f := range oldFinalizers { + for _, f := range accessor.GetFinalizers() { if f == metav1.FinalizerOrphanDependents || f == metav1.FinalizerDeleteDependents { continue } newFinalizers = append(newFinalizers, f) } + if shouldOrphan { newFinalizers = append(newFinalizers, metav1.FinalizerOrphanDependents) } if shouldDeleteDependentInForeground { newFinalizers = append(newFinalizers, metav1.FinalizerDeleteDependents) } + + oldFinalizerSet := sets.NewString(accessor.GetFinalizers()...) + newFinalizersSet := sets.NewString(newFinalizers...) + if oldFinalizerSet.Equal(newFinalizersSet) { + return false, accessor.GetFinalizers() + } return true, newFinalizers } @@ -871,8 +872,8 @@ func (e *Store) updateForGracefulDeletionAndFinalizers(ctx genericapirequest.Con if err != nil { return nil, err } - shouldUpdate, newFinalizers := shouldUpdateFinalizers(e, existingAccessor, options) - if shouldUpdate { + needsUpdate, newFinalizers := deletionFinalizers(e, existingAccessor, options) + if needsUpdate { existingAccessor.SetFinalizers(newFinalizers) } @@ -964,7 +965,7 @@ func (e *Store) Delete(ctx genericapirequest.Context, name string, options *meta // Handle combinations of graceful deletion and finalization by issuing // the correct updates. if e.EnableGarbageCollection { - shouldUpdateFinalizers, _ := shouldUpdateFinalizers(e, accessor, options) + shouldUpdateFinalizers, _ := deletionFinalizers(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/rest/delete.go b/staging/src/k8s.io/apiserver/pkg/registry/rest/delete.go index ad407e3ec8..80e08fb192 100644 --- a/staging/src/k8s.io/apiserver/pkg/registry/rest/delete.go +++ b/staging/src/k8s.io/apiserver/pkg/registry/rest/delete.go @@ -39,6 +39,9 @@ type GarbageCollectionPolicy string const ( DeleteDependents GarbageCollectionPolicy = "DeleteDependents" OrphanDependents GarbageCollectionPolicy = "OrphanDependents" + // Unsupported means that the resource knows that it cannot be GC'd, so the finalizers + // should never be set in storage. + Unsupported GarbageCollectionPolicy = "Unsupported" ) // GarbageCollectionDeleteStrategy must be implemented by the registry that wants to