Add ClusterClass Support for Managed Clusters #4155

willie-yao · 2023-10-18T21:50:00Z

What type of PR is this?
/kind feature

What this PR does / why we need it:
This PR adds ClusterClass support for managed clusters.

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 #2684

Special notes for your reviewer:

cherry-pick candidate

TODOs:

squashed commits
includes documentation
adds unit tests

Release note:

Add ClusterClass support for managed clusters

willie-yao · 2023-10-18T21:51:02Z

/test pull-cluster-api-provider-azure-e2e-optional

willie-yao · 2023-10-18T21:54:40Z

api/v1beta1/azuremanagedcontrolplane_webhook.go

-func (m *AzureManagedControlPlane) validateVersion(_ client.Client) error {
-	if !kubeSemver.MatchString(m.Spec.Version) {
-		return errors.New("must be a valid semantic version")
+func validateVersion(version string, fldPath *field.Path) field.ErrorList {


Refactored these webhook validation functions to be usable by the template types

willie-yao · 2023-10-18T21:55:29Z

api/v1beta1/azuremanagedmachinepool_webhook.go

 }

 // validateLastSystemNodePool is used to check if the existing system node pool is the last system node pool.
 // If it is a last system node pool it cannot be deleted or mutated to user node pool as AKS expects min 1 system node pool.
-func (m *AzureManagedMachinePool) validateLastSystemNodePool(cli client.Client) error {
+func validateLastSystemNodePool(cli client.Client, labels map[string]string, namespace string) error {


Refactored these functions for the same purpose as above ^^

test/e2e/config/azure-dev.yaml

codecov · 2023-10-18T23:50:10Z

Codecov Report

Attention: 187 lines in your changes are missing coverage. Please review.

Comparison is base (55e0559) 59.55% compared to head (224b7dc) 59.73%.
Report is 2 commits behind head on main.

Files	Patch %	Lines
...v1beta1/azuremanagedmachinepooltemplate_webhook.go	56.73%	85 Missing and 5 partials ⚠️
...1beta1/azuremanagedcontrolplanetemplate_webhook.go	74.61%	43 Missing and 7 partials ⚠️
api/v1beta1/azuremanagedclustertemplate_webhook.go	0.00%	18 Missing ⚠️
api/v1beta1/azuremanagedcontrolplane_webhook.go	93.66%	7 Missing and 2 partials ⚠️
api/v1beta1/azuremanagedmachinepool_webhook.go	95.65%	5 Missing ⚠️
api/v1beta1/azuremanagedcontrolplane_default.go	93.33%	3 Missing and 1 partial ⚠️
...1beta1/azuremanagedcontrolplanetemplate_default.go	88.23%	3 Missing and 1 partial ⚠️
controllers/helpers.go	0.00%	3 Missing and 1 partial ⚠️
azure/defaults.go	0.00%	2 Missing ⚠️
controllers/azuremanagedcontrolplane_controller.go	0.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #4155      +/-   ##
==========================================
+ Coverage   59.55%   59.73%   +0.17%     
==========================================
  Files         185      192       +7     
  Lines       18782    19303     +521     
==========================================
+ Hits        11186    11531     +345     
- Misses       6981     7142     +161     
- Partials      615      630      +15

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Jont828 · 2023-10-18T23:51:39Z

/assign

Jont828

This is looking good! Here's some comments from a first pass. It seems like most of it is template stuff, so I think if we can get all the tests passing, we can consider the templates to be good to go.

api/v1beta1/azuremanagedclustertemplate_types.go

api/v1beta1/azuremanagedclustertemplate_webhook.go

api/v1beta1/azuremanagedcontrolplane_types.go

api/v1beta1/azuremanagedcontrolplanetemplate_types.go

docs/book/src/topics/clusterclass.md

templates/test/ci/prow-aks-clusterclass/patches/aks-clusterclass-pool1.yaml

test/e2e/azure_test.go

test/e2e/config/azure-dev.yaml

willie-yao · 2023-10-19T16:05:37Z

/retest

jackfrancis · 2023-10-19T16:52:37Z

/assign @nojnhuh

jackfrancis · 2023-10-19T16:52:55Z

/assign

willie-yao · 2023-10-19T18:31:11Z

Not sure what's going on, but looks like the E2E test is having trouble applying the cluster template. It was working before but I made some updates to the webhoook validation since. Will be debugging this

willie-yao · 2023-10-20T18:34:37Z

Not sure what's going on, but looks like the E2E test is having trouble applying the cluster template. It was working before but I made some updates to the webhoook validation since. Will be debugging this

Fixed!

nojnhuh

Still working through this, but wanted to share what I had so far.

api/v1beta1/azuremanagedclustertemplate_webhook.go

api/v1beta1/azuremanagedcontrolplane_types.go

api/v1beta1/consts.go

api/v1beta1/azuremanagedcontrolplane_webhook.go

api/v1beta1/azuremanagedcontrolplane_webhook_test.go

api/v1beta1/azuremanagedcontrolplanetemplate_default.go

test/e2e/config/azure-dev.yaml

api/v1beta1/types_template.go

k8s-ci-robot · 2023-11-16T18:23:24Z

LGTM label has been added.

Git tree hash: 8bb8429606aaa9c36a8f27c4995b47a0d54b3434

CecileRobertMichon · 2023-11-16T19:05:13Z

api/v1beta1/consts.go

@@ -170,4 +170,6 @@ const (
 	AzureManagedControlPlaneKind = "AzureManagedControlPlane"
 	// AzureClusterIdentityKind indicates the kind of an AzureClusterIdentity.
 	AzureClusterIdentityKind = "AzureClusterIdentity"
+	// AzureNetworkPluginName is the name of the Azure network plugin.
+	AzureNetworkPluginName = "azure"


nit: can we move this out of that const block, it's not a Kind so doesn't belong with the others

can do in follow up

CecileRobertMichon · 2023-11-16T19:06:43Z

api/v1beta1/azuremanagedcontrolplanetemplate_webhook.go

+		return nil, mcp.validateManagedControlPlaneTemplate(mcpw.Client)
+	}
+
+	return nil, apierrors.NewInvalid(GroupVersion.WithKind("AzureManagedControlPlaneTemplate").GroupKind(), mcp.Name, allErrs)


now that we have consts for kinds, would be good to also use constants for these new kinds

Can be a follow up since that other PR just merged

CecileRobertMichon

/lgtm
/approve
/hold cancel

@willie-yao let's merge this to get it into the release. Can you please open a follow-up PR to include the new kinds as constants following #4233 ?

k8s-ci-robot · 2023-11-16T19:07:54Z

LGTM label has been added.

Git tree hash: 9b2c6cf19c88392c511758bfef11333d9e2f4744

k8s-ci-robot · 2023-11-16T19:07:57Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: CecileRobertMichon

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

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

willie-yao · 2023-11-16T19:28:24Z

let's merge this to get it into the release. Can you please open a follow-up PR to include the new kinds as constants following #4233 ?

Will do, thanks!

willie-yao · 2023-11-16T19:51:40Z

/retest

CecileRobertMichon · 2023-11-16T20:06:21Z

/retest

k8s-ci-robot · 2023-11-16T20:09:43Z

@willie-yao: 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	`224b7dc`	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/test-infra repository. I understand the commands that are listed here.

CecileRobertMichon · 2023-11-16T21:36:46Z

Bypassing test rerun to speed up release as the last main commit is a metadata.yaml only update (#4284) and we have green signal on the current commit already from the previous run: https://prow.k8s.io/view/gs/kubernetes-jenkins/pr-logs/pull/kubernetes-sigs_cluster-api-provider-azure/4155/pull-cluster-api-provider-azure-e2e-aks/1725228064436129792

/override pull-cluster-api-provider-azure-ci-entrypoint
/override pull-cluster-api-provider-azure-e2e
/override pull-cluster-api-provider-azure-e2e-aks

k8s-ci-robot · 2023-11-16T21:37:53Z

@CecileRobertMichon: Overrode contexts on behalf of CecileRobertMichon: pull-cluster-api-provider-azure-ci-entrypoint

In response to this:

/override pull-cluster-api-provider-azure-ci-entrypoint

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.

k8s-ci-robot · 2023-11-16T21:38:21Z

@CecileRobertMichon: Overrode contexts on behalf of CecileRobertMichon: pull-cluster-api-provider-azure-e2e, pull-cluster-api-provider-azure-e2e-aks

In response to this:

/override pull-cluster-api-provider-azure-e2e
/override pull-cluster-api-provider-azure-e2e-aks

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.

k8s-ci-robot added release-note Denotes a PR that will be considered when it comes time to generate release notes. 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. labels Oct 18, 2023

k8s-ci-robot requested review from mboersma and sonasingh46 October 18, 2023 21:50

k8s-ci-robot added the size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. label Oct 18, 2023

willie-yao mentioned this pull request Oct 18, 2023

Add ClusterClass Support for Managed Clusters #3197

Closed

3 tasks

willie-yao commented Oct 18, 2023

View reviewed changes

willie-yao force-pushed the clusterclass-aks-final branch from 8a22191 to 04c455e Compare October 18, 2023 22:07

willie-yao commented Oct 18, 2023

View reviewed changes

test/e2e/config/azure-dev.yaml Outdated Show resolved Hide resolved

willie-yao force-pushed the clusterclass-aks-final branch from 04c455e to 1b5519d Compare October 18, 2023 23:22

k8s-ci-robot assigned Jont828 Oct 18, 2023

Jont828 reviewed Oct 19, 2023

View reviewed changes

k8s-ci-robot assigned nojnhuh Oct 19, 2023

k8s-ci-robot assigned jackfrancis Oct 19, 2023

willie-yao force-pushed the clusterclass-aks-final branch 3 times, most recently from 7f66d45 to 322243d Compare October 19, 2023 20:42

nojnhuh reviewed Oct 23, 2023

View reviewed changes

willie-yao force-pushed the clusterclass-aks-final branch 2 times, most recently from 46c2516 to 20bad8e Compare October 24, 2023 01:31

k8s-ci-robot added approved Indicates a PR has been approved by an approver from all required OWNERS files. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Nov 16, 2023

Add ClusterClass support for Managed Clusters

224b7dc

willie-yao force-pushed the clusterclass-aks-final branch from ce7d6b8 to 224b7dc Compare November 16, 2023 19:02

k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Nov 16, 2023

k8s-ci-robot requested a review from CecileRobertMichon November 16, 2023 19:02

k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Nov 16, 2023

CecileRobertMichon reviewed Nov 16, 2023

View reviewed changes

k8s-ci-robot added lgtm "Looks good to me", indicates that a PR is ready to be merged. and removed do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. labels Nov 16, 2023

This was referenced Nov 16, 2023

update test metadata for 1.12 release #4285

Merged

Revert "Pin Azure disk CSI driver Helm chart to v1.29.0" #4279

Merged

This comment was marked as duplicate.

Sign in to view

This comment was marked as outdated.

Sign in to view

k8s-ci-robot merged commit 005b9f8 into kubernetes-sigs:main Nov 16, 2023
29 of 30 checks passed

willie-yao mentioned this pull request Nov 16, 2023

Add missing kind constants for API #4287

Merged

4 tasks

nojnhuh added the area/managedclusters Issues related to managed AKS clusters created through the CAPZ ManagedCluster Type label Feb 14, 2024

richardcase mentioned this pull request Nov 4, 2024

Add ClusterClass support machine pools kubernetes-sigs/cluster-api-provider-aws#4658

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add ClusterClass Support for Managed Clusters #4155

Add ClusterClass Support for Managed Clusters #4155

willie-yao commented Oct 18, 2023 •

edited

Loading

willie-yao commented Oct 18, 2023

willie-yao Oct 18, 2023

willie-yao Oct 18, 2023

codecov bot commented Oct 18, 2023 •

edited

Loading

Jont828 commented Oct 18, 2023

Jont828 left a comment

willie-yao commented Oct 19, 2023

jackfrancis commented Oct 19, 2023

jackfrancis commented Oct 19, 2023

willie-yao commented Oct 19, 2023

willie-yao commented Oct 20, 2023

nojnhuh left a comment

k8s-ci-robot commented Nov 16, 2023

CecileRobertMichon Nov 16, 2023

CecileRobertMichon Nov 16, 2023

CecileRobertMichon Nov 16, 2023

CecileRobertMichon left a comment

k8s-ci-robot commented Nov 16, 2023

k8s-ci-robot commented Nov 16, 2023

willie-yao commented Nov 16, 2023

willie-yao commented Nov 16, 2023

CecileRobertMichon commented Nov 16, 2023

k8s-ci-robot commented Nov 16, 2023 •

edited

Loading

CecileRobertMichon commented Nov 16, 2023 •

edited

Loading

This comment was marked as duplicate.

k8s-ci-robot commented Nov 16, 2023

This comment was marked as outdated.

k8s-ci-robot commented Nov 16, 2023

Add ClusterClass Support for Managed Clusters #4155

Add ClusterClass Support for Managed Clusters #4155

Conversation

willie-yao commented Oct 18, 2023 • edited Loading

willie-yao commented Oct 18, 2023

willie-yao Oct 18, 2023

Choose a reason for hiding this comment

willie-yao Oct 18, 2023

Choose a reason for hiding this comment

codecov bot commented Oct 18, 2023 • edited Loading

Codecov Report

Jont828 commented Oct 18, 2023

Jont828 left a comment

Choose a reason for hiding this comment

willie-yao commented Oct 19, 2023

jackfrancis commented Oct 19, 2023

jackfrancis commented Oct 19, 2023

willie-yao commented Oct 19, 2023

willie-yao commented Oct 20, 2023

nojnhuh left a comment

Choose a reason for hiding this comment

k8s-ci-robot commented Nov 16, 2023

CecileRobertMichon Nov 16, 2023

Choose a reason for hiding this comment

CecileRobertMichon Nov 16, 2023

Choose a reason for hiding this comment

CecileRobertMichon Nov 16, 2023

Choose a reason for hiding this comment

CecileRobertMichon left a comment

Choose a reason for hiding this comment

k8s-ci-robot commented Nov 16, 2023

k8s-ci-robot commented Nov 16, 2023

willie-yao commented Nov 16, 2023

willie-yao commented Nov 16, 2023

CecileRobertMichon commented Nov 16, 2023

k8s-ci-robot commented Nov 16, 2023 • edited Loading

CecileRobertMichon commented Nov 16, 2023 • edited Loading

This comment was marked as duplicate.

k8s-ci-robot commented Nov 16, 2023

This comment was marked as outdated.

k8s-ci-robot commented Nov 16, 2023

willie-yao commented Oct 18, 2023 •

edited

Loading

codecov bot commented Oct 18, 2023 •

edited

Loading

k8s-ci-robot commented Nov 16, 2023 •

edited

Loading

CecileRobertMichon commented Nov 16, 2023 •

edited

Loading