-
Notifications
You must be signed in to change notification settings - Fork 531
OpenStack Bring Your Own External Connectivity #342
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
OpenStack Bring Your Own External Connectivity #342
Conversation
|
@iamemilio: GitHub didn't allow me to request PR reviews from the following users: jichenjc. Note that only openshift members and repo collaborators can review this PR, and authors cannot review their own PRs. 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/test-infra repository. |
adduarte
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.
Just comments in general.
russellb
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.
Please take a look at the enhancement template and ensure you leave all the sections that weren't marked optional. It seems several were removed.
russellb
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.
this lgtm, though I think someone from the installer team should sign off on this to ensure there's no installer integration concerns. The use case absolutely makes sense and the plan seems sane to me.
|
/assign @abhinavdahiya |
|
Seems fine to me. But I don't understand the specific details to provide detailed feedback. |
|
|
||
| ##### Example Usage | ||
|
|
||
| Using the proposed feature, the user would not set `externalNetwork` or `lbFloatingIP`. Then, they will set`machinesSubnet` to the ID of an existing subnet on a provider network that they want to install the cluster onto, and set the `networking.machineNetwork` to the CIDR of that subnet. They will also set custom ports IPs for the `apiVP` and `ingressVIP` to ensure the IP they take is available. Here is an example install config: |
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.
couldn't we dynamically query the cidr using the subnet id? the equivalent of openstack command :
"openstack subnet show -c cidr -f value", then validate the choice of the apivp and ingress vip
This would assure the correct cidr and avoid user error
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, this should be one of the new validation mentioned in the proposal.
We need query the subnet CIDR to check that it matches the user-provided networking.machineNetwork.
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.
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 real improvement we should consider is auto-fill, but I think that should be a part of the validation epic
fdfeea9 to
678bfda
Compare
|
Coped comments from openshift/installer#2670 (comment) Anyone can help to answer, in case that we can NOT assume machines in provider network have network connection to openstack management network:
@bjhuangr FYI |
|
Thanks @jichenjc, that is a good point to bring up. I don't think that we can support a cluster in IPI where the nodes are unable to reach OpenStack's APIs and services. Aside from the bootstrap phase, worker creation would also not be possible in this scenario. It would essentially disable the machine-controllers/cluster-api. This should be listed as a constraint in the design doc and the usage docs because administrators looking to create their own networks should be aware of this. |
| ## Summary | ||
|
|
||
| Numerous customers have requested the ability to run the OpenShift on OpenStack IPI installer without using Floating IP addresses. | ||
| In the installer, we use floating IPs as a way to set up exeternal connectivity to the OpenShift cluster. Depending on how the OpenStack cluster |
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.
| In the installer, we use floating IPs as a way to set up exeternal connectivity to the OpenShift cluster. Depending on how the OpenStack cluster | |
| In the installer, we use floating IPs as a way to set up external connectivity to the OpenShift cluster. Depending on how the OpenStack cluster |
|
|
||
| Numerous customers have requested the ability to run the OpenShift on OpenStack IPI installer without using Floating IP addresses. | ||
| In the installer, we use floating IPs as a way to set up exeternal connectivity to the OpenShift cluster. Depending on how the OpenStack cluster | ||
| is set up, the use of floating IPs may be impossible or very inconvenient. In order to support these customers, we intend to support installations |
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.
In order to support these customers, we intend to support installations on these types of OpenStack clusters.
to me this line is really confusing esp these are exactly are these types? Is it where floating IPs may be impossible or very inconvenient or where there are is X alternative available for load balancing ?
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.
I think you can just drop the "In order to support these customers, "
| ### Goals | ||
|
|
||
| - Extend the installer to support provider networks | ||
| - Allow the installer to be run without setting up external connectivity |
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.
Allow the installer to be run without setting up external connectivity
does this mean that endpoints like API, Ingress are only accessible to the internal network and not Internet or is it something else. because if it is the former we should keep in mind how this interacts with
/openshift-install explain installconfig.publish
KIND: InstallConfig
VERSION: v1
RESOURCE: <string>
Publish controls how the user facing endpoints of the cluster like the Kubernetes API, OpenShift routes etc. are exposed. When no strategy is specified, the strategy is "External".
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.
regardless of installation setup, the real requirement is ip access from the host you are adding to the api endpoints. I can think of a half a dozen ways users can accomplished and setup the rquired routing, from the very complicated to the simplest. We usually set the routing up for the user in IP installations while UPI expects the user to take some action on the mater.
technically fip addresses are not required for instllation. They are only required to provide access to the cluster. the reason the user needs to provide it during a IPI installation is because the installer goes the "extra mile" and makes the association.
We are removing the "requirement" that fips be available, I don't think we are preventing the user from setting up any routing they want.
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.
I do think, after reading more on the customer requirements, and other, this should probably be changed to:
"Allow the installer to be run without the use of FIPs" The phrase "Allow the installer to be run without setting up external connectivity" could be read to mean that the installer will succeed without any external connectivity. But looking at the title the intent is that the user bring their own external connectivity, which funny enough could include using FIPs.
@iamemilio I think the phrase means the installer will NOT setup the external connectivity, but there should be external connecitvity, right? ( you state that below).
|
|
||
| #### Make the API and ingress VIPs dynamically assigned by default, and only assign fixed IPs if they are specified in the install config | ||
|
|
||
| This should be handled by adding OpenStack specific code to the baremetal-runtime-config to enable it to look up ports in a cluster. This would break the circular dependency chain the installer currently has, and allow us to allocate the VIP ports dynamically, rather than assigning fixed IPs. |
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.
If the ingress and API VIPs are assigned dynamically, we cannot we those IPs in any assets created by the installer like the ignition configs, or tls certs etc. How we do we solve that?
Also this talks about baremetal-runtime-config to enable it to look up ports in a cluster, I think it makes sense to provide a little more guidance for that component on how it can perform that action and what constraints it needs to follow when doing that. Like can it use ports from the machineNetworks
openshift-install explain installconfig.networking.machineNetwork
KIND: InstallConfig
VERSION: v1
RESOURCE: <[]object>
MachineNetwork is the list of IP address pools for machines. This field replaces MachineCIDR, and if set MachineCIDR must be empty or match the first entry in the list. Default is 10.0.0.0/16 for all platforms other than libvirt. For libvirt, the default is 192.168.126.0/24.
FIELDS:
cidr <Any> -required-
CIDR is the IP block address pool for machines within the cluster.
or outside it etc.
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.
if the ips are assigned dynamically they will probably be assigned by dhcp. Which means we won't need them in the ignition config. However the dhcp and dns systems do need to be synced up. Will this be up to user?.
If the dns system is setup correctly, then the tls certs and any of the systems that address a host by network address, should use hostnames not ip addresses.
We do expect the networkcidr to be assigned explicitely. We will have to make sure dns registers the dynamically picked ip addresses.
It seems to me that perhaps we might want an operator running where hosts that are being added can submit their information (ip address, hostname, etc..) which they will get when booted.
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.
Neutron DHCP and DNS are documented requirements for the OpenStack IPI installer. This guarantees that when ports are created in a subnet, they are assigned a port in that subnet's allocation pool dynamically using dhcp and then added to neutron dns
Should we add a "is not" section to this document. Seems we need to spell out scenarios we know we won't support at this time. (although I don't think you can ever support this case, api access is a basic requirement for any type of installation of openshift). |
+1 to me , either some 'NOTE' or 'limitation' will be good enough , just provide those info to manage expectation of admin or the openshift cluster creator |
|
I can add this as a |
…usotmize the external connectivity of their cluster install.
678bfda to
a31711d
Compare
| ### Non-Goals | ||
|
|
||
| This enhancement does not intend to provide any guidance or features that go further than allowing customers to deploy without Floating IPs. Operations like attaching loadbalancers to the entrypoints, or any day 2 operations are beyond the scope of this enhancement. It is important to note that while we aim to support installs without external connectivity, we are not supporting installs on networks that are unable to reach the host cluster's OpenStack APIs and services. | ||
|
|
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.
If external connectivity then it will up to the user to setup the appropriate private registries to provide the necessary images to deploy. We can probably assume the user will have to setup following instructions similar to here https://docs.openshift.com/container-platform/4.4/installing/install_config/installing-restricted-networks-preparations.html
|
|
||
| #### Make the API and ingress VIPs dynamically assigned by default, and only assign fixed IPs if they are specified in the install config | ||
|
|
||
| This should be handled by adding OpenStack specific code to the baremetal-runtime-config to enable it to look up ports in a cluster. This would break the circular dependency chain the installer currently has, and allow us to allocate the VIP ports dynamically, rather than assigning fixed IPs. |
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.
can we get more details on how this will be implemented. What is the circular dependency that will be broken?
A bit more explanation perhaps.
| In order to implement this feature fully, the following changes must be made: | ||
| 1. Make `externalNetwork` an optional argument in the install-config when `machinesSubnet` is set | ||
| 2. Make setting `lbFloatingIP` when no external network is passed enforced as invalid usage | ||
| 3. Make the API and ingress VIPs dynamically assigned by default, and only assign fixed IPs if they are specified in the install config |
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 one might be a little tricky. We could list it as a possible improvement.
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.
I am going to remove it, it requires far to many far reaching changes to the installer.
|
|
||
| ### Goals | ||
|
|
||
| - Extend the installer to support provider networks |
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.
IIUC we won't be able to fully support provider networks until we make the switch to the external cloud provider. That's because provider network likely does not have nova-metadata agent and it's currently a requirement for the in-tree openstack cloud provider.
After switching to the external cloud provider, we'll be able to drop the nova-metadata requirement and use config-drive instead.
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.
I asked same question because the in tree hard code to metadata and don't support config drive
so the questions come to when the external cloud provider will be considered?
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.
We'd like to switch to external cloud provider in openshift 4.7. From what I understand, the in-tree cloud provider will be removed from the k8s code base in the 1.21 cycle, mapping to openshift 4.8 so we'll have to make the switch eventually. And we already have our hands full for 4.6.
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: iamemilio, mandre, russellb 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 |
We have received numerous customer requests for the ability to run the IPI installer without using floating IP addresses. This enhancement was made in accordance with this RFE.
/cc @adduarte @jichenjc