From 0c9ed9316e21cc7311388194e759b9f59a73cb0e Mon Sep 17 00:00:00 2001 From: Michael Pleshakov Date: Wed, 5 Oct 2022 13:46:14 -0700 Subject: [PATCH] Support ObservedGeneration in HTTPRoute status Previously, NGINX Kubernetes Gateway used the hard-coded value 123 for the ObservedGeneration in the Conditions reported in the status of an HTTPRoute resource. This commit ensures that the Gateway uses the actual observed Generation of an HTTPRoute resource in the Conditions. --- internal/state/change_processor_test.go | 14 ++++++++++++-- internal/state/statuses.go | 7 ++++++- internal/state/statuses_test.go | 14 ++++++++++++++ internal/status/httproute.go | 15 +++++++-------- internal/status/httproute_test.go | 5 +++-- internal/status/updater_test.go | 3 ++- 6 files changed, 44 insertions(+), 14 deletions(-) diff --git a/internal/state/change_processor_test.go b/internal/state/change_processor_test.go index 18ed41c63f..45da87975a 100644 --- a/internal/state/change_processor_test.go +++ b/internal/state/change_processor_test.go @@ -31,8 +31,9 @@ const ( func createRoute(name string, gateway string, hostname string, backendRefs ...v1beta1.HTTPBackendRef) *v1beta1.HTTPRoute { return &v1beta1.HTTPRoute{ ObjectMeta: metav1.ObjectMeta{ - Namespace: "test", - Name: name, + Namespace: "test", + Name: name, + Generation: 1, }, Spec: v1beta1.HTTPRouteSpec{ CommonRouteSpec: v1beta1.CommonRouteSpec{ @@ -247,6 +248,7 @@ var _ = Describe("ChangeProcessor", func() { IgnoredGatewayStatuses: map[types.NamespacedName]state.IgnoredGatewayStatus{}, HTTPRouteStatuses: map[types.NamespacedName]state.HTTPRouteStatus{ {Namespace: "test", Name: "hr-1"}: { + ObservedGeneration: hr1.Generation, ParentStatuses: map[string]state.ParentStatus{ "listener-80-1": {Attached: false}, "listener-443-1": {Attached: false}, @@ -331,6 +333,7 @@ var _ = Describe("ChangeProcessor", func() { IgnoredGatewayStatuses: map[types.NamespacedName]state.IgnoredGatewayStatus{}, HTTPRouteStatuses: map[types.NamespacedName]state.HTTPRouteStatus{ {Namespace: "test", Name: "hr-1"}: { + ObservedGeneration: hr1.Generation, ParentStatuses: map[string]state.ParentStatus{ "listener-80-1": {Attached: true}, "listener-443-1": {Attached: true}, @@ -424,6 +427,7 @@ var _ = Describe("ChangeProcessor", func() { IgnoredGatewayStatuses: map[types.NamespacedName]state.IgnoredGatewayStatus{}, HTTPRouteStatuses: map[types.NamespacedName]state.HTTPRouteStatus{ {Namespace: "test", Name: "hr-1"}: { + ObservedGeneration: hr1Updated.Generation, ParentStatuses: map[string]state.ParentStatus{ "listener-80-1": {Attached: true}, "listener-443-1": {Attached: true}, @@ -517,6 +521,7 @@ var _ = Describe("ChangeProcessor", func() { IgnoredGatewayStatuses: map[types.NamespacedName]state.IgnoredGatewayStatus{}, HTTPRouteStatuses: map[types.NamespacedName]state.HTTPRouteStatus{ {Namespace: "test", Name: "hr-1"}: { + ObservedGeneration: hr1Updated.Generation, ParentStatuses: map[string]state.ParentStatus{ "listener-80-1": {Attached: true}, "listener-443-1": {Attached: true}, @@ -610,6 +615,7 @@ var _ = Describe("ChangeProcessor", func() { IgnoredGatewayStatuses: map[types.NamespacedName]state.IgnoredGatewayStatus{}, HTTPRouteStatuses: map[types.NamespacedName]state.HTTPRouteStatus{ {Namespace: "test", Name: "hr-1"}: { + ObservedGeneration: hr1Updated.Generation, ParentStatuses: map[string]state.ParentStatus{ "listener-80-1": {Attached: true}, "listener-443-1": {Attached: true}, @@ -706,6 +712,7 @@ var _ = Describe("ChangeProcessor", func() { }, HTTPRouteStatuses: map[types.NamespacedName]state.HTTPRouteStatus{ {Namespace: "test", Name: "hr-1"}: { + ObservedGeneration: hr1Updated.Generation, ParentStatuses: map[string]state.ParentStatus{ "listener-80-1": {Attached: true}, "listener-443-1": {Attached: true}, @@ -792,12 +799,14 @@ var _ = Describe("ChangeProcessor", func() { }, HTTPRouteStatuses: map[types.NamespacedName]state.HTTPRouteStatus{ {Namespace: "test", Name: "hr-1"}: { + ObservedGeneration: hr1Updated.Generation, ParentStatuses: map[string]state.ParentStatus{ "listener-80-1": {Attached: true}, "listener-443-1": {Attached: true}, }, }, {Namespace: "test", Name: "hr-2"}: { + ObservedGeneration: hr2.Generation, ParentStatuses: map[string]state.ParentStatus{ "listener-80-1": {Attached: false}, "listener-443-1": {Attached: false}, @@ -880,6 +889,7 @@ var _ = Describe("ChangeProcessor", func() { IgnoredGatewayStatuses: map[types.NamespacedName]state.IgnoredGatewayStatus{}, HTTPRouteStatuses: map[types.NamespacedName]state.HTTPRouteStatus{ {Namespace: "test", Name: "hr-2"}: { + ObservedGeneration: hr2.Generation, ParentStatuses: map[string]state.ParentStatus{ "listener-80-1": {Attached: true}, "listener-443-1": {Attached: true}, diff --git a/internal/state/statuses.go b/internal/state/statuses.go index 02c38bf074..f3f0eb4de7 100644 --- a/internal/state/statuses.go +++ b/internal/state/statuses.go @@ -44,7 +44,11 @@ type ListenerStatus struct { // ParentStatuses holds the statuses of parents where the key is the section name in a parentRef. type ParentStatuses map[string]ParentStatus +// HTTPRouteStatus holds the status-related information about an HTTPRoute resource. type HTTPRouteStatus struct { + // ObservedGeneration is the generation of the resource that was processed. + ObservedGeneration int64 + // ParentStatuses holds the statuses for parentRefs of the HTTPRoute. ParentStatuses ParentStatuses } @@ -116,7 +120,8 @@ func buildStatuses(graph *graph) Statuses { } statuses.HTTPRouteStatuses[nsname] = HTTPRouteStatus{ - ParentStatuses: parentStatuses, + ObservedGeneration: r.Source.Generation, + ParentStatuses: parentStatuses, } } diff --git a/internal/state/statuses_test.go b/internal/state/statuses_test.go index dcfe474483..78171611fb 100644 --- a/internal/state/statuses_test.go +++ b/internal/state/statuses_test.go @@ -21,6 +21,11 @@ func TestBuildStatuses(t *testing.T) { routes := map[types.NamespacedName]*route{ {Namespace: "test", Name: "hr-1"}: { + Source: &v1beta1.HTTPRoute{ + ObjectMeta: metav1.ObjectMeta{ + Generation: 3, + }, + }, ValidSectionNameRefs: map[string]struct{}{ "listener-80-1": {}, }, @@ -32,6 +37,11 @@ func TestBuildStatuses(t *testing.T) { routesAllRefsInvalid := map[types.NamespacedName]*route{ {Namespace: "test", Name: "hr-1"}: { + Source: &v1beta1.HTTPRoute{ + ObjectMeta: metav1.ObjectMeta{ + Generation: 4, + }, + }, InvalidSectionNameRefs: map[string]struct{}{ "listener-80-2": {}, "listener-80-1": {}, @@ -95,6 +105,7 @@ func TestBuildStatuses(t *testing.T) { }, HTTPRouteStatuses: map[types.NamespacedName]HTTPRouteStatus{ {Namespace: "test", Name: "hr-1"}: { + ObservedGeneration: 3, ParentStatuses: map[string]ParentStatus{ "listener-80-1": { Attached: true, @@ -136,6 +147,7 @@ func TestBuildStatuses(t *testing.T) { }, HTTPRouteStatuses: map[types.NamespacedName]HTTPRouteStatus{ {Namespace: "test", Name: "hr-1"}: { + ObservedGeneration: 3, ParentStatuses: map[string]ParentStatus{ "listener-80-1": { Attached: false, @@ -187,6 +199,7 @@ func TestBuildStatuses(t *testing.T) { }, HTTPRouteStatuses: map[types.NamespacedName]HTTPRouteStatus{ {Namespace: "test", Name: "hr-1"}: { + ObservedGeneration: 3, ParentStatuses: map[string]ParentStatus{ "listener-80-1": { Attached: false, @@ -221,6 +234,7 @@ func TestBuildStatuses(t *testing.T) { IgnoredGatewayStatuses: map[types.NamespacedName]IgnoredGatewayStatus{}, HTTPRouteStatuses: map[types.NamespacedName]HTTPRouteStatus{ {Namespace: "test", Name: "hr-1"}: { + ObservedGeneration: 4, ParentStatuses: map[string]ParentStatus{ "listener-80-1": { Attached: false, diff --git a/internal/status/httproute.go b/internal/status/httproute.go index 315d6ef09d..94591a10d0 100644 --- a/internal/status/httproute.go +++ b/internal/status/httproute.go @@ -33,15 +33,15 @@ func prepareHTTPRouteStatus( ps := status.ParentStatuses[name] var ( - status metav1.ConditionStatus - reason string // FIXME(pleshakov) use RouteConditionReason once we upgrade to v1beta1 + conditionStatus metav1.ConditionStatus + reason string // FIXME(pleshakov) use RouteConditionReason once we upgrade to v1beta1 ) if ps.Attached { - status = metav1.ConditionTrue + conditionStatus = metav1.ConditionTrue reason = "Accepted" // FIXME(pleshakov): use RouteReasonAccepted once we upgrade to v1beta1 } else { - status = metav1.ConditionFalse + conditionStatus = metav1.ConditionFalse reason = "NotAttached" // FIXME(pleshakov): use a more specific message from the defined constants (available in v1beta1) } @@ -56,10 +56,9 @@ func prepareHTTPRouteStatus( ControllerName: v1beta1.GatewayController(gatewayCtlrName), Conditions: []metav1.Condition{ { - Type: string(v1beta1.RouteConditionAccepted), - Status: status, - // FIXME(pleshakov) Set the observed generation to the last processed generation of the HTTPRoute resource. - ObservedGeneration: 123, + Type: string(v1beta1.RouteConditionAccepted), + Status: conditionStatus, + ObservedGeneration: status.ObservedGeneration, LastTransitionTime: transitionTime, Reason: reason, Message: "", // FIXME(pleshakov): Figure out a good message diff --git a/internal/status/httproute_test.go b/internal/status/httproute_test.go index 1ae1a0e6fb..7b2965661d 100644 --- a/internal/status/httproute_test.go +++ b/internal/status/httproute_test.go @@ -15,6 +15,7 @@ import ( func TestPrepareHTTPRouteStatus(t *testing.T) { status := state.HTTPRouteStatus{ + ObservedGeneration: 1, ParentStatuses: map[string]state.ParentStatus{ "attached": { Attached: true, @@ -44,7 +45,7 @@ func TestPrepareHTTPRouteStatus(t *testing.T) { { Type: string(v1beta1.RouteConditionAccepted), Status: metav1.ConditionTrue, - ObservedGeneration: 123, + ObservedGeneration: 1, LastTransitionTime: transitionTime, Reason: "Accepted", }, @@ -61,7 +62,7 @@ func TestPrepareHTTPRouteStatus(t *testing.T) { { Type: string(v1beta1.RouteConditionAccepted), Status: metav1.ConditionFalse, - ObservedGeneration: 123, + ObservedGeneration: 1, LastTransitionTime: transitionTime, Reason: "NotAttached", }, diff --git a/internal/status/updater_test.go b/internal/status/updater_test.go index f70c4e809c..8dba0db100 100644 --- a/internal/status/updater_test.go +++ b/internal/status/updater_test.go @@ -92,6 +92,7 @@ var _ = Describe("Updater", func() { }, HTTPRouteStatuses: map[types.NamespacedName]state.HTTPRouteStatus{ {Namespace: "test", Name: "route1"}: { + ObservedGeneration: 5, ParentStatuses: map[string]state.ParentStatus{ "http": { Attached: valid, @@ -210,7 +211,7 @@ var _ = Describe("Updater", func() { { Type: string(gatewayv1beta1.RouteConditionAccepted), Status: metav1.ConditionTrue, - ObservedGeneration: 123, + ObservedGeneration: 5, LastTransitionTime: fakeClockTime, Reason: "Accepted", },