From 867c0e8a3e7dc6796d411c2e5034591fb1359fc7 Mon Sep 17 00:00:00 2001 From: Chi-Sheng Liu Date: Tue, 16 Jul 2024 20:33:10 +0800 Subject: [PATCH] [Refactor][RayCluster] Delete unused functions Resolves: ray-project/kuberay#2235 Signed-off-by: Chi-Sheng Liu --- .../controllers/ray/raycluster_controller.go | 34 ++++++------------- .../ray/raycluster_controller_unit_test.go | 8 +++-- 2 files changed, 16 insertions(+), 26 deletions(-) diff --git a/ray-operator/controllers/ray/raycluster_controller.go b/ray-operator/controllers/ray/raycluster_controller.go index d88b7eda662..c3724b28cb7 100644 --- a/ray-operator/controllers/ray/raycluster_controller.go +++ b/ray-operator/controllers/ray/raycluster_controller.go @@ -332,18 +332,11 @@ func (r *RayClusterReconciler) rayClusterReconcile(ctx context.Context, request // Calculate the new status for the RayCluster. Note that the function will deep copy `instance` instead of mutating it. newInstance, calculateErr := r.calculateStatus(ctx, instance, reconcileErr) + var updateErr error if calculateErr != nil { logger.Info("Got error when calculating new status", "cluster name", request.Name, "error", calculateErr) - } - - // Check if the status needs to be updated. - var updateErr error - if calculateErr == nil && r.inconsistentRayClusterStatus(ctx, originalRayClusterInstance.Status, newInstance.Status) { - logger.Info("rayClusterReconcile", "Update CR status", request.Name, "status", newInstance.Status) - updateErr = r.Status().Update(ctx, newInstance) - if updateErr != nil { - logger.Info("Got error when updating status", "cluster name", request.Name, "error", updateErr, "RayCluster", newInstance) - } + } else { + updateErr = r.updateRayClusterStatus(ctx, originalRayClusterInstance, newInstance) } // Return error based on order. @@ -1451,24 +1444,17 @@ func (r *RayClusterReconciler) reconcileAutoscalerRoleBinding(ctx context.Contex return nil } -func (r *RayClusterReconciler) updateClusterState(ctx context.Context, instance *rayv1.RayCluster, clusterState rayv1.ClusterState) error { +func (r *RayClusterReconciler) updateRayClusterStatus(ctx context.Context, originalRayClusterInstance, newInstance *rayv1.RayCluster) error { logger := ctrl.LoggerFrom(ctx) - if instance.Status.State == clusterState { + if !r.inconsistentRayClusterStatus(ctx, originalRayClusterInstance.Status, newInstance.Status) { return nil } - instance.Status.State = clusterState - logger.Info("updateClusterState", "Update CR Status.State", clusterState) - return r.Status().Update(ctx, instance) -} - -func (r *RayClusterReconciler) updateClusterReason(ctx context.Context, instance *rayv1.RayCluster, clusterReason string) error { - logger := ctrl.LoggerFrom(ctx) - if instance.Status.Reason == clusterReason { - return nil + logger.Info("updateRayClusterStatus", "name", originalRayClusterInstance.Name, "old status", originalRayClusterInstance.Status, "new status", newInstance.Status) + err := r.Status().Update(ctx, newInstance) + if err != nil { + logger.Info("Error updating status", "name", originalRayClusterInstance.Name, "error", err, "RayCluster", newInstance) } - instance.Status.Reason = clusterReason - logger.Info("updateClusterReason", "Update CR Status.Reason", clusterReason) - return r.Status().Update(ctx, instance) + return err } // sumGPUs sums the GPUs in the given resource list. diff --git a/ray-operator/controllers/ray/raycluster_controller_unit_test.go b/ray-operator/controllers/ray/raycluster_controller_unit_test.go index ebeee870596..db5df7240ae 100644 --- a/ray-operator/controllers/ray/raycluster_controller_unit_test.go +++ b/ray-operator/controllers/ray/raycluster_controller_unit_test.go @@ -1257,7 +1257,9 @@ func TestReconcile_UpdateClusterReason(t *testing.T) { } reason := "test reason" - err = testRayClusterReconciler.updateClusterReason(ctx, testRayCluster, reason) + newTestRayCluster := testRayCluster.DeepCopy() + newTestRayCluster.Status.Reason = reason + err = testRayClusterReconciler.updateRayClusterStatus(ctx, testRayCluster, newTestRayCluster) assert.Nil(t, err, "Fail to update cluster reason") err = fakeClient.Get(ctx, namespacedName, &cluster) @@ -1532,7 +1534,9 @@ func TestReconcile_UpdateClusterState(t *testing.T) { } state := rayv1.Ready - err = testRayClusterReconciler.updateClusterState(ctx, testRayCluster, state) + newTestRayCluster := testRayCluster.DeepCopy() + newTestRayCluster.Status.State = state + err = testRayClusterReconciler.updateRayClusterStatus(ctx, testRayCluster, newTestRayCluster) assert.Nil(t, err, "Fail to update cluster state") err = fakeClient.Get(ctx, namespacedName, &cluster)