Upgrade never completes when storageServersPerPod > 1

padamson · March 7, 2023, 12:37pm

Hey Folks.

(Bug report in lieu of github issues)

I have storageServersPerPod set to 2 which gives me 12 storage processes spread across 6 pods.

When I attempt to upgrade from 7.1.21 → 7.1.27 the operator 1.4.0 [CORRECTION: 1.14.0] gets stuck with the following message:
"expected 25 processes, got 31 processes ready to restart" as the reason for delaying the upgrade; it never gets beyond this.

Some brief maths later and i found that the 25 was the number of pods i had running, and 31 was the number of processes I had running, the additional 6 being the doubled up storage nodes.

The code that is causing the upgrade to wait indefinitely is the following:

github.com

FoundationDB/fdb-kubernetes-operator/blob/d7376e419789613bbd043d34c3522787986aad9f/controllers/bounce_processes.go#L242


      
          		}
          	}
          
          
	// If we upgrade the cluster wait until all processes are ready for the restart. We don't want to block the restart
          	// if some processes are already upgraded e.g. in the case of version compatible upgrades and we also don't want to
          	// block the restart command if a process is missing longer than the specified GetIgnoreMissingProcessesSeconds.
          	// Those checks should ensure we only run the restart command if all processes that have to be restarted and are connected
          	// to cluster are ready to be restarted.
          	if cluster.IsBeingUpgraded() && (counts.Total()-missingProcesses-upgradedProcesses) != len(addresses) {
          		return nil, &requeue{
          			message:        fmt.Sprintf("expected %d processes, got %d processes ready to restart", counts.Total(), len(addresses)),
          			delayedRequeue: true,
          		}
          	}
          
          
	return addresses, nil
          }
          
          
// getAddressesForUpgrade checks that all processes in a cluster are ready to be
          // upgraded and returns the full list of addresses.
          func getAddressesForUpgrade(logger logr.Logger, r *FoundationDBClusterReconciler, databaseStatus *fdbv1beta2.FoundationDBStatus, lockClient fdbadminclient.LockClient, cluster *fdbv1beta2.FoundationDBCluster, version fdbv1beta2.Version) ([]fdbv1beta2.ProcessAddress, *requeue) {

It looks like it’s expecting the total number of addresses to neatly align with the exact process count, but in my case I have more ready processes than expected!

I reduced storageServersPerPod to 1 successfully, which brought the address count down to exactly 25 and the upgrade immediately progressed. Afterwards I scaled in again afterwards back to 2 per pod, which was a feasible workaround.

johscheuer · March 7, 2023, 6:26pm

Thanks for reporting that issue! The operator version 1.4.0 should work without any issues, as the check is not present there. For the operator version 1.14.0 and newer you are right, the counts.Total() method doesn’t take the storageServersPerPod into account.I open a GitHub issue for that and we should be able to fix that soon.

Created this issue: Upgrade blocked when storageServersPerPod > 1 · Issue #1535 · FoundationDB/fdb-kubernetes-operator · GitHub

johscheuer · March 13, 2023, 3:34pm

JFYI: We have a fix for this bug: Fix upgrade logic for multiple storage server per Pod by johscheuer · Pull Request #1538 · FoundationDB/fdb-kubernetes-operator · GitHub. Thanks again for reporting!