From a40609443621b46f071d1f2401e46292ad4526be Mon Sep 17 00:00:00 2001 From: Soren Mathiasen Date: Thu, 1 Feb 2018 21:09:05 +0100 Subject: [PATCH 1/2] fix upgrade of broken install If the first `upgrade --install` results in a state FAILED, you can not run the same command `upgrade --install` again without a failure. This happens becuase we are search only for releases with the status DEPLOYED. This change will if the search for DEPLOYED fails, then try to search for a release with the state FAILED, and if found upgrade that. This fixes issue #3353 --- pkg/storage/storage.go | 28 +++++++++++++++----- pkg/storage/storage_test.go | 25 ++++++++++++++++++ pkg/tiller/release_server_test.go | 14 ++++++++++ pkg/tiller/release_update.go | 7 +++-- pkg/tiller/release_update_test.go | 43 +++++++++++++++++++++++++++++++ 5 files changed, 109 insertions(+), 8 deletions(-) diff --git a/pkg/storage/storage.go b/pkg/storage/storage.go index 4ac5dbf4f..e7244001c 100644 --- a/pkg/storage/storage.go +++ b/pkg/storage/storage.go @@ -18,7 +18,6 @@ package storage // import "k8s.io/helm/pkg/storage" import ( "fmt" - "strings" rspb "k8s.io/helm/pkg/proto/hapi/release" relutil "k8s.io/helm/pkg/releaseutil" @@ -128,13 +127,30 @@ func (s *Storage) Deployed(name string) (*rspb.Release, error) { "OWNER": "TILLER", "STATUS": "DEPLOYED", }) - if err == nil { - return ls[0], nil - } - if strings.Contains(err.Error(), "not found") { + if err != nil || len(ls) == 0 { return nil, fmt.Errorf("%q has no deployed releases", name) } - return nil, err + + return ls[0], nil +} + +// Failed returns the failed release with the provided release name, or +// returns ErrReleaseNotFound if not found. +func (s *Storage) Failed(name string) (*rspb.Release, error) { + s.Log("getting failed release from %q history", name) + + ls, err := s.Driver.Query(map[string]string{ + "NAME": name, + "OWNER": "TILLER", + "STATUS": "FAILED", + }) + + if err != nil || len(ls) == 0 { + return nil, fmt.Errorf("%q has no failed releases", name) + } + + return ls[0], nil + } // History returns the revision history for the release with the provided name, or diff --git a/pkg/storage/storage_test.go b/pkg/storage/storage_test.go index fb2824de7..c9ae2622e 100644 --- a/pkg/storage/storage_test.go +++ b/pkg/storage/storage_test.go @@ -325,6 +325,31 @@ func TestStorageLast(t *testing.T) { } } +func TestStorageFailed(t *testing.T) { + storage := Init(driver.NewMemory()) + + const name = "angry-bird" + + // Set up storage with test releases. + setup := func() { + // release records + rls0 := ReleaseTestData{Name: name, Version: 1, Status: rspb.Status_FAILED}.ToRelease() + + // create the release records in the storage + assertErrNil(t.Fatal, storage.Create(rls0), "Storing release 'angry-bird' (v1)") + } + setup() + + h, err := storage.Failed(name) + if err != nil { + t.Fatalf("Failed to query for failed release (%v): %v\n", name, err) + } + if h.Info.Status.Code != rspb.Status_FAILED { + t.Errorf("Expected a failed status, got %s", h.Info.Status.Code) + } + +} + type ReleaseTestData struct { Name string Version int32 diff --git a/pkg/tiller/release_server_test.go b/pkg/tiller/release_server_test.go index ce95acc8f..ba628860a 100644 --- a/pkg/tiller/release_server_test.go +++ b/pkg/tiller/release_server_test.go @@ -309,6 +309,20 @@ func newHookFailingKubeClient() *hookFailingKubeClient { } } +func newInstallFailingKubClient() *installFailingKubeClient { + return &installFailingKubeClient{ + PrintingKubeClient: environment.PrintingKubeClient{Out: os.Stdout}, + } +} + +type installFailingKubeClient struct { + environment.PrintingKubeClient +} + +func (h *installFailingKubeClient) Create(namespace string, reader io.Reader, timeout int64, shouldWait bool) error { + return errors.New("Failed create") +} + type hookFailingKubeClient struct { environment.PrintingKubeClient } diff --git a/pkg/tiller/release_update.go b/pkg/tiller/release_update.go index d251db753..e46d71e7d 100644 --- a/pkg/tiller/release_update.go +++ b/pkg/tiller/release_update.go @@ -72,9 +72,12 @@ func (s *ReleaseServer) prepareUpdate(req *services.UpdateReleaseRequest) (*rele // finds the deployed release with the given name currentRelease, err := s.env.Releases.Deployed(req.Name) if err != nil { - return nil, nil, err + // try to find a failed release we can upgrade + currentRelease, err = s.env.Releases.Failed(req.Name) + if err != nil { + return nil, nil, err + } } - // If new values were not supplied in the upgrade, re-use the existing values. if err := s.reuseValues(req, currentRelease); err != nil { return nil, nil, err diff --git a/pkg/tiller/release_update_test.go b/pkg/tiller/release_update_test.go index 0f2bcbabd..fdd5bd50a 100644 --- a/pkg/tiller/release_update_test.go +++ b/pkg/tiller/release_update_test.go @@ -239,6 +239,49 @@ func TestUpdateReleaseFailure(t *testing.T) { } } +func TestUpdateInstalledFailure(t *testing.T) { + c := helm.NewContext() + rs := rsFixture() + rel := releaseStub() + + rs.env.KubeClient = newInstallFailingKubClient() + rs.Log = t.Logf + + _, err := rs.InstallRelease(c, &services.InstallReleaseRequest{ + Name: rel.Name, + DisableHooks: true, + Chart: &chart.Chart{ + Metadata: &chart.Metadata{Name: "hello"}, + Templates: []*chart.Template{ + {Name: "templates/something", Data: []byte("hello: world")}, + }, + }, + }) + if err == nil { + t.Error("Expected failed install") + } + // check that the first release failed + _, err = rs.env.Releases.Failed(rel.Name) + if err != nil { + t.Fatalf("Failed to get failed: %s", err) + } + + okReq := &services.UpdateReleaseRequest{ + Name: rel.Name, + Chart: &chart.Chart{ + Metadata: &chart.Metadata{Name: "hello"}, + Templates: []*chart.Template{ + {Name: "templates/hello", Data: []byte("hello: world")}, + }, + }, + } + // try to update a release where the previous version was a failure + _, err = rs.UpdateRelease(c, okReq) + if err != nil { + t.Fatalf("Failed updated: %s", err) + } +} + func TestUpdateReleaseNoHooks(t *testing.T) { c := helm.NewContext() rs := rsFixture() From 8527ebc642088f1c857de920f8479a7704e09aee Mon Sep 17 00:00:00 2001 From: Soren Mathiasen Date: Fri, 2 Feb 2018 08:33:23 +0100 Subject: [PATCH 2/2] fixup! fix upgrade of broken install --- pkg/tiller/release_update.go | 11 +++--- pkg/tiller/release_update_test.go | 56 +++++++++++++++++++++++++++++++ 2 files changed, 62 insertions(+), 5 deletions(-) diff --git a/pkg/tiller/release_update.go b/pkg/tiller/release_update.go index e46d71e7d..66563b8ab 100644 --- a/pkg/tiller/release_update.go +++ b/pkg/tiller/release_update.go @@ -71,17 +71,18 @@ func (s *ReleaseServer) prepareUpdate(req *services.UpdateReleaseRequest) (*rele // finds the deployed release with the given name currentRelease, err := s.env.Releases.Deployed(req.Name) - if err != nil { + if err == nil { + // If new values were not supplied in the upgrade, re-use the existing values. + if err := s.reuseValues(req, currentRelease); err != nil { + return nil, nil, err + } + } else { // try to find a failed release we can upgrade currentRelease, err = s.env.Releases.Failed(req.Name) if err != nil { return nil, nil, err } } - // If new values were not supplied in the upgrade, re-use the existing values. - if err := s.reuseValues(req, currentRelease); err != nil { - return nil, nil, err - } // finds the non-deleted release with the given name lastRelease, err := s.env.Releases.Last(req.Name) diff --git a/pkg/tiller/release_update_test.go b/pkg/tiller/release_update_test.go index fdd5bd50a..044ace31b 100644 --- a/pkg/tiller/release_update_test.go +++ b/pkg/tiller/release_update_test.go @@ -282,6 +282,62 @@ func TestUpdateInstalledFailure(t *testing.T) { } } +func TestUpdateInstalledFailureUpdateConfig(t *testing.T) { + c := helm.NewContext() + rs := rsFixture() + rel := releaseStub() + + rs.env.KubeClient = newInstallFailingKubClient() + rs.Log = t.Logf + + instResp, err := rs.InstallRelease(c, &services.InstallReleaseRequest{ + Name: rel.Name, + DisableHooks: true, + Chart: &chart.Chart{ + Metadata: &chart.Metadata{Name: "hello"}, + Templates: []*chart.Template{ + {Name: "templates/something", Data: []byte("hello: world")}, + }, + Values: &chart.Config{ + Values: map[string]*chart.Value{"animal": {Value: "goat"}}, + }, + }, + }) + if err == nil { + t.Error("Expected failed install") + } + if instResp.Release.Chart.Values.Values["animal"].Value != "goat" { + t.Errorf("expected %s in the chart values got %s", "goat", instResp.Release.Chart.Values.Values["animal"].Value) + } + + // check that the first release failed + _, err = rs.env.Releases.Failed(rel.Name) + if err != nil { + t.Fatalf("Failed to get failed: %s", err) + } + + okReq := &services.UpdateReleaseRequest{ + Name: rel.Name, + Chart: &chart.Chart{ + Metadata: &chart.Metadata{Name: "hello"}, + Templates: []*chart.Template{ + {Name: "templates/hello", Data: []byte("hello: world")}, + }, + Values: &chart.Config{ + Values: map[string]*chart.Value{"animal": {Value: "cow"}}, + }, + }, + } + // try to update a release where the previous version was a failure + resp, err := rs.UpdateRelease(c, okReq) + if err != nil { + t.Fatalf("Failed updated: %s", err) + } + if resp.Release.Chart.Values.Values["animal"].Value != "cow" { + t.Errorf("should have override value in chart, after upgrading a failed release") + } +} + func TestUpdateReleaseNoHooks(t *testing.T) { c := helm.NewContext() rs := rsFixture()