From c93c724102f60161e4a51a37e903280464fd2d33 Mon Sep 17 00:00:00 2001 From: Martin Hickey Date: Tue, 18 Sep 2018 16:39:47 +0100 Subject: [PATCH] Update after review - https://github.com/helm/helm/pull/4524#pullrequestreview-155183390 - https://github.com/helm/helm/pull/4524#pullrequestreview-155379922 - Fix where app.kubernetes.io/instance was incorrect in service selector Signed-off-by: Martin Hickey --- cmd/helm/create.go | 13 +++++++------ pkg/chartutil/create.go | 34 ++++++++++++++++++---------------- pkg/chartutil/create_test.go | 10 +++++++++- pkg/chartutil/save.go | 2 +- pkg/urlutil/urlutil_test.go | 4 ++-- 5 files changed, 37 insertions(+), 26 deletions(-) diff --git a/cmd/helm/create.go b/cmd/helm/create.go index eaea81e5b..7f0f99af8 100644 --- a/cmd/helm/create.go +++ b/cmd/helm/create.go @@ -38,15 +38,17 @@ something like this: foo/ | - |- .helmignore # Contains patterns to ignore when packaging Helm charts. + |- .helmignore # Contains patterns to ignore when packaging Helm charts. | - |- Chart.yaml # Information about your chart + |- Chart.yaml # Information about your chart | - |- values.yaml # The default values for your templates + |- values.yaml # The default values for your templates | - |- charts/ # Charts that this chart depends on + |- charts/ # Charts that this chart depends on | - |- templates/ # The template files + |- templates/ # The template files + | + |- templates/tests/ # The test files 'helm create' takes a path for an argument. If directories in the given path do not exist, Helm will attempt to create them as it goes. If the given @@ -84,7 +86,6 @@ func newCreateCmd(out io.Writer) *cobra.Command { func (c *createCmd) run() error { fmt.Fprintf(c.out, "Creating %s\n", c.name) - chartname := filepath.Base(c.name) cfile := &chart.Metadata{ Name: chartname, diff --git a/pkg/chartutil/create.go b/pkg/chartutil/create.go index 89c9da589..52b561c22 100644 --- a/pkg/chartutil/create.go +++ b/pkg/chartutil/create.go @@ -46,8 +46,10 @@ const ( NotesName = "NOTES.txt" // HelpersName is the name of the example helpers file. HelpersName = "_helpers.tpl" - // ServiceTestName is the name of the example test service file. - ServiceTestName = "service-test.yaml" + // TemplatesTestsDir is the relative directory name for templates tests. + TemplatesTestsDir = "templates/tests" + // TestConnectionName is the name of the example connection test file. + TestConnectionName = "test-connection.yaml" ) const defaultValues = `# Default values for %s. @@ -292,23 +294,23 @@ Create chart name and version as used by the chart label. {{- end -}} ` -const defaultServiceTest = `apiVersion: v1 +const defaultTestConnection = `apiVersion: v1 kind: Pod metadata: - name: "{{ template ".fullname" . }}-service-test" + name: "{{ include ".fullname" . }}-test-connection" labels: - heritage: {{ .Release.Service }} - release: {{ .Release.Name }} - chart: {{ .Chart.Name }}-{{ .Chart.Version }} - app: {{ template ".name" . }} + app.kubernetes.io/name: {{ include ".name" . }} + helm.sh/chart: {{ include ".chart" . }} + app.kubernetes.io/instance: {{ .Release.Name }} + app.kubernetes.io/managed-by: {{ .Release.Service }} annotations: "helm.sh/hook": test-success spec: containers: - - name: curl - image: radial/busyboxplus:curl - command: ['curl'] - args: ['{{ template ".fullname" . }}:{{ .Values.service.port }}'] + - name: wget + image: busybox + command: ['wget'] + args: ['{{ include ".fullname" . }}:{{ .Values.service.port }}'] restartPolicy: Never ` @@ -376,7 +378,7 @@ func Create(chartfile *chart.Metadata, dir string) (string, error) { } } - for _, d := range []string{TemplatesDir, ChartsDir} { + for _, d := range []string{TemplatesDir, TemplatesTestsDir, ChartsDir} { if err := os.MkdirAll(filepath.Join(cdir, d), 0755); err != nil { return cdir, err } @@ -422,9 +424,9 @@ func Create(chartfile *chart.Metadata, dir string) (string, error) { content: Transform(defaultHelpers, "", chartfile.Name), }, { - // service-test.yaml - path: filepath.Join(cdir, TemplatesDir, ServiceTestName), - content: Transform(defaultServiceTest, "", chartfile.Name), + // test-connection.yaml + path: filepath.Join(cdir, TemplatesTestsDir, TestConnectionName), + content: Transform(defaultTestConnection, "", chartfile.Name), }, } diff --git a/pkg/chartutil/create_test.go b/pkg/chartutil/create_test.go index d4b41fd4b..a0ddf8fa2 100644 --- a/pkg/chartutil/create_test.go +++ b/pkg/chartutil/create_test.go @@ -67,7 +67,7 @@ func TestCreate(t *testing.T) { } } - for _, f := range []string{NotesName, DeploymentName, ServiceName, HelpersName, ServiceTestName} { + for _, f := range []string{NotesName, DeploymentName, ServiceName, HelpersName} { if fi, err := os.Stat(filepath.Join(dir, TemplatesDir, f)); err != nil { t.Errorf("Expected %s file: %s", f, err) } else if fi.IsDir() { @@ -75,6 +75,14 @@ func TestCreate(t *testing.T) { } } + for _, f := range []string{TestConnectionName} { + if fi, err := os.Stat(filepath.Join(dir, TemplatesTestsDir, f)); err != nil { + t.Errorf("Expected %s file: %s", f, err) + } else if fi.IsDir() { + t.Errorf("Expected %s to be a file.", f) + } + } + } func TestCreateFrom(t *testing.T) { diff --git a/pkg/chartutil/save.go b/pkg/chartutil/save.go index 9b85d0714..5d60485bf 100644 --- a/pkg/chartutil/save.go +++ b/pkg/chartutil/save.go @@ -54,7 +54,7 @@ func SaveDir(c *chart.Chart, dest string) error { } } - for _, d := range []string{TemplatesDir, ChartsDir} { + for _, d := range []string{TemplatesDir, ChartsDir, TemplatesTestsDir} { if err := os.MkdirAll(filepath.Join(outdir, d), 0755); err != nil { return err } diff --git a/pkg/urlutil/urlutil_test.go b/pkg/urlutil/urlutil_test.go index b2af24e63..616c4f14f 100644 --- a/pkg/urlutil/urlutil_test.go +++ b/pkg/urlutil/urlutil_test.go @@ -65,8 +65,8 @@ func TestEqual(t *testing.T) { func TestExtractHostname(t *testing.T) { tests := map[string]string{ - "http://example.com": "example.com", - "https://example.com/foo": "example.com", + "http://example.com": "example.com", + "https://example.com/foo": "example.com", "https://example.com:31337/not/with/a/bang/but/a/whimper": "example.com", } for start, expect := range tests {