diff --git a/pkg/controlplane/reconcilers/lease.go b/pkg/controlplane/reconcilers/lease.go index 4d794ff74b7..ca3e1dee04b 100644 --- a/pkg/controlplane/reconcilers/lease.go +++ b/pkg/controlplane/reconcilers/lease.go @@ -113,7 +113,7 @@ func (s *storageLeases) UpdateLease(ip string) error { // RemoveLease removes the lease on a master IP in storage func (s *storageLeases) RemoveLease(ip string) error { - return s.storage.Delete(apirequest.NewDefaultContext(), s.baseKey+"/"+ip, &corev1.Endpoints{}, nil, rest.ValidateAllObjectFunc) + return s.storage.Delete(apirequest.NewDefaultContext(), s.baseKey+"/"+ip, &corev1.Endpoints{}, nil, rest.ValidateAllObjectFunc, nil) } // NewLeases creates a new etcd-based Leases implementation. diff --git a/pkg/registry/core/pod/storage/storage_test.go b/pkg/registry/core/pod/storage/storage_test.go index 978b467d794..0ef8198c89c 100644 --- a/pkg/registry/core/pod/storage/storage_test.go +++ b/pkg/registry/core/pod/storage/storage_test.go @@ -159,7 +159,7 @@ type FailDeletionStorage struct { Called *bool } -func (f FailDeletionStorage) Delete(ctx context.Context, key string, out runtime.Object, precondition *apiserverstorage.Preconditions, _ apiserverstorage.ValidateObjectFunc) error { +func (f FailDeletionStorage) Delete(_ context.Context, key string, _ runtime.Object, _ *apiserverstorage.Preconditions, _ apiserverstorage.ValidateObjectFunc, _ runtime.Object) error { *f.Called = true return apiserverstorage.NewKeyNotFoundError(key, 0) } diff --git a/staging/src/k8s.io/apiserver/pkg/registry/generic/registry/dryrun.go b/staging/src/k8s.io/apiserver/pkg/registry/generic/registry/dryrun.go index 2f184c50e01..469893cd1ef 100644 --- a/staging/src/k8s.io/apiserver/pkg/registry/generic/registry/dryrun.go +++ b/staging/src/k8s.io/apiserver/pkg/registry/generic/registry/dryrun.go @@ -43,7 +43,7 @@ func (s *DryRunnableStorage) Create(ctx context.Context, key string, obj, out ru return s.Storage.Create(ctx, key, obj, out, ttl) } -func (s *DryRunnableStorage) Delete(ctx context.Context, key string, out runtime.Object, preconditions *storage.Preconditions, deleteValidation storage.ValidateObjectFunc, dryRun bool) error { +func (s *DryRunnableStorage) Delete(ctx context.Context, key string, out runtime.Object, preconditions *storage.Preconditions, deleteValidation storage.ValidateObjectFunc, dryRun bool, cachedExistingObject runtime.Object) error { if dryRun { if err := s.Storage.Get(ctx, key, storage.GetOptions{}, out); err != nil { return err @@ -53,7 +53,7 @@ func (s *DryRunnableStorage) Delete(ctx context.Context, key string, out runtime } return deleteValidation(ctx, out) } - return s.Storage.Delete(ctx, key, out, preconditions, deleteValidation) + return s.Storage.Delete(ctx, key, out, preconditions, deleteValidation, cachedExistingObject) } func (s *DryRunnableStorage) Watch(ctx context.Context, key string, opts storage.ListOptions) (watch.Interface, error) { diff --git a/staging/src/k8s.io/apiserver/pkg/registry/generic/registry/dryrun_test.go b/staging/src/k8s.io/apiserver/pkg/registry/generic/registry/dryrun_test.go index 5d40004272c..f725b5c8717 100644 --- a/staging/src/k8s.io/apiserver/pkg/registry/generic/registry/dryrun_test.go +++ b/staging/src/k8s.io/apiserver/pkg/registry/generic/registry/dryrun_test.go @@ -237,7 +237,7 @@ func TestDryRunDeleteDoesntDelete(t *testing.T) { t.Fatalf("Failed to create new object: %v", err) } - err = s.Delete(context.Background(), "key", out, nil, rest.ValidateAllObjectFunc, true) + err = s.Delete(context.Background(), "key", out, nil, rest.ValidateAllObjectFunc, true, nil) if err != nil { t.Fatalf("Failed to dry-run delete the object: %v", err) } @@ -253,7 +253,7 @@ func TestDryRunDeleteMissingObjectFails(t *testing.T) { defer destroy() out := UnstructuredOrDie(`{}`) - err := s.Delete(context.Background(), "key", out, nil, rest.ValidateAllObjectFunc, true) + err := s.Delete(context.Background(), "key", out, nil, rest.ValidateAllObjectFunc, true, nil) if e, ok := err.(*storage.StorageError); !ok || e.Code != storage.ErrCodeKeyNotFound { t.Errorf("Expected key to be not found, error: %v", err) } @@ -273,7 +273,7 @@ func TestDryRunDeleteReturnsObject(t *testing.T) { out = UnstructuredOrDie(`{}`) expected := UnstructuredOrDie(`{"kind": "Pod", "metadata": {"resourceVersion": "2"}}`) - err = s.Delete(context.Background(), "key", out, nil, rest.ValidateAllObjectFunc, true) + err = s.Delete(context.Background(), "key", out, nil, rest.ValidateAllObjectFunc, true, nil) if err != nil { t.Fatalf("Failed to delete with valid precondition: %v", err) } @@ -296,12 +296,12 @@ func TestDryRunDeletePreconditions(t *testing.T) { wrongID := types.UID("wrong-uid") myID := types.UID("my-uid") - err = s.Delete(context.Background(), "key", out, &storage.Preconditions{UID: &wrongID}, rest.ValidateAllObjectFunc, true) + err = s.Delete(context.Background(), "key", out, &storage.Preconditions{UID: &wrongID}, rest.ValidateAllObjectFunc, true, nil) if e, ok := err.(*storage.StorageError); !ok || e.Code != storage.ErrCodeInvalidObj { t.Errorf("Expected invalid object, error: %v", err) } - err = s.Delete(context.Background(), "key", out, &storage.Preconditions{UID: &myID}, rest.ValidateAllObjectFunc, true) + err = s.Delete(context.Background(), "key", out, &storage.Preconditions{UID: &myID}, rest.ValidateAllObjectFunc, true, nil) if err != nil { t.Fatalf("Failed to delete with valid precondition: %v", err) } 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 7c2f4c390eb..bbc167d3996 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 @@ -428,7 +428,7 @@ func (e *Store) deleteWithoutFinalizers(ctx context.Context, name, key string, o out := e.NewFunc() klog.V(6).Infof("going to delete %s from registry, triggered by update", name) // Using the rest.ValidateAllObjectFunc because the request is an UPDATE request and has already passed the admission for the UPDATE verb. - if err := e.Storage.Delete(ctx, key, out, preconditions, rest.ValidateAllObjectFunc, dryRun); err != nil { + if err := e.Storage.Delete(ctx, key, out, preconditions, rest.ValidateAllObjectFunc, dryRun, nil); err != nil { // Deletion is racy, i.e., there could be multiple update // requests to remove all finalizers from the object, so we // ignore the NotFound error. @@ -963,7 +963,7 @@ func (e *Store) Delete(ctx context.Context, name string, deleteValidation rest.V // delete immediately, or no graceful deletion supported klog.V(6).Infof("going to delete %s from registry: ", name) out = e.NewFunc() - if err := e.Storage.Delete(ctx, key, out, &preconditions, storage.ValidateObjectFunc(deleteValidation), dryrun.IsDryRun(options.DryRun)); err != nil { + if err := e.Storage.Delete(ctx, key, out, &preconditions, storage.ValidateObjectFunc(deleteValidation), dryrun.IsDryRun(options.DryRun), nil); err != nil { // Please refer to the place where we set ignoreNotFound for the reason // why we ignore the NotFound error . if storage.IsNotFound(err) && ignoreNotFound && lastExisting != nil { diff --git a/staging/src/k8s.io/apiserver/pkg/storage/cacher/cacher.go b/staging/src/k8s.io/apiserver/pkg/storage/cacher/cacher.go index 45951776754..9170a63a46e 100644 --- a/staging/src/k8s.io/apiserver/pkg/storage/cacher/cacher.go +++ b/staging/src/k8s.io/apiserver/pkg/storage/cacher/cacher.go @@ -431,8 +431,21 @@ func (c *Cacher) Create(ctx context.Context, key string, obj, out runtime.Object } // Delete implements storage.Interface. -func (c *Cacher) Delete(ctx context.Context, key string, out runtime.Object, preconditions *storage.Preconditions, validateDeletion storage.ValidateObjectFunc) error { - return c.storage.Delete(ctx, key, out, preconditions, validateDeletion) +func (c *Cacher) Delete( + ctx context.Context, key string, out runtime.Object, preconditions *storage.Preconditions, + validateDeletion storage.ValidateObjectFunc, _ runtime.Object) error { + // Ignore the suggestion and try to pass down the current version of the object + // read from cache. + if elem, exists, err := c.watchCache.GetByKey(key); err != nil { + klog.Errorf("GetByKey returned error: %v", err) + } else if exists { + // DeepCopy the object since we modify resource version when serializing the + // current object. + currObj := elem.(*storeElement).Object.DeepCopyObject() + return c.storage.Delete(ctx, key, out, preconditions, validateDeletion, currObj) + } + // If we couldn't get the object, fallback to no-suggestion. + return c.storage.Delete(ctx, key, out, preconditions, validateDeletion, nil) } // Watch implements storage.Interface. diff --git a/staging/src/k8s.io/apiserver/pkg/storage/cacher/cacher_whitebox_test.go b/staging/src/k8s.io/apiserver/pkg/storage/cacher/cacher_whitebox_test.go index 2908c25c0f5..13deb0ee591 100644 --- a/staging/src/k8s.io/apiserver/pkg/storage/cacher/cacher_whitebox_test.go +++ b/staging/src/k8s.io/apiserver/pkg/storage/cacher/cacher_whitebox_test.go @@ -299,7 +299,7 @@ func (d *dummyStorage) Versioner() storage.Versioner { return nil } func (d *dummyStorage) Create(_ context.Context, _ string, _, _ runtime.Object, _ uint64) error { return fmt.Errorf("unimplemented") } -func (d *dummyStorage) Delete(_ context.Context, _ string, _ runtime.Object, _ *storage.Preconditions, _ storage.ValidateObjectFunc) error { +func (d *dummyStorage) Delete(_ context.Context, _ string, _ runtime.Object, _ *storage.Preconditions, _ storage.ValidateObjectFunc, _ runtime.Object) error { return fmt.Errorf("unimplemented") } func (d *dummyStorage) Watch(_ context.Context, _ string, _ storage.ListOptions) (watch.Interface, error) { diff --git a/staging/src/k8s.io/apiserver/pkg/storage/etcd3/store.go b/staging/src/k8s.io/apiserver/pkg/storage/etcd3/store.go index f1b5725338e..b5de5a84c8b 100644 --- a/staging/src/k8s.io/apiserver/pkg/storage/etcd3/store.go +++ b/staging/src/k8s.io/apiserver/pkg/storage/etcd3/store.go @@ -185,35 +185,77 @@ func (s *store) Create(ctx context.Context, key string, obj, out runtime.Object, } // Delete implements storage.Interface.Delete. -func (s *store) Delete(ctx context.Context, key string, out runtime.Object, preconditions *storage.Preconditions, validateDeletion storage.ValidateObjectFunc) error { +func (s *store) Delete( + ctx context.Context, key string, out runtime.Object, preconditions *storage.Preconditions, + validateDeletion storage.ValidateObjectFunc, cachedExistingObject runtime.Object) error { v, err := conversion.EnforcePtr(out) if err != nil { return fmt.Errorf("unable to convert output object to pointer: %v", err) } key = path.Join(s.pathPrefix, key) - return s.conditionalDelete(ctx, key, out, v, preconditions, validateDeletion) + return s.conditionalDelete(ctx, key, out, v, preconditions, validateDeletion, cachedExistingObject) } -func (s *store) conditionalDelete(ctx context.Context, key string, out runtime.Object, v reflect.Value, preconditions *storage.Preconditions, validateDeletion storage.ValidateObjectFunc) error { - startTime := time.Now() - getResp, err := s.client.KV.Get(ctx, key) - metrics.RecordEtcdRequestLatency("get", getTypeName(out), startTime) +func (s *store) conditionalDelete( + ctx context.Context, key string, out runtime.Object, v reflect.Value, preconditions *storage.Preconditions, + validateDeletion storage.ValidateObjectFunc, cachedExistingObject runtime.Object) error { + getCurrentState := func() (*objState, error) { + startTime := time.Now() + getResp, err := s.client.KV.Get(ctx, key) + metrics.RecordEtcdRequestLatency("get", getTypeName(out), startTime) + if err != nil { + return nil, err + } + return s.getState(getResp, key, v, false) + } + + var origState *objState + var err error + var origStateIsCurrent bool + if cachedExistingObject != nil { + origState, err = s.getStateFromObject(cachedExistingObject) + } else { + origState, err = getCurrentState() + origStateIsCurrent = true + } if err != nil { return err } + for { - origState, err := s.getState(getResp, key, v, false) - if err != nil { - return err - } if preconditions != nil { if err := preconditions.Check(key, origState.obj); err != nil { - return err + if origStateIsCurrent { + return err + } + + // It's possible we're working with stale data. + // Actually fetch + origState, err = getCurrentState() + if err != nil { + return err + } + origStateIsCurrent = true + // Retry + continue } } if err := validateDeletion(ctx, origState.obj); err != nil { - return err + if origStateIsCurrent { + return err + } + + // It's possible we're working with stale data. + // Actually fetch + origState, err = getCurrentState() + if err != nil { + return err + } + origStateIsCurrent = true + // Retry + continue } + startTime := time.Now() txnResp, err := s.client.KV.Txn(ctx).If( clientv3.Compare(clientv3.ModRevision(key), "=", origState.rev), @@ -227,8 +269,13 @@ func (s *store) conditionalDelete(ctx context.Context, key string, out runtime.O return err } if !txnResp.Succeeded { - getResp = (*clientv3.GetResponse)(txnResp.Responses[0].GetResponseRange()) + getResp := (*clientv3.GetResponse)(txnResp.Responses[0].GetResponseRange()) klog.V(4).Infof("deletion of %s failed because of a conflict, going to retry", key) + origState, err = s.getState(getResp, key, v, false) + if err != nil { + return err + } + origStateIsCurrent = true continue } return decode(s.codec, s.versioner, origState.data, out, origState.rev) @@ -262,15 +309,12 @@ func (s *store) GuaranteedUpdate( var mustCheckData bool if suggestion != nil { origState, err = s.getStateFromObject(suggestion) - if err != nil { - return err - } mustCheckData = true } else { origState, err = getCurrentState() - if err != nil { - return err - } + } + if err != nil { + return err } trace.Step("initial value restored") diff --git a/staging/src/k8s.io/apiserver/pkg/storage/etcd3/store_test.go b/staging/src/k8s.io/apiserver/pkg/storage/etcd3/store_test.go index a42121ede7f..190d7892a34 100644 --- a/staging/src/k8s.io/apiserver/pkg/storage/etcd3/store_test.go +++ b/staging/src/k8s.io/apiserver/pkg/storage/etcd3/store_test.go @@ -324,7 +324,7 @@ func TestUnconditionalDelete(t *testing.T) { for i, tt := range tests { out := &example.Pod{} // reset - err := store.Delete(ctx, tt.key, out, nil, storage.ValidateAllObjectFunc) + err := store.Delete(ctx, tt.key, out, nil, storage.ValidateAllObjectFunc, nil) if tt.expectNotFoundErr { if err == nil || !storage.IsNotFound(err) { t.Errorf("#%d: expecting not found error, but get: %s", i, err) @@ -358,7 +358,7 @@ func TestConditionalDelete(t *testing.T) { for i, tt := range tests { out := &example.Pod{} - err := store.Delete(ctx, key, out, tt.precondition, storage.ValidateAllObjectFunc) + err := store.Delete(ctx, key, out, tt.precondition, storage.ValidateAllObjectFunc, nil) if tt.expectInvalidObjErr { if err == nil || !storage.IsInvalidObj(err) { t.Errorf("#%d: expecting invalid UID error, but get: %s", i, err) @@ -375,6 +375,172 @@ func TestConditionalDelete(t *testing.T) { } } +// The following set of Delete tests are testing the logic of adding `suggestion` +// as a parameter with probably value of the current state. +// Introducing it for GuaranteedUpdate cause a number of issues, so we're addressing +// all of those upfront by adding appropriate tests: +// - https://github.com/kubernetes/kubernetes/pull/35415 +// [DONE] Lack of tests originally - added TestDeleteWithSuggestion. +// - https://github.com/kubernetes/kubernetes/pull/40664 +// [DONE] Irrelevant for delete, as Delete doesn't write data (nor compare it). +// - https://github.com/kubernetes/kubernetes/pull/47703 +// [DONE] Irrelevant for delete, because Delete doesn't persist data. +// - https://github.com/kubernetes/kubernetes/pull/48394/ +// [DONE] Irrelevant for delete, because Delete doesn't compare data. +// - https://github.com/kubernetes/kubernetes/pull/43152 +// [DONE] Added TestDeleteWithSuggestionAndConflict +// - https://github.com/kubernetes/kubernetes/pull/54780 +// [DONE] Irrelevant for delete, because Delete doesn't compare data. +// - https://github.com/kubernetes/kubernetes/pull/58375 +// [DONE] Irrelevant for delete, because Delete doesn't compare data. +// - https://github.com/kubernetes/kubernetes/pull/77619 +// [DONE] Added TestValidateDeletionWithSuggestion for corresponding delete checks. +// - https://github.com/kubernetes/kubernetes/pull/78713 +// [DONE] Bug was in getState function which is shared with the new code. +// - https://github.com/kubernetes/kubernetes/pull/78713 +// [DONE] Added TestPreconditionalDeleteWithSuggestion + +func TestDeleteWithSuggestion(t *testing.T) { + ctx, store, cluster := testSetup(t) + defer cluster.Terminate(t) + + key, originalPod := testPropogateStore(ctx, t, store, &example.Pod{ObjectMeta: metav1.ObjectMeta{Name: "name"}}) + + out := &example.Pod{} + if err := store.Delete(ctx, key, out, nil, storage.ValidateAllObjectFunc, originalPod); err != nil { + t.Errorf("Unexpected failure during deletion: %v", err) + } + + if err := store.Get(ctx, key, storage.GetOptions{}, &example.Pod{}); !storage.IsNotFound(err) { + t.Errorf("Unexpected error on reading object: %v", err) + } +} + +func TestDeleteWithSuggestionAndConflict(t *testing.T) { + ctx, store, cluster := testSetup(t) + defer cluster.Terminate(t) + + key, originalPod := testPropogateStore(ctx, t, store, &example.Pod{ObjectMeta: metav1.ObjectMeta{Name: "name"}}) + + // First update, so originalPod is outdated. + updatedPod := &example.Pod{} + if err := store.GuaranteedUpdate(ctx, key, updatedPod, false, nil, + storage.SimpleUpdate(func(obj runtime.Object) (runtime.Object, error) { + pod := obj.(*example.Pod) + pod.ObjectMeta.Labels = map[string]string{"foo": "bar"} + return pod, nil + }), nil); err != nil { + t.Errorf("Unexpected failure during updated: %v", err) + } + + out := &example.Pod{} + if err := store.Delete(ctx, key, out, nil, storage.ValidateAllObjectFunc, originalPod); err != nil { + t.Errorf("Unexpected failure during deletion: %v", err) + } + + if err := store.Get(ctx, key, storage.GetOptions{}, &example.Pod{}); !storage.IsNotFound(err) { + t.Errorf("Unexpected error on reading object: %v", err) + } +} + +func TestDeleteWithSuggestionOfDeletedObject(t *testing.T) { + ctx, store, cluster := testSetup(t) + defer cluster.Terminate(t) + + key, originalPod := testPropogateStore(ctx, t, store, &example.Pod{ObjectMeta: metav1.ObjectMeta{Name: "name"}}) + + // First delete, so originalPod is outdated. + deletedPod := &example.Pod{} + if err := store.Delete(ctx, key, deletedPod, nil, storage.ValidateAllObjectFunc, originalPod); err != nil { + t.Errorf("Unexpected failure during deletion: %v", err) + } + + // Now try deleting with stale object. + out := &example.Pod{} + if err := store.Delete(ctx, key, out, nil, storage.ValidateAllObjectFunc, originalPod); !storage.IsNotFound(err) { + t.Errorf("Unexpected error during deletion: %v, expected not-found", err) + } +} + +func TestValidateDeletionWithSuggestion(t *testing.T) { + ctx, store, cluster := testSetup(t) + defer cluster.Terminate(t) + + key, originalPod := testPropogateStore(ctx, t, store, &example.Pod{ObjectMeta: metav1.ObjectMeta{Name: "name"}}) + + // Check that validaing fresh object fails. + validationError := fmt.Errorf("validation error") + validateNothing := func(_ context.Context, _ runtime.Object) error { + return validationError + } + out := &example.Pod{} + if err := store.Delete(ctx, key, out, nil, validateNothing, originalPod); err != validationError { + t.Errorf("Unexpected failure during deletion: %v", err) + } + + // First update, so originalPod is outdated. + updatedPod := &example.Pod{} + if err := store.GuaranteedUpdate(ctx, key, updatedPod, false, nil, + storage.SimpleUpdate(func(obj runtime.Object) (runtime.Object, error) { + pod := obj.(*example.Pod) + pod.ObjectMeta.Labels = map[string]string{"foo": "bar"} + return pod, nil + }), nil); err != nil { + t.Errorf("Unexpected failure during updated: %v", err) + } + + calls := 0 + validateFresh := func(_ context.Context, obj runtime.Object) error { + calls++ + pod := obj.(*example.Pod) + if pod.ObjectMeta.Labels == nil || pod.ObjectMeta.Labels["foo"] != "bar" { + return fmt.Errorf("stale object") + } + return nil + } + + if err := store.Delete(ctx, key, out, nil, validateFresh, originalPod); err != nil { + t.Errorf("Unexpected failure during deletion: %v", err) + } + + if calls != 2 { + t.Errorf("validate function should have been called twice, called %d", calls) + } + + if err := store.Get(ctx, key, storage.GetOptions{}, &example.Pod{}); !storage.IsNotFound(err) { + t.Errorf("Unexpected error on reading object: %v", err) + } +} + +func TestPreconditionalDeleteWithSuggestion(t *testing.T) { + ctx, store, cluster := testSetup(t) + defer cluster.Terminate(t) + + key, originalPod := testPropogateStore(ctx, t, store, &example.Pod{ObjectMeta: metav1.ObjectMeta{Name: "name"}}) + + // First update, so originalPod is outdated. + updatedPod := &example.Pod{} + if err := store.GuaranteedUpdate(ctx, key, updatedPod, false, nil, + storage.SimpleUpdate(func(obj runtime.Object) (runtime.Object, error) { + pod := obj.(*example.Pod) + pod.ObjectMeta.UID = "myUID" + return pod, nil + }), nil); err != nil { + t.Errorf("Unexpected failure during updated: %v", err) + } + + prec := storage.NewUIDPreconditions("myUID") + + out := &example.Pod{} + if err := store.Delete(ctx, key, out, prec, storage.ValidateAllObjectFunc, originalPod); err != nil { + t.Errorf("Unexpected failure during deletion: %v", err) + } + + if err := store.Get(ctx, key, storage.GetOptions{}, &example.Pod{}); !storage.IsNotFound(err) { + t.Errorf("Unexpected error on reading object: %v", err) + } +} + func TestGetToList(t *testing.T) { ctx, store, cluster := testSetup(t) defer cluster.Terminate(t) @@ -888,7 +1054,7 @@ func TestTransformationFailure(t *testing.T) { } // Delete fails with internal error. - if err := store.Delete(ctx, preset[1].key, &example.Pod{}, nil, storage.ValidateAllObjectFunc); !storage.IsInternalError(err) { + if err := store.Delete(ctx, preset[1].key, &example.Pod{}, nil, storage.ValidateAllObjectFunc, nil); !storage.IsInternalError(err) { t.Errorf("Unexpected error: %v", err) } if err := store.Get(ctx, preset[1].key, storage.GetOptions{}, &example.Pod{}); !storage.IsInternalError(err) { @@ -1833,7 +1999,7 @@ func testPropogateStoreWithKey(ctx context.Context, t *testing.T, store *store, if err != nil { panic("unable to convert output object to pointer") } - err = store.conditionalDelete(ctx, key, &example.Pod{}, v, nil, storage.ValidateAllObjectFunc) + err = store.conditionalDelete(ctx, key, &example.Pod{}, v, nil, storage.ValidateAllObjectFunc, nil) if err != nil && !storage.IsNotFound(err) { t.Fatalf("Cleanup failed: %v", err) } diff --git a/staging/src/k8s.io/apiserver/pkg/storage/etcd3/watcher_test.go b/staging/src/k8s.io/apiserver/pkg/storage/etcd3/watcher_test.go index 8294778edd4..8fd68e45b11 100644 --- a/staging/src/k8s.io/apiserver/pkg/storage/etcd3/watcher_test.go +++ b/staging/src/k8s.io/apiserver/pkg/storage/etcd3/watcher_test.go @@ -136,7 +136,7 @@ func TestDeleteTriggerWatch(t *testing.T) { if err != nil { t.Fatalf("Watch failed: %v", err) } - if err := store.Delete(ctx, key, &example.Pod{}, nil, storage.ValidateAllObjectFunc); err != nil { + if err := store.Delete(ctx, key, &example.Pod{}, nil, storage.ValidateAllObjectFunc, nil); err != nil { t.Fatalf("Delete failed: %v", err) } testCheckEventType(t, watch.Deleted, w) @@ -296,7 +296,7 @@ func TestWatchDeleteEventObjectHaveLatestRV(t *testing.T) { } etcdW := cluster.RandClient().Watch(ctx, "/", clientv3.WithPrefix()) - if err := store.Delete(ctx, key, &example.Pod{}, &storage.Preconditions{}, storage.ValidateAllObjectFunc); err != nil { + if err := store.Delete(ctx, key, &example.Pod{}, &storage.Preconditions{}, storage.ValidateAllObjectFunc, nil); err != nil { t.Fatalf("Delete failed: %v", err) } diff --git a/staging/src/k8s.io/apiserver/pkg/storage/interfaces.go b/staging/src/k8s.io/apiserver/pkg/storage/interfaces.go index 01f9132f55d..7de3bf9363a 100644 --- a/staging/src/k8s.io/apiserver/pkg/storage/interfaces.go +++ b/staging/src/k8s.io/apiserver/pkg/storage/interfaces.go @@ -167,7 +167,12 @@ type Interface interface { // Delete removes the specified key and returns the value that existed at that spot. // If key didn't exist, it will return NotFound storage error. - Delete(ctx context.Context, key string, out runtime.Object, preconditions *Preconditions, validateDeletion ValidateObjectFunc) error + // If 'cachedExistingObject' is non-nil, it can be used as a suggestion about the + // current version of the object to avoid read operation from storage to get it. + // However, the implementations have to retry in case suggestion is stale. + Delete( + ctx context.Context, key string, out runtime.Object, preconditions *Preconditions, + validateDeletion ValidateObjectFunc, cachedExistingObject runtime.Object) error // Watch begins watching the specified key. Events are decoded into API objects, // and any items selected by 'p' are sent down to returned watch.Interface. diff --git a/staging/src/k8s.io/apiserver/pkg/storage/tests/cacher_test.go b/staging/src/k8s.io/apiserver/pkg/storage/tests/cacher_test.go index 7d6360a7d2d..b114997c209 100644 --- a/staging/src/k8s.io/apiserver/pkg/storage/tests/cacher_test.go +++ b/staging/src/k8s.io/apiserver/pkg/storage/tests/cacher_test.go @@ -283,7 +283,7 @@ func TestList(t *testing.T) { updatePod(t, etcdStorage, podFooNS2, nil) deleted := example.Pod{} - if err := etcdStorage.Delete(context.TODO(), "pods/ns/bar", &deleted, nil, storage.ValidateAllObjectFunc); err != nil { + if err := etcdStorage.Delete(context.TODO(), "pods/ns/bar", &deleted, nil, storage.ValidateAllObjectFunc, nil); err != nil { t.Errorf("Unexpected error: %v", err) } @@ -576,7 +576,7 @@ func TestFiltering(t *testing.T) { _ = updatePod(t, etcdStorage, podFooPrime, fooUnfiltered) deleted := example.Pod{} - if err := etcdStorage.Delete(context.TODO(), "pods/ns/foo", &deleted, nil, storage.ValidateAllObjectFunc); err != nil { + if err := etcdStorage.Delete(context.TODO(), "pods/ns/foo", &deleted, nil, storage.ValidateAllObjectFunc, nil); err != nil { t.Errorf("Unexpected error: %v", err) }