Skip to content

Commit 0a8ee50

Browse files
kevin85421kevin85421
committed
Maintain the order from the Gateway resource
* Problem: The status of the Gateway does not correspond to the order of `Spec.Listeners`. * Solution: * The order of the `v1.GatewayStatus.Listeners` depends on `GatewayStatus.ListenerStatuses`. However, `GatewayStatus.ListenerStatuses` is a map `map[string]ListenerStatus` which the order of iteration is not guaranteed. * [1] This PR changes the type of `GatewayStatus.ListenerStatuses` from map to slice. * However, [1] is not enough. The order of `GatewayStatus.ListenerStatuses` is based on the order of `graph.Gateway.Listeners`, but the type of `graph.Gateway.Listeners` is also a map. * This PR changes `Gateway.Listeners` from a map to a slice.
1 parent 3d38822 commit 0a8ee50

15 files changed

+370
-272
lines changed

‎internal/framework/status/gateway.go

Lines changed: 2 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,6 @@
11
package status
22

33
import (
4-
"sort"
5-
64
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
75
v1 "sigs.k8s.io/gateway-api/apis/v1"
86
)
@@ -14,19 +12,9 @@ func prepareGatewayStatus(
1412
) v1.GatewayStatus {
1513
listenerStatuses := make([]v1.ListenerStatus, 0, len(gatewayStatus.ListenerStatuses))
1614

17-
// FIXME(pleshakov) Maintain the order from the Gateway resource
18-
// https://github.com/nginxinc/nginx-gateway-fabric/issues/689
19-
names := make([]string, 0, len(gatewayStatus.ListenerStatuses))
20-
for name := range gatewayStatus.ListenerStatuses {
21-
names = append(names, name)
22-
}
23-
sort.Strings(names)
24-
25-
for _, name := range names {
26-
s := gatewayStatus.ListenerStatuses[name]
27-
15+
for _, s := range gatewayStatus.ListenerStatuses {
2816
listenerStatuses = append(listenerStatuses, v1.ListenerStatus{
29-
Name: v1.SectionName(name),
17+
Name: s.Name,
3018
SupportedKinds: s.SupportedKinds,
3119
AttachedRoutes: s.AttachedRoutes,
3220
Conditions: convertConditions(s.Conditions, gatewayStatus.ObservedGeneration, transitionTime),

‎internal/framework/status/gateway_test.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,8 @@ func TestPrepareGatewayStatus(t *testing.T) {
1919
status := GatewayStatus{
2020
Conditions: CreateTestConditions("GatewayTest"),
2121
ListenerStatuses: ListenerStatuses{
22-
"listener": {
22+
{
23+
Name: "listener",
2324
AttachedRoutes: 3,
2425
Conditions: CreateTestConditions("ListenerTest"),
2526
SupportedKinds: []v1.RouteGroupKind{

‎internal/framework/status/statuses.go

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ func (n *NginxGatewayStatus) APIGroup() string {
4040
}
4141

4242
// ListenerStatuses holds the statuses of listeners where the key is the name of a listener in the Gateway resource.
43-
type ListenerStatuses map[string]ListenerStatus
43+
type ListenerStatuses []ListenerStatus
4444

4545
// HTTPRouteStatuses holds the statuses of HTTPRoutes where the key is the namespaced name of an HTTPRoute.
4646
type HTTPRouteStatuses map[types.NamespacedName]HTTPRouteStatus
@@ -67,6 +67,8 @@ type GatewayStatus struct {
6767

6868
// ListenerStatus holds the status-related information about a listener in the Gateway resource.
6969
type ListenerStatus struct {
70+
// Name is the name of the Listener that this status corresponds to.
71+
Name v1.SectionName
7072
// Conditions is the list of conditions for this listener.
7173
Conditions []conditions.Condition
7274
// SupportedKinds is the list of SupportedKinds for this listener.

‎internal/framework/status/updater_test.go

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -89,8 +89,9 @@ var _ = Describe("Updater", func() {
8989
GatewayStatuses: status.GatewayStatuses{
9090
{Namespace: "test", Name: "gateway"}: {
9191
Conditions: status.CreateTestConditions("Test"),
92-
ListenerStatuses: map[string]status.ListenerStatus{
93-
"http": {
92+
ListenerStatuses: []status.ListenerStatus{
93+
{
94+
Name: "http",
9495
AttachedRoutes: 1,
9596
Conditions: status.CreateTestConditions("Test"),
9697
SupportedKinds: []v1.RouteGroupKind{{Kind: "HTTPRoute"}},

‎internal/mode/static/build_statuses.go

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -141,10 +141,10 @@ func buildGatewayStatus(
141141
}
142142
}
143143

144-
listenerStatuses := make(map[string]status.ListenerStatus)
144+
listenerStatuses := make([]status.ListenerStatus, 0, len(gateway.Listeners))
145145

146146
validListenerCount := 0
147-
for name, l := range gateway.Listeners {
147+
for _, l := range gateway.Listeners {
148148
var conds []conditions.Condition
149149

150150
if l.Valid {
@@ -161,11 +161,12 @@ func buildGatewayStatus(
161161
)
162162
}
163163

164-
listenerStatuses[name] = status.ListenerStatus{
164+
listenerStatuses = append(listenerStatuses, status.ListenerStatus{
165+
Name: v1.SectionName(l.Name),
165166
AttachedRoutes: int32(len(l.Routes)),
166167
Conditions: conditions.DeduplicateConditions(conds),
167168
SupportedKinds: l.SupportedKinds,
168-
}
169+
})
169170
}
170171

171172
gwConds := staticConds.NewDefaultGatewayConditions()

‎internal/mode/static/build_statuses_test.go

Lines changed: 48 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -126,8 +126,9 @@ func TestBuildStatuses(t *testing.T) {
126126
},
127127
Gateway: &graph.Gateway{
128128
Source: gw,
129-
Listeners: map[string]*graph.Listener{
130-
"listener-80-1": {
129+
Listeners: []*graph.Listener{
130+
{
131+
Name: "listener-80-1",
131132
Valid: true,
132133
Routes: map[types.NamespacedName]*graph.Route{
133134
{Namespace: "test", Name: "hr-1"}: {},
@@ -152,8 +153,9 @@ func TestBuildStatuses(t *testing.T) {
152153
GatewayStatuses: status.GatewayStatuses{
153154
{Namespace: "test", Name: "gateway"}: {
154155
Conditions: staticConds.NewDefaultGatewayConditions(),
155-
ListenerStatuses: map[string]status.ListenerStatus{
156-
"listener-80-1": {
156+
ListenerStatuses: []status.ListenerStatus{
157+
{
158+
Name: "listener-80-1",
157159
AttachedRoutes: 1,
158160
Conditions: staticConds.NewDefaultListenerConditions(),
159161
},
@@ -249,8 +251,9 @@ func TestBuildStatusesNginxErr(t *testing.T) {
249251
graph := &graph.Graph{
250252
Gateway: &graph.Gateway{
251253
Source: gw,
252-
Listeners: map[string]*graph.Listener{
253-
"listener-80-1": {
254+
Listeners: []*graph.Listener{
255+
{
256+
Name: "listener-80-1",
254257
Valid: true,
255258
Routes: map[types.NamespacedName]*graph.Route{
256259
{Namespace: "test", Name: "hr-1"}: {},
@@ -270,8 +273,9 @@ func TestBuildStatusesNginxErr(t *testing.T) {
270273
staticConds.NewGatewayAccepted(),
271274
staticConds.NewGatewayNotProgrammedInvalid(staticConds.GatewayMessageFailedNginxReload),
272275
},
273-
ListenerStatuses: map[string]status.ListenerStatus{
274-
"listener-80-1": {
276+
ListenerStatuses: []status.ListenerStatus{
277+
{
278+
Name: "listener-80-1",
275279
AttachedRoutes: 1,
276280
Conditions: []conditions.Condition{
277281
staticConds.NewListenerAccepted(),
@@ -424,14 +428,16 @@ func TestBuildGatewayStatuses(t *testing.T) {
424428
name: "valid gateway; all valid listeners",
425429
gateway: &graph.Gateway{
426430
Source: gw,
427-
Listeners: map[string]*graph.Listener{
428-
"listener-valid-1": {
431+
Listeners: []*graph.Listener{
432+
{
433+
Name: "listener-valid-1",
429434
Valid: true,
430435
Routes: map[types.NamespacedName]*graph.Route{
431436
{Namespace: "test", Name: "hr-1"}: {},
432437
},
433438
},
434-
"listener-valid-2": {
439+
{
440+
Name: "listener-valid-2",
435441
Valid: true,
436442
Routes: map[types.NamespacedName]*graph.Route{
437443
{Namespace: "test", Name: "hr-1"}: {},
@@ -443,12 +449,14 @@ func TestBuildGatewayStatuses(t *testing.T) {
443449
expected: status.GatewayStatuses{
444450
{Namespace: "test", Name: "gateway"}: {
445451
Conditions: staticConds.NewDefaultGatewayConditions(),
446-
ListenerStatuses: map[string]status.ListenerStatus{
447-
"listener-valid-1": {
452+
ListenerStatuses: []status.ListenerStatus{
453+
{
454+
Name: "listener-valid-1",
448455
AttachedRoutes: 1,
449456
Conditions: staticConds.NewDefaultListenerConditions(),
450457
},
451-
"listener-valid-2": {
458+
{
459+
Name: "listener-valid-2",
452460
AttachedRoutes: 1,
453461
Conditions: staticConds.NewDefaultListenerConditions(),
454462
},
@@ -462,14 +470,16 @@ func TestBuildGatewayStatuses(t *testing.T) {
462470
name: "valid gateway; some valid listeners",
463471
gateway: &graph.Gateway{
464472
Source: gw,
465-
Listeners: map[string]*graph.Listener{
466-
"listener-valid": {
473+
Listeners: []*graph.Listener{
474+
{
475+
Name: "listener-valid",
467476
Valid: true,
468477
Routes: map[types.NamespacedName]*graph.Route{
469478
{Namespace: "test", Name: "hr-1"}: {},
470479
},
471480
},
472-
"listener-invalid": {
481+
{
482+
Name: "listener-invalid",
473483
Valid: false,
474484
Conditions: staticConds.NewListenerUnsupportedValue("unsupported value"),
475485
},
@@ -482,12 +492,14 @@ func TestBuildGatewayStatuses(t *testing.T) {
482492
staticConds.NewGatewayProgrammed(),
483493
staticConds.NewGatewayAcceptedListenersNotValid(),
484494
},
485-
ListenerStatuses: map[string]status.ListenerStatus{
486-
"listener-valid": {
495+
ListenerStatuses: []status.ListenerStatus{
496+
{
497+
Name: "listener-valid",
487498
AttachedRoutes: 1,
488499
Conditions: staticConds.NewDefaultListenerConditions(),
489500
},
490-
"listener-invalid": {
501+
{
502+
Name: "listener-invalid",
491503
Conditions: staticConds.NewListenerUnsupportedValue("unsupported value"),
492504
},
493505
},
@@ -500,12 +512,14 @@ func TestBuildGatewayStatuses(t *testing.T) {
500512
name: "valid gateway; no valid listeners",
501513
gateway: &graph.Gateway{
502514
Source: gw,
503-
Listeners: map[string]*graph.Listener{
504-
"listener-invalid-1": {
515+
Listeners: []*graph.Listener{
516+
{
517+
Name: "listener-invalid-1",
505518
Valid: false,
506519
Conditions: staticConds.NewListenerUnsupportedProtocol("unsupported protocol"),
507520
},
508-
"listener-invalid-2": {
521+
{
522+
Name: "listener-invalid-2",
509523
Valid: false,
510524
Conditions: staticConds.NewListenerUnsupportedValue("unsupported value"),
511525
},
@@ -515,11 +529,13 @@ func TestBuildGatewayStatuses(t *testing.T) {
515529
expected: status.GatewayStatuses{
516530
{Namespace: "test", Name: "gateway"}: {
517531
Conditions: staticConds.NewGatewayNotAcceptedListenersNotValid(),
518-
ListenerStatuses: map[string]status.ListenerStatus{
519-
"listener-invalid-1": {
532+
ListenerStatuses: []status.ListenerStatus{
533+
{
534+
Name: "listener-invalid-1",
520535
Conditions: staticConds.NewListenerUnsupportedProtocol("unsupported protocol"),
521536
},
522-
"listener-invalid-2": {
537+
{
538+
Name: "listener-invalid-2",
523539
Conditions: staticConds.NewListenerUnsupportedValue("unsupported value"),
524540
},
525541
},
@@ -548,8 +564,9 @@ func TestBuildGatewayStatuses(t *testing.T) {
548564
Source: gw,
549565
Valid: true,
550566
Conditions: staticConds.NewDefaultGatewayConditions(),
551-
Listeners: map[string]*graph.Listener{
552-
"listener-valid": {
567+
Listeners: []*graph.Listener{
568+
{
569+
Name: "listener-valid",
553570
Valid: true,
554571
Routes: map[types.NamespacedName]*graph.Route{
555572
{Namespace: "test", Name: "hr-1"}: {},
@@ -563,8 +580,9 @@ func TestBuildGatewayStatuses(t *testing.T) {
563580
staticConds.NewGatewayAccepted(),
564581
staticConds.NewGatewayNotProgrammedInvalid(staticConds.GatewayMessageFailedNginxReload),
565582
},
566-
ListenerStatuses: map[string]status.ListenerStatus{
567-
"listener-valid": {
583+
ListenerStatuses: []status.ListenerStatus{
584+
{
585+
Name: "listener-valid",
568586
AttachedRoutes: 1,
569587
Conditions: []conditions.Condition{
570588
staticConds.NewListenerAccepted(),

0 commit comments

Comments
 (0)