Skip to content
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

vSphere: zonal with external lb #1277

Closed
wants to merge 3 commits into from

Conversation

jcpowermac
Copy link
Contributor

No description provided.

@openshift-ci openshift-ci bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Nov 3, 2022
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Nov 3, 2022

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Nov 3, 2022

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign frobware for approval by writing /assign @frobware in a comment. For more information see the Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@jcpowermac
Copy link
Contributor Author

@jcpowermac
Copy link
Contributor Author

Currently working with haproxy and zonal, with changes to the installer. Obviously this isn't complete just a test.
https://github.com/jcpowermac/ocp-install-external-lb-hack

Describes the requirements for a MVP implementation
of a external lb and the use of multiple subnets.
Copy link
Member

@cybertron cybertron left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know we have similar requests for other on-prem platforms (https://issues.redhat.com/browse/OPNET-14 ) so I'd be interested in a cross-platform solution to this. My thought was less removing all of the IPI components and making it behave like UPI, and more allow the DNS records to be pointed at an external loadbalancer.

That said I don't know all the details about your use cases. There are situations my idea would not support.


## Summary

The goal of this enhancement is to provide the ability to install vSphere IPI zonal on separate L2 segments. As of this writing the only way to
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this distinct from the remote workers functionality that we already have for baremetal? AFAIK that should already work for vsphere, we just haven't tested it.

https://issues.redhat.com/browse/OPNET-27

Copy link
Contributor Author

@jcpowermac jcpowermac Nov 3, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ingress was resolved for vsphere
https://issues.redhat.com/browse/SPLAT-409

The scope is install time use of a external lb without the static pods of IPI with support for api and ingress.

@jcpowermac jcpowermac marked this pull request as ready for review November 3, 2022 19:00
@openshift-ci openshift-ci bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Nov 3, 2022
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Nov 4, 2022

@jcpowermac: 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.


## Summary

The goal of this enhancement is to provide the ability to install vSphere IPI zonal on separate L2 segments without the static pods (keepalived, haproxy, coredns).

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why without coredns?


### MCO

The existing switch to deploy keepalived, haproxy and coredns static pods in the machine config operator is the apivip variable. This would change to be the Infrastructure status parameter: `LoadBalancerType`.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

for all on prem platforms or only for vsphere?
I would prefer to have it a generic enhancement to the on prem networking

)

// Platform stores any global configuration used for vsphere platforms
type Platform struct {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why is this vpshere specific?

@@ -0,0 +1,201 @@
---
title: vsphere-ipi-zonal-external-lb

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

general thought:
The fact that there is a inter-dependency between IPI and onprem networking is a big limitation that we've been imposing on our customers. Hence I think having this enhancement is a great step forward. I do believe that it should be solved in a holistic manner - for all on prem platforms, rather than for vpshere only.
In assisted - the way we are categorizing it is - CMN (cluster managed netoworking) and UMN (user managed networking) - where, currently, it boils down to ipi/upi manifests (which define the networking yamls). Once this enhancement is implemented, we'll have to adjust our logic there.

- "@jcpowermac"
reviewers:
- "@rvanderp3"
- "@bostrt"

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we need assisted installer team to chime in since it will affect vsphere integration of AI.
@oourfali @filanov @gamli75


### api

#### Infrastructure Status

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what about existence / non-existence of VIPs - why it cannot be the signal? then it will be aligned with upi/ipi networking parts

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

and, the installer can set the lodbalancer type as the derivative of those values, without letting the user specifying it

@jcpowermac
Copy link
Contributor Author

/hold

After a slack thread chat last week with @tsorya this enhancement will change significantly.
tl;dr we can just change the IPI validation to allow no VIP(s) like UPI.

@openshift-ci openshift-ci bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Nov 14, 2022
@jcpowermac
Copy link
Contributor Author

/close
This PR from the installer already changed the VIP validation required to make a external load balancer for vSphere IPI zonal possible: openshift/installer#5798

@openshift-ci openshift-ci bot closed this Nov 22, 2022
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Nov 22, 2022

@jcpowermac: Closed this PR.

In response to this:

/close
This PR from the installer already changed the VIP validation required to make a external load balancer for vSphere IPI zonal possible: openshift/installer#5798

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants