Refactor cluster and clusters flags (#327)

Also remove unused flag in legacy kubeconfig flags

Signed-off-by: Jian Qiu <[email protected]>

cmd/clusteradm/clusteradm.go

@@ -18,6 +18,7 @@ import (
 	"k8s.io/kubectl/pkg/cmd/plugin"
 	cmdutil "k8s.io/kubectl/pkg/cmd/util"
 	ktemplates "k8s.io/kubectl/pkg/util/templates"
+	utilpointer "k8s.io/utils/pointer"
 
 	// Import all Kubernetes client auth plugins (e.g. Azure, GCP, OIDC, etc.)
 	// to ensure that exec-entrypoint and run can make use of them.
@@ -59,7 +60,11 @@ func main() {
 	flags.SetNormalizeFunc(cliflag.WarnWordSepNormalizeFunc) // Warn for "_" flags
 	flags.SetNormalizeFunc(cliflag.WordSepNormalizeFunc)
 
-	kubeConfigFlags := genericclioptions.NewConfigFlags(true).WithDeprecatedPasswordFlag()
+	//kubeConfigFlags := genericclioptions.NewConfigFlags(true).WithDeprecatedPasswordFlag()
+	kubeConfigFlags := &genericclioptions.ConfigFlags{
+		KubeConfig: utilpointer.String(""),
+		Context:    utilpointer.String(""),
+	}
 	kubeConfigFlags.AddFlags(flags)
 	matchVersionKubeConfigFlags := cmdutil.NewMatchVersionFlags(kubeConfigFlags)
 	matchVersionKubeConfigFlags.AddFlags(flags)

pkg/cmd/accept/cmd.go

@@ -47,7 +47,7 @@ func NewCmd(clusteradmFlags *genericclioptionsclusteradm.ClusteradmFlags, stream
 		},
 	}
 
-	cmd.Flags().StringVar(&o.Clusters, "clusters", "", "Names of the cluster to accept (comma separated)")
+	o.ClusterOptions.AddFlags(cmd.Flags())
 	cmd.Flags().BoolVar(&o.Wait, "wait", false, "If set, wait for the managedcluster and CSR in foreground.")
 	cmd.Flags().BoolVar(&o.SkipApproveCheck, "skip-approve-check", false, "If set, then skip check and approve csr directly.")
 	return cmd

pkg/cmd/accept/exec.go

@@ -31,28 +31,16 @@ const (
 )
 
 func (o *Options) complete(cmd *cobra.Command, args []string) (err error) {
-	klog.V(1).InfoS("accept options:", "dry-run", o.ClusteradmFlags.DryRun, "clusters", o.Clusters, "wait", o.Wait)
-	alreadyProvidedCluster := make(map[string]bool)
-	clusters := make([]string, 0)
-	if o.Clusters != "" {
-		cs := strings.Split(o.Clusters, ",")
-		for _, c := range cs {
-			if _, ok := alreadyProvidedCluster[c]; !ok {
-				alreadyProvidedCluster[c] = true
-				clusters = append(clusters, strings.TrimSpace(c))
-			}
-		}
-		o.Values.Clusters = clusters
-	} else {
-		return fmt.Errorf("values or name are missing")
-	}
-	klog.V(3).InfoS("values:", "clusters", o.Values.Clusters)
+	o.Values.Clusters = o.ClusterOptions.AllClusters().List()
+	klog.V(1).InfoS("accept options:", "dry-run", o.ClusteradmFlags.DryRun, "clusters", o.Values.Clusters, "wait", o.Wait)
 	return nil
 }
 
 func (o *Options) Validate() error {
-	err := o.ClusteradmFlags.ValidateHub()
-	if err != nil {
+	if err := o.ClusteradmFlags.ValidateHub(); err != nil {
+		return err
+	}
+	if err := o.ClusterOptions.Validate(); err != nil {
 		return err
 	}
 

pkg/cmd/accept/options.go

@@ -10,7 +10,7 @@ type Options struct {
 	//ClusteradmFlags: The generic options from the clusteradm cli-runtime.
 	ClusteradmFlags *genericclioptionsclusteradm.ClusteradmFlags
 	//A list of comma separated cluster names
-	Clusters string
+	ClusterOptions *genericclioptionsclusteradm.ClusterOption
 	//Wait to wait for managedcluster and CSR
 	Wait bool
 	//If true the csr will approve directly and check of requester will skip.
@@ -29,6 +29,7 @@ type Values struct {
 func NewOptions(clusteradmFlags *genericclioptionsclusteradm.ClusteradmFlags, streams genericclioptions.IOStreams) *Options {
 	return &Options{
 		ClusteradmFlags: clusteradmFlags,
+		ClusterOptions:  genericclioptionsclusteradm.NewClusterOption(),
 		Streams:         streams,
 	}
 }

pkg/cmd/addon/disable/cmd.go

@@ -66,9 +66,8 @@ func NewCmd(clusteradmFlags *genericclioptionsclusteradm.ClusteradmFlags, stream
 		},
 	}
 
+	o.ClusterOptions.AddFlags(cmd.Flags())
 	cmd.Flags().StringSliceVar(&o.Names, "names", []string{}, "Names of the add-on to deploy (comma separated)")
-	cmd.Flags().StringSliceVar(&o.Clusters, "clusters", []string{}, "Names of the managed cluster to deploy the add-on to (comma separated)")
-	cmd.Flags().BoolVar(&o.Allclusters, "all-clusters", false, "Make all managed clusters to disable the add-on")
 
 	return cmd
 }

pkg/cmd/addon/disable/exec.go

@@ -18,28 +18,24 @@ import (
 	"open-cluster-management.io/clusteradm/pkg/helpers"
 )
 
-func (o *Options) complete(cmd *cobra.Command, args []string) (err error) {
-	klog.V(1).InfoS("disable options:", "dry-run", o.ClusteradmFlags.DryRun, "names", o.Names, "clusters", o.Clusters, "all-clusters", o.Allclusters)
+func (o *Options) complete(cmd *cobra.Command, args []string) error {
+	klog.V(1).InfoS("disable options:", "dry-run", o.ClusteradmFlags.DryRun, "names", o.Names, "clusters", o.ClusterOptions.AllClusters())
 
 	return nil
 }
 
-func (o *Options) Validate() (err error) {
-	err = o.ClusteradmFlags.ValidateHub()
-	if err != nil {
-		return err
-	}
+func (o *Options) Validate() error {
 
-	if len(o.Names) == 0 {
-		return fmt.Errorf("names is missing")
+	if err := o.ClusteradmFlags.ValidateHub(); err != nil {
+		return err
 	}
 
-	if !o.Allclusters && len(o.Clusters) == 0 {
-		return fmt.Errorf("clusters is missing")
+	if err := o.ClusterOptions.Validate(); err != nil {
+		return err
 	}
 
-	if o.Allclusters && len(o.Clusters) != 0 {
-		return fmt.Errorf("flag --all-clusters and --clusters can not be set together")
+	if len(o.Names) == 0 {
+		return fmt.Errorf("names is missing")
 	}
 
 	return nil
@@ -68,7 +64,7 @@ func (o *Options) Run() (err error) {
 	addons := sets.NewString(o.Names...)
 
 	var clusters sets.String
-	if o.Allclusters {
+	if o.ClusterOptions.AllClusters().Len() == 0 {
 		clusters = sets.NewString()
 		mcllist, err := clusterClient.ClusterV1().ManagedClusters().List(context.TODO(),
 			metav1.ListOptions{})
@@ -79,7 +75,7 @@ func (o *Options) Run() (err error) {
 			clusters.Insert(item.ObjectMeta.Name)
 		}
 	} else {
-		clusters = sets.NewString(o.Clusters...)
+		clusters = o.ClusterOptions.AllClusters()
 	}
 
 	klog.V(3).InfoS("addon to be disabled with cluster values:", "addon", addons.List(), "clusters", clusters.List())

pkg/cmd/addon/disable/options.go

@@ -9,14 +9,11 @@ import (
 type Options struct {
 	//ClusteradmFlags: The generic options from the clusteradm cli-runtime.
 	ClusteradmFlags *genericclioptionsclusteradm.ClusteradmFlags
+	ClusterOptions  *genericclioptionsclusteradm.ClusterOption
 	//A list of comma separated addon names
 	Names []string
 	//The specified namespace for addon to disable
 	Namespace string
-	//A list of comma separated cluster names
-	Clusters []string
-	//A bool value shows whether specified add-on will be disable in all managed clusters.
-	Allclusters bool
 
 	Streams genericclioptions.IOStreams
 }
@@ -25,5 +22,6 @@ func NewOptions(clusteradmFlags *genericclioptionsclusteradm.ClusteradmFlags, st
 	return &Options{
 		ClusteradmFlags: clusteradmFlags,
 		Streams:         streams,
+		ClusterOptions:  genericclioptionsclusteradm.NewClusterOption().AllowUnset(),
 	}
 }

pkg/cmd/addon/enable/cmd.go

@@ -64,9 +64,9 @@ func NewCmd(clusteradmFlags *genericclioptionsclusteradm.ClusteradmFlags, stream
 		},
 	}
 
+	o.ClusterOptions.AddFlags(cmd.Flags())
 	cmd.Flags().StringSliceVar(&o.Names, "names", []string{}, "Names of the add-on to deploy (comma separated)")
 	cmd.Flags().StringVarP(&o.Namespace, "namespace", "n", "open-cluster-management-agent-addon", "Specified namespace to addon addon")
-	cmd.Flags().StringSliceVar(&o.Clusters, "clusters", []string{}, "Names of the managed cluster to deploy the add-on to (comma separated)")
 	cmd.Flags().StringVar(&o.OutputFile, "output-file", "", "The generated resources will be copied in the specified file")
 	cmd.Flags().StringSliceVar(&o.Annotate, "annotate", []string{}, "Annotations to add to the ManagedClusterAddon (eg. key1=value1,key2=value2)")
 

pkg/cmd/addon/enable/exec.go

@@ -46,7 +46,7 @@ func NewClusterAddonInfo(cn string, o *Options, an string) (ClusterAddonInfo, er
 }
 
 func (o *Options) complete(cmd *cobra.Command, args []string) (err error) {
-	klog.V(1).InfoS("enable options:", "dry-run", o.ClusteradmFlags.DryRun, "names", o.Names, "clusters", o.Clusters, "output-file", o.OutputFile)
+	klog.V(1).InfoS("enable options:", "dry-run", o.ClusteradmFlags.DryRun, "names", o.Names, "clusters", o.ClusterOptions.AllClusters().List(), "output-file", o.OutputFile)
 
 	return nil
 }
@@ -61,16 +61,16 @@ func (o *Options) Validate() (err error) {
 		return fmt.Errorf("names is missing")
 	}
 
-	if len(o.Clusters) == 0 {
-		return fmt.Errorf("clusters is missing")
+	if err := o.ClusterOptions.Validate(); err != nil {
+		return err
 	}
 
 	return nil
 }
 
 func (o *Options) Run() error {
 	addons := sets.NewString(o.Names...)
-	clusters := sets.NewString(o.Clusters...)
+	clusters := o.ClusterOptions.AllClusters()
 
 	klog.V(3).InfoS("values:", "addon", addons, "clusters", clusters)
 

pkg/cmd/addon/enable/options.go

@@ -9,12 +9,12 @@ import (
 type Options struct {
 	//ClusteradmFlags: The generic options from the clusteradm cli-runtime.
 	ClusteradmFlags *genericclioptionsclusteradm.ClusteradmFlags
+	// ClusterOptions is the option for setting clusters
+	ClusterOptions *genericclioptionsclusteradm.ClusterOption
 	//A list of comma separated addon names
 	Names []string
 	//The specified namespace for addon to install
 	Namespace string
-	//A list of comma separated cluster names
-	Clusters []string
 	//The file to output the resources will be sent to the file.
 	OutputFile string
 	//Annotations to add to the addon
@@ -27,5 +27,6 @@ func NewOptions(clusteradmFlags *genericclioptionsclusteradm.ClusteradmFlags, st
 	return &Options{
 		ClusteradmFlags: clusteradmFlags,
 		Streams:         streams,
+		ClusterOptions:  genericclioptionsclusteradm.NewClusterOption(),
 	}
 }

pkg/cmd/create/work/cmd.go

@@ -59,7 +59,7 @@ func NewCmd(clusteradmFlags *genericclioptionsclusteradm.ClusteradmFlags, stream
 		},
 	}
 
-	cmd.Flags().StringVar(&o.Cluster, "clusters", "", "Names of the managed cluster to apply work")
+	o.ClusterOption.AddFlags(cmd.Flags())
 	cmd.Flags().StringVar(&o.Placement, "placement", "", "Specify an existing placement with format <namespace>/<name>")
 	cmd.Flags().BoolVar(&o.Overwrite, "overwrite", false, "Overwrite the existing work if it exists already")
 	o.FileNameFlags.AddFlags(cmd.Flags())

pkg/cmd/create/work/exec.go

@@ -33,16 +33,20 @@ func (o *Options) complete(cmd *cobra.Command, args []string) (err error) {
 	return nil
 }
 
-func (o *Options) validate() (err error) {
-	err = o.ClusteradmFlags.ValidateHub()
-	if err != nil {
+func (o *Options) validate() error {
+
+	if err := o.ClusteradmFlags.ValidateHub(); err != nil {
+		return err
+	}
+	if err := o.ClusterOption.Validate(); err != nil {
 		return err
 	}
 
-	if len(o.Cluster) == 0 && len(o.Placement) == 0 {
+	clusters := o.ClusterOption.AllClusters()
+	if clusters.Len() == 0 && len(o.Placement) == 0 {
 		return fmt.Errorf("--clusters or --placement must be specified")
 	}
-	if len(o.Cluster) > 0 && len(o.Placement) > 0 {
+	if clusters.Len() > 0 && len(o.Placement) > 0 {
 		return fmt.Errorf("--clusters and --placement can only specify one")
 	}
 	if len(o.Placement) > 0 && len(strings.Split(o.Placement, "/")) != 2 {
@@ -150,8 +154,9 @@ func (o *Options) getClusters(workClient workclientset.Interface, clusterClient
 	}
 
 	// if define --clusters, return that as addedClusters and no deletedClusters
-	if len(o.Cluster) > 0 {
-		return sets.NewString().Insert(o.Cluster), nil, nil
+	clusters := o.ClusterOption.AllClusters()
+	if clusters.Len() > 0 {
+		return clusters, nil, nil
 	}
 
 	placement, err := o.getPlacement(clusterClient)

pkg/cmd/create/work/options.go

@@ -10,9 +10,9 @@ type Options struct {
 	//ClusteradmFlags: The generic options from the clusteradm cli-runtime.
 	ClusteradmFlags *genericclioptionsclusteradm.ClusteradmFlags
 
-	Streams genericclioptions.IOStreams
+	ClusterOption *genericclioptionsclusteradm.ClusterOption
 
-	Cluster string
+	Streams genericclioptions.IOStreams
 
 	Placement string
 
@@ -27,7 +27,7 @@ func newOptions(clusteradmFlags *genericclioptionsclusteradm.ClusteradmFlags, st
 	return &Options{
 		ClusteradmFlags: clusteradmFlags,
 		Streams:         streams,
-		Cluster:         "",
+		ClusterOption:   genericclioptionsclusteradm.NewClusterOption().AllowUnset(),
 		FileNameFlags: genericclioptions.FileNameFlags{
 			Filenames: &[]string{},
 			Recursive: boolPtr(true),

pkg/cmd/delete/work/cmd.go

@@ -43,7 +43,7 @@ func NewCmd(clusteradmFlags *genericclioptionsclusteradm.ClusteradmFlags, stream
 		},
 	}
 
-	cmd.Flags().StringVar(&o.Cluster, "cluster", "", "Name of the managed cluster applied work")
+	o.ClusterOptions.AddFlags(cmd.Flags())
 	cmd.Flags().BoolVar(&o.Force, "force", false, "set force flag to enable force delete")
 
 	return cmd