Skip to content

Commit

Permalink
Fix Observed Generation for Gateway Status (nginx#351)
Browse files Browse the repository at this point in the history 
Use the Gateway's real generation in the Observed Generation field of the Gateway's status instead of "123".
  • Loading branch information
@miledxz
miledxz committed Jan 6, 2023
1 parent d98ce0d commit 343cab3
Show file tree
Hide file tree
Showing 6 changed files with 45 additions and 25 deletions.
30 changes: 20 additions & 10 deletions internal/state/change_processor_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -282,7 +282,8 @@ var _ = Describe("ChangeProcessor", func() {
expectedConf := state.Configuration{}
expectedStatuses := state.Statuses{
GatewayStatus: &state.GatewayStatus{
NsName: types.NamespacedName{Namespace: "test", Name: "gateway-1"},
NsName: types.NamespacedName{Namespace: "test", Name: "gateway-1"},
ObservedGeneration: gw1.Generation,
ListenerStatuses: map[string]state.ListenerStatus{
"listener-80-1": {
Valid: false,
Expand Down Expand Up @@ -386,7 +387,8 @@ var _ = Describe("ChangeProcessor", func() {
ObservedGeneration: gc.Generation,
},
GatewayStatus: &state.GatewayStatus{
NsName: types.NamespacedName{Namespace: "test", Name: "gateway-1"},
NsName: types.NamespacedName{Namespace: "test", Name: "gateway-1"},
ObservedGeneration: gw1.Generation,
ListenerStatuses: map[string]state.ListenerStatus{
"listener-80-1": {
Valid: true,
Expand Down Expand Up @@ -494,7 +496,8 @@ var _ = Describe("ChangeProcessor", func() {
ObservedGeneration: gc.Generation,
},
GatewayStatus: &state.GatewayStatus{
NsName: types.NamespacedName{Namespace: "test", Name: "gateway-1"},
NsName: types.NamespacedName{Namespace: "test", Name: "gateway-1"},
ObservedGeneration: gw1.Generation,
ListenerStatuses: map[string]state.ListenerStatus{
"listener-80-1": {
Valid: true,
Expand Down Expand Up @@ -603,7 +606,8 @@ var _ = Describe("ChangeProcessor", func() {
ObservedGeneration: gc.Generation,
},
GatewayStatus: &state.GatewayStatus{
NsName: types.NamespacedName{Namespace: "test", Name: "gateway-1"},
NsName: types.NamespacedName{Namespace: "test", Name: "gateway-1"},
ObservedGeneration: gw1Updated.Generation,
ListenerStatuses: map[string]state.ListenerStatus{
"listener-80-1": {
Valid: true,
Expand Down Expand Up @@ -711,7 +715,8 @@ var _ = Describe("ChangeProcessor", func() {
ObservedGeneration: gcUpdated.Generation,
},
GatewayStatus: &state.GatewayStatus{
NsName: types.NamespacedName{Namespace: "test", Name: "gateway-1"},
NsName: types.NamespacedName{Namespace: "test", Name: "gateway-1"},
ObservedGeneration: gw1Updated.Generation,
ListenerStatuses: map[string]state.ListenerStatus{
"listener-80-1": {
Valid: true,
Expand Down Expand Up @@ -818,7 +823,8 @@ var _ = Describe("ChangeProcessor", func() {
ObservedGeneration: gcUpdated.Generation,
},
GatewayStatus: &state.GatewayStatus{
NsName: types.NamespacedName{Namespace: "test", Name: "gateway-1"},
NsName: types.NamespacedName{Namespace: "test", Name: "gateway-1"},
ObservedGeneration: gw1Updated.Generation,
ListenerStatuses: map[string]state.ListenerStatus{
"listener-80-1": {
Valid: true,
Expand Down Expand Up @@ -918,7 +924,8 @@ var _ = Describe("ChangeProcessor", func() {
ObservedGeneration: gcUpdated.Generation,
},
GatewayStatus: &state.GatewayStatus{
NsName: types.NamespacedName{Namespace: "test", Name: "gateway-1"},
NsName: types.NamespacedName{Namespace: "test", Name: "gateway-1"},
ObservedGeneration: gw1Updated.Generation,
ListenerStatuses: map[string]state.ListenerStatus{
"listener-80-1": {
Valid: true,
Expand Down Expand Up @@ -1038,7 +1045,8 @@ var _ = Describe("ChangeProcessor", func() {
ObservedGeneration: gcUpdated.Generation,
},
GatewayStatus: &state.GatewayStatus{
NsName: types.NamespacedName{Namespace: "test", Name: "gateway-2"},
NsName: types.NamespacedName{Namespace: "test", Name: "gateway-2"},
ObservedGeneration: gw2.Generation,
ListenerStatuses: map[string]state.ListenerStatus{
"listener-80-1": {
Valid: true,
Expand Down Expand Up @@ -1101,7 +1109,8 @@ var _ = Describe("ChangeProcessor", func() {
ObservedGeneration: gcUpdated.Generation,
},
GatewayStatus: &state.GatewayStatus{
NsName: types.NamespacedName{Namespace: "test", Name: "gateway-2"},
NsName: types.NamespacedName{Namespace: "test", Name: "gateway-2"},
ObservedGeneration: gw2.Generation,
ListenerStatuses: map[string]state.ListenerStatus{
"listener-80-1": {
Valid: true,
Expand Down Expand Up @@ -1133,7 +1142,8 @@ var _ = Describe("ChangeProcessor", func() {
expectedConf := state.Configuration{}
expectedStatuses := state.Statuses{
GatewayStatus: &state.GatewayStatus{
NsName: types.NamespacedName{Namespace: "test", Name: "gateway-2"},
NsName: types.NamespacedName{Namespace: "test", Name: "gateway-2"},
ObservedGeneration: gw2.Generation,
ListenerStatuses: map[string]state.ListenerStatus{
"listener-80-1": {
Valid: false,
Expand Down
11 changes: 8 additions & 3 deletions internal/state/statuses.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,12 @@ type Statuses struct {

// 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 types.NamespacedName
// NsName is the namespaced name of the winning Gateway resource.
NsName types.NamespacedName
// ObservedGeneration is the generation of the resource that was processed.
ObservedGeneration int64
}

// IgnoredGatewayStatuses holds the statuses of the ignored Gateway resources.
Expand Down Expand Up @@ -98,8 +102,9 @@ func buildStatuses(graph *graph) Statuses {
}

statuses.GatewayStatus = &GatewayStatus{
NsName: client.ObjectKeyFromObject(graph.Gateway.Source),
ListenerStatuses: listenerStatuses,
NsName: client.ObjectKeyFromObject(graph.Gateway.Source),
ListenerStatuses: listenerStatuses,
ObservedGeneration: graph.Gateway.Source.Generation,
}
}

Expand Down
8 changes: 6 additions & 2 deletions internal/state/statuses_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -59,8 +59,9 @@ func TestBuildStatuses(t *testing.T) {

gw := &v1beta1.Gateway{
ObjectMeta: metav1.ObjectMeta{
Namespace: "test",
Name: "gateway",
Namespace: "test",
Name: "gateway",
Generation: 2,
},
}

Expand Down Expand Up @@ -107,6 +108,7 @@ func TestBuildStatuses(t *testing.T) {
AttachedRoutes: 1,
},
},
ObservedGeneration: 2,
},
IgnoredGatewayStatuses: map[types.NamespacedName]IgnoredGatewayStatus{
{Namespace: "test", Name: "ignored-gateway"}: {ObservedGeneration: 1},
Expand Down Expand Up @@ -152,6 +154,7 @@ func TestBuildStatuses(t *testing.T) {
AttachedRoutes: 1,
},
},
ObservedGeneration: 2,
},
IgnoredGatewayStatuses: map[types.NamespacedName]IgnoredGatewayStatus{
{Namespace: "test", Name: "ignored-gateway"}: {ObservedGeneration: 1},
Expand Down Expand Up @@ -211,6 +214,7 @@ func TestBuildStatuses(t *testing.T) {
AttachedRoutes: 1,
},
},
ObservedGeneration: 2,
},
IgnoredGatewayStatuses: map[types.NamespacedName]IgnoredGatewayStatus{
{Namespace: "test", Name: "ignored-gateway"}: {ObservedGeneration: 1},
Expand Down
7 changes: 3 additions & 4 deletions internal/status/gateway.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -50,10 +50,9 @@ func prepareGatewayStatus(gatewayStatus state.GatewayStatus, transitionTime meta
}

cond := metav1.Condition{
Type: string(v1beta1.ListenerConditionReady),
Status: status,
// FIXME(pleshakov) Set the observed generation to the last processed generation of the Gateway resource.
ObservedGeneration: 123,
Type: string(v1beta1.ListenerConditionReady),
Status: status,
ObservedGeneration: gatewayStatus.ObservedGeneration,
LastTransitionTime: transitionTime,
Reason: string(reason),
Message: "", // FIXME(pleshakov) Come up with a good message
Expand Down
5 changes: 3 additions & 2 deletions internal/status/gateway_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ func TestPrepareGatewayStatus(t *testing.T) {
AttachedRoutes: 1,
},
},
ObservedGeneration: 1,
}

transitionTime := metav1.NewTime(time.Now())
Expand All @@ -41,7 +42,7 @@ func TestPrepareGatewayStatus(t *testing.T) {
{
Type: string(v1beta1.ListenerConditionReady),
Status: metav1.ConditionFalse,
ObservedGeneration: 123,
ObservedGeneration: 1,
LastTransitionTime: transitionTime,
Reason: string(v1beta1.ListenerReasonInvalid),
},
Expand All @@ -59,7 +60,7 @@ func TestPrepareGatewayStatus(t *testing.T) {
{
Type: string(v1beta1.ListenerConditionReady),
Status: metav1.ConditionTrue,
ObservedGeneration: 123,
ObservedGeneration: 1,
LastTransitionTime: transitionTime,
Reason: string(v1beta1.ListenerReasonReady),
},
Expand Down
9 changes: 5 additions & 4 deletions internal/status/updater_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,7 @@ var _ = Describe("Updater", func() {
AttachedRoutes: 1,
},
},
ObservedGeneration: generation,
},
IgnoredGatewayStatuses: map[types.NamespacedName]state.IgnoredGatewayStatus{
{Namespace: "test", Name: "ignored-gateway"}: {
Expand Down Expand Up @@ -138,7 +139,7 @@ var _ = Describe("Updater", func() {
}
}

createExpectedGw = func(status metav1.ConditionStatus, reason string) *v1beta1.Gateway {
createExpectedGw = func(status metav1.ConditionStatus, generation int64, reason string) *v1beta1.Gateway {
return &v1beta1.Gateway{
ObjectMeta: metav1.ObjectMeta{
Namespace: "test",
Expand All @@ -162,7 +163,7 @@ var _ = Describe("Updater", func() {
{
Type: string(v1beta1.ListenerConditionReady),
Status: status,
ObservedGeneration: 123,
ObservedGeneration: generation,
LastTransitionTime: fakeClockTime,
Reason: reason,
},
Expand Down Expand Up @@ -306,7 +307,7 @@ var _ = Describe("Updater", func() {

It("should have the updated status of Gateway in the API server", func() {
latestGw := &v1beta1.Gateway{}
expectedGw := createExpectedGw(metav1.ConditionTrue, string(v1beta1.ListenerReasonReady))
expectedGw := createExpectedGw(metav1.ConditionTrue, 1, string(v1beta1.ListenerReasonReady))

err := client.Get(context.Background(), types.NamespacedName{Namespace: "test", Name: "gateway"}, latestGw)
Expect(err).Should(Not(HaveOccurred()))
Expand Down Expand Up @@ -370,7 +371,7 @@ var _ = Describe("Updater", func() {

It("should have the updated status of Gateway in the API server", func() {
latestGw := &v1beta1.Gateway{}
expectedGw := createExpectedGw(metav1.ConditionFalse, string(v1beta1.ListenerReasonInvalid))
expectedGw := createExpectedGw(metav1.ConditionFalse, 2, string(v1beta1.ListenerReasonInvalid))

err := client.Get(
context.Background(),
Expand Down

0 comments on commit 343cab3

Please sign in to comment.