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.

Sign up for GitHub

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

Jump to bottom

✨ Add Equinix Metal Load Balancer support #740

Merged
merged 16 commits into from
May 28, 2024
Merged

✨ Add Equinix Metal Load Balancer support #740

merged 16 commits into from
May 28, 2024

Conversation

cprivitere
Copy link
Member

@cprivitere cprivitere commented Mar 12, 2024 •
edited

What this PR does / why we need it:
This PR adds support to CAPP to create Equinix Metal Load Balancers and configure the cluster to use them for its API Server. It does not configure service load balancing and users are encouraged to configure CPEM in their clusters to enable that particular feature.

Fixes: #693

Current Status:

  • Add "EMLB" VIPManager type
  • Create emlb based template that creates a cluster with an EMLB-based VIP
  • Add LBaaS SDK to codebase (since it's not publically available yet)
  • Add emlb internal package
  • Can create new load balancer
  • Can create new origins
  • Can create new pools
  • Can create new listener ports
  • Can add origins to pools
  • Can add pool to listener port
  • Creates new load balancer, pool, port, and origin with new cluster using EMLB VIPManager type.

Edit: Splitting this into multiple PRs. Please see #693 for the full checklist of what must be done.

How to Run
You can use the cluster-template-emlb.yaml template file to deploy a cluster with EMLB. When we do a new release, you can just use --flavor emlb to get that template. For now, it'll be more like:

clusterctl generate cluster quickstart --from go/src/sigs.k8s.io/cluster-api-provider-packet/templates/cluster-template-emlb.yaml > quickstart.yaml

@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Mar 12, 2024
@k8s-ci-robot k8s-ci-robot added approved Indicates a PR has been approved by an approver from all required OWNERS files. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Mar 12, 2024
@cprivitere cprivitere marked this pull request as draft March 12, 2024 23:25
@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 Mar 12, 2024
@cprivitere cprivitere force-pushed the add-emlb-support branch 5 times, most recently from 0519aff to 9d9de3c Compare March 12, 2024 23:55
@k8s-ci-robot k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Apr 11, 2024
dependabot bot and others added 4 commits May 13, 2024 21:17
@dependabot @cprivitere
feat: Add Equinix Metal Load Balancer support
a3e31a0 
- Add internal package for interacting with Equinix Metal Load Balancer API
- packetcluster_controller creates load balancer and listener port and stores their ids in packetCluster annotations.
- packetmachine_controller creates an origin pool and origin port for each machine and stores their IDs in the packetMachine annotations.
- CPEMLBConfig and EMLBID added to the packet cloud client package to be able to provide a config for the CPEM loadBalancer setting in the emlb templates.
- Memory request for the Cluster API Provider Packet controller increased to 300Mi to avoid OOMing while debugging.
- EMLB added as a valid VIPManager enum type.

Signed-off-by: Chris Privitere <[email protected]>
@cprivitere
fix: don't tell cpem about the VIP load balancer
8a27c63 
Signed-off-by: Chris Privitere <[email protected]>
@cprivitere
fix: fix duplicate "origin" in origin naming.
08b9879 
Signed-off-by: Chris Privitere <[email protected]>
@cprivitere
fix: only plumb ip address on localhost on the first machine
3e53a22 
Signed-off-by: Chris Privitere <[email protected]>
@k8s-ci-robot k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label May 14, 2024
@cprivitere
chore: add boilerplate
68c3793 
Signed-off-by: Chris Privitere <[email protected]>
@cprivitere cprivitere removed the request for review from detiber May 20, 2024 19:52
@cprivitere
chore: fix linting issues
36203e2 
Signed-off-by: Chris Privitere <[email protected]>
@cprivitere
test: add additional tests
f440057 
Signed-off-by: Chris Privitere <[email protected]>
ctreatma
ctreatma approved these changes May 22, 2024
View reviewed changes
Copy link
Contributor

@ctreatma ctreatma left a comment

Choose a reason for hiding this comment

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

🚢

@displague
Copy link
Member

If CAPP is creating LBaaS instances (rather than just accessing them by a referenced/existing ID), then it is important for CAPP to cleanup after itself.

Deletion of controller-created resources is a common problem with multiple solutions. I've seen this implemented as labels, annotations, and spec. persistentVolumeReclaimPolicy and deletionPolicy come to mind.

The E2E tests should include a test that creates an LBaaS managed VIP for the control-plane. Upon deleting that cluster, a test should ensure that the loadbalancer was actually deleted.

A near term approach may be to include LBaaS cleanup to the existing post-test sweeper/cleanup code.

If we have existing warning text about indirect CAPP's cluster deletion, we may need to add to that warning that LBaaS instances may also be orphaned. https://docs.crossplane.io/latest/software/uninstall/ as an example.

@displague displague mentioned this pull request May 28, 2024
Open
@cprivitere cprivitere marked this pull request as ready for review May 28, 2024 20:40
@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 May 28, 2024
@k8s-ci-robot k8s-ci-robot requested a review from detiber May 28, 2024 20:40
displague
displague approved these changes May 28, 2024
View reviewed changes
Copy link
Member

@displague displague left a comment

Choose a reason for hiding this comment

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

/approve

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: cprivitere, ctreatma, displague

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:
  • OWNERS [cprivitere,displague]

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

@displague
Copy link
Member

displague commented May 28, 2024 •
edited

This PR lacks any documentation on the new capabilities (ignoring the -emlb.yaml example).
This is intentional to avoid usage until the remaining components of #693 can be incorporated.

@displague
Copy link
Member

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label May 28, 2024
@k8s-ci-robot k8s-ci-robot merged commit 105c296 into kubernetes-sigs:main May 28, 2024
5 checks passed
@cprivitere cprivitere deleted the add-emlb-support branch May 28, 2024 21:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@displague displague displague approved these changes

@ctreatma ctreatma ctreatma approved these changes

@deitch deitch Awaiting requested review from deitch

@detiber detiber Awaiting requested review from detiber

Assignees

@displague displague

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm "Looks good to me", indicates that a PR is ready to be merged. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Use Equinix Metal LB service to provision the Kubernetes API VIP
4 participants
@cprivitere @displague @k8s-ci-robot @ctreatma