-
Notifications
You must be signed in to change notification settings - Fork 38.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
make kubectl config set-cluster easier to use #3768
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -25,26 +25,29 @@ import ( | |
|
||
"github.com/GoogleCloudPlatform/kubernetes/pkg/client/clientcmd" | ||
clientcmdapi "github.com/GoogleCloudPlatform/kubernetes/pkg/client/clientcmd/api" | ||
"github.com/GoogleCloudPlatform/kubernetes/pkg/util" | ||
) | ||
|
||
type createClusterOptions struct { | ||
pathOptions *pathOptions | ||
name string | ||
server string | ||
apiVersion string | ||
insecureSkipTLSVerify bool | ||
certificateAuthority string | ||
server util.StringFlag | ||
apiVersion util.StringFlag | ||
insecureSkipTLSVerify util.BoolFlag | ||
certificateAuthority util.StringFlag | ||
} | ||
|
||
func NewCmdConfigSetCluster(out io.Writer, pathOptions *pathOptions) *cobra.Command { | ||
options := &createClusterOptions{pathOptions: pathOptions} | ||
|
||
cmd := &cobra.Command{ | ||
Use: "set-cluster name [server] [insecure-skip-tls-verify] [certificate-authority] [api-version]", | ||
Use: fmt.Sprintf("set-cluster name [--%v=server] [--%v=path/to/certficate/authority] [--%v=apiversion] [--%v=true]", clientcmd.FlagAPIServer, clientcmd.FlagCAFile, clientcmd.FlagAPIVersion, clientcmd.FlagInsecure), | ||
Short: "Sets a cluster entry in .kubeconfig", | ||
Long: `Sets a cluster entry in .kubeconfig | ||
|
||
Specifying a name that already exists overwrites that cluster entry. | ||
Specifying a name that already exists will merge new fields on top of existing values for those fields. | ||
e.g. | ||
kubectl config set-cluster e2e --certificate-authority=~/.kube/e2e/.kubernetes.ca.cert | ||
only sets the certificate-authority field on the e2e cluster entry without touching other values. | ||
`, | ||
Run: func(cmd *cobra.Command, args []string) { | ||
if !options.complete(cmd) { | ||
|
@@ -58,10 +61,12 @@ func NewCmdConfigSetCluster(out io.Writer, pathOptions *pathOptions) *cobra.Comm | |
}, | ||
} | ||
|
||
cmd.Flags().StringVar(&options.server, clientcmd.FlagAPIServer, "", clientcmd.FlagAPIServer+" for the cluster entry in .kubeconfig") | ||
cmd.Flags().StringVar(&options.apiVersion, clientcmd.FlagAPIVersion, "", clientcmd.FlagAPIVersion+" for the cluster entry in .kubeconfig") | ||
cmd.Flags().BoolVar(&options.insecureSkipTLSVerify, clientcmd.FlagInsecure, false, clientcmd.FlagInsecure+" for the cluster entry in .kubeconfig") | ||
cmd.Flags().StringVar(&options.certificateAuthority, clientcmd.FlagCAFile, "", clientcmd.FlagCAFile+" for the cluster entry in .kubeconfig") | ||
options.insecureSkipTLSVerify.Default(false) | ||
|
||
cmd.Flags().Var(&options.server, clientcmd.FlagAPIServer, clientcmd.FlagAPIServer+" for the cluster entry in .kubeconfig") | ||
cmd.Flags().Var(&options.apiVersion, clientcmd.FlagAPIVersion, clientcmd.FlagAPIVersion+" for the cluster entry in .kubeconfig") | ||
cmd.Flags().Var(&options.insecureSkipTLSVerify, clientcmd.FlagInsecure, clientcmd.FlagInsecure+" for the cluster entry in .kubeconfig") | ||
cmd.Flags().Var(&options.certificateAuthority, clientcmd.FlagCAFile, clientcmd.FlagCAFile+" for the cluster entry in .kubeconfig") | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Are you sure this does what you want it to? I believe Cobra will call Set() on the variable with the default value if none is provided. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Yes. We use this technique in openshift: https://github.com/openshift/origin/blob/master/pkg/cmd/flagtypes/addr.go#L111. pflag.Var looks clean: https://github.com/spf13/pflag/blob/master/flag.go#L355 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ok then, this approach with .Provided() lgtm There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Great. I'll get on updating the other sets. |
||
|
||
return cmd | ||
} | ||
|
@@ -81,7 +86,7 @@ func (o createClusterOptions) run() error { | |
config.Clusters = make(map[string]clientcmdapi.Cluster) | ||
} | ||
|
||
cluster := o.cluster() | ||
cluster := o.modifyCluster(config.Clusters[o.name]) | ||
config.Clusters[o.name] = cluster | ||
|
||
err = clientcmd.WriteToFile(*config, filename) | ||
|
@@ -93,15 +98,23 @@ func (o createClusterOptions) run() error { | |
} | ||
|
||
// cluster builds a Cluster object from the options | ||
func (o *createClusterOptions) cluster() clientcmdapi.Cluster { | ||
cluster := clientcmdapi.Cluster{ | ||
Server: o.server, | ||
APIVersion: o.apiVersion, | ||
InsecureSkipTLSVerify: o.insecureSkipTLSVerify, | ||
CertificateAuthority: o.certificateAuthority, | ||
func (o *createClusterOptions) modifyCluster(existingCluster clientcmdapi.Cluster) clientcmdapi.Cluster { | ||
modifiedCluster := existingCluster | ||
|
||
if o.server.Provided() { | ||
modifiedCluster.Server = o.server.Value() | ||
} | ||
if o.apiVersion.Provided() { | ||
modifiedCluster.APIVersion = o.apiVersion.Value() | ||
} | ||
if o.insecureSkipTLSVerify.Provided() { | ||
modifiedCluster.InsecureSkipTLSVerify = o.insecureSkipTLSVerify.Value() | ||
} | ||
if o.certificateAuthority.Provided() { | ||
modifiedCluster.CertificateAuthority = o.certificateAuthority.Value() | ||
} | ||
|
||
return cluster | ||
return modifiedCluster | ||
} | ||
|
||
func (o *createClusterOptions) complete(cmd *cobra.Command) bool { | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Line 449 is the one that needs to be changed. Usage string should list server|insecure-skip-tls-verify|certificate-authority|api-version as flags.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, two branches and I missed that part of the change.
How about the new flag types for .Provided() and merging the values?