OCPBUGS-100303: podman-etcd: resolve force_new_cluster race during simultaneous dual-start - #2194
Conversation
|
Can one of the project admins check and authorise this run please: https://haci.fast.eng.rdu2.dc.redhat.com/job/resource-agents/job/resource-agents-pipeline/job/PR-2194/1/input |
|
Can one of the project admins check and authorise this run please: https://haci.fast.eng.rdu2.dc.redhat.com/job/resource-agents/job/resource-agents-pipeline/job/PR-2194/2/input |
|
Can one of the project admins check and authorise this run please: https://haci.fast.eng.rdu2.dc.redhat.com/job/resource-agents/job/resource-agents-pipeline/job/PR-2194/3/input |
|
Can one of the project admins check and authorise this run please: https://haci.fast.eng.rdu2.dc.redhat.com/job/resource-agents/job/resource-agents-pipeline/job/PR-2194/4/input |
03eae0f to
e76cd92
Compare
|
Can one of the project admins check and authorise this run please: https://haci.fast.eng.rdu2.dc.redhat.com/job/resource-agents/job/resource-agents-pipeline/job/PR-2194/5/input |
e76cd92 to
5f25c1a
Compare
|
Can one of the project admins check and authorise this run please: https://haci.fast.eng.rdu2.dc.redhat.com/job/resource-agents/job/resource-agents-pipeline/job/PR-2194/6/input |
5f25c1a to
02a6f02
Compare
|
Can one of the project admins check and authorise this run please: https://haci.fast.eng.rdu2.dc.redhat.com/job/resource-agents/job/resource-agents-pipeline/job/PR-2194/7/input |
02a6f02 to
742f408
Compare
|
Can one of the project admins check and authorise this run please: https://haci.fast.eng.rdu2.dc.redhat.com/job/resource-agents/job/resource-agents-pipeline/job/PR-2194/8/input |
742f408 to
561b994
Compare
|
Can one of the project admins check and authorise this run please: https://haci.fast.eng.rdu2.dc.redhat.com/job/resource-agents/job/resource-agents-pipeline/job/PR-2194/9/input |
561b994 to
6596483
Compare
|
Can one of the project admins check and authorise this run please: https://haci.fast.eng.rdu2.dc.redhat.com/job/resource-agents/job/resource-agents-pipeline/job/PR-2194/10/input |
fonta-rh
left a comment
There was a problem hiding this comment.
Solid design -- the conflict verdict + post-set guard + stale-claim cleanup is the right approach over the old OCF_ERR_CONFIGURED deadlock. Four items below, three blocking.
| ocf_log err "force_new_cluster holders changed after decision ($fnc_guard_holders): retrying" | ||
| return "$OCF_ERR_GENERIC" | ||
| fi | ||
| fi |
There was a problem hiding this comment.
Non-blocking: two requests on this guard.
-
Add a comment documenting the theoretical failure mode: if CPG attribute propagation exceeds 3s (no hard SLA from attrd), both nodes pass this guard, both force-new-cluster, and the learner wipe at line 2550 destroys the loser's data. Probability is very low (CPG is typically sub-100ms, so this is a 30x+ margin), but documenting it helps future debugging.
-
The guard is silent on the success path. Add a log after the check passes:
ocf_log notice "post-set guard: $NODENAME is sole FNC holder ($fnc_guard_holders), proceeding with force-new-cluster"If this message appears on both nodes within the same 3s window, that is conclusive evidence of a double-FNC event. The current err-level log at line 2504 only fires when the guard catches a conflict -- we need the symmetric case logged too.
There was a problem hiding this comment.
Addressed. Added the residual risk comment describing the attrd propagation window and the learner data wipe consequence. Also added the success path notice log so both nodes passing within 3s is visible evidence.
| clear_force_new_cluster | ||
| sleep 2 | ||
| holders=$(get_force_new_cluster 2>/dev/null) |
There was a problem hiding this comment.
Blocking: both error paths in this function are swallowed.
Line 1296: clear_force_new_cluster return is unchecked. If the delete fails, FNC persists, the post-set guard at line 2495 fires and proceeds with FNC start -- but the EQUAL verdict intended a normal start. Same data on both nodes so no data loss, but unnecessary learner rebuild.
Line 1298: get_force_new_cluster 2>/dev/null discards the return code. On CIB failure, holders is empty, grep does not match the peer, function concludes "joint normal start." Since FNC was already cleared at line 1296, is_force_new_cluster at 2495 returns false and the post-set guard is entirely bypassed -- proceeding blind.
Fix: check both return codes, return OCF_ERR_GENERIC on failure.
There was a problem hiding this comment.
Addressed. Both clear_force_new_cluster and get_force_new_cluster return codes are now checked, returning OCF_ERR_GENERIC on failure.
There was a problem hiding this comment.
Updated: the caller in the EQUAL case branch now propagates the return code (if ! fnc_equal_clear_and_confirm; then return OCF_ERR_GENERIC), matching the ADMIN branch style. Both the in-function checks and the caller propagation are in place so the full error chain is covered.
| # All inputs read from CIB (symmetric); each node wrote revision and | ||
| # cluster_id before setting FNC, so both are visible wherever both | ||
| # FNC attributes are. | ||
| fnc_conflict_verdict() | ||
| { | ||
| local local_rev peer_rev peer_node_name | ||
|
|
||
| peer_node_name=$(get_peer_node_name) | ||
| local_rev=$(get_cib_revision "$NODENAME") | ||
| peer_rev=$(get_cib_revision "$peer_node_name") | ||
| ocf_log info "FNC conflict inputs: $NODENAME=$local_rev $peer_node_name=$peer_rev" | ||
|
|
||
| if is_numeric "$local_rev" && is_numeric "$peer_rev"; then | ||
| if [ "$local_rev" -gt "$peer_rev" ]; then | ||
| echo "$NODENAME" | ||
| elif [ "$local_rev" -lt "$peer_rev" ]; then | ||
| echo "$peer_node_name" | ||
| else | ||
| local local_cid peer_cid | ||
| local_cid=$(attribute_node_cluster_id get) | ||
| peer_cid=$(attribute_node_cluster_id_peer) |
There was a problem hiding this comment.
Blocking: the comment at line 1252 states "All inputs read from CIB (symmetric)" but attribute_node_cluster_id get reads from disk (jq on $ETCD_REVISION_JSON, line 915), while attribute_node_cluster_id_peer reads from CIB (crm_attribute --query). Since attribute_node_cluster_id update swallows CIB write failures (lines 929-934, returns OCF_SUCCESS on error), a silent write failure produces asymmetric verdicts: one node sees EQUAL, the other sees ADMIN.
Self-heals on retry (ADMIN returns OCF_ERR_GENERIC), not a data-loss path, but the comment is wrong and the asymmetry is avoidable.
Fix: add a get_cib_cluster_id helper analogous to get_cib_revision and use it for both local and peer reads here. Correct the comment.
There was a problem hiding this comment.
Good catch, this mirrors the original bug's disk vs CIB asymmetry. Added get_cib_cluster_id helper (same pattern as get_cib_revision) and both sides now read from CIB. Comment corrected.
| peer_has_pending_start() | ||
| { | ||
| local rc | ||
| cibadmin -Q --xpath "//node_state[@uname='$1']//lrm_rsc_op[@operation='start'][@op-status='-1']" >/dev/null 2>&1 |
There was a problem hiding this comment.
Blocking: this xpath matches pending starts on any resource on the peer, not just the etcd resource. The // descendant-axis from node_state hits lrm_rsc_op across all lrm_resource elements.
During cold boot (the exact scenario this PR targets), co-located resources (VIP, STONITH) routinely have pending starts. A pending VIP start causes peer_has_pending_start to return 0, blocking stale-claim cleanup. The post-set guard then sees 2 holders and aborts -- delaying recovery by one or more retry cycles.
Fix: scope to the etcd resource:
cibadmin -Q --xpath "//node_state[@uname='$1']//lrm_resource[@id='$OCF_RESOURCE_INSTANCE']/lrm_rsc_op[@operation='start'][@op-status='-1']" >/dev/null 2>&1There was a problem hiding this comment.
Addressed. Scoped to the etcd resource using lrm_resource[@id] with the instance suffix stripped (${OCF_RESOURCE_INSTANCE%%:*}) for clone safety. Re-verified the xpath on a live cluster in both directions: exit 0 during a real pending etcd start, exit 105 when healthy.
6596483 to
5ce1355
Compare
|
Can one of the project admins check and authorise this run please: https://haci.fast.eng.rdu2.dc.redhat.com/job/resource-agents/job/resource-agents-pipeline/job/PR-2194/11/input |
5ce1355 to
8c338eb
Compare
|
Can one of the project admins check and authorise this run please: https://haci.fast.eng.rdu2.dc.redhat.com/job/resource-agents/job/resource-agents-pipeline/job/PR-2194/12/input |
clobrano
left a comment
There was a problem hiding this comment.
The logic is sounds to me. It makes much more sense to let both nodes read from the same source.
I only left a couple of comments and an idea, but LGTM
| } | ||
|
|
||
| # Read a node's revision from CIB (not local disk) for symmetric evaluation. | ||
| get_cib_revision() |
There was a problem hiding this comment.
Nitpick (it would help maintenance)
I think this function could replace attribute_node_revision_peer, or be used inside it to avoid multiple calls of the same line crm_attribute ... --name "revision".
There was a problem hiding this comment.
Good call, delegated attribute_node_revision_peer to get_cib_revision so the query lives in one place.
| } | ||
|
|
||
| # Read a node's cluster_id from CIB (not local disk) for symmetric evaluation. | ||
| get_cib_cluster_id() |
There was a problem hiding this comment.
Isn't it the same as attribute_node_cluster_id get?
There was a problem hiding this comment.
They look similar but read from different sources: attribute_node_cluster_id get reads local disk (jq on the revision JSON file), get_cib_cluster_id reads from the CIB. That disk vs CIB asymmetry was the same class of bug that caused the original race, which is why the helper exists. Added a NOTE comment above attribute_node_cluster_id to make this clear for future readers.
| fnc_admin_escalate "$fnc_holders" | ||
| return "$OCF_ERR_GENERIC" | ||
| ;; | ||
| *) |
There was a problem hiding this comment.
A comment here might help a newcomer to understand what's happening here without reading through the full logic :)
There was a problem hiding this comment.
Addressed, added a comment explaining the verdict named the peer and we yield.
| # Residual risk: if attrd propagation exceeds 3s (no hard SLA, but | ||
| # typically sub-100ms), both nodes pass this guard and both | ||
| # force-new-cluster; the loser's data directory is wiped during | ||
| # learner rejoin. Probability is very low given the 30x+ margin. |
There was a problem hiding this comment.
If we make revision and cluster-id lifetime reboot, and let each node write them to CIB at startup, isn't reading the data correctly from CIB (i.e., matching what we have locally) a sign that attrd have been propagated?
There was a problem hiding this comment.
Took your round-trip idea and applied it to the FNC write itself so it covers the actual race window. The revision/cluster_id round-trip would prove propagation up to a point before either node claims, but the FNC write happens later, so both could still pass unaware. With the FNC attribute as the barrier, CIB total ordering guarantees the later claimant always sees the conflict. Faster than the fixed 3s on healthy clusters and robust under load. Thanks for the insight.
| # Residual risk: if attrd propagation exceeds 3s (no hard SLA, but | ||
| # typically sub-100ms), both nodes pass this guard and both | ||
| # force-new-cluster; the loser's data directory is wiped during | ||
| # learner rejoin. Probability is very low given the 30x+ margin. |
There was a problem hiding this comment.
If we make revision and cluster-id lifetime reboot, and let each node write them to CIB at startup, isn't reading the data correctly from CIB (i.e., matching what we have locally) a sign that the attrd have been propagated?
This should let the resource agent pass this step quicker than 3s most of the times and also deal with longer network delays in heavy network load scenarios.
8c338eb to
c11b5d7
Compare
|
Can one of the project admins check and authorise this run please: https://haci.fast.eng.rdu2.dc.redhat.com/job/resource-agents/job/resource-agents-pipeline/job/PR-2194/13/input |
|
/lgtm |
oalbrigt
left a comment
There was a problem hiding this comment.
Just a few changes, and it should be good to go.
| return 1 | ||
| } | ||
|
|
||
| is_numeric() |
There was a problem hiding this comment.
You should be able to use ocf_is_decimal() instead.
There was a problem hiding this comment.
Done, switched to ocf_is_decimal and removed the custom is_numeric function.
| } | ||
|
|
||
| # Read a node's revision from CIB (not local disk) for symmetric evaluation. | ||
| get_cib_revision() |
There was a problem hiding this comment.
Can you merge these 2 functions into get_attribute() or a similarly named function? Seems like the only difference is the attribute name.
There was a problem hiding this comment.
Done, merged both into get_cib_attribute(node, attr_name). Also updated attribute_node_revision_peer to delegate to it.
|
|
||
| peer_node_name=$(get_peer_node_name) | ||
| if ! clear_force_new_cluster; then | ||
| ocf_log err "fnc_equal_clear_and_confirm: failed to clear own FNC" |
There was a problem hiding this comment.
You probably want to use ocf_exit_reason() for these errors.
There was a problem hiding this comment.
Done, switched both to ocf_exit_reason.
| attribute_node_cluster_id update | ||
| attribute_node_revision update | ||
|
|
||
| if [ -n "${OCF_RESKEY_test_race_delay:-}" ]; then |
There was a problem hiding this comment.
There shouldnt be any reason to do :- in a if [ -n ... ]
There was a problem hiding this comment.
Done, removed the :- at both hook sites.
| # bounded by CIB ordering correctness, not timing. | ||
| if is_force_new_cluster; then | ||
| local fnc_guard_holders fnc_guard_wait=0 | ||
| while true; do |
There was a problem hiding this comment.
while :; is probably more effective here, as it's an sh internal, and not a separate command.
There was a problem hiding this comment.
Done, switched to while :; do.
…multaneous dual-start When both TNF nodes restart simultaneously, CIB replication delay can cause both to set the force_new_cluster attribute before either sees the other's write. The existing multi-holder check returned OCF_ERR_CONFIGURED (fatal, no retry), causing permanent etcd outage. Replace the fatal abort with a four-outcome conflict verdict (fnc_conflict_verdict) that reads both revisions and cluster_ids from CIB via get_cib_attribute for symmetric evaluation: - Winner (higher revision): force-new-clusters, loser yields - EQUAL (same revision + same cluster_id): both clear FNC and start normally without learner rebuild - ADMIN (no data, or equal revision with different cluster_id): never guess -- soft-fail with recovery guidance showing actual holder names - Active voter peer: always yield regardless of verdict Add an adaptive post-set safety guard that waits for the local FNC write to be visible in the CIB replica (write round-trip through the DC), then verifies this node is the sole holder. CIB total ordering guarantees that once our write is visible, any peer claim ordered before ours is visible too -- the later claimant always detects the conflict. Adaptive: passes as soon as propagation completes instead of a fixed sleep. On any anomaly it soft-fails and lets Pacemaker retry. The data-backed winner in the early check clears stale peer FNC when the peer is not starting, not active, and has no pending start in the CIB. peer_has_pending_start queries the CIB status section scoped to the etcd resource (avoids false positives from co-located resources during cold boot). Accepts exit codes 105 (Pacemaker 2.x) and 6 (legacy 1.x). Query errors treated as "pending" (fail-safe). Dual-claim always logged at err level even when auto-resolved. All error paths return OCF_ERR_GENERIC (soft, retriable). Signed-off-by: Neil Hamza <nhamza@redhat.com>
Add OCF_RESKEY_test_race_delay env-gated sleep hooks at two injection points for deterministic race reproduction. Both hooks are inert when the variable is unset. Intentionally not declared in RA metadata. This commit is separable -- drop it if maintainers prefer keeping test instrumentation downstream only. Signed-off-by: Neil Hamza <nhamza@redhat.com>
c11b5d7 to
6638f20
Compare
|
Can one of the project admins check and authorise this run please: https://haci.fast.eng.rdu2.dc.redhat.com/job/resource-agents/job/resource-agents-pipeline/job/PR-2194/14/input |
|
Thanks. |
…ce_new_cluster set_force_new_cluster() writes the CIB attribute without checking whether the peer already holds it. In the start path a lost race is caught by the post-set guard (OCPBUGS-100303, PR ClusterLabs#2194) but costs a full failed-start retry cycle; in the monitor path (detect_cluster_leadership_loss) there is no protection at all. Add a holder pre-check: query get_force_new_cluster() before writing and return rc=2 if any non-self node holds the attribute. This narrows the CHECK->SET window from the caller's decision point down to the primitive itself, where the test_race_delay hook is now positioned. Return-code contract: 0 - attribute set (holders were empty or self-only) 1 - crm/CIB error (caller should soft-fail) 2 - another node already holds force_new_cluster (caller should defer) Self-holding still writes (idempotent attrd update, keeps recovery robust if a prior cycle died between set and restart). This is a churn-reducing first layer, not a correctness fix. The merged post-set guard and multi-holder tiebreaker remain the correctness mechanism (CIB total ordering: once our write is visible, any peer claim ordered before ours is visible too). Signed-off-by: Neil Hamza <nhamza@redhat.com>
…ce_new_cluster set_force_new_cluster() writes the CIB attribute without checking whether the peer already holds it. The start path already has an outer holder check (~line 2370) that catches the common case, and a lost race past that check is caught by the post-set guard (OCPBUGS-100303, PR ClusterLabs#2194), but the guard costs a full failed-start retry cycle. The monitor path (detect_cluster_leadership_loss) has no protection at all: in a symmetric "no leader" partition both nodes write FNC and assert standalone_node, converging only after restart churn through the tiebreaker. Add a holder pre-check inside set_force_new_cluster(): query get_force_new_cluster() before writing and return rc=2 if any non-self node holds the attribute. In the start path this covers the residual window between the outer holder check and the write. In the monitor path it is the sole defense — the bigger vector, since the monitor had no holder awareness at all. The test_race_delay hook is now positioned between pre-check and write to exercise exactly this window. Return-code contract: 0 - attribute set (holders were empty or self-only) 1 - crm/CIB error (caller should soft-fail) 2 - another node already holds force_new_cluster (caller should defer) Self-holding still writes (idempotent attrd update, keeps recovery robust if a prior cycle died between set and restart). This is a churn-reducing first layer, not a correctness fix. The merged post-set guard and multi-holder tiebreaker remain the correctness mechanism (CIB total ordering: once our write is visible, any peer claim ordered before ours is visible too). Signed-off-by: Neil Hamza <nhamza@redhat.com>
…ce_new_cluster set_force_new_cluster() writes the CIB attribute without checking whether the peer already holds it. The start path already has an outer holder check (~line 2370) that catches the common case, and a lost race past that check is caught by the post-set guard (OCPBUGS-100303, PR ClusterLabs#2194), but the guard costs a full failed-start retry cycle. The monitor path (detect_cluster_leadership_loss) has no protection at all: in a symmetric "no leader" partition both nodes write FNC and assert standalone_node, converging only after restart churn through the tiebreaker. Add a holder pre-check inside set_force_new_cluster(): query get_force_new_cluster() before writing and return rc=2 if any non-self node holds the attribute. In the start path this covers the residual window between the outer holder check and the write. In the monitor path it is the sole defense — the bigger vector, since the monitor had no holder awareness at all. The test_race_delay hook is now positioned between pre-check and write to exercise exactly this window. Return-code contract: 0 - attribute set (holders were empty or self-only) 1 - crm/CIB error (caller should soft-fail) 2 - another node already holds force_new_cluster (caller should defer) Self-holding still writes (idempotent attrd update, keeps recovery robust if a prior cycle died between set and restart). This is a churn-reducing first layer, not a correctness fix. The merged post-set guard and multi-holder tiebreaker remain the correctness mechanism (CIB total ordering: once our write is visible, any peer claim ordered before ours is visible too). Signed-off-by: Neil Hamza <nhamza@redhat.com>
set_force_new_cluster() writes the CIB attribute without checking whether the peer already holds it. The start path has an outer holder check that catches the common case, and a lost race past it is caught by the post-set guard (OCPBUGS-100303, PR ClusterLabs#2194), but the guard costs a full failed-start retry cycle. The monitor path (detect_cluster_leadership_loss) has no protection at all: in a symmetric "no leader" partition both nodes write FNC and assert standalone_node, converging only after restart churn through the tiebreaker. Add a holder pre-check inside set_force_new_cluster(): query get_force_new_cluster() before writing and return rc=2 if any non-self node holds the attribute. In the start path this covers the residual window between the outer holder check and the write; in the monitor path it is the sole defense. The test_race_delay hook moves between pre-check and write to exercise exactly this window. Return-code contract: 0 - attribute set (holders were empty or self-only) 1 - crm/CIB error (caller should soft-fail) 2 - another node already holds force_new_cluster (caller should defer) Handle rc=2 at the three call sites. Start path (start_resources_count=1 and =2): set JOIN_AS_LEARNER=true and continue — the peer holding FNC is driving recovery, and converting in place avoids one guard-triggered retry cycle; the post-set guard (gated on is_force_new_cluster) stays inert since this node never set FNC. In the =2 "newer" arm this means first-claimant-wins in the residual window: for the peer to claim first despite our "newer" verdict its comparison must have used stale saved attributes — the same data fnc_conflict_verdict would read, so the tiebreaker is no more authoritative there. Monitor path: skip set_standalone_node, log the deferral, and still return OCF_ERR_GENERIC (contract unchanged); the restart lands in podman_start which joins as learner. No post-set guard on the monitor path: it is timeout-sensitive, and the restart -> start-path tiebreaker is the backstop. Self-holding still writes (idempotent attrd update, keeps recovery robust if a prior cycle died between set and restart). If the peer's claim is stale (it never starts), the learner wait burns its timeout and retries through the existing single-peer-holder path — a pre-existing exposure this change converges to rather than introduces. This is a churn-reducing first layer, not a correctness fix. The merged post-set guard and multi-holder tiebreaker remain the correctness mechanism (CIB total ordering: once our write is visible, any peer claim ordered before ours is visible too). Signed-off-by: Neil Hamza <nhamza@redhat.com>
The function is named try_set_force_new_cluster to reflect that it may decline to write. set_force_new_cluster() writes the CIB attribute without checking whether the peer already holds it. The start path has an outer holder check that catches the common case, and a lost race past it is caught by the post-set guard (OCPBUGS-100303, PR ClusterLabs#2194), but the guard costs a full failed-start retry cycle. The monitor path (detect_cluster_leadership_loss) has no protection at all: in a symmetric "no leader" partition both nodes write FNC and assert standalone_node, converging only after restart churn through the tiebreaker. Add a holder pre-check inside try_set_force_new_cluster(): query get_force_new_cluster() before writing and return rc=2 if any non-self node holds the attribute. In the start path this covers the residual window between the outer holder check and the write; in the monitor path it is the sole defense. The test_race_delay hook moves between pre-check and write to exercise exactly this window. Return-code contract: 0 - attribute set (holders were empty or self-only) 1 - crm/CIB error (caller should soft-fail) 2 - another node already holds force_new_cluster (caller should defer) Handle rc=2 at the three call sites. Start path (start_resources_count=1 and =2): set JOIN_AS_LEARNER=true and continue — the peer holding FNC is driving recovery, and converting in place avoids one guard-triggered retry cycle; the post-set guard (gated on is_force_new_cluster) stays inert since this node never set FNC. In the =2 "newer" arm this means first-claimant-wins in the residual window: for the peer to claim first despite our "newer" verdict its comparison must have used stale saved attributes — the same data fnc_conflict_verdict would read, so the tiebreaker is no more authoritative there. Monitor path: skip set_standalone_node, log the deferral, and still return OCF_ERR_GENERIC (contract unchanged); the restart lands in podman_start which joins as learner. No post-set guard on the monitor path: it is timeout-sensitive, and the restart -> start-path tiebreaker is the backstop. Self-holding still writes (idempotent attrd update, keeps recovery robust if a prior cycle died between set and restart). If the peer's claim is stale (it never starts), the learner wait burns its timeout and retries through the existing single-peer-holder path — a pre-existing exposure this change converges to rather than introduces. This is a churn-reducing first layer, not a correctness fix. The merged post-set guard and multi-holder tiebreaker remain the correctness mechanism (CIB total ordering: once our write is visible, any peer claim ordered before ours is visible too). Signed-off-by: Neil Hamza <nhamza@redhat.com>
Problem
When both TNF control-plane nodes restart simultaneously (dual power loss, certificate expiry), the podman-etcd resource agent can enter a permanent deadlock. Both nodes set the force_new_cluster CIB attribute due to two race vectors:
CIB replication delay: Both nodes update their revision in CIB then call compare_revision(), which reads local disk for self but CIB for peer. If the peer's update hasn't propagated, both see stale peer revision and both determine they're "newer."
Inconsistent start_resources_count: During simultaneous boot, OCF_RESKEY_CRM_meta_notify_start_resource can disagree between nodes, causing both to enter the case 1 path which sets force_new_cluster for any revision that isn't "older" (including "equal").
Once both hold force_new_cluster, the multi-holder safety check returns OCF_ERR_CONFIGURED -- a fatal error with no automatic retry -- causing total etcd outage requiring manual intervention.
Considered alternative
An atomic claim via cibadmin --create (create-fails-if-exists) would eliminate the verdict, guard, and stale-claim machinery in one stroke. However it sacrifices --lifetime reboot auto-cleanup and requires staleness machinery (boot-id, dead-claimant detection) with similar net complexity and higher migration risk for clusters already running with per-node FNC attributes.
Solution
This PR addresses OCPBUGS-100303 (force_new_cluster race during simultaneous dual-start). Fixing the bug correctly required addressing three additional aspects beyond the race itself: the fatal abort that prevents self-healing, the stale-claim cleanup for banned peers, and the active-peer guard that prevents split-brain.
The PR contains 2 commits:
Conflict verdict (fnc_conflict_verdict)
When both nodes hold FNC, the verdict function determines the outcome using CIB-sourced revision and cluster_id data (symmetric inputs -- both nodes evaluate the same data):
Alphabetic node-name guessing was explicitly rejected -- when there is no data to distinguish the nodes, the agent reports the conflict and lets an administrator inspect the etcd data directories rather than risking a wrong choice.
Post-set safety guard
After the start-flow decision logic but before container creation, a one-shot guard sleeps 3s (load-bearing: lets a peer's in-flight FNC write propagate through attrd/CIB) then verifies this node is the sole visible holder. On any anomaly -- two holders (race), zero holders (own write not yet visible), or sole holder is the peer -- it soft-fails with OCF_ERR_GENERIC and lets Pacemaker retry. The early multi-holder check resolves the conflict on the retry with fresh notify vars.
Stale-claim cleanup
The data-backed winner in the early multi-holder check clears a stale peer FNC attribute when the peer's resource is neither starting, active, nor has an in-flight start in the CIB. This handles the banned-peer scenario where reboot-lifetime attributes persist until membership loss and no agent ever runs to yield. peer_has_pending_start queries the CIB status section for pending ops (accepts both exit code 105/CRM_EX_NOSUCH for Pacemaker 2.x and 6/ENXIO for legacy 1.x -- both verified empirically on Pacemaker 2.1.10; xpath positive-match against real pending start ops also confirmed). Query errors are treated as "pending" to prevent deletion during CIB instability.
Other design decisions
New helper functions
OCF_RESKEY_test_race_delay is an intentionally undeclared test hook for deterministic race reproduction (commit 2, separable). Both injection points are inert when the variable is unset.
What this PR touches beyond the race fix
Testing
Live 4.23 TNF cluster (two-node DualReplica, OCP 4.23.0-0.nightly-2026-08-01-032511, Pacemaker 2.1.10, dev-scripts on AWS). Tests run across two cluster deployments. All tests ran without manual intervention -- the cluster self-healed in every scenario. All 35 Cluster Operators verified healthy after each test.
Test 1: Conflict resolution resolves existing deadlock
Test 2: Quorum-loss regression (kill peer VM)
Test 3: Normal single-node failover (fence master-0)
Test 4: Equal-revision EQUAL verdict (joint normal start)
Test 5: ADMIN verdict (code-trace verified)
Test 6: Race window exercised via test hook
Test 7: Stale-claim deletion (banned loser)
Verification: cibadmin exit code and peer_has_pending_start xpath
//node_state[@uname='master-1']//lrm_rsc_op[@operation='start'][@op-status='-1']returned exit code 0 with the pending op element. The xpath is correct.Final cluster state
Related: OCPBUGS-86897 (fixed by #2181), OCPBUGS-100303