-
Notifications
You must be signed in to change notification settings - Fork 596
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
[occm] pattern based subnet detection for loadbalancers #1375
Conversation
Welcome @mandelsoft! |
Hi @mandelsoft. Thanks for your PR. I'm waiting for a kubernetes member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. 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. |
Build succeeded.
|
This PR intended to fix #1328 |
1427ed7
to
93a4eb5
Compare
Build failed.
|
Build failed.
|
Build failed.
|
Build failed.
|
Build succeeded.
|
Build succeeded.
|
Build succeeded.
|
Build succeeded.
|
Build succeeded.
|
Build succeeded.
|
Build succeeded.
|
Build succeeded.
|
Thanks for proposing this PR. First, I want to make sure that in your cloud, is normal user able to get subnets of an external network? I'm asking because in devstack (also in our cloud), that's not allowed. |
/hold |
Additionally, I still prefer to use subnet tag instead of a name pattern, as suggested in the issue. |
Please add that info in the PR description. |
Build succeeded.
|
The job cloud-provider-openstack-acceptance-test-e2e-conformance-stable-branch-v1.18 may have some problem. |
@lingxiankong: The specified target(s) for
Use 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. |
/test cloud-provider-openstack-acceptance-test-lb-octavia |
@lingxiankong: The specified target(s) for
Use 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. |
Build succeeded.
|
don't worry about |
/unhold |
/ok-to-test |
/override openlab/cloud-provider-openstack-acceptance-test-e2e-conformance-stable-branch-v1.18 |
@lingxiankong: Overrode contexts on behalf of lingxiankong: openlab/cloud-provider-openstack-acceptance-test-e2e-conformance-stable-branch-v1.18 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. |
Simplified the release note |
/lgtm I will probably do a follow up patch for some minor improvements. |
Thank you @lingxiankong for reviewing this PR. |
We need review from another maintainer. |
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.
@mandelsoft Thanks for the PR. Have minor suggestion inline.
Approving this, later could be improved in followup PRs
/approve
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: ramineni 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 |
) * pattern based subnet detection for loadbalancers * rework subnet detection to additionally support tags * rework defaulting of network/subnet spec handling annotations * support tag lists and REST api based tag filtering * improve error handling * incorporate requested changes * rework subnet spec based filtering * rework subnet filter
Which issue this PR fixes(if applicable):
fixes #1328
What this PR does / why we need it:
A floating IP for a loadbalancer is created on a subnet of a floating pool (specified by a network id of the provider network).
Such a subnet has a dedicated IP range (CIDR). Therefore it might happen, that the subnet runs out of IP addresses. To not disrupt the existing FIP users in such a case a new subnet can be added to the provider network. This one will potentially automatically be selected for new FIP creation if the creator does not explicitly specify the subnet id.
But this automated subnet selection makes only sense if all subnets configured for a provider network provide the same kind
of routing. In Openstack it is possible to configure subnets for the same provider network that feature different kinds of routing (for example public internet or company network). In such a scenario so far a dedicated subnet must be specified for the FIP creation of a loadbalancer. If this setting is not done via service annotation, but as configuration of the cloud controller manager it is not possible to transparently add new matching subnets to the provider network by an administrator which is then used for further FIP creation.
This PR provides support for a new configuration possibility to overcome this problem. Instead of a dedicated subnet id it is possible now to optionally configure a subnet name pattern (supporting the standard glob mechanism) or tags. For this configuration option all matching subnets of a provider network are tried to create a new FIP. The first one where the FIP creation succeeds is used.
This allows configuring subnets with similar names according to name patterns specific for the routing environment, so that FIPs can transparently be created for loadbalancers just by adding a subnet with a name matching the dedicated name pattern.
Special notes for reviewers:
Release note: