diff --git a/pkg/registry/core/namespace/storage/storage.go b/pkg/registry/core/namespace/storage/storage.go index 2fe25f1ee5..82f54b72a1 100644 --- a/pkg/registry/core/namespace/storage/storage.go +++ b/pkg/registry/core/namespace/storage/storage.go @@ -139,6 +139,7 @@ func (r *REST) Delete(ctx context.Context, name string, options *metav1.DeleteOp } if options.Preconditions.UID == nil { options.Preconditions.UID = &namespace.UID + options.Preconditions.ResourceVersion = &namespace.ResourceVersion } else if *options.Preconditions.UID != namespace.UID { err = apierrors.NewConflict( api.Resource("namespaces"), @@ -146,6 +147,13 @@ func (r *REST) Delete(ctx context.Context, name string, options *metav1.DeleteOp fmt.Errorf("Precondition failed: UID in precondition: %v, UID in object meta: %v", *options.Preconditions.UID, namespace.UID), ) return nil, false, err + } else if *options.Preconditions.ResourceVersion != namespace.ResourceVersion { + err = apierrors.NewConflict( + api.Resource("namespaces"), + name, + fmt.Errorf("Precondition failed: ResourceVersion in precondition: %v, ResourceVersion in object meta: %v", *options.Preconditions.ResourceVersion, namespace.ResourceVersion), + ) + return nil, false, err } // upon first request to delete, we switch the phase to start namespace termination @@ -156,7 +164,7 @@ func (r *REST) Delete(ctx context.Context, name string, options *metav1.DeleteOp return nil, false, err } - preconditions := storage.Preconditions{UID: options.Preconditions.UID} + preconditions := storage.Preconditions{UID: options.Preconditions.UID, ResourceVersion: options.Preconditions.ResourceVersion} out := r.store.NewFunc() err = r.store.Storage.GuaranteedUpdate( diff --git a/staging/src/k8s.io/apiextensions-apiserver/pkg/registry/customresourcedefinition/etcd.go b/staging/src/k8s.io/apiextensions-apiserver/pkg/registry/customresourcedefinition/etcd.go index c6b50915f1..fe7c3bafd6 100644 --- a/staging/src/k8s.io/apiextensions-apiserver/pkg/registry/customresourcedefinition/etcd.go +++ b/staging/src/k8s.io/apiextensions-apiserver/pkg/registry/customresourcedefinition/etcd.go @@ -84,6 +84,7 @@ func (r *REST) Delete(ctx context.Context, name string, options *metav1.DeleteOp } if options.Preconditions.UID == nil { options.Preconditions.UID = &crd.UID + options.Preconditions.ResourceVersion = &crd.ResourceVersion } else if *options.Preconditions.UID != crd.UID { err = apierrors.NewConflict( apiextensions.Resource("customresourcedefinitions"), @@ -91,6 +92,13 @@ func (r *REST) Delete(ctx context.Context, name string, options *metav1.DeleteOp fmt.Errorf("Precondition failed: UID in precondition: %v, UID in object meta: %v", *options.Preconditions.UID, crd.UID), ) return nil, false, err + } else if *options.Preconditions.ResourceVersion != crd.ResourceVersion { + err = apierrors.NewConflict( + apiextensions.Resource("customresourcedefinitions"), + name, + fmt.Errorf("Precondition failed: ResourceVersion in precondition: %v, ResourceVersion in object meta: %v", *options.Preconditions.ResourceVersion, crd.ResourceVersion), + ) + return nil, false, err } // upon first request to delete, add our finalizer and then delegate @@ -100,7 +108,7 @@ func (r *REST) Delete(ctx context.Context, name string, options *metav1.DeleteOp return nil, false, err } - preconditions := storage.Preconditions{UID: options.Preconditions.UID} + preconditions := storage.Preconditions{UID: options.Preconditions.UID, ResourceVersion: options.Preconditions.ResourceVersion} out := r.Store.NewFunc() err = r.Store.Storage.GuaranteedUpdate( diff --git a/staging/src/k8s.io/apimachinery/pkg/apis/meta/v1/helpers.go b/staging/src/k8s.io/apimachinery/pkg/apis/meta/v1/helpers.go index b41d549a25..9da8008331 100644 --- a/staging/src/k8s.io/apimachinery/pkg/apis/meta/v1/helpers.go +++ b/staging/src/k8s.io/apimachinery/pkg/apis/meta/v1/helpers.go @@ -228,6 +228,12 @@ func NewUIDPreconditions(uid string) *Preconditions { return &Preconditions{UID: &u} } +// NewPreconditionDeleteOptionsWithResourceVersion returns a DeleteOptions with a UID precondition set. +func NewPreconditionDeleteOptionsWithResourceVersion(rv string) *DeleteOptions { + p := Preconditions{ResourceVersion: &rv} + return &DeleteOptions{Preconditions: &p} +} + // HasObjectMetaSystemFieldValues returns true if fields that are managed by the system on ObjectMeta have values. func HasObjectMetaSystemFieldValues(meta Object) bool { return !meta.GetCreationTimestamp().Time.IsZero() || diff --git a/staging/src/k8s.io/apimachinery/pkg/apis/meta/v1/types.go b/staging/src/k8s.io/apimachinery/pkg/apis/meta/v1/types.go index 23cbc2c6df..fd6395256a 100644 --- a/staging/src/k8s.io/apimachinery/pkg/apis/meta/v1/types.go +++ b/staging/src/k8s.io/apimachinery/pkg/apis/meta/v1/types.go @@ -575,6 +575,9 @@ type Preconditions struct { // Specifies the target UID. // +optional UID *types.UID `json:"uid,omitempty" protobuf:"bytes,1,opt,name=uid,casttype=k8s.io/apimachinery/pkg/types.UID"` + // Specifies the target ResourceVersion + // +optional + ResourceVersion *string `json:"resourceVersion,omitempty" protobuf:"bytes,2,opt,name=resourceVersion"` } // +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object 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 c84a3276c4..be15e2bb3b 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 @@ -448,6 +448,7 @@ func (e *Store) Update(ctx context.Context, name string, objInfo rest.UpdatedObj storagePreconditions := &storage.Preconditions{} if preconditions := objInfo.Preconditions(); preconditions != nil { storagePreconditions.UID = preconditions.UID + storagePreconditions.ResourceVersion = preconditions.ResourceVersion } out := e.NewFunc() @@ -879,6 +880,7 @@ func (e *Store) Delete(ctx context.Context, name string, options *metav1.DeleteO var preconditions storage.Preconditions if options.Preconditions != nil { preconditions.UID = options.Preconditions.UID + preconditions.ResourceVersion = options.Preconditions.ResourceVersion } graceful, pendingGraceful, err := rest.BeforeDelete(e.DeleteStrategy, ctx, obj, options) if err != nil { 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 7c39f6be10..0713464e00 100644 --- a/staging/src/k8s.io/apiserver/pkg/registry/rest/delete.go +++ b/staging/src/k8s.io/apiserver/pkg/registry/rest/delete.go @@ -77,8 +77,13 @@ func BeforeDelete(strategy RESTDeleteStrategy, ctx context.Context, obj runtime. return false, false, errors.NewInvalid(schema.GroupKind{Group: metav1.GroupName, Kind: "DeleteOptions"}, "", errs) } // Checking the Preconditions here to fail early. They'll be enforced later on when we actually do the deletion, too. - if options.Preconditions != nil && options.Preconditions.UID != nil && *options.Preconditions.UID != objectMeta.GetUID() { - return false, false, errors.NewConflict(schema.GroupResource{Group: gvk.Group, Resource: gvk.Kind}, objectMeta.GetName(), fmt.Errorf("the UID in the precondition (%s) does not match the UID in record (%s). The object might have been deleted and then recreated", *options.Preconditions.UID, objectMeta.GetUID())) + if options.Preconditions != nil { + if options.Preconditions.UID != nil && *options.Preconditions.UID != objectMeta.GetUID() { + return false, false, errors.NewConflict(schema.GroupResource{Group: gvk.Group, Resource: gvk.Kind}, objectMeta.GetName(), fmt.Errorf("the UID in the precondition (%s) does not match the UID in record (%s). The object might have been deleted and then recreated", *options.Preconditions.UID, objectMeta.GetUID())) + } + if options.Preconditions.ResourceVersion != nil && *options.Preconditions.ResourceVersion != objectMeta.GetResourceVersion() { + return false, false, errors.NewConflict(schema.GroupResource{Group: gvk.Group, Resource: gvk.Kind}, objectMeta.GetName(), fmt.Errorf("the ResourceVersion in the precondition (%s) does not match the ResourceVersion in record (%s). The object might have been deleted and then recreated", *options.Preconditions.ResourceVersion, objectMeta.GetResourceVersion())) + } } gracefulStrategy, ok := strategy.(RESTGracefulDeleteStrategy) if !ok { 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 8e6b555426..4316592f46 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 @@ -909,6 +909,43 @@ func (t *Tester) testDeleteWithUID(obj runtime.Object, createFn CreateFunc, getF } } +// This test the fast-fail path. We test that the precondition gets verified +// again before deleting the object in tests of pkg/storage/etcd. +func (t *Tester) testDeleteWithResourceVersion(obj runtime.Object, createFn CreateFunc, getFn GetFunc, isNotFoundFn IsErrorFunc, opts metav1.DeleteOptions) { + ctx := t.TestContext() + + foo := obj.DeepCopyObject() + t.setObjectMeta(foo, t.namer(1)) + objectMeta := t.getObjectMetaOrFail(foo) + objectMeta.SetResourceVersion("RV0000") + if err := createFn(ctx, foo); err != nil { + t.Errorf("unexpected error: %v", err) + } + opts.Preconditions = metav1.NewPreconditionDeleteOptionsWithResourceVersion("RV1111").Preconditions + obj, _, err := t.storage.(rest.GracefulDeleter).Delete(ctx, objectMeta.GetName(), &opts) + if err == nil || !errors.IsConflict(err) { + t.Errorf("unexpected error: %v", err) + } + + obj, _, err = t.storage.(rest.GracefulDeleter).Delete(ctx, objectMeta.GetName(), metav1.NewPreconditionDeleteOptionsWithResourceVersion("RV0000")) + if err != nil { + t.Errorf("unexpected error: %v", err) + } + + if !t.returnDeletedObject { + if status, ok := obj.(*metav1.Status); !ok { + t.Errorf("expected status of delete, got %v", status) + } else if status.Status != metav1.StatusSuccess { + t.Errorf("expected success, got: %v", status.Status) + } + } + + _, err = getFn(ctx, foo) + if err == nil || !isNotFoundFn(err) { + t.Errorf("unexpected error: %v", err) + } +} + // ============================================================================= // Graceful Deletion tests. diff --git a/staging/src/k8s.io/apiserver/pkg/storage/interfaces.go b/staging/src/k8s.io/apiserver/pkg/storage/interfaces.go index 56dd6dbdc9..93b3f5deb8 100644 --- a/staging/src/k8s.io/apiserver/pkg/storage/interfaces.go +++ b/staging/src/k8s.io/apiserver/pkg/storage/interfaces.go @@ -98,6 +98,9 @@ type Preconditions struct { // Specifies the target UID. // +optional UID *types.UID `json:"uid,omitempty"` + // Specifies the target ResourceVersion + // +optional + ResourceVersion *string `json:"resourceVersion,omitempty"` } // NewUIDPreconditions returns a Preconditions with UID set.