Implement control plane template #88

anmazzotti · 2024-01-04T12:22:54Z

This should be part of #62
The main focus of this PR is to enable Cluster Classes.

The KThreesControlPlaneTemplateSpec was introduced for this very reason.
Changes to the existing KThreesControlPlaneMachineTemplate are also required for the correct reconciliation of the Cluster topology.

Still in draft as I'm testing if everything still works.
I am not sure whether this will introduce backward compatibility, probably yes since the API changed, however I am not sure how else this could be implemented differently.

richardcase · 2024-01-04T12:36:10Z

As you mention this change really needs to be done as part of a API version bump to v1beta2 as the change is API breaking.

richardcase · 2024-01-04T12:42:36Z

@zawachte - i'll start work on a PR to bump the API version so that we can start work on v1beta1 (will also create the 0.1 release branch)

anmazzotti · 2024-01-11T09:36:58Z

PR is ready for review.
I did implement the new v1beta2 version, added conversion and the webhooks (for conversion, validation is empty).

I bumped the GOLANGCI_LINT_VER to solve an issue with the depguard linter I'm having. Didn't really help but the bump should not harm.
Same for CONTROLLER_GEN_VER.

Also note that I tweaked the generated manifests. For example when generating the bootstrap manifests, we now only include the bootstrap subfolder: paths=./bootstrap/...
The reason for this is to avoid generating overlapping manifests. This is fine in the current version, but it was conflicting with the creation of the webhooks (generating the same webhook for the same resource on both controllers).

I also tested this patch locally by:

Provisioning a v1beta1 k3s cluster from this cluster template and the resources are correctly converted to v1beta2 as expected.
Provisioning a v1beta2 k3s cluster from a cluster class and it works as expected thanks to the addition of the KThreesControlPlaneTemplate as required by CAPI.

richardcase · 2024-01-11T11:06:14Z

Thanks @anmazzotti . I will start reviewing.

richardcase

Thanks for this @anmazzotti . A few minor comments, nothing major. Let me know if i need to clarify anything.

It would have been easier for reviewers if the introduction of the v1beta2 api version was in a separate PR to the introduction of the control plane template.

Also, before we merge will you be able to squash your commits so they tell a store. Perhaps into 2 commits: 1 for the API version bump and 1 for the control plane template.

bootstrap/api/v1beta1/conversion.go

richardcase · 2024-01-11T11:56:48Z

bootstrap/api/v1beta2/condition_consts.go

@@ -0,0 +1,71 @@
+/*
+Copyright 2020 The Kubernetes Authors.


nit: as its a new file maybe 2024 would be better?

bootstrap/api/v1beta2/conversion.go

richardcase · 2024-01-11T12:00:36Z

bootstrap/api/v1beta2/kthreesconfigtemplate_types.go

+
+// KThreesConfigTemplateSpec defines the desired state of KThreesConfigTemplate.
+type KThreesConfigTemplateSpec struct {
+	// INSERT ADDITIONAL SPEC FIELDS - desired state of cluster


We can probably remove this autogenerated comment

controlplane/api/v1beta1/conversion.go

richardcase · 2024-01-11T12:10:11Z

controlplane/api/v1beta1/conversion_test.go

+	g.Expect(AddToScheme(scheme)).To(Succeed())
+	g.Expect(cabp3v1.AddToScheme(scheme)).To(Succeed())
+
+	t.Run("for KThreesConfig", utilconversion.FuzzTestFunc(utilconversion.FuzzTestFuncInput{


nit:

Suggested change

t.Run("for KThreesConfig", utilconversion.FuzzTestFunc(utilconversion.FuzzTestFuncInput{

t.Run("for KThreesControlPlane", utilconversion.FuzzTestFunc(utilconversion.FuzzTestFuncInput{

richardcase · 2024-01-11T12:10:37Z

controlplane/api/v1beta2/condition_consts.go

@@ -0,0 +1,130 @@
+/*
+Copyright 2020 The Kubernetes Authors.


Suggested change

Copyright 2020 The Kubernetes Authors.

Copyright 2024 The Kubernetes Authors.

controlplane/api/v1beta2/conversion.go

anmazzotti · 2024-01-11T13:25:03Z

It would have been easier for reviewers if the introduction of the v1beta2 api version was in a separate PR to the introduction of the control plane template.

I did not see this PR coming, sorry for that and thank you for the review.
I'll also squash as you recommended.

zawachte · 2024-02-03T14:59:29Z

@anmazzotti what is the status here? Are you waiting for my review?

anmazzotti · 2024-02-05T08:54:02Z

@anmazzotti what is the status here? Are you waiting for my review?

Affirmative.

anmazzotti · 2024-02-05T08:56:04Z

Also please do not release after this is merged. I have a follow up PR to remove the DisableExternalCloudProvider flag I introduced earlier. I found out it doesn't make sense, so I'll remove in the new version v1beta2

richardcase

This is looking good @anmazzotti. Great work.

Just a couple of things:

Could you confirm what manual testing you have done with the new control plane template?
Could you squash the commits so they tell a story?

anmazzotti · 2024-02-15T14:40:44Z

This is looking good @anmazzotti. Great work.

Just a couple of things:
* Could you confirm what manual testing you have done with the new control plane template?

* Could you squash the commits so they tell a story?

Thank you.
Yes I did manual test using the Elemental provider and it works.
I will also start working on deprecating the DisableExternalCloudProvider flag as soon as this is merged and I will do a full round of testing for that as well, so we are confident before the next release.

I'll squash two commits to highlight the new version being added and the KThreesControlPlaneTemplate addition too. I think those two are the most important features for this PR, rest is scaffolding.

richardcase · 2024-02-15T14:46:06Z

I'll squash two commits to highlight the new version being added and the KThreesControlPlaneTemplate addition too. I think those two are the most important features for this PR, rest is scaffolding.

Sounds good to me 👍

mogliang · 2024-02-22T01:51:33Z

I see there are thousands of new files in vendor folder, do we need check in this folder?

mogliang

LGTM

mogliang · 2024-05-08T03:05:34Z

@anmazzotti would you please resolve conflicts?

richardcase

@anmazzotti - would you be able to squash the commits so they tell a story?

Apart from squashing the commits i think this is good to go from my side.

bootstrap/api/v1beta2/kthreesconfig_types.go

Signed-off-by: Andrea Mazzotti <[email protected]>

mogliang · 2024-05-09T01:11:48Z

@anmazzotti would you mind adding a e2e test case to cover clusterclass?

anmazzotti marked this pull request as ready for review January 11, 2024 09:37

richardcase requested review from richardcase and zawachte January 11, 2024 11:05

richardcase reviewed Jan 11, 2024

View reviewed changes

anmazzotti force-pushed the implement-control-plane-template branch from 29d8ae4 to 9badbb7 Compare February 8, 2024 12:21

richardcase reviewed Feb 15, 2024

View reviewed changes

anmazzotti force-pushed the implement-control-plane-template branch from 9badbb7 to 4fe6c8a Compare February 16, 2024 08:56

anmazzotti force-pushed the implement-control-plane-template branch from 4fe6c8a to b2841f1 Compare March 27, 2024 08:47

mogliang requested a review from richardcase April 8, 2024 05:43

anmazzotti force-pushed the implement-control-plane-template branch from b74ef2d to 5b87294 Compare April 16, 2024 14:10

mogliang previously approved these changes May 8, 2024

View reviewed changes

anmazzotti dismissed mogliang’s stale review via 979c577 May 8, 2024 07:30

anmazzotti force-pushed the implement-control-plane-template branch from 5b87294 to 979c577 Compare May 8, 2024 07:30

This was referenced May 8, 2024

Expose --disable-cloud-provider feature and cloud-provider= kubelet arg #106

Closed

Remove deprecated WaitingForControlPlaneAvailableReason in v1beta2 #107

Open

richardcase reviewed May 8, 2024

View reviewed changes

bootstrap/api/v1beta2/kthreesconfig_types.go Outdated Show resolved Hide resolved

bootstrap/api/v1beta2/kthreesconfig_types.go Show resolved Hide resolved

bootstrap/api/v1beta2/kthreesconfig_types.go Show resolved Hide resolved

Add v1beta2 version and implement KThreesControlPlaneTemplate

1e126bb

Signed-off-by: Andrea Mazzotti <[email protected]>

anmazzotti force-pushed the implement-control-plane-template branch from 1db1973 to 1e126bb Compare May 8, 2024 09:21

richardcase approved these changes May 8, 2024

View reviewed changes

mogliang merged commit 7d52020 into k3s-io:main May 9, 2024
5 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement control plane template #88

Implement control plane template #88

anmazzotti commented Jan 4, 2024

richardcase commented Jan 4, 2024

richardcase commented Jan 4, 2024

anmazzotti commented Jan 11, 2024

richardcase commented Jan 11, 2024

richardcase left a comment

richardcase Jan 11, 2024

richardcase Jan 11, 2024

richardcase Jan 11, 2024

richardcase Jan 11, 2024

anmazzotti commented Jan 11, 2024

zawachte commented Feb 3, 2024

anmazzotti commented Feb 5, 2024

anmazzotti commented Feb 5, 2024

richardcase left a comment

anmazzotti commented Feb 15, 2024

richardcase commented Feb 15, 2024

mogliang commented Feb 22, 2024

mogliang left a comment

mogliang commented May 8, 2024

richardcase left a comment

mogliang commented May 9, 2024

	t.Run("for KThreesConfig", utilconversion.FuzzTestFunc(utilconversion.FuzzTestFuncInput{
	t.Run("for KThreesControlPlane", utilconversion.FuzzTestFunc(utilconversion.FuzzTestFuncInput{

	Copyright 2020 The Kubernetes Authors.
	Copyright 2024 The Kubernetes Authors.

Implement control plane template #88

Implement control plane template #88

Conversation

anmazzotti commented Jan 4, 2024

richardcase commented Jan 4, 2024

richardcase commented Jan 4, 2024

anmazzotti commented Jan 11, 2024

richardcase commented Jan 11, 2024

richardcase left a comment

Choose a reason for hiding this comment

richardcase Jan 11, 2024

Choose a reason for hiding this comment

richardcase Jan 11, 2024

Choose a reason for hiding this comment

richardcase Jan 11, 2024

Choose a reason for hiding this comment

richardcase Jan 11, 2024

Choose a reason for hiding this comment

anmazzotti commented Jan 11, 2024

zawachte commented Feb 3, 2024

anmazzotti commented Feb 5, 2024

anmazzotti commented Feb 5, 2024

richardcase left a comment

Choose a reason for hiding this comment

anmazzotti commented Feb 15, 2024

richardcase commented Feb 15, 2024

mogliang commented Feb 22, 2024

mogliang left a comment

Choose a reason for hiding this comment

mogliang commented May 8, 2024

richardcase left a comment

Choose a reason for hiding this comment

mogliang commented May 9, 2024