Skip to content

Commit 89fd7f3

Browse files
committed
Review changes v1
1 parent 988ff00 commit 89fd7f3

File tree

16 files changed

+78
-48
lines changed

16 files changed

+78
-48
lines changed

api/core/v1beta1/conversion.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -408,11 +408,11 @@ func (src *Machine) ConvertTo(dstRaw conversion.Hub) error {
408408
// Recover other values.
409409
if ok {
410410
dst.Spec.MinReadySeconds = restored.Spec.MinReadySeconds
411+
dst.Spec.Taints = restored.Spec.Taints
411412
// Restore the phase, this also means that any client using v1beta1 during a round-trip
412413
// won't be able to write the Phase field. But that's okay as the only client writing the Phase
413414
// field should be the Machine controller.
414415
dst.Status.Phase = restored.Status.Phase
415-
dst.Spec.Taints = restored.Spec.Taints
416416
}
417417

418418
return nil

api/core/v1beta2/common_types.go

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -419,15 +419,17 @@ type MachineTaint struct {
419419

420420
// value is the taint value corresponding to the taint key.
421421
// +optional
422+
// +kubebuilder:validation:MinLength=1
422423
// +kubebuilder:validation:MaxLength=63
423-
Value *string `json:"value,omitempty"`
424+
Value string `json:"value,omitempty"`
424425

425426
// effect is the effect for the taint. Valid values are NoSchedule, PreferNoSchedule and NoExecute.
426427
// +required
427428
// +kubebuilder:validation:Enum=NoSchedule;PreferNoSchedule;NoExecute
428429
Effect corev1.TaintEffect `json:"effect,omitempty"`
429430

430431
// propagation defines how this taint should be propagated to nodes.
432+
// Valid values are 'Always' and 'OnInitialization'.
431433
// Always: The taint will be continuously reconciled. If it is not set for a node, it will be added during reconciliation.
432434
// OnInitialization: The taint will be added during node initialization. If it gets removed from the node later on it will not get added again.
433435
// +required
@@ -439,18 +441,18 @@ type MachineTaint struct {
439441
type MachineTaintPropagation string
440442

441443
const (
442-
// TaintPropagationAlways means the taint should be continuously reconciled and kept on the node.
444+
// MachineTaintPropagationAlways means the taint should be continuously reconciled and kept on the node.
443445
// - If an Always taint is added to the Machine, the taint will be added to the node.
444446
// - If an Always taint is removed from the Machine, the taint will be removed from the node.
445447
// - If an OnInitialization taint is changed to Always, the Machine controller will ensure the taint is set on the node.
446448
// - If an Always taint is removed from the node, it will be re-added during reconciliation.
447-
TaintPropagationAlways MachineTaintPropagation = "Always"
449+
MachineTaintPropagationAlways MachineTaintPropagation = "Always"
448450

449-
// TaintPropagationOnInitialization means the taint should be set once during initialization and then
451+
// MachineTaintPropagationOnInitialization means the taint should be set once during initialization and then
450452
// left alone.
451453
// - If an OnInitialization taint is added to the Machine, the taint will only be added to the node on initialization.
452454
// - If an OnInitialization taint is removed from the Machine nothing will be changed on the node.
453455
// - If an Always taint is changed to OnInitialization, the taint will only be added to the node on initialization.
454456
// - If an OnInitialization taint is removed from the node, it will not be re-added during reconciliation.
455-
TaintPropagationOnInitialization MachineTaintPropagation = "OnInitialization"
457+
MachineTaintPropagationOnInitialization MachineTaintPropagation = "OnInitialization"
456458
)

api/core/v1beta2/machine_types.go

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -490,11 +490,13 @@ type MachineSpec struct {
490490
Deletion MachineDeletionSpec `json:"deletion,omitempty,omitzero"`
491491

492492
// taints are the node taints that Cluster API will manage.
493-
// This list is not necessarily complete: other Kubernetes components may add or remove other taints.
493+
// This list is not necessarily complete: other Kubernetes components may add or remove other taints from nodes,
494+
// e.g. the node controller might add the node.kubernetes.io/not-ready taint.
494495
// Only those taints defined in this list will be added or removed by core Cluster API controllers.
495496
//
497+
// There can be at most 64 taints.
498+
//
496499
// NOTE: This list is implemented as a "map" type, meaning that individual elements can be managed by different owners.
497-
// 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.
498500
// +optional
499501
// +listType=map
500502
// +listMapKey=key

controlplane/kubeadm/internal/controllers/inplace_canupdatemachine_test.go

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,6 @@ import (
2323
"fmt"
2424
"testing"
2525

26-
"github.com/google/go-cmp/cmp"
2726
. "github.com/onsi/gomega"
2827
"github.com/pkg/errors"
2928
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -465,7 +464,7 @@ func Test_canExtensionsUpdateMachine(t *testing.T) {
465464
g.Expect(err).ToNot(HaveOccurred())
466465
}
467466
g.Expect(canUpdateMachine).To(Equal(tt.wantCanUpdateMachine))
468-
g.Expect(reasons).To(BeEquivalentTo(tt.wantReasons), cmp.Diff(reasons, tt.wantReasons))
467+
g.Expect(reasons).To(BeComparableTo(tt.wantReasons))
469468
})
470469
}
471470
}

docs/book/src/tasks/experimental-features/experimental-features.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,8 @@ Currently Cluster API has the following experimental features:
1414
* `ClusterTopology` (env var: `CLUSTER_TOPOLOGY`): [ClusterClass](./cluster-class/index.md)
1515
* `RuntimeSDK` (env var: `EXP_RUNTIME_SDK`): [RuntimeSDK](./runtime-sdk/index.md)
1616
* `KubeadmBootstrapFormatIgnition` (env var: `EXP_KUBEADM_BOOTSTRAP_FORMAT_IGNITION`): [Ignition](./ignition.md)
17+
* `MachineTaintPropagation` (env var: `EXP_MACHINE_TAINT_PROPAGATION`):
18+
* Allows in-place propagation of taints to nodes using the taint fields within Machines, MachineSets, and MachineDeployments.
1719

1820
## Enabling Experimental Features for Management Clusters Started with clusterctl
1921

feature/feature.go

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,6 @@ const (
8383
InPlaceUpdates featuregate.Feature = "InPlaceUpdates"
8484

8585
// MachineTaintPropagation is a feature gate for the machine taint propagation functionality.
86-
// See docs/proposals/20250513-propogate-taints.md for more details.
8786
//
8887
// alpha: v1.12
8988
MachineTaintPropagation featuregate.Feature = "MachineTaintPropagation"

internal/controllers/machine/machine_controller_noderef.go

Lines changed: 7 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,6 @@ import (
2929
"k8s.io/apimachinery/pkg/runtime/schema"
3030
"k8s.io/apimachinery/pkg/util/sets"
3131
"k8s.io/klog/v2"
32-
"k8s.io/utils/ptr"
3332
ctrl "sigs.k8s.io/controller-runtime"
3433
"sigs.k8s.io/controller-runtime/pkg/client"
3534

@@ -342,7 +341,7 @@ func (r *Reconciler) patchNode(ctx context.Context, remoteClient client.Client,
342341
if feature.Gates.Enabled(feature.MachineTaintPropagation) {
343342
var err error
344343
if propagateTaintsChanges, err = propagateMachineTaintsToNode(newNode, m.Spec.Taints); err != nil {
345-
return errors.Wrapf(err, "failed to propagate machine taints to node %s", klog.KRef("", node.Name))
344+
return errors.Wrapf(err, "failed to propagate Machine taints to Node %s", klog.KObj(node))
346345
}
347346
}
348347

@@ -361,10 +360,8 @@ func (r *Reconciler) patchNode(ctx context.Context, remoteClient client.Client,
361360
return nil
362361
}
363362

364-
var mergeOptions []client.MergeFromOption
365-
if feature.Gates.Enabled(feature.MachineTaintPropagation) {
366-
mergeOptions = append(mergeOptions, client.MergeFromWithOptimisticLock{})
367-
}
363+
// Use optimistic locking to avoid conflicts with other controllers.
364+
mergeOptions := []client.MergeFromOption{client.MergeFromWithOptimisticLock{}}
368365

369366
return remoteClient.Patch(ctx, newNode, client.StrategicMergeFrom(node, mergeOptions...))
370367
}
@@ -389,14 +386,14 @@ func propagateMachineTaintsToNode(node *corev1.Node, machineTaints []clusterv1.M
389386
// Collect Always and OnInitialization taints to identify taints to delete.
390387
// Separating Always taints so the tracking annotation can be updated accordingly.
391388
switch taint.Propagation {
392-
case clusterv1.TaintPropagationAlways:
389+
case clusterv1.MachineTaintPropagationAlways:
393390
newOwnedTaints.Insert(fmt.Sprintf("%s:%s", taint.Key, taint.Effect))
394-
case clusterv1.TaintPropagationOnInitialization:
391+
case clusterv1.MachineTaintPropagationOnInitialization:
395392
onInitializationTaints.Insert(fmt.Sprintf("%s:%s", taint.Key, taint.Effect))
396393
}
397394

398395
// Only add OnInitialization taints if the tracking annotation has not been set yet.
399-
if taint.Propagation == clusterv1.TaintPropagationOnInitialization && nodeTaintsInitialized {
396+
if taint.Propagation == clusterv1.MachineTaintPropagationOnInitialization && nodeTaintsInitialized {
400397
continue
401398
}
402399

@@ -454,7 +451,7 @@ func unmarshalMachineTaintsAnnotation(annotationValue string) sets.Set[string] {
454451
func convertMachineTaintToCoreV1Taint(machineTaint clusterv1.MachineTaint) corev1.Taint {
455452
return corev1.Taint{
456453
Key: machineTaint.Key,
457-
Value: ptr.Deref(machineTaint.Value, ""),
454+
Value: machineTaint.Value,
458455
Effect: machineTaint.Effect,
459456
}
460457
}

internal/controllers/machine/machine_controller_noderef_test.go

Lines changed: 13 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -65,15 +65,15 @@ func TestReconcileNode(t *testing.T) {
6565
defaultMachineWithTaints.Spec.Taints = []clusterv1.MachineTaint{
6666
{
6767
Key: "test-always-taint",
68-
Value: ptr.To("test-value1"),
68+
Value: "test-value1",
6969
Effect: corev1.TaintEffectNoSchedule,
70-
Propagation: clusterv1.TaintPropagationAlways,
70+
Propagation: clusterv1.MachineTaintPropagationAlways,
7171
},
7272
{
7373
Key: "test-on-initialization-taint",
74-
Value: ptr.To("test-value2"),
74+
Value: "test-value2",
7575
Effect: corev1.TaintEffectNoSchedule,
76-
Propagation: clusterv1.TaintPropagationOnInitialization,
76+
Propagation: clusterv1.MachineTaintPropagationOnInitialization,
7777
},
7878
}
7979

@@ -1556,32 +1556,32 @@ func Test_shouldNodeHaveOutdatedTaint(t *testing.T) {
15561556
func Test_propagateMachineTaintsToNode(t *testing.T) {
15571557
alwaysTaint := clusterv1.MachineTaint{
15581558
Key: "added-always",
1559-
Value: ptr.To("always-value"),
1559+
Value: "always-value",
15601560
Effect: corev1.TaintEffectNoSchedule,
1561-
Propagation: clusterv1.TaintPropagationAlways,
1561+
Propagation: clusterv1.MachineTaintPropagationAlways,
15621562
}
15631563
onInitializationTaint := clusterv1.MachineTaint{
15641564
Key: "added-on-initialization",
1565-
Value: ptr.To("on-initialization-value"),
1565+
Value: "on-initialization-value",
15661566
Effect: corev1.TaintEffectNoSchedule,
1567-
Propagation: clusterv1.TaintPropagationOnInitialization,
1567+
Propagation: clusterv1.MachineTaintPropagationOnInitialization,
15681568
}
15691569

15701570
existingAlwaysTaint := clusterv1.MachineTaint{
15711571
Key: "existing-always",
1572-
Value: ptr.To("existing-always-value"),
1572+
Value: "existing-always-value",
15731573
Effect: corev1.TaintEffectNoExecute,
1574-
Propagation: clusterv1.TaintPropagationAlways,
1574+
Propagation: clusterv1.MachineTaintPropagationAlways,
15751575
}
15761576

15771577
transitionAlways := clusterv1.MachineTaint{
15781578
Key: "transition-taint",
1579-
Value: ptr.To("transition-value"),
1579+
Value: "transition-value",
15801580
Effect: corev1.TaintEffectNoSchedule,
1581-
Propagation: clusterv1.TaintPropagationAlways,
1581+
Propagation: clusterv1.MachineTaintPropagationAlways,
15821582
}
15831583
transitionOnInitialization := transitionAlways
1584-
transitionOnInitialization.Propagation = clusterv1.TaintPropagationOnInitialization
1584+
transitionOnInitialization.Propagation = clusterv1.MachineTaintPropagationOnInitialization
15851585

15861586
tests := []struct {
15871587
name string

internal/controllers/machinedeployment/machinedeployment_rollout_planner_test.go

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -282,6 +282,9 @@ func TestComputeDesiredMS(t *testing.T) {
282282
NodeVolumeDetachTimeoutSeconds: duration10s,
283283
NodeDeletionTimeoutSeconds: duration10s,
284284
},
285+
Taints: []clusterv1.MachineTaint{
286+
{Key: "taint-key", Value: "taint-value", Effect: corev1.TaintEffectNoSchedule, Propagation: clusterv1.MachineTaintPropagationAlways},
287+
},
285288
},
286289
},
287290
},
@@ -375,6 +378,7 @@ func TestComputeDesiredMS(t *testing.T) {
375378
NodeVolumeDetachTimeoutSeconds: nil,
376379
NodeDeletionTimeoutSeconds: nil,
377380
},
381+
Taints: nil,
378382
},
379383
},
380384
},
@@ -398,6 +402,7 @@ func TestComputeDesiredMS(t *testing.T) {
398402
expectedMS.Spec.Template.Spec.Deletion.NodeDrainTimeoutSeconds = deployment.Spec.Template.Spec.Deletion.NodeDrainTimeoutSeconds
399403
expectedMS.Spec.Template.Spec.Deletion.NodeDeletionTimeoutSeconds = deployment.Spec.Template.Spec.Deletion.NodeDeletionTimeoutSeconds
400404
expectedMS.Spec.Template.Spec.Deletion.NodeVolumeDetachTimeoutSeconds = deployment.Spec.Template.Spec.Deletion.NodeVolumeDetachTimeoutSeconds
405+
expectedMS.Spec.Template.Spec.Taints = deployment.Spec.Template.Spec.Taints
401406

402407
g := NewWithT(t)
403408
actualMS, err := computeDesiredMS(ctx, deployment, currentMS)

internal/controllers/machinedeployment/mdutil/util.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -428,6 +428,7 @@ func MachineTemplateDeepCopyRolloutFields(template *clusterv1.MachineTemplateSpe
428428
templateCopy.Spec.Deletion.NodeDrainTimeoutSeconds = nil
429429
templateCopy.Spec.Deletion.NodeDeletionTimeoutSeconds = nil
430430
templateCopy.Spec.Deletion.NodeVolumeDetachTimeoutSeconds = nil
431+
templateCopy.Spec.Taints = nil
431432

432433
return templateCopy
433434
}

0 commit comments

Comments
 (0)