Repository: helm/helm
Pull request #7204 (2019-12-10 23:14:40)

fix(template): helm template "--show-only" flag producing duplicates when flag used more than once
## Issue
https://github.com/helm/helm/issues/7203

## Summary
---
* While migrating from helm2 to helm3, we began switching `--execute` parameters to the new `--show-only` parameter that was listed as being equivalent. 
* We found our `helm template <name> <chart> --show-only template1.yaml --show-only template2.yaml --values values.yaml | kubectl replace --force -f -` commands failing due to "Resource already exists" errors. 
* Using grep, we were able to identify our issue, duplicated outputs when multiple --show-only were used.
```
$ helm template myapp myapp --show-only templates/job.yaml --values values.yaml | grep Source 
# Source: app/templates/job.yaml

$ helm template myapp myapp --show-only templates/configmap.yaml --values values.yaml | grep Source 
# Source: myapp/templates/configmap.yaml

$ helm template myapp myapp --show-only templates/configmap.yaml --show-only templates/job.yaml --values values.yaml  | grep Source   
# Source: myapp/templates/configmap.yaml
# Source: myapp/templates/configmap.yaml
# Source: myapp/templates/job.yaml
```

## Solution
---
* Identified an incorrect looping logic. 
* Tested using `make` and `./bin/helm` 
```
$ helm template myapp myapp --show-only templates/configmap.yaml --values values.yaml | grep Source 
# Source: myapp/templates/configmap.yaml

$ helm template myapp myapp --show-only templates/configmap.yaml --show-only templates/job.yaml --values values.yaml  | grep Source   
# Source: myapp/templates/configmap.yaml
# Source: myapp/templates/migrations-job.yaml
```

bacongobbler: Thanks! Would you mind writing some unit tests to cover this? Have a look at [template_test.go](https://github.com/helm/helm/blob/master/cmd/helm/template_test.go) for some examples. Let me know if you have any questions.
bacongobbler: testdata output can be generated by running

```
$ cd cmd/helm # this is important; seems to only work when in the cmd/helm directory
$ go test -update
```

Run this once the unit test is created, and the testdata output will be generated for you.
hoboken515: I want to create the tests for this, may take me awhile to find the time / get up to speed on how to do so in the code base. Thanks for the inputs on how to do that @bacongobbler 
hoboken515: @bacongobbler @hickeyma unittests added
hickeyma: @hoboken515 Can you rebase to fix the conflicts?

Patch:
diff --git a/cmd/helm/template.go b/cmd/helm/template.go
index 1c34d7245..dc62c6e95 100644
--- a/cmd/helm/template.go
+++ b/cmd/helm/template.go
@@ -112,9 +112,9 @@ func newTemplateCmd(cfg *action.Configuration, out io.Writer) *cobra.Command {
 					if missing {
 						return fmt.Errorf("could not find template %s in chart", f)
 					}
-					for _, m := range manifestsToRender {
-						fmt.Fprintf(out, "---\n%s\n", m)
-					}
+				}
+				for _, m := range manifestsToRender {
+					fmt.Fprintf(out, "---\n%s\n", m)
 				}
 			} else {
 				fmt.Fprintf(out, "%s", manifests.String())
diff --git a/go.mod b/go.mod
index c7b25ac13..626df86bb 100644
--- a/go.mod
+++ b/go.mod
@@ -49,6 +49,7 @@ require (
 	golang.org/x/net v0.0.0-20191028085509-fe3aa8a45271 // indirect
 	golang.org/x/sys v0.0.0-20191028164358-195ce5e7f934 // indirect
 	golang.org/x/time v0.0.0-20191024005414-555d28b269f0 // indirect
+	golang.org/x/tools v0.0.0-20191218225520-84f0c7cf60ea // indirect
 	google.golang.org/appengine v1.6.5 // indirect
 	google.golang.org/genproto v0.0.0-20191028173616-919d9bdd9fe6 // indirect
 	google.golang.org/grpc v1.24.0 // indirect
diff --git a/go.sum b/go.sum
index 312152e1f..914ccd116 100644
--- a/go.sum
+++ b/go.sum
@@ -520,6 +520,7 @@ golang.org/x/crypto v0.0.0-20190617133340-57b3e21c3d56/go.mod h1:yigFU9vqHzYiE8U
 golang.org/x/crypto v0.0.0-20190820162420-60c769a6c586/go.mod h1:yigFU9vqHzYiE8UmvKecakEJjdnWj3jj499lnFckfCI=
 golang.org/x/crypto v0.0.0-20190911031432-227b76d455e7 h1:0hQKqeLdqlt5iIwVOBErRisrHJAN57yOiPRQItI20fU=
 golang.org/x/crypto v0.0.0-20190911031432-227b76d455e7/go.mod h1:yigFU9vqHzYiE8UmvKecakEJjdnWj3jj499lnFckfCI=
+golang.org/x/crypto v0.0.0-20191011191535-87dc89f01550/go.mod h1:yigFU9vqHzYiE8UmvKecakEJjdnWj3jj499lnFckfCI=
 golang.org/x/crypto v0.0.0-20191028145041-f83a4685e152 h1:ZC1Xn5A1nlpSmQCIva4bZ3ob3lmhYIefc+GU+DLg1Ow=
 golang.org/x/crypto v0.0.0-20191028145041-f83a4685e152/go.mod h1:LzIPMQfyMNhhGPhUkYOs5KpL4U8rLKemX1yGLhDgUto=
 golang.org/x/exp v0.0.0-20190121172915-509febef88a4/go.mod h1:CJ0aWSM057203Lf6IL+f9T1iT9GByDxfZKAQTCR3kQA=
@@ -532,6 +533,7 @@ golang.org/x/lint v0.0.0-20190227174305-5b3e6a55c961/go.mod h1:wehouNa3lNwaWXcvx
 golang.org/x/lint v0.0.0-20190301231843-5614ed5bae6f/go.mod h1:UVdnD1Gm6xHRNCYTkRU2/jEulfH38KcIWyp/GAMgvoE=
 golang.org/x/lint v0.0.0-20190313153728-d0100b6bd8b3/go.mod h1:6SW0HCj/g11FgYtHlgUYUwCkIfeOF89ocIRzGO/8vkc=
 golang.org/x/mobile v0.0.0-20190312151609-d3739f865fa6/go.mod h1:z+o9i4GpDbdi3rU15maQ/Ox0txvL9dWGYEHz965HBQE=
+golang.org/x/mod v0.1.1-0.20191105210325-c90efee705ee/go.mod h1:QqPTAvyqsEbceGzBzNggFXnrqF1CaUcvgkdR5Ot7KZg=
 golang.org/x/net v0.0.0-20170114055629-f2499483f923/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4=
 golang.org/x/net v0.0.0-20180724234803-3673e40ba225/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4=
 golang.org/x/net v0.0.0-20180826012351-8a410e7b638d/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4=
@@ -549,6 +551,8 @@ golang.org/x/net v0.0.0-20190404232315-eb5bcb51f2a3/go.mod h1:t9HGtf8HONx5eT2rtn
 golang.org/x/net v0.0.0-20190603091049-60506f45cf65/go.mod h1:HSz+uSET+XFnRR8LxR5pz3Of3rY3CfYBVs4xY44aLks=
 golang.org/x/net v0.0.0-20190613194153-d28f0bde5980/go.mod h1:z5CRVTTTmAJ677TzLLGU+0bjPO0LkuOLi4/5GtJWs/s=
 golang.org/x/net v0.0.0-20190620200207-3b0461eec859/go.mod h1:z5CRVTTTmAJ677TzLLGU+0bjPO0LkuOLi4/5GtJWs/s=
+golang.org/x/net v0.0.0-20190812203447-cdfb69ac37fc h1:gkKoSkUmnU6bpS/VhkuO27bzQeSA51uaEfbOW5dNb68=
+golang.org/x/net v0.0.0-20190812203447-cdfb69ac37fc/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4=
 golang.org/x/net v0.0.0-20190813141303-74dc4d7220e7/go.mod h1:z5CRVTTTmAJ677TzLLGU+0bjPO0LkuOLi4/5GtJWs/s=
 golang.org/x/net v0.0.0-20190827160401-ba9fcec4b297/go.mod h1:z5CRVTTTmAJ677TzLLGU+0bjPO0LkuOLi4/5GtJWs/s=
 golang.org/x/net v0.0.0-20191004110552-13f9640d40b9/go.mod h1:z5CRVTTTmAJ677TzLLGU+0bjPO0LkuOLi4/5GtJWs/s=
@@ -609,6 +613,11 @@ golang.org/x/tools v0.0.0-20190312151545-0bb0c0a6e846/go.mod h1:LCzVGOaR6xXOjkQ3
 golang.org/x/tools v0.0.0-20190312170243-e65039ee4138/go.mod h1:LCzVGOaR6xXOjkQ3onu1FJEFr0SW1gC7cKk1uF8kGRs=
 golang.org/x/tools v0.0.0-20190524140312-2c0ae7006135/go.mod h1:RgjU9mgBXZiqYHBnxXauZ1Gv1EHHAz9KjViQ78xBX0Q=
 golang.org/x/tools v0.0.0-20190614205625-5aca471b1d59/go.mod h1:/rFqwRUd4F7ZHNgwSSTFct+R/Kf4OFW1sUzUTQQTgfc=
+golang.org/x/tools v0.0.0-20190621195816-6e04913cbbac h1:MQEvx39qSf8vyrx3XRaOe+j1UDIzKwkYOVObRgGPVqI=
+golang.org/x/tools v0.0.0-20190621195816-6e04913cbbac/go.mod h1:/rFqwRUd4F7ZHNgwSSTFct+R/Kf4OFW1sUzUTQQTgfc=
+golang.org/x/tools v0.0.0-20191218225520-84f0c7cf60ea h1:mtRJM/ln5qwEigajtnZtuARALEPOooGf5lwkM5a9tt4=
+golang.org/x/tools v0.0.0-20191218225520-84f0c7cf60ea/go.mod h1:TB2adYChydJhpapKDTa4BR/hXlZSLoq2Wpct/0txZ28=
+golang.org/x/xerrors v0.0.0-20191011141410-1b5146add898/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=
 golang.org/x/tools v0.0.0-20190617190820-da514acc4774/go.mod h1:/rFqwRUd4F7ZHNgwSSTFct+R/Kf4OFW1sUzUTQQTgfc=
 golang.org/x/tools v0.0.0-20190920225731-5eefd052ad72/go.mod h1:b+2E5dAYhXwXZwtnZ6UAqBI28+e2cm9otk0dWdXHAEo=
 golang.org/x/xerrors v0.0.0-20190717185122-a985d3407aa7/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=
diff --git a/pkg/action/list.go b/pkg/action/list.go
index 5d3417203..5be60ac42 100644
--- a/pkg/action/list.go
+++ b/pkg/action/list.go
@@ -26,7 +26,7 @@ import (
 
 // ListStates represents zero or more status codes that a list item may have set
 //
-// Because this is used as a bitmask filter, more than one one bit can be flipped
+// Because this is used as a bitmask filter, more than one bit can be flipped
 // in the ListStates.
 type ListStates uint
 
diff --git a/pkg/action/upgrade.go b/pkg/action/upgrade.go
index cdc40eaaa..1db4184ff 100644
--- a/pkg/action/upgrade.go
+++ b/pkg/action/upgrade.go
@@ -291,7 +291,7 @@ func (u *Upgrade) failRelease(rel *release.Release, created kube.ResourceList, e
 	rel.Info.Status = release.StatusFailed
 	rel.Info.Description = msg
 	u.cfg.recordRelease(rel)
-	if u.CleanupOnFail {
+	if u.CleanupOnFail && len(created) > 0 {
 		u.cfg.Log("Cleanup on fail set, cleaning up %d resources", len(created))
 		_, errs := u.cfg.KubeClient.Delete(created)
 		if errs != nil {
diff --git a/pkg/getter/getter.go b/pkg/getter/getter.go
index e11dbfcae..68638c2ca 100644
--- a/pkg/getter/getter.go
+++ b/pkg/getter/getter.go
@@ -64,7 +64,7 @@ func WithUserAgent(userAgent string) Option {
 	}
 }
 
-// WithTLSClientConfig sets the client client auth with the provided credentials.
+// WithTLSClientConfig sets the client auth with the provided credentials.
 func WithTLSClientConfig(certFile, keyFile, caFile string) Option {
 	return func(opts *options) {
 		opts.certFile = certFile
diff --git a/pkg/kube/client.go b/pkg/kube/client.go
index adf195e34..3853e9036 100644
--- a/pkg/kube/client.go
+++ b/pkg/kube/client.go
@@ -21,7 +21,6 @@ import (
 	"encoding/json"
 	"fmt"
 	"io"
-	"log"
 	"strings"
 	"sync"
 	"time"
@@ -421,7 +420,7 @@ func updateResource(c *Client, target *resource.Info, currentObj runtime.Object,
 		if err != nil {
 			return errors.Wrap(err, "failed to replace object")
 		}
-		log.Printf("Replaced %q with kind %s for kind %s\n", target.Name, currentObj.GetObjectKind().GroupVersionKind().Kind, kind)
+		c.Log("Replaced %q with kind %s for kind %s\n", target.Name, currentObj.GetObjectKind().GroupVersionKind().Kind, kind)
 	} else {
 		// send patch to server
 		obj, err = helper.Patch(target.Namespace, target.Name, patchType, patch, nil)


Test patch:
diff --git a/cmd/helm/template_test.go b/cmd/helm/template_test.go
index 35a8e996b..dc7987d01 100644
--- a/cmd/helm/template_test.go
+++ b/cmd/helm/template_test.go
@@ -84,6 +84,16 @@ func TestTemplateCmd(t *testing.T) {
 			cmd:    fmt.Sprintf("template '%s' --include-crds", chartPath),
 			golden: "output/template-with-crds.txt",
 		},
+		{
+			name:   "template with show-only one",
+			cmd:    fmt.Sprintf("template '%s' --show-only templates/service.yaml", chartPath),
+			golden: "output/template-show-only-one.txt",
+		},
+		{
+			name:   "template with show-only multiple",
+			cmd:    fmt.Sprintf("template '%s' --show-only templates/service.yaml --show-only charts/subcharta/templates/service.yaml", chartPath),
+			golden: "output/template-show-only-multiple.txt",
+		},
 		{
 			name:   "sorted output of manifests (order of filenames, then order of objects within each YAML file)",
 			cmd:    fmt.Sprintf("template '%s'", "testdata/testcharts/object-order"),
diff --git a/cmd/helm/testdata/output/template-show-only-multiple.txt b/cmd/helm/testdata/output/template-show-only-multiple.txt
new file mode 100644
index 000000000..abb9a2e10
--- /dev/null
+++ b/cmd/helm/testdata/output/template-show-only-multiple.txt
@@ -0,0 +1,39 @@
+---
+# Source: subchart1/templates/service.yaml
+apiVersion: v1
+kind: Service
+metadata:
+  name: subchart1
+  labels:
+    helm.sh/chart: "subchart1-0.1.0"
+    app.kubernetes.io/instance: "RELEASE-NAME"
+    kube-version/major: "1"
+    kube-version/minor: "16"
+    kube-version/version: "v1.16.0"
+    kube-api-version/test: v1
+spec:
+  type: ClusterIP
+  ports:
+  - port: 80
+    targetPort: 80
+    protocol: TCP
+    name: nginx
+  selector:
+    app.kubernetes.io/name: subchart1
+---
+# Source: subchart1/charts/subcharta/templates/service.yaml
+apiVersion: v1
+kind: Service
+metadata:
+  name: subcharta
+  labels:
+    helm.sh/chart: "subcharta-0.1.0"
+spec:
+  type: ClusterIP
+  ports:
+  - port: 80
+    targetPort: 80
+    protocol: TCP
+    name: apache
+  selector:
+    app.kubernetes.io/name: subcharta
diff --git a/cmd/helm/testdata/output/template-show-only-one.txt b/cmd/helm/testdata/output/template-show-only-one.txt
new file mode 100644
index 000000000..f0dd0834e
--- /dev/null
+++ b/cmd/helm/testdata/output/template-show-only-one.txt
@@ -0,0 +1,22 @@
+---
+# Source: subchart1/templates/service.yaml
+apiVersion: v1
+kind: Service
+metadata:
+  name: subchart1
+  labels:
+    helm.sh/chart: "subchart1-0.1.0"
+    app.kubernetes.io/instance: "RELEASE-NAME"
+    kube-version/major: "1"
+    kube-version/minor: "16"
+    kube-version/version: "v1.16.0"
+    kube-api-version/test: v1
+spec:
+  type: ClusterIP
+  ports:
+  - port: 80
+    targetPort: 80
+    protocol: TCP
+    name: nginx
+  selector:
+    app.kubernetes.io/name: subchart1
diff --git a/pkg/action/upgrade_test.go b/pkg/action/upgrade_test.go
index e7bfeefc5..f25d115c4 100644
--- a/pkg/action/upgrade_test.go
+++ b/pkg/action/upgrade_test.go
@@ -60,6 +60,31 @@ func TestUpgradeRelease_Wait(t *testing.T) {
 	is.Equal(res.Info.Status, release.StatusFailed)
 }
 
+func TestUpgradeRelease_CleanupOnFail(t *testing.T) {
+	is := assert.New(t)
+	req := require.New(t)
+
+	upAction := upgradeAction(t)
+	rel := releaseStub()
+	rel.Name = "come-fail-away"
+	rel.Info.Status = release.StatusDeployed
+	upAction.cfg.Releases.Create(rel)
+
+	failer := upAction.cfg.KubeClient.(*kubefake.FailingKubeClient)
+	failer.WaitError = fmt.Errorf("I timed out")
+	failer.DeleteError = fmt.Errorf("I tried to delete nil")
+	upAction.cfg.KubeClient = failer
+	upAction.Wait = true
+	upAction.CleanupOnFail = true
+	vals := map[string]interface{}{}
+
+	res, err := upAction.Run(rel.Name, buildChart(), vals)
+	req.Error(err)
+	is.NotContains(err.Error(), "unable to cleanup resources")
+	is.Contains(res.Info.Description, "I timed out")
+	is.Equal(res.Info.Status, release.StatusFailed)
+}
+
 func TestUpgradeRelease_Atomic(t *testing.T) {
 	is := assert.New(t)
 	req := require.New(t)