[Feat] Deprecate ForcedClusterUpgrade (#2075)

Author: MortalHappiness

ray-operator/apis/config/v1alpha1/configuration_types.go

@@ -32,9 +32,6 @@ type Configuration struct {
 	// If empty, all namespaces will be watched.
 	WatchNamespace string `json:"watchNamespace,omitempty"`
 
-	// ForcedClusterUpgrade enables force upgrading clusters.
-	ForcedClusterUpgrade bool `json:"forcedClusterUpgrade,omitempty"`
-
 	// LogFile is a path to a local file for synchronizing logs.
 	LogFile string `json:"logFile,omitempty"`
 

ray-operator/controllers/ray/raycluster_controller.go

@@ -46,7 +46,6 @@ import (
 
 var (
 	DefaultRequeueDuration = 2 * time.Second
-	ForcedClusterUpgrade   bool
 	EnableBatchScheduler   bool
 
 	// Definition of a index field for pod name
@@ -697,42 +696,6 @@ func (r *RayClusterReconciler) reconcilePods(ctx context.Context, instance *rayv
 		}
 	}
 
-	if ForcedClusterUpgrade {
-		if len(headPods.Items) == 1 {
-			// head node amount is exactly 1, but we need to check if it has been changed
-			res := utils.PodNotMatchingTemplate(headPods.Items[0], instance.Spec.HeadGroupSpec.Template)
-			if res {
-				logger.Info(fmt.Sprintf("need to delete old head pod %s", headPods.Items[0].Name))
-				if err := r.Delete(ctx, &headPods.Items[0]); err != nil {
-					return err
-				}
-				return nil
-			}
-		}
-
-		// check if WorkerGroupSpecs has been changed and we need to kill worker pods
-		for _, worker := range instance.Spec.WorkerGroupSpecs {
-			workerPods := corev1.PodList{}
-			if err := r.List(ctx, &workerPods, common.RayClusterGroupPodsAssociationOptions(instance, worker.GroupName).ToListOptions()...); err != nil {
-				return err
-			}
-			updatedWorkerPods := false
-			for _, item := range workerPods.Items {
-				if utils.PodNotMatchingTemplate(item, worker.Template) {
-					logger.Info(fmt.Sprintf("need to delete old worker pod %s", item.Name))
-					if err := r.Delete(ctx, &item); err != nil {
-						logger.Info(fmt.Sprintf("error deleting worker pod %s", item.Name))
-						return err
-					}
-					updatedWorkerPods = true
-				}
-			}
-			if updatedWorkerPods {
-				return nil
-			}
-		}
-	}
-
 	// Reconcile worker pods now
 	for _, worker := range instance.Spec.WorkerGroupSpecs {
 		// workerReplicas will store the target number of pods for this worker group.

ray-operator/controllers/ray/utils/util.go

@@ -414,65 +414,6 @@ func CheckAllPodsRunning(ctx context.Context, runningPods corev1.PodList) bool {
 	return true
 }
 
-func PodNotMatchingTemplate(pod corev1.Pod, template corev1.PodTemplateSpec) bool {
-	if pod.Status.Phase == corev1.PodRunning && pod.ObjectMeta.DeletionTimestamp == nil {
-		if len(template.Spec.Containers) != len(pod.Spec.Containers) {
-			return true
-		}
-		cmap := map[string]*corev1.Container{}
-		for _, container := range pod.Spec.Containers {
-			cmap[container.Name] = &container
-		}
-		for _, container1 := range template.Spec.Containers {
-			if container2, ok := cmap[container1.Name]; ok {
-				if container1.Image != container2.Image {
-					// image name do not match
-					return true
-				}
-				if len(container1.Resources.Requests) != len(container2.Resources.Requests) ||
-					len(container1.Resources.Limits) != len(container2.Resources.Limits) {
-					// resource entries do not match
-					return true
-				}
-
-				resources1 := []corev1.ResourceList{
-					container1.Resources.Requests,
-					container1.Resources.Limits,
-				}
-				resources2 := []corev1.ResourceList{
-					container2.Resources.Requests,
-					container2.Resources.Limits,
-				}
-				for i := range resources1 {
-					// we need to make sure all fields match
-					for name, quantity1 := range resources1[i] {
-						if quantity2, ok := resources2[i][name]; ok {
-							if quantity1.Cmp(quantity2) != 0 {
-								// request amount does not match
-								return true
-							}
-						} else {
-							// no such request
-							return true
-						}
-					}
-				}
-
-				// now we consider them equal
-				delete(cmap, container1.Name)
-			} else {
-				// container name do not match
-				return true
-			}
-		}
-		if len(cmap) != 0 {
-			// one or more containers do not match
-			return true
-		}
-	}
-	return false
-}
-
 // CompareJsonStruct This is a way to better compare if two objects are the same when they are json/yaml structs. reflect.DeepEqual will fail in some cases.
 func CompareJsonStruct(objA interface{}, objB interface{}) bool {
 	a, err := json.Marshal(objA)

ray-operator/controllers/ray/utils/util_test.go

@@ -5,7 +5,6 @@ import (
 	"testing"
 
 	"github.com/stretchr/testify/assert"
-	"k8s.io/apimachinery/pkg/api/resource"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/utils/pointer"
 
@@ -252,181 +251,6 @@ func TestGetHeadGroupServiceAccountName(t *testing.T) {
 	}
 }
 
-func TestReconcile_CheckNeedRemoveOldPod(t *testing.T) {
-	namespaceStr := "default"
-
-	headTemplate := corev1.PodTemplateSpec{
-		Spec: corev1.PodSpec{
-			Containers: []corev1.Container{
-				{
-					Name:    "ray-head",
-					Image:   "rayproject/autoscaler",
-					Command: []string{"python"},
-					Args:    []string{"/opt/code.py"},
-					Env: []corev1.EnvVar{
-						{
-							Name: "MY_POD_IP",
-							ValueFrom: &corev1.EnvVarSource{
-								FieldRef: &corev1.ObjectFieldSelector{
-									FieldPath: "status.podIP",
-								},
-							},
-						},
-					},
-				},
-			},
-		},
-	}
-
-	pod := corev1.Pod{
-		ObjectMeta: metav1.ObjectMeta{
-			Name:      "headNode",
-			Namespace: namespaceStr,
-		},
-		Spec: corev1.PodSpec{
-			Containers: []corev1.Container{
-				{
-					Name:    "ray-head",
-					Image:   "rayproject/autoscaler",
-					Command: []string{"python"},
-					Args:    []string{"/opt/code.py"},
-				},
-			},
-		},
-		Status: corev1.PodStatus{
-			Phase: corev1.PodRunning,
-		},
-	}
-
-	assert.Equal(t, PodNotMatchingTemplate(pod, headTemplate), false, "expect template & pod matching")
-
-	pod.Spec.Containers = []corev1.Container{
-		{
-			Name:    "ray-head",
-			Image:   "rayproject/autoscaler",
-			Command: []string{"python"},
-			Args:    []string{"/opt/code.py"},
-		},
-		{
-			Name:    "ray-head",
-			Image:   "rayproject/autoscaler",
-			Command: []string{"python"},
-			Args:    []string{"/opt/code.py"},
-		},
-	}
-
-	assert.Equal(t, PodNotMatchingTemplate(pod, headTemplate), true, "expect template & pod with 2 containers not matching")
-
-	workerTemplate := corev1.PodTemplateSpec{
-		Spec: corev1.PodSpec{
-			Containers: []corev1.Container{
-				{
-					Name:    "ray-worker",
-					Image:   "rayproject/autoscaler",
-					Command: []string{"echo"},
-					Args:    []string{"Hello Ray"},
-					Env: []corev1.EnvVar{
-						{
-							Name: "MY_POD_IP",
-							ValueFrom: &corev1.EnvVarSource{
-								FieldRef: &corev1.ObjectFieldSelector{
-									FieldPath: "status.podIP",
-								},
-							},
-						},
-					},
-				},
-			},
-		},
-	}
-
-	pod = corev1.Pod{
-		ObjectMeta: metav1.ObjectMeta{
-			Name:      "pod1",
-			Namespace: namespaceStr,
-		},
-		Spec: corev1.PodSpec{
-			Containers: []corev1.Container{
-				{
-					Name:    "ray-worker",
-					Image:   "rayproject/autoscaler",
-					Command: []string{"echo"},
-					Args:    []string{"Hello Ray"},
-				},
-			},
-		},
-		Status: corev1.PodStatus{
-			Phase: corev1.PodRunning,
-		},
-	}
-
-	assert.Equal(t, PodNotMatchingTemplate(pod, workerTemplate), false, "expect template & pod matching")
-
-	workerTemplate = corev1.PodTemplateSpec{
-		Spec: corev1.PodSpec{
-			Containers: []corev1.Container{
-				{
-					Name:    "ray-worker",
-					Image:   "rayproject/autoscaler",
-					Command: []string{"echo"},
-					Args:    []string{"Hello Ray"},
-					Resources: corev1.ResourceRequirements{
-						Limits: corev1.ResourceList{
-							corev1.ResourceCPU:    resource.MustParse("500m"),
-							corev1.ResourceMemory: resource.MustParse("512Mi"),
-						},
-						Requests: corev1.ResourceList{
-							corev1.ResourceCPU:    resource.MustParse("256m"),
-							corev1.ResourceMemory: resource.MustParse("256Mi"),
-						},
-					},
-				},
-			},
-		},
-	}
-
-	pod = corev1.Pod{
-		ObjectMeta: metav1.ObjectMeta{
-			Name:      "pod1",
-			Namespace: namespaceStr,
-		},
-		Spec: corev1.PodSpec{
-			Containers: []corev1.Container{
-				{
-					Name:    "ray-worker",
-					Image:   "rayproject/autoscaler",
-					Command: []string{"echo"},
-					Args:    []string{"Hello Ray"},
-					Resources: corev1.ResourceRequirements{
-						Limits: corev1.ResourceList{
-							corev1.ResourceCPU:    resource.MustParse("500m"),
-							corev1.ResourceMemory: resource.MustParse("512Mi"),
-						},
-						Requests: corev1.ResourceList{
-							corev1.ResourceCPU:    resource.MustParse("256m"),
-							corev1.ResourceMemory: resource.MustParse("256Mi"),
-						},
-					},
-				},
-			},
-		},
-		Status: corev1.PodStatus{
-			Phase: corev1.PodRunning,
-		},
-	}
-
-	assert.Equal(t, PodNotMatchingTemplate(pod, workerTemplate), false, "expect template & pod matching")
-
-	pod.Spec.Containers[0].Resources.Limits[corev1.ResourceCPU] = resource.MustParse("50m")
-
-	assert.Equal(t, PodNotMatchingTemplate(pod, workerTemplate), true, "expect template & pod not matching")
-
-	pod.Spec.Containers[0].Resources.Limits[corev1.ResourceCPU] = resource.MustParse("500m")
-	pod.Spec.Containers[0].Resources.Requests[corev1.ResourceCPU] = resource.MustParse("250m")
-
-	assert.Equal(t, PodNotMatchingTemplate(pod, workerTemplate), true, "expect template & pod not matching")
-}
-
 func TestCalculateAvailableReplicas(t *testing.T) {
 	podList := corev1.PodList{
 		Items: []corev1.Pod{

ray-operator/main.go

@@ -59,6 +59,7 @@ func main() {
 	var probeAddr string
 	var reconcileConcurrency int
 	var watchNamespace string
+	var forcedClusterUpgrade bool
 	var logFile string
 	var logFileEncoder string
 	var logStdoutEncoder string
@@ -78,8 +79,8 @@ func main() {
 		"watch-namespace",
 		"",
 		"Specify a list of namespaces to watch for custom resources, separated by commas. If left empty, all namespaces will be watched.")
-	flag.BoolVar(&ray.ForcedClusterUpgrade, "forced-cluster-upgrade", false,
-		"Forced cluster upgrade flag")
+	flag.BoolVar(&forcedClusterUpgrade, "forced-cluster-upgrade", false,
+		"(Deprecated) Forced cluster upgrade flag")
 	flag.StringVar(&logFile, "log-file-path", "",
 		"Synchronize logs to local file")
 	flag.StringVar(&logFileEncoder, "log-file-encoder", "json",
@@ -108,7 +109,6 @@ func main() {
 		exitOnError(err, "failed to decode config file")
 
 		// TODO: remove globally-scoped variables
-		ray.ForcedClusterUpgrade = config.ForcedClusterUpgrade
 		ray.EnableBatchScheduler = config.EnableBatchScheduler
 	} else {
 		config.MetricsAddr = metricsAddr
@@ -117,7 +117,6 @@ func main() {
 		config.LeaderElectionNamespace = leaderElectionNamespace
 		config.ReconcileConcurrency = reconcileConcurrency
 		config.WatchNamespace = watchNamespace
-		config.ForcedClusterUpgrade = ray.ForcedClusterUpgrade
 		config.LogFile = logFile
 		config.LogFileEncoder = logFileEncoder
 		config.LogStdoutEncoder = logStdoutEncoder
@@ -153,8 +152,8 @@ func main() {
 		ctrl.SetLogger(k8szap.New(k8szap.UseFlagOptions(&opts)))
 	}
 
-	if ray.ForcedClusterUpgrade {
-		setupLog.Info("Feature flag forced-cluster-upgrade is enabled.")
+	if forcedClusterUpgrade {
+		setupLog.Info("Deprecated feature flag forced-cluster-upgrade is enabled, which has no effect.")
 	}
 	if ray.EnableBatchScheduler {
 		setupLog.Info("Feature flag enable-batch-scheduler is enabled.")