Bugfix: Multi-hop etcd upgrade (#2275)

* fix a glitch in the upgrade planning for multi-hop upgrades that selects the wrong etcd base version

* update e-ref

assets/robotest/config/pr.sh

@@ -24,8 +24,7 @@ INTERMEDIATE_UPGRADE_VERSION=$(recommended_upgrade_tag $(branch 6.1.x))
 # For example: v3.3.11 to v3.3.22 to v3.4.9
 declare -A INTERMEDIATE_UPGRADE_MAP
 INTERMEDIATE_UPGRADE_MAP[$(recommended_upgrade_tag $(branch 5.5.x))]="centos:7"
-# 5.5.10 is disabled to an issue with etcd upgrades which needs further investigation (#2013).
-# INTERMEDIATE_UPGRADE_MAP[5.5.10]="centos:7"
+INTERMEDIATE_UPGRADE_MAP[5.5.10]="centos:7"
 
 # 6.2 and 6.3 ignored in PR builds per https://github.com/gravitational/gravity/pull/1760#pullrequestreview-437838773
 # UPGRADE_MAP[$(recommended_upgrade_tag $(branch 6.3.x))]="redhat:7" # compatible non-LTS version

lib/update/cluster/steps.go

@@ -43,13 +43,14 @@ import (
 	log "github.com/sirupsen/logrus"
 )
 
-func (r *phaseBuilder) initSteps(ctx context.Context) error {
+func (r *phaseBuilder) initSteps(ctx context.Context) (err error) {
+	var installedOrUpgradedEtcdVersion *semver.Version
+
 	if !r.isDirectUpgrade() {
-		steps, err := r.buildIntermediateSteps(ctx)
+		r.steps, installedOrUpgradedEtcdVersion, err = r.buildIntermediateSteps(ctx)
 		if err != nil {
 			return trace.Wrap(err)
 		}
-		r.steps = steps
 	}
 	installedRuntimeFunc := getRuntimePackageFromManifest(r.installedApp.Manifest)
 	updateRuntimeFunc := getRuntimePackageFromManifest(r.updateApp.Manifest)
@@ -71,19 +72,21 @@ func (r *phaseBuilder) initSteps(ctx context.Context) error {
 	if err != nil {
 		return trace.Wrap(err)
 	}
-	// TODO: should somehow maintain etcd version invariant across runtime packages
-	installedEtcdVersion, err := getEtcdVersionFromManifest(r.installedApp.Manifest, r.packages)
-	if err != nil && !trace.IsNotFound(err) {
-		return trace.Wrap(err)
+
+	if installedOrUpgradedEtcdVersion == nil {
+		installedOrUpgradedEtcdVersion, err = getEtcdVersionFromManifest(r.installedApp.Manifest, r.packages)
+		if err != nil && !trace.IsNotFound(err) {
+			return trace.Wrap(err)
+		}
 	}
-	if installedEtcdVersion == nil {
-		installedEtcdVersion = &r.currentEtcdVersion
+	if installedOrUpgradedEtcdVersion == nil {
+		installedOrUpgradedEtcdVersion = &r.currentEtcdVersion
 	}
 	updateEtcdVersion, err := getEtcdVersionFromManifest(r.updateApp.Manifest, r.packages)
 	if err != nil {
 		return trace.Wrap(err)
 	}
-	etcd := shouldUpdateEtcd(*installedEtcdVersion, *updateEtcdVersion)
+	etcd := shouldUpdateEtcd(*installedOrUpgradedEtcdVersion, *updateEtcdVersion)
 	// check if OpenEBS integration has been enabled in the new application
 	openEBSEnabled := !r.installedApp.Manifest.OpenEBSEnabled() && r.updateApp.Manifest.OpenEBSEnabled()
 	r.targetStep = newTargetUpdateStep(updateStep{
@@ -101,27 +104,28 @@ func (r phaseBuilder) hasRuntimeUpdates() bool {
 	return len(r.steps) != 0 || len(r.targetStep.runtimeUpdates) != 0
 }
 
-func (r phaseBuilder) buildIntermediateSteps(context.Context) (updates []intermediateUpdateStep, err error) {
+func (r phaseBuilder) buildIntermediateSteps(context.Context) (updates []intermediateUpdateStep, lastUpgradeEtcdVersion *semver.Version, err error) {
 	result, err := r.collectIntermediateSteps()
 	if err != nil {
-		return nil, trace.Wrap(err)
+		return nil, nil, trace.Wrap(err)
 	}
 	updates = make([]intermediateUpdateStep, 0, len(result))
 	prevRuntimeApp := r.installedRuntimeApp
 	prevTeleport := r.installedTeleport
 	prevRuntimeFunc := getRuntimePackageFromManifest(r.installedApp.Manifest)
 	for version, update := range result {
 		if err := update.validate(); err != nil {
-			return nil, trace.Wrap(err)
+			return nil, nil, trace.Wrap(err)
 		}
 		installedEtcdVersion, err := getEtcdVersionFromManifest(prevRuntimeApp.Manifest, r.packages)
 		if err != nil {
-			return nil, trace.Wrap(err)
+			return nil, nil, trace.Wrap(err)
 		}
 		updateEtcdVersion, err := getEtcdVersionFromManifest(update.runtimeApp.Manifest, r.packages)
 		if err != nil {
-			return nil, trace.Wrap(err)
+			return nil, nil, trace.Wrap(err)
 		}
+		lastUpgradeEtcdVersion = updateEtcdVersion
 		update.etcd = shouldUpdateEtcd(*installedEtcdVersion, *updateEtcdVersion)
 		result[version] = update
 		serverUpdates, err := r.intermediateConfigUpdates(
@@ -130,21 +134,21 @@ func (r phaseBuilder) buildIntermediateSteps(context.Context) (updates []interme
 			prevTeleport, &update.teleport,
 			r.operator)
 		if err != nil {
-			return nil, trace.Wrap(err)
+			return nil, nil, trace.Wrap(err)
 		}
 		log.WithField("updates", serverUpdates).Info("Intermediate server upgrade step.")
 		update.servers = serverUpdates
 		update.runtimeUpdates, err = runtimeUpdates(prevRuntimeApp, update.runtimeApp, r.installedApp)
 		if err != nil {
-			return nil, trace.Wrap(err)
+			return nil, nil, trace.Wrap(err)
 		}
 		updates = append(updates, update)
 		prevRuntimeApp = update.runtimeApp
 		prevTeleport = update.teleport
 		prevRuntimeFunc = getRuntimePackageStatic(update.runtime)
 	}
 	sort.Sort(updatesByVersion(updates))
-	return updates, nil
+	return updates, lastUpgradeEtcdVersion, nil
 }
 
 func (r phaseBuilder) collectIntermediateSteps() (result map[string]intermediateUpdateStep, err error) {