[Chore][Log] Delete error loggings right before returned errors

Signed-off-by: Chi-Sheng Liu <[email protected]>

Author: MortalHappiness

ray-operator/controllers/ray/raycluster_controller.go

@@ -216,7 +216,6 @@ func (r *RayClusterReconciler) rayClusterReconcile(ctx context.Context, request
 					"finalizer", utils.GCSFaultToleranceRedisCleanupFinalizer)
 				controllerutil.AddFinalizer(instance, utils.GCSFaultToleranceRedisCleanupFinalizer)
 				if err := r.Update(ctx, instance); err != nil {
-					logger.Error(err, fmt.Sprintf("Failed to add the finalizer %s to the RayCluster.", utils.GCSFaultToleranceRedisCleanupFinalizer))
 					return ctrl.Result{RequeueAfter: DefaultRequeueDuration}, err
 				}
 				// Only start the RayCluster reconciliation after the finalizer is added.
@@ -287,7 +286,6 @@ func (r *RayClusterReconciler) rayClusterReconcile(ctx context.Context, request
 						logger.Info(fmt.Sprintf("Redis cleanup Job already exists. Requeue the RayCluster CR %s.", instance.Name))
 						return ctrl.Result{RequeueAfter: DefaultRequeueDuration}, nil
 					}
-					logger.Error(err, "Failed to create Redis cleanup Job")
 					return ctrl.Result{RequeueAfter: DefaultRequeueDuration}, err
 				}
 				logger.Info("Successfully created Redis cleanup Job", "Job name", redisCleanupJob.Name)
@@ -439,7 +437,6 @@ func (r *RayClusterReconciler) reconcileRouteOpenShift(ctx context.Context, inst
 	headRoutes := routev1.RouteList{}
 	filterLabels := client.MatchingLabels{utils.RayClusterLabelKey: instance.Name}
 	if err := r.List(ctx, &headRoutes, client.InNamespace(instance.Namespace), filterLabels); err != nil {
-		logger.Error(err, "Route Listing error!", "Route.Error", err)
 		return err
 	}
 
@@ -451,7 +448,6 @@ func (r *RayClusterReconciler) reconcileRouteOpenShift(ctx context.Context, inst
 	if headRoutes.Items == nil || len(headRoutes.Items) == 0 {
 		route, err := common.BuildRouteForHeadService(*instance)
 		if err != nil {
-			logger.Error(err, "Failed building route!", "Route.Error", err)
 			return err
 		}
 
@@ -461,7 +457,6 @@ func (r *RayClusterReconciler) reconcileRouteOpenShift(ctx context.Context, inst
 
 		err = r.createHeadRoute(ctx, route, instance)
 		if err != nil {
-			logger.Error(err, "Failed creating route!", "Route.Error", err)
 			return err
 		}
 	}
@@ -925,7 +920,6 @@ func (r *RayClusterReconciler) createHeadIngress(ctx context.Context, ingress *n
 			logger.Info("Ingress already exists, no need to create")
 			return nil
 		}
-		logger.Error(err, "Ingress create error!", "Ingress.Error", err)
 		return err
 	}
 	logger.Info("Ingress created successfully", "ingress name", ingress.Name)
@@ -944,7 +938,6 @@ func (r *RayClusterReconciler) createHeadRoute(ctx context.Context, route *route
 			logger.Info("Route already exists, no need to create")
 			return nil
 		}
-		logger.Error(err, "Route create error!", "Route.Error", err)
 		return err
 	}
 	logger.Info("Route created successfully", "route name", route.Name)
@@ -967,7 +960,6 @@ func (r *RayClusterReconciler) createService(ctx context.Context, raySvc *corev1
 			logger.Info("Pod service already exist, no need to create")
 			return nil
 		}
-		logger.Error(err, "Pod Service create error!", "Pod.Service.Error", err)
 		return err
 	}
 	logger.Info("Pod Service created successfully", "service name", raySvc.Name)
@@ -999,7 +991,6 @@ func (r *RayClusterReconciler) createHeadPod(ctx context.Context, instance rayv1
 			// the pod might be in terminating state, we need to check
 			if errPod := r.Get(ctx, podIdentifier, &fetchedPod); errPod == nil {
 				if fetchedPod.DeletionTimestamp != nil {
-					logger.Error(errPod, "create pod error!", "pod is in a terminating state, we will wait until it is cleaned up", podIdentifier)
 					return err
 				}
 			}
@@ -1036,13 +1027,11 @@ func (r *RayClusterReconciler) createWorkerPod(ctx context.Context, instance ray
 			// the pod might be in terminating state, we need to check
 			if errPod := r.Get(ctx, podIdentifier, &fetchedPod); errPod == nil {
 				if fetchedPod.DeletionTimestamp != nil {
-					logger.Error(errPod, "create pod error!", "pod is in a terminating state, we will wait until it is cleaned up", podIdentifier)
 					return err
 				}
 			}
 			logger.Info("Creating pod", "Pod already exists", pod.Name)
 		} else {
-			logger.Error(fmt.Errorf("createWorkerPod error"), "error creating pod", "pod", pod, "err = ", err)
 			return err
 		}
 	}
@@ -1262,7 +1251,6 @@ func (r *RayClusterReconciler) getHeadPodIP(ctx context.Context, instance *rayv1
 	runtimePods := corev1.PodList{}
 	filterLabels := client.MatchingLabels{utils.RayClusterLabelKey: instance.Name, utils.RayNodeTypeLabelKey: string(rayv1.HeadNode)}
 	if err := r.List(ctx, &runtimePods, client.InNamespace(instance.Namespace), filterLabels); err != nil {
-		logger.Error(err, "Failed to list pods while getting head pod ip.")
 		return "", err
 	}
 	if len(runtimePods.Items) != 1 {
@@ -1366,10 +1354,6 @@ func (r *RayClusterReconciler) reconcileAutoscalerServiceAccount(ctx context.Con
 		// zero-downtime rolling updates when RayService is performed. See https://github.com/ray-project/kuberay/issues/1123
 		// for more details.
 		if instance.Spec.HeadGroupSpec.Template.Spec.ServiceAccountName == namespacedName.Name {
-			logger.Error(err, fmt.Sprintf(
-				"If users specify ServiceAccountName for the head Pod, they need to create a ServiceAccount themselves. "+
-					"However, ServiceAccount %s is not found. Please create one. "+
-					"See the PR description of https://github.com/ray-project/kuberay/pull/1128 for more details.", namespacedName.Name), "ServiceAccount", namespacedName)
 			return err
 		}
 
@@ -1392,7 +1376,6 @@ func (r *RayClusterReconciler) reconcileAutoscalerServiceAccount(ctx context.Con
 				logger.Info("Pod service account already exist, no need to create")
 				return nil
 			}
-			logger.Error(err, "Pod Service Account create error!", "Pod.ServiceAccount.Error", err)
 			return err
 		}
 		logger.Info("Pod ServiceAccount created successfully", "service account name", serviceAccount.Name)
@@ -1434,7 +1417,6 @@ func (r *RayClusterReconciler) reconcileAutoscalerRole(ctx context.Context, inst
 				logger.Info("role already exist, no need to create")
 				return nil
 			}
-			logger.Error(err, "Role create error!", "Role.Error", err)
 			return err
 		}
 		logger.Info("Role created successfully", "role name", role.Name)
@@ -1476,7 +1458,6 @@ func (r *RayClusterReconciler) reconcileAutoscalerRoleBinding(ctx context.Contex
 				logger.Info("role binding already exist, no need to create")
 				return nil
 			}
-			logger.Error(err, "Role binding create error!", "RoleBinding.Error", err)
 			return err
 		}
 		logger.Info("RoleBinding created successfully", "role binding name", roleBinding.Name)

ray-operator/controllers/ray/rayjob_controller.go

@@ -84,8 +84,6 @@ func (r *RayJobReconciler) Reconcile(ctx context.Context, request ctrl.Request)
 			logger.Info("RayJob resource not found. Ignoring since object must be deleted", "name", request.NamespacedName)
 			return ctrl.Result{}, nil
 		}
-		// Error reading the object - requeue the request.
-		logger.Error(err, "Failed to get RayJob")
 		return ctrl.Result{RequeueAfter: RayJobDefaultRequeueDuration}, err
 	}
 
@@ -115,14 +113,12 @@ func (r *RayJobReconciler) Reconcile(ctx context.Context, request ctrl.Request)
 		controllerutil.RemoveFinalizer(rayJobInstance, utils.RayJobStopJobFinalizer)
 		err := r.Update(ctx, rayJobInstance)
 		if err != nil {
-			logger.Error(err, "Failed to remove finalizer for RayJob")
 			return ctrl.Result{RequeueAfter: RayJobDefaultRequeueDuration}, err
 		}
 		return ctrl.Result{RequeueAfter: RayJobDefaultRequeueDuration}, err
 	}
 
 	if err := validateRayJobSpec(rayJobInstance); err != nil {
-		logger.Error(err, "The RayJob spec is invalid")
 		return ctrl.Result{RequeueAfter: RayJobDefaultRequeueDuration}, err
 	}
 
@@ -136,7 +132,6 @@ func (r *RayJobReconciler) Reconcile(ctx context.Context, request ctrl.Request)
 			logger.Info("Add a finalizer", "finalizer", utils.RayJobStopJobFinalizer)
 			controllerutil.AddFinalizer(rayJobInstance, utils.RayJobStopJobFinalizer)
 			if err := r.Update(ctx, rayJobInstance); err != nil {
-				logger.Error(err, "Failed to update RayJob with finalizer")
 				return ctrl.Result{RequeueAfter: RayJobDefaultRequeueDuration}, err
 			}
 		}
@@ -168,7 +163,6 @@ func (r *RayJobReconciler) Reconcile(ctx context.Context, request ctrl.Request)
 			}
 
 			if clientURL, err = utils.FetchHeadServiceURL(ctx, r.Client, rayClusterInstance, utils.DashboardPortName); err != nil || clientURL == "" {
-				logger.Error(err, "Failed to get the dashboard URL after the RayCluster is ready!", "RayCluster", rayClusterInstance.Name)
 				return ctrl.Result{RequeueAfter: RayJobDefaultRequeueDuration}, err
 			}
 			rayJobInstance.Status.DashboardURL = clientURL
@@ -201,7 +195,6 @@ func (r *RayJobReconciler) Reconcile(ctx context.Context, request ctrl.Request)
 			// mode is not stuck in the `Running` status indefinitely.
 			namespacedName := common.RayJobK8sJobNamespacedName(rayJobInstance)
 			if err := r.Client.Get(ctx, namespacedName, job); err != nil {
-				logger.Error(err, "Failed to get the submitter Kubernetes Job", "NamespacedName", namespacedName)
 				return ctrl.Result{RequeueAfter: RayJobDefaultRequeueDuration}, err
 			}
 			if shouldUpdate := r.checkK8sJobAndUpdateStatusIfNeeded(ctx, rayJobInstance, job); shouldUpdate {
@@ -228,12 +221,10 @@ func (r *RayJobReconciler) Reconcile(ctx context.Context, request ctrl.Request)
 			if rayJobInstance.Spec.SubmissionMode == rayv1.HTTPMode && errors.IsBadRequest(err) {
 				logger.Info("The Ray job was not found. Submit a Ray job via an HTTP request.", "JobId", rayJobInstance.Status.JobId)
 				if _, err := rayDashboardClient.SubmitJob(ctx, rayJobInstance); err != nil {
-					logger.Error(err, "Failed to submit the Ray job", "JobId", rayJobInstance.Status.JobId)
 					return ctrl.Result{RequeueAfter: RayJobDefaultRequeueDuration}, err
 				}
 				return ctrl.Result{RequeueAfter: RayJobDefaultRequeueDuration}, nil
 			}
-			logger.Error(err, "Failed to get job info", "JobId", rayJobInstance.Status.JobId)
 			return ctrl.Result{RequeueAfter: RayJobDefaultRequeueDuration}, err
 		}
 		logger.Info("GetJobInfo", "Job Info", jobInfo)
@@ -357,14 +348,11 @@ func (r *RayJobReconciler) createK8sJobIfNeed(ctx context.Context, rayJobInstanc
 		if errors.IsNotFound(err) {
 			submitterTemplate, err := r.getSubmitterTemplate(ctx, rayJobInstance, rayClusterInstance)
 			if err != nil {
-				logger.Error(err, "failed to get submitter template")
 				return err
 			}
 			return r.createNewK8sJob(ctx, rayJobInstance, submitterTemplate)
 		}
 
-		// Some other error occurred while trying to get the Job
-		logger.Error(err, "failed to get Kubernetes Job")
 		return err
 	}
 
@@ -444,13 +432,11 @@ func (r *RayJobReconciler) createNewK8sJob(ctx context.Context, rayJobInstance *
 
 	// Set the ownership in order to do the garbage collection by k8s.
 	if err := ctrl.SetControllerReference(rayJobInstance, job, r.Scheme); err != nil {
-		logger.Error(err, "failed to set controller reference")
 		return err
 	}
 
 	// Create the Kubernetes Job
 	if err := r.Client.Create(ctx, job); err != nil {
-		logger.Error(err, "failed to create k8s Job")
 		return err
 	}
 	logger.Info("Kubernetes Job created", "RayJob", rayJobInstance.Name, "Kubernetes Job", job.Name)
@@ -476,7 +462,6 @@ func (r *RayJobReconciler) deleteSubmitterJob(ctx context.Context, rayJobInstanc
 			isJobDeleted = true
 			logger.Info("The submitter Kubernetes Job has been already deleted", "RayJob", rayJobInstance.Name, "Kubernetes Job", job.Name)
 		} else {
-			logger.Error(err, "Failed to get Kubernetes Job")
 			return false, err
 		}
 	} else {
@@ -629,16 +614,13 @@ func (r *RayJobReconciler) getOrCreateRayClusterInstance(ctx context.Context, ra
 			logger.Info("RayCluster not found, creating RayCluster!", "RayCluster", rayClusterNamespacedName)
 			rayClusterInstance, err = r.constructRayClusterForRayJob(rayJobInstance, rayClusterNamespacedName.Name)
 			if err != nil {
-				logger.Error(err, "unable to construct a new RayCluster")
 				return nil, err
 			}
 			if err := r.Create(ctx, rayClusterInstance); err != nil {
-				logger.Error(err, "unable to create RayCluster for RayJob", "RayCluster", rayClusterInstance)
 				return nil, err
 			}
 			r.Recorder.Eventf(rayJobInstance, corev1.EventTypeNormal, "Created", "Created RayCluster %s", rayJobInstance.Status.RayClusterName)
 		} else {
-			logger.Error(err, "Fail to get RayCluster!")
 			return nil, err
 		}
 	}

ray-operator/controllers/ray/rayservice_controller.go

@@ -221,7 +221,6 @@ func (r *RayServiceReconciler) Reconcile(ctx context.Context, request ctrl.Reque
 	if r.inconsistentRayServiceStatuses(ctx, originalRayServiceInstance.Status, rayServiceInstance.Status) {
 		rayServiceInstance.Status.LastUpdateTime = &metav1.Time{Time: time.Now()}
 		if errStatus := r.Status().Update(ctx, rayServiceInstance); errStatus != nil {
-			logger.Error(errStatus, "Failed to update RayService status", "rayServiceInstance", rayServiceInstance)
 			return ctrl.Result{RequeueAfter: ServiceDefaultRequeueDuration}, errStatus
 		}
 	}
@@ -230,10 +229,8 @@ func (r *RayServiceReconciler) Reconcile(ctx context.Context, request ctrl.Reque
 }
 
 func (r *RayServiceReconciler) calculateStatus(ctx context.Context, rayServiceInstance *rayv1.RayService) error {
-	logger := ctrl.LoggerFrom(ctx)
 	serveEndPoints := &corev1.Endpoints{}
 	if err := r.Get(ctx, common.RayServiceServeServiceNamespacedName(rayServiceInstance), serveEndPoints); err != nil && !errors.IsNotFound(err) {
-		logger.Error(err, "Fail to retrieve the Kubernetes Endpoints from the cluster!")
 		return err
 	}
 
@@ -438,7 +435,6 @@ func (r *RayServiceReconciler) cleanUpRayClusterInstance(ctx context.Context, ra
 
 	var err error
 	if err = r.List(ctx, &rayClusterList, common.RayServiceRayClustersAssociationOptions(rayServiceInstance).ToListOptions()...); err != nil {
-		logger.Error(err, "Fail to list RayCluster for "+rayServiceInstance.Name)
 		return err
 	}
 
@@ -463,7 +459,6 @@ func (r *RayServiceReconciler) cleanUpRayClusterInstance(ctx context.Context, ra
 				if reasonForDeletion != "" {
 					logger.Info("reconcileRayCluster", "delete Ray cluster", rayClusterInstance.Name, "reason", reasonForDeletion)
 					if err := r.Delete(ctx, &rayClusterInstance, client.PropagationPolicy(metav1.DeletePropagationBackground)); err != nil {
-						logger.Error(err, "Fail to delete RayCluster "+rayClusterInstance.Name)
 						return err
 					}
 				}
@@ -475,12 +470,10 @@ func (r *RayServiceReconciler) cleanUpRayClusterInstance(ctx context.Context, ra
 }
 
 func (r *RayServiceReconciler) getRayClusterByNamespacedName(ctx context.Context, clusterKey client.ObjectKey) (*rayv1.RayCluster, error) {
-	logger := ctrl.LoggerFrom(ctx)
 	rayCluster := &rayv1.RayCluster{}
 	if clusterKey.Name != "" {
 		// Ignore not found since in that case we should return RayCluster as nil.
 		if err := r.Get(ctx, clusterKey, rayCluster); client.IgnoreNotFound(err) != nil {
-			logger.Error(err, "Fail to get RayCluster "+clusterKey.String())
 			return nil, err
 		}
 	} else {
@@ -603,7 +596,6 @@ func (r *RayServiceReconciler) createRayClusterInstanceIfNeeded(ctx context.Cont
 	} else {
 		clusterAction, err = getClusterAction(pendingRayCluster.Spec, rayServiceInstance.Spec.RayClusterSpec)
 		if err != nil {
-			logger.Error(err, "Fail to generate hash for RayClusterSpec")
 			return nil, err
 		}
 	}
@@ -640,7 +632,6 @@ func (r *RayServiceReconciler) updateRayClusterInstance(ctx context.Context, ray
 		Name:      rayClusterInstance.Name,
 	})
 	if err != nil {
-		logger.Error(err, "Failed to get the current state of RayCluster", "Namespace", rayClusterInstance.Namespace, "Name", rayClusterInstance.Name)
 		return err
 	}
 
@@ -658,7 +649,6 @@ func (r *RayServiceReconciler) updateRayClusterInstance(ctx context.Context, ray
 
 	// Update the RayCluster
 	if err = r.Update(ctx, currentRayCluster); err != nil {
-		logger.Error(err, "Fail to update RayCluster "+currentRayCluster.Name)
 		return err
 	}
 
@@ -692,19 +682,16 @@ func (r *RayServiceReconciler) createRayClusterInstance(ctx context.Context, ray
 		}
 		// if error is `not found`, then continue.
 	} else if !errors.IsNotFound(err) {
-		logger.Error(err, "Get request rayCluster instance error!")
 		return nil, err
 		// if error is `not found`, then continue.
 	}
 
 	logger.Info("No pending RayCluster, creating RayCluster.")
 	rayClusterInstance, err = r.constructRayClusterForRayService(ctx, rayServiceInstance, rayClusterKey.Name)
 	if err != nil {
-		logger.Error(err, "unable to construct rayCluster from spec")
 		return nil, err
 	}
 	if err = r.Create(ctx, rayClusterInstance); err != nil {
-		logger.Error(err, "unable to create rayCluster for rayService", "rayCluster", rayClusterInstance)
 		return nil, err
 	}
 	logger.Info("created rayCluster for rayService", "rayCluster", rayClusterInstance)
@@ -713,8 +700,6 @@ func (r *RayServiceReconciler) createRayClusterInstance(ctx context.Context, ray
 }
 
 func (r *RayServiceReconciler) constructRayClusterForRayService(ctx context.Context, rayService *rayv1.RayService, rayClusterName string) (*rayv1.RayCluster, error) {
-	logger := ctrl.LoggerFrom(ctx)
-
 	var err error
 	rayClusterLabel := make(map[string]string)
 	for k, v := range rayService.Labels {
@@ -727,12 +712,8 @@ func (r *RayServiceReconciler) constructRayClusterForRayService(ctx context.Cont
 	for k, v := range rayService.Annotations {
 		rayClusterAnnotations[k] = v
 	}
-	errContext := "Failed to serialize RayCluster config. " +
-		"Manual config updates will NOT be tracked accurately. " +
-		"Please tear down the cluster and apply a new config."
 	rayClusterAnnotations[utils.HashWithoutReplicasAndWorkersToDeleteKey], err = generateHashWithoutReplicasAndWorkersToDelete(rayService.Spec.RayClusterSpec)
 	if err != nil {
-		logger.Error(err, errContext)
 		return nil, err
 	}
 	rayClusterAnnotations[utils.NumWorkerGroupsKey] = strconv.Itoa(len(rayService.Spec.RayClusterSpec.WorkerGroupSpecs))
@@ -991,7 +972,6 @@ func (r *RayServiceReconciler) reconcileServices(ctx context.Context, rayService
 		oldSvc.Spec = *newSvc.Spec.DeepCopy()
 		logger.Info(fmt.Sprintf("Update Kubernetes Service serviceType %v", serviceType))
 		if updateErr := r.Update(ctx, oldSvc); updateErr != nil {
-			logger.Error(updateErr, fmt.Sprintf("Fail to update Kubernetes Service serviceType %v", serviceType), "Error", updateErr)
 			return updateErr
 		}
 	} else if errors.IsNotFound(err) {
@@ -1004,11 +984,9 @@ func (r *RayServiceReconciler) reconcileServices(ctx context.Context, rayService
 				logger.Info("The Kubernetes Service already exists, no need to create.")
 				return nil
 			}
-			logger.Error(createErr, fmt.Sprintf("Fail to create Kubernetes Service serviceType %v", serviceType), "Error", createErr)
 			return createErr
 		}
 	} else {
-		logger.Error(err, "Fail to retrieve the Kubernetes Service from the cluster!")
 		return err
 	}
 
@@ -1120,7 +1098,6 @@ func (r *RayServiceReconciler) reconcileServe(ctx context.Context, rayServiceIns
 }
 
 func (r *RayServiceReconciler) labelHeadPodForServeStatus(ctx context.Context, rayClusterInstance *rayv1.RayCluster) error {
-	logger := ctrl.LoggerFrom(ctx)
 	headPod, err := r.getHeadPod(ctx, rayClusterInstance)
 	if err != nil {
 		return err
@@ -1151,7 +1128,6 @@ func (r *RayServiceReconciler) labelHeadPodForServeStatus(ctx context.Context, r
 
 	if !reflect.DeepEqual(originalLabels, headPod.Labels) {
 		if updateErr := r.Update(ctx, headPod); updateErr != nil {
-			logger.Error(updateErr, "Pod label Update error!", "Pod.Error", updateErr)
 			return updateErr
 		}
 	}