Clean up redundant post-finalizer deletions

pkg/controller/namespace/deletion/namespaced_resources_deleter.go

@@ -45,7 +45,7 @@ type NamespacedResourcesDeleterInterface interface {
 func NewNamespacedResourcesDeleter(nsClient v1clientset.NamespaceInterface,
 	metadataClient metadata.Interface, podsGetter v1clientset.PodsGetter,
 	discoverResourcesFn func() ([]*metav1.APIResourceList, error),
-	finalizerToken v1.FinalizerName, deleteNamespaceWhenDone bool) NamespacedResourcesDeleterInterface {
+	finalizerToken v1.FinalizerName) NamespacedResourcesDeleterInterface {
 	d := &namespacedResourcesDeleter{
 		nsClient:       nsClient,
 		metadataClient: metadataClient,
@@ -53,9 +53,8 @@ func NewNamespacedResourcesDeleter(nsClient v1clientset.NamespaceInterface,
 		opCache: &operationNotSupportedCache{
 			m: make(map[operationKey]bool),
 		},
-		discoverResourcesFn:     discoverResourcesFn,
+		discoverResourcesFn: discoverResourcesFn,
-		finalizerToken:          finalizerToken,
+		finalizerToken:      finalizerToken,
-		deleteNamespaceWhenDone: deleteNamespaceWhenDone,
 	}
 	d.initOpCache()
 	return d
@@ -77,8 +76,6 @@ type namespacedResourcesDeleter struct {
 	// The finalizer token that should be removed from the namespace
 	// when all resources in that namespace have been deleted.
 	finalizerToken v1.FinalizerName
-	// Also delete the namespace when all resources in the namespace have been deleted.
-	deleteNamespaceWhenDone bool
 }
 
 // Delete deletes all resources in the given namespace.
@@ -89,7 +86,6 @@ type namespacedResourcesDeleter struct {
 //   (updates the namespace phase if it is not yet marked terminating)
 // After deleting the resources:
 // * It removes finalizer token from the given namespace.
-// * Deletes the namespace if deleteNamespaceWhenDone is true.
 //
 // Returns an error if any of those steps fail.
 // Returns ResourcesRemainingError if it deleted some resources but needs
@@ -127,10 +123,9 @@ func (d *namespacedResourcesDeleter) Delete(nsName string) error {
 		return nil
 	}
 
-	// Delete the namespace if it is already finalized.
+	// return if it is already finalized.
-	if d.deleteNamespaceWhenDone && finalized(namespace) {
+	if finalized(namespace) {
-		// TODO(liggitt): just return in 1.16, once n-1 apiservers automatically delete when finalizers are all removed
+		return nil
-		return d.deleteNamespace(namespace)
 	}
 
 	// there may still be content for us to remove
@@ -153,12 +148,6 @@ func (d *namespacedResourcesDeleter) Delete(nsName string) error {
 		}
 		return err
 	}
-
-	// 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
 }
 

pkg/controller/namespace/deletion/namespaced_resources_deleter_test.go

@@ -147,7 +147,6 @@ func testSyncNamespaceThatIsTerminating(t *testing.T, versions *metav1.APIVersio
 				strings.Join([]string{"create", "namespaces", "finalize"}, "-"),
 				strings.Join([]string{"list", "pods", ""}, "-"),
 				strings.Join([]string{"update", "namespaces", "status"}, "-"),
-				strings.Join([]string{"delete", "namespaces", ""}, "-"),
 			),
 			metadataClientActionSet: metadataClientActionSet,
 		},
@@ -155,7 +154,6 @@ func testSyncNamespaceThatIsTerminating(t *testing.T, versions *metav1.APIVersio
 			testNamespace: testNamespaceFinalizeComplete,
 			kubeClientActionSet: sets.NewString(
 				strings.Join([]string{"get", "namespaces", ""}, "-"),
-				strings.Join([]string{"delete", "namespaces", ""}, "-"),
 			),
 			metadataClientActionSet: sets.NewString(),
 		},
@@ -163,7 +161,6 @@ func testSyncNamespaceThatIsTerminating(t *testing.T, versions *metav1.APIVersio
 			testNamespace: testNamespaceFinalizeComplete,
 			kubeClientActionSet: sets.NewString(
 				strings.Join([]string{"get", "namespaces", ""}, "-"),
-				strings.Join([]string{"delete", "namespaces", ""}, "-"),
 			),
 			metadataClientActionSet: sets.NewString(),
 			gvrError:                fmt.Errorf("test error"),
@@ -202,7 +199,7 @@ func testSyncNamespaceThatIsTerminating(t *testing.T, versions *metav1.APIVersio
 			fn := func() ([]*metav1.APIResourceList, error) {
 				return resources, testInput.gvrError
 			}
-			d := NewNamespacedResourcesDeleter(mockClient.CoreV1().Namespaces(), metadataClient, mockClient.CoreV1(), fn, v1.FinalizerKubernetes, true)
+			d := NewNamespacedResourcesDeleter(mockClient.CoreV1().Namespaces(), metadataClient, mockClient.CoreV1(), fn, v1.FinalizerKubernetes)
 			if err := d.Delete(testInput.testNamespace.Name); !matchErrors(err, testInput.expectErrorOnDelete) {
 				t.Errorf("expected error %q when syncing namespace, got %q, %v", testInput.expectErrorOnDelete, err, testInput.expectErrorOnDelete == err)
 			}
@@ -300,7 +297,7 @@ func TestSyncNamespaceThatIsActive(t *testing.T) {
 		return testResources(), nil
 	}
 	d := NewNamespacedResourcesDeleter(mockClient.CoreV1().Namespaces(), nil, mockClient.CoreV1(),
-		fn, v1.FinalizerKubernetes, true)
+		fn, v1.FinalizerKubernetes)
 	err := d.Delete(testNamespace.Name)
 	if err != nil {
 		t.Errorf("Unexpected error when synching namespace %v", err)

pkg/controller/namespace/namespace_controller.go

@@ -72,7 +72,7 @@ func NewNamespaceController(
 	// create the controller so we can inject the enqueue function
 	namespaceController := &NamespaceController{
 		queue:                      workqueue.NewNamedRateLimitingQueue(workqueue.DefaultControllerRateLimiter(), "namespace"),
-		namespacedResourcesDeleter: deletion.NewNamespacedResourcesDeleter(kubeClient.CoreV1().Namespaces(), metadataClient, kubeClient.CoreV1(), discoverResourcesFn, finalizerToken, true),
+		namespacedResourcesDeleter: deletion.NewNamespacedResourcesDeleter(kubeClient.CoreV1().Namespaces(), metadataClient, kubeClient.CoreV1(), discoverResourcesFn, finalizerToken),
 	}
 
 	if kubeClient != nil && kubeClient.CoreV1().RESTClient().GetRateLimiter() != nil {

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

@@ -170,16 +170,6 @@ func (c *CRDFinalizer) sync(key string) error {
 		// deleted or changed in the meantime, we'll get called again
 		return nil
 	}
-	if err != nil {
-		return err
-	}
-
-	// and now issue another delete, which should clean it all up if no finalizers remain or no-op if they do
-	// 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
 }