Adds ErrorNotFound Handling for Reconciler (#286)

Signed-off-by: Daneyon Hansen <[email protected]>

Author: danehans

pkg/ext-proc/backend/inferencemodel_reconciler.go

@@ -5,6 +5,7 @@ import (
 
 	"inference.networking.x-k8s.io/gateway-api-inference-extension/api/v1alpha1"
 	logutil "inference.networking.x-k8s.io/gateway-api-inference-extension/pkg/ext-proc/util/logging"
+	"k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/client-go/tools/record"
@@ -25,32 +26,37 @@ func (c *InferenceModelReconciler) Reconcile(ctx context.Context, req ctrl.Reque
 	if req.Namespace != c.PoolNamespacedName.Namespace {
 		return ctrl.Result{}, nil
 	}
-	klog.V(1).Infof("reconciling InferenceModel %v", req.NamespacedName)
-
-	service := &v1alpha1.InferenceModel{}
-	if err := c.Get(ctx, req.NamespacedName, service); err != nil {
-		klog.Error(err, "unable to get InferencePool")
+	klog.V(1).Infof("Reconciling InferenceModel %v", req.NamespacedName)
+
+	infModel := &v1alpha1.InferenceModel{}
+	if err := c.Get(ctx, req.NamespacedName, infModel); err != nil {
+		if errors.IsNotFound(err) {
+			klog.V(1).Infof("InferenceModel %v not found. Removing from datastore since object must be deleted", req.NamespacedName)
+			c.Datastore.InferenceModels.Delete(infModel.Spec.ModelName)
+			return ctrl.Result{}, nil
+		}
+		klog.Error(err, "Unable to get InferenceModel")
 		return ctrl.Result{}, err
 	}
 
-	c.updateDatastore(service)
+	c.updateDatastore(infModel)
 	return ctrl.Result{}, nil
 }
 
-func (c *InferenceModelReconciler) SetupWithManager(mgr ctrl.Manager) error {
-	return ctrl.NewControllerManagedBy(mgr).
-		For(&v1alpha1.InferenceModel{}).
-		Complete(c)
-}
-
 func (c *InferenceModelReconciler) updateDatastore(infModel *v1alpha1.InferenceModel) {
 	if infModel.Spec.PoolRef.Name == c.PoolNamespacedName.Name {
 		klog.V(1).Infof("Incoming pool ref %v, server pool name: %v", infModel.Spec.PoolRef, c.PoolNamespacedName.Name)
-		klog.V(1).Infof("Adding/Updating inference model: %v", infModel.Spec.ModelName)
+		klog.V(1).Infof("Adding/Updating InferenceModel: %v", infModel.Spec.ModelName)
 		c.Datastore.InferenceModels.Store(infModel.Spec.ModelName, infModel)
 		return
 	}
-	klog.V(logutil.DEFAULT).Infof("Removing/Not adding inference model: %v", infModel.Spec.ModelName)
+	klog.V(logutil.DEFAULT).Infof("Removing/Not adding InferenceModel: %v", infModel.Spec.ModelName)
 	// If we get here. The model is not relevant to this pool, remove.
 	c.Datastore.InferenceModels.Delete(infModel.Spec.ModelName)
 }
+
+func (c *InferenceModelReconciler) SetupWithManager(mgr ctrl.Manager) error {
+	return ctrl.NewControllerManagedBy(mgr).
+		For(&v1alpha1.InferenceModel{}).
+		Complete(c)
+}

pkg/ext-proc/backend/inferencemodel_reconciler_test.go

@@ -1,16 +1,22 @@
 package backend
 
 import (
+	"context"
 	"sync"
 	"testing"
 
+	"k8s.io/apimachinery/pkg/runtime"
+	"k8s.io/client-go/tools/record"
+	ctrl "sigs.k8s.io/controller-runtime"
+	"sigs.k8s.io/controller-runtime/pkg/client/fake"
+
 	"inference.networking.x-k8s.io/gateway-api-inference-extension/api/v1alpha1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/types"
 )
 
 var (
-	service1 = &v1alpha1.InferenceModel{
+	infModel1 = &v1alpha1.InferenceModel{
 		Spec: v1alpha1.InferenceModelSpec{
 			ModelName: "fake model1",
 			PoolRef:   v1alpha1.PoolObjectReference{Name: "test-pool"},
@@ -19,7 +25,7 @@ var (
 			Name: "test-service",
 		},
 	}
-	service1Modified = &v1alpha1.InferenceModel{
+	infModel1Modified = &v1alpha1.InferenceModel{
 		Spec: v1alpha1.InferenceModelSpec{
 			ModelName: "fake model1",
 			PoolRef:   v1alpha1.PoolObjectReference{Name: "test-poolio"},
@@ -28,7 +34,7 @@ var (
 			Name: "test-service",
 		},
 	}
-	service2 = &v1alpha1.InferenceModel{
+	infModel2 = &v1alpha1.InferenceModel{
 		Spec: v1alpha1.InferenceModelSpec{
 			ModelName: "fake model",
 			PoolRef:   v1alpha1.PoolObjectReference{Name: "test-pool"},
@@ -60,8 +66,8 @@ func TestUpdateDatastore_InferenceModelReconciler(t *testing.T) {
 				},
 				InferenceModels: &sync.Map{},
 			},
-			incomingService:     service1,
-			wantInferenceModels: populateServiceMap(service1),
+			incomingService:     infModel1,
+			wantInferenceModels: populateServiceMap(infModel1),
 		},
 		{
 			name: "Removing existing service.",
@@ -75,9 +81,9 @@ func TestUpdateDatastore_InferenceModelReconciler(t *testing.T) {
 						ResourceVersion: "Old and boring",
 					},
 				},
-				InferenceModels: populateServiceMap(service1),
+				InferenceModels: populateServiceMap(infModel1),
 			},
-			incomingService:     service1Modified,
+			incomingService:     infModel1Modified,
 			wantInferenceModels: populateServiceMap(),
 		},
 		{
@@ -92,7 +98,7 @@ func TestUpdateDatastore_InferenceModelReconciler(t *testing.T) {
 						ResourceVersion: "Old and boring",
 					},
 				},
-				InferenceModels: populateServiceMap(service1),
+				InferenceModels: populateServiceMap(infModel1),
 			},
 			incomingService: &v1alpha1.InferenceModel{
 				Spec: v1alpha1.InferenceModelSpec{
@@ -103,7 +109,7 @@ func TestUpdateDatastore_InferenceModelReconciler(t *testing.T) {
 					Name: "unrelated-service",
 				},
 			},
-			wantInferenceModels: populateServiceMap(service1),
+			wantInferenceModels: populateServiceMap(infModel1),
 		},
 		{
 			name: "Add to existing",
@@ -117,27 +123,124 @@ func TestUpdateDatastore_InferenceModelReconciler(t *testing.T) {
 						ResourceVersion: "Old and boring",
 					},
 				},
-				InferenceModels: populateServiceMap(service1),
+				InferenceModels: populateServiceMap(infModel1),
 			},
-			incomingService:     service2,
-			wantInferenceModels: populateServiceMap(service1, service2),
+			incomingService:     infModel2,
+			wantInferenceModels: populateServiceMap(infModel1, infModel2),
 		},
 	}
 	for _, test := range tests {
 		t.Run(test.name, func(t *testing.T) {
-			InferenceModelReconciler := &InferenceModelReconciler{
+			reconciler := &InferenceModelReconciler{
 				Datastore:          test.datastore,
 				PoolNamespacedName: types.NamespacedName{Name: test.datastore.inferencePool.Name},
 			}
-			InferenceModelReconciler.updateDatastore(test.incomingService)
+			reconciler.updateDatastore(test.incomingService)
 
-			if ok := mapsEqual(InferenceModelReconciler.Datastore.InferenceModels, test.wantInferenceModels); !ok {
+			if ok := mapsEqual(reconciler.Datastore.InferenceModels, test.wantInferenceModels); !ok {
 				t.Error("Maps are not equal")
 			}
 		})
 	}
 }
 
+func TestReconcile_ResourceNotFound(t *testing.T) {
+	// Set up the scheme.
+	scheme := runtime.NewScheme()
+	_ = v1alpha1.AddToScheme(scheme)
+
+	// Create a fake client with no InferenceModel objects.
+	fakeClient := fake.NewClientBuilder().WithScheme(scheme).Build()
+
+	// Create a minimal datastore.
+	datastore := &K8sDatastore{
+		InferenceModels: &sync.Map{},
+		inferencePool: &v1alpha1.InferencePool{
+			ObjectMeta: metav1.ObjectMeta{Name: "test-pool"},
+		},
+	}
+
+	// Create the reconciler.
+	reconciler := &InferenceModelReconciler{
+		Client:             fakeClient,
+		Scheme:             scheme,
+		Record:             record.NewFakeRecorder(10),
+		Datastore:          datastore,
+		PoolNamespacedName: types.NamespacedName{Name: "test-pool"},
+	}
+
+	// Create a request for a non-existent resource.
+	req := ctrl.Request{NamespacedName: types.NamespacedName{Name: "non-existent-model", Namespace: "default"}}
+
+	// Call Reconcile.
+	result, err := reconciler.Reconcile(context.Background(), req)
+	if err != nil {
+		t.Fatalf("expected no error when resource is not found, got %v", err)
+	}
+
+	// Check that no requeue is requested.
+	if result.Requeue || result.RequeueAfter != 0 {
+		t.Errorf("expected no requeue, got %+v", result)
+	}
+}
+
+func TestReconcile_ResourceExists(t *testing.T) {
+	// Set up the scheme.
+	scheme := runtime.NewScheme()
+	_ = v1alpha1.AddToScheme(scheme)
+
+	// Create an InferenceModel object.
+	existingModel := &v1alpha1.InferenceModel{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      "existing-model",
+			Namespace: "default",
+		},
+		Spec: v1alpha1.InferenceModelSpec{
+			ModelName: "fake-model",
+			PoolRef:   v1alpha1.PoolObjectReference{Name: "test-pool"},
+		},
+	}
+
+	// Create a fake client with the existing model.
+	fakeClient := fake.NewClientBuilder().WithScheme(scheme).WithObjects(existingModel).Build()
+
+	// Create a minimal datastore.
+	datastore := &K8sDatastore{
+		InferenceModels: &sync.Map{},
+		inferencePool: &v1alpha1.InferencePool{
+			ObjectMeta: metav1.ObjectMeta{Name: "test-pool"},
+		},
+	}
+
+	// Create the reconciler.
+	reconciler := &InferenceModelReconciler{
+		Client:             fakeClient,
+		Scheme:             scheme,
+		Record:             record.NewFakeRecorder(10),
+		Datastore:          datastore,
+		PoolNamespacedName: types.NamespacedName{Name: "test-pool", Namespace: "default"},
+	}
+
+	// Create a request for the existing resource.
+	req := ctrl.Request{NamespacedName: types.NamespacedName{Name: "existing-model", Namespace: "default"}}
+
+	// Call Reconcile.
+	result, err := reconciler.Reconcile(context.Background(), req)
+	if err != nil {
+		t.Fatalf("expected no error when resource exists, got %v", err)
+	}
+
+	// Check that no requeue is requested.
+	if result.Requeue || result.RequeueAfter != 0 {
+		t.Errorf("expected no requeue, got %+v", result)
+	}
+
+	// Verify that the datastore was updated.
+	if _, ok := datastore.InferenceModels.Load(existingModel.Spec.ModelName); !ok {
+		t.Errorf("expected datastore to contain model %q", existingModel.Spec.ModelName)
+	}
+}
+
 func populateServiceMap(services ...*v1alpha1.InferenceModel) *sync.Map {
 	returnVal := &sync.Map{}