Skip to content

Commit

Permalink
Revert "move where ctx gets set"
Browse files Browse the repository at this point in the history
This reverts commit f5e3456.
  • Loading branch information
AustinAbro321 committed Oct 1, 2024
1 parent f5e3456 commit c5f8e8f
Showing 1 changed file with 9 additions and 9 deletions.
18 changes: 9 additions & 9 deletions src/internal/packager/helm/chart.go
Original file line number Diff line number Diff line change
Expand Up @@ -63,12 +63,7 @@ func (h *Helm) InstallOrUpgradeChart(ctx context.Context) (types.ConnectStrings,
histClient := action.NewHistory(h.actionConfig)
var release *release.Release

// Declare this here so we share the Helm timeout with health checks
var helmTimeoutCtx context.Context
err = retry.Do(func() error {
var cancel context.CancelFunc
helmTimeoutCtx, cancel = context.WithTimeout(ctx, h.timeout)
defer cancel()
var err error

releases, histErr := histClient.Run(h.chart.ReleaseName)
Expand All @@ -79,14 +74,14 @@ func (h *Helm) InstallOrUpgradeChart(ctx context.Context) (types.ConnectStrings,
// No prior release, try to install it.
spinner.Updatef("Attempting chart installation")

release, err = h.installChart(helmTimeoutCtx, postRender)
release, err = h.installChart(ctx, postRender)
} else if histErr == nil && len(releases) > 0 {
// Otherwise, there is a prior release so upgrade it.
spinner.Updatef("Attempting chart upgrade")

lastRelease := releases[len(releases)-1]

release, err = h.upgradeChart(helmTimeoutCtx, lastRelease, postRender)
release, err = h.upgradeChart(ctx, lastRelease, postRender)
} else {
return fmt.Errorf("unable to verify the chart installation status: %w", histErr)
}
Expand Down Expand Up @@ -126,7 +121,6 @@ func (h *Helm) InstallOrUpgradeChart(ctx context.Context) (types.ConnectStrings,
return nil, "", installErr
}

// This build step is necessary as the raw manifests may not have the namespace they will be deployed into by Helm
resourceList, err := h.actionConfig.KubeClient.Build(bytes.NewBufferString(release.Manifest), true)
if err != nil {
return nil, "", fmt.Errorf("unable to build the resource list: %w", err)
Expand All @@ -143,8 +137,14 @@ func (h *Helm) InstallOrUpgradeChart(ctx context.Context) (types.ConnectStrings,
})
}
if !h.chart.NoWait {
// This effectively doubles the amount of time a timeout can take
// 15 minute timeout -> 14 minutes to install -> another 14 minutes of waiting
// This could be avoided by creating the context in the start of the retry loop
// Technically would be a breaking change, in practice this is unlikely to affect anyone
healthChecksCtx, cancel := context.WithTimeout(ctx, h.timeout)
defer cancel()
spinner.Updatef("Running health checks")
if err := healthchecks.Run(helmTimeoutCtx, h.cluster.Watcher, healthChecks); err != nil {
if err := healthchecks.Run(healthChecksCtx, h.cluster.Watcher, healthChecks); err != nil {
return nil, "", err
}
}
Expand Down

0 comments on commit c5f8e8f

Please sign in to comment.