diff --git a/cmd/helm/testdata/output/upgrade-with-bad-or-missing-existing-release.txt b/cmd/helm/testdata/output/upgrade-with-bad-or-missing-existing-release.txt new file mode 100644 index 000000000..8f24574a6 --- /dev/null +++ b/cmd/helm/testdata/output/upgrade-with-bad-or-missing-existing-release.txt @@ -0,0 +1 @@ +Error: UPGRADE FAILED: "funny-bunny" has no deployed releases diff --git a/cmd/helm/upgrade_test.go b/cmd/helm/upgrade_test.go index 3a6d75adc..6f260ae57 100644 --- a/cmd/helm/upgrade_test.go +++ b/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) } diff --git a/pkg/action/upgrade.go b/pkg/action/upgrade.go index 7565da5a9..67872aa2f 100644 --- a/pkg/action/upgrade.go +++ b/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 diff --git a/pkg/storage/driver/driver.go b/pkg/storage/driver/driver.go index 9a1fbc579..9c01f3766 100644 --- a/pkg/storage/driver/driver.go +++ b/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 diff --git a/pkg/storage/storage.go b/pkg/storage/storage.go index 3e62ae9ee..c195120cd 100644 --- a/pkg/storage/storage.go +++ b/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 } diff --git a/scripts/get b/scripts/get index 3da11d4a4..afa02bbb1 100755 --- a/scripts/get +++ b/scripts/get @@ -82,9 +82,9 @@ checkDesiredVersion() { local release_url="https://github.com/helm/helm/releases" if type "curl" > /dev/null; then - TAG=$(curl -Ls $release_url | grep 'href="/helm/helm/releases/tag/v2.' | grep -v no-underline | head -n 1 | cut -d '"' -f 2 | awk '{n=split($NF,a,"/");print a[n]}' | awk 'a !~ $0{print}; {a=$0}') + TAG=$(curl -Ls $release_url | grep 'href="/helm/helm/releases/tag/v2.[0-9]*.[0-9]*\"' | grep -v no-underline | head -n 1 | cut -d '"' -f 2 | awk '{n=split($NF,a,"/");print a[n]}' | awk 'a !~ $0{print}; {a=$0}') elif type "wget" > /dev/null; then - TAG=$(wget $release_url -O - 2>&1 | grep 'href="/helm/helm/releases/tag/v2.' | grep -v no-underline | head -n 1 | cut -d '"' -f 2 | awk '{n=split($NF,a,"/");print a[n]}' | awk 'a !~ $0{print}; {a=$0}') + TAG=$(wget $release_url -O - 2>&1 | grep 'href="/helm/helm/releases/tag/v2.[0-9]*.[0-9]*\"' | grep -v no-underline | head -n 1 | cut -d '"' -f 2 | awk '{n=split($NF,a,"/");print a[n]}' | awk 'a !~ $0{print}; {a=$0}') fi else TAG=$DESIRED_VERSION diff --git a/scripts/get-helm-3 b/scripts/get-helm-3 index 310c4a00c..c5993b985 100755 --- a/scripts/get-helm-3 +++ b/scripts/get-helm-3 @@ -88,9 +88,9 @@ checkDesiredVersion() { # Get tag from release URL local latest_release_url="https://github.com/helm/helm/releases" if [ "${HAS_CURL}" == "true" ]; then - TAG=$(curl -Ls $latest_release_url | grep 'href="/helm/helm/releases/tag/v3.' | grep -v no-underline | head -n 1 | cut -d '"' -f 2 | awk '{n=split($NF,a,"/");print a[n]}' | awk 'a !~ $0{print}; {a=$0}') + TAG=$(curl -Ls $latest_release_url | grep 'href="/helm/helm/releases/tag/v3.[0-9]*.[0-9]*\"' | grep -v no-underline | head -n 1 | cut -d '"' -f 2 | awk '{n=split($NF,a,"/");print a[n]}' | awk 'a !~ $0{print}; {a=$0}') elif [ "${HAS_WGET}" == "true" ]; then - TAG=$(wget $latest_release_url -O - 2>&1 | grep 'href="/helm/helm/releases/tag/v3.' | grep -v no-underline | head -n 1 | cut -d '"' -f 2 | awk '{n=split($NF,a,"/");print a[n]}' | awk 'a !~ $0{print}; {a=$0}') + TAG=$(wget $latest_release_url -O - 2>&1 | grep 'href="/helm/helm/releases/tag/v3.[0-9]*.[0-9]*\"' | grep -v no-underline | head -n 1 | cut -d '"' -f 2 | awk '{n=split($NF,a,"/");print a[n]}' | awk 'a !~ $0{print}; {a=$0}') fi else TAG=$DESIRED_VERSION @@ -163,12 +163,14 @@ verifyChecksum() { echo "Please install openssl or set VERIFY_CHECKSUM=false in your environment." exit 1 fi + printf "Verifying checksum... " local sum=$(openssl sha1 -sha256 ${HELM_TMP_FILE} | awk '{print $2}') local expected_sum=$(cat ${HELM_SUM_FILE}) if [ "$sum" != "$expected_sum" ]; then echo "SHA sum of ${HELM_TMP_FILE} does not match. Aborting." exit 1 fi + echo "Done." } # verifySignatures obtains the KEYS and signature .asc files from GitHub, @@ -179,6 +181,7 @@ verifySignatures() { echo "Please install gpg or set VERIFY_SIGNATURES=false in your environment." exit 1 fi + printf "Verifying signatures... " local keys_filename="KEYS" local github_keys_url="https://raw.githubusercontent.com/helm/helm/master/${keys_filename}" if [ "${HAS_CURL}" == "true" ]; then @@ -186,7 +189,11 @@ verifySignatures() { elif [ "${HAS_WGET}" == "true" ]; then wget -q -O "${github_keys_url}" "${HELM_TMP_ROOT}/${keys_filename}" fi - gpg --import "${HELM_TMP_ROOT}/${keys_filename}" + local gpg_stderr_device="/dev/null" + if [ "${DEBUG}" == "true" ]; then + gpg_stderr_device="/dev/stderr" + fi + gpg --import "${HELM_TMP_ROOT}/${keys_filename}" 2> "${gpg_stderr_device}" local github_release_url="https://github.com/helm/helm/releases/download/${TAG}" if [ "${HAS_CURL}" == "true" ]; then curl -SsL "${github_release_url}/helm-${TAG}-${OS}-${ARCH}.tar.gz.sha256.asc" -o "${HELM_TMP_ROOT}/helm-${TAG}-${OS}-${ARCH}.tar.gz.sha256.asc" @@ -197,16 +204,17 @@ verifySignatures() { fi local error_text="Double-check the PGP key provided. If you think this is a security issue," error_text="${error_text}\nplease see here: https://github.com/helm/community/blob/master/SECURITY.md" - if ! gpg --verify "${HELM_TMP_ROOT}/helm-${TAG}-${OS}-${ARCH}.tar.gz.sha256.asc"; then + if ! gpg --verify "${HELM_TMP_ROOT}/helm-${TAG}-${OS}-${ARCH}.tar.gz.sha256.asc" 2> "${gpg_stderr_device}"; then echo "Unable to verify the signature of helm-${TAG}-${OS}-${ARCH}.tar.gz.sha256!" echo -e "${error_text}" exit 1 fi - if ! gpg --verify "${HELM_TMP_ROOT}/helm-${TAG}-${OS}-${ARCH}.tar.gz.asc"; then + if ! gpg --verify "${HELM_TMP_ROOT}/helm-${TAG}-${OS}-${ARCH}.tar.gz.asc" 2> "${gpg_stderr_device}"; then echo "Unable to verify the signature of helm-${TAG}-${OS}-${ARCH}.tar.gz!" echo -e "${error_text}" exit 1 fi + echo "Done." } # fail_trap is executed if an error occurs.