Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Enable finalizers independent of GC enablement #51469

Merged
merged 1 commit into from Aug 29, 2017
Merged

Enable finalizers independent of GC enablement #51469

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
105 changes: 19 additions & 86 deletions staging/src/k8s.io/apiserver/pkg/registry/generic/registry/store.go
View file
Open in desktop
Expand Up @@ -403,12 +403,8 @@ func (e *Store) WaitForInitialized(ctx genericapirequest.Context, obj runtime.Ob

// shouldDeleteDuringUpdate checks if a Update is removing all the object's
// finalizers. If so, it further checks if the object's
// DeletionGracePeriodSeconds is 0. If so, it returns true. If garbage collection
// is disabled it always returns false.
// DeletionGracePeriodSeconds is 0. If so, it returns true.
func (e *Store) shouldDeleteDuringUpdate(ctx genericapirequest.Context, key string, obj, existing runtime.Object) bool {
if !e.EnableGarbageCollection {
return false
}
newMeta, err := meta.Accessor(obj)
if err != nil {
utilruntime.HandleError(err)
Expand Down Expand Up @@ -719,9 +715,18 @@ func shouldUpdateFinalizerDeleteDependents(e *Store, accessor metav1.Object, opt
return shouldUpdate, shouldDeleteDependentInForeground
}

// shouldUpdateFinalizers returns if we need to update the finalizers of the
// object, and the desired list of finalizers.
func shouldUpdateFinalizers(e *Store, accessor metav1.Object, options *metav1.DeleteOptions) (shouldUpdate bool, newFinalizers []string) {
// deletionFinalizersForGarbageCollection analyzes the object and delete options
// to determine whether the object is in need of finalization by the garbage
// collector. If so, returns the set of deletion finalizers to apply and a bool
// indicating whether the finalizer list has changed and is in need of updating.
//
// The finalizers returned are intended to be handled by the garbage collector.
// If garbage collection is disabled for the store, this function returns false
// to ensure finalizers aren't set which will never be cleared.
func deletionFinalizersForGarbageCollection(e *Store, accessor metav1.Object, options *metav1.DeleteOptions) (shouldUpdate bool, newFinalizers []string) {
if !e.EnableGarbageCollection {
return false, []string{}
}
shouldUpdate1, shouldOrphan := shouldUpdateFinalizerOrphanDependents(e, accessor, options)
shouldUpdate2, shouldDeleteDependentInForeground := shouldUpdateFinalizerDeleteDependents(e, accessor, options)
oldFinalizers := accessor.GetFinalizers()
Expand Down Expand Up @@ -766,72 +771,6 @@ func markAsDeleting(obj runtime.Object) (err error) {
return nil
}

// updateForGracefulDeletion and updateForGracefulDeletionAndFinalizers both
// implement deletion flows for graceful deletion. Graceful deletion is
// implemented as setting the deletion timestamp in an update. If the
// implementation of graceful deletion is changed, both of these methods
// should be changed together.

// updateForGracefulDeletion updates the given object for graceful deletion by
// setting the deletion timestamp and grace period seconds and returns:
//
// 1. an error
// 2. a boolean indicating that the object was not found, but it should be
// ignored
// 3. a boolean indicating that the object's grace period is exhausted and it
// should be deleted immediately
// 4. a new output object with the state that was updated
// 5. a copy of the last existing state of the object
func (e *Store) updateForGracefulDeletion(ctx genericapirequest.Context, name, key string, options *metav1.DeleteOptions, preconditions storage.Preconditions, in runtime.Object) (err error, ignoreNotFound, deleteImmediately bool, out, lastExisting runtime.Object) {
lastGraceful := int64(0)
out = e.NewFunc()
err = e.Storage.GuaranteedUpdate(
ctx,
key,
out,
false, /* ignoreNotFound */
&preconditions,
storage.SimpleUpdate(func(existing runtime.Object) (runtime.Object, error) {
graceful, pendingGraceful, err := rest.BeforeDelete(e.DeleteStrategy, ctx, existing, options)
if err != nil {
return nil, err
}
if pendingGraceful {
return nil, errAlreadyDeleting
}
if !graceful {
return nil, errDeleteNow
}
lastGraceful = *options.GracePeriodSeconds
lastExisting = existing
return existing, nil
}),
)
switch err {
case nil:
if lastGraceful > 0 {
return nil, false, false, out, lastExisting
}
// If we are here, the registry supports grace period mechanism and
// we are intentionally delete gracelessly. In this case, we may
// enter a race with other k8s components. If other component wins
// the race, the object will not be found, and we should tolerate
// the NotFound error. See
// https://github.com/kubernetes/kubernetes/issues/19403 for
// details.
return nil, true, true, out, lastExisting
case errDeleteNow:
// we've updated the object to have a zero grace period, or it's already at 0, so
// we should fall through and truly delete the object.
return nil, false, true, out, lastExisting
case errAlreadyDeleting:
out, err = e.finalizeDelete(in, true)
return err, false, false, out, lastExisting
default:
return storeerr.InterpretUpdateError(err, e.QualifiedResource, name), false, false, out, lastExisting
}
}

// updateForGracefulDeletionAndFinalizers updates the given object for
// graceful deletion and finalization by setting the deletion timestamp and
// grace period seconds (graceful deletion) and updating the list of
Expand Down Expand Up @@ -871,7 +810,7 @@ func (e *Store) updateForGracefulDeletionAndFinalizers(ctx genericapirequest.Con
if err != nil {
return nil, err
}
shouldUpdate, newFinalizers := shouldUpdateFinalizers(e, existingAccessor, options)
shouldUpdate, newFinalizers := deletionFinalizersForGarbageCollection(e, existingAccessor, options)
if shouldUpdate {
existingAccessor.SetFinalizers(newFinalizers)
}
Expand Down Expand Up @@ -963,18 +902,12 @@ func (e *Store) Delete(ctx genericapirequest.Context, name string, options *meta

// Handle combinations of graceful deletion and finalization by issuing
// the correct updates.
if e.EnableGarbageCollection {
shouldUpdateFinalizers, _ := shouldUpdateFinalizers(e, accessor, options)
// TODO: remove the check, because we support no-op updates now.
if graceful || pendingFinalizers || shouldUpdateFinalizers {
err, ignoreNotFound, deleteImmediately, out, lastExisting = e.updateForGracefulDeletionAndFinalizers(ctx, name, key, options, preconditions, obj)
}
} else {
// TODO: remove the check on graceful, because we support no-op updates now.
if graceful {
err, ignoreNotFound, deleteImmediately, out, lastExisting = e.updateForGracefulDeletion(ctx, name, key, options, preconditions, obj)
}
shouldUpdateFinalizers, _ := deletionFinalizersForGarbageCollection(e, accessor, options)
// TODO: remove the check, because we support no-op updates now.
if graceful || pendingFinalizers || shouldUpdateFinalizers {
err, ignoreNotFound, deleteImmediately, out, lastExisting = e.updateForGracefulDeletionAndFinalizers(ctx, name, key, options, preconditions, obj)
}

// !deleteImmediately covers all cases where err != nil. We keep both to be future-proof.
if !deleteImmediately || err != nil {
return out, false, err
Expand Down
220 changes: 116 additions & 104 deletions staging/src/k8s.io/apiserver/pkg/registry/generic/registry/store_test.go
View file
Open in desktop
Expand Up @@ -923,56 +923,62 @@ func TestGracefulStoreHandleFinalizers(t *testing.T) {

testContext := genericapirequest.WithNamespace(genericapirequest.NewContext(), "test")
destroyFunc, registry := NewTestGenericStoreRegistry(t)
registry.EnableGarbageCollection = true
defaultDeleteStrategy := testRESTStrategy{scheme, names.SimpleNameGenerator, true, false, true}
registry.DeleteStrategy = testGracefulStrategy{defaultDeleteStrategy}
defer destroyFunc()
// create pod
_, err := registry.Create(testContext, podWithFinalizer, false)
if err != nil {
t.Errorf("Unexpected error: %v", err)
}

// delete the pod with grace period=0, the pod should still exist because it has a finalizer
_, wasDeleted, err := registry.Delete(testContext, podWithFinalizer.Name, metav1.NewDeleteOptions(0))
if err != nil {
t.Fatalf("Unexpected error: %v", err)
}
if wasDeleted {
t.Errorf("unexpected, pod %s should not have been deleted immediately", podWithFinalizer.Name)
}
_, err = registry.Get(testContext, podWithFinalizer.Name, &metav1.GetOptions{})
if err != nil {
t.Fatalf("Unexpected error: %v", err)
}
gcStates := []bool{true, false}
for _, gcEnabled := range gcStates {
t.Logf("garbage collection enabled: %t", gcEnabled)
registry.EnableGarbageCollection = gcEnabled

updatedPodWithFinalizer := &example.Pod{
ObjectMeta: metav1.ObjectMeta{Name: "foo", Finalizers: []string{"foo.com/x"}, ResourceVersion: podWithFinalizer.ObjectMeta.ResourceVersion},
Spec: example.PodSpec{NodeName: "machine"},
}
_, _, err = registry.Update(testContext, updatedPodWithFinalizer.ObjectMeta.Name, rest.DefaultUpdatedObjectInfo(updatedPodWithFinalizer, scheme))
if err != nil {
t.Fatalf("Unexpected error: %v", err)
}
// create pod
_, err := registry.Create(testContext, podWithFinalizer, false)
if err != nil {
t.Errorf("Unexpected error: %v", err)
}

// the object should still exist, because it still has a finalizer
_, err = registry.Get(testContext, podWithFinalizer.Name, &metav1.GetOptions{})
if err != nil {
t.Fatalf("Unexpected error: %v", err)
}
// delete the pod with grace period=0, the pod should still exist because it has a finalizer
_, wasDeleted, err := registry.Delete(testContext, podWithFinalizer.Name, metav1.NewDeleteOptions(0))
if err != nil {
t.Fatalf("Unexpected error: %v", err)
}
if wasDeleted {
t.Errorf("unexpected, pod %s should not have been deleted immediately", podWithFinalizer.Name)
}
_, err = registry.Get(testContext, podWithFinalizer.Name, &metav1.GetOptions{})
if err != nil {
t.Fatalf("Unexpected error: %v", err)
}

podWithNoFinalizer := &example.Pod{
ObjectMeta: metav1.ObjectMeta{Name: "foo", ResourceVersion: podWithFinalizer.ObjectMeta.ResourceVersion},
Spec: example.PodSpec{NodeName: "anothermachine"},
}
_, _, err = registry.Update(testContext, podWithFinalizer.ObjectMeta.Name, rest.DefaultUpdatedObjectInfo(podWithNoFinalizer, scheme))
if err != nil {
t.Fatalf("Unexpected error: %v", err)
}
// the pod should be removed, because its finalizer is removed
_, err = registry.Get(testContext, podWithFinalizer.Name, &metav1.GetOptions{})
if !errors.IsNotFound(err) {
t.Fatalf("Unexpected error: %v", err)
updatedPodWithFinalizer := &example.Pod{
ObjectMeta: metav1.ObjectMeta{Name: "foo", Finalizers: []string{"foo.com/x"}, ResourceVersion: podWithFinalizer.ObjectMeta.ResourceVersion},
Spec: example.PodSpec{NodeName: "machine"},
}
_, _, err = registry.Update(testContext, updatedPodWithFinalizer.ObjectMeta.Name, rest.DefaultUpdatedObjectInfo(updatedPodWithFinalizer, scheme))
if err != nil {
t.Fatalf("Unexpected error: %v", err)
}

// the object should still exist, because it still has a finalizer
_, err = registry.Get(testContext, podWithFinalizer.Name, &metav1.GetOptions{})
if err != nil {
t.Fatalf("Unexpected error: %v", err)
}

podWithNoFinalizer := &example.Pod{
ObjectMeta: metav1.ObjectMeta{Name: "foo", ResourceVersion: podWithFinalizer.ObjectMeta.ResourceVersion},
Spec: example.PodSpec{NodeName: "anothermachine"},
}
_, _, err = registry.Update(testContext, podWithFinalizer.ObjectMeta.Name, rest.DefaultUpdatedObjectInfo(podWithNoFinalizer, scheme))
if err != nil {
t.Fatalf("Unexpected error: %v", err)
}
// the pod should be removed, because its finalizer is removed
_, err = registry.Get(testContext, podWithFinalizer.Name, &metav1.GetOptions{})
if !errors.IsNotFound(err) {
t.Fatalf("Unexpected error: %v", err)
}
}
}

Expand Down Expand Up @@ -1022,73 +1028,79 @@ func TestNonGracefulStoreHandleFinalizers(t *testing.T) {

testContext := genericapirequest.WithNamespace(genericapirequest.NewContext(), "test")
destroyFunc, registry := NewTestGenericStoreRegistry(t)
registry.EnableGarbageCollection = true

defer destroyFunc()
// create pod
_, err := registry.Create(testContext, podWithFinalizer, false)
if err != nil {
t.Errorf("Unexpected error: %v", err)
}
gcStates := []bool{true, false}
for _, gcEnabled := range gcStates {
t.Logf("garbage collection enabled: %t", gcEnabled)
registry.EnableGarbageCollection = gcEnabled

// delete object with nil delete options doesn't delete the object
_, wasDeleted, err := registry.Delete(testContext, podWithFinalizer.Name, nil)
if err != nil {
t.Errorf("Unexpected error: %v", err)
}
if wasDeleted {
t.Errorf("unexpected, pod %s should not have been deleted immediately", podWithFinalizer.Name)
}
// create pod
_, err := registry.Create(testContext, podWithFinalizer, false)
if err != nil {
t.Errorf("Unexpected error: %v", err)
}

// the object should still exist
obj, err := registry.Get(testContext, podWithFinalizer.Name, &metav1.GetOptions{})
if err != nil {
t.Errorf("Unexpected error: %v", err)
}
podWithFinalizer, ok := obj.(*example.Pod)
if !ok {
t.Errorf("Unexpected object: %#v", obj)
}
if podWithFinalizer.ObjectMeta.DeletionTimestamp == nil {
t.Errorf("Expect the object to have DeletionTimestamp set, but got %#v", podWithFinalizer.ObjectMeta)
}
if podWithFinalizer.ObjectMeta.DeletionGracePeriodSeconds == nil || *podWithFinalizer.ObjectMeta.DeletionGracePeriodSeconds != 0 {
t.Errorf("Expect the object to have 0 DeletionGracePeriodSecond, but got %#v", podWithFinalizer.ObjectMeta)
}
if podWithFinalizer.Generation <= initialGeneration {
t.Errorf("Deletion didn't increase Generation.")
}
// delete object with nil delete options doesn't delete the object
_, wasDeleted, err := registry.Delete(testContext, podWithFinalizer.Name, nil)
if err != nil {
t.Errorf("Unexpected error: %v", err)
}
if wasDeleted {
t.Errorf("unexpected, pod %s should not have been deleted immediately", podWithFinalizer.Name)
}

updatedPodWithFinalizer := &example.Pod{
ObjectMeta: metav1.ObjectMeta{Name: "foo", Finalizers: []string{"foo.com/x"}, ResourceVersion: podWithFinalizer.ObjectMeta.ResourceVersion},
Spec: example.PodSpec{NodeName: "machine"},
}
_, _, err = registry.Update(testContext, updatedPodWithFinalizer.ObjectMeta.Name, rest.DefaultUpdatedObjectInfo(updatedPodWithFinalizer, scheme))
if err != nil {
t.Errorf("Unexpected error: %v", err)
}
// the object should still exist
obj, err := registry.Get(testContext, podWithFinalizer.Name, &metav1.GetOptions{})
if err != nil {
t.Errorf("Unexpected error: %v", err)
}
podWithFinalizer, ok := obj.(*example.Pod)
if !ok {
t.Errorf("Unexpected object: %#v", obj)
}
if podWithFinalizer.ObjectMeta.DeletionTimestamp == nil {
t.Errorf("Expect the object to have DeletionTimestamp set, but got %#v", podWithFinalizer.ObjectMeta)
}
if podWithFinalizer.ObjectMeta.DeletionGracePeriodSeconds == nil || *podWithFinalizer.ObjectMeta.DeletionGracePeriodSeconds != 0 {
t.Errorf("Expect the object to have 0 DeletionGracePeriodSecond, but got %#v", podWithFinalizer.ObjectMeta)
}
if podWithFinalizer.Generation <= initialGeneration {
t.Errorf("Deletion didn't increase Generation.")
}

// the object should still exist, because it still has a finalizer
obj, err = registry.Get(testContext, podWithFinalizer.Name, &metav1.GetOptions{})
if err != nil {
t.Errorf("Unexpected error: %v", err)
}
podWithFinalizer, ok = obj.(*example.Pod)
if !ok {
t.Errorf("Unexpected object: %#v", obj)
}
updatedPodWithFinalizer := &example.Pod{
ObjectMeta: metav1.ObjectMeta{Name: "foo", Finalizers: []string{"foo.com/x"}, ResourceVersion: podWithFinalizer.ObjectMeta.ResourceVersion},
Spec: example.PodSpec{NodeName: "machine"},
}
_, _, err = registry.Update(testContext, updatedPodWithFinalizer.ObjectMeta.Name, rest.DefaultUpdatedObjectInfo(updatedPodWithFinalizer, scheme))
if err != nil {
t.Errorf("Unexpected error: %v", err)
}

podWithNoFinalizer := &example.Pod{
ObjectMeta: metav1.ObjectMeta{Name: "foo", ResourceVersion: podWithFinalizer.ObjectMeta.ResourceVersion},
Spec: example.PodSpec{NodeName: "anothermachine"},
}
_, _, err = registry.Update(testContext, podWithFinalizer.ObjectMeta.Name, rest.DefaultUpdatedObjectInfo(podWithNoFinalizer, scheme))
if err != nil {
t.Errorf("Unexpected error: %v", err)
}
// the pod should be removed, because its finalizer is removed
_, err = registry.Get(testContext, podWithFinalizer.Name, &metav1.GetOptions{})
if !errors.IsNotFound(err) {
t.Errorf("Unexpected error: %v", err)
// the object should still exist, because it still has a finalizer
obj, err = registry.Get(testContext, podWithFinalizer.Name, &metav1.GetOptions{})
if err != nil {
t.Errorf("Unexpected error: %v", err)
}
podWithFinalizer, ok = obj.(*example.Pod)
if !ok {
t.Errorf("Unexpected object: %#v", obj)
}

podWithNoFinalizer := &example.Pod{
ObjectMeta: metav1.ObjectMeta{Name: "foo", ResourceVersion: podWithFinalizer.ObjectMeta.ResourceVersion},
Spec: example.PodSpec{NodeName: "anothermachine"},
}
_, _, err = registry.Update(testContext, podWithFinalizer.ObjectMeta.Name, rest.DefaultUpdatedObjectInfo(podWithNoFinalizer, scheme))
if err != nil {
t.Errorf("Unexpected error: %v", err)
}
// the pod should be removed, because its finalizer is removed
_, err = registry.Get(testContext, podWithFinalizer.Name, &metav1.GetOptions{})
if !errors.IsNotFound(err) {
t.Errorf("Unexpected error: %v", err)
}
}
}

Expand Down