Merge HTTPRoute conditions (#1220)

Problem: NGF overwrites the HTTPRoute parent statuses written by other Gateway API controllers. Solution: Merge the existing HTTPRoute parent statuses owned by other controllers with NGF's parent statuses.
nginx · Nov 3, 2023 · 40143a1 · 40143a1
1 parent c0c586b
commit 40143a1
Show file tree

Hide file tree

Showing 4 changed files with 75 additions and 5 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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()