storage: Deleting a namespace while spec.finalizers pending should not error

All objects with graceful deletion allow multiple DELETE calls in the pending
state. Namespace is the one outlier, and the error here predates graceful
deletion and finalizers. We should make this behavior consistent with other
calls and merely indicate success and return the state of the object, the
same as if there were pending metadata finalizers.

Clients that previously checked for a conflict error during delete to know
that the server is already deleting will now no longer receive an error
(as if the object were rapidly deleted). There is a small chance that some
clients have error checking for this state, but a much larger chance that
clients that want to trigger a delete of the namespace do not handle this
error case.

Discovered in an e2e test that used the framework namespace and triggered
deletion of that ns itself, and then the AfterEach step in e2e failed
because the namespace was already pending deletion.
This commit is contained in:
Clayton Coleman 2019-10-19 22:27:52 -04:00
parent 9c25b16d88
commit 2ddeb94b56
No known key found for this signature in database
GPG Key ID: 3D16906B4F1C5CB3
2 changed files with 11 additions and 6 deletions

View File

@ -238,8 +238,7 @@ func (r *REST) Delete(ctx context.Context, name string, deleteValidation rest.Va
// 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."))
return nil, false, err
return namespace, false, nil
}
return r.store.Delete(ctx, name, deleteValidation, options)
}

View File

@ -165,12 +165,18 @@ func TestDeleteNamespaceWithIncompleteFinalizers(t *testing.T) {
if err := storage.store.Storage.Create(ctx, key, namespace, nil, 0, false); err != nil {
t.Fatalf("unexpected error: %v", err)
}
if _, _, err := storage.Delete(ctx, "foo", rest.ValidateAllObjectFunc, nil); err == nil {
t.Errorf("unexpected no error")
obj, immediate, err := storage.Delete(ctx, "foo", rest.ValidateAllObjectFunc, nil)
if err != nil {
t.Fatalf("unexpected error")
}
if immediate {
t.Fatalf("unexpected immediate flag")
}
if ns, ok := obj.(*api.Namespace); !ok || obj == nil || ns == nil || ns.Name != namespace.Name {
t.Fatalf("object not returned by delete")
}
// should still exist
_, err := storage.Get(ctx, "foo", &metav1.GetOptions{})
if err != nil {
if _, err := storage.Get(ctx, "foo", &metav1.GetOptions{}); err != nil {
t.Errorf("unexpected error: %v", err)
}
}