ref: require name by default on 'helm install' (#4858)

This is described in the official Helm 3 proposal: https://github.com/helm/community/blob/master/helm-v3/000-helm-v3.md



Signed-off-by: Matt Butcher <matt.butcher@microsoft.com>

Gopkg.lock

@@ -561,14 +561,6 @@
   pruneopts = "UT"
   revision = "e3a8ff8ce36581f87a15341206f205b1da467059"
 
-[[projects]]
-  branch = "master"
-  digest = "1:9123998e9b4a6ed0fcf9cae137a6cd9e265a5d18823e34d1cd12e9d9845b2719"
-  name = "github.com/technosophos/moniker"
-  packages = ["."]
-  pruneopts = "UT"
-  revision = "ab470f5e105a44d0c87ea21bacd6a335c4816d83"
-
 [[projects]]
   digest = "1:9601e4354239b69f62c86d24c74a19d7c7e3c7f7d2d9f01d42e5830b4673e121"
   name = "golang.org/x/crypto"
@@ -1174,7 +1166,6 @@
     "github.com/spf13/cobra/doc",
     "github.com/spf13/pflag",
     "github.com/stretchr/testify/assert",
-    "github.com/technosophos/moniker",
     "golang.org/x/crypto/openpgp",
     "golang.org/x/crypto/openpgp/clearsign",
     "golang.org/x/crypto/openpgp/errors",

Gopkg.toml

@@ -27,10 +27,6 @@
   name = "github.com/gosuri/uitable"
   branch = "master"
 
-[[constraint]]
-  name = "github.com/technosophos/moniker"
-  branch = "master"
-
 [[constraint]]
   name = "k8s.io/api"
   branch = "release-1.12"

Makefile

 BINDIR     := $(CURDIR)/bin
 DIST_DIRS  := find * -type d -exec
 TARGETS    := darwin/amd64 linux/amd64 linux/386 linux/arm linux/arm64 linux/ppc64le windows/amd64
+BINNAME    ?= helm
 
 # go option
 GO         ?= go
@@ -41,12 +42,12 @@ all: build
 
 .PHONY: build
 build:
-	$(GO) build $(GOFLAGS) -tags '$(TAGS)' -ldflags '$(LDFLAGS)' -o $(BINDIR)/helm k8s.io/helm/cmd/helm
+	$(GO) build $(GOFLAGS) -tags '$(TAGS)' -ldflags '$(LDFLAGS)' -o $(BINDIR)/$(BINNAME) k8s.io/helm/cmd/helm
 
 .PHONY: build-cross
 build-cross: LDFLAGS += -extldflags "-static"
 build-cross:
-	CGO_ENABLED=0 gox -parallel=3 -output="_dist/{{.OS}}-{{.Arch}}/{{.Dir}}" -osarch='$(TARGETS)' $(GOFLAGS) -tags '$(TAGS)' -ldflags '$(LDFLAGS)' k8s.io/helm/cmd/helm
+	CGO_ENABLED=0 gox -parallel=3 -output="_dist/{{.OS}}-{{.Arch}}/$(BINNAME)" -osarch='$(TARGETS)' $(GOFLAGS) -tags '$(TAGS)' -ldflags '$(LDFLAGS)' k8s.io/helm/cmd/helm
 
 .PHONY: dist
 dist:

cmd/helm/install.go

@@ -20,8 +20,10 @@ import (
 	"bytes"
 	"fmt"
 	"io"
+	"path/filepath"
 	"strings"
 	"text/template"
+	"time"
 
 	"github.com/Masterminds/sprig"
 	"github.com/pkg/errors"
@@ -46,27 +48,27 @@ To override values in a chart, use either the '--values' flag and pass in a file
 or use the '--set' flag and pass configuration from the command line, to force
 a string value use '--set-string'.
 
-	$ helm install -f myvalues.yaml ./redis
+	$ helm install -f myvalues.yaml myredis ./redis
 
 or
 
-	$ helm install --set name=prod ./redis
+	$ helm install --set name=prod myredis ./redis
 
 or
 
-	$ helm install --set-string long_int=1234567890 ./redis
+	$ helm install --set-string long_int=1234567890 myredis ./redis
 
 You can specify the '--values'/'-f' flag multiple times. The priority will be given to the
 last (right-most) file specified. For example, if both myvalues.yaml and override.yaml
 contained a key called 'Test', the value set in override.yaml would take precedence:
 
-	$ helm install -f myvalues.yaml -f override.yaml ./redis
+	$ helm install -f myvalues.yaml -f override.yaml  myredis ./redis
 
 You can specify the '--set' flag multiple times. The priority will be given to the
 last (right-most) set specified. For example, if both 'bar' and 'newbar' values are
 set for a key called 'foo', the 'newbar' value would take precedence:
 
-	$ helm install --set foo=bar --set foo=newbar ./redis
+	$ helm install --set foo=bar --set foo=newbar  myredis ./redis
 
 
 To check the generated manifests of a release without installing the chart,
@@ -99,7 +101,7 @@ charts in a repository, use 'helm search'.
 `
 
 type installOptions struct {
-	name         string // --name
+	name         string // arg 0
 	dryRun       bool   // --dry-run
 	disableHooks bool   // --disable-hooks
 	replace      bool   // --replace
@@ -108,7 +110,8 @@ type installOptions struct {
 	wait         bool   // --wait
 	devel        bool   // --devel
 	depUp        bool   // --dep-up
-	chartPath    string // arg
+	chartPath    string // arg 1
+	generateName bool   // --generate-name
 
 	valuesOptions
 	chartPathOptions
@@ -120,10 +123,10 @@ func newInstallCmd(c helm.Interface, out io.Writer) *cobra.Command {
 	o := &installOptions{client: c}
 
 	cmd := &cobra.Command{
-		Use:   "install [CHART]",
-		Short: "install a chart archive",
+		Use:   "install [NAME] [CHART]",
+		Short: "install a chart",
 		Long:  installDesc,
-		Args:  require.ExactArgs(1),
+		Args:  require.MinimumNArgs(1),
 		RunE: func(cmd *cobra.Command, args []string) error {
 			debug("Original chart version: %q", o.version)
 			if o.version == "" && o.devel {
@@ -131,7 +134,13 @@ func newInstallCmd(c helm.Interface, out io.Writer) *cobra.Command {
 				o.version = ">0.0.0-0"
 			}
 
-			cp, err := o.locateChart(args[0])
+			name, chart, err := o.nameAndChart(args)
+			if err != nil {
+				return err
+			}
+			o.name = name // FIXME
+
+			cp, err := o.locateChart(chart)
 			if err != nil {
 				return err
 			}
@@ -142,7 +151,7 @@ func newInstallCmd(c helm.Interface, out io.Writer) *cobra.Command {
 	}
 
 	f := cmd.Flags()
-	f.StringVarP(&o.name, "name", "", "", "release name. If unspecified, it will autogenerate one for you")
+	f.BoolVarP(&o.generateName, "generate-name", "g", false, "generate the name (and omit the NAME parameter)")
 	f.BoolVar(&o.dryRun, "dry-run", false, "simulate an install")
 	f.BoolVar(&o.disableHooks, "no-hooks", false, "prevent hooks from running during install")
 	f.BoolVar(&o.replace, "replace", false, "re-use the given name, even if that name is already used. This is unsafe in production")
@@ -157,6 +166,41 @@ func newInstallCmd(c helm.Interface, out io.Writer) *cobra.Command {
 	return cmd
 }
 
+// nameAndChart returns the name of the release and the chart that should be used.
+//
+// This will read the flags and handle name generation if necessary.
+func (o *installOptions) nameAndChart(args []string) (string, string, error) {
+	flagsNotSet := func() error {
+		if o.generateName {
+			return errors.New("cannot set --generate-name and also specify a name")
+		}
+		if o.nameTemplate != "" {
+			return errors.New("cannot set --name-template and also specify a name")
+		}
+		return nil
+	}
+	if len(args) == 2 {
+		return args[0], args[1], flagsNotSet()
+	}
+
+	if o.nameTemplate != "" {
+		newName, err := templateName(o.nameTemplate)
+		return newName, args[0], err
+	}
+
+	if !o.generateName {
+		return "", args[0], errors.New("must either provide a name or specify --generate-name")
+	}
+
+	base := filepath.Base(args[0])
+	if base == "." || base == "" {
+		base = "chart"
+	}
+	newName := fmt.Sprintf("%s-%d", base, time.Now().Unix())
+
+	return newName, args[0], nil
+}
+
 func (o *installOptions) run(out io.Writer) error {
 	debug("CHART PATH: %s\n", o.chartPath)
 
@@ -167,7 +211,7 @@ func (o *installOptions) run(out io.Writer) error {
 
 	// If template is specified, try to run the template.
 	if o.nameTemplate != "" {
-		o.name, err = generateName(o.nameTemplate)
+		o.name, err = templateName(o.nameTemplate)
 		if err != nil {
 			return err
 		}
@@ -276,7 +320,7 @@ func (o *installOptions) printRelease(out io.Writer, rel *release.Release) {
 	}
 }
 
-func generateName(nameTemplate string) (string, error) {
+func templateName(nameTemplate string) (string, error) {
 	t, err := template.New("name-template").Funcs(sprig.TxtFuncMap()).Parse(nameTemplate)
 	if err != nil {
 		return "", err

cmd/helm/install_test.go

@@ -27,37 +27,37 @@ func TestInstall(t *testing.T) {
 		// Install, base case
 		{
 			name:   "basic install",
-			cmd:    "install testdata/testcharts/alpine --name aeneas",
+			cmd:    "install aeneas testdata/testcharts/alpine ",
 			golden: "output/install.txt",
 		},
 		// Install, no hooks
 		{
 			name:   "install without hooks",
-			cmd:    "install testdata/testcharts/alpine --name aeneas --no-hooks",
+			cmd:    "install aeneas testdata/testcharts/alpine --no-hooks",
 			golden: "output/install-no-hooks.txt",
 		},
 		// Install, values from cli
 		{
 			name:   "install with values",
-			cmd:    "install testdata/testcharts/alpine --name virgil --set foo=bar",
+			cmd:    "install virgil testdata/testcharts/alpine --set foo=bar",
 			golden: "output/install-with-values.txt",
 		},
 		// Install, values from cli via multiple --set
 		{
 			name:   "install with multiple values",
-			cmd:    "install testdata/testcharts/alpine --name virgil --set foo=bar --set bar=foo",
+			cmd:    "install virgil testdata/testcharts/alpine --set foo=bar --set bar=foo",
 			golden: "output/install-with-multiple-values.txt",
 		},
 		// Install, values from yaml
 		{
 			name:   "install with values file",
-			cmd:    "install testdata/testcharts/alpine --name virgil -f testdata/testcharts/alpine/extra_values.yaml",
+			cmd:    "install virgil testdata/testcharts/alpine  -f testdata/testcharts/alpine/extra_values.yaml",
 			golden: "output/install-with-values-file.txt",
 		},
 		// Install, values from multiple yaml
 		{
 			name:   "install with values",
-			cmd:    "install testdata/testcharts/alpine --name virgil -f testdata/testcharts/alpine/extra_values.yaml -f testdata/testcharts/alpine/more_values.yaml",
+			cmd:    "install virgil testdata/testcharts/alpine -f testdata/testcharts/alpine/extra_values.yaml -f testdata/testcharts/alpine/more_values.yaml",
 			golden: "output/install-with-multiple-values-files.txt",
 		},
 		// Install, no charts
@@ -70,19 +70,19 @@ func TestInstall(t *testing.T) {
 		// Install, re-use name
 		{
 			name:   "install and replace release",
-			cmd:    "install testdata/testcharts/alpine --name aeneas --replace",
+			cmd:    "install aeneas testdata/testcharts/alpine --replace",
 			golden: "output/install-and-replace.txt",
 		},
 		// Install, with timeout
 		{
 			name:   "install with a timeout",
-			cmd:    "install testdata/testcharts/alpine --name foobar --timeout 120",
+			cmd:    "install foobar testdata/testcharts/alpine --timeout 120",
 			golden: "output/install-with-timeout.txt",
 		},
 		// Install, with wait
 		{
 			name:   "install with a wait",
-			cmd:    "install testdata/testcharts/alpine --name apollo --wait",
+			cmd:    "install apollo testdata/testcharts/alpine --wait",
 			golden: "output/install-with-wait.txt",
 		},
 		// Install, using the name-template
@@ -94,28 +94,28 @@ func TestInstall(t *testing.T) {
 		// Install, perform chart verification along the way.
 		{
 			name:      "install with verification, missing provenance",
-			cmd:       "install testdata/testcharts/compressedchart-0.1.0.tgz --verify --keyring testdata/helm-test-key.pub",
+			cmd:       "install bogus testdata/testcharts/compressedchart-0.1.0.tgz --verify --keyring testdata/helm-test-key.pub",
 			wantError: true,
 		},
 		{
 			name:      "install with verification, directory instead of file",
-			cmd:       "install testdata/testcharts/signtest --verify --keyring testdata/helm-test-key.pub",
+			cmd:       "install bogus testdata/testcharts/signtest --verify --keyring testdata/helm-test-key.pub",
 			wantError: true,
 		},
 		{
 			name: "install with verification, valid",
-			cmd:  "install testdata/testcharts/signtest-0.1.0.tgz --verify --keyring testdata/helm-test-key.pub",
+			cmd:  "install signtest testdata/testcharts/signtest-0.1.0.tgz --verify --keyring testdata/helm-test-key.pub",
 		},
 		// Install, chart with missing dependencies in /charts
 		{
 			name:      "install chart with missing dependencies",
-			cmd:       "install testdata/testcharts/chart-missing-deps",
+			cmd:       "install nodeps testdata/testcharts/chart-missing-deps",
 			wantError: true,
 		},
 		// Install, chart with bad dependencies in Chart.yaml in /charts
 		{
 			name:      "install chart with bad  dependencies in Chart.yaml",
-			cmd:       "install testdata/testcharts/chart-bad-requirements",
+			cmd:       "install badreq testdata/testcharts/chart-bad-requirements",
 			wantError: true,
 		},
 	}
@@ -165,7 +165,7 @@ func TestNameTemplate(t *testing.T) {
 
 	for _, tc := range testCases {
 
-		n, err := generateName(tc.tpl)
+		n, err := templateName(tc.tpl)
 		if err != nil {
 			if tc.expectedErrorStr == "" {
 				t.Errorf("Was not expecting error, but got: %v", err)

cmd/helm/template.go

@@ -146,7 +146,7 @@ func (o *templateOptions) run(out io.Writer) error {
 
 	// If template is specified, try to run the template.
 	if o.nameTemplate != "" {
-		o.releaseName, err = generateName(o.nameTemplate)
+		o.releaseName, err = templateName(o.nameTemplate)
 		if err != nil {
 			return err
 		}

cmd/helm/testdata/output/install-no-args.txt

-Error: "helm install" requires 1 argument
+Error: "helm install" requires at least 1 argument
 
-Usage:  helm install [CHART] [flags]
+Usage:  helm install [NAME] [CHART] [flags]

pkg/tiller/release_install_test.go

@@ -28,12 +28,12 @@ import (
 func TestInstallRelease(t *testing.T) {
 	rs := rsFixture(t)
 
-	req := installRequest()
+	req := installRequest(withName("test-install-release"))
 	res, err := rs.InstallRelease(req)
 	if err != nil {
 		t.Fatalf("Failed install: %s", err)
 	}
-	if res.Name == "" {
+	if res.Name != "test-install-release" {
 		t.Errorf("Expected release name.")
 	}
 	if res.Namespace != "spaced" {
@@ -78,11 +78,26 @@ func TestInstallRelease(t *testing.T) {
 	}
 }
 
+func TestInstallRelease_NoName(t *testing.T) {
+	rs := rsFixture(t)
+
+	// No name supplied here, should cause failure.
+	req := installRequest()
+	_, err := rs.InstallRelease(req)
+	if err == nil {
+		t.Fatal("expected failure when no name is specified")
+	}
+	if !strings.Contains(err.Error(), "name is required") {
+		t.Errorf("Expected message %q to include 'name is required'", err.Error())
+	}
+}
+
 func TestInstallRelease_WithNotes(t *testing.T) {
 	rs := rsFixture(t)
 
 	req := installRequest(
 		withChart(withNotes(notesText)),
+		withName("with-notes"),
 	)
 	res, err := rs.InstallRelease(req)
 	if err != nil {
@@ -141,7 +156,8 @@ func TestInstallRelease_WithNotesRendered(t *testing.T) {
 	rs := rsFixture(t)
 
 	req := installRequest(
-		withChart(withNotes(notesText + " {{.Release.Name}}")),
+		withChart(withNotes(notesText+" {{.Release.Name}}")),
+		withName("with-notes"),
 	)
 	res, err := rs.InstallRelease(req)
 	if err != nil {
@@ -203,7 +219,7 @@ func TestInstallRelease_WithChartAndDependencyNotes(t *testing.T) {
 	req := installRequest(withChart(
 		withNotes(notesText),
 		withDependency(withNotes(notesText+" child")),
-	))
+	), withName("with-chart-and-dependency-notes"))
 	res, err := rs.InstallRelease(req)
 	if err != nil {
 		t.Fatalf("Failed install: %s", err)
@@ -233,13 +249,14 @@ func TestInstallRelease_DryRun(t *testing.T) {
 
 	req := installRequest(withDryRun(),
 		withChart(withSampleTemplates()),
+		withName("test-dry-run"),
 	)
 	res, err := rs.InstallRelease(req)
 	if err != nil {
 		t.Errorf("Failed install: %s", err)
 	}
-	if res.Name == "" {
-		t.Errorf("Expected release name.")
+	if res.Name != "test-dry-run" {
+		t.Errorf("unexpected release name: %q", res.Name)
 	}
 
 	if !strings.Contains(res.Manifest, "---\n# Source: hello/templates/hello\nhello: world") {
@@ -283,7 +300,7 @@ func TestInstallRelease_NoHooks(t *testing.T) {
 	rs := rsFixture(t)
 	rs.Releases.Create(releaseStub())
 
-	req := installRequest(withDisabledHooks())
+	req := installRequest(withDisabledHooks(), withName("no-hooks"))
 	res, err := rs.InstallRelease(req)
 	if err != nil {
 		t.Errorf("Failed install: %s", err)
@@ -299,7 +316,7 @@ func TestInstallRelease_FailedHooks(t *testing.T) {
 	rs.Releases.Create(releaseStub())
 	rs.KubeClient = newHookFailingKubeClient()
 
-	req := installRequest()
+	req := installRequest(withName("failed-hooks"))
 	res, err := rs.InstallRelease(req)
 	if err == nil {
 		t.Error("Expected failed install")
@@ -345,6 +362,7 @@ func TestInstallRelease_KubeVersion(t *testing.T) {
 
 	req := installRequest(
 		withChart(withKube(">=0.0.0")),
+		withName("kube-version"),
 	)
 	_, err := rs.InstallRelease(req)
 	if err != nil {
@@ -357,6 +375,7 @@ func TestInstallRelease_WrongKubeVersion(t *testing.T) {
 
 	req := installRequest(
 		withChart(withKube(">=5.0.0")),
+		withName("wrong-kube-version"),
 	)
 
 	_, err := rs.InstallRelease(req)

pkg/tiller/release_server.go

@@ -25,7 +25,6 @@ import (
 	"time"
 
 	"github.com/pkg/errors"
-	"github.com/technosophos/moniker"
 	"gopkg.in/yaml.v2"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/client-go/discovery"
@@ -160,47 +159,31 @@ func (s *ReleaseServer) reuseValues(req *hapi.UpdateReleaseRequest, current *rel
 
 func (s *ReleaseServer) uniqName(start string, reuse bool) (string, error) {
 
-	// If a name is supplied, we check to see if that name is taken. If not, it
-	// is granted. If reuse is true and a deleted release with that name exists,
-	// we re-grant it. Otherwise, an error is returned.
-	if start != "" {
-
-		if len(start) > releaseNameMaxLen {
-			return "", errors.Errorf("release name %q exceeds max length of %d", start, releaseNameMaxLen)
-		}
-
-		h, err := s.Releases.History(start)
-		if err != nil || len(h) < 1 {
-			return start, nil
-		}
-		relutil.Reverse(h, relutil.SortByRevision)
-		rel := h[0]
-
-		if st := rel.Info.Status; reuse && (st == release.StatusUninstalled || st == release.StatusFailed) {
-			// Allowe re-use of names if the previous release is marked deleted.
-			s.Log("name %s exists but is not in use, reusing name", start)
-			return start, nil
-		} else if reuse {
-			return "", errors.New("cannot re-use a name that is still in use")
-		}
+	if start == "" {
+		return "", errors.New("name is required")
+	}
 
-		return "", errors.Errorf("a release named %s already exists.\nRun: helm ls --all %s; to check the status of the release\nOr run: helm del --purge %s; to delete it", start, start, start)
+	if len(start) > releaseNameMaxLen {
+		return "", errors.Errorf("release name %q exceeds max length of %d", start, releaseNameMaxLen)
 	}
 
-	maxTries := 5
-	for i := 0; i < maxTries; i++ {
-		namer := moniker.New()
-		name := namer.NameSep("-")
-		if len(name) > releaseNameMaxLen {
-			name = name[:releaseNameMaxLen]
-		}
-		if _, err := s.Releases.Get(name, 1); strings.Contains(err.Error(), "not found") {
-			return name, nil
-		}
-		s.Log("info: generated name %s is taken. Searching again.", name)
+	h, err := s.Releases.History(start)
+	if err != nil || len(h) < 1 {
+		return start, nil
+	}
+	relutil.Reverse(h, relutil.SortByRevision)
+	rel := h[0]
+
+	if st := rel.Info.Status; reuse && (st == release.StatusUninstalled || st == release.StatusFailed) {
+		// Allowe re-use of names if the previous release is marked deleted.
+		s.Log("name %s exists but is not in use, reusing name", start)
+		return start, nil
+	} else if reuse {
+		return "", errors.New("cannot re-use a name that is still in use")
 	}
-	s.Log("warning: No available release names found after %d tries", maxTries)
-	return "ERROR", errors.New("no available release name found")
+
+	return "", errors.Errorf("a release named %s already exists.\nRun: helm ls --all %s; to check the status of the release\nOr run: helm del --purge %s; to delete it", start, start, start)
+
 }
 
 // capabilities builds a Capabilities from discovery information.

pkg/tiller/release_server_test.go

@@ -334,8 +334,8 @@ func TestUniqName(t *testing.T) {
 		reuse  bool
 		err    bool
 	}{
+		{"", "", false, true}, // Blank name is illegal
 		{"first", "first", false, false},
-		{"", "[a-z]+-[a-z]+", false, false},
 		{"angry-panda", "", false, true},
 		{"happy-panda", "", false, true},
 		{"happy-panda", "happy-panda", true, false},

pkg/tiller/release_update_test.go

@@ -130,6 +130,7 @@ func TestUpdateRelease_ComplexReuseValues(t *testing.T) {
 	rs := rsFixture(t)
 
 	installReq := &hapi.InstallReleaseRequest{
+		Name:      "complex-reuse-values",
 		Namespace: "spaced",
 		Chart: &chart.Chart{
 			Metadata: &chart.Metadata{Name: "hello"},