PRODENG-2966 mcr upgrade skip detect refactor

- mcr upgrade now:
	- skips if an install was just run
	- skips if the host config has the "skipUpgrade" flag

ALSO:

- small go fmt change on the helm testutil

Signed-off-by: James Nesbitt <[email protected]>

Author: james-nesbitt

pkg/helm/testutil.go

@@ -77,8 +77,6 @@ func NewHelmTestClient(t *testing.T, options ...HelmTestClientOption) *Helm {
 	}
 }
 
-
-
 // InstallCertManagerChart installs cert-manager
 // to use as a chart to query for testing purposes and returns the
 // ReleaseDetails for the chart as well as a function to uninstall the chart.

pkg/mcr/mcr.go

@@ -9,9 +9,9 @@ import (
 )
 
 // EnsureMCRVersion ensure that MCR is running after install/upgrade, and update the host information
-// @NOTE will reboot the machine if MCR isn't detected, and MCR can be force restarted if desired (I could drop the mcr restart, but I kept it in case we want a run-time flag for it.)
+// @NOTE will reboot the machine if MCR isn't detected
 // @SEE PRODENG-2789 : we no longer perform version checks, as the MCR versions don't always match the spec string.
-func EnsureMCRVersion(h *api.Host, specMcrVersion string, forceMCRRestart bool) error {
+func EnsureMCRVersion(h *api.Host, specMcrVersion string) error {
 	currentVersion, err := h.MCRVersion()
 	if err != nil {
 		if err := h.Reboot(); err != nil {
@@ -21,20 +21,12 @@ func EnsureMCRVersion(h *api.Host, specMcrVersion string, forceMCRRestart bool)
 		if err != nil {
 			return fmt.Errorf("%s: failed to query container runtime version after installation: %w", h, err)
 		}
-	} else if !forceMCRRestart {
-		err = h.Configurer.RestartMCR(h)
-		if err != nil {
-			return fmt.Errorf("%s: failed to restart container runtime: %w", h, err)
-		}
-		currentVersion, err = h.MCRVersion()
-		if err != nil {
-			return fmt.Errorf("%s: failed to query container runtime version after restart: %w", h, err)
-		}
+		// as we rebooted the machine, no need to also restart MCR
+		h.Metadata.MCRRestartRequired = false
 	}
 
 	log.Infof("%s: MCR version %s (requested: %s)", h, currentVersion, specMcrVersion)
 	h.Metadata.MCRVersion = currentVersion
-	h.Metadata.MCRRestartRequired = false
 
 	return nil
 }

pkg/product/mke/api/host.go

@@ -348,7 +348,7 @@ func (h *Host) ConfigureMCR() error {
 
 	if h.Metadata.MCRVersion != "" {
 		h.Metadata.MCRRestartRequired = true
-		log.Debugf("%s: host marked for mcr restart", h)
+		log.Debugf("%s: host marked for mcr restart as MCR config was changed", h)
 	}
 
 	return nil

pkg/product/mke/phase/install_mcr.go

@@ -70,11 +70,13 @@ func (p *InstallMCR) installMCR(h *api.Host) error {
 		return fmt.Errorf("%s: failed to authorize docker: %w", h, err)
 	}
 
-	// check MCR is running, maybe rebooting and updating metadata (don't bother restarting MCR, as we just installed/started it
-	if err := mcr.EnsureMCRVersion(h, p.Config.Spec.MCR.Version, false); err != nil {
+	// check MCR is running, maybe rebooting and updating metadata
+	if err := mcr.EnsureMCRVersion(h, p.Config.Spec.MCR.Version); err != nil {
 		return fmt.Errorf("failed while attempting to ensure the installed version %w", err)
 	}
 
+	log.Infof("%s: mcr installed", h)
 	h.Metadata.MCRInstalled = true
+	h.Metadata.MCRRestartRequired = false // we just installed, so a restart is not required
 	return nil
 }

pkg/product/mke/phase/uninstall_mcr.go

@@ -65,7 +65,7 @@ func (p *UninstallMCR) uninstallMCR(h *api.Host, config *api.ClusterConfig) erro
 	}
 
 	if err := h.Configurer.UninstallMCR(h, h.Metadata.MCRInstallScript, config.Spec.MCR); err != nil {
-		return fmt.Errorf("%s: uninstall container runtime: %w", h, err)
+		return fmt.Errorf("%s: uninstall container runtime failed: %w", h, err)
 	}
 
 	log.Infof("%s: mirantis container runtime uninstalled", h)

pkg/product/mke/phase/upgrade_mcr.go

@@ -27,18 +27,21 @@ type UpgradeMCR struct {
 
 // HostFilterFunc returns true for hosts that do not have engine installed.
 func (p *UpgradeMCR) HostFilterFunc(h *api.Host) bool {
-	if h.Metadata.MCRVersion != p.Config.Spec.MCR.Version {
-		return true
+	if h.Metadata.MCRInstalled {
+		// we just did an install, no need to run an upgrade
+		return false
 	}
 	if h.MCRUpgradeSkip {
 		log.Warnf("%s: MCR Upgrade configuration for host instructs launchpad to skip upgrading this host.", h)
 		return false
 	}
-	if p.ForceUpgrade && !h.Metadata.MCRInstalled {
+	if p.ForceUpgrade {
 		log.Warnf("%s: MCR version is already %s but attempting an upgrade anyway because --force-upgrade was given", h, h.Metadata.MCRVersion)
 		return true
 	}
-	return false
+
+	// the following version check prevents upgrades on MCR<25, but MCR25 should always upgrade.
+	return h.Metadata.MCRVersion != p.Config.Spec.MCR.Version
 }
 
 // Prepare collects the hosts.
@@ -180,10 +183,14 @@ func (p *UpgradeMCR) upgradeMCR(h *api.Host) error {
 		return fmt.Errorf("retry count exceeded: %w", err)
 	}
 
-	// check MCR is running, and updating metadata
-	if err := mcr.EnsureMCRVersion(h, p.Config.Spec.MCR.Version, false); err != nil {
+	// ensure that MCR is installed and running
+	if err := mcr.EnsureMCRVersion(h, p.Config.Spec.MCR.Version); err != nil {
 		return fmt.Errorf("failed while attempting to ensure the installed version: %w", err)
 	}
 
+	log.Infof("%s: mcr upgrade has been run", h)
+	log.Debugf("%s: mcr upgrade means that any planned MCR restarts are cancelled", h)
+	// the upgraded machine "may" have been restarted. We don't really know, but we err on the side of reducing restarts. We likely need a flag to force a restart.
+	h.Metadata.MCRRestartRequired = false
 	return nil
 }