Merge pull request #35950 from nikhiljindal/nsdelreuse

Automatic merge from submit-queue

federation: Refactoring namespaced resources deletion code from kube ns controller and sharing it with fed ns controller

Ref https://github.com/kubernetes/kubernetes/issues/33612

Refactoring code in kube namespace controller to delete all resources in a namespace when the namespace is deleted. Refactored this code into a separate NamespacedResourcesDeleter class and calling it from federation namespace controller.
This is required for enabling cascading deletion of namespaced resources in federation apiserver.
Before this PR, we were directly deleting the namespaced resources and assuming that they go away immediately. With cascading deletion, we will have to wait for the corresponding controllers to first delete the resources from underlying clusters and then delete the resource from federation control plane. NamespacedResourcesDeleter has this waiting logic.

cc @kubernetes/sig-federation-misc @caesarxuchao @derekwaynecarr @mwielgus

federation/cmd/federation-controller-manager/app/BUILD

@@ -44,6 +44,7 @@ go_library(
         "//vendor:k8s.io/apiserver/pkg/server/healthz",
         "//vendor:k8s.io/apiserver/pkg/util/flag",
         "//vendor:k8s.io/client-go/discovery",
+        "//vendor:k8s.io/client-go/dynamic",
         "//vendor:k8s.io/client-go/rest",
         "//vendor:k8s.io/client-go/tools/clientcmd",
     ],

federation/cmd/federation-controller-manager/app/controllermanager.go

@@ -30,6 +30,7 @@ import (
 	"k8s.io/apimachinery/pkg/util/wait"
 	"k8s.io/apiserver/pkg/server/healthz"
 	utilflag "k8s.io/apiserver/pkg/util/flag"
+	"k8s.io/client-go/dynamic"
 	restclient "k8s.io/client-go/rest"
 	"k8s.io/client-go/tools/clientcmd"
 	federationclientset "k8s.io/kubernetes/federation/client/clientset_generated/federation_clientset"
@@ -167,7 +168,7 @@ func StartControllers(s *options.CMServer, restClientCfg *restclient.Config) err
 
 	glog.Infof("Loading client config for namespace controller %q", "namespace-controller")
 	nsClientset := federationclientset.NewForConfigOrDie(restclient.AddUserAgent(restClientCfg, "namespace-controller"))
-	namespaceController := namespacecontroller.NewNamespaceController(nsClientset)
+	namespaceController := namespacecontroller.NewNamespaceController(nsClientset, dynamic.NewDynamicClientPool(restclient.AddUserAgent(restClientCfg, "namespace-controller")))
 	glog.Infof("Running namespace controller")
 	namespaceController.Run(wait.NeverStop)
 

federation/pkg/federation-controller/namespace/BUILD

@@ -22,11 +22,13 @@ go_library(
         "//pkg/api/v1:go_default_library",
         "//pkg/client/clientset_generated/clientset:go_default_library",
         "//pkg/controller:go_default_library",
+        "//pkg/controller/namespace/deletion:go_default_library",
         "//vendor:github.com/golang/glog",
         "//vendor:k8s.io/apimachinery/pkg/api/errors",
         "//vendor:k8s.io/apimachinery/pkg/apis/meta/v1",
         "//vendor:k8s.io/apimachinery/pkg/runtime",
         "//vendor:k8s.io/apimachinery/pkg/watch",
+        "//vendor:k8s.io/client-go/dynamic",
         "//vendor:k8s.io/client-go/pkg/api/v1",
         "//vendor:k8s.io/client-go/tools/cache",
         "//vendor:k8s.io/client-go/tools/record",
@@ -46,13 +48,14 @@ go_test(
         "//federation/pkg/federation-controller/util/deletionhelper:go_default_library",
         "//federation/pkg/federation-controller/util/test:go_default_library",
         "//pkg/api/v1:go_default_library",
-        "//pkg/apis/extensions/v1beta1:go_default_library",
         "//pkg/client/clientset_generated/clientset:go_default_library",
         "//pkg/client/clientset_generated/clientset/fake:go_default_library",
         "//vendor:github.com/stretchr/testify/assert",
         "//vendor:k8s.io/apimachinery/pkg/apis/meta/v1",
         "//vendor:k8s.io/apimachinery/pkg/runtime",
         "//vendor:k8s.io/apimachinery/pkg/util/wait",
+        "//vendor:k8s.io/client-go/dynamic",
+        "//vendor:k8s.io/client-go/rest",
         "//vendor:k8s.io/client-go/testing",
     ],
 )

federation/pkg/federation-controller/namespace/namespace_controller.go

@@ -24,6 +24,7 @@ import (
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/watch"
+	"k8s.io/client-go/dynamic"
 	clientv1 "k8s.io/client-go/pkg/api/v1"
 	"k8s.io/client-go/tools/cache"
 	"k8s.io/client-go/tools/record"
@@ -37,6 +38,7 @@ import (
 	apiv1 "k8s.io/kubernetes/pkg/api/v1"
 	kubeclientset "k8s.io/kubernetes/pkg/client/clientset_generated/clientset"
 	"k8s.io/kubernetes/pkg/controller"
+	"k8s.io/kubernetes/pkg/controller/namespace/deletion"
 
 	"github.com/golang/glog"
 )
@@ -75,6 +77,9 @@ type NamespaceController struct {
 
 	deletionHelper *deletionhelper.DeletionHelper
 
+	// Helper to delete all resources in a namespace.
+	namespacedResourcesDeleter deletion.NamespacedResourcesDeleterInterface
+
 	namespaceReviewDelay  time.Duration
 	clusterAvailableDelay time.Duration
 	smallDelay            time.Duration
@@ -82,7 +87,7 @@ type NamespaceController struct {
 }
 
 // NewNamespaceController returns a new namespace controller
-func NewNamespaceController(client federationclientset.Interface) *NamespaceController {
+func NewNamespaceController(client federationclientset.Interface, dynamicClientPool dynamic.ClientPool) *NamespaceController {
 	broadcaster := record.NewBroadcaster()
 	broadcaster.StartRecordingToSink(eventsink.NewFederatedEventSink(client))
 	recorder := broadcaster.NewRecorder(api.Scheme, clientv1.EventSource{Component: "federated-namespace-controller"})
@@ -180,6 +185,11 @@ func NewNamespaceController(client federationclientset.Interface) *NamespaceCont
 		nc.namespaceFederatedInformer,
 		nc.federatedUpdater,
 	)
+
+	discoverResourcesFn := nc.federatedApiClient.Discovery().ServerPreferredNamespacedResources
+	nc.namespacedResourcesDeleter = deletion.NewNamespacedResourcesDeleter(
+		client.Core().Namespaces(), dynamicClientPool, nil,
+		discoverResourcesFn, apiv1.FinalizerKubernetes, false)
 	return nc
 }
 
@@ -463,11 +473,16 @@ func (nc *NamespaceController) delete(namespace *apiv1.Namespace) error {
 
 	if nc.hasFinalizerFuncInSpec(updatedNamespace, apiv1.FinalizerKubernetes) {
 		// Delete resources in this namespace.
-		updatedNamespace, err = nc.removeKubernetesFinalizer(updatedNamespace)
+		err = nc.namespacedResourcesDeleter.Delete(updatedNamespace.Name)
 		if err != nil {
 			return fmt.Errorf("error in deleting resources in namespace %s: %v", namespace.Name, err)
 		}
 		glog.V(2).Infof("Removed kubernetes finalizer from ns %s", namespace.Name)
+		// Fetch the updated Namespace.
+		updatedNamespace, err = nc.federatedApiClient.Core().Namespaces().Get(updatedNamespace.Name, metav1.GetOptions{})
+		if err != nil {
+			return fmt.Errorf("error in fetching updated namespace %s: %s", updatedNamespace.Name, err)
+		}
 	}
 
 	// Delete the namespace from all underlying clusters.
@@ -487,44 +502,3 @@ func (nc *NamespaceController) delete(namespace *apiv1.Namespace) error {
 	}
 	return nil
 }
-
-// Ensures that all resources in this namespace are deleted and then removes the kubernetes finalizer.
-func (nc *NamespaceController) removeKubernetesFinalizer(namespace *apiv1.Namespace) (*apiv1.Namespace, error) {
-	// Right now there are just 7 types of objects: Deployments, DaemonSets, ReplicaSet, Secret, Ingress, Events and Service.
-	// Temporarily these items are simply deleted one by one to squeeze this code into 1.4.
-	// TODO: Make it generic (like in the regular namespace controller) and parallel.
-	err := nc.federatedApiClient.Core().Services(namespace.Name).DeleteCollection(&metav1.DeleteOptions{}, metav1.ListOptions{})
-	if err != nil {
-		return nil, fmt.Errorf("failed to delete service list: %v", err)
-	}
-	err = nc.federatedApiClient.Extensions().ReplicaSets(namespace.Name).DeleteCollection(&metav1.DeleteOptions{}, metav1.ListOptions{})
-	if err != nil {
-		return nil, fmt.Errorf("failed to delete replicaset list from namespace: %v", err)
-	}
-	err = nc.federatedApiClient.Core().Secrets(namespace.Name).DeleteCollection(&metav1.DeleteOptions{}, metav1.ListOptions{})
-	if err != nil {
-		return nil, fmt.Errorf("failed to delete secret list from namespace: %v", err)
-	}
-	err = nc.federatedApiClient.Extensions().Ingresses(namespace.Name).DeleteCollection(&metav1.DeleteOptions{}, metav1.ListOptions{})
-	if err != nil {
-		return nil, fmt.Errorf("failed to delete ingresses list from namespace: %v", err)
-	}
-	err = nc.federatedApiClient.Extensions().DaemonSets(namespace.Name).DeleteCollection(&metav1.DeleteOptions{}, metav1.ListOptions{})
-	if err != nil {
-		return nil, fmt.Errorf("failed to delete daemonsets list from namespace: %v", err)
-	}
-	err = nc.federatedApiClient.Extensions().Deployments(namespace.Name).DeleteCollection(&metav1.DeleteOptions{}, metav1.ListOptions{})
-	if err != nil {
-		return nil, fmt.Errorf("failed to delete deployments list from namespace: %v", err)
-	}
-	err = nc.federatedApiClient.Core().Events(namespace.Name).DeleteCollection(&metav1.DeleteOptions{}, metav1.ListOptions{})
-	if err != nil {
-		return nil, fmt.Errorf("failed to delete events list from namespace: %v", err)
-	}
-
-	// Remove kube_api.FinalizerKubernetes
-	if len(namespace.Spec.Finalizers) != 0 {
-		return nc.removeFinalizerFromSpec(namespace, apiv1.FinalizerKubernetes)
-	}
-	return namespace, nil
-}

federation/pkg/federation-controller/namespace/namespace_controller_test.go

@@ -24,6 +24,8 @@ import (
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/util/wait"
+	"k8s.io/client-go/dynamic"
+	restclient "k8s.io/client-go/rest"
 	core "k8s.io/client-go/testing"
 	federationapi "k8s.io/kubernetes/federation/apis/federation/v1beta1"
 	fakefedclientset "k8s.io/kubernetes/federation/client/clientset_generated/federation_clientset/fake"
@@ -31,7 +33,6 @@ import (
 	"k8s.io/kubernetes/federation/pkg/federation-controller/util/deletionhelper"
 	. "k8s.io/kubernetes/federation/pkg/federation-controller/util/test"
 	apiv1 "k8s.io/kubernetes/pkg/api/v1"
-	extensionsv1 "k8s.io/kubernetes/pkg/apis/extensions/v1beta1"
 	kubeclientset "k8s.io/kubernetes/pkg/client/clientset_generated/clientset"
 	fakekubeclientset "k8s.io/kubernetes/pkg/client/clientset_generated/clientset/fake"
 
@@ -69,30 +70,8 @@ func TestNamespaceController(t *testing.T) {
 	RegisterFakeList("namespaces", &cluster2Client.Fake, &apiv1.NamespaceList{Items: []apiv1.Namespace{}})
 	cluster2CreateChan := RegisterFakeCopyOnCreate("namespaces", &cluster2Client.Fake, cluster2Watch)
 
-	RegisterFakeList("replicasets", &fakeClient.Fake, &extensionsv1.ReplicaSetList{Items: []extensionsv1.ReplicaSet{
-		{
-			ObjectMeta: metav1.ObjectMeta{
-				Name:      "test-rs",
-				Namespace: ns1.Namespace,
-			}}}})
-	RegisterFakeList("secrets", &fakeClient.Fake, &apiv1.SecretList{Items: []apiv1.Secret{
-		{
-			ObjectMeta: metav1.ObjectMeta{
-				Name:      "test-secret",
-				Namespace: ns1.Namespace,
-			}}}})
-	RegisterFakeList("services", &fakeClient.Fake, &apiv1.ServiceList{Items: []apiv1.Service{
-		{
-			ObjectMeta: metav1.ObjectMeta{
-				Name:      "test-service",
-				Namespace: ns1.Namespace,
-			}}}})
 	nsDeleteChan := RegisterDelete(&fakeClient.Fake, "namespaces")
-	rsDeleteChan := RegisterDeleteCollection(&fakeClient.Fake, "replicasets")
+	namespaceController := NewNamespaceController(fakeClient, dynamic.NewDynamicClientPool(&restclient.Config{}))
-	serviceDeleteChan := RegisterDeleteCollection(&fakeClient.Fake, "services")
-	secretDeleteChan := RegisterDeleteCollection(&fakeClient.Fake, "secrets")
-
-	namespaceController := NewNamespaceController(fakeClient)
 	informerClientFactory := func(cluster *federationapi.Cluster) (kubeclientset.Interface, error) {
 		switch cluster.Name {
 		case cluster1.Name:
@@ -153,10 +132,12 @@ func TestNamespaceController(t *testing.T) {
 	ns1.DeletionTimestamp = &metav1.Time{Time: time.Now()}
 	namespaceWatch.Modify(&ns1)
 	assert.Equal(t, ns1.Name, GetStringFromChan(nsDeleteChan))
-	assert.Equal(t, "all", GetStringFromChan(rsDeleteChan))
+	// TODO: Add a test for verifying that resources in the namespace are deleted
-	assert.Equal(t, "all", GetStringFromChan(serviceDeleteChan))
+	// when the namespace is deleted.
-	assert.Equal(t, "all", GetStringFromChan(secretDeleteChan))
+	// Need a fake dynamic client to mock list and delete actions to be able to test this.
-
+	// TODO: Add a fake dynamic client and test this.
+	// In the meantime, e2e test verify that the resources in a namespace are
+	// deleted when the namespace is deleted.
 	close(stop)
 }
 

pkg/controller/namespace/BUILD

@@ -5,7 +5,6 @@ licenses(["notice"])
 load(
     "@io_bazel_rules_go//go:def.bzl",
     "go_library",
-    "go_test",
 )
 
 go_library(
@@ -13,53 +12,26 @@ go_library(
     srcs = [
         "doc.go",
         "namespace_controller.go",
-        "namespace_controller_utils.go",
     ],
     tags = ["automanaged"],
     deps = [
         "//pkg/api/v1:go_default_library",
         "//pkg/client/clientset_generated/clientset:go_default_library",
         "//pkg/controller:go_default_library",
+        "//pkg/controller/namespace/deletion:go_default_library",
         "//pkg/util/metrics:go_default_library",
         "//vendor:github.com/golang/glog",
-        "//vendor:k8s.io/apimachinery/pkg/api/errors",
         "//vendor:k8s.io/apimachinery/pkg/apis/meta/v1",
-        "//vendor:k8s.io/apimachinery/pkg/apis/meta/v1/unstructured",
         "//vendor:k8s.io/apimachinery/pkg/runtime",
-        "//vendor:k8s.io/apimachinery/pkg/runtime/schema",
         "//vendor:k8s.io/apimachinery/pkg/util/runtime",
-        "//vendor:k8s.io/apimachinery/pkg/util/sets",
         "//vendor:k8s.io/apimachinery/pkg/util/wait",
         "//vendor:k8s.io/apimachinery/pkg/watch",
-        "//vendor:k8s.io/client-go/discovery",
         "//vendor:k8s.io/client-go/dynamic",
         "//vendor:k8s.io/client-go/tools/cache",
         "//vendor:k8s.io/client-go/util/workqueue",
     ],
 )
 
-go_test(
-    name = "go_default_test",
-    srcs = ["namespace_controller_test.go"],
-    library = ":go_default_library",
-    tags = ["automanaged"],
-    deps = [
-        "//pkg/api:go_default_library",
-        "//pkg/api/v1:go_default_library",
-        "//pkg/client/clientset_generated/clientset:go_default_library",
-        "//pkg/client/clientset_generated/clientset/fake:go_default_library",
-        "//vendor:k8s.io/apimachinery/pkg/api/errors",
-        "//vendor:k8s.io/apimachinery/pkg/apis/meta/v1",
-        "//vendor:k8s.io/apimachinery/pkg/runtime",
-        "//vendor:k8s.io/apimachinery/pkg/runtime/schema",
-        "//vendor:k8s.io/apimachinery/pkg/util/sets",
-        "//vendor:k8s.io/client-go/discovery",
-        "//vendor:k8s.io/client-go/dynamic",
-        "//vendor:k8s.io/client-go/rest",
-        "//vendor:k8s.io/client-go/testing",
-    ],
-)
-
 filegroup(
     name = "package-srcs",
     srcs = glob(["**"]),
@@ -69,6 +41,9 @@ filegroup(
 
 filegroup(
     name = "all-srcs",
-    srcs = [":package-srcs"],
+    srcs = [
+        ":package-srcs",
+        "//pkg/controller/namespace/deletion:all-srcs",
+    ],
     tags = ["automanaged"],
 )

pkg/controller/namespace/deletion/BUILD

@@ -0,0 +1,61 @@
+package(default_visibility = ["//visibility:public"])
+
+licenses(["notice"])
+
+load(
+    "@io_bazel_rules_go//go:def.bzl",
+    "go_library",
+    "go_test",
+)
+
+go_library(
+    name = "go_default_library",
+    srcs = ["namespaced_resources_deleter.go"],
+    tags = ["automanaged"],
+    deps = [
+        "//pkg/api/v1:go_default_library",
+        "//pkg/client/clientset_generated/clientset/typed/core/v1:go_default_library",
+        "//vendor:github.com/golang/glog",
+        "//vendor:k8s.io/apimachinery/pkg/api/errors",
+        "//vendor:k8s.io/apimachinery/pkg/apis/meta/v1",
+        "//vendor:k8s.io/apimachinery/pkg/apis/meta/v1/unstructured",
+        "//vendor:k8s.io/apimachinery/pkg/runtime/schema",
+        "//vendor:k8s.io/apimachinery/pkg/util/sets",
+        "//vendor:k8s.io/client-go/discovery",
+        "//vendor:k8s.io/client-go/dynamic",
+    ],
+)
+
+go_test(
+    name = "go_default_test",
+    srcs = ["namespaced_resources_deleter_test.go"],
+    library = ":go_default_library",
+    tags = ["automanaged"],
+    deps = [
+        "//pkg/api:go_default_library",
+        "//pkg/api/v1:go_default_library",
+        "//pkg/client/clientset_generated/clientset/fake:go_default_library",
+        "//vendor:k8s.io/apimachinery/pkg/api/errors",
+        "//vendor:k8s.io/apimachinery/pkg/apis/meta/v1",
+        "//vendor:k8s.io/apimachinery/pkg/runtime",
+        "//vendor:k8s.io/apimachinery/pkg/runtime/schema",
+        "//vendor:k8s.io/apimachinery/pkg/util/sets",
+        "//vendor:k8s.io/client-go/discovery",
+        "//vendor:k8s.io/client-go/dynamic",
+        "//vendor:k8s.io/client-go/rest",
+        "//vendor:k8s.io/client-go/testing",
+    ],
+)
+
+filegroup(
+    name = "package-srcs",
+    srcs = glob(["**"]),
+    tags = ["automanaged"],
+    visibility = ["//visibility:private"],
+)
+
+filegroup(
+    name = "all-srcs",
+    srcs = [":package-srcs"],
+    tags = ["automanaged"],
+)

pkg/controller/namespace/deletion/namespaced_resources_deleter.go

@@ -14,10 +14,11 @@ See the License for the specific language governing permissions and
 limitations under the License.
 */
 
-package namespace
+package deletion
 
 import (
 	"fmt"
+	"reflect"
 	"sync"
 	"time"
 
@@ -28,18 +29,190 @@ import (
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 	"k8s.io/apimachinery/pkg/runtime/schema"
 	"k8s.io/apimachinery/pkg/util/sets"
+	"k8s.io/kubernetes/pkg/api/v1"
+	// "k8s.io/kubernetes/pkg/client/clientset_generated/clientset"
 	"k8s.io/client-go/discovery"
 	"k8s.io/client-go/dynamic"
-	"k8s.io/kubernetes/pkg/api/v1"
+	v1clientset "k8s.io/kubernetes/pkg/client/clientset_generated/clientset/typed/core/v1"
-	"k8s.io/kubernetes/pkg/client/clientset_generated/clientset"
 )
 
-// contentRemainingError is used to inform the caller that content is not fully removed from the namespace
+// Interface to delete a namespace with all resources in it.
-type contentRemainingError struct {
+type NamespacedResourcesDeleterInterface interface {
+	Delete(nsName string) error
+}
+
+func NewNamespacedResourcesDeleter(nsClient v1clientset.NamespaceInterface,
+	clientPool dynamic.ClientPool, podsGetter v1clientset.PodsGetter,
+	discoverResourcesFn func() ([]*metav1.APIResourceList, error),
+	finalizerToken v1.FinalizerName, deleteNamespaceWhenDone bool) NamespacedResourcesDeleterInterface {
+	d := &namespacedResourcesDeleter{
+		nsClient:   nsClient,
+		clientPool: clientPool,
+		podsGetter: podsGetter,
+		opCache: &operationNotSupportedCache{
+			m: make(map[operationKey]bool),
+		},
+		discoverResourcesFn:     discoverResourcesFn,
+		finalizerToken:          finalizerToken,
+		deleteNamespaceWhenDone: deleteNamespaceWhenDone,
+	}
+	d.initOpCache()
+	return d
+}
+
+var _ NamespacedResourcesDeleterInterface = &namespacedResourcesDeleter{}
+
+// namespacedResourcesDeleter is used to delete all resources in a given namespace.
+type namespacedResourcesDeleter struct {
+	// Client to manipulate the namespace.
+	nsClient v1clientset.NamespaceInterface
+	// Dynamic client to list and delete all namespaced resources.
+	clientPool dynamic.ClientPool
+	// Interface to get PodInterface.
+	podsGetter v1clientset.PodsGetter
+	// Cache of what operations are not supported on each group version resource.
+	opCache             *operationNotSupportedCache
+	discoverResourcesFn func() ([]*metav1.APIResourceList, error)
+	// 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.
+// Before deleting resources:
+// * It ensures that deletion timestamp is set on the
+//   namespace (does nothing if deletion timestamp is missing).
+// * Verifies that the namespace is in the "terminating" phase
+//   (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
+// to wait for them to go away.
+// Caller is expected to keep calling this until it succeeds.
+func (d *namespacedResourcesDeleter) Delete(nsName string) error {
+	// Multiple controllers may edit a namespace during termination
+	// first get the latest state of the namespace before proceeding
+	// if the namespace was deleted already, don't do anything
+	namespace, err := d.nsClient.Get(nsName, metav1.GetOptions{})
+	if err != nil {
+		if errors.IsNotFound(err) {
+			return nil
+		}
+		return err
+	}
+	if namespace.DeletionTimestamp == nil {
+		return nil
+	}
+
+	glog.V(5).Infof("namespace controller - syncNamespace - namespace: %s, finalizerToken: %s", namespace.Name, d.finalizerToken)
+
+	// ensure that the status is up to date on the namespace
+	// if we get a not found error, we assume the namespace is truly gone
+	namespace, err = d.retryOnConflictError(namespace, d.updateNamespaceStatusFunc)
+	if err != nil {
+		if errors.IsNotFound(err) {
+			return nil
+		}
+		return err
+	}
+
+	// the latest view of the namespace asserts that namespace is no longer deleting..
+	if namespace.DeletionTimestamp.IsZero() {
+		return nil
+	}
+
+	// Delete the namespace if it is already finalized.
+	if d.deleteNamespaceWhenDone && finalized(namespace) {
+		return d.deleteNamespace(namespace)
+	}
+
+	// there may still be content for us to remove
+	estimate, err := d.deleteAllContent(namespace.Name, *namespace.DeletionTimestamp)
+	if err != nil {
+		return err
+	}
+	if estimate > 0 {
+		return &ResourcesRemainingError{estimate}
+	}
+
+	// we have removed content, so mark it finalized by us
+	namespace, err = d.retryOnConflictError(namespace, d.finalizeNamespace)
+	if err != nil {
+		// in normal practice, this should not be possible, but if a deployment is running
+		// two controllers to do namespace deletion that share a common finalizer token it's
+		// possible that a not found could occur since the other controller would have finished the delete.
+		if errors.IsNotFound(err) {
+			return nil
+		}
+		return err
+	}
+
+	// Check if we can delete now.
+	if d.deleteNamespaceWhenDone && finalized(namespace) {
+		return d.deleteNamespace(namespace)
+	}
+	return nil
+}
+
+func (d *namespacedResourcesDeleter) initOpCache() {
+	// pre-fill opCache with the discovery info
+	//
+	// TODO(sttts): get rid of opCache and http 405 logic around it and trust discovery info
+	resources, err := d.discoverResourcesFn()
+	if err != nil {
+		glog.Fatalf("Failed to get supported resources: %v", err)
+	}
+	deletableGroupVersionResources := []schema.GroupVersionResource{}
+	for _, rl := range resources {
+		gv, err := schema.ParseGroupVersion(rl.GroupVersion)
+		if err != nil {
+			glog.Errorf("Failed to parse GroupVersion %q, skipping: %v", rl.GroupVersion, err)
+			continue
+		}
+
+		for _, r := range rl.APIResources {
+			gvr := schema.GroupVersionResource{Group: gv.Group, Version: gv.Version, Resource: r.Name}
+			verbs := sets.NewString([]string(r.Verbs)...)
+
+			if !verbs.Has("delete") {
+				glog.V(6).Infof("Skipping resource %v because it cannot be deleted.", gvr)
+			}
+
+			for _, op := range []operation{operationList, operationDeleteCollection} {
+				if !verbs.Has(string(op)) {
+					d.opCache.setNotSupported(operationKey{operation: op, gvr: gvr})
+				}
+			}
+			deletableGroupVersionResources = append(deletableGroupVersionResources, gvr)
+		}
+	}
+}
+
+// Deletes the given namespace.
+func (d *namespacedResourcesDeleter) deleteNamespace(namespace *v1.Namespace) error {
+	var opts *metav1.DeleteOptions
+	uid := namespace.UID
+	if len(uid) > 0 {
+		opts = &metav1.DeleteOptions{Preconditions: &metav1.Preconditions{UID: &uid}}
+	}
+	err := d.nsClient.Delete(namespace.Name, opts)
+	if err != nil && !errors.IsNotFound(err) {
+		return err
+	}
+	return nil
+}
+
+// ResourcesRemainingError is used to inform the caller that all resources are not yet fully removed from the namespace.
+type ResourcesRemainingError struct {
 	Estimate int64
 }
 
-func (e *contentRemainingError) Error() string {
+func (e *ResourcesRemainingError) Error() string {
 	return fmt.Sprintf("some content remains in the namespace, estimate %d seconds before it is removed", e.Estimate)
 }
 
@@ -47,7 +220,7 @@ func (e *contentRemainingError) Error() string {
 type operation string
 
 const (
-	operationDeleteCollection operation = "deleteCollection"
+	operationDeleteCollection operation = "deletecollection"
 	operationList             operation = "list"
 	// assume a default estimate for finalizers to complete when found on items pending deletion.
 	finalizerEstimateSeconds int64 = int64(15)
@@ -55,7 +228,7 @@ const (
 
 // operationKey is an entry in a cache.
 type operationKey struct {
-	op  operation
+	operation operation
 	gvr       schema.GroupVersionResource
 }
 
@@ -80,15 +253,15 @@ func (o *operationNotSupportedCache) setNotSupported(key operationKey) {
 }
 
 // updateNamespaceFunc is a function that makes an update to a namespace
-type updateNamespaceFunc func(kubeClient clientset.Interface, namespace *v1.Namespace) (*v1.Namespace, error)
+type updateNamespaceFunc func(namespace *v1.Namespace) (*v1.Namespace, error)
 
 // retryOnConflictError retries the specified fn if there was a conflict error
 // it will return an error if the UID for an object changes across retry operations.
 // TODO RetryOnConflict should be a generic concept in client code
-func retryOnConflictError(kubeClient clientset.Interface, namespace *v1.Namespace, fn updateNamespaceFunc) (result *v1.Namespace, err error) {
+func (d *namespacedResourcesDeleter) retryOnConflictError(namespace *v1.Namespace, fn updateNamespaceFunc) (result *v1.Namespace, err error) {
 	latestNamespace := namespace
 	for {
-		result, err = fn(kubeClient, latestNamespace)
+		result, err = fn(latestNamespace)
 		if err == nil {
 			return result, nil
 		}
@@ -96,7 +269,7 @@ func retryOnConflictError(kubeClient clientset.Interface, namespace *v1.Namespac
 			return nil, err
 		}
 		prevNamespace := latestNamespace
-		latestNamespace, err = kubeClient.Core().Namespaces().Get(latestNamespace.Name, metav1.GetOptions{})
+		latestNamespace, err = d.nsClient.Get(latestNamespace.Name, metav1.GetOptions{})
 		if err != nil {
 			return nil, err
 		}
@@ -107,7 +280,7 @@ func retryOnConflictError(kubeClient clientset.Interface, namespace *v1.Namespac
 }
 
 // updateNamespaceStatusFunc will verify that the status of the namespace is correct
-func updateNamespaceStatusFunc(kubeClient clientset.Interface, namespace *v1.Namespace) (*v1.Namespace, error) {
+func (d *namespacedResourcesDeleter) updateNamespaceStatusFunc(namespace *v1.Namespace) (*v1.Namespace, error) {
 	if namespace.DeletionTimestamp.IsZero() || namespace.Status.Phase == v1.NamespaceTerminating {
 		return namespace, nil
 	}
@@ -115,7 +288,7 @@ func updateNamespaceStatusFunc(kubeClient clientset.Interface, namespace *v1.Nam
 	newNamespace.ObjectMeta = namespace.ObjectMeta
 	newNamespace.Status = namespace.Status
 	newNamespace.Status.Phase = v1.NamespaceTerminating
-	return kubeClient.Core().Namespaces().UpdateStatus(&newNamespace)
+	return d.nsClient.UpdateStatus(&newNamespace)
 }
 
 // finalized returns true if the namespace.Spec.Finalizers is an empty list
@@ -123,21 +296,14 @@ func finalized(namespace *v1.Namespace) bool {
 	return len(namespace.Spec.Finalizers) == 0
 }
 
-// finalizeNamespaceFunc returns a function that knows how to finalize a namespace for specified token.
-func finalizeNamespaceFunc(finalizerToken v1.FinalizerName) updateNamespaceFunc {
-	return func(kubeClient clientset.Interface, namespace *v1.Namespace) (*v1.Namespace, error) {
-		return finalizeNamespace(kubeClient, namespace, finalizerToken)
-	}
-}
-
 // finalizeNamespace removes the specified finalizerToken and finalizes the namespace
-func finalizeNamespace(kubeClient clientset.Interface, namespace *v1.Namespace, finalizerToken v1.FinalizerName) (*v1.Namespace, error) {
+func (d *namespacedResourcesDeleter) finalizeNamespace(namespace *v1.Namespace) (*v1.Namespace, error) {
 	namespaceFinalize := v1.Namespace{}
 	namespaceFinalize.ObjectMeta = namespace.ObjectMeta
 	namespaceFinalize.Spec = namespace.Spec
 	finalizerSet := sets.NewString()
 	for i := range namespace.Spec.Finalizers {
-		if namespace.Spec.Finalizers[i] != finalizerToken {
+		if namespace.Spec.Finalizers[i] != d.finalizerToken {
 			finalizerSet.Insert(string(namespace.Spec.Finalizers[i]))
 		}
 	}
@@ -145,7 +311,7 @@ func finalizeNamespace(kubeClient clientset.Interface, namespace *v1.Namespace,
 	for _, value := range finalizerSet.List() {
 		namespaceFinalize.Spec.Finalizers = append(namespaceFinalize.Spec.Finalizers, v1.FinalizerName(value))
 	}
-	namespace, err := kubeClient.Core().Namespaces().Finalize(&namespaceFinalize)
+	namespace, err := d.nsClient.Finalize(&namespaceFinalize)
 	if err != nil {
 		// it was removed already, so life is good
 		if errors.IsNotFound(err) {
@@ -158,16 +324,13 @@ func finalizeNamespace(kubeClient clientset.Interface, namespace *v1.Namespace,
 // deleteCollection is a helper function that will delete the collection of resources
 // it returns true if the operation was supported on the server.
 // it returns an error if the operation was supported on the server but was unable to complete.
-func deleteCollection(
+func (d *namespacedResourcesDeleter) deleteCollection(
-	dynamicClient *dynamic.Client,
+	dynamicClient *dynamic.Client, gvr schema.GroupVersionResource,
-	opCache *operationNotSupportedCache,
+	namespace string) (bool, error) {
-	gvr schema.GroupVersionResource,
-	namespace string,
-) (bool, error) {
 	glog.V(5).Infof("namespace controller - deleteCollection - namespace: %s, gvr: %v", namespace, gvr)
 
-	key := operationKey{op: operationDeleteCollection, gvr: gvr}
+	key := operationKey{operation: operationDeleteCollection, gvr: gvr}
-	if !opCache.isSupported(key) {
+	if !d.opCache.isSupported(key) {
 		glog.V(5).Infof("namespace controller - deleteCollection ignored since not supported - namespace: %s, gvr: %v", namespace, gvr)
 		return false, nil
 	}
@@ -192,7 +355,7 @@ func deleteCollection(
 	// remember next time that this resource does not support delete collection...
 	if errors.IsMethodNotSupported(err) || errors.IsNotFound(err) {
 		glog.V(5).Infof("namespace controller - deleteCollection not supported - namespace: %s, gvr: %v", namespace, gvr)
-		opCache.setNotSupported(key)
+		d.opCache.setNotSupported(key)
 		return false, nil
 	}
 
@@ -205,16 +368,12 @@ func deleteCollection(
 //  the list of items in the collection (if found)
 //  a boolean if the operation is supported
 //  an error if the operation is supported but could not be completed.
-func listCollection(
+func (d *namespacedResourcesDeleter) listCollection(
-	dynamicClient *dynamic.Client,
+	dynamicClient *dynamic.Client, gvr schema.GroupVersionResource, namespace string) (*unstructured.UnstructuredList, bool, error) {
-	opCache *operationNotSupportedCache,
-	gvr schema.GroupVersionResource,
-	namespace string,
-) (*unstructured.UnstructuredList, bool, error) {
 	glog.V(5).Infof("namespace controller - listCollection - namespace: %s, gvr: %v", namespace, gvr)
 
-	key := operationKey{op: operationList, gvr: gvr}
+	key := operationKey{operation: operationList, gvr: gvr}
-	if !opCache.isSupported(key) {
+	if !d.opCache.isSupported(key) {
 		glog.V(5).Infof("namespace controller - listCollection ignored since not supported - namespace: %s, gvr: %v", namespace, gvr)
 		return nil, false, nil
 	}
@@ -237,7 +396,7 @@ func listCollection(
 	// remember next time that this resource does not support delete collection...
 	if errors.IsMethodNotSupported(err) || errors.IsNotFound(err) {
 		glog.V(5).Infof("namespace controller - listCollection not supported - namespace: %s, gvr: %v", namespace, gvr)
-		opCache.setNotSupported(key)
+		d.opCache.setNotSupported(key)
 		return nil, false, nil
 	}
 
@@ -245,15 +404,11 @@ func listCollection(
 }
 
 // deleteEachItem is a helper function that will list the collection of resources and delete each item 1 by 1.
-func deleteEachItem(
+func (d *namespacedResourcesDeleter) deleteEachItem(
-	dynamicClient *dynamic.Client,
+	dynamicClient *dynamic.Client, gvr schema.GroupVersionResource, namespace string) error {
-	opCache *operationNotSupportedCache,
-	gvr schema.GroupVersionResource,
-	namespace string,
-) error {
 	glog.V(5).Infof("namespace controller - deleteEachItem - namespace: %s, gvr: %v", namespace, gvr)
 
-	unstructuredList, listSupported, err := listCollection(dynamicClient, opCache, gvr, namespace)
+	unstructuredList, listSupported, err := d.listCollection(dynamicClient, gvr, namespace)
 	if err != nil {
 		return err
 	}
@@ -272,18 +427,13 @@ func deleteEachItem(
 // deleteAllContentForGroupVersionResource will use the dynamic client to delete each resource identified in gvr.
 // It returns an estimate of the time remaining before the remaining resources are deleted.
 // If estimate > 0, not all resources are guaranteed to be gone.
-func deleteAllContentForGroupVersionResource(
+func (d *namespacedResourcesDeleter) deleteAllContentForGroupVersionResource(
-	kubeClient clientset.Interface,
+	gvr schema.GroupVersionResource, namespace string,
-	clientPool dynamic.ClientPool,
+	namespaceDeletedAt metav1.Time) (int64, error) {
-	opCache *operationNotSupportedCache,
-	gvr schema.GroupVersionResource,
-	namespace string,
-	namespaceDeletedAt metav1.Time,
-) (int64, error) {
 	glog.V(5).Infof("namespace controller - deleteAllContentForGroupVersionResource - namespace: %s, gvr: %v", namespace, gvr)
 
 	// estimate how long it will take for the resource to be deleted (needed for objects that support graceful delete)
-	estimate, err := estimateGracefulTermination(kubeClient, gvr, namespace, namespaceDeletedAt)
+	estimate, err := d.estimateGracefulTermination(gvr, namespace, namespaceDeletedAt)
 	if err != nil {
 		glog.V(5).Infof("namespace controller - deleteAllContentForGroupVersionResource - unable to estimate - namespace: %s, gvr: %v, err: %v", namespace, gvr, err)
 		return estimate, err
@@ -291,21 +441,21 @@ func deleteAllContentForGroupVersionResource(
 	glog.V(5).Infof("namespace controller - deleteAllContentForGroupVersionResource - estimate - namespace: %s, gvr: %v, estimate: %v", namespace, gvr, estimate)
 
 	// get a client for this group version...
-	dynamicClient, err := clientPool.ClientForGroupVersionResource(gvr)
+	dynamicClient, err := d.clientPool.ClientForGroupVersionResource(gvr)
 	if err != nil {
 		glog.V(5).Infof("namespace controller - deleteAllContentForGroupVersionResource - unable to get client - namespace: %s, gvr: %v, err: %v", namespace, gvr, err)
 		return estimate, err
 	}
 
 	// first try to delete the entire collection
-	deleteCollectionSupported, err := deleteCollection(dynamicClient, opCache, gvr, namespace)
+	deleteCollectionSupported, err := d.deleteCollection(dynamicClient, gvr, namespace)
 	if err != nil {
 		return estimate, err
 	}
 
 	// delete collection was not supported, so we list and delete each item...
 	if !deleteCollectionSupported {
-		err = deleteEachItem(dynamicClient, opCache, gvr, namespace)
+		err = d.deleteEachItem(dynamicClient, gvr, namespace)
 		if err != nil {
 			return estimate, err
 		}
@@ -314,7 +464,7 @@ func deleteAllContentForGroupVersionResource(
 	// verify there are no more remaining items
 	// it is not an error condition for there to be remaining items if local estimate is non-zero
 	glog.V(5).Infof("namespace controller - deleteAllContentForGroupVersionResource - checking for no more items in namespace: %s, gvr: %v", namespace, gvr)
-	unstructuredList, listSupported, err := listCollection(dynamicClient, opCache, gvr, namespace)
+	unstructuredList, listSupported, err := d.listCollection(dynamicClient, gvr, namespace)
 	if err != nil {
 		glog.V(5).Infof("namespace controller - deleteAllContentForGroupVersionResource - error verifying no items in namespace: %s, gvr: %v, err: %v", namespace, gvr, err)
 		return estimate, err
@@ -340,18 +490,22 @@ func deleteAllContentForGroupVersionResource(
 // deleteAllContent will use the dynamic client to delete each resource identified in groupVersionResources.
 // It returns an estimate of the time remaining before the remaining resources are deleted.
 // If estimate > 0, not all resources are guaranteed to be gone.
-func deleteAllContent(
+func (d *namespacedResourcesDeleter) deleteAllContent(
-	kubeClient clientset.Interface,
+	namespace string, namespaceDeletedAt metav1.Time) (int64, error) {
-	clientPool dynamic.ClientPool,
-	opCache *operationNotSupportedCache,
-	groupVersionResources map[schema.GroupVersionResource]struct{},
-	namespace string,
-	namespaceDeletedAt metav1.Time,
-) (int64, error) {
 	estimate := int64(0)
-	glog.V(4).Infof("namespace controller - deleteAllContent - namespace: %s, gvrs: %v", namespace, groupVersionResources)
+	glog.V(4).Infof("namespace controller - deleteAllContent - namespace: %s", namespace)
+	resources, err := d.discoverResourcesFn()
+	if err != nil {
+		return estimate, err
+	}
+	// TODO(sttts): get rid of opCache and pass the verbs (especially "deletecollection") down into the deleter
+	deletableResources := discovery.FilteredBy(discovery.SupportsAllVerbs{Verbs: []string{"delete"}}, resources)
+	groupVersionResources, err := discovery.GroupVersionResources(deletableResources)
+	if err != nil {
+		return estimate, err
+	}
 	for gvr := range groupVersionResources {
-		gvrEstimate, err := deleteAllContentForGroupVersionResource(kubeClient, clientPool, opCache, gvr, namespace, namespaceDeletedAt)
+		gvrEstimate, err := d.deleteAllContentForGroupVersionResource(gvr, namespace, namespaceDeletedAt)
 		if err != nil {
 			return estimate, err
 		}
@@ -363,112 +517,15 @@ func deleteAllContent(
 	return estimate, nil
 }
 
-// syncNamespace orchestrates deletion of a Namespace and its associated content.
-func syncNamespace(
-	kubeClient clientset.Interface,
-	clientPool dynamic.ClientPool,
-	opCache *operationNotSupportedCache,
-	discoverResourcesFn func() ([]*metav1.APIResourceList, error),
-	namespace *v1.Namespace,
-	finalizerToken v1.FinalizerName,
-) error {
-	if namespace.DeletionTimestamp == nil {
-		return nil
-	}
-
-	// multiple controllers may edit a namespace during termination
-	// first get the latest state of the namespace before proceeding
-	// if the namespace was deleted already, don't do anything
-	namespace, err := kubeClient.Core().Namespaces().Get(namespace.Name, metav1.GetOptions{})
-	if err != nil {
-		if errors.IsNotFound(err) {
-			return nil
-		}
-		return err
-	}
-
-	glog.V(5).Infof("namespace controller - syncNamespace - namespace: %s, finalizerToken: %s", namespace.Name, finalizerToken)
-
-	// ensure that the status is up to date on the namespace
-	// if we get a not found error, we assume the namespace is truly gone
-	namespace, err = retryOnConflictError(kubeClient, namespace, updateNamespaceStatusFunc)
-	if err != nil {
-		if errors.IsNotFound(err) {
-			return nil
-		}
-		return err
-	}
-
-	// the latest view of the namespace asserts that namespace is no longer deleting..
-	if namespace.DeletionTimestamp.IsZero() {
-		return nil
-	}
-
-	// if the namespace is already finalized, delete it
-	if finalized(namespace) {
-		var opts *metav1.DeleteOptions
-		uid := namespace.UID
-		if len(uid) > 0 {
-			opts = &metav1.DeleteOptions{Preconditions: &metav1.Preconditions{UID: &uid}}
-		}
-		err = kubeClient.Core().Namespaces().Delete(namespace.Name, opts)
-		if err != nil && !errors.IsNotFound(err) {
-			return err
-		}
-		return nil
-	}
-
-	// there may still be content for us to remove
-	resources, err := discoverResourcesFn()
-	if err != nil {
-		return err
-	}
-	// TODO(sttts): get rid of opCache and pass the verbs (especially "deletecollection") down into the deleter
-	deletableResources := discovery.FilteredBy(discovery.SupportsAllVerbs{Verbs: []string{"delete"}}, resources)
-	groupVersionResources, err := discovery.GroupVersionResources(deletableResources)
-	if err != nil {
-		return err
-	}
-	estimate, err := deleteAllContent(kubeClient, clientPool, opCache, groupVersionResources, namespace.Name, *namespace.DeletionTimestamp)
-	if err != nil {
-		return err
-	}
-	if estimate > 0 {
-		return &contentRemainingError{estimate}
-	}
-
-	// we have removed content, so mark it finalized by us
-	result, err := retryOnConflictError(kubeClient, namespace, finalizeNamespaceFunc(finalizerToken))
-	if err != nil {
-		// in normal practice, this should not be possible, but if a deployment is running
-		// two controllers to do namespace deletion that share a common finalizer token it's
-		// possible that a not found could occur since the other controller would have finished the delete.
-		if errors.IsNotFound(err) {
-			return nil
-		}
-		return err
-	}
-
-	// now check if all finalizers have reported that we delete now
-	if finalized(result) {
-		err = kubeClient.Core().Namespaces().Delete(namespace.Name, nil)
-		if err != nil && !errors.IsNotFound(err) {
-			return err
-		}
-	}
-
-	return nil
-}
-
 // estimateGrracefulTermination will estimate the graceful termination required for the specific entity in the namespace
-func estimateGracefulTermination(kubeClient clientset.Interface, groupVersionResource schema.GroupVersionResource, ns string, namespaceDeletedAt metav1.Time) (int64, error) {
+func (d *namespacedResourcesDeleter) estimateGracefulTermination(gvr schema.GroupVersionResource, ns string, namespaceDeletedAt metav1.Time) (int64, error) {
-	groupResource := groupVersionResource.GroupResource()
+	groupResource := gvr.GroupResource()
 	glog.V(5).Infof("namespace controller - estimateGracefulTermination - group %s, resource: %s", groupResource.Group, groupResource.Resource)
 	estimate := int64(0)
 	var err error
 	switch groupResource {
 	case schema.GroupResource{Group: "", Resource: "pods"}:
-		estimate, err = estimateGracefulTerminationForPods(kubeClient, ns)
+		estimate, err = d.estimateGracefulTerminationForPods(ns)
 	}
 	if err != nil {
 		return estimate, err
@@ -483,21 +540,26 @@ func estimateGracefulTermination(kubeClient clientset.Interface, groupVersionRes
 }
 
 // estimateGracefulTerminationForPods determines the graceful termination period for pods in the namespace
-func estimateGracefulTerminationForPods(kubeClient clientset.Interface, ns string) (int64, error) {
+func (d *namespacedResourcesDeleter) estimateGracefulTerminationForPods(ns string) (int64, error) {
 	glog.V(5).Infof("namespace controller - estimateGracefulTerminationForPods - namespace %s", ns)
 	estimate := int64(0)
-	items, err := kubeClient.Core().Pods(ns).List(metav1.ListOptions{})
+	podsGetter := d.podsGetter
+	if podsGetter == nil || reflect.ValueOf(podsGetter).IsNil() {
+		return estimate, fmt.Errorf("unexpected: podsGetter is nil. Cannot estimate grace period seconds for pods")
+	}
+	items, err := podsGetter.Pods(ns).List(metav1.ListOptions{})
 	if err != nil {
 		return estimate, err
 	}
 	for i := range items.Items {
+		pod := items.Items[i]
 		// filter out terminal pods
-		phase := items.Items[i].Status.Phase
+		phase := pod.Status.Phase
 		if v1.PodSucceeded == phase || v1.PodFailed == phase {
 			continue
 		}
-		if items.Items[i].Spec.TerminationGracePeriodSeconds != nil {
+		if pod.Spec.TerminationGracePeriodSeconds != nil {
-			grace := *items.Items[i].Spec.TerminationGracePeriodSeconds
+			grace := *pod.Spec.TerminationGracePeriodSeconds
 			if grace > estimate {
 				estimate = grace
 			}

pkg/controller/namespace/deletion/namespaced_resources_deleter_test.go

@@ -14,7 +14,7 @@ See the License for the specific language governing permissions and
 limitations under the License.
 */
 
-package namespace
+package deletion
 
 import (
 	"fmt"
@@ -36,7 +36,6 @@ import (
 	core "k8s.io/client-go/testing"
 	"k8s.io/kubernetes/pkg/api"
 	"k8s.io/kubernetes/pkg/api/v1"
-	"k8s.io/kubernetes/pkg/client/clientset_generated/clientset"
 	"k8s.io/kubernetes/pkg/client/clientset_generated/clientset/fake"
 )
 
@@ -66,7 +65,11 @@ func TestFinalizeNamespaceFunc(t *testing.T) {
 			Finalizers: []v1.FinalizerName{"kubernetes", "other"},
 		},
 	}
-	finalizeNamespace(mockClient, testNamespace, v1.FinalizerKubernetes)
+	d := namespacedResourcesDeleter{
+		nsClient:       mockClient.Core().Namespaces(),
+		finalizerToken: v1.FinalizerKubernetes,
+	}
+	d.finalizeNamespace(testNamespace)
 	actions := mockClient.Actions()
 	if len(actions) != 1 {
 		t.Errorf("Expected 1 mock client action, but got %v", len(actions))
@@ -174,8 +177,8 @@ func testSyncNamespaceThatIsTerminating(t *testing.T, versions *metav1.APIVersio
 		fn := func() ([]*metav1.APIResourceList, error) {
 			return resources, nil
 		}
-
+		d := NewNamespacedResourcesDeleter(mockClient.Core().Namespaces(), clientPool, mockClient.Core(), fn, v1.FinalizerKubernetes, true)
-		err := syncNamespace(mockClient, clientPool, &operationNotSupportedCache{m: make(map[operationKey]bool)}, fn, testInput.testNamespace, v1.FinalizerKubernetes)
+		err := d.Delete(testInput.testNamespace.Name)
 		if err != nil {
 			t.Errorf("scenario %s - Unexpected error when synching namespace %v", scenario, err)
 		}
@@ -205,7 +208,7 @@ func testSyncNamespaceThatIsTerminating(t *testing.T, versions *metav1.APIVersio
 func TestRetryOnConflictError(t *testing.T) {
 	mockClient := &fake.Clientset{}
 	numTries := 0
-	retryOnce := func(kubeClient clientset.Interface, namespace *v1.Namespace) (*v1.Namespace, error) {
+	retryOnce := func(namespace *v1.Namespace) (*v1.Namespace, error) {
 		numTries++
 		if numTries <= 1 {
 			return namespace, errors.NewConflict(api.Resource("namespaces"), namespace.Name, fmt.Errorf("ERROR!"))
@@ -213,7 +216,10 @@ func TestRetryOnConflictError(t *testing.T) {
 		return namespace, nil
 	}
 	namespace := &v1.Namespace{}
-	_, err := retryOnConflictError(mockClient, namespace, retryOnce)
+	d := namespacedResourcesDeleter{
+		nsClient: mockClient.Core().Namespaces(),
+	}
+	_, err := d.retryOnConflictError(namespace, retryOnce)
 	if err != nil {
 		t.Errorf("Unexpected error %v", err)
 	}
@@ -247,12 +253,18 @@ func TestSyncNamespaceThatIsActive(t *testing.T) {
 	fn := func() ([]*metav1.APIResourceList, error) {
 		return testResources(), nil
 	}
-	err := syncNamespace(mockClient, nil, &operationNotSupportedCache{m: make(map[operationKey]bool)}, fn, testNamespace, v1.FinalizerKubernetes)
+	d := NewNamespacedResourcesDeleter(mockClient.Core().Namespaces(), nil, mockClient.Core(),
+		fn, v1.FinalizerKubernetes, true)
+	err := d.Delete(testNamespace.Name)
 	if err != nil {
 		t.Errorf("Unexpected error when synching namespace %v", err)
 	}
-	if len(mockClient.Actions()) != 0 {
+	if len(mockClient.Actions()) != 1 {
-		t.Errorf("Expected no action from controller, but got: %v", mockClient.Actions())
+		t.Errorf("Expected only one action from controller, but got: %d %v", len(mockClient.Actions()), mockClient.Actions())
+	}
+	action := mockClient.Actions()[0]
+	if !action.Matches("get", "namespaces") {
+		t.Errorf("Expected get namespaces, got: %v", action)
 	}
 }
 

pkg/controller/namespace/namespace_controller.go

@@ -21,9 +21,7 @@ import (
 
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
-	"k8s.io/apimachinery/pkg/runtime/schema"
 	utilruntime "k8s.io/apimachinery/pkg/util/runtime"
-	"k8s.io/apimachinery/pkg/util/sets"
 	"k8s.io/apimachinery/pkg/util/wait"
 	"k8s.io/apimachinery/pkg/watch"
 	"k8s.io/client-go/dynamic"
@@ -32,6 +30,7 @@ import (
 	"k8s.io/kubernetes/pkg/api/v1"
 	"k8s.io/kubernetes/pkg/client/clientset_generated/clientset"
 	"k8s.io/kubernetes/pkg/controller"
+	"k8s.io/kubernetes/pkg/controller/namespace/deletion"
 	"k8s.io/kubernetes/pkg/util/metrics"
 
 	"github.com/golang/glog"
@@ -61,10 +60,10 @@ type NamespaceController struct {
 	queue workqueue.RateLimitingInterface
 	// function to list of preferred resources for namespace deletion
 	discoverResourcesFn func() ([]*metav1.APIResourceList, error)
-	// opCache is a cache to remember if a particular operation is not supported to aid dynamic client.
-	opCache *operationNotSupportedCache
 	// finalizerToken is the finalizer token managed by this controller
 	finalizerToken v1.FinalizerName
+	// helper to delete all resources in the namespace when the namespace is deleted.
+	namespacedResourcesDeleter deletion.NamespacedResourcesDeleterInterface
 }
 
 // NewNamespaceController creates a new NamespaceController
@@ -75,51 +74,14 @@ func NewNamespaceController(
 	resyncPeriod time.Duration,
 	finalizerToken v1.FinalizerName) *NamespaceController {
 
-	opCache := &operationNotSupportedCache{
-		m: make(map[operationKey]bool),
-	}
-
-	// pre-fill opCache with the discovery info
-	//
-	// TODO(sttts): get rid of opCache and http 405 logic around it and trust discovery info
-	resources, err := discoverResourcesFn()
-	if err != nil {
-		glog.Fatalf("Failed to get supported resources: %v", err)
-	}
-	deletableGroupVersionResources := []schema.GroupVersionResource{}
-	for _, rl := range resources {
-		gv, err := schema.ParseGroupVersion(rl.GroupVersion)
-		if err != nil {
-			glog.Errorf("Failed to parse GroupVersion %q, skipping: %v", rl.GroupVersion, err)
-			continue
-		}
-
-		for _, r := range rl.APIResources {
-			gvr := schema.GroupVersionResource{Group: gv.Group, Version: gv.Version, Resource: r.Name}
-			verbs := sets.NewString([]string(r.Verbs)...)
-
-			if !verbs.Has("delete") {
-				glog.V(6).Infof("Skipping resource %v because it cannot be deleted.", gvr)
-			}
-
-			for _, op := range []operation{operationList, operationDeleteCollection} {
-				if !verbs.Has(string(op)) {
-					opCache.setNotSupported(operationKey{op: op, gvr: gvr})
-				}
-			}
-
-			deletableGroupVersionResources = append(deletableGroupVersionResources, gvr)
-		}
-	}
-
 	// create the controller so we can inject the enqueue function
 	namespaceController := &NamespaceController{
 		kubeClient:                 kubeClient,
 		clientPool:                 clientPool,
 		queue:                      workqueue.NewNamedRateLimitingQueue(workqueue.DefaultControllerRateLimiter(), "namespace"),
 		discoverResourcesFn:        discoverResourcesFn,
-		opCache:             opCache,
 		finalizerToken:             finalizerToken,
+		namespacedResourcesDeleter: deletion.NewNamespacedResourcesDeleter(kubeClient.Core().Namespaces(), clientPool, kubeClient.Core(), discoverResourcesFn, finalizerToken, true),
 	}
 
 	if kubeClient != nil && kubeClient.Core().RESTClient().GetRateLimiter() != nil {
@@ -187,7 +149,7 @@ func (nm *NamespaceController) worker() {
 			return false
 		}
 
-		if estimate, ok := err.(*contentRemainingError); ok {
+		if estimate, ok := err.(*deletion.ResourcesRemainingError); ok {
 			t := estimate.Estimate/2 + 1
 			glog.V(4).Infof("Content remaining in namespace %s, waiting %d seconds", key, t)
 			nm.queue.AddAfter(key, time.Duration(t)*time.Second)
@@ -224,7 +186,7 @@ func (nm *NamespaceController) syncNamespaceFromKey(key string) (err error) {
 		return err
 	}
 	namespace := obj.(*v1.Namespace)
-	return syncNamespace(nm.kubeClient, nm.clientPool, nm.opCache, nm.discoverResourcesFn, namespace, nm.finalizerToken)
+	return nm.namespacedResourcesDeleter.Delete(namespace.Name)
 }
 
 // Run starts observing the system with the specified number of workers.

staging/src/k8s.io/client-go/dynamic/client_pool.go

@@ -21,6 +21,7 @@ import (
 
 	"k8s.io/apimachinery/pkg/api/meta"
 	"k8s.io/apimachinery/pkg/runtime/schema"
+	"k8s.io/client-go/pkg/api"
 	restclient "k8s.io/client-go/rest"
 )
 
@@ -70,6 +71,13 @@ func NewClientPool(config *restclient.Config, mapper meta.RESTMapper, apiPathRes
 	}
 }
 
+// Instantiates a new dynamic client pool with the given config.
+func NewDynamicClientPool(cfg *restclient.Config) ClientPool {
+	// TODO: should use a dynamic RESTMapper built from the discovery results.
+	restMapper := api.Registry.RESTMapper()
+	return NewClientPool(cfg, restMapper, LegacyAPIPathResolverFunc)
+}
+
 // ClientForGroupVersionResource uses the provided RESTMapper to identify the appropriate resource. Resource may
 // be empty. If no matching kind is found the underlying client for that group is still returned.
 func (c *clientPoolImpl) ClientForGroupVersionResource(resource schema.GroupVersionResource) (*Client, error) {