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 34102690e86..a7e788ba3df 100644 --- a/staging/src/k8s.io/apiserver/pkg/registry/rest/delete.go +++ b/staging/src/k8s.io/apiserver/pkg/registry/rest/delete.go @@ -126,9 +126,22 @@ func BeforeDelete(strategy RESTDeleteStrategy, ctx context.Context, obj runtime. if period >= *objectMeta.GetDeletionGracePeriodSeconds() { return false, true, nil } + // Move the existing deletionTimestamp back by existing object.DeletionGracePeriod, then forward by options.DeletionGracePeriod. + // This moves the deletionTimestamp back, since the grace period can only be shortened in this code path. newDeletionTimestamp := metav1.NewTime( objectMeta.GetDeletionTimestamp().Add(-time.Second * time.Duration(*objectMeta.GetDeletionGracePeriodSeconds())). Add(time.Second * time.Duration(*options.GracePeriodSeconds))) + // Prevent shortening the grace period moving deletionTimestamp into the past + if now := metav1Now(); newDeletionTimestamp.Before(&now) { + newDeletionTimestamp = now + if period != 0 { + // Since a graceful deletion was requested (options.GracePeriodSeconds != 0), but the entire grace period has already expired, + // shorten to the minimum period possible while still treating this as a graceful delete. + // This means the API server updates the object, another actor observes the update + // and is still responsible for the final delete with options.GracePeriodSeconds == 0. + period = int64(1) + } + } objectMeta.SetDeletionTimestamp(&newDeletionTimestamp) objectMeta.SetDeletionGracePeriodSeconds(&period) return true, false, nil @@ -147,8 +160,8 @@ func BeforeDelete(strategy RESTDeleteStrategy, ctx context.Context, obj runtime. return false, false, errors.NewInternalError(fmt.Errorf("options.GracePeriodSeconds should not be nil")) } - now := metav1.NewTime(metav1.Now().Add(time.Second * time.Duration(*options.GracePeriodSeconds))) - objectMeta.SetDeletionTimestamp(&now) + requestedDeletionTimestamp := metav1.NewTime(metav1Now().Add(time.Second * time.Duration(*options.GracePeriodSeconds))) + objectMeta.SetDeletionTimestamp(&requestedDeletionTimestamp) objectMeta.SetDeletionGracePeriodSeconds(options.GracePeriodSeconds) // If it's the first graceful deletion we are going to set the DeletionTimestamp to non-nil. // Controllers of the object that's being deleted shouldn't take any nontrivial actions, hence its behavior changes. diff --git a/staging/src/k8s.io/apiserver/pkg/registry/rest/delete_test.go b/staging/src/k8s.io/apiserver/pkg/registry/rest/delete_test.go index 2aaf2501fe5..770a0ce62aa 100644 --- a/staging/src/k8s.io/apiserver/pkg/registry/rest/delete_test.go +++ b/staging/src/k8s.io/apiserver/pkg/registry/rest/delete_test.go @@ -18,7 +18,9 @@ package rest import ( "context" + "reflect" "testing" + "time" v1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -52,15 +54,32 @@ func TestBeforeDelete(t *testing.T) { options *metav1.DeleteOptions } - makePod := func(deletionGracePeriodSeconds int64) *v1.Pod { + // snapshot and restore real metav1Now function + originalMetav1Now := metav1Now + t.Cleanup(func() { + metav1Now = originalMetav1Now + }) + + // make now refer to a fixed point in time + now := metav1.Time{Time: time.Now().Truncate(time.Second)} + metav1Now = func() metav1.Time { + return now + } + + makePodWithDeletionTimestamp := func(deletionTimestamp *metav1.Time, deletionGracePeriodSeconds int64) *v1.Pod { return &v1.Pod{ TypeMeta: metav1.TypeMeta{APIVersion: "v1", Kind: "Pod"}, ObjectMeta: metav1.ObjectMeta{ - DeletionTimestamp: &metav1.Time{}, + DeletionTimestamp: deletionTimestamp, DeletionGracePeriodSeconds: &deletionGracePeriodSeconds, }, } } + makePod := func(deletionGracePeriodSeconds int64) *v1.Pod { + deletionTimestamp := now + deletionTimestamp.Time = deletionTimestamp.Time.Add(time.Duration(deletionGracePeriodSeconds) * time.Second) + return makePodWithDeletionTimestamp(&deletionTimestamp, deletionGracePeriodSeconds) + } makeOption := func(gracePeriodSeconds int64) *metav1.DeleteOptions { return &metav1.DeleteOptions{ GracePeriodSeconds: &gracePeriodSeconds, @@ -74,6 +93,7 @@ func TestBeforeDelete(t *testing.T) { wantGracefulPending bool wantGracePeriodSeconds *int64 wantDeletionGracePeriodSeconds *int64 + wantDeletionTimestamp *metav1.Time wantErr bool }{ { @@ -271,6 +291,28 @@ func TestBeforeDelete(t *testing.T) { wantGraceful: false, wantGracefulPending: true, }, + { + name: "when a shorter non-zero grace period would move into the past", + args: args{ + pod: makePodWithDeletionTimestamp(&metav1.Time{Time: now.Time.Add(-time.Minute)}, 60), + options: makeOption(50), + }, + wantDeletionTimestamp: &now, + wantDeletionGracePeriodSeconds: utilpointer.Int64(1), + wantGracePeriodSeconds: utilpointer.Int64(50), + wantGraceful: true, + }, + { + name: "when a zero grace period would move into the past", + args: args{ + pod: makePodWithDeletionTimestamp(&metav1.Time{Time: now.Time.Add(-time.Minute)}, 60), + options: makeOption(0), + }, + wantDeletionTimestamp: &now, + wantDeletionGracePeriodSeconds: utilpointer.Int64(0), + wantGracePeriodSeconds: utilpointer.Int64(0), + wantGraceful: true, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -301,6 +343,11 @@ func TestBeforeDelete(t *testing.T) { if !utilpointer.Int64Equal(tt.args.options.GracePeriodSeconds, tt.wantGracePeriodSeconds) { t.Errorf("options.GracePeriodSeconds = %v, want %v", ptr.Deref(tt.args.options.GracePeriodSeconds, 0), ptr.Deref(tt.wantGracePeriodSeconds, 0)) } + if tt.wantDeletionTimestamp != nil { + if !reflect.DeepEqual(tt.args.pod.DeletionTimestamp, tt.wantDeletionTimestamp) { + t.Errorf("pod.deletionTimestamp = %v, want %v", tt.args.pod.DeletionTimestamp, tt.wantDeletionTimestamp) + } + } }) } } diff --git a/staging/src/k8s.io/apiserver/pkg/registry/rest/meta.go b/staging/src/k8s.io/apiserver/pkg/registry/rest/meta.go index 12c1e5f98c1..fc4fc81e138 100644 --- a/staging/src/k8s.io/apiserver/pkg/registry/rest/meta.go +++ b/staging/src/k8s.io/apiserver/pkg/registry/rest/meta.go @@ -23,6 +23,9 @@ import ( "k8s.io/apimachinery/pkg/util/uuid" ) +// metav1Now returns metav1.Now(), but allows override for unit testing +var metav1Now = func() metav1.Time { return metav1.Now() } + // WipeObjectMetaSystemFields erases fields that are managed by the system on ObjectMeta. func WipeObjectMetaSystemFields(meta metav1.Object) { meta.SetCreationTimestamp(metav1.Time{}) @@ -34,7 +37,7 @@ func WipeObjectMetaSystemFields(meta metav1.Object) { // FillObjectMetaSystemFields populates fields that are managed by the system on ObjectMeta. func FillObjectMetaSystemFields(meta metav1.Object) { - meta.SetCreationTimestamp(metav1.Now()) + meta.SetCreationTimestamp(metav1Now()) meta.SetUID(uuid.NewUUID()) }