Skip to content

Commit a9b0f3f

Browse files
bjee19bjee19
committed
Add fix for inference status conformance test
1 parent ba7bd6d commit a9b0f3f

File tree

3 files changed

+207
-40
lines changed

3 files changed

+207
-40
lines changed

‎internal/controller/handler.go‎

Lines changed: 23 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ import (
1818
"k8s.io/client-go/tools/record"
1919
"sigs.k8s.io/controller-runtime/pkg/client"
2020
"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
21+
inference "sigs.k8s.io/gateway-api-inference-extension/api/v1"
2122
gatewayv1 "sigs.k8s.io/gateway-api/apis/v1"
2223

2324
ngfAPI "github.com/nginx/nginx-gateway-fabric/v2/apis/v1alpha1"
@@ -368,7 +369,28 @@ func (h *eventHandlerImpl) updateStatuses(ctx context.Context, gr *graph.Graph,
368369
transitionTime,
369370
h.cfg.gatewayCtlrName,
370371
)
371-
inferencePoolReqs := status.PrepareInferencePoolRequests(gr.ReferencedInferencePools, transitionTime)
372+
373+
// unfortunately, status is not on clusterState stored by the change processor, so we need to make a k8sAPI call here
374+
ipList := &inference.InferencePoolList{}
375+
err = h.cfg.k8sClient.List(ctx, ipList)
376+
if err != nil {
377+
msg := "error listing InferencePools for status update"
378+
h.cfg.logger.Error(err, msg)
379+
h.cfg.eventRecorder.Eventf(
380+
&inference.InferencePoolList{},
381+
v1.EventTypeWarning,
382+
"ListInferencePoolsFailed",
383+
msg+": %s",
384+
err.Error(),
385+
)
386+
ipList = &inference.InferencePoolList{} // reset to empty list to avoid nil pointer dereference
387+
}
388+
inferencePoolReqs := status.PrepareInferencePoolRequests(
389+
gr.ReferencedInferencePools,
390+
ipList,
391+
gr.Gateways,
392+
transitionTime,
393+
)
372394

373395
reqs := make(
374396
[]status.UpdateRequest,

‎internal/controller/status/prepare_requests.go‎

Lines changed: 69 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -523,12 +523,64 @@ func PrepareNginxGatewayStatus(
523523

524524
// PrepareInferencePoolRequests prepares status UpdateRequests for the given InferencePools.
525525
func PrepareInferencePoolRequests(
526-
inferencePools map[types.NamespacedName]*graph.ReferencedInferencePool,
526+
referencedInferencePools map[types.NamespacedName]*graph.ReferencedInferencePool,
527+
clusterInferencePoolList *inference.InferencePoolList,
528+
referencedGateways map[types.NamespacedName]*graph.Gateway,
527529
transitionTime metav1.Time,
528530
) []UpdateRequest {
529-
reqs := make([]UpdateRequest, 0, len(inferencePools))
531+
reqs := make([]UpdateRequest, 0, len(referencedInferencePools))
532+
533+
// Create parent references from referenced gateways
534+
nginxGatewayParentRefs := make([]inference.ParentReference, 0, len(referencedGateways))
535+
for _, gateway := range referencedGateways {
536+
parentRef := inference.ParentReference{
537+
Name: inference.ObjectName(gateway.Source.GetName()),
538+
Namespace: inference.Namespace(gateway.Source.GetNamespace()),
539+
Group: helpers.GetPointer(inference.Group(gateway.Source.GroupVersionKind().Group)),
540+
Kind: kinds.Gateway,
541+
}
542+
nginxGatewayParentRefs = append(nginxGatewayParentRefs, parentRef)
543+
}
544+
545+
if clusterInferencePoolList != nil {
546+
for _, pool := range clusterInferencePoolList.Items {
547+
nsname := types.NamespacedName{
548+
Namespace: pool.Namespace,
549+
Name: pool.Name,
550+
}
551+
552+
// If the pool is in the cluster, but not referenced, we need to check
553+
// if any of its parents are an nginx Gateway, if so, we need to remove them.
554+
if referencedInferencePools[nsname] == nil {
555+
// represents parentRefs that are NOT nginx gateways
556+
filteredParents := make([]inference.ParentStatus, 0, len(pool.Status.Parents))
557+
for _, parent := range pool.Status.Parents {
558+
// if the parent.ParentRef is not in the list of nginx gateways, keep it
559+
// otherwise, we are removing it from the status
560+
if !containsParentReference(nginxGatewayParentRefs, parent.ParentRef) {
561+
filteredParents = append(filteredParents, parent)
562+
}
563+
}
564+
565+
// Create an update request to set the filtered parents
566+
if len(filteredParents) != len(pool.Status.Parents) {
567+
status := inference.InferencePoolStatus{
568+
Parents: filteredParents,
569+
}
570+
571+
req := UpdateRequest{
572+
NsName: nsname,
573+
ResourceType: &inference.InferencePool{},
574+
Setter: newInferencePoolStatusSetter(status),
575+
}
530576

531-
for nsname, pool := range inferencePools {
577+
reqs = append(reqs, req)
578+
}
579+
}
580+
}
581+
}
582+
583+
for nsname, pool := range referencedInferencePools {
532584
if pool.Source == nil {
533585
continue
534586
}
@@ -573,3 +625,17 @@ func PrepareInferencePoolRequests(
573625

574626
return reqs
575627
}
628+
629+
// containsParentReference checks if a ParentReference exists in a slice of ParentReferences
630+
// by comparing Name, Namespace, and Kind fields.
631+
func containsParentReference(parentRefs []inference.ParentReference, target inference.ParentReference) bool {
632+
for _, ref := range parentRefs {
633+
if ref.Name == target.Name &&
634+
ref.Namespace == target.Namespace &&
635+
ref.Kind == target.Kind {
636+
return true
637+
}
638+
}
639+
640+
return false
641+
}

‎internal/controller/status/prepare_requests_test.go‎

Lines changed: 115 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -2183,27 +2183,88 @@ func TestBuildInferencePoolStatuses(t *testing.T) {
21832183
Message: "Inference pool references a valid ExtensionRef.",
21842184
}
21852185

2186+
referencedGateways := map[types.NamespacedName]*graph.Gateway{
2187+
{Namespace: "test", Name: "gateway-1"}: {
2188+
Source: &v1.Gateway{
2189+
ObjectMeta: metav1.ObjectMeta{
2190+
Name: "gateway-1",
2191+
Namespace: "test",
2192+
},
2193+
},
2194+
Valid: true,
2195+
},
2196+
{Namespace: "test", Name: "gateway-2"}: {
2197+
Source: &v1.Gateway{
2198+
ObjectMeta: metav1.ObjectMeta{
2199+
Name: "gateway-2",
2200+
Namespace: "test",
2201+
},
2202+
},
2203+
Valid: true,
2204+
},
2205+
}
2206+
2207+
validInferencePool := &inference.InferencePool{
2208+
ObjectMeta: metav1.ObjectMeta{
2209+
Name: "valid-inference-pool",
2210+
Namespace: "test",
2211+
Generation: 1,
2212+
},
2213+
}
2214+
2215+
validInferencePoolWithInvalidExtensionRef := &inference.InferencePool{
2216+
ObjectMeta: metav1.ObjectMeta{
2217+
Name: "valid-inference-pool",
2218+
Namespace: "test",
2219+
Generation: 1,
2220+
},
2221+
Spec: inference.InferencePoolSpec{
2222+
EndpointPickerRef: inference.EndpointPickerRef{
2223+
Name: inference.ObjectName("invalid-extension-ref"),
2224+
},
2225+
},
2226+
}
2227+
2228+
validInferencePoolWithStatus := &inference.InferencePool{
2229+
ObjectMeta: metav1.ObjectMeta{
2230+
Name: "valid-inference-pool-with-status",
2231+
Namespace: "test",
2232+
Generation: 1,
2233+
},
2234+
Status: inference.InferencePoolStatus{
2235+
Parents: []inference.ParentStatus{
2236+
{
2237+
Conditions: []metav1.Condition{
2238+
validAcceptedCondition,
2239+
validResolvedRefsCondition,
2240+
},
2241+
ParentRef: inference.ParentReference{
2242+
Namespace: inference.Namespace("test"),
2243+
Name: "gateway-1",
2244+
Kind: kinds.Gateway,
2245+
Group: helpers.GetPointer(inference.Group(group)),
2246+
},
2247+
},
2248+
},
2249+
},
2250+
}
2251+
21862252
tests := []struct {
2187-
inferencePool map[types.NamespacedName]*graph.ReferencedInferencePool
2188-
expectedPoolWithStatus map[types.NamespacedName]inference.InferencePoolStatus
2189-
name string
2190-
expectedReqs int
2253+
referencedInferencePool map[types.NamespacedName]*graph.ReferencedInferencePool
2254+
expectedPoolWithStatus map[types.NamespacedName]inference.InferencePoolStatus
2255+
name string
2256+
clusterInferencePools inference.InferencePoolList
2257+
expectedReqs int
21912258
}{
21922259
{
21932260
name: "no referenced inferencePools",
21942261
expectedReqs: 0,
21952262
},
21962263
{
21972264
name: "an inference pool has valid status for multiple gateways",
2198-
inferencePool: map[types.NamespacedName]*graph.ReferencedInferencePool{
2265+
referencedInferencePool: map[types.NamespacedName]*graph.ReferencedInferencePool{
21992266
{Namespace: "test", Name: "valid-inference-pool"}: {
2200-
Source: &inference.InferencePool{
2201-
ObjectMeta: metav1.ObjectMeta{
2202-
Name: "valid-inference-pool",
2203-
Namespace: "test",
2204-
Generation: 1,
2205-
},
2206-
},
2267+
Source: validInferencePool,
22072268
Gateways: []*v1.Gateway{
22082269
{
22092270
ObjectMeta: metav1.ObjectMeta{
@@ -2220,6 +2281,11 @@ func TestBuildInferencePoolStatuses(t *testing.T) {
22202281
},
22212282
},
22222283
},
2284+
clusterInferencePools: inference.InferencePoolList{
2285+
Items: []inference.InferencePool{
2286+
*validInferencePool,
2287+
},
2288+
},
22232289
expectedReqs: 1,
22242290
expectedPoolWithStatus: map[types.NamespacedName]inference.InferencePoolStatus{
22252291
{Namespace: "test", Name: "valid-inference-pool"}: {
@@ -2254,20 +2320,9 @@ func TestBuildInferencePoolStatuses(t *testing.T) {
22542320
},
22552321
{
22562322
name: "an inference pool has accepted valid status and is referenced by invalid extension ref",
2257-
inferencePool: map[types.NamespacedName]*graph.ReferencedInferencePool{
2323+
referencedInferencePool: map[types.NamespacedName]*graph.ReferencedInferencePool{
22582324
{Namespace: "test", Name: "valid-inference-pool"}: {
2259-
Source: &inference.InferencePool{
2260-
ObjectMeta: metav1.ObjectMeta{
2261-
Name: "valid-inference-pool",
2262-
Namespace: "test",
2263-
Generation: 1,
2264-
},
2265-
Spec: inference.InferencePoolSpec{
2266-
EndpointPickerRef: inference.EndpointPickerRef{
2267-
Name: inference.ObjectName("invalid-extension-ref"),
2268-
},
2269-
},
2270-
},
2325+
Source: validInferencePoolWithInvalidExtensionRef,
22712326
Gateways: []*v1.Gateway{
22722327
{
22732328
ObjectMeta: metav1.ObjectMeta{
@@ -2281,6 +2336,11 @@ func TestBuildInferencePoolStatuses(t *testing.T) {
22812336
},
22822337
},
22832338
},
2339+
clusterInferencePools: inference.InferencePoolList{
2340+
Items: []inference.InferencePool{
2341+
*validInferencePoolWithInvalidExtensionRef,
2342+
},
2343+
},
22842344
expectedReqs: 1,
22852345
expectedPoolWithStatus: map[types.NamespacedName]inference.InferencePoolStatus{
22862346
{Namespace: "test", Name: "valid-inference-pool"}: {
@@ -2310,15 +2370,9 @@ func TestBuildInferencePoolStatuses(t *testing.T) {
23102370
},
23112371
{
23122372
name: "an inference pool is referencing an invalid route and is referenced by invalid extension ref",
2313-
inferencePool: map[types.NamespacedName]*graph.ReferencedInferencePool{
2373+
referencedInferencePool: map[types.NamespacedName]*graph.ReferencedInferencePool{
23142374
{Namespace: "test", Name: "valid-inference-pool"}: {
2315-
Source: &inference.InferencePool{
2316-
ObjectMeta: metav1.ObjectMeta{
2317-
Name: "valid-inference-pool",
2318-
Namespace: "test",
2319-
Generation: 1,
2320-
},
2321-
},
2375+
Source: validInferencePool,
23222376
Gateways: []*v1.Gateway{
23232377
{
23242378
ObjectMeta: metav1.ObjectMeta{
@@ -2333,6 +2387,11 @@ func TestBuildInferencePoolStatuses(t *testing.T) {
23332387
},
23342388
},
23352389
},
2390+
clusterInferencePools: inference.InferencePoolList{
2391+
Items: []inference.InferencePool{
2392+
*validInferencePool,
2393+
},
2394+
},
23362395
expectedReqs: 1,
23372396
expectedPoolWithStatus: map[types.NamespacedName]inference.InferencePoolStatus{
23382397
{Namespace: "test", Name: "valid-inference-pool"}: {
@@ -2367,6 +2426,21 @@ func TestBuildInferencePoolStatuses(t *testing.T) {
23672426
},
23682427
},
23692428
},
2429+
{
2430+
name: "inference pool status gets removed if no longer referenced",
2431+
referencedInferencePool: map[types.NamespacedName]*graph.ReferencedInferencePool{},
2432+
clusterInferencePools: inference.InferencePoolList{
2433+
Items: []inference.InferencePool{
2434+
*validInferencePoolWithStatus,
2435+
},
2436+
},
2437+
expectedReqs: 1,
2438+
expectedPoolWithStatus: map[types.NamespacedName]inference.InferencePoolStatus{
2439+
{Namespace: "test", Name: "valid-inference-pool-with-status"}: {
2440+
Parents: nil,
2441+
},
2442+
},
2443+
},
23702444
}
23712445

23722446
for _, test := range tests {
@@ -2375,13 +2449,18 @@ func TestBuildInferencePoolStatuses(t *testing.T) {
23752449
g := NewWithT(t)
23762450

23772451
k8sClient := createK8sClientFor(&inference.InferencePool{})
2378-
for _, ip := range test.inferencePool {
2379-
err := k8sClient.Create(context.Background(), ip.Source)
2452+
for _, ip := range test.clusterInferencePools.Items {
2453+
err := k8sClient.Create(context.Background(), &ip)
23802454
g.Expect(err).ToNot(HaveOccurred())
23812455
}
23822456

23832457
updater := NewUpdater(k8sClient, logr.Discard())
2384-
reqs := PrepareInferencePoolRequests(test.inferencePool, transitionTime)
2458+
reqs := PrepareInferencePoolRequests(
2459+
test.referencedInferencePool,
2460+
&test.clusterInferencePools,
2461+
referencedGateways,
2462+
transitionTime,
2463+
)
23852464
g.Expect(reqs).To(HaveLen(test.expectedReqs))
23862465
updater.Update(context.Background(), reqs...)
23872466

0 commit comments

Comments
 (0)