Skip to content
New issue

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

Sign up for GitHub

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

Already on GitHub? Sign in to your account

Fix Observed Generation for Gateway Status #351

Merged
merged 3 commits into from
Jan 6, 2023
Merged

Fix Observed Generation for Gateway Status #351

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
cd764c0
Fix Observed Generation for Gateway Status
kate-osborn Jan 5, 2023
af4c145
Fix updater test
kate-osborn Jan 6, 2023
91c7369
Add gw gen to status test
kate-osborn Jan 6, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 20 additions & 10 deletions internal/state/change_processor_test.go
View file
Open in desktop
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
Open in desktop
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,
kate-osborn marked this conversation as resolved.
Show resolved Hide resolved
}
}

Expand Down
8 changes: 6 additions & 2 deletions internal/state/statuses_test.go
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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