enable the use of an external control plane #4611

rpahli · 2024-03-01T10:05:32Z

What type of PR is this?

/kind feature

This PR enables the use off an external control plane (e.g. https://github.com/clastix/cluster-api-control-plane-provider-kamaji)

What this PR does / why we need it:

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 #

Special notes for your reviewer:

cherry-pick candidate

TODOs:

squashed commits
includes documentation
adds unit tests

Release note:

Enable the use of an external control plane

k8s-ci-robot · 2024-03-01T10:05:41Z

Welcome @rpahli!

It looks like this is your first PR to kubernetes-sigs/cluster-api-provider-azure 🎉. 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-azure 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. 😃

k8s-ci-robot · 2024-03-01T10:05:43Z

Hi @rpahli. Thanks for your PR.

I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

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.

api/v1beta1/types.go

rpahli · 2024-04-16T13:28:43Z

@marosset @mboersma any updates on this? Can I get a first review?

k8s-triage-robot · 2024-07-15T14:17:32Z

The Kubernetes project currently lacks enough contributors to adequately respond to all PRs.

This bot triages PRs according to the following rules:

After 90d of inactivity, lifecycle/stale is applied
After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
After 30d of inactivity since lifecycle/rotten was applied, the PR is closed

You can:

Mark this PR as fresh with /remove-lifecycle stale
Close this PR with /close
Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

rpahli · 2024-07-15T14:20:31Z

/remove-lifecycle stale

prometherion · 2024-07-23T08:38:56Z

api/v1beta1/azurecluster_default.go

-	c.setAPIServerLBDefaults()
+	if c.Spec.ControlPlaneEnabled {
+		c.setAPIServerLBDefaults()
+	}
 	c.SetNodeOutboundLBDefaults()
-	c.SetControlPlaneOutboundLBDefaults()


Does order here matter?

It would be cool if we could group these functions together to reduce cyclomatic complexity.

prometherion · 2024-07-23T08:40:45Z

api/v1beta1/azurecluster_default.go

+		c.SetControlPlaneOutboundLBDefaults()
+	}
+	if !c.Spec.ControlPlaneEnabled {
+		c.Spec.NetworkSpec.APIServerLB = nil
+	}


Rather than nulling values, it would be interesting to have a validation webhook complaining about the disparity of values: if the CP is disabled no values are allowed for the APIServerLB, if provided.

prometherion · 2024-07-23T08:45:04Z

api/v1beta1/azurecluster_default.go

@@ -249,7 +266,7 @@ func (c *AzureCluster) setAPIServerLBDefaults() {
 // SetNodeOutboundLBDefaults sets the default values for the NodeOutboundLB.
 func (c *AzureCluster) SetNodeOutboundLBDefaults() {


Do worker nodes use the node outbound LB too?

api/v1beta1/azurecluster_types.go

prometherion · 2024-07-23T08:49:46Z

api/v1beta1/azurecluster_validation.go

 	requiredSubnetRoles := map[string]bool{
-		"control-plane": false,
+		"control-plane": !controlPlaneEnabled,
 		"node":          false,
 	}


This should be refactored this way:

requiredSubnetRoles := map[string]bool{ "node": false, } if controlPlaneEnabled { requiredSubnetRoles["control-plane"] = false }

The key control-plane must not be present when the Control Plane is externally managed.

api/v1beta1/types.go

prometherion · 2024-07-23T08:56:14Z

azure/scope/cluster.go

-			for _, ip := range s.ControlPlaneOutboundLB().FrontendIPs {
-				controlPlaneOutboundIPSpecs = append(controlPlaneOutboundIPSpecs, &publicips.PublicIPSpec{
-					Name:             ip.PublicIP.Name,
+	if s.ControlPlaneEnabled() {


We could avoid the multiple if nesting here, just having the IsAPIServerPrivate block, and put the ControlPlaneEnabled function in the else clause, since we need outbound LB only if the CP Load Balancer is enabled.

azure/scope/cluster.go

prometherion · 2024-07-23T08:59:41Z

config/capz/manager_image_patch.yaml

@@ -8,5 +8,5 @@ spec:
    spec:
      containers:
      # Change the value of image field below to your controller image URL
-      - image: gcr.io/k8s-staging-cluster-api-azure/cluster-api-azure-controller:main
+      - image: localhost:5000/cluster-api-azure-controller-amd64:dev


Just a reminder, amend the changes here.

prometherion · 2024-07-23T09:06:12Z

controllers/azurecluster_controller.go

-	}
-	if azureCluster.Spec.ControlPlaneEndpoint.Port == 0 {
-		azureCluster.Spec.ControlPlaneEndpoint.Port = clusterScope.APIServerPort()
+	if azureCluster.Spec.ControlPlaneEnabled {


We need to check the values also when the Control Plane is externally managed, something like this:

if !azureCluster.Spec.ControlPlaneEnabled { if azureCluster.Spec.ControlPlaneEndpoint.Host == "" { conditions.MarkFalse(azureCluster, infrav1.NetworkInfrastructureReadyCondition, "ExternallyManagedControlPlane", clusterv1.ConditionSeverityInfo, "Waiting for the Control Plane host") return reconcile.Result{}, nil } else if azureCluster.Spec.ControlPlaneEndpoint.Port == 0 { conditions.MarkFalse(azureCluster, infrav1.NetworkInfrastructureReadyCondition, "ExternallyManagedControlPlane", clusterv1.ConditionSeverityInfo, "Waiting for the Control Plane port") return reconcile.Result{}, nil } }

mboersma · 2024-07-25T15:36:36Z

/ok-to-test

rpahli · 2024-11-13T20:58:25Z

Hey @nawazkh I'm currently trying to fix the e2e tests. But all other tests look ok to me.

rpahli · 2024-11-14T11:13:09Z

@nawazkh can someone maybe support me with the e2e tests? Is it possible to debug the tests locally? Or do I need a azure account for that?

nawazkh · 2024-11-14T18:33:36Z

/retest

nawazkh · 2024-11-14T18:34:44Z

@nawazkh can someone maybe support me with the e2e tests? Is it possible to debug the tests locally? Or do I need a azure account for that?

I have triggered a retest since on the first look it looked like a timeout errors. Will dig deeper once it fails.

The best way to debug this would be to run the tests locally with an Azure account.

rpahli · 2024-11-14T20:06:21Z

Ok now it looks only the API die is failing which is expected.

willie-yao · 2024-11-20T00:32:40Z

/assign @mboersma

mboersma

This looks good. Could you update the comment in the new field and do make generate? Sorry for the nitpick, but I see this needs to be rebased anyway.

api/v1beta1/azurecluster_types.go

k8s-ci-robot · 2024-11-25T11:11:19Z

@rpahli: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
pull-cluster-api-provider-azure-apidiff	`9698685`	link	false	`/test pull-cluster-api-provider-azure-apidiff`

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

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-sigs/prow repository. I understand the commands that are listed here.

willie-yao

/lgtm

k8s-ci-robot · 2024-11-25T18:08:36Z

LGTM label has been added.

Git tree hash: 10c3c7622804a9040dad3fff5b252caa398566f0

willie-yao · 2024-11-25T18:09:14Z

/test pull-cluster-api-provider-azure-apiversion-upgrade

mboersma

/lgtm
/approve

Thanks for your perseverance @rpahli!

k8s-ci-robot · 2024-11-25T20:26:21Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: mboersma

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

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 kind/feature do-not-merge/release-note-label-needed cncf-cla: yes labels Mar 1, 2024

k8s-ci-robot requested review from Jont828 and mboersma March 1, 2024 10:05

k8s-ci-robot added size/L needs-ok-to-test labels Mar 1, 2024

rpahli mentioned this pull request Mar 1, 2024

feat: azure support clastix/cluster-api-control-plane-provider-kamaji#84

Merged

prometherion reviewed Apr 2, 2024

View reviewed changes

api/v1beta1/types.go Show resolved Hide resolved

k8s-ci-robot added size/XL and removed size/L labels Apr 8, 2024

rpahli marked this pull request as ready for review April 9, 2024 07:12

k8s-ci-robot removed the do-not-merge/work-in-progress label Apr 9, 2024

k8s-ci-robot requested a review from marosset April 9, 2024 07:12

k8s-ci-robot added the lifecycle/stale label Jul 15, 2024

k8s-ci-robot removed the lifecycle/stale label Jul 15, 2024

prometherion suggested changes Jul 23, 2024

View reviewed changes

k8s-ci-robot added release-note and removed do-not-merge/release-note-label-needed labels Jul 25, 2024

k8s-ci-robot added ok-to-test and removed needs-ok-to-test labels Jul 25, 2024

mboersma added this to the v1.17 milestone Jul 25, 2024

k8s-ci-robot added the needs-rebase label Nov 15, 2024

rpahli force-pushed the external-control-plane branch from 3ad59ca to 21d3915 Compare November 18, 2024 09:00

k8s-ci-robot removed the needs-rebase label Nov 18, 2024

rpahli force-pushed the external-control-plane branch from 21d3915 to 0571951 Compare November 18, 2024 10:22

prometherion approved these changes Nov 18, 2024

View reviewed changes

k8s-ci-robot assigned mboersma Nov 20, 2024

k8s-ci-robot added the needs-rebase label Nov 22, 2024

mboersma reviewed Nov 22, 2024

View reviewed changes

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

rpahli force-pushed the external-control-plane branch from 0571951 to 59e54a4 Compare November 25, 2024 10:50

enable the use off an external control plane

Loading
Loading status checks…

9698685

rpahli force-pushed the external-control-plane branch from 59e54a4 to 9698685 Compare November 25, 2024 11:00

k8s-ci-robot removed the needs-rebase label Nov 25, 2024

willie-yao approved these changes Nov 25, 2024

View reviewed changes

k8s-ci-robot assigned willie-yao Nov 25, 2024

k8s-ci-robot added the lgtm label Nov 25, 2024

mboersma approved these changes Nov 25, 2024

View reviewed changes

k8s-ci-robot added the approved label Nov 25, 2024

k8s-ci-robot merged commit d54979f into kubernetes-sigs:main Nov 25, 2024
19 of 20 checks passed

k8s-ci-robot added this to the v1.18 milestone Nov 25, 2024

rpahli deleted the external-control-plane branch November 26, 2024 08:12

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

enable the use of an external control plane #4611

enable the use of an external control plane #4611

rpahli commented Mar 1, 2024 •

edited by mboersma

Loading

k8s-ci-robot commented Mar 1, 2024

k8s-ci-robot commented Mar 1, 2024

rpahli commented Apr 16, 2024

k8s-triage-robot commented Jul 15, 2024

rpahli commented Jul 15, 2024

prometherion Jul 23, 2024

prometherion Jul 23, 2024

prometherion Jul 23, 2024

prometherion Jul 23, 2024

prometherion Jul 23, 2024

prometherion Jul 23, 2024

prometherion Jul 23, 2024

mboersma commented Jul 25, 2024

rpahli commented Nov 13, 2024

rpahli commented Nov 14, 2024

nawazkh commented Nov 14, 2024

nawazkh commented Nov 14, 2024

rpahli commented Nov 14, 2024

willie-yao commented Nov 20, 2024

mboersma left a comment

k8s-ci-robot commented Nov 25, 2024 •

edited

Loading

willie-yao left a comment

k8s-ci-robot commented Nov 25, 2024

willie-yao commented Nov 25, 2024

mboersma left a comment

k8s-ci-robot commented Nov 25, 2024

		@@ -249,7 +266,7 @@ func (c *AzureCluster) setAPIServerLBDefaults() {
		// SetNodeOutboundLBDefaults sets the default values for the NodeOutboundLB.
		func (c *AzureCluster) SetNodeOutboundLBDefaults() {

enable the use of an external control plane #4611

enable the use of an external control plane #4611

Conversation

rpahli commented Mar 1, 2024 • edited by mboersma Loading

k8s-ci-robot commented Mar 1, 2024

k8s-ci-robot commented Mar 1, 2024

rpahli commented Apr 16, 2024

k8s-triage-robot commented Jul 15, 2024

rpahli commented Jul 15, 2024

prometherion Jul 23, 2024

Choose a reason for hiding this comment

prometherion Jul 23, 2024

Choose a reason for hiding this comment

prometherion Jul 23, 2024

Choose a reason for hiding this comment

prometherion Jul 23, 2024

Choose a reason for hiding this comment

prometherion Jul 23, 2024

Choose a reason for hiding this comment

prometherion Jul 23, 2024

Choose a reason for hiding this comment

prometherion Jul 23, 2024

Choose a reason for hiding this comment

mboersma commented Jul 25, 2024

rpahli commented Nov 13, 2024

rpahli commented Nov 14, 2024

nawazkh commented Nov 14, 2024

nawazkh commented Nov 14, 2024

rpahli commented Nov 14, 2024

willie-yao commented Nov 20, 2024

mboersma left a comment

Choose a reason for hiding this comment

k8s-ci-robot commented Nov 25, 2024 • edited Loading

willie-yao left a comment

Choose a reason for hiding this comment

k8s-ci-robot commented Nov 25, 2024

willie-yao commented Nov 25, 2024

mboersma left a comment

Choose a reason for hiding this comment

k8s-ci-robot commented Nov 25, 2024

rpahli commented Mar 1, 2024 •

edited by mboersma

Loading

k8s-ci-robot commented Nov 25, 2024 •

edited

Loading