From 7f8ed8e061da259c4d4d47316c3aa4aa6ecb257b Mon Sep 17 00:00:00 2001 From: Mike McKiernan Date: Mon, 12 Aug 2024 09:05:20 -0400 Subject: [PATCH 1/2] Error with predicted name If the PVC isn't created, return predicted name rather than pvc.Name. Signed-off-by: Mike McKiernan --- internal/controller/nimcache_controller.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/internal/controller/nimcache_controller.go b/internal/controller/nimcache_controller.go index 6c01da7f..ec56a94e 100644 --- a/internal/controller/nimcache_controller.go +++ b/internal/controller/nimcache_controller.go @@ -237,7 +237,7 @@ func (r *NIMCacheReconciler) reconcilePVC(ctx context.Context, nimCache *appsv1a if nimCache.Spec.Storage.PVC.Create != nil && *nimCache.Spec.Storage.PVC.Create { pvc, err = shared.ConstructPVC(nimCache.Spec.Storage.PVC, metav1.ObjectMeta{Name: pvcName, Namespace: nimCache.GetNamespace()}) if err != nil { - logger.Error(err, "Failed to construct pvc", "name", pvc.Name) + logger.Error(err, "Failed to construct pvc", "name", pvcName) return err } if err := controllerutil.SetControllerReference(nimCache, pvc, r.GetScheme()); err != nil { @@ -245,12 +245,12 @@ func (r *NIMCacheReconciler) reconcilePVC(ctx context.Context, nimCache *appsv1a } err = r.Create(ctx, pvc) if err != nil { - logger.Error(err, "Failed to create pvc", "name", pvc.Name) + logger.Error(err, "Failed to create pvc", "name", pvcName) return err } - logger.Info("Created PVC for NIM Cache", "pvc", pvcName) + logger.Info("Created PVC for NIM Cache", "pvc", pvc.Name) - conditions.UpdateCondition(&nimCache.Status.Conditions, appsv1alpha1.NimCacheConditionPVCCreated, metav1.ConditionTrue, "PVCCreated", "The PVC has been created for caching NIM") + conditions.UpdateCondition(&nimCache.Status.Conditions, appsv1alpha1.NimCacheConditionPVCCreated, metav1.ConditionTrue, "PVCCreated", "The PVC has been created for caching NIM model") nimCache.Status.State = appsv1alpha1.NimCacheStatusPVCCreated if err := r.Status().Update(ctx, nimCache); err != nil { logger.Error(err, "Failed to update status", "NIMCache", nimCache.Name) From a57125c3e98a67845b124becf026c9e496428f1d Mon Sep 17 00:00:00 2001 From: Mike McKiernan Date: Mon, 12 Aug 2024 09:16:41 -0400 Subject: [PATCH 2/2] Test for bad PVC specification Signed-off-by: Mike McKiernan --- .../controller/nimcache_controller_test.go | 23 +++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/internal/controller/nimcache_controller_test.go b/internal/controller/nimcache_controller_test.go index 6ce4f709..5a5b9367 100644 --- a/internal/controller/nimcache_controller_test.go +++ b/internal/controller/nimcache_controller_test.go @@ -109,6 +109,29 @@ var _ = Describe("NIMCache Controller", func() { return client.Get(ctx, pvcName, pvc) }, time.Second*10).Should(Succeed()) }) + + It("should return an error if the PVC size is not specified", func() { + ctx := context.TODO() + NIMCache := &appsv1alpha1.NIMCache{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-nimcache", + Namespace: "default", + }, + Spec: appsv1alpha1.NIMCacheSpec{ + Source: appsv1alpha1.NIMSource{NGC: &appsv1alpha1.NGCSource{ModelPuller: "test-container", PullSecret: "my-secret"}}, + Storage: appsv1alpha1.Storage{PVC: appsv1alpha1.PersistentVolumeClaim{Create: ptr.To[bool](true), StorageClass: "standard"}}, + }, + Status: appsv1alpha1.NIMCacheStatus{ + State: appsv1alpha1.NimCacheStatusNotReady, + }, + } + Expect(client.Create(ctx, NIMCache)).To(Succeed()) + + // Reconcile the resource + _, err := reconciler.reconcileNIMCache(ctx, NIMCache) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("failed to parse size for pvc creation")) + }) }) Context("When the Job completes", func() {