Repository: helm/helm
Pull request #7820 (2020-03-25 22:03:10)

fix(helm): Data race in kube/client Delete func
helm uninstall seems to have a data race in its Delete function.
This should resolve it with a mutex.


Patch:
diff --git a/README.md b/README.md
index 745a60c2b..bb6908fdb 100644
--- a/README.md
+++ b/README.md
@@ -41,6 +41,7 @@ If you want to use a package manager:
 - [Chocolatey](https://chocolatey.org/) users can use `choco install kubernetes-helm`.
 - [Scoop](https://scoop.sh/) users can use `scoop install helm`.
 - [GoFish](https://gofi.sh/) users can use `gofish install helm`.
+- [Snapcraft](https://snapcraft.io/) users can use `snap install helm --classic`
 
 To rapidly get Helm up and running, start with the [Quick Start Guide](https://docs.helm.sh/using_helm/#quickstart-guide).
 
diff --git a/cmd/helm/template.go b/cmd/helm/template.go
index bd14cde1d..a4438b50c 100644
--- a/cmd/helm/template.go
+++ b/cmd/helm/template.go
@@ -22,6 +22,7 @@ import (
 	"io"
 	"path/filepath"
 	"regexp"
+	"sort"
 	"strings"
 
 	"github.com/spf13/cobra"
@@ -86,12 +87,21 @@ func newTemplateCmd(cfg *action.Configuration, out io.Writer) *cobra.Command {
 				// if we have a list of files to render, then check that each of the
 				// provided files exists in the chart.
 				if len(showFiles) > 0 {
+					// This is necessary to ensure consistent manifest ordering when using --show-only
+					// with globs or directory names.
 					splitManifests := releaseutil.SplitManifests(manifests.String())
+					manifestsKeys := make([]string, 0, len(splitManifests))
+					for k := range splitManifests {
+						manifestsKeys = append(manifestsKeys, k)
+					}
+					sort.Sort(releaseutil.BySplitManifestsOrder(manifestsKeys))
+
 					manifestNameRegex := regexp.MustCompile("# Source: [^/]+/(.+)")
 					var manifestsToRender []string
 					for _, f := range showFiles {
 						missing := true
-						for _, manifest := range splitManifests {
+						for _, manifestKey := range manifestsKeys {
+							manifest := splitManifests[manifestKey]
 							submatch := manifestNameRegex.FindStringSubmatch(manifest)
 							if len(submatch) == 0 {
 								continue
@@ -104,10 +114,11 @@ func newTemplateCmd(cfg *action.Configuration, out io.Writer) *cobra.Command {
 
 							// if the filepath provided matches a manifest path in the
 							// chart, render that manifest
-							if f == manifestPath {
-								manifestsToRender = append(manifestsToRender, manifest)
-								missing = false
+							if matched, _ := filepath.Match(f, manifestPath); !matched {
+								continue
 							}
+							manifestsToRender = append(manifestsToRender, manifest)
+							missing = false
 						}
 						if missing {
 							return fmt.Errorf("could not find template %s in chart", f)
diff --git a/cmd/helm/upgrade.go b/cmd/helm/upgrade.go
index af8ff68e3..48eac0b0f 100644
--- a/cmd/helm/upgrade.go
+++ b/cmd/helm/upgrade.go
@@ -112,6 +112,7 @@ func newUpgradeCmd(cfg *action.Configuration, out io.Writer) *cobra.Command {
 					instClient.Atomic = client.Atomic
 					instClient.PostRenderer = client.PostRenderer
 					instClient.DisableOpenAPIValidation = client.DisableOpenAPIValidation
+					instClient.SubNotes = client.SubNotes
 
 					rel, err := runInstall(args, instClient, valueOpts, out)
 					if err != nil {
diff --git a/pkg/chartutil/create.go b/pkg/chartutil/create.go
index 24eb1e277..b088f20ce 100644
--- a/pkg/chartutil/create.go
+++ b/pkg/chartutil/create.go
@@ -53,6 +53,8 @@ const (
 	ServiceName = TemplatesDir + sep + "service.yaml"
 	// ServiceAccountName is the name of the example serviceaccount file.
 	ServiceAccountName = TemplatesDir + sep + "serviceaccount.yaml"
+	// HorizontalPodAutoscalerName is the name of the example hpa file.
+	HorizontalPodAutoscalerName = TemplatesDir + sep + "hpa.yaml"
 	// NotesName is the name of the example NOTES.txt file.
 	NotesName = TemplatesDir + sep + "NOTES.txt"
 	// HelpersName is the name of the example helpers file.
@@ -149,6 +151,13 @@ resources: {}
   #   cpu: 100m
   #   memory: 128Mi
 
+autoscaling:
+  enabled: false
+  minReplicas: 1
+  maxReplicas: 100
+  targetCPUUtilizationPercentage: 80
+  # targetMemoryUtilizationPercentage: 80
+
 nodeSelector: {}
 
 tolerations: []
@@ -231,7 +240,9 @@ metadata:
   labels:
     {{- include "<CHARTNAME>.labels" . | nindent 4 }}
 spec:
+{{- if not .Values.autoscaling.enabled }}
   replicas: {{ .Values.replicaCount }}
+{{- end }}
   selector:
     matchLabels:
       {{- include "<CHARTNAME>.selectorLabels" . | nindent 6 }}
@@ -312,6 +323,36 @@ metadata:
 {{- end -}}
 `
 
+const defaultHorizontalPodAutoscaler = `{{- if .Values.autoscaling.enabled }}
+apiVersion: autoscaling/v2beta1
+kind: HorizontalPodAutoscaler
+metadata:
+  name: {{ include "<CHARTNAME>.fullname" . }}
+  labels:
+    {{- include "<CHARTNAME>.labels" . | nindent 4 }}
+spec:
+  scaleTargetRef:
+    apiVersion: apps/v1
+    kind: Deployment
+    name: {{ include "<CHARTNAME>.fullname" . }}
+  minReplicas: {{ .Values.autoscaling.minReplicas }}
+  maxReplicas: {{ .Values.autoscaling.maxReplicas }}
+  metrics:
+  {{- if .Values.autoscaling.targetCPUUtilizationPercentage }}
+    - type: Resource
+      resource:
+        name: cpu
+        targetAverageUtilization: {{ .Values.autoscaling.targetCPUUtilizationPercentage }}
+  {{- end }}
+  {{- if .Values.autoscaling.targetMemoryUtilizationPercentage }}
+    - type: Resource
+      resource:
+        name: memory
+        targetAverageUtilization: {{ .Values.autoscaling.targetMemoryUtilizationPercentage }}
+  {{- end }}
+{{- end }}
+`
+
 const defaultNotes = `1. Get the application URL by running these commands:
 {{- if .Values.ingress.enabled }}
 {{- range $host := .Values.ingress.hosts }}
@@ -526,6 +567,11 @@ func Create(name, dir string) (string, error) {
 			path:    filepath.Join(cdir, ServiceAccountName),
 			content: transform(defaultServiceAccount, name),
 		},
+		{
+			// hpa.yaml
+			path:    filepath.Join(cdir, HorizontalPodAutoscalerName),
+			content: transform(defaultHorizontalPodAutoscaler, name),
+		},
 		{
 			// NOTES.txt
 			path:    filepath.Join(cdir, NotesName),
diff --git a/pkg/downloader/chart_downloader.go b/pkg/downloader/chart_downloader.go
index 340a65472..0013dbdf0 100644
--- a/pkg/downloader/chart_downloader.go
+++ b/pkg/downloader/chart_downloader.go
@@ -72,6 +72,31 @@ type ChartDownloader struct {
 	RepositoryCache  string
 }
 
+// atomicWriteFile atomically (as atomic as os.Rename allows) writes a file to a
+// disk.
+func atomicWriteFile(filename string, body io.Reader, mode os.FileMode) error {
+	tempFile, err := ioutil.TempFile(filepath.Split(filename))
+	if err != nil {
+		return err
+	}
+	tempName := tempFile.Name()
+
+	if _, err := io.Copy(tempFile, body); err != nil {
+		tempFile.Close() // return value is ignored as we are already on error path
+		return err
+	}
+
+	if err := tempFile.Close(); err != nil {
+		return err
+	}
+
+	if err := os.Chmod(tempName, mode); err != nil {
+		return err
+	}
+
+	return os.Rename(tempName, filename)
+}
+
 // DownloadTo retrieves a chart. Depending on the settings, it may also download a provenance file.
 //
 // If Verify is set to VerifyNever, the verification will be nil.
@@ -101,7 +126,7 @@ func (c *ChartDownloader) DownloadTo(ref, version, dest string) (string, *proven
 
 	name := filepath.Base(u.Path)
 	destfile := filepath.Join(dest, name)
-	if err := ioutil.WriteFile(destfile, data.Bytes(), 0644); err != nil {
+	if err := atomicWriteFile(destfile, data, 0644); err != nil {
 		return destfile, nil, err
 	}
 
@@ -117,7 +142,7 @@ func (c *ChartDownloader) DownloadTo(ref, version, dest string) (string, *proven
 			return destfile, ver, nil
 		}
 		provfile := destfile + ".prov"
-		if err := ioutil.WriteFile(provfile, body.Bytes(), 0644); err != nil {
+		if err := atomicWriteFile(provfile, body, 0644); err != nil {
 			return destfile, nil, err
 		}
 
diff --git a/pkg/engine/funcs.go b/pkg/engine/funcs.go
index dac105e74..e5769cbe0 100644
--- a/pkg/engine/funcs.go
+++ b/pkg/engine/funcs.go
@@ -48,11 +48,13 @@ func funcMap() template.FuncMap {
 
 	// Add some extra functionality
 	extra := template.FuncMap{
-		"toToml":   toTOML,
-		"toYaml":   toYAML,
-		"fromYaml": fromYAML,
-		"toJson":   toJSON,
-		"fromJson": fromJSON,
+		"toToml":        toTOML,
+		"toYaml":        toYAML,
+		"fromYaml":      fromYAML,
+		"fromYamlArray": fromYAMLArray,
+		"toJson":        toJSON,
+		"fromJson":      fromJSON,
+		"fromJsonArray": fromJSONArray,
 
 		// This is a placeholder for the "include" function, which is
 		// late-bound to a template. By declaring it here, we preserve the
@@ -97,6 +99,21 @@ func fromYAML(str string) map[string]interface{} {
 	return m
 }
 
+// fromYAMLArray converts a YAML array into a []interface{}.
+//
+// This is not a general-purpose YAML parser, and will not parse all valid
+// YAML documents. Additionally, because its intended use is within templates
+// it tolerates errors. It will insert the returned error message string as
+// the first and only item in the returned array.
+func fromYAMLArray(str string) []interface{} {
+	a := []interface{}{}
+
+	if err := yaml.Unmarshal([]byte(str), &a); err != nil {
+		a = []interface{}{err.Error()}
+	}
+	return a
+}
+
 // toTOML takes an interface, marshals it to toml, and returns a string. It will
 // always return a string, even on marshal error (empty string).
 //
@@ -138,3 +155,18 @@ func fromJSON(str string) map[string]interface{} {
 	}
 	return m
 }
+
+// fromJSONArray converts a JSON array into a []interface{}.
+//
+// This is not a general-purpose JSON parser, and will not parse all valid
+// JSON documents. Additionally, because its intended use is within templates
+// it tolerates errors. It will insert the returned error message string as
+// the first and only item in the returned array.
+func fromJSONArray(str string) []interface{} {
+	a := []interface{}{}
+
+	if err := json.Unmarshal([]byte(str), &a); err != nil {
+		a = []interface{}{err.Error()}
+	}
+	return a
+}
diff --git a/pkg/kube/client.go b/pkg/kube/client.go
index b761c6d12..2cb7e45cf 100644
--- a/pkg/kube/client.go
+++ b/pkg/kube/client.go
@@ -246,12 +246,17 @@ func (c *Client) Update(original, target ResourceList, force bool) (*Result, err
 func (c *Client) Delete(resources ResourceList) (*Result, []error) {
 	var errs []error
 	res := &Result{}
+	mtx := sync.Mutex{}
 	err := perform(resources, func(info *resource.Info) error {
 		c.Log("Starting delete for %q %s", info.Name, info.Mapping.GroupVersionKind.Kind)
 		if err := c.skipIfNotFound(deleteResource(info)); err != nil {
+			mtx.Lock()
+			defer mtx.Unlock()
 			// Collect the error and continue on
 			errs = append(errs, err)
 		} else {
+			mtx.Lock()
+			defer mtx.Unlock()
 			res.Deleted = append(res.Deleted, info)
 		}
 		return nil


Test patch:
diff --git a/cmd/helm/create_test.go b/cmd/helm/create_test.go
index 0a9b7b9a5..bbb8d394a 100644
--- a/cmd/helm/create_test.go
+++ b/cmd/helm/create_test.go
@@ -106,7 +106,7 @@ func TestCreateStarterCmd(t *testing.T) {
 		t.Errorf("Wrong API version: %q", c.Metadata.APIVersion)
 	}
 
-	expectedNumberOfTemplates := 8
+	expectedNumberOfTemplates := 9
 	if l := len(c.Templates); l != expectedNumberOfTemplates {
 		t.Errorf("Expected %d templates, got %d", expectedNumberOfTemplates, l)
 	}
@@ -174,7 +174,7 @@ func TestCreateStarterAbsoluteCmd(t *testing.T) {
 		t.Errorf("Wrong API version: %q", c.Metadata.APIVersion)
 	}
 
-	expectedNumberOfTemplates := 8
+	expectedNumberOfTemplates := 9
 	if l := len(c.Templates); l != expectedNumberOfTemplates {
 		t.Errorf("Expected %d templates, got %d", expectedNumberOfTemplates, l)
 	}
diff --git a/cmd/helm/template_test.go b/cmd/helm/template_test.go
index 3fd139fad..87ee79e5a 100644
--- a/cmd/helm/template_test.go
+++ b/cmd/helm/template_test.go
@@ -94,6 +94,13 @@ func TestTemplateCmd(t *testing.T) {
 			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:   "template with show-only glob",
+			cmd:    fmt.Sprintf("template '%s' --show-only templates/subdir/role*", chartPath),
+			golden: "output/template-show-only-glob.txt",
+			// Repeat to ensure manifest ordering regressions are caught
+			repeat: 10,
+		},
 		{
 			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-name-template.txt b/cmd/helm/testdata/output/template-name-template.txt
index acba50360..0130a2a92 100644
--- a/cmd/helm/testdata/output/template-name-template.txt
+++ b/cmd/helm/testdata/output/template-name-template.txt
@@ -1,4 +1,33 @@
 ---
+# Source: subchart1/templates/subdir/serviceaccount.yaml
+apiVersion: v1
+kind: ServiceAccount
+metadata:
+  name: subchart1-sa
+---
+# Source: subchart1/templates/subdir/role.yaml
+apiVersion: rbac.authorization.k8s.io/v1
+kind: Role
+metadata:
+  name: subchart1-role
+rules:
+- resources: ["*"]
+  verbs: ["get","list","watch"]
+---
+# Source: subchart1/templates/subdir/rolebinding.yaml
+apiVersion: rbac.authorization.k8s.io/v1
+kind: RoleBinding
+metadata:
+  name: subchart1-binding
+roleRef:
+  apiGroup: rbac.authorization.k8s.io
+  kind: Role
+  name: subchart1-role
+subjects:
+- kind: ServiceAccount
+  name: subchart1-sa
+  namespace: default
+---
 # Source: subchart1/charts/subcharta/templates/service.yaml
 apiVersion: v1
 kind: Service
diff --git a/cmd/helm/testdata/output/template-set.txt b/cmd/helm/testdata/output/template-set.txt
index b0924b5b6..ddaa8886b 100644
--- a/cmd/helm/testdata/output/template-set.txt
+++ b/cmd/helm/testdata/output/template-set.txt
@@ -1,4 +1,33 @@
 ---
+# Source: subchart1/templates/subdir/serviceaccount.yaml
+apiVersion: v1
+kind: ServiceAccount
+metadata:
+  name: subchart1-sa
+---
+# Source: subchart1/templates/subdir/role.yaml
+apiVersion: rbac.authorization.k8s.io/v1
+kind: Role
+metadata:
+  name: subchart1-role
+rules:
+- resources: ["*"]
+  verbs: ["get","list","watch"]
+---
+# Source: subchart1/templates/subdir/rolebinding.yaml
+apiVersion: rbac.authorization.k8s.io/v1
+kind: RoleBinding
+metadata:
+  name: subchart1-binding
+roleRef:
+  apiGroup: rbac.authorization.k8s.io
+  kind: Role
+  name: subchart1-role
+subjects:
+- kind: ServiceAccount
+  name: subchart1-sa
+  namespace: default
+---
 # Source: subchart1/charts/subcharta/templates/service.yaml
 apiVersion: v1
 kind: Service
diff --git a/cmd/helm/testdata/output/template-show-only-glob.txt b/cmd/helm/testdata/output/template-show-only-glob.txt
new file mode 100644
index 000000000..0970e6cd3
--- /dev/null
+++ b/cmd/helm/testdata/output/template-show-only-glob.txt
@@ -0,0 +1,23 @@
+---
+# Source: subchart1/templates/subdir/role.yaml
+apiVersion: rbac.authorization.k8s.io/v1
+kind: Role
+metadata:
+  name: subchart1-role
+rules:
+- resources: ["*"]
+  verbs: ["get","list","watch"]
+---
+# Source: subchart1/templates/subdir/rolebinding.yaml
+apiVersion: rbac.authorization.k8s.io/v1
+kind: RoleBinding
+metadata:
+  name: subchart1-binding
+roleRef:
+  apiGroup: rbac.authorization.k8s.io
+  kind: Role
+  name: subchart1-role
+subjects:
+- kind: ServiceAccount
+  name: subchart1-sa
+  namespace: default
diff --git a/cmd/helm/testdata/output/template-values-files.txt b/cmd/helm/testdata/output/template-values-files.txt
index b0924b5b6..ddaa8886b 100644
--- a/cmd/helm/testdata/output/template-values-files.txt
+++ b/cmd/helm/testdata/output/template-values-files.txt
@@ -1,4 +1,33 @@
 ---
+# Source: subchart1/templates/subdir/serviceaccount.yaml
+apiVersion: v1
+kind: ServiceAccount
+metadata:
+  name: subchart1-sa
+---
+# Source: subchart1/templates/subdir/role.yaml
+apiVersion: rbac.authorization.k8s.io/v1
+kind: Role
+metadata:
+  name: subchart1-role
+rules:
+- resources: ["*"]
+  verbs: ["get","list","watch"]
+---
+# Source: subchart1/templates/subdir/rolebinding.yaml
+apiVersion: rbac.authorization.k8s.io/v1
+kind: RoleBinding
+metadata:
+  name: subchart1-binding
+roleRef:
+  apiGroup: rbac.authorization.k8s.io
+  kind: Role
+  name: subchart1-role
+subjects:
+- kind: ServiceAccount
+  name: subchart1-sa
+  namespace: default
+---
 # Source: subchart1/charts/subcharta/templates/service.yaml
 apiVersion: v1
 kind: Service
diff --git a/cmd/helm/testdata/output/template-with-api-version.txt b/cmd/helm/testdata/output/template-with-api-version.txt
index da3559082..7a2a4d5bf 100644
--- a/cmd/helm/testdata/output/template-with-api-version.txt
+++ b/cmd/helm/testdata/output/template-with-api-version.txt
@@ -1,4 +1,33 @@
 ---
+# Source: subchart1/templates/subdir/serviceaccount.yaml
+apiVersion: v1
+kind: ServiceAccount
+metadata:
+  name: subchart1-sa
+---
+# Source: subchart1/templates/subdir/role.yaml
+apiVersion: rbac.authorization.k8s.io/v1
+kind: Role
+metadata:
+  name: subchart1-role
+rules:
+- resources: ["*"]
+  verbs: ["get","list","watch"]
+---
+# Source: subchart1/templates/subdir/rolebinding.yaml
+apiVersion: rbac.authorization.k8s.io/v1
+kind: RoleBinding
+metadata:
+  name: subchart1-binding
+roleRef:
+  apiGroup: rbac.authorization.k8s.io
+  kind: Role
+  name: subchart1-role
+subjects:
+- kind: ServiceAccount
+  name: subchart1-sa
+  namespace: default
+---
 # Source: subchart1/charts/subcharta/templates/service.yaml
 apiVersion: v1
 kind: Service
diff --git a/cmd/helm/testdata/output/template-with-crds.txt b/cmd/helm/testdata/output/template-with-crds.txt
index 9fa1c7e6d..b04a4d5d2 100644
--- a/cmd/helm/testdata/output/template-with-crds.txt
+++ b/cmd/helm/testdata/output/template-with-crds.txt
@@ -15,6 +15,35 @@ spec:
     singular: authconfig
 
 ---
+# Source: subchart1/templates/subdir/serviceaccount.yaml
+apiVersion: v1
+kind: ServiceAccount
+metadata:
+  name: subchart1-sa
+---
+# Source: subchart1/templates/subdir/role.yaml
+apiVersion: rbac.authorization.k8s.io/v1
+kind: Role
+metadata:
+  name: subchart1-role
+rules:
+- resources: ["*"]
+  verbs: ["get","list","watch"]
+---
+# Source: subchart1/templates/subdir/rolebinding.yaml
+apiVersion: rbac.authorization.k8s.io/v1
+kind: RoleBinding
+metadata:
+  name: subchart1-binding
+roleRef:
+  apiGroup: rbac.authorization.k8s.io
+  kind: Role
+  name: subchart1-role
+subjects:
+- kind: ServiceAccount
+  name: subchart1-sa
+  namespace: default
+---
 # Source: subchart1/charts/subcharta/templates/service.yaml
 apiVersion: v1
 kind: Service
diff --git a/cmd/helm/testdata/output/template.txt b/cmd/helm/testdata/output/template.txt
index 080be618c..8301c2231 100644
--- a/cmd/helm/testdata/output/template.txt
+++ b/cmd/helm/testdata/output/template.txt
@@ -1,4 +1,33 @@
 ---
+# Source: subchart1/templates/subdir/serviceaccount.yaml
+apiVersion: v1
+kind: ServiceAccount
+metadata:
+  name: subchart1-sa
+---
+# Source: subchart1/templates/subdir/role.yaml
+apiVersion: rbac.authorization.k8s.io/v1
+kind: Role
+metadata:
+  name: subchart1-role
+rules:
+- resources: ["*"]
+  verbs: ["get","list","watch"]
+---
+# Source: subchart1/templates/subdir/rolebinding.yaml
+apiVersion: rbac.authorization.k8s.io/v1
+kind: RoleBinding
+metadata:
+  name: subchart1-binding
+roleRef:
+  apiGroup: rbac.authorization.k8s.io
+  kind: Role
+  name: subchart1-role
+subjects:
+- kind: ServiceAccount
+  name: subchart1-sa
+  namespace: default
+---
 # Source: subchart1/charts/subcharta/templates/service.yaml
 apiVersion: v1
 kind: Service
diff --git a/cmd/helm/testdata/testcharts/chart-with-subchart-notes/Chart.yaml b/cmd/helm/testdata/testcharts/chart-with-subchart-notes/Chart.yaml
new file mode 100644
index 000000000..90545a6a3
--- /dev/null
+++ b/cmd/helm/testdata/testcharts/chart-with-subchart-notes/Chart.yaml
@@ -0,0 +1,7 @@
+apiVersion: v2
+description: Chart with subchart notes
+name: chart-with-subchart-notes
+version: 0.0.1
+dependencies:
+  - name: subchart-with-notes
+    version: 0.0.1
diff --git a/cmd/helm/testdata/testcharts/chart-with-subchart-notes/charts/subchart-with-notes/Chart.yaml b/cmd/helm/testdata/testcharts/chart-with-subchart-notes/charts/subchart-with-notes/Chart.yaml
new file mode 100644
index 000000000..f0fead9ee
--- /dev/null
+++ b/cmd/helm/testdata/testcharts/chart-with-subchart-notes/charts/subchart-with-notes/Chart.yaml
@@ -0,0 +1,4 @@
+apiVersion: v2
+description: Subchart with notes
+name: subchart-with-notes
+version: 0.0.1
diff --git a/cmd/helm/testdata/testcharts/chart-with-subchart-notes/charts/subchart-with-notes/templates/NOTES.txt b/cmd/helm/testdata/testcharts/chart-with-subchart-notes/charts/subchart-with-notes/templates/NOTES.txt
new file mode 100644
index 000000000..1f61a294e
--- /dev/null
+++ b/cmd/helm/testdata/testcharts/chart-with-subchart-notes/charts/subchart-with-notes/templates/NOTES.txt
@@ -0,0 +1,1 @@
+SUBCHART NOTES
diff --git a/cmd/helm/testdata/testcharts/chart-with-subchart-notes/templates/NOTES.txt b/cmd/helm/testdata/testcharts/chart-with-subchart-notes/templates/NOTES.txt
new file mode 100644
index 000000000..9e166d370
--- /dev/null
+++ b/cmd/helm/testdata/testcharts/chart-with-subchart-notes/templates/NOTES.txt
@@ -0,0 +1,1 @@
+PARENT NOTES
diff --git a/cmd/helm/upgrade_test.go b/cmd/helm/upgrade_test.go
index 3cecbe6d3..4956c0247 100644
--- a/cmd/helm/upgrade_test.go
+++ b/cmd/helm/upgrade_test.go
@@ -196,6 +196,38 @@ func TestUpgradeWithStringValue(t *testing.T) {
 
 }
 
+func TestUpgradeInstallWithSubchartNotes(t *testing.T) {
+
+	releaseName := "wacky-bunny-v1"
+	relMock, ch, _ := prepareMockRelease(releaseName, t)
+
+	defer resetEnv()()
+
+	store := storageFixture()
+
+	store.Create(relMock(releaseName, 1, ch))
+
+	cmd := fmt.Sprintf("upgrade %s -i --render-subchart-notes '%s'", releaseName, "testdata/testcharts/chart-with-subchart-notes")
+	_, _, err := executeActionCommandC(store, cmd)
+	if err != nil {
+		t.Errorf("unexpected error, got '%v'", err)
+	}
+
+	upgradedRel, err := store.Get(releaseName, 2)
+	if err != nil {
+		t.Errorf("unexpected error, got '%v'", err)
+	}
+
+	if !strings.Contains(upgradedRel.Info.Notes, "PARENT NOTES") {
+		t.Errorf("The parent notes are not set correctly. NOTES: %s", upgradedRel.Info.Notes)
+	}
+
+	if !strings.Contains(upgradedRel.Info.Notes, "SUBCHART NOTES") {
+		t.Errorf("The subchart notes are not set correctly. NOTES: %s", upgradedRel.Info.Notes)
+	}
+
+}
+
 func TestUpgradeWithValuesFile(t *testing.T) {
 
 	releaseName := "funny-bunny-v4"
diff --git a/pkg/chart/chart_test.go b/pkg/chart/chart_test.go
index ef623fff6..4f19bf87d 100644
--- a/pkg/chart/chart_test.go
+++ b/pkg/chart/chart_test.go
@@ -78,3 +78,21 @@ func TestSaveChartNoRawData(t *testing.T) {
 
 	is.Equal([]*File(nil), res.Raw)
 }
+
+func TestMetadata(t *testing.T) {
+	chrt := Chart{
+		Metadata: &Metadata{
+			Name:       "foo.yaml",
+			AppVersion: "1.0.0",
+			APIVersion: "v2",
+			Version:    "1.0.0",
+			Type:       "application",
+		},
+	}
+
+	is := assert.New(t)
+
+	is.Equal("foo.yaml", chrt.Name())
+	is.Equal("1.0.0", chrt.AppVersion())
+	is.Equal(nil, chrt.Validate())
+}
diff --git a/pkg/chartutil/testdata/subpop/charts/subchart1/templates/subdir/role.yaml b/pkg/chartutil/testdata/subpop/charts/subchart1/templates/subdir/role.yaml
new file mode 100644
index 000000000..91b954e5f
--- /dev/null
+++ b/pkg/chartutil/testdata/subpop/charts/subchart1/templates/subdir/role.yaml
@@ -0,0 +1,7 @@
+apiVersion: rbac.authorization.k8s.io/v1
+kind: Role
+metadata:
+  name: {{ .Chart.Name }}-role
+rules:
+- resources: ["*"]
+  verbs: ["get","list","watch"]
diff --git a/pkg/chartutil/testdata/subpop/charts/subchart1/templates/subdir/rolebinding.yaml b/pkg/chartutil/testdata/subpop/charts/subchart1/templates/subdir/rolebinding.yaml
new file mode 100644
index 000000000..5d193f1a6
--- /dev/null
+++ b/pkg/chartutil/testdata/subpop/charts/subchart1/templates/subdir/rolebinding.yaml
@@ -0,0 +1,12 @@
+apiVersion: rbac.authorization.k8s.io/v1
+kind: RoleBinding
+metadata:
+  name: {{ .Chart.Name }}-binding
+roleRef:
+  apiGroup: rbac.authorization.k8s.io
+  kind: Role
+  name: {{ .Chart.Name }}-role
+subjects:
+- kind: ServiceAccount
+  name: {{ .Chart.Name }}-sa
+  namespace: default
diff --git a/pkg/chartutil/testdata/subpop/charts/subchart1/templates/subdir/serviceaccount.yaml b/pkg/chartutil/testdata/subpop/charts/subchart1/templates/subdir/serviceaccount.yaml
new file mode 100644
index 000000000..7126c7d89
--- /dev/null
+++ b/pkg/chartutil/testdata/subpop/charts/subchart1/templates/subdir/serviceaccount.yaml
@@ -0,0 +1,4 @@
+apiVersion: v1
+kind: ServiceAccount
+metadata:
+  name: {{ .Chart.Name }}-sa
diff --git a/pkg/engine/funcs_test.go b/pkg/engine/funcs_test.go
index a94ff257e..a405c1c47 100644
--- a/pkg/engine/funcs_test.go
+++ b/pkg/engine/funcs_test.go
@@ -45,6 +45,14 @@ func TestFuncs(t *testing.T) {
 		tpl:    `{{ fromYaml . }}`,
 		expect: "map[hello:world]",
 		vars:   `hello: world`,
+	}, {
+		tpl:    `{{ fromYamlArray . }}`,
+		expect: "[one 2 map[name:helm]]",
+		vars:   "- one\n- 2\n- name: helm\n",
+	}, {
+		tpl:    `{{ fromYamlArray . }}`,
+		expect: "[one 2 map[name:helm]]",
+		vars:   `["one", 2, { "name": "helm" }]`,
 	}, {
 		// Regression for https://github.com/helm/helm/issues/2271
 		tpl:    `{{ toToml . }}`,
@@ -62,6 +70,14 @@ func TestFuncs(t *testing.T) {
 		tpl:    `{{ fromJson . }}`,
 		expect: `map[Error:json: cannot unmarshal array into Go value of type map[string]interface {}]`,
 		vars:   `["one", "two"]`,
+	}, {
+		tpl:    `{{ fromJsonArray . }}`,
+		expect: `[one 2 map[name:helm]]`,
+		vars:   `["one", 2, { "name": "helm" }]`,
+	}, {
+		tpl:    `{{ fromJsonArray . }}`,
+		expect: `[json: cannot unmarshal object into Go value of type []interface {}]`,
+		vars:   `{"hello": "world"}`,
 	}, {
 		tpl:    `{{ merge .dict (fromYaml .yaml) }}`,
 		expect: `map[a:map[b:c]]`,
@@ -74,6 +90,10 @@ func TestFuncs(t *testing.T) {
 		tpl:    `{{ fromYaml . }}`,
 		expect: `map[Error:error unmarshaling JSON: while decoding JSON: json: cannot unmarshal array into Go value of type map[string]interface {}]`,
 		vars:   `["one", "two"]`,
+	}, {
+		tpl:    `{{ fromYamlArray . }}`,
+		expect: `[error unmarshaling JSON: while decoding JSON: json: cannot unmarshal object into Go value of type []interface {}]`,
+		vars:   `hello: world`,
 	}}
 
 	for _, tt := range tests {
diff --git a/pkg/repo/chartrepo_test.go b/pkg/repo/chartrepo_test.go
index c6b227acf..f50d6a2b6 100644
--- a/pkg/repo/chartrepo_test.go
+++ b/pkg/repo/chartrepo_test.go
@@ -308,7 +308,7 @@ func TestErrorFindChartInRepoURL(t *testing.T) {
 
 	if _, err := FindChartInRepoURL("http://someserver/something", "nginx", "", "", "", "", g); err == nil {
 		t.Errorf("Expected error for bad chart URL, but did not get any errors")
-	} else if !strings.Contains(err.Error(), `looks like "http://someserver/something" is not a valid chart repository or cannot be reached: Get http://someserver/something/index.yaml`) {
+	} else if !strings.Contains(err.Error(), `looks like "http://someserver/something" is not a valid chart repository or cannot be reached`) {
 		t.Errorf("Expected error for bad chart URL, but got a different error (%v)", err)
 	}