From b4aa26704dcf118617fd04acb620ea0726eb95fe Mon Sep 17 00:00:00 2001 From: Maru Newby Date: Thu, 9 Feb 2017 13:26:06 -0800 Subject: [PATCH] kubefed: Bind flag values automatically --- federation/pkg/kubefed/BUILD | 1 + federation/pkg/kubefed/init/BUILD | 1 + federation/pkg/kubefed/init/init.go | 145 +++++++++++++++++----------- federation/pkg/kubefed/join.go | 69 ++++++++----- federation/pkg/kubefed/unjoin.go | 31 +++--- federation/pkg/kubefed/util/BUILD | 1 + federation/pkg/kubefed/util/util.go | 30 +++--- 7 files changed, 166 insertions(+), 112 deletions(-) diff --git a/federation/pkg/kubefed/BUILD b/federation/pkg/kubefed/BUILD index 6f5c0f3fc9..584982cfcf 100644 --- a/federation/pkg/kubefed/BUILD +++ b/federation/pkg/kubefed/BUILD @@ -27,6 +27,7 @@ go_library( "//pkg/kubectl/resource:go_default_library", "//vendor:github.com/golang/glog", "//vendor:github.com/spf13/cobra", + "//vendor:github.com/spf13/pflag", "//vendor:k8s.io/apimachinery/pkg/api/errors", "//vendor:k8s.io/apimachinery/pkg/apis/meta/v1", "//vendor:k8s.io/apimachinery/pkg/runtime", diff --git a/federation/pkg/kubefed/init/BUILD b/federation/pkg/kubefed/init/BUILD index 7c844d3714..96938a8d7f 100644 --- a/federation/pkg/kubefed/init/BUILD +++ b/federation/pkg/kubefed/init/BUILD @@ -24,6 +24,7 @@ go_library( "//pkg/kubectl/cmd/util:go_default_library", "//pkg/version:go_default_library", "//vendor:github.com/spf13/cobra", + "//vendor:github.com/spf13/pflag", "//vendor:k8s.io/apimachinery/pkg/api/resource", "//vendor:k8s.io/apimachinery/pkg/apis/meta/v1", "//vendor:k8s.io/apimachinery/pkg/util/wait", diff --git a/federation/pkg/kubefed/init/init.go b/federation/pkg/kubefed/init/init.go index 6307c752b8..2d8632352d 100644 --- a/federation/pkg/kubefed/init/init.go +++ b/federation/pkg/kubefed/init/init.go @@ -34,6 +34,7 @@ import ( "fmt" "io" "net" + "sort" "strconv" "strings" "time" @@ -57,7 +58,7 @@ import ( "k8s.io/kubernetes/pkg/version" "github.com/spf13/cobra" - "sort" + "github.com/spf13/pflag" ) const ( @@ -119,34 +120,64 @@ var ( hyperkubeImageName = "gcr.io/google_containers/hyperkube-amd64" ) +type initFederation struct { + commonOptions util.SubcommandOptions + options initFederationOptions +} + +type initFederationOptions struct { + dnsZoneName string + image string + dnsProvider string + etcdPVCapacity string + etcdPersistentStorage bool + dryRun bool + storageBackend string + apiServerOverridesString string + apiServerOverrides map[string]string + controllerManagerOverridesString string + controllerManagerOverrides map[string]string + apiServerServiceTypeString string + apiServerServiceType v1.ServiceType + apiServerAdvertiseAddress string +} + +func (o *initFederationOptions) Bind(flags *pflag.FlagSet) { + defaultImage := fmt.Sprintf("%s:%s", hyperkubeImageName, version.Get()) + + flags.StringVar(&o.dnsZoneName, "dns-zone-name", "", "DNS suffix for this federation. Federated Service DNS names are published with this suffix.") + flags.StringVar(&o.image, "image", defaultImage, "Image to use for federation API server and controller manager binaries.") + flags.StringVar(&o.dnsProvider, "dns-provider", "google-clouddns", "Dns provider to be used for this deployment.") + flags.StringVar(&o.etcdPVCapacity, "etcd-pv-capacity", "10Gi", "Size of persistent volume claim to be used for etcd.") + flags.BoolVar(&o.etcdPersistentStorage, "etcd-persistent-storage", true, "Use persistent volume for etcd. Defaults to 'true'.") + flags.BoolVar(&o.dryRun, "dry-run", false, "dry run without sending commands to server.") + flags.StringVar(&o.storageBackend, "storage-backend", "etcd2", "The storage backend for persistence. Options: 'etcd2' (default), 'etcd3'.") + flags.StringVar(&o.apiServerOverridesString, "apiserver-arg-overrides", "", "comma separated list of federation-apiserver arguments to override: Example \"--arg1=value1,--arg2=value2...\"") + flags.StringVar(&o.controllerManagerOverridesString, "controllermanager-arg-overrides", "", "comma separated list of federation-controller-manager arguments to override: Example \"--arg1=value1,--arg2=value2...\"") + flags.StringVar(&o.apiServerServiceTypeString, apiserverServiceTypeFlag, string(v1.ServiceTypeLoadBalancer), "The type of service to create for federation API server. Options: 'LoadBalancer' (default), 'NodePort'.") + flags.StringVar(&o.apiServerAdvertiseAddress, apiserverAdvertiseAddressFlag, "", "Preferred address to advertise api server nodeport service. Valid only if '"+apiserverServiceTypeFlag+"=NodePort'.") +} + // NewCmdInit defines the `init` command that bootstraps a federation // control plane inside a set of host clusters. func NewCmdInit(cmdOut io.Writer, config util.AdminConfig) *cobra.Command { + opts := &initFederation{} + cmd := &cobra.Command{ Use: "init FEDERATION_NAME --host-cluster-context=HOST_CONTEXT", Short: "init initializes a federation control plane", Long: init_long, Example: init_example, Run: func(cmd *cobra.Command, args []string) { - err := initFederation(cmdOut, config, cmd, args) - cmdutil.CheckErr(err) + cmdutil.CheckErr(opts.Complete(cmd, args)) + cmdutil.CheckErr(opts.Run(cmdOut, config)) }, } - defaultImage := fmt.Sprintf("%s:%s", hyperkubeImageName, version.Get()) + flags := cmd.Flags() + opts.commonOptions.Bind(flags) + opts.options.Bind(flags) - util.AddSubcommandFlags(cmd) - cmd.Flags().String("dns-zone-name", "", "DNS suffix for this federation. Federated Service DNS names are published with this suffix.") - cmd.Flags().String("image", defaultImage, "Image to use for federation API server and controller manager binaries.") - cmd.Flags().String("dns-provider", "google-clouddns", "Dns provider to be used for this deployment.") - cmd.Flags().String("etcd-pv-capacity", "10Gi", "Size of persistent volume claim to be used for etcd.") - cmd.Flags().Bool("etcd-persistent-storage", true, "Use persistent volume for etcd. Defaults to 'true'.") - cmd.Flags().Bool("dry-run", false, "dry run without sending commands to server.") - cmd.Flags().String("apiserver-arg-overrides", "", "comma separated list of federation-apiserver arguments to override: Example \"--arg1=value1,--arg2=value2...\"") - cmd.Flags().String("controllermanager-arg-overrides", "", "comma separated list of federation-controller-manager arguments to override: Example \"--arg1=value1,--arg2=value2...\"") - cmd.Flags().String("storage-backend", "etcd2", "The storage backend for persistence. Options: 'etcd2' (default), 'etcd3'.") - cmd.Flags().String(apiserverServiceTypeFlag, string(v1.ServiceTypeLoadBalancer), "The type of service to create for federation API server. Options: 'LoadBalancer' (default), 'NodePort'.") - cmd.Flags().String(apiserverAdvertiseAddressFlag, "", "Preferred address to advertise api server nodeport service. Valid only if '"+apiserverServiceTypeFlag+"=NodePort'.") return cmd } @@ -157,81 +188,79 @@ type entityKeyPairs struct { admin *triple.KeyPair } -// initFederation initializes a federation control plane. -// See the design doc in https://github.com/kubernetes/kubernetes/pull/34484 -// for details. -func initFederation(cmdOut io.Writer, config util.AdminConfig, cmd *cobra.Command, args []string) error { - initFlags, err := util.GetSubcommandFlags(cmd, args) +// Complete ensures that options are valid and marshals them if necessary. +func (i *initFederation) Complete(cmd *cobra.Command, args []string) error { + err := i.commonOptions.SetName(cmd, args) if err != nil { return err } - dnsZoneName := cmdutil.GetFlagString(cmd, "dns-zone-name") - image := cmdutil.GetFlagString(cmd, "image") - dnsProvider := cmdutil.GetFlagString(cmd, "dns-provider") - etcdPVCapacity := cmdutil.GetFlagString(cmd, "etcd-pv-capacity") - etcdPersistence := cmdutil.GetFlagBool(cmd, "etcd-persistent-storage") - dryRun := cmdutil.GetDryRunFlag(cmd) - storageBackend := cmdutil.GetFlagString(cmd, "storage-backend") - apiserverServiceType := v1.ServiceType(cmdutil.GetFlagString(cmd, apiserverServiceTypeFlag)) - apiserverAdvertiseAddress := cmdutil.GetFlagString(cmd, apiserverAdvertiseAddressFlag) - if apiserverServiceType != v1.ServiceTypeLoadBalancer && apiserverServiceType != v1.ServiceTypeNodePort { - return fmt.Errorf("invalid %s: %s, should be either %s or %s", apiserverServiceTypeFlag, apiserverServiceType, v1.ServiceTypeLoadBalancer, v1.ServiceTypeNodePort) + i.options.apiServerServiceType = v1.ServiceType(i.options.apiServerServiceTypeString) + if i.options.apiServerServiceType != v1.ServiceTypeLoadBalancer && i.options.apiServerServiceType != v1.ServiceTypeNodePort { + return fmt.Errorf("invalid %s: %s, should be either %s or %s", apiserverServiceTypeFlag, i.options.apiServerServiceType, v1.ServiceTypeLoadBalancer, v1.ServiceTypeNodePort) } - if apiserverAdvertiseAddress != "" { - ip := net.ParseIP(apiserverAdvertiseAddress) + if i.options.apiServerAdvertiseAddress != "" { + ip := net.ParseIP(i.options.apiServerAdvertiseAddress) if ip == nil { - return fmt.Errorf("invalid %s: %s, should be a valid ip address", apiserverAdvertiseAddressFlag, apiserverAdvertiseAddress) + return fmt.Errorf("invalid %s: %s, should be a valid ip address", apiserverAdvertiseAddressFlag, i.options.apiServerAdvertiseAddress) } - if apiserverServiceType != v1.ServiceTypeNodePort { + if i.options.apiServerServiceType != v1.ServiceTypeNodePort { return fmt.Errorf("%s should be passed only with '%s=NodePort'", apiserverAdvertiseAddressFlag, apiserverServiceTypeFlag) } } - apiserverArgOverrides, err := marshallOverrides(cmdutil.GetFlagString(cmd, "apiserver-arg-overrides")) + + i.options.apiServerOverrides, err = marshallOverrides(i.options.apiServerOverridesString) if err != nil { return fmt.Errorf("Error marshalling --apiserver-arg-overrides: %v", err) } - cmArgOverrides, err := marshallOverrides(cmdutil.GetFlagString(cmd, "controllermanager-arg-overrides")) + i.options.controllerManagerOverrides, err = marshallOverrides(i.options.controllerManagerOverridesString) if err != nil { return fmt.Errorf("Error marshalling --controllermanager-arg-overrides: %v", err) } - hostFactory := config.HostFactory(initFlags.Host, initFlags.Kubeconfig) + return nil +} + +// Run initializes a federation control plane. +// 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) hostClientset, err := hostFactory.ClientSet() if err != nil { return err } - serverName := fmt.Sprintf("%s-apiserver", initFlags.Name) + serverName := fmt.Sprintf("%s-apiserver", i.commonOptions.Name) serverCredName := fmt.Sprintf("%s-credentials", serverName) - cmName := fmt.Sprintf("%s-controller-manager", initFlags.Name) + cmName := fmt.Sprintf("%s-controller-manager", i.commonOptions.Name) cmKubeconfigName := fmt.Sprintf("%s-kubeconfig", cmName) // 1. Create a namespace for federation system components - _, err = createNamespace(hostClientset, initFlags.FederationSystemNamespace, dryRun) + _, err = createNamespace(hostClientset, i.commonOptions.FederationSystemNamespace, i.options.dryRun) if err != nil { return err } // 2. Expose a network endpoint for the federation API server - svc, ips, hostnames, err := createService(hostClientset, initFlags.FederationSystemNamespace, serverName, apiserverAdvertiseAddress, apiserverServiceType, dryRun) + svc, ips, hostnames, err := createService(hostClientset, i.commonOptions.FederationSystemNamespace, serverName, i.options.apiServerAdvertiseAddress, i.options.apiServerServiceType, i.options.dryRun) if err != nil { return err } // 3. Generate TLS certificates and credentials - entKeyPairs, err := genCerts(initFlags.FederationSystemNamespace, initFlags.Name, svc.Name, HostClusterLocalDNSZoneName, ips, hostnames) + entKeyPairs, err := genCerts(i.commonOptions.FederationSystemNamespace, i.commonOptions.Name, svc.Name, HostClusterLocalDNSZoneName, ips, hostnames) if err != nil { return err } - _, err = createAPIServerCredentialsSecret(hostClientset, initFlags.FederationSystemNamespace, serverCredName, entKeyPairs, dryRun) + _, err = createAPIServerCredentialsSecret(hostClientset, i.commonOptions.FederationSystemNamespace, serverCredName, entKeyPairs, i.options.dryRun) if err != nil { return err } // 4. Create a kubeconfig secret - _, err = createControllerManagerKubeconfigSecret(hostClientset, initFlags.FederationSystemNamespace, initFlags.Name, svc.Name, cmKubeconfigName, entKeyPairs, dryRun) + _, err = createControllerManagerKubeconfigSecret(hostClientset, i.commonOptions.FederationSystemNamespace, i.commonOptions.Name, svc.Name, cmKubeconfigName, entKeyPairs, i.options.dryRun) if err != nil { return err } @@ -240,8 +269,8 @@ func initFederation(cmdOut io.Writer, config util.AdminConfig, cmd *cobra.Comman // API server's state. This is where federation API server's etcd // stores its data. var pvc *api.PersistentVolumeClaim - if etcdPersistence { - pvc, err = createPVC(hostClientset, initFlags.FederationSystemNamespace, svc.Name, etcdPVCapacity, dryRun) + if i.options.etcdPersistentStorage { + pvc, err = createPVC(hostClientset, i.commonOptions.FederationSystemNamespace, svc.Name, i.options.etcdPVCapacity, i.options.dryRun) if err != nil { return err } @@ -250,13 +279,13 @@ func initFederation(cmdOut io.Writer, config util.AdminConfig, cmd *cobra.Comman // Since only one IP address can be specified as advertise address, // we arbitrarily pick the first available IP address // Pick user provided apiserverAdvertiseAddress over other available IP addresses. - advertiseAddress := apiserverAdvertiseAddress + advertiseAddress := i.options.apiServerAdvertiseAddress if advertiseAddress == "" && len(ips) > 0 { advertiseAddress = ips[0] } // 6. Create federation API server - _, err = createAPIServer(hostClientset, initFlags.FederationSystemNamespace, serverName, image, serverCredName, advertiseAddress, storageBackend, apiserverArgOverrides, pvc, dryRun) + _, err = createAPIServer(hostClientset, i.commonOptions.FederationSystemNamespace, serverName, i.options.image, serverCredName, advertiseAddress, i.options.storageBackend, i.options.apiServerOverrides, pvc, i.options.dryRun) if err != nil { return err } @@ -264,20 +293,20 @@ func initFederation(cmdOut io.Writer, config util.AdminConfig, cmd *cobra.Comman // 7. Create federation controller manager // 7a. Create a service account in the host cluster for federation // controller manager. - sa, err := createControllerManagerSA(hostClientset, initFlags.FederationSystemNamespace, dryRun) + sa, err := createControllerManagerSA(hostClientset, i.commonOptions.FederationSystemNamespace, i.options.dryRun) if err != nil { return err } // 7b. Create RBAC role and role binding for federation controller // manager service account. - _, _, err = createRoleBindings(hostClientset, initFlags.FederationSystemNamespace, sa.Name, dryRun) + _, _, err = createRoleBindings(hostClientset, i.commonOptions.FederationSystemNamespace, sa.Name, i.options.dryRun) if err != nil { return err } // 7c. Create federation controller manager deployment. - _, err = createControllerManager(hostClientset, initFlags.FederationSystemNamespace, initFlags.Name, svc.Name, cmName, image, cmKubeconfigName, dnsZoneName, dnsProvider, sa.Name, cmArgOverrides, dryRun) + _, err = createControllerManager(hostClientset, i.commonOptions.FederationSystemNamespace, i.commonOptions.Name, svc.Name, cmName, i.options.image, cmKubeconfigName, i.options.dnsZoneName, i.options.dnsProvider, sa.Name, i.options.controllerManagerOverrides, i.options.dryRun) if err != nil { return err } @@ -291,24 +320,24 @@ func initFederation(cmdOut io.Writer, config util.AdminConfig, cmd *cobra.Comman endpoint = hostnames[0] } // If the service is nodeport, need to append the port to endpoint as it is non-standard port - if apiserverServiceType == v1.ServiceTypeNodePort { + if i.options.apiServerServiceType == v1.ServiceTypeNodePort { endpoint = endpoint + ":" + strconv.Itoa(int(svc.Spec.Ports[0].NodePort)) } // 8. Write the federation API server endpoint info, credentials // and context to kubeconfig - err = updateKubeconfig(config, initFlags.Name, endpoint, entKeyPairs, dryRun) + err = updateKubeconfig(config, i.commonOptions.Name, endpoint, entKeyPairs, i.options.dryRun) if err != nil { return err } - if !dryRun { + if !i.options.dryRun { fedPods := []string{serverName, cmName} - err = waitForPods(hostClientset, fedPods, initFlags.FederationSystemNamespace) + err = waitForPods(hostClientset, fedPods, i.commonOptions.FederationSystemNamespace) if err != nil { return err } - err = waitSrvHealthy(config, initFlags.Name, initFlags.Kubeconfig) + err = waitSrvHealthy(config, i.commonOptions.Name, i.commonOptions.Kubeconfig) if err != nil { return err } diff --git a/federation/pkg/kubefed/join.go b/federation/pkg/kubefed/join.go index 28f7c605c1..7ada7e7690 100644 --- a/federation/pkg/kubefed/join.go +++ b/federation/pkg/kubefed/join.go @@ -31,6 +31,7 @@ import ( "github.com/golang/glog" "github.com/spf13/cobra" + "github.com/spf13/pflag" ) const ( @@ -57,17 +58,35 @@ var ( kubefed join foo --host-cluster-context=bar`) ) +type joinFederation struct { + commonOptions util.SubcommandOptions + options joinFederationOptions +} + +type joinFederationOptions struct { + clusterContext string + secretName string + dryRun bool +} + +func (o *joinFederationOptions) Bind(flags *pflag.FlagSet) { + flags.StringVar(&o.clusterContext, "cluster-context", "", "Name of the cluster's context in the local kubeconfig. Defaults to cluster name if unspecified.") + flags.StringVar(&o.secretName, "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.") +} + // NewCmdJoin defines the `join` command that joins a cluster to a // federation. func NewCmdJoin(f cmdutil.Factory, cmdOut io.Writer, config util.AdminConfig) *cobra.Command { + opts := &joinFederation{} + cmd := &cobra.Command{ Use: "join CLUSTER_NAME --host-cluster-context=HOST_CONTEXT", Short: "Join a cluster to a federation", Long: join_long, Example: join_example, Run: func(cmd *cobra.Command, args []string) { - err := joinFederation(f, cmdOut, config, cmd, args) - cmdutil.CheckErr(err) + cmdutil.CheckErr(opts.Complete(cmd, args)) + cmdutil.CheckErr(opts.Run(f, cmdOut, config, cmd)) }, } @@ -75,45 +94,51 @@ func NewCmdJoin(f cmdutil.Factory, cmdOut io.Writer, config util.AdminConfig) *c cmdutil.AddValidateFlags(cmd) 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.") + + flags := cmd.Flags() + opts.commonOptions.Bind(flags) + opts.options.Bind(flags) + return cmd } -// joinFederation is the implementation of the `join federation` command. -func joinFederation(f cmdutil.Factory, cmdOut io.Writer, config util.AdminConfig, cmd *cobra.Command, args []string) error { - joinFlags, err := util.GetSubcommandFlags(cmd, args) +// Complete ensures that options are valid and marshals them if necessary. +func (j *joinFederation) Complete(cmd *cobra.Command, args []string) error { + err := j.commonOptions.SetName(cmd, args) if err != nil { return err } - clusterContext := cmdutil.GetFlagString(cmd, "cluster-context") - secretName := cmdutil.GetFlagString(cmd, "secret-name") - dryRun := cmdutil.GetDryRunFlag(cmd) - if clusterContext == "" { - clusterContext = joinFlags.Name + j.options.dryRun = cmdutil.GetDryRunFlag(cmd) + + if j.options.clusterContext == "" { + j.options.clusterContext = j.commonOptions.Name } - if secretName == "" { - secretName = joinFlags.Name + if j.options.secretName == "" { + j.options.secretName = j.commonOptions.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) + 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", j.commonOptions.Name, j.commonOptions.Host, j.commonOptions.FederationSystemNamespace, j.commonOptions.Kubeconfig, j.options.clusterContext, j.options.secretName, j.options.dryRun) + return nil +} + +// Run is the implementation of the `join federation` command. +func (j *joinFederation) Run(f cmdutil.Factory, cmdOut io.Writer, config util.AdminConfig, cmd *cobra.Command) error { po := config.PathOptions() - po.LoadingRules.ExplicitPath = joinFlags.Kubeconfig + po.LoadingRules.ExplicitPath = j.commonOptions.Kubeconfig clientConfig, err := po.GetStartingConfig() if err != nil { return err } - generator, err := clusterGenerator(clientConfig, joinFlags.Name, clusterContext, secretName) + generator, err := clusterGenerator(clientConfig, j.commonOptions.Name, j.options.clusterContext, j.options.secretName) if err != nil { glog.V(2).Infof("Failed creating cluster generator: %v", err) return err } glog.V(2).Infof("Created cluster generator: %#v", generator) - hostFactory := config.HostFactory(joinFlags.Host, joinFlags.Kubeconfig) + hostFactory := config.HostFactory(j.commonOptions.Host, j.commonOptions.Kubeconfig) // We are not using the `kubectl create secret` machinery through // `RunCreateSubcommand` as we do to the cluster resource below @@ -130,7 +155,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, clusterContext, secretName, dryRun) + _, err = createSecret(hostFactory, 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 @@ -138,9 +163,9 @@ func joinFederation(f cmdutil.Factory, cmdOut io.Writer, config util.AdminConfig glog.V(2).Infof("Cluster credentials secret created") return kubectlcmd.RunCreateSubcommand(f, cmd, cmdOut, &kubectlcmd.CreateSubcommandOptions{ - Name: joinFlags.Name, + Name: j.commonOptions.Name, StructuredGenerator: generator, - DryRun: dryRun, + DryRun: j.options.dryRun, OutputFormat: cmdutil.GetFlagString(cmd, "output"), }) } diff --git a/federation/pkg/kubefed/unjoin.go b/federation/pkg/kubefed/unjoin.go index 4a9be8d0de..002324aa60 100644 --- a/federation/pkg/kubefed/unjoin.go +++ b/federation/pkg/kubefed/unjoin.go @@ -47,50 +47,53 @@ var ( kubectl unjoin foo --host-cluster-context=bar`) ) +type unjoinFederation struct { + commonOptions util.SubcommandOptions +} + // NewCmdUnjoin defines the `unjoin` command that removes a cluster // from a federation. func NewCmdUnjoin(f cmdutil.Factory, cmdOut, cmdErr io.Writer, config util.AdminConfig) *cobra.Command { + opts := &unjoinFederation{} + cmd := &cobra.Command{ Use: "unjoin CLUSTER_NAME --host-cluster-context=HOST_CONTEXT", Short: "Unjoins a cluster from a federation", Long: unjoin_long, Example: unjoin_example, Run: func(cmd *cobra.Command, args []string) { - err := unjoinFederation(f, cmdOut, cmdErr, config, cmd, args) - cmdutil.CheckErr(err) + cmdutil.CheckErr(opts.commonOptions.SetName(cmd, args)) + cmdutil.CheckErr(opts.Run(f, cmdOut, cmdErr, config)) }, } - util.AddSubcommandFlags(cmd) + flags := cmd.Flags() + opts.commonOptions.Bind(flags) + return cmd } // unjoinFederation is the implementation of the `unjoin` command. -func unjoinFederation(f cmdutil.Factory, cmdOut, cmdErr io.Writer, config util.AdminConfig, cmd *cobra.Command, args []string) error { - unjoinFlags, err := util.GetSubcommandFlags(cmd, args) - if err != nil { - return err - } - - cluster, err := popCluster(f, unjoinFlags.Name) +func (u *unjoinFederation) Run(f cmdutil.Factory, cmdOut, cmdErr io.Writer, config util.AdminConfig) error { + cluster, err := popCluster(f, u.commonOptions.Name) if err != nil { return err } if cluster == nil { - fmt.Fprintf(cmdErr, "WARNING: cluster %q not found in federation, so its credentials' secret couldn't be deleted", unjoinFlags.Name) + fmt.Fprintf(cmdErr, "WARNING: cluster %q not found in federation, so its credentials' secret couldn't be deleted", u.commonOptions.Name) return nil } // We want a separate client factory to communicate with the // federation host cluster. See join_federation.go for details. - hostFactory := config.HostFactory(unjoinFlags.Host, unjoinFlags.Kubeconfig) - err = deleteSecret(hostFactory, cluster.Spec.SecretRef.Name, unjoinFlags.FederationSystemNamespace) + hostFactory := config.HostFactory(u.commonOptions.Host, u.commonOptions.Kubeconfig) + err = deleteSecret(hostFactory, cluster.Spec.SecretRef.Name, u.commonOptions.FederationSystemNamespace) if isNotFound(err) { 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 } - _, err = fmt.Fprintf(cmdOut, "Successfully removed cluster %q from federation\n", unjoinFlags.Name) + _, err = fmt.Fprintf(cmdOut, "Successfully removed cluster %q from federation\n", u.commonOptions.Name) return err } diff --git a/federation/pkg/kubefed/util/BUILD b/federation/pkg/kubefed/util/BUILD index 0e1778233a..ecfb8eca7e 100644 --- a/federation/pkg/kubefed/util/BUILD +++ b/federation/pkg/kubefed/util/BUILD @@ -18,6 +18,7 @@ go_library( "//pkg/kubectl/cmd:go_default_library", "//pkg/kubectl/cmd/util:go_default_library", "//vendor:github.com/spf13/cobra", + "//vendor:github.com/spf13/pflag", "//vendor:k8s.io/apimachinery/pkg/apis/meta/v1", "//vendor:k8s.io/client-go/tools/clientcmd", "//vendor:k8s.io/client-go/tools/clientcmd/api", diff --git a/federation/pkg/kubefed/util/util.go b/federation/pkg/kubefed/util/util.go index cf0ea426f8..6a92ce7f80 100644 --- a/federation/pkg/kubefed/util/util.go +++ b/federation/pkg/kubefed/util/util.go @@ -17,6 +17,8 @@ limitations under the License. package util import ( + "github.com/spf13/pflag" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/client-go/tools/clientcmd" clientcmdapi "k8s.io/client-go/tools/clientcmd/api" @@ -95,36 +97,28 @@ func (a *adminConfig) getClientConfig(context, kubeconfigPath string) clientcmd. return clientcmd.NewNonInteractiveDeferredLoadingClientConfig(&loadingRules, overrides) } -// SubcommandFlags holds the flags required by the subcommands of +// SubcommandOptions holds the configuration required by the subcommands of // `kubefed`. -type SubcommandFlags struct { +type SubcommandOptions struct { Name string Host string FederationSystemNamespace string Kubeconfig string } -// AddSubcommandFlags adds the definition for `kubefed` subcommand -// flags. -func AddSubcommandFlags(cmd *cobra.Command) { - cmd.Flags().String("kubeconfig", "", "Path to the kubeconfig file to use for CLI requests.") - cmd.Flags().String("host-cluster-context", "", "Host cluster context") - cmd.Flags().String("federation-system-namespace", DefaultFederationSystemNamespace, "Namespace in the host cluster where the federation system components are installed") +func (o *SubcommandOptions) Bind(flags *pflag.FlagSet) { + flags.StringVar(&o.Kubeconfig, "kubeconfig", "", "Path to the kubeconfig file to use for CLI requests.") + flags.StringVar(&o.Host, "host-cluster-context", "", "Host cluster context") + flags.StringVar(&o.FederationSystemNamespace, "federation-system-namespace", DefaultFederationSystemNamespace, "Namespace in the host cluster where the federation system components are installed") } -// GetSubcommandFlags retrieves the command line flag values for the -// `kubefed` subcommands. -func GetSubcommandFlags(cmd *cobra.Command, args []string) (*SubcommandFlags, error) { +func (o *SubcommandOptions) SetName(cmd *cobra.Command, args []string) error { name, err := kubectlcmd.NameFromCommandArgs(cmd, args) if err != nil { - return nil, err + return err } - return &SubcommandFlags{ - Name: name, - Host: cmdutil.GetFlagString(cmd, "host-cluster-context"), - FederationSystemNamespace: cmdutil.GetFlagString(cmd, "federation-system-namespace"), - Kubeconfig: cmdutil.GetFlagString(cmd, "kubeconfig"), - }, nil + o.Name = name + return nil } func CreateKubeconfigSecret(clientset *client.Clientset, kubeconfig *clientcmdapi.Config, namespace, name string, dryRun bool) (*api.Secret, error) {