Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Maintain the order from the Gateway resource #1324

Merged
merged 4 commits into from
Dec 14, 2023
Merged

Maintain the order from the Gateway resource #1324

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
0a8ee50
Maintain the order from the Gateway resource
kevin85421 Dec 6, 2023
30a78e7
Update internal/framework/status/statuses.go
kevin85421 Dec 14, 2023
9e9ecc6
Keep values instead of introducing new functions
kevin85421 Dec 14, 2023
50be6f4
Merge branch 'main' into status-order
pleshakov Dec 14, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 2 additions & 14 deletions internal/framework/status/gateway.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
package status

import (
"sort"

metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
v1 "sigs.k8s.io/gateway-api/apis/v1"
)
Expand All @@ -14,19 +12,9 @@ func prepareGatewayStatus(
) v1.GatewayStatus {
listenerStatuses := make([]v1.ListenerStatus, 0, len(gatewayStatus.ListenerStatuses))

// FIXME(pleshakov) Maintain the order from the Gateway resource
// https://github.com/nginxinc/nginx-gateway-fabric/issues/689
names := make([]string, 0, len(gatewayStatus.ListenerStatuses))
for name := range gatewayStatus.ListenerStatuses {
names = append(names, name)
}
sort.Strings(names)

for _, name := range names {
s := gatewayStatus.ListenerStatuses[name]

for _, s := range gatewayStatus.ListenerStatuses {
sjberman marked this conversation as resolved.
Show resolved Hide resolved
listenerStatuses = append(listenerStatuses, v1.ListenerStatus{
Name: v1.SectionName(name),
Name: s.Name,
SupportedKinds: s.SupportedKinds,
AttachedRoutes: s.AttachedRoutes,
Conditions: convertConditions(s.Conditions, gatewayStatus.ObservedGeneration, transitionTime),
Expand Down
3 changes: 2 additions & 1 deletion internal/framework/status/gateway_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,8 @@ func TestPrepareGatewayStatus(t *testing.T) {
status := GatewayStatus{
Conditions: CreateTestConditions("GatewayTest"),
ListenerStatuses: ListenerStatuses{
"listener": {
{
Name: "listener",
AttachedRoutes: 3,
Conditions: CreateTestConditions("ListenerTest"),
SupportedKinds: []v1.RouteGroupKind{
Expand Down
6 changes: 4 additions & 2 deletions internal/framework/status/statuses.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,8 +39,8 @@ func (n *NginxGatewayStatus) APIGroup() string {
return ngfAPI.GroupName
}

// ListenerStatuses holds the statuses of listeners where the key is the name of a listener in the Gateway resource.
type ListenerStatuses map[string]ListenerStatus
// ListenerStatuses holds the statuses of listeners.
type ListenerStatuses []ListenerStatus
sjberman marked this conversation as resolved.
Show resolved Hide resolved

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

// ListenerStatus holds the status-related information about a listener in the Gateway resource.
type ListenerStatus struct {
// Name is the name of the Listener that this status corresponds to.
Name v1.SectionName
// Conditions is the list of conditions for this listener.
Conditions []conditions.Condition
// SupportedKinds is the list of SupportedKinds for this listener.
Expand Down
5 changes: 3 additions & 2 deletions internal/framework/status/updater_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,8 +89,9 @@ var _ = Describe("Updater", func() {
GatewayStatuses: status.GatewayStatuses{
{Namespace: "test", Name: "gateway"}: {
Conditions: status.CreateTestConditions("Test"),
ListenerStatuses: map[string]status.ListenerStatus{
"http": {
ListenerStatuses: []status.ListenerStatus{
{
Name: "http",
AttachedRoutes: 1,
Conditions: status.CreateTestConditions("Test"),
SupportedKinds: []v1.RouteGroupKind{{Kind: "HTTPRoute"}},
Expand Down
9 changes: 5 additions & 4 deletions internal/mode/static/build_statuses.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -141,10 +141,10 @@ func buildGatewayStatus(
}
}

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

validListenerCount := 0
for name, l := range gateway.Listeners {
for _, l := range gateway.Listeners {
pleshakov marked this conversation as resolved.
Show resolved Hide resolved
var conds []conditions.Condition

if l.Valid {
Expand All @@ -161,11 +161,12 @@ func buildGatewayStatus(
)
}

listenerStatuses[name] = status.ListenerStatus{
listenerStatuses = append(listenerStatuses, status.ListenerStatus{
Name: v1.SectionName(l.Name),
AttachedRoutes: int32(len(l.Routes)),
Conditions: conditions.DeduplicateConditions(conds),
SupportedKinds: l.SupportedKinds,
}
})
}

gwConds := staticConds.NewDefaultGatewayConditions()
Expand Down
78 changes: 48 additions & 30 deletions internal/mode/static/build_statuses_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -126,8 +126,9 @@ func TestBuildStatuses(t *testing.T) {
},
Gateway: &graph.Gateway{
Source: gw,
Listeners: map[string]*graph.Listener{
"listener-80-1": {
Listeners: []*graph.Listener{
{
Name: "listener-80-1",
Valid: true,
Routes: map[types.NamespacedName]*graph.Route{
{Namespace: "test", Name: "hr-1"}: {},
Expand All @@ -152,8 +153,9 @@ func TestBuildStatuses(t *testing.T) {
GatewayStatuses: status.GatewayStatuses{
{Namespace: "test", Name: "gateway"}: {
Conditions: staticConds.NewDefaultGatewayConditions(),
ListenerStatuses: map[string]status.ListenerStatus{
"listener-80-1": {
ListenerStatuses: []status.ListenerStatus{
{
Name: "listener-80-1",
AttachedRoutes: 1,
Conditions: staticConds.NewDefaultListenerConditions(),
},
Expand Down Expand Up @@ -249,8 +251,9 @@ func TestBuildStatusesNginxErr(t *testing.T) {
graph := &graph.Graph{
Gateway: &graph.Gateway{
Source: gw,
Listeners: map[string]*graph.Listener{
"listener-80-1": {
Listeners: []*graph.Listener{
{
Name: "listener-80-1",
Valid: true,
Routes: map[types.NamespacedName]*graph.Route{
{Namespace: "test", Name: "hr-1"}: {},
Expand All @@ -270,8 +273,9 @@ func TestBuildStatusesNginxErr(t *testing.T) {
staticConds.NewGatewayAccepted(),
staticConds.NewGatewayNotProgrammedInvalid(staticConds.GatewayMessageFailedNginxReload),
},
ListenerStatuses: map[string]status.ListenerStatus{
"listener-80-1": {
ListenerStatuses: []status.ListenerStatus{
{
Name: "listener-80-1",
AttachedRoutes: 1,
Conditions: []conditions.Condition{
staticConds.NewListenerAccepted(),
Expand Down Expand Up @@ -424,14 +428,16 @@ func TestBuildGatewayStatuses(t *testing.T) {
name: "valid gateway; all valid listeners",
gateway: &graph.Gateway{
Source: gw,
Listeners: map[string]*graph.Listener{
"listener-valid-1": {
Listeners: []*graph.Listener{
{
Name: "listener-valid-1",
Valid: true,
Routes: map[types.NamespacedName]*graph.Route{
{Namespace: "test", Name: "hr-1"}: {},
},
},
"listener-valid-2": {
{
Name: "listener-valid-2",
Valid: true,
Routes: map[types.NamespacedName]*graph.Route{
{Namespace: "test", Name: "hr-1"}: {},
Expand All @@ -443,12 +449,14 @@ func TestBuildGatewayStatuses(t *testing.T) {
expected: status.GatewayStatuses{
{Namespace: "test", Name: "gateway"}: {
Conditions: staticConds.NewDefaultGatewayConditions(),
ListenerStatuses: map[string]status.ListenerStatus{
"listener-valid-1": {
ListenerStatuses: []status.ListenerStatus{
{
Name: "listener-valid-1",
AttachedRoutes: 1,
Conditions: staticConds.NewDefaultListenerConditions(),
},
"listener-valid-2": {
{
Name: "listener-valid-2",
AttachedRoutes: 1,
Conditions: staticConds.NewDefaultListenerConditions(),
},
Expand All @@ -462,14 +470,16 @@ func TestBuildGatewayStatuses(t *testing.T) {
name: "valid gateway; some valid listeners",
gateway: &graph.Gateway{
Source: gw,
Listeners: map[string]*graph.Listener{
"listener-valid": {
Listeners: []*graph.Listener{
{
Name: "listener-valid",
Valid: true,
Routes: map[types.NamespacedName]*graph.Route{
{Namespace: "test", Name: "hr-1"}: {},
},
},
"listener-invalid": {
{
Name: "listener-invalid",
Valid: false,
Conditions: staticConds.NewListenerUnsupportedValue("unsupported value"),
},
Expand All @@ -482,12 +492,14 @@ func TestBuildGatewayStatuses(t *testing.T) {
staticConds.NewGatewayProgrammed(),
staticConds.NewGatewayAcceptedListenersNotValid(),
},
ListenerStatuses: map[string]status.ListenerStatus{
"listener-valid": {
ListenerStatuses: []status.ListenerStatus{
{
Name: "listener-valid",
AttachedRoutes: 1,
Conditions: staticConds.NewDefaultListenerConditions(),
},
"listener-invalid": {
{
Name: "listener-invalid",
Conditions: staticConds.NewListenerUnsupportedValue("unsupported value"),
},
},
Expand All @@ -500,12 +512,14 @@ func TestBuildGatewayStatuses(t *testing.T) {
name: "valid gateway; no valid listeners",
gateway: &graph.Gateway{
Source: gw,
Listeners: map[string]*graph.Listener{
"listener-invalid-1": {
Listeners: []*graph.Listener{
{
Name: "listener-invalid-1",
Valid: false,
Conditions: staticConds.NewListenerUnsupportedProtocol("unsupported protocol"),
},
"listener-invalid-2": {
{
Name: "listener-invalid-2",
Valid: false,
Conditions: staticConds.NewListenerUnsupportedValue("unsupported value"),
},
Expand All @@ -515,11 +529,13 @@ func TestBuildGatewayStatuses(t *testing.T) {
expected: status.GatewayStatuses{
{Namespace: "test", Name: "gateway"}: {
Conditions: staticConds.NewGatewayNotAcceptedListenersNotValid(),
ListenerStatuses: map[string]status.ListenerStatus{
"listener-invalid-1": {
ListenerStatuses: []status.ListenerStatus{
{
Name: "listener-invalid-1",
Conditions: staticConds.NewListenerUnsupportedProtocol("unsupported protocol"),
},
"listener-invalid-2": {
{
Name: "listener-invalid-2",
Conditions: staticConds.NewListenerUnsupportedValue("unsupported value"),
},
},
Expand Down Expand Up @@ -548,8 +564,9 @@ func TestBuildGatewayStatuses(t *testing.T) {
Source: gw,
Valid: true,
Conditions: staticConds.NewDefaultGatewayConditions(),
Listeners: map[string]*graph.Listener{
"listener-valid": {
Listeners: []*graph.Listener{
{
Name: "listener-valid",
Valid: true,
Routes: map[types.NamespacedName]*graph.Route{
{Namespace: "test", Name: "hr-1"}: {},
Expand All @@ -563,8 +580,9 @@ func TestBuildGatewayStatuses(t *testing.T) {
staticConds.NewGatewayAccepted(),
staticConds.NewGatewayNotProgrammedInvalid(staticConds.GatewayMessageFailedNginxReload),
},
ListenerStatuses: map[string]status.ListenerStatus{
"listener-valid": {
ListenerStatuses: []status.ListenerStatus{
{
Name: "listener-valid",
AttachedRoutes: 1,
Conditions: []conditions.Condition{
staticConds.NewListenerAccepted(),
Expand Down
Loading