Add fix for inference pool status (#4088)

When setting inference pool statuses, it loops through all the inference pools and checks if there are any nginx gateways that have parentRefs in the statuses. If there are AND the infernece pool is not referenced (not connected to the graph), it will remove that parentRef.

Author: bjee19

deploy/inference-nginx-plus/deploy.yaml

@@ -15,6 +15,7 @@ metadata:
   namespace: nginx-gateway
 ---
 apiVersion: v1
+automountServiceAccountToken: false
 kind: ServiceAccount
 metadata:
   labels:
@@ -54,15 +55,33 @@ metadata:
 rules:
 - apiGroups:
   - ""
-  - apps
-  - autoscaling
   resources:
   - secrets
   - configmaps
   - serviceaccounts
   - services
+  verbs:
+  - create
+  - update
+  - delete
+  - list
+  - get
+  - watch
+- apiGroups:
+  - apps
+  resources:
   - deployments
   - daemonsets
+  verbs:
+  - create
+  - update
+  - delete
+  - list
+  - get
+  - watch
+- apiGroups:
+  - autoscaling
+  resources:
   - horizontalpodautoscalers
   verbs:
   - create
@@ -358,6 +377,7 @@ spec:
     metadata:
       annotations: null
     spec:
+      automountServiceAccountToken: true
       containers:
       - args:
         - generate-certs

deploy/inference/deploy.yaml

@@ -15,6 +15,7 @@ metadata:
   namespace: nginx-gateway
 ---
 apiVersion: v1
+automountServiceAccountToken: false
 kind: ServiceAccount
 metadata:
   labels:
@@ -54,15 +55,33 @@ metadata:
 rules:
 - apiGroups:
   - ""
-  - apps
-  - autoscaling
   resources:
   - secrets
   - configmaps
   - serviceaccounts
   - services
+  verbs:
+  - create
+  - update
+  - delete
+  - list
+  - get
+  - watch
+- apiGroups:
+  - apps
+  resources:
   - deployments
   - daemonsets
+  verbs:
+  - create
+  - update
+  - delete
+  - list
+  - get
+  - watch
+- apiGroups:
+  - autoscaling
+  resources:
   - horizontalpodautoscalers
   verbs:
   - create
@@ -354,6 +373,7 @@ spec:
     metadata:
       annotations: null
     spec:
+      automountServiceAccountToken: true
       containers:
       - args:
         - generate-certs

internal/controller/handler.go

@@ -18,6 +18,7 @@ import (
 	"k8s.io/client-go/tools/record"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
+	inference "sigs.k8s.io/gateway-api-inference-extension/api/v1"
 	gatewayv1 "sigs.k8s.io/gateway-api/apis/v1"
 
 	ngfAPI "github.com/nginx/nginx-gateway-fabric/v2/apis/v1alpha1"
@@ -368,7 +369,28 @@ func (h *eventHandlerImpl) updateStatuses(ctx context.Context, gr *graph.Graph,
 		transitionTime,
 		h.cfg.gatewayCtlrName,
 	)
-	inferencePoolReqs := status.PrepareInferencePoolRequests(gr.ReferencedInferencePools, transitionTime)
+
+	// unfortunately, status is not on clusterState stored by the change processor, so we need to make a k8sAPI call here
+	ipList := &inference.InferencePoolList{}
+	err = h.cfg.k8sClient.List(ctx, ipList)
+	if err != nil {
+		msg := "error listing InferencePools for status update"
+		h.cfg.logger.Error(err, msg)
+		h.cfg.eventRecorder.Eventf(
+			&inference.InferencePoolList{},
+			v1.EventTypeWarning,
+			"ListInferencePoolsFailed",
+			msg+": %s",
+			err.Error(),
+		)
+		ipList = &inference.InferencePoolList{} // reset to empty list to avoid nil pointer dereference
+	}
+	inferencePoolReqs := status.PrepareInferencePoolRequests(
+		gr.ReferencedInferencePools,
+		ipList,
+		gr.Gateways,
+		transitionTime,
+	)
 
 	reqs := make(
 		[]status.UpdateRequest,

internal/controller/status/prepare_requests.go

@@ -523,12 +523,64 @@ func PrepareNginxGatewayStatus(
 
 // PrepareInferencePoolRequests prepares status UpdateRequests for the given InferencePools.
 func PrepareInferencePoolRequests(
-	inferencePools map[types.NamespacedName]*graph.ReferencedInferencePool,
+	referencedInferencePools map[types.NamespacedName]*graph.ReferencedInferencePool,
+	clusterInferencePoolList *inference.InferencePoolList,
+	referencedGateways map[types.NamespacedName]*graph.Gateway,
 	transitionTime metav1.Time,
 ) []UpdateRequest {
-	reqs := make([]UpdateRequest, 0, len(inferencePools))
+	reqs := make([]UpdateRequest, 0, len(referencedInferencePools))
+
+	// Create parent references from referenced gateways
+	nginxGatewayParentRefs := make([]inference.ParentReference, 0, len(referencedGateways))
+	for _, gateway := range referencedGateways {
+		parentRef := inference.ParentReference{
+			Name:      inference.ObjectName(gateway.Source.GetName()),
+			Namespace: inference.Namespace(gateway.Source.GetNamespace()),
+			Group:     helpers.GetPointer(inference.Group(gateway.Source.GroupVersionKind().Group)),
+			Kind:      kinds.Gateway,
+		}
+		nginxGatewayParentRefs = append(nginxGatewayParentRefs, parentRef)
+	}
+
+	if clusterInferencePoolList != nil {
+		for _, pool := range clusterInferencePoolList.Items {
+			nsname := types.NamespacedName{
+				Namespace: pool.Namespace,
+				Name:      pool.Name,
+			}
+
+			// If the pool is in the cluster, but not referenced, we need to check
+			// if any of its parents are an nginx Gateway, if so, we need to remove them.
+			if referencedInferencePools[nsname] == nil {
+				// represents parentRefs that are NOT nginx gateways
+				filteredParents := make([]inference.ParentStatus, 0, len(pool.Status.Parents))
+				for _, parent := range pool.Status.Parents {
+					// if the parent.ParentRef is not in the list of nginx gateways, keep it
+					// otherwise, we are removing it from the status
+					if !containsParentReference(nginxGatewayParentRefs, parent.ParentRef) {
+						filteredParents = append(filteredParents, parent)
+					}
+				}
+
+				// Create an update request to set the filtered parents
+				if len(filteredParents) != len(pool.Status.Parents) {
+					status := inference.InferencePoolStatus{
+						Parents: filteredParents,
+					}
+
+					req := UpdateRequest{
+						NsName:       nsname,
+						ResourceType: &inference.InferencePool{},
+						Setter:       newInferencePoolStatusSetter(status),
+					}
 
-	for nsname, pool := range inferencePools {
+					reqs = append(reqs, req)
+				}
+			}
+		}
+	}
+
+	for nsname, pool := range referencedInferencePools {
 		if pool.Source == nil {
 			continue
 		}
@@ -573,3 +625,17 @@ func PrepareInferencePoolRequests(
 
 	return reqs
 }
+
+// containsParentReference checks if a ParentReference exists in a slice of ParentReferences
+// by comparing Name, Namespace, and Kind fields.
+func containsParentReference(parentRefs []inference.ParentReference, target inference.ParentReference) bool {
+	for _, ref := range parentRefs {
+		if ref.Name == target.Name &&
+			ref.Namespace == target.Namespace &&
+			ref.Kind == target.Kind {
+			return true
+		}
+	}
+
+	return false
+}