Skip to content

Commit 5f9c050

Browse files
k8s-ci-robotk8s-ci-robot
authored
Merge pull request #12416 from sbueringer/pr-ensure-order-fd-cluster
🌱 Ensure Cluster.status.failureDomains are alphabetically sorted
2 parents 4ce7960 + c80bdc2 commit 5f9c050

File tree

8 files changed

+106
-4
lines changed

8 files changed

+106
-4
lines changed

‎api/core/v1beta1/conversion.go

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,9 @@ package v1beta1
1919
import (
2020
"errors"
2121
"fmt"
22+
"maps"
23+
"slices"
24+
"sort"
2225
"unsafe"
2326

2427
corev1 "k8s.io/api/core/v1"
@@ -700,7 +703,10 @@ func Convert_v1beta1_ClusterStatus_To_v1beta2_ClusterStatus(in *ClusterStatus, o
700703
// Move FailureDomains
701704
if in.FailureDomains != nil {
702705
out.FailureDomains = []clusterv1.FailureDomain{}
703-
for name, fd := range in.FailureDomains {
706+
domainNames := slices.Collect(maps.Keys(in.FailureDomains))
707+
sort.Strings(domainNames)
708+
for _, name := range domainNames {
709+
fd := in.FailureDomains[name]
704710
out.FailureDomains = append(out.FailureDomains, clusterv1.FailureDomain{
705711
Name: name,
706712
ControlPlane: fd.ControlPlane,

‎api/core/v1beta1/conversion_test.go

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ package v1beta1
2121
import (
2222
"fmt"
2323
"reflect"
24+
"slices"
2425
"strconv"
2526
"testing"
2627
"time"
@@ -139,6 +140,25 @@ func hubClusterStatus(in *clusterv1.ClusterStatus, c randfill.Continue) {
139140
in.Initialization = nil
140141
}
141142
}
143+
144+
if len(in.FailureDomains) > 0 {
145+
in.FailureDomains = nil // Remove all pre-existing potentially invalid FailureDomains
146+
for i := range c.Int31n(20) {
147+
in.FailureDomains = append(in.FailureDomains,
148+
clusterv1.FailureDomain{
149+
Name: fmt.Sprintf("%d-%s", i, c.String(255)), // Ensure valid unique non-empty names.
150+
ControlPlane: c.Bool(),
151+
},
152+
)
153+
}
154+
// The Cluster controller always ensures alphabetic sorting when writing this field.
155+
slices.SortFunc(in.FailureDomains, func(a, b clusterv1.FailureDomain) int {
156+
if a.Name < b.Name {
157+
return -1
158+
}
159+
return 1
160+
})
161+
}
142162
}
143163

144164
func spokeCluster(in *Cluster, c randfill.Continue) {

‎internal/api/core/v1alpha3/conversion.go

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,9 @@ package v1alpha3
1919
import (
2020
"errors"
2121
"fmt"
22+
"maps"
23+
"slices"
24+
"sort"
2225
"unsafe"
2326

2427
corev1 "k8s.io/api/core/v1"
@@ -701,7 +704,10 @@ func Convert_v1alpha3_ClusterStatus_To_v1beta2_ClusterStatus(in *ClusterStatus,
701704
// Move FailureDomains
702705
if in.FailureDomains != nil {
703706
out.FailureDomains = []clusterv1.FailureDomain{}
704-
for name, fd := range in.FailureDomains {
707+
domainNames := slices.Collect(maps.Keys(in.FailureDomains))
708+
sort.Strings(domainNames)
709+
for _, name := range domainNames {
710+
fd := in.FailureDomains[name]
705711
out.FailureDomains = append(out.FailureDomains, clusterv1.FailureDomain{
706712
Name: name,
707713
ControlPlane: fd.ControlPlane,

‎internal/api/core/v1alpha3/conversion_test.go

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ package v1alpha3
2121
import (
2222
"fmt"
2323
"reflect"
24+
"slices"
2425
"testing"
2526
"time"
2627

@@ -343,6 +344,25 @@ func hubClusterStatus(in *clusterv1.ClusterStatus, c randfill.Continue) {
343344
in.Initialization = nil
344345
}
345346
}
347+
348+
if len(in.FailureDomains) > 0 {
349+
in.FailureDomains = nil // Remove all pre-existing potentially invalid FailureDomains
350+
for i := range c.Int31n(20) {
351+
in.FailureDomains = append(in.FailureDomains,
352+
clusterv1.FailureDomain{
353+
Name: fmt.Sprintf("%d-%s", i, c.String(255)), // Ensure valid unique non-empty names.
354+
ControlPlane: c.Bool(),
355+
},
356+
)
357+
}
358+
// The Cluster controller always ensures alphabetic sorting when writing this field.
359+
slices.SortFunc(in.FailureDomains, func(a, b clusterv1.FailureDomain) int {
360+
if a.Name < b.Name {
361+
return -1
362+
}
363+
return 1
364+
})
365+
}
346366
}
347367

348368
func hubClusterVariable(in *clusterv1.ClusterVariable, c randfill.Continue) {

‎internal/api/core/v1alpha4/conversion.go

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,9 @@ package v1alpha4
1919
import (
2020
"errors"
2121
"fmt"
22+
"maps"
23+
"slices"
24+
"sort"
2225
"unsafe"
2326

2427
corev1 "k8s.io/api/core/v1"
@@ -916,7 +919,10 @@ func Convert_v1alpha4_ClusterStatus_To_v1beta2_ClusterStatus(in *ClusterStatus,
916919
// Move FailureDomains
917920
if in.FailureDomains != nil {
918921
out.FailureDomains = []clusterv1.FailureDomain{}
919-
for name, fd := range in.FailureDomains {
922+
domainNames := slices.Collect(maps.Keys(in.FailureDomains))
923+
sort.Strings(domainNames)
924+
for _, name := range domainNames {
925+
fd := in.FailureDomains[name]
920926
out.FailureDomains = append(out.FailureDomains, clusterv1.FailureDomain{
921927
Name: name,
922928
ControlPlane: fd.ControlPlane,

‎internal/api/core/v1alpha4/conversion_test.go

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ package v1alpha4
2121
import (
2222
"fmt"
2323
"reflect"
24+
"slices"
2425
"strconv"
2526
"testing"
2627
"time"
@@ -225,6 +226,25 @@ func hubClusterStatus(in *clusterv1.ClusterStatus, c randfill.Continue) {
225226
in.Initialization = nil
226227
}
227228
}
229+
230+
if len(in.FailureDomains) > 0 {
231+
in.FailureDomains = nil // Remove all pre-existing potentially invalid FailureDomains
232+
for i := range c.Int31n(20) {
233+
in.FailureDomains = append(in.FailureDomains,
234+
clusterv1.FailureDomain{
235+
Name: fmt.Sprintf("%d-%s", i, c.String(255)), // Ensure valid unique non-empty names.
236+
ControlPlane: c.Bool(),
237+
},
238+
)
239+
}
240+
// The Cluster controller always ensures alphabetic sorting when writing this field.
241+
slices.SortFunc(in.FailureDomains, func(a, b clusterv1.FailureDomain) int {
242+
if a.Name < b.Name {
243+
return -1
244+
}
245+
return 1
246+
})
247+
}
228248
}
229249

230250
func hubClusterVariable(in *clusterv1.ClusterVariable, c randfill.Continue) {

‎internal/contract/infrastructure_cluster.go

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -190,6 +190,15 @@ func (d *FailureDomains) Get(obj *unstructured.Unstructured) ([]clusterv1.Failur
190190
return nil, errors.Wrapf(err, "failed to unmarshal field at %s to json", "."+strings.Join(d.path, "."))
191191
}
192192

193+
// Sort the failureDomains to ensure deterministic order even if the failureDomains field
194+
// on the InfraCluster is not sorted.
195+
slices.SortFunc(domains, func(a, b clusterv1.FailureDomain) int {
196+
if a.Name < b.Name {
197+
return -1
198+
}
199+
return 1
200+
})
201+
193202
return domains, nil
194203
}
195204

‎internal/contract/infrastructure_cluster_test.go

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -174,22 +174,37 @@ func TestInfrastructureCluster(t *testing.T) {
174174
"status": map[string]interface{}{
175175
"failureDomains": []interface{}{
176176
map[string]interface{}{
177-
"name": "newdomain",
177+
"name": "oldDomain",
178178
"controlPlane": true,
179179
"attributes": map[string]interface{}{
180180
"attribute1": "value1",
181181
},
182182
},
183+
map[string]interface{}{
184+
"name": "newdomain",
185+
"controlPlane": true,
186+
"attributes": map[string]interface{}{
187+
"attribute2": "value2",
188+
},
189+
},
183190
},
184191
},
185192
}}
186193
got, err = InfrastructureCluster().FailureDomains("v1beta2").Get(infraClusterV1Beta2)
187194
g.Expect(err).ToNot(HaveOccurred())
188195
g.Expect(got).ToNot(BeNil())
196+
// Note: Also validates the failureDomains are sorted.
189197
g.Expect(got).To(BeComparableTo([]clusterv1.FailureDomain{
190198
{
191199
Name: "newdomain",
192200
ControlPlane: true,
201+
Attributes: map[string]string{
202+
"attribute2": "value2",
203+
},
204+
},
205+
{
206+
Name: "oldDomain",
207+
ControlPlane: true,
193208
Attributes: map[string]string{
194209
"attribute1": "value1",
195210
},

0 commit comments

Comments
 (0)