cli: remove support for comma-separated template values

Author: mmilata

docs/man/man1/oc-cluster-up.1

@@ -39,7 +39,7 @@ A public hostname can also be specified for the server with the \-\-public\-host
 
 .PP
 \fB\-e\fP, \fB\-\-env\fP=[]
-    Specify key value pairs of environment variables to set on OpenShift container
+    Specify a key\-value pair for an environment variable to set on OpenShift container
 
 .PP
 \fB\-\-forward\-ports\fP=false

docs/man/man1/oc-env.1

@@ -27,7 +27,7 @@ DEPRECATED: This command has been moved to "oc set env"
 
 .PP
 \fB\-e\fP, \fB\-\-env\fP=[]
-    Specify key value pairs of environment variables to set into each container.
+    Specify a key\-value pair for an environment variable to set into each container.
 
 .PP
 \fB\-f\fP, \fB\-\-filename\fP=[]

docs/man/man1/oc-new-app.1

@@ -56,7 +56,7 @@ If you provide source code, a new build will be automatically triggered. You can
 
 .PP
 \fB\-e\fP, \fB\-\-env\fP=[]
-    Specify key\-value pairs of environment variables to set into each container. This doesn't apply to objects created from a template, use parameters instead.
+    Specify a key\-value pair for an environment variable to set into each container. This doesn't apply to objects created from a template, use parameters instead.
 
 .PP
 \fB\-f\fP, \fB\-\-file\fP=[]
@@ -108,7 +108,7 @@ If you provide source code, a new build will be automatically triggered. You can
 
 .PP
 \fB\-p\fP, \fB\-\-param\fP=[]
-    Specify a list of key value pairs (e.g., \-p FOO=BAR,BAR=FOO) to set/override parameter values in the template.
+    Specify a key\-value pair (e.g., \-p FOO=BAR) to set/override a parameter value in the template.
 
 .PP
 \fB\-S\fP, \fB\-\-search\fP=false

docs/man/man1/oc-new-build.1

@@ -64,7 +64,7 @@ Once the build configuration is created a new build will be automatically trigge
 
 .PP
 \fB\-e\fP, \fB\-\-env\fP=[]
-    Specify key value pairs of environment variables to set into resulting image.
+    Specify a key\-value pair for an environment variable to set into resulting image.
 
 .PP
 \fB\-\-image\fP=[]

docs/man/man1/oc-process.1

@@ -54,7 +54,7 @@ The output of the process command is always a list of one or more resources. You
 
 .PP
 \fB\-v\fP, \fB\-\-value\fP=[]
-    Specify a list of key\-value pairs (eg. \-v FOO=BAR,BAR=FOO) to set/override parameter values
+    Specify a key\-value pair (eg. \-v FOO=BAR) to set/override a parameter value in the template.
 
 
 .SH OPTIONS INHERITED FROM PARENT COMMANDS

docs/man/man1/oc-set-deployment-hook.1

@@ -42,7 +42,7 @@ Each hook can have its own cancellation policy. One of: abort, retry, or ignore.
 
 .PP
 \fB\-e\fP, \fB\-\-environment\fP=[]
-    Environment variables to use in the deployment hook pod
+    Environment variable to use in the deployment hook pod
 
 .PP
 \fB\-\-failure\-policy\fP="ignore"

docs/man/man1/oc-set-env.1

@@ -33,7 +33,7 @@ If "\-\-env \-" is passed, environment variables can be read from STDIN using th
 
 .PP
 \fB\-e\fP, \fB\-\-env\fP=[]
-    Specify key value pairs of environment variables to set into each container.
+    Specify a key\-value pair for an environment variable to set into each container.
 
 .PP
 \fB\-f\fP, \fB\-\-filename\fP=[]

docs/man/man1/oc-start-build.1

@@ -33,7 +33,7 @@ In addition, you can pass a file, directory, or source code repository with the
 
 .PP
 \fB\-e\fP, \fB\-\-env\fP=[]
-    Specify key value pairs of environment variables to set for the build container.
+    Specify a key\-value pair for an environment variable to set for the build container.
 
 .PP
 \fB\-F\fP, \fB\-\-follow\fP=false

docs/man/man1/openshift-cli-cluster-up.1

@@ -39,7 +39,7 @@ A public hostname can also be specified for the server with the \-\-public\-host
 
 .PP
 \fB\-e\fP, \fB\-\-env\fP=[]
-    Specify key value pairs of environment variables to set on OpenShift container
+    Specify a key\-value pair for an environment variable to set on OpenShift container
 
 .PP
 \fB\-\-forward\-ports\fP=false

docs/man/man1/openshift-cli-env.1

@@ -27,7 +27,7 @@ DEPRECATED: This command has been moved to "openshift cli set env"
 
 .PP
 \fB\-e\fP, \fB\-\-env\fP=[]
-    Specify key value pairs of environment variables to set into each container.
+    Specify a key\-value pair for an environment variable to set into each container.
 
 .PP
 \fB\-f\fP, \fB\-\-filename\fP=[]

docs/man/man1/openshift-cli-new-app.1

@@ -56,7 +56,7 @@ If you provide source code, a new build will be automatically triggered. You can
 
 .PP
 \fB\-e\fP, \fB\-\-env\fP=[]
-    Specify key\-value pairs of environment variables to set into each container. This doesn't apply to objects created from a template, use parameters instead.
+    Specify a key\-value pair for an environment variable to set into each container. This doesn't apply to objects created from a template, use parameters instead.
 
 .PP
 \fB\-f\fP, \fB\-\-file\fP=[]
@@ -108,7 +108,7 @@ If you provide source code, a new build will be automatically triggered. You can
 
 .PP
 \fB\-p\fP, \fB\-\-param\fP=[]
-    Specify a list of key value pairs (e.g., \-p FOO=BAR,BAR=FOO) to set/override parameter values in the template.
+    Specify a key\-value pair (e.g., \-p FOO=BAR) to set/override a parameter value in the template.
 
 .PP
 \fB\-S\fP, \fB\-\-search\fP=false

docs/man/man1/openshift-cli-new-build.1

@@ -64,7 +64,7 @@ Once the build configuration is created a new build will be automatically trigge
 
 .PP
 \fB\-e\fP, \fB\-\-env\fP=[]
-    Specify key value pairs of environment variables to set into resulting image.
+    Specify a key\-value pair for an environment variable to set into resulting image.
 
 .PP
 \fB\-\-image\fP=[]

docs/man/man1/openshift-cli-process.1

@@ -54,7 +54,7 @@ The output of the process command is always a list of one or more resources. You
 
 .PP
 \fB\-v\fP, \fB\-\-value\fP=[]
-    Specify a list of key\-value pairs (eg. \-v FOO=BAR,BAR=FOO) to set/override parameter values
+    Specify a key\-value pair (eg. \-v FOO=BAR) to set/override a parameter value in the template.
 
 
 .SH OPTIONS INHERITED FROM PARENT COMMANDS

docs/man/man1/openshift-cli-set-deployment-hook.1

@@ -42,7 +42,7 @@ Each hook can have its own cancellation policy. One of: abort, retry, or ignore.
 
 .PP
 \fB\-e\fP, \fB\-\-environment\fP=[]
-    Environment variables to use in the deployment hook pod
+    Environment variable to use in the deployment hook pod
 
 .PP
 \fB\-\-failure\-policy\fP="ignore"

docs/man/man1/openshift-cli-set-env.1

@@ -33,7 +33,7 @@ If "\-\-env \-" is passed, environment variables can be read from STDIN using th
 
 .PP
 \fB\-e\fP, \fB\-\-env\fP=[]
-    Specify key value pairs of environment variables to set into each container.
+    Specify a key\-value pair for an environment variable to set into each container.
 
 .PP
 \fB\-f\fP, \fB\-\-filename\fP=[]

docs/man/man1/openshift-cli-start-build.1

@@ -33,7 +33,7 @@ In addition, you can pass a file, directory, or source code repository with the
 
 .PP
 \fB\-e\fP, \fB\-\-env\fP=[]
-    Specify key value pairs of environment variables to set for the build container.
+    Specify a key\-value pair for an environment variable to set for the build container.
 
 .PP
 \fB\-F\fP, \fB\-\-follow\fP=false

pkg/bootstrap/docker/up.go

@@ -108,7 +108,7 @@ var (
 )
 
 // NewCmdUp creates a command that starts openshift on Docker with reasonable defaults
-func NewCmdUp(name, fullName string, f *osclientcmd.Factory, out io.Writer) *cobra.Command {
+func NewCmdUp(name, fullName string, f *osclientcmd.Factory, out, errout io.Writer) *cobra.Command {
 	config := &ClientStartConfig{
 		Out:            out,
 		PortForwarding: defaultPortForwarding(),
@@ -120,7 +120,7 @@ func NewCmdUp(name, fullName string, f *osclientcmd.Factory, out io.Writer) *cob
 		Example: fmt.Sprintf(cmdUpExample, fullName),
 		Run: func(c *cobra.Command, args []string) {
 			kcmdutil.CheckErr(config.Complete(f, c))
-			kcmdutil.CheckErr(config.Validate(out))
+			kcmdutil.CheckErr(config.Validate(out, errout))
 			if err := config.Start(out); err != nil {
 				os.Exit(1)
 			}
@@ -139,7 +139,7 @@ func NewCmdUp(name, fullName string, f *osclientcmd.Factory, out io.Writer) *cob
 	cmd.Flags().StringVar(&config.HostDataDir, "host-data-dir", "", "Directory on Docker host for OpenShift data. If not specified, etcd data will not be persisted on the host.")
 	cmd.Flags().BoolVar(&config.PortForwarding, "forward-ports", config.PortForwarding, "Use Docker port-forwarding to communicate with origin container. Requires 'socat' locally.")
 	cmd.Flags().IntVar(&config.ServerLogLevel, "server-loglevel", 0, "Log level for OpenShift server")
-	cmd.Flags().StringSliceVarP(&config.Environment, "env", "e", config.Environment, "Specify key value pairs of environment variables to set on OpenShift container")
+	cmd.Flags().StringArrayVarP(&config.Environment, "env", "e", config.Environment, "Specify a key-value pair for an environment variable to set on OpenShift container")
 	cmd.Flags().BoolVar(&config.ShouldInstallMetrics, "metrics", false, "Install metrics (experimental)")
 	cmd.Flags().BoolVar(&config.ShouldInstallLogging, "logging", false, "Install logging (experimental)")
 	return cmd
@@ -313,7 +313,8 @@ func (c *ClientStartConfig) Complete(f *osclientcmd.Factory, cmd *cobra.Command)
 }
 
 // Validate validates that required fields in StartConfig have been populated
-func (c *ClientStartConfig) Validate(out io.Writer) error {
+func (c *ClientStartConfig) Validate(out, errout io.Writer) error {
+	cmdutil.WarnAboutCommaSeparation(errout, c.Environment, "--env")
 	if len(c.Tasks) == 0 {
 		return fmt.Errorf("no startup tasks to execute")
 	}

pkg/cmd/cli/cli.go

@@ -100,7 +100,7 @@ func NewCommandCLI(name, fullName string, in io.Reader, out, errout io.Writer) *
 				cmd.NewCmdProject(fullName+" project", f, out),
 				cmd.NewCmdProjects(fullName, f, out),
 				cmd.NewCmdExplain(fullName, f, out, errout),
-				cluster.NewCmdCluster(cluster.ClusterRecommendedName, fullName+" "+cluster.ClusterRecommendedName, f, out),
+				cluster.NewCmdCluster(cluster.ClusterRecommendedName, fullName+" "+cluster.ClusterRecommendedName, f, out, errout),
 				cmd.NewCmdIdle(fullName, f, out, errout),
 			},
 		},
@@ -156,7 +156,7 @@ func NewCommandCLI(name, fullName string, in io.Reader, out, errout io.Writer) *
 				cmd.NewCmdReplace(fullName, f, out),
 				cmd.NewCmdApply(fullName, f, out),
 				cmd.NewCmdPatch(fullName, f, out),
-				cmd.NewCmdProcess(fullName, f, out),
+				cmd.NewCmdProcess(fullName, f, out, errout),
 				cmd.NewCmdExport(fullName, f, in, out),
 				cmd.NewCmdExtract(fullName, f, in, out, errout),
 				observe.NewCmdObserve(fullName, f, out, errout),

pkg/cmd/cli/cmd/cluster/cluster.go

@@ -33,7 +33,7 @@ var (
 		routing suffix, use the --routing-suffix flag.`)
 )
 
-func NewCmdCluster(name, fullName string, f *clientcmd.Factory, out io.Writer) *cobra.Command {
+func NewCmdCluster(name, fullName string, f *clientcmd.Factory, out, errout io.Writer) *cobra.Command {
 	// Parent command to which all subcommands are added.
 	cmds := &cobra.Command{
 		Use:   fmt.Sprintf("%s ACTION", name),
@@ -42,7 +42,7 @@ func NewCmdCluster(name, fullName string, f *clientcmd.Factory, out io.Writer) *
 		Run:   cmdutil.DefaultSubCommandRun(out),
 	}
 
-	cmds.AddCommand(docker.NewCmdUp(docker.CmdUpRecommendedName, fullName+" "+docker.CmdUpRecommendedName, f, out))
+	cmds.AddCommand(docker.NewCmdUp(docker.CmdUpRecommendedName, fullName+" "+docker.CmdUpRecommendedName, f, out, errout))
 	cmds.AddCommand(docker.NewCmdDown(docker.CmdDownRecommendedName, fullName+" "+docker.CmdDownRecommendedName, f, out))
 	cmds.AddCommand(docker.NewCmdStatus(docker.CmdStatusRecommendedName, fullName+" "+docker.CmdStatusRecommendedName, f, out))
 	return cmds

pkg/cmd/cli/cmd/helper.go

@@ -1,6 +1,11 @@
 package cmd
 
-import "strings"
+import (
+	"strings"
+
+	"github.com/golang/glog"
+	"github.com/spf13/cobra"
+)
 
 // parseNamespaceResourceName parses the value and returns namespace, resource and the
 // value (resource name) itself. The valid syntax is:
@@ -20,3 +25,13 @@ func parseNamespaceResourceName(v, defaultNamespace string) (ns, resource, name
 	}
 	return "", "", "", false
 }
+
+// TODO: GetFlagStringArray already exists in kubernetes master, remove this
+// after rebasing to version that has it
+func getFlagStringArray(cmd *cobra.Command, flag string) []string {
+	s, err := cmd.Flags().GetStringArray(flag)
+	if err != nil {
+		glog.Fatalf("error accessing flag %s for command %s: %v", flag, cmd.Name(), err)
+	}
+	return s
+}

pkg/cmd/cli/cmd/newapp.go

@@ -166,9 +166,9 @@ func NewCmdNewApplication(name, baseName string, f *clientcmd.Factory, out, erro
 	cmd.Flags().StringSliceVar(&config.Templates, "template", config.Templates, "Name of a stored template to use in the app.")
 	cmd.Flags().StringSliceVarP(&config.TemplateFiles, "file", "f", config.TemplateFiles, "Path to a template file to use for the app.")
 	cmd.MarkFlagFilename("file", "yaml", "yml", "json")
-	cmd.Flags().StringSliceVarP(&config.TemplateParameters, "param", "p", config.TemplateParameters, "Specify a list of key value pairs (e.g., -p FOO=BAR,BAR=FOO) to set/override parameter values in the template.")
+	cmd.Flags().StringArrayVarP(&config.TemplateParameters, "param", "p", config.TemplateParameters, "Specify a key-value pair (e.g., -p FOO=BAR) to set/override a parameter value in the template.")
 	cmd.Flags().StringSliceVar(&config.Groups, "group", config.Groups, "Indicate components that should be grouped together as <comp1>+<comp2>.")
-	cmd.Flags().StringSliceVarP(&config.Environment, "env", "e", config.Environment, "Specify key-value pairs of environment variables to set into each container. This doesn't apply to objects created from a template, use parameters instead.")
+	cmd.Flags().StringArrayVarP(&config.Environment, "env", "e", config.Environment, "Specify a key-value pair for an environment variable to set into each container. This doesn't apply to objects created from a template, use parameters instead.")
 	cmd.Flags().StringVar(&config.Name, "name", "", "Set name to use for generated application artifacts")
 	cmd.Flags().StringVar(&config.Strategy, "strategy", "", "Specify the build strategy to use if you don't want to detect (docker|source).")
 	cmd.Flags().StringP("labels", "l", "", "Label to set in all resources for this application.")
@@ -209,6 +209,9 @@ func (o *NewAppOptions) Complete(baseName, name string, f *clientcmd.Factory, c
 
 	o.Config.DryRun = o.Action.DryRun
 
+	cmdutil.WarnAboutCommaSeparation(o.ErrOut, o.Config.Environment, "--env")
+	cmdutil.WarnAboutCommaSeparation(o.ErrOut, o.Config.TemplateParameters, "--param")
+
 	o.CommandPath = c.CommandPath()
 	o.BaseName = baseName
 	o.CommandName = name

pkg/cmd/cli/cmd/newbuild.go

@@ -128,7 +128,7 @@ func NewCmdNewBuild(name, baseName string, f *clientcmd.Factory, in io.Reader, o
 	cmd.Flags().StringVar(&config.Name, "name", "", "Set name to use for generated build artifacts.")
 	cmd.Flags().StringVar(&config.To, "to", "", "Push built images to this image stream tag (or Docker image repository if --to-docker is set).")
 	cmd.Flags().BoolVar(&config.OutputDocker, "to-docker", false, "Have the build output push to a Docker repository.")
-	cmd.Flags().StringSliceVarP(&config.Environment, "env", "e", config.Environment, "Specify key value pairs of environment variables to set into resulting image.")
+	cmd.Flags().StringArrayVarP(&config.Environment, "env", "e", config.Environment, "Specify a key-value pair for an environment variable to set into resulting image.")
 	cmd.Flags().StringVar(&config.Strategy, "strategy", "", "Specify the build strategy to use if you don't want to detect (docker|source).")
 	cmd.Flags().StringVarP(&config.Dockerfile, "dockerfile", "D", "", "Specify the contents of a Dockerfile to build directly, implies --strategy=docker. Pass '-' to read from STDIN.")
 	cmd.Flags().BoolVar(&config.BinaryBuild, "binary", false, "Instead of expecting a source URL, set the build to expect binary contents. Will disable triggers.")
@@ -173,6 +173,8 @@ func (o *NewBuildOptions) Complete(baseName, commandName string, f *clientcmd.Fa
 	o.CommandPath = c.CommandPath()
 	o.CommandName = commandName
 
+	cmdutil.WarnAboutCommaSeparation(o.ErrOut, o.Config.Environment, "--env")
+
 	mapper, _ := f.Object(false)
 	o.PrintObject = cmdutil.VersionedPrintObject(f.PrintObject, c, mapper, out)
 	o.LogsForObject = f.LogsForObject

pkg/cmd/cli/cmd/process.go

@@ -20,6 +20,7 @@ import (
 
 	"github.com/openshift/origin/pkg/cmd/cli/describe"
 	"github.com/openshift/origin/pkg/cmd/templates"
+	cmdutil "github.com/openshift/origin/pkg/cmd/util"
 	"github.com/openshift/origin/pkg/cmd/util/clientcmd"
 	"github.com/openshift/origin/pkg/template"
 	templateapi "github.com/openshift/origin/pkg/template/api"
@@ -57,20 +58,20 @@ var (
 )
 
 // NewCmdProcess implements the OpenShift cli process command
-func NewCmdProcess(fullName string, f *clientcmd.Factory, out io.Writer) *cobra.Command {
+func NewCmdProcess(fullName string, f *clientcmd.Factory, out, errout io.Writer) *cobra.Command {
 	cmd := &cobra.Command{
 		Use:     "process (TEMPLATE | -f FILENAME) [-v=KEY=VALUE]",
 		Short:   "Process a template into list of resources",
 		Long:    processLong,
 		Example: fmt.Sprintf(processExample, fullName),
 		Run: func(cmd *cobra.Command, args []string) {
-			err := RunProcess(f, out, cmd, args)
+			err := RunProcess(f, out, errout, cmd, args)
 			kcmdutil.CheckErr(err)
 		},
 	}
 	cmd.Flags().StringP("filename", "f", "", "Filename or URL to file to read a template")
 	cmd.MarkFlagFilename("filename", "yaml", "yml", "json")
-	cmd.Flags().StringSliceP("value", "v", nil, "Specify a list of key-value pairs (eg. -v FOO=BAR,BAR=FOO) to set/override parameter values")
+	cmd.Flags().StringArrayP("value", "v", nil, "Specify a key-value pair (eg. -v FOO=BAR) to set/override a parameter value in the template.")
 	cmd.Flags().BoolP("parameters", "", false, "Do not process but only print available parameters")
 	cmd.Flags().StringP("labels", "l", "", "Label to set in all resources for this template")
 
@@ -83,7 +84,7 @@ func NewCmdProcess(fullName string, f *clientcmd.Factory, out io.Writer) *cobra.
 }
 
 // RunProcess contains all the necessary functionality for the OpenShift cli process command
-func RunProcess(f *clientcmd.Factory, out io.Writer, cmd *cobra.Command, args []string) error {
+func RunProcess(f *clientcmd.Factory, out, errout io.Writer, cmd *cobra.Command, args []string) error {
 	templateName, valueArgs := "", []string{}
 	for _, s := range args {
 		isValue := strings.Contains(s, "=")
@@ -102,9 +103,11 @@ func RunProcess(f *clientcmd.Factory, out io.Writer, cmd *cobra.Command, args []
 
 	var flagValues []string
 	if cmd.Flag("value").Changed {
-		flagValues = kcmdutil.GetFlagStringSlice(cmd, "value")
+		flagValues = getFlagStringArray(cmd, "value")
 	}
 
+	cmdutil.WarnAboutCommaSeparation(errout, flagValues, "--value")
+
 	for _, value := range flagValues {
 		key := strings.Split(value, "=")[0]
 		if keys.Has(key) {
@@ -238,7 +241,7 @@ func RunProcess(f *clientcmd.Factory, out io.Writer, cmd *cobra.Command, args []
 	// Override the values for the current template parameters
 	// when user specify the --value
 	if cmd.Flag("value").Changed {
-		values := kcmdutil.GetFlagStringSlice(cmd, "value")
+		values := getFlagStringArray(cmd, "value")
 		if errs := injectUserVars(values, obj); errs != nil {
 			return kerrors.NewAggregate(errs)
 		}

pkg/cmd/cli/cmd/set/deploymenthook.go

@@ -123,7 +123,7 @@ func NewCmdDeploymentHook(fullName string, f *clientcmd.Factory, out, errOut io.
 	cmd.Flags().BoolVar(&options.Mid, "mid", options.Mid, "Set or remove a mid deployment hook")
 	cmd.Flags().BoolVar(&options.Post, "post", options.Post, "Set or remove a post deployment hook")
 
-	cmd.Flags().StringSliceVarP(&options.Environment, "environment", "e", options.Environment, "Environment variables to use in the deployment hook pod")
+	cmd.Flags().StringArrayVarP(&options.Environment, "environment", "e", options.Environment, "Environment variable to use in the deployment hook pod")
 	cmd.Flags().StringSliceVarP(&options.Volumes, "volumes", "v", options.Volumes, "Volumes from the pod template to use in the deployment hook pod")
 
 	cmd.Flags().String("failure-policy", "ignore", "The failure policy for the deployment hook. Valid values are: abort,retry,ignore")
@@ -233,6 +233,9 @@ func (o *DeploymentHookOptions) Validate() error {
 	if len(o.Command) == 0 {
 		return fmt.Errorf("you must specify a command for the deployment hook")
 	}
+
+	cmdutil.WarnAboutCommaSeparation(o.Err, o.Environment, "--environment")
+
 	return nil
 }