Skip to content

ARO-26557: Fix etcd bootstrap race when replicas join after quorum #8425

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

Open
patriksuba wants to merge 1 commit into
base: main
Choose a base branch
from
+282 −13
Open

ARO-26557: Fix etcd bootstrap race when replicas join after quorum #8425

Show file tree
Hide file tree
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
23 changes: 21 additions & 2 deletions ...lplane/testdata/etcd/AROSwift/zz_fixture_TestControlPlaneComponents_etcd_statefulset.yaml
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

23 changes: 21 additions & 2 deletions ...ontrolplane/testdata/etcd/GCP/zz_fixture_TestControlPlaneComponents_etcd_statefulset.yaml
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

23 changes: 21 additions & 2 deletions ...lplane/testdata/etcd/IBMCloud/zz_fixture_TestControlPlaneComponents_etcd_statefulset.yaml
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

23 changes: 21 additions & 2 deletions ...ata/etcd/TechPreviewNoUpgrade/zz_fixture_TestControlPlaneComponents_etcd_statefulset.yaml
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

23 changes: 21 additions & 2 deletions ...tedcontrolplane/testdata/etcd/zz_fixture_TestControlPlaneComponents_etcd_statefulset.yaml
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

19 changes: 17 additions & 2 deletions control-plane-operator/controllers/hostedcontrolplane/v2/assets/etcd/statefulset.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -227,15 +227,30 @@ spec:
echo "Checking if cluster is functional"
if etcdctl member list; then
echo "Cluster is functional"
MEMBER_ID=$(etcdctl member list -w simple | grep "${HOSTNAME}" | awk -F, '{ print $1 }')
MEMBER_LIST=$(etcdctl member list -w simple)
MEMBER_ID=$(echo "${MEMBER_LIST}" | grep "${HOSTNAME}" | awk -F, '{ print $1 }')
Comment on lines +230 to +231
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot May 21, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

grep "${HOSTNAME}" is a substring/regex match across the member list.

grep "${HOSTNAME}" matches any line containing HOSTNAME, and treats it as a regex. For etcd member names sharing prefixes (e.g. etcd-1 would also match a line for etcd-10/etcd-11), this can pick up the wrong row and yield a bogus MEMBER_ID (or a multi-line value that breaks the subsequent etcdctl member remove). Today HyperShift caps etcd at a small replica count so the collision is unlikely in practice, but since this line is being modified it’s cheap to harden.

Two safer options:

  • Anchor the match on the member-list column (with -w simple, the member name is field 3): awk -F, -v name="${HOSTNAME}" '$3 == name { print $1 }'
  • Or use grep -F -w -- "${HOSTNAME}" to disable regex and require a whole-word match (note: -w still treats - as a word boundary, so prefer the awk form for full safety).
♻️ Proposed change 
-              MEMBER_LIST=$(etcdctl member list -w simple)
-              MEMBER_ID=$(echo "${MEMBER_LIST}" | grep "${HOSTNAME}" | awk -F, '{ print $1 }')
+              MEMBER_LIST=$(etcdctl member list -w simple)
+              MEMBER_ID=$(echo "${MEMBER_LIST}" | awk -F, -v name="${HOSTNAME}" '$3 == name { print $1 }')
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
MEMBER_LIST=$(etcdctl member list -w simple)
MEMBER_ID=$(echo "${MEMBER_LIST}" | grep "${HOSTNAME}" | awk -F, '{ print $1 }')
MEMBER_LIST=$(etcdctl member list -w simple)
MEMBER_ID=$(echo "${MEMBER_LIST}" | awk -F, -v name="${HOSTNAME}" '$3 == name { print $1 }')
🤖 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
`@control-plane-operator/controllers/hostedcontrolplane/v2/assets/etcd/statefulset.yaml`
around lines 230 - 231, The current MEMBER_ID extraction uses grep "${HOSTNAME}"
which can match substrings/regex and return wrong rows; replace that grep-based
extraction by selecting the exact member-name column from MEMBER_LIST using awk
to compare field 3 to HOSTNAME (i.e., use MEMBER_LIST assignment and then
compute MEMBER_ID with awk -F, -v name="${HOSTNAME}" '$3 == name { print $1 }');
update the lines referencing MEMBER_LIST and MEMBER_ID (the two lines that set
MEMBER_LIST and MEMBER_ID) so MEMBER_ID is derived with the exact-match awk
approach to avoid partial/regex matches.

if [[ -n "${MEMBER_ID}" ]]; then
echo "A member with this name (${HOSTNAME}) already exists, removing"
etcdctl member remove "${MEMBER_ID}"
echo "Adding new member"
etcdctl member add ${HOSTNAME} --peer-urls https://${HOSTNAME}.etcd-discovery.${NAMESPACE}.svc:2380
echo "existing" > /etc/etcd/clusterstate/state
else
echo "A member does not exist with name (${HOSTNAME}), nothing to do"
echo "A member does not exist with name (${HOSTNAME}), evaluating straggler join"
if [[ -n "${ETCD_EXPECTED_MEMBER_COUNT:-}" && -n "${ETCD_QUORUM_MIN_MEMBERS:-}" ]]; then
MEMBER_COUNT=$(echo "${MEMBER_LIST}" | grep -c . || true)
if [[ "${MEMBER_COUNT}" -ge "${ETCD_QUORUM_MIN_MEMBERS}" && "${MEMBER_COUNT}" -lt "${ETCD_EXPECTED_MEMBER_COUNT}" ]]; then
echo "Cluster reports at least quorum members (${MEMBER_COUNT} >= ${ETCD_QUORUM_MIN_MEMBERS} for size ${ETCD_EXPECTED_MEMBER_COUNT}); adding this member dynamically"
etcdctl member add ${HOSTNAME} --peer-urls https://${HOSTNAME}.etcd-discovery.${NAMESPACE}.svc:2380
echo "existing" > /etc/etcd/clusterstate/state
elif [[ "${MEMBER_COUNT}" -ge "${ETCD_EXPECTED_MEMBER_COUNT}" ]]; then
echo "Cluster already has the expected member count (${MEMBER_COUNT}/${ETCD_EXPECTED_MEMBER_COUNT}); refusing to grow membership dynamically"
else
echo "Cluster membership (${MEMBER_COUNT}) is below quorum (${ETCD_QUORUM_MIN_MEMBERS}); static bootstrap (new) path"
Comment thread
patriksuba marked this conversation as resolved.
fi
else
echo "Skipping straggler join: ETCD_EXPECTED_MEMBER_COUNT and ETCD_QUORUM_MIN_MEMBERS must both be set; not adding member dynamically"
fi
fi
else
echo "Cannot list members in cluster, so likely not up yet"
Expand Down
21 changes: 21 additions & 0 deletions control-plane-operator/controllers/hostedcontrolplane/v2/etcd/membership.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
package etcd

// etcdRaftQuorumSize returns the minimum number of voting members that must be
// present for a Raft quorum for a cluster of clusterSize members.
func etcdRaftQuorumSize(clusterSize int) int {
if clusterSize <= 0 {
return 0
}
return clusterSize/2 + 1
}

// resetMemberStragglerJoinQuorumMet returns true when the number of members
// reported by etcdctl member list is already sufficient for the cluster to have
// raft quorum for an expected cluster of expectedClusterSize, and the cluster
// is not already at the expected size (so dynamic member add is appropriate).
func resetMemberStragglerJoinQuorumMet(memberCount, expectedClusterSize int) bool {
if expectedClusterSize <= 0 {
return false
}
return memberCount >= etcdRaftQuorumSize(expectedClusterSize) && memberCount < expectedClusterSize
}
Loading
Loading