Skip to content

Commit 4d422a8

Browse files
fabriziopandinifabriziopandini
committed
More comments
1 parent b2bd2fa commit 4d422a8

File tree

2 files changed

+49
-39
lines changed

2 files changed

+49
-39
lines changed

‎internal/controllers/machinedeployment/machinedeployment_status.go

+7-3
Original file line numberDiff line numberDiff line change
@@ -135,7 +135,7 @@ func setAvailableCondition(_ context.Context, machineDeployment *clusterv1.Machi
135135

136136
message := fmt.Sprintf("%d available replicas, at least %d required", *machineDeployment.Status.V1Beta2.AvailableReplicas, minReplicasNeeded)
137137
if machineDeployment.Spec.Strategy != nil && mdutil.IsRollingUpdate(machineDeployment) && machineDeployment.Spec.Strategy.RollingUpdate != nil {
138-
message += fmt.Sprintf(" (spec.strategy.rollout.maxUnavailable is %s)", machineDeployment.Spec.Strategy.RollingUpdate.MaxUnavailable)
138+
message += fmt.Sprintf(" (spec.strategy.rollout.maxUnavailable is %s, spec.replicas is %d)", machineDeployment.Spec.Strategy.RollingUpdate.MaxUnavailable, *machineDeployment.Spec.Replicas)
139139
}
140140
v1beta2conditions.Set(machineDeployment, metav1.Condition{
141141
Type: clusterv1.MachineDeploymentAvailableV1Beta2Condition,
@@ -406,7 +406,11 @@ func setDeletingCondition(_ context.Context, machineDeployment *clusterv1.Machin
406406

407407
message := ""
408408
if len(machines) > 0 {
409-
message = fmt.Sprintf("Deleting %d Machines", len(machines))
409+
if len(machines) == 1 {
410+
message = fmt.Sprintf("Deleting %d Machine", len(machines))
411+
} else {
412+
message = fmt.Sprintf("Deleting %d Machines", len(machines))
413+
}
410414
staleMessage := aggregateStaleMachines(machines)
411415
if staleMessage != "" {
412416
message += fmt.Sprintf(" and %s", staleMessage)
@@ -505,7 +509,7 @@ func aggregateUnhealthyMachines(machines collections.Machines) string {
505509
} else {
506510
message += " are "
507511
}
508-
message += "not healthy (not to be remediated by KCP)"
512+
message += "not healthy (not to be remediated by MachineDeployment/MachineSet)"
509513

510514
return message
511515
}

‎internal/controllers/machinedeployment/machinedeployment_status_test.go

+42-36
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,6 @@ import (
2222

2323
. "github.com/onsi/gomega"
2424
corev1 "k8s.io/api/core/v1"
25-
"k8s.io/apimachinery/pkg/api/meta"
2625
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
2726
"k8s.io/apimachinery/pkg/util/intstr"
2827
"k8s.io/utils/ptr"
@@ -124,7 +123,7 @@ func Test_setAvailableCondition(t *testing.T) {
124123
},
125124
},
126125
{
127-
name: "all the expected replicase are available",
126+
name: "all the expected replicas are available",
128127
machineDeployment: &clusterv1.MachineDeployment{
129128
Spec: clusterv1.MachineDeploymentSpec{
130129
Replicas: ptr.To(int32(5)),
@@ -146,7 +145,7 @@ func Test_setAvailableCondition(t *testing.T) {
146145
},
147146
},
148147
{
149-
name: "some replicase are not available, but within MaxUnavailable range",
148+
name: "some replicas are not available, but within MaxUnavailable range",
150149
machineDeployment: &clusterv1.MachineDeployment{
151150
Spec: clusterv1.MachineDeploymentSpec{
152151
Replicas: ptr.To(int32(5)),
@@ -168,30 +167,7 @@ func Test_setAvailableCondition(t *testing.T) {
168167
},
169168
},
170169
{
171-
name: "some replicase are not available, less than MaxUnavailable",
172-
machineDeployment: &clusterv1.MachineDeployment{
173-
Spec: clusterv1.MachineDeploymentSpec{
174-
Replicas: ptr.To(int32(5)),
175-
Strategy: &clusterv1.MachineDeploymentStrategy{
176-
Type: clusterv1.RollingUpdateMachineDeploymentStrategyType,
177-
RollingUpdate: &clusterv1.MachineRollingUpdateDeployment{
178-
MaxSurge: ptr.To(intstr.FromInt32(1)),
179-
MaxUnavailable: ptr.To(intstr.FromInt32(1)),
180-
},
181-
},
182-
},
183-
Status: clusterv1.MachineDeploymentStatus{V1Beta2: &clusterv1.MachineDeploymentV1Beta2Status{AvailableReplicas: ptr.To(int32(4))}},
184-
},
185-
getAndAdoptMachineSetsForDeploymentSucceeded: true,
186-
expectCondition: metav1.Condition{
187-
Type: clusterv1.MachineDeploymentAvailableV1Beta2Condition,
188-
Status: metav1.ConditionTrue,
189-
Reason: clusterv1.MachineDeploymentAvailableV1Beta2Reason,
190-
Message: "",
191-
},
192-
},
193-
{
194-
name: "some replicase are not available, more than MaxUnavailable",
170+
name: "some replicas are not available, more than MaxUnavailable",
195171
machineDeployment: &clusterv1.MachineDeployment{
196172
Spec: clusterv1.MachineDeploymentSpec{
197173
Replicas: ptr.To(int32(5)),
@@ -210,7 +186,7 @@ func Test_setAvailableCondition(t *testing.T) {
210186
Type: clusterv1.MachineDeploymentAvailableV1Beta2Condition,
211187
Status: metav1.ConditionFalse,
212188
Reason: clusterv1.MachineDeploymentNotAvailableV1Beta2Reason,
213-
Message: "3 available replicas, at least 4 required (spec.strategy.rollout.maxUnavailable is 1)",
189+
Message: "3 available replicas, at least 4 required (spec.strategy.rollout.maxUnavailable is 1, spec.replicas is 5)",
214190
},
215191
},
216192
}
@@ -279,6 +255,22 @@ func Test_setScalingUpCondition(t *testing.T) {
279255
Message: "Please check controller logs for errors",
280256
},
281257
},
258+
{
259+
name: "replicas not set",
260+
machineDeployment: func() *clusterv1.MachineDeployment {
261+
md := defaultMachineDeployment.DeepCopy()
262+
md.Spec.Replicas = nil
263+
return md
264+
}(),
265+
bootstrapTemplateNotFound: false,
266+
infrastructureTemplateNotFound: false,
267+
getAndAdoptMachineSetsForDeploymentSucceeded: true,
268+
expectCondition: metav1.Condition{
269+
Type: clusterv1.MachineDeploymentScalingUpV1Beta2Condition,
270+
Status: metav1.ConditionUnknown,
271+
Reason: clusterv1.MachineDeploymentScalingUpWaitingForReplicasSetV1Beta2Reason,
272+
},
273+
},
282274
{
283275
name: "not scaling up and no machines",
284276
machineDeployment: defaultMachineDeployment,
@@ -463,6 +455,20 @@ func Test_setScalingDownCondition(t *testing.T) {
463455
Message: "Please check controller logs for errors",
464456
},
465457
},
458+
{
459+
name: "replicas not set",
460+
machineDeployment: func() *clusterv1.MachineDeployment {
461+
md := defaultMachineDeployment.DeepCopy()
462+
md.Spec.Replicas = nil
463+
return md
464+
}(),
465+
getAndAdoptMachineSetsForDeploymentSucceeded: true,
466+
expectCondition: metav1.Condition{
467+
Type: clusterv1.MachineDeploymentScalingDownV1Beta2Condition,
468+
Status: metav1.ConditionUnknown,
469+
Reason: clusterv1.MachineDeploymentScalingDownWaitingForReplicasSetV1Beta2Reason,
470+
},
471+
},
466472
{
467473
name: "not scaling down and no machines",
468474
machineDeployment: defaultMachineDeployment,
@@ -914,7 +920,7 @@ func Test_setRemediatingCondition(t *testing.T) {
914920
},
915921
},
916922
{
917-
name: "With machines to be remediated by KCP",
923+
name: "With machines to be remediated by MD/MS",
918924
machineDeployment: &clusterv1.MachineDeployment{},
919925
machines: []*clusterv1.Machine{
920926
fakeMachine("m1", withConditions(healthCheckSucceeded)), // Healthy machine
@@ -930,7 +936,7 @@ func Test_setRemediatingCondition(t *testing.T) {
930936
},
931937
},
932938
{
933-
name: "With one unhealthy machine not to be remediated by KCP",
939+
name: "With one unhealthy machine not to be remediated by MD/MS",
934940
machineDeployment: &clusterv1.MachineDeployment{},
935941
machines: []*clusterv1.Machine{
936942
fakeMachine("m1", withConditions(healthCheckSucceeded)), // Healthy machine
@@ -942,11 +948,11 @@ func Test_setRemediatingCondition(t *testing.T) {
942948
Type: clusterv1.MachineDeploymentRemediatingV1Beta2Condition,
943949
Status: metav1.ConditionFalse,
944950
Reason: clusterv1.MachineDeploymentNotRemediatingV1Beta2Reason,
945-
Message: "Machine m2 is not healthy (not to be remediated by KCP)",
951+
Message: "Machine m2 is not healthy (not to be remediated by MachineDeployment/MachineSet)",
946952
},
947953
},
948954
{
949-
name: "With two unhealthy machine not to be remediated by KCP",
955+
name: "With two unhealthy machine not to be remediated by MD/MS",
950956
machineDeployment: &clusterv1.MachineDeployment{},
951957
machines: []*clusterv1.Machine{
952958
fakeMachine("m1", withConditions(healthCheckNotSucceeded)), // Unhealthy machine, not yet marked for remediation
@@ -958,7 +964,7 @@ func Test_setRemediatingCondition(t *testing.T) {
958964
Type: clusterv1.MachineDeploymentRemediatingV1Beta2Condition,
959965
Status: metav1.ConditionFalse,
960966
Reason: clusterv1.MachineDeploymentNotRemediatingV1Beta2Reason,
961-
Message: "Machines m1, m2 are not healthy (not to be remediated by KCP)",
967+
Message: "Machines m1, m2 are not healthy (not to be remediated by MachineDeployment/MachineSet)",
962968
},
963969
},
964970
}
@@ -1047,7 +1053,7 @@ func Test_setDeletingCondition(t *testing.T) {
10471053
Type: clusterv1.MachineDeploymentDeletingV1Beta2Condition,
10481054
Status: metav1.ConditionTrue,
10491055
Reason: clusterv1.MachineDeploymentDeletingDeletionTimestampSetV1Beta2Reason,
1050-
Message: "Deleting 1 Machines",
1056+
Message: "Deleting 1 Machine",
10511057
},
10521058
},
10531059
{
@@ -1065,7 +1071,7 @@ func Test_setDeletingCondition(t *testing.T) {
10651071
Type: clusterv1.MachineDeploymentDeletingV1Beta2Condition,
10661072
Status: metav1.ConditionTrue,
10671073
Reason: clusterv1.MachineDeploymentDeletingDeletionTimestampSetV1Beta2Reason,
1068-
Message: "Deleting 1 Machines and Machine m1 is in deletion since more than 30m",
1074+
Message: "Deleting 1 Machine and Machine m1 is in deletion since more than 30m",
10691075
},
10701076
},
10711077
{
@@ -1174,7 +1180,7 @@ func withV1Beta2Condition(c metav1.Condition) fakeMachinesOption {
11741180
if m.Status.V1Beta2 == nil {
11751181
m.Status.V1Beta2 = &clusterv1.MachineV1Beta2Status{}
11761182
}
1177-
meta.SetStatusCondition(&m.Status.V1Beta2.Conditions, c)
1183+
v1beta2conditions.Set(m, c)
11781184
}
11791185
}
11801186

0 commit comments

Comments
 (0)