Repository: helm/helm
Pull request #7455 (2020-01-22 17:04:08)

`helm create` generates more beautiful templates
Hi! I'm a big fan of helm!
This is a small PullRequest, so it doesn't take much time to review.

### Overview

`helm create` is a useful command for helm beginner like me. However the template is a little dirty.

```
$ helm create test
Creating test
$ helm template test
---
# Source: test/templates/serviceaccount.yaml
apiVersion: v1
kind: ServiceAccount
metadata:
  name: RELEASE-NAME-test
  labels:

    helm.sh/chart: test-0.1.0
    app.kubernetes.io/name: test
    app.kubernetes.io/instance: RELEASE-NAME
    app.kubernetes.io/version: "1.16.0"
    app.kubernetes.io/managed-by: Helm
---
# Source: test/templates/service.yaml
apiVersion: v1
kind: Service
metadata:
...
---
# Source: test/templates/tests/test-connection.yaml
apiVersion: v1
kind: Pod
metadata:
  name: "RELEASE-NAME-test-test-connection"
  labels:

    helm.sh/chart: test-0.1.0
    app.kubernetes.io/name: test
    app.kubernetes.io/instance: RELEASE-NAME
    app.kubernetes.io/version: "1.16.0"
    app.kubernetes.io/managed-by: Helm
  annotations:
    "helm.sh/hook": test-success
spec:
  containers:
    - name: wget
      image: busybox
      command: ['wget']
      args:  ['RELEASE-NAME-test:80']
  restartPolicy: Never
```

There are two empty lines. 

I read [Flow Control](https://helm.sh/docs/chart_template_guide/control_structures/). The doc says for a empty line in yaml:
 > looks a little funny

Therefore I removed them, and an extra space between `args: ` and `['RELEASE` .




Patch:
diff --git a/cmd/helm/package.go b/cmd/helm/package.go
index 9f7961f95..20cde6e4c 100644
--- a/cmd/helm/package.go
+++ b/cmd/helm/package.go
@@ -20,6 +20,7 @@ import (
 	"fmt"
 	"io"
 	"io/ioutil"
+	"os"
 	"path/filepath"
 
 	"github.com/pkg/errors"
@@ -80,6 +81,9 @@ func newPackageCmd(out io.Writer) *cobra.Command {
 				if err != nil {
 					return err
 				}
+				if _, err := os.Stat(args[i]); err != nil {
+					return err
+				}
 
 				if client.DependencyUpdate {
 					downloadManager := &downloader.Manager{
diff --git a/pkg/chartutil/create.go b/pkg/chartutil/create.go
index 390f12f4c..496f20166 100644
--- a/pkg/chartutil/create.go
+++ b/pkg/chartutil/create.go
@@ -304,7 +304,7 @@ kind: ServiceAccount
 metadata:
   name: {{ include "<CHARTNAME>.serviceAccountName" . }}
   labels:
-{{ include "<CHARTNAME>.labels" . | nindent 4 }}
+    {{- include "<CHARTNAME>.labels" . | nindent 4 }}
   {{- with .Values.serviceAccount.annotations }}
   annotations:
     {{- toYaml . | nindent 4 }}
@@ -405,7 +405,7 @@ kind: Pod
 metadata:
   name: "{{ include "<CHARTNAME>.fullname" . }}-test-connection"
   labels:
-{{ include "<CHARTNAME>.labels" . | nindent 4 }}
+    {{- include "<CHARTNAME>.labels" . | nindent 4 }}
   annotations:
     "helm.sh/hook": test-success
 spec:
@@ -413,7 +413,7 @@ spec:
     - name: wget
       image: busybox
       command: ['wget']
-      args:  ['{{ include "<CHARTNAME>.fullname" . }}:{{ .Values.service.port }}']
+      args: ['{{ include "<CHARTNAME>.fullname" . }}:{{ .Values.service.port }}']
   restartPolicy: Never
 `
 
diff --git a/pkg/storage/storage.go b/pkg/storage/storage.go
index 56881493b..89183355f 100644
--- a/pkg/storage/storage.go
+++ b/pkg/storage/storage.go
@@ -119,6 +119,10 @@ func (s *Storage) Deployed(name string) (*rspb.Release, error) {
 		return nil, errors.Errorf("%q has no deployed releases", name)
 	}
 
+	// If executed concurrently, Helm's database gets corrupted
+	// and multiple releases are DEPLOYED. Take the latest.
+	relutil.Reverse(ls, relutil.SortByRevision)
+
 	return ls[0], nil
 }
 


Test patch:
diff --git a/cmd/helm/package_test.go b/cmd/helm/package_test.go
index 739857fa9..38938b4af 100644
--- a/cmd/helm/package_test.go
+++ b/cmd/helm/package_test.go
@@ -296,6 +296,41 @@ func TestPackageValues(t *testing.T) {
 	}
 }
 
+func TestNonExistentDirAndBadPermission(t *testing.T) {
+	nonExistentDir := "testdata/testcharts/non-existent-directory"
+
+	tests := []struct {
+		name    string
+		flags   map[string]string
+		args    []string
+		expect  string
+		hasfile string
+		err     bool
+	}{
+		{
+			name:   "package testdata/testcharts/non-existent-directory",
+			args:   []string{"testdata/testcharts/non-existent-directory"},
+			expect: fmt.Sprintf("stat %s: no such file or directory", nonExistentDir),
+			err:    true,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			var buf bytes.Buffer
+			c := newPackageCmd(&buf)
+			re := regexp.MustCompile(tt.expect)
+			err := c.RunE(c, tt.args)
+			if err != nil {
+				if tt.err && re.MatchString(err.Error()) {
+					return
+				}
+				t.Fatalf("%q: expected error %q, got %q", tt.name, tt.expect, err)
+			}
+		})
+	}
+}
+
 func createValuesFile(t *testing.T, data string) string {
 	outputDir := ensure.TempDir(t)
 
diff --git a/cmd/helm/testdata/testcharts/alpine/templates/alpine-pod.yaml b/cmd/helm/testdata/testcharts/alpine/templates/alpine-pod.yaml
index ae19a1127..a1a44e53f 100644
--- a/cmd/helm/testdata/testcharts/alpine/templates/alpine-pod.yaml
+++ b/cmd/helm/testdata/testcharts/alpine/templates/alpine-pod.yaml
@@ -14,7 +14,7 @@ metadata:
     app.kubernetes.io/version: {{ .Chart.AppVersion }}
     # This makes it easy to audit chart usage.
     helm.sh/chart: "{{.Chart.Name}}-{{.Chart.Version}}"
-    values: {{.Values.test.Name}}
+    values: {{.Values.Name}}
 spec:
   # This shows how to use a simple value. This will look for a passed-in value
   # called restartPolicy. If it is not found, it will use the default value.
diff --git a/pkg/storage/storage_test.go b/pkg/storage/storage_test.go
index b8c333d24..ee9c68b80 100644
--- a/pkg/storage/storage_test.go
+++ b/pkg/storage/storage_test.go
@@ -205,6 +205,46 @@ func TestStorageDeployed(t *testing.T) {
 	}
 }
 
+func TestStorageDeployedWithCorruption(t *testing.T) {
+	storage := Init(driver.NewMemory())
+
+	const name = "angry-bird"
+	const vers = int(4)
+
+	// setup storage with test releases
+	setup := func() {
+		// release records (notice odd order and corruption)
+		rls0 := ReleaseTestData{Name: name, Version: 1, Status: rspb.StatusSuperseded}.ToRelease()
+		rls1 := ReleaseTestData{Name: name, Version: 4, Status: rspb.StatusDeployed}.ToRelease()
+		rls2 := ReleaseTestData{Name: name, Version: 3, Status: rspb.StatusSuperseded}.ToRelease()
+		rls3 := ReleaseTestData{Name: name, Version: 2, Status: rspb.StatusDeployed}.ToRelease()
+
+		// create the release records in the storage
+		assertErrNil(t.Fatal, storage.Create(rls0), "Storing release 'angry-bird' (v1)")
+		assertErrNil(t.Fatal, storage.Create(rls1), "Storing release 'angry-bird' (v2)")
+		assertErrNil(t.Fatal, storage.Create(rls2), "Storing release 'angry-bird' (v3)")
+		assertErrNil(t.Fatal, storage.Create(rls3), "Storing release 'angry-bird' (v4)")
+	}
+
+	setup()
+
+	rls, err := storage.Deployed(name)
+	if err != nil {
+		t.Fatalf("Failed to query for deployed release: %s\n", err)
+	}
+
+	switch {
+	case rls == nil:
+		t.Fatalf("Release is nil")
+	case rls.Name != name:
+		t.Fatalf("Expected release name %q, actual %q\n", name, rls.Name)
+	case rls.Version != vers:
+		t.Fatalf("Expected release version %d, actual %d\n", vers, rls.Version)
+	case rls.Info.Status != rspb.StatusDeployed:
+		t.Fatalf("Expected release status 'DEPLOYED', actual %s\n", rls.Info.Status.String())
+	}
+}
+
 func TestStorageHistory(t *testing.T) {
 	storage := Init(driver.NewMemory())