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

feat: add support for Equinix Metal Load Balancer #470

Merged
merged 48 commits into from
Oct 26, 2023

Conversation

ctreatma
Copy link
Contributor

@ctreatma ctreatma commented Oct 24, 2023

This updates CPEM to support the beta Load Balancer service provided by Equinix Metal. CPEM users can now configure CPEM to create external, managed Layer 4 load balancers for service load balancing and control plane load balancing.

This PR also adds a local development workflow using Tilt.

@k8s-ci-robot k8s-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Oct 24, 2023
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: ctreatma
Once this PR has been reviewed and has the lgtm label, please assign cheftako for approval. 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

@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Oct 24, 2023
cprivitere and others added 23 commits October 24, 2023 14:24
Signed-off-by: Chris Privitere <[email protected]>
Signed-off-by: Chris Privitere <[email protected]>
Signed-off-by: Chris Privitere <[email protected]>
Signed-off-by: Chris Privitere <[email protected]>
Signed-off-by: Chris Privitere <[email protected]>
- add the front end port
- use the front end port for the load balancer
- point out the service annotations don't work

Signed-off-by: Chris Privitere <[email protected]>
Signed-off-by: Chris Privitere <[email protected]>
ctreatma and others added 20 commits October 24, 2023 14:25
Signed-off-by: Chris Privitere <[email protected]>
Signed-off-by: Chris Privitere <[email protected]>
To avoid adding knowledge of specific load balancer implementers in
`loadbalancers.go`, this moves the `GetLoadBalancer` logic for EMLB
into `emlb.go`.

In the future we can refactor the pre-existing `GetLoadBalancer`
code in order to push that down to `metallb`, `kubevip`, `empty`,
etc., keeping `loadbalancers.go` focused on the work of deciding
which implementer to use and how to translate from the cloud-provider
interface to our internal implementer interface.
When deploying a new LoadBalancer service, we would see a "Failed
to update..." error message in the logs, and 2 load balancers would
be created on the Equinix side, with only 1 of them reflected in
the service annotations.

This changes our `Update` call to update the k8s service object with
a `Patch` call; by doing this we focus our k8s API calls on changing
only the properties that our provider has modified, which reduces the
chance that something else has concurrently made conflicting changes.
Co-authored-by: Charles Treatman <[email protected]>
Signed-off-by: Chris Privitere <[email protected]>
After inspecting API responses in the Equinix Metal console, I saw
that origin pool data is not nested under the corresponding port.

Instead, `ports` is an array of ports, and `pools` is an array of
arrays of pools.  The top-level index of `pools` corresponds to
the top-level index of `ports`, so `pools[i]` is an array of all
of the origin pools assigned to `ports[i]`.

The old code was looking for origin data in the wrong place, so it
was failing to delete origins that are no longer needed.  Now it
is looking in the right place and successfully deleting old origins.
Signed-off-by: Chris Privitere <[email protected]>
- Allow the control plane LoadBalancer service to be managed
  by the provider if `usesBGP` is false for the selected load
  balancer implementation
- Add a reconciler for LBaaS-based control plane LoadBalancer
- Add a `LoadBalancerID` config option to specify the ID of the
  load balancer to use for the control plane
@ctreatma ctreatma marked this pull request as ready for review October 24, 2023 19:26
@k8s-ci-robot k8s-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Oct 24, 2023
- [kube-vip](#kube-vip)
- [MetalLB](#metallb)
- [empty](#empty)

CCM does **not** deploy _any_ load balancers for you. It limits itself to managing the Equinix Metal-specific
API calls to support a load balancer, and providing configuration for supported load balancers.

##### Equinix Metal Load Balancer

Equinix Metal Load Balancer (EMLB) is a beta service that is available to a limited number of Equinix Metal customers that provides managed layer 4 load balancers.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
Equinix Metal Load Balancer (EMLB) is a beta service that is available to a limited number of Equinix Metal customers that provides managed layer 4 load balancers.
[Equinix Metal Load Balancer](https://github.com/equinix/lbaas-api-docs) (EMLB) is a [service under development](https://feedback.equinixmetal.com/network/p/load-balancer-as-a-service-lbaas), currently in [beta](https://deploy.equinix.com/enroll-beta-lab/), that provides managed layer 4 load balancers.

For the control plane nodes, the Equinix Metal CCM uses static Elastic IP assignment, via the Equinix Metal API, to tell the
Equinix Metal network which control plane node should receive the traffic. For more details on the control plane
load-balancer, see [this section](#control-plane-load-balancing).

#### Service LoadBalancer Implementations
Copy link
Member

@displague displague Oct 24, 2023

Choose a reason for hiding this comment

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

nit: We should link to this section above in the table where the loadbalancer setting is introduced without any options described.

8a048dc#diff-b335630551682c19a781afebcf4d07bf978fb1f8ac04c6bf87428ed5106870f5R224

@@ -233,6 +233,7 @@ This section lists each configuration option, and whether it can be set by each
| Kubernetes Service annotation to set EIP metro | | `METAL_ANNOTATION_EIP_METRO` | `annotationEIPMetro` | `"metal.equinix.com/eip-metro"` |
| Kubernetes Service annotation to set EIP facility | | `METAL_ANNOTATION_EIP_FACILITY` | `annotationEIPFacility` | `"metal.equinix.com/eip-facility"` |
| Tag for control plane Elastic IP | | `METAL_EIP_TAG` | `eipTag` | No control plane Elastic IP |
| ID for control plane Equinix Metal Load Balancer | | `METAL_LOAD_BALANCER_ID` | `loadBalancerID` | No control plane Equinix Metal Load Balancer |
Copy link
Member

@displague displague Oct 24, 2023

Choose a reason for hiding this comment

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

I think this option would fit with how we offer arguments in METAL_LOAD_BALANCER, as used by metallb:

  • METAL_LOAD_BALANCER=emlb://sv
  • METAL_LOAD_BALANCER=emlb://?loadbalancerID=123
  • METAL_LOAD_BALANCER=emlb://?existing-only-tagged=cpem

Perhaps we don't need a separate option for ID.

(This is conversational, not merge blocking)

Copy link
Member

Choose a reason for hiding this comment

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

I know there is no support for tags today.

I'm curious how well CPEM will handle the limitation of working with a single LoadBalancer and whether this is more of a development feature or if this is an argument that users will want in their clusters.

Copy link
Member

@displague displague Oct 24, 2023

Choose a reason for hiding this comment

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

Alternatively, if new options are the preferred approach, perhaps with #472, we may want to expose CLI / environment / config arguments in the future for the metallb options only offered through METAL_LOAD_BALANCER query parameters today.

@displague displague merged commit 5a5aa8a into kubernetes-sigs:main Oct 26, 2023
5 of 6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants