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

operator: Add LB-IPAM #21764

Merged
merged 5 commits into from
Nov 26, 2022
Merged

operator: Add LB-IPAM #21764

merged 5 commits into from
Nov 26, 2022

Conversation

dylandreimerink
Copy link
Member

This PR introduces a new feature called LoadBalancer IP Address Management, LB-IPAM for short. The goal of LB-IPAM is to provide a mechanism to allocate and assign IP addresses to services of type LoadBalancer, to be used when no external IPAM is available. The primary use-case we are targeting is a private cloud/bare metal deployment where one or more CIDRs are assigned to the cluster to be used. Additional Cilium features should be utilized to direct traffic for the assigned IPs to the cluster and the correct nodes(for now just BGP, but not exclusively).

LB-IPAM has the concept of an IP Pool which is a list of CIDRs and an optional service label selector. Users can add, modify or remove these at will via the CiliumLoadBalancerIPPool custom resource (formally the CiliumBGPLoadBalancerIPPool, which was renamed to make it more generic). The service label selector can be used to limit which services can get IPs from the given pool.

LB-IPAM can allocate and assign both IPv4 and IPv6 addresses in SingleStack and DualStack operation. LB-IPAM can be deployed in a cluster with multiple load balancers since it respects the loadBalancerClass values of services. Lastly LB-IPAM allows services to request one or more specific IPs.

Add LoadBalancer IP address management (LB-IPAM)

@maintainer-s-little-helper maintainer-s-little-helper bot added the dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. label Oct 17, 2022
@dylandreimerink dylandreimerink added area/documentation Impacts the documentation, including textual changes, sphinx, or other doc generation code. kind/feature This introduces new functionality. release-note/major This PR introduces major new functionality to Cilium. area/operator Impacts the cilium-operator component sig/ipam IP address management, including cloud IPAM labels Oct 17, 2022
@maintainer-s-little-helper maintainer-s-little-helper bot removed dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. labels Oct 17, 2022
@lazedo
Copy link

lazedo commented Oct 17, 2022

Hello,
if this is to replace the metallb loadbalancer then can you ensure that something like metallb.universe.tf/allow-shared-ip: "my-shared-ip" is implemented (sharing a loadbalancer ip) ? I do see that we can request a specific ip, but requesting by a known name is very useful and allows to deploy service manifests without knowing the shared ip in advance

@dylandreimerink
Copy link
Member Author

if this is to replace the metallb loadbalancer

That is the eventual intent combined with some other work that is still in progress.

then can you ensure that something like metallb.universe.tf/allow-shared-ip: "my-shared-ip" is implemented (sharing a loadbalancer ip) ?

The LB-IPAM version in this PR doesn't support that feature. I am not going for full feature parity with MetalLB, at least not in the initial release. The feature seems quite involved, not easily added to the PR as it stands, though I am open to adding it in a future iteration. I have created issue #21776 to track it.

@dylandreimerink dylandreimerink force-pushed the feature/lb-ipam branch 3 times, most recently from 31d6552 to 5b92978 Compare October 18, 2022 10:43
@dylandreimerink dylandreimerink marked this pull request as ready for review October 18, 2022 11:16
@dylandreimerink dylandreimerink requested review from a team as code owners October 18, 2022 11:16
@dylandreimerink dylandreimerink force-pushed the feature/lb-ipam branch 2 times, most recently from 3103b60 to 8973c8f Compare November 16, 2022 15:07
@dylandreimerink
Copy link
Member Author

@ldelossa I have now refactored and processed feedback, replaced the cell-wide context+cancel+done channel with a workerpool as discussed offline and plumbed the context via params so there is no longer a ctx field in the cell.

@ldelossa
Copy link
Contributor

thanks @dylandreimerink - LGTM.

joamaki and others added 3 commits November 24, 2022 10:39
This adds:
- metav1.Condition, metav1.ConditionStatus
- metav1.ObjectMeta.Generation
- corev1.IPFamilyPolicy
- corev1.IPFamilyPolicyType
- corev1.LoadBalancerClass
- corev1.Service.{IPFamilyPolicy, LoadBalancerClass}
- corev1.ServiceStatus.Condition

Signed-off-by: Jussi Maki <jussi@isovalent.com>
To make it easier to partially transition to using Resource[T],
expose the underlying cache.Store. Hopefully temporary :fingerscrossed:.

Signed-off-by: Jussi Maki <jussi@isovalent.com>
This commit renames the CiliumBGPLoadBalancerIPPool CRD to the
CiliumLoadBalancerIPPool so it may be used for load balancers other
than those who use BGP.

The IP Pool will be used by the operators LB IPAM component, and the
contents of the CRD have been updated to match the new requirements.

Signed-off-by: Dylan Reimerink <dylan.reimerink@isovalent.com>
@dylandreimerink
Copy link
Member Author

/test

dylandreimerink and others added 2 commits November 25, 2022 14:47
This commit adds the LB-IPAM feature. LB-IPAM allows users to specify
a set of pools containing one or more CIDRs. Services of type
LoadBalancer will receive Ingress IPs from these pools.

LB-IPAM is part of the ongoing work to add service announcements to
the BGP Control Plane. However, the component is designed to be generic
so it can be used by other features as well.

Co-authored-by: Jussi Maki <jussi@isovalent.com>
Signed-off-by: Dylan Reimerink <dylan.reimerink@isovalent.com>
This commit adds documentation for the LB-IPAM feature.

Signed-off-by: Dylan Reimerink <dylan.reimerink@isovalent.com>
@dylandreimerink
Copy link
Member Author

/test

@dylandreimerink
Copy link
Member Author

dylandreimerink commented Nov 25, 2022

Alright got 5 approvals covering all requested teams. All feedback is addresses. All tests are green except for:

ConformaceAKS: fails with Running test client-egress-to-echo-expression-deny: setting up test: applying network policies: policies were not applied on all Cilium nodes in time: error dialing backend: read unix @->/tunnel-uds/proxysocket: read: connection reset by peer. This seems unrelated to any changes made in this PR but is also isn't a known flake, I will be making a new issue for this one. Edit: #22367

ConformanceGKE: fails at install time with "Unable to install Cilium: timeout while waiting for status to become successful: context deadline exceeded". However, by the time we collect the sysdump all agents are up and cilium status is happy, so it looks like something is waiting to short. In any case, it doesn't look related to this PR.

ConformanceKind1.19 / installation-and-connectivity: Fails due to a curl timeout in the client-egress-to-echo-service-account-deny senario. I am unable to find an existing GH issue for this flake, but it again doesn't seem to have much to do with this PR since it is in policy enforcement.

Travis CI hits #21730.

None of these failures seem to be caused by changed made in this PR since they are known flakes. So I will add the ready-to-merge label.

@dylandreimerink dylandreimerink added the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label Nov 25, 2022
@aanm aanm merged commit 655ed8d into master Nov 26, 2022
@aanm aanm deleted the feature/lb-ipam branch November 26, 2022 00:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/documentation Impacts the documentation, including textual changes, sphinx, or other doc generation code. area/operator Impacts the cilium-operator component kind/feature This introduces new functionality. ready-to-merge This PR has passed all tests and received consensus from code owners to merge. release-note/major This PR introduces major new functionality to Cilium. sig/ipam IP address management, including cloud IPAM
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet