-
Notifications
You must be signed in to change notification settings - Fork 265
OCPBUGS-66417: Add kubevirt platform to dual-stack supported platforms #2849
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.
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
OCPBUGS-66417: Add kubevirt platform to dual-stack supported platforms #2849
Conversation
The kubevirt platform in Hosted Control Planes (HCP) supports a dual-stack topology. Adding it to the dualStackPlatforms list to avoid reconciler error when the hosted cluster is dual-stack. Signed-off-by: Oren Cohen <ocohen@redhat.com>
WalkthroughThe change adds Kubevirt platform type to the dual-stack platforms set in network rendering logic, expanding platform coverage for dual-stack validation support. Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~2 minutes ✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro Cache: Disabled due to data retention organization setting Knowledge base: Disabled due to 📒 Files selected for processing (1)
🧰 Additional context used📓 Path-based instructions (1)**⚙️ CodeRabbit configuration file
Files:
🔇 Additional comments (1)
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.5.0)Error: can't load config: unsupported version of the configuration: "" See https://golangci-lint.run/docs/product/migration-guide for migration instructions Comment |
qinqon
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/lgtm
|
/test e2e-aws-ovn-windows PR merged |
|
@orenc1: This pull request references Jira Issue OCPBUGS-66253, 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. |
|
/retest |
|
@orenc1: This pull request references Jira Issue OCPBUGS-66417, which is valid. The bug has been moved to the POST state. 3 validation(s) were run on this bug
No GitHub users were found matching the public email listed for the QA contact in Jira (yli2@redhat.com), skipping review request. 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. |
|
/retest-required |
|
Isn't dualstack support for kubevirt supposed to be a feature? Why is this a bug fix? Where are the links and history for this feature on JIRA? Also what about CI? How do we know doing this is going to work? Which team's QE is testing this? -> is it hypershift qe? kubevirt qe? supposed to be core-net-qe? |
From what I see on the enhancement it was part of the Goals
Well since it was part of the goals it was implemented, so this is a bugfix, what is weird is that this means that
Since the enhancement goals include IPv6 then it's part of the task. Also what about CI? How do we know doing this is going to work? Which team's QE is testing this? This should be tested by hypershift qe team, but clearly it was never tested and we don't have CI. Did we always support dualstack or its new? Does our product docs cover this? SO from the enhancement we were supposed to suport it from the beginning, for sure we implemented it at ovn-kubernetes level and at MCO level.
The hypershift qe team is suppose to test this. |
Thanks @qinqon for the details, then I need a pre-merge testing label "verified" from hypershift-qe OR a CI here in this repo I guess? I think former ack from QE is better.. Do we document this in hypershift-docs as supported? User facing docs I mean? |
|
The dualstack supported platform check is not new. |
|
We have verified that a dual-stack Hosted Cluster created with kubevirt plaform is working as expected with both stacks. Hosted Cluster yamlNodePool yamlCluster OperatorsClusterVersion: |
|
@orenc1: 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. |
|
/test e2e-metal-ipi-ovn-dualstack-bgp |
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: kyrtapz, orenc1, qinqon 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 |
|
/cherry-pick release-4.21 |
|
@orenc1: once the present PR merges, I will cherry-pick it on top of 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 kubernetes-sigs/prow repository. |
|
/override ci/prow/e2e-metal-ipi-ovn-ipv6-ipsec |
|
@kyrtapz: Overrode contexts on behalf of kyrtapz: ci/prow/e2e-metal-ipi-ovn-ipv6-ipsec 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 kubernetes-sigs/prow repository. |
|
unrelated to this change: |
|
/test e2e-aws-ovn-hypershift-conformance |
|
@kyrtapz: Overrode contexts on behalf of kyrtapz: ci/prow/e2e-aws-ovn-windows, ci/prow/e2e-metal-ipi-ovn-dualstack-bgp 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 kubernetes-sigs/prow repository. |
|
/test e2e-aws-ovn-hypershift-conformance |
|
@orenc1: 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. |
|
Hypershift job passed before on this pr. |
|
@kyrtapz: Overrode contexts on behalf of kyrtapz: ci/prow/e2e-aws-ovn-hypershift-conformance 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 kubernetes-sigs/prow repository. |
9d8ab48
into
openshift:master
|
@orenc1: Jira Issue Verification Checks: Jira Issue OCPBUGS-66417 Jira Issue OCPBUGS-66417 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. |
|
@orenc1: new pull request created: #2861 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 kubernetes-sigs/prow repository. |
The kubevirt platform in Hosted Control Planes (HCP) supports a dual-stack topology.
Adding it to the
dualStackPlatformslist to avoid reconciler error when the hosted cluster is dual-stack.