From b3c734b80432224de27811fc16fb0652f6e34413 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Wei=C3=9Fe?= <66256922+daniel-weisse@users.noreply.github.com> Date: Wed, 29 Nov 2023 14:55:10 +0100 Subject: [PATCH] helm: re-enable timeout flag (#2658) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Honor (hidden) timeout flag for applying helm charts * Set only internally used structs to private --------- Signed-off-by: Daniel Weiße --- cli/internal/cmd/apply.go | 20 +++--- cli/internal/cmd/apply_test.go | 14 ++--- cli/internal/cmd/applyhelm.go | 1 + internal/helm/action.go | 36 +++++------ internal/helm/actionfactory.go | 55 +++++++++-------- internal/helm/actionfactory_test.go | 95 +++++++++++++++-------------- internal/helm/helm.go | 9 ++- internal/helm/loader.go | 18 +++--- internal/helm/loader_test.go | 4 +- internal/helm/release.go | 10 +-- internal/helm/versionlister.go | 4 +- 11 files changed, 137 insertions(+), 129 deletions(-) diff --git a/cli/internal/cmd/apply.go b/cli/internal/cmd/apply.go index 2fbb7f0446..312fec3926 100644 --- a/cli/internal/cmd/apply.go +++ b/cli/internal/cmd/apply.go @@ -126,12 +126,12 @@ func NewApplyCmd() *cobra.Command { cmd.Flags().Bool("merge-kubeconfig", false, "merge Constellation kubeconfig file with default kubeconfig file in $HOME/.kube/config") cmd.Flags().BoolP("yes", "y", false, "run command without further confirmation\n"+ "WARNING: the command might delete or update existing resources without additional checks. Please read the docs.\n") - cmd.Flags().Duration("timeout", 5*time.Minute, "change helm upgrade timeout\n"+ + cmd.Flags().Duration("helm-timeout", 10*time.Minute, "change helm install/upgrade timeout\n"+ "Might be useful for slow connections or big clusters.") cmd.Flags().StringSlice("skip-phases", nil, "comma-separated list of upgrade phases to skip\n"+ fmt.Sprintf("one or multiple of %s", formatSkipPhases())) - must(cmd.Flags().MarkHidden("timeout")) + must(cmd.Flags().MarkHidden("helm-timeout")) must(cmd.RegisterFlagCompletionFunc("skip-phases", skipPhasesCompletion)) return cmd @@ -140,12 +140,12 @@ func NewApplyCmd() *cobra.Command { // applyFlags defines the flags for the apply command. type applyFlags struct { rootFlags - yes bool - conformance bool - mergeConfigs bool - upgradeTimeout time.Duration - helmWaitMode helm.WaitMode - skipPhases skipPhases + yes bool + conformance bool + mergeConfigs bool + helmTimeout time.Duration + helmWaitMode helm.WaitMode + skipPhases skipPhases } // parse the apply command flags. @@ -174,9 +174,9 @@ func (f *applyFlags) parse(flags *pflag.FlagSet) error { return fmt.Errorf("getting 'yes' flag: %w", err) } - f.upgradeTimeout, err = flags.GetDuration("timeout") + f.helmTimeout, err = flags.GetDuration("helm-timeout") if err != nil { - return fmt.Errorf("getting 'timeout' flag: %w", err) + return fmt.Errorf("getting 'helm-timeout' flag: %w", err) } f.conformance, err = flags.GetBool("conformance") diff --git a/cli/internal/cmd/apply_test.go b/cli/internal/cmd/apply_test.go index 43ee030e67..ab6b5b7425 100644 --- a/cli/internal/cmd/apply_test.go +++ b/cli/internal/cmd/apply_test.go @@ -97,8 +97,8 @@ func TestParseApplyFlags(t *testing.T) { "default flags": { flags: defaultFlags(), wantFlags: applyFlags{ - helmWaitMode: helm.WaitModeAtomic, - upgradeTimeout: 5 * time.Minute, + helmWaitMode: helm.WaitModeAtomic, + helmTimeout: 10 * time.Minute, }, }, "skip phases": { @@ -108,9 +108,9 @@ func TestParseApplyFlags(t *testing.T) { return flags }(), wantFlags: applyFlags{ - skipPhases: newPhases(skipHelmPhase, skipK8sPhase), - helmWaitMode: helm.WaitModeAtomic, - upgradeTimeout: 5 * time.Minute, + skipPhases: newPhases(skipHelmPhase, skipK8sPhase), + helmWaitMode: helm.WaitModeAtomic, + helmTimeout: 10 * time.Minute, }, }, "skip helm wait": { @@ -120,8 +120,8 @@ func TestParseApplyFlags(t *testing.T) { return flags }(), wantFlags: applyFlags{ - helmWaitMode: helm.WaitModeNone, - upgradeTimeout: 5 * time.Minute, + helmWaitMode: helm.WaitModeNone, + helmTimeout: 10 * time.Minute, }, }, } diff --git a/cli/internal/cmd/applyhelm.go b/cli/internal/cmd/applyhelm.go index 12dcd4c5f9..339fc6f4f5 100644 --- a/cli/internal/cmd/applyhelm.go +++ b/cli/internal/cmd/applyhelm.go @@ -37,6 +37,7 @@ func (a *applyCmd) runHelmApply( Force: a.flags.force, Conformance: a.flags.conformance, HelmWaitMode: a.flags.helmWaitMode, + ApplyTimeout: a.flags.helmTimeout, AllowDestructive: helm.DenyDestructive, } helmApplier, err := a.newHelmClient(constants.AdminConfFilename, a.log) diff --git a/internal/helm/action.go b/internal/helm/action.go index 621d92b89e..fc49dffb43 100644 --- a/internal/helm/action.go +++ b/internal/helm/action.go @@ -19,11 +19,7 @@ import ( "helm.sh/helm/v3/pkg/cli" ) -const ( - // timeout is the maximum time given per helm action. - timeout = 10 * time.Minute - applyRetryInterval = 30 * time.Second -) +const applyRetryInterval = 30 * time.Second type applyAction interface { Apply(context.Context) error @@ -45,12 +41,12 @@ func newActionConfig(kubeconfig string, logger debugLog) (*action.Configuration, return actionConfig, nil } -func newHelmInstallAction(config *action.Configuration, release Release) *action.Install { +func newHelmInstallAction(config *action.Configuration, release release, timeout time.Duration) *action.Install { action := action.NewInstall(config) action.Namespace = constants.HelmNamespace action.Timeout = timeout - action.ReleaseName = release.ReleaseName - setWaitMode(action, release.WaitMode) + action.ReleaseName = release.releaseName + setWaitMode(action, release.waitMode) return action } @@ -73,7 +69,7 @@ func setWaitMode(a *action.Install, waitMode WaitMode) { // installAction is an action that installs a helm chart. type installAction struct { preInstall func(context.Context) error - release Release + release release helmAction *action.Install postInstall func(context.Context) error log debugLog @@ -104,13 +100,13 @@ func (a *installAction) SaveChart(chartsDir string, fileHandler file.Handler) er } func (a *installAction) apply(ctx context.Context) error { - _, err := a.helmAction.RunWithContext(ctx, a.release.Chart, a.release.Values) + _, err := a.helmAction.RunWithContext(ctx, a.release.chart, a.release.values) return err } // ReleaseName returns the release name. func (a *installAction) ReleaseName() string { - return a.release.ReleaseName + return a.release.releaseName } // IsAtomic returns true if the action is atomic. @@ -118,7 +114,7 @@ func (a *installAction) IsAtomic() bool { return a.helmAction.Atomic } -func newHelmUpgradeAction(config *action.Configuration) *action.Upgrade { +func newHelmUpgradeAction(config *action.Configuration, timeout time.Duration) *action.Upgrade { action := action.NewUpgrade(config) action.Namespace = constants.HelmNamespace action.Timeout = timeout @@ -131,7 +127,7 @@ func newHelmUpgradeAction(config *action.Configuration) *action.Upgrade { type upgradeAction struct { preUpgrade func(context.Context) error postUpgrade func(context.Context) error - release Release + release release helmAction *action.Upgrade log debugLog } @@ -160,13 +156,13 @@ func (a *upgradeAction) SaveChart(chartsDir string, fileHandler file.Handler) er } func (a *upgradeAction) apply(ctx context.Context) error { - _, err := a.helmAction.RunWithContext(ctx, a.release.ReleaseName, a.release.Chart, a.release.Values) + _, err := a.helmAction.RunWithContext(ctx, a.release.releaseName, a.release.chart, a.release.values) return err } // ReleaseName returns the release name. func (a *upgradeAction) ReleaseName() string { - return a.release.ReleaseName + return a.release.releaseName } // IsAtomic returns true if the action is atomic. @@ -174,12 +170,12 @@ func (a *upgradeAction) IsAtomic() bool { return a.helmAction.Atomic } -func saveChart(release Release, chartsDir string, fileHandler file.Handler) error { - if err := saveChartToDisk(release.Chart, chartsDir, fileHandler); err != nil { - return fmt.Errorf("saving chart %s to %q: %w", release.ReleaseName, chartsDir, err) +func saveChart(release release, chartsDir string, fileHandler file.Handler) error { + if err := saveChartToDisk(release.chart, chartsDir, fileHandler); err != nil { + return fmt.Errorf("saving chart %s to %q: %w", release.releaseName, chartsDir, err) } - if err := fileHandler.WriteYAML(filepath.Join(chartsDir, release.Chart.Metadata.Name, "overrides.yaml"), release.Values); err != nil { - return fmt.Errorf("saving override values for chart %s to %q: %w", release.ReleaseName, filepath.Join(chartsDir, release.Chart.Metadata.Name), err) + if err := fileHandler.WriteYAML(filepath.Join(chartsDir, release.chart.Metadata.Name, "overrides.yaml"), release.values); err != nil { + return fmt.Errorf("saving override values for chart %s to %q: %w", release.releaseName, filepath.Join(chartsDir, release.chart.Metadata.Name), err) } return nil diff --git a/internal/helm/actionfactory.go b/internal/helm/actionfactory.go index f8fe622f7e..1ad3366b74 100644 --- a/internal/helm/actionfactory.go +++ b/internal/helm/actionfactory.go @@ -11,6 +11,7 @@ import ( "errors" "fmt" "strings" + "time" "github.com/edgelesssys/constellation/v2/internal/compatibility" "github.com/edgelesssys/constellation/v2/internal/semver" @@ -45,17 +46,19 @@ func newActionFactory(kubeClient crdClient, lister releaseVersionLister, actionC } // GetActions returns a list of actions to apply the given releases. -func (a actionFactory) GetActions(releases []Release, configTargetVersion semver.Semver, force, allowDestructive bool) (actions []applyAction, includesUpgrade bool, err error) { +func (a actionFactory) GetActions( + releases []release, configTargetVersion semver.Semver, force, allowDestructive bool, timeout time.Duration, +) (actions []applyAction, includesUpgrade bool, err error) { upgradeErrs := []error{} for _, release := range releases { - err := a.appendNewAction(release, configTargetVersion, force, allowDestructive, &actions) + err := a.appendNewAction(release, configTargetVersion, force, allowDestructive, timeout, &actions) var invalidUpgrade *compatibility.InvalidUpgradeError if errors.As(err, &invalidUpgrade) { upgradeErrs = append(upgradeErrs, err) continue } if err != nil { - return actions, includesUpgrade, fmt.Errorf("creating action for %s: %w", release.ReleaseName, err) + return actions, includesUpgrade, fmt.Errorf("creating action for %s: %w", release.releaseName, err) } } for _, action := range actions { @@ -67,17 +70,19 @@ func (a actionFactory) GetActions(releases []Release, configTargetVersion semver return actions, includesUpgrade, errors.Join(upgradeErrs...) } -func (a actionFactory) appendNewAction(release Release, configTargetVersion semver.Semver, force, allowDestructive bool, actions *[]applyAction) error { - newVersion, err := semver.New(release.Chart.Metadata.Version) +func (a actionFactory) appendNewAction( + release release, configTargetVersion semver.Semver, force, allowDestructive bool, timeout time.Duration, actions *[]applyAction, +) error { + newVersion, err := semver.New(release.chart.Metadata.Version) if err != nil { return fmt.Errorf("parsing chart version: %w", err) } cliSupportsConfigVersion := configTargetVersion.Compare(newVersion) != 0 - currentVersion, err := a.versionLister.currentVersion(release.ReleaseName) + currentVersion, err := a.versionLister.currentVersion(release.releaseName) if errors.Is(err, errReleaseNotFound) { // Don't install a new release if the user's config specifies a different version than the CLI offers. - if !force && isCLIVersionedRelease(release.ReleaseName) && cliSupportsConfigVersion { + if !force && isCLIVersionedRelease(release.releaseName) && cliSupportsConfigVersion { return compatibility.NewInvalidUpgradeError( currentVersion.String(), configTargetVersion.String(), @@ -85,23 +90,23 @@ func (a actionFactory) appendNewAction(release Release, configTargetVersion semv ) } - a.log.Debugf("Release %s not found, adding to new releases...", release.ReleaseName) - *actions = append(*actions, a.newInstall(release)) + a.log.Debugf("release %s not found, adding to new releases...", release.releaseName) + *actions = append(*actions, a.newInstall(release, timeout)) return nil } if err != nil { - return fmt.Errorf("getting version for %s: %w", release.ReleaseName, err) + return fmt.Errorf("getting version for %s: %w", release.releaseName, err) } - a.log.Debugf("Current %s version: %s", release.ReleaseName, currentVersion) - a.log.Debugf("New %s version: %s", release.ReleaseName, newVersion) + a.log.Debugf("Current %s version: %s", release.releaseName, currentVersion) + a.log.Debugf("New %s version: %s", release.releaseName, newVersion) if !force { // For charts we package ourselves, the version is equal to the CLI version (charts are embedded in the binary). // We need to make sure this matches with the version in a user's config, if an upgrade should be applied. - if isCLIVersionedRelease(release.ReleaseName) { + if isCLIVersionedRelease(release.releaseName) { // If target version is not a valid upgrade, don't upgrade any charts. if err := configTargetVersion.IsUpgradeTo(currentVersion); err != nil { - return fmt.Errorf("invalid upgrade for %s: %w", release.ReleaseName, err) + return fmt.Errorf("invalid upgrade for %s: %w", release.releaseName, err) } // Target version is newer than current version, so we should perform an upgrade. // Now make sure the target version is equal to the the CLI version. @@ -117,32 +122,32 @@ func (a actionFactory) appendNewAction(release Release, configTargetVersion semv if err := newVersion.IsUpgradeTo(currentVersion); err != nil { // TODO(3u13r): Remove when Constellation v2.14 is released. // We need to ignore that we jump from Cilium v1.12 to v1.15-pre. We have verified that this works. - if !(errors.Is(err, compatibility.ErrMinorDrift) && release.ReleaseName == "cilium") { - return fmt.Errorf("invalid upgrade for %s: %w", release.ReleaseName, err) + if !(errors.Is(err, compatibility.ErrMinorDrift) && release.releaseName == "cilium") { + return fmt.Errorf("invalid upgrade for %s: %w", release.releaseName, err) } } } } if !allowDestructive && - release.ReleaseName == certManagerInfo.releaseName { + release.releaseName == certManagerInfo.releaseName { return ErrConfirmationMissing } - a.log.Debugf("Upgrading %s from %s to %s", release.ReleaseName, currentVersion, newVersion) - *actions = append(*actions, a.newUpgrade(release)) + a.log.Debugf("Upgrading %s from %s to %s", release.releaseName, currentVersion, newVersion) + *actions = append(*actions, a.newUpgrade(release, timeout)) return nil } -func (a actionFactory) newInstall(release Release) *installAction { - action := &installAction{helmAction: newHelmInstallAction(a.cfg, release), release: release, log: a.log} +func (a actionFactory) newInstall(release release, timeout time.Duration) *installAction { + action := &installAction{helmAction: newHelmInstallAction(a.cfg, release, timeout), release: release, log: a.log} return action } -func (a actionFactory) newUpgrade(release Release) *upgradeAction { - action := &upgradeAction{helmAction: newHelmUpgradeAction(a.cfg), release: release, log: a.log} - if release.ReleaseName == constellationOperatorsInfo.releaseName { +func (a actionFactory) newUpgrade(release release, timeout time.Duration) *upgradeAction { + action := &upgradeAction{helmAction: newHelmUpgradeAction(a.cfg, timeout), release: release, log: a.log} + if release.releaseName == constellationOperatorsInfo.releaseName { action.preUpgrade = func(ctx context.Context) error { - if err := a.updateCRDs(ctx, release.Chart); err != nil { + if err := a.updateCRDs(ctx, release.chart); err != nil { return fmt.Errorf("updating operator CRDs: %w", err) } return nil diff --git a/internal/helm/actionfactory_test.go b/internal/helm/actionfactory_test.go index 6d6c5901af..960ea5a520 100644 --- a/internal/helm/actionfactory_test.go +++ b/internal/helm/actionfactory_test.go @@ -9,6 +9,7 @@ package helm import ( "errors" "testing" + "time" "github.com/edgelesssys/constellation/v2/internal/compatibility" "github.com/edgelesssys/constellation/v2/internal/logger" @@ -26,7 +27,7 @@ func TestAppendNewAction(t *testing.T) { testCases := map[string]struct { lister stubLister - release Release + release release configTargetVersion semver.Semver force bool allowDestructive bool @@ -35,9 +36,9 @@ func TestAppendNewAction(t *testing.T) { }{ "upgrade release": { lister: stubLister{version: semver.NewFromInt(1, 0, 0, "")}, - release: Release{ - ReleaseName: "test", - Chart: &chart.Chart{ + release: release{ + releaseName: "test", + chart: &chart.Chart{ Metadata: &chart.Metadata{ Version: "1.1.0", }, @@ -47,9 +48,9 @@ func TestAppendNewAction(t *testing.T) { }, "upgrade to same version": { lister: stubLister{version: semver.NewFromInt(1, 0, 0, "")}, - release: Release{ - ReleaseName: "test", - Chart: &chart.Chart{ + release: release{ + releaseName: "test", + chart: &chart.Chart{ Metadata: &chart.Metadata{ Version: "1.0.0", }, @@ -61,9 +62,9 @@ func TestAppendNewAction(t *testing.T) { }, "upgrade to older version": { lister: stubLister{version: semver.NewFromInt(1, 1, 0, "")}, - release: Release{ - ReleaseName: "test", - Chart: &chart.Chart{ + release: release{ + releaseName: "test", + chart: &chart.Chart{ Metadata: &chart.Metadata{ Version: "1.0.0", }, @@ -75,9 +76,9 @@ func TestAppendNewAction(t *testing.T) { }, "upgrade to older version can be forced": { lister: stubLister{version: semver.NewFromInt(1, 1, 0, "")}, - release: Release{ - ReleaseName: "test", - Chart: &chart.Chart{ + release: release{ + releaseName: "test", + chart: &chart.Chart{ Metadata: &chart.Metadata{ Version: "1.0.0", }, @@ -88,9 +89,9 @@ func TestAppendNewAction(t *testing.T) { }, "non semver in chart metadata": { lister: stubLister{version: semver.NewFromInt(1, 0, 0, "")}, - release: Release{ - ReleaseName: "test", - Chart: &chart.Chart{ + release: release{ + releaseName: "test", + chart: &chart.Chart{ Metadata: &chart.Metadata{ Version: "some-version", }, @@ -100,9 +101,9 @@ func TestAppendNewAction(t *testing.T) { }, "listing release fails": { lister: stubLister{err: assert.AnError}, - release: Release{ - ReleaseName: "test", - Chart: &chart.Chart{ + release: release{ + releaseName: "test", + chart: &chart.Chart{ Metadata: &chart.Metadata{ Version: "1.1.0", }, @@ -113,9 +114,9 @@ func TestAppendNewAction(t *testing.T) { }, "release not installed": { lister: stubLister{err: errReleaseNotFound}, - release: Release{ - ReleaseName: "test", - Chart: &chart.Chart{ + release: release{ + releaseName: "test", + chart: &chart.Chart{ Metadata: &chart.Metadata{ Version: "1.1.0", }, @@ -125,13 +126,13 @@ func TestAppendNewAction(t *testing.T) { }, "destructive release upgrade requires confirmation": { lister: stubLister{version: semver.NewFromInt(1, 0, 0, "")}, - release: Release{ - Chart: &chart.Chart{ + release: release{ + chart: &chart.Chart{ Metadata: &chart.Metadata{ Version: "1.1.0", }, }, - ReleaseName: certManagerInfo.releaseName, + releaseName: certManagerInfo.releaseName, }, configTargetVersion: semver.NewFromInt(1, 1, 0, ""), wantErr: true, @@ -141,22 +142,22 @@ func TestAppendNewAction(t *testing.T) { }, "destructive release upgrade can be accepted": { lister: stubLister{version: semver.NewFromInt(1, 0, 0, "")}, - release: Release{ - Chart: &chart.Chart{ + release: release{ + chart: &chart.Chart{ Metadata: &chart.Metadata{ Version: "1.1.0", }, }, - ReleaseName: certManagerInfo.releaseName, + releaseName: certManagerInfo.releaseName, }, configTargetVersion: semver.NewFromInt(1, 1, 0, ""), allowDestructive: true, }, "config version takes precedence over CLI version": { lister: stubLister{version: semver.NewFromInt(1, 0, 0, "")}, - release: Release{ - ReleaseName: constellationServicesInfo.releaseName, - Chart: &chart.Chart{ + release: release{ + releaseName: constellationServicesInfo.releaseName, + chart: &chart.Chart{ Metadata: &chart.Metadata{ Version: "1.1.0", }, @@ -168,9 +169,9 @@ func TestAppendNewAction(t *testing.T) { }, "error if CLI version does not match config version on upgrade": { lister: stubLister{version: semver.NewFromInt(1, 0, 0, "")}, - release: Release{ - ReleaseName: constellationServicesInfo.releaseName, - Chart: &chart.Chart{ + release: release{ + releaseName: constellationServicesInfo.releaseName, + chart: &chart.Chart{ Metadata: &chart.Metadata{ Version: "1.1.5", }, @@ -182,9 +183,9 @@ func TestAppendNewAction(t *testing.T) { }, "config version matches CLI version on upgrade": { lister: stubLister{version: semver.NewFromInt(1, 0, 0, "")}, - release: Release{ - ReleaseName: constellationServicesInfo.releaseName, - Chart: &chart.Chart{ + release: release{ + releaseName: constellationServicesInfo.releaseName, + chart: &chart.Chart{ Metadata: &chart.Metadata{ Version: "1.1.5", }, @@ -194,9 +195,9 @@ func TestAppendNewAction(t *testing.T) { }, "config - CLI version mismatch can be forced through": { lister: stubLister{version: semver.NewFromInt(1, 0, 0, "")}, - release: Release{ - ReleaseName: constellationServicesInfo.releaseName, - Chart: &chart.Chart{ + release: release{ + releaseName: constellationServicesInfo.releaseName, + chart: &chart.Chart{ Metadata: &chart.Metadata{ Version: "1.1.5", }, @@ -207,9 +208,9 @@ func TestAppendNewAction(t *testing.T) { }, "installing new release requires matching config and CLI version": { lister: stubLister{err: errReleaseNotFound}, - release: Release{ - ReleaseName: constellationServicesInfo.releaseName, - Chart: &chart.Chart{ + release: release{ + releaseName: constellationServicesInfo.releaseName, + chart: &chart.Chart{ Metadata: &chart.Metadata{ Version: "1.1.0", }, @@ -221,9 +222,9 @@ func TestAppendNewAction(t *testing.T) { }, "config - CLI version mismatch for new releases can be forced through": { lister: stubLister{err: errReleaseNotFound}, - release: Release{ - ReleaseName: constellationServicesInfo.releaseName, - Chart: &chart.Chart{ + release: release{ + releaseName: constellationServicesInfo.releaseName, + chart: &chart.Chart{ Metadata: &chart.Metadata{ Version: "1.1.0", }, @@ -241,7 +242,7 @@ func TestAppendNewAction(t *testing.T) { actions := []applyAction{} actionFactory := newActionFactory(nil, tc.lister, &action.Configuration{}, logger.NewTest(t)) - err := actionFactory.appendNewAction(tc.release, tc.configTargetVersion, tc.force, tc.allowDestructive, &actions) + err := actionFactory.appendNewAction(tc.release, tc.configTargetVersion, tc.force, tc.allowDestructive, time.Second, &actions) if tc.wantErr { assert.Error(err) if tc.assertErr != nil { diff --git a/internal/helm/helm.go b/internal/helm/helm.go index a0ae6bd9bf..2307cd98d0 100644 --- a/internal/helm/helm.go +++ b/internal/helm/helm.go @@ -31,6 +31,7 @@ package helm import ( "context" "fmt" + "time" "github.com/edgelesssys/constellation/v2/internal/config" "github.com/edgelesssys/constellation/v2/internal/constants" @@ -81,6 +82,7 @@ type Options struct { HelmWaitMode WaitMode AllowDestructive bool Force bool + ApplyTimeout time.Duration } // PrepareApply loads the charts and returns the executor to apply them. @@ -93,15 +95,18 @@ func (h Client) PrepareApply( if err != nil { return nil, false, fmt.Errorf("loading Helm releases: %w", err) } + h.log.Debugf("Loaded Helm releases") - actions, includesUpgrades, err := h.factory.GetActions(releases, conf.MicroserviceVersion, flags.Force, flags.AllowDestructive) + actions, includesUpgrades, err := h.factory.GetActions( + releases, conf.MicroserviceVersion, flags.Force, flags.AllowDestructive, flags.ApplyTimeout, + ) return &ChartApplyExecutor{actions: actions, log: h.log}, includesUpgrades, err } func (h Client) loadReleases( conf *config.Config, secret uri.MasterSecret, stateFile *state.State, flags Options, serviceAccURI string, -) ([]Release, error) { +) ([]release, error) { helmLoader := newLoader(conf, stateFile, h.cliVersion) h.log.Debugf("Created new Helm loader") return helmLoader.loadReleases(flags.Conformance, flags.HelmWaitMode, secret, serviceAccURI) diff --git a/internal/helm/loader.go b/internal/helm/loader.go index fdcdd91452..71c9a3b63a 100644 --- a/internal/helm/loader.go +++ b/internal/helm/loader.go @@ -115,7 +115,7 @@ func newLoader(config *config.Config, stateFile *state.State, cliVersion semver. // makes sure if a release was removed as a dependency from one chart, // and then added as a new standalone chart (or as a dependency of another chart), // that the new release is installed after the existing one to avoid name conflicts. -type releaseApplyOrder []Release +type releaseApplyOrder []release // loadReleases loads the embedded helm charts and returns them as a HelmReleases object. func (i *chartLoader) loadReleases(conformanceMode bool, helmWaitMode WaitMode, masterSecret uri.MasterSecret, @@ -126,7 +126,7 @@ func (i *chartLoader) loadReleases(conformanceMode bool, helmWaitMode WaitMode, return nil, fmt.Errorf("loading cilium: %w", err) } ciliumVals := extraCiliumValues(i.config.GetProvider(), conformanceMode, i.stateFile.Infrastructure) - ciliumRelease.Values = mergeMaps(ciliumRelease.Values, ciliumVals) + ciliumRelease.values = mergeMaps(ciliumRelease.values, ciliumVals) certManagerRelease, err := i.loadRelease(certManagerInfo, helmWaitMode) if err != nil { @@ -137,7 +137,7 @@ func (i *chartLoader) loadReleases(conformanceMode bool, helmWaitMode WaitMode, if err != nil { return nil, fmt.Errorf("loading operators: %w", err) } - operatorRelease.Values = mergeMaps(operatorRelease.Values, extraOperatorValues(i.stateFile.Infrastructure.UID)) + operatorRelease.values = mergeMaps(operatorRelease.values, extraOperatorValues(i.stateFile.Infrastructure.UID)) conServicesRelease, err := i.loadRelease(constellationServicesInfo, helmWaitMode) if err != nil { @@ -148,7 +148,7 @@ func (i *chartLoader) loadReleases(conformanceMode bool, helmWaitMode WaitMode, if err != nil { return nil, fmt.Errorf("extending constellation-services values: %w", err) } - conServicesRelease.Values = mergeMaps(conServicesRelease.Values, svcVals) + conServicesRelease.values = mergeMaps(conServicesRelease.values, svcVals) releases := releaseApplyOrder{ciliumRelease, conServicesRelease, certManagerRelease} if i.config.DeployCSIDriver() { @@ -160,7 +160,7 @@ func (i *chartLoader) loadReleases(conformanceMode bool, helmWaitMode WaitMode, if err != nil { return nil, fmt.Errorf("extending CSI values: %w", err) } - csiRelease.Values = mergeMaps(csiRelease.Values, extraCSIvals) + csiRelease.values = mergeMaps(csiRelease.values, extraCSIvals) releases = append(releases, csiRelease) } if i.config.HasProvider(cloudprovider.AWS) { @@ -177,10 +177,10 @@ func (i *chartLoader) loadReleases(conformanceMode bool, helmWaitMode WaitMode, // loadRelease loads the embedded chart and values depending on the given info argument. // IMPORTANT: .helmignore rules specifying files in subdirectories are not applied (e.g. crds/kustomization.yaml). -func (i *chartLoader) loadRelease(info chartInfo, helmWaitMode WaitMode) (Release, error) { +func (i *chartLoader) loadRelease(info chartInfo, helmWaitMode WaitMode) (release, error) { chart, err := loadChartsDir(helmFS, info.path) if err != nil { - return Release{}, fmt.Errorf("loading %s chart: %w", info.releaseName, err) + return release{}, fmt.Errorf("loading %s chart: %w", info.releaseName, err) } var values map[string]any @@ -190,7 +190,7 @@ func (i *chartLoader) loadRelease(info chartInfo, helmWaitMode WaitMode) (Releas var ok bool values, ok = ciliumVals[i.csp.String()] if !ok { - return Release{}, fmt.Errorf("cilium values for csp %q not found", i.csp.String()) + return release{}, fmt.Errorf("cilium values for csp %q not found", i.csp.String()) } case certManagerInfo.releaseName: values = i.loadCertManagerValues() @@ -210,7 +210,7 @@ func (i *chartLoader) loadRelease(info chartInfo, helmWaitMode WaitMode) (Releas updateVersions(chart, i.cliVersion) } - return Release{Chart: chart, Values: values, ReleaseName: info.releaseName, WaitMode: helmWaitMode}, nil + return release{chart: chart, values: values, releaseName: info.releaseName, waitMode: helmWaitMode}, nil } func (i *chartLoader) loadAWSLBControllerValues() map[string]any { diff --git a/internal/helm/loader_test.go b/internal/helm/loader_test.go index 456722ce03..7ab43ce841 100644 --- a/internal/helm/loader_test.go +++ b/internal/helm/loader_test.go @@ -84,8 +84,8 @@ func TestLoadReleases(t *testing.T) { ) require.NoError(err) for _, release := range helmReleases { - if release.ReleaseName == constellationServicesInfo.releaseName { - assert.NotNil(release.Chart.Dependencies()) + if release.releaseName == constellationServicesInfo.releaseName { + assert.NotNil(release.chart.Dependencies()) } } } diff --git a/internal/helm/release.go b/internal/helm/release.go index 3e68fed45b..c7be7ab5ce 100644 --- a/internal/helm/release.go +++ b/internal/helm/release.go @@ -10,11 +10,11 @@ package helm import "helm.sh/helm/v3/pkg/chart" // Release bundles all information necessary to create a helm release. -type Release struct { - Chart *chart.Chart - Values map[string]any - ReleaseName string - WaitMode WaitMode +type release struct { + chart *chart.Chart + values map[string]any + releaseName string + waitMode WaitMode } // WaitMode specifies the wait mode for a helm release. diff --git a/internal/helm/versionlister.go b/internal/helm/versionlister.go index fa458e3f41..ae45e2d766 100644 --- a/internal/helm/versionlister.go +++ b/internal/helm/versionlister.go @@ -12,7 +12,7 @@ import ( "github.com/edgelesssys/constellation/v2/internal/semver" "helm.sh/helm/v3/pkg/action" - "helm.sh/helm/v3/pkg/release" + helmrelease "helm.sh/helm/v3/pkg/release" "k8s.io/client-go/util/retry" ) @@ -34,7 +34,7 @@ func NewReleaseVersionClient(kubeConfigPath string, log debugLog) (*ReleaseVersi // listAction execute a List action by wrapping helm's action package. // It creates the action, runs it at returns results and errors. -func (c ReleaseVersionClient) listAction(release string) (res []*release.Release, err error) { +func (c ReleaseVersionClient) listAction(release string) (res []*helmrelease.Release, err error) { action := action.NewList(c.config) action.Filter = release // during init, the kube API might not yet be reachable, so we retry