Repository: knative-sandbox/kn-plugin-func
Pull request #736 (2022-01-06 14:38:41)

feat: add possibility to disable pushing of image in `deploy` command
Signed-off-by: Zbynek Roubalik <zroubali@redhat.com>

<!-- Thanks for sending a pull request! -->

This is needed to have standalone `deploy` Tekton task.

Basically implementing the same functionality that has been done in https://github.com/knative-sandbox/kn-plugin-func/pull/681 for `build`.

# Changes
- :gift: add possibility to disable pushing of image in `deploy` command

/kind enhancement

Relates: #620 

knative-prow-robot: [APPROVALNOTIFIER] This PR is **APPROVED**

This pull-request has been approved by: *<a href="https://github.com/knative-sandbox/kn-plugin-func/pull/736#" title="Author self-approved">zroubalik</a>*

The full list of commands accepted by this bot can be found [here](https://go.k8s.io/bot-commands?repo=knative-sandbox%2Fkn-plugin-func).

The pull request process is described [here](https://git.k8s.io/community/contributors/guide/owners.md#the-code-review-process)

<details >
Needs approval from an approver in each of these files:

- ~~[OWNERS](https://github.com/knative-sandbox/kn-plugin-func/blob/main/OWNERS)~~ [zroubalik]

Approvers can indicate their approval by writing `/approve` in a comment
Approvers can cancel approval by writing `/approve cancel` in a comment
</details>
<!-- META={"approvers":[]} -->
codecov[bot]: # [Codecov](https://codecov.io/gh/knative-sandbox/kn-plugin-func/pull/736?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=knative-sandbox) Report
> Merging [#736](https://codecov.io/gh/knative-sandbox/kn-plugin-func/pull/736?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=knative-sandbox) (1b5ccf2) into [main](https://codecov.io/gh/knative-sandbox/kn-plugin-func/commit/3177287571d0d304e16b27d14c416a67e410d500?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=knative-sandbox) (3177287) will **increase** coverage by `0.00%`.
> The diff coverage is `28.57%`.

[![Impacted file tree graph](https://codecov.io/gh/knative-sandbox/kn-plugin-func/pull/736/graphs/tree.svg?width=650&height=150&src=pr&token=da1B9bMsQ9&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=knative-sandbox)](https://codecov.io/gh/knative-sandbox/kn-plugin-func/pull/736?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=knative-sandbox)

```diff
@@           Coverage Diff           @@
##             main     #736   +/-   ##
=======================================
  Coverage   41.56%   41.56%           
=======================================
  Files          42       42           
  Lines        4083     4102   +19     
=======================================
+ Hits         1697     1705    +8     
- Misses       2135     2144    +9     
- Partials      251      253    +2     
```


| [Impacted Files](https://codecov.io/gh/knative-sandbox/kn-plugin-func/pull/736?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=knative-sandbox) | Coverage Δ | |
|---|---|---|
| [cmd/deploy.go](https://codecov.io/gh/knative-sandbox/kn-plugin-func/pull/736/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=knative-sandbox#diff-Y21kL2RlcGxveS5nbw==) | `16.03% <5.00%> (-0.14%)` | :arrow_down: |
| [client.go](https://codecov.io/gh/knative-sandbox/kn-plugin-func/pull/736/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=knative-sandbox#diff-Y2xpZW50Lmdv) | `65.02% <57.14%> (-0.07%)` | :arrow_down: |
| [cmd/build.go](https://codecov.io/gh/knative-sandbox/kn-plugin-func/pull/736/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=knative-sandbox#diff-Y21kL2J1aWxkLmdv) | `58.08% <100.00%> (ø)` | |

------

[Continue to review full report at Codecov](https://codecov.io/gh/knative-sandbox/kn-plugin-func/pull/736?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=knative-sandbox).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=knative-sandbox)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/knative-sandbox/kn-plugin-func/pull/736?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=knative-sandbox). Last update [3177287...1b5ccf2](https://codecov.io/gh/knative-sandbox/kn-plugin-func/pull/736?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=knative-sandbox). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=knative-sandbox).

knative-prow-robot: [APPROVALNOTIFIER] This PR is **APPROVED**

This pull-request has been approved by: *<a href="https://github.com/knative-sandbox/kn-plugin-func/pull/736#pullrequestreview-845737257" title="LGTM">salaboy</a>*, *<a href="https://github.com/knative-sandbox/kn-plugin-func/pull/736#" title="Author self-approved">zroubalik</a>*

The full list of commands accepted by this bot can be found [here](https://go.k8s.io/bot-commands?repo=knative-sandbox%2Fkn-plugin-func).

The pull request process is described [here](https://git.k8s.io/community/contributors/guide/owners.md#the-code-review-process)

<details >
Needs approval from an approver in each of these files:

- ~~[OWNERS](https://github.com/knative-sandbox/kn-plugin-func/blob/main/OWNERS)~~ [zroubalik]

Approvers can indicate their approval by writing `/approve` in a comment
Approvers can cancel approval by writing `/approve cancel` in a comment
</details>
<!-- META={"approvers":[]} -->

Patch:
diff --git a/client.go b/client.go
index 10188eec..1575dce7 100644
--- a/client.go
+++ b/client.go
@@ -424,6 +424,12 @@ func (c *Client) New(ctx context.Context, cfg Function) (err error) {
 		return
 	}
 
+	// Push the produced function image
+	c.progressListener.Increment("Pushing container image to registry")
+	if err = c.Push(ctx, f.Root); err != nil {
+		return
+	}
+
 	// Deploy the initialized Function, returning its publicly
 	// addressible name for possible registration.
 	c.progressListener.Increment("Deploying Function to cluster")
@@ -602,10 +608,6 @@ func (c *Client) Deploy(ctx context.Context, path string) (err error) {
 		return ErrNotBuilt
 	}
 
-	if err = c.Push(ctx, &f); err != nil {
-		return
-	}
-
 	// Deploy a new or Update the previously-deployed Function
 	c.progressListener.Increment("Deploying function to the cluster")
 	result, err := c.deployer.Deploy(ctx, f)
@@ -717,8 +719,23 @@ func (c *Client) Emit(ctx context.Context, endpoint string) error {
 }
 
 // Push the image for the named service to the configured registry
-func (c *Client) Push(ctx context.Context, f *Function) (err error) {
-	imageDigest, err := c.pusher.Push(ctx, *f)
+func (c *Client) Push(ctx context.Context, path string) (err error) {
+	go func() {
+		<-ctx.Done()
+		c.progressListener.Stopping()
+	}()
+
+	f, err := NewFunction(path)
+	if err != nil {
+		return
+	}
+
+	if !f.Built() {
+		return ErrNotBuilt
+	}
+
+	c.progressListener.Increment("Pushing function image to the registry")
+	imageDigest, err := c.pusher.Push(ctx, f)
 	if err != nil {
 		return
 	}
diff --git a/cmd/build.go b/cmd/build.go
index c61a7a75..16c897a5 100644
--- a/cmd/build.go
+++ b/cmd/build.go
@@ -51,7 +51,6 @@ func newBuildClient(cfg buildConfig) (*fn.Client, error) {
 
 	return fn.New(
 		fn.WithBuilder(builder),
-		// fn.WithPusher(pusher),
 		pusherOption,
 		fn.WithProgressListener(listener),
 		fn.WithRegistry(cfg.Registry), // for image name when --image not provided
@@ -201,7 +200,7 @@ func runBuild(cmd *cobra.Command, _ []string, clientFn buildClientFn) (err error
 
 	err = client.Build(cmd.Context(), config.Path)
 	if err == nil && config.Push {
-		err = client.Push(cmd.Context(), &function)
+		err = client.Push(cmd.Context(), config.Path)
 	}
 	return
 }
diff --git a/cmd/deploy.go b/cmd/deploy.go
index 6fbb71c2..54495ecb 100644
--- a/cmd/deploy.go
+++ b/cmd/deploy.go
@@ -27,19 +27,23 @@ func init() {
 
 func newDeployClient(cfg deployConfig) (*fn.Client, error) {
 	listener := progress.New()
-
 	builder := buildpacks.NewBuilder()
 
-	credentialsProvider := creds.NewCredentialsProvider(
-		creds.WithPromptForCredentials(newPromptForCredentials()),
-		creds.WithPromptForCredentialStore(newPromptForCredentialStore()),
-		creds.WithTransport(cfg.Transport))
-	pusher, err := docker.NewPusher(
-		docker.WithCredentialsProvider(credentialsProvider),
-		docker.WithProgressListener(listener),
-		docker.WithTransport(cfg.Transport))
-	if err != nil {
-		return nil, err
+	pusherOption := fn.WithPusher(nil)
+	if cfg.Push {
+		credentialsProvider := creds.NewCredentialsProvider(
+			creds.WithPromptForCredentials(newPromptForCredentials()),
+			creds.WithPromptForCredentialStore(newPromptForCredentialStore()),
+			creds.WithTransport(cfg.Transport))
+		pusher, err := docker.NewPusher(
+			docker.WithCredentialsProvider(credentialsProvider),
+			docker.WithProgressListener(listener),
+			docker.WithTransport(cfg.Transport))
+		if err != nil {
+			return nil, err
+		}
+		pusher.Verbose = cfg.Verbose
+		pusherOption = fn.WithPusher(pusher)
 	}
 
 	deployer, err := knative.NewDeployer(cfg.Namespace)
@@ -49,13 +53,12 @@ func newDeployClient(cfg deployConfig) (*fn.Client, error) {
 
 	listener.Verbose = cfg.Verbose
 	builder.Verbose = cfg.Verbose
-	pusher.Verbose = cfg.Verbose
 	deployer.Verbose = cfg.Verbose
 
 	return fn.New(
 		fn.WithProgressListener(listener),
 		fn.WithBuilder(builder),
-		fn.WithPusher(pusher),
+		pusherOption,
 		fn.WithDeployer(deployer),
 		fn.WithRegistry(cfg.Registry), // for deriving name when no --image value
 		fn.WithVerbose(cfg.Verbose),
@@ -100,6 +103,7 @@ kn func deploy --image quay.io/myuser/myfunc -n myns
 	cmd.Flags().StringP("image", "i", "", "Full image name in the form [registry]/[namespace]/[name]:[tag] (optional). This option takes precedence over --registry (Env: $FUNC_IMAGE)")
 	cmd.Flags().StringP("registry", "r", "", "Registry + namespace part of the image to build, ex 'quay.io/myuser'.  The full image name is automatically determined based on the local directory name. If not provided the registry will be taken from func.yaml (Env: $FUNC_REGISTRY)")
 	cmd.Flags().BoolP("build", "b", true, "Build the image before deploying (Env: $FUNC_BUILD)")
+	cmd.Flags().BoolP("push", "u", true, "Attempt to push the function image to registry before deploying.")
 	setPathFlag(cmd)
 	setNamespaceFlag(cmd)
 
@@ -198,6 +202,12 @@ func runDeploy(cmd *cobra.Command, _ []string, clientFn deployClientFn) (err err
 		}
 	}
 
+	if config.Push {
+		if err := client.Push(cmd.Context(), config.Path); err != nil {
+			return err
+		}
+	}
+
 	return client.Deploy(cmd.Context(), config.Path)
 
 	// NOTE: Namespace is optional, default is that used by k8s client
@@ -289,6 +299,9 @@ type deployConfig struct {
 	// Build the associated Function before deploying.
 	Build bool
 
+	// Push function image to the registry before deploying.
+	Push bool
+
 	// Envs passed via cmd to be added/updated
 	EnvToUpdate *util.OrderedMap
 
@@ -313,6 +326,7 @@ func newDeployConfig(cmd *cobra.Command) (deployConfig, error) {
 		Verbose:     viper.GetBool("verbose"), // defined on root
 		Confirm:     viper.GetBool("confirm"),
 		Build:       viper.GetBool("build"),
+		Push:        viper.GetBool("push"),
 		EnvToUpdate: envToUpdate,
 		EnvToRemove: envToRemove,
 	}, nil


Test patch:
diff --git a/cmd/build_test.go b/cmd/build_test.go
index 56b733dc..00d9be66 100644
--- a/cmd/build_test.go
+++ b/cmd/build_test.go
@@ -144,31 +144,58 @@ created: 2009-11-10 23:00:00`,
 
 func Test_newBuildClient(t *testing.T) {
 	tests := []struct {
-		name    string
-		cfg     buildConfig
-		succeed bool
+		name         string
+		cfg          buildConfig
+		fileContents string
+		succeed      bool
 	}{
 		{
 			name: "push flag set to false avoids pusher instanciation",
 			cfg: buildConfig{
 				Push: false,
 			},
+			fileContents: `name: test-func
+runtime: go
+image: registry.io/foo/bar:latest
+imageDigest: sha256:1111111111111111111111
+created: 2009-11-10 23:00:00`,
 			succeed: false,
 		},
 	}
 
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
+
+			tempDir, err := os.MkdirTemp("", "func-tests")
+			if err != nil {
+				t.Fatalf("temp dir couldn't be created %v", err)
+			}
+			//	t.Log("tempDir created:", tempDir)
+			t.Cleanup(func() {
+				os.RemoveAll(tempDir)
+			})
+
+			fullPath := tempDir + "/func.yaml"
+			tempFile, err := os.Create(fullPath)
+			if err != nil {
+				t.Fatalf("temp file couldn't be created %v", err)
+			}
+			_, err = tempFile.WriteString(tt.fileContents)
+			if err != nil {
+				t.Fatalf("file content was not written %v", err)
+			}
+
 			client, err := newBuildClient(tt.cfg)
 			if err != nil {
 				t.Error(err)
 			}
+
 			defer func() {
 				if r := recover(); r != nil && tt.succeed {
 					t.Errorf("expected function call to succeed %v, got actually %v", tt.succeed, r)
 				}
 			}()
-			err = client.Push(context.TODO(), &fn.Function{})
+			err = client.Push(context.TODO(), tempDir)
 			if err != nil {
 				t.Error(err)
 			}