Skip to content

Reduce race condition in extra worker BMH application - #1947

Open
elfosardo wants to merge 1 commit into
openshift-metal3:masterfrom
elfosardo:relax-waiting-time-extraworkers
Open

Reduce race condition in extra worker BMH application#1947
elfosardo wants to merge 1 commit into
openshift-metal3:masterfrom
elfosardo:relax-waiting-time-extraworkers

Conversation

@elfosardo

Copy link
Copy Markdown
Member

Wait for all worker Machines to have a BareMetalHost assigned before applying extra worker manifests. This gives slow-registering workers (e.g. Redfish, ~17 min) time to claim their BMH slot, reducing the chance of machine-api grabbing the extra worker for the primary MachineSet.

The wait is non-fatal (default 120 retries * 10s = 20 min): if not all workers register within the timeout (e.g. permanently-unreachable worker BMH in some dualstack environments), the extra workers are applied anyway to preserve existing fallback behavior. The retry count is configurable via EXTRA_WORKER_WAIT_RETRIES.

Assisted-By: Claude Opus 4.6

Wait for all worker Machines to have a BareMetalHost assigned before
applying extra worker manifests.  This gives slow-registering workers
(e.g. Redfish, ~17 min) time to claim their BMH slot, reducing the
chance of machine-api grabbing the extra worker for the primary
MachineSet.

The wait is non-fatal (default 120 retries * 10s = 20 min): if not all
workers register within the timeout (e.g. permanently-unreachable worker
BMH in some dualstack environments), the extra workers are applied
anyway to preserve existing fallback behavior.  The retry count is
configurable via EXTRA_WORKER_WAIT_RETRIES.

Assisted-By: Claude Opus 4.6
@tdomnesc

Copy link
Copy Markdown
Contributor

/lgtm

@openshift-ci openshift-ci Bot added the lgtm Indicates that a PR is ready to be merged. label Aug 14, 2026
@elfosardo

Copy link
Copy Markdown
Member Author

/retest

ah great, the same unrelated test failed here and in another job in a different patch

@elfosardo

Copy link
Copy Markdown
Member Author

/retest

@elfosardo

Copy link
Copy Markdown
Member Author

/retest

more unrelated failures

@elfosardo

Copy link
Copy Markdown
Member Author

/retest

2 similar comments
@elfosardo

Copy link
Copy Markdown
Member Author

/retest

@tdomnesc

Copy link
Copy Markdown
Contributor

/retest

Comment thread 06_create_cluster.sh
break
fi
echo "Waiting: ${ASSIGNED}/${NUM_WORKERS} worker machines have a host assigned"
sleep 10

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This is 20 minutes of waiting by default, are you sure the environments with unreachable workers won't timeout anyway?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

the timeout is a non fatal wait, and 20 minutes should be ok in the overall time budget
we can evaluate after some runs if we want to reduce it to 15 or even 10 minute, but I would start with this

@tdomnesc

Copy link
Copy Markdown
Contributor

/retest

@tdomnesc

Copy link
Copy Markdown
Contributor

/retest-required

@elfosardo

Copy link
Copy Markdown
Member Author

/retest

@tdomnesc

Copy link
Copy Markdown
Contributor

/retest-required

@elfosardo

Copy link
Copy Markdown
Member Author

/retest

@openshift-ci

openshift-ci Bot commented Aug 19, 2026

Copy link
Copy Markdown

@elfosardo: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
ci/prow/e2e-metal-ipi-ovn-dualstack dbf21e9 link false /test e2e-metal-ipi-ovn-dualstack

Full PR test history. Your PR dashboard.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.

@elfosardo

Copy link
Copy Markdown
Member Author

/retest

@elfosardo
elfosardo requested a review from dtantsur August 20, 2026 08:16
@dtantsur

Copy link
Copy Markdown
Member

/approve

@openshift-ci

openshift-ci Bot commented Aug 20, 2026

Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: dtantsur

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-ci openshift-ci Bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Aug 20, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants