✨ taint propagation: machine related API changes, conversion and feature gate #12936

chrischdi · 2025-11-03T13:47:55Z

What this PR does / why we need it:

Part of:

Taints Propagation Proposal implementation #12908

Implements:

Machines:
- Implement MachineSpec API changes (including validation)
- Implement Machine controller changes
- Implement unit-test coverage

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 #

Area example:
/area api
/area machine
/area machineset
/area machinedeployment

chrischdi · 2025-11-03T13:48:28Z

/test help

k8s-ci-robot · 2025-11-03T13:48:31Z

@chrischdi: 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-latestk8s-main

/test pull-cluster-api-e2e-main

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

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

/test pull-cluster-api-e2e-upgrade-1-34-1-35-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 help

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-sigs/prow repository.

chrischdi · 2025-11-03T13:48:53Z

/test pull-cluster-api-e2e-conformance-main
/test pull-cluster-api-e2e-latestk8s-main
/test pull-cluster-api-e2e-main-gke
/test pull-cluster-api-e2e-mink8s-main
/test pull-cluster-api-e2e-upgrade-1-34-1-35-main

chrischdi · 2025-11-03T14:01:22Z

/test pull-cluster-api-e2e-conformance-main
/test pull-cluster-api-e2e-latestk8s-main
/test pull-cluster-api-e2e-main-gke
/test pull-cluster-api-e2e-mink8s-main
/test pull-cluster-api-e2e-upgrade-1-34-1-35-main

chrischdi · 2025-11-03T15:40:45Z

/test pull-cluster-api-e2e-conformance-main
/test pull-cluster-api-e2e-latestk8s-main
/test pull-cluster-api-e2e-main-gke
/test pull-cluster-api-e2e-mink8s-main
/test pull-cluster-api-e2e-upgrade-1-34-1-35-main

chrischdi · 2025-11-04T09:07:10Z

Rebased to resolve merge conflict (was on conversion due to #12940 )

chrischdi · 2025-11-04T09:07:17Z

/test pull-cluster-api-e2e-conformance-main
/test pull-cluster-api-e2e-latestk8s-main
/test pull-cluster-api-e2e-main-gke
/test pull-cluster-api-e2e-mink8s-main
/test pull-cluster-api-e2e-upgrade-1-34-1-35-main

chrischdi · 2025-11-05T14:31:17Z

cc @fabriziopandini @sbueringer

api/core/v1beta1/conversion.go

api/core/v1beta2/common_types.go

controlplane/kubeadm/internal/controllers/inplace_canupdatemachine_test.go

internal/webhooks/machine_test.go

internal/webhooks/machinepool.go

internal/controllers/machine/machine_controller_noderef.go

api/core/v1beta2/common_types.go

JoelSpeed · 2025-11-07T18:02:32Z

api/core/v1beta2/machine_types.go

+	// Only those taints defined in this list will be added or removed by core Cluster API controllers.
+	//
+	// NOTE: This list is implemented as a "map" type, meaning that individual elements can be managed by different owners.
+	// As of Kubernetes 1.33, this is different from the implementation on corev1.NodeSpec, but provides a more flexible API for components building on top of Cluster API.


Why 1.33 in this message? 1.34 also suffers the same issue

Is this difference really something that anyone cares about and do we have to document it?

Cluster API supports co-ownership of this field and I don't see why we shouldn't be able to just do that even if the same is not possible on the corresponding Node field.

Removed the second part of the note to simplify.

JoelSpeed · 2025-11-07T18:03:02Z

api/core/v1beta2/machine_types.go

+	// +listMapKey=key
+	// +listMapKey=effect
+	// +kubebuilder:validation:MinItems=1
+	// +kubebuilder:validation:MaxItems=64


Should explain the upper bound of this list length in the godoc

Added:

// There can be at most 64 taints. // A pod would have to tolerate all existing taints to run on the corresponding node.

Prior discussion: #12329 (comment)

fabriziopandini

Nice, just a few nits from my side

api/core/v1beta2/machine_types.go

api/core/v1beta1/conversion.go

internal/controllers/machinedeployment/machinedeployment_rollout_planner.go

internal/controllers/machinedeployment/machinedeployment_controller_test.go

internal/webhooks/machinepool.go

internal/webhooks/test/machine_test.go

internal/controllers/machine/machine_controller_noderef_test.go

sbueringer · 2025-11-10T18:20:03Z

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

internal/controllers/machine/machine_controller_noderef_test.go

sbueringer · 2025-11-11T09:16:22Z

One quick follow-up comment. Otherwise delta looks good. I'll do another full review a bit later in the week and give others some time to review as well in the meantime.
(but I wouldn't expect much/any new findings, was just a bit messy to review through the rebase before)

fabriziopandini · 2025-11-11T17:09:17Z

Great work!
/lgtm

k8s-ci-robot · 2025-11-11T17:09:25Z

LGTM label has been added.

Git tree hash: 80b9a154866295ca494ed3c31948c9bb35465975

internal/controllers/machine/machine_controller_noderef.go

chrischdi · 2025-11-12T12:49:07Z

/retest

Unrelated flake I think (MHC)

sbueringer · 2025-11-12T13:25:23Z

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

sbueringer · 2025-11-12T13:32:55Z

Thank you very much!!

/lgtm
/approve

k8s-ci-robot · 2025-11-12T13:33:03Z

LGTM label has been added.

Git tree hash: f6b9a804180234b4f4b6ebd9345d4b64a1ad37b5

k8s-ci-robot · 2025-11-12T13:33:08Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: sbueringer

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

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

sbueringer · 2025-11-12T16:44:07Z

/override pull-cluster-api-e2e-main-gke

k8s-ci-robot · 2025-11-12T16:44:11Z

@sbueringer: Overrode contexts on behalf of sbueringer: pull-cluster-api-e2e-main-gke

In response to this:

/override pull-cluster-api-e2e-main-gke

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-sigs/prow repository.

k8s-ci-robot added area/api Issues or PRs related to the APIs area/machine Issues or PRs related to machine lifecycle management area/machineset Issues or PRs related to machinesets area/machinedeployment Issues or PRs related to machinedeployments labels Nov 3, 2025

k8s-ci-robot requested review from g-gaston and sivchari November 3, 2025 13:48

k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Nov 3, 2025

chrischdi mentioned this pull request Nov 3, 2025

Taints Propagation Proposal implementation #12908

Open

20 tasks

chrischdi force-pushed the pr-implement-taint-propagation branch from 6a66d3b to 00f4464 Compare November 3, 2025 14:01

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

chrischdi force-pushed the pr-implement-taint-propagation branch from 9f68423 to aaecaf2 Compare November 4, 2025 09:06

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

chrischdi added the tide/merge-method-squash Denotes a PR that should be squashed by tide when it merges. label Nov 5, 2025

chrischdi force-pushed the pr-implement-taint-propagation branch from aaecaf2 to 6b2c7e2 Compare November 6, 2025 16:20

chrischdi mentioned this pull request Nov 7, 2025

🌱 taint propagation: e2e coverage via md rollout test #12966

Open

2 tasks

sbueringer reviewed Nov 7, 2025

View reviewed changes

JoelSpeed reviewed Nov 7, 2025

View reviewed changes

fabriziopandini reviewed Nov 8, 2025

View reviewed changes

api/core/v1beta2/machine_types.go Outdated Show resolved Hide resolved

api/core/v1beta1/conversion.go Outdated Show resolved Hide resolved

sbueringer reviewed Nov 10, 2025

View reviewed changes

internal/controllers/machinedeployment/machinedeployment_rollout_planner.go Show resolved Hide resolved

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

chrischdi force-pushed the pr-implement-taint-propagation branch from 2b066cb to 56b4da3 Compare November 10, 2025 12:41

Update unit tests

de843f8

chrischdi force-pushed the pr-implement-taint-propagation branch from 63fd146 to de843f8 Compare November 10, 2025 16:49

fix tests after rebase

5f79290

sbueringer reviewed Nov 10, 2025

View reviewed changes

Review fixes v4

7f76609

sbueringer reviewed Nov 11, 2025

View reviewed changes

internal/controllers/machine/machine_controller_noderef_test.go Outdated Show resolved Hide resolved

Review fixes v5

9fa37ab

k8s-ci-robot assigned fabriziopandini Nov 11, 2025

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Nov 11, 2025

sbueringer reviewed Nov 12, 2025

View reviewed changes

internal/controllers/machine/machine_controller_noderef.go Outdated Show resolved Hide resolved

chrischdi added 2 commits November 12, 2025 13:28

Review fixes

71e76c2

fixup godoc and make generate

0a902b3

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

k8s-ci-robot requested a review from fabriziopandini November 12, 2025 12:31

k8s-ci-robot assigned sbueringer Nov 12, 2025

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Nov 12, 2025

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Nov 12, 2025

k8s-ci-robot merged commit 1c66d11 into kubernetes-sigs:main Nov 12, 2025
20 checks passed

k8s-ci-robot added this to the v1.12 milestone Nov 12, 2025

✨ taint propagation: machine related API changes, conversion and feature gate #12936

✨ taint propagation: machine related API changes, conversion and feature gate #12936

Conversation

chrischdi commented Nov 3, 2025

Uh oh!

chrischdi commented Nov 3, 2025

Uh oh!

k8s-ci-robot commented Nov 3, 2025

Uh oh!

chrischdi commented Nov 3, 2025

Uh oh!

chrischdi commented Nov 3, 2025

Uh oh!

chrischdi commented Nov 3, 2025

Uh oh!

chrischdi commented Nov 4, 2025

Uh oh!

chrischdi commented Nov 4, 2025

Uh oh!

chrischdi commented Nov 5, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

JoelSpeed Nov 7, 2025

Choose a reason for hiding this comment

Uh oh!

sbueringer Nov 8, 2025 • 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!

chrischdi Nov 10, 2025

Choose a reason for hiding this comment

Uh oh!

JoelSpeed Nov 7, 2025

Choose a reason for hiding this comment

Uh oh!

chrischdi Nov 10, 2025

Choose a reason for hiding this comment

Uh oh!

fabriziopandini left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

sbueringer commented Nov 10, 2025

Uh oh!

Uh oh!

sbueringer commented Nov 11, 2025

Uh oh!

fabriziopandini commented Nov 11, 2025

Uh oh!

k8s-ci-robot commented Nov 11, 2025

Uh oh!

Uh oh!

chrischdi commented Nov 12, 2025

Uh oh!

sbueringer commented Nov 12, 2025

Uh oh!

sbueringer commented Nov 12, 2025

Uh oh!

k8s-ci-robot commented Nov 12, 2025

Uh oh!

k8s-ci-robot commented Nov 12, 2025

sbueringer Nov 8, 2025 •

edited

Loading