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 4ff379aef5d..7a626855a18 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 @@ -982,6 +982,7 @@ func (e *Store) updateForGracefulDeletionAndFinalizers(ctx context.Context, name } // Delete removes the item from storage. +// options can be mutated by rest.BeforeDelete due to a graceful deletion strategy. func (e *Store) Delete(ctx context.Context, name string, deleteValidation rest.ValidateObjectFunc, options *metav1.DeleteOptions) (runtime.Object, bool, error) { key, err := e.KeyFunc(ctx, name) if err != nil { @@ -1148,7 +1149,11 @@ func (e *Store) DeleteCollection(ctx context.Context, deleteValidation rest.Vali errs <- err return } - if _, _, err := e.Delete(ctx, accessor.GetName(), deleteValidation, options); err != nil && !apierrors.IsNotFound(err) { + // DeepCopy the deletion options because individual graceful deleters communicate changes via a mutating + // function in the delete strategy called in the delete method. While that is always ugly, it works + // when making a single call. When making multiple calls via delete collection, the mutation applied to + // pod/A can change the option ultimately used for pod/B. + if _, _, err := e.Delete(ctx, accessor.GetName(), deleteValidation, options.DeepCopy()); err != nil && !apierrors.IsNotFound(err) { klog.V(4).InfoS("Delete object in DeleteCollection failed", "object", klog.KObj(accessor), "err", err) errs <- err return diff --git a/staging/src/k8s.io/apiserver/pkg/registry/generic/registry/store_test.go b/staging/src/k8s.io/apiserver/pkg/registry/generic/registry/store_test.go index c6a9a543c8b..74d57b1859c 100644 --- a/staging/src/k8s.io/apiserver/pkg/registry/generic/registry/store_test.go +++ b/staging/src/k8s.io/apiserver/pkg/registry/generic/registry/store_test.go @@ -86,6 +86,16 @@ func (t *testOrphanDeleteStrategy) DefaultGarbageCollectionPolicy(ctx context.Co return rest.OrphanDependents } +type mutatingDeleteRESTStrategy struct { + runtime.ObjectTyper +} + +func (t *mutatingDeleteRESTStrategy) CheckGracefulDelete(ctx context.Context, obj runtime.Object, options *metav1.DeleteOptions) bool { + n := int64(10) + options.GracePeriodSeconds = &n + return true +} + type testRESTStrategy struct { runtime.ObjectTyper names.NameGenerator @@ -2041,6 +2051,36 @@ func TestStoreDeleteCollection(t *testing.T) { } } +func TestStoreDeleteCollectionNoMutateOptions(t *testing.T) { + podA := &example.Pod{ObjectMeta: metav1.ObjectMeta{Name: "foo"}} + podB := &example.Pod{ObjectMeta: metav1.ObjectMeta{Name: "bar"}} + + testContext := genericapirequest.WithNamespace(genericapirequest.NewContext(), "test") + destroyFunc, registry := NewTestGenericStoreRegistry(t) + registry.DeleteStrategy = &mutatingDeleteRESTStrategy{scheme} + defer destroyFunc() + + if _, err := registry.Create(testContext, podA, rest.ValidateAllObjectFunc, &metav1.CreateOptions{}); err != nil { + t.Errorf("Unexpected error: %v", err) + } + if _, err := registry.Create(testContext, podB, rest.ValidateAllObjectFunc, &metav1.CreateOptions{}); err != nil { + t.Errorf("Unexpected error: %v", err) + } + + n := int64(50) + inputDeleteOptions := &metav1.DeleteOptions{GracePeriodSeconds: &n} + safeCopyOfDelete := inputDeleteOptions.DeepCopy() + // Delete all pods. + _, err := registry.DeleteCollection(testContext, rest.ValidateAllObjectFunc, inputDeleteOptions, &metainternalversion.ListOptions{}) + if err != nil { + t.Fatalf("Unexpected error: %v", err) + } + + if !reflect.DeepEqual(inputDeleteOptions, safeCopyOfDelete) { + t.Error(inputDeleteOptions) + } +} + func TestStoreDeleteCollectionNotFound(t *testing.T) { destroyFunc, registry := NewTestGenericStoreRegistry(t) defer destroyFunc() diff --git a/test/integration/scheduler/util.go b/test/integration/scheduler/util.go index 8fe19922941..da6d036ffa8 100644 --- a/test/integration/scheduler/util.go +++ b/test/integration/scheduler/util.go @@ -474,9 +474,12 @@ func noPodsInNamespace(c clientset.Interface, podNamespace string) wait.Conditio } // cleanupPodsInNamespace deletes the pods in the given namespace and waits for them to -// be actually deleted. +// be actually deleted. They are removed with no grace. func cleanupPodsInNamespace(cs clientset.Interface, t *testing.T, ns string) { - if err := cs.CoreV1().Pods(ns).DeleteCollection(context.TODO(), metav1.DeleteOptions{}, metav1.ListOptions{}); err != nil { + t.Helper() + + zero := int64(0) + if err := cs.CoreV1().Pods(ns).DeleteCollection(context.TODO(), metav1.DeleteOptions{GracePeriodSeconds: &zero}, metav1.ListOptions{}); err != nil { t.Errorf("error while listing pod in namespace %v: %v", ns, err) return }