Updating federation namespace controller to share namespaced resources deletion…

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/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")
-	serviceDeleteChan := RegisterDeleteCollection(&fakeClient.Fake, "services")
-	secretDeleteChan := RegisterDeleteCollection(&fakeClient.Fake, "secrets")
-
-	namespaceController := NewNamespaceController(fakeClient)
+	namespaceController := NewNamespaceController(fakeClient, dynamic.NewDynamicClientPool(&restclient.Config{}))
 	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))
-	assert.Equal(t, "all", GetStringFromChan(serviceDeleteChan))
-	assert.Equal(t, "all", GetStringFromChan(secretDeleteChan))
-
+	// TODO: Add a test for verifying that resources in the namespace are deleted
+	// when the namespace is deleted.
+	// 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/deletion/namespaced_resources_deleter_test.go

@@ -65,7 +65,7 @@ func TestFinalizeNamespaceFunc(t *testing.T) {
 			Finalizers: []v1.FinalizerName{"kubernetes", "other"},
 		},
 	}
-	d := NamespacedResourcesDeleter{
+	d := namespacedResourcesDeleter{
 		nsClient:       mockClient.Core().Namespaces(),
 		finalizerToken: v1.FinalizerKubernetes,
 	}
@@ -130,11 +130,6 @@ func testSyncNamespaceThatIsTerminating(t *testing.T, versions *metav1.APIVersio
 		dynamicClientActionSet.Insert((&fakeAction{method: "GET", path: urlPath}).String())
 		dynamicClientActionSet.Insert((&fakeAction{method: "DELETE", path: urlPath}).String())
 	}
-	// One additional GET for listing pods (to estimate graceful deletion).
-	urlPath := path.Join([]string{
-		dynamic.LegacyAPIPathResolverFunc(schema.GroupVersionKind{Group: "", Version: "v1"}),
-		"", "v1", "namespaces", namespaceName}...)
-	dynamicClientActionSet.Insert((&fakeAction{method: "GET", path: urlPath}).String())
 
 	scenarios := map[string]struct {
 		testNamespace          *v1.Namespace
@@ -147,6 +142,7 @@ func testSyncNamespaceThatIsTerminating(t *testing.T, versions *metav1.APIVersio
 			kubeClientActionSet: sets.NewString(
 				strings.Join([]string{"get", "namespaces", ""}, "-"),
 				strings.Join([]string{"create", "namespaces", "finalize"}, "-"),
+				strings.Join([]string{"list", "pods", ""}, "-"),
 				strings.Join([]string{"delete", "namespaces", ""}, "-"),
 			),
 			dynamicClientActionSet: dynamicClientActionSet,
@@ -181,7 +177,7 @@ func testSyncNamespaceThatIsTerminating(t *testing.T, versions *metav1.APIVersio
 		fn := func() ([]*metav1.APIResourceList, error) {
 			return resources, nil
 		}
-		d := NewNamespacedResourcesDeleter(mockClient.Core().Namespaces(), clientPool, &OperationNotSupportedCache{M: make(map[OperationKey]bool)}, fn, v1.FinalizerKubernetes, true)
+		d := NewNamespacedResourcesDeleter(mockClient.Core().Namespaces(), clientPool, mockClient.Core(), fn, v1.FinalizerKubernetes, true)
 		err := d.Delete(testInput.testNamespace.Name)
 		if err != nil {
 			t.Errorf("scenario %s - Unexpected error when synching namespace %v", scenario, err)
@@ -220,7 +216,7 @@ func TestRetryOnConflictError(t *testing.T) {
 		return namespace, nil
 	}
 	namespace := &v1.Namespace{}
-	d := NamespacedResourcesDeleter{
+	d := namespacedResourcesDeleter{
 		nsClient: mockClient.Core().Namespaces(),
 	}
 	_, err := d.retryOnConflictError(namespace, retryOnce)
@@ -257,9 +253,8 @@ 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,
-		&OperationNotSupportedCache{M: make(map[OperationKey]bool)}, fn, v1.FinalizerKubernetes, true)
+	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)

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"
@@ -62,12 +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 *deletion.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.NamespacedResourcesDeleter
+	namespacedResourcesDeleter deletion.NamespacedResourcesDeleterInterface
 }
 
 // NewNamespaceController creates a new NamespaceController
@@ -78,52 +74,14 @@ func NewNamespaceController(
 	resyncPeriod time.Duration,
 	finalizerToken v1.FinalizerName) *NamespaceController {
 
-	opCache := &deletion.OperationNotSupportedCache{
-		M: make(map[deletion.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 []deletion.Operation{deletion.OperationList, deletion.OperationDeleteCollection} {
-				if !verbs.Has(string(op)) {
-					opCache.SetNotSupported(deletion.OperationKey{Operation: 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, opCache, discoverResourcesFn, finalizerToken, true),
+		namespacedResourcesDeleter: deletion.NewNamespacedResourcesDeleter(kubeClient.Core().Namespaces(), clientPool, kubeClient.Core(), discoverResourcesFn, finalizerToken, true),
 	}
 
 	if kubeClient != nil && kubeClient.Core().RESTClient().GetRateLimiter() != nil {

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) {