-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
OSASINFRA-3090: External LB support for on-prem platforms #6812
OSASINFRA-3090: External LB support for on-prem platforms #6812
Conversation
@EmilienM: This pull request references OSASINFRA-3090 which is a valid jira issue. In 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/test-infra repository. |
4f857ca
to
526eb2e
Compare
/uncc AnnaZivkovic bostrt This is highly VIP now but I'll use it as a baseline and apply changes from the enhancement in this PR. |
17fab84
to
ccdb5df
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.
Seems like a good start. We'll need to see what type of validation we want to add.
I suggest we iterate on the patch with the OpenStack code first until we're happy with the result then port the same logic to other platforms.
@EmilienM if loadBalancerType is external we can check for the existance of the LB and warn if not found. I thought this was tech preview? If so it should be added here: A little concerned we are changing how vSphere UPI is installed, @patrickdillon @rvanderp3 wdyt? |
@EmilienM: This pull request references OSASINFRA-3090 which is a valid jira issue. In 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/test-infra repository. |
This was a mistake, I removed the revert.
Ok, I didn't know that interface. I'll add it here of course.
I agree it'll require your users to add |
Right...but as currently implemented you are requiring this change in 4.13 even though this is tech preview. |
d6bdd8a
to
10d133d
Compare
I understand the concern and I don't know how to handle it, I'll defer to installer folks to make a decision. I'm ok with whatever option. |
I think it makes sense to keep this behind the tech preview feature gate for 4.13. @jcpowermac and I were under the impression this wasn't targeting 4.13 for GA. |
1d76c77
to
7390880
Compare
/retest-required |
/retest-required |
1 similar comment
/retest-required |
we have to wait for openshift/origin#27736 |
/retest-required |
1f3aae2
to
d93a921
Compare
/hold for CI |
/lgtm |
/hold cancel |
d93a921
to
625932f
Compare
/lgtm |
/test e2e-vsphere-upi-zones |
/retest-required |
@EmilienM: The following tests failed, say
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. |
Support for External LB for BM, vsphere, OpenStack, ovirt and Nutanix platforms