Skip to content

Commit

Permalink
Do not uninstall helm chart after failed install or upgrade
Browse files Browse the repository at this point in the history
  • Loading branch information
Lucas Rodriguez committed Apr 23, 2024
1 parent 087a4a3 commit 98f1562
Show file tree
Hide file tree
Showing 2 changed files with 8 additions and 11 deletions.
6 changes: 0 additions & 6 deletions src/internal/packager/helm/chart.go
Original file line number Diff line number Diff line change
Expand Up @@ -115,12 +115,6 @@ func (h *Helm) InstallOrUpgradeChart() (types.ConnectStrings, string, error) {
return nil, "", fmt.Errorf("unable to upgrade chart after %d attempts", h.retries)
}

spinner.Updatef("Performing chart uninstall")
_, err = h.uninstallChart(h.chart.ReleaseName)
if err != nil {
return nil, "", fmt.Errorf("unable to install chart after %d attempts and unable to uninstall: %w", h.retries, err)
}

return nil, "", fmt.Errorf("unable to install chart after %d attempts", h.retries)
}

Expand Down
13 changes: 8 additions & 5 deletions src/test/e2e/25_helm_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -130,16 +130,19 @@ func testHelmUninstallRollback(t *testing.T) {
// This package contains SBOMable things but was created with --skip-sbom
require.Contains(t, string(stdErr), "This package does NOT contain an SBOM.")

// Ensure that this does not leave behind a dos-games chart
// Ensure this leaves behind a dos-games chart.
// We do not want to uninstall charts that had failed installs/upgrades
// to prevent unintentional deletion and/or data loss in production environments.
// https://github.com/defenseunicorns/zarf/issues/2455
helmOut, err := exec.Command("helm", "list", "-n", "dos-games").Output()
require.NoError(t, err)
require.NotContains(t, string(helmOut), "zarf-f53a99d4a4dd9a3575bedf59cd42d48d751ae866")
require.Contains(t, string(helmOut), "zarf-f53a99d4a4dd9a3575bedf59cd42d48d751ae866")

// Deploy the good package.
stdOut, stdErr, err = e2e.Zarf("package", "deploy", goodPath, "--confirm")
require.NoError(t, err, stdOut, stdErr)

// Ensure that this does create a dos-games chart
// Ensure this upgrades/fixes the dos-games chart.
helmOut, err = exec.Command("helm", "list", "-n", "dos-games").Output()
require.NoError(t, err)
require.Contains(t, string(helmOut), "zarf-f53a99d4a4dd9a3575bedf59cd42d48d751ae866")
Expand All @@ -151,7 +154,7 @@ func testHelmUninstallRollback(t *testing.T) {
// Ensure that we rollback properly
helmOut, err = exec.Command("helm", "history", "-n", "dos-games", "zarf-f53a99d4a4dd9a3575bedf59cd42d48d751ae866", "--max", "1").Output()
require.NoError(t, err)
require.Contains(t, string(helmOut), "Rollback to 1")
require.Contains(t, string(helmOut), "Rollback to 4")

// Deploy the evil package (again to ensure we check full history)
stdOut, stdErr, err = e2e.Zarf("package", "deploy", evilPath, "--timeout", "10s", "--confirm")
Expand All @@ -160,7 +163,7 @@ func testHelmUninstallRollback(t *testing.T) {
// Ensure that we rollback properly
helmOut, err = exec.Command("helm", "history", "-n", "dos-games", "zarf-f53a99d4a4dd9a3575bedf59cd42d48d751ae866", "--max", "1").Output()
require.NoError(t, err)
require.Contains(t, string(helmOut), "Rollback to 5")
require.Contains(t, string(helmOut), "Rollback to 8")

// Remove the package.
stdOut, stdErr, err = e2e.Zarf("package", "remove", "dos-games", "--confirm")
Expand Down

0 comments on commit 98f1562

Please sign in to comment.