⚠️ Remove redundant/unused KCPTemplate fields by sbueringer · Pull Request #5788 · kubernetes-sigs/cluster-api

sbueringer · 2021-12-03T16:55:16Z

Signed-off-by: Stefan Büringer buringerst@vmware.com

What this PR does / why we need it:

This PR proposes a solution to implement: #5678

Please see #5788 (comment) for the options we have and further discussions

Note:

Follow-up if this PR is merged: drop redundant fields in CAPA quickstart e2e test.

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

controlplane/kubeadm/api/v1beta1/kubeadmcontrolplanetemplate_types.go

sbueringer · 2021-12-06T16:02:47Z

/test pull-cluster-api-e2e-full-main

sbueringer · 2021-12-06T18:05:51Z

/test pull-cluster-api-e2e-full-main

sbueringer · 2021-12-06T18:08:27Z

/test pull-cluster-api-e2e-full-main

vincepri · 2021-12-06T21:06:46Z

/retitle ⚠️ Remove redundant/unused KCPTemplate fields

vincepri · 2021-12-06T21:06:54Z

/lgtm
/assign @CecileRobertMichon

controlplane/kubeadm/api/v1beta1/kubeadmcontrolplanetemplate_types.go

sbueringer · 2021-12-08T20:29:03Z

/test pull-cluster-api-e2e-full-main

sbueringer · 2021-12-08T20:30:34Z

controlplane/kubeadm/api/v1beta1/kubeadmcontrolplanetemplate_types.go

+	// MachineTemplate contains information about how machines
+	// should be shaped when creating or updating a control plane.
+	// +optional
+	MachineTemplate *KubeadmControlPlaneTemplateMachineTemplate `json:"machineTemplate,omitempty"`


@vincepri @CecileRobertMichon

I did another manual test. This time I tried not setting machineTemplate at all, so I noticed that machineTemplate was still mandatory. (the CI test sets machineTemplate.nodeDrainTimeout to speed up the test)

I think we shouldn't force users to specify an empty machineTemplate object, so I made it optional

Signed-off-by: Stefan Büringer buringerst@vmware.com

sbueringer · 2021-12-08T20:33:49Z

/test pull-cluster-api-e2e-full-main

sbueringer · 2021-12-08T21:03:13Z

/test pull-cluster-api-e2e-full-main
unrelated flake

fabriziopandini

ready to lgtm approve as soon as we get a green CI

fabriziopandini · 2021-12-08T20:59:55Z

controlplane/kubeadm/api/v1alpha4/conversion.go

+	apiconversion "k8s.io/apimachinery/pkg/conversion"
 	"sigs.k8s.io/controller-runtime/pkg/conversion"

+	kubeadmbootstrapv1alpha4 "sigs.k8s.io/cluster-api/bootstrap/kubeadm/api/v1alpha4"


nit we are kind of standardizing on boostrapvx, but we can eventually tackle this is a separated PR

Suggested change

kubeadmbootstrapv1alpha4 "sigs.k8s.io/cluster-api/bootstrap/kubeadm/api/v1alpha4"

bootstrapv1alpha4 "sigs.k8s.io/cluster-api/bootstrap/kubeadm/api/v1alpha4"

Yup. I'll take a look tomorrow (in a separate PR)

sbueringer · 2021-12-09T09:18:08Z

@fabriziopandini Should be ready for merge now

fabriziopandini · 2021-12-09T13:11:30Z

/hold cancel
/lgtm
/approve

k8s-ci-robot · 2021-12-09T13:11:46Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: CecileRobertMichon, fabriziopandini

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details

Needs approval from an approver in each of these files:

~~controlplane/kubeadm/OWNERS~~ [CecileRobertMichon,fabriziopandini]
~~test/OWNERS~~ [CecileRobertMichon,fabriziopandini]

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 · 2021-12-09T13:16:08Z

@sbueringer: 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-apidiff-main	`0f7e22d`	link	false	`/test pull-cluster-api-apidiff-main`

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.

Details

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.

sbueringer · 2021-12-09T13:16:28Z

flake
/test
pull-cluster-api-test-mink8s-main

k8s-ci-robot · 2021-12-09T13:16:29Z

@sbueringer: The /test command needs one or more targets.
The following commands are available to trigger required jobs:

/test pull-cluster-api-build-main
/test pull-cluster-api-e2e-main
/test pull-cluster-api-test-main
/test pull-cluster-api-test-mink8s-main
/test pull-cluster-api-verify-main

The following commands are available to trigger optional jobs:

/test pull-cluster-api-apidiff-main
/test pull-cluster-api-e2e-full-main
/test pull-cluster-api-e2e-informing-ipv6-main
/test pull-cluster-api-e2e-informing-main
/test pull-cluster-api-e2e-workload-upgrade-1-23-latest-main
/test pull-cluster-api-make-main

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

pull-cluster-api-apidiff-main
pull-cluster-api-build-main
pull-cluster-api-e2e-informing-ipv6-main
pull-cluster-api-e2e-informing-main
pull-cluster-api-e2e-main
pull-cluster-api-test-main
pull-cluster-api-test-mink8s-main
pull-cluster-api-verify-main

Details

In response to this:

flake
/test
pull-cluster-api-test-mink8s-main

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.

sbueringer · 2021-12-09T13:16:38Z

/test pull-cluster-api-test-mink8s-main

…redundant-kcp-fields ⚠️ Remove redundant/unused KCPTemplate fields

…rClass machineTemplate in KubeadmControlPlaneTemplate is removed with CAPI v1.1.0 See kubernetes-sigs/cluster-api#5788 We need to move that part to clusterClass

…rClass (#46) * Move machineInfrastructure from KubeadmControlPlaneTemplate to ClusterClass machineTemplate in KubeadmControlPlaneTemplate is removed with CAPI v1.1.0 See kubernetes-sigs/cluster-api#5788 We need to move that part to clusterClass. Co-authored-by: Thomas Fussell <thomas@giantswarm.io>

k8s-ci-robot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Dec 3, 2021

k8s-ci-robot requested review from JoelSpeed and stmcginnis December 3, 2021 16:55

sbueringer commented Dec 3, 2021

View reviewed changes

controlplane/kubeadm/api/v1beta1/kubeadmcontrolplanetemplate_types.go Show resolved Hide resolved

sbueringer force-pushed the pr-deprecate-redundant-kcp-fields branch from e42292d to 6d08be0 Compare December 6, 2021 14:58

k8s-ci-robot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Dec 6, 2021

sbueringer force-pushed the pr-deprecate-redundant-kcp-fields branch 3 times, most recently from d6fcea8 to 3e15512 Compare December 6, 2021 15:55

sbueringer changed the title ~~[WIP] ⚠️ Deprecate redundant KCPTemplate fields~~ ⚠️ Deprecate redundant KCPTemplate fields Dec 6, 2021

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 Dec 6, 2021

k8s-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Dec 6, 2021

sbueringer force-pushed the pr-deprecate-redundant-kcp-fields branch from 5ceb9c8 to d27574c Compare December 6, 2021 18:08

k8s-ci-robot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Dec 6, 2021

sbueringer mentioned this pull request Dec 6, 2021

🌱 CAPD: add ClusterClass cluster-template #5805

Merged

k8s-ci-robot changed the title ~~⚠️ Deprecate redundant KCPTemplate fields~~ ⚠️ Remove redundant/unused KCPTemplate fields Dec 6, 2021

k8s-ci-robot assigned CecileRobertMichon and vincepri Dec 6, 2021

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

CecileRobertMichon reviewed Dec 6, 2021

View reviewed changes

controlplane/kubeadm/api/v1beta1/kubeadmcontrolplanetemplate_types.go Outdated Show resolved Hide resolved

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

sbueringer force-pushed the pr-deprecate-redundant-kcp-fields branch from 40122c1 to b9982ca Compare December 8, 2021 20:28

sbueringer commented Dec 8, 2021

View reviewed changes

KCP: remove redundant and unused KubeadmControlPlaneTemplate fields

0f7e22d

Signed-off-by: Stefan Büringer buringerst@vmware.com

sbueringer force-pushed the pr-deprecate-redundant-kcp-fields branch from b9982ca to 0f7e22d Compare December 8, 2021 20:33

fabriziopandini reviewed Dec 8, 2021

View reviewed changes

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Dec 8, 2021

sbueringer mentioned this pull request Dec 9, 2021

🌱 Standardize api import aliases #5830

Merged

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

k8s-ci-robot assigned fabriziopandini Dec 9, 2021

k8s-ci-robot merged commit e889246 into kubernetes-sigs:main Dec 9, 2021

k8s-ci-robot added this to the v1.1 milestone Dec 9, 2021

sbueringer deleted the pr-deprecate-redundant-kcp-fields branch December 9, 2021 13:28

sbueringer pushed a commit to sbueringer/cluster-api that referenced this pull request Dec 9, 2021

Merge pull request kubernetes-sigs#5788 from sbueringer/pr-deprecate-…

64264bc

…redundant-kcp-fields ⚠️ Remove redundant/unused KCPTemplate fields

sbueringer pushed a commit to sbueringer/cluster-api that referenced this pull request Dec 9, 2021

Merge pull request kubernetes-sigs#5788 from sbueringer/pr-deprecate-…

d411869

…redundant-kcp-fields ⚠️ Remove redundant/unused KCPTemplate fields

erkanerol mentioned this pull request Feb 7, 2022

Move machineInfrastructure from KubeadmControlPlaneTemplate to ClusterClass giantswarm/cluster-openstack#46

Merged

	kubeadmbootstrapv1alpha4 "sigs.k8s.io/cluster-api/bootstrap/kubeadm/api/v1alpha4"
	bootstrapv1alpha4 "sigs.k8s.io/cluster-api/bootstrap/kubeadm/api/v1alpha4"

Conversation

sbueringer commented Dec 3, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

sbueringer commented Dec 6, 2021

Uh oh!

sbueringer commented Dec 6, 2021

Uh oh!

sbueringer commented Dec 6, 2021

Uh oh!

vincepri commented Dec 6, 2021

Uh oh!

vincepri commented Dec 6, 2021

Uh oh!

Uh oh!

sbueringer commented Dec 8, 2021

Uh oh!

sbueringer Dec 8, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

CecileRobertMichon Dec 8, 2021

Choose a reason for hiding this comment

Uh oh!

sbueringer commented Dec 8, 2021

Uh oh!

sbueringer commented Dec 8, 2021

Uh oh!

fabriziopandini left a comment

Choose a reason for hiding this comment

Uh oh!

fabriziopandini Dec 8, 2021

Choose a reason for hiding this comment

Uh oh!

sbueringer Dec 8, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sbueringer commented Dec 9, 2021

Uh oh!

fabriziopandini commented Dec 9, 2021

Uh oh!

k8s-ci-robot commented Dec 9, 2021

Uh oh!

k8s-ci-robot commented Dec 9, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sbueringer commented Dec 9, 2021

Uh oh!

k8s-ci-robot commented Dec 9, 2021

Uh oh!

sbueringer commented Dec 9, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

sbueringer commented Dec 3, 2021 •

edited

Loading

sbueringer Dec 8, 2021 •

edited

Loading

sbueringer Dec 8, 2021 •

edited

Loading

k8s-ci-robot commented Dec 9, 2021 •

edited

Loading