Merge pull request #53 from shivamerla/fix_external_pvc

Fix check for external PVC
NVIDIA · Aug 16, 2024 · d7030c1 · d7030c1
2 parents 8a01783 + 1f5caa8
commit d7030c1
Show file tree

Hide file tree

Showing 6 changed files with 17 additions and 25 deletions.
diff --git a/api/apps/v1alpha1/nimcache_types.go b/api/apps/v1alpha1/nimcache_types.go
@@ -124,7 +124,7 @@ type PersistentVolumeClaim struct {
 	// Create indicates to create a new PVC
 	Create *bool `json:"create,omitempty"`
 	// Name is the name of the PVC
-	Name *string `json:"name,omitempty"`
+	Name string `json:"name,omitempty"`
 	// StorageClass to be used for PVC creation. Leave it as empty if the PVC is already created.
 	StorageClass string `json:"storageClass,omitempty"`
 	// Size of the NIM cache in Gi, used during PVC creation
@@ -236,8 +236,8 @@ type NIMCache struct {
 // Prefers pvc.Name if explicitly set by the user in the NIMCache instance
 func (n *NIMCache) GetPVCName(pvc PersistentVolumeClaim) string {
 	pvcName := fmt.Sprintf("%s-pvc", n.GetName())
-	if pvc.Name != nil {
-		pvcName = *pvc.Name
+	if pvc.Name != "" {
+		pvcName = pvc.Name
 	}
 	return pvcName
 }

diff --git a/api/apps/v1alpha1/nimservice_types.go b/api/apps/v1alpha1/nimservice_types.go
@@ -120,8 +120,8 @@ type NIMServiceList struct {
 // Prefers pvc.Name if explicitly set by the user in the NIMService instance
 func (n *NIMService) GetPVCName(pvc PersistentVolumeClaim) string {
 	pvcName := fmt.Sprintf("%s-pvc", n.GetName())
-	if pvc.Name != nil {
-		pvcName = *pvc.Name
+	if pvc.Name != "" {
+		pvcName = pvc.Name
 	}
 	return pvcName
 }
@@ -393,7 +393,7 @@ func (n *NIMService) GetVolumes(modelPVC PersistentVolumeClaim) []corev1.Volume
 			Name: "model-store",
 			VolumeSource: corev1.VolumeSource{
 				PersistentVolumeClaim: &corev1.PersistentVolumeClaimVolumeSource{
-					ClaimName: *modelPVC.Name,
+					ClaimName: modelPVC.Name,
 				},
 			},
 		},
@@ -434,11 +434,6 @@ func (n *NIMService) GetNIMCacheProfile() string {
 	return n.Spec.NIMCache.Profile
 }
 
-// GetExternalPVC returns the external PVC name to use for the NIMService deployment
-func (n *NIMService) GetExternalPVC() *PersistentVolumeClaim {
-	return &n.Spec.Storage.PVC
-}
-
 // GetHPA returns the HPA spec for the NIMService deployment
 func (n *NIMService) GetHPA() HorizontalPodAutoscalerSpec {
 	return n.Spec.Scale.HPA

diff --git a/api/apps/v1alpha1/zz_generated.deepcopy.go b/api/apps/v1alpha1/zz_generated.deepcopy.go
diff --git a/internal/controller/nimcache_controller.go b/internal/controller/nimcache_controller.go
@@ -574,8 +574,8 @@ func getJobName(nimCache *appsv1alpha1.NIMCache) string {
 
 func getPvcName(parent client.Object, pvc appsv1alpha1.PersistentVolumeClaim) string {
 	pvcName := fmt.Sprintf("%s-pvc", parent.GetName())
-	if pvc.Name != nil {
-		pvcName = *pvc.Name
+	if pvc.Name != "" {
+		pvcName = pvc.Name
 	}
 	return pvcName
 }

diff --git a/internal/controller/platform/standalone/nimservice.go b/internal/controller/platform/standalone/nimservice.go
@@ -136,7 +136,7 @@ func (r *NIMServiceReconciler) reconcileNIMService(ctx context.Context, nimServi
 	var modelPVC *appsv1alpha1.PersistentVolumeClaim
 	modelProfile := ""
 
-	// If external PVC is provided, use that as model-store
+	// Select PVC for model store
 	if nimService.GetNIMCacheName() != "" {
 		// Fetch PVC for the associated NIMCache instance and mount it
 		nimCachePVC, err := r.getNIMCachePVC(ctx, nimService)
@@ -151,16 +151,18 @@ func (r *NIMServiceReconciler) reconcileNIMService(ctx context.Context, nimServi
 			logger.Info("overriding model profile", "profile", profile)
 			modelProfile = profile
 		}
-	} else if externalPVC := nimService.GetExternalPVC(); externalPVC != nil {
-		modelPVC = externalPVC
 	} else if nimService.Spec.Storage.PVC.Create != nil && *nimService.Spec.Storage.PVC.Create {
+		// Create a new PVC
 		modelPVC, err = r.reconcilePVC(ctx, nimService)
 		if err != nil {
 			logger.Error(err, "unable to create pvc")
 			return ctrl.Result{}, err
 		}
+	} else if nimService.Spec.Storage.PVC.Name != "" {
+		// Use an existing PVC
+		modelPVC = &nimService.Spec.Storage.PVC
 	} else {
-		err := fmt.Errorf("neither external PVC or NIMCache volume is provided")
+		err := fmt.Errorf("neither external PVC name or NIMCache volume is provided")
 		logger.Error(err, "failed to determine PVC for model-store")
 		return ctrl.Result{}, err
 	}
@@ -334,8 +336,8 @@ func (r *NIMServiceReconciler) getNIMCachePVC(ctx context.Context, nimService *a
 		return nil, fmt.Errorf("missing PVC for the nimcache instance %s, nimservice %s", nimCache.GetName(), nimService.GetName())
 	}
 
-	if nimCache.Spec.Storage.PVC.Name == nil {
-		nimCache.Spec.Storage.PVC.Name = &nimCache.Status.PVC
+	if nimCache.Spec.Storage.PVC.Name == "" {
+		nimCache.Spec.Storage.PVC.Name = nimCache.Status.PVC
 	}
 	// Get the underlying PVC for the NIMCache instance
 	return &nimCache.Spec.Storage.PVC, nil

diff --git a/internal/controller/platform/standalone/nimservice_test.go b/internal/controller/platform/standalone/nimservice_test.go
@@ -110,7 +110,7 @@ var _ = Describe("NIMServiceReconciler for a standalone platform", func() {
 				Image:       appsv1alpha1.Image{Repository: "nvcr.io/nvidia/nim-llm", PullPolicy: "IfNotPresent", Tag: "v0.1.0", PullSecrets: []string{"ngc-secret"}},
 				Storage: appsv1alpha1.Storage{
 					PVC: appsv1alpha1.PersistentVolumeClaim{
-						Name: &pvcName,
+						Name: pvcName,
 					},
 				},
 				NIMCache: appsv1alpha1.NIMCacheVolSpec{