Fix namespace controller cleanup orphaning

pkg/controller/namespace/deletion/BUILD

@@ -46,7 +46,9 @@ go_test(
         "//staging/src/k8s.io/client-go/discovery:go_default_library",
         "//staging/src/k8s.io/client-go/dynamic:go_default_library",
         "//staging/src/k8s.io/client-go/kubernetes/fake:go_default_library",
+        "//staging/src/k8s.io/client-go/kubernetes/scheme:go_default_library",
         "//staging/src/k8s.io/client-go/metadata:go_default_library",
+        "//staging/src/k8s.io/client-go/metadata/fake:go_default_library",
         "//staging/src/k8s.io/client-go/rest:go_default_library",
         "//staging/src/k8s.io/client-go/testing:go_default_library",
     ],

pkg/controller/namespace/deletion/namespaced_resources_deleter.go

@@ -265,11 +265,9 @@ func (d *namespacedResourcesDeleter) updateNamespaceStatusFunc(namespace *v1.Nam
 	if namespace.DeletionTimestamp.IsZero() || namespace.Status.Phase == v1.NamespaceTerminating {
 		return namespace, nil
 	}
-	newNamespace := v1.Namespace{}
+	newNamespace := namespace.DeepCopy()
-	newNamespace.ObjectMeta = namespace.ObjectMeta
-	newNamespace.Status = *namespace.Status.DeepCopy()
 	newNamespace.Status.Phase = v1.NamespaceTerminating
-	return d.nsClient.UpdateStatus(context.TODO(), &newNamespace, metav1.UpdateOptions{})
+	return d.nsClient.UpdateStatus(context.TODO(), newNamespace, metav1.UpdateOptions{})
 }
 
 // finalized returns true if the namespace.Spec.Finalizers is an empty list
@@ -330,10 +328,8 @@ func (d *namespacedResourcesDeleter) deleteCollection(gvr schema.GroupVersionRes
 	// we have a resource returned in the discovery API that supports no top-level verbs:
 	//  /apis/extensions/v1beta1/namespaces/default/replicationcontrollers
 	// when working with this resource type, we will get a literal not found error rather than expected method not supported
-	// remember next time that this resource does not support delete collection...
 	if errors.IsMethodNotSupported(err) || errors.IsNotFound(err) {
 		klog.V(5).Infof("namespace controller - deleteCollection not supported - namespace: %s, gvr: %v", namespace, gvr)
-		d.opCache.setNotSupported(key)
 		return false, nil
 	}
 
@@ -365,10 +361,8 @@ func (d *namespacedResourcesDeleter) listCollection(gvr schema.GroupVersionResou
 	// we have a resource returned in the discovery API that supports no top-level verbs:
 	//  /apis/extensions/v1beta1/namespaces/default/replicationcontrollers
 	// when working with this resource type, we will get a literal not found error rather than expected method not supported
-	// remember next time that this resource does not support delete collection...
 	if errors.IsMethodNotSupported(err) || errors.IsNotFound(err) {
 		klog.V(5).Infof("namespace controller - listCollection not supported - namespace: %s, gvr: %v", namespace, gvr)
-		d.opCache.setNotSupported(key)
 		return nil, false, nil
 	}
 

pkg/controller/namespace/deletion/namespaced_resources_deleter_test.go

@@ -34,7 +34,9 @@ import (
 	"k8s.io/client-go/discovery"
 	"k8s.io/client-go/dynamic"
 	"k8s.io/client-go/kubernetes/fake"
+	scheme "k8s.io/client-go/kubernetes/scheme"
 	"k8s.io/client-go/metadata"
+	metadatafake "k8s.io/client-go/metadata/fake"
 	restclient "k8s.io/client-go/rest"
 	core "k8s.io/client-go/testing"
 	api "k8s.io/kubernetes/pkg/apis/core"
@@ -396,3 +398,67 @@ func testResources() []*metav1.APIResourceList {
 	}
 	return results
 }
+
+func TestDeleteEncounters404(t *testing.T) {
+	now := metav1.Now()
+	ns1 := &v1.Namespace{
+		ObjectMeta: metav1.ObjectMeta{Name: "ns1", ResourceVersion: "1", DeletionTimestamp: &now},
+		Spec:       v1.NamespaceSpec{Finalizers: []v1.FinalizerName{"kubernetes"}},
+		Status:     v1.NamespaceStatus{Phase: v1.NamespaceActive},
+	}
+	ns2 := &v1.Namespace{
+		ObjectMeta: metav1.ObjectMeta{Name: "ns2", ResourceVersion: "1", DeletionTimestamp: &now},
+		Spec:       v1.NamespaceSpec{Finalizers: []v1.FinalizerName{"kubernetes"}},
+		Status:     v1.NamespaceStatus{Phase: v1.NamespaceActive},
+	}
+	mockClient := fake.NewSimpleClientset(ns1, ns2)
+
+	ns1FlakesNotFound := func(action core.Action) (handled bool, ret runtime.Object, err error) {
+		if action.GetNamespace() == "ns1" {
+			// simulate the flakes resource not existing when ns1 is processed
+			return true, nil, errors.NewNotFound(schema.GroupResource{}, "")
+		}
+		return false, nil, nil
+	}
+	mockMetadataClient := metadatafake.NewSimpleMetadataClient(scheme.Scheme)
+	mockMetadataClient.PrependReactor("delete-collection", "flakes", ns1FlakesNotFound)
+	mockMetadataClient.PrependReactor("list", "flakes", ns1FlakesNotFound)
+
+	resourcesFn := func() ([]*metav1.APIResourceList, error) {
+		return []*metav1.APIResourceList{{
+			GroupVersion: "example.com/v1",
+			APIResources: []metav1.APIResource{{Name: "flakes", Namespaced: true, Kind: "Flake", Verbs: []string{"get", "list", "delete", "deletecollection", "create", "update"}}},
+		}}, nil
+	}
+
+	d := NewNamespacedResourcesDeleter(mockClient.CoreV1().Namespaces(), mockMetadataClient, mockClient.CoreV1(), resourcesFn, v1.FinalizerKubernetes)
+
+	// Delete ns1 and get NotFound errors for the flakes resource
+	mockMetadataClient.ClearActions()
+	if err := d.Delete(ns1.Name); err != nil {
+		t.Fatal(err)
+	}
+	if len(mockMetadataClient.Actions()) != 3 ||
+		!mockMetadataClient.Actions()[0].Matches("delete-collection", "flakes") ||
+		!mockMetadataClient.Actions()[1].Matches("list", "flakes") ||
+		!mockMetadataClient.Actions()[2].Matches("list", "flakes") {
+		for _, action := range mockMetadataClient.Actions() {
+			t.Log("ns1", action)
+		}
+		t.Error("ns1: expected delete-collection -> fallback to list -> list to verify 0 items")
+	}
+
+	// Delete ns2
+	mockMetadataClient.ClearActions()
+	if err := d.Delete(ns2.Name); err != nil {
+		t.Fatal(err)
+	}
+	if len(mockMetadataClient.Actions()) != 2 ||
+		!mockMetadataClient.Actions()[0].Matches("delete-collection", "flakes") ||
+		!mockMetadataClient.Actions()[1].Matches("list", "flakes") {
+		for _, action := range mockMetadataClient.Actions() {
+			t.Log("ns2", action)
+		}
+		t.Error("ns2: expected delete-collection -> list to verify 0 items")
+	}
+}

vendor/modules.txt

@@ -2045,6 +2045,7 @@ k8s.io/client-go/listers/storage/v1
 k8s.io/client-go/listers/storage/v1alpha1
 k8s.io/client-go/listers/storage/v1beta1
 k8s.io/client-go/metadata
+k8s.io/client-go/metadata/fake
 k8s.io/client-go/metadata/metadatainformer
 k8s.io/client-go/metadata/metadatalister
 k8s.io/client-go/pkg/apis/clientauthentication