diff --git a/pkg/registry/core/namespace/storage/storage_test.go b/pkg/registry/core/namespace/storage/storage_test.go index a6c763d7497..f91fb03a276 100644 --- a/pkg/registry/core/namespace/storage/storage_test.go +++ b/pkg/registry/core/namespace/storage/storage_test.go @@ -157,7 +157,7 @@ func TestDeleteNamespaceWithIncompleteFinalizers(t *testing.T) { if err := storage.Storage.Create(ctx, key, namespace, nil, 0); err != nil { t.Fatalf("unexpected error: %v", err) } - if _, err := storage.Delete(ctx, "foo", nil); err == nil { + if _, _, err := storage.Delete(ctx, "foo", nil); err == nil { t.Errorf("unexpected error: %v", err) } } @@ -182,7 +182,7 @@ func TestDeleteNamespaceWithCompleteFinalizers(t *testing.T) { if err := storage.Storage.Create(ctx, key, namespace, nil, 0); err != nil { t.Fatalf("unexpected error: %v", err) } - if _, err := storage.Delete(ctx, "foo", nil); err != nil { + if _, _, err := storage.Delete(ctx, "foo", nil); err != nil { t.Errorf("unexpected error: %v", err) } } diff --git a/pkg/registry/core/pod/storage/storage_test.go b/pkg/registry/core/pod/storage/storage_test.go index 9c0a27ac2e5..093fbfbfdcc 100644 --- a/pkg/registry/core/pod/storage/storage_test.go +++ b/pkg/registry/core/pod/storage/storage_test.go @@ -176,7 +176,7 @@ func TestIgnoreDeleteNotFound(t *testing.T) { defer registry.Store.DestroyFunc() // should fail if pod A is not created yet. - _, err := registry.Delete(testContext, pod.Name, nil) + _, _, err := registry.Delete(testContext, pod.Name, nil) if !errors.IsNotFound(err) { t.Errorf("Unexpected error: %v", err) } @@ -191,7 +191,7 @@ func TestIgnoreDeleteNotFound(t *testing.T) { // registry shouldn't get any error since we ignore the NotFound error. zero := int64(0) opt := &metav1.DeleteOptions{GracePeriodSeconds: &zero} - obj, err := registry.Delete(testContext, pod.Name, opt) + obj, _, err := registry.Delete(testContext, pod.Name, opt) if err != nil { t.Fatalf("Unexpected error: %v", err) } diff --git a/pkg/registry/registrytest/etcd.go b/pkg/registry/registrytest/etcd.go index f4e31d9ba3c..13abeac2baa 100644 --- a/pkg/registry/registrytest/etcd.go +++ b/pkg/registry/registrytest/etcd.go @@ -221,7 +221,7 @@ func (t *Tester) emitObject(obj runtime.Object, action string) error { if err != nil { return err } - _, err = t.storage.Delete(ctx, accessor.GetName(), nil) + _, _, err = t.storage.Delete(ctx, accessor.GetName(), nil) default: err = fmt.Errorf("unexpected action: %v", action) } diff --git a/staging/src/k8s.io/apiserver/pkg/endpoints/apiserver_test.go b/staging/src/k8s.io/apiserver/pkg/endpoints/apiserver_test.go index 55571975068..397f0c43230 100644 --- a/staging/src/k8s.io/apiserver/pkg/endpoints/apiserver_test.go +++ b/staging/src/k8s.io/apiserver/pkg/endpoints/apiserver_test.go @@ -457,19 +457,19 @@ func (storage *SimpleRESTStorage) checkContext(ctx request.Context) { storage.actualNamespace, storage.namespacePresent = request.NamespaceFrom(ctx) } -func (storage *SimpleRESTStorage) Delete(ctx request.Context, id string, options *metav1.DeleteOptions) (runtime.Object, error) { +func (storage *SimpleRESTStorage) Delete(ctx request.Context, id string, options *metav1.DeleteOptions) (runtime.Object, bool, error) { storage.checkContext(ctx) storage.deleted = id storage.deleteOptions = options if err := storage.errors["delete"]; err != nil { - return nil, err + return nil, false, err } var obj runtime.Object = &metav1.Status{Status: metav1.StatusSuccess} var err error if storage.injectedFunction != nil { obj, err = storage.injectedFunction(&genericapitesting.Simple{ObjectMeta: metav1.ObjectMeta{Name: id}}) } - return obj, err + return obj, true, err } func (storage *SimpleRESTStorage) New() runtime.Object { @@ -605,7 +605,8 @@ type LegacyRESTStorage struct { } func (storage LegacyRESTStorage) Delete(ctx request.Context, id string) (runtime.Object, error) { - return storage.SimpleRESTStorage.Delete(ctx, id, nil) + obj, _, err := storage.SimpleRESTStorage.Delete(ctx, id, nil) + return obj, err } type MetadataRESTStorage struct { 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 005917cff45..5fa68c67776 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 @@ -335,7 +335,7 @@ func TestStoreCreate(t *testing.T) { // now delete pod with graceful period set delOpts := &metav1.DeleteOptions{GracePeriodSeconds: &gracefulPeriod} - _, err = registry.Delete(testContext, podA.Name, delOpts) + _, _, err = registry.Delete(testContext, podA.Name, delOpts) if err != nil { t.Fatalf("Failed to delete pod gracefully. Unexpected error: %v", err) } @@ -609,7 +609,7 @@ func TestStoreDelete(t *testing.T) { defer destroyFunc() // test failure condition - _, err := registry.Delete(testContext, podA.Name, nil) + _, _, err := registry.Delete(testContext, podA.Name, nil) if !errors.IsNotFound(err) { t.Errorf("Unexpected error: %v", err) } @@ -621,10 +621,13 @@ func TestStoreDelete(t *testing.T) { } // delete object - _, err = registry.Delete(testContext, podA.Name, nil) + _, wasDeleted, err := registry.Delete(testContext, podA.Name, nil) if err != nil { t.Errorf("Unexpected error: %v", err) } + if !wasDeleted { + t.Errorf("unexpected, pod %s should have been deleted immediately", podA.Name) + } // try to get a item which should be deleted _, err = registry.Get(testContext, podA.Name, &metav1.GetOptions{}) @@ -690,10 +693,13 @@ func TestGracefulStoreHandleFinalizers(t *testing.T) { } // delete the pod with grace period=0, the pod should still exist because it has a finalizer - _, err = registry.Delete(testContext, podWithFinalizer.Name, metav1.NewDeleteOptions(0)) + _, wasDeleted, err := registry.Delete(testContext, podWithFinalizer.Name, metav1.NewDeleteOptions(0)) if err != nil { t.Fatalf("Unexpected error: %v", err) } + if wasDeleted { + t.Errorf("unexpected, pod %s should not have been deleted immediately", podWithFinalizer.Name) + } _, err = registry.Get(testContext, podWithFinalizer.Name, &metav1.GetOptions{}) if err != nil { t.Fatalf("Unexpected error: %v", err) @@ -747,10 +753,13 @@ func TestNonGracefulStoreHandleFinalizers(t *testing.T) { } // delete object with nil delete options doesn't delete the object - _, err = registry.Delete(testContext, podWithFinalizer.Name, nil) + _, wasDeleted, err := registry.Delete(testContext, podWithFinalizer.Name, nil) if err != nil { t.Errorf("Unexpected error: %v", err) } + if wasDeleted { + t.Errorf("unexpected, pod %s should not have been deleted immediately", podWithFinalizer.Name) + } // the object should still exist obj, err := registry.Get(testContext, podWithFinalizer.Name, &metav1.GetOptions{}) @@ -1043,7 +1052,7 @@ func TestStoreDeleteWithOrphanDependents(t *testing.T) { if err != nil { t.Fatalf("Unexpected error: %v", err) } - _, err = registry.Delete(testContext, tc.pod.Name, tc.options) + _, _, err = registry.Delete(testContext, tc.pod.Name, tc.options) if err != nil { t.Fatalf("Unexpected error: %v", err) } diff --git a/staging/src/k8s.io/apiserver/pkg/registry/rest/resttest/resttest.go b/staging/src/k8s.io/apiserver/pkg/registry/rest/resttest/resttest.go index 8fdb59886c4..3d680e4dc66 100644 --- a/staging/src/k8s.io/apiserver/pkg/registry/rest/resttest/resttest.go +++ b/staging/src/k8s.io/apiserver/pkg/registry/rest/resttest/resttest.go @@ -236,7 +236,7 @@ func (t *Tester) delete(ctx genericapirequest.Context, obj runtime.Object) error if !ok { return fmt.Errorf("Expected deleting storage, got %v", t.storage) } - _, err = deleter.Delete(ctx, objectMeta.Name, nil) + _, _, err = deleter.Delete(ctx, objectMeta.Name, nil) return err } @@ -765,10 +765,13 @@ func (t *Tester) testDeleteNoGraceful(obj runtime.Object, createFn CreateFunc, g t.Errorf("unexpected error: %v", err) } objectMeta := t.getObjectMetaOrFail(foo) - obj, err := t.storage.(rest.GracefulDeleter).Delete(ctx, objectMeta.Name, metav1.NewDeleteOptions(10)) + obj, wasDeleted, err := t.storage.(rest.GracefulDeleter).Delete(ctx, objectMeta.Name, metav1.NewDeleteOptions(10)) if err != nil { t.Errorf("unexpected error: %v", err) } + if !wasDeleted { + t.Errorf("unexpected, object %s should have been deleted immediately", objectMeta.Name) + } if !t.returnDeletedObject { if status, ok := obj.(*metav1.Status); !ok { t.Errorf("expected status of delete, got %v", status) @@ -786,7 +789,7 @@ func (t *Tester) testDeleteNoGraceful(obj runtime.Object, createFn CreateFunc, g func (t *Tester) testDeleteNonExist(obj runtime.Object) { objectMeta := t.getObjectMetaOrFail(obj) - _, err := t.storage.(rest.GracefulDeleter).Delete(t.TestContext(), objectMeta.Name, nil) + _, _, err := t.storage.(rest.GracefulDeleter).Delete(t.TestContext(), objectMeta.Name, nil) if err == nil || !errors.IsNotFound(err) { t.Errorf("unexpected error: %v", err) } @@ -805,12 +808,12 @@ func (t *Tester) testDeleteWithUID(obj runtime.Object, createFn CreateFunc, getF if err := createFn(ctx, foo); err != nil { t.Errorf("unexpected error: %v", err) } - obj, err := t.storage.(rest.GracefulDeleter).Delete(ctx, objectMeta.Name, metav1.NewPreconditionDeleteOptions("UID1111")) + obj, _, err := t.storage.(rest.GracefulDeleter).Delete(ctx, objectMeta.Name, metav1.NewPreconditionDeleteOptions("UID1111")) if err == nil || !errors.IsConflict(err) { t.Errorf("unexpected error: %v", err) } - obj, err = t.storage.(rest.GracefulDeleter).Delete(ctx, objectMeta.Name, metav1.NewPreconditionDeleteOptions("UID0000")) + obj, _, err = t.storage.(rest.GracefulDeleter).Delete(ctx, objectMeta.Name, metav1.NewPreconditionDeleteOptions("UID0000")) if err != nil { t.Errorf("unexpected error: %v", err) } @@ -842,10 +845,13 @@ func (t *Tester) testDeleteGracefulHasDefault(obj runtime.Object, createFn Creat } objectMeta := t.getObjectMetaOrFail(foo) generation := objectMeta.Generation - _, err := t.storage.(rest.GracefulDeleter).Delete(ctx, objectMeta.Name, &metav1.DeleteOptions{}) + _, wasDeleted, err := t.storage.(rest.GracefulDeleter).Delete(ctx, objectMeta.Name, &metav1.DeleteOptions{}) if err != nil { t.Errorf("unexpected error: %v", err) } + if wasDeleted { + t.Errorf("unexpected, object %s should not have been deleted immediately", objectMeta.Name) + } if _, err := getFn(ctx, foo); err != nil { t.Fatalf("did not gracefully delete resource: %v", err) } @@ -873,10 +879,13 @@ func (t *Tester) testDeleteGracefulWithValue(obj runtime.Object, createFn Create } objectMeta := t.getObjectMetaOrFail(foo) generation := objectMeta.Generation - _, err := t.storage.(rest.GracefulDeleter).Delete(ctx, objectMeta.Name, metav1.NewDeleteOptions(expectedGrace+2)) + _, wasDeleted, err := t.storage.(rest.GracefulDeleter).Delete(ctx, objectMeta.Name, metav1.NewDeleteOptions(expectedGrace+2)) if err != nil { t.Errorf("unexpected error: %v", err) } + if wasDeleted { + t.Errorf("unexpected, object %s should not have been deleted immediately", objectMeta.Name) + } if _, err := getFn(ctx, foo); err != nil { t.Fatalf("did not gracefully delete resource: %v", err) } @@ -904,19 +913,25 @@ func (t *Tester) testDeleteGracefulExtend(obj runtime.Object, createFn CreateFun } objectMeta := t.getObjectMetaOrFail(foo) generation := objectMeta.Generation - _, err := t.storage.(rest.GracefulDeleter).Delete(ctx, objectMeta.Name, metav1.NewDeleteOptions(expectedGrace)) + _, wasDeleted, err := t.storage.(rest.GracefulDeleter).Delete(ctx, objectMeta.Name, metav1.NewDeleteOptions(expectedGrace)) if err != nil { t.Errorf("unexpected error: %v", err) } + if wasDeleted { + t.Errorf("unexpected, object %s should not have been deleted immediately", objectMeta.Name) + } if _, err := getFn(ctx, foo); err != nil { t.Fatalf("did not gracefully delete resource: %v", err) } // second delete duration is ignored - _, err = t.storage.(rest.GracefulDeleter).Delete(ctx, objectMeta.Name, metav1.NewDeleteOptions(expectedGrace+2)) + _, wasDeleted, err = t.storage.(rest.GracefulDeleter).Delete(ctx, objectMeta.Name, metav1.NewDeleteOptions(expectedGrace+2)) if err != nil { t.Errorf("unexpected error: %v", err) } + if wasDeleted { + t.Errorf("unexpected, object %s should not have been deleted immediately", objectMeta.Name) + } object, err := t.storage.(rest.Getter).Get(ctx, objectMeta.Name, &metav1.GetOptions{}) if err != nil { t.Errorf("unexpected error, object should exist: %v", err) @@ -940,19 +955,25 @@ func (t *Tester) testDeleteGracefulImmediate(obj runtime.Object, createFn Create } objectMeta := t.getObjectMetaOrFail(foo) generation := objectMeta.Generation - _, err := t.storage.(rest.GracefulDeleter).Delete(ctx, objectMeta.Name, metav1.NewDeleteOptions(expectedGrace)) + _, wasDeleted, err := t.storage.(rest.GracefulDeleter).Delete(ctx, objectMeta.Name, metav1.NewDeleteOptions(expectedGrace)) if err != nil { t.Errorf("unexpected error: %v", err) } + if wasDeleted { + t.Errorf("unexpected, object %s should not have been deleted immediately", objectMeta.Name) + } if _, err := getFn(ctx, foo); err != nil { t.Fatalf("did not gracefully delete resource: %v", err) } // second delete is immediate, resource is deleted - out, err := t.storage.(rest.GracefulDeleter).Delete(ctx, objectMeta.Name, metav1.NewDeleteOptions(0)) + out, wasDeleted, err := t.storage.(rest.GracefulDeleter).Delete(ctx, objectMeta.Name, metav1.NewDeleteOptions(0)) if err != nil { t.Errorf("unexpected error: %v", err) } + if wasDeleted != true { + t.Errorf("unexpected, object %s should have been deleted immediately", objectMeta.Name) + } _, err = t.storage.(rest.Getter).Get(ctx, objectMeta.Name, &metav1.GetOptions{}) if !errors.IsNotFound(err) { t.Errorf("unexpected error, object should be deleted immediately: %v", err) @@ -976,10 +997,13 @@ func (t *Tester) testDeleteGracefulUsesZeroOnNil(obj runtime.Object, createFn Cr t.Errorf("unexpected error: %v", err) } objectMeta := t.getObjectMetaOrFail(foo) - _, err := t.storage.(rest.GracefulDeleter).Delete(ctx, objectMeta.Name, nil) + _, wasDeleted, err := t.storage.(rest.GracefulDeleter).Delete(ctx, objectMeta.Name, nil) if err != nil { t.Errorf("unexpected error: %v", err) } + if !wasDeleted { + t.Errorf("unexpected, object %s should have been deleted immediately", objectMeta.Name) + } if _, err := t.storage.(rest.Getter).Get(ctx, objectMeta.Name, &metav1.GetOptions{}); !errors.IsNotFound(err) { t.Errorf("unexpected error, object should not exist: %v", err) } @@ -999,10 +1023,13 @@ func (t *Tester) testDeleteGracefulShorten(obj runtime.Object, createFn CreateFu bigGrace = 2 * expectedGrace } objectMeta := t.getObjectMetaOrFail(foo) - _, err := t.storage.(rest.GracefulDeleter).Delete(ctx, objectMeta.Name, metav1.NewDeleteOptions(bigGrace)) + _, wasDeleted, err := t.storage.(rest.GracefulDeleter).Delete(ctx, objectMeta.Name, metav1.NewDeleteOptions(bigGrace)) if err != nil { t.Errorf("unexpected error: %v", err) } + if wasDeleted { + t.Errorf("unexpected, object %s should not have been deleted immediately", objectMeta.Name) + } object, err := getFn(ctx, foo) if err != nil { t.Fatalf("did not gracefully delete resource: %v", err) @@ -1011,10 +1038,13 @@ func (t *Tester) testDeleteGracefulShorten(obj runtime.Object, createFn CreateFu deletionTimestamp := *objectMeta.DeletionTimestamp // second delete duration is ignored - _, err = t.storage.(rest.GracefulDeleter).Delete(ctx, objectMeta.Name, metav1.NewDeleteOptions(expectedGrace)) + _, wasDeleted, err = t.storage.(rest.GracefulDeleter).Delete(ctx, objectMeta.Name, metav1.NewDeleteOptions(expectedGrace)) if err != nil { t.Errorf("unexpected error: %v", err) } + if wasDeleted { + t.Errorf("unexpected, object %s should not have been deleted immediately", objectMeta.Name) + } object, err = t.storage.(rest.Getter).Get(ctx, objectMeta.Name, &metav1.GetOptions{}) if err != nil { t.Errorf("unexpected error, object should exist: %v", err)