Repository: knative/client (go)

Issue:
Add support for cmd and args when creating a new service
## In what area(s)?
Classifications:
/kind good-first-issue
/kind feature
/kind proposal

## Describe the feature:

Knative, via yaml, supports specifying the cmd and args in the podSpec - kn should support this too since there are times when people may need to specify which exe in the image to run or what args to pass to the exe. Sometimes the default values in the image are not sufficient.

Pull request:
Add support for command and args in service create
<!--
Request Prow to automatically lint any go code in this PR:

/lint
-->

Fixes #604 

## Proposed Changes

* Add new flags `--cmd` and `--arg` 
* `--cmd` accepts a single `string` input
* `--arg` can be used multiple times to provide as many as neeed
* Examples:
  * --cmd /app/start
  * --cmd /app/start --arg myArg
  * --arg myArg1 --arg --myArg2 --arg myArg3=3.

Even though `podSpec.Command` accepts `stringArray` and arguments can be passed directly to this field. Per discussion on #604  I've decided to follow more verbose and strict approach to make a clear distinction to improve usability and user-exp.

/assign @rhuss 
/assign @navidshaikh 

<!--
Release Note:

In the following cases, add a short description of PR to the unreleased section in CHANGELOG.adoc:

- 🎁 Add new feature
- 🐛 Fix bug
- 🧽 Update or clean up current behaviour
- 🗑️ Remove feature or internal logic

See other entries in CHANGELOG.adoc as an example.

PLEASE DON'T ADD THAT LINE HERE IN THE PULL-REQUEST DESCRIPTION BUT DIRECTLY IN CHANGELOG.ADOC AND ADD CHANGELOG.ADOC AS PART OF YOUR PULL-REQUEST.
-->


Patch:
diff --git a/CHANGELOG.adoc b/CHANGELOG.adoc
index d35bc71b..198f6510 100644
--- a/CHANGELOG.adoc
+++ b/CHANGELOG.adoc
@@ -24,6 +24,10 @@
 | Show envFrom when running describe service or revision
 | https://github.com/knative/client/pull/630
 
+| 🎁
+| Add `--cmd` and `--arg` for customization of container entrypoint
+| https://github.com/knative/client/pull/635[#635]
+
 ## v0.12.0 (2020-01-29)
 
 [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 ed8502d4..891f8c84 100644
--- a/docs/cmd/kn_service_create.md
+++ b/docs/cmd/kn_service_create.md
@@ -43,8 +43,10 @@ kn service create NAME --image IMAGE [flags]
 
 ```
       --annotation 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-).
+      --arg stringArray           Add argument to the container command. Example: --arg myArg1 --arg --myArg2 --arg myArg3=3. You can use this flag multiple times.
       --async                     Create service and don't wait for it to become ready.
       --autoscale-window string   Duration to look back for making auto-scaling decisions. The service is scaled to zero if no request was received in during that time. (eg: 10s)
+      --cmd string                Specify command to be used as entrypoint instead of default one. Example: --cmd /app/start or --cmd /app/start --arg myArg to pass aditional arguments.
       --concurrency-limit int     Hard Limit of concurrent requests to be processed by a single replica.
       --concurrency-target int    Recommendation for when to scale up based on the concurrent number of incoming request. Defaults to --concurrency-limit when given.
   -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-).
diff --git a/docs/cmd/kn_service_update.md b/docs/cmd/kn_service_update.md
index 154d2dfc..2f333c43 100644
--- a/docs/cmd/kn_service_update.md
+++ b/docs/cmd/kn_service_update.md
@@ -39,8 +39,10 @@ kn service update NAME [flags]
 
 ```
       --annotation 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-).
+      --arg stringArray           Add argument to the container command. Example: --arg myArg1 --arg --myArg2 --arg myArg3=3. You can use this flag multiple times.
       --async                     Update service and don't wait for it to become ready.
       --autoscale-window string   Duration to look back for making auto-scaling decisions. The service is scaled to zero if no request was received in during that time. (eg: 10s)
+      --cmd string                Specify command to be used as entrypoint instead of default one. Example: --cmd /app/start or --cmd /app/start --arg myArg to pass aditional arguments.
       --concurrency-limit int     Hard Limit of concurrent requests to be processed by a single replica.
       --concurrency-target int    Recommendation for when to scale up based on the concurrent number of incoming request. Defaults to --concurrency-limit when given.
   -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-).
diff --git a/pkg/kn/commands/service/configuration_edit_flags.go b/pkg/kn/commands/service/configuration_edit_flags.go
index d3aa169d..a51d1317 100644
--- a/pkg/kn/commands/service/configuration_edit_flags.go
+++ b/pkg/kn/commands/service/configuration_edit_flags.go
@@ -36,6 +36,9 @@ type ConfigurationEditFlags struct {
 	Mount   []string
 	Volume  []string
 
+	Command string
+	Arg     []string
+
 	RequestsFlags, LimitsFlags ResourceFlags
 	MinScale                   int
 	MaxScale                   int
@@ -102,6 +105,16 @@ func (p *ConfigurationEditFlags) addSharedFlags(command *cobra.Command) {
 			"To unset a ConfigMap/Secret reference, append \"-\" to the name, e.g. --volume myvolume-.")
 	p.markFlagMakesRevision("volume")
 
+	command.Flags().StringVarP(&p.Command, "cmd", "", "",
+		"Specify command to be used as entrypoint instead of default one. "+
+			"Example: --cmd /app/start or --cmd /app/start --arg myArg to pass aditional arguments.")
+	p.markFlagMakesRevision("cmd")
+	command.Flags().StringArrayVarP(&p.Arg, "arg", "", []string{},
+		"Add argument to the container command. "+
+			"Example: --arg myArg1 --arg --myArg2 --arg myArg3=3. "+
+			"You can use this flag multiple times.")
+	p.markFlagMakesRevision("arg")
+
 	command.Flags().StringVar(&p.RequestsFlags.CPU, "requests-cpu", "", "The requested CPU (e.g., 250m).")
 	p.markFlagMakesRevision("requests-cpu")
 	command.Flags().StringVar(&p.RequestsFlags.Memory, "requests-memory", "", "The requested memory (e.g., 64Mi).")
@@ -281,6 +294,20 @@ func (p *ConfigurationEditFlags) Apply(
 		return err
 	}
 
+	if cmd.Flags().Changed("cmd") {
+		err = servinglib.UpdateContainerCommand(template, p.Command)
+		if err != nil {
+			return err
+		}
+	}
+
+	if cmd.Flags().Changed("arg") {
+		err = servinglib.UpdateContainerArg(template, p.Arg)
+		if err != nil {
+			return err
+		}
+	}
+
 	if cmd.Flags().Changed("port") {
 		err = servinglib.UpdateContainerPort(template, p.Port)
 		if err != nil {
diff --git a/pkg/serving/config_changes.go b/pkg/serving/config_changes.go
index 86b9a3a8..9504d3fb 100644
--- a/pkg/serving/config_changes.go
+++ b/pkg/serving/config_changes.go
@@ -314,6 +314,26 @@ func FreezeImageToDigest(template *servingv1alpha1.RevisionTemplateSpec, baseRev
 	return nil
 }
 
+// UpdateContainerCommand updates container with a given argument
+func UpdateContainerCommand(template *servingv1alpha1.RevisionTemplateSpec, command string) error {
+	container, err := ContainerOfRevisionTemplate(template)
+	if err != nil {
+		return err
+	}
+	container.Command = []string{command}
+	return nil
+}
+
+// UpdateContainerArg updates container with a given argument
+func UpdateContainerArg(template *servingv1alpha1.RevisionTemplateSpec, arg []string) error {
+	container, err := ContainerOfRevisionTemplate(template)
+	if err != nil {
+		return err
+	}
+	container.Args = arg
+	return nil
+}
+
 // UpdateContainerPort updates container with a give port
 func UpdateContainerPort(template *servingv1alpha1.RevisionTemplateSpec, port int32) error {
 	container, err := ContainerOfRevisionTemplate(template)


Test patch:
diff --git a/pkg/kn/commands/service/create_test.go b/pkg/kn/commands/service/create_test.go
index e9ad1557..12b905aa 100644
--- a/pkg/kn/commands/service/create_test.go
+++ b/pkg/kn/commands/service/create_test.go
@@ -21,6 +21,8 @@ import (
 	"strings"
 	"testing"
 
+	"gotest.tools/assert"
+
 	api_errors "k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/apimachinery/pkg/runtime/schema"
 	"k8s.io/apimachinery/pkg/watch"
@@ -156,6 +158,30 @@ func TestServiceCreateImageSync(t *testing.T) {
 	}
 }
 
+func TestServiceCreateCommand(t *testing.T) {
+	action, created, _, err := fakeServiceCreate([]string{
+		"service", "create", "foo", "--image", "gcr.io/foo/bar:baz", "--cmd", "/app/start", "--async"}, false, false)
+	assert.NilError(t, err)
+	assert.Assert(t, action.Matches("create", "services"))
+
+	template, err := servinglib.RevisionTemplateOfService(created)
+	assert.NilError(t, err)
+	assert.DeepEqual(t, template.Spec.Containers[0].Command, []string{"/app/start"})
+}
+
+func TestServiceCreateArg(t *testing.T) {
+	action, created, _, err := fakeServiceCreate([]string{
+		"service", "create", "foo", "--image", "gcr.io/foo/bar:baz", "--arg", "myArg1", "--arg", "--myArg2", "--arg", "--myArg3=3", "--async"}, false, false)
+	assert.NilError(t, err)
+	assert.Assert(t, action.Matches("create", "services"))
+
+	expectedArg := []string{"myArg1", "--myArg2", "--myArg3=3"}
+
+	template, err := servinglib.RevisionTemplateOfService(created)
+	assert.NilError(t, err)
+	assert.DeepEqual(t, template.Spec.Containers[0].Args, expectedArg)
+}
+
 func TestServiceCreateEnv(t *testing.T) {
 	action, created, _, err := fakeServiceCreate([]string{
 		"service", "create", "foo", "--image", "gcr.io/foo/bar:baz", "-e", "A=DOGS", "--env", "B=WOLVES", "--env=EMPTY", "--async"}, false, false)
diff --git a/pkg/kn/commands/service/update_test.go b/pkg/kn/commands/service/update_test.go
index 12afac63..9d5355e6 100644
--- a/pkg/kn/commands/service/update_test.go
+++ b/pkg/kn/commands/service/update_test.go
@@ -203,6 +203,44 @@ func TestServiceUpdateImage(t *testing.T) {
 	}
 }
 
+func TestServiceUpdateCommand(t *testing.T) {
+	orig := newEmptyService()
+
+	origTemplate, err := servinglib.RevisionTemplateOfService(orig)
+	assert.NilError(t, err)
+
+	err = servinglib.UpdateContainerCommand(origTemplate, "./start")
+	assert.NilError(t, err)
+
+	action, updated, _, err := fakeServiceUpdate(orig, []string{
+		"service", "update", "foo", "--cmd", "/app/start", "--async"}, false)
+	assert.NilError(t, err)
+	assert.Assert(t, action.Matches("update", "services"))
+
+	updatedTemplate, err := servinglib.RevisionTemplateOfService(updated)
+	assert.NilError(t, err)
+	assert.DeepEqual(t, updatedTemplate.Spec.Containers[0].Command, []string{"/app/start"})
+}
+
+func TestServiceUpdateArg(t *testing.T) {
+	orig := newEmptyService()
+
+	origTemplate, err := servinglib.RevisionTemplateOfService(orig)
+	assert.NilError(t, err)
+
+	err = servinglib.UpdateContainerArg(origTemplate, []string{"myArg0"})
+	assert.NilError(t, err)
+
+	action, updated, _, err := fakeServiceUpdate(orig, []string{
+		"service", "update", "foo", "--arg", "myArg1", "--arg", "--myArg2", "--arg", "--myArg3=3", "--async"}, false)
+	assert.NilError(t, err)
+	assert.Assert(t, action.Matches("update", "services"))
+
+	updatedTemplate, err := servinglib.RevisionTemplateOfService(updated)
+	assert.NilError(t, err)
+	assert.DeepEqual(t, updatedTemplate.Spec.Containers[0].Args, []string{"myArg1", "--myArg2", "--myArg3=3"})
+}
+
 func TestServiceUpdateRevisionNameExplicit(t *testing.T) {
 	orig := newEmptyServiceBetaAPIStyle()
 
diff --git a/pkg/serving/config_changes_test.go b/pkg/serving/config_changes_test.go
index 46fa666a..37150e81 100644
--- a/pkg/serving/config_changes_test.go
+++ b/pkg/serving/config_changes_test.go
@@ -295,6 +295,28 @@ func checkContainerImage(t *testing.T, template *servingv1alpha1.RevisionTemplat
 	}
 }
 
+func TestUpdateContainerCommand(t *testing.T) {
+	template, _ := getV1alpha1RevisionTemplateWithOldFields()
+	err := UpdateContainerCommand(template, "/app/start")
+	assert.NilError(t, err)
+	assert.DeepEqual(t, template.Spec.GetContainer().Command, []string{"/app/start"})
+
+	err = UpdateContainerCommand(template, "/app/latest")
+	assert.NilError(t, err)
+	assert.DeepEqual(t, template.Spec.GetContainer().Command, []string{"/app/latest"})
+}
+
+func TestUpdateContainerArg(t *testing.T) {
+	template, _ := getV1alpha1RevisionTemplateWithOldFields()
+	err := UpdateContainerArg(template, []string{"--myArg"})
+	assert.NilError(t, err)
+	assert.DeepEqual(t, template.Spec.GetContainer().Args, []string{"--myArg"})
+
+	err = UpdateContainerArg(template, []string{"myArg1", "--myArg2"})
+	assert.NilError(t, err)
+	assert.DeepEqual(t, template.Spec.GetContainer().Args, []string{"myArg1", "--myArg2"})
+}
+
 func TestUpdateContainerPort(t *testing.T) {
 	template, _ := getV1alpha1Config()
 	err := UpdateContainerPort(template, 8888)