Skip to content

Allow concurrent node drain within the same failure domain when failure domains are enabled #383

Description

@geoffrey1330

Enhancement: Per-failure-domain drain concurrency in NodeDrainCoordinatorReconciler


Background

The drain coordinator currently uses a single global concurrency gate controlled by
StorageCluster.status.maxFaultTolerance. At most N workers across the entire
StorageNodeSet may be in the active drain window (shutdown_called, draining,
restart_called) simultaneously.

This is correct for clusters without failure domains — any two workers could be
primary/secondary peers, so draining them at the same time risks data availability.

However, when StorageCluster.spec.enableFailureDomains=true, the cluster topology
provides a stronger safety guarantee: data is replicated across failure domains, not
within them. Draining all workers that share the same failure domain simultaneously
is safe — the cluster was already designed to tolerate that entire domain going offline.
The current global gate is therefore unnecessarily conservative, serialising drains that
could safely run in parallel.


Current Behaviour

countActiveDrains (nodedrain_controller.go:1153) counts all active workers globally.
handleDetected (nodedrain_controller.go:455) blocks any new drain when
activeDrains >= maxFaultTolerance, regardless of whether the waiting worker is in the
same failure domain as the in-progress one.

Example: a cluster with maxFaultTolerance=1 and two failure domains, each with
three workers. Under maintenance, draining domain A requires three sequential drain
cycles — each serialised behind the last — even though all three nodes share the same
fault domain.


Desired Behaviour

When enableFailureDomains=true:

  • Workers in the same failure domain as an already-draining worker are allowed into
    the active drain window concurrently — the domain-level fault tolerance is not exceeded.
  • Workers in a different failure domain are still gated: at most maxFaultTolerance
    distinct failure domains may be actively draining simultaneously.
  • When enableFailureDomains=false (or not set), behaviour is unchanged — the
    existing global counter applies.

Implementation Sketch

1. Resolve each worker's failure domain

Each worker's domain is available via StorageNodeSet.spec.nodeFailureDomains
(map of hostname → int32) with per-node overrides in
StorageNode.spec.overrides.failureDomain. The existing effectiveFailureDomain helper
in storagenode_controller.go already encapsulates this logic and should be reused or
extracted to a shared utility.

2. Change the slot gate in handleDetected

Replace the current check (around nodedrain_controller.go:455):

activeDrains := countActiveDrains(ctx, snCR, apiClient, clusterUUID)
if activeDrains >= maxFaultTolerance {
    // block
}

With a domain-aware check when failure domains are enabled:

if clusterCR.Spec.EnableFailureDomains != nil && *clusterCR.Spec.EnableFailureDomains {
    activeDomains := countActiveFailureDomains(ctx, snCR, apiClient, clusterUUID, workerFailureDomains)
    thisDomain := workerFailureDomains[state.Hostname]
    if !isDomainAlreadyActive(snCR, thisDomain) && activeDomains >= maxFaultTolerance {
        // block — a different domain is already consuming a slot
    }
    // same domain already draining → allow through
} else {
    // existing global path
}

3. Add countActiveFailureDomains

A new helper alongside countActiveDrains that groups active drains by failure domain
number and returns the count of distinct domains currently in the drain window — mirroring
the existing dual-source logic (controller state + backend status).

4. Pass the failure-domain map into processWorker

processWorker currently takes maxFaultTolerance int. The signature needs to accept
the failure-domain map so handleDetected can look up the current worker's domain
without an extra API call.


What Does NOT Change

  • The DrainPhaseShutdownCalled → DrainPhaseDraining → DrainPhaseRestartCalled state
    machine is unchanged.
  • PDB creation, pod labelling, and cleanup are unchanged — each worker still gets its
    own PDB.
  • countActiveDrains is retained unchanged for the enableFailureDomains=false path.
  • The concurrency shouldBreak logic in processWorker (nodedrain_controller.go:294)
    is unchanged.

Acceptance Criteria

  • When enableFailureDomains=false or unset, drain behaviour is identical to today.
  • When enableFailureDomains=true, all workers in the same failure domain can enter
    shutdown_called in the same reconcile cycle without queuing behind each other.
  • When enableFailureDomains=true, workers from a second failure domain are blocked
    until the first domain's active drain count falls below maxFaultTolerance.
  • A worker with no failure domain assigned is treated as domain 0 (no regression
    for partially-configured clusters).
  • Unit tests cover: same-domain parallel allowed, cross-domain gated,
    feature-flag-disabled falls back to global gate.

Metadata

Metadata

Assignees

Labels

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions