From 2a5914d61c3eadb84ec54e339ddee9ac7a505a51 Mon Sep 17 00:00:00 2001 From: gmarek Date: Thu, 16 Jun 2016 15:58:54 +0200 Subject: [PATCH] Fix logic of consecutive DELETE calls when gracefull deletion is enabled --- pkg/api/rest/delete.go | 9 ++++--- pkg/api/rest/resttest/resttest.go | 42 +++++++++++++++++++++++++++++++ 2 files changed, 47 insertions(+), 4 deletions(-) diff --git a/pkg/api/rest/delete.go b/pkg/api/rest/delete.go index b6ddf86242b..6967acbd104 100644 --- a/pkg/api/rest/delete.go +++ b/pkg/api/rest/delete.go @@ -72,13 +72,14 @@ func BeforeDelete(strategy RESTDeleteStrategy, ctx api.Context, obj runtime.Obje // only a shorter grace period may be provided by a user if options.GracePeriodSeconds != nil { period := int64(*options.GracePeriodSeconds) - if period > *objectMeta.DeletionGracePeriodSeconds { + if period >= *objectMeta.DeletionGracePeriodSeconds { return false, true, nil } - now := unversioned.NewTime(unversioned.Now().Add(time.Second * time.Duration(*options.GracePeriodSeconds))) - objectMeta.DeletionTimestamp = &now + newDeletionTimestamp := unversioned.NewTime( + objectMeta.DeletionTimestamp.Add(-time.Second * time.Duration(*objectMeta.DeletionGracePeriodSeconds)). + Add(time.Second * time.Duration(*options.GracePeriodSeconds))) + objectMeta.DeletionTimestamp = &newDeletionTimestamp objectMeta.DeletionGracePeriodSeconds = &period - options.GracePeriodSeconds = &period return true, false, nil } // graceful deletion is pending, do nothing diff --git a/pkg/api/rest/resttest/resttest.go b/pkg/api/rest/resttest/resttest.go index ade5d7170a2..9c71f1a6f7d 100644 --- a/pkg/api/rest/resttest/resttest.go +++ b/pkg/api/rest/resttest/resttest.go @@ -188,6 +188,7 @@ func (t *Tester) TestDeleteGraceful(valid runtime.Object, createFn CreateFunc, g t.testDeleteGracefulWithValue(copyOrDie(valid), createFn, getFn, expectedGrace) t.testDeleteGracefulUsesZeroOnNil(copyOrDie(valid), createFn, expectedGrace) t.testDeleteGracefulExtend(copyOrDie(valid), createFn, getFn, expectedGrace) + t.testDeleteGracefulShorten(copyOrDie(valid), createFn, getFn, expectedGrace) t.testDeleteGracefulImmediate(copyOrDie(valid), createFn, getFn, expectedGrace) } @@ -887,6 +888,47 @@ func (t *Tester) testDeleteGracefulUsesZeroOnNil(obj runtime.Object, createFn Cr } } +// Regression test for bug discussed in #27539 +func (t *Tester) testDeleteGracefulShorten(obj runtime.Object, createFn CreateFunc, getFn GetFunc, expectedGrace int64) { + ctx := t.TestContext() + + foo := copyOrDie(obj) + t.setObjectMeta(foo, t.namer(6)) + if err := createFn(ctx, foo); err != nil { + t.Errorf("unexpected error: %v", err) + } + bigGrace := int64(time.Hour) + if expectedGrace > bigGrace { + bigGrace = 2 * expectedGrace + } + objectMeta := t.getObjectMetaOrFail(foo) + _, err := t.storage.(rest.GracefulDeleter).Delete(ctx, objectMeta.Name, api.NewDeleteOptions(bigGrace)) + if err != nil { + t.Errorf("unexpected error: %v", err) + } + object, err := getFn(ctx, foo) + if err != nil { + t.Fatalf("did not gracefully delete resource: %v", err) + } + objectMeta = t.getObjectMetaOrFail(object) + deletionTimestamp := *objectMeta.DeletionTimestamp + + // second delete duration is ignored + _, err = t.storage.(rest.GracefulDeleter).Delete(ctx, objectMeta.Name, api.NewDeleteOptions(expectedGrace)) + if err != nil { + t.Errorf("unexpected error: %v", err) + } + object, err = t.storage.(rest.Getter).Get(ctx, objectMeta.Name) + if err != nil { + t.Errorf("unexpected error, object should exist: %v", err) + } + objectMeta = t.getObjectMetaOrFail(object) + if objectMeta.DeletionTimestamp == nil || objectMeta.DeletionGracePeriodSeconds == nil || + *objectMeta.DeletionGracePeriodSeconds != expectedGrace || !objectMeta.DeletionTimestamp.Before(deletionTimestamp) { + t.Errorf("unexpected deleted meta: %+v", objectMeta) + } +} + // ============================================================================= // Get tests.