Skip to content

Commit

Permalink
Fix tests and in place propagation for MachineDeployment and MachineSet
Browse files Browse the repository at this point in the history
  • Loading branch information
fabriziopandini committed Oct 1, 2024
1 parent cd6623a commit 7626629
Show file tree
Hide file tree
Showing 6 changed files with 37 additions and 25 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -324,6 +324,7 @@ func (r *Reconciler) computeDesiredMachineSet(ctx context.Context, deployment *c
} else {
desiredMS.Spec.DeletePolicy = ""
}
desiredMS.Spec.Template.Spec.ReadinessGates = deployment.Spec.Template.Spec.ReadinessGates
desiredMS.Spec.Template.Spec.NodeDrainTimeout = deployment.Spec.Template.Spec.NodeDrainTimeout
desiredMS.Spec.Template.Spec.NodeDeletionTimeout = deployment.Spec.Template.Spec.NodeDeletionTimeout
desiredMS.Spec.Template.Spec.NodeVolumeDetachTimeout = deployment.Spec.Template.Spec.NodeVolumeDetachTimeout
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -582,6 +582,7 @@ func TestComputeDesiredMachineSet(t *testing.T) {
Bootstrap: clusterv1.Bootstrap{
ConfigRef: &bootstrapRef,
},
ReadinessGates: []clusterv1.MachineReadinessGate{{ConditionType: "foo"}},
NodeDrainTimeout: duration10s,
NodeVolumeDetachTimeout: duration10s,
NodeDeletionTimeout: duration10s,
Expand Down Expand Up @@ -649,6 +650,7 @@ func TestComputeDesiredMachineSet(t *testing.T) {
"ms-label-2": "ms-value-2",
}
existingMS.Spec.Template.Annotations = nil
existingMS.Spec.Template.Spec.ReadinessGates = []clusterv1.MachineReadinessGate{{ConditionType: "bar"}}
existingMS.Spec.Template.Spec.NodeDrainTimeout = duration5s
existingMS.Spec.Template.Spec.NodeDeletionTimeout = duration5s
existingMS.Spec.Template.Spec.NodeVolumeDetachTimeout = duration5s
Expand Down Expand Up @@ -688,6 +690,7 @@ func TestComputeDesiredMachineSet(t *testing.T) {
"ms-label-2": "ms-value-2",
}
existingMS.Spec.Template.Annotations = nil
existingMS.Spec.Template.Spec.ReadinessGates = []clusterv1.MachineReadinessGate{{ConditionType: "bar"}}
existingMS.Spec.Template.Spec.NodeDrainTimeout = duration5s
existingMS.Spec.Template.Spec.NodeDeletionTimeout = duration5s
existingMS.Spec.Template.Spec.NodeVolumeDetachTimeout = duration5s
Expand Down Expand Up @@ -741,6 +744,7 @@ func TestComputeDesiredMachineSet(t *testing.T) {
"ms-label-2": "ms-value-2",
}
existingMS.Spec.Template.Annotations = nil
existingMS.Spec.Template.Spec.ReadinessGates = []clusterv1.MachineReadinessGate{{ConditionType: "bar"}}
existingMS.Spec.Template.Spec.NodeDrainTimeout = duration5s
existingMS.Spec.Template.Spec.NodeDeletionTimeout = duration5s
existingMS.Spec.Template.Spec.NodeVolumeDetachTimeout = duration5s
Expand Down
1 change: 1 addition & 0 deletions internal/controllers/machinedeployment/mdutil/util.go
Original file line number Diff line number Diff line change
Expand Up @@ -390,6 +390,7 @@ func MachineTemplateDeepCopyRolloutFields(template *clusterv1.MachineTemplateSpe
templateCopy.Annotations = nil

// Drop node timeout values
templateCopy.Spec.ReadinessGates = nil
templateCopy.Spec.NodeDrainTimeout = nil
templateCopy.Spec.NodeDeletionTimeout = nil
templateCopy.Spec.NodeVolumeDetachTimeout = nil
Expand Down
51 changes: 26 additions & 25 deletions internal/controllers/machinedeployment/mdutil/util_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -216,6 +216,7 @@ func TestEqualMachineTemplate(t *testing.T) {
machineTemplateWithDifferentAnnotations.Annotations = map[string]string{"a2": "v2"}

machineTemplateWithDifferentInPlaceMutableSpecFields := machineTemplate.DeepCopy()
machineTemplateWithDifferentInPlaceMutableSpecFields.Spec.ReadinessGates = []clusterv1.MachineReadinessGate{{ConditionType: "foo"}}
machineTemplateWithDifferentInPlaceMutableSpecFields.Spec.NodeDrainTimeout = &metav1.Duration{Duration: 20 * time.Second}
machineTemplateWithDifferentInPlaceMutableSpecFields.Spec.NodeDeletionTimeout = &metav1.Duration{Duration: 20 * time.Second}
machineTemplateWithDifferentInPlaceMutableSpecFields.Spec.NodeVolumeDetachTimeout = &metav1.Duration{Duration: 20 * time.Second}
Expand Down Expand Up @@ -302,7 +303,7 @@ func TestEqualMachineTemplate(t *testing.T) {
+ ClusterName: "cluster2",
Bootstrap: {ConfigRef: &{Kind: "BootstrapConfig", Namespace: "default", Name: "bootstrap1", APIVersion: "bootstrap.cluster.x-k8s.io", ...}},
InfrastructureRef: {Kind: "InfrastructureMachine", Namespace: "default", Name: "infra1", APIVersion: "infrastructure.cluster.x-k8s.io", ...},
... // 6 identical fields
... // 7 identical fields
},
}`,
Diff2: `&v1beta1.MachineTemplateSpec{
Expand All @@ -312,7 +313,7 @@ func TestEqualMachineTemplate(t *testing.T) {
+ ClusterName: "cluster1",
Bootstrap: {ConfigRef: &{Kind: "BootstrapConfig", Namespace: "default", Name: "bootstrap1", APIVersion: "bootstrap.cluster.x-k8s.io", ...}},
InfrastructureRef: {Kind: "InfrastructureMachine", Namespace: "default", Name: "infra1", APIVersion: "infrastructure.cluster.x-k8s.io", ...},
... // 6 identical fields
... // 7 identical fields
},
}`,
},
Expand All @@ -331,7 +332,7 @@ func TestEqualMachineTemplate(t *testing.T) {
+ Version: &"v1.26.0",
ProviderID: nil,
FailureDomain: &"failure-domain1",
... // 3 identical fields
... // 4 identical fields
},
}`,
Diff2: `&v1beta1.MachineTemplateSpec{
Expand All @@ -344,7 +345,7 @@ func TestEqualMachineTemplate(t *testing.T) {
+ Version: &"v1.25.0",
ProviderID: nil,
FailureDomain: &"failure-domain1",
... // 3 identical fields
... // 4 identical fields
},
}`,
},
Expand All @@ -357,26 +358,26 @@ func TestEqualMachineTemplate(t *testing.T) {
ObjectMeta: {},
Spec: v1beta1.MachineSpec{
... // 3 identical fields
Version: &"v1.25.0",
ProviderID: nil,
- FailureDomain: &"failure-domain1",
+ FailureDomain: &"failure-domain2",
NodeDrainTimeout: nil,
NodeVolumeDetachTimeout: nil,
NodeDeletionTimeout: nil,
Version: &"v1.25.0",
ProviderID: nil,
- FailureDomain: &"failure-domain1",
+ FailureDomain: &"failure-domain2",
ReadinessGates: nil,
NodeDrainTimeout: nil,
... // 2 identical fields
},
}`,
Diff2: `&v1beta1.MachineTemplateSpec{
ObjectMeta: {},
Spec: v1beta1.MachineSpec{
... // 3 identical fields
Version: &"v1.25.0",
ProviderID: nil,
- FailureDomain: &"failure-domain2",
+ FailureDomain: &"failure-domain1",
NodeDrainTimeout: nil,
NodeVolumeDetachTimeout: nil,
NodeDeletionTimeout: nil,
Version: &"v1.25.0",
ProviderID: nil,
- FailureDomain: &"failure-domain2",
+ FailureDomain: &"failure-domain1",
ReadinessGates: nil,
NodeDrainTimeout: nil,
... // 2 identical fields
},
}`,
},
Expand All @@ -401,7 +402,7 @@ func TestEqualMachineTemplate(t *testing.T) {
},
Version: &"v1.25.0",
ProviderID: nil,
... // 4 identical fields
... // 5 identical fields
},
}`,
Diff2: `&v1beta1.MachineTemplateSpec{
Expand All @@ -420,7 +421,7 @@ func TestEqualMachineTemplate(t *testing.T) {
},
Version: &"v1.25.0",
ProviderID: nil,
... // 4 identical fields
... // 5 identical fields
},
}`,
},
Expand All @@ -441,7 +442,7 @@ func TestEqualMachineTemplate(t *testing.T) {
},
InfrastructureRef: {Kind: "InfrastructureMachine", Namespace: "default", Name: "infra1", APIVersion: "infrastructure.cluster.x-k8s.io", ...},
Version: &"v1.25.0",
... // 5 identical fields
... // 6 identical fields
},
}`,
Diff2: `&v1beta1.MachineTemplateSpec{
Expand All @@ -456,7 +457,7 @@ func TestEqualMachineTemplate(t *testing.T) {
},
InfrastructureRef: {Kind: "InfrastructureMachine", Namespace: "default", Name: "infra1", APIVersion: "infrastructure.cluster.x-k8s.io", ...},
Version: &"v1.25.0",
... // 5 identical fields
... // 6 identical fields
},
}`,
},
Expand All @@ -483,7 +484,7 @@ func TestEqualMachineTemplate(t *testing.T) {
},
InfrastructureRef: {Kind: "InfrastructureMachine", Namespace: "default", Name: "infra1", APIVersion: "infrastructure.cluster.x-k8s.io", ...},
Version: &"v1.25.0",
... // 5 identical fields
... // 6 identical fields
},
}`,
Diff2: `&v1beta1.MachineTemplateSpec{
Expand All @@ -504,7 +505,7 @@ func TestEqualMachineTemplate(t *testing.T) {
},
InfrastructureRef: {Kind: "InfrastructureMachine", Namespace: "default", Name: "infra1", APIVersion: "infrastructure.cluster.x-k8s.io", ...},
Version: &"v1.25.0",
... // 5 identical fields
... // 6 identical fields
},
}`,
},
Expand Down Expand Up @@ -627,7 +628,7 @@ func TestFindNewMachineSet(t *testing.T) {
},
Version: nil,
ProviderID: nil,
... // 4 identical fields
... // 5 identical fields
},
}`, oldMS.Name),
},
Expand Down
1 change: 1 addition & 0 deletions internal/controllers/machineset/machineset_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -424,6 +424,7 @@ func (r *Reconciler) syncMachines(ctx context.Context, machineSet *clusterv1.Mac
}

// Set all other in-place mutable fields that impact the ability to tear down existing machines.
m.Spec.ReadinessGates = machineSet.Spec.Template.Spec.ReadinessGates
m.Spec.NodeDrainTimeout = machineSet.Spec.Template.Spec.NodeDrainTimeout
m.Spec.NodeDeletionTimeout = machineSet.Spec.Template.Spec.NodeDeletionTimeout
m.Spec.NodeVolumeDetachTimeout = machineSet.Spec.Template.Spec.NodeVolumeDetachTimeout
Expand Down
4 changes: 4 additions & 0 deletions internal/controllers/machineset/machineset_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1341,6 +1341,8 @@ func TestMachineSetReconciler_syncMachines(t *testing.T) {
"modified-annotation": "modified-value-2", // Modify the value of the annotation
// Drop "dropped-annotation"
}
readinessGates := []clusterv1.MachineReadinessGate{{ConditionType: "foo"}}
ms.Spec.Template.Spec.ReadinessGates = readinessGates
ms.Spec.Template.Spec.NodeDrainTimeout = duration10s
ms.Spec.Template.Spec.NodeDeletionTimeout = duration10s
ms.Spec.Template.Spec.NodeVolumeDetachTimeout = duration10s
Expand All @@ -1367,6 +1369,8 @@ func TestMachineSetReconciler_syncMachines(t *testing.T) {
Not(BeNil()),
HaveValue(Equal(*ms.Spec.Template.Spec.NodeVolumeDetachTimeout)),
))
// Verify readiness gates.
g.Expect(updatedInPlaceMutatingMachine.Spec.ReadinessGates).Should(Equal(readinessGates))
}, timeout).Should(Succeed())

// Verify in-place mutable fields are updated on InfrastructureMachine
Expand Down

0 comments on commit 7626629

Please sign in to comment.