From be51e36c7fefba5a3ae23701e0e094fad1771a4a Mon Sep 17 00:00:00 2001 From: Kate Osborn <50597707+kate-osborn@users.noreply.github.com> Date: Wed, 17 May 2023 15:18:55 -0600 Subject: [PATCH] Set Accepted condition type on Gateway status (#633) * Set Accepted condition type on Gateway status We always report the Accepted condition on the Gateway status. Its value depends on the validity of its spec and its Listeners. If the Gateway spec is invalid, we report the Accepted/False/Invalid Gateway condition but do not report Listener statuses. In all other cases, the Listener statuses will be reported. For routes that reference an invalid Gateway, we set the Route condition Accepted/False/InvalidGateway. * Reduce the complexity of bindRouteToListeners The gocyclo linter complained about the complexity of the bindRouteToListeners function. This commit refactors this function to reduce its complexity. --- docs/gateway-api-compatibility.md | 8 +- internal/state/change_processor_test.go | 336 +++++++++++------------ internal/state/conditions/conditions.go | 239 ++++++++++------ internal/state/graph/gateway.go | 38 ++- internal/state/graph/gateway_listener.go | 33 +-- internal/state/graph/gateway_test.go | 58 ++-- internal/state/graph/graph_test.go | 1 + internal/state/graph/httproute.go | 121 +++++--- internal/state/graph/httproute_test.go | 34 +++ internal/state/statuses.go | 116 +++++--- internal/state/statuses_test.go | 228 +++++++++++++-- internal/status/conditions_test.go | 20 +- internal/status/gateway.go | 31 +-- internal/status/gateway_test.go | 33 +-- internal/status/gatewayclass_test.go | 4 +- internal/status/httproute_test.go | 8 +- internal/status/updater.go | 17 +- internal/status/updater_test.go | 53 ++-- 18 files changed, 830 insertions(+), 548 deletions(-) diff --git a/docs/gateway-api-compatibility.md b/docs/gateway-api-compatibility.md index a327263fb3..d7b510020d 100644 --- a/docs/gateway-api-compatibility.md +++ b/docs/gateway-api-compatibility.md @@ -71,7 +71,13 @@ Fields: * `name` - supported. * `supportedKinds` - not supported. * `attachedRoutes` - supported. - * `conditions` - partially supported. + * `conditions` - Supported (Condition/Status/Reason): + * `Accepted/True/Accepted` + * `Accepted/True/ListenersNotValid` + * `Accepted/False/Invalid` + * `Accepted/False/ListenersNotValid` + * `Accepted/False/UnsupportedValue`: Custom reason for when a value of a field in a Gateway is invalid or not supported. + * `Accepted/False/GatewayConflict`: Custom reason for when the Gateway is ignored due to a conflicting Gateway. NKG only supports a single Gateway. ### HTTPRoute diff --git a/internal/state/change_processor_test.go b/internal/state/change_processor_test.go index 3d9217416d..e679f3031d 100644 --- a/internal/state/change_processor_test.go +++ b/internal/state/change_processor_test.go @@ -293,8 +293,8 @@ var _ = Describe("ChangeProcessor", func() { expectedConf := dataplane.Configuration{} expectedStatuses := state.Statuses{ - IgnoredGatewayStatuses: map[types.NamespacedName]state.IgnoredGatewayStatus{}, - HTTPRouteStatuses: map[types.NamespacedName]state.HTTPRouteStatus{}, + GatewayStatuses: state.GatewayStatuses{}, + HTTPRouteStatuses: state.HTTPRouteStatuses{}, } changed, conf, statuses := processor.Process(context.TODO()) @@ -310,30 +310,15 @@ var _ = Describe("ChangeProcessor", func() { expectedConf := dataplane.Configuration{} expectedStatuses := state.Statuses{ - GatewayStatus: &state.GatewayStatus{ - NsName: types.NamespacedName{Namespace: "test", Name: "gateway-1"}, - ObservedGeneration: gw1.Generation, - ListenerStatuses: map[string]state.ListenerStatus{ - "listener-80-1": { - AttachedRoutes: 0, - Conditions: []conditions.Condition{ - conditions.NewListenerResolvedRefs(), - conditions.NewListenerNoConflicts(), - conditions.NewListenerNoValidGatewayClass("GatewayClass doesn't exist"), - }, - }, - "listener-443-1": { - AttachedRoutes: 0, - Conditions: []conditions.Condition{ - conditions.NewListenerResolvedRefs(), - conditions.NewListenerNoConflicts(), - conditions.NewListenerNoValidGatewayClass("GatewayClass doesn't exist"), - }, + GatewayStatuses: state.GatewayStatuses{ + {Namespace: "test", Name: "gateway-1"}: { + Conditions: []conditions.Condition{ + conditions.NewGatewayInvalid("GatewayClass doesn't exist"), }, + ObservedGeneration: gw1.Generation, }, }, - IgnoredGatewayStatuses: map[types.NamespacedName]state.IgnoredGatewayStatus{}, - HTTPRouteStatuses: map[types.NamespacedName]state.HTTPRouteStatus{ + HTTPRouteStatuses: state.HTTPRouteStatuses{ {Namespace: "test", Name: "hr-1"}: { ObservedGeneration: hr1.Generation, ParentStatuses: []state.ParentStatus{ @@ -341,16 +326,16 @@ var _ = Describe("ChangeProcessor", func() { GatewayNsName: client.ObjectKeyFromObject(gw1), SectionName: helpers.GetPointer[v1beta1.SectionName]("listener-80-1"), Conditions: []conditions.Condition{ - conditions.NewRouteInvalidListener(), conditions.NewRouteResolvedRefs(), + conditions.NewRouteInvalidGateway(), }, }, { GatewayNsName: client.ObjectKeyFromObject(gw1), SectionName: helpers.GetPointer[v1beta1.SectionName]("listener-443-1"), Conditions: []conditions.Condition{ - conditions.NewRouteInvalidListener(), conditions.NewRouteResolvedRefs(), + conditions.NewRouteInvalidGateway(), }, }, }, @@ -427,22 +412,23 @@ var _ = Describe("ChangeProcessor", func() { ObservedGeneration: gc.Generation, Conditions: conditions.NewDefaultGatewayClassConditions(), }, - GatewayStatus: &state.GatewayStatus{ - NsName: types.NamespacedName{Namespace: "test", Name: "gateway-1"}, - ObservedGeneration: gw1.Generation, - ListenerStatuses: map[string]state.ListenerStatus{ - "listener-80-1": { - AttachedRoutes: 1, - Conditions: conditions.NewDefaultListenerConditions(), - }, - "listener-443-1": { - AttachedRoutes: 1, - Conditions: conditions.NewDefaultListenerConditions(), + GatewayStatuses: state.GatewayStatuses{ + {Namespace: "test", Name: "gateway-1"}: { + Conditions: conditions.NewDefaultGatewayConditions(), + ObservedGeneration: gw1.Generation, + ListenerStatuses: map[string]state.ListenerStatus{ + "listener-80-1": { + AttachedRoutes: 1, + Conditions: conditions.NewDefaultListenerConditions(), + }, + "listener-443-1": { + AttachedRoutes: 1, + Conditions: conditions.NewDefaultListenerConditions(), + }, }, }, }, - IgnoredGatewayStatuses: map[types.NamespacedName]state.IgnoredGatewayStatus{}, - HTTPRouteStatuses: map[types.NamespacedName]state.HTTPRouteStatus{ + HTTPRouteStatuses: state.HTTPRouteStatuses{ {Namespace: "test", Name: "hr-1"}: { ObservedGeneration: hr1.Generation, ParentStatuses: []state.ParentStatus{ @@ -540,22 +526,23 @@ var _ = Describe("ChangeProcessor", func() { ObservedGeneration: gc.Generation, Conditions: conditions.NewDefaultGatewayClassConditions(), }, - GatewayStatus: &state.GatewayStatus{ - NsName: types.NamespacedName{Namespace: "test", Name: "gateway-1"}, - ObservedGeneration: gw1.Generation, - ListenerStatuses: map[string]state.ListenerStatus{ - "listener-80-1": { - AttachedRoutes: 1, - Conditions: conditions.NewDefaultListenerConditions(), - }, - "listener-443-1": { - AttachedRoutes: 1, - Conditions: conditions.NewDefaultListenerConditions(), + GatewayStatuses: state.GatewayStatuses{ + {Namespace: "test", Name: "gateway-1"}: { + Conditions: conditions.NewDefaultGatewayConditions(), + ObservedGeneration: gw1.Generation, + ListenerStatuses: map[string]state.ListenerStatus{ + "listener-80-1": { + AttachedRoutes: 1, + Conditions: conditions.NewDefaultListenerConditions(), + }, + "listener-443-1": { + AttachedRoutes: 1, + Conditions: conditions.NewDefaultListenerConditions(), + }, }, }, }, - IgnoredGatewayStatuses: map[types.NamespacedName]state.IgnoredGatewayStatus{}, - HTTPRouteStatuses: map[types.NamespacedName]state.HTTPRouteStatus{ + HTTPRouteStatuses: state.HTTPRouteStatuses{ {Namespace: "test", Name: "hr-1"}: { ObservedGeneration: hr1Updated.Generation, ParentStatuses: []state.ParentStatus{ @@ -654,22 +641,23 @@ var _ = Describe("ChangeProcessor", func() { ObservedGeneration: gc.Generation, Conditions: conditions.NewDefaultGatewayClassConditions(), }, - GatewayStatus: &state.GatewayStatus{ - NsName: types.NamespacedName{Namespace: "test", Name: "gateway-1"}, - ObservedGeneration: gw1Updated.Generation, - ListenerStatuses: map[string]state.ListenerStatus{ - "listener-80-1": { - AttachedRoutes: 1, - Conditions: conditions.NewDefaultListenerConditions(), - }, - "listener-443-1": { - AttachedRoutes: 1, - Conditions: conditions.NewDefaultListenerConditions(), + GatewayStatuses: state.GatewayStatuses{ + {Namespace: "test", Name: "gateway-1"}: { + Conditions: conditions.NewDefaultGatewayConditions(), + ObservedGeneration: gw1Updated.Generation, + ListenerStatuses: map[string]state.ListenerStatus{ + "listener-80-1": { + AttachedRoutes: 1, + Conditions: conditions.NewDefaultListenerConditions(), + }, + "listener-443-1": { + AttachedRoutes: 1, + Conditions: conditions.NewDefaultListenerConditions(), + }, }, }, }, - IgnoredGatewayStatuses: map[types.NamespacedName]state.IgnoredGatewayStatus{}, - HTTPRouteStatuses: map[types.NamespacedName]state.HTTPRouteStatus{ + HTTPRouteStatuses: state.HTTPRouteStatuses{ {Namespace: "test", Name: "hr-1"}: { ObservedGeneration: hr1Updated.Generation, ParentStatuses: []state.ParentStatus{ @@ -767,22 +755,23 @@ var _ = Describe("ChangeProcessor", func() { ObservedGeneration: gcUpdated.Generation, Conditions: conditions.NewDefaultGatewayClassConditions(), }, - GatewayStatus: &state.GatewayStatus{ - NsName: types.NamespacedName{Namespace: "test", Name: "gateway-1"}, - ObservedGeneration: gw1Updated.Generation, - ListenerStatuses: map[string]state.ListenerStatus{ - "listener-80-1": { - AttachedRoutes: 1, - Conditions: conditions.NewDefaultListenerConditions(), - }, - "listener-443-1": { - AttachedRoutes: 1, - Conditions: conditions.NewDefaultListenerConditions(), + GatewayStatuses: state.GatewayStatuses{ + {Namespace: "test", Name: "gateway-1"}: { + Conditions: conditions.NewDefaultGatewayConditions(), + ObservedGeneration: gw1Updated.Generation, + ListenerStatuses: map[string]state.ListenerStatus{ + "listener-80-1": { + AttachedRoutes: 1, + Conditions: conditions.NewDefaultListenerConditions(), + }, + "listener-443-1": { + AttachedRoutes: 1, + Conditions: conditions.NewDefaultListenerConditions(), + }, }, }, }, - IgnoredGatewayStatuses: map[types.NamespacedName]state.IgnoredGatewayStatus{}, - HTTPRouteStatuses: map[types.NamespacedName]state.HTTPRouteStatus{ + HTTPRouteStatuses: state.HTTPRouteStatuses{ {Namespace: "test", Name: "hr-1"}: { ObservedGeneration: hr1Updated.Generation, ParentStatuses: []state.ParentStatus{ @@ -879,26 +868,27 @@ var _ = Describe("ChangeProcessor", func() { ObservedGeneration: gcUpdated.Generation, Conditions: conditions.NewDefaultGatewayClassConditions(), }, - GatewayStatus: &state.GatewayStatus{ - NsName: types.NamespacedName{Namespace: "test", Name: "gateway-1"}, - ObservedGeneration: gw1Updated.Generation, - ListenerStatuses: map[string]state.ListenerStatus{ - "listener-80-1": { - AttachedRoutes: 1, - Conditions: conditions.NewDefaultListenerConditions(), - }, - "listener-443-1": { - AttachedRoutes: 1, - Conditions: conditions.NewDefaultListenerConditions(), + GatewayStatuses: state.GatewayStatuses{ + {Namespace: "test", Name: "gateway-1"}: { + Conditions: conditions.NewDefaultGatewayConditions(), + ObservedGeneration: gw1Updated.Generation, + ListenerStatuses: map[string]state.ListenerStatus{ + "listener-80-1": { + AttachedRoutes: 1, + Conditions: conditions.NewDefaultListenerConditions(), + }, + "listener-443-1": { + AttachedRoutes: 1, + Conditions: conditions.NewDefaultListenerConditions(), + }, }, }, - }, - IgnoredGatewayStatuses: map[types.NamespacedName]state.IgnoredGatewayStatus{ {Namespace: "test", Name: "gateway-2"}: { + Conditions: []conditions.Condition{conditions.NewGatewayConflict()}, ObservedGeneration: gw2.Generation, }, }, - HTTPRouteStatuses: map[types.NamespacedName]state.HTTPRouteStatus{ + HTTPRouteStatuses: state.HTTPRouteStatuses{ {Namespace: "test", Name: "hr-1"}: { ObservedGeneration: hr1Updated.Generation, ParentStatuses: []state.ParentStatus{ @@ -984,26 +974,27 @@ var _ = Describe("ChangeProcessor", func() { ObservedGeneration: gcUpdated.Generation, Conditions: conditions.NewDefaultGatewayClassConditions(), }, - GatewayStatus: &state.GatewayStatus{ - NsName: types.NamespacedName{Namespace: "test", Name: "gateway-1"}, - ObservedGeneration: gw1Updated.Generation, - ListenerStatuses: map[string]state.ListenerStatus{ - "listener-80-1": { - AttachedRoutes: 1, - Conditions: conditions.NewDefaultListenerConditions(), - }, - "listener-443-1": { - AttachedRoutes: 1, - Conditions: conditions.NewDefaultListenerConditions(), + GatewayStatuses: state.GatewayStatuses{ + {Namespace: "test", Name: "gateway-1"}: { + Conditions: conditions.NewDefaultGatewayConditions(), + ObservedGeneration: gw1Updated.Generation, + ListenerStatuses: map[string]state.ListenerStatus{ + "listener-80-1": { + AttachedRoutes: 1, + Conditions: conditions.NewDefaultListenerConditions(), + }, + "listener-443-1": { + AttachedRoutes: 1, + Conditions: conditions.NewDefaultListenerConditions(), + }, }, }, - }, - IgnoredGatewayStatuses: map[types.NamespacedName]state.IgnoredGatewayStatus{ {Namespace: "test", Name: "gateway-2"}: { ObservedGeneration: gw2.Generation, + Conditions: []conditions.Condition{conditions.NewGatewayConflict()}, }, }, - HTTPRouteStatuses: map[types.NamespacedName]state.HTTPRouteStatus{ + HTTPRouteStatuses: state.HTTPRouteStatuses{ {Namespace: "test", Name: "hr-1"}: { ObservedGeneration: hr1Updated.Generation, ParentStatuses: []state.ParentStatus{ @@ -1113,22 +1104,23 @@ var _ = Describe("ChangeProcessor", func() { ObservedGeneration: gcUpdated.Generation, Conditions: conditions.NewDefaultGatewayClassConditions(), }, - GatewayStatus: &state.GatewayStatus{ - NsName: types.NamespacedName{Namespace: "test", Name: "gateway-2"}, - ObservedGeneration: gw2.Generation, - ListenerStatuses: map[string]state.ListenerStatus{ - "listener-80-1": { - AttachedRoutes: 1, - Conditions: conditions.NewDefaultListenerConditions(), - }, - "listener-443-1": { - AttachedRoutes: 1, - Conditions: conditions.NewDefaultListenerConditions(), + GatewayStatuses: state.GatewayStatuses{ + {Namespace: "test", Name: "gateway-2"}: { + Conditions: conditions.NewDefaultGatewayConditions(), + ObservedGeneration: gw2.Generation, + ListenerStatuses: map[string]state.ListenerStatus{ + "listener-80-1": { + AttachedRoutes: 1, + Conditions: conditions.NewDefaultListenerConditions(), + }, + "listener-443-1": { + AttachedRoutes: 1, + Conditions: conditions.NewDefaultListenerConditions(), + }, }, }, }, - IgnoredGatewayStatuses: map[types.NamespacedName]state.IgnoredGatewayStatus{}, - HTTPRouteStatuses: map[types.NamespacedName]state.HTTPRouteStatus{ + HTTPRouteStatuses: state.HTTPRouteStatuses{ {Namespace: "test", Name: "hr-2"}: { ObservedGeneration: hr2.Generation, ParentStatuses: []state.ParentStatus{ @@ -1181,22 +1173,23 @@ var _ = Describe("ChangeProcessor", func() { ObservedGeneration: gcUpdated.Generation, Conditions: conditions.NewDefaultGatewayClassConditions(), }, - GatewayStatus: &state.GatewayStatus{ - NsName: types.NamespacedName{Namespace: "test", Name: "gateway-2"}, - ObservedGeneration: gw2.Generation, - ListenerStatuses: map[string]state.ListenerStatus{ - "listener-80-1": { - AttachedRoutes: 0, - Conditions: conditions.NewDefaultListenerConditions(), - }, - "listener-443-1": { - AttachedRoutes: 0, - Conditions: conditions.NewDefaultListenerConditions(), + GatewayStatuses: state.GatewayStatuses{ + {Namespace: "test", Name: "gateway-2"}: { + Conditions: conditions.NewDefaultGatewayConditions(), + ObservedGeneration: gw2.Generation, + ListenerStatuses: map[string]state.ListenerStatus{ + "listener-80-1": { + AttachedRoutes: 0, + Conditions: conditions.NewDefaultListenerConditions(), + }, + "listener-443-1": { + AttachedRoutes: 0, + Conditions: conditions.NewDefaultListenerConditions(), + }, }, }, }, - IgnoredGatewayStatuses: map[types.NamespacedName]state.IgnoredGatewayStatus{}, - HTTPRouteStatuses: map[types.NamespacedName]state.HTTPRouteStatus{}, + HTTPRouteStatuses: state.HTTPRouteStatuses{}, } changed, conf, statuses := processor.Process(context.TODO()) @@ -1214,30 +1207,15 @@ var _ = Describe("ChangeProcessor", func() { expectedConf := dataplane.Configuration{} expectedStatuses := state.Statuses{ - GatewayStatus: &state.GatewayStatus{ - NsName: types.NamespacedName{Namespace: "test", Name: "gateway-2"}, - ObservedGeneration: gw2.Generation, - ListenerStatuses: map[string]state.ListenerStatus{ - "listener-80-1": { - AttachedRoutes: 0, - Conditions: []conditions.Condition{ - conditions.NewListenerResolvedRefs(), - conditions.NewListenerNoConflicts(), - conditions.NewListenerNoValidGatewayClass("GatewayClass doesn't exist"), - }, - }, - "listener-443-1": { - AttachedRoutes: 0, - Conditions: []conditions.Condition{ - conditions.NewListenerResolvedRefs(), - conditions.NewListenerNoConflicts(), - conditions.NewListenerNoValidGatewayClass("GatewayClass doesn't exist"), - }, + GatewayStatuses: state.GatewayStatuses{ + {Namespace: "test", Name: "gateway-2"}: { + Conditions: []conditions.Condition{ + conditions.NewGatewayInvalid("GatewayClass doesn't exist"), }, + ObservedGeneration: gw2.Generation, }, }, - IgnoredGatewayStatuses: map[types.NamespacedName]state.IgnoredGatewayStatus{}, - HTTPRouteStatuses: map[types.NamespacedName]state.HTTPRouteStatus{}, + HTTPRouteStatuses: state.HTTPRouteStatuses{}, } changed, conf, statuses := processor.Process(context.TODO()) @@ -1255,8 +1233,8 @@ var _ = Describe("ChangeProcessor", func() { expectedConf := dataplane.Configuration{} expectedStatuses := state.Statuses{ - IgnoredGatewayStatuses: map[types.NamespacedName]state.IgnoredGatewayStatus{}, - HTTPRouteStatuses: map[types.NamespacedName]state.HTTPRouteStatus{}, + GatewayStatuses: state.GatewayStatuses{}, + HTTPRouteStatuses: state.HTTPRouteStatuses{}, } changed, conf, statuses := processor.Process(context.TODO()) @@ -1274,8 +1252,8 @@ var _ = Describe("ChangeProcessor", func() { expectedConf := dataplane.Configuration{} expectedStatuses := state.Statuses{ - IgnoredGatewayStatuses: map[types.NamespacedName]state.IgnoredGatewayStatus{}, - HTTPRouteStatuses: map[types.NamespacedName]state.HTTPRouteStatus{}, + GatewayStatuses: state.GatewayStatuses{}, + HTTPRouteStatuses: state.HTTPRouteStatuses{}, } changed, conf, statuses := processor.Process(context.TODO()) @@ -2056,8 +2034,8 @@ var _ = Describe("ChangeProcessor", func() { ObservedGeneration: gc.Generation, Conditions: conditions.NewDefaultGatewayClassConditions(), }, - IgnoredGatewayStatuses: map[types.NamespacedName]state.IgnoredGatewayStatus{}, - HTTPRouteStatuses: map[types.NamespacedName]state.HTTPRouteStatus{}, + GatewayStatuses: state.GatewayStatuses{}, + HTTPRouteStatuses: state.HTTPRouteStatuses{}, } changed, conf, statuses := processor.Process(context.TODO()) @@ -2128,18 +2106,19 @@ var _ = Describe("ChangeProcessor", func() { ObservedGeneration: gc.Generation, Conditions: conditions.NewDefaultGatewayClassConditions(), }, - GatewayStatus: &state.GatewayStatus{ - NsName: gwNsName, - ObservedGeneration: gw.Generation, - ListenerStatuses: map[string]state.ListenerStatus{ - "listener-80-1": { - AttachedRoutes: 1, - Conditions: conditions.NewDefaultListenerConditions(), + GatewayStatuses: state.GatewayStatuses{ + gwNsName: { + Conditions: conditions.NewDefaultGatewayConditions(), + ObservedGeneration: gw.Generation, + ListenerStatuses: map[string]state.ListenerStatus{ + "listener-80-1": { + AttachedRoutes: 1, + Conditions: conditions.NewDefaultListenerConditions(), + }, }, }, }, - IgnoredGatewayStatuses: map[types.NamespacedName]state.IgnoredGatewayStatus{}, - HTTPRouteStatuses: map[types.NamespacedName]state.HTTPRouteStatus{ + HTTPRouteStatuses: state.HTTPRouteStatuses{ hrNsName: { ObservedGeneration: hr.Generation, ParentStatuses: []state.ParentStatus{ @@ -2180,18 +2159,19 @@ var _ = Describe("ChangeProcessor", func() { ObservedGeneration: gc.Generation, Conditions: conditions.NewDefaultGatewayClassConditions(), }, - GatewayStatus: &state.GatewayStatus{ - NsName: gwNsName, - ObservedGeneration: gw.Generation, - ListenerStatuses: map[string]state.ListenerStatus{ - "listener-80-1": { - AttachedRoutes: 0, - Conditions: conditions.NewDefaultListenerConditions(), + GatewayStatuses: state.GatewayStatuses{ + gwNsName: { + Conditions: conditions.NewDefaultGatewayConditions(), + ObservedGeneration: gw.Generation, + ListenerStatuses: map[string]state.ListenerStatus{ + "listener-80-1": { + AttachedRoutes: 0, + Conditions: conditions.NewDefaultListenerConditions(), + }, }, }, }, - IgnoredGatewayStatuses: map[types.NamespacedName]state.IgnoredGatewayStatus{}, - HTTPRouteStatuses: map[types.NamespacedName]state.HTTPRouteStatus{}, + HTTPRouteStatuses: state.HTTPRouteStatuses{}, } changed, conf, statuses := processor.Process(context.TODO()) @@ -2215,8 +2195,8 @@ var _ = Describe("ChangeProcessor", func() { ObservedGeneration: gc.Generation, Conditions: conditions.NewDefaultGatewayClassConditions(), }, - IgnoredGatewayStatuses: map[types.NamespacedName]state.IgnoredGatewayStatus{}, - HTTPRouteStatuses: map[types.NamespacedName]state.HTTPRouteStatus{}, + GatewayStatuses: state.GatewayStatuses{}, + HTTPRouteStatuses: state.HTTPRouteStatuses{}, } changed, conf, statuses := processor.Process(context.TODO()) diff --git a/internal/state/conditions/conditions.go b/internal/state/conditions/conditions.go index c610dcb7c0..974efcc35d 100644 --- a/internal/state/conditions/conditions.go +++ b/internal/state/conditions/conditions.go @@ -8,20 +8,32 @@ import ( ) const ( - // RouteReasonInvalidListener is used with the "Accepted" condition when the Route references an invalid listener. - RouteReasonInvalidListener v1beta1.RouteConditionReason = "InvalidListener" - // ListenerReasonUnsupportedValue is used with the "Accepted" condition when a value of a field in a Listener // is invalid or not supported. ListenerReasonUnsupportedValue v1beta1.ListenerConditionReason = "UnsupportedValue" - // ListenerReasonNoValidGatewayClass is used with the "Accepted" condition when there is no valid GatewayClass - // in the cluster. - ListenerReasonNoValidGatewayClass v1beta1.ListenerConditionReason = "NoValidGatewayClass" - // RouteReasonBackendRefUnsupportedValue is used with the "ResolvedRefs" condition when one of the // Route rules has a backendRef with an unsupported value. RouteReasonBackendRefUnsupportedValue = "UnsupportedValue" + + // RouteReasonInvalidGateway is used with the "Accepted" (false) condition when the Gateway the Route + // references is invalid. + RouteReasonInvalidGateway = "InvalidGateway" + + // RouteReasonInvalidListener is used with the "Accepted" condition when the Route references an invalid listener. + RouteReasonInvalidListener v1beta1.RouteConditionReason = "InvalidListener" + + // GatewayReasonGatewayConflict indicates there are multiple Gateway resources to choose from, + // and we ignored the resource in question and picked another Gateway as the winner. + // This reason is used with GatewayConditionAccepted (false). + GatewayReasonGatewayConflict v1beta1.GatewayConditionReason = "GatewayConflict" + + // GatewayMessageGatewayConflict is message that describes GatewayReasonGatewayConflict. + GatewayMessageGatewayConflict = "The resource is ignored due to a conflicting Gateway resource" + + // GatewayReasonUnsupportedValue is used with GatewayConditionAccepted (false) when a value of a field in a Gateway + // is invalid or not supported. + GatewayReasonUnsupportedValue v1beta1.GatewayConditionReason = "UnsupportedValue" ) // Condition defines a condition to be reported in the status of resources. @@ -64,6 +76,16 @@ func DeduplicateConditions(conds []Condition) []Condition { return result } +// NewTODO returns a Condition that can be used as a placeholder for a condition that is not yet implemented. +func NewTODO(msg string) Condition { + return Condition{ + Type: "TODO", + Status: metav1.ConditionTrue, + Reason: "TODO", + Message: fmt.Sprintf("The condition for this has not been implemented yet: %s", msg), + } +} + // NewDefaultRouteConditions returns the default conditions that must be present in the status of an HTTPRoute. func NewDefaultRouteConditions() []Condition { return []Condition{ @@ -103,16 +125,6 @@ func NewRouteUnsupportedValue(msg string) Condition { } } -// NewTODO returns a Condition that can be used as a placeholder for a condition that is not yet implemented. -func NewTODO(msg string) Condition { - return Condition{ - Type: "TODO", - Status: metav1.ConditionTrue, - Reason: "TODO", - Message: fmt.Sprintf("The condition for this has not been implemented yet: %s", msg), - } -} - // NewRouteInvalidListener returns a Condition that indicates that the HTTPRoute is not accepted because of an // invalid listener. func NewRouteInvalidListener() Condition { @@ -124,6 +136,80 @@ func NewRouteInvalidListener() Condition { } } +// NewRouteResolvedRefs returns a Condition that indicates that all the references on the Route are resolved. +func NewRouteResolvedRefs() Condition { + return Condition{ + Type: string(v1beta1.RouteConditionResolvedRefs), + Status: metav1.ConditionTrue, + Reason: string(v1beta1.RouteReasonResolvedRefs), + Message: "All references are resolved", + } +} + +// NewRouteBackendRefInvalidKind returns a Condition that indicates that the Route has a backendRef with an +// invalid kind. +func NewRouteBackendRefInvalidKind(msg string) Condition { + return Condition{ + Type: string(v1beta1.RouteConditionResolvedRefs), + Status: metav1.ConditionFalse, + Reason: string(v1beta1.RouteReasonInvalidKind), + Message: msg, + } +} + +// NewRouteBackendRefRefNotPermitted returns a Condition that indicates that the Route has a backendRef that +// is not permitted. +func NewRouteBackendRefRefNotPermitted(msg string) Condition { + return Condition{ + Type: string(v1beta1.RouteConditionResolvedRefs), + Status: metav1.ConditionFalse, + Reason: string(v1beta1.RouteReasonRefNotPermitted), + Message: msg, + } +} + +// NewRouteBackendRefRefBackendNotFound returns a Condition that indicates that the Route has a backendRef that +// points to non-existing backend. +func NewRouteBackendRefRefBackendNotFound(msg string) Condition { + return Condition{ + Type: string(v1beta1.RouteConditionResolvedRefs), + Status: metav1.ConditionFalse, + Reason: string(v1beta1.RouteReasonBackendNotFound), + Message: msg, + } +} + +// NewRouteBackendRefUnsupportedValue returns a Condition that indicates that the Route has a backendRef with +// an unsupported value. +func NewRouteBackendRefUnsupportedValue(msg string) Condition { + return Condition{ + Type: string(v1beta1.RouteConditionResolvedRefs), + Status: metav1.ConditionFalse, + Reason: RouteReasonBackendRefUnsupportedValue, + Message: msg, + } +} + +// NewRouteInvalidGateway returns a Condition that indicates that the Route is not Accepted because the Gateway it +// references is invalid. +func NewRouteInvalidGateway() Condition { + return Condition{ + Type: string(v1beta1.RouteConditionAccepted), + Status: metav1.ConditionFalse, + Reason: RouteReasonInvalidGateway, + Message: "Gateway is invalid", + } +} + +// NewDefaultListenerConditions returns the default Conditions that must be present in the status of a Listener. +func NewDefaultListenerConditions() []Condition { + return []Condition{ + NewListenerAccepted(), + NewListenerResolvedRefs(), + NewListenerNoConflicts(), + } +} + // NewListenerPortUnavailable returns a Condition that indicates a port is unavailable in a Listener. func NewListenerPortUnavailable(msg string) Condition { return Condition{ @@ -164,15 +250,6 @@ func NewListenerNoConflicts() Condition { } } -// NewDefaultListenerConditions returns the default Conditions that must be present in the status of a Listener. -func NewDefaultListenerConditions() []Condition { - return []Condition{ - NewListenerAccepted(), - NewListenerResolvedRefs(), - NewListenerNoConflicts(), - } -} - // NewListenerUnsupportedValue returns a Condition that indicates that a field of a Listener has an unsupported value. // Unsupported means that the value is not supported by the implementation or invalid. func NewListenerUnsupportedValue(msg string) Condition { @@ -230,89 +307,95 @@ func NewListenerUnsupportedProtocol(msg string) Condition { } } -// NewListenerNoValidGatewayClass returns a Condition that indicates that the Listener is not accepted because -// there is no valid GatewayClass. -func NewListenerNoValidGatewayClass(msg string) Condition { - return Condition{ - Type: string(v1beta1.ListenerConditionAccepted), - Status: metav1.ConditionFalse, - Reason: string(ListenerReasonNoValidGatewayClass), - Message: msg, +// NewDefaultGatewayClassConditions returns the default Conditions that must be present in the status of a GatewayClass. +func NewDefaultGatewayClassConditions() []Condition { + return []Condition{ + { + Type: string(v1beta1.GatewayClassConditionStatusAccepted), + Status: metav1.ConditionTrue, + Reason: string(v1beta1.GatewayClassReasonAccepted), + Message: "GatewayClass is accepted", + }, } } -// NewRouteBackendRefInvalidKind returns a Condition that indicates that the Route has a backendRef with an -// invalid kind. -func NewRouteBackendRefInvalidKind(msg string) Condition { +// NewGatewayClassInvalidParameters returns a Condition that indicates that the GatewayClass has invalid parameters. +func NewGatewayClassInvalidParameters(msg string) Condition { return Condition{ - Type: string(v1beta1.RouteConditionResolvedRefs), + Type: string(v1beta1.GatewayClassConditionStatusAccepted), Status: metav1.ConditionFalse, - Reason: string(v1beta1.RouteReasonInvalidKind), + Reason: string(v1beta1.GatewayClassReasonInvalidParameters), Message: msg, } } -// NewRouteBackendRefRefNotPermitted returns a Condition that indicates that the Route has a backendRef that -// is not permitted. -func NewRouteBackendRefRefNotPermitted(msg string) Condition { - return Condition{ - Type: string(v1beta1.RouteConditionResolvedRefs), - Status: metav1.ConditionFalse, - Reason: string(v1beta1.RouteReasonRefNotPermitted), - Message: msg, +// NewDefaultGatewayConditions returns the default Condition that must be present in the status of a Gateway. +func NewDefaultGatewayConditions() []Condition { + return []Condition{ + NewGatewayAccepted(), } } -// NewRouteBackendRefRefBackendNotFound returns a Condition that indicates that the Route has a backendRef that -// points to non-existing backend. -func NewRouteBackendRefRefBackendNotFound(msg string) Condition { +// NewGatewayAccepted returns a Condition that indicates the Gateway is accepted. +func NewGatewayAccepted() Condition { return Condition{ - Type: string(v1beta1.RouteConditionResolvedRefs), - Status: metav1.ConditionFalse, - Reason: string(v1beta1.RouteReasonBackendNotFound), - Message: msg, + Type: string(v1beta1.GatewayConditionAccepted), + Status: metav1.ConditionTrue, + Reason: string(v1beta1.GatewayReasonAccepted), + Message: "Gateway is accepted", } } -// NewRouteBackendRefUnsupportedValue returns a Condition that indicates that the Route has a backendRef with -// an unsupported value. -func NewRouteBackendRefUnsupportedValue(msg string) Condition { +// NewGatewayConflict returns a Condition that indicates the Gateway has a conflict with another Gateway. +func NewGatewayConflict() Condition { return Condition{ - Type: string(v1beta1.RouteConditionResolvedRefs), + Type: string(v1beta1.GatewayConditionAccepted), Status: metav1.ConditionFalse, - Reason: RouteReasonBackendRefUnsupportedValue, - Message: msg, + Reason: string(GatewayReasonGatewayConflict), + Message: GatewayMessageGatewayConflict, } } -// NewRouteResolvedRefs returns a Condition that indicates that all the references on the Route are resolved. -func NewRouteResolvedRefs() Condition { +// NewGatewayAcceptedListenersNotValid returns a Condition that indicates the Gateway is accepted, +// but has at least one listener that is invalid. +func NewGatewayAcceptedListenersNotValid() Condition { return Condition{ - Type: string(v1beta1.RouteConditionResolvedRefs), + Type: string(v1beta1.GatewayConditionAccepted), Status: metav1.ConditionTrue, - Reason: string(v1beta1.RouteReasonResolvedRefs), - Message: "All references are resolved", + Reason: string(v1beta1.GatewayReasonListenersNotValid), + Message: "Gateway has at least one valid listener", } } -// NewGatewayClassInvalidParameters returns a Condition that indicates that the GatewayClass has invalid parameters. -func NewGatewayClassInvalidParameters(msg string) Condition { +// NewGatewayNotAcceptedListenersNotValid returns a Condition that indicates the Gateway is not accepted, +// because all listeners are invalid. +func NewGatewayNotAcceptedListenersNotValid() Condition { return Condition{ - Type: string(v1beta1.GatewayClassConditionStatusAccepted), + Type: string(v1beta1.GatewayConditionAccepted), Status: metav1.ConditionFalse, - Reason: string(v1beta1.GatewayClassReasonInvalidParameters), + Reason: string(v1beta1.GatewayReasonListenersNotValid), + Message: "Gateway has no valid listeners", + } +} + +// NewGatewayInvalid returns a Condition that indicates the Gateway is not accepted because it is +// semantically or syntactically invalid. The provided message contains the details of why the Gateway is invalid. +func NewGatewayInvalid(msg string) Condition { + return Condition{ + Type: string(v1beta1.GatewayConditionAccepted), + Status: metav1.ConditionFalse, + Reason: string(v1beta1.GatewayReasonInvalid), Message: msg, } } -// NewDefaultGatewayClassConditions returns the default Conditions that must be present in the status of a GatewayClass. -func NewDefaultGatewayClassConditions() []Condition { - return []Condition{ - { - Type: string(v1beta1.GatewayClassConditionStatusAccepted), - Status: metav1.ConditionTrue, - Reason: string(v1beta1.GatewayClassReasonAccepted), - Message: "GatewayClass is accepted", - }, +// NewGatewayUnsupportedValue returns a Condition that indicates that a field of the Gateway has an unsupported value. +// Unsupported means that the value is not supported by the implementation or invalid. +func NewGatewayUnsupportedValue(msg string) Condition { + return Condition{ + Type: string(v1beta1.GatewayConditionAccepted), + Status: metav1.ConditionFalse, + Reason: string(GatewayReasonUnsupportedValue), + Message: msg, } } diff --git a/internal/state/graph/gateway.go b/internal/state/graph/gateway.go index 90a47fb3e7..e83cc0c693 100644 --- a/internal/state/graph/gateway.go +++ b/internal/state/graph/gateway.go @@ -4,10 +4,12 @@ import ( "sort" "k8s.io/apimachinery/pkg/types" + "k8s.io/apimachinery/pkg/util/validation/field" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/gateway-api/apis/v1beta1" nkgsort "github.com/nginxinc/nginx-kubernetes-gateway/internal/sort" + "github.com/nginxinc/nginx-kubernetes-gateway/internal/state/conditions" "github.com/nginxinc/nginx-kubernetes-gateway/internal/state/secrets" ) @@ -17,6 +19,10 @@ type Gateway struct { Source *v1beta1.Gateway // Listeners include the listeners of the Gateway. Listeners map[string]*Listener + // Conditions holds the conditions for the Gateway. + Conditions []conditions.Condition + // Valid indicates whether the Gateway Spec is valid. + Valid bool } // processedGateways holds the resources that belong to NKG. @@ -89,8 +95,38 @@ func buildGateway(gw *v1beta1.Gateway, secretMemoryMgr secrets.SecretDiskMemoryM return nil } + conds := validateGateway(gw, gc) + + if len(conds) > 0 { + return &Gateway{ + Source: gw, + Valid: false, + Conditions: conds, + } + } + return &Gateway{ Source: gw, - Listeners: buildListeners(gw, secretMemoryMgr, gc), + Listeners: buildListeners(gw, secretMemoryMgr), + Valid: true, + } +} + +func validateGateway(gw *v1beta1.Gateway, gc *GatewayClass) []conditions.Condition { + var conds []conditions.Condition + + if gc == nil { + conds = append(conds, conditions.NewGatewayInvalid("GatewayClass doesn't exist")) + } else if !gc.Valid { + conds = append(conds, conditions.NewGatewayInvalid("GatewayClass is invalid")) + } + + if len(gw.Spec.Addresses) > 0 { + path := field.NewPath("spec", "addresses") + valErr := field.Forbidden(path, "addresses are not supported") + + conds = append(conds, conditions.NewGatewayUnsupportedValue(valErr.Error())) } + + return conds } diff --git a/internal/state/graph/gateway_listener.go b/internal/state/graph/gateway_listener.go index c359d6c0a7..7c97c6ae38 100644 --- a/internal/state/graph/gateway_listener.go +++ b/internal/state/graph/gateway_listener.go @@ -34,11 +34,10 @@ type Listener struct { func buildListeners( gw *v1beta1.Gateway, secretMemoryMgr secrets.SecretDiskMemoryManager, - gc *GatewayClass, ) map[string]*Listener { listeners := make(map[string]*Listener) - listenerFactory := newListenerConfiguratorFactory(gw, secretMemoryMgr, gc) + listenerFactory := newListenerConfiguratorFactory(gw, secretMemoryMgr) for _, gl := range gw.Spec.Listeners { configurator := listenerFactory.getConfiguratorForListener(gl) @@ -66,7 +65,6 @@ func (f *listenerConfiguratorFactory) getConfiguratorForListener(l v1beta1.Liste func newListenerConfiguratorFactory( gw *v1beta1.Gateway, secretMemoryMgr secrets.SecretDiskMemoryManager, - gc *GatewayClass, ) *listenerConfiguratorFactory { return &listenerConfiguratorFactory{ unsupportedProtocol: &listenerConfigurator{ @@ -84,8 +82,6 @@ func newListenerConfiguratorFactory( http: &listenerConfigurator{ validators: []listenerValidator{ validateListenerHostname, - createAddressesValidator(gw), - createNoValidGatewayClassValidator(gc), validateHTTPListener, }, conflictResolvers: []listenerConflictResolver{ @@ -95,8 +91,6 @@ func newListenerConfiguratorFactory( https: &listenerConfigurator{ validators: []listenerValidator{ validateListenerHostname, - createAddressesValidator(gw), - createNoValidGatewayClassValidator(gc), createHTTPSListenerValidator(gw.Namespace), }, conflictResolvers: []listenerConflictResolver{ @@ -192,31 +186,6 @@ func validateListenerHostname(listener v1beta1.Listener) []conditions.Condition return nil } -func createAddressesValidator(gw *v1beta1.Gateway) listenerValidator { - return func(listener v1beta1.Listener) []conditions.Condition { - if len(gw.Spec.Addresses) > 0 { - path := field.NewPath("spec", "addresses") - valErr := field.Forbidden(path, "addresses are not supported") - return []conditions.Condition{conditions.NewListenerUnsupportedValue(valErr.Error())} - } - return nil - } -} - -func createNoValidGatewayClassValidator(gc *GatewayClass) listenerValidator { - return func(listener v1beta1.Listener) []conditions.Condition { - if gc == nil { - return []conditions.Condition{conditions.NewListenerNoValidGatewayClass("GatewayClass doesn't exist")} - } - - if !gc.Valid { - return []conditions.Condition{conditions.NewListenerNoValidGatewayClass("GatewayClass is invalid")} - } - - return nil - } -} - func validateHTTPListener(listener v1beta1.Listener) []conditions.Condition { if listener.Port != 80 { path := field.NewPath("port") diff --git a/internal/state/graph/gateway_test.go b/internal/state/graph/gateway_test.go index 00e2a314fb..20ad44e8d9 100644 --- a/internal/state/graph/gateway_test.go +++ b/internal/state/graph/gateway_test.go @@ -304,6 +304,7 @@ func TestBuildGateway(t *testing.T) { AcceptedHostnames: map[string]struct{}{}, }, }, + Valid: true, }, name: "valid http listener", }, @@ -321,6 +322,7 @@ func TestBuildGateway(t *testing.T) { SecretPath: secretPath, }, }, + Valid: true, }, name: "valid https listener", }, @@ -340,6 +342,7 @@ func TestBuildGateway(t *testing.T) { }, }, }, + Valid: true, }, name: "invalid listener protocol", }, @@ -359,6 +362,7 @@ func TestBuildGateway(t *testing.T) { }, }, }, + Valid: true, }, name: "invalid http listener", }, @@ -378,6 +382,7 @@ func TestBuildGateway(t *testing.T) { }, }, }, + Valid: true, }, name: "invalid https listener", }, @@ -402,6 +407,7 @@ func TestBuildGateway(t *testing.T) { }, }, }, + Valid: true, }, name: "invalid hostnames", }, @@ -421,6 +427,7 @@ func TestBuildGateway(t *testing.T) { ), }, }, + Valid: true, }, name: "invalid https listener (secret does not exist)", }, @@ -459,6 +466,7 @@ func TestBuildGateway(t *testing.T) { SecretPath: secretPath, }, }, + Valid: true, }, name: "multiple valid http/https listeners", }, @@ -501,6 +509,7 @@ func TestBuildGateway(t *testing.T) { SecretPath: "/etc/nginx/secrets/test_secret", }, }, + Valid: true, }, name: "collisions", }, @@ -514,26 +523,9 @@ func TestBuildGateway(t *testing.T) { gatewayClass: validGC, expected: &Gateway{ Source: getLastCreatedGetaway(), - Listeners: map[string]*Listener{ - "listener-80-1": { - Source: listener801, - Valid: false, - Conditions: []conditions.Condition{ - conditions.NewListenerUnsupportedValue( - "spec.addresses: Forbidden: addresses are not supported", - ), - }, - }, - "listener-443-1": { - Source: listener4431, - Valid: false, - SecretPath: "", - Conditions: []conditions.Condition{ - conditions.NewListenerUnsupportedValue( - "spec.addresses: Forbidden: addresses are not supported", - ), - }, - }, + Valid: false, + Conditions: []conditions.Condition{ + conditions.NewGatewayUnsupportedValue("spec.addresses: Forbidden: addresses are not supported"), }, }, name: "gateway addresses are not supported", @@ -544,35 +536,25 @@ func TestBuildGateway(t *testing.T) { name: "nil gateway", }, { - gateway: createGateway(gatewayCfg{listeners: []v1beta1.Listener{listener801}}), + gateway: createGateway(gatewayCfg{listeners: []v1beta1.Listener{listener801, listener802}}), gatewayClass: invalidGC, expected: &Gateway{ Source: getLastCreatedGetaway(), - Listeners: map[string]*Listener{ - "listener-80-1": { - Source: listener801, - Valid: false, - Conditions: []conditions.Condition{ - conditions.NewListenerNoValidGatewayClass("GatewayClass is invalid"), - }, - }, + Valid: false, + Conditions: []conditions.Condition{ + conditions.NewGatewayInvalid("GatewayClass is invalid"), }, }, name: "invalid gatewayclass", }, { - gateway: createGateway(gatewayCfg{listeners: []v1beta1.Listener{listener801}}), + gateway: createGateway(gatewayCfg{listeners: []v1beta1.Listener{listener801, listener802}}), gatewayClass: nil, expected: &Gateway{ Source: getLastCreatedGetaway(), - Listeners: map[string]*Listener{ - "listener-80-1": { - Source: listener801, - Valid: false, - Conditions: []conditions.Condition{ - conditions.NewListenerNoValidGatewayClass("GatewayClass doesn't exist"), - }, - }, + Valid: false, + Conditions: []conditions.Condition{ + conditions.NewGatewayInvalid("GatewayClass doesn't exist"), }, }, name: "nil gatewayclass", diff --git a/internal/state/graph/graph_test.go b/internal/state/graph/graph_test.go index ed51f38c08..7b292a88b4 100644 --- a/internal/state/graph/graph_test.go +++ b/internal/state/graph/graph_test.go @@ -234,6 +234,7 @@ func TestBuildGraph(t *testing.T) { SecretPath: secretPath, }, }, + Valid: true, }, IgnoredGateways: map[types.NamespacedName]*v1beta1.Gateway{ {Namespace: "test", Name: "gateway-2"}: gw2, diff --git a/internal/state/graph/httproute.go b/internal/state/graph/httproute.go index dd10882996..02990edb02 100644 --- a/internal/state/graph/httproute.go +++ b/internal/state/graph/httproute.go @@ -281,57 +281,100 @@ func bindRouteToListeners(r *Route, gw *Gateway) { continue } - // Case 3 - winning Gateway + // Case 3: Attachment is not possible because Gateway is invalid - // Find a listener + if !gw.Valid { + attachment.FailedCondition = conditions.NewRouteInvalidGateway() + continue + } - // FIXME(pleshakov) - // For now, let's do simple matching. - // However, we need to also support wildcard matching. + // Case 4 - winning Gateway - bind := func(l *Listener) (valid bool) { - if !l.Valid { - return false - } + // Try to attach Route to all matching listeners + cond, attached := tryToAttachRouteToListeners(routeRef, r, gw.Listeners) + if !attached { + attachment.FailedCondition = cond + continue + } - hostnames := findAcceptedHostnames(l.Source.Hostname, r.Source.Spec.Hostnames) - if len(hostnames) == 0 { - return true // listener is valid, but return without attaching due to no matching hostnames - } + attachment.Attached = true + } +} - attachment.Attached = true - for _, h := range hostnames { - l.AcceptedHostnames[h] = struct{}{} - } - l.Routes[client.ObjectKeyFromObject(r.Source)] = r +// tryToAttachRouteToListeners tries to attach the route to the listeners that match the parentRef and the hostnames. +// If it succeeds in attaching at least one listener it will return true and the condition will be empty. +// If it fails to attach the route, it will return false and the failure condition. +// FIXME(pleshakov) +// For now, let's do simple matching. +// However, we need to also support wildcard matching. +func tryToAttachRouteToListeners( + ref v1beta1.ParentReference, + route *Route, + listeners map[string]*Listener, +) (conditions.Condition, bool) { + validListeners, listenerExists := findValidListeners(getSectionName(ref.SectionName), listeners) - return true + if !listenerExists { + // FIXME(pleshakov): Add a proper condition once it is available in the Gateway API. + // https://github.com/nginxinc/nginx-kubernetes-gateway/issues/306 + return conditions.NewTODO("listener is not found"), false + } + + if len(validListeners) == 0 { + return conditions.NewRouteInvalidListener(), false + } + + bind := func(l *Listener) (attached bool) { + hostnames := findAcceptedHostnames(l.Source.Hostname, route.Source.Spec.Hostnames) + if len(hostnames) == 0 { + return false } - var validListener bool - if getSectionName(routeRef.SectionName) == "" { - for _, l := range gw.Listeners { - validListener = bind(l) || validListener - } - } else { - l, exists := gw.Listeners[string(*routeRef.SectionName)] - if !exists { - // FIXME(pleshakov): Add a proper condition once it is available in the Gateway API. - // https://github.com/nginxinc/nginx-kubernetes-gateway/issues/306 - attachment.FailedCondition = conditions.NewTODO("listener is not found") - continue - } - - validListener = bind(l) + for _, h := range hostnames { + l.AcceptedHostnames[h] = struct{}{} } - if !validListener { - attachment.FailedCondition = conditions.NewRouteInvalidListener() - continue + l.Routes[client.ObjectKeyFromObject(route.Source)] = route + + return true + } + + attached := false + for _, l := range validListeners { + attached = attached || bind(l) + } + + if !attached { + return conditions.NewRouteNoMatchingListenerHostname(), false + } + + return conditions.Condition{}, true +} + +// findValidListeners returns a list of valid listeners and whether the listener exists for a non-empty sectionName. +func findValidListeners(sectionName string, listeners map[string]*Listener) ([]*Listener, bool) { + if sectionName != "" { + l, exists := listeners[sectionName] + if !exists { + return nil, false } - if !attachment.Attached { - attachment.FailedCondition = conditions.NewRouteNoMatchingListenerHostname() + + if l.Valid { + return []*Listener{l}, true } + + return nil, true } + + validListeners := make([]*Listener, 0, len(listeners)) + for _, l := range listeners { + if !l.Valid { + continue + } + + validListeners = append(validListeners, l) + } + + return validListeners, true } func findAcceptedHostnames(listenerHostname *v1beta1.Hostname, routeHostnames []v1beta1.Hostname) []string { diff --git a/internal/state/graph/httproute_test.go b/internal/state/graph/httproute_test.go index 48300041e7..80f96f4cef 100644 --- a/internal/state/graph/httproute_test.go +++ b/internal/state/graph/httproute_test.go @@ -688,6 +688,7 @@ func TestBindRouteToListeners(t *testing.T) { route: createNormalRoute(), gateway: &Gateway{ Source: gw, + Valid: true, Listeners: map[string]*Listener{ "listener-80-1": createListener(), }, @@ -717,6 +718,7 @@ func TestBindRouteToListeners(t *testing.T) { route: routeWithMissingSectionName, gateway: &Gateway{ Source: gw, + Valid: true, Listeners: map[string]*Listener{ "listener-80-1": createListener(), }, @@ -746,6 +748,7 @@ func TestBindRouteToListeners(t *testing.T) { route: routeWithEmptySectionName, gateway: &Gateway{ Source: gw, + Valid: true, Listeners: map[string]*Listener{ "listener-80-1": createListener(), }, @@ -775,6 +778,7 @@ func TestBindRouteToListeners(t *testing.T) { route: routeWithEmptySectionName, gateway: &Gateway{ Source: gw, + Valid: true, Listeners: map[string]*Listener{ "listener-80-1": notValidListener, }, @@ -798,6 +802,7 @@ func TestBindRouteToListeners(t *testing.T) { route: routeWithPort, gateway: &Gateway{ Source: gw, + Valid: true, Listeners: map[string]*Listener{ "listener-80-1": createListener(), }, @@ -823,6 +828,7 @@ func TestBindRouteToListeners(t *testing.T) { route: routeWithNonExistingListener, gateway: &Gateway{ Source: gw, + Valid: true, Listeners: map[string]*Listener{ "listener-80-1": createListener(), }, @@ -846,6 +852,7 @@ func TestBindRouteToListeners(t *testing.T) { route: createNormalRoute(), gateway: &Gateway{ Source: gw, + Valid: true, Listeners: map[string]*Listener{ "listener-80-1": notValidListener, }, @@ -869,6 +876,7 @@ func TestBindRouteToListeners(t *testing.T) { route: createNormalRoute(), gateway: &Gateway{ Source: gw, + Valid: true, Listeners: map[string]*Listener{ "listener-80-1": nonMatchingHostnameListener, }, @@ -892,6 +900,7 @@ func TestBindRouteToListeners(t *testing.T) { route: routeWithIgnoredGateway, gateway: &Gateway{ Source: gw, + Valid: true, Listeners: map[string]*Listener{ "listener-80-1": createListener(), }, @@ -915,6 +924,7 @@ func TestBindRouteToListeners(t *testing.T) { route: notValidRoute, gateway: &Gateway{ Source: gw, + Valid: true, Listeners: map[string]*Listener{ "listener-80-1": createListener(), }, @@ -931,6 +941,30 @@ func TestBindRouteToListeners(t *testing.T) { }, name: "route isn't valid", }, + { + route: createNormalRoute(), + gateway: &Gateway{ + Source: gw, + Valid: false, + Listeners: map[string]*Listener{ + "listener-80-1": createListener(), + }, + }, + expectedSectionNameRefs: []ParentRef{ + { + Idx: 0, + Gateway: client.ObjectKeyFromObject(gw), + Attachment: &ParentRefAttachmentStatus{ + Attached: false, + FailedCondition: conditions.NewRouteInvalidGateway(), + }, + }, + }, + expectedGatewayListeners: map[string]*Listener{ + "listener-80-1": createListener(), + }, + name: "invalid gateway", + }, } for _, test := range tests { diff --git a/internal/state/statuses.go b/internal/state/statuses.go index 50c2a7be1a..e1f9b78d88 100644 --- a/internal/state/statuses.go +++ b/internal/state/statuses.go @@ -15,32 +15,26 @@ type ListenerStatuses map[string]ListenerStatus // HTTPRouteStatuses holds the statuses of HTTPRoutes where the key is the namespaced name of an HTTPRoute. type HTTPRouteStatuses map[types.NamespacedName]HTTPRouteStatus +// GatewayStatuses holds the statuses of Gateways where the key is the namespaced name of a Gateway. +type GatewayStatuses map[types.NamespacedName]GatewayStatus + // Statuses holds the status-related information about Gateway API resources. type Statuses struct { - GatewayClassStatus *GatewayClassStatus - GatewayStatus *GatewayStatus - IgnoredGatewayStatuses IgnoredGatewayStatuses - HTTPRouteStatuses HTTPRouteStatuses + GatewayClassStatus *GatewayClassStatus + GatewayStatuses GatewayStatuses + HTTPRouteStatuses HTTPRouteStatuses } // GatewayStatus holds the status of the winning Gateway resource. type GatewayStatus struct { // ListenerStatuses holds the statuses of listeners defined on the Gateway. ListenerStatuses ListenerStatuses - // NsName is the namespaced name of the winning Gateway resource. - NsName types.NamespacedName + // Conditions is the list of conditions for this Gateway. + Conditions []conditions.Condition // ObservedGeneration is the generation of the resource that was processed. ObservedGeneration int64 } -// IgnoredGatewayStatuses holds the statuses of the ignored Gateway resources. -type IgnoredGatewayStatuses map[types.NamespacedName]IgnoredGatewayStatus - -// IgnoredGatewayStatus holds the status of an ignored Gateway resource. -type IgnoredGatewayStatus struct { - ObservedGeneration int64 -} - // ListenerStatus holds the status-related information about a listener in the Gateway resource. type ListenerStatus struct { // Conditions is the list of conditions for this listener. @@ -76,8 +70,7 @@ type GatewayClassStatus struct { // buildStatuses builds statuses from a Graph. func buildStatuses(graph *graph.Graph) Statuses { statuses := Statuses{ - HTTPRouteStatuses: make(map[types.NamespacedName]HTTPRouteStatus), - IgnoredGatewayStatuses: make(map[types.NamespacedName]IgnoredGatewayStatus), + HTTPRouteStatuses: make(HTTPRouteStatuses), } if graph.GatewayClass != nil { @@ -96,35 +89,7 @@ func buildStatuses(graph *graph.Graph) Statuses { } } - if graph.Gateway != nil { - listenerStatuses := make(map[string]ListenerStatus) - - defaultConds := conditions.NewDefaultListenerConditions() - - for name, l := range graph.Gateway.Listeners { - conds := make([]conditions.Condition, 0, len(l.Conditions)+len(defaultConds)) - - // We add default conds first, so that any additional conditions will override them, which is - // ensured by DeduplicateConditions. - conds = append(conds, defaultConds...) - conds = append(conds, l.Conditions...) - - listenerStatuses[name] = ListenerStatus{ - AttachedRoutes: int32(len(l.Routes)), - Conditions: conditions.DeduplicateConditions(conds), - } - } - - statuses.GatewayStatus = &GatewayStatus{ - NsName: client.ObjectKeyFromObject(graph.Gateway.Source), - ListenerStatuses: listenerStatuses, - ObservedGeneration: graph.Gateway.Source.Generation, - } - } - - for nsname, gw := range graph.IgnoredGateways { - statuses.IgnoredGatewayStatuses[nsname] = IgnoredGatewayStatus{ObservedGeneration: gw.Generation} - } + statuses.GatewayStatuses = buildGatewayStatuses(graph.Gateway, graph.IgnoredGateways) for nsname, r := range graph.Routes { parentStatuses := make([]ParentStatus, 0, len(r.ParentRefs)) @@ -163,3 +128,64 @@ func buildStatuses(graph *graph.Graph) Statuses { return statuses } + +func buildGatewayStatuses( + gateway *graph.Gateway, + ignoredGateways map[types.NamespacedName]*v1beta1.Gateway, +) GatewayStatuses { + statuses := make(GatewayStatuses) + + if gateway != nil { + statuses[client.ObjectKeyFromObject(gateway.Source)] = buildGatewayStatus(gateway) + } + + for nsname, gw := range ignoredGateways { + statuses[nsname] = GatewayStatus{ + Conditions: []conditions.Condition{conditions.NewGatewayConflict()}, + ObservedGeneration: gw.Generation, + } + } + + return statuses +} + +func buildGatewayStatus(gateway *graph.Gateway) GatewayStatus { + if !gateway.Valid { + return GatewayStatus{ + Conditions: conditions.DeduplicateConditions(gateway.Conditions), + ObservedGeneration: gateway.Source.Generation, + } + } + + listenerStatuses := make(map[string]ListenerStatus) + + validListenerCount := 0 + for name, l := range gateway.Listeners { + var conds []conditions.Condition + + if l.Valid { + conds = conditions.NewDefaultListenerConditions() + validListenerCount++ + } else { + conds = l.Conditions + } + + listenerStatuses[name] = ListenerStatus{ + AttachedRoutes: int32(len(l.Routes)), + Conditions: conditions.DeduplicateConditions(conds), + } + } + + gwConds := conditions.NewDefaultGatewayConditions() + if validListenerCount == 0 { + gwConds = append(gwConds, conditions.NewGatewayNotAcceptedListenersNotValid()) + } else if validListenerCount < len(gateway.Listeners) { + gwConds = append(gwConds, conditions.NewGatewayAcceptedListenersNotValid()) + } + + return GatewayStatus{ + Conditions: conditions.DeduplicateConditions(gwConds), + ListenerStatuses: listenerStatuses, + ObservedGeneration: gateway.Source.Generation, + } +} diff --git a/internal/state/statuses_test.go b/internal/state/statuses_test.go index ad8e52fb36..aff1dd70eb 100644 --- a/internal/state/statuses_test.go +++ b/internal/state/statuses_test.go @@ -14,17 +14,8 @@ import ( "github.com/nginxinc/nginx-kubernetes-gateway/internal/state/graph" ) -func TestBuildStatuses(t *testing.T) { - invalidRouteCondition := conditions.Condition{ - Type: "TestInvalidRoute", - Status: metav1.ConditionTrue, - } - invalidAttachmentCondition := conditions.Condition{ - Type: "TestInvalidAttachment", - Status: metav1.ConditionTrue, - } - - gw := &v1beta1.Gateway{ +var ( + gw = &v1beta1.Gateway{ ObjectMeta: metav1.ObjectMeta{ Namespace: "test", Name: "gateway", @@ -32,13 +23,24 @@ func TestBuildStatuses(t *testing.T) { }, } - ignoredGw := &v1beta1.Gateway{ + ignoredGw = &v1beta1.Gateway{ ObjectMeta: metav1.ObjectMeta{ Namespace: "test", Name: "ignored-gateway", Generation: 1, }, } +) + +func TestBuildStatuses(t *testing.T) { + invalidRouteCondition := conditions.Condition{ + Type: "TestInvalidRoute", + Status: metav1.ConditionTrue, + } + invalidAttachmentCondition := conditions.Condition{ + Type: "TestInvalidAttachment", + Status: metav1.ConditionTrue, + } routes := map[types.NamespacedName]*graph.Route{ {Namespace: "test", Name: "hr-valid"}: { @@ -122,6 +124,7 @@ func TestBuildStatuses(t *testing.T) { }, }, }, + Valid: true, }, IgnoredGateways: map[types.NamespacedName]*v1beta1.Gateway{ client.ObjectKeyFromObject(ignoredGw): ignoredGw, @@ -134,20 +137,23 @@ func TestBuildStatuses(t *testing.T) { ObservedGeneration: 1, Conditions: conditions.NewDefaultGatewayClassConditions(), }, - GatewayStatus: &GatewayStatus{ - NsName: types.NamespacedName{Namespace: "test", Name: "gateway"}, - ListenerStatuses: map[string]ListenerStatus{ - "listener-80-1": { - AttachedRoutes: 1, - Conditions: conditions.NewDefaultListenerConditions(), + GatewayStatuses: GatewayStatuses{ + {Namespace: "test", Name: "gateway"}: { + Conditions: conditions.NewDefaultGatewayConditions(), + ListenerStatuses: map[string]ListenerStatus{ + "listener-80-1": { + AttachedRoutes: 1, + Conditions: conditions.NewDefaultListenerConditions(), + }, }, + ObservedGeneration: 2, + }, + {Namespace: "test", Name: "ignored-gateway"}: { + Conditions: []conditions.Condition{conditions.NewGatewayConflict()}, + ObservedGeneration: 1, }, - ObservedGeneration: 2, - }, - IgnoredGatewayStatuses: map[types.NamespacedName]IgnoredGatewayStatus{ - {Namespace: "test", Name: "ignored-gateway"}: {ObservedGeneration: 1}, }, - HTTPRouteStatuses: map[types.NamespacedName]HTTPRouteStatus{ + HTTPRouteStatuses: HTTPRouteStatuses{ {Namespace: "test", Name: "hr-valid"}: { ObservedGeneration: 3, ParentStatuses: []ParentStatus{ @@ -187,3 +193,179 @@ func TestBuildStatuses(t *testing.T) { result := buildStatuses(graph) g.Expect(helpers.Diff(expected, result)).To(BeEmpty()) } + +func TestBuildGatewayStatuses(t *testing.T) { + tests := []struct { + gateway *graph.Gateway + ignoredGateways map[types.NamespacedName]*v1beta1.Gateway + expected GatewayStatuses + name string + }{ + { + name: "nil gateway and no ignored gateways", + expected: GatewayStatuses{}, + }, + { + name: "nil gateway and ignored gateways", + ignoredGateways: map[types.NamespacedName]*v1beta1.Gateway{ + {Namespace: "test", Name: "ignored-1"}: { + ObjectMeta: metav1.ObjectMeta{ + Generation: 1, + }, + }, + {Namespace: "test", Name: "ignored-2"}: { + ObjectMeta: metav1.ObjectMeta{ + Generation: 2, + }, + }, + }, + expected: GatewayStatuses{ + {Namespace: "test", Name: "ignored-1"}: { + Conditions: []conditions.Condition{conditions.NewGatewayConflict()}, + ObservedGeneration: 1, + }, + {Namespace: "test", Name: "ignored-2"}: { + Conditions: []conditions.Condition{conditions.NewGatewayConflict()}, + ObservedGeneration: 2, + }, + }, + }, + { + name: "valid gateway; all valid listeners", + gateway: &graph.Gateway{ + Source: gw, + Listeners: map[string]*graph.Listener{ + "listener-valid-1": { + Valid: true, + Routes: map[types.NamespacedName]*graph.Route{ + {Namespace: "test", Name: "hr-1"}: {}, + }, + }, + "listener-valid-2": { + Valid: true, + Routes: map[types.NamespacedName]*graph.Route{ + {Namespace: "test", Name: "hr-1"}: {}, + }, + }, + }, + Valid: true, + }, + expected: GatewayStatuses{ + {Namespace: "test", Name: "gateway"}: { + Conditions: conditions.NewDefaultGatewayConditions(), + ListenerStatuses: map[string]ListenerStatus{ + "listener-valid-1": { + AttachedRoutes: 1, + Conditions: conditions.NewDefaultListenerConditions(), + }, + "listener-valid-2": { + AttachedRoutes: 1, + Conditions: conditions.NewDefaultListenerConditions(), + }, + }, + ObservedGeneration: 2, + }, + }, + }, + { + name: "valid gateway; some valid listeners", + gateway: &graph.Gateway{ + Source: gw, + Listeners: map[string]*graph.Listener{ + "listener-valid": { + Valid: true, + Routes: map[types.NamespacedName]*graph.Route{ + {Namespace: "test", Name: "hr-1"}: {}, + }, + }, + "listener-invalid": { + Valid: false, + Conditions: []conditions.Condition{ + conditions.NewListenerUnsupportedValue("unsupported value"), + }, + }, + }, + Valid: true, + }, + expected: GatewayStatuses{ + {Namespace: "test", Name: "gateway"}: { + Conditions: []conditions.Condition{conditions.NewGatewayAcceptedListenersNotValid()}, + ListenerStatuses: map[string]ListenerStatus{ + "listener-valid": { + AttachedRoutes: 1, + Conditions: conditions.NewDefaultListenerConditions(), + }, + "listener-invalid": { + Conditions: []conditions.Condition{ + conditions.NewListenerUnsupportedValue("unsupported value"), + }, + }, + }, + ObservedGeneration: 2, + }, + }, + }, + { + name: "valid gateway; no valid listeners", + gateway: &graph.Gateway{ + Source: gw, + Listeners: map[string]*graph.Listener{ + "listener-invalid-1": { + Valid: false, + Conditions: []conditions.Condition{ + conditions.NewListenerPortUnavailable("port unavailable"), + }, + }, + "listener-invalid-2": { + Valid: false, + Conditions: []conditions.Condition{ + conditions.NewListenerUnsupportedValue("unsupported value"), + }, + }, + }, + Valid: true, + }, + expected: GatewayStatuses{ + {Namespace: "test", Name: "gateway"}: { + Conditions: []conditions.Condition{conditions.NewGatewayNotAcceptedListenersNotValid()}, + ListenerStatuses: map[string]ListenerStatus{ + "listener-invalid-1": { + Conditions: []conditions.Condition{ + conditions.NewListenerPortUnavailable("port unavailable"), + }, + }, + "listener-invalid-2": { + Conditions: []conditions.Condition{ + conditions.NewListenerUnsupportedValue("unsupported value"), + }, + }, + }, + ObservedGeneration: 2, + }, + }, + }, + { + name: "invalid gateway", + gateway: &graph.Gateway{ + Source: gw, + Valid: false, + Conditions: []conditions.Condition{conditions.NewGatewayInvalid("no gateway class")}, + }, + expected: GatewayStatuses{ + {Namespace: "test", Name: "gateway"}: { + Conditions: []conditions.Condition{conditions.NewGatewayInvalid("no gateway class")}, + ObservedGeneration: 2, + }, + }, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + g := NewGomegaWithT(t) + + result := buildGatewayStatuses(test.gateway, test.ignoredGateways) + g.Expect(helpers.Diff(test.expected, result)).To(BeEmpty()) + }) + } +} diff --git a/internal/status/conditions_test.go b/internal/status/conditions_test.go index c38216b33c..8b52896ffa 100644 --- a/internal/status/conditions_test.go +++ b/internal/status/conditions_test.go @@ -11,16 +11,16 @@ import ( "github.com/nginxinc/nginx-kubernetes-gateway/internal/state/conditions" ) -func CreateTestConditions() []conditions.Condition { +func CreateTestConditions(condType string) []conditions.Condition { return []conditions.Condition{ { - Type: "Test", + Type: condType, Status: metav1.ConditionTrue, Reason: "TestReason1", Message: "Test message1", }, { - Type: "Test", + Type: condType, Status: metav1.ConditionFalse, Reason: "TestReason2", Message: "Test message2", @@ -28,10 +28,14 @@ func CreateTestConditions() []conditions.Condition { } } -func CreateExpectedAPIConditions(observedGeneration int64, transitionTime metav1.Time) []metav1.Condition { +func CreateExpectedAPIConditions( + condType string, + observedGeneration int64, + transitionTime metav1.Time, +) []metav1.Condition { return []metav1.Condition{ { - Type: "Test", + Type: condType, Status: metav1.ConditionTrue, ObservedGeneration: observedGeneration, LastTransitionTime: transitionTime, @@ -39,7 +43,7 @@ func CreateExpectedAPIConditions(observedGeneration int64, transitionTime metav1 Message: "Test message1", }, { - Type: "Test", + Type: condType, Status: metav1.ConditionFalse, ObservedGeneration: observedGeneration, LastTransitionTime: transitionTime, @@ -55,8 +59,8 @@ func TestConvertRouteConditions(t *testing.T) { var generation int64 = 1 transitionTime := metav1.NewTime(time.Now()) - expected := CreateExpectedAPIConditions(generation, transitionTime) + expected := CreateExpectedAPIConditions("Test", generation, transitionTime) - result := convertConditions(CreateTestConditions(), generation, transitionTime) + result := convertConditions(CreateTestConditions("Test"), generation, transitionTime) g.Expect(helpers.Diff(expected, result)).To(BeEmpty()) } diff --git a/internal/status/gateway.go b/internal/status/gateway.go index bd04e623b3..349785de01 100644 --- a/internal/status/gateway.go +++ b/internal/status/gateway.go @@ -9,16 +9,6 @@ import ( "github.com/nginxinc/nginx-kubernetes-gateway/internal/state" ) -const ( - // GetawayReasonGatewayConflict indicates there are multiple Gateway resources for NGINX Gateway to choose from, - // and NGINX Gateway ignored the resource in question and picked another Gateway as the winner. - // NGINX Gateway will use this reason with GatewayConditionReady (false). - GetawayReasonGatewayConflict v1beta1.GatewayConditionReason = "GatewayConflict" - - // GatewayMessageGatewayConflict is message that describes GetawayReasonGatewayConflict. - GatewayMessageGatewayConflict = "The resource is ignored due to a conflicting Gateway resource" -) - // prepareGatewayStatus prepares the status for a Gateway resource. // FIXME(pleshakov): Be compliant with in the Gateway API. // Currently, we only support simple valid/invalid status per each listener. @@ -61,25 +51,6 @@ func prepareGatewayStatus( return v1beta1.GatewayStatus{ Listeners: listenerStatuses, Addresses: []v1beta1.GatewayAddress{gwPodIP}, - Conditions: nil, // FIXME(pleshakov) Create conditions for the Gateway resource. - } -} - -// prepareIgnoredGatewayStatus prepares the status for an ignored Gateway resource. -// TODO: is it reasonable to not set the listener statuses? -// FIXME(pleshakov): Simplify the code, so that we don't need a separate prepareIgnoredGatewayStatus -// and state.IgnoredGatewayStatus. -func prepareIgnoredGatewayStatus(status state.IgnoredGatewayStatus, transitionTime metav1.Time) v1beta1.GatewayStatus { - return v1beta1.GatewayStatus{ - Conditions: []metav1.Condition{ - { - Type: string(v1beta1.GatewayConditionReady), - Status: metav1.ConditionFalse, - ObservedGeneration: status.ObservedGeneration, - LastTransitionTime: transitionTime, - Reason: string(GetawayReasonGatewayConflict), - Message: GatewayMessageGatewayConflict, - }, - }, + Conditions: convertConditions(gatewayStatus.Conditions, gatewayStatus.ObservedGeneration, transitionTime), } } diff --git a/internal/status/gateway_test.go b/internal/status/gateway_test.go index 42cc85337f..5be4f53f5e 100644 --- a/internal/status/gateway_test.go +++ b/internal/status/gateway_test.go @@ -4,7 +4,6 @@ import ( "testing" "time" - "github.com/google/go-cmp/cmp" . "github.com/onsi/gomega" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "sigs.k8s.io/gateway-api/apis/v1beta1" @@ -21,10 +20,11 @@ func TestPrepareGatewayStatus(t *testing.T) { } status := state.GatewayStatus{ + Conditions: CreateTestConditions("GatewayTest"), ListenerStatuses: state.ListenerStatuses{ "listener": { AttachedRoutes: 3, - Conditions: CreateTestConditions(), + Conditions: CreateTestConditions("ListenerTest"), }, }, ObservedGeneration: 1, @@ -33,6 +33,7 @@ func TestPrepareGatewayStatus(t *testing.T) { transitionTime := metav1.NewTime(time.Now()) expected := v1beta1.GatewayStatus{ + Conditions: CreateExpectedAPIConditions("GatewayTest", 1, transitionTime), Listeners: []v1beta1.ListenerStatus{ { Name: "listener", @@ -42,7 +43,7 @@ func TestPrepareGatewayStatus(t *testing.T) { }, }, AttachedRoutes: 3, - Conditions: CreateExpectedAPIConditions(1, transitionTime), + Conditions: CreateExpectedAPIConditions("ListenerTest", 1, transitionTime), }, }, Addresses: []v1beta1.GatewayAddress{podIP}, @@ -53,29 +54,3 @@ func TestPrepareGatewayStatus(t *testing.T) { result := prepareGatewayStatus(status, "1.2.3.4", transitionTime) g.Expect(helpers.Diff(expected, result)).To(BeEmpty()) } - -func TestPrepareIgnoredGatewayStatus(t *testing.T) { - status := state.IgnoredGatewayStatus{ - ObservedGeneration: 1, - } - - transitionTime := metav1.NewTime(time.Now()) - - expected := v1beta1.GatewayStatus{ - Conditions: []metav1.Condition{ - { - Type: string(v1beta1.GatewayConditionReady), - Status: metav1.ConditionFalse, - ObservedGeneration: status.ObservedGeneration, - LastTransitionTime: transitionTime, - Reason: string(GetawayReasonGatewayConflict), - Message: GatewayMessageGatewayConflict, - }, - }, - } - - result := prepareIgnoredGatewayStatus(status, transitionTime) - if diff := cmp.Diff(expected, result); diff != "" { - t.Errorf("prepareIgnoredGatewayStatus() mismatch (-want +got):\n%s", diff) - } -} diff --git a/internal/status/gatewayclass_test.go b/internal/status/gatewayclass_test.go index 5987727e2a..d208f22351 100644 --- a/internal/status/gatewayclass_test.go +++ b/internal/status/gatewayclass_test.go @@ -17,10 +17,10 @@ func TestPrepareGatewayClassStatus(t *testing.T) { status := state.GatewayClassStatus{ ObservedGeneration: 1, - Conditions: CreateTestConditions(), + Conditions: CreateTestConditions("Test"), } expected := v1beta1.GatewayClassStatus{ - Conditions: CreateExpectedAPIConditions(1, transitionTime), + Conditions: CreateExpectedAPIConditions("Test", 1, transitionTime), } g := NewGomegaWithT(t) diff --git a/internal/status/httproute_test.go b/internal/status/httproute_test.go index 3804eddd6c..a56451c87f 100644 --- a/internal/status/httproute_test.go +++ b/internal/status/httproute_test.go @@ -23,12 +23,12 @@ func TestPrepareHTTPRouteStatus(t *testing.T) { { GatewayNsName: gwNsName1, SectionName: helpers.GetPointer[v1beta1.SectionName]("http"), - Conditions: CreateTestConditions(), + Conditions: CreateTestConditions("Test"), }, { GatewayNsName: gwNsName2, SectionName: nil, - Conditions: CreateTestConditions(), + Conditions: CreateTestConditions("Test"), }, }, } @@ -46,7 +46,7 @@ func TestPrepareHTTPRouteStatus(t *testing.T) { SectionName: helpers.GetPointer[v1beta1.SectionName]("http"), }, ControllerName: v1beta1.GatewayController(gatewayCtlrName), - Conditions: CreateExpectedAPIConditions(1, transitionTime), + Conditions: CreateExpectedAPIConditions("Test", 1, transitionTime), }, { ParentRef: v1beta1.ParentReference{ @@ -55,7 +55,7 @@ func TestPrepareHTTPRouteStatus(t *testing.T) { SectionName: nil, }, ControllerName: v1beta1.GatewayController(gatewayCtlrName), - Conditions: CreateExpectedAPIConditions(1, transitionTime), + Conditions: CreateExpectedAPIConditions("Test", 1, transitionTime), }, }, }, diff --git a/internal/status/updater.go b/internal/status/updater.go index 879a1ccd41..0859bb4f3f 100644 --- a/internal/status/updater.go +++ b/internal/status/updater.go @@ -100,23 +100,10 @@ func (upd *updaterImpl) Update(ctx context.Context, statuses state.Statuses) { ) } - if statuses.GatewayStatus != nil { - upd.update(ctx, statuses.GatewayStatus.NsName, &v1beta1.Gateway{}, func(object client.Object) { - gw := object.(*v1beta1.Gateway) - gw.Status = prepareGatewayStatus(*statuses.GatewayStatus, upd.cfg.PodIP, upd.cfg.Clock.Now()) - }) - } - - for nsname, gs := range statuses.IgnoredGatewayStatuses { - select { - case <-ctx.Done(): - return - default: - } - + for nsname, gs := range statuses.GatewayStatuses { upd.update(ctx, nsname, &v1beta1.Gateway{}, func(object client.Object) { gw := object.(*v1beta1.Gateway) - gw.Status = prepareIgnoredGatewayStatus(gs, upd.cfg.Clock.Now()) + gw.Status = prepareGatewayStatus(gs, upd.cfg.PodIP, upd.cfg.Clock.Now()) }) } diff --git a/internal/status/updater_test.go b/internal/status/updater_test.go index ebd47956bd..aac7df883f 100644 --- a/internal/status/updater_test.go +++ b/internal/status/updater_test.go @@ -17,6 +17,7 @@ import ( "github.com/nginxinc/nginx-kubernetes-gateway/internal/helpers" "github.com/nginxinc/nginx-kubernetes-gateway/internal/state" + "github.com/nginxinc/nginx-kubernetes-gateway/internal/state/conditions" "github.com/nginxinc/nginx-kubernetes-gateway/internal/status" "github.com/nginxinc/nginx-kubernetes-gateway/internal/status/statusfakes" ) @@ -69,36 +70,42 @@ var _ = Describe("Updater", func() { gc *v1beta1.GatewayClass gw, ignoredGw *v1beta1.Gateway hr *v1beta1.HTTPRoute + ipAddrType = v1beta1.IPAddressType + addr = v1beta1.GatewayAddress{ + Type: &ipAddrType, + Value: "1.2.3.4", + } createStatuses = func(gens generations) state.Statuses { return state.Statuses{ GatewayClassStatus: &state.GatewayClassStatus{ ObservedGeneration: gens.gatewayClass, - Conditions: status.CreateTestConditions(), + Conditions: status.CreateTestConditions("Test"), }, - GatewayStatus: &state.GatewayStatus{ - NsName: types.NamespacedName{Namespace: "test", Name: "gateway"}, - ListenerStatuses: map[string]state.ListenerStatus{ - "http": { - AttachedRoutes: 1, - Conditions: status.CreateTestConditions(), + GatewayStatuses: state.GatewayStatuses{ + {Namespace: "test", Name: "gateway"}: { + Conditions: status.CreateTestConditions("Test"), + ListenerStatuses: map[string]state.ListenerStatus{ + "http": { + AttachedRoutes: 1, + Conditions: status.CreateTestConditions("Test"), + }, }, + ObservedGeneration: gens.gateways, }, - ObservedGeneration: gens.gateways, - }, - IgnoredGatewayStatuses: map[types.NamespacedName]state.IgnoredGatewayStatus{ {Namespace: "test", Name: "ignored-gateway"}: { - ObservedGeneration: gens.gateways, + Conditions: []conditions.Condition{conditions.NewGatewayConflict()}, + ObservedGeneration: 1, }, }, - HTTPRouteStatuses: map[types.NamespacedName]state.HTTPRouteStatus{ + HTTPRouteStatuses: state.HTTPRouteStatuses{ {Namespace: "test", Name: "route1"}: { ObservedGeneration: 5, ParentStatuses: []state.ParentStatus{ { GatewayNsName: types.NamespacedName{Namespace: "test", Name: "gateway"}, SectionName: helpers.GetPointer[v1beta1.SectionName]("http"), - Conditions: status.CreateTestConditions(), + Conditions: status.CreateTestConditions("Test"), }, }, }, @@ -116,18 +123,12 @@ var _ = Describe("Updater", func() { APIVersion: "gateway.networking.k8s.io/v1beta1", }, Status: v1beta1.GatewayClassStatus{ - Conditions: status.CreateExpectedAPIConditions(generation, fakeClockTime), + Conditions: status.CreateExpectedAPIConditions("Test", generation, fakeClockTime), }, } } createExpectedGwWithGeneration = func(generation int64) *v1beta1.Gateway { - ipAddrType := v1beta1.IPAddressType - addr := v1beta1.GatewayAddress{ - Type: &ipAddrType, - Value: "1.2.3.4", - } - return &v1beta1.Gateway{ ObjectMeta: metav1.ObjectMeta{ Namespace: "test", @@ -138,6 +139,7 @@ var _ = Describe("Updater", func() { APIVersion: "gateway.networking.k8s.io/v1beta1", }, Status: v1beta1.GatewayStatus{ + Conditions: status.CreateExpectedAPIConditions("Test", generation, fakeClockTime), Listeners: []gatewayv1beta1.ListenerStatus{ { Name: "http", @@ -147,7 +149,7 @@ var _ = Describe("Updater", func() { }, }, AttachedRoutes: 1, - Conditions: status.CreateExpectedAPIConditions(generation, fakeClockTime), + Conditions: status.CreateExpectedAPIConditions("Test", generation, fakeClockTime), }, }, Addresses: []v1beta1.GatewayAddress{addr}, @@ -168,14 +170,15 @@ var _ = Describe("Updater", func() { Status: v1beta1.GatewayStatus{ Conditions: []metav1.Condition{ { - Type: string(v1beta1.GatewayConditionReady), + Type: string(v1beta1.GatewayConditionAccepted), Status: metav1.ConditionFalse, ObservedGeneration: 1, LastTransitionTime: fakeClockTime, - Reason: string(status.GetawayReasonGatewayConflict), - Message: status.GatewayMessageGatewayConflict, + Reason: string(conditions.GatewayReasonGatewayConflict), + Message: conditions.GatewayMessageGatewayConflict, }, }, + Addresses: []v1beta1.GatewayAddress{addr}, }, } } @@ -200,7 +203,7 @@ var _ = Describe("Updater", func() { Name: "gateway", SectionName: (*v1beta1.SectionName)(helpers.GetStringPointer("http")), }, - Conditions: status.CreateExpectedAPIConditions(5, fakeClockTime), + Conditions: status.CreateExpectedAPIConditions("Test", 5, fakeClockTime), }, }, },