Repository: helm/helm
Pull request #7814 (2020-03-25 14:01:50)

Add unit test for pkg/chart/chart.go
Signed-off-by: Hu Shuai <hus.fnst@cn.fujitsu.com>

hs0210: PTAL
hs0210: PTAL

Patch:
diff --git a/README.md b/README.md
index bb6908fdb..745a60c2b 100644
--- a/README.md
+++ b/README.md
@@ -41,7 +41,6 @@ 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/upgrade.go b/cmd/helm/upgrade.go
index 48eac0b0f..af8ff68e3 100644
--- a/cmd/helm/upgrade.go
+++ b/cmd/helm/upgrade.go
@@ -112,7 +112,6 @@ 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 b088f20ce..24eb1e277 100644
--- a/pkg/chartutil/create.go
+++ b/pkg/chartutil/create.go
@@ -53,8 +53,6 @@ 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.
@@ -151,13 +149,6 @@ resources: {}
   #   cpu: 100m
   #   memory: 128Mi
 
-autoscaling:
-  enabled: false
-  minReplicas: 1
-  maxReplicas: 100
-  targetCPUUtilizationPercentage: 80
-  # targetMemoryUtilizationPercentage: 80
-
 nodeSelector: {}
 
 tolerations: []
@@ -240,9 +231,7 @@ metadata:
   labels:
     {{- include "<CHARTNAME>.labels" . | nindent 4 }}
 spec:
-{{- if not .Values.autoscaling.enabled }}
   replicas: {{ .Values.replicaCount }}
-{{- end }}
   selector:
     matchLabels:
       {{- include "<CHARTNAME>.selectorLabels" . | nindent 6 }}
@@ -323,36 +312,6 @@ 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 }}
@@ -567,11 +526,6 @@ 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 0013dbdf0..340a65472 100644
--- a/pkg/downloader/chart_downloader.go
+++ b/pkg/downloader/chart_downloader.go
@@ -72,31 +72,6 @@ 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.
@@ -126,7 +101,7 @@ func (c *ChartDownloader) DownloadTo(ref, version, dest string) (string, *proven
 
 	name := filepath.Base(u.Path)
 	destfile := filepath.Join(dest, name)
-	if err := atomicWriteFile(destfile, data, 0644); err != nil {
+	if err := ioutil.WriteFile(destfile, data.Bytes(), 0644); err != nil {
 		return destfile, nil, err
 	}
 
@@ -142,7 +117,7 @@ func (c *ChartDownloader) DownloadTo(ref, version, dest string) (string, *proven
 			return destfile, ver, nil
 		}
 		provfile := destfile + ".prov"
-		if err := atomicWriteFile(provfile, body, 0644); err != nil {
+		if err := ioutil.WriteFile(provfile, body.Bytes(), 0644); err != nil {
 			return destfile, nil, err
 		}
 
diff --git a/pkg/engine/funcs.go b/pkg/engine/funcs.go
index e5769cbe0..dac105e74 100644
--- a/pkg/engine/funcs.go
+++ b/pkg/engine/funcs.go
@@ -48,13 +48,11 @@ func funcMap() template.FuncMap {
 
 	// Add some extra functionality
 	extra := template.FuncMap{
-		"toToml":        toTOML,
-		"toYaml":        toYAML,
-		"fromYaml":      fromYAML,
-		"fromYamlArray": fromYAMLArray,
-		"toJson":        toJSON,
-		"fromJson":      fromJSON,
-		"fromJsonArray": fromJSONArray,
+		"toToml":   toTOML,
+		"toYaml":   toYAML,
+		"fromYaml": fromYAML,
+		"toJson":   toJSON,
+		"fromJson": fromJSON,
 
 		// This is a placeholder for the "include" function, which is
 		// late-bound to a template. By declaring it here, we preserve the
@@ -99,21 +97,6 @@ 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).
 //
@@ -155,18 +138,3 @@ 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
-}


Test patch:
diff --git a/cmd/helm/create_test.go b/cmd/helm/create_test.go
index bbb8d394a..0a9b7b9a5 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 := 9
+	expectedNumberOfTemplates := 8
 	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 := 9
+	expectedNumberOfTemplates := 8
 	if l := len(c.Templates); l != expectedNumberOfTemplates {
 		t.Errorf("Expected %d templates, got %d", expectedNumberOfTemplates, l)
 	}
diff --git a/cmd/helm/testdata/testcharts/chart-with-subchart-notes/Chart.yaml b/cmd/helm/testdata/testcharts/chart-with-subchart-notes/Chart.yaml
deleted file mode 100644
index 90545a6a3..000000000
--- a/cmd/helm/testdata/testcharts/chart-with-subchart-notes/Chart.yaml
+++ /dev/null
@@ -1,7 +0,0 @@
-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
deleted file mode 100644
index f0fead9ee..000000000
--- a/cmd/helm/testdata/testcharts/chart-with-subchart-notes/charts/subchart-with-notes/Chart.yaml
+++ /dev/null
@@ -1,4 +0,0 @@
-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
deleted file mode 100644
index 1f61a294e..000000000
--- a/cmd/helm/testdata/testcharts/chart-with-subchart-notes/charts/subchart-with-notes/templates/NOTES.txt
+++ /dev/null
@@ -1,1 +0,0 @@
-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
deleted file mode 100644
index 9e166d370..000000000
--- a/cmd/helm/testdata/testcharts/chart-with-subchart-notes/templates/NOTES.txt
+++ /dev/null
@@ -1,1 +0,0 @@
-PARENT NOTES
diff --git a/cmd/helm/upgrade_test.go b/cmd/helm/upgrade_test.go
index 4956c0247..3cecbe6d3 100644
--- a/cmd/helm/upgrade_test.go
+++ b/cmd/helm/upgrade_test.go
@@ -196,38 +196,6 @@ 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/engine/funcs_test.go b/pkg/engine/funcs_test.go
index a405c1c47..a94ff257e 100644
--- a/pkg/engine/funcs_test.go
+++ b/pkg/engine/funcs_test.go
@@ -45,14 +45,6 @@ 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 . }}`,
@@ -70,14 +62,6 @@ 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]]`,
@@ -90,10 +74,6 @@ 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 f50d6a2b6..c6b227acf 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`) {
+	} 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`) {
 		t.Errorf("Expected error for bad chart URL, but got a different error (%v)", err)
 	}