OCPBUGS-81309: Fix vSphere with static IPs on TPNU#10442
OCPBUGS-81309: Fix vSphere with static IPs on TPNU#10442openshift-merge-bot[bot] merged 1 commit intoopenshift:mainfrom
Conversation
|
@mdbooth: This pull request references Jira Issue OCPBUGS-81309, 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: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughRemoved feature-gate conditional that rewrote IPAM object APIVersion; code now always appends generated IPClaim and IPAddress objects unchanged. Updated OpenStack CI Dockerfile base image tags from OpenShift 4.17 to 4.22 and removed several Ansible/OpenStack yum packages from test deps. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
/jira refresh |
|
@mdbooth: This pull request references Jira Issue OCPBUGS-81309, which is valid. The bug has been moved to the POST state. 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. |
|
/payload-job periodic-ci-openshift-release-main-nightly-4.22-e2e-vsphere-static-ovn-techpreview |
|
@mdbooth: trigger 1 job(s) for the /payload-(with-prs|job|aggregate|job-with-prs|aggregate-with-prs) command
See details on https://pr-payload-tests.ci.openshift.org/runs/ci/9d7528f0-2c46-11f1-81eb-188b8f5fdaa1-0 |
|
The code being backed out here was added as a fix for https://redhat.atlassian.net/browse/OCPBUGS-69434--it seemed v1beta2 IPAM was expected. Are we not going to hit the same issue? Perhaps something changed and now v1beta is ok? |
We flipped the requirement again in openshift/cluster-api#263. The technical constraint here is that installer-generated manifests which are applied during bootstrap cannot require a conversion webhook. When we initially bumped CAPI to whichever version contains v1beta2 manifests, it also bumped the CRD storage version to v1beta2. This meant that applying v1beta1 required calling a conversion webhook. Consequently we added this change which ensured that we used v1beta2, which didn't require conversion. We later realised that the CAPI bump broke upgrades because it is not permitted to both introduce a new version and make it the storage version at the same time. Consequently we added openshift/cluster-api#263 which bumped the storage version temporarily back to v1beta1 but retained v1beta2. This second change broke vSphere again because now it's v1beta2 that requires conversion, not v1beta1. This is very fragile and I suspect we will break it several more times in the future before we have a proper handle on it, but for now v1beta1 should be the 'correct' version here. Again. |
|
/test yaml-lint |
|
Looks like prow was unhappy for a while there. /payload-job periodic-ci-openshift-release-main-nightly-4.22-e2e-vsphere-static-ovn-techpreview |
|
@mdbooth: trigger 1 job(s) for the /payload-(with-prs|job|aggregate|job-with-prs|aggregate-with-prs) command
See details on https://pr-payload-tests.ci.openshift.org/runs/ci/6d7def30-2c58-11f1-875e-82471d9c3a14-0 |
|
/test artifacts-images images okd-scos-images verify-deps verify-vendor images gover golint gofmt |
|
/test govet shellcheck |
|
/payload-job periodic-ci-openshift-release-main-nightly-4.22-e2e-vsphere-static-ovn-techpreview |
|
@mdbooth: trigger 1 job(s) for the /payload-(with-prs|job|aggregate|job-with-prs|aggregate-with-prs) command
See details on https://pr-payload-tests.ci.openshift.org/runs/ci/f1cd39b0-2c5f-11f1-8dfe-851cd79328a7-0 |
|
@mdbooth: This pull request references Jira Issue OCPBUGS-81309, 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. |
|
/payload-job periodic-ci-openshift-release-main-nightly-4.22-e2e-vsphere-static-ovn-techpreview |
|
@mdbooth: trigger 1 job(s) for the /payload-(with-prs|job|aggregate|job-with-prs|aggregate-with-prs) command
See details on https://pr-payload-tests.ci.openshift.org/runs/ci/fc33b710-2c66-11f1-8c1c-0d3632815be7-0 |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@images/openstack/Dockerfile.ci`:
- Line 26: The Dockerfile.ci copies the OpenStack UPI playbooks (COPY
--from=builder ... upi/openstack) which include network.yaml,
security-groups.yaml, control-plane.yaml, compute-nodes.yaml, bootstrap.yaml and
cleanup scripts that import common.yaml and require Ansible + openstacksdk;
either ensure no CI path will ever execute those playbooks or install the
required runtime before copying/using them: locate the COPY line that copies
/upi/openstack in Dockerfile.ci and either (A) remove or move those playbooks
out of the CI image so they are not present for any CI job that might invoke
them, or (B) add installation of Ansible (ansible-core or ansible package) and
openstacksdk (python3-openstacksdk or pip install openstacksdk) and any other
Ansible plugin deps (e.g., ansible-collections or python3-ansible-*) to the
Dockerfile.ci so any accidental execution will succeed.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: a28ec0a9-65ed-4cae-826b-dfaf1f0a3f55
📒 Files selected for processing (1)
images/openstack/Dockerfile.ci
|
/payload-job periodic-ci-openshift-release-main-nightly-4.22-e2e-vsphere-static-ovn-techpreview |
|
@mdbooth: trigger 1 job(s) for the /payload-(with-prs|job|aggregate|job-with-prs|aggregate-with-prs) command
See details on https://pr-payload-tests.ci.openshift.org/runs/ci/054c9510-2c8a-11f1-9a85-602c80bd0161-0 |
|
The payload job passed: https://prow.ci.openshift.org/view/gs/test-platform-results/logs/openshift-installer-10442-nightly-4.22-e2e-vsphere-static-ovn-techpreview/2038749310417375232 I'm going to take out the OpenStack image changes as they were only required to let me run the payload job and they're not related to this. |
|
/pipeline required |
|
All failures should be resolved by openshift/release#77140 /retest-required |
|
For completeness I'll run the payload again. It should pass now that the OpenStack image is fixed. /payload-job periodic-ci-openshift-release-main-nightly-4.22-e2e-vsphere-static-ovn-techpreview |
|
@mdbooth: trigger 1 job(s) for the /payload-(with-prs|job|aggregate|job-with-prs|aggregate-with-prs) command
See details on https://pr-payload-tests.ci.openshift.org/runs/ci/17a823c0-2d1e-11f1-9428-6fb3738e17af-0 |
|
/test e2e-vsphere-static-ovn |
|
/retest-required |
damdo
left a comment
There was a problem hiding this comment.
/lgtm
/assign @vr4manta @jcpowermac @patrickdillon
|
The payload job above ran successfully, which verifies this is fixed. /verified by CI |
|
@mdbooth: This PR has been marked as verified by 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. |
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: patrickdillon The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
/retest-required |
|
@mdbooth: The following tests 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. |
30edf44
into
openshift:main
|
@mdbooth: Jira Issue Verification Checks: Jira Issue OCPBUGS-81309 Jira Issue OCPBUGS-81309 has been moved to the MODIFIED state and will move to the VERIFIED state when the change is available in an accepted nightly payload. 🕓 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. |
The bootstrap KAS cannot currently reach conversion webhooks, so installer-generated manifests must use the storage version of any CRDs they rely on. If they use a non-storage version then KAS will try to reach a conversion webhook, which will fail, and the object will not be admitted.
In this case the IPAM CRD storage versions were reverted to v1beta1 from v1beta2, so the installer manifests must be updated to follow.