From da8e7d25329f9539cf034588d5638099fd69fda9 Mon Sep 17 00:00:00 2001 From: Timofey Kirillov Date: Tue, 31 Aug 2021 17:14:54 +0300 Subject: [PATCH] fix: "... has no deployed releases" error when release history contains only failed releases and history limit reached Fixed old releases rotation procedure to not require a deployed release to exists. An error will arise when there are no successfully deployed release yet, but releases history limit has been reached. In such situation helm will refuse to upgrade release anymore with "... has no deployed releases" error. Furthermore, release rotation procedure already expecting lastDeployedRelease to be either nil, or not nil. So it is assumed that deployed release may exist or may not and these both outcomes were already expected as a valid situation rather than a failure. Reworked storage_test.go TestStorageRemoveLeastRecentWithError test case: use mocked driver and test release creation procedure does not shadows errors from the underneath release rotation procedure. Signed-off-by: Timofey Kirillov --- pkg/storage/storage.go | 2 +- pkg/storage/storage_test.go | 97 +++++++++++++++++++++++++++++++++++-- 2 files changed, 95 insertions(+), 4 deletions(-) diff --git a/pkg/storage/storage.go b/pkg/storage/storage.go index 370fec4b4..0a18b34a0 100644 --- a/pkg/storage/storage.go +++ b/pkg/storage/storage.go @@ -177,7 +177,7 @@ func (s *Storage) removeLeastRecent(name string, max int) error { relutil.SortByRevision(h) lastDeployed, err := s.Deployed(name) - if err != nil { + if err != nil && !errors.Is(err, driver.ErrNoDeployedReleases) { return err } diff --git a/pkg/storage/storage_test.go b/pkg/storage/storage_test.go index ac79ca2fd..058b077e8 100644 --- a/pkg/storage/storage_test.go +++ b/pkg/storage/storage_test.go @@ -278,8 +278,40 @@ func TestStorageHistory(t *testing.T) { } } -func TestStorageRemoveLeastRecentWithError(t *testing.T) { - storage := Init(driver.NewMemory()) +var errMaxHistoryMockDriverSomethingHappened = errors.New("something happened") + +type MaxHistoryMockDriver struct { + Driver driver.Driver +} + +func NewMaxHistoryMockDriver(d driver.Driver) *MaxHistoryMockDriver { + return &MaxHistoryMockDriver{Driver: d} +} +func (d *MaxHistoryMockDriver) Create(key string, rls *rspb.Release) error { + return d.Driver.Create(key, rls) +} +func (d *MaxHistoryMockDriver) Update(key string, rls *rspb.Release) error { + return d.Driver.Update(key, rls) +} +func (d *MaxHistoryMockDriver) Delete(key string) (*rspb.Release, error) { + return nil, errMaxHistoryMockDriverSomethingHappened +} +func (d *MaxHistoryMockDriver) Get(key string) (*rspb.Release, error) { + return d.Driver.Get(key) +} +func (d *MaxHistoryMockDriver) List(filter func(*rspb.Release) bool) ([]*rspb.Release, error) { + return d.Driver.List(filter) +} +func (d *MaxHistoryMockDriver) Query(labels map[string]string) ([]*rspb.Release, error) { + return d.Driver.Query(labels) +} +func (d *MaxHistoryMockDriver) Name() string { + return d.Driver.Name() +} + +func TestMaxHistoryErrorHandling(t *testing.T) { + //func TestStorageRemoveLeastRecentWithError(t *testing.T) { + storage := Init(NewMaxHistoryMockDriver(driver.NewMemory())) storage.Log = t.Logf storage.MaxHistory = 1 @@ -297,7 +329,7 @@ func TestStorageRemoveLeastRecentWithError(t *testing.T) { setup() rls2 := ReleaseTestData{Name: name, Version: 2, Status: rspb.StatusSuperseded}.ToRelease() - wantErr := driver.ErrNoDeployedReleases + wantErr := errMaxHistoryMockDriverSomethingHappened gotErr := storage.Create(rls2) if !errors.Is(gotErr, wantErr) { t.Fatalf("Storing release 'angry-bird' (v2) should return the error %#v, but returned %#v", wantErr, gotErr) @@ -444,6 +476,65 @@ func TestStorageLast(t *testing.T) { } } +// TestUpgradeInitiallyFailedRelease tests a case when there are no deployed release yet, but history limit has been +// reached: the has-no-deployed-releases error should not occur in such case. +func TestUpgradeInitiallyFailedReleaseWithHistoryLimit(t *testing.T) { + storage := Init(driver.NewMemory()) + storage.MaxHistory = 4 + + const name = "angry-bird" + + // setup storage with test releases + setup := func() { + // release records + rls0 := ReleaseTestData{Name: name, Version: 1, Status: rspb.StatusFailed}.ToRelease() + rls1 := ReleaseTestData{Name: name, Version: 2, Status: rspb.StatusFailed}.ToRelease() + rls2 := ReleaseTestData{Name: name, Version: 3, Status: rspb.StatusFailed}.ToRelease() + rls3 := ReleaseTestData{Name: name, Version: 4, Status: rspb.StatusFailed}.ToRelease() + + // create the release records in the storage + assertErrNil(t.Fatal, storage.Create(rls0), "Storing release 'angry-bird' (v1)") + assertErrNil(t.Fatal, storage.Create(rls1), "Storing release 'angry-bird' (v2)") + assertErrNil(t.Fatal, storage.Create(rls2), "Storing release 'angry-bird' (v3)") + assertErrNil(t.Fatal, storage.Create(rls3), "Storing release 'angry-bird' (v4)") + + hist, err := storage.History(name) + if err != nil { + t.Fatalf("unexpected error: %s", err) + } + + wantHistoryLen := 4 + if len(hist) != wantHistoryLen { + t.Fatalf("expected history of release %q to contain %d releases, got %d", name, wantHistoryLen, len(hist)) + } + } + + setup() + + rls5 := ReleaseTestData{Name: name, Version: 5, Status: rspb.StatusFailed}.ToRelease() + err := storage.Create(rls5) + if err != nil { + t.Fatalf("Failed to create a new release version: %s", err) + } + + hist, err := storage.History(name) + if err != nil { + t.Fatalf("unexpected error: %s", err) + } + + for i, rel := range hist { + wantVersion := i + 2 + if rel.Version != wantVersion { + t.Fatalf("Expected history release %d version to equal %d, got %d", i+1, wantVersion, rel.Version) + } + + wantStatus := rspb.StatusFailed + if rel.Info.Status != wantStatus { + t.Fatalf("Expected history release %d status to equal %q, got %q", i+1, wantStatus, rel.Info.Status) + } + } +} + type ReleaseTestData struct { Name string Version int