From ffc945d7e50c11c98d271e4de29e28c81fd782bc Mon Sep 17 00:00:00 2001 From: Christian Schlotter Date: Fri, 24 Oct 2025 13:55:49 +0200 Subject: [PATCH 01/21] taint propagation: machine related API changes, conversion and feature gate --- api/core/v1beta1/conversion.go | 25 +++++++++++- api/core/v1beta2/common_types.go | 49 ++++++++++++++++++++++++ api/core/v1beta2/machine_types.go | 14 +++++++ config/manager/manager.yaml | 2 +- feature/feature.go | 7 ++++ internal/api/core/v1alpha3/conversion.go | 4 ++ internal/api/core/v1alpha4/conversion.go | 4 ++ 7 files changed, 103 insertions(+), 2 deletions(-) diff --git a/api/core/v1beta1/conversion.go b/api/core/v1beta1/conversion.go index 0bc356ce993c..c1007274361e 100644 --- a/api/core/v1beta1/conversion.go +++ b/api/core/v1beta1/conversion.go @@ -412,6 +412,7 @@ func (src *Machine) ConvertTo(dstRaw conversion.Hub) error { // won't be able to write the Phase field. But that's okay as the only client writing the Phase // field should be the Machine controller. dst.Status.Phase = restored.Status.Phase + dst.Spec.Taints = restored.Spec.Taints } return nil @@ -450,6 +451,17 @@ func (src *MachineSet) ConvertTo(dstRaw conversion.Hub) error { dst.Spec.Template.Spec.MinReadySeconds = &src.Spec.MinReadySeconds } + restored := &clusterv1.MachineSet{} + ok, err := utilconversion.UnmarshalData(src, restored) + if err != nil { + return err + } + + // Recover other values + if ok { + dst.Spec.Template.Spec.Taints = restored.Spec.Template.Spec.Taints + } + return nil } @@ -467,7 +479,8 @@ func (dst *MachineSet) ConvertFrom(srcRaw conversion.Hub) error { dst.Spec.MinReadySeconds = ptr.Deref(src.Spec.Template.Spec.MinReadySeconds, 0) dropEmptyStringsMachineSpec(&dst.Spec.Template.Spec) - return nil + + return utilconversion.MarshalData(src, dst) } func (src *MachineDeployment) ConvertTo(dstRaw conversion.Hub) error { @@ -492,6 +505,11 @@ func (src *MachineDeployment) ConvertTo(dstRaw conversion.Hub) error { // Recover intent for bool values converted to *bool. clusterv1.Convert_bool_To_Pointer_bool(src.Spec.Paused, ok, restored.Spec.Paused, &dst.Spec.Paused) + // Recover other values + if ok { + dst.Spec.Template.Spec.Taints = restored.Spec.Template.Spec.Taints + } + return nil } @@ -578,6 +596,11 @@ func (src *MachinePool) ConvertTo(dstRaw conversion.Hub) error { dst.Status.Initialization = initialization } + // Recover other values + if ok { + dst.Spec.Template.Spec.Taints = restored.Spec.Template.Spec.Taints + } + return nil } diff --git a/api/core/v1beta2/common_types.go b/api/core/v1beta2/common_types.go index 26ff7d15084a..d4fa3cc01b62 100644 --- a/api/core/v1beta2/common_types.go +++ b/api/core/v1beta2/common_types.go @@ -99,6 +99,9 @@ const ( // AnnotationsFromMachineAnnotation is the annotation set on nodes to track the annotations that originated from machines. AnnotationsFromMachineAnnotation = "cluster.x-k8s.io/annotations-from-machine" + // TaintsFromMachineAnnotation is the annotation set on nodes to track the taints that originated from machines. + TaintsFromMachineAnnotation = "cluster.x-k8s.io/taints-from-machine" + // OwnerNameAnnotation is the annotation set on nodes identifying the owner name. OwnerNameAnnotation = "cluster.x-k8s.io/owner-name" @@ -405,3 +408,49 @@ func (r *ContractVersionedObjectReference) GroupKind() schema.GroupKind { Kind: r.Kind, } } + +// MachineTaint defines a taint equivalent to corev1.Taint, but additionally having a propagation field. +type MachineTaint struct { + // key is the taint key to be applied to a node. + // +required + // +kubebuilder:validation:MinLength=1 + // +kubebuilder:validation:MaxLength=253 + Key string `json:"key,omitempty"` + + // value is the taint value corresponding to the taint key. + // +optional + // +kubebuilder:validation:MaxLength=63 + Value *string `json:"value,omitempty"` + + // effect is the effect for the taint. Valid values are NoSchedule, PreferNoSchedule and NoExecute. + // +required + // +kubebuilder:validation:Enum=NoSchedule;PreferNoSchedule;NoExecute + Effect corev1.TaintEffect `json:"effect,omitempty"` + + // propagation defines how this taint should be propagated to nodes. + // Always: The taint will be continuously reconciled. If it is not set for a node, it will be added during reconciliation. + // OnInitialization: The taint will be added during node initialization. If it gets removed from the node later on it will not get added again. + // +required + Propagation MachineTaintPropagation `json:"propagation,omitempty"` +} + +// MachineTaintPropagation defines when a taint should be propagated to nodes. +// +kubebuilder:validation:Enum=Always;OnInitialization +type MachineTaintPropagation string + +const ( + // TaintPropagationAlways means the taint should be continuously reconciled and kept on the node. + // - If an Always taint is added to the Machine, the taint will be added to the node. + // - If an Always taint is removed from the Machine, the taint will be removed from the node. + // - If an OnInitialization taint is changed to Always, the Machine controller will ensure the taint is set on the node. + // - If an Always taint is removed from the node, it will be re-added during reconciliation. + TaintPropagationAlways MachineTaintPropagation = "Always" + + // TaintPropagationOnInitialization means the taint should be set once during initialization and then + // left alone. + // - If an OnInitialization taint is added to the Machine, the taint will only be added to the node on initialization. + // - If an OnInitialization taint is removed from the Machine nothing will be changed on the node. + // - If an Always taint is changed to OnInitialization, the taint will only be added to the node on initialization. + // - If an OnInitialization taint is removed from the node, it will not be re-added during reconciliation. + TaintPropagationOnInitialization MachineTaintPropagation = "OnInitialization" +) diff --git a/api/core/v1beta2/machine_types.go b/api/core/v1beta2/machine_types.go index d8d557bd1746..2cc7ec4e2ec5 100644 --- a/api/core/v1beta2/machine_types.go +++ b/api/core/v1beta2/machine_types.go @@ -488,6 +488,20 @@ type MachineSpec struct { // deletion contains configuration options for Machine deletion. // +optional Deletion MachineDeletionSpec `json:"deletion,omitempty,omitzero"` + + // taints are the node taints that Cluster API will manage. + // This list is not necessarily complete: other Kubernetes components may add or remove other taints. + // 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. + // +optional + // +listType=map + // +listMapKey=key + // +listMapKey=effect + // +kubebuilder:validation:MinItems=1 + // +kubebuilder:validation:MaxItems=64 + Taints []MachineTaint `json:"taints,omitempty"` } // MachineDeletionSpec contains configuration options for Machine deletion. diff --git a/config/manager/manager.yaml b/config/manager/manager.yaml index c2cab3b37dba..4d46daf4c9e0 100644 --- a/config/manager/manager.yaml +++ b/config/manager/manager.yaml @@ -23,7 +23,7 @@ spec: - "--leader-elect" - "--diagnostics-address=${CAPI_DIAGNOSTICS_ADDRESS:=:8443}" - "--insecure-diagnostics=${CAPI_INSECURE_DIAGNOSTICS:=false}" - - "--feature-gates=MachinePool=${EXP_MACHINE_POOL:=true},ClusterTopology=${CLUSTER_TOPOLOGY:=false},RuntimeSDK=${EXP_RUNTIME_SDK:=false},MachineSetPreflightChecks=${EXP_MACHINE_SET_PREFLIGHT_CHECKS:=true},MachineWaitForVolumeDetachConsiderVolumeAttachments=${EXP_MACHINE_WAITFORVOLUMEDETACH_CONSIDER_VOLUMEATTACHMENTS:=true},PriorityQueue=${EXP_PRIORITY_QUEUE:=false},InPlaceUpdates=${EXP_IN_PLACE_UPDATES:=false}" + - "--feature-gates=MachinePool=${EXP_MACHINE_POOL:=true},ClusterTopology=${CLUSTER_TOPOLOGY:=false},RuntimeSDK=${EXP_RUNTIME_SDK:=false},MachineSetPreflightChecks=${EXP_MACHINE_SET_PREFLIGHT_CHECKS:=true},MachineWaitForVolumeDetachConsiderVolumeAttachments=${EXP_MACHINE_WAITFORVOLUMEDETACH_CONSIDER_VOLUMEATTACHMENTS:=true},PriorityQueue=${EXP_PRIORITY_QUEUE:=false},InPlaceUpdates=${EXP_IN_PLACE_UPDATES:=false},MachineTaintPropagation=${EXP_MACHINE_TAINT_PROPAGATION:=false}" image: controller:latest name: manager env: diff --git a/feature/feature.go b/feature/feature.go index f384df0475ab..4ad93cf0f721 100644 --- a/feature/feature.go +++ b/feature/feature.go @@ -72,6 +72,12 @@ const ( // InPlaceUpdates is a feature gate for the in-place machine updates functionality. // alpha: v1.12 InPlaceUpdates featuregate.Feature = "InPlaceUpdates" + + // MachineTaintPropagation is a feature gate for the machine taint propagation functionality. + // See docs/proposals/20250513-propogate-taints.md for more details. + // + // alpha: v1.12 + MachineTaintPropagation featuregate.Feature = "MachineTaintPropagation" ) func init() { @@ -90,4 +96,5 @@ var defaultClusterAPIFeatureGates = map[featuregate.Feature]featuregate.FeatureS KubeadmBootstrapFormatIgnition: {Default: false, PreRelease: featuregate.Alpha}, RuntimeSDK: {Default: false, PreRelease: featuregate.Alpha}, InPlaceUpdates: {Default: false, PreRelease: featuregate.Alpha}, + MachineTaintPropagation: {Default: false, PreRelease: featuregate.Alpha}, } diff --git a/internal/api/core/v1alpha3/conversion.go b/internal/api/core/v1alpha3/conversion.go index 5d6c73735258..4f08b27ab70e 100644 --- a/internal/api/core/v1alpha3/conversion.go +++ b/internal/api/core/v1alpha3/conversion.go @@ -246,6 +246,7 @@ func (src *Machine) ConvertTo(dstRaw conversion.Hub) error { if ok { dst.Spec.MinReadySeconds = restored.Spec.MinReadySeconds dst.Spec.ReadinessGates = restored.Spec.ReadinessGates + dst.Spec.Taints = restored.Spec.Taints dst.Spec.Deletion.NodeDeletionTimeoutSeconds = restored.Spec.Deletion.NodeDeletionTimeoutSeconds dst.Spec.Deletion.NodeVolumeDetachTimeoutSeconds = restored.Spec.Deletion.NodeVolumeDetachTimeoutSeconds dst.Status.NodeInfo = restored.Status.NodeInfo @@ -333,6 +334,7 @@ func (src *MachineSet) ConvertTo(dstRaw conversion.Hub) error { return err } dst.Spec.Template.Spec.ReadinessGates = restored.Spec.Template.Spec.ReadinessGates + dst.Spec.Template.Spec.Taints = restored.Spec.Template.Spec.Taints dst.Spec.Template.Spec.Deletion.NodeDeletionTimeoutSeconds = restored.Spec.Template.Spec.Deletion.NodeDeletionTimeoutSeconds dst.Spec.Template.Spec.Deletion.NodeVolumeDetachTimeoutSeconds = restored.Spec.Template.Spec.Deletion.NodeVolumeDetachTimeoutSeconds if restored.Status.Deprecated != nil && restored.Status.Deprecated.V1Beta1 != nil { @@ -425,6 +427,7 @@ func (src *MachineDeployment) ConvertTo(dstRaw conversion.Hub) error { dst.Spec.Remediation = restored.Spec.Remediation dst.Spec.MachineNaming = restored.Spec.MachineNaming dst.Spec.Template.Spec.ReadinessGates = restored.Spec.Template.Spec.ReadinessGates + dst.Spec.Template.Spec.Taints = restored.Spec.Template.Spec.Taints dst.Spec.Template.Spec.Deletion.NodeDeletionTimeoutSeconds = restored.Spec.Template.Spec.Deletion.NodeDeletionTimeoutSeconds dst.Spec.Template.Spec.Deletion.NodeVolumeDetachTimeoutSeconds = restored.Spec.Template.Spec.Deletion.NodeVolumeDetachTimeoutSeconds dst.Spec.Rollout.After = restored.Spec.Rollout.After @@ -603,6 +606,7 @@ func (src *MachinePool) ConvertTo(dstRaw conversion.Hub) error { dst.Spec.Template.Spec.ReadinessGates = restored.Spec.Template.Spec.ReadinessGates dst.Spec.Template.Spec.Deletion.NodeDeletionTimeoutSeconds = restored.Spec.Template.Spec.Deletion.NodeDeletionTimeoutSeconds dst.Spec.Template.Spec.Deletion.NodeVolumeDetachTimeoutSeconds = restored.Spec.Template.Spec.Deletion.NodeVolumeDetachTimeoutSeconds + dst.Spec.Template.Spec.Taints = restored.Spec.Template.Spec.Taints dst.Status.Conditions = restored.Status.Conditions dst.Status.AvailableReplicas = restored.Status.AvailableReplicas dst.Status.ReadyReplicas = restored.Status.ReadyReplicas diff --git a/internal/api/core/v1alpha4/conversion.go b/internal/api/core/v1alpha4/conversion.go index cc921319b13b..112662de7341 100644 --- a/internal/api/core/v1alpha4/conversion.go +++ b/internal/api/core/v1alpha4/conversion.go @@ -333,6 +333,7 @@ func (src *Machine) ConvertTo(dstRaw conversion.Hub) error { dst.Spec.Deletion.NodeDeletionTimeoutSeconds = restored.Spec.Deletion.NodeDeletionTimeoutSeconds dst.Status.CertificatesExpiryDate = restored.Status.CertificatesExpiryDate dst.Spec.Deletion.NodeVolumeDetachTimeoutSeconds = restored.Spec.Deletion.NodeVolumeDetachTimeoutSeconds + dst.Spec.Taints = restored.Spec.Taints dst.Status.Deletion = restored.Status.Deletion dst.Status.Conditions = restored.Status.Conditions } @@ -422,6 +423,7 @@ func (src *MachineSet) ConvertTo(dstRaw conversion.Hub) error { dst.Spec.Template.Spec.ReadinessGates = restored.Spec.Template.Spec.ReadinessGates dst.Spec.Template.Spec.Deletion.NodeDeletionTimeoutSeconds = restored.Spec.Template.Spec.Deletion.NodeDeletionTimeoutSeconds dst.Spec.Template.Spec.Deletion.NodeVolumeDetachTimeoutSeconds = restored.Spec.Template.Spec.Deletion.NodeVolumeDetachTimeoutSeconds + dst.Spec.Template.Spec.Taints = restored.Spec.Template.Spec.Taints dst.Status.Conditions = restored.Status.Conditions dst.Status.AvailableReplicas = restored.Status.AvailableReplicas dst.Status.ReadyReplicas = restored.Status.ReadyReplicas @@ -518,6 +520,7 @@ func (src *MachineDeployment) ConvertTo(dstRaw conversion.Hub) error { dst.Spec.Rollout.After = restored.Spec.Rollout.After dst.Spec.Remediation = restored.Spec.Remediation dst.Spec.MachineNaming = restored.Spec.MachineNaming + dst.Spec.Template.Spec.Taints = restored.Spec.Template.Spec.Taints dst.Status.Conditions = restored.Status.Conditions dst.Status.AvailableReplicas = restored.Status.AvailableReplicas dst.Status.ReadyReplicas = restored.Status.ReadyReplicas @@ -686,6 +689,7 @@ func (src *MachinePool) ConvertTo(dstRaw conversion.Hub) error { dst.Spec.Template.Spec.ReadinessGates = restored.Spec.Template.Spec.ReadinessGates dst.Spec.Template.Spec.Deletion.NodeDeletionTimeoutSeconds = restored.Spec.Template.Spec.Deletion.NodeDeletionTimeoutSeconds dst.Spec.Template.Spec.Deletion.NodeVolumeDetachTimeoutSeconds = restored.Spec.Template.Spec.Deletion.NodeVolumeDetachTimeoutSeconds + dst.Spec.Template.Spec.Taints = restored.Spec.Template.Spec.Taints dst.Status.Conditions = restored.Status.Conditions dst.Status.AvailableReplicas = restored.Status.AvailableReplicas dst.Status.ReadyReplicas = restored.Status.ReadyReplicas From 7204e582daa818d4e3aa0471a055b21523ee8338 Mon Sep 17 00:00:00 2001 From: Christian Schlotter Date: Fri, 24 Oct 2025 14:20:23 +0200 Subject: [PATCH 02/21] make generate --- api/core/v1beta1/zz_generated.conversion.go | 1 + api/core/v1beta2/zz_generated.deepcopy.go | 27 ++++++++ api/core/v1beta2/zz_generated.openapi.go | 68 ++++++++++++++++++- .../cluster.x-k8s.io_machinedeployments.yaml | 52 ++++++++++++++ .../bases/cluster.x-k8s.io_machinepools.yaml | 52 ++++++++++++++ .../crd/bases/cluster.x-k8s.io_machines.yaml | 51 ++++++++++++++ .../bases/cluster.x-k8s.io_machinesets.yaml | 52 ++++++++++++++ .../core/v1alpha3/zz_generated.conversion.go | 1 + .../core/v1alpha4/zz_generated.conversion.go | 1 + 9 files changed, 304 insertions(+), 1 deletion(-) diff --git a/api/core/v1beta1/zz_generated.conversion.go b/api/core/v1beta1/zz_generated.conversion.go index 00cf25f842ad..c0514e67d223 100644 --- a/api/core/v1beta1/zz_generated.conversion.go +++ b/api/core/v1beta1/zz_generated.conversion.go @@ -3167,6 +3167,7 @@ func autoConvert_v1beta2_MachineSpec_To_v1beta1_MachineSpec(in *v1beta2.MachineS // WARNING: in.MinReadySeconds requires manual conversion: does not exist in peer-type out.ReadinessGates = *(*[]MachineReadinessGate)(unsafe.Pointer(&in.ReadinessGates)) // WARNING: in.Deletion requires manual conversion: does not exist in peer-type + // WARNING: in.Taints requires manual conversion: does not exist in peer-type return nil } diff --git a/api/core/v1beta2/zz_generated.deepcopy.go b/api/core/v1beta2/zz_generated.deepcopy.go index b8707d841111..c6e7b4d75a5b 100644 --- a/api/core/v1beta2/zz_generated.deepcopy.go +++ b/api/core/v1beta2/zz_generated.deepcopy.go @@ -3441,6 +3441,13 @@ func (in *MachineSpec) DeepCopyInto(out *MachineSpec) { copy(*out, *in) } in.Deletion.DeepCopyInto(&out.Deletion) + if in.Taints != nil { + in, out := &in.Taints, &out.Taints + *out = make([]MachineTaint, len(*in)) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) + } + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new MachineSpec. @@ -3499,6 +3506,26 @@ func (in *MachineStatus) DeepCopy() *MachineStatus { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *MachineTaint) DeepCopyInto(out *MachineTaint) { + *out = *in + if in.Value != nil { + in, out := &in.Value, &out.Value + *out = new(string) + **out = **in + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new MachineTaint. +func (in *MachineTaint) DeepCopy() *MachineTaint { + if in == nil { + return nil + } + out := new(MachineTaint) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *MachineTemplateSpec) DeepCopyInto(out *MachineTemplateSpec) { *out = *in diff --git a/api/core/v1beta2/zz_generated.openapi.go b/api/core/v1beta2/zz_generated.openapi.go index c9bdfb70a604..83e0fcebed46 100644 --- a/api/core/v1beta2/zz_generated.openapi.go +++ b/api/core/v1beta2/zz_generated.openapi.go @@ -163,6 +163,7 @@ func GetOpenAPIDefinitions(ref common.ReferenceCallback) map[string]common.OpenA "sigs.k8s.io/cluster-api/api/core/v1beta2.MachineSetV1Beta1DeprecatedStatus": schema_cluster_api_api_core_v1beta2_MachineSetV1Beta1DeprecatedStatus(ref), "sigs.k8s.io/cluster-api/api/core/v1beta2.MachineSpec": schema_cluster_api_api_core_v1beta2_MachineSpec(ref), "sigs.k8s.io/cluster-api/api/core/v1beta2.MachineStatus": schema_cluster_api_api_core_v1beta2_MachineStatus(ref), + "sigs.k8s.io/cluster-api/api/core/v1beta2.MachineTaint": schema_cluster_api_api_core_v1beta2_MachineTaint(ref), "sigs.k8s.io/cluster-api/api/core/v1beta2.MachineTemplateSpec": schema_cluster_api_api_core_v1beta2_MachineTemplateSpec(ref), "sigs.k8s.io/cluster-api/api/core/v1beta2.MachineV1Beta1DeprecatedStatus": schema_cluster_api_api_core_v1beta2_MachineV1Beta1DeprecatedStatus(ref), "sigs.k8s.io/cluster-api/api/core/v1beta2.NetworkRanges": schema_cluster_api_api_core_v1beta2_NetworkRanges(ref), @@ -6163,12 +6164,35 @@ func schema_cluster_api_api_core_v1beta2_MachineSpec(ref common.ReferenceCallbac Ref: ref("sigs.k8s.io/cluster-api/api/core/v1beta2.MachineDeletionSpec"), }, }, + "taints": { + VendorExtensible: spec.VendorExtensible{ + Extensions: spec.Extensions{ + "x-kubernetes-list-map-keys": []interface{}{ + "key", + "effect", + }, + "x-kubernetes-list-type": "map", + }, + }, + SchemaProps: spec.SchemaProps{ + Description: "taints are the node taints that Cluster API will manage. This list is not necessarily complete: other Kubernetes components may add or remove other taints. Only those taints defined in this list will be added or removed by core Cluster API controllers.\n\nNOTE: 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.", + Type: []string{"array"}, + Items: &spec.SchemaOrArray{ + Schema: &spec.Schema{ + SchemaProps: spec.SchemaProps{ + Default: map[string]interface{}{}, + Ref: ref("sigs.k8s.io/cluster-api/api/core/v1beta2.MachineTaint"), + }, + }, + }, + }, + }, }, Required: []string{"clusterName", "bootstrap", "infrastructureRef"}, }, }, Dependencies: []string{ - "sigs.k8s.io/cluster-api/api/core/v1beta2.Bootstrap", "sigs.k8s.io/cluster-api/api/core/v1beta2.ContractVersionedObjectReference", "sigs.k8s.io/cluster-api/api/core/v1beta2.MachineDeletionSpec", "sigs.k8s.io/cluster-api/api/core/v1beta2.MachineReadinessGate"}, + "sigs.k8s.io/cluster-api/api/core/v1beta2.Bootstrap", "sigs.k8s.io/cluster-api/api/core/v1beta2.ContractVersionedObjectReference", "sigs.k8s.io/cluster-api/api/core/v1beta2.MachineDeletionSpec", "sigs.k8s.io/cluster-api/api/core/v1beta2.MachineReadinessGate", "sigs.k8s.io/cluster-api/api/core/v1beta2.MachineTaint"}, } } @@ -6281,6 +6305,48 @@ func schema_cluster_api_api_core_v1beta2_MachineStatus(ref common.ReferenceCallb } } +func schema_cluster_api_api_core_v1beta2_MachineTaint(ref common.ReferenceCallback) common.OpenAPIDefinition { + return common.OpenAPIDefinition{ + Schema: spec.Schema{ + SchemaProps: spec.SchemaProps{ + Description: "MachineTaint defines a taint equivalent to corev1.Taint, but additionally having a propagation field.", + Type: []string{"object"}, + Properties: map[string]spec.Schema{ + "key": { + SchemaProps: spec.SchemaProps{ + Description: "key is the taint key to be applied to a node.", + Type: []string{"string"}, + Format: "", + }, + }, + "value": { + SchemaProps: spec.SchemaProps{ + Description: "value is the taint value corresponding to the taint key.", + Type: []string{"string"}, + Format: "", + }, + }, + "effect": { + SchemaProps: spec.SchemaProps{ + Description: "effect is the effect for the taint. Valid values are NoSchedule, PreferNoSchedule and NoExecute.", + Type: []string{"string"}, + Format: "", + }, + }, + "propagation": { + SchemaProps: spec.SchemaProps{ + Description: "propagation defines how this taint should be propagated to nodes. Always: The taint will be continuously reconciled. If it is not set for a node, it will be added during reconciliation. OnInitialization: The taint will be added during node initialization. If it gets removed from the node later on it will not get added again.", + Type: []string{"string"}, + Format: "", + }, + }, + }, + Required: []string{"key", "effect", "propagation"}, + }, + }, + } +} + func schema_cluster_api_api_core_v1beta2_MachineTemplateSpec(ref common.ReferenceCallback) common.OpenAPIDefinition { return common.OpenAPIDefinition{ Schema: spec.Schema{ diff --git a/config/crd/bases/cluster.x-k8s.io_machinedeployments.yaml b/config/crd/bases/cluster.x-k8s.io_machinedeployments.yaml index deeb748a2a6c..e20be8b4f70d 100644 --- a/config/crd/bases/cluster.x-k8s.io_machinedeployments.yaml +++ b/config/crd/bases/cluster.x-k8s.io_machinedeployments.yaml @@ -2340,6 +2340,58 @@ spec: x-kubernetes-list-map-keys: - conditionType x-kubernetes-list-type: map + taints: + description: |- + taints are the node taints that Cluster API will manage. + This list is not necessarily complete: other Kubernetes components may add or remove other taints. + 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. + items: + description: MachineTaint defines a taint equivalent to + corev1.Taint, but additionally having a propagation field. + properties: + effect: + description: effect is the effect for the taint. Valid + values are NoSchedule, PreferNoSchedule and NoExecute. + enum: + - NoSchedule + - PreferNoSchedule + - NoExecute + type: string + key: + description: key is the taint key to be applied to a + node. + maxLength: 253 + minLength: 1 + type: string + propagation: + description: |- + propagation defines how this taint should be propagated to nodes. + Always: The taint will be continuously reconciled. If it is not set for a node, it will be added during reconciliation. + OnInitialization: The taint will be added during node initialization. If it gets removed from the node later on it will not get added again. + enum: + - Always + - OnInitialization + type: string + value: + description: value is the taint value corresponding + to the taint key. + maxLength: 63 + type: string + required: + - effect + - key + - propagation + type: object + maxItems: 64 + minItems: 1 + type: array + x-kubernetes-list-map-keys: + - key + - effect + x-kubernetes-list-type: map version: description: |- version defines the desired Kubernetes version. diff --git a/config/crd/bases/cluster.x-k8s.io_machinepools.yaml b/config/crd/bases/cluster.x-k8s.io_machinepools.yaml index 9f287129fd04..9d7770be7709 100644 --- a/config/crd/bases/cluster.x-k8s.io_machinepools.yaml +++ b/config/crd/bases/cluster.x-k8s.io_machinepools.yaml @@ -1944,6 +1944,58 @@ spec: x-kubernetes-list-map-keys: - conditionType x-kubernetes-list-type: map + taints: + description: |- + taints are the node taints that Cluster API will manage. + This list is not necessarily complete: other Kubernetes components may add or remove other taints. + 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. + items: + description: MachineTaint defines a taint equivalent to + corev1.Taint, but additionally having a propagation field. + properties: + effect: + description: effect is the effect for the taint. Valid + values are NoSchedule, PreferNoSchedule and NoExecute. + enum: + - NoSchedule + - PreferNoSchedule + - NoExecute + type: string + key: + description: key is the taint key to be applied to a + node. + maxLength: 253 + minLength: 1 + type: string + propagation: + description: |- + propagation defines how this taint should be propagated to nodes. + Always: The taint will be continuously reconciled. If it is not set for a node, it will be added during reconciliation. + OnInitialization: The taint will be added during node initialization. If it gets removed from the node later on it will not get added again. + enum: + - Always + - OnInitialization + type: string + value: + description: value is the taint value corresponding + to the taint key. + maxLength: 63 + type: string + required: + - effect + - key + - propagation + type: object + maxItems: 64 + minItems: 1 + type: array + x-kubernetes-list-map-keys: + - key + - effect + x-kubernetes-list-type: map version: description: |- version defines the desired Kubernetes version. diff --git a/config/crd/bases/cluster.x-k8s.io_machines.yaml b/config/crd/bases/cluster.x-k8s.io_machines.yaml index bc29bfcfe003..c8637832f267 100644 --- a/config/crd/bases/cluster.x-k8s.io_machines.yaml +++ b/config/crd/bases/cluster.x-k8s.io_machines.yaml @@ -1754,6 +1754,57 @@ spec: x-kubernetes-list-map-keys: - conditionType x-kubernetes-list-type: map + taints: + description: |- + taints are the node taints that Cluster API will manage. + This list is not necessarily complete: other Kubernetes components may add or remove other taints. + 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. + items: + description: MachineTaint defines a taint equivalent to corev1.Taint, + but additionally having a propagation field. + properties: + effect: + description: effect is the effect for the taint. Valid values + are NoSchedule, PreferNoSchedule and NoExecute. + enum: + - NoSchedule + - PreferNoSchedule + - NoExecute + type: string + key: + description: key is the taint key to be applied to a node. + maxLength: 253 + minLength: 1 + type: string + propagation: + description: |- + propagation defines how this taint should be propagated to nodes. + Always: The taint will be continuously reconciled. If it is not set for a node, it will be added during reconciliation. + OnInitialization: The taint will be added during node initialization. If it gets removed from the node later on it will not get added again. + enum: + - Always + - OnInitialization + type: string + value: + description: value is the taint value corresponding to the taint + key. + maxLength: 63 + type: string + required: + - effect + - key + - propagation + type: object + maxItems: 64 + minItems: 1 + type: array + x-kubernetes-list-map-keys: + - key + - effect + x-kubernetes-list-type: map version: description: |- version defines the desired Kubernetes version. diff --git a/config/crd/bases/cluster.x-k8s.io_machinesets.yaml b/config/crd/bases/cluster.x-k8s.io_machinesets.yaml index bb806f6b3935..2d9e2b5327a0 100644 --- a/config/crd/bases/cluster.x-k8s.io_machinesets.yaml +++ b/config/crd/bases/cluster.x-k8s.io_machinesets.yaml @@ -1996,6 +1996,58 @@ spec: x-kubernetes-list-map-keys: - conditionType x-kubernetes-list-type: map + taints: + description: |- + taints are the node taints that Cluster API will manage. + This list is not necessarily complete: other Kubernetes components may add or remove other taints. + 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. + items: + description: MachineTaint defines a taint equivalent to + corev1.Taint, but additionally having a propagation field. + properties: + effect: + description: effect is the effect for the taint. Valid + values are NoSchedule, PreferNoSchedule and NoExecute. + enum: + - NoSchedule + - PreferNoSchedule + - NoExecute + type: string + key: + description: key is the taint key to be applied to a + node. + maxLength: 253 + minLength: 1 + type: string + propagation: + description: |- + propagation defines how this taint should be propagated to nodes. + Always: The taint will be continuously reconciled. If it is not set for a node, it will be added during reconciliation. + OnInitialization: The taint will be added during node initialization. If it gets removed from the node later on it will not get added again. + enum: + - Always + - OnInitialization + type: string + value: + description: value is the taint value corresponding + to the taint key. + maxLength: 63 + type: string + required: + - effect + - key + - propagation + type: object + maxItems: 64 + minItems: 1 + type: array + x-kubernetes-list-map-keys: + - key + - effect + x-kubernetes-list-type: map version: description: |- version defines the desired Kubernetes version. diff --git a/internal/api/core/v1alpha3/zz_generated.conversion.go b/internal/api/core/v1alpha3/zz_generated.conversion.go index 0c9c762839fe..532cb03ea71f 100644 --- a/internal/api/core/v1alpha3/zz_generated.conversion.go +++ b/internal/api/core/v1alpha3/zz_generated.conversion.go @@ -1335,6 +1335,7 @@ func autoConvert_v1beta2_MachineSpec_To_v1alpha3_MachineSpec(in *v1beta2.Machine // WARNING: in.MinReadySeconds requires manual conversion: does not exist in peer-type // WARNING: in.ReadinessGates requires manual conversion: does not exist in peer-type // WARNING: in.Deletion requires manual conversion: does not exist in peer-type + // WARNING: in.Taints requires manual conversion: does not exist in peer-type return nil } diff --git a/internal/api/core/v1alpha4/zz_generated.conversion.go b/internal/api/core/v1alpha4/zz_generated.conversion.go index aca82dc6be1d..c807a20e0660 100644 --- a/internal/api/core/v1alpha4/zz_generated.conversion.go +++ b/internal/api/core/v1alpha4/zz_generated.conversion.go @@ -1726,6 +1726,7 @@ func autoConvert_v1beta2_MachineSpec_To_v1alpha4_MachineSpec(in *v1beta2.Machine // WARNING: in.MinReadySeconds requires manual conversion: does not exist in peer-type // WARNING: in.ReadinessGates requires manual conversion: does not exist in peer-type // WARNING: in.Deletion requires manual conversion: does not exist in peer-type + // WARNING: in.Taints requires manual conversion: does not exist in peer-type return nil } From 38d4fc3d8625fa939eb0110aa1990f37198f98c1 Mon Sep 17 00:00:00 2001 From: Christian Schlotter Date: Fri, 24 Oct 2025 14:21:06 +0200 Subject: [PATCH 03/21] taint propagation: implement validation --- internal/webhooks/machine.go | 77 ++++++++++++++++++++++++++ internal/webhooks/machinedeployment.go | 3 + internal/webhooks/machinepool.go | 8 +++ internal/webhooks/machineset.go | 3 + 4 files changed, 91 insertions(+) diff --git a/internal/webhooks/machine.go b/internal/webhooks/machine.go index bcd434fce574..86c61f4b6b42 100644 --- a/internal/webhooks/machine.go +++ b/internal/webhooks/machine.go @@ -22,7 +22,9 @@ import ( "strings" apierrors "k8s.io/apimachinery/pkg/api/errors" + metav1validation "k8s.io/apimachinery/pkg/apis/meta/v1/validation" "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/util/validation" "k8s.io/apimachinery/pkg/util/validation/field" "k8s.io/utils/ptr" ctrl "sigs.k8s.io/controller-runtime" @@ -30,6 +32,7 @@ import ( "sigs.k8s.io/controller-runtime/pkg/webhook/admission" clusterv1 "sigs.k8s.io/cluster-api/api/core/v1beta2" + "sigs.k8s.io/cluster-api/feature" "sigs.k8s.io/cluster-api/util/labels" "sigs.k8s.io/cluster-api/util/version" ) @@ -136,8 +139,82 @@ func (webhook *Machine) validate(oldM, newM *clusterv1.Machine) error { } } + allErrs = append(allErrs, validateMachineTaints(newM.Spec.Taints, specPath.Child("taints"))...) + allErrs = append(allErrs, validateMachineTaintsForWorkers(newM.Spec.Taints, newM, specPath.Child("taints"))...) + if len(allErrs) == 0 { return nil } return apierrors.NewInvalid(clusterv1.GroupVersion.WithKind("Machine").GroupKind(), newM.Name, allErrs) } + +func validateMachineTaints(taints []clusterv1.MachineTaint, taintsPath *field.Path) field.ErrorList { + var allErrs field.ErrorList + + if !feature.Gates.Enabled(feature.MachineTaintPropagation) { + if len(taints) > 0 { + allErrs = append(allErrs, field.Forbidden(taintsPath, "taints are not allowed to be set when the feature gate MachineTaintPropagation is disabled")) + } + } + + for i, taint := range taints { + idxPath := taintsPath.Index(i) + + // Validate key syntax. + if errs := metav1validation.ValidateLabelName(taint.Key, idxPath); len(errs) > 0 { + allErrs = append(allErrs, errs...) + } + + // Validate value syntax. + if errs := validation.IsValidLabelValue(ptr.Deref(taint.Value, "")); len(errs) > 0 { + allErrs = append(allErrs, field.Invalid(idxPath.Child("value"), taint.Value, strings.Join(errs, ";"))) + } + + // The following validations uses a switch statement, because if one of them matches, then the others won't. + + switch { + // Validate for keys which are reserved for usage by the cluster-api or providers. + case taint.Key == "node.cluster.x-k8s.io/uninitialized": + allErrs = append(allErrs, field.Invalid(idxPath.Child("key"), taint.Key, "taint key must not be node.cluster.x-k8s.io/uninitialized")) + case taint.Key == "node.cluster.x-k8s.io/outdated-revision": + allErrs = append(allErrs, field.Invalid(idxPath.Child("key"), taint.Key, "taint key must not be node.cluster.x-k8s.io/uninitialized or node.cluster.x-k8s.io/outdated-revision")) + // Validate for key's which are reserved for usage by the node or node-lifecycle-controller, but allow `node.kubernetes.io/out-of-service`. + case strings.HasPrefix(taint.Key, "node.kubernetes.io/") && taint.Key != "node.kubernetes.io/out-of-service": + allErrs = append(allErrs, field.Invalid(idxPath.Child("key"), taint.Key, "taint key must not have the prefix node.kubernetes.io/, except for node.kubernetes.io/out-of-service")) + // Validate for keys which are reserved for usage by the cloud-controller-manager or kubelet. + case strings.HasPrefix(taint.Key, "node.cloudprovider.kubernetes.io/"): + allErrs = append(allErrs, field.Invalid(idxPath.Child("key"), taint.Key, "taint key must not have the prefix node.cloudprovider.kubernetes.io/")) + // Validate for the deprecated kubeadm node-role taint. + case taint.Key == "node-role.kubernetes.io/master": + allErrs = append(allErrs, field.Invalid(idxPath.Child("key"), taint.Key, "taint is deprecated since 1.24 and should not be used anymore")) + } + } + + return allErrs +} + +func validateMachineTaintsForWorkers(taints []clusterv1.MachineTaint, machine *clusterv1.Machine, taintsPath *field.Path) field.ErrorList { + var allErrs field.ErrorList + + if !feature.Gates.Enabled(feature.MachineTaintPropagation) { + return allErrs + } + + // Skip for control-plane machines. + if machine != nil && machine.Labels != nil { + if _, ok := machine.Labels[clusterv1.MachineControlPlaneLabel]; ok { + return allErrs + } + } + + // Validate taints for worker machines. + for i, taint := range taints { + idxPath := taintsPath.Index(i) + + if taint.Key == "node-role.kubernetes.io/control-plane" { + allErrs = append(allErrs, field.Invalid(idxPath.Child("key"), taint.Key, "taint is not allowed for worker machines")) + } + } + + return allErrs +} diff --git a/internal/webhooks/machinedeployment.go b/internal/webhooks/machinedeployment.go index dadec0df3ec9..40e3ef90f5a3 100644 --- a/internal/webhooks/machinedeployment.go +++ b/internal/webhooks/machinedeployment.go @@ -254,6 +254,9 @@ func (webhook *MachineDeployment) validate(oldMD, newMD *clusterv1.MachineDeploy allErrs = append(allErrs, validateMDMachineNaming(newMD.Spec.MachineNaming, specPath.Child("machineNaming"))...) + allErrs = append(allErrs, validateMachineTaints(newMD.Spec.Template.Spec.Taints, specPath.Child("template", "spec", "taints"))...) + allErrs = append(allErrs, validateMachineTaintsForWorkers(newMD.Spec.Template.Spec.Taints, nil, specPath.Child("template", "spec", "taints"))...) + // Validate the metadata of the template. allErrs = append(allErrs, newMD.Spec.Template.Validate(specPath.Child("template", "metadata"))...) diff --git a/internal/webhooks/machinepool.go b/internal/webhooks/machinepool.go index 0336db1e9dfb..e1bd2be347fd 100644 --- a/internal/webhooks/machinepool.go +++ b/internal/webhooks/machinepool.go @@ -189,6 +189,14 @@ func (webhook *MachinePool) validate(oldObj, newObj *clusterv1.MachinePool) erro } } + if len(newObj.Spec.Template.Spec.Taints) > 0 { + if !feature.Gates.Enabled(feature.MachineTaintPropagation) { + allErrs = append(allErrs, field.Forbidden(specPath.Child("taints"), "taints are not allowed to be set when the feature gate MachineTaintPropagation is disabled")) + } else { + allErrs = append(allErrs, field.Forbidden(specPath.Child("taints"), "taints feature for MachinePools is not yet implemented")) + } + } + // Validate the metadata of the MachinePool template. allErrs = append(allErrs, newObj.Spec.Template.Validate(specPath.Child("template", "metadata"))...) diff --git a/internal/webhooks/machineset.go b/internal/webhooks/machineset.go index 856108b02bb4..75d011a2d209 100644 --- a/internal/webhooks/machineset.go +++ b/internal/webhooks/machineset.go @@ -229,6 +229,9 @@ func (webhook *MachineSet) validate(oldMS, newMS *clusterv1.MachineSet) error { } } + allErrs = append(allErrs, validateMachineTaints(newMS.Spec.Template.Spec.Taints, specPath.Child("template", "spec", "taints"))...) + allErrs = append(allErrs, validateMachineTaintsForWorkers(newMS.Spec.Template.Spec.Taints, nil, specPath.Child("template", "spec", "taints"))...) + allErrs = append(allErrs, validateMSMachineNaming(newMS.Spec.MachineNaming, specPath.Child("machineNaming"))...) // Validate the metadata of the template. From 58b87b4b5947b092fbbd3b832973593b24a6d7f7 Mon Sep 17 00:00:00 2001 From: Christian Schlotter Date: Mon, 3 Nov 2025 14:44:45 +0100 Subject: [PATCH 04/21] taint propagation: implement validation unit tests --- internal/webhooks/machine_test.go | 120 ++++++++++++++++++++ internal/webhooks/machinedeployment_test.go | 110 ++++++++++++++++++ internal/webhooks/machineset_test.go | 110 ++++++++++++++++++ util/test/builder/builders.go | 30 +++++ 4 files changed, 370 insertions(+) diff --git a/internal/webhooks/machine_test.go b/internal/webhooks/machine_test.go index d1e9ddd46ba4..a6de7a33cbb5 100644 --- a/internal/webhooks/machine_test.go +++ b/internal/webhooks/machine_test.go @@ -20,11 +20,15 @@ import ( "testing" . "github.com/onsi/gomega" + corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + utilfeature "k8s.io/component-base/featuregate/testing" "k8s.io/utils/ptr" clusterv1 "sigs.k8s.io/cluster-api/api/core/v1beta2" + "sigs.k8s.io/cluster-api/feature" "sigs.k8s.io/cluster-api/internal/webhooks/util" + "sigs.k8s.io/cluster-api/util/test/builder" ) func TestMachineDefault(t *testing.T) { @@ -225,3 +229,119 @@ func TestMachineVersionValidation(t *testing.T) { }) } } + +func TestMachineTaintValidation(t *testing.T) { + m := builder.Machine("default", "machine1"). + WithBootstrapTemplate(builder.BootstrapTemplate("default", "bootstrap-template").Build()) + webhook := &Machine{} + + tests := []struct { + name string + machine *clusterv1.Machine + featureEnabled bool + expectErr bool + }{ + { + name: "should allow empty taints with feature gate disabled", + featureEnabled: false, + machine: m.DeepCopy().Build(), + expectErr: false, + }, + { + name: "should allow empty taints with feature gate enabled", + featureEnabled: true, + machine: m.DeepCopy().Build(), + expectErr: false, + }, + { + name: "should block taint key node.cluster.x-k8s.io/uninitialized", + featureEnabled: true, + machine: m.DeepCopy().WithTaints(clusterv1.MachineTaint{ + Key: "node.cluster.x-k8s.io/uninitialized", Effect: corev1.TaintEffectNoSchedule, + }).Build(), + expectErr: true, + }, + { + name: "should block taint key node.cluster.x-k8s.io/outdated-revision", + featureEnabled: true, + machine: m.DeepCopy().WithTaints(clusterv1.MachineTaint{ + Key: "node.cluster.x-k8s.io/outdated-revision", Effect: corev1.TaintEffectNoSchedule, + }).Build(), + expectErr: true, + }, + { + name: "should block taint with key prefix node.kubernetes.io/, which is not `out-of-service`", + featureEnabled: true, + machine: m.DeepCopy().WithTaints(clusterv1.MachineTaint{ + Key: "node.kubernetes.io/some-taint", Effect: corev1.TaintEffectNoSchedule, + }).Build(), + expectErr: true, + }, + { + name: "should allow taint node.kubernetes.io/out-of-service", + featureEnabled: true, + machine: m.DeepCopy().WithTaints(clusterv1.MachineTaint{ + Key: "node.kubernetes.io/out-of-service", Effect: corev1.TaintEffectNoSchedule, + }).Build(), + expectErr: false, + }, + { + name: "should block taint with keyprefix node.cloudprovider.kubernetes.io/", + featureEnabled: true, + machine: m.DeepCopy().WithTaints(clusterv1.MachineTaint{ + Key: "node.cloudprovider.kubernetes.io/some-taint", Effect: corev1.TaintEffectNoSchedule, + }).Build(), + expectErr: true, + }, + { + name: "should block taint key node-role.kubernetes.io/master", + featureEnabled: true, + machine: m.DeepCopy().WithTaints(clusterv1.MachineTaint{ + Key: "node-role.kubernetes.io/master", Effect: corev1.TaintEffectNoSchedule, + }).Build(), + expectErr: true, + }, + { + name: "should allow taint key node-role.kubernetes.io/control-plane for control-plane nodes", + featureEnabled: true, + machine: m.DeepCopy(). + WithLabels(map[string]string{clusterv1.MachineControlPlaneLabel: "true"}). + WithTaints(clusterv1.MachineTaint{ + Key: "node-role.kubernetes.io/control-plane", Effect: corev1.TaintEffectNoSchedule, + }).Build(), + expectErr: false, + }, + { + name: "should block taint key node-role.kubernetes.io/control-plane for worker nodes", + featureEnabled: true, + machine: m.DeepCopy().WithTaints(clusterv1.MachineTaint{ + Key: "node-role.kubernetes.io/control-plane", Effect: corev1.TaintEffectNoSchedule, + }).Build(), + expectErr: true, + }, + } + for i := range tests { + tt := tests[i] + t.Run(tt.name, func(t *testing.T) { + g := NewWithT(t) + + utilfeature.SetFeatureGateDuringTest(t, feature.Gates, feature.MachineTaintPropagation, tt.featureEnabled) + + warnings, err := webhook.ValidateCreate(ctx, tt.machine) + if tt.expectErr { + g.Expect(err).To(HaveOccurred()) + } else { + g.Expect(err).ToNot(HaveOccurred()) + } + g.Expect(warnings).To(BeEmpty()) + + warnings, err = webhook.ValidateUpdate(ctx, tt.machine, tt.machine) + if tt.expectErr { + g.Expect(err).To(HaveOccurred()) + } else { + g.Expect(err).ToNot(HaveOccurred()) + } + g.Expect(warnings).To(BeEmpty()) + }) + } +} diff --git a/internal/webhooks/machinedeployment_test.go b/internal/webhooks/machinedeployment_test.go index b5a807acd0ca..de6450c3bdc3 100644 --- a/internal/webhooks/machinedeployment_test.go +++ b/internal/webhooks/machinedeployment_test.go @@ -23,14 +23,18 @@ import ( . "github.com/onsi/gomega" admissionv1 "k8s.io/api/admission/v1" + corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/util/intstr" + utilfeature "k8s.io/component-base/featuregate/testing" "k8s.io/utils/ptr" "sigs.k8s.io/controller-runtime/pkg/webhook/admission" clusterv1 "sigs.k8s.io/cluster-api/api/core/v1beta2" + "sigs.k8s.io/cluster-api/feature" "sigs.k8s.io/cluster-api/internal/webhooks/util" + "sigs.k8s.io/cluster-api/util/test/builder" ) func TestMachineDeploymentDefault(t *testing.T) { @@ -838,3 +842,109 @@ func TestMachineDeploymentTemplateMetadataValidation(t *testing.T) { }) } } + +func TestMachineDeploymentTaintValidation(t *testing.T) { + md := builder.MachineDeployment("default", "md"). + WithBootstrapTemplate(builder.BootstrapTemplate("default", "bootstrap-template").Build()) + webhook := &MachineDeployment{} + + tests := []struct { + name string + machineDeployment *clusterv1.MachineDeployment + featureEnabled bool + expectErr bool + }{ + { + name: "should allow empty taints with feature gate disabled", + featureEnabled: false, + machineDeployment: md.DeepCopy().Build(), + expectErr: false, + }, + { + name: "should allow empty taints with feature gate enabled", + featureEnabled: true, + machineDeployment: md.DeepCopy().Build(), + expectErr: false, + }, + { + name: "should block taint key node.cluster.x-k8s.io/uninitialized", + featureEnabled: true, + machineDeployment: md.DeepCopy().WithTaints(clusterv1.MachineTaint{ + Key: "node.cluster.x-k8s.io/uninitialized", Effect: corev1.TaintEffectNoSchedule, + }).Build(), + expectErr: true, + }, + { + name: "should block taint key node.cluster.x-k8s.io/outdated-revision", + featureEnabled: true, + machineDeployment: md.DeepCopy().WithTaints(clusterv1.MachineTaint{ + Key: "node.cluster.x-k8s.io/outdated-revision", Effect: corev1.TaintEffectNoSchedule, + }).Build(), + expectErr: true, + }, + { + name: "should block taint with key prefix node.kubernetes.io/, which is not `out-of-service`", + featureEnabled: true, + machineDeployment: md.DeepCopy().WithTaints(clusterv1.MachineTaint{ + Key: "node.kubernetes.io/some-taint", Effect: corev1.TaintEffectNoSchedule, + }).Build(), + expectErr: true, + }, + { + name: "should allow taint node.kubernetes.io/out-of-service", + featureEnabled: true, + machineDeployment: md.DeepCopy().WithTaints(clusterv1.MachineTaint{ + Key: "node.kubernetes.io/out-of-service", Effect: corev1.TaintEffectNoSchedule, + }).Build(), + expectErr: false, + }, + { + name: "should block taint with key prefix node.cloudprovider.kubernetes.io/", + featureEnabled: true, + machineDeployment: md.DeepCopy().WithTaints(clusterv1.MachineTaint{ + Key: "node.cloudprovider.kubernetes.io/some-taint", Effect: corev1.TaintEffectNoSchedule, + }).Build(), + expectErr: true, + }, + { + name: "should block taint key node-role.kubernetes.io/master", + featureEnabled: true, + machineDeployment: md.DeepCopy().WithTaints(clusterv1.MachineTaint{ + Key: "node-role.kubernetes.io/master", Effect: corev1.TaintEffectNoSchedule, + }).Build(), + expectErr: true, + }, + { + name: "should block taint key node-role.kubernetes.io/control-plane for worker nodes", + featureEnabled: true, + machineDeployment: md.DeepCopy().WithTaints(clusterv1.MachineTaint{ + Key: "node-role.kubernetes.io/control-plane", Effect: corev1.TaintEffectNoSchedule, + }).Build(), + expectErr: true, + }, + } + for i := range tests { + tt := tests[i] + t.Run(tt.name, func(t *testing.T) { + g := NewWithT(t) + + utilfeature.SetFeatureGateDuringTest(t, feature.Gates, feature.MachineTaintPropagation, tt.featureEnabled) + + warnings, err := webhook.ValidateCreate(ctx, tt.machineDeployment) + if tt.expectErr { + g.Expect(err).To(HaveOccurred()) + } else { + g.Expect(err).ToNot(HaveOccurred()) + } + g.Expect(warnings).To(BeEmpty()) + + warnings, err = webhook.ValidateUpdate(ctx, tt.machineDeployment, tt.machineDeployment) + if tt.expectErr { + g.Expect(err).To(HaveOccurred()) + } else { + g.Expect(err).ToNot(HaveOccurred()) + } + g.Expect(warnings).To(BeEmpty()) + }) + } +} diff --git a/internal/webhooks/machineset_test.go b/internal/webhooks/machineset_test.go index 43bb428058dc..6e55f64b1a3a 100644 --- a/internal/webhooks/machineset_test.go +++ b/internal/webhooks/machineset_test.go @@ -22,13 +22,17 @@ import ( "testing" . "github.com/onsi/gomega" + corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" + utilfeature "k8s.io/component-base/featuregate/testing" "k8s.io/utils/ptr" "sigs.k8s.io/controller-runtime/pkg/webhook/admission" clusterv1 "sigs.k8s.io/cluster-api/api/core/v1beta2" + "sigs.k8s.io/cluster-api/feature" "sigs.k8s.io/cluster-api/internal/webhooks/util" + "sigs.k8s.io/cluster-api/util/test/builder" ) func TestMachineSetDefault(t *testing.T) { @@ -742,3 +746,109 @@ func TestMachineSetMachineNamingValidation(t *testing.T) { }) } } + +func TestMachineSetTaintValidation(t *testing.T) { + ms := builder.MachineSet("default", "machineset1"). + WithBootstrapTemplate(builder.BootstrapTemplate("default", "bootstrap-template").Build()) + webhook := &MachineSet{} + + tests := []struct { + name string + machineSet *clusterv1.MachineSet + featureEnabled bool + expectErr bool + }{ + { + name: "should allow empty taints with feature gate disabled", + featureEnabled: false, + machineSet: ms.DeepCopy().Build(), + expectErr: false, + }, + { + name: "should allow empty taints with feature gate enabled", + featureEnabled: true, + machineSet: ms.DeepCopy().Build(), + expectErr: false, + }, + { + name: "should block taint key node.cluster.x-k8s.io/uninitialized", + featureEnabled: true, + machineSet: ms.DeepCopy().WithTaints(clusterv1.MachineTaint{ + Key: "node.cluster.x-k8s.io/uninitialized", Effect: corev1.TaintEffectNoSchedule, + }).Build(), + expectErr: true, + }, + { + name: "should block taint key node.cluster.x-k8s.io/outdated-revision", + featureEnabled: true, + machineSet: ms.DeepCopy().WithTaints(clusterv1.MachineTaint{ + Key: "node.cluster.x-k8s.io/outdated-revision", Effect: corev1.TaintEffectNoSchedule, + }).Build(), + expectErr: true, + }, + { + name: "should block taint with key prefix node.kubernetes.io/, which is not `out-of-service`", + featureEnabled: true, + machineSet: ms.DeepCopy().WithTaints(clusterv1.MachineTaint{ + Key: "node.kubernetes.io/some-taint", Effect: corev1.TaintEffectNoSchedule, + }).Build(), + expectErr: true, + }, + { + name: "should allow taint node.kubernetes.io/out-of-service", + featureEnabled: true, + machineSet: ms.DeepCopy().WithTaints(clusterv1.MachineTaint{ + Key: "node.kubernetes.io/out-of-service", Effect: corev1.TaintEffectNoSchedule, + }).Build(), + expectErr: false, + }, + { + name: "should block taint with keyprefix node.cloudprovider.kubernetes.io/", + featureEnabled: true, + machineSet: ms.DeepCopy().WithTaints(clusterv1.MachineTaint{ + Key: "node.cloudprovider.kubernetes.io/some-taint", Effect: corev1.TaintEffectNoSchedule, + }).Build(), + expectErr: true, + }, + { + name: "should block taint key node-role.kubernetes.io/master", + featureEnabled: true, + machineSet: ms.DeepCopy().WithTaints(clusterv1.MachineTaint{ + Key: "node-role.kubernetes.io/master", Effect: corev1.TaintEffectNoSchedule, + }).Build(), + expectErr: true, + }, + { + name: "should block taint key node-role.kubernetes.io/control-plane for worker nodes", + featureEnabled: true, + machineSet: ms.DeepCopy().WithTaints(clusterv1.MachineTaint{ + Key: "node-role.kubernetes.io/control-plane", Effect: corev1.TaintEffectNoSchedule, + }).Build(), + expectErr: true, + }, + } + for i := range tests { + tt := tests[i] + t.Run(tt.name, func(t *testing.T) { + g := NewWithT(t) + + utilfeature.SetFeatureGateDuringTest(t, feature.Gates, feature.MachineTaintPropagation, tt.featureEnabled) + + warnings, err := webhook.ValidateCreate(ctx, tt.machineSet) + if tt.expectErr { + g.Expect(err).To(HaveOccurred()) + } else { + g.Expect(err).ToNot(HaveOccurred()) + } + g.Expect(warnings).To(BeEmpty()) + + warnings, err = webhook.ValidateUpdate(ctx, tt.machineSet, tt.machineSet) + if tt.expectErr { + g.Expect(err).To(HaveOccurred()) + } else { + g.Expect(err).ToNot(HaveOccurred()) + } + g.Expect(warnings).To(BeEmpty()) + }) + } +} diff --git a/util/test/builder/builders.go b/util/test/builder/builders.go index 983c99310fb3..c3baadd9bb42 100644 --- a/util/test/builder/builders.go +++ b/util/test/builder/builders.go @@ -1764,6 +1764,7 @@ type MachineDeploymentBuilder struct { annotations map[string]string status *clusterv1.MachineDeploymentStatus minReadySeconds *int32 + taints []clusterv1.MachineTaint } // MachineDeployment creates a MachineDeploymentBuilder with the given name and namespace. @@ -1840,6 +1841,12 @@ func (m *MachineDeploymentBuilder) WithMinReadySeconds(minReadySeconds int32) *M return m } +// WithTaints adds the given taints to the MachineDeploymentBuilder. +func (m *MachineDeploymentBuilder) WithTaints(taints ...clusterv1.MachineTaint) *MachineDeploymentBuilder { + m.taints = taints + return m +} + // Build creates a new MachineDeployment with the variables and objects passed to the MachineDeploymentBuilder. func (m *MachineDeploymentBuilder) Build() *clusterv1.MachineDeployment { obj := &clusterv1.MachineDeployment{ @@ -1883,6 +1890,9 @@ func (m *MachineDeploymentBuilder) Build() *clusterv1.MachineDeployment { if m.minReadySeconds != nil { obj.Spec.Template.Spec.MinReadySeconds = m.minReadySeconds } + if m.taints != nil { + obj.Spec.Template.Spec.Taints = m.taints + } return obj } @@ -1897,6 +1907,7 @@ type MachineSetBuilder struct { labels map[string]string clusterName string ownerRefs []metav1.OwnerReference + taints []clusterv1.MachineTaint } // MachineSet creates a MachineSetBuilder with the given name and namespace. @@ -1943,6 +1954,12 @@ func (m *MachineSetBuilder) WithOwnerReferences(ownerRefs []metav1.OwnerReferenc return m } +// WithTaints adds the given taints to the MachineSetBuilder. +func (m *MachineSetBuilder) WithTaints(taints ...clusterv1.MachineTaint) *MachineSetBuilder { + m.taints = taints + return m +} + // Build creates a new MachineSet with the variables and objects passed to the MachineSetBuilder. func (m *MachineSetBuilder) Build() *clusterv1.MachineSet { obj := &clusterv1.MachineSet{ @@ -1962,6 +1979,9 @@ func (m *MachineSetBuilder) Build() *clusterv1.MachineSet { if m.infrastructureTemplate != nil { obj.Spec.Template.Spec.InfrastructureRef = objToRef(m.infrastructureTemplate) } + if m.taints != nil { + obj.Spec.Template.Spec.Taints = m.taints + } return obj } @@ -1974,6 +1994,7 @@ type MachineBuilder struct { bootstrap *unstructured.Unstructured infraMachine *unstructured.Unstructured labels map[string]string + taints []clusterv1.MachineTaint } // Machine returns a MachineBuilder. @@ -2014,6 +2035,12 @@ func (m *MachineBuilder) WithLabels(labels map[string]string) *MachineBuilder { return m } +// WithTaints adds the given taints to the MachineBuilder. +func (m *MachineBuilder) WithTaints(taints ...clusterv1.MachineTaint) *MachineBuilder { + m.taints = taints + return m +} + // Build produces a Machine object from the information passed to the MachineBuilder. func (m *MachineBuilder) Build() *clusterv1.Machine { machine := &clusterv1.Machine{ @@ -2039,6 +2066,9 @@ func (m *MachineBuilder) Build() *clusterv1.Machine { } machine.Labels[clusterv1.ClusterNameLabel] = m.clusterName } + if m.taints != nil { + machine.Spec.Taints = m.taints + } return machine } From b54464912a65385c9c4f39963ca389b9ba97f3b7 Mon Sep 17 00:00:00 2001 From: Christian Schlotter Date: Fri, 24 Oct 2025 14:21:46 +0200 Subject: [PATCH 05/21] taint propagation: in-place propagate via MachineDeployment and MachineSet --- .../machinedeployment/machinedeployment_rollout_planner.go | 1 + internal/controllers/machineset/machineset_controller.go | 2 ++ 2 files changed, 3 insertions(+) diff --git a/internal/controllers/machinedeployment/machinedeployment_rollout_planner.go b/internal/controllers/machinedeployment/machinedeployment_rollout_planner.go index 5d7ac25cd9e5..900b09e73834 100644 --- a/internal/controllers/machinedeployment/machinedeployment_rollout_planner.go +++ b/internal/controllers/machinedeployment/machinedeployment_rollout_planner.go @@ -322,6 +322,7 @@ func computeDesiredMS(ctx context.Context, deployment *clusterv1.MachineDeployme desiredMS.Spec.Template.Spec.Deletion.NodeDrainTimeoutSeconds = deployment.Spec.Template.Spec.Deletion.NodeDrainTimeoutSeconds desiredMS.Spec.Template.Spec.Deletion.NodeDeletionTimeoutSeconds = deployment.Spec.Template.Spec.Deletion.NodeDeletionTimeoutSeconds desiredMS.Spec.Template.Spec.Deletion.NodeVolumeDetachTimeoutSeconds = deployment.Spec.Template.Spec.Deletion.NodeVolumeDetachTimeoutSeconds + desiredMS.Spec.Template.Spec.Taints = deployment.Spec.Template.Spec.Taints return desiredMS, nil } diff --git a/internal/controllers/machineset/machineset_controller.go b/internal/controllers/machineset/machineset_controller.go index cbfc066bff7d..13c8594a2f19 100644 --- a/internal/controllers/machineset/machineset_controller.go +++ b/internal/controllers/machineset/machineset_controller.go @@ -669,6 +669,7 @@ func (r *Reconciler) syncMachines(ctx context.Context, s *scope) (ctrl.Result, e m.Spec.Deletion.NodeDeletionTimeoutSeconds = machineSet.Spec.Template.Spec.Deletion.NodeDeletionTimeoutSeconds m.Spec.Deletion.NodeVolumeDetachTimeoutSeconds = machineSet.Spec.Template.Spec.Deletion.NodeVolumeDetachTimeoutSeconds m.Spec.MinReadySeconds = machineSet.Spec.Template.Spec.MinReadySeconds + m.Spec.Taints = machineSet.Spec.Template.Spec.Taints if err := patchHelper.Patch(ctx, m); err != nil { return ctrl.Result{}, err @@ -1153,6 +1154,7 @@ func (r *Reconciler) computeDesiredMachine(machineSet *clusterv1.MachineSet, exi desiredMachine.Spec.Deletion.NodeDeletionTimeoutSeconds = machineSet.Spec.Template.Spec.Deletion.NodeDeletionTimeoutSeconds desiredMachine.Spec.Deletion.NodeVolumeDetachTimeoutSeconds = machineSet.Spec.Template.Spec.Deletion.NodeVolumeDetachTimeoutSeconds desiredMachine.Spec.MinReadySeconds = machineSet.Spec.Template.Spec.MinReadySeconds + desiredMachine.Spec.Taints = machineSet.Spec.Template.Spec.Taints return desiredMachine, nil } From 5e5a43354a23d02fc36020a99be236e482bf3b12 Mon Sep 17 00:00:00 2001 From: Christian Schlotter Date: Fri, 24 Oct 2025 14:22:12 +0200 Subject: [PATCH 06/21] taint propagation: implement propagation in machine controller --- .../machine/machine_controller_noderef.go | 111 +++++++++++++++++- 1 file changed, 109 insertions(+), 2 deletions(-) diff --git a/internal/controllers/machine/machine_controller_noderef.go b/internal/controllers/machine/machine_controller_noderef.go index afa5414351ff..a17e1a8ea59d 100644 --- a/internal/controllers/machine/machine_controller_noderef.go +++ b/internal/controllers/machine/machine_controller_noderef.go @@ -27,12 +27,15 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/apimachinery/pkg/util/sets" "k8s.io/klog/v2" + "k8s.io/utils/ptr" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" clusterv1 "sigs.k8s.io/cluster-api/api/core/v1beta2" "sigs.k8s.io/cluster-api/api/core/v1beta2/index" + "sigs.k8s.io/cluster-api/feature" "sigs.k8s.io/cluster-api/internal/controllers/machinedeployment/mdutil" "sigs.k8s.io/cluster-api/internal/util/taints" "sigs.k8s.io/cluster-api/util" @@ -334,6 +337,15 @@ func (r *Reconciler) patchNode(ctx context.Context, remoteClient client.Client, // Drop the NodeUninitializedTaint taint on the node given that we are reconciling labels. hasTaintChanges := taints.RemoveNodeTaint(newNode, clusterv1.NodeUninitializedTaint) + // Propagate taints set on the Machine to the Node. + var propagateTaintsChanges bool + if feature.Gates.Enabled(feature.MachineTaintPropagation) { + var err error + if propagateTaintsChanges, err = propagateMachineTaintsToNode(newNode, m.Spec.Taints); err != nil { + return errors.Wrapf(err, "failed to propagate machine taints to node %s", klog.KRef("", node.Name)) + } + } + // Set Taint to a node in an old MachineSet and unset Taint from a node in a new MachineSet isOutdated, err := shouldNodeHaveOutdatedTaint(ms, md) if err != nil { @@ -345,11 +357,106 @@ func (r *Reconciler) patchNode(ctx context.Context, remoteClient client.Client, hasTaintChanges = taints.RemoveNodeTaint(newNode, clusterv1.NodeOutdatedRevisionTaint) || hasTaintChanges } - if !hasAnnotationChanges && !hasLabelChanges && !hasTaintChanges { + if !hasAnnotationChanges && !hasLabelChanges && !hasTaintChanges && !propagateTaintsChanges { return nil } - return remoteClient.Patch(ctx, newNode, client.StrategicMergeFrom(node)) + var mergeOptions []client.MergeFromOption + if feature.Gates.Enabled(feature.MachineTaintPropagation) { + mergeOptions = append(mergeOptions, client.MergeFromWithOptimisticLock{}) + } + + return remoteClient.Patch(ctx, newNode, client.StrategicMergeFrom(node, mergeOptions...)) +} + +// propagateMachineTaintsToNode handles propagation of taints defined on a machine to a node. +// It makes use of the annotation clusterv1.TaintsFromMachineAnnotation to track which taints are owned by the controller. +// OnInitialization taints are only added to the node if the tracking annotation has not been set yet. +func propagateMachineTaintsToNode(node *corev1.Node, machineTaints []clusterv1.MachineTaint) (bool, error) { + changed := false + + // Get the value of the tracking annotation. If it is not set at all we also have to add the OnInitialization taints. + oldTaintsAnnotation, nodeTaintsInitialized := node.Annotations[clusterv1.TaintsFromMachineAnnotation] + + // ownedTaints contains all Always taints that the controller is owning. + ownedTaints := unmarshalMachineTaintsAnnotation(oldTaintsAnnotation) + + // newOwnedTaints will contain all Always taints from the current machine's spec. + newOwnedTaints := sets.New[string]() + onInitializationTaints := sets.New[string]() + + for _, taint := range machineTaints { + // Collect Always and OnInitialization taints to identify taints to delete. + // Separating Always taints so the tracking annotation can be updated accordingly. + switch taint.Propagation { + case clusterv1.TaintPropagationAlways: + newOwnedTaints.Insert(fmt.Sprintf("%s:%s", taint.Key, taint.Effect)) + case clusterv1.TaintPropagationOnInitialization: + onInitializationTaints.Insert(fmt.Sprintf("%s:%s", taint.Key, taint.Effect)) + } + + // Only add OnInitialization taints if the tracking annotation has not been set yet. + if taint.Propagation == clusterv1.TaintPropagationOnInitialization && nodeTaintsInitialized { + continue + } + + // Ensure the taint is set on the node and has the correct value. + if changedTaints := taints.EnsureNodeTaint(node, convertMachineTaintToCoreV1Taint(taint)); changedTaints { + changed = true + } + } + + // Calculate ownedTaints - newOwnedTaints to identify old taints which need to be deleted from the node. + taintsToDelete := ownedTaints.Difference(newOwnedTaints).Difference(onInitializationTaints) + + // Remove all identified taints from the node. + for taintToDelete := range taintsToDelete { + splitted := strings.Split(taintToDelete, ":") + if len(splitted) != 2 { + return changed, fmt.Errorf("invalid taint format: %q", taintToDelete) + } + + if removedTaint := taints.RemoveNodeTaint(node, corev1.Taint{Key: splitted[0], Effect: corev1.TaintEffect(splitted[1])}); removedTaint { + changed = true + } + } + + // Update the tracking annotation with newOwnedTaints.. + if newTaintsAnnotation := marshalMachineTaintsAnnotation(newOwnedTaints); !nodeTaintsInitialized || newTaintsAnnotation != oldTaintsAnnotation { + if node.Annotations == nil { + node.Annotations = map[string]string{} + } + node.Annotations[clusterv1.TaintsFromMachineAnnotation] = newTaintsAnnotation + changed = true + } + + return changed, nil +} + +// marshalMachineTaintsAnnotation marshals the tracking annotation value. +func marshalMachineTaintsAnnotation(ownedTaints sets.Set[string]) string { + taints := ownedTaints.UnsortedList() + slices.Sort(taints) + + return strings.Join(taints, ",") +} + +// unmarshalMachineTaintsAnnotation unmarshals the tracking annotation value. +func unmarshalMachineTaintsAnnotation(annotationValue string) sets.Set[string] { + if annotationValue == "" { + return nil + } + + return sets.New(strings.Split(annotationValue, ",")...) +} + +// convertMachineTaintToCoreV1Taint converts a MachineTaint to a corev1.Taint. +func convertMachineTaintToCoreV1Taint(machineTaint clusterv1.MachineTaint) corev1.Taint { + return corev1.Taint{ + Key: machineTaint.Key, + Value: ptr.Deref(machineTaint.Value, ""), + Effect: machineTaint.Effect, + } } // shouldNodeHaveOutdatedTaint tries to compare the revision of the owning MachineSet to the MachineDeployment. From 0c0ecce948f513da06248ea4edadcda8ce7ad9b7 Mon Sep 17 00:00:00 2001 From: Christian Schlotter Date: Fri, 24 Oct 2025 14:26:17 +0200 Subject: [PATCH 07/21] taint propagation: implement unit tests for machine controller --- .../machine_controller_noderef_test.go | 253 ++++++++++++++++++ util/test/builder/builders.go | 24 +- util/test/builder/zz_generated.deepcopy.go | 15 ++ 3 files changed, 289 insertions(+), 3 deletions(-) diff --git a/internal/controllers/machine/machine_controller_noderef_test.go b/internal/controllers/machine/machine_controller_noderef_test.go index 27163f1fc798..be34eac878b7 100644 --- a/internal/controllers/machine/machine_controller_noderef_test.go +++ b/internal/controllers/machine/machine_controller_noderef_test.go @@ -27,6 +27,7 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/client-go/tools/record" + utilfeature "k8s.io/component-base/featuregate/testing" "k8s.io/utils/ptr" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" @@ -39,6 +40,7 @@ import ( "sigs.k8s.io/cluster-api/api/core/v1beta2/index" "sigs.k8s.io/cluster-api/controllers/clustercache" "sigs.k8s.io/cluster-api/controllers/remote" + "sigs.k8s.io/cluster-api/feature" "sigs.k8s.io/cluster-api/internal/topology/ownerrefs" "sigs.k8s.io/cluster-api/util" "sigs.k8s.io/cluster-api/util/kubeconfig" @@ -59,6 +61,22 @@ func TestReconcileNode(t *testing.T) { }, } + defaultMachineWithTaints := defaultMachine.DeepCopy() + defaultMachineWithTaints.Spec.Taints = []clusterv1.MachineTaint{ + { + Key: "test-always-taint", + Value: ptr.To("test-value1"), + Effect: corev1.TaintEffectNoSchedule, + Propagation: clusterv1.TaintPropagationAlways, + }, + { + Key: "test-on-initialization-taint", + Value: ptr.To("test-value2"), + Effect: corev1.TaintEffectNoSchedule, + Propagation: clusterv1.TaintPropagationOnInitialization, + }, + } + defaultCluster := &clusterv1.Cluster{ ObjectMeta: metav1.ObjectMeta{ Name: "test-cluster", @@ -75,6 +93,7 @@ func TestReconcileNode(t *testing.T) { expectError bool expected func(g *WithT, m *clusterv1.Machine) expectNodeGetError bool + expectedNode func(g *WithT, m *corev1.Node) }{ { name: "No op if provider ID is not set", @@ -186,12 +205,87 @@ func TestReconcileNode(t *testing.T) { expectResult: ctrl.Result{}, expectError: false, }, + { + name: "node found, should propagate taints", + machine: defaultMachineWithTaints.DeepCopy(), + node: &corev1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-node-1", + }, + Spec: corev1.NodeSpec{ + ProviderID: "aws://us-east-1/test-node-1", + }, + Status: corev1.NodeStatus{ + NodeInfo: corev1.NodeSystemInfo{ + MachineID: "foo", + }, + Addresses: []corev1.NodeAddress{ + { + Type: corev1.NodeInternalIP, + Address: "1.1.1.1", + }, + }, + }, + }, + nodeGetErr: false, + expectResult: ctrl.Result{}, + expectError: false, + expectedNode: func(g *WithT, n *corev1.Node) { + g.Expect(n.Spec.Taints).To(BeComparableTo([]corev1.Taint{ + { + Key: "test-always-taint", + Value: "test-value1", + Effect: corev1.TaintEffectNoSchedule, + }, + { + Key: "test-on-initialization-taint", + Value: "test-value2", + Effect: corev1.TaintEffectNoSchedule, + }, + })) + g.Expect(n.Annotations[clusterv1.TaintsFromMachineAnnotation]).To(Equal("test-always-taint:NoSchedule")) + }, + }, + { + name: "node found, should not add taints annotation if taints feature gate is disabled", + machine: defaultMachine.DeepCopy(), // The test only enables the feature gate if machine has taints. + node: &corev1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-node-1", + }, + Spec: corev1.NodeSpec{ + ProviderID: "aws://us-east-1/test-node-1", + }, + Status: corev1.NodeStatus{ + NodeInfo: corev1.NodeSystemInfo{ + MachineID: "foo", + }, + Addresses: []corev1.NodeAddress{ + { + Type: corev1.NodeInternalIP, + Address: "1.1.1.1", + }, + }, + }, + }, + nodeGetErr: false, + expectResult: ctrl.Result{}, + expectError: false, + expectedNode: func(g *WithT, n *corev1.Node) { + g.Expect(n.Spec.Taints).To(BeEmpty()) + g.Expect(n.Annotations).ToNot(HaveKey(clusterv1.TaintsFromMachineAnnotation)) + }, + }, } for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { g := NewWithT(t) + if tc.machine.Spec.Taints != nil { + utilfeature.SetFeatureGateDuringTest(t, feature.Gates, feature.MachineTaintPropagation, true) + } + c := fake.NewClientBuilder().WithObjects(tc.machine).WithIndex(&corev1.Node{}, "spec.providerID", index.NodeByProviderID).Build() if tc.nodeGetErr { c = fake.NewClientBuilder().WithObjects(tc.machine).Build() // No Index @@ -221,6 +315,12 @@ func TestReconcileNode(t *testing.T) { } g.Expect(s.nodeGetError != nil).To(Equal(tc.expectNodeGetError)) + + if tc.expectedNode != nil { + node := &corev1.Node{} + g.Expect(c.Get(ctx, client.ObjectKeyFromObject(tc.node), node)).To(Succeed()) + tc.expectedNode(g, node) + } }) } } @@ -1452,3 +1552,156 @@ func Test_shouldNodeHaveOutdatedTaint(t *testing.T) { }) } } + +func Test_propagateMachineTaintsToNode(t *testing.T) { + alwaysTaint := clusterv1.MachineTaint{ + Key: "added-always", + Value: ptr.To("always-value"), + Effect: corev1.TaintEffectNoSchedule, + Propagation: clusterv1.TaintPropagationAlways, + } + onInitializationTaint := clusterv1.MachineTaint{ + Key: "added-on-initialization", + Value: ptr.To("on-initialization-value"), + Effect: corev1.TaintEffectNoSchedule, + Propagation: clusterv1.TaintPropagationOnInitialization, + } + + existingAlwaysTaint := clusterv1.MachineTaint{ + Key: "existing-always", + Value: ptr.To("existing-always-value"), + Effect: corev1.TaintEffectNoExecute, + Propagation: clusterv1.TaintPropagationAlways, + } + + transitionAlways := clusterv1.MachineTaint{ + Key: "transition-taint", + Value: ptr.To("transition-value"), + Effect: corev1.TaintEffectNoSchedule, + Propagation: clusterv1.TaintPropagationAlways, + } + transitionOnInitialization := transitionAlways + transitionOnInitialization.Propagation = clusterv1.TaintPropagationOnInitialization + + tests := []struct { + name string + node *corev1.Node + machineTaints []clusterv1.MachineTaint + expectedTaints []corev1.Taint + expectedAnnotation string + expectChanged bool + }{ + { + name: "no taints set, no taints to set, adds empty annotation", + node: builder.Node("").Build(), + machineTaints: []clusterv1.MachineTaint{}, + expectedTaints: nil, + expectedAnnotation: "", + expectChanged: true, + }, + { + name: "no taints set, no taints to set, keeps empty annotation", + node: builder.Node("").WithAnnotations(map[string]string{clusterv1.TaintsFromMachineAnnotation: ""}).Build(), + machineTaints: []clusterv1.MachineTaint{}, + expectedTaints: nil, + expectedAnnotation: "", + expectChanged: false, + }, + { + name: "no taints set, no taints to set, cleans up empty annotation", + node: builder.Node("").WithAnnotations(map[string]string{clusterv1.TaintsFromMachineAnnotation: "does-not-exist:NoSchedule"}).Build(), + machineTaints: []clusterv1.MachineTaint{}, + expectedTaints: []corev1.Taint{}, // The taints utility initializes an empty slice on no-op removal. + expectedAnnotation: "", + expectChanged: true, + }, + // Basic Always taint operations: + { + name: "Add missing Always taint, no tracking annotation, no other taints", + node: builder.Node("").Build(), + machineTaints: []clusterv1.MachineTaint{alwaysTaint}, + expectedTaints: []corev1.Taint{convertMachineTaintToCoreV1Taint(alwaysTaint)}, + expectedAnnotation: "added-always:NoSchedule", + expectChanged: true, + }, + { + name: "Add missing Always taint, tracking annotation, no other taints", + node: builder.Node(""). + WithAnnotations(map[string]string{clusterv1.TaintsFromMachineAnnotation: ""}).Build(), + machineTaints: []clusterv1.MachineTaint{alwaysTaint}, + expectedTaints: []corev1.Taint{convertMachineTaintToCoreV1Taint(alwaysTaint)}, + expectedAnnotation: "added-always:NoSchedule", + expectChanged: true, + }, + { + name: "Delete Always taint, tracking annotation, no other taints", + node: builder.Node(""). + WithAnnotations(map[string]string{clusterv1.TaintsFromMachineAnnotation: "existing-always:NoExecute"}). + WithTaints(convertMachineTaintToCoreV1Taint(existingAlwaysTaint)).Build(), + machineTaints: []clusterv1.MachineTaint{}, + expectedTaints: []corev1.Taint{}, + expectedAnnotation: "", + expectChanged: true, + }, + // Basic OnInitialization taint operations: + { + name: "Add missing OnInitialization taint, no tracking annotation, no other taints", + node: builder.Node("").Build(), + machineTaints: []clusterv1.MachineTaint{onInitializationTaint}, + expectedTaints: []corev1.Taint{convertMachineTaintToCoreV1Taint(onInitializationTaint)}, + expectedAnnotation: "", + expectChanged: true, + }, + { + name: "Don't add missing OnInitialization taint, tracking annotation, no other taints", + node: builder.Node(""). + WithAnnotations(map[string]string{clusterv1.TaintsFromMachineAnnotation: ""}).Build(), + machineTaints: []clusterv1.MachineTaint{onInitializationTaint}, + expectedTaints: nil, + expectedAnnotation: "", + expectChanged: false, + }, + { + name: "Don't delete OnInitialization taint, tracking annotation, no other taints", + node: builder.Node(""). + WithAnnotations(map[string]string{clusterv1.TaintsFromMachineAnnotation: ""}). + WithTaints(convertMachineTaintToCoreV1Taint(onInitializationTaint)).Build(), + machineTaints: []clusterv1.MachineTaint{}, + expectedTaints: []corev1.Taint{convertMachineTaintToCoreV1Taint(onInitializationTaint)}, + expectedAnnotation: "", + expectChanged: false, + }, + // Transitions + { + name: "Transition Always to OnInitialization should remove from annotation but be kept on the node", + node: builder.Node(""). + WithAnnotations(map[string]string{clusterv1.TaintsFromMachineAnnotation: "transition-taint:NoSchedule"}). + WithTaints(convertMachineTaintToCoreV1Taint(transitionAlways)).Build(), + machineTaints: []clusterv1.MachineTaint{transitionOnInitialization}, + expectedTaints: []corev1.Taint{convertMachineTaintToCoreV1Taint(transitionAlways)}, + expectedAnnotation: "", + expectChanged: true, + }, + { + name: "Transition OnInitialization to Always should add to annotation and be kept on the node", + node: builder.Node(""). + WithAnnotations(map[string]string{clusterv1.TaintsFromMachineAnnotation: ""}). + WithTaints(convertMachineTaintToCoreV1Taint(transitionOnInitialization)).Build(), + machineTaints: []clusterv1.MachineTaint{transitionAlways}, + expectedTaints: []corev1.Taint{convertMachineTaintToCoreV1Taint(transitionOnInitialization)}, + expectedAnnotation: "transition-taint:NoSchedule", + expectChanged: true, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + g := NewWithT(t) + changed, err := propagateMachineTaintsToNode(tt.node, tt.machineTaints) + g.Expect(err).ToNot(HaveOccurred()) + g.Expect(changed).To(Equal(tt.expectChanged)) + g.Expect(tt.node.Spec.Taints).To(Equal(tt.expectedTaints)) + g.Expect(tt.node.Annotations).To(HaveKey(clusterv1.TaintsFromMachineAnnotation)) + g.Expect(tt.node.Annotations[clusterv1.TaintsFromMachineAnnotation]).To(Equal(tt.expectedAnnotation)) + }) + } +} diff --git a/util/test/builder/builders.go b/util/test/builder/builders.go index c3baadd9bb42..5187827d3383 100644 --- a/util/test/builder/builders.go +++ b/util/test/builder/builders.go @@ -1609,8 +1609,10 @@ func (c *TestControlPlaneBuilder) Build() *unstructured.Unstructured { // NodeBuilder holds the variables required to build a Node. type NodeBuilder struct { - name string - status corev1.NodeStatus + name string + annotations map[string]string + taints []corev1.Taint + status corev1.NodeStatus } // Node returns a NodeBuilder. @@ -1620,6 +1622,18 @@ func Node(name string) *NodeBuilder { } } +// WithAnnotations adds the given annotations to the NodeBuilder. +func (n *NodeBuilder) WithAnnotations(annotations map[string]string) *NodeBuilder { + n.annotations = annotations + return n +} + +// WithTaints adds the given taints to the NodeBuilder. +func (n *NodeBuilder) WithTaints(taints ...corev1.Taint) *NodeBuilder { + n.taints = taints + return n +} + // WithStatus adds Status to the NodeBuilder. func (n *NodeBuilder) WithStatus(status corev1.NodeStatus) *NodeBuilder { n.status = status @@ -1630,7 +1644,11 @@ func (n *NodeBuilder) WithStatus(status corev1.NodeStatus) *NodeBuilder { func (n *NodeBuilder) Build() *corev1.Node { obj := &corev1.Node{ ObjectMeta: metav1.ObjectMeta{ - Name: n.name, + Name: n.name, + Annotations: n.annotations, + }, + Spec: corev1.NodeSpec{ + Taints: n.taints, }, Status: n.status, } diff --git a/util/test/builder/zz_generated.deepcopy.go b/util/test/builder/zz_generated.deepcopy.go index 74e4dc55e03d..0e269338bcd8 100644 --- a/util/test/builder/zz_generated.deepcopy.go +++ b/util/test/builder/zz_generated.deepcopy.go @@ -21,6 +21,7 @@ limitations under the License. package builder import ( + corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/util/intstr" @@ -862,6 +863,20 @@ func (in *MachineSetBuilder) DeepCopy() *MachineSetBuilder { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *NodeBuilder) DeepCopyInto(out *NodeBuilder) { *out = *in + if in.annotations != nil { + in, out := &in.annotations, &out.annotations + *out = make(map[string]string, len(*in)) + for key, val := range *in { + (*out)[key] = val + } + } + if in.taints != nil { + in, out := &in.taints, &out.taints + *out = make([]corev1.Taint, len(*in)) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) + } + } in.status.DeepCopyInto(&out.status) } From 7d1297db9b2c409b6f624498b6596244922cf740 Mon Sep 17 00:00:00 2001 From: Christian Schlotter Date: Mon, 3 Nov 2025 14:44:57 +0100 Subject: [PATCH 08/21] fixup arbitrary test --- .../internal/controllers/inplace_canupdatemachine_test.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/controlplane/kubeadm/internal/controllers/inplace_canupdatemachine_test.go b/controlplane/kubeadm/internal/controllers/inplace_canupdatemachine_test.go index 7662899f11fe..b80c120ecb9c 100644 --- a/controlplane/kubeadm/internal/controllers/inplace_canupdatemachine_test.go +++ b/controlplane/kubeadm/internal/controllers/inplace_canupdatemachine_test.go @@ -23,6 +23,7 @@ import ( "fmt" "testing" + "github.com/google/go-cmp/cmp" . "github.com/onsi/gomega" "github.com/pkg/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -312,7 +313,7 @@ func Test_canExtensionsUpdateMachine(t *testing.T) { + Version: "v1.31.0", ProviderID: "", FailureDomain: "", - ... // 3 identical fields + ... // 4 identical fields }, Status: {}, }`, @@ -465,7 +466,7 @@ func Test_canExtensionsUpdateMachine(t *testing.T) { g.Expect(err).ToNot(HaveOccurred()) } g.Expect(canUpdateMachine).To(Equal(tt.wantCanUpdateMachine)) - g.Expect(reasons).To(Equal(tt.wantReasons)) + g.Expect(reasons).To(BeEquivalentTo(tt.wantReasons), cmp.Diff(reasons, tt.wantReasons)) }) } } From e36a44f0b5638ca0b84ad220138d7f78367e3be9 Mon Sep 17 00:00:00 2001 From: Christian Schlotter Date: Mon, 3 Nov 2025 16:40:32 +0100 Subject: [PATCH 09/21] fix generate --- util/test/builder/zz_generated.deepcopy.go | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/util/test/builder/zz_generated.deepcopy.go b/util/test/builder/zz_generated.deepcopy.go index 0e269338bcd8..6813b2c1fdbe 100644 --- a/util/test/builder/zz_generated.deepcopy.go +++ b/util/test/builder/zz_generated.deepcopy.go @@ -432,6 +432,13 @@ func (in *MachineBuilder) DeepCopyInto(out *MachineBuilder) { (*out)[key] = val } } + if in.taints != nil { + in, out := &in.taints, &out.taints + *out = make([]v1beta2.MachineTaint, len(*in)) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) + } + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new MachineBuilder. @@ -499,6 +506,13 @@ func (in *MachineDeploymentBuilder) DeepCopyInto(out *MachineDeploymentBuilder) *out = new(int32) **out = **in } + if in.taints != nil { + in, out := &in.taints, &out.taints + *out = make([]v1beta2.MachineTaint, len(*in)) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) + } + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new MachineDeploymentBuilder. @@ -848,6 +862,13 @@ func (in *MachineSetBuilder) DeepCopyInto(out *MachineSetBuilder) { (*in)[i].DeepCopyInto(&(*out)[i]) } } + if in.taints != nil { + in, out := &in.taints, &out.taints + *out = make([]v1beta2.MachineTaint, len(*in)) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) + } + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new MachineSetBuilder. From 5c976eb339d9499f353a4b729c9bd8b36cdd7113 Mon Sep 17 00:00:00 2001 From: Christian Schlotter Date: Thu, 6 Nov 2025 17:47:11 +0100 Subject: [PATCH 10/21] rebase fixes --- internal/controllers/machine/machine_controller_noderef.go | 4 ++-- .../controllers/machine/machine_controller_noderef_test.go | 6 +++--- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/internal/controllers/machine/machine_controller_noderef.go b/internal/controllers/machine/machine_controller_noderef.go index a17e1a8ea59d..d4378f4193ab 100644 --- a/internal/controllers/machine/machine_controller_noderef.go +++ b/internal/controllers/machine/machine_controller_noderef.go @@ -145,7 +145,7 @@ func (r *Reconciler) reconcileNode(ctx context.Context, s *scope) (ctrl.Result, _, nodeHadInterruptibleLabel := s.node.Labels[clusterv1.InterruptibleLabel] // Reconcile node taints - if err := r.patchNode(ctx, remoteClient, s.node, nodeLabels, nodeAnnotations, s.owningMachineSet, s.owningMachineDeployment); err != nil { + if err := r.patchNode(ctx, remoteClient, s.node, nodeLabels, nodeAnnotations, machine, s.owningMachineSet, s.owningMachineDeployment); err != nil { return ctrl.Result{}, errors.Wrapf(err, "failed to reconcile Node %s", klog.KObj(s.node)) } if !nodeHadInterruptibleLabel && interruptible { @@ -248,7 +248,7 @@ func (r *Reconciler) getNode(ctx context.Context, c client.Reader, providerID st // PatchNode is required to workaround an issue on Node.Status.Address which is incorrectly annotated as patchStrategy=merge // and this causes SSA patch to fail in case there are two addresses with the same key https://github.com/kubernetes-sigs/cluster-api/issues/8417 -func (r *Reconciler) patchNode(ctx context.Context, remoteClient client.Client, node *corev1.Node, newLabels, newAnnotations map[string]string, ms *clusterv1.MachineSet, md *clusterv1.MachineDeployment) error { +func (r *Reconciler) patchNode(ctx context.Context, remoteClient client.Client, node *corev1.Node, newLabels, newAnnotations map[string]string, m *clusterv1.Machine, ms *clusterv1.MachineSet, md *clusterv1.MachineDeployment) error { newNode := node.DeepCopy() // Adds the annotations from the Machine. diff --git a/internal/controllers/machine/machine_controller_noderef_test.go b/internal/controllers/machine/machine_controller_noderef_test.go index be34eac878b7..7394ff66f9d7 100644 --- a/internal/controllers/machine/machine_controller_noderef_test.go +++ b/internal/controllers/machine/machine_controller_noderef_test.go @@ -1299,7 +1299,7 @@ func TestPatchNode(t *testing.T) { _ = env.CleanupAndWait(ctx, oldNode, machine, ms, md) }) - err := r.patchNode(ctx, env, oldNode, tc.newLabels, tc.newAnnotations, ms, md) + err := r.patchNode(ctx, env, oldNode, tc.newLabels, tc.newAnnotations, tc.machine, ms, md) g.Expect(err).ToNot(HaveOccurred()) g.Eventually(func(g Gomega) { @@ -1404,7 +1404,7 @@ func TestMultiplePatchNode(t *testing.T) { _ = env.CleanupAndWait(ctx, oldNode, machine) }) - err := r.patchNode(ctx, env, oldNode, labels, tc.newAnnotations, nil, nil) + err := r.patchNode(ctx, env, oldNode, labels, tc.newAnnotations, machine, nil, nil) g.Expect(err).ToNot(HaveOccurred()) newNode := &corev1.Node{} @@ -1418,7 +1418,7 @@ func TestMultiplePatchNode(t *testing.T) { }, 10*time.Second).Should(Succeed()) // Re-reconcile with the same metadata - err = r.patchNode(ctx, env, newNode, labels, tc.newAnnotations, nil, nil) + err = r.patchNode(ctx, env, newNode, labels, tc.newAnnotations, machine, nil, nil) g.Expect(err).ToNot(HaveOccurred()) g.Eventually(func(g Gomega) { From d1e84e03639d506f86691533c5ce2f19f7f5ed56 Mon Sep 17 00:00:00 2001 From: Christian Schlotter Date: Mon, 10 Nov 2025 10:46:57 +0100 Subject: [PATCH 11/21] Review changes v1 --- api/core/v1beta1/conversion.go | 2 +- api/core/v1beta2/common_types.go | 12 +++++---- api/core/v1beta2/machine_types.go | 6 +++-- .../inplace_canupdatemachine_test.go | 3 +-- .../experimental-features.md | 2 ++ feature/feature.go | 1 - .../machine/machine_controller_noderef.go | 17 +++++------- .../machine_controller_noderef_test.go | 26 +++++++++---------- .../machinedeployment_rollout_planner_test.go | 5 ++++ .../machinedeployment/mdutil/util.go | 1 + .../machinedeployment/mdutil/util_test.go | 7 +++++ .../machineset/machineset_controller_test.go | 19 +++++++++++++- internal/util/inplace/inplace.go | 1 + internal/webhooks/machine.go | 16 ++++++------ internal/webhooks/machine_test.go | 2 +- internal/webhooks/machinepool.go | 5 ++-- internal/webhooks/machineset_test.go | 2 +- 17 files changed, 79 insertions(+), 48 deletions(-) diff --git a/api/core/v1beta1/conversion.go b/api/core/v1beta1/conversion.go index c1007274361e..a240707da77a 100644 --- a/api/core/v1beta1/conversion.go +++ b/api/core/v1beta1/conversion.go @@ -408,11 +408,11 @@ func (src *Machine) ConvertTo(dstRaw conversion.Hub) error { // Recover other values. if ok { dst.Spec.MinReadySeconds = restored.Spec.MinReadySeconds + dst.Spec.Taints = restored.Spec.Taints // Restore the phase, this also means that any client using v1beta1 during a round-trip // won't be able to write the Phase field. But that's okay as the only client writing the Phase // field should be the Machine controller. dst.Status.Phase = restored.Status.Phase - dst.Spec.Taints = restored.Spec.Taints } return nil diff --git a/api/core/v1beta2/common_types.go b/api/core/v1beta2/common_types.go index d4fa3cc01b62..83948d7c2ba2 100644 --- a/api/core/v1beta2/common_types.go +++ b/api/core/v1beta2/common_types.go @@ -419,8 +419,9 @@ type MachineTaint struct { // value is the taint value corresponding to the taint key. // +optional + // +kubebuilder:validation:MinLength=1 // +kubebuilder:validation:MaxLength=63 - Value *string `json:"value,omitempty"` + Value string `json:"value,omitempty"` // effect is the effect for the taint. Valid values are NoSchedule, PreferNoSchedule and NoExecute. // +required @@ -428,6 +429,7 @@ type MachineTaint struct { Effect corev1.TaintEffect `json:"effect,omitempty"` // propagation defines how this taint should be propagated to nodes. + // Valid values are 'Always' and 'OnInitialization'. // Always: The taint will be continuously reconciled. If it is not set for a node, it will be added during reconciliation. // OnInitialization: The taint will be added during node initialization. If it gets removed from the node later on it will not get added again. // +required @@ -439,18 +441,18 @@ type MachineTaint struct { type MachineTaintPropagation string const ( - // TaintPropagationAlways means the taint should be continuously reconciled and kept on the node. + // MachineTaintPropagationAlways means the taint should be continuously reconciled and kept on the node. // - If an Always taint is added to the Machine, the taint will be added to the node. // - If an Always taint is removed from the Machine, the taint will be removed from the node. // - If an OnInitialization taint is changed to Always, the Machine controller will ensure the taint is set on the node. // - If an Always taint is removed from the node, it will be re-added during reconciliation. - TaintPropagationAlways MachineTaintPropagation = "Always" + MachineTaintPropagationAlways MachineTaintPropagation = "Always" - // TaintPropagationOnInitialization means the taint should be set once during initialization and then + // MachineTaintPropagationOnInitialization means the taint should be set once during initialization and then // left alone. // - If an OnInitialization taint is added to the Machine, the taint will only be added to the node on initialization. // - If an OnInitialization taint is removed from the Machine nothing will be changed on the node. // - If an Always taint is changed to OnInitialization, the taint will only be added to the node on initialization. // - If an OnInitialization taint is removed from the node, it will not be re-added during reconciliation. - TaintPropagationOnInitialization MachineTaintPropagation = "OnInitialization" + MachineTaintPropagationOnInitialization MachineTaintPropagation = "OnInitialization" ) diff --git a/api/core/v1beta2/machine_types.go b/api/core/v1beta2/machine_types.go index 2cc7ec4e2ec5..44a6f794f9c4 100644 --- a/api/core/v1beta2/machine_types.go +++ b/api/core/v1beta2/machine_types.go @@ -490,11 +490,13 @@ type MachineSpec struct { Deletion MachineDeletionSpec `json:"deletion,omitempty,omitzero"` // taints are the node taints that Cluster API will manage. - // This list is not necessarily complete: other Kubernetes components may add or remove other taints. + // This list is not necessarily complete: other Kubernetes components may add or remove other taints from nodes, + // e.g. the node controller might add the node.kubernetes.io/not-ready taint. // Only those taints defined in this list will be added or removed by core Cluster API controllers. // + // There can be at most 64 taints. + // // 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. // +optional // +listType=map // +listMapKey=key diff --git a/controlplane/kubeadm/internal/controllers/inplace_canupdatemachine_test.go b/controlplane/kubeadm/internal/controllers/inplace_canupdatemachine_test.go index b80c120ecb9c..3d3f09797d9d 100644 --- a/controlplane/kubeadm/internal/controllers/inplace_canupdatemachine_test.go +++ b/controlplane/kubeadm/internal/controllers/inplace_canupdatemachine_test.go @@ -23,7 +23,6 @@ import ( "fmt" "testing" - "github.com/google/go-cmp/cmp" . "github.com/onsi/gomega" "github.com/pkg/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -466,7 +465,7 @@ func Test_canExtensionsUpdateMachine(t *testing.T) { g.Expect(err).ToNot(HaveOccurred()) } g.Expect(canUpdateMachine).To(Equal(tt.wantCanUpdateMachine)) - g.Expect(reasons).To(BeEquivalentTo(tt.wantReasons), cmp.Diff(reasons, tt.wantReasons)) + g.Expect(reasons).To(BeComparableTo(tt.wantReasons)) }) } } diff --git a/docs/book/src/tasks/experimental-features/experimental-features.md b/docs/book/src/tasks/experimental-features/experimental-features.md index 04eac58daab9..a74b4e0c2420 100644 --- a/docs/book/src/tasks/experimental-features/experimental-features.md +++ b/docs/book/src/tasks/experimental-features/experimental-features.md @@ -14,6 +14,8 @@ Currently Cluster API has the following experimental features: * `ClusterTopology` (env var: `CLUSTER_TOPOLOGY`): [ClusterClass](./cluster-class/index.md) * `RuntimeSDK` (env var: `EXP_RUNTIME_SDK`): [RuntimeSDK](./runtime-sdk/index.md) * `KubeadmBootstrapFormatIgnition` (env var: `EXP_KUBEADM_BOOTSTRAP_FORMAT_IGNITION`): [Ignition](./ignition.md) +* `MachineTaintPropagation` (env var: `EXP_MACHINE_TAINT_PROPAGATION`): + * Allows in-place propagation of taints to nodes using the taint fields within Machines, MachineSets, and MachineDeployments. ## Enabling Experimental Features for Management Clusters Started with clusterctl diff --git a/feature/feature.go b/feature/feature.go index 4ad93cf0f721..a1855d01a065 100644 --- a/feature/feature.go +++ b/feature/feature.go @@ -74,7 +74,6 @@ const ( InPlaceUpdates featuregate.Feature = "InPlaceUpdates" // MachineTaintPropagation is a feature gate for the machine taint propagation functionality. - // See docs/proposals/20250513-propogate-taints.md for more details. // // alpha: v1.12 MachineTaintPropagation featuregate.Feature = "MachineTaintPropagation" diff --git a/internal/controllers/machine/machine_controller_noderef.go b/internal/controllers/machine/machine_controller_noderef.go index d4378f4193ab..c01975c8e3f3 100644 --- a/internal/controllers/machine/machine_controller_noderef.go +++ b/internal/controllers/machine/machine_controller_noderef.go @@ -29,7 +29,6 @@ import ( "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apimachinery/pkg/util/sets" "k8s.io/klog/v2" - "k8s.io/utils/ptr" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" @@ -342,7 +341,7 @@ func (r *Reconciler) patchNode(ctx context.Context, remoteClient client.Client, if feature.Gates.Enabled(feature.MachineTaintPropagation) { var err error if propagateTaintsChanges, err = propagateMachineTaintsToNode(newNode, m.Spec.Taints); err != nil { - return errors.Wrapf(err, "failed to propagate machine taints to node %s", klog.KRef("", node.Name)) + return errors.Wrapf(err, "failed to propagate Machine taints to Node %s", klog.KObj(node)) } } @@ -361,10 +360,8 @@ func (r *Reconciler) patchNode(ctx context.Context, remoteClient client.Client, return nil } - var mergeOptions []client.MergeFromOption - if feature.Gates.Enabled(feature.MachineTaintPropagation) { - mergeOptions = append(mergeOptions, client.MergeFromWithOptimisticLock{}) - } + // Use optimistic locking to avoid conflicts with other controllers. + mergeOptions := []client.MergeFromOption{client.MergeFromWithOptimisticLock{}} return remoteClient.Patch(ctx, newNode, client.StrategicMergeFrom(node, mergeOptions...)) } @@ -389,14 +386,14 @@ func propagateMachineTaintsToNode(node *corev1.Node, machineTaints []clusterv1.M // Collect Always and OnInitialization taints to identify taints to delete. // Separating Always taints so the tracking annotation can be updated accordingly. switch taint.Propagation { - case clusterv1.TaintPropagationAlways: + case clusterv1.MachineTaintPropagationAlways: newOwnedTaints.Insert(fmt.Sprintf("%s:%s", taint.Key, taint.Effect)) - case clusterv1.TaintPropagationOnInitialization: + case clusterv1.MachineTaintPropagationOnInitialization: onInitializationTaints.Insert(fmt.Sprintf("%s:%s", taint.Key, taint.Effect)) } // Only add OnInitialization taints if the tracking annotation has not been set yet. - if taint.Propagation == clusterv1.TaintPropagationOnInitialization && nodeTaintsInitialized { + if taint.Propagation == clusterv1.MachineTaintPropagationOnInitialization && nodeTaintsInitialized { continue } @@ -454,7 +451,7 @@ func unmarshalMachineTaintsAnnotation(annotationValue string) sets.Set[string] { func convertMachineTaintToCoreV1Taint(machineTaint clusterv1.MachineTaint) corev1.Taint { return corev1.Taint{ Key: machineTaint.Key, - Value: ptr.Deref(machineTaint.Value, ""), + Value: machineTaint.Value, Effect: machineTaint.Effect, } } diff --git a/internal/controllers/machine/machine_controller_noderef_test.go b/internal/controllers/machine/machine_controller_noderef_test.go index 7394ff66f9d7..2004334d96f0 100644 --- a/internal/controllers/machine/machine_controller_noderef_test.go +++ b/internal/controllers/machine/machine_controller_noderef_test.go @@ -65,15 +65,15 @@ func TestReconcileNode(t *testing.T) { defaultMachineWithTaints.Spec.Taints = []clusterv1.MachineTaint{ { Key: "test-always-taint", - Value: ptr.To("test-value1"), + Value: "test-value1", Effect: corev1.TaintEffectNoSchedule, - Propagation: clusterv1.TaintPropagationAlways, + Propagation: clusterv1.MachineTaintPropagationAlways, }, { Key: "test-on-initialization-taint", - Value: ptr.To("test-value2"), + Value: "test-value2", Effect: corev1.TaintEffectNoSchedule, - Propagation: clusterv1.TaintPropagationOnInitialization, + Propagation: clusterv1.MachineTaintPropagationOnInitialization, }, } @@ -1556,32 +1556,32 @@ func Test_shouldNodeHaveOutdatedTaint(t *testing.T) { func Test_propagateMachineTaintsToNode(t *testing.T) { alwaysTaint := clusterv1.MachineTaint{ Key: "added-always", - Value: ptr.To("always-value"), + Value: "always-value", Effect: corev1.TaintEffectNoSchedule, - Propagation: clusterv1.TaintPropagationAlways, + Propagation: clusterv1.MachineTaintPropagationAlways, } onInitializationTaint := clusterv1.MachineTaint{ Key: "added-on-initialization", - Value: ptr.To("on-initialization-value"), + Value: "on-initialization-value", Effect: corev1.TaintEffectNoSchedule, - Propagation: clusterv1.TaintPropagationOnInitialization, + Propagation: clusterv1.MachineTaintPropagationOnInitialization, } existingAlwaysTaint := clusterv1.MachineTaint{ Key: "existing-always", - Value: ptr.To("existing-always-value"), + Value: "existing-always-value", Effect: corev1.TaintEffectNoExecute, - Propagation: clusterv1.TaintPropagationAlways, + Propagation: clusterv1.MachineTaintPropagationAlways, } transitionAlways := clusterv1.MachineTaint{ Key: "transition-taint", - Value: ptr.To("transition-value"), + Value: "transition-value", Effect: corev1.TaintEffectNoSchedule, - Propagation: clusterv1.TaintPropagationAlways, + Propagation: clusterv1.MachineTaintPropagationAlways, } transitionOnInitialization := transitionAlways - transitionOnInitialization.Propagation = clusterv1.TaintPropagationOnInitialization + transitionOnInitialization.Propagation = clusterv1.MachineTaintPropagationOnInitialization tests := []struct { name string diff --git a/internal/controllers/machinedeployment/machinedeployment_rollout_planner_test.go b/internal/controllers/machinedeployment/machinedeployment_rollout_planner_test.go index 5abea19eab4f..e4e3868acb88 100644 --- a/internal/controllers/machinedeployment/machinedeployment_rollout_planner_test.go +++ b/internal/controllers/machinedeployment/machinedeployment_rollout_planner_test.go @@ -282,6 +282,9 @@ func TestComputeDesiredMS(t *testing.T) { NodeVolumeDetachTimeoutSeconds: duration10s, NodeDeletionTimeoutSeconds: duration10s, }, + Taints: []clusterv1.MachineTaint{ + {Key: "taint-key", Value: "taint-value", Effect: corev1.TaintEffectNoSchedule, Propagation: clusterv1.MachineTaintPropagationAlways}, + }, }, }, }, @@ -375,6 +378,7 @@ func TestComputeDesiredMS(t *testing.T) { NodeVolumeDetachTimeoutSeconds: nil, NodeDeletionTimeoutSeconds: nil, }, + Taints: nil, }, }, }, @@ -398,6 +402,7 @@ func TestComputeDesiredMS(t *testing.T) { expectedMS.Spec.Template.Spec.Deletion.NodeDrainTimeoutSeconds = deployment.Spec.Template.Spec.Deletion.NodeDrainTimeoutSeconds expectedMS.Spec.Template.Spec.Deletion.NodeDeletionTimeoutSeconds = deployment.Spec.Template.Spec.Deletion.NodeDeletionTimeoutSeconds expectedMS.Spec.Template.Spec.Deletion.NodeVolumeDetachTimeoutSeconds = deployment.Spec.Template.Spec.Deletion.NodeVolumeDetachTimeoutSeconds + expectedMS.Spec.Template.Spec.Taints = deployment.Spec.Template.Spec.Taints g := NewWithT(t) actualMS, err := computeDesiredMS(ctx, deployment, currentMS) diff --git a/internal/controllers/machinedeployment/mdutil/util.go b/internal/controllers/machinedeployment/mdutil/util.go index f98d4ba4475f..80c4a536c273 100644 --- a/internal/controllers/machinedeployment/mdutil/util.go +++ b/internal/controllers/machinedeployment/mdutil/util.go @@ -440,6 +440,7 @@ func MachineTemplateDeepCopyRolloutFields(template *clusterv1.MachineTemplateSpe spec.Deletion.NodeDrainTimeoutSeconds = nil spec.Deletion.NodeVolumeDetachTimeoutSeconds = nil spec.Deletion.NodeDeletionTimeoutSeconds = nil + spec.Taints = nil return templateCopy } diff --git a/internal/controllers/machinedeployment/mdutil/util_test.go b/internal/controllers/machinedeployment/mdutil/util_test.go index b8c9f9443868..7a57125a56a2 100644 --- a/internal/controllers/machinedeployment/mdutil/util_test.go +++ b/internal/controllers/machinedeployment/mdutil/util_test.go @@ -202,6 +202,9 @@ func TestMachineTemplateUpToDate(t *testing.T) { APIGroup: clusterv1.GroupVersionBootstrap.Group, }, }, + Taints: []clusterv1.MachineTaint{ + {Key: "taint-key", Value: "taint-value", Effect: corev1.TaintEffectNoSchedule, Propagation: clusterv1.MachineTaintPropagationAlways}, + }, }, } @@ -225,6 +228,10 @@ func TestMachineTemplateUpToDate(t *testing.T) { machineTemplateWithDifferentInPlaceMutableSpecFields.Spec.Deletion.NodeDeletionTimeoutSeconds = ptr.To(int32(20)) machineTemplateWithDifferentInPlaceMutableSpecFields.Spec.Deletion.NodeVolumeDetachTimeoutSeconds = ptr.To(int32(20)) machineTemplateWithDifferentInPlaceMutableSpecFields.Spec.MinReadySeconds = ptr.To[int32](20) + machineTemplateWithDifferentInPlaceMutableSpecFields.Spec.Taints = []clusterv1.MachineTaint{ + {Key: "taint-key", Value: "taint-value", Effect: corev1.TaintEffectNoSchedule, Propagation: clusterv1.MachineTaintPropagationAlways}, + {Key: "other-key", Value: "other-value", Effect: corev1.TaintEffectNoExecute, Propagation: clusterv1.MachineTaintPropagationAlways}, + } machineTemplateWithDifferentClusterName := machineTemplate.DeepCopy() machineTemplateWithDifferentClusterName.Spec.ClusterName = "cluster2" diff --git a/internal/controllers/machineset/machineset_controller_test.go b/internal/controllers/machineset/machineset_controller_test.go index 9e4b11a7bd54..532d75f2952e 100644 --- a/internal/controllers/machineset/machineset_controller_test.go +++ b/internal/controllers/machineset/machineset_controller_test.go @@ -37,6 +37,7 @@ import ( "k8s.io/apimachinery/pkg/util/intstr" "k8s.io/apimachinery/pkg/util/sets" "k8s.io/client-go/tools/record" + utilfeature "k8s.io/component-base/featuregate/testing" "k8s.io/utils/ptr" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" @@ -48,6 +49,7 @@ import ( clusterv1 "sigs.k8s.io/cluster-api/api/core/v1beta2" runtimev1 "sigs.k8s.io/cluster-api/api/runtime/v1beta2" "sigs.k8s.io/cluster-api/controllers/external" + "sigs.k8s.io/cluster-api/feature" "sigs.k8s.io/cluster-api/internal/contract" "sigs.k8s.io/cluster-api/internal/util/ssa" "sigs.k8s.io/cluster-api/util" @@ -61,6 +63,8 @@ var _ reconcile.Reconciler = &Reconciler{} func TestMachineSetReconciler(t *testing.T) { setup := func(t *testing.T, g *WithT) (*corev1.Namespace, *clusterv1.Cluster) { + utilfeature.SetFeatureGateDuringTest(t, feature.Gates, feature.MachineTaintPropagation, true) + t.Helper() t.Log("Creating the namespace") @@ -118,6 +122,9 @@ func TestMachineSetReconciler(t *testing.T) { duration10m := ptr.To(int32(10 * 60)) duration5m := ptr.To(int32(5 * 60)) + machineTaints := []clusterv1.MachineTaint{ + {Key: "taint-key", Value: "taint-value", Effect: corev1.TaintEffectNoSchedule, Propagation: clusterv1.MachineTaintPropagationAlways}, + } replicas := int32(2) version := "v1.14.2" machineTemplateSpec := clusterv1.MachineTemplateSpec{ @@ -152,6 +159,7 @@ func TestMachineSetReconciler(t *testing.T) { NodeVolumeDetachTimeoutSeconds: duration10m, }, MinReadySeconds: ptr.To[int32](0), + Taints: machineTaints, }, } @@ -1208,6 +1216,8 @@ func TestMachineSetReconciler_updateStatusResizedCondition(t *testing.T) { } func TestMachineSetReconciler_syncMachines(t *testing.T) { + utilfeature.SetFeatureGateDuringTest(t, feature.Gates, feature.MachineTaintPropagation, true) + teardown := func(t *testing.T, g *WithT, ns *corev1.Namespace, cluster *clusterv1.Cluster) { t.Helper() @@ -1563,6 +1573,10 @@ func TestMachineSetReconciler_syncMachines(t *testing.T) { ms.Spec.Template.Spec.Deletion.NodeDeletionTimeoutSeconds = duration10s ms.Spec.Template.Spec.Deletion.NodeVolumeDetachTimeoutSeconds = duration10s ms.Spec.Template.Spec.MinReadySeconds = ptr.To[int32](10) + machineTaints := []clusterv1.MachineTaint{ + {Key: "taint-key", Value: "taint-value", Effect: corev1.TaintEffectNoSchedule, Propagation: clusterv1.MachineTaintPropagationAlways}, + } + ms.Spec.Template.Spec.Taints = machineTaints s = &scope{ machineSet: ms, machines: []*clusterv1.Machine{updatedInPlaceMutatingMachine, deletingMachine}, @@ -1581,6 +1595,7 @@ func TestMachineSetReconciler_syncMachines(t *testing.T) { g.Expect(updatedInPlaceMutatingMachine.Spec.Deletion.NodeVolumeDetachTimeoutSeconds).Should(Equal(ms.Spec.Template.Spec.Deletion.NodeVolumeDetachTimeoutSeconds)) g.Expect(updatedInPlaceMutatingMachine.Spec.MinReadySeconds).Should(Equal(ms.Spec.Template.Spec.MinReadySeconds)) g.Expect(updatedInPlaceMutatingMachine.Spec.ReadinessGates).Should(Equal(readinessGates)) + g.Expect(updatedInPlaceMutatingMachine.Spec.Taints).Should(Equal(machineTaints)) // Verify in-place mutable fields are updated on InfrastructureMachine updatedInfraMachine = infraMachine.DeepCopy() @@ -1615,7 +1630,7 @@ func TestMachineSetReconciler_syncMachines(t *testing.T) { Manager: "manager", Operation: metav1.ManagedFieldsOperationUpdate, APIVersion: clusterv1.GroupVersion.String(), - FieldsV1: "{\"f:spec\":{\"f:deletion\":{\"f:nodeDrainTimeoutSeconds\":{},\"f:nodeVolumeDetachTimeoutSeconds\":{}},\"f:minReadySeconds\":{},\"f:readinessGates\":{\".\":{},\"k:{\\\"conditionType\\\":\\\"foo\\\"}\":{\".\":{},\"f:conditionType\":{}}}}}", + FieldsV1: "{\"f:spec\":{\"f:deletion\":{\"f:nodeDrainTimeoutSeconds\":{},\"f:nodeVolumeDetachTimeoutSeconds\":{}},\"f:minReadySeconds\":{},\"f:readinessGates\":{\".\":{},\"k:{\\\"conditionType\\\":\\\"foo\\\"}\":{\".\":{},\"f:conditionType\":{}}},\"f:taints\":{\".\":{},\"k:{\\\"effect\\\":\\\"NoSchedule\\\",\\\"key\\\":\\\"taint-key\\\"}\":{\".\":{},\"f:effect\":{},\"f:key\":{},\"f:propagation\":{},\"f:value\":{}}}}}", }, { // manager owns status. Manager: "manager", @@ -1637,12 +1652,14 @@ func TestMachineSetReconciler_syncMachines(t *testing.T) { g.Expect(updatedDeletingMachine.Spec.Deletion.NodeVolumeDetachTimeoutSeconds).Should(Equal(ms.Spec.Template.Spec.Deletion.NodeVolumeDetachTimeoutSeconds)) g.Expect(updatedDeletingMachine.Spec.MinReadySeconds).Should(Equal(ms.Spec.Template.Spec.MinReadySeconds)) g.Expect(updatedDeletingMachine.Spec.ReadinessGates).Should(Equal(readinessGates)) + g.Expect(updatedDeletingMachine.Spec.Taints).Should(Equal(machineTaints)) // Verify the machine spec is otherwise unchanged. deletingMachine.Spec.Deletion.NodeDrainTimeoutSeconds = ms.Spec.Template.Spec.Deletion.NodeDrainTimeoutSeconds deletingMachine.Spec.Deletion.NodeDeletionTimeoutSeconds = ms.Spec.Template.Spec.Deletion.NodeDeletionTimeoutSeconds deletingMachine.Spec.Deletion.NodeVolumeDetachTimeoutSeconds = ms.Spec.Template.Spec.Deletion.NodeVolumeDetachTimeoutSeconds deletingMachine.Spec.MinReadySeconds = ms.Spec.Template.Spec.MinReadySeconds deletingMachine.Spec.ReadinessGates = ms.Spec.Template.Spec.ReadinessGates + deletingMachine.Spec.Taints = machineTaints g.Expect(updatedDeletingMachine.Spec).Should(BeComparableTo(deletingMachine.Spec)) } diff --git a/internal/util/inplace/inplace.go b/internal/util/inplace/inplace.go index 05d22c739193..6de704efc361 100644 --- a/internal/util/inplace/inplace.go +++ b/internal/util/inplace/inplace.go @@ -63,6 +63,7 @@ func CleanupMachineSpecForDiff(spec *clusterv1.MachineSpec) *clusterv1.MachineSp spec.Deletion.NodeDrainTimeoutSeconds = nil spec.Deletion.NodeVolumeDetachTimeoutSeconds = nil spec.Deletion.NodeDeletionTimeoutSeconds = nil + spec.Taints = nil return spec } diff --git a/internal/webhooks/machine.go b/internal/webhooks/machine.go index 86c61f4b6b42..b666ca46bffb 100644 --- a/internal/webhooks/machine.go +++ b/internal/webhooks/machine.go @@ -161,12 +161,12 @@ func validateMachineTaints(taints []clusterv1.MachineTaint, taintsPath *field.Pa idxPath := taintsPath.Index(i) // Validate key syntax. - if errs := metav1validation.ValidateLabelName(taint.Key, idxPath); len(errs) > 0 { + if errs := metav1validation.ValidateLabelName(taint.Key, idxPath.Child("key")); len(errs) > 0 { allErrs = append(allErrs, errs...) } // Validate value syntax. - if errs := validation.IsValidLabelValue(ptr.Deref(taint.Value, "")); len(errs) > 0 { + if errs := validation.IsValidLabelValue(taint.Value); len(errs) > 0 { allErrs = append(allErrs, field.Invalid(idxPath.Child("value"), taint.Value, strings.Join(errs, ";"))) } @@ -174,11 +174,11 @@ func validateMachineTaints(taints []clusterv1.MachineTaint, taintsPath *field.Pa switch { // Validate for keys which are reserved for usage by the cluster-api or providers. - case taint.Key == "node.cluster.x-k8s.io/uninitialized": - allErrs = append(allErrs, field.Invalid(idxPath.Child("key"), taint.Key, "taint key must not be node.cluster.x-k8s.io/uninitialized")) - case taint.Key == "node.cluster.x-k8s.io/outdated-revision": - allErrs = append(allErrs, field.Invalid(idxPath.Child("key"), taint.Key, "taint key must not be node.cluster.x-k8s.io/uninitialized or node.cluster.x-k8s.io/outdated-revision")) - // Validate for key's which are reserved for usage by the node or node-lifecycle-controller, but allow `node.kubernetes.io/out-of-service`. + case taint.Key == clusterv1.NodeUninitializedTaint.Key: + allErrs = append(allErrs, field.Invalid(idxPath.Child("key"), taint.Key, "taint key is not allowed")) + case taint.Key == clusterv1.NodeOutdatedRevisionTaint.Key: + allErrs = append(allErrs, field.Invalid(idxPath.Child("key"), taint.Key, "taint key is not allowed")) + // Validate for keys which are reserved for usage by the node or node-lifecycle-controller, but allow `node.kubernetes.io/out-of-service`. case strings.HasPrefix(taint.Key, "node.kubernetes.io/") && taint.Key != "node.kubernetes.io/out-of-service": allErrs = append(allErrs, field.Invalid(idxPath.Child("key"), taint.Key, "taint key must not have the prefix node.kubernetes.io/, except for node.kubernetes.io/out-of-service")) // Validate for keys which are reserved for usage by the cloud-controller-manager or kubelet. @@ -212,7 +212,7 @@ func validateMachineTaintsForWorkers(taints []clusterv1.MachineTaint, machine *c idxPath := taintsPath.Index(i) if taint.Key == "node-role.kubernetes.io/control-plane" { - allErrs = append(allErrs, field.Invalid(idxPath.Child("key"), taint.Key, "taint is not allowed for worker machines")) + allErrs = append(allErrs, field.Invalid(idxPath.Child("key"), taint.Key, "taint is not allowed for worker Machines")) } } diff --git a/internal/webhooks/machine_test.go b/internal/webhooks/machine_test.go index a6de7a33cbb5..80138cf7679f 100644 --- a/internal/webhooks/machine_test.go +++ b/internal/webhooks/machine_test.go @@ -286,7 +286,7 @@ func TestMachineTaintValidation(t *testing.T) { expectErr: false, }, { - name: "should block taint with keyprefix node.cloudprovider.kubernetes.io/", + name: "should block taint with key prefix node.cloudprovider.kubernetes.io/", featureEnabled: true, machine: m.DeepCopy().WithTaints(clusterv1.MachineTaint{ Key: "node.cloudprovider.kubernetes.io/some-taint", Effect: corev1.TaintEffectNoSchedule, diff --git a/internal/webhooks/machinepool.go b/internal/webhooks/machinepool.go index e1bd2be347fd..784d1303546a 100644 --- a/internal/webhooks/machinepool.go +++ b/internal/webhooks/machinepool.go @@ -190,10 +190,9 @@ func (webhook *MachinePool) validate(oldObj, newObj *clusterv1.MachinePool) erro } if len(newObj.Spec.Template.Spec.Taints) > 0 { + allErrs = append(allErrs, field.Forbidden(specPath.Child("taints"), "taints feature for MachinePools is not yet implemented")) if !feature.Gates.Enabled(feature.MachineTaintPropagation) { - allErrs = append(allErrs, field.Forbidden(specPath.Child("taints"), "taints are not allowed to be set when the feature gate MachineTaintPropagation is disabled")) - } else { - allErrs = append(allErrs, field.Forbidden(specPath.Child("taints"), "taints feature for MachinePools is not yet implemented")) + allErrs = append(allErrs, field.Forbidden(specPath.Child("taints"), "taints are not allowed to be set when the feature gate MachineTaintPropagation is disabled")) } } diff --git a/internal/webhooks/machineset_test.go b/internal/webhooks/machineset_test.go index 6e55f64b1a3a..d75a0a38b554 100644 --- a/internal/webhooks/machineset_test.go +++ b/internal/webhooks/machineset_test.go @@ -803,7 +803,7 @@ func TestMachineSetTaintValidation(t *testing.T) { expectErr: false, }, { - name: "should block taint with keyprefix node.cloudprovider.kubernetes.io/", + name: "should block taint with key prefix node.cloudprovider.kubernetes.io/", featureEnabled: true, machineSet: ms.DeepCopy().WithTaints(clusterv1.MachineTaint{ Key: "node.cloudprovider.kubernetes.io/some-taint", Effect: corev1.TaintEffectNoSchedule, From 53cf6f6b00e4b098fb910ca1f2b59019d742560a Mon Sep 17 00:00:00 2001 From: Christian Schlotter Date: Mon, 10 Nov 2025 10:48:34 +0100 Subject: [PATCH 12/21] make generate --- api/core/v1beta2/zz_generated.deepcopy.go | 9 +-------- api/core/v1beta2/zz_generated.openapi.go | 4 ++-- .../bases/cluster.x-k8s.io_machinedeployments.yaml | 8 ++++++-- config/crd/bases/cluster.x-k8s.io_machinepools.yaml | 8 ++++++-- config/crd/bases/cluster.x-k8s.io_machines.yaml | 8 ++++++-- config/crd/bases/cluster.x-k8s.io_machinesets.yaml | 8 ++++++-- util/test/builder/zz_generated.deepcopy.go | 12 +++--------- 7 files changed, 30 insertions(+), 27 deletions(-) diff --git a/api/core/v1beta2/zz_generated.deepcopy.go b/api/core/v1beta2/zz_generated.deepcopy.go index c6e7b4d75a5b..5adb8a56c456 100644 --- a/api/core/v1beta2/zz_generated.deepcopy.go +++ b/api/core/v1beta2/zz_generated.deepcopy.go @@ -3444,9 +3444,7 @@ func (in *MachineSpec) DeepCopyInto(out *MachineSpec) { if in.Taints != nil { in, out := &in.Taints, &out.Taints *out = make([]MachineTaint, len(*in)) - for i := range *in { - (*in)[i].DeepCopyInto(&(*out)[i]) - } + copy(*out, *in) } } @@ -3509,11 +3507,6 @@ func (in *MachineStatus) DeepCopy() *MachineStatus { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *MachineTaint) DeepCopyInto(out *MachineTaint) { *out = *in - if in.Value != nil { - in, out := &in.Value, &out.Value - *out = new(string) - **out = **in - } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new MachineTaint. diff --git a/api/core/v1beta2/zz_generated.openapi.go b/api/core/v1beta2/zz_generated.openapi.go index 83e0fcebed46..fd2d88064b28 100644 --- a/api/core/v1beta2/zz_generated.openapi.go +++ b/api/core/v1beta2/zz_generated.openapi.go @@ -6175,7 +6175,7 @@ func schema_cluster_api_api_core_v1beta2_MachineSpec(ref common.ReferenceCallbac }, }, SchemaProps: spec.SchemaProps{ - Description: "taints are the node taints that Cluster API will manage. This list is not necessarily complete: other Kubernetes components may add or remove other taints. Only those taints defined in this list will be added or removed by core Cluster API controllers.\n\nNOTE: 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.", + Description: "taints are the node taints that Cluster API will manage. This list is not necessarily complete: other Kubernetes components may add or remove other taints from nodes, e.g. the node controller might add the node.kubernetes.io/not-ready taint. Only those taints defined in this list will be added or removed by core Cluster API controllers.\n\nThere can be at most 64 taints.\n\nNOTE: This list is implemented as a \"map\" type, meaning that individual elements can be managed by different owners.", Type: []string{"array"}, Items: &spec.SchemaOrArray{ Schema: &spec.Schema{ @@ -6335,7 +6335,7 @@ func schema_cluster_api_api_core_v1beta2_MachineTaint(ref common.ReferenceCallba }, "propagation": { SchemaProps: spec.SchemaProps{ - Description: "propagation defines how this taint should be propagated to nodes. Always: The taint will be continuously reconciled. If it is not set for a node, it will be added during reconciliation. OnInitialization: The taint will be added during node initialization. If it gets removed from the node later on it will not get added again.", + Description: "propagation defines how this taint should be propagated to nodes. Valid values are 'Always' and 'OnInitialization'. Always: The taint will be continuously reconciled. If it is not set for a node, it will be added during reconciliation. OnInitialization: The taint will be added during node initialization. If it gets removed from the node later on it will not get added again.", Type: []string{"string"}, Format: "", }, diff --git a/config/crd/bases/cluster.x-k8s.io_machinedeployments.yaml b/config/crd/bases/cluster.x-k8s.io_machinedeployments.yaml index e20be8b4f70d..ec03086ab643 100644 --- a/config/crd/bases/cluster.x-k8s.io_machinedeployments.yaml +++ b/config/crd/bases/cluster.x-k8s.io_machinedeployments.yaml @@ -2343,11 +2343,13 @@ spec: taints: description: |- taints are the node taints that Cluster API will manage. - This list is not necessarily complete: other Kubernetes components may add or remove other taints. + This list is not necessarily complete: other Kubernetes components may add or remove other taints from nodes, + e.g. the node controller might add the node.kubernetes.io/not-ready taint. Only those taints defined in this list will be added or removed by core Cluster API controllers. + There can be at most 64 taints. + 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. items: description: MachineTaint defines a taint equivalent to corev1.Taint, but additionally having a propagation field. @@ -2369,6 +2371,7 @@ spec: propagation: description: |- propagation defines how this taint should be propagated to nodes. + Valid values are 'Always' and 'OnInitialization'. Always: The taint will be continuously reconciled. If it is not set for a node, it will be added during reconciliation. OnInitialization: The taint will be added during node initialization. If it gets removed from the node later on it will not get added again. enum: @@ -2379,6 +2382,7 @@ spec: description: value is the taint value corresponding to the taint key. maxLength: 63 + minLength: 1 type: string required: - effect diff --git a/config/crd/bases/cluster.x-k8s.io_machinepools.yaml b/config/crd/bases/cluster.x-k8s.io_machinepools.yaml index 9d7770be7709..17369f34a143 100644 --- a/config/crd/bases/cluster.x-k8s.io_machinepools.yaml +++ b/config/crd/bases/cluster.x-k8s.io_machinepools.yaml @@ -1947,11 +1947,13 @@ spec: taints: description: |- taints are the node taints that Cluster API will manage. - This list is not necessarily complete: other Kubernetes components may add or remove other taints. + This list is not necessarily complete: other Kubernetes components may add or remove other taints from nodes, + e.g. the node controller might add the node.kubernetes.io/not-ready taint. Only those taints defined in this list will be added or removed by core Cluster API controllers. + There can be at most 64 taints. + 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. items: description: MachineTaint defines a taint equivalent to corev1.Taint, but additionally having a propagation field. @@ -1973,6 +1975,7 @@ spec: propagation: description: |- propagation defines how this taint should be propagated to nodes. + Valid values are 'Always' and 'OnInitialization'. Always: The taint will be continuously reconciled. If it is not set for a node, it will be added during reconciliation. OnInitialization: The taint will be added during node initialization. If it gets removed from the node later on it will not get added again. enum: @@ -1983,6 +1986,7 @@ spec: description: value is the taint value corresponding to the taint key. maxLength: 63 + minLength: 1 type: string required: - effect diff --git a/config/crd/bases/cluster.x-k8s.io_machines.yaml b/config/crd/bases/cluster.x-k8s.io_machines.yaml index c8637832f267..ed9fa1b73290 100644 --- a/config/crd/bases/cluster.x-k8s.io_machines.yaml +++ b/config/crd/bases/cluster.x-k8s.io_machines.yaml @@ -1757,11 +1757,13 @@ spec: taints: description: |- taints are the node taints that Cluster API will manage. - This list is not necessarily complete: other Kubernetes components may add or remove other taints. + This list is not necessarily complete: other Kubernetes components may add or remove other taints from nodes, + e.g. the node controller might add the node.kubernetes.io/not-ready taint. Only those taints defined in this list will be added or removed by core Cluster API controllers. + There can be at most 64 taints. + 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. items: description: MachineTaint defines a taint equivalent to corev1.Taint, but additionally having a propagation field. @@ -1782,6 +1784,7 @@ spec: propagation: description: |- propagation defines how this taint should be propagated to nodes. + Valid values are 'Always' and 'OnInitialization'. Always: The taint will be continuously reconciled. If it is not set for a node, it will be added during reconciliation. OnInitialization: The taint will be added during node initialization. If it gets removed from the node later on it will not get added again. enum: @@ -1792,6 +1795,7 @@ spec: description: value is the taint value corresponding to the taint key. maxLength: 63 + minLength: 1 type: string required: - effect diff --git a/config/crd/bases/cluster.x-k8s.io_machinesets.yaml b/config/crd/bases/cluster.x-k8s.io_machinesets.yaml index 2d9e2b5327a0..def0dc7c63dd 100644 --- a/config/crd/bases/cluster.x-k8s.io_machinesets.yaml +++ b/config/crd/bases/cluster.x-k8s.io_machinesets.yaml @@ -1999,11 +1999,13 @@ spec: taints: description: |- taints are the node taints that Cluster API will manage. - This list is not necessarily complete: other Kubernetes components may add or remove other taints. + This list is not necessarily complete: other Kubernetes components may add or remove other taints from nodes, + e.g. the node controller might add the node.kubernetes.io/not-ready taint. Only those taints defined in this list will be added or removed by core Cluster API controllers. + There can be at most 64 taints. + 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. items: description: MachineTaint defines a taint equivalent to corev1.Taint, but additionally having a propagation field. @@ -2025,6 +2027,7 @@ spec: propagation: description: |- propagation defines how this taint should be propagated to nodes. + Valid values are 'Always' and 'OnInitialization'. Always: The taint will be continuously reconciled. If it is not set for a node, it will be added during reconciliation. OnInitialization: The taint will be added during node initialization. If it gets removed from the node later on it will not get added again. enum: @@ -2035,6 +2038,7 @@ spec: description: value is the taint value corresponding to the taint key. maxLength: 63 + minLength: 1 type: string required: - effect diff --git a/util/test/builder/zz_generated.deepcopy.go b/util/test/builder/zz_generated.deepcopy.go index 6813b2c1fdbe..b7c3d32289e8 100644 --- a/util/test/builder/zz_generated.deepcopy.go +++ b/util/test/builder/zz_generated.deepcopy.go @@ -435,9 +435,7 @@ func (in *MachineBuilder) DeepCopyInto(out *MachineBuilder) { if in.taints != nil { in, out := &in.taints, &out.taints *out = make([]v1beta2.MachineTaint, len(*in)) - for i := range *in { - (*in)[i].DeepCopyInto(&(*out)[i]) - } + copy(*out, *in) } } @@ -509,9 +507,7 @@ func (in *MachineDeploymentBuilder) DeepCopyInto(out *MachineDeploymentBuilder) if in.taints != nil { in, out := &in.taints, &out.taints *out = make([]v1beta2.MachineTaint, len(*in)) - for i := range *in { - (*in)[i].DeepCopyInto(&(*out)[i]) - } + copy(*out, *in) } } @@ -865,9 +861,7 @@ func (in *MachineSetBuilder) DeepCopyInto(out *MachineSetBuilder) { if in.taints != nil { in, out := &in.taints, &out.taints *out = make([]v1beta2.MachineTaint, len(*in)) - for i := range *in { - (*in)[i].DeepCopyInto(&(*out)[i]) - } + copy(*out, *in) } } From 6dff2aaf60f8be8d4b9b23a6b294b0bd4969e8d7 Mon Sep 17 00:00:00 2001 From: Christian Schlotter Date: Mon, 10 Nov 2025 13:36:50 +0100 Subject: [PATCH 13/21] Review changes v2 --- api/core/v1beta2/common_types.go | 9 +- .../machineset/machineset_controller_test.go | 4 +- internal/webhooks/machine.go | 12 - internal/webhooks/machine_test.go | 120 ---------- internal/webhooks/test/machine_test.go | 206 ++++++++++++++++++ 5 files changed, 216 insertions(+), 135 deletions(-) create mode 100644 internal/webhooks/test/machine_test.go diff --git a/api/core/v1beta2/common_types.go b/api/core/v1beta2/common_types.go index 83948d7c2ba2..2fbc60b7dcb9 100644 --- a/api/core/v1beta2/common_types.go +++ b/api/core/v1beta2/common_types.go @@ -412,15 +412,22 @@ func (r *ContractVersionedObjectReference) GroupKind() schema.GroupKind { // MachineTaint defines a taint equivalent to corev1.Taint, but additionally having a propagation field. type MachineTaint struct { // key is the taint key to be applied to a node. + // Must be a valid qualified name of maximum size 63 characters + // with an optional subdomain prefix of maximum size 253 characters, + // separated by a `/`. // +required // +kubebuilder:validation:MinLength=1 - // +kubebuilder:validation:MaxLength=253 + // +kubebuilder:validation:MaxLength=317 + // +kubebuilder:validation:Pattern=^([a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*\/)?([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9]$ + // +kubebuilder:validation:XValidation:rule="self.contains('/') ? ( self.split('/') [0].size() <= 253 && self.split('/') [1].size() <= 63 && self.split('/').size() == 2 ) : self.size() <= 63",message="key must be a valid qualified name of max size 63 characters with an optional subdomain prefix of max size 253 characters" Key string `json:"key,omitempty"` // value is the taint value corresponding to the taint key. + // It must be a valid label value of maximum size 63 characters. // +optional // +kubebuilder:validation:MinLength=1 // +kubebuilder:validation:MaxLength=63 + // +kubebuilder:validation:Pattern=^(([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9])?$ Value string `json:"value,omitempty"` // effect is the effect for the taint. Valid values are NoSchedule, PreferNoSchedule and NoExecute. diff --git a/internal/controllers/machineset/machineset_controller_test.go b/internal/controllers/machineset/machineset_controller_test.go index 532d75f2952e..5de09ae50cf2 100644 --- a/internal/controllers/machineset/machineset_controller_test.go +++ b/internal/controllers/machineset/machineset_controller_test.go @@ -63,10 +63,10 @@ var _ reconcile.Reconciler = &Reconciler{} func TestMachineSetReconciler(t *testing.T) { setup := func(t *testing.T, g *WithT) (*corev1.Namespace, *clusterv1.Cluster) { - utilfeature.SetFeatureGateDuringTest(t, feature.Gates, feature.MachineTaintPropagation, true) - t.Helper() + utilfeature.SetFeatureGateDuringTest(t, feature.Gates, feature.MachineTaintPropagation, true) + t.Log("Creating the namespace") ns, err := env.CreateNamespace(ctx, "test-machine-set-reconciler") g.Expect(err).ToNot(HaveOccurred()) diff --git a/internal/webhooks/machine.go b/internal/webhooks/machine.go index b666ca46bffb..f7863180cc94 100644 --- a/internal/webhooks/machine.go +++ b/internal/webhooks/machine.go @@ -22,9 +22,7 @@ import ( "strings" apierrors "k8s.io/apimachinery/pkg/api/errors" - metav1validation "k8s.io/apimachinery/pkg/apis/meta/v1/validation" "k8s.io/apimachinery/pkg/runtime" - "k8s.io/apimachinery/pkg/util/validation" "k8s.io/apimachinery/pkg/util/validation/field" "k8s.io/utils/ptr" ctrl "sigs.k8s.io/controller-runtime" @@ -160,16 +158,6 @@ func validateMachineTaints(taints []clusterv1.MachineTaint, taintsPath *field.Pa for i, taint := range taints { idxPath := taintsPath.Index(i) - // Validate key syntax. - if errs := metav1validation.ValidateLabelName(taint.Key, idxPath.Child("key")); len(errs) > 0 { - allErrs = append(allErrs, errs...) - } - - // Validate value syntax. - if errs := validation.IsValidLabelValue(taint.Value); len(errs) > 0 { - allErrs = append(allErrs, field.Invalid(idxPath.Child("value"), taint.Value, strings.Join(errs, ";"))) - } - // The following validations uses a switch statement, because if one of them matches, then the others won't. switch { diff --git a/internal/webhooks/machine_test.go b/internal/webhooks/machine_test.go index 80138cf7679f..d1e9ddd46ba4 100644 --- a/internal/webhooks/machine_test.go +++ b/internal/webhooks/machine_test.go @@ -20,15 +20,11 @@ import ( "testing" . "github.com/onsi/gomega" - corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - utilfeature "k8s.io/component-base/featuregate/testing" "k8s.io/utils/ptr" clusterv1 "sigs.k8s.io/cluster-api/api/core/v1beta2" - "sigs.k8s.io/cluster-api/feature" "sigs.k8s.io/cluster-api/internal/webhooks/util" - "sigs.k8s.io/cluster-api/util/test/builder" ) func TestMachineDefault(t *testing.T) { @@ -229,119 +225,3 @@ func TestMachineVersionValidation(t *testing.T) { }) } } - -func TestMachineTaintValidation(t *testing.T) { - m := builder.Machine("default", "machine1"). - WithBootstrapTemplate(builder.BootstrapTemplate("default", "bootstrap-template").Build()) - webhook := &Machine{} - - tests := []struct { - name string - machine *clusterv1.Machine - featureEnabled bool - expectErr bool - }{ - { - name: "should allow empty taints with feature gate disabled", - featureEnabled: false, - machine: m.DeepCopy().Build(), - expectErr: false, - }, - { - name: "should allow empty taints with feature gate enabled", - featureEnabled: true, - machine: m.DeepCopy().Build(), - expectErr: false, - }, - { - name: "should block taint key node.cluster.x-k8s.io/uninitialized", - featureEnabled: true, - machine: m.DeepCopy().WithTaints(clusterv1.MachineTaint{ - Key: "node.cluster.x-k8s.io/uninitialized", Effect: corev1.TaintEffectNoSchedule, - }).Build(), - expectErr: true, - }, - { - name: "should block taint key node.cluster.x-k8s.io/outdated-revision", - featureEnabled: true, - machine: m.DeepCopy().WithTaints(clusterv1.MachineTaint{ - Key: "node.cluster.x-k8s.io/outdated-revision", Effect: corev1.TaintEffectNoSchedule, - }).Build(), - expectErr: true, - }, - { - name: "should block taint with key prefix node.kubernetes.io/, which is not `out-of-service`", - featureEnabled: true, - machine: m.DeepCopy().WithTaints(clusterv1.MachineTaint{ - Key: "node.kubernetes.io/some-taint", Effect: corev1.TaintEffectNoSchedule, - }).Build(), - expectErr: true, - }, - { - name: "should allow taint node.kubernetes.io/out-of-service", - featureEnabled: true, - machine: m.DeepCopy().WithTaints(clusterv1.MachineTaint{ - Key: "node.kubernetes.io/out-of-service", Effect: corev1.TaintEffectNoSchedule, - }).Build(), - expectErr: false, - }, - { - name: "should block taint with key prefix node.cloudprovider.kubernetes.io/", - featureEnabled: true, - machine: m.DeepCopy().WithTaints(clusterv1.MachineTaint{ - Key: "node.cloudprovider.kubernetes.io/some-taint", Effect: corev1.TaintEffectNoSchedule, - }).Build(), - expectErr: true, - }, - { - name: "should block taint key node-role.kubernetes.io/master", - featureEnabled: true, - machine: m.DeepCopy().WithTaints(clusterv1.MachineTaint{ - Key: "node-role.kubernetes.io/master", Effect: corev1.TaintEffectNoSchedule, - }).Build(), - expectErr: true, - }, - { - name: "should allow taint key node-role.kubernetes.io/control-plane for control-plane nodes", - featureEnabled: true, - machine: m.DeepCopy(). - WithLabels(map[string]string{clusterv1.MachineControlPlaneLabel: "true"}). - WithTaints(clusterv1.MachineTaint{ - Key: "node-role.kubernetes.io/control-plane", Effect: corev1.TaintEffectNoSchedule, - }).Build(), - expectErr: false, - }, - { - name: "should block taint key node-role.kubernetes.io/control-plane for worker nodes", - featureEnabled: true, - machine: m.DeepCopy().WithTaints(clusterv1.MachineTaint{ - Key: "node-role.kubernetes.io/control-plane", Effect: corev1.TaintEffectNoSchedule, - }).Build(), - expectErr: true, - }, - } - for i := range tests { - tt := tests[i] - t.Run(tt.name, func(t *testing.T) { - g := NewWithT(t) - - utilfeature.SetFeatureGateDuringTest(t, feature.Gates, feature.MachineTaintPropagation, tt.featureEnabled) - - warnings, err := webhook.ValidateCreate(ctx, tt.machine) - if tt.expectErr { - g.Expect(err).To(HaveOccurred()) - } else { - g.Expect(err).ToNot(HaveOccurred()) - } - g.Expect(warnings).To(BeEmpty()) - - warnings, err = webhook.ValidateUpdate(ctx, tt.machine, tt.machine) - if tt.expectErr { - g.Expect(err).To(HaveOccurred()) - } else { - g.Expect(err).ToNot(HaveOccurred()) - } - g.Expect(warnings).To(BeEmpty()) - }) - } -} diff --git a/internal/webhooks/test/machine_test.go b/internal/webhooks/test/machine_test.go new file mode 100644 index 000000000000..72f1a55f7056 --- /dev/null +++ b/internal/webhooks/test/machine_test.go @@ -0,0 +1,206 @@ +/* +Copyright 202 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package test + +import ( + "fmt" + "testing" + + . "github.com/onsi/gomega" + corev1 "k8s.io/api/core/v1" + utilfeature "k8s.io/component-base/featuregate/testing" + + clusterv1 "sigs.k8s.io/cluster-api/api/core/v1beta2" + "sigs.k8s.io/cluster-api/feature" + "sigs.k8s.io/cluster-api/util/test/builder" +) + +func TestMachineTaintValidation(t *testing.T) { + infraMachine := builder.InfrastructureMachine("default", "infrastructure-machine1").Build() + m := builder.Machine("default", "m"). + WithClusterName("cluster1"). + WithBootstrapTemplate(builder.BootstrapTemplate("default", "bootstrap-template").Build()). + WithInfrastructureMachine(infraMachine) + + chars254 := "moreThen253Charactersaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaabbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaadddz" + chars253 := "253charactersaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaabbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaddd" + chars63 := "63charactersaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaabbbbbbbbbbddd" + chars64 := "moreThene63Charactersaaaaaaaaaaaaaaaaaaaaaaaaaaaaabbbbbbbbbbdddz" + + tests := []struct { + name string + machine *clusterv1.Machine + featureEnabled bool + expectErr bool + }{ + { + name: "should allow empty taints with feature gate disabled", + featureEnabled: false, + machine: m.DeepCopy().Build(), + expectErr: false, + }, + { + name: "should forbid taints with feature gate disabled", + featureEnabled: false, + machine: m.DeepCopy().WithTaints(clusterv1.MachineTaint{ + Key: "some/taint", Effect: corev1.TaintEffectNoSchedule, Propagation: clusterv1.MachineTaintPropagationAlways, + }).Build(), + expectErr: true, + }, + { + name: "should allow empty taints with feature gate enabled", + featureEnabled: true, + machine: m.DeepCopy().Build(), + expectErr: false, + }, + { + name: "should block taint key node.cluster.x-k8s.io/uninitialized", + featureEnabled: true, + machine: m.DeepCopy().WithTaints(clusterv1.MachineTaint{ + Key: "node.cluster.x-k8s.io/uninitialized", Effect: corev1.TaintEffectNoSchedule, Propagation: clusterv1.MachineTaintPropagationAlways, + }).Build(), + expectErr: true, + }, + { + name: "should block taint key node.cluster.x-k8s.io/outdated-revision", + featureEnabled: true, + machine: m.DeepCopy().WithTaints(clusterv1.MachineTaint{ + Key: "node.cluster.x-k8s.io/outdated-revision", Effect: corev1.TaintEffectNoSchedule, + }).Build(), + expectErr: true, + }, + { + name: "should block taint with key prefix node.kubernetes.io/, which is not `out-of-service`", + featureEnabled: true, + machine: m.DeepCopy().WithTaints(clusterv1.MachineTaint{ + Key: "node.kubernetes.io/some-taint", Effect: corev1.TaintEffectNoSchedule, Propagation: clusterv1.MachineTaintPropagationAlways, + }).Build(), + expectErr: true, + }, + { + name: "should allow taint node.kubernetes.io/out-of-service", + featureEnabled: true, + machine: m.DeepCopy().WithTaints(clusterv1.MachineTaint{ + Key: "node.kubernetes.io/out-of-service", Effect: corev1.TaintEffectNoSchedule, Propagation: clusterv1.MachineTaintPropagationAlways, + }).Build(), + expectErr: false, + }, + { + name: "should block taint with key prefix node.cloudprovider.kubernetes.io/", + featureEnabled: true, + machine: m.DeepCopy().WithTaints(clusterv1.MachineTaint{ + Key: "node.cloudprovider.kubernetes.io/some-taint", Effect: corev1.TaintEffectNoSchedule, Propagation: clusterv1.MachineTaintPropagationAlways, + }).Build(), + expectErr: true, + }, + { + name: "should block taint key node-role.kubernetes.io/master", + featureEnabled: true, + machine: m.DeepCopy().WithTaints(clusterv1.MachineTaint{ + Key: "node-role.kubernetes.io/master", Effect: corev1.TaintEffectNoSchedule, Propagation: clusterv1.MachineTaintPropagationAlways, + }).Build(), + expectErr: true, + }, + { + name: "should allow taint key node-role.kubernetes.io/control-plane for control-plane nodes", + featureEnabled: true, + machine: m.DeepCopy(). + WithLabels(map[string]string{clusterv1.MachineControlPlaneLabel: "true"}). + WithTaints(clusterv1.MachineTaint{ + Key: "node-role.kubernetes.io/control-plane", Effect: corev1.TaintEffectNoSchedule, Propagation: clusterv1.MachineTaintPropagationAlways, + }).Build(), + expectErr: false, + }, + { + name: "should block taint key node-role.kubernetes.io/control-plane for worker nodes", + featureEnabled: true, + machine: m.DeepCopy().WithTaints(clusterv1.MachineTaint{ + Key: "node-role.kubernetes.io/control-plane", Effect: corev1.TaintEffectNoSchedule, Propagation: clusterv1.MachineTaintPropagationAlways, + }).Build(), + expectErr: true, + }, + { + name: "should allow taint key without prefix and 63 characters", + featureEnabled: true, + machine: m.DeepCopy().WithTaints(clusterv1.MachineTaint{ + Key: chars63, + Effect: corev1.TaintEffectNoSchedule, Propagation: clusterv1.MachineTaintPropagationAlways, + }).Build(), + expectErr: false, + }, + { + name: "should block taint key without prefix and more than 63 characters", + featureEnabled: true, + machine: m.DeepCopy().WithTaints(clusterv1.MachineTaint{ + Key: chars64, + Effect: corev1.TaintEffectNoSchedule, Propagation: clusterv1.MachineTaintPropagationAlways, + }).Build(), + expectErr: true, + }, + { + name: "should allow taint key with prefix of 253 characters", + featureEnabled: true, + machine: m.DeepCopy().WithTaints(clusterv1.MachineTaint{ + Key: chars253 + "/" + chars63, + Effect: corev1.TaintEffectNoSchedule, Propagation: clusterv1.MachineTaintPropagationAlways, + }).Build(), + expectErr: false, + }, + { + name: "should block taint key with prefix of more than 253 characters", + featureEnabled: true, + machine: m.DeepCopy().WithTaints(clusterv1.MachineTaint{ + Key: chars254 + "/" + chars63, + Effect: corev1.TaintEffectNoSchedule, Propagation: clusterv1.MachineTaintPropagationAlways, + }).Build(), + expectErr: true, + }, + { + name: "should block taint key with empty prefix", + featureEnabled: true, + machine: m.DeepCopy().WithTaints(clusterv1.MachineTaint{ + Key: "/with-empty-prefix", Effect: corev1.TaintEffectNoSchedule, Propagation: clusterv1.MachineTaintPropagationAlways, + }).Build(), + expectErr: true, + }, + { + name: "should block taint key with multuple slashes", + featureEnabled: true, + machine: m.DeepCopy().WithTaints(clusterv1.MachineTaint{ + Key: "one/two/with-prefix", Effect: corev1.TaintEffectNoSchedule, Propagation: clusterv1.MachineTaintPropagationAlways, + }).Build(), + expectErr: true, + }, + } + for i := range tests { + tt := tests[i] + t.Run(tt.name, func(t *testing.T) { + g := NewWithT(t) + + utilfeature.SetFeatureGateDuringTest(t, feature.Gates, feature.MachineTaintPropagation, tt.featureEnabled) + + tt.machine.Name = fmt.Sprintf("machine-%02d", i) + + err := env.CreateAndWait(ctx, tt.machine) + if tt.expectErr { + g.Expect(err).To(HaveOccurred()) + } else { + g.Expect(err).ToNot(HaveOccurred()) + } + }) + } +} From 51740934e8d5f88c596fc1245f594594e3b46149 Mon Sep 17 00:00:00 2001 From: Christian Schlotter Date: Mon, 10 Nov 2025 13:37:09 +0100 Subject: [PATCH 14/21] make generate --- api/core/v1beta2/zz_generated.openapi.go | 4 ++-- .../cluster.x-k8s.io_machinedeployments.yaml | 24 +++++++++++++++---- .../bases/cluster.x-k8s.io_machinepools.yaml | 24 +++++++++++++++---- .../crd/bases/cluster.x-k8s.io_machines.yaml | 22 +++++++++++++---- .../bases/cluster.x-k8s.io_machinesets.yaml | 24 +++++++++++++++---- 5 files changed, 77 insertions(+), 21 deletions(-) diff --git a/api/core/v1beta2/zz_generated.openapi.go b/api/core/v1beta2/zz_generated.openapi.go index fd2d88064b28..c349d50cb1d0 100644 --- a/api/core/v1beta2/zz_generated.openapi.go +++ b/api/core/v1beta2/zz_generated.openapi.go @@ -6314,14 +6314,14 @@ func schema_cluster_api_api_core_v1beta2_MachineTaint(ref common.ReferenceCallba Properties: map[string]spec.Schema{ "key": { SchemaProps: spec.SchemaProps{ - Description: "key is the taint key to be applied to a node.", + Description: "key is the taint key to be applied to a node. Must be a valid qualified name of maximum size 63 characters with an optional subdomain prefix of maximum size 253 characters, separated by a `/`.", Type: []string{"string"}, Format: "", }, }, "value": { SchemaProps: spec.SchemaProps{ - Description: "value is the taint value corresponding to the taint key.", + Description: "value is the taint value corresponding to the taint key. It must be a valid label value of maximum size 63 characters.", Type: []string{"string"}, Format: "", }, diff --git a/config/crd/bases/cluster.x-k8s.io_machinedeployments.yaml b/config/crd/bases/cluster.x-k8s.io_machinedeployments.yaml index ec03086ab643..ba3697d9acd8 100644 --- a/config/crd/bases/cluster.x-k8s.io_machinedeployments.yaml +++ b/config/crd/bases/cluster.x-k8s.io_machinedeployments.yaml @@ -2363,11 +2363,23 @@ spec: - NoExecute type: string key: - description: key is the taint key to be applied to a - node. - maxLength: 253 + description: |- + key is the taint key to be applied to a node. + Must be a valid qualified name of maximum size 63 characters + with an optional subdomain prefix of maximum size 253 characters, + separated by a `/`. + maxLength: 317 minLength: 1 + pattern: ^([a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*\/)?([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9]$ type: string + x-kubernetes-validations: + - message: key must be a valid qualified name of max + size 63 characters with an optional subdomain prefix + of max size 253 characters + rule: 'self.contains(''/'') ? ( self.split(''/'') + [0].size() <= 253 && self.split(''/'') [1].size() + <= 63 && self.split(''/'').size() == 2 ) : self.size() + <= 63' propagation: description: |- propagation defines how this taint should be propagated to nodes. @@ -2379,10 +2391,12 @@ spec: - OnInitialization type: string value: - description: value is the taint value corresponding - to the taint key. + description: |- + value is the taint value corresponding to the taint key. + It must be a valid label value of maximum size 63 characters. maxLength: 63 minLength: 1 + pattern: ^(([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9])?$ type: string required: - effect diff --git a/config/crd/bases/cluster.x-k8s.io_machinepools.yaml b/config/crd/bases/cluster.x-k8s.io_machinepools.yaml index 17369f34a143..d0c199a26930 100644 --- a/config/crd/bases/cluster.x-k8s.io_machinepools.yaml +++ b/config/crd/bases/cluster.x-k8s.io_machinepools.yaml @@ -1967,11 +1967,23 @@ spec: - NoExecute type: string key: - description: key is the taint key to be applied to a - node. - maxLength: 253 + description: |- + key is the taint key to be applied to a node. + Must be a valid qualified name of maximum size 63 characters + with an optional subdomain prefix of maximum size 253 characters, + separated by a `/`. + maxLength: 317 minLength: 1 + pattern: ^([a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*\/)?([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9]$ type: string + x-kubernetes-validations: + - message: key must be a valid qualified name of max + size 63 characters with an optional subdomain prefix + of max size 253 characters + rule: 'self.contains(''/'') ? ( self.split(''/'') + [0].size() <= 253 && self.split(''/'') [1].size() + <= 63 && self.split(''/'').size() == 2 ) : self.size() + <= 63' propagation: description: |- propagation defines how this taint should be propagated to nodes. @@ -1983,10 +1995,12 @@ spec: - OnInitialization type: string value: - description: value is the taint value corresponding - to the taint key. + description: |- + value is the taint value corresponding to the taint key. + It must be a valid label value of maximum size 63 characters. maxLength: 63 minLength: 1 + pattern: ^(([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9])?$ type: string required: - effect diff --git a/config/crd/bases/cluster.x-k8s.io_machines.yaml b/config/crd/bases/cluster.x-k8s.io_machines.yaml index ed9fa1b73290..dc7c4cf6582a 100644 --- a/config/crd/bases/cluster.x-k8s.io_machines.yaml +++ b/config/crd/bases/cluster.x-k8s.io_machines.yaml @@ -1777,10 +1777,22 @@ spec: - NoExecute type: string key: - description: key is the taint key to be applied to a node. - maxLength: 253 + description: |- + key is the taint key to be applied to a node. + Must be a valid qualified name of maximum size 63 characters + with an optional subdomain prefix of maximum size 253 characters, + separated by a `/`. + maxLength: 317 minLength: 1 + pattern: ^([a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*\/)?([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9]$ type: string + x-kubernetes-validations: + - message: key must be a valid qualified name of max size 63 + characters with an optional subdomain prefix of max size + 253 characters + rule: 'self.contains(''/'') ? ( self.split(''/'') [0].size() + <= 253 && self.split(''/'') [1].size() <= 63 && self.split(''/'').size() + == 2 ) : self.size() <= 63' propagation: description: |- propagation defines how this taint should be propagated to nodes. @@ -1792,10 +1804,12 @@ spec: - OnInitialization type: string value: - description: value is the taint value corresponding to the taint - key. + description: |- + value is the taint value corresponding to the taint key. + It must be a valid label value of maximum size 63 characters. maxLength: 63 minLength: 1 + pattern: ^(([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9])?$ type: string required: - effect diff --git a/config/crd/bases/cluster.x-k8s.io_machinesets.yaml b/config/crd/bases/cluster.x-k8s.io_machinesets.yaml index def0dc7c63dd..9019e12a6508 100644 --- a/config/crd/bases/cluster.x-k8s.io_machinesets.yaml +++ b/config/crd/bases/cluster.x-k8s.io_machinesets.yaml @@ -2019,11 +2019,23 @@ spec: - NoExecute type: string key: - description: key is the taint key to be applied to a - node. - maxLength: 253 + description: |- + key is the taint key to be applied to a node. + Must be a valid qualified name of maximum size 63 characters + with an optional subdomain prefix of maximum size 253 characters, + separated by a `/`. + maxLength: 317 minLength: 1 + pattern: ^([a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*\/)?([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9]$ type: string + x-kubernetes-validations: + - message: key must be a valid qualified name of max + size 63 characters with an optional subdomain prefix + of max size 253 characters + rule: 'self.contains(''/'') ? ( self.split(''/'') + [0].size() <= 253 && self.split(''/'') [1].size() + <= 63 && self.split(''/'').size() == 2 ) : self.size() + <= 63' propagation: description: |- propagation defines how this taint should be propagated to nodes. @@ -2035,10 +2047,12 @@ spec: - OnInitialization type: string value: - description: value is the taint value corresponding - to the taint key. + description: |- + value is the taint value corresponding to the taint key. + It must be a valid label value of maximum size 63 characters. maxLength: 63 minLength: 1 + pattern: ^(([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9])?$ type: string required: - effect From 16f81fd9c23b385d7b78e428e5c9f4706984f2b3 Mon Sep 17 00:00:00 2001 From: Christian Schlotter Date: Mon, 10 Nov 2025 13:43:53 +0100 Subject: [PATCH 15/21] Review fixes v3 --- internal/webhooks/test/machine_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/webhooks/test/machine_test.go b/internal/webhooks/test/machine_test.go index 72f1a55f7056..22e98e77ceb2 100644 --- a/internal/webhooks/test/machine_test.go +++ b/internal/webhooks/test/machine_test.go @@ -1,5 +1,5 @@ /* -Copyright 202 The Kubernetes Authors. +Copyright 2025 The Kubernetes Authors. Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. From de843f81f23a3b263b1255109ffe64d842f1830c Mon Sep 17 00:00:00 2001 From: Christian Schlotter Date: Mon, 10 Nov 2025 14:04:12 +0100 Subject: [PATCH 16/21] Update unit tests --- .../machinedeployment_controller_test.go | 28 +++++++++++++++ .../machinedeployment/mdutil/util_test.go | 3 ++ .../machineset/machineset_controller_test.go | 35 +++++++++++++++++-- 3 files changed, 63 insertions(+), 3 deletions(-) diff --git a/internal/controllers/machinedeployment/machinedeployment_controller_test.go b/internal/controllers/machinedeployment/machinedeployment_controller_test.go index d8172c796753..155d99b88fc4 100644 --- a/internal/controllers/machinedeployment/machinedeployment_controller_test.go +++ b/internal/controllers/machinedeployment/machinedeployment_controller_test.go @@ -26,6 +26,7 @@ import ( "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/client-go/tools/record" "k8s.io/client-go/util/retry" + utilfeature "k8s.io/component-base/featuregate/testing" "k8s.io/utils/ptr" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/client/fake" @@ -33,6 +34,7 @@ import ( clusterv1 "sigs.k8s.io/cluster-api/api/core/v1beta2" "sigs.k8s.io/cluster-api/controllers/external" + "sigs.k8s.io/cluster-api/feature" "sigs.k8s.io/cluster-api/util" v1beta1conditions "sigs.k8s.io/cluster-api/util/conditions/deprecated/v1beta1" "sigs.k8s.io/cluster-api/util/patch" @@ -49,6 +51,8 @@ func TestMachineDeploymentReconciler(t *testing.T) { setup := func(t *testing.T, g *WithT) (*corev1.Namespace, *clusterv1.Cluster) { t.Helper() + utilfeature.SetFeatureGateDuringTest(t, feature.Gates, feature.MachineTaintPropagation, true) + t.Log("Creating the namespace") ns, err := env.CreateNamespace(ctx, machineDeploymentNamespace) g.Expect(err).ToNot(HaveOccurred()) @@ -416,6 +420,30 @@ func TestMachineDeploymentReconciler(t *testing.T) { g.Expect(machineSets.Items[1].Spec.Deletion.Order).Should(Equal(clusterv1.NewestMachineSetDeletionOrder)) }).Should(Succeed()) + // Update the taints of the MachineDeployment, + // expect the Reconcile to be called and the MachineSet to be updated in-place. + t.Log("Updating deletion.order on the MachineDeployment") + additionalTaint := clusterv1.MachineTaint{ + Key: "additional-taint-key", + Value: "additional-taint-value", + Effect: corev1.TaintEffectNoSchedule, + Propagation: clusterv1.MachineTaintPropagationAlways, + } + modifyFunc = func(d *clusterv1.MachineDeployment) { + d.Spec.Template.Spec.Taints = append(d.Spec.Template.Spec.Taints, additionalTaint) + } + g.Expect(updateMachineDeployment(ctx, env, deployment, modifyFunc)).To(Succeed()) + g.Eventually(func(g Gomega) { + g.Expect(env.List(ctx, machineSets, msListOpts...)).Should(Succeed()) + // Verify we still only have 2 MachineSets. + g.Expect(machineSets.Items).To(HaveLen(2)) + // Verify the taints value is updated + g.Expect(machineSets.Items[0].Spec.Template.Spec.Taints).Should(ContainElement(additionalTaint)) + + // Verify that the old machine set has the new taints. + g.Expect(machineSets.Items[1].Spec.Template.Spec.Taints).Should(ContainElement(additionalTaint)) + }).Should(Succeed()) + // Verify that all the MachineSets have the expected OwnerRef. t.Log("Verifying MachineSet owner references") g.Eventually(func() bool { diff --git a/internal/controllers/machinedeployment/mdutil/util_test.go b/internal/controllers/machinedeployment/mdutil/util_test.go index 7a57125a56a2..6a3c4e6c91f3 100644 --- a/internal/controllers/machinedeployment/mdutil/util_test.go +++ b/internal/controllers/machinedeployment/mdutil/util_test.go @@ -415,6 +415,9 @@ func TestFindNewAndOldMachineSets(t *testing.T) { matchingMSDiffersInPlaceMutableFields := generateMS(deployment) matchingMSDiffersInPlaceMutableFields.Spec.Template.Spec.Deletion.NodeDrainTimeoutSeconds = ptr.To(int32(20)) + matchingMSDiffersInPlaceMutableFields.Spec.Template.Spec.Taints = []clusterv1.MachineTaint{ + {Key: "taint-key", Value: "taint-value", Effect: corev1.TaintEffectNoSchedule, Propagation: clusterv1.MachineTaintPropagationAlways}, + } oldMS := generateMS(deployment) oldMS.Spec.Template.Spec.InfrastructureRef.Name = "old-infra-ref" diff --git a/internal/controllers/machineset/machineset_controller_test.go b/internal/controllers/machineset/machineset_controller_test.go index 5de09ae50cf2..95d1e2d7a49a 100644 --- a/internal/controllers/machineset/machineset_controller_test.go +++ b/internal/controllers/machineset/machineset_controller_test.go @@ -472,10 +472,17 @@ func TestMachineSetReconciler(t *testing.T) { } // Verify that in-place mutable fields propagate from MachineSet to Machines. - t.Log("Updating NodeDrainTimeoutSeconds on MachineSet") + t.Log("Updating NodeDrainTimeoutSeconds and Taints on MachineSet") patchHelper, err := patch.NewHelper(instance, env) g.Expect(err).ToNot(HaveOccurred()) instance.Spec.Template.Spec.Deletion.NodeDrainTimeoutSeconds = duration5m + additionalTaint := clusterv1.MachineTaint{ + Key: "additional-taint-key", + Value: "additional-taint-value", + Effect: corev1.TaintEffectNoSchedule, + Propagation: clusterv1.MachineTaintPropagationAlways, + } + instance.Spec.Template.Spec.Taints = []clusterv1.MachineTaint{additionalTaint} g.Expect(patchHelper.Patch(ctx, instance)).Should(Succeed()) t.Log("Verifying new NodeDrainTimeoutSeconds value is set on Machines") @@ -483,7 +490,7 @@ func TestMachineSetReconciler(t *testing.T) { if err := env.List(ctx, machines, client.InNamespace(namespace.Name)); err != nil { return false } - // All the machines should have the new NodeDrainTimeoutValue + // All the machines should have the new NodeDrainTimeoutValue and the new taint for _, m := range machines.Items { if m.Spec.Deletion.NodeDrainTimeoutSeconds == nil { return false @@ -491,9 +498,13 @@ func TestMachineSetReconciler(t *testing.T) { if *m.Spec.Deletion.NodeDrainTimeoutSeconds != *duration5m { return false } + if len(m.Spec.Taints) != 1 || m.Spec.Taints[0].Key != additionalTaint.Key || + m.Spec.Taints[0].Value != additionalTaint.Value || m.Spec.Taints[0].Effect != additionalTaint.Effect { + return false + } } return true - }, timeout).Should(BeTrue(), "machine should have the updated NodeDrainTimeoutSeconds value") + }, timeout).Should(BeTrue(), "machine should have the updated NodeDrainTimeoutSeconds and Taints values") // Try to delete 1 machine and check the MachineSet scales back up. machineToBeDeleted := machines.Items[0] @@ -3528,6 +3539,19 @@ func TestComputeDesiredMachine(t *testing.T) { duration5s := ptr.To(int32(5)) duration10s := ptr.To(int32(10)) + machineTaint := clusterv1.MachineTaint{ + Key: "taint-key", + Value: "taint-value", + Effect: corev1.TaintEffectNoSchedule, + Propagation: clusterv1.MachineTaintPropagationAlways, + } + changedMachineTaint := clusterv1.MachineTaint{ + Key: "changed-taint-key", + Value: "changed-taint-value", + Effect: corev1.TaintEffectNoSchedule, + Propagation: clusterv1.MachineTaintPropagationAlways, + } + namingTemplateKey := "-md" mdName := "testmd" msName := "ms1" @@ -3567,6 +3591,9 @@ func TestComputeDesiredMachine(t *testing.T) { NodeDeletionTimeoutSeconds: duration10s, }, MinReadySeconds: ptr.To[int32](10), + Taints: []clusterv1.MachineTaint{ + machineTaint, + }, }, } @@ -3590,6 +3617,7 @@ func TestComputeDesiredMachine(t *testing.T) { NodeDeletionTimeoutSeconds: duration10s, }, MinReadySeconds: ptr.To[int32](10), + Taints: []clusterv1.MachineTaint{machineTaint}, }, } @@ -3618,6 +3646,7 @@ func TestComputeDesiredMachine(t *testing.T) { existingMachine.Spec.Deletion.NodeDeletionTimeoutSeconds = duration5s existingMachine.Spec.Deletion.NodeVolumeDetachTimeoutSeconds = duration5s existingMachine.Spec.MinReadySeconds = ptr.To[int32](5) + existingMachine.Spec.Taints = []clusterv1.MachineTaint{changedMachineTaint} expectedUpdatedMachine := skeletonMachine.DeepCopy() expectedUpdatedMachine.Name = existingMachine.Name From 5f7929095d5219a31b91be0334dc18c603c027c1 Mon Sep 17 00:00:00 2001 From: Christian Schlotter Date: Mon, 10 Nov 2025 18:45:13 +0100 Subject: [PATCH 17/21] fix tests after rebase --- .../machinedeployment_canupdatemachineset_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/internal/controllers/machinedeployment/machinedeployment_canupdatemachineset_test.go b/internal/controllers/machinedeployment/machinedeployment_canupdatemachineset_test.go index 493547073076..845b6ae424c9 100644 --- a/internal/controllers/machinedeployment/machinedeployment_canupdatemachineset_test.go +++ b/internal/controllers/machinedeployment/machinedeployment_canupdatemachineset_test.go @@ -410,7 +410,7 @@ func Test_canExtensionsUpdateMachineSet(t *testing.T) { + Version: "v1.31.0", ProviderID: "", FailureDomain: "", - ... // 3 identical fields + ... // 4 identical fields }, }, MachineNaming: {}, @@ -590,7 +590,7 @@ func Test_canExtensionsUpdateMachineSet(t *testing.T) { g.Expect(err).ToNot(HaveOccurred()) } g.Expect(canUpdateMachineSet).To(Equal(tt.wantCanUpdateMachineSet)) - g.Expect(reasons).To(Equal(tt.wantReasons)) + g.Expect(reasons).To(BeComparableTo(tt.wantReasons)) }) } } From 7f76609c5d1aa1fec9eb4cfc9d896ec2a3250265 Mon Sep 17 00:00:00 2001 From: Christian Schlotter Date: Tue, 11 Nov 2025 08:25:15 +0100 Subject: [PATCH 18/21] Review fixes v4 --- .../machine_controller_noderef_test.go | 103 ++++++++++-------- .../machinedeployment_controller_test.go | 2 +- internal/webhooks/machinepool.go | 2 +- internal/webhooks/test/machine_test.go | 2 +- 4 files changed, 59 insertions(+), 50 deletions(-) diff --git a/internal/controllers/machine/machine_controller_noderef_test.go b/internal/controllers/machine/machine_controller_noderef_test.go index 2004334d96f0..b1ae274083ab 100644 --- a/internal/controllers/machine/machine_controller_noderef_test.go +++ b/internal/controllers/machine/machine_controller_noderef_test.go @@ -85,40 +85,44 @@ func TestReconcileNode(t *testing.T) { } testCases := []struct { - name string - machine *clusterv1.Machine - node *corev1.Node - nodeGetErr bool - expectResult ctrl.Result - expectError bool - expected func(g *WithT, m *clusterv1.Machine) - expectNodeGetError bool - expectedNode func(g *WithT, m *corev1.Node) + name string + machine *clusterv1.Machine + node *corev1.Node + featureGateMachineTaintsEnabled bool + nodeGetErr bool + expectResult ctrl.Result + expectError bool + expected func(g *WithT, m *clusterv1.Machine) + expectNodeGetError bool + expectedNode func(g *WithT, m *corev1.Node) }{ { - name: "No op if provider ID is not set", - machine: &clusterv1.Machine{}, - node: nil, - nodeGetErr: false, - expectResult: ctrl.Result{}, - expectError: false, + name: "No op if provider ID is not set", + machine: &clusterv1.Machine{}, + node: nil, + featureGateMachineTaintsEnabled: false, + nodeGetErr: false, + expectResult: ctrl.Result{}, + expectError: false, }, { - name: "err reading node (something different than not found), it should return error", - machine: defaultMachine.DeepCopy(), - node: nil, - nodeGetErr: true, - expectResult: ctrl.Result{}, - expectError: true, - expectNodeGetError: true, + name: "err reading node (something different than not found), it should return error", + machine: defaultMachine.DeepCopy(), + node: nil, + featureGateMachineTaintsEnabled: false, + nodeGetErr: true, + expectResult: ctrl.Result{}, + expectError: true, + expectNodeGetError: true, }, { - name: "waiting for the node to exist, no op", - machine: defaultMachine.DeepCopy(), - node: nil, - nodeGetErr: false, - expectResult: ctrl.Result{}, - expectError: false, + name: "waiting for the node to exist, no op", + machine: defaultMachine.DeepCopy(), + node: nil, + featureGateMachineTaintsEnabled: false, + nodeGetErr: false, + expectResult: ctrl.Result{}, + expectError: false, }, { name: "node found, should surface info", @@ -146,9 +150,10 @@ func TestReconcileNode(t *testing.T) { }, }, }, - nodeGetErr: false, - expectResult: ctrl.Result{}, - expectError: false, + featureGateMachineTaintsEnabled: false, + nodeGetErr: false, + expectResult: ctrl.Result{}, + expectError: false, expected: func(g *WithT, m *clusterv1.Machine) { g.Expect(m.Status.NodeRef.Name).To(Equal("test-node-1")) g.Expect(m.Status.NodeInfo).ToNot(BeNil()) @@ -174,10 +179,11 @@ func TestReconcileNode(t *testing.T) { }, }, }, - node: nil, - nodeGetErr: false, - expectResult: ctrl.Result{}, - expectError: true, + node: nil, + featureGateMachineTaintsEnabled: false, + nodeGetErr: false, + expectResult: ctrl.Result{}, + expectError: true, }, { name: "node not found is tolerated when machine is deleting", @@ -200,10 +206,11 @@ func TestReconcileNode(t *testing.T) { }, }, }, - node: nil, - nodeGetErr: false, - expectResult: ctrl.Result{}, - expectError: false, + node: nil, + featureGateMachineTaintsEnabled: false, + nodeGetErr: false, + expectResult: ctrl.Result{}, + expectError: false, }, { name: "node found, should propagate taints", @@ -227,9 +234,10 @@ func TestReconcileNode(t *testing.T) { }, }, }, - nodeGetErr: false, - expectResult: ctrl.Result{}, - expectError: false, + featureGateMachineTaintsEnabled: true, + nodeGetErr: false, + expectResult: ctrl.Result{}, + expectError: false, expectedNode: func(g *WithT, n *corev1.Node) { g.Expect(n.Spec.Taints).To(BeComparableTo([]corev1.Taint{ { @@ -248,7 +256,7 @@ func TestReconcileNode(t *testing.T) { }, { name: "node found, should not add taints annotation if taints feature gate is disabled", - machine: defaultMachine.DeepCopy(), // The test only enables the feature gate if machine has taints. + machine: defaultMachineWithTaints.DeepCopy(), // The test only enables the feature gate if machine has taints. node: &corev1.Node{ ObjectMeta: metav1.ObjectMeta{ Name: "test-node-1", @@ -268,9 +276,10 @@ func TestReconcileNode(t *testing.T) { }, }, }, - nodeGetErr: false, - expectResult: ctrl.Result{}, - expectError: false, + featureGateMachineTaintsEnabled: false, + nodeGetErr: false, + expectResult: ctrl.Result{}, + expectError: false, expectedNode: func(g *WithT, n *corev1.Node) { g.Expect(n.Spec.Taints).To(BeEmpty()) g.Expect(n.Annotations).ToNot(HaveKey(clusterv1.TaintsFromMachineAnnotation)) @@ -282,7 +291,7 @@ func TestReconcileNode(t *testing.T) { t.Run(tc.name, func(t *testing.T) { g := NewWithT(t) - if tc.machine.Spec.Taints != nil { + if tc.featureGateMachineTaintsEnabled { utilfeature.SetFeatureGateDuringTest(t, feature.Gates, feature.MachineTaintPropagation, true) } diff --git a/internal/controllers/machinedeployment/machinedeployment_controller_test.go b/internal/controllers/machinedeployment/machinedeployment_controller_test.go index 155d99b88fc4..9c1a13dd93f1 100644 --- a/internal/controllers/machinedeployment/machinedeployment_controller_test.go +++ b/internal/controllers/machinedeployment/machinedeployment_controller_test.go @@ -422,7 +422,7 @@ func TestMachineDeploymentReconciler(t *testing.T) { // Update the taints of the MachineDeployment, // expect the Reconcile to be called and the MachineSet to be updated in-place. - t.Log("Updating deletion.order on the MachineDeployment") + t.Log("Updating template.spec.taints on the MachineDeployment") additionalTaint := clusterv1.MachineTaint{ Key: "additional-taint-key", Value: "additional-taint-value", diff --git a/internal/webhooks/machinepool.go b/internal/webhooks/machinepool.go index 784d1303546a..631e42e5bb89 100644 --- a/internal/webhooks/machinepool.go +++ b/internal/webhooks/machinepool.go @@ -192,7 +192,7 @@ func (webhook *MachinePool) validate(oldObj, newObj *clusterv1.MachinePool) erro if len(newObj.Spec.Template.Spec.Taints) > 0 { allErrs = append(allErrs, field.Forbidden(specPath.Child("taints"), "taints feature for MachinePools is not yet implemented")) if !feature.Gates.Enabled(feature.MachineTaintPropagation) { - allErrs = append(allErrs, field.Forbidden(specPath.Child("taints"), "taints are not allowed to be set when the feature gate MachineTaintPropagation is disabled")) + allErrs = append(allErrs, field.Forbidden(specPath.Child("taints"), "taints are not allowed to be set when the feature gate MachineTaintPropagation is disabled")) } } diff --git a/internal/webhooks/test/machine_test.go b/internal/webhooks/test/machine_test.go index 22e98e77ceb2..1dbc0f8c732d 100644 --- a/internal/webhooks/test/machine_test.go +++ b/internal/webhooks/test/machine_test.go @@ -178,7 +178,7 @@ func TestMachineTaintValidation(t *testing.T) { expectErr: true, }, { - name: "should block taint key with multuple slashes", + name: "should block taint key with multiple slashes", featureEnabled: true, machine: m.DeepCopy().WithTaints(clusterv1.MachineTaint{ Key: "one/two/with-prefix", Effect: corev1.TaintEffectNoSchedule, Propagation: clusterv1.MachineTaintPropagationAlways, From 9fa37ab35cf408c0dab173a19c58daf8f3e4cdf7 Mon Sep 17 00:00:00 2001 From: Christian Schlotter Date: Tue, 11 Nov 2025 14:55:11 +0100 Subject: [PATCH 19/21] Review fixes v5 --- internal/controllers/machine/machine_controller_noderef_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/controllers/machine/machine_controller_noderef_test.go b/internal/controllers/machine/machine_controller_noderef_test.go index b1ae274083ab..559a3d875589 100644 --- a/internal/controllers/machine/machine_controller_noderef_test.go +++ b/internal/controllers/machine/machine_controller_noderef_test.go @@ -256,7 +256,7 @@ func TestReconcileNode(t *testing.T) { }, { name: "node found, should not add taints annotation if taints feature gate is disabled", - machine: defaultMachineWithTaints.DeepCopy(), // The test only enables the feature gate if machine has taints. + machine: defaultMachineWithTaints.DeepCopy(), node: &corev1.Node{ ObjectMeta: metav1.ObjectMeta{ Name: "test-node-1", From 71e76c2bf154f079ee19b39cd7259de9d476d4d1 Mon Sep 17 00:00:00 2001 From: Christian Schlotter Date: Wed, 12 Nov 2025 13:28:17 +0100 Subject: [PATCH 20/21] Review fixes --- .../machine/machine_controller_noderef.go | 29 +++++++- .../machine_controller_noderef_test.go | 66 +++++++++++++++++++ 2 files changed, 94 insertions(+), 1 deletion(-) diff --git a/internal/controllers/machine/machine_controller_noderef.go b/internal/controllers/machine/machine_controller_noderef.go index c01975c8e3f3..08f0757066e9 100644 --- a/internal/controllers/machine/machine_controller_noderef.go +++ b/internal/controllers/machine/machine_controller_noderef.go @@ -398,7 +398,7 @@ func propagateMachineTaintsToNode(node *corev1.Node, machineTaints []clusterv1.M } // Ensure the taint is set on the node and has the correct value. - if changedTaints := taints.EnsureNodeTaint(node, convertMachineTaintToCoreV1Taint(taint)); changedTaints { + if changedTaints := ensureNodeTaintWithValue(node, convertMachineTaintToCoreV1Taint(taint)); changedTaints { changed = true } } @@ -408,6 +408,10 @@ func propagateMachineTaintsToNode(node *corev1.Node, machineTaints []clusterv1.M // Remove all identified taints from the node. for taintToDelete := range taintsToDelete { + if taintToDelete == "" { + continue + } + splitted := strings.Split(taintToDelete, ":") if len(splitted) != 2 { return changed, fmt.Errorf("invalid taint format: %q", taintToDelete) @@ -430,6 +434,29 @@ func propagateMachineTaintsToNode(node *corev1.Node, machineTaints []clusterv1.M return changed, nil } +// ensureNodeTaintWithValue makes sure the node has the Taint with the expected value. +// It returns true if the taints are modified, false otherwise. +func ensureNodeTaintWithValue(node *corev1.Node, taint corev1.Taint) bool { + for i, currentTaint := range node.Spec.Taints { + if !taint.MatchTaint(¤tTaint) { + continue + } + + // Modify the taint if the value is different. + if currentTaint.Value != taint.Value { + node.Spec.Taints[i] = taint + return true + } + + // The taint is already set and has the correct value. + return false + } + + // Add the taint if not present. + node.Spec.Taints = append(node.Spec.Taints, taint) + return true +} + // marshalMachineTaintsAnnotation marshals the tracking annotation value. func marshalMachineTaintsAnnotation(ownedTaints sets.Set[string]) string { taints := ownedTaints.UnsortedList() diff --git a/internal/controllers/machine/machine_controller_noderef_test.go b/internal/controllers/machine/machine_controller_noderef_test.go index 559a3d875589..7319f6c6bb32 100644 --- a/internal/controllers/machine/machine_controller_noderef_test.go +++ b/internal/controllers/machine/machine_controller_noderef_test.go @@ -1589,9 +1589,18 @@ func Test_propagateMachineTaintsToNode(t *testing.T) { Effect: corev1.TaintEffectNoSchedule, Propagation: clusterv1.MachineTaintPropagationAlways, } + transitionOnInitialization := transitionAlways transitionOnInitialization.Propagation = clusterv1.MachineTaintPropagationOnInitialization + transitionExistingAlwaysNewValue := existingAlwaysTaint + transitionExistingAlwaysNewValue.Value = "transition-value-new" + + transitionExistingAlwaysNewEffect := existingAlwaysTaint + transitionExistingAlwaysNewEffect.Effect = corev1.TaintEffectNoSchedule + + externalNodeTaint := corev1.Taint{Key: "external-taint", Value: "external-value", Effect: corev1.TaintEffectNoExecute} + tests := []struct { name string node *corev1.Node @@ -1642,6 +1651,15 @@ func Test_propagateMachineTaintsToNode(t *testing.T) { expectedAnnotation: "added-always:NoSchedule", expectChanged: true, }, + { + name: "Add missing but tracked Always taint, tracking annotation, no other taints", + node: builder.Node(""). + WithAnnotations(map[string]string{clusterv1.TaintsFromMachineAnnotation: "existing-always:NoExecute"}).Build(), + machineTaints: []clusterv1.MachineTaint{existingAlwaysTaint}, + expectedTaints: []corev1.Taint{convertMachineTaintToCoreV1Taint(existingAlwaysTaint)}, + expectedAnnotation: "existing-always:NoExecute", + expectChanged: true, + }, { name: "Delete Always taint, tracking annotation, no other taints", node: builder.Node(""). @@ -1701,6 +1719,54 @@ func Test_propagateMachineTaintsToNode(t *testing.T) { expectedAnnotation: "transition-taint:NoSchedule", expectChanged: true, }, + { + name: "Transition Always taint to have a new value should change the value also on the node", + node: builder.Node(""). + WithAnnotations(map[string]string{clusterv1.TaintsFromMachineAnnotation: "existing-always:NoExecute"}). + WithTaints(convertMachineTaintToCoreV1Taint(existingAlwaysTaint)).Build(), + machineTaints: []clusterv1.MachineTaint{transitionExistingAlwaysNewValue}, + expectedTaints: []corev1.Taint{convertMachineTaintToCoreV1Taint(transitionExistingAlwaysNewValue)}, + expectedAnnotation: "existing-always:NoExecute", + expectChanged: true, + }, + { + name: "Transition Always taint to have a new effect should change the effect also on the node", + node: builder.Node(""). + WithAnnotations(map[string]string{clusterv1.TaintsFromMachineAnnotation: "existing-always:NoExecute"}). + WithTaints(convertMachineTaintToCoreV1Taint(existingAlwaysTaint)).Build(), + machineTaints: []clusterv1.MachineTaint{transitionExistingAlwaysNewEffect}, + expectedTaints: []corev1.Taint{convertMachineTaintToCoreV1Taint(transitionExistingAlwaysNewEffect)}, + expectedAnnotation: "existing-always:NoSchedule", + expectChanged: true, + }, + { + name: "Add missing taints, no tracking annotation, preserve other taints", + node: builder.Node(""). + WithTaints(externalNodeTaint).Build(), + machineTaints: []clusterv1.MachineTaint{alwaysTaint, onInitializationTaint}, + expectedTaints: []corev1.Taint{externalNodeTaint, convertMachineTaintToCoreV1Taint(alwaysTaint), convertMachineTaintToCoreV1Taint(onInitializationTaint)}, + expectedAnnotation: "added-always:NoSchedule", + expectChanged: true, + }, + { + name: "Adopt existing taint, no tracking annotation", + node: builder.Node(""). + WithTaints(convertMachineTaintToCoreV1Taint(existingAlwaysTaint)).Build(), + machineTaints: []clusterv1.MachineTaint{existingAlwaysTaint}, + expectedTaints: []corev1.Taint{convertMachineTaintToCoreV1Taint(existingAlwaysTaint)}, + expectedAnnotation: "existing-always:NoExecute", + expectChanged: true, + }, + { + name: "Recover from broken tracking annotation", + node: builder.Node(""). + WithAnnotations(map[string]string{clusterv1.TaintsFromMachineAnnotation: "existing-always:NoExecute,"}). + WithTaints(convertMachineTaintToCoreV1Taint(existingAlwaysTaint)).Build(), + machineTaints: []clusterv1.MachineTaint{existingAlwaysTaint}, + expectedTaints: []corev1.Taint{convertMachineTaintToCoreV1Taint(existingAlwaysTaint)}, + expectedAnnotation: "existing-always:NoExecute", + expectChanged: true, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { From 0a902b3901f6179834ffde2d953cb01a4f19e1d3 Mon Sep 17 00:00:00 2001 From: Christian Schlotter Date: Wed, 12 Nov 2025 13:31:24 +0100 Subject: [PATCH 21/21] fixup godoc and make generate --- api/core/v1beta2/machine_types.go | 1 + api/core/v1beta2/zz_generated.openapi.go | 2 +- config/crd/bases/cluster.x-k8s.io_machinedeployments.yaml | 1 + config/crd/bases/cluster.x-k8s.io_machinepools.yaml | 1 + config/crd/bases/cluster.x-k8s.io_machines.yaml | 1 + config/crd/bases/cluster.x-k8s.io_machinesets.yaml | 1 + 6 files changed, 6 insertions(+), 1 deletion(-) diff --git a/api/core/v1beta2/machine_types.go b/api/core/v1beta2/machine_types.go index 44a6f794f9c4..e1be384800ff 100644 --- a/api/core/v1beta2/machine_types.go +++ b/api/core/v1beta2/machine_types.go @@ -495,6 +495,7 @@ type MachineSpec struct { // Only those taints defined in this list will be added or removed by core Cluster API controllers. // // There can be at most 64 taints. + // A pod would have to tolerate all existing taints to run on the corresponding node. // // NOTE: This list is implemented as a "map" type, meaning that individual elements can be managed by different owners. // +optional diff --git a/api/core/v1beta2/zz_generated.openapi.go b/api/core/v1beta2/zz_generated.openapi.go index c349d50cb1d0..4f5fd72dddf2 100644 --- a/api/core/v1beta2/zz_generated.openapi.go +++ b/api/core/v1beta2/zz_generated.openapi.go @@ -6175,7 +6175,7 @@ func schema_cluster_api_api_core_v1beta2_MachineSpec(ref common.ReferenceCallbac }, }, SchemaProps: spec.SchemaProps{ - Description: "taints are the node taints that Cluster API will manage. This list is not necessarily complete: other Kubernetes components may add or remove other taints from nodes, e.g. the node controller might add the node.kubernetes.io/not-ready taint. Only those taints defined in this list will be added or removed by core Cluster API controllers.\n\nThere can be at most 64 taints.\n\nNOTE: This list is implemented as a \"map\" type, meaning that individual elements can be managed by different owners.", + Description: "taints are the node taints that Cluster API will manage. This list is not necessarily complete: other Kubernetes components may add or remove other taints from nodes, e.g. the node controller might add the node.kubernetes.io/not-ready taint. Only those taints defined in this list will be added or removed by core Cluster API controllers.\n\nThere can be at most 64 taints. A pod would have to tolerate all existing taints to run on the corresponding node.\n\nNOTE: This list is implemented as a \"map\" type, meaning that individual elements can be managed by different owners.", Type: []string{"array"}, Items: &spec.SchemaOrArray{ Schema: &spec.Schema{ diff --git a/config/crd/bases/cluster.x-k8s.io_machinedeployments.yaml b/config/crd/bases/cluster.x-k8s.io_machinedeployments.yaml index ba3697d9acd8..58cba176177b 100644 --- a/config/crd/bases/cluster.x-k8s.io_machinedeployments.yaml +++ b/config/crd/bases/cluster.x-k8s.io_machinedeployments.yaml @@ -2348,6 +2348,7 @@ spec: Only those taints defined in this list will be added or removed by core Cluster API controllers. There can be at most 64 taints. + A pod would have to tolerate all existing taints to run on the corresponding node. NOTE: This list is implemented as a "map" type, meaning that individual elements can be managed by different owners. items: diff --git a/config/crd/bases/cluster.x-k8s.io_machinepools.yaml b/config/crd/bases/cluster.x-k8s.io_machinepools.yaml index d0c199a26930..e968bc6d1524 100644 --- a/config/crd/bases/cluster.x-k8s.io_machinepools.yaml +++ b/config/crd/bases/cluster.x-k8s.io_machinepools.yaml @@ -1952,6 +1952,7 @@ spec: Only those taints defined in this list will be added or removed by core Cluster API controllers. There can be at most 64 taints. + A pod would have to tolerate all existing taints to run on the corresponding node. NOTE: This list is implemented as a "map" type, meaning that individual elements can be managed by different owners. items: diff --git a/config/crd/bases/cluster.x-k8s.io_machines.yaml b/config/crd/bases/cluster.x-k8s.io_machines.yaml index dc7c4cf6582a..a31f2604eb6a 100644 --- a/config/crd/bases/cluster.x-k8s.io_machines.yaml +++ b/config/crd/bases/cluster.x-k8s.io_machines.yaml @@ -1762,6 +1762,7 @@ spec: Only those taints defined in this list will be added or removed by core Cluster API controllers. There can be at most 64 taints. + A pod would have to tolerate all existing taints to run on the corresponding node. NOTE: This list is implemented as a "map" type, meaning that individual elements can be managed by different owners. items: diff --git a/config/crd/bases/cluster.x-k8s.io_machinesets.yaml b/config/crd/bases/cluster.x-k8s.io_machinesets.yaml index 9019e12a6508..6f7e928372ee 100644 --- a/config/crd/bases/cluster.x-k8s.io_machinesets.yaml +++ b/config/crd/bases/cluster.x-k8s.io_machinesets.yaml @@ -2004,6 +2004,7 @@ spec: Only those taints defined in this list will be added or removed by core Cluster API controllers. There can be at most 64 taints. + A pod would have to tolerate all existing taints to run on the corresponding node. NOTE: This list is implemented as a "map" type, meaning that individual elements can be managed by different owners. items: