Merge pull request #41216 from marun/kubefed-auto-bind-flags

Automatic merge from submit-queue

[Federation][kubefed]: Bind flag values automatically

Update ``kubefed`` to take advantage of cobra's ability to declare variable bindings at the same time as declaring flags, so that flag values are bound automatically.

Targets #41209 

cc: @kubernetes/sig-federation-pr-reviews @irfanurrehman @madhusudancs

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",

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",

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(opts.Complete(cmd, args))
-			cmdutil.CheckErr(err)
+			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.
+// Complete ensures that options are valid and marshals them if necessary.
-// See the design doc in https://github.com/kubernetes/kubernetes/pull/34484
+func (i *initFederation) Complete(cmd *cobra.Command, args []string) error {
-// for details.
+	err := i.commonOptions.SetName(cmd, args)
-func initFederation(cmdOut io.Writer, config util.AdminConfig, cmd *cobra.Command, args []string) error {
-	initFlags, err := util.GetSubcommandFlags(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 {
+	i.options.apiServerServiceType = v1.ServiceType(i.options.apiServerServiceTypeString)
-		return fmt.Errorf("invalid %s: %s, should be either %s or %s", apiserverServiceTypeFlag, apiserverServiceType, v1.ServiceTypeLoadBalancer, v1.ServiceTypeNodePort)
+	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 != "" {
+	if i.options.apiServerAdvertiseAddress != "" {
-		ip := net.ParseIP(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 {
+	if i.options.etcdPersistentStorage {
-		pvc, err = createPVC(hostClientset, initFlags.FederationSystemNamespace, svc.Name, etcdPVCapacity, dryRun)
+		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
 		}

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(opts.Complete(cmd, args))
-			cmdutil.CheckErr(err)
+			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.")
+	flags := cmd.Flags()
-	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.")
+	opts.commonOptions.Bind(flags)
+	opts.options.Bind(flags)
+
 	return cmd
 }
 
-// joinFederation is the implementation of the `join federation` command.
+// Complete ensures that options are valid and marshals them if necessary.
-func joinFederation(f cmdutil.Factory, cmdOut io.Writer, config util.AdminConfig, cmd *cobra.Command, args []string) error {
+func (j *joinFederation) Complete(cmd *cobra.Command, args []string) error {
-	joinFlags, err := util.GetSubcommandFlags(cmd, args)
+	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 == "" {
+	j.options.dryRun = cmdutil.GetDryRunFlag(cmd)
-		clusterContext = joinFlags.Name
+
+	if j.options.clusterContext == "" {
+		j.options.clusterContext = j.commonOptions.Name
 	}
-	if secretName == "" {
+	if j.options.secretName == "" {
-		secretName = joinFlags.Name
+		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"),
 	})
 }

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(opts.commonOptions.SetName(cmd, args))
-			cmdutil.CheckErr(err)
+			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 {
+func (u *unjoinFederation) Run(f cmdutil.Factory, cmdOut, cmdErr io.Writer, config util.AdminConfig) error {
-	unjoinFlags, err := util.GetSubcommandFlags(cmd, args)
+	cluster, err := popCluster(f, u.commonOptions.Name)
-	if err != nil {
-		return err
-	}
-
-	cluster, err := popCluster(f, unjoinFlags.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)
+	hostFactory := config.HostFactory(u.commonOptions.Host, u.commonOptions.Kubeconfig)
-	err = deleteSecret(hostFactory, cluster.Spec.SecretRef.Name, unjoinFlags.FederationSystemNamespace)
+	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
 }
 

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",

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
+func (o *SubcommandOptions) Bind(flags *pflag.FlagSet) {
-// flags.
+	flags.StringVar(&o.Kubeconfig, "kubeconfig", "", "Path to the kubeconfig file to use for CLI requests.")
-func AddSubcommandFlags(cmd *cobra.Command) {
+	flags.StringVar(&o.Host, "host-cluster-context", "", "Host cluster context")
-	cmd.Flags().String("kubeconfig", "", "Path to the kubeconfig file to use for CLI requests.")
+	flags.StringVar(&o.FederationSystemNamespace, "federation-system-namespace", DefaultFederationSystemNamespace, "Namespace in the host cluster where the federation system components are installed")
-	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")
 }
 
-// GetSubcommandFlags retrieves the command line flag values for the
+func (o *SubcommandOptions) SetName(cmd *cobra.Command, args []string) error {
-// `kubefed` subcommands.
-func GetSubcommandFlags(cmd *cobra.Command, args []string) (*SubcommandFlags, error) {
 	name, err := kubectlcmd.NameFromCommandArgs(cmd, args)
 	if err != nil {
-		return nil, err
+		return err
 	}
-	return &SubcommandFlags{
+	o.Name = name
-		Name: name,
+	return nil
-		Host: cmdutil.GetFlagString(cmd, "host-cluster-context"),
-		FederationSystemNamespace: cmdutil.GetFlagString(cmd, "federation-system-namespace"),
-		Kubeconfig:                cmdutil.GetFlagString(cmd, "kubeconfig"),
-	}, nil
 }
 
 func CreateKubeconfigSecret(clientset *client.Clientset, kubeconfig *clientcmdapi.Config, namespace, name string, dryRun bool) (*api.Secret, error) {