OCPBUGS-90534: openstack: Increate bootstrap timeout#10638
Conversation
openshift-ci-robot
added
jira/valid-reference
|
@stephenfin: This pull request references Jira Issue OCPBUGS-90534, which is valid. The bug has been moved to the POST state. 3 validation(s) were run on this bug
The bug has been updated to refer to the pull request using the external bug tracker. DetailsIn response to this:
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 openshift-eng/jira-lifecycle-plugin repository. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository YAML (base), Central YAML (inherited) Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughTwo files extend the bootstrap waiting logic to include the OpenStack platform. ChangesOpenStack Bootstrap Timeout Extension
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 11❌ Failed checks (1 warning, 10 inconclusive)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 golangci-lint (2.12.2)Error: can't load config: unsupported version of the configuration: "" See https://golangci-lint.run/docs/product/migration-guide for migration instructions Comment |
We frequently see build timeouts in CI. Many of these issues are due to slow infra, but if we are seeing these issues then it is likely customers see the same issues. Increase the timeout for OpenStack like we have already done for VMWare and baremetal. Signed-off-by: Stephen Finucane <stephenfin@redhat.com> Assisted-by: Claude Opus 4.6 <noreply@anthropic.com>
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
stephenfin
force-pushed
the
increase-bootstrap-timeout
branch
from
7dd6fd4 to
7243b77
Compare
|
@stephenfin: This pull request references Jira Issue OCPBUGS-90534, which is valid. 3 validation(s) were run on this bug
DetailsIn response to this:
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 openshift-eng/jira-lifecycle-plugin repository. |
While working on extending the OpenStack timeout, it was noted the bootstrap timeout in create.go was extended to 60 minutes for vSphere (alongside baremetal) but the cluster initialization timeout in waitfor.go was not updated to match. This meant vSphere got 60 minutes for bootstrap but only the default 40 minutes for cluster initialization, which is inconsistent. Fix it. Signed-off-by: Stephen Finucane <stephenfin@redhat.com> Assisted-By: Claude Opus 4.6 <noreply@anthropic.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with 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.
Inline comments:
In `@cmd/openshift-install/waitfor.go`:
- Around line 30-31: The field `ExtendTimeoutForBaremetal` is set to true for
multiple platforms (baremetal, openstack, and vsphere), not just baremetal,
making its name and documentation inaccurate. Rename the field to a more generic
name that reflects it controls timeout extension for multiple platforms
requiring extended initialization time, and update its documentation comment to
accurately describe its purpose across all platforms that use it. Ensure all
references to the field are updated, including in the
`waitForInitializedCluster()` function where it is consumed.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: 47cd89ac-e574-496d-a817-cf63484af1c4
📒 Files selected for processing (2)
cmd/openshift-install/create.gocmd/openshift-install/waitfor.go
| case baremetal.Name, openstack.Name: | ||
| options.ExtendTimeoutForBaremetal = true |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Read-only verification: inspect all uses of ExtendTimeoutForBaremetal.
rg -n -C3 '\bExtendTimeoutForBaremetal\b'Repository: openshift/installer
Length of output: 2053
Update field name and documentation; timeout-only usage is correct.
The field is correctly used as a generic boolean timeout extension across waitForInitializedCluster() (line 119 in cmd/openshift-install/command/waitfor.go). However, the field name ExtendTimeoutForBaremetal and its documentation comment ("extends the initialization timeout for baremetal platforms") are now inaccurate—the field is set to true for baremetal, openstack, and vsphere (line 31). Rename the field and update its documentation to reflect that it controls timeout extension for multiple platforms that require extended initialization time.
🤖 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 `@cmd/openshift-install/waitfor.go` around lines 30 - 31, The field
`ExtendTimeoutForBaremetal` is set to true for multiple platforms (baremetal,
openstack, and vsphere), not just baremetal, making its name and documentation
inaccurate. Rename the field to a more generic name that reflects it controls
timeout extension for multiple platforms requiring extended initialization time,
and update its documentation comment to accurately describe its purpose across
all platforms that use it. Ensure all references to the field are updated,
including in the `waitForInitializedCluster()` function where it is consumed.
|
@stephenfin: all tests passed! Full PR test history. Your PR dashboard. DetailsInstructions 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. |
2 participants
We frequently see build timeouts in CI. Many of these issues are due to slow infra, but if we are seeing these issues then it is likely customers see the same issues. Increase the timeout for OpenStack like we have already done for VMWare and baremetal.
Summary by CodeRabbit