Skip to content

Commit

Permalink
Support ObservedGeneration in HTTPRoute status (nginx#254)
Browse files Browse the repository at this point in the history 
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.
  • Loading branch information
@miledxz
miledxz committed Oct 6, 2022
1 parent f36d7e6 commit fe1528f
Show file tree
Hide file tree
Showing 6 changed files with 44 additions and 14 deletions.
14 changes: 12 additions & 2 deletions internal/state/change_processor_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand Down Expand Up @@ -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},
Expand Down Expand Up @@ -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},
Expand Down Expand Up @@ -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},
Expand Down Expand Up @@ -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},
Expand Down Expand Up @@ -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},
Expand Down Expand Up @@ -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},
Expand Down Expand Up @@ -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},
Expand Down Expand Up @@ -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},
Expand Down
7 changes: 6 additions & 1 deletion internal/state/statuses.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down Expand Up @@ -116,7 +120,8 @@ func buildStatuses(graph *graph) Statuses {
}

statuses.HTTPRouteStatuses[nsname] = HTTPRouteStatus{
ParentStatuses: parentStatuses,
ObservedGeneration: r.Source.Generation,
ParentStatuses: parentStatuses,
}
}

Expand Down
14 changes: 14 additions & 0 deletions internal/state/statuses_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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": {},
},
Expand All @@ -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": {},
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down
15 changes: 7 additions & 8 deletions internal/status/httproute.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}

Expand All @@ -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
Expand Down
5 changes: 3 additions & 2 deletions internal/status/httproute_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import (

func TestPrepareHTTPRouteStatus(t *testing.T) {
status := state.HTTPRouteStatus{
ObservedGeneration: 1,
ParentStatuses: map[string]state.ParentStatus{
"attached": {
Attached: true,
Expand Down Expand Up @@ -44,7 +45,7 @@ func TestPrepareHTTPRouteStatus(t *testing.T) {
{
Type: string(v1beta1.RouteConditionAccepted),
Status: metav1.ConditionTrue,
ObservedGeneration: 123,
ObservedGeneration: 1,
LastTransitionTime: transitionTime,
Reason: "Accepted",
},
Expand All @@ -61,7 +62,7 @@ func TestPrepareHTTPRouteStatus(t *testing.T) {
{
Type: string(v1beta1.RouteConditionAccepted),
Status: metav1.ConditionFalse,
ObservedGeneration: 123,
ObservedGeneration: 1,
LastTransitionTime: transitionTime,
Reason: "NotAttached",
},
Expand Down
3 changes: 2 additions & 1 deletion internal/status/updater_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -210,7 +211,7 @@ var _ = Describe("Updater", func() {
{
Type: string(gatewayv1beta1.RouteConditionAccepted),
Status: metav1.ConditionTrue,
ObservedGeneration: 123,
ObservedGeneration: 5,
LastTransitionTime: fakeClockTime,
Reason: "Accepted",
},
Expand Down

0 comments on commit fe1528f

Please sign in to comment.