github
/
k3s
mirror of https://github.com/k3s-io/k3s
1
0
Fork 0
Code Issues Projects Releases Wiki Activity

Merge pull request #77952 from liggitt/delete-on-update 

Handle updates removing remaining finalizers on deleted objects
k3s-v1.15.3
Kubernetes Prow Robot 2019-05-17 02:04:32 -07:00 committed by GitHub
parent da4d6f5829 fba885a0d2
commit 27410955e2
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
7 changed files with 241 additions and 17 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
pkg/controller/namespace/deletion/namespaced_resources_deleter.go
View File

@ -129,6 +129,7 @@ func (d *namespacedResourcesDeleter) Delete(nsName string) error {
// Delete the namespace if it is already finalized.
if d.deleteNamespaceWhenDone && finalized(namespace) {
// TODO(liggitt): just return in 1.16, once n-1 apiservers automatically delete when finalizers are all removed
return d.deleteNamespace(namespace)
}
@ -155,6 +156,7 @@ func (d *namespacedResourcesDeleter) Delete(nsName string) error {
// Check if we can delete now.
if d.deleteNamespaceWhenDone && finalized(namespace) {
// TODO(liggitt): just return in 1.16, once n-1 apiservers automatically delete when finalizers are all removed
return d.deleteNamespace(namespace)
}
return nil

2
pkg/registry/core/namespace/storage/BUILD
View File

@ -13,6 +13,7 @@ go_test(
deps = [
"//pkg/apis/core:go_default_library",
"//pkg/registry/registrytest:go_default_library",
"//staging/src/k8s.io/apimachinery/pkg/api/errors:go_default_library",
"//staging/src/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library",
"//staging/src/k8s.io/apimachinery/pkg/fields:go_default_library",
"//staging/src/k8s.io/apimachinery/pkg/labels:go_default_library",
@ -40,6 +41,7 @@ go_library(
"//staging/src/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library",
"//staging/src/k8s.io/apimachinery/pkg/apis/meta/v1beta1:go_default_library",
"//staging/src/k8s.io/apimachinery/pkg/runtime:go_default_library",
"//staging/src/k8s.io/apimachinery/pkg/util/runtime:go_default_library",
"//staging/src/k8s.io/apimachinery/pkg/watch:go_default_library",
"//staging/src/k8s.io/apiserver/pkg/registry/generic:go_default_library",
"//staging/src/k8s.io/apiserver/pkg/registry/generic/registry:go_default_library",

13
pkg/registry/core/namespace/storage/storage.go
View File

@ -33,6 +33,7 @@ import (
storageerr "k8s.io/apiserver/pkg/storage/errors"
"k8s.io/apiserver/pkg/util/dryrun"
utilruntime "k8s.io/apimachinery/pkg/util/runtime"
api "k8s.io/kubernetes/pkg/apis/core"
"k8s.io/kubernetes/pkg/printers"
printersinternal "k8s.io/kubernetes/pkg/printers/internalversion"
@ -69,6 +70,8 @@ func NewREST(optsGetter generic.RESTOptionsGetter) (*REST, *StatusREST, *Finaliz
DeleteStrategy: namespace.Strategy,
ReturnDeletedObject: true,
ShouldDeleteDuringUpdate: ShouldDeleteNamespaceDuringUpdate,
TableConvertor: printerstorage.TableConvertor{TableGenerator: printers.NewTableGenerator().With(printersinternal.AddHandlers)},
}
options := &generic.StoreOptions{RESTOptions: optsGetter, AttrFunc: namespace.GetAttrs}
@ -238,6 +241,16 @@ func (r *REST) Delete(ctx context.Context, name string, options *metav1.DeleteOp
return r.store.Delete(ctx, name, options)
}
// ShouldDeleteNamespaceDuringUpdate adds namespace-specific spec.finalizer checks on top of the default generic ShouldDeleteDuringUpdate behavior
func ShouldDeleteNamespaceDuringUpdate(ctx context.Context, key string, obj, existing runtime.Object) bool {
ns, ok := obj.(*api.Namespace)
if !ok {
utilruntime.HandleError(fmt.Errorf("unexpected type %T", obj))
return false
}
return len(ns.Spec.Finalizers) == 0 && genericregistry.ShouldDeleteDuringUpdate(ctx, key, obj, existing)
}
func shouldHaveOrphanFinalizer(options *metav1.DeleteOptions, haveOrphanFinalizer bool) bool {
if options.OrphanDependents != nil {
return *options.OrphanDependents

194
pkg/registry/core/namespace/storage/storage_test.go
View File

@ -19,6 +19,7 @@ package storage
import (
"testing"
apierrors "k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/fields"
"k8s.io/apimachinery/pkg/labels"
@ -164,6 +165,194 @@ func TestDeleteNamespaceWithIncompleteFinalizers(t *testing.T) {
if _, _, err := storage.Delete(ctx, "foo", nil); err == nil {
t.Errorf("unexpected error: %v", err)
}
// should still exist
_, err := storage.Get(ctx, "foo", &metav1.GetOptions{})
if err != nil {
t.Errorf("unexpected error: %v", err)
}
}
func TestUpdateDeletingNamespaceWithIncompleteMetadataFinalizers(t *testing.T) {
storage, server := newStorage(t)
defer server.Terminate(t)
defer storage.store.DestroyFunc()
key := "namespaces/foo"
ctx := genericapirequest.NewContext()
now := metav1.Now()
namespace := &api.Namespace{
ObjectMeta: metav1.ObjectMeta{
Name: "foo",
DeletionTimestamp: &now,
Finalizers: []string{"example.com/foo"},
},
Spec: api.NamespaceSpec{
Finalizers: []api.FinalizerName{},
},
Status: api.NamespaceStatus{Phase: api.NamespaceActive},
}
if err := storage.store.Storage.Create(ctx, key, namespace, nil, 0, false); err != nil {
t.Fatalf("unexpected error: %v", err)
}
ns, err := storage.Get(ctx, "foo", &metav1.GetOptions{})
if err != nil {
t.Fatalf("unexpected error: %v", err)
}
if _, _, err = storage.Update(ctx, "foo", rest.DefaultUpdatedObjectInfo(ns), rest.ValidateAllObjectFunc, rest.ValidateAllObjectUpdateFunc, false, &metav1.UpdateOptions{}); err != nil {
t.Fatalf("unexpected error: %v", err)
}
// should still exist
_, err = storage.Get(ctx, "foo", &metav1.GetOptions{})
if err != nil {
t.Fatalf("unexpected error: %v", err)
}
}
func TestUpdateDeletingNamespaceWithIncompleteSpecFinalizers(t *testing.T) {
storage, server := newStorage(t)
defer server.Terminate(t)
defer storage.store.DestroyFunc()
key := "namespaces/foo"
ctx := genericapirequest.NewContext()
now := metav1.Now()
namespace := &api.Namespace{
ObjectMeta: metav1.ObjectMeta{
Name: "foo",
DeletionTimestamp: &now,
},
Spec: api.NamespaceSpec{
Finalizers: []api.FinalizerName{api.FinalizerKubernetes},
},
Status: api.NamespaceStatus{Phase: api.NamespaceActive},
}
if err := storage.store.Storage.Create(ctx, key, namespace, nil, 0, false); err != nil {
t.Fatalf("unexpected error: %v", err)
}
ns, err := storage.Get(ctx, "foo", &metav1.GetOptions{})
if err != nil {
t.Fatalf("unexpected error: %v", err)
}
if _, _, err = storage.Update(ctx, "foo", rest.DefaultUpdatedObjectInfo(ns), rest.ValidateAllObjectFunc, rest.ValidateAllObjectUpdateFunc, false, &metav1.UpdateOptions{}); err != nil {
t.Fatalf("unexpected error: %v", err)
}
// should still exist
_, err = storage.Get(ctx, "foo", &metav1.GetOptions{})
if err != nil {
t.Fatalf("unexpected error: %v", err)
}
}
func TestUpdateDeletingNamespaceWithCompleteFinalizers(t *testing.T) {
storage, server := newStorage(t)
defer server.Terminate(t)
defer storage.store.DestroyFunc()
key := "namespaces/foo"
ctx := genericapirequest.NewContext()
now := metav1.Now()
namespace := &api.Namespace{
ObjectMeta: metav1.ObjectMeta{
Name: "foo",
DeletionTimestamp: &now,
Finalizers: []string{"example.com/foo"},
},
Status: api.NamespaceStatus{Phase: api.NamespaceActive},
}
if err := storage.store.Storage.Create(ctx, key, namespace, nil, 0, false); err != nil {
t.Fatalf("unexpected error: %v", err)
}
ns, err := storage.Get(ctx, "foo", &metav1.GetOptions{})
if err != nil {
t.Fatalf("unexpected error: %v", err)
}
ns.(*api.Namespace).Finalizers = nil
if _, _, err = storage.Update(ctx, "foo", rest.DefaultUpdatedObjectInfo(ns), rest.ValidateAllObjectFunc, rest.ValidateAllObjectUpdateFunc, false, &metav1.UpdateOptions{}); err != nil {
t.Fatalf("unexpected error: %v", err)
}
// should not exist
_, err = storage.Get(ctx, "foo", &metav1.GetOptions{})
if !apierrors.IsNotFound(err) {
t.Errorf("expected NotFound, got %v", err)
}
}
func TestFinalizeDeletingNamespaceWithCompleteFinalizers(t *testing.T) {
// get finalize storage
etcdStorage, server := registrytest.NewEtcdStorage(t, "")
restOptions := generic.RESTOptions{StorageConfig: etcdStorage, Decorator: generic.UndecoratedStorage, DeleteCollectionWorkers: 1, ResourcePrefix: "namespaces"}
storage, _, finalizeStorage := NewREST(restOptions)
defer server.Terminate(t)
defer storage.store.DestroyFunc()
defer finalizeStorage.store.DestroyFunc()
key := "namespaces/foo"
ctx := genericapirequest.NewContext()
now := metav1.Now()
namespace := &api.Namespace{
ObjectMeta: metav1.ObjectMeta{
Name: "foo",
DeletionTimestamp: &now,
},
Spec: api.NamespaceSpec{
Finalizers: []api.FinalizerName{api.FinalizerKubernetes},
},
Status: api.NamespaceStatus{Phase: api.NamespaceActive},
}
if err := storage.store.Storage.Create(ctx, key, namespace, nil, 0, false); err != nil {
t.Fatalf("unexpected error: %v", err)
}
ns, err := storage.Get(ctx, "foo", &metav1.GetOptions{})
if err != nil {
t.Fatalf("unexpected error: %v", err)
}
ns.(*api.Namespace).Spec.Finalizers = nil
if _, _, err = finalizeStorage.Update(ctx, "foo", rest.DefaultUpdatedObjectInfo(ns), rest.ValidateAllObjectFunc, rest.ValidateAllObjectUpdateFunc, false, &metav1.UpdateOptions{}); err != nil {
t.Fatalf("unexpected error: %v", err)
}
// should not exist
_, err = storage.Get(ctx, "foo", &metav1.GetOptions{})
if !apierrors.IsNotFound(err) {
t.Errorf("expected NotFound, got %v", err)
}
}
func TestFinalizeDeletingNamespaceWithIncompleteMetadataFinalizers(t *testing.T) {
// get finalize storage
etcdStorage, server := registrytest.NewEtcdStorage(t, "")
restOptions := generic.RESTOptions{StorageConfig: etcdStorage, Decorator: generic.UndecoratedStorage, DeleteCollectionWorkers: 1, ResourcePrefix: "namespaces"}
storage, _, finalizeStorage := NewREST(restOptions)
defer server.Terminate(t)
defer storage.store.DestroyFunc()
defer finalizeStorage.store.DestroyFunc()
key := "namespaces/foo"
ctx := genericapirequest.NewContext()
now := metav1.Now()
namespace := &api.Namespace{
ObjectMeta: metav1.ObjectMeta{
Name: "foo",
DeletionTimestamp: &now,
Finalizers: []string{"example.com/foo"},
},
Spec: api.NamespaceSpec{
Finalizers: []api.FinalizerName{api.FinalizerKubernetes},
},
Status: api.NamespaceStatus{Phase: api.NamespaceActive},
}
if err := storage.store.Storage.Create(ctx, key, namespace, nil, 0, false); err != nil {
t.Fatalf("unexpected error: %v", err)
}
ns, err := storage.Get(ctx, "foo", &metav1.GetOptions{})
if err != nil {
t.Fatalf("unexpected error: %v", err)
}
ns.(*api.Namespace).Spec.Finalizers = nil
if _, _, err = finalizeStorage.Update(ctx, "foo", rest.DefaultUpdatedObjectInfo(ns), rest.ValidateAllObjectFunc, rest.ValidateAllObjectUpdateFunc, false, &metav1.UpdateOptions{}); err != nil {
t.Fatalf("unexpected error: %v", err)
}
// should still exist
_, err = storage.Get(ctx, "foo", &metav1.GetOptions{})
if err != nil {
t.Fatalf("unexpected error: %v", err)
}
}
func TestDeleteNamespaceWithCompleteFinalizers(t *testing.T) {
@ -189,6 +378,11 @@ func TestDeleteNamespaceWithCompleteFinalizers(t *testing.T) {
if _, _, err := storage.Delete(ctx, "foo", nil); err != nil {
t.Errorf("unexpected error: %v", err)
}
// should not exist
_, err := storage.Get(ctx, "foo", &metav1.GetOptions{})
if !apierrors.IsNotFound(err) {
t.Errorf("expected NotFound, got %v", err)
}
}
func TestDeleteWithGCFinalizers(t *testing.T) {

7
staging/src/k8s.io/apiextensions-apiserver/pkg/controller/finalizer/crd_finalizer.go
View File

@ -156,7 +156,12 @@ func (c *CRDFinalizer) sync(key string) error {
}
// and now issue another delete, which should clean it all up if no finalizers remain or no-op if they do
return c.crdClient.CustomResourceDefinitions().Delete(crd.Name, nil)
// TODO(liggitt): just return in 1.16, once n-1 apiservers automatically delete when finalizers are all removed
err = c.crdClient.CustomResourceDefinitions().Delete(crd.Name, nil)
if apierrors.IsNotFound(err) {
return nil
}
return err
}
func (c *CRDFinalizer) deleteInstances(crd *apiextensions.CustomResourceDefinition) (apiextensions.CustomResourceDefinitionCondition, error) {

30
staging/src/k8s.io/apiserver/pkg/registry/generic/registry/store.go
View File

@ -166,6 +166,11 @@ type Store struct {
// ReturnDeletedObject determines whether the Store returns the object
// that was deleted. Otherwise, return a generic success status response.
ReturnDeletedObject bool
// ShouldDeleteDuringUpdate is an optional function to determine whether
// an update from existing to obj should result in a delete.
// If specified, this is checked in addition to standard finalizer,
// deletionTimestamp, and deletionGracePeriodSeconds checks.
ShouldDeleteDuringUpdate func(ctx context.Context, key string, obj, existing runtime.Object) bool
// ExportStrategy implements resource-specific behavior during export,
// optional. Exported objects are not decorated.
ExportStrategy rest.RESTExportStrategy
@ -388,10 +393,12 @@ func (e *Store) Create(ctx context.Context, obj runtime.Object, createValidation
return out, nil
}
// shouldDeleteDuringUpdate checks if a Update is removing all the object's
// finalizers. If so, it further checks if the object's
// DeletionGracePeriodSeconds is 0.
func (e *Store) shouldDeleteDuringUpdate(ctx context.Context, key string, obj, existing runtime.Object) bool {
// ShouldDeleteDuringUpdate is the default function for
// checking if an object should be deleted during an update.
// It checks if the new object has no finalizers,
// the existing object's deletionTimestamp is set, and
// the existing object's deletionGracePeriodSeconds is 0 or nil
func ShouldDeleteDuringUpdate(ctx context.Context, key string, obj, existing runtime.Object) bool {
newMeta, err := meta.Accessor(obj)
if err != nil {
utilruntime.HandleError(err)
@ -402,7 +409,16 @@ func (e *Store) shouldDeleteDuringUpdate(ctx context.Context, key string, obj, e
utilruntime.HandleError(err)
return false
}
return len(newMeta.GetFinalizers()) == 0 && oldMeta.GetDeletionGracePeriodSeconds() != nil && *oldMeta.GetDeletionGracePeriodSeconds() == 0
if len(newMeta.GetFinalizers()) > 0 {
// don't delete with finalizers remaining in the new object
return false
}
if oldMeta.GetDeletionTimestamp() == nil {
// don't delete if the existing object hasn't had a delete request made
return false
}
// delete if the existing object has no grace period or a grace period of 0
return oldMeta.GetDeletionGracePeriodSeconds() == nil || *oldMeta.GetDeletionGracePeriodSeconds() == 0
}
// deleteWithoutFinalizers handles deleting an object ignoring its finalizer list.
@ -533,7 +549,9 @@ func (e *Store) Update(ctx context.Context, name string, objInfo rest.UpdatedObj
return nil, nil, err
}
}
if e.shouldDeleteDuringUpdate(ctx, key, obj, existing) {
// Check the default delete-during-update conditions, and store-specific conditions if provided
if ShouldDeleteDuringUpdate(ctx, key, obj, existing) &&
(e.ShouldDeleteDuringUpdate == nil || e.ShouldDeleteDuringUpdate(ctx, key, obj, existing)) {
deleteObj = obj
return nil, nil, errEmptiedFinalizers
}

10
test/integration/apiserver/admissionwebhook/admission_test.go
View File

@ -729,16 +729,6 @@ func testNamespaceDelete(c *testContext) {
c.t.Error(err)
return
}
// then run the final delete and make sure admission is called again
c.admissionHolder.expect(c.gvr, gvk(c.resource.Group, c.resource.Version, c.resource.Kind), gvkDeleteOptions, v1beta1.Delete, obj.GetName(), obj.GetNamespace(), false, false, true)
err = c.client.Resource(c.gvr).Namespace(obj.GetNamespace()).Delete(obj.GetName(), &metav1.DeleteOptions{GracePeriodSeconds: &zero, PropagationPolicy: &background})
if err != nil {
c.t.Error(err)
return
}
c.admissionHolder.verify(c.t)
// verify namespace is gone
obj, err = c.client.Resource(c.gvr).Namespace(obj.GetNamespace()).Get(obj.GetName(), metav1.GetOptions{})
if err == nil || !errors.IsNotFound(err) {