[Bug] Shallow copy causes different worker configurations

[kevin85421]

Why are these changes needed?

(1) Shallow copy of a go struct is not encouraged, and see article for more details.

(2) The following code snippet is from raycluster_controller.go. We passed the struct worker (type WorkerGroupSpec struct) to the function createWorkerPod in the for loop. Due to shallow copy (See (3) for more details), worker will be updated in each iteration. That is, the specifications of created worker pods may differ based on i.

kuberay/ray-operator/controllers/ray/raycluster_controller.go

Lines 511 to 516 in 0579c90

	for i = 0; i < diff; i++ {
	r.Log.Info("reconcilePods", "creating worker for group", worker.GroupName, fmt.Sprintf("index %d", i), fmt.Sprintf("in total %d", diff))
	if err := r.createWorkerPod(*instance, worker); err != nil {
	return err
	}
	}

For example, we can create a RayCluster CR with worker.replicas = 2. We can see the following error messages from kuberay-operator. After the first worker pod is created, the variable worker is updated and has 1 volumeMount for /dev/shm. The second worker pod's specification has two volumeMounts for /dev/shm. Hence, Kubernetes API server reports the error. You must use kuberay/operator without commit #690 (e.g. kuberay/operator:v0.3.0) to reproduce this error. #690 can avoid misconfiguration, but cannot solve the root cause of this PR.

2022-11-12T18:37:23.184Z	ERROR	controllers.RayCluster	error creating pod	{"pod": {"kind":"Pod","apiVersion":"v1","metadata":{"generateName":"ray-cluster-kuberay-worker-workergroup-","namespace":"default","creationTimestamp":null,"labels":{"app.kubernetes.io/created-by":"kuberay-operator","app.kubernetes.io/instance":"ray-cluster","app.kubernetes.io/managed-by":"Helm","app.kubernetes.io/name":"kuberay","app.kubernetes.io/version":"1.0","groupName":"workergroup","helm.sh/chart":"ray-cluster-0.3.0","ray.io/cluster":"ray-cluster-kuberay","ray.io/cluster-dashboard":"ray-cluster-kuberay-dashboard","ray.io/group":"workergroup","ray.io/identifier":"ray-cluster-kuberay-worker","ray.io/is-ray-node":"yes","ray.io/node-type":"worker","rayCluster":"ray-cluster-kuberay"},"annotations":{"key":"value","ray.io/ft-enabled":"false","ray.io/health-state":""},"ownerReferences":[{"apiVersion":"ray.io/v1alpha1","kind":"RayCluster","name":"ray-cluster-kuberay","uid":"600b68df-91a8-4753-b34a-d23585b16de5","controller":true,"blockOwnerDeletion":true}]},"spec":{"volumes":[{"name":"log-volume","emptyDir":{}},{"name":"shared-mem","emptyDir":{"medium":"Memory"}}],"initContainers":[{"name":"init-myservice","image":"busybox:1.28","command":["sh","-c","until nslookup $RAY_IP.$(cat /var/run/secrets/kubernetes.io/serviceaccount/namespace).svc.cluster.local; do echo waiting for myservice; sleep 2; done"],"env":[{"name":"RAY_IP","value":"ray-cluster-kuberay-head-svc"}],"resources":{}}],"containers":[{"name":"ray-worker","image":"rayproject/ray:2.0.0","command":["/bin/bash","-c","--"],"args":["ulimit -n 65536; ray start  --address=ray-cluster-kuberay-head-svc:6379  --metrics-export-port=8080  --num-cpus=1  --redis-password=LetMeInRay  --block  --node-ip-address=$MY_POD_IP "],"ports":[{"containerPort":80,"protocol":"TCP"},{"name":"metrics","containerPort":8080}],"env":[{"name":"TYPE","value":"worker"},{"name":"MY_POD_IP","valueFrom":{"fieldRef":{"fieldPath":"status.podIP"}}},{"name":"RAY_DISABLE_DOCKER_CPU_WARNING","value":"1"},{"name":"CPU_REQUEST","valueFrom":{"resourceFieldRef":{"containerName":"ray-worker","resource":"requests.cpu","divisor":"0"}}},{"name":"MY_POD_NAME","valueFrom":{"fieldRef":{"fieldPath":"metadata.name"}}},{"name":"RAY_IP","value":"ray-cluster-kuberay-head-svc"},{"name":"RAY_PORT","value":"6379"},{"name":"RAY_ADDRESS","value":"ray-cluster-kuberay-head-svc:6379"},{"name":"REDIS_PASSWORD","value":"LetMeInRay"}],"resources":{"limits":{"cpu":"1"},"requests":{"cpu":"200m"}},"volumeMounts":[{"name":"log-volume","mountPath":"/tmp/ray"},{"name":"shared-mem","mountPath":"/dev/shm"},{"name":"shared-mem","mountPath":"/dev/shm"}],"imagePullPolicy":"IfNotPresent"}],"affinity":{}},"status":{}}, "err = ": "Pod \"ray-cluster-kuberay-worker-workergroup-txqk6\" is invalid: spec.containers[0].volumeMounts[2].mountPath: Invalid value: \"/dev/shm\": must be unique", "error": "createWorkerPod error"}
github.com/ray-project/kuberay/ray-operator/controllers/ray.(*RayClusterReconciler).reconcilePods
	/workspace/controllers/ray/raycluster_controller.go:489
github.com/ray-project/kuberay/ray-operator/controllers/ray.(*RayClusterReconciler).rayClusterReconcile
	/workspace/controllers/ray/raycluster_controller.go:211
github.com/ray-project/kuberay/ray-operator/controllers/ray.(*RayClusterReconciler).Reconcile
	/workspace/controllers/ray/raycluster_controller.go:95
sigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller).Reconcile
	/go/pkg/mod/sigs.k8s.io/[email protected]/pkg/internal/controller/controller.go:114
sigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller).reconcileHandler
	/go/pkg/mod/sigs.k8s.io/[email protected]/pkg/internal/controller/controller.go:311
sigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller).processNextWorkItem
	/go/pkg/mod/sigs.k8s.io/[email protected]/pkg/internal/controller/controller.go:266
sigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller).Start.func2.2
	/go/pkg/mod/sigs.k8s.io/[email protected]/pkg/internal/controller/controller.go:227

(3) "shallow copy of struct" I mentioned in (2) is not totally accurate. A struct is by default deep copied, but elements in the struct may be shallow copied or deep copied. See Shallow copy and Deep copy in Go for more details.

Related issue number

This PR solves a part of #716.

Checks

• I've made sure the tests are passing.
• Testing Strategy
  • Unit tests
  • Manual tests
  • This PR is not tested :(

[DmitriGekhtman]

Did we forget to run a CI step when we merged #468?

[DmitriGekhtman]

Opened an issue to track so we don't forget about it
#720

[DmitriGekhtman]

Thanks for the attention to detail here.