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

Check if workerGroupSpec is not empty before accessing it #549

Merged
merged 4 commits into from
May 29, 2024
Merged

Check if workerGroupSpec is not empty before accessing it #549

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
9fe55ad
Check if workerGroupSpec is not empty before accessing it
ChristianZaccaria May 8, 2024
730ef14
Add positive and negative test cases for RayCluster Webhook functiona…
ChristianZaccaria May 9, 2024
3b89d26
Run controller tests with test-unit make target
ChristianZaccaria May 21, 2024
954fa07
Use ResourceName generic function for raycluster_webhook tests
ChristianZaccaria May 27, 2024
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
2 changes: 1 addition & 1 deletion Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -381,7 +381,7 @@ catalog-push: ## Push a catalog image.

.PHONY: test-unit
test-unit: manifests fmt vet envtest ## Run unit tests.
KUBEBUILDER_ASSETS="$(shell $(ENVTEST) use $(ENVTEST_K8S_VERSION) --bin-dir $(LOCALBIN) -p path)" go test $(go list ./... | grep -v /test/) -coverprofile cover.out
KUBEBUILDER_ASSETS="$(shell $(ENVTEST) use $(ENVTEST_K8S_VERSION) --bin-dir $(LOCALBIN) -p path)" go test -v ./pkg/controllers/ -coverprofile cover.out

.PHONY: test-component
test-component: envtest ginkgo ## Run component tests.
Expand Down
2 changes: 1 addition & 1 deletion go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ require (
github.com/openshift/api v0.0.0-20230823114715-5fdd7511b790
github.com/openshift/client-go v0.0.0-20221019143426-16aed247da5c
github.com/project-codeflare/appwrapper v0.12.0
github.com/project-codeflare/codeflare-common v0.0.0-20240422163521-380101642c8f
github.com/project-codeflare/codeflare-common v0.0.0-20240528061920-68eadc29b5b0
github.com/ray-project/kuberay/ray-operator v1.1.0
go.uber.org/zap v1.26.0
golang.org/x/exp v0.0.0-20230905200255-921286631fa9
Expand Down
4 changes: 2 additions & 2 deletions go.sum
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -244,8 +244,8 @@ github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZb
github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4=
github.com/project-codeflare/appwrapper v0.12.0 h1:QMxryBPK6ir3VK6Qx4NWOA05/s4xU6uDHI/nXmLznvw=
github.com/project-codeflare/appwrapper v0.12.0/go.mod h1:sH9j/rXX6WIlZzFXUOuqK5pagASPZNhuCtdFK+3BDkw=
github.com/project-codeflare/codeflare-common v0.0.0-20240422163521-380101642c8f h1:9Uron4ej4Tt5ULX5CMzjmPqIZu3q/m07d4jhbNSwdPY=
github.com/project-codeflare/codeflare-common v0.0.0-20240422163521-380101642c8f/go.mod h1:tlPi2e1HZQuf7AAFc7keWdVUNcxV+Gfh6Ss4KAQs1O0=
github.com/project-codeflare/codeflare-common v0.0.0-20240528061920-68eadc29b5b0 h1:3Vz7D9/TwzrBNujHQZGb4L6UKu3siAWwVP4Bj3ByUrU=
github.com/project-codeflare/codeflare-common v0.0.0-20240528061920-68eadc29b5b0/go.mod h1:tlPi2e1HZQuf7AAFc7keWdVUNcxV+Gfh6Ss4KAQs1O0=
github.com/prometheus/client_golang v1.18.0 h1:HzFfmkOzH5Q8L8G+kSJKUx5dtG87sewO+FoDDqP5Tbk=
github.com/prometheus/client_golang v1.18.0/go.mod h1:T+GXkCk5wSJyOqMIzVgvvjFDlkOQntgjkJWKrN5txjA=
github.com/prometheus/client_model v0.0.0-20190812154241-14fe0d1b01d4/go.mod h1:xMI15A0UPsDsEKsMN9yxemIoYk6Tm2C1GtYGdfGttqA=
Expand Down
72 changes: 42 additions & 30 deletions pkg/controllers/raycluster_webhook.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -100,24 +100,27 @@ func (w *rayClusterWebhook) Default(ctx context.Context, obj runtime.Object) err
}

// WorkerGroupSpec

// Append the list of environment variables for the worker container
for _, envVar := range envVarList() {
rayCluster.Spec.WorkerGroupSpecs[0].Template.Spec.Containers[0].Env = upsert(rayCluster.Spec.WorkerGroupSpecs[0].Template.Spec.Containers[0].Env, envVar, withEnvVarName(envVar.Name))
}

// Append the CA volumes
for _, caVol := range caVolumes(rayCluster) {
rayCluster.Spec.WorkerGroupSpecs[0].Template.Spec.Volumes = upsert(rayCluster.Spec.WorkerGroupSpecs[0].Template.Spec.Volumes, caVol, withVolumeName(caVol.Name))
for i := range rayCluster.Spec.WorkerGroupSpecs {
workerSpec := &rayCluster.Spec.WorkerGroupSpecs[i]

// Append the list of environment variables for the worker container
for _, envVar := range envVarList() {
workerSpec.Template.Spec.Containers[0].Env = upsert(workerSpec.Template.Spec.Containers[0].Env, envVar, withEnvVarName(envVar.Name))
}

// Append the CA volumes
for _, caVol := range caVolumes(rayCluster) {
workerSpec.Template.Spec.Volumes = upsert(workerSpec.Template.Spec.Volumes, caVol, withVolumeName(caVol.Name))
}

// Append the certificate volume mounts
for _, mount := range certVolumeMounts() {
workerSpec.Template.Spec.Containers[0].VolumeMounts = upsert(workerSpec.Template.Spec.Containers[0].VolumeMounts, mount, byVolumeMountName)
}

// Append the create-cert Init Container
workerSpec.Template.Spec.InitContainers = upsert(workerSpec.Template.Spec.InitContainers, rayWorkerInitContainer(w.Config), withContainerName(initContainerName))
}

// Append the certificate volume mounts
for _, mount := range certVolumeMounts() {
rayCluster.Spec.WorkerGroupSpecs[0].Template.Spec.Containers[0].VolumeMounts = upsert(rayCluster.Spec.WorkerGroupSpecs[0].Template.Spec.Containers[0].VolumeMounts, mount, byVolumeMountName)
}

// Append the create-cert Init Container
rayCluster.Spec.WorkerGroupSpecs[0].Template.Spec.InitContainers = upsert(rayCluster.Spec.WorkerGroupSpecs[0].Template.Spec.InitContainers, rayWorkerInitContainer(w.Config), withContainerName(initContainerName))
}

return nil
Expand Down Expand Up @@ -386,10 +389,13 @@ func validateHeadInitContainer(rayCluster *rayv1.RayCluster, config *config.Kube
func validateWorkerInitContainer(rayCluster *rayv1.RayCluster, config *config.KubeRayConfiguration) field.ErrorList {
var allErrors field.ErrorList

if err := contains(rayCluster.Spec.WorkerGroupSpecs[0].Template.Spec.InitContainers, rayWorkerInitContainer(config), byContainerName,
field.NewPath("spec", "workerGroupSpecs", "0", "template", "spec", "initContainers"),
"create-cert Init Container is immutable"); err != nil {
allErrors = append(allErrors, err)
for i := range rayCluster.Spec.WorkerGroupSpecs {
workerSpec := &rayCluster.Spec.WorkerGroupSpecs[i]
if err := contains(workerSpec.Template.Spec.InitContainers, rayWorkerInitContainer(config), byContainerName,
field.NewPath("spec", "workerGroupSpecs", strconv.Itoa(i), "template", "spec", "initContainers"),
"create-cert Init Container is immutable"); err != nil {
allErrors = append(allErrors, err)
}
}

return allErrors
Expand All @@ -404,10 +410,13 @@ func validateCaVolumes(rayCluster *rayv1.RayCluster) field.ErrorList {
"ca-vol and server-cert Secret volumes are immutable"); err != nil {
allErrors = append(allErrors, err)
}
if err := contains(rayCluster.Spec.WorkerGroupSpecs[0].Template.Spec.Volumes, caVol, byVolumeName,
field.NewPath("spec", "workerGroupSpecs", "0", "template", "spec", "volumes"),
"ca-vol and server-cert Secret volumes are immutable"); err != nil {
allErrors = append(allErrors, err)
for i := range rayCluster.Spec.WorkerGroupSpecs {
workerSpec := &rayCluster.Spec.WorkerGroupSpecs[i]
if err := contains(workerSpec.Template.Spec.Volumes, caVol, byVolumeName,
field.NewPath("spec", "workerGroupSpecs", strconv.Itoa(i), "template", "spec", "volumes"),
"ca-vol and server-cert Secret volumes are immutable"); err != nil {
allErrors = append(allErrors, err)
}
}
}

Expand All @@ -431,11 +440,14 @@ func validateHeadEnvVars(rayCluster *rayv1.RayCluster) field.ErrorList {
func validateWorkerEnvVars(rayCluster *rayv1.RayCluster) field.ErrorList {
var allErrors field.ErrorList

for _, envVar := range envVarList() {
if err := contains(rayCluster.Spec.WorkerGroupSpecs[0].Template.Spec.Containers[0].Env, envVar, byEnvVarName,
field.NewPath("spec", "workerGroupSpecs", "0", "template", "spec", "containers", strconv.Itoa(0), "env"),
"RAY_TLS related environment variables are immutable"); err != nil {
allErrors = append(allErrors, err)
for i := range rayCluster.Spec.WorkerGroupSpecs {
workerSpec := &rayCluster.Spec.WorkerGroupSpecs[i]
for _, envVar := range envVarList() {
if err := contains(workerSpec.Template.Spec.Containers[0].Env, envVar, byEnvVarName,
field.NewPath("spec", "workerGroupSpecs", strconv.Itoa(i), "template", "spec", "containers", strconv.Itoa(0), "env"),
"RAY_TLS related environment variables are immutable"); err != nil {
allErrors = append(allErrors, err)
}
}
}

Expand Down
Loading