✨ Add support for a secondary control plane load balancer #4733

nrb · 2024-01-11T21:22:18Z

What type of PR is this?
/kind feature

What this PR does / why we need it:

Certain topologies of clusters may use an internal load balancer for the API server for a few reasons. A primary reason is to keep traffic within the cloud provider in order to reduce egress costs.

This PR adds support for a secondary control plane load balancer, internal to the Kubernetes cluster for such a use case.

Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged):
Fixes #4554

Special notes for your reviewer:

Previously, it was believed that security groups could not be added to a network load balancer, which meant that there were exceptions and exclusions around NLBs in the code. This is only true if the load balancer was created without any security groups - SGs cannot be added after creation. If the network load balancer is created with at least one security group, more can be added or removed.

See the Network Load Balancer docs for more details.

Checklist:

Release note:

Adds support for a secondary, internal API server load balancer within clusters.

k8s-ci-robot · 2024-01-11T21:22:21Z

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

nrb · 2024-01-11T21:22:50Z

/test pull-cluster-api-provider-aws-test

nrb · 2024-01-13T23:12:21Z

/retest-required

nrb · 2024-01-18T23:22:00Z

I'm now able to create a 2nd load balancer within my AWS clusters.

The target group associated with it is also healthy.

I just have rebasing, writing automated tests, and writing docs left before I can remove the WIP.

nrb · 2024-01-19T20:33:27Z

Confirming that nodes within the cluster can reach the load balancer:

✦ ❯ kubectl --kubeconfig $HOME/secondlb.kubeconfig attach curl-nrb -c curl-nrb -it
If you don't see a command prompt, try pressing enter.
[ root@curl-nrb:/ ]$ curl -k https://numbertwo-e4d5b20bb984138d.elb.us-east-2.amazonaws.com:6443
{
  "kind": "Status",
  "apiVersion": "v1",
  "metadata": {},
  "status": "Failure",
  "message": "forbidden: User \"system:anonymous\" cannot get path \"/\"",
  "reason": "Forbidden",
  "details": {},
  "code": 403
}[ root@curl-nrb:/ ]$

api/v1beta1/awscluster_conversion.go

vincepri · 2024-01-19T21:01:42Z

api/v1beta2/awscluster_webhook.go

+	// TODO(nrb): The validate-on-update default test was failing when newlb was nil
+	// Upon investigation, both values were nil, which is unlikely in a real world scenario
+	if oldlb == nil && newlb == nil {
+		return nil
+	}


newLoadBalancer := &AWSLoadBalancerSpec{} existingLoadBalancer := &AWSLoadBalancerSpec{} if r.Spec.ControlPlaneLoadBalancer != nil { newLoadBalancer = r.Spec.ControlPlaneLoadBalancer.DeepCopy() } if oldC.Spec.ControlPlaneLoadBalancer != nil { existingLoadBalancer = oldC.Spec.ControlPlaneLoadBalancer.DeepCopy() }

This was previously handled this way

Which to be honest, maybe better to restructure, given that it was a bit confusing

api/v1beta2/awscluster_webhook.go

controllers/awsmachine_controller.go

docs/book/src/topics/secondary-load-balancer.md

pkg/cloud/scope/elb.go

pkg/cloud/services/elb/loadbalancer.go

richardcase · 2024-01-22T17:18:48Z

/milestone v2.4.0

richardcase · 2024-01-23T14:46:37Z

/milestone v2.4.0

* A secondary control plane load balancer can be added. * It will be a network load balancer. * Its scheme must not match that of the ControlPlaneLoadBalancer. Previously, NLBs could not have security groups attached. This has now changed, and an NLB can have a security group attached at creation. If a security group is _not_ present at creation, then the NLB can never have security groups added.

nrb · 2024-01-24T22:57:57Z

/test pull-cluster-api-provider-aws-e2e
/test pull-cluster-api-provider-aws-e2e-conformance

vincepri · 2024-01-24T22:59:13Z

/test ?

k8s-ci-robot · 2024-01-24T22:59:16Z

@vincepri: The following commands are available to trigger required jobs:

/test pull-cluster-api-provider-aws-build
/test pull-cluster-api-provider-aws-test
/test pull-cluster-api-provider-aws-verify

The following commands are available to trigger optional jobs:

/test pull-cluster-api-provider-aws-apidiff-main
/test pull-cluster-api-provider-aws-e2e
/test pull-cluster-api-provider-aws-e2e-blocking
/test pull-cluster-api-provider-aws-e2e-clusterclass
/test pull-cluster-api-provider-aws-e2e-conformance
/test pull-cluster-api-provider-aws-e2e-conformance-with-ci-artifacts
/test pull-cluster-api-provider-aws-e2e-eks
/test pull-cluster-api-provider-aws-e2e-eks-gc
/test pull-cluster-api-provider-aws-e2e-eks-testing

Use /test all to run the following jobs that were automatically triggered:

pull-cluster-api-provider-aws-apidiff-main
pull-cluster-api-provider-aws-build
pull-cluster-api-provider-aws-test
pull-cluster-api-provider-aws-verify

In response to this:

/test ?

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.

vincepri · 2024-01-24T22:59:30Z

/test pull-cluster-api-provider-aws-apidiff-main
/test pull-cluster-api-provider-aws-e2e
/test pull-cluster-api-provider-aws-e2e-blocking
/test pull-cluster-api-provider-aws-e2e-clusterclass
/test pull-cluster-api-provider-aws-e2e-conformance
/test pull-cluster-api-provider-aws-e2e-conformance-with-ci-artifacts
/test pull-cluster-api-provider-aws-e2e-eks
/test pull-cluster-api-provider-aws-e2e-eks-gc
/test pull-cluster-api-provider-aws-e2e-eks-testing

nrb · 2024-01-24T23:04:13Z

Looks like some sort of account lookup issue with Boskos; it fails here with a 404.

nrb · 2024-01-24T23:04:43Z

/test pull-cluster-api-provider-aws-e2e-blocking

nrb · 2024-01-25T15:55:03Z

/retest

vincepri · 2024-01-25T22:20:33Z

/retest

nrb · 2024-01-26T16:11:38Z

/retest

vincepri

/lgtm
/assign @Ankitasw @richardcase

vincepri · 2024-01-26T18:02:30Z

cc @JoelSpeed

richardcase

Great work @nrb . We should try to get this covered via e2e in the future.

richardcase · 2024-02-04T11:52:40Z

api/v1beta2/network_types.go

@@ -37,6 +37,9 @@ type NetworkStatus struct {
 	// APIServerELB is the Kubernetes api server load balancer.
 	APIServerELB LoadBalancer `json:"apiServerElb,omitempty"`

+	// SecondaryAPIServerELB is the secondary Kubernetes api server load balancer.
+	SecondaryAPIServerELB LoadBalancer `json:"secondaryAPIServerELB,omitempty"`


nit: would be good to mark it optional

richardcase · 2024-02-04T11:53:48Z

controllers/awsmachine_controller.go

@@ -331,7 +332,8 @@ func (r *AWSMachineReconciler) reconcileDelete(machineScope *scope.MachineScope,
 	if err := r.reconcileLBAttachment(machineScope, elbScope, instance); err != nil {
 		// We are tolerating AccessDenied error, so this won't block for users with older version of IAM;
 		// all the other errors are blocking.
-		if !elb.IsAccessDenied(err) && !elb.IsNotFound(err) {
+		// Because we are reconciling all load balancers, attempt to treat the error as a list of errors.
+		if err = kerrors.FilterOut(err, elb.IsAccessDenied, elb.IsNotFound); err != nil {


I didn't know about kerrors.FilterOut. Excellent.

richardcase · 2024-02-04T11:57:36Z

docs/book/src/topics/secondary-load-balancer.md

+This secondary control plane load balancer is primarily meant to be used for internal cluster traffic, for use cases where traffic between nodes and pods should be kept internal to the VPC network.
+This adds a layer of privacy to traffic, as well as potentially saving on egress costs for traffic to the Kubernetes API server.
+
+A dual load balancer topology is not used as a default in order to maintain backward compatibility with existing CAPA clusters.


We should consider adding a template for this. Also, we should think about whether we think having a 2nd internal LB is a good default for the future (i.e. then you opt out).

Yeah, I'll make an issue for the template.

One thing I'm looking at upstream is to support a slice of ControlPlaneEndpoints, which I think would tend toward a default set up of two LBs, but I'm not positive. This PR is sort of a trial run, though I think CAPZ already supports internal/external LBs.

richardcase · 2024-02-04T12:01:14Z

/approve

k8s-ci-robot · 2024-02-04T12:01:21Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: richardcase

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~~ [richardcase]

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 added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. release-note Denotes a PR that will be considered when it comes time to generate release notes. labels Jan 11, 2024

k8s-ci-robot added kind/feature Categorizes issue or PR as related to a new feature. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. needs-priority size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Jan 11, 2024

k8s-ci-robot requested review from fiunchinho and vincepri January 11, 2024 21:22

nrb force-pushed the secondary-lb branch from f6ccb77 to 7160bd1 Compare January 18, 2024 23:40

nrb marked this pull request as ready for review January 19, 2024 18:22

k8s-ci-robot requested review from AverageMarcus and faiq January 19, 2024 18:22

nrb force-pushed the secondary-lb branch from 0a6d6e9 to 9ec6413 Compare January 19, 2024 18:35

nrb changed the title ~~WIP: ✨ Add support for a secondary control plane load balancer~~ ✨ Add support for a secondary control plane load balancer Jan 19, 2024

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 Jan 19, 2024

nrb force-pushed the secondary-lb branch 3 times, most recently from 0e71456 to b08fe0c Compare January 19, 2024 20:17

vincepri reviewed Jan 19, 2024

View reviewed changes

api/v1beta1/awscluster_conversion.go Outdated Show resolved Hide resolved

vincepri reviewed Jan 19, 2024

View reviewed changes

api/v1beta2/awscluster_webhook.go Show resolved Hide resolved

vincepri reviewed Jan 19, 2024

View reviewed changes

nrb commented Jan 19, 2024

View reviewed changes

pkg/cloud/services/elb/loadbalancer.go Outdated Show resolved Hide resolved

k8s-ci-robot added this to the v2.4.0 milestone Jan 22, 2024

richardcase removed this from the v2.4.0 milestone Jan 23, 2024

k8s-ci-robot added this to the v2.4.0 milestone Jan 23, 2024

nrb force-pushed the secondary-lb branch from b08fe0c to c72f25d Compare January 24, 2024 20:34

vincepri approved these changes Jan 26, 2024

View reviewed changes

k8s-ci-robot assigned Ankitasw, richardcase and vincepri Jan 26, 2024

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jan 26, 2024

richardcase reviewed Feb 4, 2024

View reviewed changes

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Feb 4, 2024

richardcase mentioned this pull request Feb 4, 2024

✨ Supporting externally managed Control Plane #4438

Merged

4 tasks

k8s-ci-robot merged commit ee2c5f4 into kubernetes-sigs:main Feb 4, 2024
25 checks passed

nrb mentioned this pull request Feb 6, 2024

Add clusterctl template for secondary LB configuration #4778

Open

This was referenced Feb 15, 2024

Network Load Balancers won't reconcile in main #4790

Closed

🐛 Don't reconcile security groups on NLBs from older versions of CAPA #4794

Merged

T-Kukawka mentioned this pull request Apr 17, 2024

Kubernetes API concurrent private and public load balancers giantswarm/roadmap#492

Open

2 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

✨ Add support for a secondary control plane load balancer #4733

✨ Add support for a secondary control plane load balancer #4733

nrb commented Jan 11, 2024 •

edited

Loading

k8s-ci-robot commented Jan 11, 2024

nrb commented Jan 11, 2024

nrb commented Jan 13, 2024

nrb commented Jan 18, 2024

nrb commented Jan 19, 2024

vincepri Jan 19, 2024

vincepri Jan 19, 2024

richardcase commented Jan 22, 2024

richardcase commented Jan 23, 2024

nrb commented Jan 24, 2024

vincepri commented Jan 24, 2024

k8s-ci-robot commented Jan 24, 2024

vincepri commented Jan 24, 2024

nrb commented Jan 24, 2024 •

edited

Loading

nrb commented Jan 24, 2024

nrb commented Jan 25, 2024

vincepri commented Jan 25, 2024

nrb commented Jan 26, 2024

vincepri left a comment

vincepri commented Jan 26, 2024

richardcase left a comment

richardcase Feb 4, 2024

richardcase Feb 4, 2024

richardcase Feb 4, 2024

nrb Feb 5, 2024

richardcase commented Feb 4, 2024

k8s-ci-robot commented Feb 4, 2024

✨ Add support for a secondary control plane load balancer #4733

✨ Add support for a secondary control plane load balancer #4733

Conversation

nrb commented Jan 11, 2024 • edited Loading

k8s-ci-robot commented Jan 11, 2024

nrb commented Jan 11, 2024

nrb commented Jan 13, 2024

nrb commented Jan 18, 2024

nrb commented Jan 19, 2024

vincepri Jan 19, 2024

Choose a reason for hiding this comment

vincepri Jan 19, 2024

Choose a reason for hiding this comment

richardcase commented Jan 22, 2024

richardcase commented Jan 23, 2024

nrb commented Jan 24, 2024

vincepri commented Jan 24, 2024

k8s-ci-robot commented Jan 24, 2024

vincepri commented Jan 24, 2024

nrb commented Jan 24, 2024 • edited Loading

nrb commented Jan 24, 2024

nrb commented Jan 25, 2024

vincepri commented Jan 25, 2024

nrb commented Jan 26, 2024

vincepri left a comment

Choose a reason for hiding this comment

vincepri commented Jan 26, 2024

richardcase left a comment

Choose a reason for hiding this comment

richardcase Feb 4, 2024

Choose a reason for hiding this comment

richardcase Feb 4, 2024

Choose a reason for hiding this comment

richardcase Feb 4, 2024

Choose a reason for hiding this comment

nrb Feb 5, 2024

Choose a reason for hiding this comment

richardcase commented Feb 4, 2024

k8s-ci-robot commented Feb 4, 2024

nrb commented Jan 11, 2024 •

edited

Loading

nrb commented Jan 24, 2024 •

edited

Loading