From 6851dd9d94e07725579d54072583f20fbff108ed Mon Sep 17 00:00:00 2001 From: Kate Osborn Date: Fri, 3 Nov 2023 09:17:54 -0600 Subject: [PATCH] Merge HTTPRoute conditions --- internal/framework/status/httproute.go | 13 ++++- internal/framework/status/httproute_test.go | 63 ++++++++++++++++++++- internal/framework/status/setters.go | 1 + internal/framework/status/updater.go | 3 - 4 files changed, 75 insertions(+), 5 deletions(-) diff --git a/internal/framework/status/httproute.go b/internal/framework/status/httproute.go index 9b93e378a..1912f80c8 100644 --- a/internal/framework/status/httproute.go +++ b/internal/framework/status/httproute.go @@ -7,11 +7,22 @@ import ( // prepareHTTPRouteStatus prepares the status for an HTTPRoute resource. func prepareHTTPRouteStatus( + oldStatus v1beta1.HTTPRouteStatus, status HTTPRouteStatus, gatewayCtlrName string, transitionTime metav1.Time, ) v1beta1.HTTPRouteStatus { - parents := make([]v1beta1.RouteParentStatus, 0, len(status.ParentStatuses)) + // 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([]v1beta1.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 diff --git a/internal/framework/status/httproute_test.go b/internal/framework/status/httproute_test.go index deafbde2d..1c31ae43b 100644 --- a/internal/framework/status/httproute_test.go +++ b/internal/framework/status/httproute_test.go @@ -35,9 +35,70 @@ func TestPrepareHTTPRouteStatus(t *testing.T) { gatewayCtlrName := "test.example.com" transitionTime := metav1.NewTime(time.Now()) + oldStatus := v1beta1.HTTPRouteStatus{ + RouteStatus: v1beta1.RouteStatus{ + Parents: []v1beta1.RouteParentStatus{ + { + ParentRef: v1beta1.ParentReference{ + Namespace: helpers.GetPointer(v1beta1.Namespace(gwNsName1.Namespace)), + Name: v1beta1.ObjectName(gwNsName1.Name), + SectionName: helpers.GetPointer[v1beta1.SectionName]("http"), + }, + ControllerName: v1beta1.GatewayController(gatewayCtlrName), + Conditions: CreateExpectedAPIConditions("Old", 1, transitionTime), + }, + { + ParentRef: v1beta1.ParentReference{ + Namespace: helpers.GetPointer(v1beta1.Namespace(gwNsName1.Namespace)), + Name: v1beta1.ObjectName(gwNsName1.Name), + SectionName: helpers.GetPointer[v1beta1.SectionName]("http"), + }, + ControllerName: v1beta1.GatewayController("not-our-controller"), + Conditions: CreateExpectedAPIConditions("Test", 1, transitionTime), + }, + { + ParentRef: v1beta1.ParentReference{ + Namespace: helpers.GetPointer(v1beta1.Namespace(gwNsName2.Namespace)), + Name: v1beta1.ObjectName(gwNsName2.Name), + SectionName: nil, + }, + ControllerName: v1beta1.GatewayController(gatewayCtlrName), + Conditions: CreateExpectedAPIConditions("Old", 1, transitionTime), + }, + { + ParentRef: v1beta1.ParentReference{ + Namespace: helpers.GetPointer(v1beta1.Namespace(gwNsName2.Namespace)), + Name: v1beta1.ObjectName(gwNsName2.Name), + SectionName: nil, + }, + ControllerName: v1beta1.GatewayController("not-our-controller"), + Conditions: CreateExpectedAPIConditions("Test", 1, transitionTime), + }, + }, + }, + } + expected := v1beta1.HTTPRouteStatus{ RouteStatus: v1beta1.RouteStatus{ Parents: []v1beta1.RouteParentStatus{ + { + ParentRef: v1beta1.ParentReference{ + Namespace: helpers.GetPointer(v1beta1.Namespace(gwNsName1.Namespace)), + Name: v1beta1.ObjectName(gwNsName1.Name), + SectionName: helpers.GetPointer[v1beta1.SectionName]("http"), + }, + ControllerName: v1beta1.GatewayController("not-our-controller"), + Conditions: CreateExpectedAPIConditions("Test", 1, transitionTime), + }, + { + ParentRef: v1beta1.ParentReference{ + Namespace: helpers.GetPointer(v1beta1.Namespace(gwNsName2.Namespace)), + Name: v1beta1.ObjectName(gwNsName2.Name), + SectionName: nil, + }, + ControllerName: v1beta1.GatewayController("not-our-controller"), + Conditions: CreateExpectedAPIConditions("Test", 1, transitionTime), + }, { ParentRef: v1beta1.ParentReference{ Namespace: helpers.GetPointer(v1beta1.Namespace(gwNsName1.Namespace)), @@ -62,6 +123,6 @@ func TestPrepareHTTPRouteStatus(t *testing.T) { g := NewWithT(t) - result := prepareHTTPRouteStatus(status, gatewayCtlrName, transitionTime) + result := prepareHTTPRouteStatus(oldStatus, status, gatewayCtlrName, transitionTime) g.Expect(helpers.Diff(expected, result)).To(BeEmpty()) } diff --git a/internal/framework/status/setters.go b/internal/framework/status/setters.go index 40effc14c..3f4f1982d 100644 --- a/internal/framework/status/setters.go +++ b/internal/framework/status/setters.go @@ -69,6 +69,7 @@ func newHTTPRouteStatusSetter(gatewayCtlrName string, clock Clock, rs HTTPRouteS return func(object client.Object) bool { hr := object.(*v1beta1.HTTPRoute) status := prepareHTTPRouteStatus( + hr.Status, rs, gatewayCtlrName, clock.Now(), diff --git a/internal/framework/status/updater.go b/internal/framework/status/updater.go index 16797ea34..3397915e2 100644 --- a/internal/framework/status/updater.go +++ b/internal/framework/status/updater.go @@ -125,9 +125,6 @@ func NewUpdater(cfg UpdaterConfig) *UpdaterImpl { } func (upd *UpdaterImpl) Update(ctx context.Context, status Status) { - // FIXME(pleshakov) Merge the new Conditions in the status with the existing Conditions - // https://github.com/nginxinc/nginx-gateway-fabric/issues/558 - defer upd.lock.Unlock() upd.lock.Lock()