✨ Add support for kube-vip #320

davidspek · 2022-04-08T13:24:00Z

Replacement of: detiber#65.
What this PR does / why we need it:
This replaces the binding of an elastic IP to one of the control plane nodes with deploying kube-vip and having it load balance the Kubernetes API between the various control plane nodes.

linux-foundation-easycla · 2022-04-08T13:24:08Z

The committers listed above are authorized under a signed CLA.

✅ login: DavidSpek / name: David van der Spek (ecc3976, 843f036, 5d41478, bf68262, 9751345, be61a92, d95b01a, 088bbf9, 7d7c0b7)
✅ login: cprivitere / name: Chris Privitere (a6c222c, 40c7de0, b68c838, a0fc851, b05171b, f3f5d78, 857424f, 277e4a4, d1b8a2b, d552aad, ae0ed29, f1fb7a9)

k8s-ci-robot · 2022-04-08T13:24:10Z

Welcome @davidspek!

It looks like this is your first PR to kubernetes-sigs/cluster-api-provider-packet 🎉. Please refer to our pull request process documentation to help your PR have a smooth ride to approval.

You will be prompted by a bot to use commands during the review process. Do not be afraid to follow the prompts! It is okay to experiment. Here is the bot commands documentation.

You can also check if kubernetes-sigs/cluster-api-provider-packet has its own contribution guidelines.

You may want to refer to our testing guide if you run into trouble with your tests not passing.

If you are having difficulty getting your pull request seen, please follow the recommended escalation practices. Also, for tips and tricks in the contribution process you may want to read the Kubernetes contributor cheat sheet. We want to make sure your contribution gets all the attention it needs!

Thank you, and welcome to Kubernetes. 😃

…rvices

davidspek · 2022-04-08T13:27:01Z

@cprivitere I still need to validate that nothing went wrong during the rebase, but here is the PR in any case.

davidspek · 2022-04-08T13:34:51Z

@cprivitere Looks like nothing strange happened during the rebasing. I will do a test deployment though to be sure, but it should be good to continue on from here.

WillsonHG · 2022-04-08T16:13:07Z

/easycla

cprivitere · 2022-04-08T18:14:38Z

@davidspek So I recently changed my Github username from @cprivite to @cprivitere . Because of this the email address associated with my github is now 23177737+cprivitere@users.noreply.github.com. This is causing EasyCLA to not be able to validate that I've signed the CLA and thus block this PR from going through. It sounds like you either need to squash all the commits to just be from you or have you update the author of each commit to be my new username. Do you have a preference?

Signed-off-by: Chris Privitere <cprivite@users.noreply.github.com>

cprivitere · 2022-04-11T18:50:07Z

It looks like prow isn't managing the CI jobs. Not sure what this does but it is waiting for approval. https://github.com/kubernetes-sigs/cluster-api-provider-packet/actions/runs/2150416972

Yeah, I've already messaged @detiber for help.

displague

It looks like my review comments are a little late and some discussion has already happened about making kube-vip non-default. I have some concerns about BGP being enabled by default.

displague · 2022-04-11T16:26:06Z

pkg/cloud/packet/client.go

+	// - bgpConfig struct does not have Status=="disabled"
+	if err == nil && bgpConfig != nil && bgpConfig.ID != "" && strings.ToLower(bgpConfig.Status) != "disabled" {
+		return nil
+	}


What will we do if the BGPConfig Get request fails? Invalid project or token, timeout, API availability issues?
Let's handle err != nil here. We should log at the very least.

If BGP can not be enabled, we can return that error and this error will cascade through the reconciliation loop and we will await the next reconciliation loop to attempt to enable BGP. That sounds good. The log messages will be helpful if the resource can not resolve because of this.

displague · 2022-04-11T16:48:43Z

controllers/packetmachine_controller.go

-		// when the node is a control plan we should check if the elastic ip
-		// for this cluster is not assigned. If it is free we can prepare the
-		// current node to use it.
+		// when the node is a control plan we need the elastic IP


Suggested change

// when the node is a control plan we need the elastic IP

// when a node is a control plane node we need the elastic IP

displague · 2022-04-11T16:54:02Z

controllers/packetcluster_controller.go

@@ -137,6 +137,11 @@ func (r *PacketClusterReconciler) reconcileNormal(ctx context.Context, clusterSc
 		}
 	}

+	if err := r.PacketClient.EnableProjectBGP(packetCluster.Spec.ProjectID); err != nil {


BGP + KubeVIP mode should be optional

Once enabled in a project, BGP can not be easily disabled and some of the initial settings can not be modified. We shouldn't enable this by default for everyone.

displague · 2022-04-11T16:55:59Z

controllers/packetmachine_controller.go

@@ -362,6 +353,11 @@ func (r *PacketMachineReconciler) reconcile(ctx context.Context, machineScope *s
 	machineScope.SetProviderID(dev.ID)
 	machineScope.SetInstanceStatus(infrav1.PacketResourceStatus(dev.State))

+	if err := r.PacketClient.EnsureNodeBGPEnabled(dev.ID); err != nil {


This should be optional, based on a CAPP setting.

You'll want to look at the latest version.

cprivitere · 2022-04-12T15:31:51Z

controllers/packetmachine_controller.go

@@ -222,7 +223,7 @@ func (r *PacketMachineReconciler) PacketClusterToPacketMachines(ctx context.Cont
 	}
 }

-func (r *PacketMachineReconciler) reconcile(ctx context.Context, machineScope *scope.MachineScope) (ctrl.Result, error) { //nolint:gocyclo
+func (r *PacketMachineReconciler) reconcile(ctx context.Context, machineScope *scope.MachineScope) (ctrl.Result, error) {


Suggested change

func (r *PacketMachineReconciler) reconcile(ctx context.Context, machineScope *scope.MachineScope) (ctrl.Result, error) {

func (r *PacketMachineReconciler) reconcile(ctx context.Context, machineScope *scope.MachineScope) (ctrl.Result, error) { //nolint:gocyclo

I think we need to add this nolint comment back in.

Signed-off-by: DavidSpek <vanderspek.david@gmail.com>

davidspek · 2022-04-21T11:16:16Z

@cprivitere @displague Sorry for the slow response here. I've just pushed the suggested changes.

cprivitere · 2022-04-29T19:12:40Z

@davidspek can you make generate and re-push?

Signed-off-by: DavidSpek <vanderspek.david@gmail.com>

davidspek · 2022-05-03T09:56:00Z

@cprivitere The lint error should be resolved now

davidspek · 2022-05-03T13:40:45Z

I've just come across this error during the creation of a control plane node:

E0503 13:30:41.782725       1 controller.go:317] controller/packetmachine "msg"="Reconciler error" "error"="failed to enable bpg on machine eqm-cluster-small-on-demand-4wlm8: POST https://api.equinix.com/metal/v1/devices/6e646e83-08bf-468c-8395-6398ee17128c/bgp/sessions: 422 Device has no assigned IP addresses " "name"="eqm-cluster-small-on-demand-4wlm8" "namespace"="bootstrap" "reconciler group"="infrastructure.cluster.x-k8s.io" "reconciler kind"="PacketMachine"

This error should probably trigger a reconciliation so it will retry enabling BGP on the node.

EDIT:
I just noticed this error occurs when the machine is still provisioning, so it shouldn't be an issue actually.

Signed-off-by: DavidSpek <vanderspek.david@gmail.com>

…uster Type (#5) * Convert to having the EIP_MANAGEMENT variable as part of the packetcluster type * Make vipmanager field immutable. Signed-off-by: Chris Privitere <23177737+cprivitere@users.noreply.github.com> * Rename field to VIPManager Signed-off-by: Chris Privitere <23177737+cprivitere@users.noreply.github.com> * rename to vipmanager, fix defaults, rm services Signed-off-by: Chris Privitere <23177737+cprivitere@users.noreply.github.com> * Fix typo and make VIPManager an enum Signed-off-by: Chris Privitere <23177737+cprivitere@users.noreply.github.com>

cprivitere · 2022-05-17T23:42:16Z

/lgtm

cprivitere · 2022-05-17T23:42:59Z

/approve

k8s-ci-robot · 2022-05-17T23:43:13Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: cprivitere, DavidSpek

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]

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

cprivitere · 2022-05-17T23:45:01Z

/retest

k8s-ci-robot added the cncf-cla: no Indicates the PR's author has not signed the CNCF CLA. label Apr 8, 2022

k8s-ci-robot requested a review from deitch April 8, 2022 13:24

k8s-ci-robot added the size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. label Apr 8, 2022

k8s-ci-robot requested a review from detiber April 8, 2022 13:24

davidspek force-pushed the kube-vip branch from 1a09ca6 to 7b1f2ba Compare April 8, 2022 13:24

davidspek added 8 commits April 8, 2022 15:25

init

ecc3976

run make generate

843f036

don't have CCM manage the control plane EIP

5d41478

fix linting errors

bf68262

remove comments from template

9751345

Add facility to CCM config so EIPs can be created for LoadBalancer se…

be61a92

…rvices

pin kube-vip version

d95b01a

fix hardcoded project id and make kube-vip version configurable

088bbf9

davidspek force-pushed the kube-vip branch from 7b1f2ba to 7544fc0 Compare April 8, 2022 13:25

cprivitere and others added 10 commits April 11, 2022 17:43

Clean up scripting, fix awk, add ip routes for IBX datacenters

a6c222c

Signed-off-by: Chris Privitere <cprivite@users.noreply.github.com>

remove caching of metadata where it isn't used

40c7de0

Remove echo used for debuggin

b68c838

Signed-off-by: Chris Privitere <cprivite@users.noreply.github.com>

Adding a space for readability.

a0fc851

Signed-off-by: Chris Privitere <cprivite@users.noreply.github.com>

Have kubeadm ignore manifests directory already existing.

b05171b

Signed-off-by: Chris Privitere <cprivite@users.noreply.github.com>

run make generate

7d7c0b7

revert to non-kubevip state

f3f5d78

split kube-vip version to separate template

857424f

run make generate

277e4a4

Signed-off-by: Chris Privitere <cprivite@users.noreply.github.com>

Generate kube-vip template via kustomize

d1b8a2b

Signed-off-by: Chris Privitere <cprivite@users.noreply.github.com>

displague requested changes Apr 11, 2022

View reviewed changes

cprivitere requested changes Apr 12, 2022

View reviewed changes

k8s-ci-robot removed the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Apr 12, 2022

add suggested changes

4509155

Signed-off-by: DavidSpek <vanderspek.david@gmail.com>

davidspek mentioned this pull request Apr 21, 2022

Add cluster autoscaler scale from 0 support #327

Open

jhead-slg mentioned this pull request Apr 26, 2022

Not all nodes BGP enabled since v3.4.0 kubernetes-sigs/cloud-provider-equinix-metal#259

Closed

cprivitere changed the title ~~✨ Add support for kube-vip~~ [WIP] ✨ Add support for kube-vip Apr 29, 2022

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 Apr 29, 2022

davidspek added 2 commits May 2, 2022 12:01

run make generate

1e09a52

Signed-off-by: DavidSpek <vanderspek.david@gmail.com>

fix lint error

467fadc

Signed-off-by: DavidSpek <vanderspek.david@gmail.com>

davidspek and others added 2 commits May 4, 2022 17:23

remove services from kube-vip config

98c29cd

Signed-off-by: DavidSpek <vanderspek.david@gmail.com>

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label May 17, 2022

cprivitere changed the title ~~[WIP] ✨ Add support for kube-vip~~ ✨ Add support for kube-vip May 17, 2022

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 17, 2022

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label May 17, 2022

k8s-ci-robot merged commit a6d3608 into kubernetes-sigs:main May 17, 2022

displague added this to the 0.6.0 milestone Jul 15, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

✨ Add support for kube-vip #320

✨ Add support for kube-vip #320

davidspek commented Apr 8, 2022

linux-foundation-easycla bot commented Apr 8, 2022 •

edited

k8s-ci-robot commented Apr 8, 2022

davidspek commented Apr 8, 2022

davidspek commented Apr 8, 2022

WillsonHG commented Apr 8, 2022

cprivitere commented Apr 8, 2022 •

edited

cprivitere commented Apr 11, 2022

displague left a comment

displague Apr 11, 2022

displague Apr 11, 2022

displague Apr 11, 2022

displague Apr 11, 2022

displague Apr 11, 2022

cprivitere Apr 11, 2022

cprivitere Apr 12, 2022

cprivitere Apr 12, 2022

davidspek commented Apr 21, 2022

cprivitere commented Apr 29, 2022

davidspek commented May 3, 2022

davidspek commented May 3, 2022 •

edited

cprivitere commented May 17, 2022

cprivitere commented May 17, 2022

k8s-ci-robot commented May 17, 2022

cprivitere commented May 17, 2022

	// when the node is a control plan we need the elastic IP
	// when a node is a control plane node we need the elastic IP

	func (r PacketMachineReconciler) reconcile(ctx context.Context, machineScope scope.MachineScope) (ctrl.Result, error) {
	func (r PacketMachineReconciler) reconcile(ctx context.Context, machineScope scope.MachineScope) (ctrl.Result, error) { //nolint:gocyclo

✨ Add support for kube-vip #320

✨ Add support for kube-vip #320

Conversation

davidspek commented Apr 8, 2022

linux-foundation-easycla bot commented Apr 8, 2022 • edited

k8s-ci-robot commented Apr 8, 2022

davidspek commented Apr 8, 2022

davidspek commented Apr 8, 2022

WillsonHG commented Apr 8, 2022

cprivitere commented Apr 8, 2022 • edited

cprivitere commented Apr 11, 2022

displague left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

davidspek commented Apr 21, 2022

cprivitere commented Apr 29, 2022

davidspek commented May 3, 2022

davidspek commented May 3, 2022 • edited

cprivitere commented May 17, 2022

cprivitere commented May 17, 2022

k8s-ci-robot commented May 17, 2022

cprivitere commented May 17, 2022

linux-foundation-easycla bot commented Apr 8, 2022 •

edited

cprivitere commented Apr 8, 2022 •

edited

davidspek commented May 3, 2022 •

edited