Skip to content

Commit 8f48d76

Browse files
ajatprabhaajatprabha
committed
add ResourceVersion to DeleteOptions.Preconditions
1 parent 9cbccd3 commit 8f48d76

File tree

8 files changed

+76
-4
lines changed

8 files changed

+76
-4
lines changed

‎pkg/registry/core/namespace/storage/storage.go‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -139,13 +139,21 @@ func (r *REST) Delete(ctx context.Context, name string, options *metav1.DeleteOp
139139
}
140140
if options.Preconditions.UID == nil {
141141
options.Preconditions.UID = &namespace.UID
142+
options.Preconditions.ResourceVersion = &namespace.ResourceVersion
142143
} else if *options.Preconditions.UID != namespace.UID {
143144
err = apierrors.NewConflict(
144145
api.Resource("namespaces"),
145146
name,
146147
fmt.Errorf("Precondition failed: UID in precondition: %v, UID in object meta: %v", *options.Preconditions.UID, namespace.UID),
147148
)
148149
return nil, false, err
150+
} else if *options.Preconditions.ResourceVersion != namespace.ResourceVersion {
151+
err = apierrors.NewConflict(
152+
api.Resource("namespaces"),
153+
name,
154+
fmt.Errorf("Precondition failed: ResourceVersion in precondition: %v, ResourceVersion in object meta: %v", *options.Preconditions.ResourceVersion, namespace.ResourceVersion),
155+
)
156+
return nil, false, err
149157
}
150158

151159
// 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
156164
return nil, false, err
157165
}
158166

159-
preconditions := storage.Preconditions{UID: options.Preconditions.UID}
167+
preconditions := storage.Preconditions{UID: options.Preconditions.UID, ResourceVersion: options.Preconditions.ResourceVersion}
160168

161169
out := r.store.NewFunc()
162170
err = r.store.Storage.GuaranteedUpdate(

‎staging/src/k8s.io/apiextensions-apiserver/pkg/registry/customresourcedefinition/etcd.go‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -84,13 +84,21 @@ func (r *REST) Delete(ctx context.Context, name string, options *metav1.DeleteOp
8484
}
8585
if options.Preconditions.UID == nil {
8686
options.Preconditions.UID = &crd.UID
87+
options.Preconditions.ResourceVersion = &crd.ResourceVersion
8788
} else if *options.Preconditions.UID != crd.UID {
8889
err = apierrors.NewConflict(
8990
apiextensions.Resource("customresourcedefinitions"),
9091
name,
9192
fmt.Errorf("Precondition failed: UID in precondition: %v, UID in object meta: %v", *options.Preconditions.UID, crd.UID),
9293
)
9394
return nil, false, err
95+
} else if *options.Preconditions.ResourceVersion != crd.ResourceVersion {
96+
err = apierrors.NewConflict(
97+
apiextensions.Resource("customresourcedefinitions"),
98+
name,
99+
fmt.Errorf("Precondition failed: ResourceVersion in precondition: %v, ResourceVersion in object meta: %v", *options.Preconditions.ResourceVersion, crd.ResourceVersion),
100+
)
101+
return nil, false, err
94102
}
95103

96104
// 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
100108
return nil, false, err
101109
}
102110

103-
preconditions := storage.Preconditions{UID: options.Preconditions.UID}
111+
preconditions := storage.Preconditions{UID: options.Preconditions.UID, ResourceVersion: options.Preconditions.ResourceVersion}
104112

105113
out := r.Store.NewFunc()
106114
err = r.Store.Storage.GuaranteedUpdate(

‎staging/src/k8s.io/apimachinery/pkg/apis/meta/v1/helpers.go‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -228,6 +228,12 @@ func NewUIDPreconditions(uid string) *Preconditions {
228228
return &Preconditions{UID: &u}
229229
}
230230

231+
// NewPreconditionDeleteOptionsWithResourceVersion returns a DeleteOptions with a UID precondition set.
232+
func NewPreconditionDeleteOptionsWithResourceVersion(rv string) *DeleteOptions {
233+
p := Preconditions{ResourceVersion: &rv}
234+
return &DeleteOptions{Preconditions: &p}
235+
}
236+
231237
// HasObjectMetaSystemFieldValues returns true if fields that are managed by the system on ObjectMeta have values.
232238
func HasObjectMetaSystemFieldValues(meta Object) bool {
233239
return !meta.GetCreationTimestamp().Time.IsZero() ||

‎staging/src/k8s.io/apimachinery/pkg/apis/meta/v1/types.go‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -575,6 +575,9 @@ type Preconditions struct {
575575
// Specifies the target UID.
576576
// +optional
577577
UID *types.UID `json:"uid,omitempty" protobuf:"bytes,1,opt,name=uid,casttype=k8s.io/apimachinery/pkg/types.UID"`
578+
// Specifies the target ResourceVersion
579+
// +optional
580+
ResourceVersion *string `json:"resourceVersion,omitempty" protobuf:"bytes,2,opt,name=resourceVersion"`
578581
}
579582

580583
// +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object

‎staging/src/k8s.io/apiserver/pkg/registry/generic/registry/store.go‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -448,6 +448,7 @@ func (e *Store) Update(ctx context.Context, name string, objInfo rest.UpdatedObj
448448
storagePreconditions := &storage.Preconditions{}
449449
if preconditions := objInfo.Preconditions(); preconditions != nil {
450450
storagePreconditions.UID = preconditions.UID
451+
storagePreconditions.ResourceVersion = preconditions.ResourceVersion
451452
}
452453

453454
out := e.NewFunc()
@@ -879,6 +880,7 @@ func (e *Store) Delete(ctx context.Context, name string, options *metav1.DeleteO
879880
var preconditions storage.Preconditions
880881
if options.Preconditions != nil {
881882
preconditions.UID = options.Preconditions.UID
883+
preconditions.ResourceVersion = options.Preconditions.ResourceVersion
882884
}
883885
graceful, pendingGraceful, err := rest.BeforeDelete(e.DeleteStrategy, ctx, obj, options)
884886
if err != nil {

‎staging/src/k8s.io/apiserver/pkg/registry/rest/delete.go‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -77,8 +77,13 @@ func BeforeDelete(strategy RESTDeleteStrategy, ctx context.Context, obj runtime.
7777
return false, false, errors.NewInvalid(schema.GroupKind{Group: metav1.GroupName, Kind: "DeleteOptions"}, "", errs)
7878
}
7979
// Checking the Preconditions here to fail early. They'll be enforced later on when we actually do the deletion, too.
80-
if options.Preconditions != nil && options.Preconditions.UID != nil && *options.Preconditions.UID != objectMeta.GetUID() {
81-
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()))
80+
if options.Preconditions != nil {
81+
if options.Preconditions.UID != nil && *options.Preconditions.UID != objectMeta.GetUID() {
82+
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()))
83+
}
84+
if options.Preconditions.ResourceVersion != nil && *options.Preconditions.ResourceVersion != objectMeta.GetResourceVersion() {
85+
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()))
86+
}
8287
}
8388
gracefulStrategy, ok := strategy.(RESTGracefulDeleteStrategy)
8489
if !ok {

‎staging/src/k8s.io/apiserver/pkg/registry/rest/resttest/resttest.go‎

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -909,6 +909,43 @@ func (t *Tester) testDeleteWithUID(obj runtime.Object, createFn CreateFunc, getF
909909
}
910910
}
911911

912+
// This test the fast-fail path. We test that the precondition gets verified
913+
// again before deleting the object in tests of pkg/storage/etcd.
914+
func (t *Tester) testDeleteWithResourceVersion(obj runtime.Object, createFn CreateFunc, getFn GetFunc, isNotFoundFn IsErrorFunc, opts metav1.DeleteOptions) {
915+
ctx := t.TestContext()
916+
917+
foo := obj.DeepCopyObject()
918+
t.setObjectMeta(foo, t.namer(1))
919+
objectMeta := t.getObjectMetaOrFail(foo)
920+
objectMeta.SetResourceVersion("RV0000")
921+
if err := createFn(ctx, foo); err != nil {
922+
t.Errorf("unexpected error: %v", err)
923+
}
924+
opts.Preconditions = metav1.NewPreconditionDeleteOptionsWithResourceVersion("RV1111").Preconditions
925+
obj, _, err := t.storage.(rest.GracefulDeleter).Delete(ctx, objectMeta.GetName(), &opts)
926+
if err == nil || !errors.IsConflict(err) {
927+
t.Errorf("unexpected error: %v", err)
928+
}
929+
930+
obj, _, err = t.storage.(rest.GracefulDeleter).Delete(ctx, objectMeta.GetName(), metav1.NewPreconditionDeleteOptionsWithResourceVersion("RV0000"))
931+
if err != nil {
932+
t.Errorf("unexpected error: %v", err)
933+
}
934+
935+
if !t.returnDeletedObject {
936+
if status, ok := obj.(*metav1.Status); !ok {
937+
t.Errorf("expected status of delete, got %v", status)
938+
} else if status.Status != metav1.StatusSuccess {
939+
t.Errorf("expected success, got: %v", status.Status)
940+
}
941+
}
942+
943+
_, err = getFn(ctx, foo)
944+
if err == nil || !isNotFoundFn(err) {
945+
t.Errorf("unexpected error: %v", err)
946+
}
947+
}
948+
912949
// =============================================================================
913950
// Graceful Deletion tests.
914951

‎staging/src/k8s.io/apiserver/pkg/storage/interfaces.go‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -98,6 +98,9 @@ type Preconditions struct {
9898
// Specifies the target UID.
9999
// +optional
100100
UID *types.UID `json:"uid,omitempty"`
101+
// Specifies the target ResourceVersion
102+
// +optional
103+
ResourceVersion *string `json:"resourceVersion,omitempty"`
101104
}
102105

103106
// NewUIDPreconditions returns a Preconditions with UID set.

0 commit comments

Comments
 (0)