Merge pull request #27539 from gmarek/graceful

Automatic merge from submit-queue

Fix logic of consecutive DELETE calls when gracefull deletion is enabled

Previously it was possible to extend delete timestamp to infinity by issuing repeated DELETE calls. This PR prevents that. Ref. discussion in #27422

cc @wojtek-t @smarterclayton @lavalamp @piosz @dchen1107
This commit is contained in:
k8s-merge-robot 2016-07-05 02:16:41 -07:00 committed by GitHub
commit b37a5ded3c
2 changed files with 47 additions and 4 deletions

View File

@ -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

View File

@ -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.