Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[Bug] Enable ResourceQuota by adding Resources for the health-check init container #1043

Merged
merged 1 commit into from
Apr 25, 2023
Merged

[Bug] Enable ResourceQuota by adding Resources for the health-check init container #1043

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 6 additions & 2 deletions ray-operator/controllers/ray/common/pod.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -191,6 +191,8 @@ func DefaultWorkerPodTemplate(instance rayiov1alpha1.RayCluster, workerSpec rayi

// The Ray worker should only start once the GCS server is ready.
rayContainerIndex := getRayContainerIndex(podTemplate.Spec)
// Do not modify `deepCopyRayContainer` anywhere.
deepCopyRayContainer := podTemplate.Spec.Containers[rayContainerIndex].DeepCopy()
initContainer := v1.Container{
Name: "wait-gcs-ready",
Image: podTemplate.Spec.Containers[rayContainerIndex].Image,
Expand All @@ -203,8 +205,10 @@ func DefaultWorkerPodTemplate(instance rayiov1alpha1.RayCluster, workerSpec rayi
// This init container requires certain environment variables to establish a secure connection with the Ray head using TLS authentication.
// Additionally, some of these environment variables may reference files stored in volumes, so we need to include both the `Env` and `VolumeMounts` fields here.
// For more details, please refer to: https://docs.ray.io/en/latest/ray-core/configure.html#tls-authentication.
Env: podTemplate.Spec.Containers[rayContainerIndex].DeepCopy().Env,
VolumeMounts: podTemplate.Spec.Containers[rayContainerIndex].DeepCopy().VolumeMounts,
Env: deepCopyRayContainer.Env,
VolumeMounts: deepCopyRayContainer.VolumeMounts,
// If users specify ResourceQuota for the namespace, the init container need to specify resource explicitly.
Resources: deepCopyRayContainer.Resources,
}
podTemplate.Spec.InitContainers = append(podTemplate.Spec.InitContainers, initContainer)

Expand Down
6 changes: 6 additions & 0 deletions ray-operator/controllers/ray/common/pod_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -827,6 +827,8 @@ func TestDefaultInitContainer(t *testing.T) {
// Ray container to the init container. This may be changed in the future.
healthCheckContainer := podTemplateSpec.Spec.InitContainers[numInitContainers-1]
rayContainer := worker.Template.Spec.Containers[getRayContainerIndex(worker.Template.Spec)]

assert.NotEqual(t, len(rayContainer.Env), 0, "The test only makes sense if the Ray container has environment variables.")
assert.Equal(t, len(rayContainer.Env), len(healthCheckContainer.Env))
for _, env := range rayContainer.Env {
// env.ValueFrom is the source for the environment variable's value. Cannot be used if value is not empty.
Expand All @@ -836,4 +838,8 @@ func TestDefaultInitContainer(t *testing.T) {
checkContainerEnv(t, healthCheckContainer, env.Name, env.ValueFrom.FieldRef.FieldPath)
}
}

// The values of `Resources` should be the same in both Ray container and health-check container.
assert.NotEmpty(t, rayContainer.Resources, "The test only makes sense if the Ray container has resource limit/request.")
assert.Equal(t, rayContainer.Resources, healthCheckContainer.Resources)
}