Skip to content

Commit 0ec3061

Browse files
chrischdichrischdi
committed
taint propagation: implement unit tests for machine controller
1 parent f7681d1 commit 0ec3061

File tree

3 files changed

+289
-3
lines changed

3 files changed

+289
-3
lines changed

‎internal/controllers/machine/machine_controller_noderef_test.go‎

Lines changed: 253 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ import (
2727
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
2828
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
2929
"k8s.io/client-go/tools/record"
30+
utilfeature "k8s.io/component-base/featuregate/testing"
3031
"k8s.io/utils/ptr"
3132
ctrl "sigs.k8s.io/controller-runtime"
3233
"sigs.k8s.io/controller-runtime/pkg/client"
@@ -39,6 +40,7 @@ import (
3940
"sigs.k8s.io/cluster-api/api/core/v1beta2/index"
4041
"sigs.k8s.io/cluster-api/controllers/clustercache"
4142
"sigs.k8s.io/cluster-api/controllers/remote"
43+
"sigs.k8s.io/cluster-api/feature"
4244
"sigs.k8s.io/cluster-api/internal/topology/ownerrefs"
4345
"sigs.k8s.io/cluster-api/util"
4446
"sigs.k8s.io/cluster-api/util/kubeconfig"
@@ -59,6 +61,22 @@ func TestReconcileNode(t *testing.T) {
5961
},
6062
}
6163

64+
defaultMachineWithTaints := defaultMachine.DeepCopy()
65+
defaultMachineWithTaints.Spec.Taints = []clusterv1.MachineTaint{
66+
{
67+
Key: "test-always-taint",
68+
Value: ptr.To("test-value1"),
69+
Effect: corev1.TaintEffectNoSchedule,
70+
Propagation: clusterv1.TaintPropagationAlways,
71+
},
72+
{
73+
Key: "test-on-initialization-taint",
74+
Value: ptr.To("test-value2"),
75+
Effect: corev1.TaintEffectNoSchedule,
76+
Propagation: clusterv1.TaintPropagationOnInitialization,
77+
},
78+
}
79+
6280
defaultCluster := &clusterv1.Cluster{
6381
ObjectMeta: metav1.ObjectMeta{
6482
Name: "test-cluster",
@@ -75,6 +93,7 @@ func TestReconcileNode(t *testing.T) {
7593
expectError bool
7694
expected func(g *WithT, m *clusterv1.Machine)
7795
expectNodeGetError bool
96+
expectedNode func(g *WithT, m *corev1.Node)
7897
}{
7998
{
8099
name: "No op if provider ID is not set",
@@ -186,12 +205,87 @@ func TestReconcileNode(t *testing.T) {
186205
expectResult: ctrl.Result{},
187206
expectError: false,
188207
},
208+
{
209+
name: "node found, should propagate taints",
210+
machine: defaultMachineWithTaints.DeepCopy(),
211+
node: &corev1.Node{
212+
ObjectMeta: metav1.ObjectMeta{
213+
Name: "test-node-1",
214+
},
215+
Spec: corev1.NodeSpec{
216+
ProviderID: "aws://us-east-1/test-node-1",
217+
},
218+
Status: corev1.NodeStatus{
219+
NodeInfo: corev1.NodeSystemInfo{
220+
MachineID: "foo",
221+
},
222+
Addresses: []corev1.NodeAddress{
223+
{
224+
Type: corev1.NodeInternalIP,
225+
Address: "1.1.1.1",
226+
},
227+
},
228+
},
229+
},
230+
nodeGetErr: false,
231+
expectResult: ctrl.Result{},
232+
expectError: false,
233+
expectedNode: func(g *WithT, n *corev1.Node) {
234+
g.Expect(n.Spec.Taints).To(BeComparableTo([]corev1.Taint{
235+
{
236+
Key: "test-always-taint",
237+
Value: "test-value1",
238+
Effect: corev1.TaintEffectNoSchedule,
239+
},
240+
{
241+
Key: "test-on-initialization-taint",
242+
Value: "test-value2",
243+
Effect: corev1.TaintEffectNoSchedule,
244+
},
245+
}))
246+
g.Expect(n.Annotations[clusterv1.TaintsFromMachineAnnotation]).To(Equal("test-always-taint:NoSchedule"))
247+
},
248+
},
249+
{
250+
name: "node found, should not add taints annotation if taints feature gate is disabled",
251+
machine: defaultMachine.DeepCopy(), // The test only enables the feature gate if machine has taints.
252+
node: &corev1.Node{
253+
ObjectMeta: metav1.ObjectMeta{
254+
Name: "test-node-1",
255+
},
256+
Spec: corev1.NodeSpec{
257+
ProviderID: "aws://us-east-1/test-node-1",
258+
},
259+
Status: corev1.NodeStatus{
260+
NodeInfo: corev1.NodeSystemInfo{
261+
MachineID: "foo",
262+
},
263+
Addresses: []corev1.NodeAddress{
264+
{
265+
Type: corev1.NodeInternalIP,
266+
Address: "1.1.1.1",
267+
},
268+
},
269+
},
270+
},
271+
nodeGetErr: false,
272+
expectResult: ctrl.Result{},
273+
expectError: false,
274+
expectedNode: func(g *WithT, n *corev1.Node) {
275+
g.Expect(n.Spec.Taints).To(BeEmpty())
276+
g.Expect(n.Annotations).ToNot(HaveKey(clusterv1.TaintsFromMachineAnnotation))
277+
},
278+
},
189279
}
190280

191281
for _, tc := range testCases {
192282
t.Run(tc.name, func(t *testing.T) {
193283
g := NewWithT(t)
194284

285+
if tc.machine.Spec.Taints != nil {
286+
utilfeature.SetFeatureGateDuringTest(t, feature.Gates, feature.MachineTaintPropagation, true)
287+
}
288+
195289
c := fake.NewClientBuilder().WithObjects(tc.machine).WithIndex(&corev1.Node{}, "spec.providerID", index.NodeByProviderID).Build()
196290
if tc.nodeGetErr {
197291
c = fake.NewClientBuilder().WithObjects(tc.machine).Build() // No Index
@@ -221,6 +315,12 @@ func TestReconcileNode(t *testing.T) {
221315
}
222316

223317
g.Expect(s.nodeGetError != nil).To(Equal(tc.expectNodeGetError))
318+
319+
if tc.expectedNode != nil {
320+
node := &corev1.Node{}
321+
g.Expect(c.Get(ctx, client.ObjectKeyFromObject(tc.node), node)).To(Succeed())
322+
tc.expectedNode(g, node)
323+
}
224324
})
225325
}
226326
}
@@ -1489,3 +1589,156 @@ func Test_shouldNodeHaveOutdatedTaint(t *testing.T) {
14891589
})
14901590
}
14911591
}
1592+
1593+
func Test_propagateMachineTaintsToNode(t *testing.T) {
1594+
alwaysTaint := clusterv1.MachineTaint{
1595+
Key: "added-always",
1596+
Value: ptr.To("always-value"),
1597+
Effect: corev1.TaintEffectNoSchedule,
1598+
Propagation: clusterv1.TaintPropagationAlways,
1599+
}
1600+
onInitializationTaint := clusterv1.MachineTaint{
1601+
Key: "added-on-initialization",
1602+
Value: ptr.To("on-initialization-value"),
1603+
Effect: corev1.TaintEffectNoSchedule,
1604+
Propagation: clusterv1.TaintPropagationOnInitialization,
1605+
}
1606+
1607+
existingAlwaysTaint := clusterv1.MachineTaint{
1608+
Key: "existing-always",
1609+
Value: ptr.To("existing-always-value"),
1610+
Effect: corev1.TaintEffectNoExecute,
1611+
Propagation: clusterv1.TaintPropagationAlways,
1612+
}
1613+
1614+
transitionAlways := clusterv1.MachineTaint{
1615+
Key: "transition-taint",
1616+
Value: ptr.To("transition-value"),
1617+
Effect: corev1.TaintEffectNoSchedule,
1618+
Propagation: clusterv1.TaintPropagationAlways,
1619+
}
1620+
transitionOnInitialization := transitionAlways
1621+
transitionOnInitialization.Propagation = clusterv1.TaintPropagationOnInitialization
1622+
1623+
tests := []struct {
1624+
name string
1625+
node *corev1.Node
1626+
machineTaints []clusterv1.MachineTaint
1627+
expectedTaints []corev1.Taint
1628+
expectedAnnotation string
1629+
expectChanged bool
1630+
}{
1631+
{
1632+
name: "no taints set, no taints to set, adds empty annotation",
1633+
node: builder.Node("").Build(),
1634+
machineTaints: []clusterv1.MachineTaint{},
1635+
expectedTaints: nil,
1636+
expectedAnnotation: "",
1637+
expectChanged: true,
1638+
},
1639+
{
1640+
name: "no taints set, no taints to set, keeps empty annotation",
1641+
node: builder.Node("").WithAnnotations(map[string]string{clusterv1.TaintsFromMachineAnnotation: ""}).Build(),
1642+
machineTaints: []clusterv1.MachineTaint{},
1643+
expectedTaints: nil,
1644+
expectedAnnotation: "",
1645+
expectChanged: false,
1646+
},
1647+
{
1648+
name: "no taints set, no taints to set, cleans up empty annotation",
1649+
node: builder.Node("").WithAnnotations(map[string]string{clusterv1.TaintsFromMachineAnnotation: "does-not-exist:NoSchedule"}).Build(),
1650+
machineTaints: []clusterv1.MachineTaint{},
1651+
expectedTaints: []corev1.Taint{}, // The taints utility initializes an empty slice on no-op removal.
1652+
expectedAnnotation: "",
1653+
expectChanged: true,
1654+
},
1655+
// Basic Always taint operations:
1656+
{
1657+
name: "Add missing Always taint, no tracking annotation, no other taints",
1658+
node: builder.Node("").Build(),
1659+
machineTaints: []clusterv1.MachineTaint{alwaysTaint},
1660+
expectedTaints: []corev1.Taint{convertMachineTaintToCoreV1Taint(alwaysTaint)},
1661+
expectedAnnotation: "added-always:NoSchedule",
1662+
expectChanged: true,
1663+
},
1664+
{
1665+
name: "Add missing Always taint, tracking annotation, no other taints",
1666+
node: builder.Node("").
1667+
WithAnnotations(map[string]string{clusterv1.TaintsFromMachineAnnotation: ""}).Build(),
1668+
machineTaints: []clusterv1.MachineTaint{alwaysTaint},
1669+
expectedTaints: []corev1.Taint{convertMachineTaintToCoreV1Taint(alwaysTaint)},
1670+
expectedAnnotation: "added-always:NoSchedule",
1671+
expectChanged: true,
1672+
},
1673+
{
1674+
name: "Delete Always taint, tracking annotation, no other taints",
1675+
node: builder.Node("").
1676+
WithAnnotations(map[string]string{clusterv1.TaintsFromMachineAnnotation: "existing-always:NoExecute"}).
1677+
WithTaints(convertMachineTaintToCoreV1Taint(existingAlwaysTaint)).Build(),
1678+
machineTaints: []clusterv1.MachineTaint{},
1679+
expectedTaints: []corev1.Taint{},
1680+
expectedAnnotation: "",
1681+
expectChanged: true,
1682+
},
1683+
// Basic OnInitialization taint operations:
1684+
{
1685+
name: "Add missing OnInitialization taint, no tracking annotation, no other taints",
1686+
node: builder.Node("").Build(),
1687+
machineTaints: []clusterv1.MachineTaint{onInitializationTaint},
1688+
expectedTaints: []corev1.Taint{convertMachineTaintToCoreV1Taint(onInitializationTaint)},
1689+
expectedAnnotation: "",
1690+
expectChanged: true,
1691+
},
1692+
{
1693+
name: "Don't add missing OnInitialization taint, tracking annotation, no other taints",
1694+
node: builder.Node("").
1695+
WithAnnotations(map[string]string{clusterv1.TaintsFromMachineAnnotation: ""}).Build(),
1696+
machineTaints: []clusterv1.MachineTaint{onInitializationTaint},
1697+
expectedTaints: nil,
1698+
expectedAnnotation: "",
1699+
expectChanged: false,
1700+
},
1701+
{
1702+
name: "Don't delete OnInitialization taint, tracking annotation, no other taints",
1703+
node: builder.Node("").
1704+
WithAnnotations(map[string]string{clusterv1.TaintsFromMachineAnnotation: ""}).
1705+
WithTaints(convertMachineTaintToCoreV1Taint(onInitializationTaint)).Build(),
1706+
machineTaints: []clusterv1.MachineTaint{},
1707+
expectedTaints: []corev1.Taint{convertMachineTaintToCoreV1Taint(onInitializationTaint)},
1708+
expectedAnnotation: "",
1709+
expectChanged: false,
1710+
},
1711+
// Transitions
1712+
{
1713+
name: "Transition Always to OnInitialization should remove from annotation but be kept on the node",
1714+
node: builder.Node("").
1715+
WithAnnotations(map[string]string{clusterv1.TaintsFromMachineAnnotation: "transition-taint:NoSchedule"}).
1716+
WithTaints(convertMachineTaintToCoreV1Taint(transitionAlways)).Build(),
1717+
machineTaints: []clusterv1.MachineTaint{transitionOnInitialization},
1718+
expectedTaints: []corev1.Taint{convertMachineTaintToCoreV1Taint(transitionAlways)},
1719+
expectedAnnotation: "",
1720+
expectChanged: true,
1721+
},
1722+
{
1723+
name: "Transition OnInitialization to Always should add to annotation and be kept on the node",
1724+
node: builder.Node("").
1725+
WithAnnotations(map[string]string{clusterv1.TaintsFromMachineAnnotation: ""}).
1726+
WithTaints(convertMachineTaintToCoreV1Taint(transitionOnInitialization)).Build(),
1727+
machineTaints: []clusterv1.MachineTaint{transitionAlways},
1728+
expectedTaints: []corev1.Taint{convertMachineTaintToCoreV1Taint(transitionOnInitialization)},
1729+
expectedAnnotation: "transition-taint:NoSchedule",
1730+
expectChanged: true,
1731+
},
1732+
}
1733+
for _, tt := range tests {
1734+
t.Run(tt.name, func(t *testing.T) {
1735+
g := NewWithT(t)
1736+
changed, err := propagateMachineTaintsToNode(tt.node, tt.machineTaints)
1737+
g.Expect(err).ToNot(HaveOccurred())
1738+
g.Expect(changed).To(Equal(tt.expectChanged))
1739+
g.Expect(tt.node.Spec.Taints).To(Equal(tt.expectedTaints))
1740+
g.Expect(tt.node.Annotations).To(HaveKey(clusterv1.TaintsFromMachineAnnotation))
1741+
g.Expect(tt.node.Annotations[clusterv1.TaintsFromMachineAnnotation]).To(Equal(tt.expectedAnnotation))
1742+
})
1743+
}
1744+
}

‎util/test/builder/builders.go‎

Lines changed: 21 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1609,8 +1609,10 @@ func (c *TestControlPlaneBuilder) Build() *unstructured.Unstructured {
16091609

16101610
// NodeBuilder holds the variables required to build a Node.
16111611
type NodeBuilder struct {
1612-
name string
1613-
status corev1.NodeStatus
1612+
name string
1613+
annotations map[string]string
1614+
taints []corev1.Taint
1615+
status corev1.NodeStatus
16141616
}
16151617

16161618
// Node returns a NodeBuilder.
@@ -1620,6 +1622,18 @@ func Node(name string) *NodeBuilder {
16201622
}
16211623
}
16221624

1625+
// WithAnnotations adds the given annotations to the NodeBuilder.
1626+
func (n *NodeBuilder) WithAnnotations(annotations map[string]string) *NodeBuilder {
1627+
n.annotations = annotations
1628+
return n
1629+
}
1630+
1631+
// WithTaints adds the given taints to the NodeBuilder.
1632+
func (n *NodeBuilder) WithTaints(taints ...corev1.Taint) *NodeBuilder {
1633+
n.taints = taints
1634+
return n
1635+
}
1636+
16231637
// WithStatus adds Status to the NodeBuilder.
16241638
func (n *NodeBuilder) WithStatus(status corev1.NodeStatus) *NodeBuilder {
16251639
n.status = status
@@ -1630,7 +1644,11 @@ func (n *NodeBuilder) WithStatus(status corev1.NodeStatus) *NodeBuilder {
16301644
func (n *NodeBuilder) Build() *corev1.Node {
16311645
obj := &corev1.Node{
16321646
ObjectMeta: metav1.ObjectMeta{
1633-
Name: n.name,
1647+
Name: n.name,
1648+
Annotations: n.annotations,
1649+
},
1650+
Spec: corev1.NodeSpec{
1651+
Taints: n.taints,
16341652
},
16351653
Status: n.status,
16361654
}

‎util/test/builder/zz_generated.deepcopy.go‎

Lines changed: 15 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)