Skip to content

[WIP][DNM]: OCPBUGS-105240: test-only delay MemberRemove to reproduce bootstrap race #1673

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
Neilhamza wants to merge 1 commit into from
+9 −1
Closed

[WIP][DNM]: OCPBUGS-105240: test-only delay MemberRemove to reproduce bootstrap race #1673

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 9 additions & 1 deletion pkg/operator/bootstrapteardown/bootstrap_teardown_controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,8 @@ func NewBootstrapTeardownController(
}

func (c *BootstrapTeardownController) sync(ctx context.Context, _ factory.SyncContext) error {
timeoutCtx, cancelFunc := context.WithTimeout(ctx, 1*time.Minute)
// TEST ONLY (OCPBUGS-105240): extend timeout from 1m to 10m to accommodate the 5m sleep in removeBootstrap(). DO NOT MERGE.
timeoutCtx, cancelFunc := context.WithTimeout(ctx, 10*time.Minute)
defer cancelFunc()

scalingStrategy, err := ceohelpers.GetBootstrapScalingStrategy(c.operatorClient, c.namespaceLister, c.infrastructureLister)
Expand Down Expand Up @@ -143,6 +144,13 @@ func (c *BootstrapTeardownController) removeBootstrap(ctx context.Context, safeT
return fmt.Errorf("error while updating EnoughEtcdMembers: %w", updateErr)
}

// TEST ONLY (OCPBUGS-105240): delay bootstrap member removal by 5 minutes
// to hold the race window open between EtcdRunningInCluster=True and MemberRemove().
// This forces the TNF pacemaker startup deadlock deterministically.
// DO NOT MERGE.
klog.Warningf("TEST ONLY (OCPBUGS-105240): delaying bootstrap member removal by 5 minutes to reproduce race condition")
time.Sleep(5 * time.Minute)

Comment on lines +147 to +153

@coderabbitai coderabbitai Bot Aug 9, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift

Make the delay truly test-only.

The TEST ONLY comment does not gate this code. Normal bootstrap teardown now blocks for five minutes on every safe removal attempt. The supplied TestRemoveBootstrap test also adds at least ten minutes across its two complete-removal cases.

Inject the delay as a test hook or guard it with an explicit test-only mechanism. Keep the normal reconciliation path unchanged.

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@pkg/operator/bootstrapteardown/bootstrap_teardown_controller.go` around lines
146 - 152, Gate the five-minute sleep in the bootstrap member removal flow
behind an explicit test-only hook or mechanism, using the surrounding removal
logic and TestRemoveBootstrap integration point. Ensure production
reconciliation never delays, while the test can still enable the delay to
reproduce the race deterministically; leave normal removal behavior unchanged.

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

rg -n -C 6 'WithTimeout|removeBootstrap|time\.Sleep|MemberRemove' \
  pkg/operator/bootstrapteardown

rg -n -C 6 '\bMemberRemove\s*\(' --glob '*.go'

Repository: openshift/cluster-etcd-operator

Length of output: 17857

🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '%s\n' '--- controller imports and methods ---'
sed -n '1,185p' pkg/operator/bootstrapteardown/bootstrap_teardown_controller.go

printf '%s\n' '--- relevant tests ---'
sed -n '330,410p' pkg/operator/bootstrapteardown/bootstrap_teardown_controller_test.go

printf '%s\n' '--- timeout and sleep usage in the package ---'
rg -n -C 4 'context\.WithTimeout|time\.Sleep|MemberRemove|removeBootstrap' pkg/operator/bootstrapteardown

printf '%s\n' '--- current change summary ---'
git status --short
git diff --stat
git diff -- pkg/operator/bootstrapteardown/bootstrap_teardown_controller.go

Repository: openshift/cluster-etcd-operator

Length of output: 24248

🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '%s\n' '--- EtcdClient and MemberRemove implementations ---'
rg -n -C 8 'type EtcdClient|func .*MemberRemove|MemberRemove\s*\(' \
  pkg --glob '*.go'

printf '%s\n' '--- timing probe ---'
python3 - <<'PY'
from dataclasses import dataclass

`@dataclass`
class Context:
    deadline_seconds: int

ctx = Context(deadline_seconds=60)
sleep_seconds = 5 * 60
member_remove_start = sleep_seconds

print(f"context deadline: {ctx.deadline_seconds}s")
print(f"MemberRemove starts: {member_remove_start}s")
print(f"context expired before MemberRemove: {member_remove_start >= ctx.deadline_seconds}")
PY

Repository: openshift/cluster-etcd-operator

Length of output: 15345

Make the test-only delay respect its context deadline.

sync passes a one-minute timeoutCtx to removeBootstrap, but this path blocks for five minutes with time.Sleep. MemberRemove then receives an expired context and returns context deadline exceeded. Replace time.Sleep with a timer that observes ctx.Done(), and use a dedicated context whose deadline covers the delay and member removal. Otherwise, remove this DO NOT MERGE block before merging.

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@pkg/operator/bootstrapteardown/bootstrap_teardown_controller.go` around lines
146 - 152, Replace the five-minute time.Sleep in removeBootstrap with a timer
that exits when ctx.Done() fires, and use a dedicated context whose deadline
spans both the intentional delay and subsequent MemberRemove call.
Alternatively, remove the entire TEST ONLY/DO NOT MERGE delay block before
merging.

Source: Path instructions

// check to see if bootstrapping is complete
if isBootstrapComplete, err := bootstrap.IsBootstrapComplete(c.configmapLister); !isBootstrapComplete || err != nil {
return err
Expand Down