Fix env, annotation and labels flags in service create/update/apply (#1516)

Signed-off-by: Boaz Shuster <boaz.shuster.github@gmail.com>

docs/cmd/kn_service_apply.md

@@ -29,7 +29,7 @@ kn service apply s0 --filename my-svc.yml
 ### Options
 
 ```
-  -a, --annotation stringArray            Annotations to set for both Service and Revision. name=value; you may provide this flag any number of times to set multiple annotations. To unset, specify the annotation name followed by a "-" (e.g., name-).
+  -a, --annotation stringArray            Annotations to set for both Service and Revision. name=value; you may provide this flag any number of times to set multiple annotations.
       --annotation-revision stringArray   Revision annotation to set. name=value; you may provide this flag any number of times to set multiple annotations. To unset, specify the annotation name followed by a "-" (e.g., name-). This flag takes precedence over the "annotation" flag.
       --annotation-service stringArray    Service annotation to set. name=value; you may provide this flag any number of times to set multiple annotations. To unset, specify the annotation name followed by a "-" (e.g., name-). This flag takes precedence over the "annotation" flag.
       --arg stringArray                   Add argument to the container command. Example: --arg myArg1 --arg --myArg2 --arg myArg3=3. You can use this flag multiple times.
@@ -37,15 +37,15 @@ kn service apply s0 --filename my-svc.yml
       --cmd stringArray                   Specify command to be used as entrypoint instead of default one. Example: --cmd /app/start or --cmd sh --cmd /app/start.sh or --cmd /app/start --arg myArg to pass additional arguments.
       --concurrency-limit int             Hard Limit of concurrent requests to be processed by a single replica.
       --containers string                 Specify path to file including definition for additional containers, alternatively use '-' to read from stdin. Example: --containers ./containers.yaml or --containers -.
-  -e, --env stringArray                   Environment variable to set. NAME=value; you may provide this flag any number of times to set multiple environment variables. To unset, specify the environment variable name followed by a "-" (e.g., NAME-).
+  -e, --env stringArray                   Environment variable to set. NAME=value; you may provide this flag any number of times to set multiple environment variables.
       --env-file string                   Path to a file containing environment variables (e.g. --env-file=/home/knative/service1/env).
-      --env-from stringArray              Add environment variables from a ConfigMap (prefix cm: or config-map:) or a Secret (prefix secret:). Example: --env-from cm:myconfigmap or --env-from secret:mysecret. You can use this flag multiple times. To unset a ConfigMap/Secret reference, append "-" to the name, e.g. --env-from cm:myconfigmap-.
+      --env-from stringArray              Add environment variables from a ConfigMap (prefix cm: or config-map:) or a Secret (prefix secret:). Example: --env-from cm:myconfigmap or --env-from secret:mysecret. You can use this flag multiple times.
-      --env-value-from stringArray        Add environment variable from a value of key in ConfigMap (prefix cm: or config-map:) or a Secret (prefix sc: or secret:). Example: --env-value-from NAME=cm:myconfigmap:key or --env-value-from NAME=secret:mysecret:key. You can use this flag multiple times. To unset a value from a ConfigMap/Secret key reference, append "-" to the key, e.g. --env-value-from ENV-.
+      --env-value-from stringArray        Add environment variable from a value of key in ConfigMap (prefix cm: or config-map:) or a Secret (prefix sc: or secret:). Example: --env-value-from NAME=cm:myconfigmap:key or --env-value-from NAME=secret:mysecret:key. You can use this flag multiple times.
   -f, --filename string                   Create a service from file. The created service can be further modified by combining with other options. For example, -f /path/to/file --env NAME=value adds also an environment variable.
       --force                             Create service forcefully, replaces existing service if any.
   -h, --help                              help for apply
       --image string                      Image to run.
-  -l, --label stringArray                 Labels to set for both Service and Revision. name=value; you may provide this flag any number of times to set multiple labels. To unset, specify the label name followed by a "-" (e.g., name-).
+  -l, --label stringArray                 Labels to set for both Service and Revision. name=value; you may provide this flag any number of times to set multiple labels.
       --label-revision stringArray        Revision label to set. name=value; you may provide this flag any number of times to set multiple labels. To unset, specify the label name followed by a "-" (e.g., name-). This flag takes precedence over the "label" flag.
       --label-service stringArray         Service label to set. name=value; you may provide this flag any number of times to set multiple labels. To unset, specify the label name followed by a "-" (e.g., name-). This flag takes precedence over the "label" flag.
       --limit strings                     The resource requirement limits for this Service. For example, 'cpu=100m,memory=256Mi'. You can use this flag multiple times. To unset a resource limit, append "-" to the resource name, e.g. '--limit memory-'.

docs/cmd/kn_service_create.md

@@ -54,7 +54,7 @@ kn service create NAME --image IMAGE
 ### Options
 
 ```
-  -a, --annotation stringArray            Annotations to set for both Service and Revision. name=value; you may provide this flag any number of times to set multiple annotations. To unset, specify the annotation name followed by a "-" (e.g., name-).
+  -a, --annotation stringArray            Annotations to set for both Service and Revision. name=value; you may provide this flag any number of times to set multiple annotations.
       --annotation-revision stringArray   Revision annotation to set. name=value; you may provide this flag any number of times to set multiple annotations. To unset, specify the annotation name followed by a "-" (e.g., name-). This flag takes precedence over the "annotation" flag.
       --annotation-service stringArray    Service annotation to set. name=value; you may provide this flag any number of times to set multiple annotations. To unset, specify the annotation name followed by a "-" (e.g., name-). This flag takes precedence over the "annotation" flag.
       --arg stringArray                   Add argument to the container command. Example: --arg myArg1 --arg --myArg2 --arg myArg3=3. You can use this flag multiple times.
@@ -62,15 +62,15 @@ kn service create NAME --image IMAGE
       --cmd stringArray                   Specify command to be used as entrypoint instead of default one. Example: --cmd /app/start or --cmd sh --cmd /app/start.sh or --cmd /app/start --arg myArg to pass additional arguments.
       --concurrency-limit int             Hard Limit of concurrent requests to be processed by a single replica.
       --containers string                 Specify path to file including definition for additional containers, alternatively use '-' to read from stdin. Example: --containers ./containers.yaml or --containers -.
-  -e, --env stringArray                   Environment variable to set. NAME=value; you may provide this flag any number of times to set multiple environment variables. To unset, specify the environment variable name followed by a "-" (e.g., NAME-).
+  -e, --env stringArray                   Environment variable to set. NAME=value; you may provide this flag any number of times to set multiple environment variables.
       --env-file string                   Path to a file containing environment variables (e.g. --env-file=/home/knative/service1/env).
-      --env-from stringArray              Add environment variables from a ConfigMap (prefix cm: or config-map:) or a Secret (prefix secret:). Example: --env-from cm:myconfigmap or --env-from secret:mysecret. You can use this flag multiple times. To unset a ConfigMap/Secret reference, append "-" to the name, e.g. --env-from cm:myconfigmap-.
+      --env-from stringArray              Add environment variables from a ConfigMap (prefix cm: or config-map:) or a Secret (prefix secret:). Example: --env-from cm:myconfigmap or --env-from secret:mysecret. You can use this flag multiple times.
-      --env-value-from stringArray        Add environment variable from a value of key in ConfigMap (prefix cm: or config-map:) or a Secret (prefix sc: or secret:). Example: --env-value-from NAME=cm:myconfigmap:key or --env-value-from NAME=secret:mysecret:key. You can use this flag multiple times. To unset a value from a ConfigMap/Secret key reference, append "-" to the key, e.g. --env-value-from ENV-.
+      --env-value-from stringArray        Add environment variable from a value of key in ConfigMap (prefix cm: or config-map:) or a Secret (prefix sc: or secret:). Example: --env-value-from NAME=cm:myconfigmap:key or --env-value-from NAME=secret:mysecret:key. You can use this flag multiple times.
   -f, --filename string                   Create a service from file. The created service can be further modified by combining with other options. For example, -f /path/to/file --env NAME=value adds also an environment variable.
       --force                             Create service forcefully, replaces existing service if any.
   -h, --help                              help for create
       --image string                      Image to run.
-  -l, --label stringArray                 Labels to set for both Service and Revision. name=value; you may provide this flag any number of times to set multiple labels. To unset, specify the label name followed by a "-" (e.g., name-).
+  -l, --label stringArray                 Labels to set for both Service and Revision. name=value; you may provide this flag any number of times to set multiple labels.
       --label-revision stringArray        Revision label to set. name=value; you may provide this flag any number of times to set multiple labels. To unset, specify the label name followed by a "-" (e.g., name-). This flag takes precedence over the "label" flag.
       --label-service stringArray         Service label to set. name=value; you may provide this flag any number of times to set multiple labels. To unset, specify the label name followed by a "-" (e.g., name-). This flag takes precedence over the "label" flag.
       --limit strings                     The resource requirement limits for this Service. For example, 'cpu=100m,memory=256Mi'. You can use this flag multiple times. To unset a resource limit, append "-" to the resource name, e.g. '--limit memory-'.

pkg/kn/commands/container/add.go

@@ -75,6 +75,7 @@ func NewContainerAddCommand(p *commands.KnParams) *cobra.Command {
 		},
 	}
 	podSpecFlags.AddFlags(cmd.Flags())
+	podSpecFlags.AddUpdateFlags(cmd.Flags())
 	// Volume is not part of ContainerSpec
 	cmd.Flag("volume").Hidden = true
 

pkg/kn/commands/service/configuration_edit_flags.go

@@ -128,12 +128,6 @@ func (p *ConfigurationEditFlags) addSharedFlags(command *cobra.Command) {
 		"Percentage of concurrent requests utilization before scaling up.")
 	p.markFlagMakesRevision("scale-utilization")
 
-	command.Flags().StringArrayVarP(&p.Labels, "label", "l", []string{},
-		"Labels to set for both Service and Revision. name=value; you may provide this flag "+
-			"any number of times to set multiple labels. "+
-			"To unset, specify the label name followed by a \"-\" (e.g., name-).")
-	p.markFlagMakesRevision("label")
-
 	command.Flags().StringArrayVarP(&p.LabelsService, "label-service", "", []string{},
 		"Service label to set. name=value; you may provide this flag "+
 			"any number of times to set multiple labels. "+
@@ -160,12 +154,6 @@ func (p *ConfigurationEditFlags) addSharedFlags(command *cobra.Command) {
 			"the image. (--no-lock-to-digest pulls the image tag afresh with each new revision)")
 	// Don't mark as changing the revision.
 
-	command.Flags().StringArrayVarP(&p.Annotations, "annotation", "a", []string{},
-		"Annotations to set for both Service and Revision. name=value; you may provide this flag "+
-			"any number of times to set multiple annotations. "+
-			"To unset, specify the annotation name followed by a \"-\" (e.g., name-).")
-	p.markFlagMakesRevision("annotation")
-
 	command.Flags().StringArrayVarP(&p.AnnotationsService, "annotation-service", "", []string{},
 		"Service annotation to set. name=value; you may provide this flag "+
 			"any number of times to set multiple annotations. "+
@@ -187,11 +175,33 @@ func (p *ConfigurationEditFlags) addSharedFlags(command *cobra.Command) {
 // AddUpdateFlags adds the flags specific to update.
 func (p *ConfigurationEditFlags) AddUpdateFlags(command *cobra.Command) {
 	p.addSharedFlags(command)
+
+	flagNames := p.PodSpecFlags.AddUpdateFlags(command.Flags())
+	for _, name := range flagNames {
+		p.markFlagMakesRevision(name)
+	}
+
+	command.Flags().StringArrayVarP(&p.Annotations, "annotation", "a", []string{},
+		"Annotations to set for both Service and Revision. name=value; you may provide this flag "+
+			"any number of times to set multiple annotations. "+
+			"To unset, specify the annotation name followed by a \"-\" (e.g., name-).")
+	p.markFlagMakesRevision("annotation")
+	command.Flags().StringArrayVarP(&p.Labels, "label", "l", []string{},
+		"Labels to set for both Service and Revision. name=value; you may provide this flag "+
+			"any number of times to set multiple labels. "+
+			"To unset, specify the label name followed by a \"-\" (e.g., name-).")
+	p.markFlagMakesRevision("label")
 }
 
 // AddCreateFlags adds the flags specific to create
 func (p *ConfigurationEditFlags) AddCreateFlags(command *cobra.Command) {
 	p.addSharedFlags(command)
+
+	flagNames := p.PodSpecFlags.AddCreateFlags(command.Flags())
+	for _, name := range flagNames {
+		p.markFlagMakesRevision(name)
+	}
+
 	command.Flags().BoolVar(&p.ForceCreate, "force", false,
 		"Create service forcefully, replaces existing service if any.")
 	command.Flags().StringVarP(&p.Filename, "filename", "f", "", "Create a service from file. "+
@@ -199,6 +209,14 @@ func (p *ConfigurationEditFlags) AddCreateFlags(command *cobra.Command) {
 		"For example, -f /path/to/file --env NAME=value adds also an environment variable.")
 	command.MarkFlagFilename("filename")
 	p.markFlagMakesRevision("filename")
+	command.Flags().StringArrayVarP(&p.Annotations, "annotation", "a", []string{},
+		"Annotations to set for both Service and Revision. name=value; you may provide this flag "+
+			"any number of times to set multiple annotations.")
+	p.markFlagMakesRevision("annotation")
+	command.Flags().StringArrayVarP(&p.Labels, "label", "l", []string{},
+		"Labels to set for both Service and Revision. name=value; you may provide this flag "+
+			"any number of times to set multiple labels.")
+	p.markFlagMakesRevision("label")
 }
 
 // Apply mutates the given service according to the flags in the command.

pkg/kn/commands/source/container/create.go

@@ -85,6 +85,7 @@ func NewContainerCreateCommand(p *commands.KnParams) *cobra.Command {
 	}
 	commands.AddNamespaceFlags(cmd.Flags(), false)
 	podFlags.AddFlags(cmd.Flags())
+	podFlags.AddUpdateFlags(cmd.Flags())
 	sinkFlags.Add(cmd)
 	cmd.MarkFlagRequired("image")
 	cmd.MarkFlagRequired("sink")

pkg/kn/commands/source/container/update.go

@@ -94,6 +94,7 @@ func NewContainerUpdateCommand(p *commands.KnParams) *cobra.Command {
 	}
 	commands.AddNamespaceFlags(cmd.Flags(), false)
 	podFlags.AddFlags(cmd.Flags())
+	podFlags.AddUpdateFlags(cmd.Flags())
 	sinkFlags.Add(cmd)
 	return cmd
 }

pkg/kn/flags/podspec.go

@@ -72,23 +72,15 @@ func (s *uniqueStringArg) Type() string {
 
 func (s *uniqueStringArg) String() string { return string(*s) }
 
-//AddFlags will add PodSpec related flags to FlagSet
+//AddUpdateFlags will add PodSpec flags related to environment variable to FlagSet of update command
-func (p *PodSpecFlags) AddFlags(flagset *pflag.FlagSet) []string {
+func (p *PodSpecFlags) AddUpdateFlags(flagset *pflag.FlagSet) []string {
-
 	flagNames := []string{}
-
-	flagset.VarP(&p.Image, "image", "", "Image to run.")
-	flagNames = append(flagNames, "image")
-
 	flagset.StringArrayVarP(&p.Env, "env", "e", []string{},
 		"Environment variable to set. NAME=value; you may provide this flag "+
 			"any number of times to set multiple environment variables. "+
 			"To unset, specify the environment variable name followed by a \"-\" (e.g., NAME-).")
 	flagNames = append(flagNames, "env")
 
-	flagset.StringVarP(&p.EnvFile, "env-file", "", "", "Path to a file containing environment variables (e.g. --env-file=/home/knative/service1/env).")
-	flagNames = append(flagNames, "env-file")
-
 	flagset.StringArrayVarP(&p.EnvValueFrom, "env-value-from", "", []string{},
 		"Add environment variable from a value of key in ConfigMap (prefix cm: or config-map:) or a Secret (prefix sc: or secret:). "+
 			"Example: --env-value-from NAME=cm:myconfigmap:key or --env-value-from NAME=secret:mysecret:key. "+
@@ -103,6 +95,43 @@ func (p *PodSpecFlags) AddFlags(flagset *pflag.FlagSet) []string {
 			"To unset a ConfigMap/Secret reference, append \"-\" to the name, e.g. --env-from cm:myconfigmap-.")
 	flagNames = append(flagNames, "env-from")
 
+	return flagNames
+}
+
+//AddCreateFlags will add PodSpec flags related to environment variable to FlagSet of create command
+func (p *PodSpecFlags) AddCreateFlags(flagset *pflag.FlagSet) []string {
+	flagNames := []string{}
+	flagset.StringArrayVarP(&p.Env, "env", "e", []string{},
+		"Environment variable to set. NAME=value; you may provide this flag "+
+			"any number of times to set multiple environment variables.")
+	flagNames = append(flagNames, "env")
+
+	flagset.StringArrayVarP(&p.EnvValueFrom, "env-value-from", "", []string{},
+		"Add environment variable from a value of key in ConfigMap (prefix cm: or config-map:) or a Secret (prefix sc: or secret:). "+
+			"Example: --env-value-from NAME=cm:myconfigmap:key or --env-value-from NAME=secret:mysecret:key. "+
+			"You can use this flag multiple times.")
+	flagNames = append(flagNames, "env-value-from")
+
+	flagset.StringArrayVarP(&p.EnvFrom, "env-from", "", []string{},
+		"Add environment variables from a ConfigMap (prefix cm: or config-map:) or a Secret (prefix secret:). "+
+			"Example: --env-from cm:myconfigmap or --env-from secret:mysecret. "+
+			"You can use this flag multiple times.")
+	flagNames = append(flagNames, "env-from")
+
+	return flagNames
+}
+
+//AddFlags will add PodSpec related flags to FlagSet
+func (p *PodSpecFlags) AddFlags(flagset *pflag.FlagSet) []string {
+
+	flagNames := []string{}
+
+	flagset.VarP(&p.Image, "image", "", "Image to run.")
+	flagNames = append(flagNames, "image")
+
+	flagset.StringVarP(&p.EnvFile, "env-file", "", "", "Path to a file containing environment variables (e.g. --env-file=/home/knative/service1/env).")
+	flagNames = append(flagNames, "env-file")
+
 	flagset.StringArrayVarP(&p.Mount, "mount", "", []string{},
 		"Mount a ConfigMap (prefix cm: or config-map:), a Secret (prefix secret: or sc:), or an existing Volume (without any prefix) on the specified directory. "+
 			"Example: --mount /mydir=cm:myconfigmap, --mount /mydir=secret:mysecret, or --mount /mydir=myvolume. "+

pkg/kn/flags/podspec_test.go

@@ -54,6 +54,7 @@ func TestPodSpecFlags(t *testing.T) {
 	}
 	testCmd.SetArgs(args)
 	flags.AddFlags(testCmd.Flags())
+	flags.AddCreateFlags(testCmd.Flags())
 	testCmd.Execute()
 }
 
@@ -130,6 +131,7 @@ func TestPodSpecResolve(t *testing.T) {
 	}
 	testCmd.SetArgs(inputArgs)
 	flags.AddFlags(testCmd.Flags())
+	flags.AddUpdateFlags(testCmd.Flags())
 	testCmd.Execute()
 }
 
@@ -251,6 +253,7 @@ containers:
 			}
 			testCmd.SetArgs(inputArgs)
 			flags.AddFlags(testCmd.Flags())
+			flags.AddUpdateFlags(testCmd.Flags())
 			testCmd.Execute()
 		})
 	}
@@ -282,6 +285,7 @@ func TestPodSpecResolveReturnError(t *testing.T) {
 
 	testCmd.SetArgs(inputArgs)
 	flags.AddFlags(testCmd.Flags())
+	flags.AddCreateFlags(testCmd.Flags())
 	testCmd.Execute()
 	out := outBuf.String()
 	assert.Assert(t, util.ContainsAll(out, "Invalid", "mount"))
@@ -326,5 +330,6 @@ func TestPodSpecResolveWithEnvFile(t *testing.T) {
 	}
 	testCmd.SetArgs(inputArgs)
 	flags.AddFlags(testCmd.Flags())
+	flags.AddUpdateFlags(testCmd.Flags())
 	testCmd.Execute()
 }