From 7db584a88eb10415bcd224ad8ce994df13704dab Mon Sep 17 00:00:00 2001 From: Otto Bittner Date: Wed, 1 Feb 2023 11:23:57 +0100 Subject: [PATCH] cli: move upgradeApply logic into separate functions * introduce handleImageUpgrade & handleServiceUpgrade * rename cloudUpgrader.Upgrade to UpgradeImage * remove helm flag * remove hint about development status --- cli/internal/cloudcmd/upgrade.go | 4 +- cli/internal/cmd/upgradeapply.go | 71 +++++++++++++-------------- cli/internal/cmd/upgradeapply_test.go | 2 +- 3 files changed, 37 insertions(+), 40 deletions(-) diff --git a/cli/internal/cloudcmd/upgrade.go b/cli/internal/cloudcmd/upgrade.go index 7db01f762..09d3f534f 100644 --- a/cli/internal/cloudcmd/upgrade.go +++ b/cli/internal/cloudcmd/upgrade.go @@ -68,8 +68,8 @@ func NewUpgrader(outWriter io.Writer, log debugLog) (*Upgrader, error) { }, nil } -// Upgrade upgrades the cluster to the given measurements and image. -func (u *Upgrader) Upgrade(ctx context.Context, imageReference, imageVersion string, measurements measurements.M) error { +// UpgradeImage upgrades the cluster to the given measurements and image. +func (u *Upgrader) UpgradeImage(ctx context.Context, imageReference, imageVersion string, measurements measurements.M) error { if err := u.updateMeasurements(ctx, measurements); err != nil { return fmt.Errorf("updating measurements: %w", err) } diff --git a/cli/internal/cmd/upgradeapply.go b/cli/internal/cmd/upgradeapply.go index 18db25807..81da55f06 100644 --- a/cli/internal/cmd/upgradeapply.go +++ b/cli/internal/cmd/upgradeapply.go @@ -31,15 +31,10 @@ func newUpgradeApplyCmd() *cobra.Command { RunE: runUpgradeApply, } - cmd.Flags().Bool("helm", false, "execute helm upgrade\n"+ - "This feature is still in development an may change without announcement. Upgrades all helm charts deployed during constellation-init.") cmd.Flags().BoolP("yes", "y", false, "run upgrades without further confirmation\n"+ "WARNING: might delete your resources in case you are using cert-manager in your cluster. Please read the docs.") cmd.Flags().Duration("timeout", 3*time.Minute, "change helm upgrade timeout\n"+ - "This feature is still in development an may change without announcement. Might be useful for slow connections or big clusters.") - if err := cmd.Flags().MarkHidden("helm"); err != nil { - panic(err) - } + "Might be useful for slow connections or big clusters.") if err := cmd.Flags().MarkHidden("timeout"); err != nil { panic(err) } @@ -74,40 +69,48 @@ func upgradeApply(cmd *cobra.Command, imageFetcher imageFetcher, upgrader cloudU return config.DisplayValidationErrors(cmd.ErrOrStderr(), err) } - if flags.helmFlag { - err = upgrader.UpgradeHelmServices(cmd.Context(), conf, flags.upgradeTimeout, helm.DenyDestructive) - if errors.Is(err, helm.ErrConfirmationMissing) { - if !flags.yes { - cmd.PrintErrln("WARNING: Upgrading cert-manager will destroy all custom resources you have manually created that are based on the current version of cert-manager.") - ok, askErr := askToConfirm(cmd, "Do you want to upgrade cert-manager anyway?") - if askErr != nil { - return fmt.Errorf("asking for confirmation: %w", err) - } - if !ok { - cmd.Println("Aborting upgrade.") - return nil - } - } - err = upgrader.UpgradeHelmServices(cmd.Context(), conf, flags.upgradeTimeout, helm.AllowDestructive) - } - if err != nil { - return fmt.Errorf("upgrading helm: %w", err) - } - - return nil + if err := handleServiceUpgrade(cmd, upgrader, conf, flags); err != nil { + return err } // TODO: validate upgrade config? Should be basic things like checking image is not an empty string // More sophisticated validation, like making sure we don't downgrade the cluster, should be done by `constellation upgrade plan` + return handleImageUpgrade(cmd.Context(), conf, imageFetcher, upgrader) +} + +func handleServiceUpgrade(cmd *cobra.Command, upgrader cloudUpgrader, conf *config.Config, flags upgradeApplyFlags) error { + err := upgrader.UpgradeHelmServices(cmd.Context(), conf, flags.upgradeTimeout, helm.DenyDestructive) + if errors.Is(err, helm.ErrConfirmationMissing) { + if !flags.yes { + cmd.PrintErrln("WARNING: Upgrading cert-manager will destroy all custom resources you have manually created that are based on the current version of cert-manager.") + ok, askErr := askToConfirm(cmd, "Do you want to upgrade cert-manager anyway?") + if askErr != nil { + return fmt.Errorf("asking for confirmation: %w", err) + } + if !ok { + cmd.Println("Aborting upgrade.") + return nil + } + } + err = upgrader.UpgradeHelmServices(cmd.Context(), conf, flags.upgradeTimeout, helm.AllowDestructive) + } + if err != nil { + return fmt.Errorf("upgrading helm: %w", err) + } + + return nil +} + +func handleImageUpgrade(ctx context.Context, conf *config.Config, imageFetcher imageFetcher, upgrader cloudUpgrader) error { // this config modification is temporary until we can remove the upgrade section from the config conf.Image = conf.Upgrade.Image - imageReference, err := imageFetcher.FetchReference(cmd.Context(), conf) + imageReference, err := imageFetcher.FetchReference(ctx, conf) if err != nil { return err } - return upgrader.Upgrade(cmd.Context(), imageReference, conf.Upgrade.Image, conf.Upgrade.Measurements) + return upgrader.UpgradeImage(ctx, imageReference, conf.Upgrade.Image, conf.Upgrade.Measurements) } func parseUpgradeApplyFlags(cmd *cobra.Command) (upgradeApplyFlags, error) { @@ -116,11 +119,6 @@ func parseUpgradeApplyFlags(cmd *cobra.Command) (upgradeApplyFlags, error) { return upgradeApplyFlags{}, err } - helmFlag, err := cmd.Flags().GetBool("helm") - if err != nil { - return upgradeApplyFlags{}, err - } - yes, err := cmd.Flags().GetBool("yes") if err != nil { return upgradeApplyFlags{}, err @@ -136,19 +134,18 @@ func parseUpgradeApplyFlags(cmd *cobra.Command) (upgradeApplyFlags, error) { return upgradeApplyFlags{}, fmt.Errorf("parsing force argument: %w", err) } - return upgradeApplyFlags{configPath: configPath, helmFlag: helmFlag, yes: yes, upgradeTimeout: timeout, force: force}, nil + return upgradeApplyFlags{configPath: configPath, yes: yes, upgradeTimeout: timeout, force: force}, nil } type upgradeApplyFlags struct { configPath string - helmFlag bool yes bool upgradeTimeout time.Duration force bool } type cloudUpgrader interface { - Upgrade(ctx context.Context, imageReference, imageVersion string, measurements measurements.M) error + UpgradeImage(ctx context.Context, imageReference, imageVersion string, measurements measurements.M) error UpgradeHelmServices(ctx context.Context, config *config.Config, timeout time.Duration, allowDestructive bool) error } diff --git a/cli/internal/cmd/upgradeapply_test.go b/cli/internal/cmd/upgradeapply_test.go index 218815c68..42578ba4a 100644 --- a/cli/internal/cmd/upgradeapply_test.go +++ b/cli/internal/cmd/upgradeapply_test.go @@ -72,7 +72,7 @@ type stubUpgrader struct { helmErr error } -func (u stubUpgrader) Upgrade(context.Context, string, string, measurements.M) error { +func (u stubUpgrader) UpgradeImage(context.Context, string, string, measurements.M) error { return u.err }