🌱 Support MachinePools without MachinePoolMachines in clusterctl upgrade test #10498

willie-yao · 2024-04-23T18:30:41Z

What this PR does / why we need it:
This PR adds support for MachinePools without MachinePoolMachines in the clusterctl upgrade test. It does so by waiting for the correct number of ready replicas rather than counting Machines by listing them from the management cluster.

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

/area testing

willie-yao · 2024-04-23T18:32:08Z

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

k8s-ci-robot · 2024-04-23T18:32:11Z

@willie-yao: The specified target(s) for /test were not found.
The following commands are available to trigger required jobs:

/test pull-cluster-api-build-main
/test pull-cluster-api-e2e-blocking-main
/test pull-cluster-api-e2e-conformance-ci-latest-main
/test pull-cluster-api-e2e-conformance-main
/test pull-cluster-api-e2e-dualstack-and-ipv6-main
/test pull-cluster-api-e2e-main
/test pull-cluster-api-e2e-mink8s-main
/test pull-cluster-api-e2e-upgrade-1-30-1-31-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

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-blocking-main
pull-cluster-api-test-main
pull-cluster-api-verify-main

In response to this:

/test pull-cluster-api-e2e-full-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.

willie-yao · 2024-04-23T18:32:33Z

/test pull-cluster-api-e2e-main

chrischdi · 2024-04-24T10:31:31Z

test/e2e/clusterctl_upgrade.go

@@ -127,6 +128,8 @@ type ClusterctlUpgradeSpecInput struct {
 	// If not set, the test will upgrade once to the v1beta1 contract.
 	// For some examples see clusterctl_upgrade_test.go
 	Upgrades []ClusterctlUpgradeSpecInputUpgrade
+
+	SkipMachinePoolMachines bool


Can we have a godoc comment that this is used for providers which don't support machine pool machines (don't remember the exact wording)

Is there a way this could be detected automatically? Maybe with the MachinePool's status. infrastructureMachineKind like the MachinePool controller does?

cluster-api/exp/internal/controllers/machinepool_controller_phases.go

Lines 335 to 337 in 14f145e

if err := util.UnstructuredUnmarshalField(infraMachinePool, &infraMachineKind, "status", "infrastructureMachineKind"); err != nil {

if errors.Is(err, util.ErrUnstructuredFieldNotFound) {

log.V(4).Info("MachinePool Machines not supported, no infraMachineKind found")

Yup this makes sense. Made this change in 7960aff

test/e2e/clusterctl_upgrade.go

chrischdi · 2024-04-26T12:02:35Z

test/e2e/clusterctl_upgrade.go

+		expectedMachineCount := *controlPlaneMachineCount
+		if !skipMachinePoolMachines {
+			expectedMachineCount += expectedWorkerCount


Don't we have to diferentiate between machinepool workers and machinedeployment workers here?

Isn't this something like:

Suggested change

expectedMachineCount := *controlPlaneMachineCount

if !skipMachinePoolMachines {

expectedMachineCount += expectedWorkerCount

expectedMachineCount := *controlPlaneMachineCount + expectedMachineDeploymentWorkerCount

if !skipMachinePoolMachines {

expectedMachineCount += expectedMachinePoolWorkerCount

sbueringer · 2024-04-30T08:54:31Z

/test pull-cluster-api-e2e-main

(I"ll take a look once tests are green and Christian is happy :))

chrischdi

/lgtmz

chrischdi

/lgtm

k8s-ci-robot · 2024-04-30T09:16:56Z

LGTM label has been added.

Git tree hash: 9a6610523c71a46a04fbceb0e707095a8d0f187f

test/e2e/clusterctl_upgrade.go

willie-yao · 2024-05-01T21:41:51Z

/test pull-cluster-api-e2e-main

willie-yao · 2024-05-02T00:25:59Z

/retest

test/e2e/clusterctl_upgrade.go

willie-yao · 2024-05-03T17:09:33Z

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

k8s-ci-robot · 2024-05-03T17:09:36Z

@willie-yao: The specified target(s) for /test were not found.
The following commands are available to trigger required jobs:

/test pull-cluster-api-build-main
/test pull-cluster-api-e2e-blocking-main
/test pull-cluster-api-e2e-conformance-ci-latest-main
/test pull-cluster-api-e2e-conformance-main
/test pull-cluster-api-e2e-dualstack-and-ipv6-main
/test pull-cluster-api-e2e-main
/test pull-cluster-api-e2e-mink8s-main
/test pull-cluster-api-e2e-upgrade-1-30-1-31-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

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-blocking-main
pull-cluster-api-test-main
pull-cluster-api-verify-main

In response to this:

/test pull-cluster-api-e2e-full-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.

willie-yao · 2024-05-03T17:15:35Z

/test pull-cluster-api-e2e-main

test/e2e/clusterctl_upgrade.go

sbueringer · 2024-05-06T11:11:11Z

Thx, last nit from my side

/assign @chrischdi

…e test

sbueringer · 2024-05-07T08:56:05Z

Thx!

/lgtm

/assign @chrischdi

k8s-ci-robot · 2024-05-07T08:56:11Z

LGTM label has been added.

Git tree hash: e263e680085cf64fc696b325bf90d12c9f0c6bf2

chrischdi · 2024-05-07T13:36:24Z

/approve

k8s-ci-robot · 2024-05-07T13:36:32Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: chrischdi

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

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

willie-yao · 2024-05-09T23:06:10Z

Had to create another fix in #10584 due to an issue with the controlplane machine count previously being hard-coded to 1.

k8s-ci-robot added area/testing Issues or PRs related to testing cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Apr 23, 2024

k8s-ci-robot requested review from elmiko and JoelSpeed April 23, 2024 18:30

k8s-ci-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Apr 23, 2024

chrischdi reviewed Apr 24, 2024

View reviewed changes

chrischdi reviewed Apr 26, 2024

View reviewed changes

chrischdi reviewed Apr 30, 2024

View reviewed changes

k8s-ci-robot assigned chrischdi Apr 30, 2024

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

sbueringer reviewed Apr 30, 2024

View reviewed changes

k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Apr 30, 2024

k8s-ci-robot requested a review from chrischdi April 30, 2024 20:51

willie-yao force-pushed the mp-upgrade branch from 139f3b3 to b2634e3 Compare April 30, 2024 21:47

sbueringer reviewed May 2, 2024

View reviewed changes

willie-yao force-pushed the mp-upgrade branch 2 times, most recently from 3d05916 to cdbe9df Compare May 3, 2024 17:09

sbueringer reviewed May 6, 2024

View reviewed changes

test/e2e/clusterctl_upgrade.go Outdated Show resolved Hide resolved

Support MachinePools without MachinePoolMachines in clusterctl upgrad…

bd1fb5a

…e test

willie-yao force-pushed the mp-upgrade branch from cdbe9df to bd1fb5a Compare May 6, 2024 17:54

k8s-ci-robot assigned sbueringer May 7, 2024

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

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

k8s-ci-robot merged commit 6ac7085 into kubernetes-sigs:main May 7, 2024
19 of 20 checks passed

k8s-ci-robot added this to the v1.8 milestone May 7, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

🌱 Support MachinePools without MachinePoolMachines in clusterctl upgrade test #10498

🌱 Support MachinePools without MachinePoolMachines in clusterctl upgrade test #10498

willie-yao commented Apr 23, 2024

willie-yao commented Apr 23, 2024

k8s-ci-robot commented Apr 23, 2024

willie-yao commented Apr 23, 2024

chrischdi Apr 24, 2024

nojnhuh Apr 25, 2024

willie-yao Apr 25, 2024

chrischdi Apr 26, 2024

sbueringer commented Apr 30, 2024

chrischdi left a comment

chrischdi left a comment

k8s-ci-robot commented Apr 30, 2024

willie-yao commented May 1, 2024

willie-yao commented May 2, 2024

willie-yao commented May 3, 2024

k8s-ci-robot commented May 3, 2024

willie-yao commented May 3, 2024

sbueringer commented May 6, 2024

sbueringer commented May 7, 2024

k8s-ci-robot commented May 7, 2024

chrischdi commented May 7, 2024

k8s-ci-robot commented May 7, 2024

willie-yao commented May 9, 2024

	if err := util.UnstructuredUnmarshalField(infraMachinePool, &infraMachineKind, "status", "infrastructureMachineKind"); err != nil {
	if errors.Is(err, util.ErrUnstructuredFieldNotFound) {
	log.V(4).Info("MachinePool Machines not supported, no infraMachineKind found")

🌱 Support MachinePools without MachinePoolMachines in clusterctl upgrade test #10498

🌱 Support MachinePools without MachinePoolMachines in clusterctl upgrade test #10498

Conversation

willie-yao commented Apr 23, 2024

willie-yao commented Apr 23, 2024

k8s-ci-robot commented Apr 23, 2024

willie-yao commented Apr 23, 2024

chrischdi Apr 24, 2024

Choose a reason for hiding this comment

nojnhuh Apr 25, 2024

Choose a reason for hiding this comment

willie-yao Apr 25, 2024

Choose a reason for hiding this comment

chrischdi Apr 26, 2024

Choose a reason for hiding this comment

sbueringer commented Apr 30, 2024

chrischdi left a comment

Choose a reason for hiding this comment

chrischdi left a comment

Choose a reason for hiding this comment

k8s-ci-robot commented Apr 30, 2024

willie-yao commented May 1, 2024

willie-yao commented May 2, 2024

willie-yao commented May 3, 2024

k8s-ci-robot commented May 3, 2024

willie-yao commented May 3, 2024

sbueringer commented May 6, 2024

sbueringer commented May 7, 2024

k8s-ci-robot commented May 7, 2024

chrischdi commented May 7, 2024

k8s-ci-robot commented May 7, 2024

willie-yao commented May 9, 2024