From 005f5cd21e07924b32cca33cf101de295a50c0c8 Mon Sep 17 00:00:00 2001 From: "Madhusudan.C.S" Date: Tue, 15 Nov 2016 19:24:35 -0800 Subject: [PATCH] [Federation][join-flags] Add flags for cluster context and secret names while joining clusters to federation. Vast majority of cluster contexts are not RFC 1123 subdomains. Since cluster and secret names for the API objects are derived from the cluster context name, there is no way for users to join clusters with such context names to federation, unless they modify the context name in their kubeconfigs itself. That's a lot of inconvenience and entirely goes against the goal and beats the purpose of the `kubefed` tool. So we are providing these flags to allow users to override these values. Also, since users register their clusters with federation, it is makes sense in terms of user experience to make the cluster name a positional argument because that feels more natural. Also, specifying cluster name in the join command as a mandatory positional argument make `kubefed join` consistent with `kubefed unjoin`. This also means `--cluster- context` is now made a flag and defaults to cluster name if unspecified. `--secret-name` also defaults to the cluster name if unspecified. --- federation/pkg/kubefed/BUILD | 1 + federation/pkg/kubefed/join.go | 46 ++++++++++------ federation/pkg/kubefed/join_test.go | 76 ++++++++++++++++++++------- federation/pkg/kubefed/unjoin_test.go | 2 +- hack/verify-flags/known-flags.txt | 2 + 5 files changed, 92 insertions(+), 35 deletions(-) diff --git a/federation/pkg/kubefed/BUILD b/federation/pkg/kubefed/BUILD index b2271accfd5..e2b351dc6d6 100644 --- a/federation/pkg/kubefed/BUILD +++ b/federation/pkg/kubefed/BUILD @@ -63,5 +63,6 @@ go_test( "//pkg/client/unversioned/clientcmd/api:go_default_library", "//pkg/kubectl/cmd/testing:go_default_library", "//pkg/kubectl/cmd/util:go_default_library", + "//vendor:k8s.io/client-go/pkg/util/diff", ], ) diff --git a/federation/pkg/kubefed/join.go b/federation/pkg/kubefed/join.go index 94f634ee2ed..2797cb2e324 100644 --- a/federation/pkg/kubefed/join.go +++ b/federation/pkg/kubefed/join.go @@ -49,8 +49,11 @@ var ( server. Please use the --context flag otherwise.`) join_example = templates.Examples(` # Join a cluster to a federation by specifying the - # cluster context name and the context name of the - # federation control plane's host cluster. + # cluster name and the context name of the federation + # control plane's host cluster. Cluster name must be + # a valid RFC 1123 subdomain name. Cluster context + # must be specified if the cluster name is different + # than the cluster's context in the local kubeconfig. kubectl join foo --host-cluster-context=bar`) ) @@ -58,7 +61,7 @@ var ( // federation. func NewCmdJoin(f cmdutil.Factory, cmdOut io.Writer, config util.AdminConfig) *cobra.Command { cmd := &cobra.Command{ - Use: "join CLUSTER_CONTEXT --host-cluster-context=HOST_CONTEXT", + Use: "join CLUSTER_NAME --host-cluster-context=HOST_CONTEXT", Short: "Join a cluster to a federation", Long: join_long, Example: join_example, @@ -73,6 +76,8 @@ func NewCmdJoin(f cmdutil.Factory, cmdOut io.Writer, config util.AdminConfig) *c cmdutil.AddPrinterFlags(cmd) cmdutil.AddGeneratorFlags(cmd, cmdutil.ClusterV1Beta1GeneratorName) util.AddSubcommandFlags(cmd) + cmd.Flags().String("cluster-context", "", "Name of the cluster's context in the local kubeconfig. Defaults to cluster name if unspecified.") + cmd.Flags().String("secret-name", "", "Name of the secret where the cluster's credentials will be stored in the host cluster. This name should be a valid RFC 1035 label. Defaults to cluster name if unspecified.") return cmd } @@ -82,9 +87,18 @@ func joinFederation(f cmdutil.Factory, cmdOut io.Writer, config util.AdminConfig if err != nil { return err } + clusterContext := cmdutil.GetFlagString(cmd, "cluster-context") + secretName := cmdutil.GetFlagString(cmd, "secret-name") dryRun := cmdutil.GetDryRunFlag(cmd) - glog.V(2).Infof("Args and flags: name %s, host: %s, host-system-namespace: %s, kubeconfig: %s, dry-run: %s", joinFlags.Name, joinFlags.Host, joinFlags.FederationSystemNamespace, joinFlags.Kubeconfig, dryRun) + if clusterContext == "" { + clusterContext = joinFlags.Name + } + if secretName == "" { + secretName = joinFlags.Name + } + + glog.V(2).Infof("Args and flags: name %s, host: %s, host-system-namespace: %s, kubeconfig: %s, cluster-context: %s, secret-name: %s, dry-run: %s", joinFlags.Name, joinFlags.Host, joinFlags.FederationSystemNamespace, joinFlags.Kubeconfig, clusterContext, secretName, dryRun) po := config.PathOptions() po.LoadingRules.ExplicitPath = joinFlags.Kubeconfig @@ -92,7 +106,7 @@ func joinFederation(f cmdutil.Factory, cmdOut io.Writer, config util.AdminConfig if err != nil { return err } - generator, err := clusterGenerator(clientConfig, joinFlags.Name) + generator, err := clusterGenerator(clientConfig, joinFlags.Name, clusterContext, secretName) if err != nil { glog.V(2).Infof("Failed creating cluster generator: %v", err) return err @@ -116,7 +130,7 @@ func joinFederation(f cmdutil.Factory, cmdOut io.Writer, config util.AdminConfig // 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, joinFlags.FederationSystemNamespace, joinFlags.Name, dryRun) + _, err = createSecret(hostFactory, clientConfig, joinFlags.FederationSystemNamespace, clusterContext, secretName, dryRun) if err != nil { glog.V(2).Infof("Failed creating the cluster credentials secret: %v", err) return err @@ -150,12 +164,12 @@ 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, name string, dryRun bool) (runtime.Object, error) { +func createSecret(hostFactory cmdutil.Factory, 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, name) + newClientConfig, err := minifyConfig(clientConfig, contextName) if err != nil { - glog.V(2).Infof("Failed to minify the kubeconfig for the given context %q: %v", name, err) + glog.V(2).Infof("Failed to minify the kubeconfig for the given context %q: %v", contextName, err) return nil, err } @@ -163,28 +177,28 @@ func createSecret(hostFactory cmdutil.Factory, clientConfig *clientcmdapi.Config // contents are inlined. err = clientcmdapi.FlattenConfig(newClientConfig) if err != nil { - glog.V(2).Infof("Failed to flatten the kubeconfig for the given context %q: %v", name, err) + glog.V(2).Infof("Failed to flatten the kubeconfig for the given context %q: %v", contextName, err) return nil, err } // Boilerplate to create the secret in the host cluster. clientset, err := hostFactory.ClientSet() if err != nil { - glog.V(2).Infof("Failed to serialize the kubeconfig for the given context %q: %v", name, err) + 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, name, dryRun) + return util.CreateKubeconfigSecret(clientset, newClientConfig, namespace, secretName, dryRun) } // clusterGenerator extracts the cluster information from the supplied // kubeconfig and builds a StructuredGenerator for the // `federation/cluster` API resource. -func clusterGenerator(clientConfig *clientcmdapi.Config, name string) (kubectl.StructuredGenerator, error) { +func clusterGenerator(clientConfig *clientcmdapi.Config, name, contextName, secretName string) (kubectl.StructuredGenerator, error) { // Get the context from the config. - ctx, found := clientConfig.Contexts[name] + ctx, found := clientConfig.Contexts[contextName] if !found { - return nil, fmt.Errorf("cluster context %q not found", name) + return nil, fmt.Errorf("cluster context %q not found", contextName) } // Get the cluster object corresponding to the supplied context. @@ -207,7 +221,7 @@ func clusterGenerator(clientConfig *clientcmdapi.Config, name string) (kubectl.S Name: name, ClientCIDR: defaultClientCIDR, ServerAddress: serverAddress, - SecretName: name, + SecretName: secretName, } return generator, nil } diff --git a/federation/pkg/kubefed/join_test.go b/federation/pkg/kubefed/join_test.go index 2865f6849be..ca0de21e123 100644 --- a/federation/pkg/kubefed/join_test.go +++ b/federation/pkg/kubefed/join_test.go @@ -23,6 +23,7 @@ import ( "net/http" "testing" + "k8s.io/client-go/pkg/util/diff" federationapi "k8s.io/kubernetes/federation/apis/federation/v1beta1" kubefedtesting "k8s.io/kubernetes/federation/pkg/kubefed/testing" "k8s.io/kubernetes/federation/pkg/kubefed/util" @@ -52,6 +53,8 @@ func TestJoinFederation(t *testing.T) { testCases := []struct { cluster string + clusterCtx string + secret string server string token string kubeconfigGlobal string @@ -61,6 +64,8 @@ func TestJoinFederation(t *testing.T) { }{ { cluster: "syndicate", + clusterCtx: "", + secret: "", server: "https://10.20.30.40", token: "badge", kubeconfigGlobal: fakeKubeFiles[0], @@ -70,6 +75,8 @@ func TestJoinFederation(t *testing.T) { }, { cluster: "ally", + clusterCtx: "", + secret: "", server: "ally256.example.com:80", token: "souvenir", kubeconfigGlobal: fakeKubeFiles[0], @@ -79,6 +86,19 @@ func TestJoinFederation(t *testing.T) { }, { cluster: "confederate", + clusterCtx: "", + secret: "", + server: "10.8.8.8", + token: "totem", + kubeconfigGlobal: fakeKubeFiles[1], + kubeconfigExplicit: fakeKubeFiles[2], + expectedServer: "https://10.8.8.8", + expectedErr: "", + }, + { + cluster: "associate", + clusterCtx: "confederate", + secret: "confidential", server: "10.8.8.8", token: "totem", kubeconfigGlobal: fakeKubeFiles[1], @@ -88,6 +108,8 @@ func TestJoinFederation(t *testing.T) { }, { cluster: "affiliate", + clusterCtx: "", + secret: "", server: "https://10.20.30.40", token: "badge", kubeconfigGlobal: fakeKubeFiles[0], @@ -99,10 +121,10 @@ func TestJoinFederation(t *testing.T) { for i, tc := range testCases { cmdErrMsg = "" - f := testJoinFederationFactory(tc.cluster, tc.expectedServer) + f := testJoinFederationFactory(tc.cluster, tc.secret, tc.expectedServer) buf := bytes.NewBuffer([]byte{}) - hostFactory, err := fakeJoinHostFactory(tc.cluster, tc.server, tc.token) + hostFactory, err := fakeJoinHostFactory(tc.cluster, tc.clusterCtx, tc.secret, tc.server, tc.token) if err != nil { t.Fatalf("[%d] unexpected error: %v", i, err) } @@ -115,7 +137,14 @@ func TestJoinFederation(t *testing.T) { cmd := NewCmdJoin(f, buf, adminConfig) cmd.Flags().Set("kubeconfig", tc.kubeconfigExplicit) - cmd.Flags().Set("host", "substrate") + cmd.Flags().Set("host-cluster-context", "substrate") + if tc.clusterCtx != "" { + cmd.Flags().Set("cluster-context", tc.clusterCtx) + } + if tc.secret != "" { + cmd.Flags().Set("secret-name", tc.secret) + } + cmd.Run(cmd, []string{tc.cluster}) if tc.expectedErr == "" { @@ -136,8 +165,12 @@ func TestJoinFederation(t *testing.T) { } } -func testJoinFederationFactory(name, server string) cmdutil.Factory { - want := fakeCluster(name, server) +func testJoinFederationFactory(clusterName, secretName, server string) cmdutil.Factory { + if secretName == "" { + secretName = clusterName + } + + want := fakeCluster(clusterName, secretName, server) f, tf, _, _ := cmdtesting.NewAPIFactory() codec := testapi.Federation.Codec() ns := dynamic.ContentConfig().NegotiatedSerializer @@ -156,7 +189,7 @@ func testJoinFederationFactory(name, server string) cmdutil.Factory { return nil, err } if !api.Semantic.DeepEqual(got, want) { - return nil, fmt.Errorf("unexpected cluster object\n\tgot: %#v\n\twant: %#v", got, want) + return nil, fmt.Errorf("Unexpected cluster object\n\tDiff: %s", diff.ObjectGoPrintDiff(got, want)) } return &http.Response{StatusCode: http.StatusCreated, Header: kubefedtesting.DefaultHeader(), Body: kubefedtesting.ObjBody(codec, &want)}, nil default: @@ -168,25 +201,32 @@ func testJoinFederationFactory(name, server string) cmdutil.Factory { return f } -func fakeJoinHostFactory(name, server, token string) (cmdutil.Factory, error) { +func fakeJoinHostFactory(clusterName, clusterCtx, secretName, server, token string) (cmdutil.Factory, error) { + if clusterCtx == "" { + clusterCtx = clusterName + } + if secretName == "" { + secretName = clusterName + } + kubeconfig := clientcmdapi.Config{ Clusters: map[string]*clientcmdapi.Cluster{ - name: { + clusterCtx: { Server: server, }, }, AuthInfos: map[string]*clientcmdapi.AuthInfo{ - name: { + clusterCtx: { Token: token, }, }, Contexts: map[string]*clientcmdapi.Context{ - name: { - Cluster: name, - AuthInfo: name, + clusterCtx: { + Cluster: clusterCtx, + AuthInfo: clusterCtx, }, }, - CurrentContext: name, + CurrentContext: clusterCtx, } configBytes, err := clientcmd.Write(kubeconfig) if err != nil { @@ -198,7 +238,7 @@ func fakeJoinHostFactory(name, server, token string) (cmdutil.Factory, error) { APIVersion: "v1", }, ObjectMeta: v1.ObjectMeta{ - Name: name, + Name: secretName, Namespace: util.DefaultFederationSystemNamespace, }, Data: map[string][]byte{ @@ -224,7 +264,7 @@ func fakeJoinHostFactory(name, server, token string) (cmdutil.Factory, error) { return nil, err } if !api.Semantic.DeepEqual(got, secretObject) { - return nil, fmt.Errorf("Unexpected secret object\n\tgot: %#v\n\twant: %#v", got, secretObject) + 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 default: @@ -235,10 +275,10 @@ func fakeJoinHostFactory(name, server, token string) (cmdutil.Factory, error) { return f, nil } -func fakeCluster(name, server string) federationapi.Cluster { +func fakeCluster(clusterName, secretName, server string) federationapi.Cluster { return federationapi.Cluster{ ObjectMeta: v1.ObjectMeta{ - Name: name, + Name: clusterName, }, Spec: federationapi.ClusterSpec{ ServerAddressByClientCIDRs: []federationapi.ServerAddressByClientCIDR{ @@ -248,7 +288,7 @@ func fakeCluster(name, server string) federationapi.Cluster { }, }, SecretRef: &v1.LocalObjectReference{ - Name: name, + Name: secretName, }, }, } diff --git a/federation/pkg/kubefed/unjoin_test.go b/federation/pkg/kubefed/unjoin_test.go index 6fc5e1405de..bc89ab725a3 100644 --- a/federation/pkg/kubefed/unjoin_test.go +++ b/federation/pkg/kubefed/unjoin_test.go @@ -159,7 +159,7 @@ func TestUnjoinFederation(t *testing.T) { func testUnjoinFederationFactory(name, server, secret string) cmdutil.Factory { urlPrefix := "/clusters/" - cluster := fakeCluster(name, server) + cluster := fakeCluster(name, name, server) if secret != "" { cluster.Spec.SecretRef.Name = secret } diff --git a/hack/verify-flags/known-flags.txt b/hack/verify-flags/known-flags.txt index 348411f09fb..a2aafa6e3a4 100644 --- a/hack/verify-flags/known-flags.txt +++ b/hack/verify-flags/known-flags.txt @@ -73,6 +73,7 @@ clientset-path cloud-config cloud-provider cluster-cidr +cluster-context cluster-dns cluster-domain cluster-ip @@ -507,6 +508,7 @@ schema-cache-dir scopes seccomp-profile-root secondary-node-eviction-rate +secret-name secure-port serialize-image-pulls server-start-timeout