fix(helm): allow a previously failed release to be upgraded (#7653)

Signed-off-by: Matt Morrissette <yinzara@gmail.com>

cmd/helm/testdata/output/upgrade-with-bad-or-missing-existing-release.txt

@@ -0,0 +1 @@
+Error: UPGRADE FAILED: "funny-bunny" has no deployed releases

cmd/helm/upgrade_test.go

@@ -80,6 +80,10 @@ func TestUpgradeCmd(t *testing.T) {
 	missingDepsPath := "testdata/testcharts/chart-missing-deps"
 	badDepsPath := "testdata/testcharts/chart-bad-requirements"
 
+	relWithStatusMock := func(n string, v int, ch *chart.Chart, status release.Status) *release.Release {
+		return release.Mock(&release.MockReleaseOptions{Name: n, Version: v, Chart: ch, Status: status})
+	}
+
 	relMock := func(n string, v int, ch *chart.Chart) *release.Release {
 		return release.Mock(&release.MockReleaseOptions{Name: n, Version: v, Chart: ch})
 	}
@@ -139,6 +143,25 @@ func TestUpgradeCmd(t *testing.T) {
 			golden:    "output/upgrade-with-bad-dependencies.txt",
 			wantError: true,
 		},
+		{
+			name:      "upgrade a non-existent release",
+			cmd:       fmt.Sprintf("upgrade funny-bunny '%s'", chartPath),
+			golden:    "output/upgrade-with-bad-or-missing-existing-release.txt",
+			wantError: true,
+		},
+		{
+			name:   "upgrade a failed release",
+			cmd:    fmt.Sprintf("upgrade funny-bunny '%s'", chartPath),
+			golden: "output/upgrade.txt",
+			rels:   []*release.Release{relWithStatusMock("funny-bunny", 2, ch, release.StatusFailed)},
+		},
+		{
+			name:      "upgrade a pending install release",
+			cmd:       fmt.Sprintf("upgrade funny-bunny '%s'", chartPath),
+			golden:    "output/upgrade-with-bad-or-missing-existing-release.txt",
+			wantError: true,
+			rels:      []*release.Release{relWithStatusMock("funny-bunny", 2, ch, release.StatusPendingInstall)},
+		},
 	}
 	runTestCmd(t, tests)
 }

pkg/action/upgrade.go

@@ -33,6 +33,7 @@ import (
 	"helm.sh/helm/v3/pkg/postrender"
 	"helm.sh/helm/v3/pkg/release"
 	"helm.sh/helm/v3/pkg/releaseutil"
+	"helm.sh/helm/v3/pkg/storage/driver"
 )
 
 // Upgrade is the action for upgrading releases.
@@ -159,12 +160,33 @@ func (u *Upgrade) prepareUpgrade(name string, chart *chart.Chart, vals map[strin
 		return nil, nil, errMissingChart
 	}
 
-	// finds the deployed release with the given name
-	currentRelease, err := u.cfg.Releases.Deployed(name)
+	// finds the last non-deleted release with the given name
+	lastRelease, err := u.cfg.Releases.Last(name)
 	if err != nil {
+		// to keep existing behavior of returning the "%q has no deployed releases" error when an existing release does not exist
+		if errors.Is(err, driver.ErrReleaseNotFound) {
+			return nil, nil, driver.NewErrNoDeployedReleases(name)
+		}
 		return nil, nil, err
 	}
 
+	var currentRelease *release.Release
+	if lastRelease.Info.Status == release.StatusDeployed {
+		// no need to retrieve the last deployed release from storage as the last release is deployed
+		currentRelease = lastRelease
+	} else {
+		// finds the deployed release with the given name
+		currentRelease, err = u.cfg.Releases.Deployed(name)
+		if err != nil {
+			if errors.Is(err, driver.ErrNoDeployedReleases) &&
+				(lastRelease.Info.Status == release.StatusFailed || lastRelease.Info.Status == release.StatusSuperseded) {
+				currentRelease = lastRelease
+			} else {
+				return nil, nil, err
+			}
+		}
+	}
+
 	// determine if values will be reused
 	vals, err = u.reuseValues(chart, currentRelease, vals)
 	if err != nil {
@@ -175,12 +197,6 @@ func (u *Upgrade) prepareUpgrade(name string, chart *chart.Chart, vals map[strin
 		return nil, nil, err
 	}
 
-	// finds the non-deleted release with the given name
-	lastRelease, err := u.cfg.Releases.Last(name)
-	if err != nil {
-		return nil, nil, err
-	}
-
 	// Increment revision count. This is passed to templates, and also stored on
 	// the release object.
 	revision := lastRelease.Version + 1

pkg/storage/driver/driver.go

@@ -17,6 +17,8 @@ limitations under the License.
 package driver // import "helm.sh/helm/v3/pkg/storage/driver"
 
 import (
+	"fmt"
+
 	"github.com/pkg/errors"
 
 	rspb "helm.sh/helm/v3/pkg/release"
@@ -28,9 +30,30 @@ var (
 	// ErrReleaseExists indicates that a release already exists.
 	ErrReleaseExists = errors.New("release: already exists")
 	// ErrInvalidKey indicates that a release key could not be parsed.
-	ErrInvalidKey = errors.Errorf("release: invalid key")
+	ErrInvalidKey = errors.New("release: invalid key")
+	// ErrNoDeployedReleases indicates that there are no releases with the given key in the deployed state
+	ErrNoDeployedReleases = errors.New("has no deployed releases")
 )
 
+// StorageDriverError records an error and the release name that caused it
+type StorageDriverError struct {
+	ReleaseName string
+	Err         error
+}
+
+func (e *StorageDriverError) Error() string {
+	return fmt.Sprintf("%q %s", e.ReleaseName, e.Err.Error())
+}
+
+func (e *StorageDriverError) Unwrap() error { return e.Err }
+
+func NewErrNoDeployedReleases(releaseName string) error {
+	return &StorageDriverError{
+		ReleaseName: releaseName,
+		Err:         ErrNoDeployedReleases,
+	}
+}
+
 // Creator is the interface that wraps the Create method.
 //
 // Create stores the release or returns ErrReleaseExists

pkg/storage/storage.go

@@ -116,7 +116,7 @@ func (s *Storage) Deployed(name string) (*rspb.Release, error) {
 	}
 
 	if len(ls) == 0 {
-		return nil, errors.Errorf("%q has no deployed releases", name)
+		return nil, driver.NewErrNoDeployedReleases(name)
 	}
 
 	// If executed concurrently, Helm's database gets corrupted
@@ -140,7 +140,7 @@ func (s *Storage) DeployedAll(name string) ([]*rspb.Release, error) {
 		return ls, nil
 	}
 	if strings.Contains(err.Error(), "not found") {
-		return nil, errors.Errorf("%q has no deployed releases", name)
+		return nil, driver.NewErrNoDeployedReleases(name)
 	}
 	return nil, err
 }