From 22770507d2393f361e3fde04aed28dec804564ee Mon Sep 17 00:00:00 2001 From: Michael Pleshakov Date: Tue, 16 Apr 2024 15:11:33 -0400 Subject: [PATCH] Refactor Status Updater to be resource agnostic (#1814) Problem: Currently, the status updater is aware of what resource it is updating the status of. This makes it difficult to extend because for each new resource we add support for, we have to modify the status updater. Solution: Replace old status updater with two new ones (1) Regular Updater, to update statuses of multiple resources via UpdateRequest type. (2) LeaderAwareGroupUpdater to update statuses of groups of resources and save any pending UpdateRequests until the updater is enabled (when pod becomes leader). It uses Regular Updater. Using groups allow replacing UpdateRequests of subset of resources, without the need to recompute UpdateRequests for all resources. The new status package is resource agnostic. This is accomplished by representing status update as a function (Setter). Other updates: - provisioner manager uses regular Updater. - static manager uses LeaderAwareGroupUpdater (because it needs to support leader election) - framework Status setter were simplified and moved to static/status package - status related functions in static package were moved to static/status package. - Previous Build* functions were replaced with PrepareRequests functions. Such functions don't create any intermediate status representation (like it was before), but create status UpdateRequests which directly set the resource statuses. - static manager handler conditionally updates statuses GatewayClass based on manager configuration. Previously, the conditional logic was implemented in the Status Updater. CLOSES -- https://github.com/nginxinc/nginx-gateway-fabric/issues/1071 --- internal/framework/conditions/conditions.go | 22 + .../framework/conditions/conditions_test.go | 48 +- internal/framework/helpers/helpers.go | 12 +- internal/framework/helpers/helpers_test.go | 25 + internal/framework/status/backend_tls.go | 45 - internal/framework/status/backend_tls_test.go | 51 - internal/framework/status/clock.go | 25 - internal/framework/status/conditions.go | 39 +- internal/framework/status/conditions_test.go | 145 +- internal/framework/status/gateway.go | 29 - internal/framework/status/gateway_test.go | 60 - internal/framework/status/gatewayclass.go | 13 - .../framework/status/gatewayclass_test.go | 29 - internal/framework/status/httproute.go | 47 - internal/framework/status/httproute_test.go | 128 -- internal/framework/status/k8s_updater.go | 4 +- .../status/leader_aware_group_updater.go | 72 + .../status/leader_aware_group_updater_test.go | 156 ++ internal/framework/status/statuses.go | 125 -- .../status/statusfakes/fake_clock.go | 103 -- .../status/statusfakes/fake_group_updater.go | 81 ++ .../status/statusfakes/fake_updater.go | 195 --- internal/framework/status/updater.go | 237 +-- internal/framework/status/updater_test.go | 893 ++---------- internal/mode/provisioner/handler.go | 41 +- internal/mode/provisioner/handler_test.go | 45 +- internal/mode/provisioner/manager.go | 10 +- internal/mode/static/build_statuses.go | 215 --- internal/mode/static/build_statuses_test.go | 746 ---------- internal/mode/static/handler.go | 112 +- internal/mode/static/handler_test.go | 152 +- internal/mode/static/manager.go | 21 +- .../mode/static/status/prepare_requests.go | 335 +++++ .../static/status/prepare_requests_test.go | 1290 +++++++++++++++++ .../static/status/status_setters.go} | 182 +-- .../static/status/status_setters_test.go} | 570 +++++--- 36 files changed, 3035 insertions(+), 3268 deletions(-) create mode 100644 internal/framework/helpers/helpers_test.go delete mode 100644 internal/framework/status/backend_tls.go delete mode 100644 internal/framework/status/backend_tls_test.go delete mode 100644 internal/framework/status/clock.go delete mode 100644 internal/framework/status/gateway.go delete mode 100644 internal/framework/status/gateway_test.go delete mode 100644 internal/framework/status/gatewayclass.go delete mode 100644 internal/framework/status/gatewayclass_test.go delete mode 100644 internal/framework/status/httproute.go delete mode 100644 internal/framework/status/httproute_test.go create mode 100644 internal/framework/status/leader_aware_group_updater.go create mode 100644 internal/framework/status/leader_aware_group_updater_test.go delete mode 100644 internal/framework/status/statuses.go delete mode 100644 internal/framework/status/statusfakes/fake_clock.go create mode 100644 internal/framework/status/statusfakes/fake_group_updater.go delete mode 100644 internal/framework/status/statusfakes/fake_updater.go delete mode 100644 internal/mode/static/build_statuses.go delete mode 100644 internal/mode/static/build_statuses_test.go create mode 100644 internal/mode/static/status/prepare_requests.go create mode 100644 internal/mode/static/status/prepare_requests_test.go rename internal/{framework/status/setters.go => mode/static/status/status_setters.go} (69%) rename internal/{framework/status/setters_test.go => mode/static/status/status_setters_test.go} (65%) diff --git a/internal/framework/conditions/conditions.go b/internal/framework/conditions/conditions.go index e13f21d2c4..bfdfe2b23e 100644 --- a/internal/framework/conditions/conditions.go +++ b/internal/framework/conditions/conditions.go @@ -129,3 +129,25 @@ func NewGatewayClassConflict() Condition { Message: GatewayClassMessageGatewayClassConflict, } } + +// ConvertConditions converts conditions to Kubernetes API conditions. +func ConvertConditions( + conds []Condition, + observedGeneration int64, + transitionTime metav1.Time, +) []metav1.Condition { + apiConds := make([]metav1.Condition, len(conds)) + + for i := range conds { + apiConds[i] = metav1.Condition{ + Type: conds[i].Type, + Status: conds[i].Status, + ObservedGeneration: observedGeneration, + LastTransitionTime: transitionTime, + Reason: conds[i].Reason, + Message: conds[i].Message, + } + } + + return apiConds +} diff --git a/internal/framework/conditions/conditions_test.go b/internal/framework/conditions/conditions_test.go index de7e31cb1e..7bf69c271b 100644 --- a/internal/framework/conditions/conditions_test.go +++ b/internal/framework/conditions/conditions_test.go @@ -8,8 +8,6 @@ import ( ) func TestDeduplicateConditions(t *testing.T) { - g := NewWithT(t) - conds := []Condition{ { Type: "Type1", @@ -56,6 +54,52 @@ func TestDeduplicateConditions(t *testing.T) { }, } + g := NewWithT(t) + result := DeduplicateConditions(conds) g.Expect(result).Should(Equal(expected)) } + +func TestConvertConditions(t *testing.T) { + conds := []Condition{ + { + Type: "Type1", + Status: metav1.ConditionTrue, + Reason: "Reason1", + Message: "Message1", + }, + { + Type: "Type2", + Status: metav1.ConditionFalse, + Reason: "Reason2", + Message: "Message2", + }, + } + + const generation = 3 + time := metav1.Now() + + expected := []metav1.Condition{ + { + Type: "Type1", + Status: metav1.ConditionTrue, + Reason: "Reason1", + Message: "Message1", + LastTransitionTime: time, + ObservedGeneration: generation, + }, + { + Type: "Type2", + Status: metav1.ConditionFalse, + Reason: "Reason2", + Message: "Message2", + LastTransitionTime: time, + ObservedGeneration: generation, + }, + } + + g := NewWithT(t) + + result := ConvertConditions(conds, generation, time) + g.Expect(result).Should(Equal(expected)) +} diff --git a/internal/framework/helpers/helpers.go b/internal/framework/helpers/helpers.go index f312e1f1c3..05d2333493 100644 --- a/internal/framework/helpers/helpers.go +++ b/internal/framework/helpers/helpers.go @@ -1,4 +1,4 @@ -// Package helpers contains helper functions for unit tests. +// Package helpers contains helper functions package helpers import ( @@ -6,6 +6,7 @@ import ( "github.com/google/go-cmp/cmp" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "sigs.k8s.io/controller-runtime/pkg/client" ) // Diff prints the diff between two structs. @@ -41,3 +42,12 @@ func PrepareTimeForFakeClient(t metav1.Time) metav1.Time { return t } + +// MustCastObject casts the client.Object to the specified type that implements it. +func MustCastObject[T client.Object](object client.Object) T { + if obj, ok := object.(T); ok { + return obj + } + + panic(fmt.Errorf("unexpected object type %T", object)) +} diff --git a/internal/framework/helpers/helpers_test.go b/internal/framework/helpers/helpers_test.go new file mode 100644 index 0000000000..8d219fd532 --- /dev/null +++ b/internal/framework/helpers/helpers_test.go @@ -0,0 +1,25 @@ +package helpers + +import ( + "testing" + + . "github.com/onsi/gomega" + + "sigs.k8s.io/controller-runtime/pkg/client" + gatewayv1 "sigs.k8s.io/gateway-api/apis/v1" + gatewayv1alpha2 "sigs.k8s.io/gateway-api/apis/v1alpha2" +) + +func TestMustCastObject(t *testing.T) { + g := NewWithT(t) + + var obj client.Object = &gatewayv1.Gateway{} + + g.Expect(func() { + _ = MustCastObject[*gatewayv1.Gateway](obj) + }).ToNot(Panic()) + + g.Expect(func() { + _ = MustCastObject[*gatewayv1alpha2.BackendTLSPolicy](obj) + }).To(Panic()) +} diff --git a/internal/framework/status/backend_tls.go b/internal/framework/status/backend_tls.go deleted file mode 100644 index 7dae216846..0000000000 --- a/internal/framework/status/backend_tls.go +++ /dev/null @@ -1,45 +0,0 @@ -package status - -import ( - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - v1 "sigs.k8s.io/gateway-api/apis/v1" - "sigs.k8s.io/gateway-api/apis/v1alpha2" -) - -// prepareBackendTLSPolicyStatus prepares the status for a BackendTLSPolicy resource. -func prepareBackendTLSPolicyStatus( - oldStatus v1alpha2.PolicyStatus, - status BackendTLSPolicyStatus, - gatewayCtlrName string, - transitionTime metav1.Time, -) v1alpha2.PolicyStatus { - // maxAncestors is the max number of ancestor statuses which is the sum of all new ancestor statuses and all old - // ancestor statuses. - maxAncestors := len(status.AncestorStatuses) + len(oldStatus.Ancestors) - ancestors := make([]v1alpha2.PolicyAncestorStatus, 0, maxAncestors) - - // keep all the ancestor statuses that belong to other controllers - for _, os := range oldStatus.Ancestors { - if string(os.ControllerName) != gatewayCtlrName { - ancestors = append(ancestors, os) - } - } - - for _, as := range status.AncestorStatuses { - // reassign the iteration variable inside the loop to fix implicit memory aliasing - as := as - a := v1alpha2.PolicyAncestorStatus{ - AncestorRef: v1.ParentReference{ - Namespace: (*v1.Namespace)(&as.GatewayNsName.Namespace), - Name: v1alpha2.ObjectName(as.GatewayNsName.Name), - }, - ControllerName: v1alpha2.GatewayController(gatewayCtlrName), - Conditions: convertConditions(as.Conditions, status.ObservedGeneration, transitionTime), - } - ancestors = append(ancestors, a) - } - - return v1alpha2.PolicyStatus{ - Ancestors: ancestors, - } -} diff --git a/internal/framework/status/backend_tls_test.go b/internal/framework/status/backend_tls_test.go deleted file mode 100644 index 8a9c967ff4..0000000000 --- a/internal/framework/status/backend_tls_test.go +++ /dev/null @@ -1,51 +0,0 @@ -package status - -import ( - "testing" - - . "github.com/onsi/gomega" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/types" - v1 "sigs.k8s.io/gateway-api/apis/v1" - "sigs.k8s.io/gateway-api/apis/v1alpha2" - - "github.com/nginxinc/nginx-gateway-fabric/internal/framework/conditions" - "github.com/nginxinc/nginx-gateway-fabric/internal/framework/helpers" -) - -func TestPrepareBackendTLSPolicyStatus(t *testing.T) { - oldStatus := v1alpha2.PolicyStatus{ - Ancestors: []v1alpha2.PolicyAncestorStatus{ - { - AncestorRef: v1.ParentReference{ - Namespace: helpers.GetPointer((v1.Namespace)("ns1")), - Name: v1alpha2.ObjectName("other-gw"), - }, - ControllerName: v1alpha2.GatewayController("otherCtlr"), - Conditions: []metav1.Condition{{Type: "otherType", Status: "otherStatus"}}, - }, - }, - } - - newStatus := BackendTLSPolicyStatus{ - AncestorStatuses: []AncestorStatus{ - { - GatewayNsName: types.NamespacedName{ - Namespace: "ns1", - Name: "gw1", - }, - Conditions: []conditions.Condition{{Type: "type1", Status: "status1"}}, - }, - }, - ObservedGeneration: 1, - } - - transistionTime := metav1.Now() - ctlrName := "nginx-gateway" - - policyStatus := prepareBackendTLSPolicyStatus(oldStatus, newStatus, ctlrName, transistionTime) - - g := NewWithT(t) - - g.Expect(policyStatus.Ancestors).To(HaveLen(2)) -} diff --git a/internal/framework/status/clock.go b/internal/framework/status/clock.go deleted file mode 100644 index 67d22daa83..0000000000 --- a/internal/framework/status/clock.go +++ /dev/null @@ -1,25 +0,0 @@ -package status - -import ( - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" -) - -//go:generate go run github.com/maxbrunsfeld/counterfeiter/v6 . Clock - -// Clock returns the current local time. -type Clock interface { - Now() metav1.Time -} - -// RealClock returns the current local time. -type RealClock struct{} - -// NewRealClock creates a new RealClock. -func NewRealClock() *RealClock { - return &RealClock{} -} - -// Now returns the current local time. -func (c *RealClock) Now() metav1.Time { - return metav1.Now() -} diff --git a/internal/framework/status/conditions.go b/internal/framework/status/conditions.go index a3dd38bfda..8da31897f9 100644 --- a/internal/framework/status/conditions.go +++ b/internal/framework/status/conditions.go @@ -1,28 +1,31 @@ package status import ( - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "slices" - "github.com/nginxinc/nginx-gateway-fabric/internal/framework/conditions" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) -func convertConditions( - conds []conditions.Condition, - observedGeneration int64, - transitionTime metav1.Time, -) []metav1.Condition { - apiConds := make([]metav1.Condition, len(conds)) +// ConditionsEqual compares conditions. +// It doesn't check the last transition time of conditions. +func ConditionsEqual(prev, cur []metav1.Condition) bool { + return slices.EqualFunc(prev, cur, func(c1, c2 metav1.Condition) bool { + if c1.ObservedGeneration != c2.ObservedGeneration { + return false + } + + if c1.Type != c2.Type { + return false + } + + if c1.Status != c2.Status { + return false + } - for i := range conds { - apiConds[i] = metav1.Condition{ - Type: conds[i].Type, - Status: conds[i].Status, - ObservedGeneration: observedGeneration, - LastTransitionTime: transitionTime, - Reason: conds[i].Reason, - Message: conds[i].Message, + if c1.Message != c2.Message { + return false } - } - return apiConds + return c1.Reason == c2.Reason + }) } diff --git a/internal/framework/status/conditions_test.go b/internal/framework/status/conditions_test.go index 0685508bb1..c3d788e69d 100644 --- a/internal/framework/status/conditions_test.go +++ b/internal/framework/status/conditions_test.go @@ -5,62 +5,115 @@ import ( "time" . "github.com/onsi/gomega" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - - "github.com/nginxinc/nginx-gateway-fabric/internal/framework/conditions" - "github.com/nginxinc/nginx-gateway-fabric/internal/framework/helpers" + v1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) -func CreateTestConditions(condType string) []conditions.Condition { - return []conditions.Condition{ +func TestConditionsEqual(t *testing.T) { + getDefaultConds := func() []v1.Condition { + return []v1.Condition{ + { + Type: "type1", + Status: "status1", + ObservedGeneration: 1, + LastTransitionTime: v1.Time{Time: time.Now()}, + Reason: "reason1", + Message: "message1", + }, + { + Type: "type2", + Status: "status2", + ObservedGeneration: 1, + LastTransitionTime: v1.Time{Time: time.Now()}, + Reason: "reason2", + Message: "message2", + }, + { + Type: "type3", + Status: "status3", + ObservedGeneration: 1, + LastTransitionTime: v1.Time{Time: time.Now()}, + Reason: "reason3", + Message: "message3", + }, + } + } + + getModifiedConds := func(mod func([]v1.Condition) []v1.Condition) []v1.Condition { + return mod(getDefaultConds()) + } + + tests := []struct { + name string + prevConds []v1.Condition + curConds []v1.Condition + expEqual bool + }{ { - Type: condType, - Status: metav1.ConditionTrue, - Reason: "TestReason1", - Message: "Test message1", + name: "different observed gen", + prevConds: getDefaultConds(), + curConds: getModifiedConds(func(conds []v1.Condition) []v1.Condition { + conds[2].ObservedGeneration++ + return conds + }), + expEqual: false, }, { - Type: condType, - Status: metav1.ConditionFalse, - Reason: "TestReason2", - Message: "Test message2", + name: "different status", + prevConds: getDefaultConds(), + curConds: getModifiedConds(func(conds []v1.Condition) []v1.Condition { + conds[1].Status = "different" + return conds + }), + expEqual: false, + }, + { + name: "different type", + prevConds: getDefaultConds(), + curConds: getModifiedConds(func(conds []v1.Condition) []v1.Condition { + conds[0].Type = "different" + return conds + }), + expEqual: false, }, - } -} - -func CreateExpectedAPIConditions( - condType string, - observedGeneration int64, - transitionTime metav1.Time, -) []metav1.Condition { - return []metav1.Condition{ { - Type: condType, - Status: metav1.ConditionTrue, - ObservedGeneration: observedGeneration, - LastTransitionTime: transitionTime, - Reason: "TestReason1", - Message: "Test message1", + name: "different message", + prevConds: getDefaultConds(), + curConds: getModifiedConds(func(conds []v1.Condition) []v1.Condition { + conds[2].Message = "different" + return conds + }), + expEqual: false, }, { - Type: condType, - Status: metav1.ConditionFalse, - ObservedGeneration: observedGeneration, - LastTransitionTime: transitionTime, - Reason: "TestReason2", - Message: "Test message2", + name: "different reason", + prevConds: getDefaultConds(), + curConds: getModifiedConds(func(conds []v1.Condition) []v1.Condition { + conds[1].Reason = "different" + return conds + }), + expEqual: false, + }, + { + name: "different number of conditions", + prevConds: getDefaultConds(), + curConds: getModifiedConds(func(conds []v1.Condition) []v1.Condition { + return conds[:2] + }), + expEqual: false, + }, + { + name: "equal", + prevConds: getDefaultConds(), + curConds: getDefaultConds(), + expEqual: true, }, } -} -func TestConvertRouteConditions(t *testing.T) { - g := NewWithT(t) - - var generation int64 = 1 - transitionTime := metav1.NewTime(time.Now()) - - expected := CreateExpectedAPIConditions("Test", generation, transitionTime) - - result := convertConditions(CreateTestConditions("Test"), generation, transitionTime) - g.Expect(helpers.Diff(expected, result)).To(BeEmpty()) + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + g := NewWithT(t) + equal := ConditionsEqual(test.prevConds, test.curConds) + g.Expect(equal).To(Equal(test.expEqual)) + }) + } } diff --git a/internal/framework/status/gateway.go b/internal/framework/status/gateway.go deleted file mode 100644 index dc13625e33..0000000000 --- a/internal/framework/status/gateway.go +++ /dev/null @@ -1,29 +0,0 @@ -package status - -import ( - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - v1 "sigs.k8s.io/gateway-api/apis/v1" -) - -// prepareGatewayStatus prepares the status for a Gateway resource. -func prepareGatewayStatus( - gatewayStatus GatewayStatus, - transitionTime metav1.Time, -) v1.GatewayStatus { - listenerStatuses := make([]v1.ListenerStatus, 0, len(gatewayStatus.ListenerStatuses)) - - for _, s := range gatewayStatus.ListenerStatuses { - listenerStatuses = append(listenerStatuses, v1.ListenerStatus{ - Name: s.Name, - SupportedKinds: s.SupportedKinds, - AttachedRoutes: s.AttachedRoutes, - Conditions: convertConditions(s.Conditions, gatewayStatus.ObservedGeneration, transitionTime), - }) - } - - return v1.GatewayStatus{ - Listeners: listenerStatuses, - Addresses: gatewayStatus.Addresses, - Conditions: convertConditions(gatewayStatus.Conditions, gatewayStatus.ObservedGeneration, transitionTime), - } -} diff --git a/internal/framework/status/gateway_test.go b/internal/framework/status/gateway_test.go deleted file mode 100644 index 0dfe517f4b..0000000000 --- a/internal/framework/status/gateway_test.go +++ /dev/null @@ -1,60 +0,0 @@ -package status - -import ( - "testing" - "time" - - . "github.com/onsi/gomega" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - v1 "sigs.k8s.io/gateway-api/apis/v1" - - "github.com/nginxinc/nginx-gateway-fabric/internal/framework/helpers" -) - -func TestPrepareGatewayStatus(t *testing.T) { - podIP := v1.GatewayStatusAddress{ - Type: helpers.GetPointer(v1.IPAddressType), - Value: "1.2.3.4", - } - status := GatewayStatus{ - Conditions: CreateTestConditions("GatewayTest"), - ListenerStatuses: ListenerStatuses{ - { - Name: "listener", - AttachedRoutes: 3, - Conditions: CreateTestConditions("ListenerTest"), - SupportedKinds: []v1.RouteGroupKind{ - { - Kind: v1.Kind("HTTPRoute"), - }, - }, - }, - }, - Addresses: []v1.GatewayStatusAddress{podIP}, - ObservedGeneration: 1, - } - - transitionTime := metav1.NewTime(time.Now()) - - expected := v1.GatewayStatus{ - Conditions: CreateExpectedAPIConditions("GatewayTest", 1, transitionTime), - Listeners: []v1.ListenerStatus{ - { - Name: "listener", - SupportedKinds: []v1.RouteGroupKind{ - { - Kind: "HTTPRoute", - }, - }, - AttachedRoutes: 3, - Conditions: CreateExpectedAPIConditions("ListenerTest", 1, transitionTime), - }, - }, - Addresses: []v1.GatewayStatusAddress{podIP}, - } - - g := NewWithT(t) - - result := prepareGatewayStatus(status, transitionTime) - g.Expect(helpers.Diff(expected, result)).To(BeEmpty()) -} diff --git a/internal/framework/status/gatewayclass.go b/internal/framework/status/gatewayclass.go deleted file mode 100644 index 9b4337a035..0000000000 --- a/internal/framework/status/gatewayclass.go +++ /dev/null @@ -1,13 +0,0 @@ -package status - -import ( - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - v1 "sigs.k8s.io/gateway-api/apis/v1" -) - -// prepareGatewayClassStatus prepares the status for the GatewayClass resource. -func prepareGatewayClassStatus(status GatewayClassStatus, transitionTime metav1.Time) v1.GatewayClassStatus { - return v1.GatewayClassStatus{ - Conditions: convertConditions(status.Conditions, status.ObservedGeneration, transitionTime), - } -} diff --git a/internal/framework/status/gatewayclass_test.go b/internal/framework/status/gatewayclass_test.go deleted file mode 100644 index aa76dd6369..0000000000 --- a/internal/framework/status/gatewayclass_test.go +++ /dev/null @@ -1,29 +0,0 @@ -package status - -import ( - "testing" - "time" - - . "github.com/onsi/gomega" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - v1 "sigs.k8s.io/gateway-api/apis/v1" - - "github.com/nginxinc/nginx-gateway-fabric/internal/framework/helpers" -) - -func TestPrepareGatewayClassStatus(t *testing.T) { - transitionTime := metav1.NewTime(time.Now()) - - status := GatewayClassStatus{ - ObservedGeneration: 1, - Conditions: CreateTestConditions("Test"), - } - expected := v1.GatewayClassStatus{ - Conditions: CreateExpectedAPIConditions("Test", 1, transitionTime), - } - - g := NewWithT(t) - - result := prepareGatewayClassStatus(status, transitionTime) - g.Expect(helpers.Diff(expected, result)).To(BeEmpty()) -} diff --git a/internal/framework/status/httproute.go b/internal/framework/status/httproute.go deleted file mode 100644 index d79178dae2..0000000000 --- a/internal/framework/status/httproute.go +++ /dev/null @@ -1,47 +0,0 @@ -package status - -import ( - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - v1 "sigs.k8s.io/gateway-api/apis/v1" -) - -// prepareHTTPRouteStatus prepares the status for an HTTPRoute resource. -func prepareHTTPRouteStatus( - oldStatus v1.HTTPRouteStatus, - status HTTPRouteStatus, - gatewayCtlrName string, - transitionTime metav1.Time, -) v1.HTTPRouteStatus { - // maxParents is the max number of parent statuses which is the sum of all new parent statuses and all old parent - // statuses. - maxParents := len(status.ParentStatuses) + len(oldStatus.Parents) - parents := make([]v1.RouteParentStatus, 0, maxParents) - - // keep all the parent statuses that belong to other controllers - for _, os := range oldStatus.Parents { - if string(os.ControllerName) != gatewayCtlrName { - parents = append(parents, os) - } - } - - for _, ps := range status.ParentStatuses { - // reassign the iteration variable inside the loop to fix implicit memory aliasing - ps := ps - p := v1.RouteParentStatus{ - ParentRef: v1.ParentReference{ - Namespace: (*v1.Namespace)(&ps.GatewayNsName.Namespace), - Name: v1.ObjectName(ps.GatewayNsName.Name), - SectionName: ps.SectionName, - }, - ControllerName: v1.GatewayController(gatewayCtlrName), - Conditions: convertConditions(ps.Conditions, status.ObservedGeneration, transitionTime), - } - parents = append(parents, p) - } - - return v1.HTTPRouteStatus{ - RouteStatus: v1.RouteStatus{ - Parents: parents, - }, - } -} diff --git a/internal/framework/status/httproute_test.go b/internal/framework/status/httproute_test.go deleted file mode 100644 index 3ea82c25e3..0000000000 --- a/internal/framework/status/httproute_test.go +++ /dev/null @@ -1,128 +0,0 @@ -package status - -import ( - "testing" - "time" - - . "github.com/onsi/gomega" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/types" - v1 "sigs.k8s.io/gateway-api/apis/v1" - - "github.com/nginxinc/nginx-gateway-fabric/internal/framework/helpers" -) - -func TestPrepareHTTPRouteStatus(t *testing.T) { - gwNsName1 := types.NamespacedName{Namespace: "test", Name: "gateway-1"} - gwNsName2 := types.NamespacedName{Namespace: "test", Name: "gateway-2"} - - status := HTTPRouteStatus{ - ObservedGeneration: 1, - ParentStatuses: []ParentStatus{ - { - GatewayNsName: gwNsName1, - SectionName: helpers.GetPointer[v1.SectionName]("http"), - Conditions: CreateTestConditions("Test"), - }, - { - GatewayNsName: gwNsName2, - SectionName: nil, - Conditions: CreateTestConditions("Test"), - }, - }, - } - - gatewayCtlrName := "test.example.com" - transitionTime := metav1.NewTime(time.Now()) - - oldStatus := v1.HTTPRouteStatus{ - RouteStatus: v1.RouteStatus{ - Parents: []v1.RouteParentStatus{ - { - ParentRef: v1.ParentReference{ - Namespace: helpers.GetPointer(v1.Namespace(gwNsName1.Namespace)), - Name: v1.ObjectName(gwNsName1.Name), - SectionName: helpers.GetPointer[v1.SectionName]("http"), - }, - ControllerName: v1.GatewayController(gatewayCtlrName), - Conditions: CreateExpectedAPIConditions("Old", 1, transitionTime), - }, - { - ParentRef: v1.ParentReference{ - Namespace: helpers.GetPointer(v1.Namespace(gwNsName1.Namespace)), - Name: v1.ObjectName(gwNsName1.Name), - SectionName: helpers.GetPointer[v1.SectionName]("http"), - }, - ControllerName: v1.GatewayController("not-our-controller"), - Conditions: CreateExpectedAPIConditions("Test", 1, transitionTime), - }, - { - ParentRef: v1.ParentReference{ - Namespace: helpers.GetPointer(v1.Namespace(gwNsName2.Namespace)), - Name: v1.ObjectName(gwNsName2.Name), - SectionName: nil, - }, - ControllerName: v1.GatewayController(gatewayCtlrName), - Conditions: CreateExpectedAPIConditions("Old", 1, transitionTime), - }, - { - ParentRef: v1.ParentReference{ - Namespace: helpers.GetPointer(v1.Namespace(gwNsName2.Namespace)), - Name: v1.ObjectName(gwNsName2.Name), - SectionName: nil, - }, - ControllerName: v1.GatewayController("not-our-controller"), - Conditions: CreateExpectedAPIConditions("Test", 1, transitionTime), - }, - }, - }, - } - - expected := v1.HTTPRouteStatus{ - RouteStatus: v1.RouteStatus{ - Parents: []v1.RouteParentStatus{ - { - ParentRef: v1.ParentReference{ - Namespace: helpers.GetPointer(v1.Namespace(gwNsName1.Namespace)), - Name: v1.ObjectName(gwNsName1.Name), - SectionName: helpers.GetPointer[v1.SectionName]("http"), - }, - ControllerName: v1.GatewayController("not-our-controller"), - Conditions: CreateExpectedAPIConditions("Test", 1, transitionTime), - }, - { - ParentRef: v1.ParentReference{ - Namespace: helpers.GetPointer(v1.Namespace(gwNsName2.Namespace)), - Name: v1.ObjectName(gwNsName2.Name), - SectionName: nil, - }, - ControllerName: v1.GatewayController("not-our-controller"), - Conditions: CreateExpectedAPIConditions("Test", 1, transitionTime), - }, - { - ParentRef: v1.ParentReference{ - Namespace: helpers.GetPointer(v1.Namespace(gwNsName1.Namespace)), - Name: v1.ObjectName(gwNsName1.Name), - SectionName: helpers.GetPointer[v1.SectionName]("http"), - }, - ControllerName: v1.GatewayController(gatewayCtlrName), - Conditions: CreateExpectedAPIConditions("Test", 1, transitionTime), - }, - { - ParentRef: v1.ParentReference{ - Namespace: helpers.GetPointer(v1.Namespace(gwNsName2.Namespace)), - Name: v1.ObjectName(gwNsName2.Name), - SectionName: nil, - }, - ControllerName: v1.GatewayController(gatewayCtlrName), - Conditions: CreateExpectedAPIConditions("Test", 1, transitionTime), - }, - }, - }, - } - - g := NewWithT(t) - - result := prepareHTTPRouteStatus(oldStatus, status, gatewayCtlrName, transitionTime) - g.Expect(helpers.Diff(expected, result)).To(BeEmpty()) -} diff --git a/internal/framework/status/k8s_updater.go b/internal/framework/status/k8s_updater.go index e249c6b021..88a2462d57 100644 --- a/internal/framework/status/k8s_updater.go +++ b/internal/framework/status/k8s_updater.go @@ -6,10 +6,10 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client" ) -//go:generate go run github.com/maxbrunsfeld/counterfeiter/v6 . K8sUpdater - // K8sUpdater updates a resource from the k8s API. // It allows us to mock the client.Reader.Status.Update method. +// +//go:generate go run github.com/maxbrunsfeld/counterfeiter/v6 . K8sUpdater type K8sUpdater interface { // Update is from client.StatusClient.SubResourceWriter. Update(ctx context.Context, obj client.Object, opts ...client.SubResourceUpdateOption) error diff --git a/internal/framework/status/leader_aware_group_updater.go b/internal/framework/status/leader_aware_group_updater.go new file mode 100644 index 0000000000..1eb1f768ea --- /dev/null +++ b/internal/framework/status/leader_aware_group_updater.go @@ -0,0 +1,72 @@ +package status + +import ( + "context" + "errors" + "sync" +) + +// GroupUpdater updates statuses of groups of resources. +// +// Note: this interface is created so that it that we can create a fake from it and use it +// in mode/static/handler_test.go (to avoid import cycles). +// +//go:generate go run github.com/maxbrunsfeld/counterfeiter/v6 . GroupUpdater +type GroupUpdater interface { + UpdateGroup(ctx context.Context, name string, reqs ...UpdateRequest) +} + +// LeaderAwareGroupUpdater updates statuses of groups of resources. +// Before it is enabled, it saves all requests. +// When it is enabled, it updates status using the saved requests. Note: it can only be enabled once. +// After it is enabled, it will not save requests anymore and update statuses immediately. +type LeaderAwareGroupUpdater struct { + updater *Updater + lock *sync.Mutex + groupReqs map[string][]UpdateRequest + enabled bool +} + +// NewLeaderAwareGroupUpdater creates a new LeaderAwareGroupUpdater. +func NewLeaderAwareGroupUpdater(updater *Updater) *LeaderAwareGroupUpdater { + return &LeaderAwareGroupUpdater{ + updater: updater, + lock: &sync.Mutex{}, + groupReqs: make(map[string][]UpdateRequest), + } +} + +// UpdateGroup updates statuses of a group of resources. +func (u *LeaderAwareGroupUpdater) UpdateGroup(ctx context.Context, name string, reqs ...UpdateRequest) { + u.lock.Lock() + defer u.lock.Unlock() + + if !u.enabled { + if len(reqs) == 0 { + delete(u.groupReqs, name) + return + } + + u.groupReqs[name] = reqs + return + } + + u.updater.Update(ctx, reqs...) +} + +// Enable enables the LeaderAwareGroupUpdater, updating statuses using the saved requests. +func (u *LeaderAwareGroupUpdater) Enable(ctx context.Context) { + u.lock.Lock() + defer u.lock.Unlock() + + if u.enabled { + panic(errors.New("LeaderAwareGroupUpdater can only be enabled once")) + } + + u.enabled = true + + for name, reqs := range u.groupReqs { + u.updater.Update(ctx, reqs...) + delete(u.groupReqs, name) + } +} diff --git a/internal/framework/status/leader_aware_group_updater_test.go b/internal/framework/status/leader_aware_group_updater_test.go new file mode 100644 index 0000000000..0b67395661 --- /dev/null +++ b/internal/framework/status/leader_aware_group_updater_test.go @@ -0,0 +1,156 @@ +package status + +import ( + "context" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/types" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/client/fake" + "sigs.k8s.io/controller-runtime/pkg/log/zap" + v1 "sigs.k8s.io/gateway-api/apis/v1" +) + +// We only use one resource type in this test - GatewayClass. +// It is enough, as the Updater is resource-agnostic. +// GatewayClass is used because it has a simple status. +var _ = Describe("LeaderAwareGroupUpdater", func() { + var k8sClient client.Client + + BeforeEach(OncePerOrdered, func() { + scheme := runtime.NewScheme() + + Expect(v1.AddToScheme(scheme)).Should(Succeed()) + + k8sClient = fake.NewClientBuilder(). + WithScheme(scheme). + WithStatusSubresource( + &v1.GatewayClass{}, + ). + Build() + }) + + Describe("Process status updates", Ordered, func() { + const group1 = "group1" + const group2 = "group2" + + var ( + updater *LeaderAwareGroupUpdater + + group1GCNames = []string{"one-first", "one-second"} + group2GCNames = []string{"two-first", "two-second"} + + allGCNames = append(append([]string{}, group1GCNames...), group2GCNames...) + ) + + BeforeAll(func() { + updater = NewLeaderAwareGroupUpdater(NewUpdater(k8sClient, zap.New())) + + for _, name := range allGCNames { + gc := createGC(name) + Expect(k8sClient.Create(context.Background(), gc)).To(Succeed()) + } + }) + + prepareReq := func(name string, condType string, updateNeeded bool) UpdateRequest { + setter := func(_ client.Object) bool { return false } + + if updateNeeded { + setter = func(obj client.Object) bool { + gc := obj.(*v1.GatewayClass) + gc.Status = createGCStatus(condType) + return true + } + } + + return UpdateRequest{ + NsName: types.NamespacedName{ + Name: name, + }, + ResourceType: &v1.GatewayClass{}, + Setter: setter, + } + } + + prepareReqs := func(names []string, condType string) []UpdateRequest { + reqs := make([]UpdateRequest, 0, len(names)) + for _, name := range names { + reqs = append(reqs, prepareReq(name, condType, updateNeeded)) + } + return reqs + } + + testStatuses := func(names []string, condType string) { + for _, name := range names { + var gc v1.GatewayClass + Expect(k8sClient.Get(context.Background(), types.NamespacedName{Name: name}, &gc)).To(Succeed()) + Expect(gc.Status).To(Equal(createGCStatus(condType))) + } + } + + testNoStatuses := func(names []string) { + for _, name := range names { + var gc v1.GatewayClass + Expect(k8sClient.Get(context.Background(), types.NamespacedName{Name: name}, &gc)).To(Succeed()) + Expect(gc.Status).To(Equal(v1.GatewayClassStatus{})) + } + } + + When("updater is disabled", func() { + It("should save requests for later", func() { + reqs1 := prepareReqs(group1GCNames, "TestAllSaveForLater") + updater.UpdateGroup(context.Background(), group1, reqs1...) + + reqs2 := prepareReqs(group2GCNames, "TestAllSaveForLater") + updater.UpdateGroup(context.Background(), group2, reqs2...) + + testNoStatuses(allGCNames) + }) + + When("passing no update requests", func() { + It("should clear saved requests of group2", func() { + updater.UpdateGroup(context.Background(), group2) + }) + }) + }) + + When("updater is enabled", func() { + It("should update statuses from saved requests", func() { + updater.Enable(context.Background()) + + testStatuses(group1GCNames, "TestAllSaveForLater") + testNoStatuses(group2GCNames) + }) + + When("passing no update requests", func() { + It("should not update statuses of group1", func() { + updater.UpdateGroup(context.Background(), group1) + testStatuses(group1GCNames, "TestAllSaveForLater") + }) + }) + + It("should update statuses of all groups", func() { + reqs1 := prepareReqs(group1GCNames, "TestAll") + updater.UpdateGroup(context.Background(), group1, reqs1...) + + reqs2 := prepareReqs(group2GCNames, "TestAll") + updater.UpdateGroup(context.Background(), group2, reqs2...) + + testStatuses(allGCNames, "TestAll") + }) + }) + + When("updater is enabled second time", func() { + It("should panic", func() { + Expect(func() { + updater.Enable(context.Background()) + }).Should(Panic()) + }) + }) + + // cases of canceling the context and updates that are not needed + // are covered in the regular Updater tests + }) +}) diff --git a/internal/framework/status/statuses.go b/internal/framework/status/statuses.go deleted file mode 100644 index 1de369768b..0000000000 --- a/internal/framework/status/statuses.go +++ /dev/null @@ -1,125 +0,0 @@ -package status - -import ( - "k8s.io/apimachinery/pkg/types" - v1 "sigs.k8s.io/gateway-api/apis/v1" - - ngfAPI "github.com/nginxinc/nginx-gateway-fabric/apis/v1alpha1" - "github.com/nginxinc/nginx-gateway-fabric/internal/framework/conditions" -) - -// Status is the status of one or more Kubernetes resources that the StatusUpdater will update. -type Status interface { - // APIGroup returns the GroupName of the resources contained in the status - APIGroup() string -} - -// GatewayAPIStatuses holds the status-related information about Gateway API resources. -type GatewayAPIStatuses struct { - GatewayClassStatuses GatewayClassStatuses - GatewayStatuses GatewayStatuses - HTTPRouteStatuses HTTPRouteStatuses - BackendTLSPolicyStatuses BackendTLSPolicyStatuses -} - -func (g GatewayAPIStatuses) APIGroup() string { - return v1.GroupName -} - -// NginxGatewayStatus holds status-related information about the NginxGateway resource. -type NginxGatewayStatus struct { - // NsName is the NamespacedName of the NginxGateway resource. - NsName types.NamespacedName - // Conditions is the list of conditions for this NginxGateway. - Conditions []conditions.Condition - // ObservedGeneration is the generation of the resource that was processed. - ObservedGeneration int64 -} - -func (n *NginxGatewayStatus) APIGroup() string { - return ngfAPI.GroupName -} - -// ListenerStatuses holds the statuses of listeners. -type ListenerStatuses []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 - -// GatewayClassStatuses holds the statuses of GatewayClasses where the key is the namespaced name of a GatewayClass. -type GatewayClassStatuses map[types.NamespacedName]GatewayClassStatus - -// BackendTLSPolicyStatuses holds the statuses of BackendTLSPolicies where the key is the namespaced name of a -// BackendTLSPolicy. -type BackendTLSPolicyStatuses map[types.NamespacedName]BackendTLSPolicyStatus - -// GatewayStatus holds the status of the winning Gateway resource. -type GatewayStatus struct { - // ListenerStatuses holds the statuses of listeners defined on the Gateway. - ListenerStatuses ListenerStatuses - // Conditions is the list of conditions for this Gateway. - Conditions []conditions.Condition - // Addresses holds the list of GatewayStatusAddresses. - Addresses []v1.GatewayStatusAddress - // ObservedGeneration is the generation of the resource that was processed. - ObservedGeneration int64 - // Ignored tells whether or not this Gateway is ignored. - Ignored bool -} - -// ListenerStatus holds the status-related information about a listener in the Gateway resource. -type ListenerStatus struct { - // Name is the name of the Listener that this status corresponds to. - Name v1.SectionName - // Conditions is the list of conditions for this listener. - Conditions []conditions.Condition - // SupportedKinds is the list of SupportedKinds for this listener. - SupportedKinds []v1.RouteGroupKind - // AttachedRoutes is the number of routes attached to the listener. - AttachedRoutes int32 -} - -// HTTPRouteStatus holds the status-related information about an HTTPRoute resource. -type HTTPRouteStatus struct { - // ParentStatuses holds the statuses for parentRefs of the HTTPRoute. - ParentStatuses []ParentStatus - // ObservedGeneration is the generation of the resource that was processed. - ObservedGeneration int64 -} - -// BackendTLSPolicyStatus holds the status-related information about a BackendTLSPolicy resource. -type BackendTLSPolicyStatus struct { - // AncestorStatuses holds the statuses for parentRefs of the BackendTLSPolicy. - AncestorStatuses []AncestorStatus - // ObservedGeneration is the generation of the resource that was processed. - ObservedGeneration int64 -} - -// ParentStatus holds status-related information related to how the HTTPRoute binds to a specific parentRef. -type ParentStatus struct { - // GatewayNsName is the Namespaced name of the Gateway, which the parentRef references. - GatewayNsName types.NamespacedName - // SectionName is the SectionName of the parentRef. - SectionName *v1.SectionName - // Conditions is the list of conditions that are relevant to the parentRef. - Conditions []conditions.Condition -} - -// GatewayClassStatus holds status-related information about the GatewayClass resource. -type GatewayClassStatus struct { - // Conditions is the list of conditions for this GatewayClass. - Conditions []conditions.Condition - // ObservedGeneration is the generation of the resource that was processed. - ObservedGeneration int64 -} - -// AncestorStatus holds status-related information related to how the BackendTLSPolicy binds to a specific ancestorRef. -type AncestorStatus struct { - // GatewayNsName is the Namespaced name of the Gateway, which the ancestorRef references. - GatewayNsName types.NamespacedName - // Conditions is the list of conditions that are relevant to the ancestor. - Conditions []conditions.Condition -} diff --git a/internal/framework/status/statusfakes/fake_clock.go b/internal/framework/status/statusfakes/fake_clock.go deleted file mode 100644 index 5b51ea34e9..0000000000 --- a/internal/framework/status/statusfakes/fake_clock.go +++ /dev/null @@ -1,103 +0,0 @@ -// Code generated by counterfeiter. DO NOT EDIT. -package statusfakes - -import ( - "sync" - - "github.com/nginxinc/nginx-gateway-fabric/internal/framework/status" - v1 "k8s.io/apimachinery/pkg/apis/meta/v1" -) - -type FakeClock struct { - NowStub func() v1.Time - nowMutex sync.RWMutex - nowArgsForCall []struct { - } - nowReturns struct { - result1 v1.Time - } - nowReturnsOnCall map[int]struct { - result1 v1.Time - } - invocations map[string][][]interface{} - invocationsMutex sync.RWMutex -} - -func (fake *FakeClock) Now() v1.Time { - fake.nowMutex.Lock() - ret, specificReturn := fake.nowReturnsOnCall[len(fake.nowArgsForCall)] - fake.nowArgsForCall = append(fake.nowArgsForCall, struct { - }{}) - stub := fake.NowStub - fakeReturns := fake.nowReturns - fake.recordInvocation("Now", []interface{}{}) - fake.nowMutex.Unlock() - if stub != nil { - return stub() - } - if specificReturn { - return ret.result1 - } - return fakeReturns.result1 -} - -func (fake *FakeClock) NowCallCount() int { - fake.nowMutex.RLock() - defer fake.nowMutex.RUnlock() - return len(fake.nowArgsForCall) -} - -func (fake *FakeClock) NowCalls(stub func() v1.Time) { - fake.nowMutex.Lock() - defer fake.nowMutex.Unlock() - fake.NowStub = stub -} - -func (fake *FakeClock) NowReturns(result1 v1.Time) { - fake.nowMutex.Lock() - defer fake.nowMutex.Unlock() - fake.NowStub = nil - fake.nowReturns = struct { - result1 v1.Time - }{result1} -} - -func (fake *FakeClock) NowReturnsOnCall(i int, result1 v1.Time) { - fake.nowMutex.Lock() - defer fake.nowMutex.Unlock() - fake.NowStub = nil - if fake.nowReturnsOnCall == nil { - fake.nowReturnsOnCall = make(map[int]struct { - result1 v1.Time - }) - } - fake.nowReturnsOnCall[i] = struct { - result1 v1.Time - }{result1} -} - -func (fake *FakeClock) Invocations() map[string][][]interface{} { - fake.invocationsMutex.RLock() - defer fake.invocationsMutex.RUnlock() - fake.nowMutex.RLock() - defer fake.nowMutex.RUnlock() - copiedInvocations := map[string][][]interface{}{} - for key, value := range fake.invocations { - copiedInvocations[key] = value - } - return copiedInvocations -} - -func (fake *FakeClock) recordInvocation(key string, args []interface{}) { - fake.invocationsMutex.Lock() - defer fake.invocationsMutex.Unlock() - if fake.invocations == nil { - fake.invocations = map[string][][]interface{}{} - } - if fake.invocations[key] == nil { - fake.invocations[key] = [][]interface{}{} - } - fake.invocations[key] = append(fake.invocations[key], args) -} - -var _ status.Clock = new(FakeClock) diff --git a/internal/framework/status/statusfakes/fake_group_updater.go b/internal/framework/status/statusfakes/fake_group_updater.go new file mode 100644 index 0000000000..4b33a10a73 --- /dev/null +++ b/internal/framework/status/statusfakes/fake_group_updater.go @@ -0,0 +1,81 @@ +// Code generated by counterfeiter. DO NOT EDIT. +package statusfakes + +import ( + "context" + "sync" + + "github.com/nginxinc/nginx-gateway-fabric/internal/framework/status" +) + +type FakeGroupUpdater struct { + UpdateGroupStub func(context.Context, string, ...status.UpdateRequest) + updateGroupMutex sync.RWMutex + updateGroupArgsForCall []struct { + arg1 context.Context + arg2 string + arg3 []status.UpdateRequest + } + invocations map[string][][]interface{} + invocationsMutex sync.RWMutex +} + +func (fake *FakeGroupUpdater) UpdateGroup(arg1 context.Context, arg2 string, arg3 ...status.UpdateRequest) { + fake.updateGroupMutex.Lock() + fake.updateGroupArgsForCall = append(fake.updateGroupArgsForCall, struct { + arg1 context.Context + arg2 string + arg3 []status.UpdateRequest + }{arg1, arg2, arg3}) + stub := fake.UpdateGroupStub + fake.recordInvocation("UpdateGroup", []interface{}{arg1, arg2, arg3}) + fake.updateGroupMutex.Unlock() + if stub != nil { + fake.UpdateGroupStub(arg1, arg2, arg3...) + } +} + +func (fake *FakeGroupUpdater) UpdateGroupCallCount() int { + fake.updateGroupMutex.RLock() + defer fake.updateGroupMutex.RUnlock() + return len(fake.updateGroupArgsForCall) +} + +func (fake *FakeGroupUpdater) UpdateGroupCalls(stub func(context.Context, string, ...status.UpdateRequest)) { + fake.updateGroupMutex.Lock() + defer fake.updateGroupMutex.Unlock() + fake.UpdateGroupStub = stub +} + +func (fake *FakeGroupUpdater) UpdateGroupArgsForCall(i int) (context.Context, string, []status.UpdateRequest) { + fake.updateGroupMutex.RLock() + defer fake.updateGroupMutex.RUnlock() + argsForCall := fake.updateGroupArgsForCall[i] + return argsForCall.arg1, argsForCall.arg2, argsForCall.arg3 +} + +func (fake *FakeGroupUpdater) Invocations() map[string][][]interface{} { + fake.invocationsMutex.RLock() + defer fake.invocationsMutex.RUnlock() + fake.updateGroupMutex.RLock() + defer fake.updateGroupMutex.RUnlock() + copiedInvocations := map[string][][]interface{}{} + for key, value := range fake.invocations { + copiedInvocations[key] = value + } + return copiedInvocations +} + +func (fake *FakeGroupUpdater) recordInvocation(key string, args []interface{}) { + fake.invocationsMutex.Lock() + defer fake.invocationsMutex.Unlock() + if fake.invocations == nil { + fake.invocations = map[string][][]interface{}{} + } + if fake.invocations[key] == nil { + fake.invocations[key] = [][]interface{}{} + } + fake.invocations[key] = append(fake.invocations[key], args) +} + +var _ status.GroupUpdater = new(FakeGroupUpdater) diff --git a/internal/framework/status/statusfakes/fake_updater.go b/internal/framework/status/statusfakes/fake_updater.go deleted file mode 100644 index 77b1242567..0000000000 --- a/internal/framework/status/statusfakes/fake_updater.go +++ /dev/null @@ -1,195 +0,0 @@ -// Code generated by counterfeiter. DO NOT EDIT. -package statusfakes - -import ( - "context" - "sync" - - "github.com/nginxinc/nginx-gateway-fabric/internal/framework/status" - v1 "sigs.k8s.io/gateway-api/apis/v1" -) - -type FakeUpdater struct { - DisableStub func() - disableMutex sync.RWMutex - disableArgsForCall []struct { - } - EnableStub func(context.Context) - enableMutex sync.RWMutex - enableArgsForCall []struct { - arg1 context.Context - } - UpdateStub func(context.Context, status.Status) - updateMutex sync.RWMutex - updateArgsForCall []struct { - arg1 context.Context - arg2 status.Status - } - UpdateAddressesStub func(context.Context, []v1.GatewayStatusAddress) - updateAddressesMutex sync.RWMutex - updateAddressesArgsForCall []struct { - arg1 context.Context - arg2 []v1.GatewayStatusAddress - } - invocations map[string][][]interface{} - invocationsMutex sync.RWMutex -} - -func (fake *FakeUpdater) Disable() { - fake.disableMutex.Lock() - fake.disableArgsForCall = append(fake.disableArgsForCall, struct { - }{}) - stub := fake.DisableStub - fake.recordInvocation("Disable", []interface{}{}) - fake.disableMutex.Unlock() - if stub != nil { - fake.DisableStub() - } -} - -func (fake *FakeUpdater) DisableCallCount() int { - fake.disableMutex.RLock() - defer fake.disableMutex.RUnlock() - return len(fake.disableArgsForCall) -} - -func (fake *FakeUpdater) DisableCalls(stub func()) { - fake.disableMutex.Lock() - defer fake.disableMutex.Unlock() - fake.DisableStub = stub -} - -func (fake *FakeUpdater) Enable(arg1 context.Context) { - fake.enableMutex.Lock() - fake.enableArgsForCall = append(fake.enableArgsForCall, struct { - arg1 context.Context - }{arg1}) - stub := fake.EnableStub - fake.recordInvocation("Enable", []interface{}{arg1}) - fake.enableMutex.Unlock() - if stub != nil { - fake.EnableStub(arg1) - } -} - -func (fake *FakeUpdater) EnableCallCount() int { - fake.enableMutex.RLock() - defer fake.enableMutex.RUnlock() - return len(fake.enableArgsForCall) -} - -func (fake *FakeUpdater) EnableCalls(stub func(context.Context)) { - fake.enableMutex.Lock() - defer fake.enableMutex.Unlock() - fake.EnableStub = stub -} - -func (fake *FakeUpdater) EnableArgsForCall(i int) context.Context { - fake.enableMutex.RLock() - defer fake.enableMutex.RUnlock() - argsForCall := fake.enableArgsForCall[i] - return argsForCall.arg1 -} - -func (fake *FakeUpdater) Update(arg1 context.Context, arg2 status.Status) { - fake.updateMutex.Lock() - fake.updateArgsForCall = append(fake.updateArgsForCall, struct { - arg1 context.Context - arg2 status.Status - }{arg1, arg2}) - stub := fake.UpdateStub - fake.recordInvocation("Update", []interface{}{arg1, arg2}) - fake.updateMutex.Unlock() - if stub != nil { - fake.UpdateStub(arg1, arg2) - } -} - -func (fake *FakeUpdater) UpdateCallCount() int { - fake.updateMutex.RLock() - defer fake.updateMutex.RUnlock() - return len(fake.updateArgsForCall) -} - -func (fake *FakeUpdater) UpdateCalls(stub func(context.Context, status.Status)) { - fake.updateMutex.Lock() - defer fake.updateMutex.Unlock() - fake.UpdateStub = stub -} - -func (fake *FakeUpdater) UpdateArgsForCall(i int) (context.Context, status.Status) { - fake.updateMutex.RLock() - defer fake.updateMutex.RUnlock() - argsForCall := fake.updateArgsForCall[i] - return argsForCall.arg1, argsForCall.arg2 -} - -func (fake *FakeUpdater) UpdateAddresses(arg1 context.Context, arg2 []v1.GatewayStatusAddress) { - var arg2Copy []v1.GatewayStatusAddress - if arg2 != nil { - arg2Copy = make([]v1.GatewayStatusAddress, len(arg2)) - copy(arg2Copy, arg2) - } - fake.updateAddressesMutex.Lock() - fake.updateAddressesArgsForCall = append(fake.updateAddressesArgsForCall, struct { - arg1 context.Context - arg2 []v1.GatewayStatusAddress - }{arg1, arg2Copy}) - stub := fake.UpdateAddressesStub - fake.recordInvocation("UpdateAddresses", []interface{}{arg1, arg2Copy}) - fake.updateAddressesMutex.Unlock() - if stub != nil { - fake.UpdateAddressesStub(arg1, arg2) - } -} - -func (fake *FakeUpdater) UpdateAddressesCallCount() int { - fake.updateAddressesMutex.RLock() - defer fake.updateAddressesMutex.RUnlock() - return len(fake.updateAddressesArgsForCall) -} - -func (fake *FakeUpdater) UpdateAddressesCalls(stub func(context.Context, []v1.GatewayStatusAddress)) { - fake.updateAddressesMutex.Lock() - defer fake.updateAddressesMutex.Unlock() - fake.UpdateAddressesStub = stub -} - -func (fake *FakeUpdater) UpdateAddressesArgsForCall(i int) (context.Context, []v1.GatewayStatusAddress) { - fake.updateAddressesMutex.RLock() - defer fake.updateAddressesMutex.RUnlock() - argsForCall := fake.updateAddressesArgsForCall[i] - return argsForCall.arg1, argsForCall.arg2 -} - -func (fake *FakeUpdater) Invocations() map[string][][]interface{} { - fake.invocationsMutex.RLock() - defer fake.invocationsMutex.RUnlock() - fake.disableMutex.RLock() - defer fake.disableMutex.RUnlock() - fake.enableMutex.RLock() - defer fake.enableMutex.RUnlock() - fake.updateMutex.RLock() - defer fake.updateMutex.RUnlock() - fake.updateAddressesMutex.RLock() - defer fake.updateAddressesMutex.RUnlock() - copiedInvocations := map[string][][]interface{}{} - for key, value := range fake.invocations { - copiedInvocations[key] = value - } - return copiedInvocations -} - -func (fake *FakeUpdater) recordInvocation(key string, args []interface{}) { - fake.invocationsMutex.Lock() - defer fake.invocationsMutex.Unlock() - if fake.invocations == nil { - fake.invocations = map[string][][]interface{}{} - } - if fake.invocations[key] == nil { - fake.invocations[key] = [][]interface{}{} - } - fake.invocations[key] = append(fake.invocations[key], args) -} - -var _ status.Updater = new(FakeUpdater) diff --git a/internal/framework/status/updater.go b/internal/framework/status/updater.go index 8b8e451915..f28fdacf5e 100644 --- a/internal/framework/status/updater.go +++ b/internal/framework/status/updater.go @@ -3,8 +3,6 @@ package status import ( "context" "errors" - "fmt" - "sync" "time" "github.com/go-logr/logr" @@ -12,51 +10,23 @@ import ( "k8s.io/apimachinery/pkg/types" "k8s.io/apimachinery/pkg/util/wait" "sigs.k8s.io/controller-runtime/pkg/client" - v1 "sigs.k8s.io/gateway-api/apis/v1" - "sigs.k8s.io/gateway-api/apis/v1alpha2" - ngfAPI "github.com/nginxinc/nginx-gateway-fabric/apis/v1alpha1" "github.com/nginxinc/nginx-gateway-fabric/internal/framework/controller" ) -//go:generate go run github.com/maxbrunsfeld/counterfeiter/v6 . Updater - -// Updater updates statuses of the Gateway API resources. -// Updater can be disabled. In this case, it will stop updating the statuses of resources, while -// always saving the statuses of the last Update call. This is used to support multiple replicas of -// control plane being able to run simultaneously where only the leader will update statuses. -type Updater interface { - // Update updates the statuses of the resources. - Update(context.Context, Status) - // UpdateAddresses updates the Gateway Addresses when the Gateway Service changes. - UpdateAddresses(context.Context, []v1.GatewayStatusAddress) - // Enable enables status updates. The updater will update the statuses in Kubernetes API to ensure they match the - // statuses of the last Update invocation. - Enable(ctx context.Context) - // Disable disables status updates. - Disable() +// UpdateRequest is a request to update the status of a resource. +type UpdateRequest struct { + ResourceType client.Object + Setter Setter + NsName types.NamespacedName } -// UpdaterConfig holds configuration parameters for Updater. -type UpdaterConfig struct { - // Client is a Kubernetes API client. - Client client.Client - // Clock is used as a source of time for the LastTransitionTime field in Conditions in resource statuses. - Clock Clock - // Logger holds a logger to be used. - Logger logr.Logger - // GatewayCtlrName is the name of the Gateway controller. - GatewayCtlrName string - // GatewayClassName is the name of the GatewayClass resource. - GatewayClassName string - // UpdateGatewayClassStatus enables updating the status of the GatewayClass resource. - UpdateGatewayClassStatus bool - // LeaderElectionEnabled indicates whether Leader Election is enabled. - // If it is not enabled, the updater will always write statuses to the Kubernetes API. - LeaderElectionEnabled bool -} +// Setter is a function that sets the status of the passed resource. +// It returns true if the status was set, false otherwise. +// The status is not set when the status is already up-to-date. +type Setter func(client.Object) (wasSet bool) -// UpdaterImpl updates statuses of the Gateway API resources. +// Updater updates the status of resources. // // It has the following limitations: // @@ -65,169 +35,59 @@ type UpdaterConfig struct { // (a) Sometimes the Gateway will need to update statuses of all resources it handles, which could be ~1000. Making 1000 // status API calls sequentially will take time. // (b) k8s API can become slow or even timeout. This will increase every update status API call. -// Making UpdaterImpl asynchronous will prevent it from adding variable delays to the event loop. +// Making Updater asynchronous will prevent it from adding variable delays to the event loop. +// FIXME(pleshakov): https://github.com/nginxinc/nginx-gateway-fabric/issues/1014 // // (2) It doesn't clear the statuses of a resources that are no longer handled by the Gateway. For example, if // an HTTPRoute resource no longer has the parentRef to the Gateway resources, the Gateway must update the status // of the resource to remove the status about the removed parentRef. +// FIXME(pleshakov): https://github.com/nginxinc/nginx-gateway-fabric/issues/1015 // // (3) If another controllers changes the status of the Gateway/HTTPRoute resource so that the information set by our // Gateway is removed, our Gateway will not restore the status until the EventLoop invokes the StatusUpdater as a // result of processing some other new change to a resource(s). -// FIXME(pleshakov): Make updater production ready -// https://github.com/nginxinc/nginx-gateway-fabric/issues/691 - -// UpdaterImpl needs to be modified to support new resources. Consider making UpdaterImpl extendable, so that it -// goes along the Open-closed principle. -type UpdaterImpl struct { - lastStatuses lastStatuses - cfg UpdaterConfig - isLeader bool - - lock sync.Mutex -} - -// lastStatuses hold the last saved statuses. Used when leader election is enabled to write the last saved statuses on -// a leader change. -type lastStatuses struct { - nginxGateway *NginxGatewayStatus - gatewayAPI GatewayAPIStatuses -} - -// Enable writes the last saved statuses for the Gateway API resources. -// Used in leader election when the Pod starts leading. It's possible that during a leader change, -// some statuses are missed. This will ensure that the latest statuses are written when a new leader takes over. -func (upd *UpdaterImpl) Enable(ctx context.Context) { - defer upd.lock.Unlock() - upd.lock.Lock() - - upd.isLeader = true - - upd.cfg.Logger.Info("Writing last statuses") - upd.updateGatewayAPI(ctx, upd.lastStatuses.gatewayAPI) - upd.updateNginxGateway(ctx, upd.lastStatuses.nginxGateway) -} - -func (upd *UpdaterImpl) Disable() { - defer upd.lock.Unlock() - upd.lock.Lock() - - upd.isLeader = false +// FIXME(pleshakov): https://github.com/nginxinc/nginx-gateway-fabric/issues/1813 +type Updater struct { + client client.Client + logger logr.Logger } // NewUpdater creates a new Updater. -func NewUpdater(cfg UpdaterConfig) *UpdaterImpl { - return &UpdaterImpl{ - cfg: cfg, - // If leader election is enabled then we should not start running as a leader. Instead, - // we wait for Enable to be invoked by the Leader Elector goroutine. - isLeader: !cfg.LeaderElectionEnabled, - } -} - -func (upd *UpdaterImpl) Update(ctx context.Context, status Status) { - defer upd.lock.Unlock() - upd.lock.Lock() - - switch s := status.(type) { - case *NginxGatewayStatus: - upd.updateNginxGateway(ctx, s) - case GatewayAPIStatuses: - upd.updateGatewayAPI(ctx, s) - default: - panic(fmt.Sprintf("unknown status type %T with group name %s", s, status.APIGroup())) +func NewUpdater(client client.Client, logger logr.Logger) *Updater { + return &Updater{ + client: client, + logger: logger, } } -func (upd *UpdaterImpl) updateNginxGateway(ctx context.Context, status *NginxGatewayStatus) { - upd.lastStatuses.nginxGateway = status - - if !upd.isLeader { - upd.cfg.Logger.Info("Skipping updating Nginx Gateway status because not leader") - return - } - - upd.cfg.Logger.Info("Updating Nginx Gateway status") - - if status != nil { - upd.writeStatuses( - ctx, - status.NsName, - &ngfAPI.NginxGateway{}, - newNginxGatewayStatusSetter(upd.cfg.Clock, *status), - ) - } -} - -func (upd *UpdaterImpl) updateGatewayAPI(ctx context.Context, statuses GatewayAPIStatuses) { - upd.lastStatuses.gatewayAPI = statuses - - if !upd.isLeader { - upd.cfg.Logger.Info("Skipping updating Gateway API status because not leader") - return - } - - upd.cfg.Logger.Info("Updating Gateway API statuses") - - if upd.cfg.UpdateGatewayClassStatus { - for nsname, gcs := range statuses.GatewayClassStatuses { - select { - case <-ctx.Done(): - return - default: - } - - upd.writeStatuses(ctx, nsname, &v1.GatewayClass{}, newGatewayClassStatusSetter(upd.cfg.Clock, gcs)) - } - } - - for nsname, gs := range statuses.GatewayStatuses { +// Update updates the status of the resources from the requests. +func (u *Updater) Update(ctx context.Context, reqs ...UpdateRequest) { + for _, r := range reqs { select { case <-ctx.Done(): return default: } - upd.writeStatuses(ctx, nsname, &v1.Gateway{}, newGatewayStatusSetter(upd.cfg.Clock, gs)) - } - - for nsname, rs := range statuses.HTTPRouteStatuses { - select { - case <-ctx.Done(): - return - default: - } - - upd.writeStatuses( - ctx, - nsname, - &v1.HTTPRoute{}, - newHTTPRouteStatusSetter(upd.cfg.GatewayCtlrName, upd.cfg.Clock, rs), + u.logger.V(1).Info( + "Updating status for resource", + "namespace", r.NsName.Namespace, + "name", r.NsName.Name, + "kind", r.ResourceType.GetObjectKind().GroupVersionKind().Kind, ) - } - for nsname, bs := range statuses.BackendTLSPolicyStatuses { - select { - case <-ctx.Done(): - return - default: - } - - upd.writeStatuses( - ctx, - nsname, - &v1alpha2.BackendTLSPolicy{}, - newBackendTLSPolicyStatusSetter(upd.cfg.GatewayCtlrName, upd.cfg.Clock, bs), - ) + u.writeStatuses(ctx, r.NsName, r.ResourceType, r.Setter) } } -func (upd *UpdaterImpl) writeStatuses( +func (u *Updater) writeStatuses( ctx context.Context, nsname types.NamespacedName, - obj client.Object, - statusSetter setter, + resourceType client.Object, + statusSetter Setter, ) { + obj := resourceType.DeepCopyObject().(client.Object) + err := wait.ExponentialBackoffWithContext( ctx, wait.Backoff{ @@ -238,34 +98,18 @@ func (upd *UpdaterImpl) writeStatuses( Cap: time.Millisecond * 3000, }, // Function returns true if the condition is satisfied, or an error if the loop should be aborted. - NewRetryUpdateFunc(upd.cfg.Client, upd.cfg.Client.Status(), nsname, obj, upd.cfg.Logger, statusSetter), + NewRetryUpdateFunc(u.client, u.client.Status(), nsname, obj, u.logger, statusSetter), ) if err != nil && !errors.Is(err, context.Canceled) { - upd.cfg.Logger.Error( + u.logger.Error( err, "Failed to update status", "namespace", nsname.Namespace, "name", nsname.Name, - "kind", obj.GetObjectKind().GroupVersionKind().Kind) + "kind", resourceType.GetObjectKind().GroupVersionKind().Kind) } } -// UpdateAddresses is called when the Gateway Status needs its addresses updated. -func (upd *UpdaterImpl) UpdateAddresses(ctx context.Context, addresses []v1.GatewayStatusAddress) { - defer upd.lock.Unlock() - upd.lock.Lock() - - for name, status := range upd.lastStatuses.gatewayAPI.GatewayStatuses { - if status.Ignored { - continue - } - status.Addresses = addresses - upd.lastStatuses.gatewayAPI.GatewayStatuses[name] = status - } - - upd.updateGatewayAPI(ctx, upd.lastStatuses.gatewayAPI) -} - // NewRetryUpdateFunc returns a function which will be used in wait.ExponentialBackoffWithContext. // The function will attempt to Update a kubernetes resource and will be retried in // wait.ExponentialBackoffWithContext if an error occurs. Exported for testing purposes. @@ -274,6 +118,9 @@ func (upd *UpdaterImpl) UpdateAddresses(ctx context.Context, addresses []v1.Gate // which is what we want if we encounter an error from the functions we call. However, // the linter will complain if we return nil if an error was found. // +// Note: this function is public because fake dependencies require us to test this function from the test package +// to avoid import cycles. +// //nolint:nilerr func NewRetryUpdateFunc( getter controller.Getter, @@ -281,7 +128,7 @@ func NewRetryUpdateFunc( nsname types.NamespacedName, obj client.Object, logger logr.Logger, - statusSetter func(client.Object) bool, + statusSetter Setter, ) func(ctx context.Context) (bool, error) { return func(ctx context.Context) (bool, error) { // The function handles errors by reporting them in the logs. diff --git a/internal/framework/status/updater_test.go b/internal/framework/status/updater_test.go index faed9195a4..7b08b54074 100644 --- a/internal/framework/status/updater_test.go +++ b/internal/framework/status/updater_test.go @@ -1,9 +1,7 @@ -package status_test +package status import ( "context" - "sync" - "time" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" @@ -14,823 +12,192 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client/fake" "sigs.k8s.io/controller-runtime/pkg/log/zap" v1 "sigs.k8s.io/gateway-api/apis/v1" - v1alpha2 "sigs.k8s.io/gateway-api/apis/v1alpha2" - ngfAPI "github.com/nginxinc/nginx-gateway-fabric/apis/v1alpha1" "github.com/nginxinc/nginx-gateway-fabric/internal/framework/helpers" - "github.com/nginxinc/nginx-gateway-fabric/internal/framework/status" - "github.com/nginxinc/nginx-gateway-fabric/internal/framework/status/statusfakes" - staticConds "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/state/conditions" ) -type unsupportedStatus struct{} +func createGC(name string) *v1.GatewayClass { + return &v1.GatewayClass{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + }, + TypeMeta: metav1.TypeMeta{ + Kind: "GatewayClass", + APIVersion: "gateway.networking.k8s.io/v1", + }, + } +} -func (u unsupportedStatus) APIGroup() string { - return "unsupported" +func createGCStatus(condType string) v1.GatewayClassStatus { + return v1.GatewayClassStatus{ + Conditions: createConditions(condType), + } } -var _ = Describe("Updater", func() { - const gcName = "my-class" +var currTime = helpers.PrepareTimeForFakeClient(metav1.Now()) + +func createConditions( + condType string, +) []metav1.Condition { + return []metav1.Condition{ + { + Type: condType, + Status: metav1.ConditionTrue, + ObservedGeneration: 1, + LastTransitionTime: currTime, + Reason: "TestReason1", + Message: "Test message1", + }, + { + Type: condType, + Status: metav1.ConditionFalse, + ObservedGeneration: 1, + LastTransitionTime: currTime, + Reason: "TestReason2", + Message: "Test message2", + }, + } +} - var ( - client client.Client - fakeClockTime metav1.Time - fakeClock *statusfakes.FakeClock - gatewayCtrlName string - ) +const ( + updateNeeded = true + updateNotNeeded = false +) + +func prepareReq(name string, condType string, updateNeeded bool) UpdateRequest { + var setter Setter + if updateNeeded { + setter = func(obj client.Object) bool { + gc := obj.(*v1.GatewayClass) + gc.Status = createGCStatus(condType) + return true + } + } else { + setter = func(_ client.Object) bool { + return false + } + } + + return UpdateRequest{ + NsName: types.NamespacedName{ + Name: name, + }, + ResourceType: &v1.GatewayClass{}, + Setter: setter, + } +} + +// We only use one resource type in this test - GatewayClass. +// It is enough, as the Updater is resource-agnostic. +// GatewayClass is used because it has a simple status. +var _ = Describe("Updater", func() { + var k8sClient client.Client BeforeEach(OncePerOrdered, func() { scheme := runtime.NewScheme() Expect(v1.AddToScheme(scheme)).Should(Succeed()) - Expect(v1alpha2.AddToScheme(scheme)).Should(Succeed()) - Expect(ngfAPI.AddToScheme(scheme)).Should(Succeed()) - client = fake.NewClientBuilder(). + k8sClient = fake.NewClientBuilder(). WithScheme(scheme). WithStatusSubresource( &v1.GatewayClass{}, - &v1.Gateway{}, - &v1.HTTPRoute{}, - &ngfAPI.NginxGateway{}, - &v1alpha2.BackendTLSPolicy{}, ). Build() - - fakeClockTime = helpers.PrepareTimeForFakeClient(metav1.NewTime(time.Now())) - fakeClock = &statusfakes.FakeClock{} - fakeClock.NowReturns(fakeClockTime) - - gatewayCtrlName = "test.example.com" }) Describe("Process status updates", Ordered, func() { - type generations struct { - gatewayClass int64 - gateways int64 - backendTLSPolicies int64 - } - var ( - updater *status.UpdaterImpl - gc *v1.GatewayClass - gw, ignoredGw *v1.Gateway - hr *v1.HTTPRoute - ng *ngfAPI.NginxGateway - btls *v1alpha2.BackendTLSPolicy - addr = v1.GatewayStatusAddress{ - Type: helpers.GetPointer(v1.IPAddressType), - Value: "1.2.3.4", - } - - createGwAPIStatuses = func(gens generations) status.GatewayAPIStatuses { - return status.GatewayAPIStatuses{ - GatewayClassStatuses: status.GatewayClassStatuses{ - {Name: gcName}: { - ObservedGeneration: gens.gatewayClass, - Conditions: status.CreateTestConditions("Test"), - }, - }, - GatewayStatuses: status.GatewayStatuses{ - {Namespace: "test", Name: "gateway"}: { - Conditions: status.CreateTestConditions("Test"), - ListenerStatuses: []status.ListenerStatus{ - { - Name: "http", - AttachedRoutes: 1, - Conditions: status.CreateTestConditions("Test"), - SupportedKinds: []v1.RouteGroupKind{{Kind: "HTTPRoute"}}, - }, - }, - Addresses: []v1.GatewayStatusAddress{addr}, - ObservedGeneration: gens.gateways, - }, - {Namespace: "test", Name: "ignored-gateway"}: { - Conditions: staticConds.NewGatewayConflict(), - ObservedGeneration: 1, - Ignored: true, - }, - }, - HTTPRouteStatuses: status.HTTPRouteStatuses{ - {Namespace: "test", Name: "route1"}: { - ObservedGeneration: 5, - ParentStatuses: []status.ParentStatus{ - { - GatewayNsName: types.NamespacedName{Namespace: "test", Name: "gateway"}, - SectionName: helpers.GetPointer[v1.SectionName]("http"), - Conditions: status.CreateTestConditions("Test"), - }, - }, - }, - }, - BackendTLSPolicyStatuses: status.BackendTLSPolicyStatuses{ - {Namespace: "test", Name: "backend-tls-policy"}: { - ObservedGeneration: gens.backendTLSPolicies, - AncestorStatuses: []status.AncestorStatus{ - { - GatewayNsName: types.NamespacedName{Namespace: "test", Name: "gateway"}, - Conditions: status.CreateTestConditions("Test"), - }, - }, - }, - }, - } - } - - createNGStatus = func(gen int64) *status.NginxGatewayStatus { - return &status.NginxGatewayStatus{ - NsName: types.NamespacedName{ - Namespace: "nginx-gateway", - Name: "nginx-gateway-config", - }, - ObservedGeneration: gen, - Conditions: status.CreateTestConditions("Test"), - } - } - - createExpectedGCWithGeneration = func(generation int64) *v1.GatewayClass { - return &v1.GatewayClass{ - ObjectMeta: metav1.ObjectMeta{ - Name: gcName, - }, - TypeMeta: metav1.TypeMeta{ - Kind: "GatewayClass", - APIVersion: "gateway.networking.k8s.io/v1", - }, - Status: v1.GatewayClassStatus{ - Conditions: status.CreateExpectedAPIConditions("Test", generation, fakeClockTime), - }, - } - } - - createExpectedGwWithGeneration = func(generation int64) *v1.Gateway { - return &v1.Gateway{ - ObjectMeta: metav1.ObjectMeta{ - Namespace: "test", - Name: "gateway", - }, - TypeMeta: metav1.TypeMeta{ - Kind: "Gateway", - APIVersion: "gateway.networking.k8s.io/v1", - }, - Status: v1.GatewayStatus{ - Conditions: status.CreateExpectedAPIConditions("Test", generation, fakeClockTime), - Listeners: []v1.ListenerStatus{ - { - Name: "http", - AttachedRoutes: 1, - Conditions: status.CreateExpectedAPIConditions("Test", generation, fakeClockTime), - SupportedKinds: []v1.RouteGroupKind{{Kind: "HTTPRoute"}}, - }, - }, - Addresses: []v1.GatewayStatusAddress{addr}, - }, - } - } - - createExpectedIgnoredGw = func() *v1.Gateway { - return &v1.Gateway{ - ObjectMeta: metav1.ObjectMeta{ - Namespace: "test", - Name: "ignored-gateway", - }, - TypeMeta: metav1.TypeMeta{ - Kind: "Gateway", - APIVersion: "gateway.networking.k8s.io/v1", - }, - Status: v1.GatewayStatus{ - Conditions: []metav1.Condition{ - { - Type: string(v1.GatewayConditionAccepted), - Status: metav1.ConditionFalse, - ObservedGeneration: 1, - LastTransitionTime: fakeClockTime, - Reason: string(staticConds.GatewayReasonGatewayConflict), - Message: staticConds.GatewayMessageGatewayConflict, - }, - { - Type: string(v1.GatewayConditionProgrammed), - Status: metav1.ConditionFalse, - ObservedGeneration: 1, - LastTransitionTime: fakeClockTime, - Reason: string(staticConds.GatewayReasonGatewayConflict), - Message: staticConds.GatewayMessageGatewayConflict, - }, - }, - }, - } - } - - createExpectedHR = func() *v1.HTTPRoute { - return &v1.HTTPRoute{ - ObjectMeta: metav1.ObjectMeta{ - Namespace: "test", - Name: "route1", - }, - TypeMeta: metav1.TypeMeta{ - Kind: "HTTPRoute", - APIVersion: "gateway.networking.k8s.io/v1", - }, - Status: v1.HTTPRouteStatus{ - RouteStatus: v1.RouteStatus{ - Parents: []v1.RouteParentStatus{ - { - ControllerName: v1.GatewayController(gatewayCtrlName), - ParentRef: v1.ParentReference{ - Namespace: (*v1.Namespace)(helpers.GetPointer("test")), - Name: "gateway", - SectionName: (*v1.SectionName)(helpers.GetPointer("http")), - }, - Conditions: status.CreateExpectedAPIConditions("Test", 5, fakeClockTime), - }, - }, - }, - }, - } - } + updater *Updater - createExpectedBtlsWithGeneration = func(gen int64) *v1alpha2.BackendTLSPolicy { - return &v1alpha2.BackendTLSPolicy{ - ObjectMeta: metav1.ObjectMeta{ - Namespace: "test", - Name: "backend-tls-policy", - }, - TypeMeta: metav1.TypeMeta{ - Kind: "BackendTLSPolicy", - APIVersion: "gateway.networking.k8s.io/v1alpha2", - }, - Status: v1alpha2.PolicyStatus{ - Ancestors: []v1alpha2.PolicyAncestorStatus{ - { - AncestorRef: v1.ParentReference{ - Namespace: (*v1.Namespace)(helpers.GetPointer("test")), - Name: "gateway", - }, - ControllerName: v1alpha2.GatewayController(gatewayCtrlName), - Conditions: status.CreateExpectedAPIConditions("Test", gen, fakeClockTime), - }, - }, - }, - } - } - - createExpectedNGWithGeneration = func(gen int64) *ngfAPI.NginxGateway { - return &ngfAPI.NginxGateway{ - ObjectMeta: metav1.ObjectMeta{ - Namespace: "nginx-gateway", - Name: "nginx-gateway-config", - }, - TypeMeta: metav1.TypeMeta{ - Kind: "NginxGateway", - APIVersion: "gateway.nginx.org/v1alpha1", - }, - Status: ngfAPI.NginxGatewayStatus{ - Conditions: status.CreateExpectedAPIConditions("Test", gen, fakeClockTime), - }, - } - } + gcNames = []string{"first", "second"} ) BeforeAll(func() { - updater = status.NewUpdater(status.UpdaterConfig{ - GatewayCtlrName: gatewayCtrlName, - GatewayClassName: gcName, - Client: client, - Logger: zap.New(), - Clock: fakeClock, - UpdateGatewayClassStatus: true, - }) + updater = NewUpdater(k8sClient, zap.New()) - gc = &v1.GatewayClass{ - ObjectMeta: metav1.ObjectMeta{ - Name: gcName, - }, - TypeMeta: metav1.TypeMeta{ - Kind: "GatewayClass", - APIVersion: "gateway.networking.k8s.io/v1", - }, - } - gw = &v1.Gateway{ - ObjectMeta: metav1.ObjectMeta{ - Namespace: "test", - Name: "gateway", - }, - TypeMeta: metav1.TypeMeta{ - Kind: "Gateway", - APIVersion: "gateway.networking.k8s.io/v1", - }, - } - ignoredGw = &v1.Gateway{ - ObjectMeta: metav1.ObjectMeta{ - Namespace: "test", - Name: "ignored-gateway", - }, - TypeMeta: metav1.TypeMeta{ - Kind: "Gateway", - APIVersion: "gateway.networking.k8s.io/v1", - }, - } - hr = &v1.HTTPRoute{ - ObjectMeta: metav1.ObjectMeta{ - Namespace: "test", - Name: "route1", - }, - TypeMeta: metav1.TypeMeta{ - Kind: "HTTPRoute", - APIVersion: "gateway.networking.k8s.io/v1", - }, - } - btls = &v1alpha2.BackendTLSPolicy{ - ObjectMeta: metav1.ObjectMeta{ - Namespace: "test", - Name: "backend-tls-policy", - }, - TypeMeta: metav1.TypeMeta{ - Kind: "BackendTLSPolicy", - APIVersion: "gateway.networking.k8s.io/v1alpha2", - }, - } - ng = &ngfAPI.NginxGateway{ - ObjectMeta: metav1.ObjectMeta{ - Namespace: "nginx-gateway", - Name: "nginx-gateway-config", - }, - TypeMeta: metav1.TypeMeta{ - Kind: "NginxGateway", - APIVersion: "gateway.nginx.org/v1alpha1", - }, + for _, name := range gcNames { + gc := createGC(name) + Expect(k8sClient.Create(context.Background(), gc)).Should(Succeed()) } }) - It("should create resources in the API server", func() { - Expect(client.Create(context.Background(), gc)).Should(Succeed()) - Expect(client.Create(context.Background(), gw)).Should(Succeed()) - Expect(client.Create(context.Background(), ignoredGw)).Should(Succeed()) - Expect(client.Create(context.Background(), hr)).Should(Succeed()) - Expect(client.Create(context.Background(), ng)).Should(Succeed()) - Expect(client.Create(context.Background(), btls)).Should(Succeed()) - }) - - It("should update gateway API statuses", func() { - updater.Update(context.Background(), createGwAPIStatuses(generations{ - gatewayClass: 1, - gateways: 1, - backendTLSPolicies: 1, - })) - }) - - It("should have the updated status of GatewayClass in the API server", func() { - latestGc := &v1.GatewayClass{} - expectedGc := createExpectedGCWithGeneration(1) - - err := client.Get(context.Background(), types.NamespacedName{Name: gcName}, latestGc) - Expect(err).ToNot(HaveOccurred()) - - expectedGc.ResourceVersion = latestGc.ResourceVersion // updating the status changes the ResourceVersion - - Expect(helpers.Diff(expectedGc, latestGc)).To(BeEmpty()) - }) - - It("should have the updated status of Gateway in the API server", func() { - latestGw := &v1.Gateway{} - expectedGw := createExpectedGwWithGeneration(1) - - err := client.Get(context.Background(), types.NamespacedName{Namespace: "test", Name: "gateway"}, latestGw) - Expect(err).ToNot(HaveOccurred()) - - expectedGw.ResourceVersion = latestGw.ResourceVersion - - Expect(helpers.Diff(expectedGw, latestGw)).To(BeEmpty()) - }) - - It("should have the updated status of ignored Gateway in the API server", func() { - latestGw := &v1.Gateway{} - expectedGw := createExpectedIgnoredGw() - - err := client.Get( - context.Background(), - types.NamespacedName{Namespace: "test", Name: "ignored-gateway"}, - latestGw, - ) - Expect(err).ToNot(HaveOccurred()) - - expectedGw.ResourceVersion = latestGw.ResourceVersion - - Expect(helpers.Diff(expectedGw, latestGw)).To(BeEmpty()) - }) - - It("should have the updated status of HTTPRoute in the API server", func() { - latestHR := &v1.HTTPRoute{} - expectedHR := createExpectedHR() + testStatus := func(name string, condType string) { + var gc v1.GatewayClass - err := client.Get(context.Background(), types.NamespacedName{Namespace: "test", Name: "route1"}, latestHR) + err := k8sClient.Get(context.Background(), types.NamespacedName{Name: name}, &gc) Expect(err).ToNot(HaveOccurred()) + Expect(gc.Status).To(Equal(createGCStatus(condType))) + } - expectedHR.ResourceVersion = latestHR.ResourceVersion - - Expect(helpers.Diff(expectedHR, latestHR)).To(BeEmpty()) - }) - - It("should have the updated status of BackendTLSPolicy in the API server", func() { - latestBtls := &v1alpha2.BackendTLSPolicy{} - expectedBtls := createExpectedBtlsWithGeneration(1) - - err := client.Get( - context.Background(), - types.NamespacedName{Namespace: "test", Name: "backend-tls-policy"}, - latestBtls, - ) - Expect(err).ToNot(HaveOccurred()) - - expectedBtls.ResourceVersion = latestBtls.ResourceVersion - - Expect(helpers.Diff(expectedBtls, latestBtls)).To(BeEmpty()) - }) - - It("should update nginx gateway status", func() { - updater.Update(context.Background(), createNGStatus(1)) - }) - - It("should have the updated status of NginxGateway in the API server", func() { - latestNG := &ngfAPI.NginxGateway{} - expectedNG := createExpectedNGWithGeneration(1) - - err := client.Get( - context.Background(), - types.NamespacedName{Namespace: "nginx-gateway", Name: "nginx-gateway-config"}, - latestNG, - ) - Expect(err).ToNot(HaveOccurred()) - - expectedNG.ResourceVersion = latestNG.ResourceVersion - - Expect(helpers.Diff(expectedNG, latestNG)).To(BeEmpty()) - }) - - When("the Gateway Service is updated with a new address", func() { - AfterEach(func() { - // reset the IP for the remaining tests - updater.UpdateAddresses(context.Background(), []v1.GatewayStatusAddress{ - { - Type: helpers.GetPointer(v1.IPAddressType), - Value: "1.2.3.4", - }, - }) - }) - - It("should update the previous Gateway statuses with new address", func() { - latestGw := &v1.Gateway{} - expectedGw := createExpectedGwWithGeneration(1) - expectedGw.Status.Addresses[0].Value = "5.6.7.8" - - updater.UpdateAddresses(context.Background(), []v1.GatewayStatusAddress{ - { - Type: helpers.GetPointer(v1.IPAddressType), - Value: "5.6.7.8", - }, - }) - - err := client.Get( - context.Background(), - types.NamespacedName{Namespace: "test", Name: "gateway"}, - latestGw, - ) - Expect(err).ToNot(HaveOccurred()) - - expectedGw.ResourceVersion = latestGw.ResourceVersion - - Expect(helpers.Diff(expectedGw, latestGw)).To(BeEmpty()) - }) - }) - - It("should not update Gateway API statuses with canceled context - function normally returns", func() { - ctx, cancel := context.WithCancel(context.Background()) - cancel() - updater.Update(ctx, createGwAPIStatuses(generations{ - gatewayClass: 2, - gateways: 2, - })) - }) - - When("updating with canceled context", func() { - It("should not have the updated status of GatewayClass in the API server", func() { - latestGc := &v1.GatewayClass{} - expectedGc := createExpectedGCWithGeneration(1) - - err := client.Get(context.Background(), types.NamespacedName{Name: gcName}, latestGc) - Expect(err).ToNot(HaveOccurred()) - - expectedGc.ResourceVersion = latestGc.ResourceVersion - - Expect(helpers.Diff(expectedGc, latestGc)).To(BeEmpty()) - }) - - It("should not have the updated status of Gateway in the API server", func() { - latestGw := &v1.Gateway{} - expectedGw := createExpectedGwWithGeneration(1) - - err := client.Get( - context.Background(), - types.NamespacedName{Namespace: "test", Name: "gateway"}, - latestGw, - ) - Expect(err).ToNot(HaveOccurred()) - - expectedGw.ResourceVersion = latestGw.ResourceVersion - - Expect(helpers.Diff(expectedGw, latestGw)).To(BeEmpty()) - }) - - It("should not have the updated status of ignored Gateway in the API server", func() { - latestGw := &v1.Gateway{} - expectedGw := createExpectedIgnoredGw() - - err := client.Get( - context.Background(), - types.NamespacedName{Namespace: "test", Name: "ignored-gateway"}, - latestGw, - ) - Expect(err).ToNot(HaveOccurred()) - - expectedGw.ResourceVersion = latestGw.ResourceVersion - - // if the status was updated, we would see a different ObservedGeneration - Expect(helpers.Diff(expectedGw, latestGw)).To(BeEmpty()) - }) - - It("should not have the updated status of HTTPRoute in the API server", func() { - latestHR := &v1.HTTPRoute{} - expectedHR := createExpectedHR() - - err := client.Get( - context.Background(), - types.NamespacedName{Namespace: "test", Name: "route1"}, - latestHR, - ) - Expect(err).ToNot(HaveOccurred()) - - expectedHR.ResourceVersion = latestHR.ResourceVersion - - // if the status was updated, we would see the route rejected (Accepted = false) - Expect(helpers.Diff(expectedHR, latestHR)).To(BeEmpty()) - }) - }) - - It("should not update NginxGateway status with canceled context - function normally returns", func() { - ctx, cancel := context.WithCancel(context.Background()) - cancel() - updater.Update(ctx, createNGStatus(2)) - }) - - When("updating with canceled context", func() { - It("should not have the updated status of the NginxGateway in the API server", func() { - latestNG := &ngfAPI.NginxGateway{} - expectedNG := createExpectedNGWithGeneration(1) - - err := client.Get( - context.Background(), - types.NamespacedName{Namespace: "nginx-gateway", Name: "nginx-gateway-config"}, - latestNG, - ) - Expect(err).ToNot(HaveOccurred()) - - expectedNG.ResourceVersion = latestNG.ResourceVersion - - Expect(helpers.Diff(expectedNG, latestNG)).To(BeEmpty()) - }) + It("should update the status of GatewayClasses individually", func() { + for _, name := range gcNames { + req := prepareReq(name, "TestIndividually", updateNeeded) + updater.Update(context.Background(), req) + testStatus(name, "TestIndividually") + } }) - When("the Pod is not the current leader", func() { - It("should not update any statuses", func() { - updater.Disable() - updater.Update(context.Background(), createGwAPIStatuses(generations{ - gateways: 3, - })) - updater.Update(context.Background(), createNGStatus(2)) - }) - - It("should not have the updated status of Gateway in the API server", func() { - latestGw := &v1.Gateway{} - // testing that the generation has not changed from 1 to 3 - expectedGw := createExpectedGwWithGeneration(1) - - err := client.Get( - context.Background(), - types.NamespacedName{Namespace: "test", Name: "gateway"}, - latestGw, - ) - Expect(err).ToNot(HaveOccurred()) + It("should update the status of all GatewayClasses", func() { + reqs := make([]UpdateRequest, 0, len(gcNames)) - expectedGw.ResourceVersion = latestGw.ResourceVersion - - Expect(helpers.Diff(expectedGw, latestGw)).To(BeEmpty()) - }) - - It("should not have the updated status of the Nginx Gateway resource in the API server", func() { - latestNG := &ngfAPI.NginxGateway{} - expectedNG := createExpectedNGWithGeneration(1) - - err := client.Get( - context.Background(), - types.NamespacedName{Namespace: "nginx-gateway", Name: "nginx-gateway-config"}, - latestNG, - ) - Expect(err).ToNot(HaveOccurred()) + for _, name := range gcNames { + reqs = append(reqs, prepareReq(name, "TestAll", updateNeeded)) + } - expectedNG.ResourceVersion = latestNG.ResourceVersion + updater.Update(context.Background(), reqs...) - Expect(helpers.Diff(expectedNG, latestNG)).To(BeEmpty()) - }) + for _, name := range gcNames { + testStatus(name, "TestAll") + } }) - When("the Pod starts leading", func() { - It("writes the last statuses", func() { - updater.Enable(context.Background()) - }) - It("should have the updated status of Gateway in the API server", func() { - latestGw := &v1.Gateway{} - expectedGw := createExpectedGwWithGeneration(3) + When("there are no updates", func() { + It("should not update the status of GatewayClasses", func() { + updater.Update(context.Background()) - err := client.Get( - context.Background(), - types.NamespacedName{Namespace: "test", Name: "gateway"}, - latestGw, - ) - Expect(err).ToNot(HaveOccurred()) - - expectedGw.ResourceVersion = latestGw.ResourceVersion - - Expect(helpers.Diff(expectedGw, latestGw)).To(BeEmpty()) - }) - - It("should have the updated status of the Nginx Gateway resource in the API server", func() { - latestNG := &ngfAPI.NginxGateway{} - expectedNG := createExpectedNGWithGeneration(2) - - err := client.Get( - context.Background(), - types.NamespacedName{Namespace: "nginx-gateway", Name: "nginx-gateway-config"}, - latestNG, - ) - Expect(err).ToNot(HaveOccurred()) - - expectedNG.ResourceVersion = latestNG.ResourceVersion - - Expect(helpers.Diff(expectedNG, latestNG)).To(BeEmpty()) + for _, name := range gcNames { + // condType from the last successful update should be present + testStatus(name, "TestAll") + } }) }) - When("the Pod is the current leader", func() { - It("should update Gateway API statuses", func() { - updater.Update(context.Background(), createGwAPIStatuses(generations{ - gateways: 4, - })) - }) - - It("should have the updated status of Gateway in the API server", func() { - latestGw := &v1.Gateway{} - expectedGw := createExpectedGwWithGeneration(4) - - err := client.Get( - context.Background(), - types.NamespacedName{Namespace: "test", Name: "gateway"}, - latestGw, - ) - Expect(err).ToNot(HaveOccurred()) - - expectedGw.ResourceVersion = latestGw.ResourceVersion - - Expect(helpers.Diff(expectedGw, latestGw)).To(BeEmpty()) - }) + When("the context is canceled", func() { + It("should not update the status of GatewayClasses", func() { + reqs := make([]UpdateRequest, 0, len(gcNames)) - It("should update Nginx Gateway status", func() { - updater.Update(context.Background(), createNGStatus(3)) - }) - It("should have the updated status of Nginx Gateway in the API server", func() { - latestNG := &ngfAPI.NginxGateway{} - expectedNG := createExpectedNGWithGeneration(3) - - err := client.Get( - context.Background(), - types.NamespacedName{Namespace: "nginx-gateway", Name: "nginx-gateway-config"}, - latestNG, - ) - Expect(err).ToNot(HaveOccurred()) - - expectedNG.ResourceVersion = latestNG.ResourceVersion - - Expect(helpers.Diff(expectedNG, latestNG)).To(BeEmpty()) - }) - It("updates and writes last statuses synchronously", func() { - wg := &sync.WaitGroup{} - ctx := context.Background() - - // Spin up 10 goroutines that Update and 10 that call Enable which writes the last statuses. - // Since we only write statuses when they've changed, we will only update the status 10 times. - // The purpose of this test is to exercise the locking mechanism embedded in the updater. - // If there is a data race, this test combined with the -race flag that we run tests with, - // should catch it. - for i := 0; i < 10; i++ { - wg.Add(2) - gen := 5 + i - go func() { - updater.Update(ctx, createGwAPIStatuses(generations{gateways: int64(gen)})) - wg.Done() - }() - - go func() { - updater.Enable(ctx) - wg.Done() - }() + for _, name := range gcNames { + reqs = append(reqs, prepareReq(name, "TestContextDone", updateNeeded)) } - wg.Wait() + ctx, cancel := context.WithCancel(context.Background()) + cancel() - latestGw := &v1.Gateway{} + updater.Update(ctx, reqs...) - err := client.Get( - context.Background(), - types.NamespacedName{Namespace: "test", Name: "gateway"}, - latestGw, - ) - Expect(err).ToNot(HaveOccurred()) - - // Before this test there were 6 updates to the Gateway resource. - // So now the resource version should equal 16. - Expect(latestGw.ResourceVersion).To(Equal("16")) + for _, name := range gcNames { + // condType from the last successful update should be present + testStatus(name, "TestAll") + } }) }) - }) - - Describe("Skip GatewayClass updates", Ordered, func() { - var ( - updater status.Updater - gc *v1.GatewayClass - ) - BeforeAll(func() { - updater = status.NewUpdater(status.UpdaterConfig{ - GatewayCtlrName: gatewayCtrlName, - GatewayClassName: gcName, - Client: client, - Logger: zap.New(), - Clock: fakeClock, - UpdateGatewayClassStatus: false, - }) - - gc = &v1.GatewayClass{ - ObjectMeta: metav1.ObjectMeta{ - Name: gcName, - }, - TypeMeta: metav1.TypeMeta{ - Kind: "GatewayClass", - APIVersion: "gateway.networking.k8s.io/v1", - }, - } - }) + When("the update is not needed", func() { + It("should not update the status of GatewayClasses", func() { + reqs := make([]UpdateRequest, 0, len(gcNames)) - It("should create resources in the API server", func() { - Expect(client.Create(context.Background(), gc)).Should(Succeed()) - }) - - It("should not update GatewayClass status", func() { - updater.Update( - context.Background(), - status.GatewayAPIStatuses{ - GatewayClassStatuses: status.GatewayClassStatuses{ - {Name: gcName}: { - ObservedGeneration: 1, - Conditions: status.CreateTestConditions("Test"), - }, - }, - }, - ) - - latestGc := &v1.GatewayClass{} - - err := client.Get(context.Background(), types.NamespacedName{Name: gcName}, latestGc) - Expect(err).ToNot(HaveOccurred()) + for _, name := range gcNames { + reqs = append(reqs, prepareReq(name, "TestNotNeeded", updateNotNeeded)) + } - Expect(latestGc.Status).To(BeZero()) - }) - }) + updater.Update(context.Background(), reqs...) - Describe("Edge cases", func() { - It("panics on update if status type is unknown", func() { - updater := status.NewUpdater(status.UpdaterConfig{ - GatewayCtlrName: gatewayCtrlName, - GatewayClassName: gcName, - Client: client, - Logger: zap.New(), - Clock: fakeClock, - UpdateGatewayClassStatus: true, + for _, name := range gcNames { + // condType from the last successful update should be present + testStatus(name, "TestAll") + } }) - - update := func() { - updater.Update(context.Background(), unsupportedStatus{}) - } - - Expect(update).Should(Panic()) }) }) }) diff --git a/internal/mode/provisioner/handler.go b/internal/mode/provisioner/handler.go index 853e814f1b..5b8c80fb1e 100644 --- a/internal/mode/provisioner/handler.go +++ b/internal/mode/provisioner/handler.go @@ -6,15 +6,20 @@ import ( "github.com/go-logr/logr" v1 "k8s.io/api/apps/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" "sigs.k8s.io/controller-runtime/pkg/client" + gatewayv1 "sigs.k8s.io/gateway-api/apis/v1" "github.com/nginxinc/nginx-gateway-fabric/internal/framework/conditions" "github.com/nginxinc/nginx-gateway-fabric/internal/framework/events" "github.com/nginxinc/nginx-gateway-fabric/internal/framework/gatewayclass" + "github.com/nginxinc/nginx-gateway-fabric/internal/framework/helpers" "github.com/nginxinc/nginx-gateway-fabric/internal/framework/status" ) +type timeNowFunc func() metav1.Time + // eventHandler ensures each Gateway for the specific GatewayClass has a corresponding Deployment // of NGF configured to use that specific Gateway. // @@ -26,8 +31,9 @@ type eventHandler struct { // provisions maps NamespacedName of Gateway to its corresponding Deployment provisions map[types.NamespacedName]*v1.Deployment - statusUpdater status.Updater + statusUpdater *status.Updater k8sClient client.Client + timeNow timeNowFunc staticModeDeploymentYAML []byte @@ -36,9 +42,10 @@ type eventHandler struct { func newEventHandler( gcName string, - statusUpdater status.Updater, + statusUpdater *status.Updater, k8sClient client.Client, staticModeDeploymentYAML []byte, + timeNow timeNowFunc, ) *eventHandler { return &eventHandler{ store: newStore(), @@ -48,13 +55,12 @@ func newEventHandler( k8sClient: k8sClient, staticModeDeploymentYAML: staticModeDeploymentYAML, gatewayNextID: 1, + timeNow: timeNow, } } func (h *eventHandler) setGatewayClassStatuses(ctx context.Context) { - statuses := status.GatewayAPIStatuses{ - GatewayClassStatuses: make(status.GatewayClassStatuses), - } + var reqs []status.UpdateRequest var gcExists bool @@ -74,17 +80,32 @@ func (h *eventHandler) setGatewayClassStatuses(ctx context.Context) { supportedVersionConds, _ := gatewayclass.ValidateCRDVersions(h.store.crdMetadata) conds = append(conds, supportedVersionConds...) - statuses.GatewayClassStatuses[nsname] = status.GatewayClassStatus{ - Conditions: conditions.DeduplicateConditions(conds), - ObservedGeneration: gc.Generation, - } + reqs = append(reqs, status.UpdateRequest{ + NsName: nsname, + ResourceType: &gatewayv1.GatewayClass{}, + Setter: func(obj client.Object) bool { + gc := helpers.MustCastObject[*gatewayv1.GatewayClass](obj) + + gcs := gatewayv1.GatewayClassStatus{ + Conditions: conditions.ConvertConditions(conditions.DeduplicateConditions(conds), gc.Generation, h.timeNow()), + } + + if status.ConditionsEqual(gc.Status.Conditions, gcs.Conditions) { + return false + } + + gc.Status = gcs + + return true + }, + }) } if !gcExists { panic(fmt.Errorf("GatewayClass %s must exist", h.gcName)) } - h.statusUpdater.Update(ctx, statuses) + h.statusUpdater.Update(ctx, reqs...) } func (h *eventHandler) ensureDeploymentsMatchGateways(ctx context.Context, logger logr.Logger) { diff --git a/internal/mode/provisioner/handler_test.go b/internal/mode/provisioner/handler_test.go index d31e36a872..fa789b62d7 100644 --- a/internal/mode/provisioner/handler_test.go +++ b/internal/mode/provisioner/handler_test.go @@ -23,7 +23,6 @@ import ( "github.com/nginxinc/nginx-gateway-fabric/internal/framework/gatewayclass" "github.com/nginxinc/nginx-gateway-fabric/internal/framework/helpers" "github.com/nginxinc/nginx-gateway-fabric/internal/framework/status" - "github.com/nginxinc/nginx-gateway-fabric/internal/framework/status/statusfakes" ) var _ = Describe("handler", func() { @@ -31,13 +30,14 @@ var _ = Describe("handler", func() { gcName = "test-gc" ) var ( - handler *eventHandler - fakeClockTime metav1.Time + handler *eventHandler - statusUpdater status.Updater + statusUpdater *status.Updater k8sclient client.Client crd *metav1.PartialObjectMetadata gc *gatewayv1.GatewayClass + + fakeTimeNow timeNowFunc ) BeforeEach(OncePerOrdered, func() { @@ -55,18 +55,12 @@ var _ = Describe("handler", func() { ). Build() - fakeClockTime = helpers.PrepareTimeForFakeClient(metav1.Now()) - fakeClock := &statusfakes.FakeClock{} - fakeClock.NowReturns(fakeClockTime) - - statusUpdater = status.NewUpdater(status.UpdaterConfig{ - Client: k8sclient, - Clock: fakeClock, - Logger: zap.New(), - GatewayCtlrName: "test.example.com", - GatewayClassName: gcName, - UpdateGatewayClassStatus: true, - }) + fakeTime := helpers.PrepareTimeForFakeClient(metav1.Now()) + fakeTimeNow = func() metav1.Time { + return fakeTime + } + + statusUpdater = status.NewUpdater(k8sclient, zap.New()) // Add GatewayClass CRD to the cluster crd = &metav1.PartialObjectMetadata{ @@ -134,7 +128,7 @@ var _ = Describe("handler", func() { Type: string(gatewayv1.GatewayClassConditionStatusAccepted), Status: metav1.ConditionTrue, ObservedGeneration: 0, - LastTransitionTime: fakeClockTime, + LastTransitionTime: fakeTimeNow(), Reason: "Accepted", Message: "GatewayClass is accepted", }, @@ -142,7 +136,7 @@ var _ = Describe("handler", func() { Type: string(gatewayv1.GatewayClassReasonSupportedVersion), Status: metav1.ConditionTrue, ObservedGeneration: 0, - LastTransitionTime: fakeClockTime, + LastTransitionTime: fakeTimeNow(), Reason: "SupportedVersion", Message: "Gateway API CRD versions are supported", }, @@ -207,7 +201,7 @@ var _ = Describe("handler", func() { Type: string(gatewayv1.GatewayClassConditionStatusAccepted), Status: metav1.ConditionFalse, ObservedGeneration: 0, - LastTransitionTime: fakeClockTime, + LastTransitionTime: fakeTimeNow(), Reason: string(gatewayv1.GatewayClassReasonUnsupportedVersion), Message: fmt.Sprintf("Gateway API CRD versions are not supported. "+ "Please install version %s", gatewayclass.SupportedVersion), @@ -216,7 +210,7 @@ var _ = Describe("handler", func() { Type: string(gatewayv1.GatewayClassReasonSupportedVersion), Status: metav1.ConditionFalse, ObservedGeneration: 0, - LastTransitionTime: fakeClockTime, + LastTransitionTime: fakeTimeNow(), Reason: string(gatewayv1.GatewayClassReasonUnsupportedVersion), Message: fmt.Sprintf("Gateway API CRD versions are not supported. "+ "Please install version %s", gatewayclass.SupportedVersion), @@ -228,7 +222,7 @@ var _ = Describe("handler", func() { Type: string(gatewayv1.GatewayClassConditionStatusAccepted), Status: metav1.ConditionTrue, ObservedGeneration: 0, - LastTransitionTime: fakeClockTime, + LastTransitionTime: fakeTimeNow(), Reason: string(gatewayv1.GatewayClassReasonAccepted), Message: "GatewayClass is accepted", }, @@ -236,7 +230,7 @@ var _ = Describe("handler", func() { Type: string(gatewayv1.GatewayClassReasonSupportedVersion), Status: metav1.ConditionFalse, ObservedGeneration: 0, - LastTransitionTime: fakeClockTime, + LastTransitionTime: fakeTimeNow(), Reason: string(gatewayv1.GatewayClassReasonUnsupportedVersion), Message: fmt.Sprintf("Gateway API CRD versions are not recommended. "+ "Recommended version is %s", gatewayclass.SupportedVersion), @@ -279,6 +273,7 @@ var _ = Describe("handler", func() { statusUpdater, k8sclient, embeddedfiles.StaticModeDeploymentYAML, + fakeTimeNow, ) }) @@ -408,7 +403,7 @@ var _ = Describe("handler", func() { Type: string(gatewayv1.GatewayClassReasonSupportedVersion), Status: metav1.ConditionTrue, ObservedGeneration: 0, - LastTransitionTime: fakeClockTime, + LastTransitionTime: fakeTimeNow(), Reason: "SupportedVersion", Message: "Gateway API CRD versions are supported", }, @@ -416,7 +411,7 @@ var _ = Describe("handler", func() { Type: string(gatewayv1.GatewayClassConditionStatusAccepted), Status: metav1.ConditionFalse, ObservedGeneration: 0, - LastTransitionTime: fakeClockTime, + LastTransitionTime: fakeTimeNow(), Reason: string(conditions.GatewayClassReasonGatewayClassConflict), Message: conditions.GatewayClassMessageGatewayClassConflict, }, @@ -452,6 +447,7 @@ var _ = Describe("handler", func() { statusUpdater, k8sclient, embeddedfiles.StaticModeDeploymentYAML, + fakeTimeNow, ) }) @@ -563,6 +559,7 @@ var _ = Describe("handler", func() { statusUpdater, k8sclient, []byte("broken YAML"), + fakeTimeNow, ) itShouldUpsertGatewayClass() diff --git a/internal/mode/provisioner/manager.go b/internal/mode/provisioner/manager.go index 1bca844bac..bbef8c653b 100644 --- a/internal/mode/provisioner/manager.go +++ b/internal/mode/provisioner/manager.go @@ -122,13 +122,8 @@ func StartManager(cfg Config) error { ) statusUpdater := status.NewUpdater( - status.UpdaterConfig{ - Client: mgr.GetClient(), - Clock: status.NewRealClock(), - Logger: cfg.Logger.WithName("statusUpdater"), - GatewayClassName: cfg.GatewayClassName, - UpdateGatewayClassStatus: true, - }, + mgr.GetClient(), + cfg.Logger.WithName("statusUpdater"), ) handler := newEventHandler( @@ -136,6 +131,7 @@ func StartManager(cfg Config) error { statusUpdater, mgr.GetClient(), embeddedfiles.StaticModeDeploymentYAML, + func() metav1.Time { return metav1.Now() }, ) eventLoop := events.NewEventLoop( diff --git a/internal/mode/static/build_statuses.go b/internal/mode/static/build_statuses.go deleted file mode 100644 index 6055d9fdd1..0000000000 --- a/internal/mode/static/build_statuses.go +++ /dev/null @@ -1,215 +0,0 @@ -package static - -import ( - "k8s.io/apimachinery/pkg/types" - "sigs.k8s.io/controller-runtime/pkg/client" - v1 "sigs.k8s.io/gateway-api/apis/v1" - - "github.com/nginxinc/nginx-gateway-fabric/internal/framework/conditions" - "github.com/nginxinc/nginx-gateway-fabric/internal/framework/status" - staticConds "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/state/conditions" - "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/state/graph" -) - -type nginxReloadResult struct { - error error -} - -// buildGatewayAPIStatuses builds status.Statuses from a Graph. -func buildGatewayAPIStatuses( - graph *graph.Graph, - gwAddresses []v1.GatewayStatusAddress, - nginxReloadRes nginxReloadResult, -) status.GatewayAPIStatuses { - statuses := status.GatewayAPIStatuses{ - HTTPRouteStatuses: make(status.HTTPRouteStatuses), - } - - statuses.GatewayClassStatuses = buildGatewayClassStatuses(graph.GatewayClass, graph.IgnoredGatewayClasses) - - statuses.GatewayStatuses = buildGatewayStatuses(graph.Gateway, graph.IgnoredGateways, gwAddresses, nginxReloadRes) - - statuses.BackendTLSPolicyStatuses = buildBackendTLSPolicyStatuses(graph.BackendTLSPolicies) - - for nsname, r := range graph.Routes { - parentStatuses := make([]status.ParentStatus, 0, len(r.ParentRefs)) - - defaultConds := staticConds.NewDefaultRouteConditions() - - for _, ref := range r.ParentRefs { - failedAttachmentCondCount := 0 - if ref.Attachment != nil && !ref.Attachment.Attached { - failedAttachmentCondCount = 1 - } - allConds := make([]conditions.Condition, 0, len(r.Conditions)+len(defaultConds)+failedAttachmentCondCount) - - // We add defaultConds first, so that any additional conditions will override them, which is - // ensured by DeduplicateConditions. - allConds = append(allConds, defaultConds...) - allConds = append(allConds, r.Conditions...) - if failedAttachmentCondCount == 1 { - allConds = append(allConds, ref.Attachment.FailedCondition) - } - - if nginxReloadRes.error != nil { - allConds = append( - allConds, - staticConds.NewRouteGatewayNotProgrammed(staticConds.RouteMessageFailedNginxReload), - ) - } - - routeRef := r.Source.Spec.ParentRefs[ref.Idx] - - parentStatuses = append(parentStatuses, status.ParentStatus{ - GatewayNsName: ref.Gateway, - SectionName: routeRef.SectionName, - Conditions: conditions.DeduplicateConditions(allConds), - }) - } - - statuses.HTTPRouteStatuses[nsname] = status.HTTPRouteStatus{ - ObservedGeneration: r.Source.Generation, - ParentStatuses: parentStatuses, - } - } - - return statuses -} - -func buildGatewayClassStatuses( - gc *graph.GatewayClass, - ignoredGwClasses map[types.NamespacedName]*v1.GatewayClass, -) status.GatewayClassStatuses { - statuses := make(status.GatewayClassStatuses) - - if gc != nil { - defaultConds := conditions.NewDefaultGatewayClassConditions() - - conds := make([]conditions.Condition, 0, len(gc.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, gc.Conditions...) - - statuses[client.ObjectKeyFromObject(gc.Source)] = status.GatewayClassStatus{ - Conditions: conditions.DeduplicateConditions(conds), - ObservedGeneration: gc.Source.Generation, - } - } - - for nsname, gwClass := range ignoredGwClasses { - statuses[nsname] = status.GatewayClassStatus{ - Conditions: []conditions.Condition{conditions.NewGatewayClassConflict()}, - ObservedGeneration: gwClass.Generation, - } - } - - return statuses -} - -func buildGatewayStatuses( - gateway *graph.Gateway, - ignoredGateways map[types.NamespacedName]*v1.Gateway, - gwAddresses []v1.GatewayStatusAddress, - nginxReloadRes nginxReloadResult, -) status.GatewayStatuses { - statuses := make(status.GatewayStatuses) - - if gateway != nil { - statuses[client.ObjectKeyFromObject(gateway.Source)] = buildGatewayStatus(gateway, gwAddresses, nginxReloadRes) - } - - for nsname, gw := range ignoredGateways { - statuses[nsname] = status.GatewayStatus{ - Conditions: staticConds.NewGatewayConflict(), - ObservedGeneration: gw.Generation, - Ignored: true, - } - } - - return statuses -} - -func buildGatewayStatus( - gateway *graph.Gateway, - gwAddresses []v1.GatewayStatusAddress, - nginxReloadRes nginxReloadResult, -) status.GatewayStatus { - if !gateway.Valid { - return status.GatewayStatus{ - Conditions: conditions.DeduplicateConditions(gateway.Conditions), - ObservedGeneration: gateway.Source.Generation, - } - } - - listenerStatuses := make([]status.ListenerStatus, 0, len(gateway.Listeners)) - - validListenerCount := 0 - for _, l := range gateway.Listeners { - var conds []conditions.Condition - - if l.Valid { - conds = staticConds.NewDefaultListenerConditions() - validListenerCount++ - } else { - conds = l.Conditions - } - - if nginxReloadRes.error != nil { - conds = append( - conds, - staticConds.NewListenerNotProgrammedInvalid(staticConds.ListenerMessageFailedNginxReload), - ) - } - - listenerStatuses = append(listenerStatuses, status.ListenerStatus{ - Name: v1.SectionName(l.Name), - AttachedRoutes: int32(len(l.Routes)), - Conditions: conditions.DeduplicateConditions(conds), - SupportedKinds: l.SupportedKinds, - }) - } - - gwConds := staticConds.NewDefaultGatewayConditions() - if validListenerCount == 0 { - gwConds = append(gwConds, staticConds.NewGatewayNotAcceptedListenersNotValid()...) - } else if validListenerCount < len(gateway.Listeners) { - gwConds = append(gwConds, staticConds.NewGatewayAcceptedListenersNotValid()) - } - - if nginxReloadRes.error != nil { - gwConds = append( - gwConds, - staticConds.NewGatewayNotProgrammedInvalid(staticConds.GatewayMessageFailedNginxReload), - ) - } - - return status.GatewayStatus{ - Conditions: conditions.DeduplicateConditions(gwConds), - ListenerStatuses: listenerStatuses, - Addresses: gwAddresses, - ObservedGeneration: gateway.Source.Generation, - } -} - -func buildBackendTLSPolicyStatuses(backendTLSPolicies map[types.NamespacedName]*graph.BackendTLSPolicy, -) status.BackendTLSPolicyStatuses { - statuses := make(status.BackendTLSPolicyStatuses, len(backendTLSPolicies)) - - for nsname, backendTLSPolicy := range backendTLSPolicies { - if backendTLSPolicy.IsReferenced { - if !backendTLSPolicy.Ignored { - statuses[nsname] = status.BackendTLSPolicyStatus{ - AncestorStatuses: []status.AncestorStatus{ - { - GatewayNsName: backendTLSPolicy.Gateway, - Conditions: conditions.DeduplicateConditions(backendTLSPolicy.Conditions), - }, - }, - } - } - } - } - return statuses -} diff --git a/internal/mode/static/build_statuses_test.go b/internal/mode/static/build_statuses_test.go deleted file mode 100644 index 146b34409b..0000000000 --- a/internal/mode/static/build_statuses_test.go +++ /dev/null @@ -1,746 +0,0 @@ -package static - -import ( - "errors" - "testing" - - . "github.com/onsi/gomega" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/types" - "sigs.k8s.io/controller-runtime/pkg/client" - v1 "sigs.k8s.io/gateway-api/apis/v1" - v1alpha2 "sigs.k8s.io/gateway-api/apis/v1alpha2" - - "github.com/nginxinc/nginx-gateway-fabric/internal/framework/conditions" - "github.com/nginxinc/nginx-gateway-fabric/internal/framework/helpers" - "github.com/nginxinc/nginx-gateway-fabric/internal/framework/status" - staticConds "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/state/conditions" - "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/state/graph" -) - -var ( - gw = &v1.Gateway{ - ObjectMeta: metav1.ObjectMeta{ - Namespace: "test", - Name: "gateway", - Generation: 2, - }, - } - - ignoredGw = &v1.Gateway{ - ObjectMeta: metav1.ObjectMeta{ - Namespace: "test", - Name: "ignored-gateway", - Generation: 1, - }, - } -) - -func TestBuildStatuses(t *testing.T) { - addr := []v1.GatewayStatusAddress{ - { - Type: helpers.GetPointer(v1.IPAddressType), - Value: "1.2.3.4", - }, - } - - 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"}: { - Valid: true, - Source: &v1.HTTPRoute{ - ObjectMeta: metav1.ObjectMeta{ - Generation: 3, - }, - Spec: v1.HTTPRouteSpec{ - CommonRouteSpec: v1.CommonRouteSpec{ - ParentRefs: []v1.ParentReference{ - { - SectionName: helpers.GetPointer[v1.SectionName]("listener-80-1"), - }, - { - SectionName: helpers.GetPointer[v1.SectionName]("listener-80-2"), - }, - }, - }, - }, - }, - ParentRefs: []graph.ParentRef{ - { - Idx: 0, - Gateway: client.ObjectKeyFromObject(gw), - Attachment: &graph.ParentRefAttachmentStatus{ - Attached: true, - }, - }, - { - Idx: 1, - Gateway: client.ObjectKeyFromObject(gw), - Attachment: &graph.ParentRefAttachmentStatus{ - Attached: false, - FailedCondition: invalidAttachmentCondition, - }, - }, - }, - }, - {Namespace: "test", Name: "hr-invalid"}: { - Valid: false, - Conditions: []conditions.Condition{invalidRouteCondition}, - Source: &v1.HTTPRoute{ - ObjectMeta: metav1.ObjectMeta{ - Generation: 3, - }, - Spec: v1.HTTPRouteSpec{ - CommonRouteSpec: v1.CommonRouteSpec{ - ParentRefs: []v1.ParentReference{ - { - SectionName: helpers.GetPointer[v1.SectionName]("listener-80-1"), - }, - }, - }, - }, - }, - ParentRefs: []graph.ParentRef{ - { - Idx: 0, - Gateway: client.ObjectKeyFromObject(gw), - Attachment: nil, - }, - }, - }, - } - - graph := &graph.Graph{ - GatewayClass: &graph.GatewayClass{ - Source: &v1.GatewayClass{ - ObjectMeta: metav1.ObjectMeta{Generation: 1}, - }, - Valid: true, - }, - Gateway: &graph.Gateway{ - Source: gw, - Listeners: []*graph.Listener{ - { - Name: "listener-80-1", - Valid: true, - Routes: map[types.NamespacedName]*graph.Route{ - {Namespace: "test", Name: "hr-1"}: {}, - }, - }, - }, - Valid: true, - }, - IgnoredGateways: map[types.NamespacedName]*v1.Gateway{ - client.ObjectKeyFromObject(ignoredGw): ignoredGw, - }, - Routes: routes, - } - - expected := status.GatewayAPIStatuses{ - GatewayClassStatuses: status.GatewayClassStatuses{ - {Name: ""}: { - ObservedGeneration: 1, - Conditions: conditions.NewDefaultGatewayClassConditions(), - }, - }, - GatewayStatuses: status.GatewayStatuses{ - {Namespace: "test", Name: "gateway"}: { - Conditions: staticConds.NewDefaultGatewayConditions(), - ListenerStatuses: []status.ListenerStatus{ - { - Name: "listener-80-1", - AttachedRoutes: 1, - Conditions: staticConds.NewDefaultListenerConditions(), - }, - }, - Addresses: addr, - ObservedGeneration: 2, - }, - {Namespace: "test", Name: "ignored-gateway"}: { - Conditions: staticConds.NewGatewayConflict(), - ObservedGeneration: 1, - Ignored: true, - }, - }, - HTTPRouteStatuses: status.HTTPRouteStatuses{ - {Namespace: "test", Name: "hr-valid"}: { - ObservedGeneration: 3, - ParentStatuses: []status.ParentStatus{ - { - GatewayNsName: client.ObjectKeyFromObject(gw), - SectionName: helpers.GetPointer[v1.SectionName]("listener-80-1"), - Conditions: staticConds.NewDefaultRouteConditions(), - }, - { - GatewayNsName: client.ObjectKeyFromObject(gw), - SectionName: helpers.GetPointer[v1.SectionName]("listener-80-2"), - Conditions: append( - staticConds.NewDefaultRouteConditions(), - invalidAttachmentCondition, - ), - }, - }, - }, - {Namespace: "test", Name: "hr-invalid"}: { - ObservedGeneration: 3, - ParentStatuses: []status.ParentStatus{ - { - GatewayNsName: client.ObjectKeyFromObject(gw), - SectionName: helpers.GetPointer[v1.SectionName]("listener-80-1"), - Conditions: append( - staticConds.NewDefaultRouteConditions(), - invalidRouteCondition, - ), - }, - }, - }, - }, - BackendTLSPolicyStatuses: status.BackendTLSPolicyStatuses{}, - } - - g := NewWithT(t) - - var nginxReloadRes nginxReloadResult - result := buildGatewayAPIStatuses(graph, addr, nginxReloadRes) - g.Expect(helpers.Diff(expected, result)).To(BeEmpty()) -} - -func TestBuildStatusesNginxErr(t *testing.T) { - addr := []v1.GatewayStatusAddress{ - { - Type: helpers.GetPointer(v1.IPAddressType), - Value: "1.2.3.4", - }, - } - - routes := map[types.NamespacedName]*graph.Route{ - {Namespace: "test", Name: "hr-valid"}: { - Valid: true, - Source: &v1.HTTPRoute{ - ObjectMeta: metav1.ObjectMeta{ - Generation: 3, - }, - Spec: v1.HTTPRouteSpec{ - CommonRouteSpec: v1.CommonRouteSpec{ - ParentRefs: []v1.ParentReference{ - { - SectionName: helpers.GetPointer[v1.SectionName]("listener-80-1"), - }, - }, - }, - }, - }, - ParentRefs: []graph.ParentRef{ - { - Idx: 0, - Gateway: client.ObjectKeyFromObject(gw), - Attachment: &graph.ParentRefAttachmentStatus{ - Attached: true, - }, - }, - }, - }, - } - - graph := &graph.Graph{ - Gateway: &graph.Gateway{ - Source: gw, - Listeners: []*graph.Listener{ - { - Name: "listener-80-1", - Valid: true, - Routes: map[types.NamespacedName]*graph.Route{ - {Namespace: "test", Name: "hr-1"}: {}, - }, - }, - }, - Valid: true, - }, - Routes: routes, - } - - expected := status.GatewayAPIStatuses{ - GatewayClassStatuses: status.GatewayClassStatuses{}, - GatewayStatuses: status.GatewayStatuses{ - {Namespace: "test", Name: "gateway"}: { - Conditions: []conditions.Condition{ - staticConds.NewGatewayAccepted(), - staticConds.NewGatewayNotProgrammedInvalid(staticConds.GatewayMessageFailedNginxReload), - }, - ListenerStatuses: []status.ListenerStatus{ - { - Name: "listener-80-1", - AttachedRoutes: 1, - Conditions: []conditions.Condition{ - staticConds.NewListenerAccepted(), - staticConds.NewListenerResolvedRefs(), - staticConds.NewListenerNoConflicts(), - staticConds.NewListenerNotProgrammedInvalid(staticConds.ListenerMessageFailedNginxReload), - }, - }, - }, - Addresses: addr, - ObservedGeneration: 2, - }, - }, - HTTPRouteStatuses: status.HTTPRouteStatuses{ - {Namespace: "test", Name: "hr-valid"}: { - ObservedGeneration: 3, - ParentStatuses: []status.ParentStatus{ - { - GatewayNsName: client.ObjectKeyFromObject(gw), - SectionName: helpers.GetPointer[v1.SectionName]("listener-80-1"), - Conditions: []conditions.Condition{ - staticConds.NewRouteResolvedRefs(), - staticConds.NewRouteGatewayNotProgrammed(staticConds.RouteMessageFailedNginxReload), - }, - }, - }, - }, - }, - BackendTLSPolicyStatuses: status.BackendTLSPolicyStatuses{}, - } - - g := NewWithT(t) - - nginxReloadRes := nginxReloadResult{error: errors.New("test error")} - result := buildGatewayAPIStatuses(graph, addr, nginxReloadRes) - g.Expect(helpers.Diff(expected, result)).To(BeEmpty()) -} - -func TestBuildGatewayClassStatuses(t *testing.T) { - tests := []struct { - gc *graph.GatewayClass - ignoredClasses map[types.NamespacedName]*v1.GatewayClass - expected status.GatewayClassStatuses - name string - }{ - { - name: "nil gatewayclass and no ignored gatewayclasses", - expected: status.GatewayClassStatuses{}, - }, - { - name: "nil gatewayclass and ignored gatewayclasses", - ignoredClasses: map[types.NamespacedName]*v1.GatewayClass{ - {Name: "ignored-1"}: { - ObjectMeta: metav1.ObjectMeta{ - Generation: 1, - }, - }, - {Name: "ignored-2"}: { - ObjectMeta: metav1.ObjectMeta{ - Generation: 2, - }, - }, - }, - expected: status.GatewayClassStatuses{ - {Name: "ignored-1"}: { - Conditions: []conditions.Condition{conditions.NewGatewayClassConflict()}, - ObservedGeneration: 1, - }, - {Name: "ignored-2"}: { - Conditions: []conditions.Condition{conditions.NewGatewayClassConflict()}, - ObservedGeneration: 2, - }, - }, - }, - { - name: "valid gatewayclass", - gc: &graph.GatewayClass{ - Source: &v1.GatewayClass{ - ObjectMeta: metav1.ObjectMeta{ - Name: "valid-gc", - Generation: 1, - }, - }, - }, - expected: status.GatewayClassStatuses{ - {Name: "valid-gc"}: { - Conditions: conditions.NewDefaultGatewayClassConditions(), - ObservedGeneration: 1, - }, - }, - }, - } - - for _, test := range tests { - t.Run(test.name, func(t *testing.T) { - g := NewWithT(t) - - result := buildGatewayClassStatuses(test.gc, test.ignoredClasses) - g.Expect(helpers.Diff(test.expected, result)).To(BeEmpty()) - }) - } -} - -func TestBuildGatewayStatuses(t *testing.T) { - addr := []v1.GatewayStatusAddress{ - { - Type: helpers.GetPointer(v1.IPAddressType), - Value: "1.2.3.4", - }, - } - - tests := []struct { - nginxReloadRes nginxReloadResult - gateway *graph.Gateway - ignoredGateways map[types.NamespacedName]*v1.Gateway - expected status.GatewayStatuses - name string - }{ - { - name: "nil gateway and no ignored gateways", - expected: status.GatewayStatuses{}, - }, - { - name: "nil gateway and ignored gateways", - ignoredGateways: map[types.NamespacedName]*v1.Gateway{ - {Namespace: "test", Name: "ignored-1"}: { - ObjectMeta: metav1.ObjectMeta{ - Generation: 1, - }, - }, - {Namespace: "test", Name: "ignored-2"}: { - ObjectMeta: metav1.ObjectMeta{ - Generation: 2, - }, - }, - }, - expected: status.GatewayStatuses{ - {Namespace: "test", Name: "ignored-1"}: { - Conditions: staticConds.NewGatewayConflict(), - ObservedGeneration: 1, - Ignored: true, - }, - {Namespace: "test", Name: "ignored-2"}: { - Conditions: staticConds.NewGatewayConflict(), - ObservedGeneration: 2, - Ignored: true, - }, - }, - }, - { - name: "valid gateway; all valid listeners", - gateway: &graph.Gateway{ - Source: gw, - Listeners: []*graph.Listener{ - { - Name: "listener-valid-1", - Valid: true, - Routes: map[types.NamespacedName]*graph.Route{ - {Namespace: "test", Name: "hr-1"}: {}, - }, - }, - { - Name: "listener-valid-2", - Valid: true, - Routes: map[types.NamespacedName]*graph.Route{ - {Namespace: "test", Name: "hr-1"}: {}, - }, - }, - }, - Valid: true, - }, - expected: status.GatewayStatuses{ - {Namespace: "test", Name: "gateway"}: { - Conditions: staticConds.NewDefaultGatewayConditions(), - ListenerStatuses: []status.ListenerStatus{ - { - Name: "listener-valid-1", - AttachedRoutes: 1, - Conditions: staticConds.NewDefaultListenerConditions(), - }, - { - Name: "listener-valid-2", - AttachedRoutes: 1, - Conditions: staticConds.NewDefaultListenerConditions(), - }, - }, - Addresses: addr, - ObservedGeneration: 2, - }, - }, - }, - { - name: "valid gateway; some valid listeners", - gateway: &graph.Gateway{ - Source: gw, - Listeners: []*graph.Listener{ - { - Name: "listener-valid", - Valid: true, - Routes: map[types.NamespacedName]*graph.Route{ - {Namespace: "test", Name: "hr-1"}: {}, - }, - }, - { - Name: "listener-invalid", - Valid: false, - Conditions: staticConds.NewListenerUnsupportedValue("unsupported value"), - }, - }, - Valid: true, - }, - expected: status.GatewayStatuses{ - {Namespace: "test", Name: "gateway"}: { - Conditions: []conditions.Condition{ - staticConds.NewGatewayProgrammed(), - staticConds.NewGatewayAcceptedListenersNotValid(), - }, - ListenerStatuses: []status.ListenerStatus{ - { - Name: "listener-valid", - AttachedRoutes: 1, - Conditions: staticConds.NewDefaultListenerConditions(), - }, - { - Name: "listener-invalid", - Conditions: staticConds.NewListenerUnsupportedValue("unsupported value"), - }, - }, - Addresses: addr, - ObservedGeneration: 2, - }, - }, - }, - { - name: "valid gateway; no valid listeners", - gateway: &graph.Gateway{ - Source: gw, - Listeners: []*graph.Listener{ - { - Name: "listener-invalid-1", - Valid: false, - Conditions: staticConds.NewListenerUnsupportedProtocol("unsupported protocol"), - }, - { - Name: "listener-invalid-2", - Valid: false, - Conditions: staticConds.NewListenerUnsupportedValue("unsupported value"), - }, - }, - Valid: true, - }, - expected: status.GatewayStatuses{ - {Namespace: "test", Name: "gateway"}: { - Conditions: staticConds.NewGatewayNotAcceptedListenersNotValid(), - ListenerStatuses: []status.ListenerStatus{ - { - Name: "listener-invalid-1", - Conditions: staticConds.NewListenerUnsupportedProtocol("unsupported protocol"), - }, - { - Name: "listener-invalid-2", - Conditions: staticConds.NewListenerUnsupportedValue("unsupported value"), - }, - }, - Addresses: addr, - ObservedGeneration: 2, - }, - }, - }, - { - name: "invalid gateway", - gateway: &graph.Gateway{ - Source: gw, - Valid: false, - Conditions: staticConds.NewGatewayInvalid("no gateway class"), - }, - expected: status.GatewayStatuses{ - {Namespace: "test", Name: "gateway"}: { - Conditions: staticConds.NewGatewayInvalid("no gateway class"), - ObservedGeneration: 2, - }, - }, - }, - { - name: "error reloading nginx; gateway/listener not programmed", - gateway: &graph.Gateway{ - Source: gw, - Valid: true, - Conditions: staticConds.NewDefaultGatewayConditions(), - Listeners: []*graph.Listener{ - { - Name: "listener-valid", - Valid: true, - Routes: map[types.NamespacedName]*graph.Route{ - {Namespace: "test", Name: "hr-1"}: {}, - }, - }, - }, - }, - expected: status.GatewayStatuses{ - {Namespace: "test", Name: "gateway"}: { - Conditions: []conditions.Condition{ - staticConds.NewGatewayAccepted(), - staticConds.NewGatewayNotProgrammedInvalid(staticConds.GatewayMessageFailedNginxReload), - }, - ListenerStatuses: []status.ListenerStatus{ - { - Name: "listener-valid", - AttachedRoutes: 1, - Conditions: []conditions.Condition{ - staticConds.NewListenerAccepted(), - staticConds.NewListenerResolvedRefs(), - staticConds.NewListenerNoConflicts(), - staticConds.NewListenerNotProgrammedInvalid( - staticConds.ListenerMessageFailedNginxReload, - ), - }, - }, - }, - Addresses: addr, - ObservedGeneration: 2, - }, - }, - nginxReloadRes: nginxReloadResult{error: errors.New("test error")}, - }, - } - - for _, test := range tests { - t.Run(test.name, func(t *testing.T) { - g := NewWithT(t) - - result := buildGatewayStatuses(test.gateway, test.ignoredGateways, addr, test.nginxReloadRes) - g.Expect(helpers.Diff(test.expected, result)).To(BeEmpty()) - }) - } -} - -func TestBuildBackendTLSPolicyStatuses(t *testing.T) { - type policyCfg struct { - Name string - Conditions []conditions.Condition - Valid bool - Ignored bool - IsReferenced bool - } - - getBackendTLSPolicy := func(policyCfg policyCfg) *graph.BackendTLSPolicy { - return &graph.BackendTLSPolicy{ - Source: &v1alpha2.BackendTLSPolicy{ - ObjectMeta: metav1.ObjectMeta{ - Namespace: "test", - Name: policyCfg.Name, - Generation: 1, - }, - }, - Valid: policyCfg.Valid, - Ignored: policyCfg.Ignored, - IsReferenced: policyCfg.IsReferenced, - Conditions: policyCfg.Conditions, - Gateway: types.NamespacedName{Name: "gateway", Namespace: "test"}, - } - } - - attachedConds := []conditions.Condition{staticConds.NewBackendTLSPolicyAccepted()} - invalidConds := []conditions.Condition{staticConds.NewBackendTLSPolicyInvalid("invalid backendTLSPolicy")} - - validPolicyCfg := policyCfg{ - Name: "valid-bt", - Valid: true, - IsReferenced: true, - Conditions: attachedConds, - } - - invalidPolicyCfg := policyCfg{ - Name: "invalid-bt", - IsReferenced: true, - Conditions: invalidConds, - } - - ignoredPolicyCfg := policyCfg{ - Name: "ignored-bt", - Ignored: true, - IsReferenced: true, - } - - notReferencedPolicyCfg := policyCfg{ - Name: "not-referenced", - Valid: true, - } - - tests := []struct { - backendTLSPolicies map[types.NamespacedName]*graph.BackendTLSPolicy - expected status.BackendTLSPolicyStatuses - name string - }{ - { - name: "nil backendTLSPolicies", - expected: status.BackendTLSPolicyStatuses{}, - }, - { - name: "valid backendTLSPolicy", - backendTLSPolicies: map[types.NamespacedName]*graph.BackendTLSPolicy{ - {Namespace: "test", Name: "valid-bt"}: getBackendTLSPolicy(validPolicyCfg), - }, - expected: status.BackendTLSPolicyStatuses{ - {Namespace: "test", Name: "valid-bt"}: { - AncestorStatuses: []status.AncestorStatus{ - { - Conditions: attachedConds, - GatewayNsName: types.NamespacedName{Name: "gateway", Namespace: "test"}, - }, - }, - }, - }, - }, - { - name: "invalid backendTLSPolicy", - backendTLSPolicies: map[types.NamespacedName]*graph.BackendTLSPolicy{ - {Namespace: "test", Name: "invalid-bt"}: getBackendTLSPolicy(invalidPolicyCfg), - }, - expected: status.BackendTLSPolicyStatuses{ - {Namespace: "test", Name: "invalid-bt"}: { - AncestorStatuses: []status.AncestorStatus{ - { - Conditions: invalidConds, - GatewayNsName: types.NamespacedName{Name: "gateway", Namespace: "test"}, - }, - }, - }, - }, - }, - { - name: "ignored or not referenced backendTLSPolicies", - backendTLSPolicies: map[types.NamespacedName]*graph.BackendTLSPolicy{ - {Namespace: "test", Name: "ignored-bt"}: getBackendTLSPolicy(ignoredPolicyCfg), - {Namespace: "test", Name: "not-referenced"}: getBackendTLSPolicy(notReferencedPolicyCfg), - }, - expected: status.BackendTLSPolicyStatuses{}, - }, - { - name: "mix valid and ignored backendTLSPolicies", - backendTLSPolicies: map[types.NamespacedName]*graph.BackendTLSPolicy{ - {Namespace: "test", Name: "ignored-bt"}: getBackendTLSPolicy(ignoredPolicyCfg), - {Namespace: "test", Name: "valid-bt"}: getBackendTLSPolicy(validPolicyCfg), - }, - expected: status.BackendTLSPolicyStatuses{ - {Namespace: "test", Name: "valid-bt"}: { - AncestorStatuses: []status.AncestorStatus{ - { - Conditions: attachedConds, - GatewayNsName: types.NamespacedName{Name: "gateway", Namespace: "test"}, - }, - }, - }, - }, - }, - } - for _, test := range tests { - t.Run(test.name, func(t *testing.T) { - g := NewWithT(t) - - result := buildBackendTLSPolicyStatuses(test.backendTLSPolicies) - g.Expect(helpers.Diff(test.expected, result)).To(BeEmpty()) - }) - } -} diff --git a/internal/mode/static/handler.go b/internal/mode/static/handler.go index 430b30474f..03480bb9d0 100644 --- a/internal/mode/static/handler.go +++ b/internal/mode/static/handler.go @@ -10,25 +10,26 @@ import ( ngxclient "github.com/nginxinc/nginx-plus-go-client/client" apiv1 "k8s.io/api/core/v1" v1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" "k8s.io/client-go/tools/record" "sigs.k8s.io/controller-runtime/pkg/client" gatewayv1 "sigs.k8s.io/gateway-api/apis/v1" ngfAPI "github.com/nginxinc/nginx-gateway-fabric/apis/v1alpha1" - "github.com/nginxinc/nginx-gateway-fabric/internal/framework/conditions" "github.com/nginxinc/nginx-gateway-fabric/internal/framework/events" "github.com/nginxinc/nginx-gateway-fabric/internal/framework/helpers" - "github.com/nginxinc/nginx-gateway-fabric/internal/framework/status" + frameworkStatus "github.com/nginxinc/nginx-gateway-fabric/internal/framework/status" "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/config" ngfConfig "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/config" ngxConfig "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/nginx/config" "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/nginx/file" "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/nginx/runtime" "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/state" - staticConds "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/state/conditions" "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/state/dataplane" + "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/state/graph" "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/state/resolver" + "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/status" ) type handlerMetricsCollector interface { @@ -47,6 +48,8 @@ type secretStorer interface { // eventHandlerConfig holds configuration parameters for eventHandlerImpl. type eventHandlerConfig struct { + // gatewayCtlrName is the name of the NGF controller. + gatewayCtlrName string // k8sClient is a Kubernetes API client k8sClient client.Client // gatewayPodConfig contains information about this Pod. @@ -66,7 +69,7 @@ type eventHandlerConfig struct { // nginxRuntimeMgr manages nginx runtime. nginxRuntimeMgr runtime.Manager // statusUpdater updates statuses on Kubernetes resources. - statusUpdater status.Updater + statusUpdater frameworkStatus.GroupUpdater // eventRecorder records events for Kubernetes resources. eventRecorder record.EventRecorder // logLevelSetter is used to update the logging level. @@ -77,8 +80,17 @@ type eventHandlerConfig struct { nginxConfiguredOnStartChecker *nginxConfiguredOnStartChecker // controlConfigNSName is the NamespacedName of the NginxGateway config for this controller. controlConfigNSName types.NamespacedName + // updateGatewayClassStatus enables updating the status of the GatewayClass resource. + updateGatewayClassStatus bool } +const ( + // groups for GroupStatusUpdater + groupAllExceptGateways = "all-graphs-except-gateways" + groupGateways = "gateways" + groupControlPlane = "control-plane" +) + // filterKey is the `kind_namespace_name" of an object being filtered. type filterKey string @@ -103,6 +115,8 @@ type eventHandlerImpl struct { // objectFilters contains all created objectFilters, with the key being a filterKey objectFilters map[filterKey]objectFilter + latestReloadResult status.NginxReloadResult + cfg eventHandlerConfig lock sync.Mutex @@ -200,10 +214,10 @@ func (h *eventHandlerImpl) HandleEventBatch(ctx context.Context, logger logr.Log ) } - var nginxReloadRes nginxReloadResult + var nginxReloadRes status.NginxReloadResult if err != nil { logger.Error(err, "Failed to update NGINX configuration") - nginxReloadRes.error = err + nginxReloadRes.Error = err if !h.cfg.nginxConfiguredOnStartChecker.ready { h.cfg.nginxConfiguredOnStartChecker.firstBatchError = err } @@ -214,12 +228,43 @@ func (h *eventHandlerImpl) HandleEventBatch(ctx context.Context, logger logr.Log } } + h.latestReloadResult = nginxReloadRes + + h.updateStatuses(ctx, logger, graph) +} + +func (h *eventHandlerImpl) updateStatuses(ctx context.Context, logger logr.Logger, graph *graph.Graph) { gwAddresses, err := getGatewayAddresses(ctx, h.cfg.k8sClient, nil, h.cfg.gatewayPodConfig) if err != nil { logger.Error(err, "Setting GatewayStatusAddress to Pod IP Address") } - h.cfg.statusUpdater.Update(ctx, buildGatewayAPIStatuses(graph, gwAddresses, nginxReloadRes)) + transitionTime := metav1.Now() + + var gcReqs []frameworkStatus.UpdateRequest + if h.cfg.updateGatewayClassStatus { + gcReqs = status.PrepareGatewayClassRequests(graph.GatewayClass, graph.IgnoredGatewayClasses, transitionTime) + } + routeReqs := status.PrepareRouteRequests(graph.Routes, transitionTime, h.latestReloadResult, h.cfg.gatewayCtlrName) + polReqs := status.PrepareBackendTLSPolicyRequests(graph.BackendTLSPolicies, transitionTime, h.cfg.gatewayCtlrName) + + reqs := make([]frameworkStatus.UpdateRequest, 0, len(gcReqs)+len(routeReqs)+len(polReqs)) + reqs = append(reqs, gcReqs...) + reqs = append(reqs, routeReqs...) + reqs = append(reqs, polReqs...) + + h.cfg.statusUpdater.UpdateGroup(ctx, groupAllExceptGateways, reqs...) + + // We put Gateway status updates separately from the rest of the statuses because we want to be able + // to update them separately from the rest of the graph whenever the public IP of NGF changes. + gwReqs := status.PrepareGatewayRequests( + graph.Gateway, + graph.IgnoredGateways, + transitionTime, + gwAddresses, + h.latestReloadResult, + ) + h.cfg.statusUpdater.UpdateGroup(ctx, groupGateways, gwReqs...) } func (h *eventHandlerImpl) parseAndCaptureEvent(ctx context.Context, logger logr.Logger, event interface{}) { @@ -359,7 +404,8 @@ func (h *eventHandlerImpl) updateControlPlaneAndSetStatus( logger logr.Logger, cfg *ngfAPI.NginxGateway, ) { - var cond []conditions.Condition + var cpUpdateRes status.ControlPlaneUpdateResult + if err := updateControlPlane( cfg, logger, @@ -376,21 +422,19 @@ func (h *eventHandlerImpl) updateControlPlaneAndSetStatus( msg+": %s", err.Error(), ) - cond = []conditions.Condition{staticConds.NewNginxGatewayInvalid(fmt.Sprintf("%s: %v", msg, err))} - } else { - cond = []conditions.Condition{staticConds.NewNginxGatewayValid()} + cpUpdateRes.Error = err } - if cfg != nil { - nginxGatewayStatus := &status.NginxGatewayStatus{ - NsName: client.ObjectKeyFromObject(cfg), - Conditions: cond, - ObservedGeneration: cfg.Generation, - } + var reqs []frameworkStatus.UpdateRequest - h.cfg.statusUpdater.Update(ctx, nginxGatewayStatus) - logger.Info("Reconfigured control plane.") + req := status.PrepareNginxGatewayStatus(cfg, metav1.Now(), cpUpdateRes) + if req != nil { + reqs = append(reqs, *req) } + + h.cfg.statusUpdater.UpdateGroup(ctx, groupControlPlane, reqs...) + + logger.Info("Reconfigured control plane.") } // getGatewayAddresses gets the addresses for the Gateway. @@ -506,7 +550,20 @@ func (h *eventHandlerImpl) nginxGatewayServiceUpsert(ctx context.Context, logger logger.Error(err, "Setting GatewayStatusAddress to Pod IP Address") } - h.cfg.statusUpdater.UpdateAddresses(ctx, gwAddresses) + graph := h.cfg.processor.GetLatestGraph() + if graph == nil { + return + } + + transitionTime := metav1.Now() + gatewayStatuses := status.PrepareGatewayRequests( + graph.Gateway, + graph.IgnoredGateways, + transitionTime, + gwAddresses, + h.latestReloadResult, + ) + h.cfg.statusUpdater.UpdateGroup(ctx, groupGateways, gatewayStatuses...) } func (h *eventHandlerImpl) nginxGatewayServiceDelete( @@ -519,7 +576,20 @@ func (h *eventHandlerImpl) nginxGatewayServiceDelete( logger.Error(err, "Setting GatewayStatusAddress to Pod IP Address") } - h.cfg.statusUpdater.UpdateAddresses(ctx, gwAddresses) + graph := h.cfg.processor.GetLatestGraph() + if graph == nil { + return + } + + transitionTime := metav1.Now() + gatewayStatuses := status.PrepareGatewayRequests( + graph.Gateway, + graph.IgnoredGateways, + transitionTime, + gwAddresses, + h.latestReloadResult, + ) + h.cfg.statusUpdater.UpdateGroup(ctx, groupGateways, gatewayStatuses...) } func (h *eventHandlerImpl) nginxPlusUsageSecretUpsert(_ context.Context, _ logr.Logger, obj client.Object) { diff --git a/internal/mode/static/handler_test.go b/internal/mode/static/handler_test.go index b99ec28663..93ad4e1b61 100644 --- a/internal/mode/static/handler_test.go +++ b/internal/mode/static/handler_test.go @@ -19,10 +19,8 @@ import ( gatewayv1 "sigs.k8s.io/gateway-api/apis/v1" ngfAPI "github.com/nginxinc/nginx-gateway-fabric/apis/v1alpha1" - "github.com/nginxinc/nginx-gateway-fabric/internal/framework/conditions" "github.com/nginxinc/nginx-gateway-fabric/internal/framework/events" "github.com/nginxinc/nginx-gateway-fabric/internal/framework/helpers" - "github.com/nginxinc/nginx-gateway-fabric/internal/framework/status" "github.com/nginxinc/nginx-gateway-fabric/internal/framework/status/statusfakes" "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/config" "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/metrics/collectors" @@ -31,7 +29,6 @@ import ( "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/nginx/file/filefakes" "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/nginx/runtime/runtimefakes" "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/state" - staticConds "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/state/conditions" "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/state/dataplane" "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/state/graph" "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/state/statefakes" @@ -45,13 +42,25 @@ var _ = Describe("eventHandler", func() { fakeGenerator *configfakes.FakeGenerator fakeNginxFileMgr *filefakes.FakeManager fakeNginxRuntimeMgr *runtimefakes.FakeManager - fakeStatusUpdater *statusfakes.FakeUpdater + fakeStatusUpdater *statusfakes.FakeGroupUpdater fakeEventRecorder *record.FakeRecorder + fakeK8sClient client.WithWatch namespace = "nginx-gateway" configName = "nginx-gateway-config" zapLogLevelSetter zapLogLevelSetter ) + const nginxGatewayServiceName = "nginx-gateway" + + createService := func(name string) *v1.Service { + return &v1.Service{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Namespace: "nginx-gateway", + }, + } + } + expectReconfig := func(expectedConf dataplane.Configuration, expectedFiles []file.File) { Expect(fakeProcessor.ProcessCallCount()).Should(Equal(1)) @@ -64,7 +73,14 @@ var _ = Describe("eventHandler", func() { Expect(fakeNginxRuntimeMgr.ReloadCallCount()).Should(Equal(1)) - Expect(fakeStatusUpdater.UpdateCallCount()).Should(Equal(1)) + Expect(fakeStatusUpdater.UpdateGroupCallCount()).Should(Equal(2)) + _, name, reqs := fakeStatusUpdater.UpdateGroupArgsForCall(0) + Expect(name).To(Equal(groupAllExceptGateways)) + Expect(reqs).To(BeEmpty()) + + _, name, reqs = fakeStatusUpdater.UpdateGroupArgsForCall(1) + Expect(name).To(Equal(groupGateways)) + Expect(reqs).To(BeEmpty()) } BeforeEach(func() { @@ -73,12 +89,16 @@ var _ = Describe("eventHandler", func() { fakeGenerator = &configfakes.FakeGenerator{} fakeNginxFileMgr = &filefakes.FakeManager{} fakeNginxRuntimeMgr = &runtimefakes.FakeManager{} - fakeStatusUpdater = &statusfakes.FakeUpdater{} + fakeStatusUpdater = &statusfakes.FakeGroupUpdater{} fakeEventRecorder = record.NewFakeRecorder(1) zapLogLevelSetter = newZapLogLevelSetter(zap.NewAtomicLevel()) + fakeK8sClient = fake.NewFakeClient() + + // Needed because handler checks the service from the API on every HandleEventBatch + Expect(fakeK8sClient.Create(context.Background(), createService(nginxGatewayServiceName))).To(Succeed()) handler = newEventHandlerImpl(eventHandlerConfig{ - k8sClient: fake.NewFakeClient(), + k8sClient: fakeK8sClient, processor: fakeProcessor, generator: fakeGenerator, logLevelSetter: zapLogLevelSetter, @@ -92,7 +112,8 @@ var _ = Describe("eventHandler", func() { ServiceName: "nginx-gateway", Namespace: "nginx-gateway", }, - metricsCollector: collectors.NewControllerNoopCollector(), + metricsCollector: collectors.NewControllerNoopCollector(), + updateGatewayClassStatus: true, }) Expect(handler.cfg.nginxConfiguredOnStartChecker.ready).To(BeFalse()) }) @@ -178,6 +199,62 @@ var _ = Describe("eventHandler", func() { }) }) + DescribeTable( + "updating statuses of GatewayClass conditionally based on handler configuration", + func(updateGatewayClassStatus bool) { + handler.cfg.updateGatewayClassStatus = updateGatewayClassStatus + + gc := &gatewayv1.GatewayClass{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test", + }, + } + ignoredGC := &gatewayv1.GatewayClass{ + ObjectMeta: metav1.ObjectMeta{ + Name: "ignored", + }, + } + + fakeProcessor.ProcessReturns(state.ClusterStateChange, &graph.Graph{ + GatewayClass: &graph.GatewayClass{ + Source: gc, + Valid: true, + }, + IgnoredGatewayClasses: map[types.NamespacedName]*gatewayv1.GatewayClass{ + client.ObjectKeyFromObject(ignoredGC): ignoredGC, + }, + }) + + e := &events.UpsertEvent{ + Resource: &gatewayv1.HTTPRoute{}, // any supported is OK + } + + batch := []interface{}{e} + + var expectedReqsCount int + if updateGatewayClassStatus { + expectedReqsCount = 2 + } + + handler.HandleEventBatch(context.Background(), ctlrZap.New(), batch) + + Expect(fakeStatusUpdater.UpdateGroupCallCount()).To(Equal(2)) + + _, name, reqs := fakeStatusUpdater.UpdateGroupArgsForCall(0) + Expect(name).To(Equal(groupAllExceptGateways)) + Expect(reqs).To(HaveLen(expectedReqsCount)) + for _, req := range reqs { + Expect(req.NsName).To(BeElementOf( + client.ObjectKeyFromObject(gc), + client.ObjectKeyFromObject(ignoredGC), + )) + Expect(req.ResourceType).To(Equal(&gatewayv1.GatewayClass{})) + } + }, + Entry("should update statuses of GatewayClass", true), + Entry("should not update statuses of GatewayClass", false), + ) + When("receiving control plane configuration updates", func() { cfg := func(level ngfAPI.ControllerLogLevel) *ngfAPI.NginxGateway { return &ngfAPI.NginxGateway{ @@ -193,26 +270,17 @@ var _ = Describe("eventHandler", func() { } } - expStatuses := func(cond conditions.Condition) *status.NginxGatewayStatus { - return &status.NginxGatewayStatus{ - NsName: types.NamespacedName{ - Namespace: namespace, - Name: configName, - }, - Conditions: []conditions.Condition{cond}, - ObservedGeneration: 0, - } - } - It("handles a valid config", func() { batch := []interface{}{&events.UpsertEvent{Resource: cfg(ngfAPI.ControllerLogLevelError)}} handler.HandleEventBatch(context.Background(), ctlrZap.New(), batch) Expect(handler.GetLatestConfiguration()).To(BeNil()) - Expect(fakeStatusUpdater.UpdateCallCount()).Should(Equal(1)) - _, statuses := fakeStatusUpdater.UpdateArgsForCall(0) - Expect(statuses).To(Equal(expStatuses(staticConds.NewNginxGatewayValid()))) + Expect(fakeStatusUpdater.UpdateGroupCallCount()).To(Equal(1)) + _, name, reqs := fakeStatusUpdater.UpdateGroupArgsForCall(0) + Expect(name).To(Equal(groupControlPlane)) + Expect(reqs).To(HaveLen(1)) + Expect(zapLogLevelSetter.Enabled(zap.DebugLevel)).To(BeFalse()) Expect(zapLogLevelSetter.Enabled(zap.ErrorLevel)).To(BeTrue()) }) @@ -223,12 +291,11 @@ var _ = Describe("eventHandler", func() { Expect(handler.GetLatestConfiguration()).To(BeNil()) - Expect(fakeStatusUpdater.UpdateCallCount()).Should(Equal(1)) - _, statuses := fakeStatusUpdater.UpdateArgsForCall(0) - cond := staticConds.NewNginxGatewayInvalid( - "Failed to update control plane configuration: logging.level: Unsupported value: " + - "\"invalid\": supported values: \"info\", \"debug\", \"error\"") - Expect(statuses).To(Equal(expStatuses(cond))) + Expect(fakeStatusUpdater.UpdateGroupCallCount()).To(Equal(1)) + _, name, reqs := fakeStatusUpdater.UpdateGroupArgsForCall(0) + Expect(name).To(Equal(groupControlPlane)) + Expect(reqs).To(HaveLen(1)) + Expect(fakeEventRecorder.Events).To(HaveLen(1)) event := <-fakeEventRecorder.Events Expect(event).To(Equal( @@ -252,6 +319,11 @@ var _ = Describe("eventHandler", func() { Expect(handler.GetLatestConfiguration()).To(BeNil()) + Expect(fakeStatusUpdater.UpdateGroupCallCount()).To(Equal(1)) + _, name, reqs := fakeStatusUpdater.UpdateGroupArgsForCall(0) + Expect(name).To(Equal(groupControlPlane)) + Expect(reqs).To(BeEmpty()) + Expect(fakeEventRecorder.Events).To(HaveLen(1)) event := <-fakeEventRecorder.Events Expect(event).To(Equal("Warning ResourceDeleted NginxGateway configuration was deleted; using defaults")) @@ -260,6 +332,14 @@ var _ = Describe("eventHandler", func() { }) When("receiving Service updates", func() { + const notNginxGatewayServiceName = "not-nginx-gateway" + + BeforeEach(func() { + fakeProcessor.GetLatestGraphReturns(&graph.Graph{}) + + Expect(fakeK8sClient.Create(context.Background(), createService(notNginxGatewayServiceName))).To(Succeed()) + }) + It("should not call UpdateAddresses if the Service is not for the Gateway Pod", func() { e := &events.UpsertEvent{Resource: &v1.Service{ ObjectMeta: metav1.ObjectMeta{ @@ -270,16 +350,17 @@ var _ = Describe("eventHandler", func() { handler.HandleEventBatch(context.Background(), ctlrZap.New(), batch) - Expect(fakeStatusUpdater.UpdateAddressesCallCount()).To(BeZero()) + Expect(fakeStatusUpdater.UpdateGroupCallCount()).To(BeZero()) de := &events.DeleteEvent{Type: &v1.Service{}} batch = []interface{}{de} + Expect(fakeK8sClient.Delete(context.Background(), createService(notNginxGatewayServiceName))).To(Succeed()) handler.HandleEventBatch(context.Background(), ctlrZap.New(), batch) Expect(handler.GetLatestConfiguration()).To(BeNil()) - Expect(fakeStatusUpdater.UpdateAddressesCallCount()).To(BeZero()) + Expect(fakeStatusUpdater.UpdateGroupCallCount()).To(BeZero()) }) It("should update the addresses when the Gateway Service is upserted", func() { @@ -294,7 +375,10 @@ var _ = Describe("eventHandler", func() { handler.HandleEventBatch(context.Background(), ctlrZap.New(), batch) Expect(handler.GetLatestConfiguration()).To(BeNil()) - Expect(fakeStatusUpdater.UpdateAddressesCallCount()).ToNot(BeZero()) + Expect(fakeStatusUpdater.UpdateGroupCallCount()).To(Equal(1)) + _, name, reqs := fakeStatusUpdater.UpdateGroupArgsForCall(0) + Expect(name).To(Equal(groupGateways)) + Expect(reqs).To(BeEmpty()) }) It("should update the addresses when the Gateway Service is deleted", func() { @@ -306,11 +390,15 @@ var _ = Describe("eventHandler", func() { }, } batch := []interface{}{e} + Expect(fakeK8sClient.Delete(context.Background(), createService(nginxGatewayServiceName))).To(Succeed()) handler.HandleEventBatch(context.Background(), ctlrZap.New(), batch) Expect(handler.GetLatestConfiguration()).To(BeNil()) - Expect(fakeStatusUpdater.UpdateAddressesCallCount()).ToNot(BeZero()) + Expect(fakeStatusUpdater.UpdateGroupCallCount()).To(Equal(1)) + _, name, reqs := fakeStatusUpdater.UpdateGroupArgsForCall(0) + Expect(name).To(Equal(groupGateways)) + Expect(reqs).To(BeEmpty()) }) }) diff --git a/internal/mode/static/manager.go b/internal/mode/static/manager.go index 21c123ba99..d810584a69 100644 --- a/internal/mode/static/manager.go +++ b/internal/mode/static/manager.go @@ -188,15 +188,12 @@ func StartManager(cfg config.Config) error { ) } - statusUpdater := status.NewUpdater(status.UpdaterConfig{ - GatewayCtlrName: cfg.GatewayCtlrName, - GatewayClassName: cfg.GatewayClassName, - Client: mgr.GetClient(), - Logger: cfg.Logger.WithName("statusUpdater"), - Clock: status.NewRealClock(), - UpdateGatewayClassStatus: cfg.UpdateGatewayClassStatus, - LeaderElectionEnabled: cfg.LeaderElection.Enabled, - }) + statusUpdater := status.NewUpdater( + mgr.GetClient(), + cfg.Logger.WithName("statusUpdater"), + ) + + groupStatusUpdater := status.NewLeaderAwareGroupUpdater(statusUpdater) eventHandler := newEventHandlerImpl(eventHandlerConfig{ k8sClient: mgr.GetClient(), @@ -213,7 +210,7 @@ func StartManager(cfg config.Config) error { ngxruntimeCollector, cfg.Logger.WithName("nginxRuntimeManager"), ), - statusUpdater: statusUpdater, + statusUpdater: groupStatusUpdater, eventRecorder: recorder, nginxConfiguredOnStartChecker: nginxChecker, controlConfigNSName: controlConfigNSName, @@ -221,6 +218,8 @@ func StartManager(cfg config.Config) error { metricsCollector: handlerCollector, usageReportConfig: cfg.UsageReportConfig, usageSecret: usageSecret, + gatewayCtlrName: cfg.GatewayCtlrName, + updateGatewayClassStatus: cfg.UpdateGatewayClassStatus, }) objects, objectLists := prepareFirstEventBatchPreparerArgs( @@ -240,7 +239,7 @@ func StartManager(cfg config.Config) error { return fmt.Errorf("cannot register event loop: %w", err) } - if err = mgr.Add(runnables.NewEnableAfterBecameLeader(statusUpdater.Enable)); err != nil { + if err = mgr.Add(runnables.NewEnableAfterBecameLeader(groupStatusUpdater.Enable)); err != nil { return fmt.Errorf("cannot register status updater: %w", err) } diff --git a/internal/mode/static/status/prepare_requests.go b/internal/mode/static/status/prepare_requests.go new file mode 100644 index 0000000000..216b3d86d5 --- /dev/null +++ b/internal/mode/static/status/prepare_requests.go @@ -0,0 +1,335 @@ +package status + +import ( + "fmt" + + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" + "sigs.k8s.io/controller-runtime/pkg/client" + v1 "sigs.k8s.io/gateway-api/apis/v1" + "sigs.k8s.io/gateway-api/apis/v1alpha2" + + ngfAPI "github.com/nginxinc/nginx-gateway-fabric/apis/v1alpha1" + "github.com/nginxinc/nginx-gateway-fabric/internal/framework/conditions" + "github.com/nginxinc/nginx-gateway-fabric/internal/framework/helpers" + frameworkStatus "github.com/nginxinc/nginx-gateway-fabric/internal/framework/status" + staticConds "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/state/conditions" + "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/state/graph" +) + +// NginxReloadResult describes the result of an NGINX reload. +type NginxReloadResult struct { + // Error is the error that occurred during the reload. + Error error +} + +// PrepareRouteRequests prepares status UpdateRequests for the given Routes. +func PrepareRouteRequests( + routes map[types.NamespacedName]*graph.Route, + transitionTime metav1.Time, + nginxReloadRes NginxReloadResult, + gatewayCtlrName string, +) []frameworkStatus.UpdateRequest { + reqs := make([]frameworkStatus.UpdateRequest, 0, len(routes)) + + for nsname, r := range routes { + parents := make([]v1.RouteParentStatus, 0, len(r.ParentRefs)) + + defaultConds := staticConds.NewDefaultRouteConditions() + + for _, ref := range r.ParentRefs { + failedAttachmentCondCount := 0 + if ref.Attachment != nil && !ref.Attachment.Attached { + failedAttachmentCondCount = 1 + } + allConds := make([]conditions.Condition, 0, len(r.Conditions)+len(defaultConds)+failedAttachmentCondCount) + + // We add defaultConds first, so that any additional conditions will override them, which is + // ensured by DeduplicateConditions. + allConds = append(allConds, defaultConds...) + allConds = append(allConds, r.Conditions...) + if failedAttachmentCondCount == 1 { + allConds = append(allConds, ref.Attachment.FailedCondition) + } + + if nginxReloadRes.Error != nil { + allConds = append( + allConds, + staticConds.NewRouteGatewayNotProgrammed(staticConds.RouteMessageFailedNginxReload), + ) + } + + routeRef := r.Source.Spec.ParentRefs[ref.Idx] + + conds := conditions.DeduplicateConditions(allConds) + apiConds := conditions.ConvertConditions(conds, r.Source.Generation, transitionTime) + + ps := v1.RouteParentStatus{ + ParentRef: v1.ParentReference{ + Namespace: helpers.GetPointer(v1.Namespace(ref.Gateway.Namespace)), + Name: v1.ObjectName(ref.Gateway.Name), + SectionName: routeRef.SectionName, + }, + ControllerName: v1.GatewayController(gatewayCtlrName), + Conditions: apiConds, + } + + parents = append(parents, ps) + } + + status := v1.HTTPRouteStatus{ + RouteStatus: v1.RouteStatus{ + Parents: parents, + }, + } + + req := frameworkStatus.UpdateRequest{ + NsName: nsname, + ResourceType: &v1.HTTPRoute{}, + Setter: newHTTPRouteStatusSetter(status, gatewayCtlrName), + } + + reqs = append(reqs, req) + } + + return reqs +} + +// PrepareGatewayClassRequests prepares status UpdateRequests for the given GatewayClasses. +func PrepareGatewayClassRequests( + gc *graph.GatewayClass, + ignoredGwClasses map[types.NamespacedName]*v1.GatewayClass, + transitionTime metav1.Time, +) []frameworkStatus.UpdateRequest { + var reqs []frameworkStatus.UpdateRequest + + if gc != nil { + defaultConds := conditions.NewDefaultGatewayClassConditions() + + conds := make([]conditions.Condition, 0, len(gc.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, gc.Conditions...) + + conds = conditions.DeduplicateConditions(conds) + + apiConds := conditions.ConvertConditions(conds, gc.Source.Generation, transitionTime) + + req := frameworkStatus.UpdateRequest{ + NsName: client.ObjectKeyFromObject(gc.Source), + ResourceType: &v1.GatewayClass{}, + Setter: newGatewayClassStatusSetter(v1.GatewayClassStatus{ + Conditions: apiConds, + }), + } + + reqs = append(reqs, req) + } + + for nsname, gwClass := range ignoredGwClasses { + req := frameworkStatus.UpdateRequest{ + NsName: nsname, + ResourceType: &v1.GatewayClass{}, + Setter: newGatewayClassStatusSetter(v1.GatewayClassStatus{ + Conditions: conditions.ConvertConditions( + []conditions.Condition{conditions.NewGatewayClassConflict()}, + gwClass.Generation, + transitionTime, + ), + }), + } + + reqs = append(reqs, req) + } + + return reqs +} + +// PrepareGatewayRequests prepares status UpdateRequests for the given Gateways. +func PrepareGatewayRequests( + gateway *graph.Gateway, + ignoredGateways map[types.NamespacedName]*v1.Gateway, + transitionTime metav1.Time, + gwAddresses []v1.GatewayStatusAddress, + nginxReloadRes NginxReloadResult, +) []frameworkStatus.UpdateRequest { + reqs := make([]frameworkStatus.UpdateRequest, 0, 1+len(ignoredGateways)) + + if gateway != nil { + reqs = append(reqs, prepareGatewayRequest(gateway, transitionTime, gwAddresses, nginxReloadRes)) + } + + for nsname, gw := range ignoredGateways { + apiConds := conditions.ConvertConditions(staticConds.NewGatewayConflict(), gw.Generation, transitionTime) + reqs = append(reqs, frameworkStatus.UpdateRequest{ + NsName: nsname, + ResourceType: &v1.Gateway{}, + Setter: newGatewayStatusSetter(v1.GatewayStatus{ + Conditions: apiConds, + }), + }) + } + + return reqs +} + +func prepareGatewayRequest( + gateway *graph.Gateway, + transitionTime metav1.Time, + gwAddresses []v1.GatewayStatusAddress, + nginxReloadRes NginxReloadResult, +) frameworkStatus.UpdateRequest { + if !gateway.Valid { + conds := conditions.ConvertConditions( + conditions.DeduplicateConditions(gateway.Conditions), + gateway.Source.Generation, + transitionTime, + ) + + return frameworkStatus.UpdateRequest{ + NsName: client.ObjectKeyFromObject(gateway.Source), + ResourceType: &v1.Gateway{}, + Setter: newGatewayStatusSetter(v1.GatewayStatus{ + Conditions: conds, + }), + } + } + + listenerStatuses := make([]v1.ListenerStatus, 0, len(gateway.Listeners)) + + validListenerCount := 0 + for _, l := range gateway.Listeners { + var conds []conditions.Condition + + if l.Valid { + conds = staticConds.NewDefaultListenerConditions() + validListenerCount++ + } else { + conds = l.Conditions + } + + if nginxReloadRes.Error != nil { + conds = append( + conds, + staticConds.NewListenerNotProgrammedInvalid(staticConds.ListenerMessageFailedNginxReload), + ) + } + + apiConds := conditions.ConvertConditions( + conditions.DeduplicateConditions(conds), + gateway.Source.Generation, + transitionTime, + ) + + listenerStatuses = append(listenerStatuses, v1.ListenerStatus{ + Name: v1.SectionName(l.Name), + SupportedKinds: l.SupportedKinds, + AttachedRoutes: int32(len(l.Routes)), + Conditions: apiConds, + }) + } + + gwConds := staticConds.NewDefaultGatewayConditions() + if validListenerCount == 0 { + gwConds = append(gwConds, staticConds.NewGatewayNotAcceptedListenersNotValid()...) + } else if validListenerCount < len(gateway.Listeners) { + gwConds = append(gwConds, staticConds.NewGatewayAcceptedListenersNotValid()) + } + + if nginxReloadRes.Error != nil { + gwConds = append( + gwConds, + staticConds.NewGatewayNotProgrammedInvalid(staticConds.GatewayMessageFailedNginxReload), + ) + } + + apiGwConds := conditions.ConvertConditions( + conditions.DeduplicateConditions(gwConds), + gateway.Source.Generation, + transitionTime, + ) + + return frameworkStatus.UpdateRequest{ + NsName: client.ObjectKeyFromObject(gateway.Source), + ResourceType: &v1.Gateway{}, + Setter: newGatewayStatusSetter(v1.GatewayStatus{ + Listeners: listenerStatuses, + Conditions: apiGwConds, + Addresses: gwAddresses, + }), + } +} + +// PrepareBackendTLSPolicyRequests prepares status UpdateRequests for the given BackendTLSPolicies. +func PrepareBackendTLSPolicyRequests( + policies map[types.NamespacedName]*graph.BackendTLSPolicy, + transitionTime metav1.Time, + gatewayCtlrName string, +) []frameworkStatus.UpdateRequest { + reqs := make([]frameworkStatus.UpdateRequest, 0, len(policies)) + + for nsname, pol := range policies { + if !pol.IsReferenced || pol.Ignored { + continue + } + + conds := conditions.DeduplicateConditions(pol.Conditions) + apiConds := conditions.ConvertConditions(conds, pol.Source.Generation, transitionTime) + + status := v1alpha2.PolicyStatus{ + Ancestors: []v1alpha2.PolicyAncestorStatus{ + { + AncestorRef: v1.ParentReference{ + Namespace: (*v1.Namespace)(&pol.Gateway.Namespace), + Name: v1alpha2.ObjectName(pol.Gateway.Name), + }, + ControllerName: v1alpha2.GatewayController(gatewayCtlrName), + Conditions: apiConds, + }, + }, + } + + reqs = append(reqs, frameworkStatus.UpdateRequest{ + NsName: nsname, + ResourceType: &v1alpha2.BackendTLSPolicy{}, + Setter: newBackendTLSPolicyStatusSetter(status, gatewayCtlrName), + }) + } + return reqs +} + +// ControlPlaneUpdateResult describes the result of a control plane update. +type ControlPlaneUpdateResult struct { + // Error is the error that occurred during the update. + Error error +} + +// PrepareNginxGatewayStatus prepares a status UpdateRequest for the given NginxGateway. +// If the NginxGateway is nil, it returns nil. +func PrepareNginxGatewayStatus( + nginxGateway *ngfAPI.NginxGateway, + transitionTime metav1.Time, + cpUpdateRes ControlPlaneUpdateResult, +) *frameworkStatus.UpdateRequest { + if nginxGateway == nil { + return nil + } + + var conds []conditions.Condition + if cpUpdateRes.Error != nil { + msg := "Failed to update control plane configuration" + conds = []conditions.Condition{staticConds.NewNginxGatewayInvalid(fmt.Sprintf("%s: %v", msg, cpUpdateRes.Error))} + } else { + conds = []conditions.Condition{staticConds.NewNginxGatewayValid()} + } + + return &frameworkStatus.UpdateRequest{ + NsName: client.ObjectKeyFromObject(nginxGateway), + ResourceType: &ngfAPI.NginxGateway{}, + Setter: newNginxGatewayStatusSetter(ngfAPI.NginxGatewayStatus{ + Conditions: conditions.ConvertConditions(conds, nginxGateway.Generation, transitionTime), + }), + } +} diff --git a/internal/mode/static/status/prepare_requests_test.go b/internal/mode/static/status/prepare_requests_test.go new file mode 100644 index 0000000000..9ad3a07df7 --- /dev/null +++ b/internal/mode/static/status/prepare_requests_test.go @@ -0,0 +1,1290 @@ +package status + +import ( + "context" + "errors" + "testing" + + . "github.com/onsi/gomega" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/types" + utilruntime "k8s.io/apimachinery/pkg/util/runtime" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/client/fake" + "sigs.k8s.io/controller-runtime/pkg/log/zap" + v1 "sigs.k8s.io/gateway-api/apis/v1" + "sigs.k8s.io/gateway-api/apis/v1alpha2" + + ngfAPI "github.com/nginxinc/nginx-gateway-fabric/apis/v1alpha1" + "github.com/nginxinc/nginx-gateway-fabric/internal/framework/conditions" + "github.com/nginxinc/nginx-gateway-fabric/internal/framework/helpers" + statusFramework "github.com/nginxinc/nginx-gateway-fabric/internal/framework/status" + staticConds "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/state/conditions" + "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/state/graph" +) + +func createK8sClientFor(resourceType client.Object) client.Client { + scheme := runtime.NewScheme() + + // for simplicity, we add all used schemes here + utilruntime.Must(v1.AddToScheme(scheme)) + utilruntime.Must(v1alpha2.AddToScheme(scheme)) + utilruntime.Must(ngfAPI.AddToScheme(scheme)) + + k8sClient := fake.NewClientBuilder(). + WithScheme(scheme). + WithStatusSubresource( + resourceType, + ). + Build() + + return k8sClient +} + +func TestBuildRouteStatuses(t *testing.T) { + const gatewayCtlrName = "controller" + + gwNsName := types.NamespacedName{Namespace: "test", Name: "gateway"} + + 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"}: { + Valid: true, + Source: &v1.HTTPRoute{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "test", + Name: "hr-valid", + Generation: 3, + }, + Spec: v1.HTTPRouteSpec{ + CommonRouteSpec: v1.CommonRouteSpec{ + ParentRefs: []v1.ParentReference{ + { + SectionName: helpers.GetPointer[v1.SectionName]("listener-80-1"), + }, + { + SectionName: helpers.GetPointer[v1.SectionName]("listener-80-2"), + }, + }, + }, + }, + }, + ParentRefs: []graph.ParentRef{ + { + Idx: 0, + Gateway: gwNsName, + Attachment: &graph.ParentRefAttachmentStatus{ + Attached: true, + }, + }, + { + Idx: 1, + Gateway: gwNsName, + Attachment: &graph.ParentRefAttachmentStatus{ + Attached: false, + FailedCondition: invalidAttachmentCondition, + }, + }, + }, + }, + {Namespace: "test", Name: "hr-invalid"}: { + Valid: false, + Conditions: []conditions.Condition{invalidRouteCondition}, + Source: &v1.HTTPRoute{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "test", + Name: "hr-invalid", + Generation: 3, + }, + Spec: v1.HTTPRouteSpec{ + CommonRouteSpec: v1.CommonRouteSpec{ + ParentRefs: []v1.ParentReference{ + { + SectionName: helpers.GetPointer[v1.SectionName]("listener-80-1"), + }, + }, + }, + }, + }, + ParentRefs: []graph.ParentRef{ + { + Idx: 0, + Gateway: gwNsName, + Attachment: nil, + }, + }, + }, + } + + transitionTime := helpers.PrepareTimeForFakeClient(metav1.Now()) + + expectedStatuses := map[types.NamespacedName]v1.HTTPRouteStatus{ + {Namespace: "test", Name: "hr-valid"}: { + RouteStatus: v1.RouteStatus{ + Parents: []v1.RouteParentStatus{ + { + ParentRef: v1.ParentReference{ + Namespace: helpers.GetPointer(v1.Namespace(gwNsName.Namespace)), + Name: v1.ObjectName(gwNsName.Name), + SectionName: helpers.GetPointer[v1.SectionName]("listener-80-1"), + }, + ControllerName: gatewayCtlrName, + Conditions: []metav1.Condition{ + { + Type: string(v1.RouteConditionAccepted), + Status: metav1.ConditionTrue, + ObservedGeneration: 3, + LastTransitionTime: transitionTime, + Reason: string(v1.RouteReasonAccepted), + Message: "The route is accepted", + }, + { + Type: string(v1.RouteConditionResolvedRefs), + Status: metav1.ConditionTrue, + ObservedGeneration: 3, + LastTransitionTime: transitionTime, + Reason: string(v1.RouteReasonResolvedRefs), + Message: "All references are resolved", + }, + }, + }, + { + ParentRef: v1.ParentReference{ + Namespace: helpers.GetPointer(v1.Namespace(gwNsName.Namespace)), + Name: v1.ObjectName(gwNsName.Name), + SectionName: helpers.GetPointer[v1.SectionName]("listener-80-2"), + }, + ControllerName: gatewayCtlrName, + Conditions: []metav1.Condition{ + { + Type: string(v1.RouteConditionAccepted), + Status: metav1.ConditionTrue, + ObservedGeneration: 3, + LastTransitionTime: transitionTime, + Reason: string(v1.RouteReasonAccepted), + Message: "The route is accepted", + }, + { + Type: string(v1.RouteConditionResolvedRefs), + Status: metav1.ConditionTrue, + ObservedGeneration: 3, + LastTransitionTime: transitionTime, + Reason: string(v1.RouteReasonResolvedRefs), + Message: "All references are resolved", + }, + { + Type: invalidAttachmentCondition.Type, + Status: metav1.ConditionTrue, + ObservedGeneration: 3, + LastTransitionTime: transitionTime, + }, + }, + }, + }, + }, + }, + {Namespace: "test", Name: "hr-invalid"}: { + RouteStatus: v1.RouteStatus{ + Parents: []v1.RouteParentStatus{ + { + ParentRef: v1.ParentReference{ + Namespace: helpers.GetPointer(v1.Namespace(gwNsName.Namespace)), + Name: v1.ObjectName(gwNsName.Name), + SectionName: helpers.GetPointer[v1.SectionName]("listener-80-1"), + }, + ControllerName: gatewayCtlrName, + Conditions: []metav1.Condition{ + { + Type: string(v1.RouteConditionAccepted), + Status: metav1.ConditionTrue, + ObservedGeneration: 3, + LastTransitionTime: transitionTime, + Reason: string(v1.RouteReasonAccepted), + Message: "The route is accepted", + }, + { + Type: string(v1.RouteConditionResolvedRefs), + Status: metav1.ConditionTrue, + ObservedGeneration: 3, + LastTransitionTime: transitionTime, + Reason: string(v1.RouteReasonResolvedRefs), + Message: "All references are resolved", + }, + { + Type: invalidRouteCondition.Type, + Status: metav1.ConditionTrue, + ObservedGeneration: 3, + LastTransitionTime: transitionTime, + }, + }, + }, + }, + }, + }, + } + + g := NewWithT(t) + + k8sClient := createK8sClientFor(&v1.HTTPRoute{}) + + for _, r := range routes { + err := k8sClient.Create(context.Background(), r.Source) + g.Expect(err).ToNot(HaveOccurred()) + } + + updater := statusFramework.NewUpdater(k8sClient, zap.New()) + + reqs := PrepareRouteRequests(routes, transitionTime, NginxReloadResult{}, gatewayCtlrName) + + updater.Update(context.Background(), reqs...) + + g.Expect(reqs).To(HaveLen(len(expectedStatuses))) + + for nsname, expected := range expectedStatuses { + var hr v1.HTTPRoute + + err := k8sClient.Get(context.Background(), nsname, &hr) + g.Expect(err).ToNot(HaveOccurred()) + g.Expect(helpers.Diff(expected, hr.Status)).To(BeEmpty()) + } +} + +func TestBuildRouteStatusesNginxErr(t *testing.T) { + const gatewayCtlrName = "controller" + + gwNsName := types.NamespacedName{Namespace: "test", Name: "gateway"} + routeNsName := types.NamespacedName{Namespace: "test", Name: "hr-valid"} + + routes := map[types.NamespacedName]*graph.Route{ + routeNsName: { + Valid: true, + Source: &v1.HTTPRoute{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: routeNsName.Namespace, + Name: routeNsName.Name, + Generation: 3, + }, + Spec: v1.HTTPRouteSpec{ + CommonRouteSpec: v1.CommonRouteSpec{ + ParentRefs: []v1.ParentReference{ + { + SectionName: helpers.GetPointer[v1.SectionName]("listener-80-1"), + }, + { + SectionName: helpers.GetPointer[v1.SectionName]("listener-80-2"), + }, + }, + }, + }, + }, + ParentRefs: []graph.ParentRef{ + { + Idx: 0, + Gateway: gwNsName, + Attachment: &graph.ParentRefAttachmentStatus{ + Attached: true, + }, + }, + }, + }, + } + + transitionTime := helpers.PrepareTimeForFakeClient(metav1.Now()) + + expectedStatus := v1.HTTPRouteStatus{ + RouteStatus: v1.RouteStatus{ + Parents: []v1.RouteParentStatus{ + { + ParentRef: v1.ParentReference{ + Namespace: helpers.GetPointer(v1.Namespace(gwNsName.Namespace)), + Name: v1.ObjectName(gwNsName.Name), + SectionName: helpers.GetPointer[v1.SectionName]("listener-80-1"), + }, + ControllerName: gatewayCtlrName, + Conditions: []metav1.Condition{ + { + Type: string(v1.RouteConditionResolvedRefs), + Status: metav1.ConditionTrue, + ObservedGeneration: 3, + LastTransitionTime: transitionTime, + Reason: string(v1.RouteReasonResolvedRefs), + Message: "All references are resolved", + }, + { + Type: string(v1.RouteConditionAccepted), + Status: metav1.ConditionFalse, + ObservedGeneration: 3, + LastTransitionTime: transitionTime, + Reason: string(staticConds.RouteReasonGatewayNotProgrammed), + Message: staticConds.RouteMessageFailedNginxReload, + }, + }, + }, + }, + }, + } + + g := NewWithT(t) + + k8sClient := createK8sClientFor(&v1.HTTPRoute{}) + + for _, r := range routes { + err := k8sClient.Create(context.Background(), r.Source) + g.Expect(err).ToNot(HaveOccurred()) + } + + updater := statusFramework.NewUpdater(k8sClient, zap.New()) + + reqs := PrepareRouteRequests( + routes, + transitionTime, + NginxReloadResult{Error: errors.New("test error")}, + gatewayCtlrName, + ) + + g.Expect(reqs).To(HaveLen(1)) + + updater.Update(context.Background(), reqs...) + + var hr v1.HTTPRoute + + err := k8sClient.Get(context.Background(), routeNsName, &hr) + g.Expect(err).ToNot(HaveOccurred()) + g.Expect(helpers.Diff(expectedStatus, hr.Status)).To(BeEmpty()) +} + +func TestBuildGatewayClassStatuses(t *testing.T) { + transitionTime := helpers.PrepareTimeForFakeClient(metav1.Now()) + + tests := []struct { + gc *graph.GatewayClass + ignoredClasses map[types.NamespacedName]*v1.GatewayClass + expected map[types.NamespacedName]v1.GatewayClassStatus + name string + }{ + { + name: "nil gatewayclass and no ignored gatewayclasses", + expected: map[types.NamespacedName]v1.GatewayClassStatus{}, + }, + { + name: "nil gatewayclass and ignored gatewayclasses", + ignoredClasses: map[types.NamespacedName]*v1.GatewayClass{ + {Name: "ignored-1"}: { + ObjectMeta: metav1.ObjectMeta{ + Name: "ignored-1", + Generation: 1, + }, + }, + {Name: "ignored-2"}: { + ObjectMeta: metav1.ObjectMeta{ + Name: "ignored-2", + Generation: 2, + }, + }, + }, + expected: map[types.NamespacedName]v1.GatewayClassStatus{ + {Name: "ignored-1"}: { + Conditions: []metav1.Condition{ + { + Type: string(v1.GatewayClassConditionStatusAccepted), + Status: metav1.ConditionFalse, + ObservedGeneration: 1, + LastTransitionTime: transitionTime, + Reason: string(conditions.GatewayClassReasonGatewayClassConflict), + Message: conditions.GatewayClassMessageGatewayClassConflict, + }, + }, + }, + {Name: "ignored-2"}: { + Conditions: []metav1.Condition{ + { + Type: string(v1.GatewayClassConditionStatusAccepted), + Status: metav1.ConditionFalse, + ObservedGeneration: 2, + LastTransitionTime: transitionTime, + Reason: string(conditions.GatewayClassReasonGatewayClassConflict), + Message: conditions.GatewayClassMessageGatewayClassConflict, + }, + }, + }, + }, + }, + { + name: "valid gatewayclass", + gc: &graph.GatewayClass{ + Source: &v1.GatewayClass{ + ObjectMeta: metav1.ObjectMeta{ + Name: "valid-gc", + Generation: 1, + }, + }, + }, + expected: map[types.NamespacedName]v1.GatewayClassStatus{ + {Name: "valid-gc"}: { + Conditions: []metav1.Condition{ + { + Type: string(v1.GatewayClassConditionStatusAccepted), + Status: metav1.ConditionTrue, + ObservedGeneration: 1, + LastTransitionTime: transitionTime, + Reason: string(v1.GatewayClassReasonAccepted), + Message: "GatewayClass is accepted", + }, + { + Type: string(v1.GatewayClassReasonSupportedVersion), + Status: metav1.ConditionTrue, + ObservedGeneration: 1, + LastTransitionTime: transitionTime, + Reason: string(v1.GatewayClassReasonSupportedVersion), + Message: "Gateway API CRD versions are supported", + }, + }, + }, + }, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + g := NewWithT(t) + + k8sClient := createK8sClientFor(&v1.GatewayClass{}) + + var expectedTotalReqs int + + if test.gc != nil { + err := k8sClient.Create(context.Background(), test.gc.Source) + g.Expect(err).ToNot(HaveOccurred()) + expectedTotalReqs++ + } + + for _, gc := range test.ignoredClasses { + err := k8sClient.Create(context.Background(), gc) + g.Expect(err).ToNot(HaveOccurred()) + expectedTotalReqs++ + } + + updater := statusFramework.NewUpdater(k8sClient, zap.New()) + + reqs := PrepareGatewayClassRequests(test.gc, test.ignoredClasses, transitionTime) + + g.Expect(reqs).To(HaveLen(expectedTotalReqs)) + + updater.Update(context.Background(), reqs...) + + for nsname, expected := range test.expected { + var gc v1.GatewayClass + + err := k8sClient.Get(context.Background(), nsname, &gc) + g.Expect(err).ToNot(HaveOccurred()) + g.Expect(helpers.Diff(expected, gc.Status)).To(BeEmpty()) + } + }) + } +} + +func TestBuildGatewayStatuses(t *testing.T) { + createGateway := func() *v1.Gateway { + return &v1.Gateway{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "test", + Name: "gateway", + Generation: 2, + }, + } + } + + transitionTime := helpers.PrepareTimeForFakeClient(metav1.Now()) + + validListenerConditions := []metav1.Condition{ + { + Type: string(v1.ListenerConditionAccepted), + Status: metav1.ConditionTrue, + ObservedGeneration: 2, + LastTransitionTime: transitionTime, + Reason: string(v1.ListenerReasonAccepted), + Message: "Listener is accepted", + }, + { + Type: string(v1.ListenerConditionProgrammed), + Status: metav1.ConditionTrue, + ObservedGeneration: 2, + LastTransitionTime: transitionTime, + Reason: string(v1.ListenerReasonProgrammed), + Message: "Listener is programmed", + }, + { + Type: string(v1.ListenerConditionResolvedRefs), + Status: metav1.ConditionTrue, + ObservedGeneration: 2, + LastTransitionTime: transitionTime, + Reason: string(v1.ListenerReasonResolvedRefs), + Message: "All references are resolved", + }, + { + Type: string(v1.ListenerConditionConflicted), + Status: metav1.ConditionFalse, + ObservedGeneration: 2, + LastTransitionTime: transitionTime, + Reason: string(v1.ListenerReasonNoConflicts), + Message: "No conflicts", + }, + } + + addr := []v1.GatewayStatusAddress{ + { + Type: helpers.GetPointer(v1.IPAddressType), + Value: "1.2.3.4", + }, + } + + tests := []struct { + nginxReloadRes NginxReloadResult + gateway *graph.Gateway + ignoredGateways map[types.NamespacedName]*v1.Gateway + expected map[types.NamespacedName]v1.GatewayStatus + name string + }{ + { + name: "nil gateway and no ignored gateways", + expected: map[types.NamespacedName]v1.GatewayStatus{}, + }, + { + name: "nil gateway and ignored gateways", + ignoredGateways: map[types.NamespacedName]*v1.Gateway{ + {Namespace: "test", Name: "ignored-1"}: { + ObjectMeta: metav1.ObjectMeta{ + Name: "ignored-1", + Namespace: "test", + Generation: 1, + }, + }, + {Namespace: "test", Name: "ignored-2"}: { + ObjectMeta: metav1.ObjectMeta{ + Name: "ignored-2", + Namespace: "test", + Generation: 2, + }, + }, + }, + expected: map[types.NamespacedName]v1.GatewayStatus{ + {Namespace: "test", Name: "ignored-1"}: { + Conditions: []metav1.Condition{ + { + Type: string(v1.GatewayConditionAccepted), + Status: metav1.ConditionFalse, + ObservedGeneration: 1, + LastTransitionTime: transitionTime, + Reason: string(staticConds.GatewayReasonGatewayConflict), + Message: staticConds.GatewayMessageGatewayConflict, + }, + { + Type: string(v1.GatewayConditionProgrammed), + Status: metav1.ConditionFalse, + ObservedGeneration: 1, + LastTransitionTime: transitionTime, + Reason: string(staticConds.GatewayReasonGatewayConflict), + Message: staticConds.GatewayMessageGatewayConflict, + }, + }, + }, + {Namespace: "test", Name: "ignored-2"}: { + Conditions: []metav1.Condition{ + { + Type: string(v1.GatewayConditionAccepted), + Status: metav1.ConditionFalse, + ObservedGeneration: 2, + LastTransitionTime: transitionTime, + Reason: string(staticConds.GatewayReasonGatewayConflict), + Message: staticConds.GatewayMessageGatewayConflict, + }, + { + Type: string(v1.GatewayConditionProgrammed), + Status: metav1.ConditionFalse, + ObservedGeneration: 2, + LastTransitionTime: transitionTime, + Reason: string(staticConds.GatewayReasonGatewayConflict), + Message: staticConds.GatewayMessageGatewayConflict, + }, + }, + }, + }, + }, + { + name: "valid gateway; all valid listeners", + gateway: &graph.Gateway{ + Source: createGateway(), + Listeners: []*graph.Listener{ + { + Name: "listener-valid-1", + Valid: true, + Routes: map[types.NamespacedName]*graph.Route{ + {Namespace: "test", Name: "hr-1"}: {}, + }, + }, + { + Name: "listener-valid-2", + Valid: true, + Routes: map[types.NamespacedName]*graph.Route{ + {Namespace: "test", Name: "hr-1"}: {}, + }, + }, + }, + Valid: true, + }, + expected: map[types.NamespacedName]v1.GatewayStatus{ + {Namespace: "test", Name: "gateway"}: { + Addresses: addr, + Conditions: []metav1.Condition{ + { + Type: string(v1.GatewayConditionAccepted), + Status: metav1.ConditionTrue, + ObservedGeneration: 2, + LastTransitionTime: transitionTime, + Reason: string(v1.GatewayReasonAccepted), + Message: "Gateway is accepted", + }, + { + Type: string(v1.GatewayConditionProgrammed), + Status: metav1.ConditionTrue, + ObservedGeneration: 2, + LastTransitionTime: transitionTime, + Reason: string(v1.GatewayReasonProgrammed), + Message: "Gateway is programmed", + }, + }, + Listeners: []v1.ListenerStatus{ + { + Name: "listener-valid-1", + AttachedRoutes: 1, + Conditions: validListenerConditions, + }, + { + Name: "listener-valid-2", + AttachedRoutes: 1, + Conditions: validListenerConditions, + }, + }, + }, + }, + }, + { + name: "valid gateway; some valid listeners", + gateway: &graph.Gateway{ + Source: createGateway(), + Listeners: []*graph.Listener{ + { + Name: "listener-valid", + Valid: true, + Routes: map[types.NamespacedName]*graph.Route{ + {Namespace: "test", Name: "hr-1"}: {}, + }, + }, + { + Name: "listener-invalid", + Valid: false, + Conditions: staticConds.NewListenerUnsupportedValue("unsupported value"), + }, + }, + Valid: true, + }, + expected: map[types.NamespacedName]v1.GatewayStatus{ + {Namespace: "test", Name: "gateway"}: { + Addresses: addr, + Conditions: []metav1.Condition{ + { + Type: string(v1.GatewayConditionProgrammed), + Status: metav1.ConditionTrue, + ObservedGeneration: 2, + LastTransitionTime: transitionTime, + Reason: string(v1.GatewayReasonProgrammed), + Message: "Gateway is programmed", + }, + { + // is it a bug? + Type: string(v1.GatewayReasonAccepted), + Status: metav1.ConditionTrue, + ObservedGeneration: 2, + LastTransitionTime: transitionTime, + Reason: string(v1.GatewayReasonListenersNotValid), + Message: "Gateway has at least one valid listener", + }, + }, + Listeners: []v1.ListenerStatus{ + { + Name: "listener-valid", + AttachedRoutes: 1, + Conditions: validListenerConditions, + }, + { + Name: "listener-invalid", + AttachedRoutes: 0, + Conditions: []metav1.Condition{ + { + Type: string(v1.ListenerConditionAccepted), + Status: metav1.ConditionFalse, + ObservedGeneration: 2, + LastTransitionTime: transitionTime, + Reason: string(staticConds.ListenerReasonUnsupportedValue), + Message: "unsupported value", + }, + { + Type: string(v1.ListenerConditionProgrammed), + Status: metav1.ConditionFalse, + ObservedGeneration: 2, + LastTransitionTime: transitionTime, + Reason: string(v1.ListenerReasonInvalid), + Message: "unsupported value", + }, + }, + }, + }, + }, + }, + }, + { + name: "valid gateway; no valid listeners", + gateway: &graph.Gateway{ + Source: createGateway(), + Listeners: []*graph.Listener{ + { + Name: "listener-invalid-1", + Valid: false, + Conditions: staticConds.NewListenerUnsupportedProtocol("unsupported protocol"), + }, + { + Name: "listener-invalid-2", + Valid: false, + Conditions: staticConds.NewListenerUnsupportedValue("unsupported value"), + }, + }, + Valid: true, + }, + expected: map[types.NamespacedName]v1.GatewayStatus{ + {Namespace: "test", Name: "gateway"}: { + Addresses: addr, + Conditions: []metav1.Condition{ + { + Type: string(v1.GatewayReasonAccepted), + Status: metav1.ConditionFalse, + ObservedGeneration: 2, + LastTransitionTime: transitionTime, + Reason: string(v1.GatewayReasonListenersNotValid), + Message: "Gateway has no valid listeners", + }, + { + Type: string(v1.GatewayConditionProgrammed), + Status: metav1.ConditionFalse, + ObservedGeneration: 2, + LastTransitionTime: transitionTime, + Reason: string(v1.GatewayReasonInvalid), + Message: "Gateway has no valid listeners", + }, + }, + Listeners: []v1.ListenerStatus{ + { + Name: "listener-invalid-1", + AttachedRoutes: 0, + Conditions: []metav1.Condition{ + { + Type: string(v1.ListenerConditionAccepted), + Status: metav1.ConditionFalse, + ObservedGeneration: 2, + LastTransitionTime: transitionTime, + Reason: string(v1.ListenerReasonUnsupportedProtocol), + Message: "unsupported protocol", + }, + { + Type: string(v1.ListenerConditionProgrammed), + Status: metav1.ConditionFalse, + ObservedGeneration: 2, + LastTransitionTime: transitionTime, + Reason: string(v1.ListenerReasonInvalid), + Message: "unsupported protocol", + }, + }, + }, + { + Name: "listener-invalid-2", + AttachedRoutes: 0, + Conditions: []metav1.Condition{ + { + Type: string(v1.ListenerConditionAccepted), + Status: metav1.ConditionFalse, + ObservedGeneration: 2, + LastTransitionTime: transitionTime, + Reason: string(staticConds.ListenerReasonUnsupportedValue), + Message: "unsupported value", + }, + { + Type: string(v1.ListenerConditionProgrammed), + Status: metav1.ConditionFalse, + ObservedGeneration: 2, + LastTransitionTime: transitionTime, + Reason: string(v1.ListenerReasonInvalid), + Message: "unsupported value", + }, + }, + }, + }, + }, + }, + }, + { + name: "invalid gateway", + gateway: &graph.Gateway{ + Source: createGateway(), + Valid: false, + Conditions: staticConds.NewGatewayInvalid("no gateway class"), + }, + expected: map[types.NamespacedName]v1.GatewayStatus{ + {Namespace: "test", Name: "gateway"}: { + Conditions: []metav1.Condition{ + { + Type: string(v1.GatewayConditionAccepted), + Status: metav1.ConditionFalse, + ObservedGeneration: 2, + LastTransitionTime: transitionTime, + Reason: string(v1.GatewayReasonInvalid), + Message: "no gateway class", + }, + { + Type: string(v1.GatewayConditionProgrammed), + Status: metav1.ConditionFalse, + ObservedGeneration: 2, + LastTransitionTime: transitionTime, + Reason: string(v1.GatewayReasonInvalid), + Message: "no gateway class", + }, + }, + }, + }, + }, + { + name: "error reloading nginx; gateway/listener not programmed", + gateway: &graph.Gateway{ + Source: createGateway(), + Valid: true, + Conditions: staticConds.NewDefaultGatewayConditions(), + Listeners: []*graph.Listener{ + { + Name: "listener-valid", + Valid: true, + Routes: map[types.NamespacedName]*graph.Route{ + {Namespace: "test", Name: "hr-1"}: {}, + }, + }, + }, + }, + expected: map[types.NamespacedName]v1.GatewayStatus{ + {Namespace: "test", Name: "gateway"}: { + Addresses: addr, + Conditions: []metav1.Condition{ + { + Type: string(v1.GatewayConditionAccepted), + Status: metav1.ConditionTrue, + ObservedGeneration: 2, + LastTransitionTime: transitionTime, + Reason: string(v1.GatewayReasonAccepted), + Message: "Gateway is accepted", + }, + { + Type: string(v1.GatewayConditionProgrammed), + Status: metav1.ConditionFalse, + ObservedGeneration: 2, + LastTransitionTime: transitionTime, + Reason: string(v1.GatewayReasonInvalid), + Message: staticConds.GatewayMessageFailedNginxReload, + }, + }, + Listeners: []v1.ListenerStatus{ + { + Name: "listener-valid", + AttachedRoutes: 1, + Conditions: []metav1.Condition{ + { + Type: string(v1.ListenerConditionAccepted), + Status: metav1.ConditionTrue, + ObservedGeneration: 2, + LastTransitionTime: transitionTime, + Reason: string(v1.ListenerReasonAccepted), + Message: "Listener is accepted", + }, + { + Type: string(v1.ListenerConditionResolvedRefs), + Status: metav1.ConditionTrue, + ObservedGeneration: 2, + LastTransitionTime: transitionTime, + Reason: string(v1.ListenerReasonResolvedRefs), + Message: "All references are resolved", + }, + { + Type: string(v1.ListenerConditionConflicted), + Status: metav1.ConditionFalse, + ObservedGeneration: 2, + LastTransitionTime: transitionTime, + Reason: string(v1.ListenerReasonNoConflicts), + Message: "No conflicts", + }, + { + Type: string(v1.ListenerConditionProgrammed), + Status: metav1.ConditionFalse, + ObservedGeneration: 2, + LastTransitionTime: transitionTime, + Reason: string(v1.ListenerReasonInvalid), + Message: staticConds.ListenerMessageFailedNginxReload, + }, + }, + }, + }, + }, + }, + nginxReloadRes: NginxReloadResult{Error: errors.New("test error")}, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + g := NewWithT(t) + + k8sClient := createK8sClientFor(&v1.Gateway{}) + + var expectedTotalReqs int + + if test.gateway != nil { + test.gateway.Source.ResourceVersion = "" + err := k8sClient.Create(context.Background(), test.gateway.Source) + g.Expect(err).ToNot(HaveOccurred()) + expectedTotalReqs++ + } + + for _, gw := range test.ignoredGateways { + err := k8sClient.Create(context.Background(), gw) + g.Expect(err).ToNot(HaveOccurred()) + expectedTotalReqs++ + } + + updater := statusFramework.NewUpdater(k8sClient, zap.New()) + + reqs := PrepareGatewayRequests(test.gateway, test.ignoredGateways, transitionTime, addr, test.nginxReloadRes) + + g.Expect(reqs).To(HaveLen(expectedTotalReqs)) + + updater.Update(context.Background(), reqs...) + + for nsname, expected := range test.expected { + var gw v1.Gateway + + err := k8sClient.Get(context.Background(), nsname, &gw) + g.Expect(err).ToNot(HaveOccurred()) + g.Expect(helpers.Diff(expected, gw.Status)).To(BeEmpty()) + } + }) + } +} + +func TestBuildBackendTLSPolicyStatuses(t *testing.T) { + const gatewayCtlrName = "controller" + + transitionTime := helpers.PrepareTimeForFakeClient(metav1.Now()) + + type policyCfg struct { + Name string + Conditions []conditions.Condition + Valid bool + Ignored bool + IsReferenced bool + } + + getBackendTLSPolicy := func(policyCfg policyCfg) *graph.BackendTLSPolicy { + return &graph.BackendTLSPolicy{ + Source: &v1alpha2.BackendTLSPolicy{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "test", + Name: policyCfg.Name, + Generation: 1, + }, + }, + Valid: policyCfg.Valid, + Ignored: policyCfg.Ignored, + IsReferenced: policyCfg.IsReferenced, + Conditions: policyCfg.Conditions, + Gateway: types.NamespacedName{Name: "gateway", Namespace: "test"}, + } + } + + attachedConds := []conditions.Condition{staticConds.NewBackendTLSPolicyAccepted()} + invalidConds := []conditions.Condition{staticConds.NewBackendTLSPolicyInvalid("invalid backendTLSPolicy")} + + validPolicyCfg := policyCfg{ + Name: "valid-bt", + Valid: true, + IsReferenced: true, + Conditions: attachedConds, + } + + invalidPolicyCfg := policyCfg{ + Name: "invalid-bt", + IsReferenced: true, + Conditions: invalidConds, + } + + ignoredPolicyCfg := policyCfg{ + Name: "ignored-bt", + Ignored: true, + IsReferenced: true, + } + + notReferencedPolicyCfg := policyCfg{ + Name: "not-referenced", + Valid: true, + } + + tests := []struct { + backendTLSPolicies map[types.NamespacedName]*graph.BackendTLSPolicy + expected map[types.NamespacedName]v1alpha2.PolicyStatus + name string + expectedReqs int + }{ + { + name: "nil backendTLSPolicies", + expectedReqs: 0, + expected: map[types.NamespacedName]v1alpha2.PolicyStatus{}, + }, + { + name: "valid backendTLSPolicy", + backendTLSPolicies: map[types.NamespacedName]*graph.BackendTLSPolicy{ + {Namespace: "test", Name: "valid-bt"}: getBackendTLSPolicy(validPolicyCfg), + }, + expectedReqs: 1, + expected: map[types.NamespacedName]v1alpha2.PolicyStatus{ + {Name: "valid-bt", Namespace: "test"}: { + Ancestors: []v1alpha2.PolicyAncestorStatus{ + { + AncestorRef: v1.ParentReference{ + Namespace: helpers.GetPointer[v1.Namespace]("test"), + Name: "gateway", + }, + ControllerName: gatewayCtlrName, + Conditions: []metav1.Condition{ + { + Type: string(v1alpha2.PolicyConditionAccepted), + Status: metav1.ConditionTrue, + ObservedGeneration: 1, + LastTransitionTime: transitionTime, + Reason: string(v1alpha2.PolicyReasonAccepted), + Message: "BackendTLSPolicy is accepted by the Gateway", + }, + }, + }, + }, + }, + }, + }, + { + name: "invalid backendTLSPolicy", + backendTLSPolicies: map[types.NamespacedName]*graph.BackendTLSPolicy{ + {Namespace: "test", Name: "invalid-bt"}: getBackendTLSPolicy(invalidPolicyCfg), + }, + expectedReqs: 1, + expected: map[types.NamespacedName]v1alpha2.PolicyStatus{ + {Name: "invalid-bt", Namespace: "test"}: { + Ancestors: []v1alpha2.PolicyAncestorStatus{ + { + AncestorRef: v1.ParentReference{ + Namespace: helpers.GetPointer[v1.Namespace]("test"), + Name: "gateway", + }, + ControllerName: gatewayCtlrName, + Conditions: []metav1.Condition{ + { + Type: string(v1alpha2.PolicyConditionAccepted), + Status: metav1.ConditionFalse, + ObservedGeneration: 1, + LastTransitionTime: transitionTime, + Reason: string(v1alpha2.PolicyReasonInvalid), + Message: "invalid backendTLSPolicy", + }, + }, + }, + }, + }, + }, + }, + { + name: "ignored or not referenced backendTLSPolicies", + backendTLSPolicies: map[types.NamespacedName]*graph.BackendTLSPolicy{ + {Namespace: "test", Name: "ignored-bt"}: getBackendTLSPolicy(ignoredPolicyCfg), + {Namespace: "test", Name: "not-referenced"}: getBackendTLSPolicy(notReferencedPolicyCfg), + }, + expectedReqs: 0, + expected: map[types.NamespacedName]v1alpha2.PolicyStatus{ + {Name: "ignored-bt", Namespace: "test"}: {}, + {Name: "not-referenced", Namespace: "test"}: {}, + }, + }, + { + name: "mix valid and ignored backendTLSPolicies", + backendTLSPolicies: map[types.NamespacedName]*graph.BackendTLSPolicy{ + {Namespace: "test", Name: "ignored-bt"}: getBackendTLSPolicy(ignoredPolicyCfg), + {Namespace: "test", Name: "valid-bt"}: getBackendTLSPolicy(validPolicyCfg), + }, + expectedReqs: 1, + expected: map[types.NamespacedName]v1alpha2.PolicyStatus{ + {Name: "ignored-bt", Namespace: "test"}: {}, + {Name: "valid-bt", Namespace: "test"}: { + Ancestors: []v1alpha2.PolicyAncestorStatus{ + { + AncestorRef: v1.ParentReference{ + Namespace: helpers.GetPointer[v1.Namespace]("test"), + Name: "gateway", + }, + ControllerName: gatewayCtlrName, + Conditions: []metav1.Condition{ + { + Type: string(v1alpha2.PolicyConditionAccepted), + Status: metav1.ConditionTrue, + ObservedGeneration: 1, + LastTransitionTime: transitionTime, + Reason: string(v1alpha2.PolicyReasonAccepted), + Message: "BackendTLSPolicy is accepted by the Gateway", + }, + }, + }, + }, + }, + }, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + g := NewWithT(t) + + k8sClient := createK8sClientFor(&v1alpha2.BackendTLSPolicy{}) + + for _, pol := range test.backendTLSPolicies { + err := k8sClient.Create(context.Background(), pol.Source) + g.Expect(err).ToNot(HaveOccurred()) + } + + updater := statusFramework.NewUpdater(k8sClient, zap.New()) + + reqs := PrepareBackendTLSPolicyRequests(test.backendTLSPolicies, transitionTime, gatewayCtlrName) + + g.Expect(reqs).To(HaveLen(test.expectedReqs)) + + updater.Update(context.Background(), reqs...) + + for nsname, expected := range test.expected { + var pol v1alpha2.BackendTLSPolicy + + err := k8sClient.Get(context.Background(), nsname, &pol) + g.Expect(err).ToNot(HaveOccurred()) + g.Expect(helpers.Diff(expected, pol.Status)).To(BeEmpty()) + } + }) + } +} + +func TestBuildNginxGatewayStatus(t *testing.T) { + transitionTime := helpers.PrepareTimeForFakeClient(metav1.Now()) + + tests := []struct { + cpUpdateResult ControlPlaneUpdateResult + nginxGateway *ngfAPI.NginxGateway + expected *ngfAPI.NginxGatewayStatus + name string + }{ + { + name: "nil NginxGateway", + }, + { + name: "NginxGateway with no update error", + nginxGateway: &ngfAPI.NginxGateway{ + ObjectMeta: metav1.ObjectMeta{ + Name: "nginx-gateway", + Namespace: "test", + Generation: 3, + }, + }, + cpUpdateResult: ControlPlaneUpdateResult{}, + expected: &ngfAPI.NginxGatewayStatus{ + Conditions: []metav1.Condition{ + { + Type: string(ngfAPI.NginxGatewayConditionValid), + Status: metav1.ConditionTrue, + ObservedGeneration: 3, + LastTransitionTime: transitionTime, + Reason: string(ngfAPI.NginxGatewayReasonValid), + Message: "NginxGateway is valid", + }, + }, + }, + }, + { + name: "NginxGateway with update error", + nginxGateway: &ngfAPI.NginxGateway{ + ObjectMeta: metav1.ObjectMeta{ + Name: "nginx-gateway", + Namespace: "test", + Generation: 3, + }, + }, + cpUpdateResult: ControlPlaneUpdateResult{ + Error: errors.New("test error"), + }, + expected: &ngfAPI.NginxGatewayStatus{ + Conditions: []metav1.Condition{ + { + Type: string(ngfAPI.NginxGatewayConditionValid), + Status: metav1.ConditionFalse, + ObservedGeneration: 3, + LastTransitionTime: transitionTime, + Reason: string(ngfAPI.NginxGatewayReasonInvalid), + Message: "Failed to update control plane configuration: test error", + }, + }, + }, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + g := NewWithT(t) + + k8sClient := createK8sClientFor(&ngfAPI.NginxGateway{}) + + if test.nginxGateway != nil { + err := k8sClient.Create(context.Background(), test.nginxGateway) + g.Expect(err).ToNot(HaveOccurred()) + } + + updater := statusFramework.NewUpdater(k8sClient, zap.New()) + + req := PrepareNginxGatewayStatus(test.nginxGateway, transitionTime, test.cpUpdateResult) + + if test.nginxGateway == nil { + g.Expect(req).To(BeNil()) + } else { + g.Expect(req).ToNot(BeNil()) + updater.Update(context.Background(), *req) + + var ngw ngfAPI.NginxGateway + + err := k8sClient.Get(context.Background(), types.NamespacedName{Namespace: "test", Name: "nginx-gateway"}, &ngw) + g.Expect(err).ToNot(HaveOccurred()) + g.Expect(helpers.Diff(*test.expected, ngw.Status)).To(BeEmpty()) + } + }) + } +} diff --git a/internal/framework/status/setters.go b/internal/mode/static/status/status_setters.go similarity index 69% rename from internal/framework/status/setters.go rename to internal/mode/static/status/status_setters.go index 0ca2be80aa..90fab63d04 100644 --- a/internal/framework/status/setters.go +++ b/internal/mode/static/status/status_setters.go @@ -3,104 +3,37 @@ package status import ( "slices" - v1 "k8s.io/apimachinery/pkg/apis/meta/v1" "sigs.k8s.io/controller-runtime/pkg/client" gatewayv1 "sigs.k8s.io/gateway-api/apis/v1" gatewayv1alpha2 "sigs.k8s.io/gateway-api/apis/v1alpha2" ngfAPI "github.com/nginxinc/nginx-gateway-fabric/apis/v1alpha1" + "github.com/nginxinc/nginx-gateway-fabric/internal/framework/helpers" + frameworkStatus "github.com/nginxinc/nginx-gateway-fabric/internal/framework/status" ) -// setter is a function that takes an object and sets the status on that object if the status has changed. -// If the status has not changed, and the setter does not set the status, it returns false. -type setter func(client.Object) bool +func newNginxGatewayStatusSetter(status ngfAPI.NginxGatewayStatus) frameworkStatus.Setter { + return func(obj client.Object) (wasSet bool) { + ng := helpers.MustCastObject[*ngfAPI.NginxGateway](obj) -func newNginxGatewayStatusSetter(clock Clock, status NginxGatewayStatus) func(client.Object) bool { - return func(object client.Object) bool { - ng := object.(*ngfAPI.NginxGateway) - conds := convertConditions( - status.Conditions, - status.ObservedGeneration, - clock.Now(), - ) - - if conditionsEqual(ng.Status.Conditions, conds) { - return false - } - - ng.Status = ngfAPI.NginxGatewayStatus{ - Conditions: conds, - } - - return true - } -} - -func newGatewayClassStatusSetter(clock Clock, gcs GatewayClassStatus) func(client.Object) bool { - return func(object client.Object) bool { - gc := object.(*gatewayv1.GatewayClass) - status := prepareGatewayClassStatus(gcs, clock.Now()) - - if conditionsEqual(gc.Status.Conditions, status.Conditions) { + if frameworkStatus.ConditionsEqual(ng.Status.Conditions, status.Conditions) { return false } - gc.Status = status - + ng.Status = status return true } } -func newGatewayStatusSetter(clock Clock, gs GatewayStatus) func(client.Object) bool { - return func(object client.Object) bool { - gw := object.(*gatewayv1.Gateway) - status := prepareGatewayStatus(gs, clock.Now()) +func newGatewayStatusSetter(status gatewayv1.GatewayStatus) frameworkStatus.Setter { + return func(obj client.Object) (wasSet bool) { + gw := helpers.MustCastObject[*gatewayv1.Gateway](obj) if gwStatusEqual(gw.Status, status) { return false } gw.Status = status - - return true - } -} - -func newHTTPRouteStatusSetter(gatewayCtlrName string, clock Clock, rs HTTPRouteStatus) func(client.Object) bool { - return func(object client.Object) bool { - hr := object.(*gatewayv1.HTTPRoute) - status := prepareHTTPRouteStatus( - hr.Status, - rs, - gatewayCtlrName, - clock.Now(), - ) - - if hrStatusEqual(gatewayCtlrName, hr.Status, status) { - return false - } - - hr.Status = status - - return true - } -} - -func newBackendTLSPolicyStatusSetter( - gatewayCtlrName string, - clock Clock, - bs BackendTLSPolicyStatus, -) func(client.Object) bool { - return func(object client.Object) bool { - btp := object.(*gatewayv1alpha2.BackendTLSPolicy) - status := prepareBackendTLSPolicyStatus(btp.Status, bs, gatewayCtlrName, clock.Now()) - - if btpStatusEqual(gatewayCtlrName, btp.Status, status) { - return false - } - - btp.Status = status - return true } } @@ -118,7 +51,7 @@ func gwStatusEqual(prev, cur gatewayv1.GatewayStatus) bool { return false } - if !conditionsEqual(prev.Conditions, cur.Conditions) { + if !frameworkStatus.ConditionsEqual(prev.Conditions, cur.Conditions) { return false } @@ -131,7 +64,7 @@ func gwStatusEqual(prev, cur gatewayv1.GatewayStatus) bool { return false } - if !conditionsEqual(s1.Conditions, s2.Conditions) { + if !frameworkStatus.ConditionsEqual(s1.Conditions, s2.Conditions) { return false } @@ -145,6 +78,27 @@ func gwStatusEqual(prev, cur gatewayv1.GatewayStatus) bool { }) } +func newHTTPRouteStatusSetter(status gatewayv1.HTTPRouteStatus, gatewayCtlrName string) frameworkStatus.Setter { + return func(object client.Object) (wasSet bool) { + hr := object.(*gatewayv1.HTTPRoute) + + // keep all the parent statuses that belong to other controllers + for _, os := range hr.Status.Parents { + if string(os.ControllerName) != gatewayCtlrName { + status.Parents = append(status.Parents, os) + } + } + + if hrStatusEqual(gatewayCtlrName, hr.Status, status) { + return false + } + + hr.Status = status + + return true + } +} + func hrStatusEqual(gatewayCtlrName string, prev, cur gatewayv1.HTTPRouteStatus) bool { // Since other controllers may update HTTPRoute status we can't assume anything about the order of the statuses, // and we have to ignore statuses written by other controllers when checking for equality. @@ -198,7 +152,51 @@ func routeParentStatusEqual(p1, p2 gatewayv1.RouteParentStatus) bool { // we ignore the rest of the ParentRef fields because we do not set them - return conditionsEqual(p1.Conditions, p2.Conditions) + return frameworkStatus.ConditionsEqual(p1.Conditions, p2.Conditions) +} + +func newGatewayClassStatusSetter(status gatewayv1.GatewayClassStatus) frameworkStatus.Setter { + return func(obj client.Object) (wasSet bool) { + gc := helpers.MustCastObject[*gatewayv1.GatewayClass](obj) + + if frameworkStatus.ConditionsEqual(gc.Status.Conditions, status.Conditions) { + return false + } + + gc.Status = status + return true + } +} + +func newBackendTLSPolicyStatusSetter( + status gatewayv1alpha2.PolicyStatus, + gatewayCtlrName string, +) frameworkStatus.Setter { + return func(object client.Object) (wasSet bool) { + btp := helpers.MustCastObject[*gatewayv1alpha2.BackendTLSPolicy](object) + + // maxAncestors is the max number of ancestor statuses which is the sum of all new ancestor statuses and all old + // ancestor statuses. + maxAncestors := 1 + len(btp.Status.Ancestors) + ancestors := make([]gatewayv1alpha2.PolicyAncestorStatus, 0, maxAncestors) + + // keep all the ancestor statuses that belong to other controllers + for _, os := range btp.Status.Ancestors { + if string(os.ControllerName) != gatewayCtlrName { + ancestors = append(ancestors, os) + } + } + + ancestors = append(ancestors, status.Ancestors...) + status.Ancestors = ancestors + + if btpStatusEqual(gatewayCtlrName, btp.Status, status) { + return false + } + + btp.Status = status + return true + } } func btpStatusEqual(gatewayCtlrName string, prev, cur gatewayv1alpha2.PolicyStatus) bool { @@ -250,29 +248,7 @@ func btpAncestorStatusEqual(p1, p2 gatewayv1alpha2.PolicyAncestorStatus) bool { // we ignore the rest of the AncestorRef fields because we do not set them - return conditionsEqual(p1.Conditions, p2.Conditions) -} - -func conditionsEqual(prev, cur []v1.Condition) bool { - return slices.EqualFunc(prev, cur, func(c1, c2 v1.Condition) bool { - if c1.ObservedGeneration != c2.ObservedGeneration { - return false - } - - if c1.Type != c2.Type { - return false - } - - if c1.Status != c2.Status { - return false - } - - if c1.Message != c2.Message { - return false - } - - return c1.Reason == c2.Reason - }) + return frameworkStatus.ConditionsEqual(p1.Conditions, p2.Conditions) } // equalPointers returns whether two pointers are equal. diff --git a/internal/framework/status/setters_test.go b/internal/mode/static/status/status_setters_test.go similarity index 65% rename from internal/framework/status/setters_test.go rename to internal/mode/static/status/status_setters_test.go index b86f14a133..9078f420e7 100644 --- a/internal/framework/status/setters_test.go +++ b/internal/mode/static/status/status_setters_test.go @@ -2,113 +2,63 @@ package status import ( "testing" - "time" . "github.com/onsi/gomega" - v1 "k8s.io/apimachinery/pkg/apis/meta/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" gatewayv1 "sigs.k8s.io/gateway-api/apis/v1" gatewayv1alpha2 "sigs.k8s.io/gateway-api/apis/v1alpha2" ngfAPI "github.com/nginxinc/nginx-gateway-fabric/apis/v1alpha1" - "github.com/nginxinc/nginx-gateway-fabric/internal/framework/conditions" "github.com/nginxinc/nginx-gateway-fabric/internal/framework/helpers" ) func TestNewNginxGatewayStatusSetter(t *testing.T) { tests := []struct { - name string - status ngfAPI.NginxGatewayStatus - newStatus NginxGatewayStatus - expStatusSet bool + name string + status, newStatus ngfAPI.NginxGatewayStatus + expStatusSet bool }{ { name: "NginxGateway has no status", expStatusSet: true, - newStatus: NginxGatewayStatus{ - Conditions: []conditions.Condition{{Message: "new condition"}}, + newStatus: ngfAPI.NginxGatewayStatus{ + Conditions: []metav1.Condition{{Message: "some condition"}}, }, + status: ngfAPI.NginxGatewayStatus{}, }, { name: "NginxGateway has old status", expStatusSet: true, - newStatus: NginxGatewayStatus{ - Conditions: []conditions.Condition{{Message: "new condition"}}, + newStatus: ngfAPI.NginxGatewayStatus{ + Conditions: []metav1.Condition{{Message: "new condition"}}, }, status: ngfAPI.NginxGatewayStatus{ - Conditions: []v1.Condition{{Message: "old condition"}}, + Conditions: []metav1.Condition{{Message: "old condition"}}, }, }, { name: "NginxGateway has same status", expStatusSet: false, - newStatus: NginxGatewayStatus{ - Conditions: []conditions.Condition{{Message: "same condition"}}, + newStatus: ngfAPI.NginxGatewayStatus{ + Conditions: []metav1.Condition{{Message: "same condition"}}, }, status: ngfAPI.NginxGatewayStatus{ - Conditions: []v1.Condition{{Message: "same condition"}}, + Conditions: []metav1.Condition{{Message: "same condition"}}, }, }, } - clock := &RealClock{} - for _, test := range tests { t.Run(test.name, func(t *testing.T) { g := NewWithT(t) - setter := newNginxGatewayStatusSetter(clock, test.newStatus) + setter := newNginxGatewayStatusSetter(test.newStatus) + obj := &ngfAPI.NginxGateway{Status: test.status} - statusSet := setter(&ngfAPI.NginxGateway{Status: test.status}) - g.Expect(statusSet).To(Equal(test.expStatusSet)) - }) - } -} + statusSet := setter(obj) -func TestNewGatewayClassStatusSetter(t *testing.T) { - tests := []struct { - name string - status gatewayv1.GatewayClassStatus - newStatus GatewayClassStatus - expStatusSet bool - }{ - { - name: "GatewayClass has no status", - newStatus: GatewayClassStatus{ - Conditions: []conditions.Condition{{Message: "new condition"}}, - }, - expStatusSet: true, - }, - { - name: "GatewayClass has old status", - newStatus: GatewayClassStatus{ - Conditions: []conditions.Condition{{Message: "new condition"}}, - }, - status: gatewayv1.GatewayClassStatus{ - Conditions: []v1.Condition{{Message: "old condition"}}, - }, - expStatusSet: true, - }, - { - name: "GatewayClass has same status", - newStatus: GatewayClassStatus{ - Conditions: []conditions.Condition{{Message: "same condition"}}, - }, - status: gatewayv1.GatewayClassStatus{ - Conditions: []v1.Condition{{Message: "same condition"}}, - }, - expStatusSet: false, - }, - } - - clock := &RealClock{} - - for _, test := range tests { - t.Run(test.name, func(t *testing.T) { - g := NewWithT(t) - - setter := newGatewayClassStatusSetter(clock, test.newStatus) - statusSet := setter(&gatewayv1.GatewayClass{Status: test.status}) g.Expect(statusSet).To(Equal(test.expStatusSet)) + g.Expect(obj.Status).To(Equal(test.newStatus)) }) } } @@ -120,74 +70,92 @@ func TestNewGatewayStatusSetter(t *testing.T) { } tests := []struct { - name string - status gatewayv1.GatewayStatus - newStatus GatewayStatus - expStatusSet bool + name string + status, newStatus gatewayv1.GatewayStatus + expStatusSet bool }{ { name: "Gateway has no status", - newStatus: GatewayStatus{ - Conditions: []conditions.Condition{{Message: "new condition"}}, + newStatus: gatewayv1.GatewayStatus{ + Conditions: []metav1.Condition{{Message: "new condition"}}, Addresses: []gatewayv1.GatewayStatusAddress{expAddress}, }, + status: gatewayv1.GatewayStatus{}, expStatusSet: true, }, { name: "Gateway has old status", - newStatus: GatewayStatus{ - Conditions: []conditions.Condition{{Message: "new condition"}}, + newStatus: gatewayv1.GatewayStatus{ + Conditions: []metav1.Condition{{Message: "new condition"}}, Addresses: []gatewayv1.GatewayStatusAddress{expAddress}, }, status: gatewayv1.GatewayStatus{ - Conditions: []v1.Condition{{Message: "old condition"}}, + Conditions: []metav1.Condition{{Message: "old condition"}}, Addresses: []gatewayv1.GatewayStatusAddress{expAddress}, }, expStatusSet: true, }, { name: "Gateway has same status", - newStatus: GatewayStatus{ - Conditions: []conditions.Condition{{Message: "same condition"}}, + newStatus: gatewayv1.GatewayStatus{ + Conditions: []metav1.Condition{{Message: "same condition"}}, Addresses: []gatewayv1.GatewayStatusAddress{expAddress}, }, status: gatewayv1.GatewayStatus{ - Conditions: []v1.Condition{{Message: "same condition"}}, + Conditions: []metav1.Condition{{Message: "same condition"}}, Addresses: []gatewayv1.GatewayStatusAddress{expAddress}, }, expStatusSet: false, }, } - clock := &RealClock{} - for _, test := range tests { t.Run(test.name, func(t *testing.T) { g := NewWithT(t) - setter := newGatewayStatusSetter(clock, test.newStatus) + setter := newGatewayStatusSetter(test.newStatus) + obj := &gatewayv1.Gateway{Status: test.status} + + statusSet := setter(obj) - statusSet := setter(&gatewayv1.Gateway{Status: test.status}) g.Expect(statusSet).To(Equal(test.expStatusSet)) + g.Expect(obj.Status).To(Equal(test.newStatus)) }) } } func TestNewHTTPRouteStatusSetter(t *testing.T) { - controllerName := "controller" + const ( + controllerName = "controller" + otherControllerName = "different" + ) tests := []struct { - name string - status gatewayv1.HTTPRouteStatus - newStatus HTTPRouteStatus - expStatusSet bool + name string + status, newStatus, expStatus gatewayv1.HTTPRouteStatus + expStatusSet bool }{ { name: "HTTPRoute has no status", - newStatus: HTTPRouteStatus{ - ParentStatuses: []ParentStatus{ - { - Conditions: []conditions.Condition{{Message: "new condition"}}, + newStatus: gatewayv1.HTTPRouteStatus{ + RouteStatus: gatewayv1.RouteStatus{ + Parents: []gatewayv1.RouteParentStatus{ + { + ParentRef: gatewayv1.ParentReference{}, + ControllerName: gatewayv1.GatewayController(controllerName), + Conditions: []metav1.Condition{{Message: "new condition"}}, + }, + }, + }, + }, + expStatus: gatewayv1.HTTPRouteStatus{ + RouteStatus: gatewayv1.RouteStatus{ + Parents: []gatewayv1.RouteParentStatus{ + { + ParentRef: gatewayv1.ParentReference{}, + ControllerName: gatewayv1.GatewayController(controllerName), + Conditions: []metav1.Condition{{Message: "new condition"}}, + }, }, }, }, @@ -195,20 +163,82 @@ func TestNewHTTPRouteStatusSetter(t *testing.T) { }, { name: "HTTPRoute has old status", - newStatus: HTTPRouteStatus{ - ParentStatuses: []ParentStatus{ - { - Conditions: []conditions.Condition{{Message: "new condition"}}, + newStatus: gatewayv1.HTTPRouteStatus{ + RouteStatus: gatewayv1.RouteStatus{ + Parents: []gatewayv1.RouteParentStatus{ + { + ParentRef: gatewayv1.ParentReference{}, + ControllerName: gatewayv1.GatewayController(controllerName), + Conditions: []metav1.Condition{{Message: "new condition"}}, + }, + }, + }, + }, + status: gatewayv1.HTTPRouteStatus{ + RouteStatus: gatewayv1.RouteStatus{ + Parents: []gatewayv1.RouteParentStatus{ + { + ParentRef: gatewayv1.ParentReference{}, + ControllerName: gatewayv1.GatewayController(controllerName), + Conditions: []metav1.Condition{{Message: "old condition"}}, + }, + }, + }, + }, + expStatus: gatewayv1.HTTPRouteStatus{ + RouteStatus: gatewayv1.RouteStatus{ + Parents: []gatewayv1.RouteParentStatus{ + { + ParentRef: gatewayv1.ParentReference{}, + ControllerName: gatewayv1.GatewayController(controllerName), + Conditions: []metav1.Condition{{Message: "new condition"}}, + }, + }, + }, + }, + expStatusSet: true, + }, + { + name: "HTTPRoute has old status, keep other controller statuses", + newStatus: gatewayv1.HTTPRouteStatus{ + RouteStatus: gatewayv1.RouteStatus{ + Parents: []gatewayv1.RouteParentStatus{ + { + ParentRef: gatewayv1.ParentReference{}, + ControllerName: gatewayv1.GatewayController(controllerName), + Conditions: []metav1.Condition{{Message: "new condition"}}, + }, }, }, }, status: gatewayv1.HTTPRouteStatus{ RouteStatus: gatewayv1.RouteStatus{ Parents: []gatewayv1.RouteParentStatus{ + { + ParentRef: gatewayv1.ParentReference{}, + ControllerName: gatewayv1.GatewayController(otherControllerName), + Conditions: []metav1.Condition{{Message: "some condition"}}, + }, { ParentRef: gatewayv1.ParentReference{}, ControllerName: gatewayv1.GatewayController(controllerName), - Conditions: []v1.Condition{{Message: "old condition"}}, + Conditions: []metav1.Condition{{Message: "old condition"}}, + }, + }, + }, + }, + expStatus: gatewayv1.HTTPRouteStatus{ + RouteStatus: gatewayv1.RouteStatus{ + Parents: []gatewayv1.RouteParentStatus{ + { + ParentRef: gatewayv1.ParentReference{}, + ControllerName: gatewayv1.GatewayController(controllerName), + Conditions: []metav1.Condition{{Message: "new condition"}}, + }, + { + ParentRef: gatewayv1.ParentReference{}, + ControllerName: gatewayv1.GatewayController(otherControllerName), + Conditions: []metav1.Condition{{Message: "some condition"}}, }, }, }, @@ -217,10 +247,14 @@ func TestNewHTTPRouteStatusSetter(t *testing.T) { }, { name: "HTTPRoute has same status", - newStatus: HTTPRouteStatus{ - ParentStatuses: []ParentStatus{ - { - Conditions: []conditions.Condition{{Message: "same condition"}}, + newStatus: gatewayv1.HTTPRouteStatus{ + RouteStatus: gatewayv1.RouteStatus{ + Parents: []gatewayv1.RouteParentStatus{ + { + ParentRef: gatewayv1.ParentReference{}, + ControllerName: gatewayv1.GatewayController(controllerName), + Conditions: []metav1.Condition{{Message: "same condition"}}, + }, }, }, }, @@ -230,7 +264,18 @@ func TestNewHTTPRouteStatusSetter(t *testing.T) { { ParentRef: gatewayv1.ParentReference{}, ControllerName: gatewayv1.GatewayController(controllerName), - Conditions: []v1.Condition{{Message: "same condition"}}, + Conditions: []metav1.Condition{{Message: "same condition"}}, + }, + }, + }, + }, + expStatus: gatewayv1.HTTPRouteStatus{ + RouteStatus: gatewayv1.RouteStatus{ + Parents: []gatewayv1.RouteParentStatus{ + { + ParentRef: gatewayv1.ParentReference{}, + ControllerName: gatewayv1.GatewayController(controllerName), + Conditions: []metav1.Condition{{Message: "same condition"}}, }, }, }, @@ -239,16 +284,207 @@ func TestNewHTTPRouteStatusSetter(t *testing.T) { }, } - clock := &RealClock{} + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + g := NewWithT(t) + + setter := newHTTPRouteStatusSetter(test.newStatus, controllerName) + obj := &gatewayv1.HTTPRoute{Status: test.status} + + statusSet := setter(obj) + + g.Expect(statusSet).To(Equal(test.expStatusSet)) + g.Expect(obj.Status).To(Equal(test.expStatus)) + }) + } +} + +func TestNewGatewayClassStatusSetter(t *testing.T) { + tests := []struct { + name string + status, newStatus gatewayv1.GatewayClassStatus + expStatusSet bool + }{ + { + name: "GatewayClass has no status", + newStatus: gatewayv1.GatewayClassStatus{ + Conditions: []metav1.Condition{{Message: "new condition"}}, + }, + expStatusSet: true, + }, + { + name: "GatewayClass has old status", + newStatus: gatewayv1.GatewayClassStatus{ + Conditions: []metav1.Condition{{Message: "new condition"}}, + }, + status: gatewayv1.GatewayClassStatus{ + Conditions: []metav1.Condition{{Message: "old condition"}}, + }, + expStatusSet: true, + }, + { + name: "GatewayClass has same status", + newStatus: gatewayv1.GatewayClassStatus{ + Conditions: []metav1.Condition{{Message: "same condition"}}, + }, + status: gatewayv1.GatewayClassStatus{ + Conditions: []metav1.Condition{{Message: "same condition"}}, + }, + expStatusSet: false, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + g := NewWithT(t) + + setter := newGatewayClassStatusSetter(test.newStatus) + obj := &gatewayv1.GatewayClass{Status: test.status} + + statusSet := setter(obj) + + g.Expect(statusSet).To(Equal(test.expStatusSet)) + g.Expect(obj.Status).To(Equal(test.newStatus)) + }) + } +} + +func TestNewBackendTLSPolicyStatusSetter(t *testing.T) { + const ( + controllerName = "controller" + otherControllerName = "other-controller" + ) + + tests := []struct { + name string + status, newStatus, expStatus gatewayv1alpha2.PolicyStatus + expStatusSet bool + }{ + { + name: "BackendTLSPolicy has no status", + newStatus: gatewayv1alpha2.PolicyStatus{ + Ancestors: []gatewayv1alpha2.PolicyAncestorStatus{ + { + ControllerName: controllerName, + Conditions: []metav1.Condition{{Message: "new condition"}}, + }, + }, + }, + expStatus: gatewayv1alpha2.PolicyStatus{ + Ancestors: []gatewayv1alpha2.PolicyAncestorStatus{ + { + ControllerName: controllerName, + Conditions: []metav1.Condition{{Message: "new condition"}}, + }, + }, + }, + expStatusSet: true, + }, + { + name: "BackendTLSPolicy has old status", + newStatus: gatewayv1alpha2.PolicyStatus{ + Ancestors: []gatewayv1alpha2.PolicyAncestorStatus{ + { + ControllerName: controllerName, + Conditions: []metav1.Condition{{Message: "new condition"}}, + }, + }, + }, + status: gatewayv1alpha2.PolicyStatus{ + Ancestors: []gatewayv1alpha2.PolicyAncestorStatus{ + { + ControllerName: controllerName, + Conditions: []metav1.Condition{{Message: "old condition"}}, + }, + }, + }, + expStatus: gatewayv1alpha2.PolicyStatus{ + Ancestors: []gatewayv1alpha2.PolicyAncestorStatus{ + { + ControllerName: controllerName, + Conditions: []metav1.Condition{{Message: "new condition"}}, + }, + }, + }, + expStatusSet: true, + }, + { + name: "BackendTLSPolicy has old status and other controller status", + newStatus: gatewayv1alpha2.PolicyStatus{ + Ancestors: []gatewayv1alpha2.PolicyAncestorStatus{ + { + ControllerName: controllerName, + Conditions: []metav1.Condition{{Message: "new condition"}}, + }, + }, + }, + status: gatewayv1alpha2.PolicyStatus{ + Ancestors: []gatewayv1alpha2.PolicyAncestorStatus{ + { + ControllerName: controllerName, + Conditions: []metav1.Condition{{Message: "old condition"}}, + }, + { + ControllerName: otherControllerName, + Conditions: []metav1.Condition{{Message: "some condition"}}, + }, + }, + }, + expStatus: gatewayv1alpha2.PolicyStatus{ + Ancestors: []gatewayv1alpha2.PolicyAncestorStatus{ + { + ControllerName: otherControllerName, + Conditions: []metav1.Condition{{Message: "some condition"}}, + }, + { + ControllerName: controllerName, + Conditions: []metav1.Condition{{Message: "new condition"}}, + }, + }, + }, + expStatusSet: true, + }, + { + name: "BackendTLSPolicy has same status", + newStatus: gatewayv1alpha2.PolicyStatus{ + Ancestors: []gatewayv1alpha2.PolicyAncestorStatus{ + { + ControllerName: controllerName, + Conditions: []metav1.Condition{{Message: "same condition"}}, + }, + }, + }, + status: gatewayv1alpha2.PolicyStatus{ + Ancestors: []gatewayv1alpha2.PolicyAncestorStatus{ + { + ControllerName: controllerName, + Conditions: []metav1.Condition{{Message: "same condition"}}, + }, + }, + }, + expStatus: gatewayv1alpha2.PolicyStatus{ + Ancestors: []gatewayv1alpha2.PolicyAncestorStatus{ + { + ControllerName: controllerName, + Conditions: []metav1.Condition{{Message: "same condition"}}, + }, + }, + }, + expStatusSet: false, + }, + } for _, test := range tests { t.Run(test.name, func(t *testing.T) { g := NewWithT(t) - setter := newHTTPRouteStatusSetter(controllerName, clock, test.newStatus) + setter := newBackendTLSPolicyStatusSetter(test.newStatus, controllerName) + obj := &gatewayv1alpha2.BackendTLSPolicy{Status: test.status} + + statusSet := setter(obj) - statusSet := setter(&gatewayv1.HTTPRoute{Status: test.status}) g.Expect(statusSet).To(Equal(test.expStatusSet)) + g.Expect(obj.Status).To(Equal(test.expStatus)) }) } } @@ -266,7 +502,7 @@ func TestGWStatusEqual(t *testing.T) { Value: "11.0.0.0", }, }, - Conditions: []v1.Condition{ + Conditions: []metav1.Condition{ { Type: "type", /* conditions are covered by another test*/ }, @@ -285,7 +521,7 @@ func TestGWStatusEqual(t *testing.T) { }, }, AttachedRoutes: 1, - Conditions: []v1.Condition{ + Conditions: []metav1.Condition{ { Type: "type", /* conditions are covered by another test*/ }, @@ -300,7 +536,7 @@ func TestGWStatusEqual(t *testing.T) { }, }, AttachedRoutes: 1, - Conditions: []v1.Condition{ + Conditions: []metav1.Condition{ { Type: "type", /* conditions are covered by another test*/ }, @@ -315,7 +551,7 @@ func TestGWStatusEqual(t *testing.T) { }, }, AttachedRoutes: 1, - Conditions: []v1.Condition{ + Conditions: []metav1.Condition{ { Type: "type", /* conditions are covered by another test*/ }, @@ -453,7 +689,7 @@ func TestGWStatusEqual(t *testing.T) { } func TestHRStatusEqual(t *testing.T) { - testConds := []v1.Condition{ + testConds := []metav1.Condition{ { Type: "type", /* conditions are covered by another test*/ }, @@ -597,7 +833,7 @@ func TestRouteParentStatusEqual(t *testing.T) { SectionName: helpers.GetPointer[gatewayv1.SectionName]("section"), }, ControllerName: "controller", - Conditions: []v1.Condition{ + Conditions: []metav1.Condition{ { Type: "type", /* conditions are covered by another test*/ }, @@ -679,116 +915,6 @@ func TestRouteParentStatusEqual(t *testing.T) { } } -func TestConditionsEqual(t *testing.T) { - getDefaultConds := func() []v1.Condition { - return []v1.Condition{ - { - Type: "type1", - Status: "status1", - ObservedGeneration: 1, - LastTransitionTime: v1.Time{Time: time.Now()}, - Reason: "reason1", - Message: "message1", - }, - { - Type: "type2", - Status: "status2", - ObservedGeneration: 1, - LastTransitionTime: v1.Time{Time: time.Now()}, - Reason: "reason2", - Message: "message2", - }, - { - Type: "type3", - Status: "status3", - ObservedGeneration: 1, - LastTransitionTime: v1.Time{Time: time.Now()}, - Reason: "reason3", - Message: "message3", - }, - } - } - - getModifiedConds := func(mod func([]v1.Condition) []v1.Condition) []v1.Condition { - return mod(getDefaultConds()) - } - - tests := []struct { - name string - prevConds []v1.Condition - curConds []v1.Condition - expEqual bool - }{ - { - name: "different observed gen", - prevConds: getDefaultConds(), - curConds: getModifiedConds(func(conds []v1.Condition) []v1.Condition { - conds[2].ObservedGeneration++ - return conds - }), - expEqual: false, - }, - { - name: "different status", - prevConds: getDefaultConds(), - curConds: getModifiedConds(func(conds []v1.Condition) []v1.Condition { - conds[1].Status = "different" - return conds - }), - expEqual: false, - }, - { - name: "different type", - prevConds: getDefaultConds(), - curConds: getModifiedConds(func(conds []v1.Condition) []v1.Condition { - conds[0].Type = "different" - return conds - }), - expEqual: false, - }, - { - name: "different message", - prevConds: getDefaultConds(), - curConds: getModifiedConds(func(conds []v1.Condition) []v1.Condition { - conds[2].Message = "different" - return conds - }), - expEqual: false, - }, - { - name: "different reason", - prevConds: getDefaultConds(), - curConds: getModifiedConds(func(conds []v1.Condition) []v1.Condition { - conds[1].Reason = "different" - return conds - }), - expEqual: false, - }, - { - name: "different number of conditions", - prevConds: getDefaultConds(), - curConds: getModifiedConds(func(conds []v1.Condition) []v1.Condition { - return conds[:2] - }), - expEqual: false, - }, - { - name: "equal", - prevConds: getDefaultConds(), - curConds: getDefaultConds(), - expEqual: true, - }, - } - - for _, test := range tests { - t.Run(test.name, func(t *testing.T) { - g := NewWithT(t) - equal := conditionsEqual(test.prevConds, test.curConds) - g.Expect(equal).To(Equal(test.expEqual)) - }) - } -} - func TestEqualPointers(t *testing.T) { tests := []struct { p1 *string @@ -860,7 +986,7 @@ func TestBtpStatusEqual(t *testing.T) { Name: gatewayv1alpha2.ObjectName(ancestorName), }, ControllerName: gatewayv1alpha2.GatewayController(ctlrName), - Conditions: []v1.Condition{{Type: "otherType", Status: "otherStatus"}}, + Conditions: []metav1.Condition{{Type: "otherType", Status: "otherStatus"}}, }, }, }