diff --git a/CHANGELOG.adoc b/CHANGELOG.adoc index 0ec5a95d3b..4fbaeb8ea4 100644 --- a/CHANGELOG.adoc +++ b/CHANGELOG.adoc @@ -12,6 +12,18 @@ | https://github.com/knative/client/pull/[#] //// +## Unreleased + +[cols="1,10,3", options="header", width="100%"] +|=== +| | Description | PR + +| 🎁 +| Add support to combine service create --filename with other options +| https://github.com/knative/client/pull/937[#937] + +|=== + ## v0.16.0 (2020-07-14) [cols="1,10,3", options="header", width="100%"] diff --git a/docs/cmd/kn_service_create.md b/docs/cmd/kn_service_create.md index e358c7c033..b07a341095 100644 --- a/docs/cmd/kn_service_create.md +++ b/docs/cmd/kn_service_create.md @@ -64,7 +64,7 @@ kn service create NAME --image IMAGE --concurrency-utilization int Percentage of concurrent requests utilization before scaling up. (default 70) -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-). --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-. - -f, --filename string Create a service from file. + -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. diff --git a/pkg/kn/commands/service/configuration_edit_flags.go b/pkg/kn/commands/service/configuration_edit_flags.go index 6321e9a8ad..1cffcaf313 100644 --- a/pkg/kn/commands/service/configuration_edit_flags.go +++ b/pkg/kn/commands/service/configuration_edit_flags.go @@ -278,8 +278,11 @@ func (p *ConfigurationEditFlags) AddCreateFlags(command *cobra.Command) { p.addSharedFlags(command) 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.") + command.Flags().StringVarP(&p.Filename, "filename", "f", "", "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.") command.MarkFlagFilename("filename") + p.markFlagMakesRevision("filename") } // Apply mutates the given service according to the flags in the command. diff --git a/pkg/kn/commands/service/create.go b/pkg/kn/commands/service/create.go index a2095d8c13..a8a93db3f8 100644 --- a/pkg/kn/commands/service/create.go +++ b/pkg/kn/commands/service/create.go @@ -303,12 +303,11 @@ func constructServiceFromFile(cmd *cobra.Command, editFlags ConfigurationEditFla // Set namespace in case it's specified as --namespace service.ObjectMeta.Namespace = namespace - // We need to generate revision to have --force replace working - revName, err := servinglib.GenerateRevisionName(editFlags.RevisionName, &service) + // Apply options provided from cmdline + err = editFlags.Apply(&service, nil, cmd) if err != nil { return nil, err } - service.Spec.Template.Name = revName return &service, nil } diff --git a/pkg/kn/commands/service/create_test.go b/pkg/kn/commands/service/create_test.go index 575d38b0c2..12746bca80 100644 --- a/pkg/kn/commands/service/create_test.go +++ b/pkg/kn/commands/service/create_test.go @@ -973,3 +973,85 @@ func TestServiceCreateInvalidDataYAML(t *testing.T) { _, _, _, err = fakeServiceCreate([]string{"service", "create", "foo", "--filename", tempFile}, false) assert.Assert(t, util.ContainsAll(err.Error(), "found", "tab", "violates", "indentation")) } + +func TestServiceCreateFromYAMLWithOverride(t *testing.T) { + tempDir, err := ioutil.TempDir("", "kn-file") + defer os.RemoveAll(tempDir) + assert.NilError(t, err) + + tempFile := filepath.Join(tempDir, "service.yaml") + err = ioutil.WriteFile(tempFile, []byte(serviceYAML), os.FileMode(0666)) + assert.NilError(t, err) + // Merge env vars + expectedEnvVars := map[string]string{ + "TARGET": "Go Sample v1", + "FOO": "BAR"} + action, created, _, err := fakeServiceCreate([]string{ + "service", "create", "foo", "--filename", tempFile, "--env", "FOO=BAR"}, false) + assert.NilError(t, err) + assert.Assert(t, action.Matches("create", "services")) + assert.Equal(t, created.Name, "foo") + + actualEnvVar, err := servinglib.EnvToMap(created.Spec.Template.Spec.GetContainer().Env) + assert.NilError(t, err) + assert.DeepEqual(t, actualEnvVar, expectedEnvVars) + + // Override env vars + expectedEnvVars = map[string]string{ + "TARGET": "FOOBAR", + "FOO": "BAR"} + action, created, _, err = fakeServiceCreate([]string{ + "service", "create", "foo", "--filename", tempFile, "--env", "TARGET=FOOBAR", "--env", "FOO=BAR"}, false) + assert.NilError(t, err) + assert.Assert(t, action.Matches("create", "services")) + assert.Equal(t, created.Name, "foo") + + actualEnvVar, err = servinglib.EnvToMap(created.Spec.Template.Spec.GetContainer().Env) + assert.NilError(t, err) + assert.DeepEqual(t, actualEnvVar, expectedEnvVars) + + // Remove existing env vars + expectedEnvVars = map[string]string{ + "FOO": "BAR"} + action, created, _, err = fakeServiceCreate([]string{ + "service", "create", "foo", "--filename", tempFile, "--env", "TARGET-", "--env", "FOO=BAR"}, false) + assert.NilError(t, err) + assert.Assert(t, action.Matches("create", "services")) + assert.Equal(t, created.Name, "foo") + + actualEnvVar, err = servinglib.EnvToMap(created.Spec.Template.Spec.GetContainer().Env) + assert.NilError(t, err) + assert.DeepEqual(t, actualEnvVar, expectedEnvVars) + + // Multiple edit flags + expectedAnnotations := map[string]string{ + "foo": "bar"} + action, created, _, err = fakeServiceCreate([]string{"service", "create", "foo", "--filename", tempFile, + "--service-account", "foo", "--cmd", "/foo/bar", "-a", "foo=bar"}, false) + assert.NilError(t, err) + assert.Assert(t, action.Matches("create", "services")) + assert.Equal(t, created.Name, "foo") + assert.DeepEqual(t, created.Spec.Template.Spec.GetContainer().Command, []string{"/foo/bar"}) + assert.Equal(t, created.Spec.Template.Spec.ServiceAccountName, "foo") + assert.DeepEqual(t, created.ObjectMeta.Annotations, expectedAnnotations) +} + +func TestServiceCreateFromYAMLWithOverrideError(t *testing.T) { + tempDir, err := ioutil.TempDir("", "kn-file") + defer os.RemoveAll(tempDir) + assert.NilError(t, err) + + tempFile := filepath.Join(tempDir, "service.yaml") + err = ioutil.WriteFile(tempFile, []byte(serviceYAML), os.FileMode(0666)) + assert.NilError(t, err) + + _, _, _, err = fakeServiceCreate([]string{ + "service", "create", "foo", "--filename", tempFile, "--image", "foo/bar", "--image", "bar/foo"}, false) + assert.Assert(t, err != nil) + assert.Assert(t, util.ContainsAll(err.Error(), "\"--image\"", "invalid", "argument", "only", "once")) + + _, _, _, err = fakeServiceCreate([]string{ + "service", "create", "foo", "--filename", tempFile, "--scale", "-1"}, false) + assert.Assert(t, err != nil) + assert.Assert(t, util.ContainsAll(err.Error(), "expected", "0", "<=", "2147483647", "autoscaling.knative.dev/maxScale")) +}