Merge pull request #31925 from nikhiljindal/freshDebugNs

Automatic merge from submit-queue

Adding namespaces/finalizer subresource to federation apiserver

Fixes https://github.com/kubernetes/kubernetes/issues/31077

cc @kubernetes/sig-cluster-federation @mwielgus 


Verified manually that I can delete federation namespaces now.
Will update federation-namespace e2e test to verify that namespace is deleted fine

federation/apis/swagger-spec/extensions_v1beta1.json

@@ -2821,7 +2821,7 @@
      },
      "flexVolume": {
       "$ref": "v1.FlexVolumeSource",
-      "description": "FlexVolume represents a generic volume resource that is provisioned/attached using a exec based plugin. This is an alpha feature and may change in future."
+      "description": "FlexVolume represents a generic volume resource that is provisioned/attached using an exec based plugin. This is an alpha feature and may change in future."
      },
      "cinder": {
       "$ref": "v1.CinderVolumeSource",
@@ -3160,7 +3160,7 @@
    },
    "v1.FlexVolumeSource": {
     "id": "v1.FlexVolumeSource",
-    "description": "FlexVolume represents a generic volume resource that is provisioned/attached using a exec based plugin. This is an alpha feature and may change in future.",
+    "description": "FlexVolume represents a generic volume resource that is provisioned/attached using an exec based plugin. This is an alpha feature and may change in future.",
     "required": [
      "driver"
     ],
@@ -3352,7 +3352,7 @@
       "items": {
        "type": "string"
       },
-      "description": "Required: FC target world wide names (WWNs)"
+      "description": "Required: FC target worldwide names (WWNs)"
      },
      "lun": {
       "type": "integer",

federation/apis/swagger-spec/v1.json

@@ -1345,6 +1345,59 @@
      }
     ]
    },
+   {
+    "path": "/api/v1/namespaces/{name}/finalize",
+    "description": "API at /api/v1",
+    "operations": [
+     {
+      "type": "v1.Namespace",
+      "method": "PUT",
+      "summary": "replace finalize of the specified Namespace",
+      "nickname": "replaceNamespaceFinalize",
+      "parameters": [
+       {
+        "type": "string",
+        "paramType": "query",
+        "name": "pretty",
+        "description": "If 'true', then the output is pretty printed.",
+        "required": false,
+        "allowMultiple": false
+       },
+       {
+        "type": "v1.Namespace",
+        "paramType": "body",
+        "name": "body",
+        "description": "",
+        "required": true,
+        "allowMultiple": false
+       },
+       {
+        "type": "string",
+        "paramType": "path",
+        "name": "name",
+        "description": "name of the Namespace",
+        "required": true,
+        "allowMultiple": false
+       }
+      ],
+      "responseMessages": [
+       {
+        "code": 200,
+        "message": "OK",
+        "responseModel": "v1.Namespace"
+       }
+      ],
+      "produces": [
+       "application/json",
+       "application/yaml",
+       "application/vnd.kubernetes.protobuf"
+      ],
+      "consumes": [
+       "*/*"
+      ]
+     }
+    ]
+   },
    {
     "path": "/api/v1/namespaces/{name}/status",
     "description": "API at /api/v1",

federation/cmd/federation-apiserver/app/core.go

@@ -42,16 +42,17 @@ import (
 
 func installCoreAPIs(s *options.ServerRunOptions, g *genericapiserver.GenericAPIServer, f genericapiserver.StorageFactory) {
 	serviceStore, serviceStatusStore := serviceetcd.NewREST(createRESTOptionsOrDie(s, g, f, api.Resource("service")))
-	namespaceStore, namespaceStatusStore, _ := namespaceetcd.NewREST(createRESTOptionsOrDie(s, g, f, api.Resource("namespaces")))
+	namespaceStore, namespaceStatusStore, namespaceFinalizeStore := namespaceetcd.NewREST(createRESTOptionsOrDie(s, g, f, api.Resource("namespaces")))
 	secretStore := secretetcd.NewREST(createRESTOptionsOrDie(s, g, f, api.Resource("secrets")))
 	eventStore := eventetcd.NewREST(createRESTOptionsOrDie(s, g, f, api.Resource("events")), uint64(s.EventTTL.Seconds()))
 	coreResources := map[string]rest.Storage{
-		"secrets":           secretStore,
+		"secrets":             secretStore,
-		"services":          serviceStore,
+		"services":            serviceStore,
-		"services/status":   serviceStatusStore,
+		"services/status":     serviceStatusStore,
-		"namespaces":        namespaceStore,
+		"namespaces":          namespaceStore,
-		"namespaces/status": namespaceStatusStore,
+		"namespaces/status":   namespaceStatusStore,
-		"events":            eventStore,
+		"namespaces/finalize": namespaceFinalizeStore,
+		"events":              eventStore,
 	}
 	coreGroupMeta := registered.GroupOrDie(core.GroupName)
 	apiGroupInfo := genericapiserver.APIGroupInfo{

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

@@ -26,6 +26,7 @@ import (
 	"k8s.io/kubernetes/federation/pkg/federation-controller/util"
 	"k8s.io/kubernetes/federation/pkg/federation-controller/util/eventsink"
 	"k8s.io/kubernetes/pkg/api"
+	"k8s.io/kubernetes/pkg/api/errors"
 	api_v1 "k8s.io/kubernetes/pkg/api/v1"
 	"k8s.io/kubernetes/pkg/client/cache"
 	kube_release_1_4 "k8s.io/kubernetes/pkg/client/clientset_generated/release_1_4"
@@ -363,7 +364,12 @@ func (nc *NamespaceController) delete(namespace *api_v1.Namespace) {
 	// TODO: What about namespaces in subclusters ???
 	err := nc.federatedApiClient.Core().Namespaces().Delete(updatedNamespace.Name, &api.DeleteOptions{})
 	if err != nil {
-		glog.Errorf("Failed to delete namespace %s: %v", namespace.Name, err)
+		// Its all good if the error is not found error. That means it is deleted already and we do not have to do anything.
-		nc.deliverNamespace(namespace.Name, 0, true)
+		// This is expected when we are processing an update as a result of namespace finalizer deletion.
+		// The process that deleted the last finalizer is also going to delete the namespace and we do not have to do anything.
+		if !errors.IsNotFound(err) {
+			glog.Errorf("Failed to delete namespace %s: %v", namespace.Name, err)
+			nc.deliverNamespace(namespace.Name, 0, true)
+		}
 	}
 }

test/e2e/federated-namespace.go

@@ -54,16 +54,13 @@ var _ = framework.KubeDescribe("Federation namespace [Feature:Federation]", func
 
 		AfterEach(func() {
 			framework.SkipUnlessFederated(f.Client)
-			// TODO: set wait to true once NS controller is fixed.
 			deleteAllTestNamespaces(
 				f.FederationClientset_1_4.Core().Namespaces().List,
-				f.FederationClientset_1_4.Core().Namespaces().Delete,
+				f.FederationClientset_1_4.Core().Namespaces().Delete)
-				false)
 			for _, clientset := range clusterClientSet {
 				deleteAllTestNamespaces(
 					clientset.Core().Namespaces().List,
-					clientset.Core().Namespaces().Delete,
+					clientset.Core().Namespaces().Delete)
-					false)
 			}
 		})
 
@@ -79,7 +76,7 @@ var _ = framework.KubeDescribe("Federation namespace [Feature:Federation]", func
 			_, err := f.FederationClientset_1_4.Core().Namespaces().Create(&ns)
 			framework.ExpectNoError(err, "Failed to create namespace %s", ns.Name)
 
-			// Check subclusters if the namespace was create there.
+			// Check subclusters if the namespace was created there.
 			err = wait.Poll(5*time.Second, 2*time.Minute, func() (bool, error) {
 				for _, client := range clusterClientSet {
 					_, err := client.Core().Namespaces().Get(ns.Name)
@@ -94,32 +91,28 @@ var _ = framework.KubeDescribe("Federation namespace [Feature:Federation]", func
 			})
 			framework.ExpectNoError(err, "Not all namespaces created")
 
-			// TODO: set wait to true once NS controller is fixed.
 			deleteAllTestNamespaces(
 				f.FederationClientset_1_4.Core().Namespaces().List,
-				f.FederationClientset_1_4.Core().Namespaces().Delete,
+				f.FederationClientset_1_4.Core().Namespaces().Delete)
-				false)
 		})
 	})
 })
 
-func deleteAllTestNamespaces(lister func(api.ListOptions) (*api_v1.NamespaceList, error), deleter func(string, *api.DeleteOptions) error, waitForDeletion bool) {
+func deleteAllTestNamespaces(lister func(api.ListOptions) (*api_v1.NamespaceList, error), deleter func(string, *api.DeleteOptions) error) {
 	list, err := lister(api.ListOptions{})
 	if err != nil {
 		framework.Failf("Failed to get all namespaes: %v", err)
 		return
 	}
 	for _, namespace := range list.Items {
-		if strings.HasPrefix(namespace.Name, namespacePrefix) && namespace.DeletionTimestamp != nil {
+		if strings.HasPrefix(namespace.Name, namespacePrefix) {
 			err := deleter(namespace.Name, &api.DeleteOptions{})
 			if err != nil {
 				framework.Failf("Failed to set %s for deletion: %v", namespace.Name, err)
 			}
 		}
 	}
-	if waitForDeletion {
+	waitForNoTestNamespaces(lister)
-		waitForNoTestNamespaces(lister)
-	}
 }
 
 func waitForNoTestNamespaces(lister func(api.ListOptions) (*api_v1.NamespaceList, error)) {

test/integration/federation/server_test.go

@@ -283,8 +283,8 @@ func testCoreResourceList(t *testing.T) {
 	}
 	assert.Equal(t, "", apiResourceList.APIVersion)
 	assert.Equal(t, v1.SchemeGroupVersion.String(), apiResourceList.GroupVersion)
-	// Assert that there are exactly 6 resources.
+	// Assert that there are exactly 7 resources.
-	assert.Equal(t, 6, len(apiResourceList.APIResources))
+	assert.Equal(t, 7, len(apiResourceList.APIResources))
 
 	// Verify services.
 	found := findResource(apiResourceList.APIResources, "services")
@@ -301,6 +301,9 @@ func testCoreResourceList(t *testing.T) {
 	found = findResource(apiResourceList.APIResources, "namespaces/status")
 	assert.NotNil(t, found)
 	assert.False(t, found.Namespaced)
+	found = findResource(apiResourceList.APIResources, "namespaces/finalize")
+	assert.NotNil(t, found)
+	assert.False(t, found.Namespaced)
 
 	// Verify events.
 	found = findResource(apiResourceList.APIResources, "events")