Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix!: do not uninstall helm chart after failed install or upgrade #2456

Merged
merged 16 commits into from
May 6, 2024
Merged

fix!: do not uninstall helm chart after failed install or upgrade #2456

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
98f1562
Do not uninstall helm chart after failed install or upgrade
Apr 23, 2024
6ab57d0
Add msg about using pkg remove and cleanup err handling
Apr 24, 2024
8b70057
Merge branch 'main' into do-not-uninstall-helm-chart-after-failed-ins…
Apr 24, 2024
46c5832
Remove redundant error return and fix retries test
Apr 24, 2024
3fa858e
just kidding the other way was better
Apr 24, 2024
8f02503
Merge branch 'main' into do-not-uninstall-helm-chart-after-failed-ins…
Apr 25, 2024
8dd2536
Add section to docs about helm usage and behavior
Apr 26, 2024
3a8e2d7
Fix rollback docs
Apr 26, 2024
33c937a
Fix retries test
Apr 26, 2024
9c33724
Keep the s for sigh
Apr 26, 2024
82d1e52
Merge branch 'main' into do-not-uninstall-helm-chart-after-failed-ins…
Apr 26, 2024
08b4ebe
Merge branch 'main' into do-not-uninstall-helm-chart-after-failed-ins…
Apr 26, 2024
bb75462
Update site/src/content/docs/ref/deploy.mdx
Apr 29, 2024
dabefdf
Merge branch 'main' into do-not-uninstall-helm-chart-after-failed-ins…
Apr 29, 2024
8f061d3
Merge branch 'main' into do-not-uninstall-helm-chart-after-failed-ins…
May 3, 2024
e339608
Merge branch 'main' into do-not-uninstall-helm-chart-after-failed-ins…
May 6, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 0 additions & 6 deletions src/internal/packager/helm/chart.go
View file
Open in desktop
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")
lucasrod16 marked this conversation as resolved.
Show resolved Hide resolved
_, 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
View file
Open in desktop
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
Loading