-
Notifications
You must be signed in to change notification settings - Fork 475
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
OCPBU-199: enhancement proposal for dual-stack support with openstack #1365
OCPBU-199: enhancement proposal for dual-stack support with openstack #1365
Conversation
Skipping CI for Draft Pull Request. |
/cc @mandre |
abd824c
to
9af1bb2
Compare
2063e64
to
f330c63
Compare
f330c63
to
5be5504
Compare
e55d4ab
to
618b613
Compare
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.
This is great! I have a few questions and suggestions inline.
/cc @patrickdillon would you mind to take a look on this enhancement? Thanks |
/lgtm |
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
/remove-lifecycle stale |
@dhellmann This enhancement looks ready. Can you have a look? Thanks in advance |
@mandre is listed as the approver for this one. If all of the reviewers from all of the affected teams have accepted the proposal, Martin can take care of the approval step. |
/lgtm |
|
||
### Risks and Mitigations | ||
|
||
The support for dual-stack in Kubelet's node-ip setting is a [feature in alpha state in Kubernetes v1.27](https://kubernetes.io/docs/concepts/services-networking/dual-stack/#configure-ipv4-ipv6-dual-stack) and requires a [fix to merge](https://github.com/kubernetes/kubernetes/pull/118662). |
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.
Are these two risks specific to openshift on openstack? or do they apply to other dualstack-supported platforms?
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.
The fix in subject is already merged and even backported to 1.27 so I wouldn't call it a risk anymore. The feature is already going to beta (kubernetes/enhancements#3975) so the risk that it's going to be dropped before GA is significantly lower.
Having said this, those both points were applying to any dual-stack platform, not only OpenStack.
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.
Does dualstack support with openstack rely on the CloudDualStackNodeIPs
feature?
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.
Yes
@sadasu hello, following our discussions about the installer part of this enhancement, is there anything else to be discussed or it's ready from your perspective? |
I looked at this enhancement from the Installer perspective. LGTM from me expect for the one minor comment. @patrickdillon has been listed as the reviewer from the Installer team. So, I'll give me him a chance to take a look too. |
e35d741
to
4f1f78f
Compare
/lgtm |
Thanks! I think @sadasu has covered this for the installer perspective and
we can update the reviewer accordingly.
LMK if there is anything specific i can help with
On Mon, Jul 31, 2023 at 4:04 PM Sandhya Dasu ***@***.***> wrote:
I looked at this enhancement from the Installer perspective. LGTM from me
expect for the one minor comment. @patrickdillon
<https://github.com/patrickdillon> has been listed as the reviewer from
the Installer team. So, I'll give me him a chance to take a look too.
—
Reply to this email directly, view it on GitHub
<#1365 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ABAVQYNKDZ3U4CQB6B7WPHTXTAFQ5ANCNFSM6AAAAAAVZCO45M>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
--
Patrick Dillon
Team Lead, Engineer, OpenShift Installer
|
@MaysaMacedo: all tests passed! Full PR test history. Your PR dashboard. 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/test-infra repository. I understand the commands that are listed here. |
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.
Great, thanks all for the reviews.
/lgtm
/approve
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: mandre The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
No description provided.