From 847343e6bf72ec3d90d3fc4903d5bbbc5463e68a Mon Sep 17 00:00:00 2001 From: Daneyon Hansen Date: Tue, 4 Feb 2025 16:50:44 +0000 Subject: [PATCH] Adds ErrorNotFound Handling for Reconciler Signed-off-by: Daneyon Hansen --- .../backend/inferencemodel_reconciler.go | 34 +++-- .../backend/inferencemodel_reconciler_test.go | 133 ++++++++++++++++-- 2 files changed, 138 insertions(+), 29 deletions(-) diff --git a/pkg/ext-proc/backend/inferencemodel_reconciler.go b/pkg/ext-proc/backend/inferencemodel_reconciler.go index 3164e0981..1c1d22787 100644 --- a/pkg/ext-proc/backend/inferencemodel_reconciler.go +++ b/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) +} diff --git a/pkg/ext-proc/backend/inferencemodel_reconciler_test.go b/pkg/ext-proc/backend/inferencemodel_reconciler_test.go index 117766b9c..57590e7e2 100644 --- a/pkg/ext-proc/backend/inferencemodel_reconciler_test.go +++ b/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{}