Skip to content

Commit 7891fad

Browse files
fabriziopandinifabriziopandini
committed
More comments
1 parent 24bc10b commit 7891fad

File tree

2 files changed

+17
-14
lines changed

2 files changed

+17
-14
lines changed

‎internal/controllers/machinedeployment/mdutil/util.go

+8-6
Original file line numberDiff line numberDiff line change
@@ -386,10 +386,11 @@ func MachineTemplateUpToDate(current, desired *clusterv1.MachineTemplateSpec) (u
386386
// struct to catch cases when either configRef or dataSecretName is set in current vs desired (usually MachineTemplates
387387
// have ConfigRef != nil, might be in some edge case dataSecret are used, but switching from one to another is not a
388388
// common operation so it is acceptable to handle it in this way).
389-
if current.Spec.Bootstrap.ConfigRef != nil {
389+
if currentCopy.Spec.Bootstrap.ConfigRef != nil {
390390
if !reflect.DeepEqual(currentCopy.Spec.Bootstrap, desiredCopy.Spec.Bootstrap) {
391-
logMessages = append(logMessages, fmt.Sprintf("spec.bootstrap.configRef %s/%s, %s/%s required", currentCopy.Spec.Bootstrap.ConfigRef.Kind, currentCopy.Spec.Bootstrap.ConfigRef.Name, ptr.Deref(desiredCopy.Spec.Bootstrap.ConfigRef, corev1.ObjectReference{}).Kind, ptr.Deref(desiredCopy.Spec.Bootstrap.ConfigRef, corev1.ObjectReference{}).Name))
392-
conditionMessages = append(conditionMessages, fmt.Sprintf("%s is not up-to-date", current.Spec.Bootstrap.ConfigRef.Kind))
391+
logMessages = append(logMessages, fmt.Sprintf("spec.bootstrap.configRef %s %s, %s %s required", currentCopy.Spec.Bootstrap.ConfigRef.Kind, currentCopy.Spec.Bootstrap.ConfigRef.Name, ptr.Deref(desiredCopy.Spec.Bootstrap.ConfigRef, corev1.ObjectReference{}).Kind, ptr.Deref(desiredCopy.Spec.Bootstrap.ConfigRef, corev1.ObjectReference{}).Name))
392+
// Note: dropping "Template" suffix because conditions message will surface on machine.
393+
conditionMessages = append(conditionMessages, fmt.Sprintf("%s is not up-to-date", strings.TrimSuffix(currentCopy.Spec.Bootstrap.ConfigRef.Kind, clusterv1.TemplateSuffix)))
393394
}
394395
} else {
395396
if !reflect.DeepEqual(currentCopy.Spec.Bootstrap, desiredCopy.Spec.Bootstrap) {
@@ -399,16 +400,17 @@ func MachineTemplateUpToDate(current, desired *clusterv1.MachineTemplateSpec) (u
399400
}
400401

401402
if !reflect.DeepEqual(currentCopy.Spec.InfrastructureRef, desiredCopy.Spec.InfrastructureRef) {
402-
logMessages = append(logMessages, fmt.Sprintf("spec.infrastructureRef %s/%s, %s/%s required", currentCopy.Spec.InfrastructureRef.Kind, currentCopy.Spec.InfrastructureRef.Name, desiredCopy.Spec.InfrastructureRef.Kind, desiredCopy.Spec.InfrastructureRef.Name))
403-
conditionMessages = append(conditionMessages, fmt.Sprintf("%s is not up-to-date", currentCopy.Spec.InfrastructureRef.Kind))
403+
logMessages = append(logMessages, fmt.Sprintf("spec.infrastructureRef %s %s, %s %s required", currentCopy.Spec.InfrastructureRef.Kind, currentCopy.Spec.InfrastructureRef.Name, desiredCopy.Spec.InfrastructureRef.Kind, desiredCopy.Spec.InfrastructureRef.Name))
404+
// Note: dropping "Template" suffix because conditions message will surface on machine.
405+
conditionMessages = append(conditionMessages, fmt.Sprintf("%s is not up-to-date", strings.TrimSuffix(currentCopy.Spec.InfrastructureRef.Kind, clusterv1.TemplateSuffix)))
404406
}
405407

406408
if !reflect.DeepEqual(currentCopy.Spec.FailureDomain, desiredCopy.Spec.FailureDomain) {
407409
logMessages = append(logMessages, fmt.Sprintf("spec.failureDomain %s, %s required", ptr.Deref(currentCopy.Spec.FailureDomain, "nil"), ptr.Deref(desiredCopy.Spec.FailureDomain, "nil")))
408410
conditionMessages = append(conditionMessages, fmt.Sprintf("Failure domain %s, %s required", ptr.Deref(currentCopy.Spec.FailureDomain, "nil"), ptr.Deref(desiredCopy.Spec.FailureDomain, "nil")))
409411
}
410412

411-
if len(conditionMessages) > 0 {
413+
if len(logMessages) > 0 || len(conditionMessages) > 0 {
412414
return false, logMessages, conditionMessages
413415
}
414416

‎internal/controllers/machinedeployment/mdutil/util_test.go

+9-8
Original file line numberDiff line numberDiff line change
@@ -187,14 +187,14 @@ func TestMachineTemplateUpToDate(t *testing.T) {
187187
InfrastructureRef: corev1.ObjectReference{
188188
Name: "infra1",
189189
Namespace: "default",
190-
Kind: "InfrastructureMachine",
190+
Kind: "InfrastructureMachineTemplate",
191191
APIVersion: "infrastructure.cluster.x-k8s.io/v1beta1",
192192
},
193193
Bootstrap: clusterv1.Bootstrap{
194194
ConfigRef: &corev1.ObjectReference{
195195
Name: "bootstrap1",
196196
Namespace: "default",
197-
Kind: "BootstrapConfig",
197+
Kind: "BootstrapConfigTemplate",
198198
APIVersion: "bootstrap.cluster.x-k8s.io/v1beta1",
199199
},
200200
},
@@ -320,8 +320,8 @@ func TestMachineTemplateUpToDate(t *testing.T) {
320320
current: machineTemplate,
321321
desired: machineTemplateWithDifferentInfraRef,
322322
expectedUpToDate: false,
323-
expectedLogMessages1: []string{"spec.infrastructureRef InfrastructureMachine/infra1, InfrastructureMachine/infra2 required"},
324-
expectedLogMessages2: []string{"spec.infrastructureRef InfrastructureMachine/infra2, InfrastructureMachine/infra1 required"},
323+
expectedLogMessages1: []string{"spec.infrastructureRef InfrastructureMachineTemplate infra1, InfrastructureMachineTemplate infra2 required"},
324+
expectedLogMessages2: []string{"spec.infrastructureRef InfrastructureMachineTemplate infra2, InfrastructureMachineTemplate infra1 required"},
325325
expectedConditionMessages1: []string{"InfrastructureMachine is not up-to-date"},
326326
expectedConditionMessages2: []string{"InfrastructureMachine is not up-to-date"},
327327
},
@@ -340,8 +340,8 @@ func TestMachineTemplateUpToDate(t *testing.T) {
340340
current: machineTemplate,
341341
desired: machineTemplateWithDifferentBootstrapConfigRef,
342342
expectedUpToDate: false,
343-
expectedLogMessages1: []string{"spec.bootstrap.configRef BootstrapConfig/bootstrap1, BootstrapConfig/bootstrap2 required"},
344-
expectedLogMessages2: []string{"spec.bootstrap.configRef BootstrapConfig/bootstrap2, BootstrapConfig/bootstrap1 required"},
343+
expectedLogMessages1: []string{"spec.bootstrap.configRef BootstrapConfigTemplate bootstrap1, BootstrapConfigTemplate bootstrap2 required"},
344+
expectedLogMessages2: []string{"spec.bootstrap.configRef BootstrapConfigTemplate bootstrap2, BootstrapConfigTemplate bootstrap1 required"},
345345
expectedConditionMessages1: []string{"BootstrapConfig is not up-to-date"},
346346
expectedConditionMessages2: []string{"BootstrapConfig is not up-to-date"},
347347
},
@@ -350,7 +350,7 @@ func TestMachineTemplateUpToDate(t *testing.T) {
350350
current: machineTemplate,
351351
desired: machineTemplateWithBootstrapDataSecret,
352352
expectedUpToDate: false,
353-
expectedLogMessages1: []string{"spec.bootstrap.configRef BootstrapConfig/bootstrap1, / required"},
353+
expectedLogMessages1: []string{"spec.bootstrap.configRef BootstrapConfigTemplate bootstrap1, required"},
354354
expectedLogMessages2: []string{"spec.bootstrap.dataSecretName data-secret1, nil required"},
355355
expectedConditionMessages1: []string{"BootstrapConfig is not up-to-date"},
356356
expectedConditionMessages2: []string{"spec.bootstrap.dataSecretName data-secret1, nil required"},
@@ -398,6 +398,7 @@ func TestFindNewMachineSet(t *testing.T) {
398398
twoAfterRolloutAfter := metav1.NewTime(oneAfterRolloutAfter.Add(time.Minute))
399399

400400
deployment := generateDeployment("nginx")
401+
deployment.Spec.Template.Spec.InfrastructureRef.Kind = "InfrastructureMachineTemplate"
401402
deployment.Spec.Template.Spec.InfrastructureRef.Name = "new-infra-ref"
402403

403404
deploymentWithRolloutAfter := deployment.DeepCopy()
@@ -458,7 +459,7 @@ func TestFindNewMachineSet(t *testing.T) {
458459
deployment: deployment,
459460
msList: []*clusterv1.MachineSet{&oldMS},
460461
expected: nil,
461-
createReason: fmt.Sprintf(`couldn't find MachineSet matching MachineDeployment spec template: MachineSet %s: diff: spec.infrastructureRef /old-infra-ref, /new-infra-ref required`, oldMS.Name),
462+
createReason: fmt.Sprintf(`couldn't find MachineSet matching MachineDeployment spec template: MachineSet %s: diff: spec.infrastructureRef InfrastructureMachineTemplate old-infra-ref, InfrastructureMachineTemplate new-infra-ref required`, oldMS.Name),
462463
},
463464
{
464465
Name: "Get the MachineSet if reconciliationTime < rolloutAfter",

0 commit comments

Comments
 (0)