From 12ace315eaac2e0e1bcc5ba79551dd90f3ebdd3e Mon Sep 17 00:00:00 2001 From: Colin Panisset Date: Thu, 27 Sep 2018 10:38:39 +1000 Subject: [PATCH] Don't assume that the returned error from the storage driver isn't nil (#3625) (#4627) Signed-off-by: Colin Panisset --- pkg/tiller/release_server.go | 21 +++++++++--- pkg/tiller/release_server_test.go | 57 +++++++++++++++++++++++++++++++ 2 files changed, 74 insertions(+), 4 deletions(-) diff --git a/pkg/tiller/release_server.go b/pkg/tiller/release_server.go index e913579aa..e562be203 100644 --- a/pkg/tiller/release_server.go +++ b/pkg/tiller/release_server.go @@ -202,15 +202,28 @@ func (s *ReleaseServer) uniqName(start string, reuse bool) (string, error) { return "", fmt.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) } + moniker := moniker.New() + newname, err := s.createUniqName(moniker) + if err != nil { + return "ERROR", err + } + + s.Log("info: Created new release name %s", newname) + return newname, nil + +} + +func (s *ReleaseServer) createUniqName(m moniker.Namer) (string, error) { maxTries := 5 for i := 0; i < maxTries; i++ { - namer := moniker.New() - name := namer.NameSep("-") + name := m.NameSep("-") if len(name) > releaseNameMaxLen { name = name[:releaseNameMaxLen] } - if _, err := s.env.Releases.Get(name, 1); strings.Contains(err.Error(), "not found") { - return name, nil + if _, err := s.env.Releases.Get(name, 1); err != nil { + if strings.Contains(err.Error(), "not found") { + return name, nil + } } s.Log("info: generated name %s is taken. Searching again.", name) } diff --git a/pkg/tiller/release_server_test.go b/pkg/tiller/release_server_test.go index f3fca7390..b8adb4bb2 100644 --- a/pkg/tiller/release_server_test.go +++ b/pkg/tiller/release_server_test.go @@ -28,6 +28,7 @@ import ( "github.com/ghodss/yaml" "github.com/golang/protobuf/ptypes/timestamp" + "github.com/technosophos/moniker" "golang.org/x/net/context" "google.golang.org/grpc/metadata" "k8s.io/kubernetes/pkg/apis/core" @@ -380,6 +381,62 @@ func TestUniqName(t *testing.T) { } } +type fakeNamer struct { + name string +} + +func NewFakeNamer(nam string) moniker.Namer { + return &fakeNamer{ + name: nam, + } +} + +func (f *fakeNamer) Name() string { + return f.NameSep(" ") +} + +func (f *fakeNamer) NameSep(sep string) string { + return f.name +} + +func TestCreateUniqueName(t *testing.T) { + rs := rsFixture() + + rel1 := releaseStub() + rel1.Name = "happy-panda" + + rs.env.Releases.Create(rel1) + + tests := []struct { + name string + expect string + err bool + }{ + {"happy-panda", "ERROR", true}, + {"wobbly-octopus", "[a-z]+-[a-z]+", false}, + } + + for _, tt := range tests { + m := NewFakeNamer(tt.name) + u, err := rs.createUniqName(m) + if err != nil { + if tt.err { + continue + } + t.Fatal(err) + } + if tt.err { + t.Errorf("Expected an error for %q", tt.name) + } + if match, err := regexp.MatchString(tt.expect, u); err != nil { + t.Fatal(err) + } else if !match { + t.Errorf("Expected %q to match %q", u, tt.expect) + } + } + +} + func releaseWithKeepStub(rlsName string) *release.Release { ch := &chart.Chart{ Metadata: &chart.Metadata{