Repository navigation
vsphere: fail CAPI machine generation on DNS or SOAP auth errors - #10889
nikhilprajapati-world wants to merge 1 commit into
Conversation
Returning nil on LookupHost or SOAP faults made IPI create manifests succeed with zero Cluster API machines. Fail instead, and give Networks its own 60s timeout instead of nesting it under the 30s DNS context. Fixes OCPBUGS-126723 Co-authored-by: Cursor <cursoragent@cursor.com>
|
Pipeline controller notification For optional jobs, comment This repository is configured in: LGTM mode |
|
@nikhilprajapati-world: This pull request references Jira Issue OCPBUGS-126723, which is invalid:
Comment 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: Advanced Run ID: 📒 Files selected for processing (2)
Included review availability: Your plan provides up to 4 included reviews per hour; 3 remain after this review. 📝 WalkthroughWalkthroughThe vSphere control-plane preflight logic now uses a dedicated helper with separate DNS and network timeouts. DNS resolution and SOAP authentication failures propagate from machine generation. New tests cover errors, success, and timeout independence. ChangesvSphere preflight handling
Priority: ⬇️ Low Estimated code review effort: 3 (Moderate) | ~20 minutes Change: Bug fix Merge Risk: ⚪ Minimal · up to The vSphere preflight now stops generation on DNS and authentication failures while retaining an independent Networks timeout. No actionable risk remains. Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 error, 1 warning)
✅ Passed checks (13 passed)
Full details: No-Sensitive-Data-In-LogsExplanation The change introduces internal vCenter hostnames into user-visible error logs. Resolution Keep the required failure behavior, but do not include the configured vCenter server in returned errors that reach command logging. Use generic messages such as
✨ Finishing Touches🧪 Generate unit tests (beta)
Warning Some tools did not complete. Review the errors below. 🔧 golangci-lint (2.13.2)Error: can't load config: unsupported version of the configuration: "" See https://golangci-lint.run/docs/product/migration-guide for migration instructions Comment |
|
[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 |
|
@nikhilprajapati-world: No Jira issue is referenced in the title of this pull request. 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. |
|
@nikhilprajapati-world: The following test failed, say
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. |
Summary
create manifestsno longer treats a vCenter DNS or SOAP auth failure as success.ClusterAPI.Generate()returnednilafter a warning, so classic IPI continued with zero CAPI control-plane machines.Networks()gets its own 60s timeout from the parent context instead of nesting under the 30s DNS lookup context.Fixes: https://redhat.atlassian.net/browse/OCPBUGS-126723
Test plan
go test -mod=vendor ./pkg/asset/machines/ -run TestVSphereCAPIPreflightinstall-config.yamlwithplatform.vsphere.vcenters[0].server: vcenter.bogus.invalid—openshift-install create manifestsfails withunable to resolve vSphere server(previously warned and exited 0 with nocluster-api/machines/)Networks()fails withauthentication failure to vCenterinstead of succeeding with zero CAPI machinescluster-api/machines/Made with Cursor
Summary by CodeRabbit