Updating error status on NIMcache object for reconcile failure (#104)

* Updating error status on NIMcache object for reconcile failure

---------

Signed-off-by: Vishesh Tanksale <[email protected]>

Author: visheshtanksale

api/apps/v1alpha1/nimcache_types.go

@@ -172,6 +172,8 @@ const (
 	NimCacheConditionJobPending = "NIM_CACHE_JOB_PENDING"
 	// NimCacheConditionPVCCreated indicates that the caching pvc is created.
 	NimCacheConditionPVCCreated = "NIM_CACHE_PVC_CREATED"
+	// NimCacheConditionReconcileFailed indicated that error occured while reconciling NIMCache object
+	NimCacheConditionReconcileFailed = "NIM_CACHE_RECONCILE_FAILED"
 
 	// NimCacheStatusNotReady indicates that cache is not ready
 	NimCacheStatusNotReady = "NotReady"

internal/conditions/conditions.go

@@ -148,3 +148,17 @@ func UpdateCondition(conditions *[]metav1.Condition, conditionType string, statu
 	})
 	// condition updated
 }
+
+func IfPresentUpdateCondition(conditions *[]metav1.Condition, conditionType string, status metav1.ConditionStatus, reason, message string) {
+	for i := range *conditions {
+		if (*conditions)[i].Type == conditionType {
+			// existing condition
+			(*conditions)[i].Status = status
+			(*conditions)[i].LastTransitionTime = metav1.Now()
+			(*conditions)[i].Reason = reason
+			(*conditions)[i].Message = message
+			// condition updated
+			return
+		}
+	}
+}

internal/controller/nimcache_controller.go

@@ -125,7 +125,6 @@ func (r *NIMCacheReconciler) Reconcile(ctx context.Context, req ctrl.Request) (c
 		}
 		return ctrl.Result{}, client.IgnoreNotFound(err)
 	}
-
 	logger.Info("Reconciling", "NIMCache", nimCache.Name)
 
 	// Check if the instance is marked for deletion
@@ -153,14 +152,19 @@ func (r *NIMCacheReconciler) Reconcile(ctx context.Context, req ctrl.Request) (c
 			return ctrl.Result{}, nil
 		}
 	}
-
 	// Handle nim-cache reconciliation
 	result, err := r.reconcileNIMCache(ctx, nimCache)
 	if err != nil {
 		logger.Error(err, "error reconciling NIMCache", "name", nimCache.Name)
+		conditions.UpdateCondition(&nimCache.Status.Conditions, appsv1alpha1.NimCacheConditionReconcileFailed, metav1.ConditionTrue, "ReconcileFailed", err.Error())
+		nimCache.Status.State = appsv1alpha1.NimCacheStatusNotReady
+		errUpdate := r.updateNIMCacheStatus(ctx, nimCache)
+		if errUpdate != nil {
+			logger.Error(err, "Failed to update NIMCache status", "NIMCache", nimCache.Name)
+			return result, errUpdate
+		}
 		return result, err
 	}
-
 	return result, nil
 }
 
@@ -444,10 +448,6 @@ func (r *NIMCacheReconciler) reconcilePVC(ctx context.Context, nimCache *appsv1a
 
 			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)
-				return err
-			}
 		} else {
 			logger.Error(err, "PVC doesn't exist and auto-creation is not enabled", "name", pvcNamespacedName)
 			return err
@@ -621,10 +621,6 @@ func (r *NIMCacheReconciler) reconcileModelSelection(ctx context.Context, nimCac
 		}
 
 		nimCache.Annotations[SelectedNIMProfilesAnnotationKey] = string(profilesJSON)
-		if err := r.Update(ctx, nimCache); err != nil {
-			logger.Error(err, "unable to update NIMCache with selected profiles annotation")
-			return err
-		}
 	}
 	return nil
 }
@@ -659,10 +655,6 @@ func (r *NIMCacheReconciler) reconcileJob(ctx context.Context, nimCache *appsv1a
 		conditions.UpdateCondition(&nimCache.Status.Conditions, appsv1alpha1.NimCacheConditionJobCreated, metav1.ConditionTrue, "JobCreated", "The Job to cache NIM has been created")
 		nimCache.Status.State = appsv1alpha1.NimCacheStatusStarted
 		nimCache.Status.Profiles = []v1alpha1.NIMProfile{}
-		if err := r.Status().Update(ctx, nimCache); err != nil {
-			return err
-		}
-		// return to reconcile later on job status update
 		return nil
 	}
 
@@ -713,39 +705,24 @@ func (r *NIMCacheReconciler) reconcileJobStatus(ctx context.Context, nimCache *a
 			}
 		}
 
-		if err := r.Status().Update(ctx, nimCache); err != nil {
-			return fmt.Errorf("failed to update status: %w", err)
-		}
-
 	case job.Status.Failed > 0 && nimCache.Status.State != appsv1alpha1.NimCacheStatusFailed:
 		logger.Info("Failed to cache NIM, job failed", "job", jobName)
 		conditions.UpdateCondition(&nimCache.Status.Conditions, appsv1alpha1.NimCacheConditionJobCompleted, metav1.ConditionFalse, "JobFailed", "The Job to cache NIM has failed")
 		nimCache.Status.State = appsv1alpha1.NimCacheStatusFailed
 		nimCache.Status.Profiles = []v1alpha1.NIMProfile{}
 
-		if err := r.Status().Update(ctx, nimCache); err != nil {
-			return fmt.Errorf("failed to update status: %w", err)
-		}
-
 	case job.Status.Active > 0 && nimCache.Status.State != appsv1alpha1.NimCacheStatusInProgress:
 		logger.Info("Caching NIM is in progress, job running", "job", jobName)
 		conditions.UpdateCondition(&nimCache.Status.Conditions, appsv1alpha1.NimCacheConditionJobPending, metav1.ConditionFalse, "JobRunning", "The Job to cache NIM is in progress")
 		nimCache.Status.State = appsv1alpha1.NimCacheStatusInProgress
 		nimCache.Status.Profiles = []v1alpha1.NIMProfile{}
 
-		if err := r.Status().Update(ctx, nimCache); err != nil {
-			return fmt.Errorf("failed to update status: %w", err)
-		}
-
 	case job.Status.Active == 0 && nimCache.Status.State != appsv1alpha1.NimCacheStatusReady && nimCache.Status.State != appsv1alpha1.NimCacheStatusPending:
 		logger.Info("Caching NIM is in progress, job pending", "job", jobName)
 		conditions.UpdateCondition(&nimCache.Status.Conditions, appsv1alpha1.NimCacheConditionJobPending, metav1.ConditionTrue, "JobPending", "The Job to cache NIM is in pending state")
 		nimCache.Status.State = appsv1alpha1.NimCacheStatusPending
 		nimCache.Status.Profiles = []v1alpha1.NIMProfile{}
 
-		if err := r.Status().Update(ctx, nimCache); err != nil {
-			return fmt.Errorf("failed to update status: %w", err)
-		}
 	}
 
 	return nil
@@ -815,9 +792,33 @@ func (r *NIMCacheReconciler) reconcileNIMCache(ctx context.Context, nimCache *ap
 		logger.Error(err, "reconciliation of caching job failed", "job", getJobName(nimCache))
 		return ctrl.Result{}, err
 	}
+
+	conditions.IfPresentUpdateCondition(&nimCache.Status.Conditions, appsv1alpha1.NimCacheConditionReconcileFailed, metav1.ConditionFalse, "Reconciled", "")
+
+	err = r.updateNIMCacheStatus(ctx, nimCache)
+	if err != nil {
+		logger.Error(err, "Failed to update NIMCache status", "NIMCache", nimCache.Name)
+		return ctrl.Result{}, err
+	}
 	return ctrl.Result{}, nil
 }
 
+func (r *NIMCacheReconciler) updateNIMCacheStatus(ctx context.Context, nimCache *appsv1alpha1.NIMCache) error {
+	logger := r.GetLogger()
+	obj := &appsv1alpha1.NIMCache{}
+	errGet := r.Get(ctx, types.NamespacedName{Name: nimCache.Name, Namespace: nimCache.GetNamespace()}, obj)
+	if errGet != nil {
+		logger.Error(errGet, "error getting NIMCache", "name", nimCache.Name)
+		return errGet
+	}
+	obj.Status = nimCache.Status
+	if err := r.Status().Update(ctx, obj); err != nil {
+		logger.Error(err, "Failed to update status", "NIMCache", nimCache.Name)
+		return err
+	}
+	return nil
+}
+
 func getJobName(nimCache *appsv1alpha1.NIMCache) string {
 	return fmt.Sprintf("%s-job", nimCache.GetName())
 }