Merge pull request #39338 from irfanurrehman/fed-join-1

Automatic merge from submit-queue (batch tested with PRs 42058, 41160, 42065, 42076, 39338)

[Federation] Create configmap for the cluster kube-dns when cluster joins and remove when it unjoins

This PR implements the functionality as needed in https://github.com/kubernetes/kubernetes/issues/38400

cc @kubernetes/sig-cluster-federation @nikhiljindal @madhusudancs 

**Release note**:

```
kubefed join can now automatically create a configmap or add information to already existing one, storing this federation name vs zone name information in the joining cluster.
Further kubefed unjoin can remove this configmap or only this information from the configmap, if the unjoin cluster is registered with multiple federations.
The name of the configmap is kube-dns and the information in it is consumed by the in-cluster dns server.
```

federation/pkg/kubefed/BUILD

@@ -20,6 +20,9 @@ go_library(
         "//federation/apis/federation:go_default_library",
         "//federation/pkg/kubefed/init:go_default_library",
         "//federation/pkg/kubefed/util:go_default_library",
+        "//pkg/api:go_default_library",
+        "//pkg/apis/extensions:go_default_library",
+        "//pkg/client/clientset_generated/internalclientset:go_default_library",
         "//pkg/kubectl:go_default_library",
         "//pkg/kubectl/cmd:go_default_library",
         "//pkg/kubectl/cmd/templates:go_default_library",
@@ -54,6 +57,7 @@ go_test(
         "//pkg/api:go_default_library",
         "//pkg/api/testapi:go_default_library",
         "//pkg/api/v1:go_default_library",
+        "//pkg/apis/extensions/v1beta1:go_default_library",
         "//pkg/kubectl/cmd/testing:go_default_library",
         "//pkg/kubectl/cmd/util:go_default_library",
         "//vendor:k8s.io/apimachinery/pkg/api/equality",

federation/pkg/kubefed/init/init.go

@@ -68,6 +68,10 @@ const (
 	ControllerManagerCN         = "federation-controller-manager"
 	AdminCN                     = "admin"
 	HostClusterLocalDNSZoneName = "cluster.local."
+	APIServerNameSuffix         = "apiserver"
+	CMNameSuffix                = "controller-manager"
+	CredentialSuffix            = "credentials"
+	KubeconfigNameSuffix        = "kubeconfig"
 
 	// User name used by federation controller manager to make
 	// calls to federation API server.
@@ -237,16 +241,16 @@ func (i *initFederation) Complete(cmd *cobra.Command, args []string) error {
 // See the design doc in https://github.com/kubernetes/kubernetes/pull/34484
 // for details.
 func (i *initFederation) Run(cmdOut io.Writer, config util.AdminConfig) error {
-	hostFactory := config.HostFactory(i.commonOptions.Host, i.commonOptions.Kubeconfig)
+	hostFactory := config.ClusterFactory(i.commonOptions.Host, i.commonOptions.Kubeconfig)
 	hostClientset, err := hostFactory.ClientSet()
 	if err != nil {
 		return err
 	}
 
-	serverName := fmt.Sprintf("%s-apiserver", i.commonOptions.Name)
+	serverName := fmt.Sprintf("%s-%s", i.commonOptions.Name, APIServerNameSuffix)
-	serverCredName := fmt.Sprintf("%s-credentials", serverName)
+	serverCredName := fmt.Sprintf("%s-%s", serverName, CredentialSuffix)
-	cmName := fmt.Sprintf("%s-controller-manager", i.commonOptions.Name)
+	cmName := fmt.Sprintf("%s-%s", i.commonOptions.Name, CMNameSuffix)
-	cmKubeconfigName := fmt.Sprintf("%s-kubeconfig", cmName)
+	cmKubeconfigName := fmt.Sprintf("%s-%s", cmName, KubeconfigNameSuffix)
 
 	var dnsProviderConfigBytes []byte
 	if i.options.dnsProviderConfig != "" {
@@ -770,6 +774,16 @@ func createControllerManager(clientset *client.Clientset, namespace, name, svcNa
 			Name:      cmName,
 			Namespace: namespace,
 			Labels:    componentLabel,
+			// We additionally update the details (in annotations) about the
+			// kube-dns config map which needs to be created in the clusters
+			// registering to this federation (at kubefed join).
+			// We wont otherwise have this information available at kubefed join.
+			Annotations: map[string]string{
+				// TODO: the name/domain name pair should ideally be checked for naming convention
+				// as done in kube-dns federation flags check.
+				// https://github.com/kubernetes/dns/blob/master/pkg/dns/federation/federation.go
+				util.FedDomainMapKey: fmt.Sprintf("%s=%s", name, dnsZoneName),
+			},
 		},
 		Spec: extensions.DeploymentSpec{
 			Replicas: 1,

federation/pkg/kubefed/init/init_test.go

@@ -216,7 +216,7 @@ func TestInitFederation(t *testing.T) {
 			t.Fatalf("[%d] unexpected error: %v", i, err)
 		}
 
-		adminConfig, err := kubefedtesting.NewFakeAdminConfig(hostFactory, tc.kubeconfigGlobal)
+		adminConfig, err := kubefedtesting.NewFakeAdminConfig(hostFactory, nil, "", tc.kubeconfigGlobal)
 		if err != nil {
 			t.Fatalf("[%d] unexpected error: %v", i, err)
 		}
@@ -932,6 +932,9 @@ func fakeInitHostFactory(apiserverServiceType v1.ServiceType, federationName, na
 			Name:      cmName,
 			Namespace: namespaceName,
 			Labels:    componentLabel,
+			Annotations: map[string]string{
+				util.FedDomainMapKey: fmt.Sprintf("%s=%s", federationName, dnsZoneName),
+			},
 		},
 		Spec: v1beta1.DeploymentSpec{
 			Replicas: &replicas,

federation/pkg/kubefed/join.go

@@ -24,6 +24,7 @@ import (
 	"k8s.io/apimachinery/pkg/runtime"
 	clientcmdapi "k8s.io/client-go/tools/clientcmd/api"
 	"k8s.io/kubernetes/federation/pkg/kubefed/util"
+	"k8s.io/kubernetes/pkg/client/clientset_generated/internalclientset"
 	"k8s.io/kubernetes/pkg/kubectl"
 	kubectlcmd "k8s.io/kubernetes/pkg/kubectl/cmd"
 	"k8s.io/kubernetes/pkg/kubectl/cmd/templates"
@@ -32,6 +33,9 @@ import (
 	"github.com/golang/glog"
 	"github.com/spf13/cobra"
 	"github.com/spf13/pflag"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/kubernetes/pkg/api"
+	extensions "k8s.io/kubernetes/pkg/apis/extensions"
 )
 
 const (
@@ -40,6 +44,7 @@ const (
 	// details.
 	// TODO(madhusudancs): Make this value customizable.
 	defaultClientCIDR = "0.0.0.0/0"
+	CMNameSuffix      = "controller-manager"
 )
 
 var (
@@ -138,7 +143,12 @@ func (j *joinFederation) Run(f cmdutil.Factory, cmdOut io.Writer, config util.Ad
 	}
 	glog.V(2).Infof("Created cluster generator: %#v", generator)
 
-	hostFactory := config.HostFactory(j.commonOptions.Host, j.commonOptions.Kubeconfig)
+	hostFactory := config.ClusterFactory(j.commonOptions.Host, j.commonOptions.Kubeconfig)
+	hostClientset, err := hostFactory.ClientSet()
+	if err != nil {
+		glog.V(2).Infof("Failed to get the cluster client for the host cluster: %q", j.commonOptions.Host, err)
+		return err
+	}
 
 	// We are not using the `kubectl create secret` machinery through
 	// `RunCreateSubcommand` as we do to the cluster resource below
@@ -155,19 +165,33 @@ func (j *joinFederation) Run(f cmdutil.Factory, cmdOut io.Writer, config util.Ad
 	//    don't have to print the created secret in the default case.
 	// Having said that, secret generation machinery could be altered to
 	// suit our needs, but it is far less invasive and readable this way.
-	_, err = createSecret(hostFactory, clientConfig, j.commonOptions.FederationSystemNamespace, j.options.clusterContext, j.options.secretName, j.options.dryRun)
+	_, err = createSecret(hostClientset, clientConfig, j.commonOptions.FederationSystemNamespace, j.options.clusterContext, j.options.secretName, j.options.dryRun)
 	if err != nil {
 		glog.V(2).Infof("Failed creating the cluster credentials secret: %v", err)
 		return err
 	}
 	glog.V(2).Infof("Cluster credentials secret created")
 
-	return kubectlcmd.RunCreateSubcommand(f, cmd, cmdOut, &kubectlcmd.CreateSubcommandOptions{
+	err = kubectlcmd.RunCreateSubcommand(f, cmd, cmdOut, &kubectlcmd.CreateSubcommandOptions{
 		Name:                j.commonOptions.Name,
 		StructuredGenerator: generator,
 		DryRun:              j.options.dryRun,
 		OutputFormat:        cmdutil.GetFlagString(cmd, "output"),
 	})
+	if err != nil {
+		return err
+	}
+
+	// We further need to create a configmap named kube-config in the
+	// just registered cluster which will be consumed by the kube-dns
+	// of this cluster.
+	_, err = createConfigMap(hostClientset, config, j.commonOptions.FederationSystemNamespace, j.options.clusterContext, j.commonOptions.Kubeconfig, j.options.dryRun)
+	if err != nil {
+		glog.V(2).Infof("Failed creating the config map in cluster: %v", err)
+		return err
+	}
+
+	return err
 }
 
 // minifyConfig is a wrapper around `clientcmdapi.MinifyConfig()` that
@@ -189,7 +213,7 @@ func minifyConfig(clientConfig *clientcmdapi.Config, context string) (*clientcmd
 
 // createSecret extracts the kubeconfig for a given cluster and populates
 // a secret with that kubeconfig.
-func createSecret(hostFactory cmdutil.Factory, clientConfig *clientcmdapi.Config, namespace, contextName, secretName string, dryRun bool) (runtime.Object, error) {
+func createSecret(clientset *internalclientset.Clientset, clientConfig *clientcmdapi.Config, namespace, contextName, secretName string, dryRun bool) (runtime.Object, error) {
 	// Minify the kubeconfig to ensure that there is only information
 	// relevant to the cluster we are registering.
 	newClientConfig, err := minifyConfig(clientConfig, contextName)
@@ -206,14 +230,65 @@ func createSecret(hostFactory cmdutil.Factory, clientConfig *clientcmdapi.Config
 		return nil, err
 	}
 
-	// Boilerplate to create the secret in the host cluster.
+	return util.CreateKubeconfigSecret(clientset, newClientConfig, namespace, secretName, dryRun)
-	clientset, err := hostFactory.ClientSet()
+}
+
+// createConfigMap creates a configmap with name kube-dns in the joining cluster
+// which stores the information about this federation zone name.
+// If the configmap with this name already exists, its updated with this information.
+func createConfigMap(hostClientSet *internalclientset.Clientset, config util.AdminConfig, fedSystemNamespace, targetClusterContext, kubeconfigPath string, dryRun bool) (*api.ConfigMap, error) {
+	cmDep, err := getCMDeployment(hostClientSet, fedSystemNamespace)
+	if err != nil {
+		return nil, err
+	}
+	domainMap, ok := cmDep.Annotations[util.FedDomainMapKey]
+	if !ok {
+		return nil, fmt.Errorf("kube-dns config map data missing from controller manager annotations")
+	}
+
+	targetFactory := config.ClusterFactory(targetClusterContext, kubeconfigPath)
+	targetClientSet, err := targetFactory.ClientSet()
 	if err != nil {
-		glog.V(2).Infof("Failed to serialize the kubeconfig for the given context %q: %v", contextName, err)
 		return nil, err
 	}
 
-	return util.CreateKubeconfigSecret(clientset, newClientConfig, namespace, secretName, dryRun)
+	existingConfigMap, err := targetClientSet.Core().ConfigMaps(metav1.NamespaceSystem).Get(util.KubeDnsConfigmapName, metav1.GetOptions{})
+	if isNotFound(err) {
+		newConfigMap := &api.ConfigMap{
+			ObjectMeta: metav1.ObjectMeta{
+				Name:      util.KubeDnsConfigmapName,
+				Namespace: metav1.NamespaceSystem,
+			},
+			Data: map[string]string{
+				util.FedDomainMapKey: domainMap,
+			},
+		}
+
+		if dryRun {
+			return newConfigMap, nil
+		}
+		return targetClientSet.Core().ConfigMaps(metav1.NamespaceSystem).Create(newConfigMap)
+	}
+	if err != nil {
+		return nil, err
+	}
+
+	if existingConfigMap.Data == nil {
+		existingConfigMap.Data = make(map[string]string)
+	}
+	if _, ok := existingConfigMap.Data[util.FedDomainMapKey]; ok {
+		// Append this federation info
+		existingConfigMap.Data[util.FedDomainMapKey] = appendConfigMapString(existingConfigMap.Data[util.FedDomainMapKey], cmDep.Annotations[util.FedDomainMapKey])
+
+	} else {
+		// For some reason the configMap exists but this data is empty
+		existingConfigMap.Data[util.FedDomainMapKey] = cmDep.Annotations[util.FedDomainMapKey]
+	}
+
+	if dryRun {
+		return existingConfigMap, nil
+	}
+	return targetClientSet.Core().ConfigMaps(metav1.NamespaceSystem).Update(existingConfigMap)
 }
 
 // clusterGenerator extracts the cluster information from the supplied
@@ -261,3 +336,33 @@ func extractScheme(url string) string {
 	}
 	return scheme
 }
+
+func getCMDeployment(hostClientSet *internalclientset.Clientset, fedNamespace string) (*extensions.Deployment, error) {
+	depList, err := hostClientSet.Extensions().Deployments(fedNamespace).List(metav1.ListOptions{})
+	if err != nil {
+		return nil, err
+	}
+
+	for _, dep := range depList.Items {
+		if strings.HasSuffix(dep.Name, CMNameSuffix) {
+			return &dep, nil
+		}
+	}
+	return nil, fmt.Errorf("could not find the deployment for controller manager in host cluster")
+}
+
+func appendConfigMapString(existing string, toAppend string) string {
+	if existing == "" {
+		return toAppend
+	}
+
+	values := strings.Split(existing, ",")
+	for _, v := range values {
+		// Somehow this federation string is already present,
+		// Nothing should be done
+		if v == toAppend {
+			return existing
+		}
+	}
+	return fmt.Sprintf("%s,%s", existing, toAppend)
+}

federation/pkg/kubefed/join_test.go

@@ -36,6 +36,7 @@ import (
 	"k8s.io/kubernetes/pkg/api"
 	"k8s.io/kubernetes/pkg/api/testapi"
 	"k8s.io/kubernetes/pkg/api/v1"
+	"k8s.io/kubernetes/pkg/apis/extensions/v1beta1"
 	cmdtesting "k8s.io/kubernetes/pkg/kubectl/cmd/testing"
 	cmdutil "k8s.io/kubernetes/pkg/kubectl/cmd/util"
 )
@@ -130,7 +131,16 @@ func TestJoinFederation(t *testing.T) {
 			t.Fatalf("[%d] unexpected error: %v", i, err)
 		}
 
-		adminConfig, err := kubefedtesting.NewFakeAdminConfig(hostFactory, tc.kubeconfigGlobal)
+		targetClusterFactory, err := fakeJoinTargetClusterFactory(tc.cluster, tc.clusterCtx)
+		if err != nil {
+			t.Fatalf("[%d] unexpected error: %v", i, err)
+		}
+
+		targetClusterContext := tc.clusterCtx
+		if targetClusterContext == "" {
+			targetClusterContext = tc.cluster
+		}
+		adminConfig, err := kubefedtesting.NewFakeAdminConfig(hostFactory, targetClusterFactory, targetClusterContext, tc.kubeconfigGlobal)
 		if err != nil {
 			t.Fatalf("[%d] unexpected error: %v", i, err)
 		}
@@ -234,6 +244,7 @@ func fakeJoinHostFactory(clusterName, clusterCtx, secretName, server, token stri
 	if err != nil {
 		return nil, err
 	}
+
 	secretObject := v1.Secret{
 		TypeMeta: metav1.TypeMeta{
 			Kind:       "Secret",
@@ -248,7 +259,31 @@ func fakeJoinHostFactory(clusterName, clusterCtx, secretName, server, token stri
 		},
 	}
 
+	cmName := "controller-manager"
+	deploymentList := v1beta1.DeploymentList{
+		TypeMeta: metav1.TypeMeta{
+			Kind:       "DeploymentList",
+			APIVersion: testapi.Extensions.GroupVersion().String(),
+		},
+		Items: []v1beta1.Deployment{
+			{
+				TypeMeta: metav1.TypeMeta{
+					Kind:       "Deployment",
+					APIVersion: testapi.Extensions.GroupVersion().String(),
+				},
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      cmName,
+					Namespace: util.DefaultFederationSystemNamespace,
+					Annotations: map[string]string{
+						util.FedDomainMapKey: fmt.Sprintf("%s=%s", clusterCtx, "test-dns-zone"),
+					},
+				},
+			},
+		},
+	}
+
 	f, tf, codec, _ := cmdtesting.NewAPIFactory()
+	extensionCodec := testapi.Extensions.Codec()
 	ns := dynamic.ContentConfig().NegotiatedSerializer
 	tf.ClientConfig = kubefedtesting.DefaultClientConfig()
 	tf.Client = &fake.RESTClient{
@@ -270,6 +305,53 @@ func fakeJoinHostFactory(clusterName, clusterCtx, secretName, server, token stri
 					return nil, fmt.Errorf("Unexpected secret object\n\tDiff: %s", diff.ObjectGoPrintDiff(got, secretObject))
 				}
 				return &http.Response{StatusCode: http.StatusCreated, Header: kubefedtesting.DefaultHeader(), Body: kubefedtesting.ObjBody(codec, &secretObject)}, nil
+			case p == "/apis/extensions/v1beta1/namespaces/federation-system/deployments" && m == http.MethodGet:
+				return &http.Response{StatusCode: http.StatusOK, Header: kubefedtesting.DefaultHeader(), Body: kubefedtesting.ObjBody(extensionCodec, &deploymentList)}, nil
+			default:
+				return nil, fmt.Errorf("unexpected request: %#v\n%#v", req.URL, req)
+			}
+		}),
+	}
+	return f, nil
+}
+
+func fakeJoinTargetClusterFactory(clusterName, clusterCtx string) (cmdutil.Factory, error) {
+	if clusterCtx == "" {
+		clusterCtx = clusterName
+	}
+
+	configmapObject := v1.ConfigMap{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      util.KubeDnsConfigmapName,
+			Namespace: metav1.NamespaceSystem,
+		},
+		Data: map[string]string{
+			util.FedDomainMapKey: fmt.Sprintf("%s=%s", clusterCtx, "test-dns-zone"),
+		},
+	}
+
+	f, tf, codec, _ := cmdtesting.NewAPIFactory()
+	ns := dynamic.ContentConfig().NegotiatedSerializer
+	tf.ClientConfig = kubefedtesting.DefaultClientConfig()
+	tf.Client = &fake.RESTClient{
+		APIRegistry:          api.Registry,
+		NegotiatedSerializer: ns,
+		Client: fake.CreateHTTPClient(func(req *http.Request) (*http.Response, error) {
+			switch p, m := req.URL.Path, req.Method; {
+			case p == "/api/v1/namespaces/kube-system/configmaps/" && m == http.MethodPost:
+				body, err := ioutil.ReadAll(req.Body)
+				if err != nil {
+					return nil, err
+				}
+				var got v1.ConfigMap
+				_, _, err = codec.Decode(body, nil, &got)
+				if err != nil {
+					return nil, err
+				}
+				if !apiequality.Semantic.DeepEqual(got, configmapObject) {
+					return nil, fmt.Errorf("Unexpected configmap object\n\tDiff: %s", diff.ObjectGoPrintDiff(got, configmapObject))
+				}
+				return &http.Response{StatusCode: http.StatusCreated, Header: kubefedtesting.DefaultHeader(), Body: kubefedtesting.ObjBody(codec, &configmapObject)}, nil
 			default:
 				return nil, fmt.Errorf("unexpected request: %#v\n%#v", req.URL, req)
 			}

federation/pkg/kubefed/testing/testing.go

@@ -34,18 +34,22 @@ import (
 )
 
 type fakeAdminConfig struct {
-	pathOptions *clientcmd.PathOptions
+	pathOptions          *clientcmd.PathOptions
-	hostFactory cmdutil.Factory
+	hostFactory          cmdutil.Factory
+	targetClusterFactory cmdutil.Factory
+	targetClusterContext string
 }
 
-func NewFakeAdminConfig(f cmdutil.Factory, kubeconfigGlobal string) (util.AdminConfig, error) {
+func NewFakeAdminConfig(hostFactory cmdutil.Factory, targetFactory cmdutil.Factory, targetClusterContext, kubeconfigGlobal string) (util.AdminConfig, error) {
 	pathOptions := clientcmd.NewDefaultPathOptions()
 	pathOptions.GlobalFile = kubeconfigGlobal
 	pathOptions.EnvVar = ""
 
 	return &fakeAdminConfig{
-		pathOptions: pathOptions,
+		pathOptions:          pathOptions,
-		hostFactory: f,
+		hostFactory:          hostFactory,
+		targetClusterFactory: targetFactory,
+		targetClusterContext: targetClusterContext,
 	}, nil
 }
 
@@ -65,7 +69,10 @@ func (f *fakeAdminConfig) FederationClientset(context, kubeconfigPath string) (*
 	return fedclient.New(fakeRestClient), nil
 }
 
-func (f *fakeAdminConfig) HostFactory(host, kubeconfigPath string) cmdutil.Factory {
+func (f *fakeAdminConfig) ClusterFactory(context, kubeconfigPath string) cmdutil.Factory {
+	if f.targetClusterContext != "" && f.targetClusterContext == context {
+		return f.targetClusterFactory
+	}
 	return f.hostFactory
 }
 

federation/pkg/kubefed/unjoin.go

@@ -20,12 +20,15 @@ import (
 	"fmt"
 	"io"
 	"net/url"
+	"strings"
 
 	"k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime/schema"
 	federationapi "k8s.io/kubernetes/federation/apis/federation"
 	"k8s.io/kubernetes/federation/pkg/kubefed/util"
+	"k8s.io/kubernetes/pkg/api"
+	"k8s.io/kubernetes/pkg/client/clientset_generated/internalclientset"
 	"k8s.io/kubernetes/pkg/kubectl/cmd/templates"
 	cmdutil "k8s.io/kubernetes/pkg/kubectl/cmd/util"
 	"k8s.io/kubernetes/pkg/kubectl/resource"
@@ -86,13 +89,38 @@ func (u *unjoinFederation) Run(f cmdutil.Factory, cmdOut, cmdErr io.Writer, conf
 
 	// We want a separate client factory to communicate with the
 	// federation host cluster. See join_federation.go for details.
-	hostFactory := config.HostFactory(u.commonOptions.Host, u.commonOptions.Kubeconfig)
+	hostFactory := config.ClusterFactory(u.commonOptions.Host, u.commonOptions.Kubeconfig)
-	err = deleteSecret(hostFactory, cluster.Spec.SecretRef.Name, u.commonOptions.FederationSystemNamespace)
+	hostClientset, err := hostFactory.ClientSet()
-	if isNotFound(err) {
+	if err != nil {
-		fmt.Fprintf(cmdErr, "WARNING: secret %q not found in the host cluster, so it couldn't be deleted", cluster.Spec.SecretRef.Name)
-	} else if err != nil {
 		return err
 	}
+
+	secretName := cluster.Spec.SecretRef.Name
+	secret, err := hostClientset.Core().Secrets(u.commonOptions.FederationSystemNamespace).Get(secretName, metav1.GetOptions{})
+	if isNotFound(err) {
+		// If this is the case, we cannot get the cluster clientset to delete the
+		// config map from that cluster and obviously cannot delete the not existing secret.
+		// We just publish the warning as cluster has already been removed from federation.
+		fmt.Fprintf(cmdErr, "WARNING: secret %q not found in the host cluster, so it couldn't be deleted", secretName)
+	} else if err != nil {
+		fmt.Fprintf(cmdErr, "WARNING: Error retrieving secret from the base cluster")
+	} else {
+		err := deleteSecret(hostClientset, cluster.Spec.SecretRef.Name, u.commonOptions.FederationSystemNamespace)
+		if err != nil {
+			fmt.Fprintf(cmdErr, "WARNING: secret %q could not be deleted: %v", secretName, err)
+			// We anyways continue to try and delete the config map but with above warning
+		}
+
+		// We need to ensure deleting the config map created in the deregistered cluster
+		// This configmap was created when the cluster joined this federation to aid
+		// the kube-dns of that cluster to aid service discovery.
+		err = deleteConfigMapFromCluster(hostClientset, secret, cluster, u.commonOptions.FederationSystemNamespace)
+		if err != nil {
+			fmt.Fprintf(cmdErr, "WARNING: Encountered error in deleting kube-dns configmap, %v", err)
+			// We anyways continue to print success message but with above warning
+		}
+	}
+
 	_, err = fmt.Fprintf(cmdOut, "Successfully removed cluster %q from federation\n", u.commonOptions.Name)
 	return err
 }
@@ -100,7 +128,6 @@ func (u *unjoinFederation) Run(f cmdutil.Factory, cmdOut, cmdErr io.Writer, conf
 // popCluster fetches the cluster object with the given name, deletes
 // it and returns the deleted cluster object.
 func popCluster(f cmdutil.Factory, name string) (*federationapi.Cluster, error) {
-	// Boilerplate to create the secret in the host cluster.
 	mapper, typer := f.Object()
 	gvks, _, err := typer.ObjectKinds(&federationapi.Cluster{})
 	if err != nil {
@@ -135,13 +162,42 @@ func popCluster(f cmdutil.Factory, name string) (*federationapi.Cluster, error)
 	return cluster, rh.Delete("", name)
 }
 
-// deleteSecret deletes the secret with the given name from the host
+func deleteConfigMapFromCluster(hostClientset *internalclientset.Clientset, secret *api.Secret, cluster *federationapi.Cluster, fedSystemNamespace string) error {
-// cluster.
+	clientset, err := getClientsetFromCluster(secret, cluster)
-func deleteSecret(hostFactory cmdutil.Factory, name, namespace string) error {
-	clientset, err := hostFactory.ClientSet()
 	if err != nil {
 		return err
 	}
+
+	cmDep, err := getCMDeployment(hostClientset, fedSystemNamespace)
+	if err != nil {
+		return err
+	}
+	domainMap, ok := cmDep.Annotations[util.FedDomainMapKey]
+	if !ok {
+		return fmt.Errorf("kube-dns config map data missing from controller manager annotations")
+	}
+
+	configMap, err := clientset.Core().ConfigMaps(metav1.NamespaceSystem).Get(util.KubeDnsConfigmapName, metav1.GetOptions{})
+	if err != nil {
+		return err
+	}
+	if _, ok := configMap.Data[util.FedDomainMapKey]; !ok {
+		return clientset.Core().ConfigMaps(metav1.NamespaceSystem).Delete(util.KubeDnsConfigmapName, &metav1.DeleteOptions{})
+	}
+
+	newFedMapValue := removeConfigMapString(configMap.Data[util.FedDomainMapKey], domainMap)
+	if newFedMapValue != "" {
+		configMap.Data[util.FedDomainMapKey] = newFedMapValue
+		_, err := clientset.Core().ConfigMaps(metav1.NamespaceSystem).Update(configMap)
+		return err
+	}
+
+	return clientset.Core().ConfigMaps(metav1.NamespaceSystem).Delete(util.KubeDnsConfigmapName, &metav1.DeleteOptions{})
+}
+
+// deleteSecret deletes the secret with the given name from the host
+// cluster.
+func deleteSecret(clientset *internalclientset.Clientset, name, namespace string) error {
 	return clientset.Core().Secrets(namespace).Delete(name, &metav1.DeleteOptions{})
 }
 
@@ -153,3 +209,51 @@ func isNotFound(err error) bool {
 	}
 	return errors.IsNotFound(statusErr)
 }
+
+func getClientsetFromCluster(secret *api.Secret, cluster *federationapi.Cluster) (*internalclientset.Clientset, error) {
+	serverAddress, err := util.GetServerAddress(cluster)
+	if err != nil {
+		return nil, err
+	}
+	if serverAddress == "" {
+		return nil, fmt.Errorf("failed to get server address for the cluster: %s", cluster.Name)
+	}
+
+	clientset, err := util.GetClientsetFromSecret(secret, serverAddress)
+	if err != nil {
+		return nil, err
+	}
+	if clientset == nil {
+		// There is a possibility that the clientset is nil without any error reported
+		return nil, fmt.Errorf("failed for get client to access cluster: %s", cluster.Name)
+	}
+
+	return clientset, nil
+}
+
+// removeConfigMapString returns an empty string if last value is removed
+// or returns the remaining comma separated strings minus the one to be removed
+func removeConfigMapString(str string, toRemove string) string {
+	if str == "" {
+		return ""
+	}
+
+	values := strings.Split(str, ",")
+	if len(values) == 1 {
+		if values[0] == toRemove {
+			return ""
+		} else {
+			// Somehow our federation string is not here
+			// Dont do anything further
+			return values[0]
+		}
+	}
+
+	for i, v := range values {
+		if v == toRemove {
+			values = append(values[:i], values[i+1:]...)
+			break
+		}
+	}
+	return strings.Join(values, ",")
+}

federation/pkg/kubefed/unjoin_test.go

@@ -27,10 +27,14 @@ import (
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/client-go/dynamic"
 	"k8s.io/client-go/rest/fake"
+	"k8s.io/client-go/tools/clientcmd"
+	clientcmdapi "k8s.io/client-go/tools/clientcmd/api"
 	federationapi "k8s.io/kubernetes/federation/apis/federation"
 	kubefedtesting "k8s.io/kubernetes/federation/pkg/kubefed/testing"
+	"k8s.io/kubernetes/federation/pkg/kubefed/util"
 	"k8s.io/kubernetes/pkg/api"
 	"k8s.io/kubernetes/pkg/api/testapi"
+	"k8s.io/kubernetes/pkg/api/v1"
 	cmdtesting "k8s.io/kubernetes/pkg/kubectl/cmd/testing"
 	cmdutil "k8s.io/kubernetes/pkg/kubectl/cmd/util"
 )
@@ -58,7 +62,7 @@ func TestUnjoinFederation(t *testing.T) {
 	}{
 		// Tests that the contexts and credentials are read from the
 		// global, default kubeconfig and the correct cluster resource
-		// is deregisterd.
+		// is deregisterd and configmap kube-dns is removed from that cluster.
 		{
 			cluster:            "syndicate",
 			wantCluster:        "syndicate",
@@ -70,8 +74,8 @@ func TestUnjoinFederation(t *testing.T) {
 		},
 		// Tests that the contexts and credentials are read from the
 		// explicit kubeconfig file specified and the correct cluster
-		// resource is deregisterd. kubeconfig contains a single
+		// resource is deregisterd and configmap kube-dns is removed from that cluster.
-		// cluster and context.
+		// kubeconfig contains a single cluster and context.
 		{
 			cluster:            "ally",
 			wantCluster:        "ally",
@@ -83,8 +87,8 @@ func TestUnjoinFederation(t *testing.T) {
 		},
 		// Tests that the contexts and credentials are read from the
 		// explicit kubeconfig file specified and the correct cluster
-		// resource is deregisterd. kubeconfig consists of multiple
+		// resource is deregisterd and configmap kube-dns is removed from that
-		// clusters and contexts.
+		// cluster. kubeconfig consists of multiple clusters and contexts.
 		{
 			cluster:            "confederate",
 			wantCluster:        "confederate",
@@ -117,6 +121,11 @@ func TestUnjoinFederation(t *testing.T) {
 			expectedServer:     "https://10.20.30.40",
 			expectedErr:        fmt.Sprintf("WARNING: secret %q not found in the host cluster, so it couldn't be deleted", "noexist"),
 		},
+		// TODO: Figure out a way to test the scenarios of configmap deletion
+		// As of now we delete the config map after deriving the clientset using
+		// the cluster object we retrieved from the federation server and the
+		// secret object retrieved from the base cluster.
+		// Still to find out a way to introduce some fakes and unit test this path.
 	}
 
 	for i, tc := range testCases {
@@ -126,7 +135,7 @@ func TestUnjoinFederation(t *testing.T) {
 		errBuf := bytes.NewBuffer([]byte{})
 
 		hostFactory := fakeUnjoinHostFactory(tc.cluster)
-		adminConfig, err := kubefedtesting.NewFakeAdminConfig(hostFactory, tc.kubeconfigGlobal)
+		adminConfig, err := kubefedtesting.NewFakeAdminConfig(hostFactory, nil, "", tc.kubeconfigGlobal)
 		if err != nil {
 			t.Fatalf("[%d] unexpected error: %v", i, err)
 		}
@@ -147,6 +156,9 @@ func TestUnjoinFederation(t *testing.T) {
 					t.Errorf("[%d] unexpected error message: %s", i, cmdErrMsg)
 				}
 			}
+			// TODO: There are warnings posted on errBuf, which we ignore as of now
+			// and we should be able to test out these warnings also in future.
+			// This is linked to the previous todo comment.
 		} else {
 			if errMsg := errBuf.String(); errMsg != tc.expectedErr {
 				t.Errorf("[%d] expected warning: %s, got: %s, output: %s", i, tc.expectedErr, errMsg, buf.String())
@@ -202,6 +214,23 @@ func testUnjoinFederationFactory(name, server, secret string) cmdutil.Factory {
 
 func fakeUnjoinHostFactory(name string) cmdutil.Factory {
 	urlPrefix := "/api/v1/namespaces/federation-system/secrets/"
+
+	// Using dummy bytes for now
+	configBytes, _ := clientcmd.Write(clientcmdapi.Config{})
+	secretObject := v1.Secret{
+		TypeMeta: metav1.TypeMeta{
+			Kind:       "Secret",
+			APIVersion: "v1",
+		},
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      name,
+			Namespace: util.DefaultFederationSystemNamespace,
+		},
+		Data: map[string][]byte{
+			"kubeconfig": configBytes,
+		},
+	}
+
 	f, tf, codec, _ := cmdtesting.NewAPIFactory()
 	ns := dynamic.ContentConfig().NegotiatedSerializer
 	tf.ClientConfig = kubefedtesting.DefaultClientConfig()
@@ -210,15 +239,26 @@ func fakeUnjoinHostFactory(name string) cmdutil.Factory {
 		NegotiatedSerializer: ns,
 		Client: fake.CreateHTTPClient(func(req *http.Request) (*http.Response, error) {
 			switch p, m := req.URL.Path, req.Method; {
-			case strings.HasPrefix(p, urlPrefix) && m == http.MethodDelete:
+			case strings.HasPrefix(p, urlPrefix):
-				got := strings.TrimPrefix(p, urlPrefix)
+				switch m {
-				if got != name {
+				case http.MethodDelete:
-					return nil, errors.NewNotFound(api.Resource("secrets"), got)
+					got := strings.TrimPrefix(p, urlPrefix)
+					if got != name {
+						return nil, errors.NewNotFound(api.Resource("secrets"), got)
+					}
+					status := metav1.Status{
+						Status: "Success",
+					}
+					return &http.Response{StatusCode: http.StatusOK, Header: kubefedtesting.DefaultHeader(), Body: kubefedtesting.ObjBody(codec, &status)}, nil
+				case http.MethodGet:
+					got := strings.TrimPrefix(p, urlPrefix)
+					if got != name {
+						return nil, errors.NewNotFound(api.Resource("secrets"), got)
+					}
+					return &http.Response{StatusCode: http.StatusOK, Header: kubefedtesting.DefaultHeader(), Body: kubefedtesting.ObjBody(codec, &secretObject)}, nil
+				default:
+					return nil, fmt.Errorf("unexpected request method: %#v\n%#v", req.URL, req)
 				}
-				status := metav1.Status{
-					Status: "Success",
-				}
-				return &http.Response{StatusCode: http.StatusOK, Header: kubefedtesting.DefaultHeader(), Body: kubefedtesting.ObjBody(codec, &status)}, nil
 			default:
 				return nil, fmt.Errorf("unexpected request: %#v\n%#v", req.URL, req)
 			}

federation/pkg/kubefed/util/BUILD

@@ -12,6 +12,7 @@ go_library(
     srcs = ["util.go"],
     tags = ["automanaged"],
     deps = [
+        "//federation/apis/federation:go_default_library",
         "//federation/client/clientset_generated/federation_clientset:go_default_library",
         "//pkg/api:go_default_library",
         "//pkg/client/clientset_generated/internalclientset:go_default_library",
@@ -20,6 +21,8 @@ go_library(
         "//vendor:github.com/spf13/cobra",
         "//vendor:github.com/spf13/pflag",
         "//vendor:k8s.io/apimachinery/pkg/apis/meta/v1",
+        "//vendor:k8s.io/apimachinery/pkg/util/net",
+        "//vendor:k8s.io/client-go/rest",
         "//vendor:k8s.io/client-go/tools/clientcmd",
         "//vendor:k8s.io/client-go/tools/clientcmd/api",
     ],

federation/pkg/kubefed/util/util.go

@@ -17,7 +17,8 @@ limitations under the License.
 package util
 
 import (
-	"github.com/spf13/pflag"
+	"fmt"
+	"net"
 
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/client-go/tools/clientcmd"
@@ -28,7 +29,12 @@ import (
 	kubectlcmd "k8s.io/kubernetes/pkg/kubectl/cmd"
 	cmdutil "k8s.io/kubernetes/pkg/kubectl/cmd/util"
 
+	utilnet "k8s.io/apimachinery/pkg/util/net"
+	restclient "k8s.io/client-go/rest"
+	federationapi "k8s.io/kubernetes/federation/apis/federation"
+
 	"github.com/spf13/cobra"
+	"github.com/spf13/pflag"
 )
 
 const (
@@ -36,9 +42,18 @@ const (
 	// stores a cluster's credentials.
 	KubeconfigSecretDataKey = "kubeconfig"
 
+	// Used in and to create the kube-dns configmap storing the zone info
+	FedDomainMapKey      = "federations"
+	KubeDnsConfigmapName = "kube-dns"
+
 	// DefaultFederationSystemNamespace is the namespace in which
 	// federation system components are hosted.
 	DefaultFederationSystemNamespace = "federation-system"
+
+	// Used to build a clientset for a cluster using the secret
+	userAgentName = "kubefed-tool"
+	KubeAPIQPS    = 20.0
+	KubeAPIBurst  = 30
 )
 
 // AdminConfig provides a filesystem based kubeconfig (via
@@ -50,9 +65,9 @@ type AdminConfig interface {
 	// FedClientSet provides a federation API compliant clientset
 	// to communicate with the federation control plane api server
 	FederationClientset(context, kubeconfigPath string) (*fedclient.Clientset, error)
-	// HostFactory provides a mechanism to communicate with the
+	// ClusterFactory provides a mechanism to communicate with the
-	// cluster where federation control plane is hosted.
+	// cluster derived from the context and the kubeconfig.
-	HostFactory(hostcontext, kubeconfigPath string) cmdutil.Factory
+	ClusterFactory(context, kubeconfigPath string) cmdutil.Factory
 }
 
 // adminConfig implements the AdminConfig interface.
@@ -81,8 +96,8 @@ func (a *adminConfig) FederationClientset(context, kubeconfigPath string) (*fedc
 	return fedclient.NewForConfigOrDie(fedClientConfig), nil
 }
 
-func (a *adminConfig) HostFactory(hostcontext, kubeconfigPath string) cmdutil.Factory {
+func (a *adminConfig) ClusterFactory(context, kubeconfigPath string) cmdutil.Factory {
-	hostClientConfig := a.getClientConfig(hostcontext, kubeconfigPath)
+	hostClientConfig := a.getClientConfig(context, kubeconfigPath)
 	return cmdutil.NewFactory(hostClientConfig)
 }
 
@@ -144,3 +159,55 @@ func CreateKubeconfigSecret(clientset *client.Clientset, kubeconfig *clientcmdap
 	}
 	return secret, nil
 }
+
+var kubeconfigGetterForSecret = func(secret *api.Secret) clientcmd.KubeconfigGetter {
+	return func() (*clientcmdapi.Config, error) {
+		var data []byte
+		ok := false
+		data, ok = secret.Data[KubeconfigSecretDataKey]
+		if !ok {
+			return nil, fmt.Errorf("secret does not have data with key: %s", KubeconfigSecretDataKey)
+		}
+		return clientcmd.Load(data)
+	}
+}
+
+func GetClientsetFromSecret(secret *api.Secret, serverAddress string) (*client.Clientset, error) {
+	clusterConfig, err := buildConfigFromSecret(secret, serverAddress)
+	if err == nil && clusterConfig != nil {
+		clientset := client.NewForConfigOrDie(restclient.AddUserAgent(clusterConfig, userAgentName))
+		return clientset, nil
+	}
+	return nil, err
+}
+
+func GetServerAddress(c *federationapi.Cluster) (string, error) {
+	hostIP, err := utilnet.ChooseHostInterface()
+	if err != nil {
+		return "", err
+	}
+
+	for _, item := range c.Spec.ServerAddressByClientCIDRs {
+		_, cidrnet, err := net.ParseCIDR(item.ClientCIDR)
+		if err != nil {
+			return "", err
+		}
+		if cidrnet.Contains(hostIP) {
+			return item.ServerAddress, nil
+		}
+	}
+
+	return "", nil
+}
+
+func buildConfigFromSecret(secret *api.Secret, serverAddress string) (*restclient.Config, error) {
+	kubeconfigGetter := kubeconfigGetterForSecret(secret)
+	clusterConfig, err := clientcmd.BuildConfigFromKubeconfigGetter(serverAddress, kubeconfigGetter)
+	if err != nil {
+		return nil, err
+	}
+	clusterConfig.QPS = KubeAPIQPS
+	clusterConfig.Burst = KubeAPIBurst
+
+	return clusterConfig, nil
+}