Skip to content

Commit af7e8c7

Browse files
sivcharisivchari
committed
sort failure domains by name when converting
Signed-off-by: sivchari <[email protected]>
1 parent c137dfe commit af7e8c7

File tree

5 files changed

+94
-67
lines changed

5 files changed

+94
-67
lines changed

‎test/infrastructure/docker/api/v1alpha3/conversion.go

Lines changed: 22 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,10 @@ limitations under the License.
1717
package v1alpha3
1818

1919
import (
20+
"maps"
21+
"slices"
22+
"sort"
23+
2024
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
2125
apiconversion "k8s.io/apimachinery/pkg/conversion"
2226
"k8s.io/utils/ptr"
@@ -249,12 +253,10 @@ func Convert_v1alpha3_DockerMachineStatus_To_v1beta2_DockerMachineStatus(in *Doc
249253
clusterv1alpha3.Convert_v1alpha3_Conditions_To_v1beta2_Deprecated_V1Beta1_Conditions(&in.Conditions, &out.Deprecated.V1Beta1.Conditions)
250254
}
251255

252-
if in.Ready {
253-
if out.Initialization == nil {
254-
out.Initialization = &infrav1.DockerMachineInitializationStatus{}
255-
}
256-
out.Initialization.Provisioned = in.Ready
256+
if out.Initialization == nil {
257+
out.Initialization = &infrav1.DockerMachineInitializationStatus{}
257258
}
259+
out.Initialization.Provisioned = ptr.To(in.Ready)
258260

259261
return nil
260262
}
@@ -274,20 +276,22 @@ func Convert_v1alpha3_DockerClusterStatus_To_v1beta2_DockerClusterStatus(in *Doc
274276
clusterv1alpha3.Convert_v1alpha3_Conditions_To_v1beta2_Deprecated_V1Beta1_Conditions(&in.Conditions, &out.Deprecated.V1Beta1.Conditions)
275277
}
276278

277-
if in.Ready {
278-
out.Initialization = &infrav1.DockerClusterInitializationStatus{
279-
Provisioned: ptr.To(in.Ready),
280-
}
279+
if out.Initialization == nil {
280+
out.Initialization = &infrav1.DockerClusterInitializationStatus{}
281281
}
282+
out.Initialization.Provisioned = ptr.To(in.Ready)
282283

283284
// Move FailureDomains
284285
if in.FailureDomains != nil {
285286
out.FailureDomains = []clusterv1.FailureDomain{}
286-
for name, fd := range in.FailureDomains {
287+
domainNames := slices.Collect(maps.Keys(in.FailureDomains))
288+
sort.Strings(domainNames)
289+
for _, name := range domainNames {
290+
domain := in.FailureDomains[name]
287291
out.FailureDomains = append(out.FailureDomains, clusterv1.FailureDomain{
288292
Name: name,
289-
ControlPlane: fd.ControlPlane,
290-
Attributes: fd.Attributes,
293+
ControlPlane: domain.ControlPlane,
294+
Attributes: domain.Attributes,
291295
})
292296
}
293297
}
@@ -303,11 +307,14 @@ func Convert_v1alpha3_DockerClusterSpec_To_v1beta2_DockerClusterSpec(in *DockerC
303307
// Move FailureDomains
304308
if in.FailureDomains != nil {
305309
out.FailureDomains = []clusterv1.FailureDomain{}
306-
for name, fd := range in.FailureDomains {
310+
domainNames := slices.Collect(maps.Keys(in.FailureDomains))
311+
sort.Strings(domainNames)
312+
for _, name := range domainNames {
313+
domain := in.FailureDomains[name]
307314
out.FailureDomains = append(out.FailureDomains, clusterv1.FailureDomain{
308315
Name: name,
309-
ControlPlane: fd.ControlPlane,
310-
Attributes: fd.Attributes,
316+
ControlPlane: domain.ControlPlane,
317+
Attributes: domain.Attributes,
311318
})
312319
}
313320
}

‎test/infrastructure/docker/api/v1alpha4/conversion.go

Lines changed: 22 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,10 @@ limitations under the License.
1717
package v1alpha4
1818

1919
import (
20+
"maps"
21+
"slices"
22+
"sort"
23+
2024
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
2125
apiconversion "k8s.io/apimachinery/pkg/conversion"
2226
"k8s.io/utils/ptr"
@@ -278,12 +282,10 @@ func Convert_v1alpha4_DockerMachineStatus_To_v1beta2_DockerMachineStatus(in *Doc
278282
clusterv1alpha4.Convert_v1alpha4_Conditions_To_v1beta2_Deprecated_V1Beta1_Conditions(&in.Conditions, &out.Deprecated.V1Beta1.Conditions)
279283
}
280284

281-
if in.Ready {
282-
if out.Initialization == nil {
283-
out.Initialization = &infrav1.DockerMachineInitializationStatus{}
284-
}
285-
out.Initialization.Provisioned = ptr.To(in.Ready)
285+
if out.Initialization == nil {
286+
out.Initialization = &infrav1.DockerMachineInitializationStatus{}
286287
}
288+
out.Initialization.Provisioned = ptr.To(in.Ready)
287289

288290
return nil
289291
}
@@ -303,20 +305,22 @@ func Convert_v1alpha4_DockerClusterStatus_To_v1beta2_DockerClusterStatus(in *Doc
303305
clusterv1alpha4.Convert_v1alpha4_Conditions_To_v1beta2_Deprecated_V1Beta1_Conditions(&in.Conditions, &out.Deprecated.V1Beta1.Conditions)
304306
}
305307

306-
if in.Ready {
307-
out.Initialization = &infrav1.DockerClusterInitializationStatus{
308-
Provisioned: ptr.To(in.Ready),
309-
}
308+
if out.Initialization == nil {
309+
out.Initialization = &infrav1.DockerClusterInitializationStatus{}
310310
}
311+
out.Initialization.Provisioned = ptr.To(in.Ready)
311312

312313
// Move FailureDomains
313314
if in.FailureDomains != nil {
314315
out.FailureDomains = []clusterv1.FailureDomain{}
315-
for name, fd := range in.FailureDomains {
316+
domainNames := slices.Collect(maps.Keys(in.FailureDomains))
317+
sort.Strings(domainNames)
318+
for _, name := range domainNames {
319+
domain := in.FailureDomains[name]
316320
out.FailureDomains = append(out.FailureDomains, clusterv1.FailureDomain{
317321
Name: name,
318-
ControlPlane: fd.ControlPlane,
319-
Attributes: fd.Attributes,
322+
ControlPlane: domain.ControlPlane,
323+
Attributes: domain.Attributes,
320324
})
321325
}
322326
}
@@ -332,11 +336,14 @@ func Convert_v1alpha4_DockerClusterSpec_To_v1beta2_DockerClusterSpec(in *DockerC
332336
// Move FailureDomains
333337
if in.FailureDomains != nil {
334338
out.FailureDomains = []clusterv1.FailureDomain{}
335-
for name, fd := range in.FailureDomains {
339+
domainNames := slices.Collect(maps.Keys(in.FailureDomains))
340+
sort.Strings(domainNames)
341+
for _, name := range domainNames {
342+
domain := in.FailureDomains[name]
336343
out.FailureDomains = append(out.FailureDomains, clusterv1.FailureDomain{
337344
Name: name,
338-
ControlPlane: fd.ControlPlane,
339-
Attributes: fd.Attributes,
345+
ControlPlane: domain.ControlPlane,
346+
Attributes: domain.Attributes,
340347
})
341348
}
342349
}

‎test/infrastructure/docker/api/v1beta1/conversion.go

Lines changed: 40 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,10 @@ limitations under the License.
1717
package v1beta1
1818

1919
import (
20+
"maps"
21+
"slices"
22+
"sort"
23+
2024
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
2125
apiconversion "k8s.io/apimachinery/pkg/conversion"
2226
"k8s.io/utils/ptr"
@@ -136,20 +140,21 @@ func Convert_v1beta1_DevClusterStatus_To_v1beta2_DevClusterStatus(in *DevCluster
136140
return err
137141
}
138142

139-
if in.Ready {
140-
if out.Initialization == nil {
141-
out.Initialization = &infrav1.DevClusterInitializationStatus{}
142-
}
143-
out.Initialization.Provisioned = ptr.To(in.Ready)
143+
if out.Initialization == nil {
144+
out.Initialization = &infrav1.DevClusterInitializationStatus{}
144145
}
146+
out.Initialization.Provisioned = ptr.To(in.Ready)
145147

146148
if in.FailureDomains != nil {
147149
out.FailureDomains = []clusterv1.FailureDomain{}
148-
for name, fd := range in.FailureDomains {
150+
domainNames := slices.Collect(maps.Keys(in.FailureDomains))
151+
sort.Strings(domainNames)
152+
for _, name := range domainNames {
153+
domain := in.FailureDomains[name]
149154
out.FailureDomains = append(out.FailureDomains, clusterv1.FailureDomain{
150155
Name: name,
151-
ControlPlane: fd.ControlPlane,
152-
Attributes: fd.Attributes,
156+
ControlPlane: domain.ControlPlane,
157+
Attributes: domain.Attributes,
153158
})
154159
}
155160
}
@@ -222,12 +227,10 @@ func Convert_v1beta1_DevMachineStatus_To_v1beta2_DevMachineStatus(in *DevMachine
222227
return err
223228
}
224229

225-
if in.Ready {
226-
if out.Initialization == nil {
227-
out.Initialization = &infrav1.DevMachineInitializationStatus{}
228-
}
229-
out.Initialization.Provisioned = ptr.To(in.Ready)
230+
if out.Initialization == nil {
231+
out.Initialization = &infrav1.DevMachineInitializationStatus{}
230232
}
233+
out.Initialization.Provisioned = ptr.To(in.Ready)
231234

232235
// Reset conditions from autogenerated conversions
233236
// NOTE: v1beta1 conditions should not be automatically be converted into v1beta2 conditions.
@@ -288,20 +291,21 @@ func Convert_v1beta1_DockerClusterStatus_To_v1beta2_DockerClusterStatus(in *Dock
288291
return err
289292
}
290293

291-
if in.Ready {
292-
if out.Initialization == nil {
293-
out.Initialization = &infrav1.DockerClusterInitializationStatus{}
294-
}
295-
out.Initialization.Provisioned = ptr.To(in.Ready)
294+
if out.Initialization == nil {
295+
out.Initialization = &infrav1.DockerClusterInitializationStatus{}
296296
}
297+
out.Initialization.Provisioned = ptr.To(in.Ready)
297298

298299
if in.FailureDomains != nil {
299300
out.FailureDomains = []clusterv1.FailureDomain{}
300-
for name, fd := range in.FailureDomains {
301+
domainNames := slices.Collect(maps.Keys(in.FailureDomains))
302+
sort.Strings(domainNames)
303+
for _, name := range domainNames {
304+
domain := in.FailureDomains[name]
301305
out.FailureDomains = append(out.FailureDomains, clusterv1.FailureDomain{
302306
Name: name,
303-
ControlPlane: fd.ControlPlane,
304-
Attributes: fd.Attributes,
307+
ControlPlane: domain.ControlPlane,
308+
Attributes: domain.Attributes,
305309
})
306310
}
307311
}
@@ -374,12 +378,10 @@ func Convert_v1beta1_DockerMachineStatus_To_v1beta2_DockerMachineStatus(in *Dock
374378
return err
375379
}
376380

377-
if in.Ready {
378-
if out.Initialization == nil {
379-
out.Initialization = &infrav1.DockerMachineInitializationStatus{}
380-
}
381-
out.Initialization.Provisioned = ptr.To(in.Ready)
381+
if out.Initialization == nil {
382+
out.Initialization = &infrav1.DockerMachineInitializationStatus{}
382383
}
384+
out.Initialization.Provisioned = ptr.To(in.Ready)
383385

384386
// Reset conditions from autogenerated conversions
385387
// NOTE: v1beta1 conditions should not be automatically be converted into v1beta2 conditions.
@@ -453,11 +455,14 @@ func Convert_v1beta1_DockerClusterSpec_To_v1beta2_DockerClusterSpec(in *DockerCl
453455
// Move FailureDomains
454456
if in.FailureDomains != nil {
455457
out.FailureDomains = make([]clusterv1.FailureDomain, 0, len(in.FailureDomains))
456-
for name, fd := range in.FailureDomains {
458+
domainNames := slices.Collect(maps.Keys(in.FailureDomains))
459+
sort.Strings(domainNames)
460+
for _, name := range domainNames {
461+
domain := in.FailureDomains[name]
457462
out.FailureDomains = append(out.FailureDomains, clusterv1.FailureDomain{
458463
Name: name,
459-
ControlPlane: fd.ControlPlane,
460-
Attributes: fd.Attributes,
464+
ControlPlane: domain.ControlPlane,
465+
Attributes: domain.Attributes,
461466
})
462467
}
463468
}
@@ -511,11 +516,14 @@ func Convert_v1beta1_DockerClusterBackendSpec_To_v1beta2_DockerClusterBackendSpe
511516
// Move FailureDomains
512517
if in.FailureDomains != nil {
513518
out.FailureDomains = make([]clusterv1.FailureDomain, 0, len(in.FailureDomains))
514-
for name, fd := range in.FailureDomains {
519+
domainNames := slices.Collect(maps.Keys(in.FailureDomains))
520+
sort.Strings(domainNames)
521+
for _, name := range domainNames {
522+
domain := in.FailureDomains[name]
515523
out.FailureDomains = append(out.FailureDomains, clusterv1.FailureDomain{
516524
Name: name,
517-
ControlPlane: fd.ControlPlane,
518-
Attributes: fd.Attributes,
525+
ControlPlane: domain.ControlPlane,
526+
Attributes: domain.Attributes,
519527
})
520528
}
521529
}

‎test/infrastructure/docker/exp/internal/controllers/dockermachinepool_controller_phases.go

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ import (
2727
"github.com/pkg/errors"
2828
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
2929
"k8s.io/klog/v2"
30+
"k8s.io/utils/ptr"
3031
ctrl "sigs.k8s.io/controller-runtime"
3132
"sigs.k8s.io/kind/pkg/cluster/constants"
3233

@@ -201,7 +202,7 @@ func (r *DockerMachinePoolReconciler) reconcileDockerMachines(ctx context.Contex
201202
dockerMachine := orderedDockerMachines[i]
202203
// TODO (v1beta2): test for v1beta2 conditions
203204
initialization := dockerMachine.Status.Initialization
204-
if initialization != nil && initialization.Provisioned || v1beta1conditions.IsTrue(&dockerMachine, clusterv1.ReadyV1Beta1Condition) {
205+
if initialization != nil && ptr.Deref(initialization.Provisioned, false) || v1beta1conditions.IsTrue(&dockerMachine, clusterv1.ReadyV1Beta1Condition) {
205206
totalReadyMachines++
206207
}
207208
}
@@ -390,7 +391,7 @@ func (r *DockerMachinePoolReconciler) getDeletionCandidates(ctx context.Context,
390391
// TODO (v1beta2): test for v1beta2 conditions
391392
if !isMachineMatchingInfrastructureSpec(ctx, externalMachine, machinePool, dockerMachinePool) {
392393
outdatedMachines = append(outdatedMachines, dockerMachine)
393-
} else if initialization != nil && initialization.Provisioned || v1beta1conditions.IsTrue(&dockerMachine, clusterv1.ReadyV1Beta1Condition) {
394+
} else if initialization != nil && ptr.Deref(initialization.Provisioned, false) || v1beta1conditions.IsTrue(&dockerMachine, clusterv1.ReadyV1Beta1Condition) {
394395
readyMatchingMachines = append(readyMatchingMachines, dockerMachine)
395396
}
396397
}

‎test/infrastructure/docker/internal/webhooks/devcluster_webhook.go

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -74,10 +74,10 @@ func (webhook *DevCluster) ValidateCreate(_ context.Context, obj runtime.Object)
7474
}
7575

7676
// ValidateUpdate implements webhook.Validator so a webhook will be registered for the type.
77-
func (webhook *DevCluster) ValidateUpdate(_ context.Context, _, new runtime.Object) (admission.Warnings, error) {
78-
cluster, ok := new.(*infrav1.DevCluster)
77+
func (webhook *DevCluster) ValidateUpdate(_ context.Context, _, newObj runtime.Object) (admission.Warnings, error) {
78+
cluster, ok := newObj.(*infrav1.DevCluster)
7979
if !ok {
80-
return nil, apierrors.NewBadRequest(fmt.Sprintf("expected a DevCluster but got a %T", new))
80+
return nil, apierrors.NewBadRequest(fmt.Sprintf("expected a DevCluster but got a %T", newObj))
8181
}
8282
if allErrs := validateDevClusterSpec(cluster.Spec); len(allErrs) > 0 {
8383
return nil, apierrors.NewInvalid(infrav1.GroupVersion.WithKind("DevCluster").GroupKind(), cluster.Name, allErrs)
@@ -104,6 +104,10 @@ func defaultDevClusterSpec(s *infrav1.DevClusterSpec) {
104104
}
105105

106106
func validateDevClusterSpec(spec infrav1.DevClusterSpec) field.ErrorList {
107+
// Only validate the Docker backend if it is set.
108+
if spec.Backend.Docker == nil {
109+
return nil
110+
}
107111
domainNames := make([]string, 0, len(spec.Backend.Docker.FailureDomains))
108112
for _, fd := range spec.Backend.Docker.FailureDomains {
109113
domainNames = append(domainNames, fd.Name)

0 commit comments

Comments
 (0)