ASOAPI: basic automated adoption #4799

nojnhuh · 2024-05-02T21:13:02Z

What type of PR is this?
/kind feature

What this PR does / why we need it:

This PR implements an automated adoption mechanism for ASOAPI-based clusters. It works by watching ASO ManagedClusters and ManagedClustersAgentPools with a sentinel sigs.k8s.io/cluster-api-provider-azure-adopt=true annotation and scaffolds the CAPZ/CAPI resources around them required to represent a full Cluster API Cluster. It's designed to work in conjunction with asoctl import azure-resource in order to work with any pre-existing AKS cluster, though any ASO YAML should work.

This PR currently includes the changes from #4798 in the first two commits, but I'll merge main into this branch after that merges to keep this PR diff clean.

Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged):
Part of #4713 and related to #1173

Special notes for your reviewer:

cherry-pick candidate

TODOs:

squashed commits
includes documentation
adds unit tests

Release note:

NONE

nojnhuh · 2024-05-02T21:15:55Z

exp/mutators/azureasomanagedcontrolplane.go

@@ -80,6 +81,10 @@ func SetManagedClusterDefaults(ctrlClient client.Client, asoManagedControlPlane
 			return err
 		}

+		if err := setManagedClusterCredentials(ctx, cluster, managedClusterPath, managedCluster); err != nil {


asoctl import doesn't populate any configuration for ASO to generate a kubeconfig Secret for the cluster, so CAPZ will now take responsibility for doing that if it's not already defined. This smooths out adoption by letting users essentially pipe the asoctl output straight to kubectl apply.

codecov · 2024-05-02T21:23:22Z

Codecov Report

Attention: Patch coverage is 6.34921% with 236 lines in your changes are missing coverage. Please review.

Project coverage is 61.93%. Comparing base (39189af) to head (076bde4).
Report is 26 commits behind head on main.

Files	Patch %	Lines
exp/controllers/agentpooladopt_controller.go	0.00%	114 Missing ⚠️
exp/controllers/managedclusteradopt_controller.go	0.00%	112 Missing ⚠️
exp/mutators/azureasomanagedcontrolplane.go	61.53%	5 Missing and 5 partials ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #4799      +/-   ##
==========================================
- Coverage   62.74%   61.93%   -0.82%     
==========================================
  Files         199      201       +2     
  Lines       16507    16821     +314     
==========================================
+ Hits        10358    10418      +60     
- Misses       5379     5620     +241     
- Partials      770      783      +13

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

nojnhuh · 2024-05-02T21:24:33Z

Known gaps here:

unit and e2e tests
~~docs~~ (Now included!)
Other Azure resources you might want represented in your CAPZ resources like AKS extensions or Fleet memberships are not currently injected into the CAPZ specs. Those resources will remain untouched by and unknown to CAPZ except that they will be deleted if they exist in the same resource group as the ManagedCluster when the CAPZ cluster is deleted. Currently we're planning to add support in the future some subset of these extra resources types.
Exactly how the CAPI/CAPZ resources look is not configurable. If you need to change things like names of those resources, the suggestion for now is to skip this annotation-based mechanism and construct those CAPI/CAPZ resources yourself. asoctl import can still do most of the heavy lifting here and that YAML can be copy-pasted piecemeal into your CAPZ resources.

jackfrancis · 2024-05-02T21:24:57Z

exp/controllers/agentpooladopt_controller.go

+)
+
+// AgentPoolAdoptReconciler adopts ASO ManagedClustersAgentPool resources into a CAPI Cluster.
+type AgentPoolAdoptReconciler struct {


I wonder if "import" would be a better term to use in the actual code (rather than "adopt"). What other use cases does "detect that an ASO resource doesn't have an equivalent CAPI/CAPZ resource" serve besides "adoption" in the sense that we've been discussing? (which I'd summarize as "purposefully migrate cluster lifecycle management of an existing cluster to Cluster API")

cc @willie-yao @mboersma

Do you mean you find one of those terms more general or specific than the other? To me "adopt" and "import" seem synonymous enough that I'm not sure I follow.

I think I prefer the term "adopt" since "import" is more related to getting the ASO Spec that matches an existing resource, while CAPZ is actively managing something that is adopted. Like Jon mentioned I think they are pretty similar too, so it can go either way.

nojnhuh · 2024-05-03T16:21:12Z

docs/book/src/topics/managedcluster.md

+<!-- markdown-link-check-disable-next-line -->
+The [experimental AzureASOManagedControlPlane and related APIs](/topics/aso.html#experimental-aso-api) support


The link checker doesn't seem to like linking to a dynamically generated domain-local path like this. The link as it appears in the docs preview here seems to work though: https://deploy-preview-4799--kubernetes-sigs-cluster-api-provider-azure.netlify.app/topics/managedcluster#option-1-using-the-experimental-aso-based-api

jackfrancis · 2024-05-03T23:49:36Z

/retest

docs/book/src/topics/managedcluster.md

willie-yao · 2024-05-04T01:18:38Z

docs/book/src/topics/managedcluster.md

+- Adopting existing clusters created with the GA AzureManagedControlPlane API to the experimental API with
+  this method is theoretically possible, but untested. Care should be taken to prevent CAPZ from reconciling
+  two different representations of the same underlying Azure resources.
+


I think it could be helpful to include an example of what the spec will look like for using the ASO experimental API. I'm following this and created the ASO resources but as a first-time user, I'm not sure how to incorporate that into a CAPZ spec. I think it also might help to link to the ASO API docs from here.

I added a link to the flavor template in the docs in #4802. Does that help?

Yup that looks good!

jackfrancis · 2024-05-06T18:30:54Z

/lgtm
/approve

k8s-ci-robot · 2024-05-06T18:30:59Z

LGTM label has been added.

Git tree hash: 2de65e57bb0a50d45bea989528d5b4f0bef346af

k8s-ci-robot · 2024-05-06T18:31:01Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: jackfrancis

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

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

jackfrancis · 2024-05-06T18:31:24Z

/hold

for squashing commits

nojnhuh · 2024-05-06T18:34:15Z

squashed!
/hold cancel

jackfrancis · 2024-05-06T20:40:44Z

/retest

k8s-ci-robot added release-note-none Denotes a PR that doesn't merit a release note. 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 May 2, 2024

k8s-ci-robot requested review from marosset and willie-yao May 2, 2024 21:13

k8s-ci-robot added the size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. label May 2, 2024

nojnhuh commented May 2, 2024

View reviewed changes

jackfrancis reviewed May 2, 2024

View reviewed changes

k8s-ci-robot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels May 2, 2024

nojnhuh commented May 3, 2024

View reviewed changes

willie-yao reviewed May 4, 2024

View reviewed changes

k8s-ci-robot assigned jackfrancis May 6, 2024

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

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

k8s-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label May 6, 2024

ASOAPI: basic automated adoption

076bde4

nojnhuh force-pushed the v2-adopt branch from 0d82749 to 076bde4 Compare May 6, 2024 18:34

k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label May 6, 2024

jackfrancis merged commit 0c64bea into kubernetes-sigs:main May 6, 2024
27 of 28 checks passed

k8s-ci-robot added this to the v1.15 milestone May 6, 2024

nojnhuh deleted the v2-adopt branch May 6, 2024 23:53

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ASOAPI: basic automated adoption #4799

ASOAPI: basic automated adoption #4799

nojnhuh commented May 2, 2024

nojnhuh May 2, 2024

codecov bot commented May 2, 2024 •

edited

Loading

nojnhuh commented May 2, 2024 •

edited

Loading

jackfrancis May 2, 2024

nojnhuh May 2, 2024

willie-yao May 2, 2024

nojnhuh May 3, 2024

jackfrancis commented May 3, 2024

willie-yao May 4, 2024

nojnhuh May 6, 2024

willie-yao May 6, 2024

jackfrancis commented May 6, 2024

k8s-ci-robot commented May 6, 2024

k8s-ci-robot commented May 6, 2024

jackfrancis commented May 6, 2024

nojnhuh commented May 6, 2024

jackfrancis commented May 6, 2024

		<!-- markdown-link-check-disable-next-line -->
		The [experimental AzureASOManagedControlPlane and related APIs](/topics/aso.html#experimental-aso-api) support

ASOAPI: basic automated adoption #4799

ASOAPI: basic automated adoption #4799

Conversation

nojnhuh commented May 2, 2024

Choose a reason for hiding this comment

codecov bot commented May 2, 2024 • edited Loading

Codecov Report

nojnhuh commented May 2, 2024 • edited Loading

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

jackfrancis commented May 3, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jackfrancis commented May 6, 2024

k8s-ci-robot commented May 6, 2024

k8s-ci-robot commented May 6, 2024

jackfrancis commented May 6, 2024

nojnhuh commented May 6, 2024

jackfrancis commented May 6, 2024

codecov bot commented May 2, 2024 •

edited

Loading

nojnhuh commented May 2, 2024 •

edited

Loading