From 7cbb52ce0418994e89e4e8380785fd9edabdc444 Mon Sep 17 00:00:00 2001 From: Eric Paris Date: Wed, 5 Aug 2015 10:21:47 -0400 Subject: [PATCH] Use the pflag StringSlice instead of implementing it ourselves Saves code and makes our code easier to read because we just use normal []string instead of custom type. --- cmd/kube-apiserver/app/server.go | 10 ++--- cmd/kubelet/app/server.go | 4 +- .../mesos/pkg/scheduler/service/service.go | 10 ++--- pkg/apiserver/apiserver_test.go | 2 +- pkg/kubectl/bash_comp_utils.go | 31 ++------------ pkg/kubectl/cmd/create.go | 12 +++--- pkg/kubectl/cmd/delete.go | 12 +++--- pkg/kubectl/cmd/delete_test.go | 11 ++--- pkg/kubectl/cmd/get.go | 3 +- pkg/kubectl/cmd/replace.go | 13 +++--- pkg/kubectl/cmd/rollingupdate.go | 16 ++++--- pkg/kubectl/cmd/stop.go | 14 +++---- pkg/kubectl/cmd/util/factory.go | 7 +++- pkg/kubectl/cmd/util/helpers.go | 42 +++++++++---------- pkg/master/master.go | 4 +- pkg/util/list.go | 39 ----------------- pkg/util/list_test.go | 32 -------------- 17 files changed, 82 insertions(+), 180 deletions(-) delete mode 100644 pkg/util/list.go delete mode 100644 pkg/util/list_test.go diff --git a/cmd/kube-apiserver/app/server.go b/cmd/kube-apiserver/app/server.go index 2c1146cd0c8..4e8416b2fe9 100644 --- a/cmd/kube-apiserver/app/server.go +++ b/cmd/kube-apiserver/app/server.go @@ -89,10 +89,10 @@ type APIServer struct { AuthorizationPolicyFile string AdmissionControl string AdmissionControlConfigFile string - EtcdServerList util.StringList + EtcdServerList []string EtcdConfigFile string EtcdPathPrefix string - CorsAllowedOriginList util.StringList + CorsAllowedOriginList []string AllowPrivileged bool ServiceClusterIPRange util.IPNet // TODO: make this a list ServiceNodePortRange util.PortRange @@ -192,10 +192,10 @@ func (s *APIServer) AddFlags(fs *pflag.FlagSet) { fs.StringVar(&s.AuthorizationPolicyFile, "authorization-policy-file", s.AuthorizationPolicyFile, "File with authorization policy in csv format, used with --authorization-mode=ABAC, on the secure port.") fs.StringVar(&s.AdmissionControl, "admission-control", s.AdmissionControl, "Ordered list of plug-ins to do admission control of resources into cluster. Comma-delimited list of: "+strings.Join(admission.GetPlugins(), ", ")) fs.StringVar(&s.AdmissionControlConfigFile, "admission-control-config-file", s.AdmissionControlConfigFile, "File with admission control configuration.") - fs.Var(&s.EtcdServerList, "etcd-servers", "List of etcd servers to watch (http://ip:port), comma separated. Mutually exclusive with -etcd-config") + fs.StringSliceVar(&s.EtcdServerList, "etcd-servers", s.EtcdServerList, "List of etcd servers to watch (http://ip:port), comma separated. Mutually exclusive with -etcd-config") fs.StringVar(&s.EtcdConfigFile, "etcd-config", s.EtcdConfigFile, "The config file for the etcd client. Mutually exclusive with -etcd-servers.") fs.StringVar(&s.EtcdPathPrefix, "etcd-prefix", s.EtcdPathPrefix, "The prefix for all resource paths in etcd.") - fs.Var(&s.CorsAllowedOriginList, "cors-allowed-origins", "List of allowed origins for CORS, comma separated. An allowed origin can be a regular expression to support subdomain matching. If this list is empty CORS will not be enabled.") + fs.StringSliceVar(&s.CorsAllowedOriginList, "cors-allowed-origins", s.CorsAllowedOriginList, "List of allowed origins for CORS, comma separated. An allowed origin can be a regular expression to support subdomain matching. If this list is empty CORS will not be enabled.") fs.BoolVar(&s.AllowPrivileged, "allow-privileged", s.AllowPrivileged, "If true, allow privileged containers.") fs.Var(&s.ServiceClusterIPRange, "service-cluster-ip-range", "A CIDR notation IP range from which to assign service cluster IPs. This must not overlap with any IP ranges assigned to nodes for pods.") fs.Var(&s.ServiceClusterIPRange, "portal-net", "Deprecated: see --service-cluster-ip-range instead.") @@ -224,7 +224,7 @@ func (s *APIServer) verifyClusterIPFlags() { } } -func newEtcd(etcdConfigFile string, etcdServerList util.StringList, interfacesFunc meta.VersionInterfacesFunc, defaultVersion, storageVersion, pathPrefix string) (etcdStorage storage.Interface, err error) { +func newEtcd(etcdConfigFile string, etcdServerList []string, interfacesFunc meta.VersionInterfacesFunc, defaultVersion, storageVersion, pathPrefix string) (etcdStorage storage.Interface, err error) { var client tools.EtcdClient if etcdConfigFile != "" { client, err = etcd.NewClientFromFile(etcdConfigFile) diff --git a/cmd/kubelet/app/server.go b/cmd/kubelet/app/server.go index 5d13bb71340..8ac5d642db1 100644 --- a/cmd/kubelet/app/server.go +++ b/cmd/kubelet/app/server.go @@ -93,7 +93,7 @@ type KubeletServer struct { HealthzPort int HealthzBindAddress util.IP OOMScoreAdj int - APIServerList util.StringList + APIServerList []string RegisterNode bool StandaloneMode bool ClusterDomain string @@ -226,7 +226,7 @@ func (s *KubeletServer) AddFlags(fs *pflag.FlagSet) { fs.IntVar(&s.HealthzPort, "healthz-port", s.HealthzPort, "The port of the localhost healthz endpoint") fs.Var(&s.HealthzBindAddress, "healthz-bind-address", "The IP address for the healthz server to serve on, defaulting to 127.0.0.1 (set to 0.0.0.0 for all interfaces)") fs.IntVar(&s.OOMScoreAdj, "oom-score-adj", s.OOMScoreAdj, "The oom_score_adj value for kubelet process. Values must be within the range [-1000, 1000]") - fs.Var(&s.APIServerList, "api-servers", "List of Kubernetes API servers for publishing events, and reading pods and services. (ip:port), comma separated.") + fs.StringSliceVar(&s.APIServerList, "api-servers", []string{}, "List of Kubernetes API servers for publishing events, and reading pods and services. (ip:port), comma separated.") fs.BoolVar(&s.RegisterNode, "register-node", s.RegisterNode, "Register the node with the apiserver (defaults to true if --api-server is set)") fs.StringVar(&s.ClusterDomain, "cluster-domain", s.ClusterDomain, "Domain for this cluster. If set, kubelet will configure all containers to search this domain in addition to the host's search domains") fs.StringVar(&s.MasterServiceNamespace, "master-service-namespace", s.MasterServiceNamespace, "The namespace from which the kubernetes master services should be injected into pods") diff --git a/contrib/mesos/pkg/scheduler/service/service.go b/contrib/mesos/pkg/scheduler/service/service.go index 56292aeffe0..4be5ce923ce 100644 --- a/contrib/mesos/pkg/scheduler/service/service.go +++ b/contrib/mesos/pkg/scheduler/service/service.go @@ -85,8 +85,8 @@ type SchedulerServer struct { Address util.IP EnableProfiling bool AuthPath string - APIServerList util.StringList - EtcdServerList util.StringList + APIServerList []string + EtcdServerList []string EtcdConfigFile string AllowPrivileged bool ExecutorPath string @@ -198,9 +198,9 @@ func (s *SchedulerServer) addCoreFlags(fs *pflag.FlagSet) { fs.IntVar(&s.Port, "port", s.Port, "The port that the scheduler's http service runs on") fs.Var(&s.Address, "address", "The IP address to serve on (set to 0.0.0.0 for all interfaces)") fs.BoolVar(&s.EnableProfiling, "profiling", s.EnableProfiling, "Enable profiling via web interface host:port/debug/pprof/") - fs.Var(&s.APIServerList, "api-servers", "List of Kubernetes API servers for publishing events, and reading pods and services. (ip:port), comma separated.") + fs.StringSliceVar(&s.APIServerList, "api-servers", s.APIServerList, "List of Kubernetes API servers for publishing events, and reading pods and services. (ip:port), comma separated.") fs.StringVar(&s.AuthPath, "auth-path", s.AuthPath, "Path to .kubernetes_auth file, specifying how to authenticate to API server.") - fs.Var(&s.EtcdServerList, "etcd-servers", "List of etcd servers to watch (http://ip:port), comma separated. Mutually exclusive with --etcd-config") + fs.StringSliceVar(&s.EtcdServerList, "etcd-servers", s.EtcdServerList, "List of etcd servers to watch (http://ip:port), comma separated. Mutually exclusive with --etcd-config") fs.StringVar(&s.EtcdConfigFile, "etcd-config", s.EtcdConfigFile, "The config file for the etcd client. Mutually exclusive with --etcd-servers.") fs.BoolVar(&s.AllowPrivileged, "allow-privileged", s.AllowPrivileged, "If true, allow privileged containers.") fs.StringVar(&s.ClusterDomain, "cluster-domain", s.ClusterDomain, "Domain for this cluster. If set, kubelet will configure all containers to search this domain in addition to the host's search domains") @@ -583,7 +583,7 @@ func validateLeadershipTransition(desired, current string) { } // hacked from https://github.com/GoogleCloudPlatform/kubernetes/blob/release-0.14/cmd/kube-apiserver/app/server.go -func newEtcd(etcdConfigFile string, etcdServerList util.StringList) (client tools.EtcdClient, err error) { +func newEtcd(etcdConfigFile string, etcdServerList []string) (client tools.EtcdClient, err error) { if etcdConfigFile != "" { client, err = etcd.NewClientFromFile(etcdConfigFile) } else { diff --git a/pkg/apiserver/apiserver_test.go b/pkg/apiserver/apiserver_test.go index 9274c7f9732..516406eacec 100644 --- a/pkg/apiserver/apiserver_test.go +++ b/pkg/apiserver/apiserver_test.go @@ -2446,7 +2446,7 @@ func TestCreateTimeout(t *testing.T) { func TestCORSAllowedOrigins(t *testing.T) { table := []struct { - allowedOrigins util.StringList + allowedOrigins []string origin string allowed bool }{ diff --git a/pkg/kubectl/bash_comp_utils.go b/pkg/kubectl/bash_comp_utils.go index 6908fca3481..0ad9afd88e0 100644 --- a/pkg/kubectl/bash_comp_utils.go +++ b/pkg/kubectl/bash_comp_utils.go @@ -20,34 +20,11 @@ package kubectl import ( "github.com/spf13/cobra" - "github.com/spf13/pflag" - "k8s.io/kubernetes/pkg/util" ) -func AddJsonFilenameFlag(cmd *cobra.Command, value *util.StringList, usage string) { +func AddJsonFilenameFlag(cmd *cobra.Command, usage string) { + cmd.Flags().StringSliceP("filename", "f", []string{}, usage) + annotations := []string{"json", "yaml", "yml"} - annotation := make(map[string][]string) - annotation[cobra.BashCompFilenameExt] = annotations - - flag := &pflag.Flag{ - Name: "filename", - Shorthand: "f", - Usage: usage, - Value: value, - DefValue: value.String(), - Annotations: annotation, - } - cmd.Flags().AddFlag(flag) -} - -// AddLabelsToColumnsFlag added a user flag to print resource labels into columns. Currently used in kubectl get command -func AddLabelsToColumnsFlag(cmd *cobra.Command, value *util.StringList, usage string) { - flag := &pflag.Flag{ - Name: "label-columns", - Shorthand: "L", - Usage: usage, - Value: value, - DefValue: value.String(), - } - cmd.Flags().AddFlag(flag) + cmd.Flags().SetAnnotation("filename", cobra.BashCompFilenameExt, annotations) } diff --git a/pkg/kubectl/cmd/create.go b/pkg/kubectl/cmd/create.go index 4eb096bb747..5fa12a877e1 100644 --- a/pkg/kubectl/cmd/create.go +++ b/pkg/kubectl/cmd/create.go @@ -28,7 +28,6 @@ import ( cmdutil "k8s.io/kubernetes/pkg/kubectl/cmd/util" "k8s.io/kubernetes/pkg/kubectl/resource" "k8s.io/kubernetes/pkg/runtime" - "k8s.io/kubernetes/pkg/util" ) const ( @@ -43,7 +42,6 @@ $ cat pod.json | kubectl create -f -` ) func NewCmdCreate(f *cmdutil.Factory, out io.Writer) *cobra.Command { - var filenames util.StringList cmd := &cobra.Command{ Use: "create -f FILENAME", Short: "Create a resource by filename or stdin", @@ -52,13 +50,12 @@ func NewCmdCreate(f *cmdutil.Factory, out io.Writer) *cobra.Command { Run: func(cmd *cobra.Command, args []string) { cmdutil.CheckErr(ValidateArgs(cmd, args)) cmdutil.CheckErr(cmdutil.ValidateOutputArgs(cmd)) - shortOutput := cmdutil.GetFlagString(cmd, "output") == "name" - cmdutil.CheckErr(RunCreate(f, out, filenames, shortOutput)) + cmdutil.CheckErr(RunCreate(f, cmd, out)) }, } usage := "Filename, directory, or URL to file to use to create the resource" - kubectl.AddJsonFilenameFlag(cmd, &filenames, usage) + kubectl.AddJsonFilenameFlag(cmd, usage) cmd.MarkFlagRequired("filename") cmdutil.AddOutputFlagsForMutation(cmd) @@ -72,7 +69,8 @@ func ValidateArgs(cmd *cobra.Command, args []string) error { return nil } -func RunCreate(f *cmdutil.Factory, out io.Writer, filenames util.StringList, shortOutput bool) error { +func RunCreate(f *cmdutil.Factory, cmd *cobra.Command, out io.Writer) error { + schema, err := f.Validator() if err != nil { return err @@ -83,6 +81,7 @@ func RunCreate(f *cmdutil.Factory, out io.Writer, filenames util.StringList, sho return err } + filenames := cmdutil.GetFlagStringSlice(cmd, "filename") mapper, typer := f.Object() r := resource.NewBuilder(mapper, typer, f.ClientMapperForCommand()). Schema(schema). @@ -108,6 +107,7 @@ func RunCreate(f *cmdutil.Factory, out io.Writer, filenames util.StringList, sho } count++ info.Refresh(obj, true) + shortOutput := cmdutil.GetFlagString(cmd, "output") == "name" if !shortOutput { printObjectSpecificMessage(info.Object, out) } diff --git a/pkg/kubectl/cmd/delete.go b/pkg/kubectl/cmd/delete.go index 5bdef1a7bc8..62e2de51179 100644 --- a/pkg/kubectl/cmd/delete.go +++ b/pkg/kubectl/cmd/delete.go @@ -29,7 +29,6 @@ import ( "k8s.io/kubernetes/pkg/kubectl" cmdutil "k8s.io/kubernetes/pkg/kubectl/cmd/util" "k8s.io/kubernetes/pkg/kubectl/resource" - "k8s.io/kubernetes/pkg/util" ) const ( @@ -59,7 +58,6 @@ $ kubectl delete pods --all` ) func NewCmdDelete(f *cmdutil.Factory, out io.Writer) *cobra.Command { - var filenames util.StringList cmd := &cobra.Command{ Use: "delete ([-f FILENAME] | TYPE [(NAME | -l label | --all)])", Short: "Delete resources by filenames, stdin, resources and names, or by resources and label selector.", @@ -67,13 +65,12 @@ func NewCmdDelete(f *cmdutil.Factory, out io.Writer) *cobra.Command { Example: delete_example, Run: func(cmd *cobra.Command, args []string) { cmdutil.CheckErr(cmdutil.ValidateOutputArgs(cmd)) - shortOutput := cmdutil.GetFlagString(cmd, "output") == "name" - err := RunDelete(f, out, cmd, args, filenames, shortOutput) + err := RunDelete(f, out, cmd, args) cmdutil.CheckErr(err) }, } usage := "Filename, directory, or URL to a file containing the resource to delete." - kubectl.AddJsonFilenameFlag(cmd, &filenames, usage) + kubectl.AddJsonFilenameFlag(cmd, usage) cmd.Flags().StringP("selector", "l", "", "Selector (label query) to filter on.") cmd.Flags().Bool("all", false, "[-all] to select all the specified resources.") cmd.Flags().Bool("ignore-not-found", false, "Treat \"resource not found\" as a successful delete. Defaults to \"true\" when --all is specified.") @@ -84,7 +81,7 @@ func NewCmdDelete(f *cmdutil.Factory, out io.Writer) *cobra.Command { return cmd } -func RunDelete(f *cmdutil.Factory, out io.Writer, cmd *cobra.Command, args []string, filenames util.StringList, shortOutput bool) error { +func RunDelete(f *cmdutil.Factory, out io.Writer, cmd *cobra.Command, args []string) error { cmdNamespace, enforceNamespace, err := f.DefaultNamespace() if err != nil { return err @@ -94,7 +91,7 @@ func RunDelete(f *cmdutil.Factory, out io.Writer, cmd *cobra.Command, args []str r := resource.NewBuilder(mapper, typer, f.ClientMapperForCommand()). ContinueOnError(). NamespaceParam(cmdNamespace).DefaultNamespace(). - FilenameParam(enforceNamespace, filenames...). + FilenameParam(enforceNamespace, cmdutil.GetFlagStringSlice(cmd, "filename")...). SelectorParam(cmdutil.GetFlagString(cmd, "selector")). SelectAllParam(deleteAll). ResourceTypeOrNameArgs(false, args...).RequireObject(false). @@ -118,6 +115,7 @@ func RunDelete(f *cmdutil.Factory, out io.Writer, cmd *cobra.Command, args []str } } + shortOutput := cmdutil.GetFlagString(cmd, "output") == "name" // By default use a reaper to delete all related resources. if cmdutil.GetFlagBool(cmd, "cascade") { return ReapResult(r, f, out, cmdutil.GetFlagBool(cmd, "cascade"), ignoreNotFound, cmdutil.GetFlagDuration(cmd, "timeout"), cmdutil.GetFlagInt(cmd, "grace-period"), shortOutput, mapper) diff --git a/pkg/kubectl/cmd/delete_test.go b/pkg/kubectl/cmd/delete_test.go index 60838984bc1..fe4c954c28a 100644 --- a/pkg/kubectl/cmd/delete_test.go +++ b/pkg/kubectl/cmd/delete_test.go @@ -26,7 +26,6 @@ import ( "k8s.io/kubernetes/pkg/api/errors" "k8s.io/kubernetes/pkg/api/testapi" "k8s.io/kubernetes/pkg/client" - "k8s.io/kubernetes/pkg/util" ) func TestDeleteObjectByTuple(t *testing.T) { @@ -147,8 +146,7 @@ func TestDeleteObjectNotFound(t *testing.T) { cmd.Flags().Set("filename", "../../../examples/guestbook/redis-master-controller.yaml") cmd.Flags().Set("cascade", "false") cmd.Flags().Set("output", "name") - filenames := cmd.Flags().Lookup("filename").Value.(*util.StringList) - err := RunDelete(f, buf, cmd, []string{}, *filenames, true) + err := RunDelete(f, buf, cmd, []string{}) if err == nil || !errors.IsNotFound(err) { t.Errorf("unexpected error: expected NotFound, got %v", err) } @@ -218,8 +216,9 @@ func TestDeleteAllNotFound(t *testing.T) { cmd.Flags().Set("cascade", "false") // Make sure we can explicitly choose to fail on NotFound errors, even with --all cmd.Flags().Set("ignore-not-found", "false") + cmd.Flags().Set("output", "name") - err := RunDelete(f, buf, cmd, []string{"services"}, nil, true) + err := RunDelete(f, buf, cmd, []string{"services"}) if err == nil || !errors.IsNotFound(err) { t.Errorf("unexpected error: expected NotFound, got %v", err) } @@ -326,9 +325,7 @@ func TestDeleteMultipleObjectContinueOnMissing(t *testing.T) { cmd.Flags().Set("filename", "../../../examples/guestbook/frontend-service.yaml") cmd.Flags().Set("cascade", "false") cmd.Flags().Set("output", "name") - filenames := cmd.Flags().Lookup("filename").Value.(*util.StringList) - t.Logf("filenames: %v\n", filenames) - err := RunDelete(f, buf, cmd, []string{}, *filenames, true) + err := RunDelete(f, buf, cmd, []string{}) if err == nil || !errors.IsNotFound(err) { t.Errorf("unexpected error: expected NotFound, got %v", err) } diff --git a/pkg/kubectl/cmd/get.go b/pkg/kubectl/cmd/get.go index 54c3bf351ca..d341b9973d0 100644 --- a/pkg/kubectl/cmd/get.go +++ b/pkg/kubectl/cmd/get.go @@ -23,7 +23,6 @@ import ( "k8s.io/kubernetes/pkg/kubectl" cmdutil "k8s.io/kubernetes/pkg/kubectl/cmd/util" "k8s.io/kubernetes/pkg/kubectl/resource" - "k8s.io/kubernetes/pkg/util" "k8s.io/kubernetes/pkg/watch" "github.com/spf13/cobra" @@ -83,7 +82,7 @@ func NewCmdGet(f *cmdutil.Factory, out io.Writer) *cobra.Command { cmd.Flags().BoolP("watch", "w", false, "After listing/getting the requested object, watch for changes.") cmd.Flags().Bool("watch-only", false, "Watch for changes to the requested object(s), without listing/getting first.") cmd.Flags().Bool("all-namespaces", false, "If present, list the requested object(s) across all namespaces. Namespace in current context is ignored even if specified with --namespace.") - kubectl.AddLabelsToColumnsFlag(cmd, &util.StringList{}, "Accepts a comma separated list of labels that are going to be presented as columns. Names are case-sensitive. You can also use multiple flag statements like -L label1 -L label2...") + cmd.Flags().StringSliceP("label-columns", "L", []string{}, "Accepts a comma separated list of labels that are going to be presented as columns. Names are case-sensitive. You can also use multiple flag statements like -L label1 -L label2...") return cmd } diff --git a/pkg/kubectl/cmd/replace.go b/pkg/kubectl/cmd/replace.go index 21ce9fffcc8..e9cf2a1b8ce 100644 --- a/pkg/kubectl/cmd/replace.go +++ b/pkg/kubectl/cmd/replace.go @@ -29,7 +29,6 @@ import ( "k8s.io/kubernetes/pkg/kubectl" cmdutil "k8s.io/kubernetes/pkg/kubectl/cmd/util" "k8s.io/kubernetes/pkg/kubectl/resource" - "k8s.io/kubernetes/pkg/util" ) const ( @@ -49,7 +48,6 @@ kubectl replace --force -f ./pod.json` ) func NewCmdReplace(f *cmdutil.Factory, out io.Writer) *cobra.Command { - var filenames util.StringList cmd := &cobra.Command{ Use: "replace -f FILENAME", // update is deprecated. @@ -59,13 +57,12 @@ func NewCmdReplace(f *cmdutil.Factory, out io.Writer) *cobra.Command { Example: replace_example, Run: func(cmd *cobra.Command, args []string) { cmdutil.CheckErr(cmdutil.ValidateOutputArgs(cmd)) - shortOutput := cmdutil.GetFlagString(cmd, "output") == "name" - err := RunReplace(f, out, cmd, args, filenames, shortOutput) + err := RunReplace(f, out, cmd, args) cmdutil.CheckErr(err) }, } usage := "Filename, directory, or URL to file to use to replace the resource." - kubectl.AddJsonFilenameFlag(cmd, &filenames, usage) + kubectl.AddJsonFilenameFlag(cmd, usage) cmd.MarkFlagRequired("filename") cmd.Flags().Bool("force", false, "Delete and re-create the specified resource") cmd.Flags().Bool("cascade", false, "Only relevant during a force replace. If true, cascade the deletion of the resources managed by this resource (e.g. Pods created by a ReplicationController). Default true.") @@ -75,7 +72,7 @@ func NewCmdReplace(f *cmdutil.Factory, out io.Writer) *cobra.Command { return cmd } -func RunReplace(f *cmdutil.Factory, out io.Writer, cmd *cobra.Command, args []string, filenames util.StringList, shortOutput bool) error { +func RunReplace(f *cmdutil.Factory, out io.Writer, cmd *cobra.Command, args []string) error { if len(os.Args) > 1 && os.Args[1] == "update" { printDeprecationWarning("replace", "update") } @@ -90,10 +87,12 @@ func RunReplace(f *cmdutil.Factory, out io.Writer, cmd *cobra.Command, args []st } force := cmdutil.GetFlagBool(cmd, "force") + filenames := cmdutil.GetFlagStringSlice(cmd, "filename") if len(filenames) == 0 { return cmdutil.UsageError(cmd, "Must specify --filename to replace") } + shortOutput := cmdutil.GetFlagString(cmd, "output") == "name" if force { return forceReplace(f, out, cmd, args, filenames, shortOutput) } @@ -127,7 +126,7 @@ func RunReplace(f *cmdutil.Factory, out io.Writer, cmd *cobra.Command, args []st }) } -func forceReplace(f *cmdutil.Factory, out io.Writer, cmd *cobra.Command, args []string, filenames util.StringList, shortOutput bool) error { +func forceReplace(f *cmdutil.Factory, out io.Writer, cmd *cobra.Command, args []string, filenames []string, shortOutput bool) error { schema, err := f.Validator() if err != nil { return err diff --git a/pkg/kubectl/cmd/rollingupdate.go b/pkg/kubectl/cmd/rollingupdate.go index d81fd2e91df..506ddb180d5 100644 --- a/pkg/kubectl/cmd/rollingupdate.go +++ b/pkg/kubectl/cmd/rollingupdate.go @@ -21,6 +21,7 @@ import ( "fmt" "io" "os" + "time" "github.com/golang/glog" @@ -34,9 +35,6 @@ import ( ) const ( - updatePeriod = "1m0s" - timeout = "5m0s" - pollInterval = "3s" rollingUpdate_long = `Perform a rolling update of the given ReplicationController. Replaces the specified replication controller with a new replication controller by updating one pod at a time to use the @@ -57,6 +55,12 @@ $ kubectl rolling-update frontend --image=image:v2 ` ) +var ( + updatePeriod, _ = time.ParseDuration("1m0s") + timeout, _ = time.ParseDuration("5m0s") + pollInterval, _ = time.ParseDuration("3s") +) + func NewCmdRollingUpdate(f *cmdutil.Factory, out io.Writer) *cobra.Command { cmd := &cobra.Command{ Use: "rolling-update OLD_CONTROLLER_NAME ([NEW_CONTROLLER_NAME] --image=NEW_CONTAINER_IMAGE | -f NEW_CONTROLLER_SPEC)", @@ -70,9 +74,9 @@ func NewCmdRollingUpdate(f *cmdutil.Factory, out io.Writer) *cobra.Command { cmdutil.CheckErr(err) }, } - cmd.Flags().String("update-period", updatePeriod, `Time to wait between updating pods. Valid time units are "ns", "us" (or "µs"), "ms", "s", "m", "h".`) - cmd.Flags().String("poll-interval", pollInterval, `Time delay between polling for replication controller status after the update. Valid time units are "ns", "us" (or "µs"), "ms", "s", "m", "h".`) - cmd.Flags().String("timeout", timeout, `Max time to wait for a replication controller to update before giving up. Valid time units are "ns", "us" (or "µs"), "ms", "s", "m", "h".`) + cmd.Flags().Duration("update-period", updatePeriod, `Time to wait between updating pods. Valid time units are "ns", "us" (or "µs"), "ms", "s", "m", "h".`) + cmd.Flags().Duration("poll-interval", pollInterval, `Time delay between polling for replication controller status after the update. Valid time units are "ns", "us" (or "µs"), "ms", "s", "m", "h".`) + cmd.Flags().Duration("timeout", timeout, `Max time to wait for a replication controller to update before giving up. Valid time units are "ns", "us" (or "µs"), "ms", "s", "m", "h".`) cmd.Flags().StringP("filename", "f", "", "Filename or URL to file to use to create the new replication controller.") cmd.Flags().String("image", "", "Image to use for upgrading the replication controller. Can not be used with --filename/-f") cmd.Flags().String("deployment-label-key", "deployment", "The key to use to differentiate between two different controllers, default 'deployment'. Only relevant when --image is specified, ignored otherwise") diff --git a/pkg/kubectl/cmd/stop.go b/pkg/kubectl/cmd/stop.go index fb999414eb8..23ccc004a5b 100644 --- a/pkg/kubectl/cmd/stop.go +++ b/pkg/kubectl/cmd/stop.go @@ -23,7 +23,6 @@ import ( "k8s.io/kubernetes/pkg/kubectl" cmdutil "k8s.io/kubernetes/pkg/kubectl/cmd/util" "k8s.io/kubernetes/pkg/kubectl/resource" - "k8s.io/kubernetes/pkg/util" ) const ( @@ -48,9 +47,6 @@ $ kubectl stop -f path/to/resources` ) func NewCmdStop(f *cmdutil.Factory, out io.Writer) *cobra.Command { - flags := &struct { - Filenames util.StringList - }{} cmd := &cobra.Command{ Use: "stop (-f FILENAME | TYPE (NAME | -l label | --all))", Short: "Deprecated: Gracefully shut down a resource by name or filename.", @@ -58,12 +54,11 @@ func NewCmdStop(f *cmdutil.Factory, out io.Writer) *cobra.Command { Example: stop_example, Run: func(cmd *cobra.Command, args []string) { cmdutil.CheckErr(cmdutil.ValidateOutputArgs(cmd)) - shortOutput := cmdutil.GetFlagString(cmd, "output") == "name" - cmdutil.CheckErr(RunStop(f, cmd, args, flags.Filenames, out, shortOutput)) + cmdutil.CheckErr(RunStop(f, cmd, args, out)) }, } usage := "Filename, directory, or URL to file of resource(s) to be stopped." - kubectl.AddJsonFilenameFlag(cmd, &flags.Filenames, usage) + kubectl.AddJsonFilenameFlag(cmd, usage) cmd.Flags().StringP("selector", "l", "", "Selector (label query) to filter on.") cmd.Flags().Bool("all", false, "[-all] to select all the specified resources.") cmd.Flags().Bool("ignore-not-found", false, "Treat \"resource not found\" as a successful stop.") @@ -73,11 +68,13 @@ func NewCmdStop(f *cmdutil.Factory, out io.Writer) *cobra.Command { return cmd } -func RunStop(f *cmdutil.Factory, cmd *cobra.Command, args []string, filenames util.StringList, out io.Writer, shortOutput bool) error { +func RunStop(f *cmdutil.Factory, cmd *cobra.Command, args []string, out io.Writer) error { cmdNamespace, enforceNamespace, err := f.DefaultNamespace() if err != nil { return err } + + filenames := cmdutil.GetFlagStringSlice(cmd, "filename") mapper, typer := f.Object() r := resource.NewBuilder(mapper, typer, f.ClientMapperForCommand()). ContinueOnError(). @@ -91,5 +88,6 @@ func RunStop(f *cmdutil.Factory, cmd *cobra.Command, args []string, filenames ut if r.Err() != nil { return r.Err() } + shortOutput := cmdutil.GetFlagString(cmd, "output") == "name" return ReapResult(r, f, out, false, cmdutil.GetFlagBool(cmd, "ignore-not-found"), cmdutil.GetFlagDuration(cmd, "timeout"), cmdutil.GetFlagInt(cmd, "grace-period"), shortOutput, mapper) } diff --git a/pkg/kubectl/cmd/util/factory.go b/pkg/kubectl/cmd/util/factory.go index 869e385a4c8..cc4e3380ba6 100644 --- a/pkg/kubectl/cmd/util/factory.go +++ b/pkg/kubectl/cmd/util/factory.go @@ -396,7 +396,12 @@ func (f *Factory) PrinterForMapping(cmd *cobra.Command, mapping *meta.RESTMappin } printer = kubectl.NewVersionedPrinter(printer, mapping.ObjectConvertor, version, mapping.APIVersion) } else { - printer, err = f.Printer(mapping, GetFlagBool(cmd, "no-headers"), withNamespace, GetWideFlag(cmd), GetFlagStringList(cmd, "label-columns")) + // Some callers do not have "label-columns" so we can't use the GetFlagStringSlice() helper + columnLabel, err := cmd.Flags().GetStringSlice("label-columns") + if err != nil { + columnLabel = []string{} + } + printer, err = f.Printer(mapping, GetFlagBool(cmd, "no-headers"), withNamespace, GetWideFlag(cmd), columnLabel) if err != nil { return nil, err } diff --git a/pkg/kubectl/cmd/util/helpers.go b/pkg/kubectl/cmd/util/helpers.go index 91e48c0b3e0..668c37464a0 100644 --- a/pkg/kubectl/cmd/util/helpers.go +++ b/pkg/kubectl/cmd/util/helpers.go @@ -25,7 +25,6 @@ import ( "net/http" "net/url" "os" - "strconv" "strings" "time" @@ -37,7 +36,6 @@ import ( "k8s.io/kubernetes/pkg/client/clientcmd" "k8s.io/kubernetes/pkg/kubectl/resource" "k8s.io/kubernetes/pkg/runtime" - "k8s.io/kubernetes/pkg/util" utilerrors "k8s.io/kubernetes/pkg/util/errors" "github.com/golang/glog" @@ -211,17 +209,20 @@ func getFlag(cmd *cobra.Command, flag string) *pflag.Flag { } func GetFlagString(cmd *cobra.Command, flag string) string { - f := getFlag(cmd, flag) - return f.Value.String() + s, err := cmd.Flags().GetString(flag) + if err != nil { + glog.Fatalf("err %v accessing flag %s for command %s: %s", err, flag, cmd.Name()) + } + return s } // GetFlagStringList can be used to accept multiple argument with flag repetition (e.g. -f arg1 -f arg2 ...) -func GetFlagStringList(cmd *cobra.Command, flag string) util.StringList { - f := cmd.Flags().Lookup(flag) - if f == nil { - return util.StringList{} +func GetFlagStringSlice(cmd *cobra.Command, flag string) []string { + s, err := cmd.Flags().GetStringSlice(flag) + if err != nil { + glog.Fatalf("err %v accessing flag %s for command %s: %s", err, flag, cmd.Name()) } - return *f.Value.(*util.StringList) + return s } // GetWideFlag is used to determine if "-o wide" is used @@ -234,33 +235,28 @@ func GetWideFlag(cmd *cobra.Command) bool { } func GetFlagBool(cmd *cobra.Command, flag string) bool { - f := getFlag(cmd, flag) - result, err := strconv.ParseBool(f.Value.String()) + b, err := cmd.Flags().GetBool(flag) if err != nil { - glog.Fatalf("Invalid value for a boolean flag: %s", f.Value.String()) + glog.Fatalf("err %v accessing flag %s for command %s: %s", err, flag, cmd.Name()) } - return result + return b } // Assumes the flag has a default value. func GetFlagInt(cmd *cobra.Command, flag string) int { - f := getFlag(cmd, flag) - v, err := strconv.Atoi(f.Value.String()) - // This is likely not a sufficiently friendly error message, but cobra - // should prevent non-integer values from reaching here. + i, err := cmd.Flags().GetInt(flag) if err != nil { - glog.Fatalf("unable to convert flag value to int: %v", err) + glog.Fatalf("err: %v accessing flag %s for command %s: %s", err, flag, cmd.Name()) } - return v + return i } func GetFlagDuration(cmd *cobra.Command, flag string) time.Duration { - f := getFlag(cmd, flag) - v, err := time.ParseDuration(f.Value.String()) + d, err := cmd.Flags().GetDuration(flag) if err != nil { - glog.Fatalf("unable to convert flag value to Duration: %v", err) + glog.Fatalf("err: %v accessing flag %s for command %s: %s", err, flag, cmd.Name()) } - return v + return d } func ReadConfigDataFromReader(reader io.Reader, source string) ([]byte, error) { diff --git a/pkg/master/master.go b/pkg/master/master.go index 2c20d732b4c..d6cd1cbdba4 100644 --- a/pkg/master/master.go +++ b/pkg/master/master.go @@ -109,7 +109,7 @@ type Config struct { EnableProfiling bool APIPrefix string ExpAPIPrefix string - CorsAllowedOriginList util.StringList + CorsAllowedOriginList []string Authenticator authenticator.Request // TODO(roberthbailey): Remove once the server no longer supports http basic auth. SupportsBasicAuth bool @@ -186,7 +186,7 @@ type Master struct { enableProfiling bool apiPrefix string expAPIPrefix string - corsAllowedOriginList util.StringList + corsAllowedOriginList []string authenticator authenticator.Request authorizer authorizer.Authorizer admissionControl admission.Interface diff --git a/pkg/util/list.go b/pkg/util/list.go deleted file mode 100644 index 018d24b9b52..00000000000 --- a/pkg/util/list.go +++ /dev/null @@ -1,39 +0,0 @@ -/* -Copyright 2014 The Kubernetes Authors All rights reserved. - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -package util - -import ( - "fmt" - "strings" -) - -type StringList []string - -func (sl *StringList) String() string { - return fmt.Sprint(*sl) -} - -func (sl *StringList) Set(value string) error { - for _, s := range strings.Split(value, ",") { - *sl = append(*sl, s) - } - return nil -} - -func (*StringList) Type() string { - return "stringList" -} diff --git a/pkg/util/list_test.go b/pkg/util/list_test.go deleted file mode 100644 index 4576e0fa86e..00000000000 --- a/pkg/util/list_test.go +++ /dev/null @@ -1,32 +0,0 @@ -/* -Copyright 2014 The Kubernetes Authors All rights reserved. - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -package util - -import ( - "reflect" - "testing" -) - -func TestStringListSet(t *testing.T) { - var sl StringList - sl.Set("foo,bar") - sl.Set("hop") - expected := []string{"foo", "bar", "hop"} - if reflect.DeepEqual(expected, []string(sl)) == false { - t.Errorf("expected: %v, got: %v:", expected, sl) - } -}