Skip to content

fix(spurctld)!: refuse to bootstrap a second raft cluster - #806

Open
pre wants to merge 3 commits into
ROCm:mainfrom
silogen:pr/raft-bootstrap-guard
Open

fix(spurctld)!: refuse to bootstrap a second raft cluster#806
pre wants to merge 3 commits into
ROCm:mainfrom
silogen:pr/raft-bootstrap-guard

Conversation

@pre

@pre pre commented Sep 2, 2026

Copy link
Copy Markdown

Required merge order: main < #806 < #810 < #843 < #844

Related issue:

  • feat(spurctld): support dynamic Raft membership changes (add/remove voter) #341.
    This change does not implement online growth, it refuses the unsafe transition that currently stands in for it. A replica with an empty store that finds a membership it does not belong to stops instead of bootstrapping a second cluster, so the replica count stays fixed at first apply. Dynamic membership through change_membership, tracked in that issue, is what removes the restriction.

Related pull requests:

Motivation

Raising the replica count of a running spurctld StatefulSet silently destroys the
cluster. A replica that starts with an empty Raft store bootstraps a second
one-member cluster instead of joining the existing one. The two then meet, and:

  • the replica that holds the data panics inside RaftCore on a higher term;
  • the job id counter restarts at 1, so new jobs collide with accounting rows that
    already exist;
  • both clusters answer behind the same headless Service, so a read returns
    whichever one the client reached.

Nothing in the current code stops this, and the operator gets no warning. The goal
is to make the unsafe transition impossible rather than merely documented.

Technical Details

A node with an empty store now asks every configured peer whether it already
belongs to a cluster, before it bootstraps.

raft_internal.proto gains a ClusterProbe RPC. It is internal
controller-to-controller plumbing, so slurm.proto is untouched and FFI and REST
are unaffected. The RPC is appended after the existing three, so a peer that
predates it answers UNIMPLEMENTED, which the caller reads as "unknown" and
treats as permission to continue.

The probe returns the peer's effective membership, voters and learners alike, so a learner that lost its volume is still read as a member. Three verdicts follow:

  • no peer answers, or every answer is "unknown" — bootstrap, as before;
  • a peer reports a membership that contains this node id — join it;
  • a peer reports a membership that does not contain this node id — refuse to
    start, with a message that explains why and what to do instead.

The refusal is deliberate. Real online growth needs change_membership, which is
a feature and not a defect fix; until then the replica count is fixed at first
apply, and the example manifests and docs/deployment/kubernetes.rst now say so.

This is a breaking change: a controller that used to start now refuses to, in the
one case where starting corrupted the cluster. Hence the ! in the title.

The docs commit also documents the two execution backends (Pod mode and Agent
mode) as a choice, and corrects the spurd DaemonSet nodeSelector from
spur.amd.com/compute to spur.amd.com/managed, which is the operator's actual
default selector (crates/spur-k8s/src/main.rs). The old value matches no node.

Related:

Test Plan

  1. On a three node RKE2 cluster, deploy spurctld with 1 replica, submit jobs,
    then raise the replica count to 3 and observe what the new replicas do.
  2. Confirm the surviving replica keeps its data and its job id counter.
  3. Confirm the guard does not fire when it should not: roll a new image through a
    healthy three member cluster, and cold start three replicas from empty PVCs.
  4. Unit tests for the three verdicts against a stub ClusterProbe server.

Test Result

Environment: three node RKE2 cluster on cloud VMs, 8 vCPU and 96 GiB each, no GPU.

  • Before this change, the 1 to 3 scale-up produced exactly the failure above:
    spurctld-0 panicked in RaftCore, the next job id was 1 although jobs 1 and 2
    existed, and sinfo gave different answers depending on which replica served it.
    The panicked Pod stayed 1/1 Running with 0 restarts.
  • After this change, the same scale-up leaves spurctld-1 and spurctld-2 in
    CrashLoopBackOff with the refusal message. spurctld-0 keeps both jobs, logs
    no panic, and the next job id is 3.
  • No false refusal. A rolling image upgrade of a healthy three member cluster
    replaced all three replicas with no refusal. A cold start of three replicas from
    empty PVCs elected a leader, replicated, survived a leader kill, and readmitted
    the restarted member.
  • cargo clippy --workspace --exclude spur-ffi --all-targets --locked reports
    nothing; cargo test --locked passes, including five new tests: three that
    drive the verdicts against a stub ClusterProbe gRPC server, one that reads
    a learner back from the probe of a real node, and one that lets an empty
    replica listed as a learner start.

Submission Checklist

@codecov-commenter

codecov-commenter commented Sep 10, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 92.85714% with 12 lines in your changes missing coverage. Please review.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #806      +/-   ##
==========================================
+ Coverage   80.40%   80.44%   +0.04%     
==========================================
  Files         187      187              
  Lines       92752    92912     +160     
==========================================
+ Hits        74577    74741     +164     
+ Misses      18175    18171       -4     
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

pre added 3 commits September 10, 2026 15:48
BREAKING CHANGE: a spurctld with an empty Raft store now refuses to start
when a configured peer already runs a cluster that does not list it. This
turns a silent corruption into a clear startup failure, and it makes the
replica count of a running controller fixed.

A replica added to a running cluster starts with an empty store, exactly
like a first start, so it bootstrapped a NEW cluster. On Kubernetes a
StatefulSet rolling update starts at the highest ordinal and gives each
new ordinal an empty volume, so raising replicas from 1 to 3 made the two
new empty replicas form a cluster and out-vote the one holding the data.
Measured results: the replica with the data panicked in openraft
("range start is greater than range end in BTreeMap"), the job id counter
restarted at 1 and collided with ids already in accounting, and the
replicas answered sinfo differently behind one headless Service.

An empty store now asks each peer over the new ClusterProbe RPC whether it
belongs to a cluster. The probe reports the whole membership, voters and
learners. A peer whose membership contains this node, in either role,
means a member lost its volume, so it starts and catches up. A peer whose
membership does not contain it means bootstrapping would split the
cluster, so startup fails with a message that says the replica count
cannot be raised. A peer that predates the RPC answers UNIMPLEMENTED,
which is read as "no cluster", so mixed versions behave as before.

ClusterProbe is added to raft_internal.proto, which is controller-to-
controller plumbing only. No field tag changes, and slurm.proto is
untouched.

Signed-off-by: Petrus Repo <petrus.repo@amd.com>
The deployment page told the reader to apply spurd.yaml AND operator.yaml.
Both register the same Kubernetes nodes, so a reader who followed it got
two agents claiming every node. They are alternatives: the page now gives
a table of the two topologies and says that Pod mode launches only a
SpurJob, so sbatch and spur submit accept a job that never gets a Pod.

Add a warning that the spurctld replica count is fixed at the first apply,
which is what the controller now enforces.

Example manifest: spurd.yaml selected spur.amd.com/compute, but the
operator default and every other reference is spur.amd.com/managed. A
reader who labelled nodes for one got nothing scheduled by the other.

Signed-off-by: Petrus Repo <petrus.repo@amd.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants