Validate deletion timestamp doesn't change on update #24839
+165
−10
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -21,6 +21,7 @@ import ( | |
|
|
||
| "k8s.io/kubernetes/pkg/api" | ||
| apierrors "k8s.io/kubernetes/pkg/api/errors" | ||
| storageerr "k8s.io/kubernetes/pkg/api/errors/storage" | ||
| "k8s.io/kubernetes/pkg/api/unversioned" | ||
| "k8s.io/kubernetes/pkg/fields" | ||
| "k8s.io/kubernetes/pkg/labels" | ||
|
|
@@ -29,6 +30,7 @@ import ( | |
| "k8s.io/kubernetes/pkg/registry/generic/registry" | ||
| "k8s.io/kubernetes/pkg/registry/namespace" | ||
| "k8s.io/kubernetes/pkg/runtime" | ||
| "k8s.io/kubernetes/pkg/storage" | ||
| ) | ||
|
|
||
| // rest implements a RESTStorage for namespaces against etcd | ||
|
|
@@ -99,21 +101,74 @@ func (r *REST) Delete(ctx api.Context, name string, options *api.DeleteOptions) | |
|
|
||
| namespace := nsObj.(*api.Namespace) | ||
|
|
||
| // Ensure we have a UID precondition | ||
| if options == nil { | ||
| options = api.NewDeleteOptions(0) | ||
| } | ||
| if options.Preconditions == nil { | ||
| options.Preconditions = &api.Preconditions{} | ||
| } | ||
| if options.Preconditions.UID == nil { | ||
| options.Preconditions.UID = &namespace.UID | ||
| } else if *options.Preconditions.UID != namespace.UID { | ||
| err = apierrors.NewConflict( | ||
| api.Resource("namespaces"), | ||
| name, | ||
| fmt.Errorf("Precondition failed: UID in precondition: %v, UID in object meta: %v", *options.Preconditions.UID, namespace.UID), | ||
| ) | ||
| return nil, err | ||
| } | ||
|
|
||
| // upon first request to delete, we switch the phase to start namespace termination | ||
| // TODO: enhance graceful deletion's calls to DeleteStrategy to allow phase change and finalizer patterns | ||
| if namespace.DeletionTimestamp.IsZero() { | ||
| key, err := r.Store.KeyFunc(ctx, name) | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
|
|
||
| preconditions := storage.Preconditions{UID: options.Preconditions.UID} | ||
|
|
||
| out := r.Store.NewFunc() | ||
| err = r.Store.Storage.GuaranteedUpdate( | ||
| ctx, key, out, false, &preconditions, | ||
| storage.SimpleUpdate(func(existing runtime.Object) (runtime.Object, error) { | ||
| existingNamespace, ok := existing.(*api.Namespace) | ||
| if !ok { | ||
| // wrong type | ||
| return nil, fmt.Errorf("expected *api.Namespace, got %v", existing) | ||
| } | ||
| // Set the deletion timestamp if needed | ||
| if existingNamespace.DeletionTimestamp.IsZero() { | ||
| now := unversioned.Now() | ||
| existingNamespace.DeletionTimestamp = &now | ||
| } | ||
| // Set the namespace phase to terminating, if needed | ||
| if existingNamespace.Status.Phase != api.NamespaceTerminating { | ||
| existingNamespace.Status.Phase = api.NamespaceTerminating | ||
| } | ||
| return existingNamespace, nil | ||
| }), | ||
| ) | ||
|
|
||
| if err != nil { | ||
| err = storageerr.InterpretGetError(err, api.Resource("namespaces"), name) | ||
| err = storageerr.InterpretUpdateError(err, api.Resource("namespaces"), name) | ||
| if _, ok := err.(*apierrors.StatusError); !ok { | ||
| err = apierrors.NewInternalError(err) | ||
| } | ||
| return nil, err | ||
| } | ||
|
|
||
| return out, nil | ||
| } | ||
|
|
||
| // prior to final deletion, we must ensure that finalizers is empty | ||
| if len(namespace.Spec.Finalizers) != 0 { | ||
| err = apierrors.NewConflict(api.Resource("namespaces"), namespace.Name, fmt.Errorf("The system is ensuring all content is removed from this namespace. Upon completion, this namespace will automatically be purged by the system.")) | ||
|
There was a problem hiding this comment. Does it get deleted automatically when the last finalizer is removed? There was a problem hiding this comment. no, the namespace controller sends an additional delete call. There was a problem hiding this comment. yes, the namespace controller handles that: There was a problem hiding this comment. For garbage collector, we were planning on making the update path auto delete things with a deletion timestamp when the last finalizer is removed. Do you have any objections to changing that? There was a problem hiding this comment. That would work much better if multiple components were removing finalizers, assuming you can craft a reasonable PATCH request that removes a single finalizer. |
||
| return nil, err | ||
| } | ||
| return r.Store.Delete(ctx, name, options) | ||
| } | ||
|
|
||
| func (r *StatusREST) New() runtime.Object { | ||
|
|
||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These changes seem good but unrelated-- do they really need to be cherrypicked?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
UID preconditions didn't exist in 1.2, I'll adjust appropriately when I cherry pick