Merge pull request #142 from red-hat-data-services/upstream-branch

pawelpaszki · web-flow · commit c37a37cf23f4 · 2025-06-09T12:08:16.000+01:00
Upstream to downstream main
diff --git a/.tekton/ray-cuda-push.yaml b/.tekton/ray-cuda-push.yaml
@@ -24,9 +24,9 @@ spec:
   - name: revision
     value: '{{revision}}'
   - name: output-image
-    value: quay.io/modh/ray:2.44.1-py311-cu121
+    value: quay.io/modh/ray:2.46.0-py311-cu121
   - name: additional-tag
-    value: 2.44.1-py311-cu121-{{revision}}
+    value: 2.46.0-py311-cu121-{{revision}}
   - name: dockerfile
     value: Dockerfile
   - name: path-context
diff --git a/.tekton/ray-rocm-push.yaml b/.tekton/ray-rocm-push.yaml
@@ -24,9 +24,9 @@ spec:
   - name: revision
     value: '{{revision}}'
   - name: output-image
-    value: quay.io/modh/ray:2.44.1-py311-rocm62
+    value: quay.io/modh/ray:2.46.0-py311-rocm62
   - name: additional-tag
-    value: 2.44.1-py311-rocm62-{{revision}}
+    value: 2.46.0-py311-rocm62-{{revision}}
   - name: dockerfile
     value: Dockerfile
   - name: path-context
diff --git a/OWNERS b/OWNERS
@@ -1,24 +1,30 @@
 approvers:
   - astefanutti
   - Bobbins228
+  - chipspeak
   - dimakis
   - Fiona-Waters
   - jiripetrlik
   - kpostoffice
+  - kryanbeane
   - laurafitzgerald
+  - pawelpaszki
   - sutaakar
   - szaher
   - varshaprasad96
 reviewers:
   - abhijeet-dhumal
   - astefanutti
   - Bobbins228
+  - chipspeak
   - ChughShilpa
   - dimakis
   - Fiona-Waters
   - jiripetrlik
   - kpostoffice
+  - kryanbeane
   - laurafitzgerald
+  - pawelpaszki
   - sutaakar
   - szaher
   - varshaprasad96
diff --git a/README.md b/README.md
@@ -49,6 +49,7 @@ To upload trained model into S3 compatible storage, use the environment variable
 
 * `ODH_NAMESPACE` - Namespace where ODH components are installed to
 * `NOTEBOOK_USER_NAME` - Username of user used for running Workbench
+* `NOTEBOOK_USER_PASSWORD` - Password of user used for running Workbench
 * `NOTEBOOK_USER_TOKEN` - Login token of user used for running Workbench
 * `NOTEBOOK_IMAGE` - Image used for running Workbench
 
diff --git a/images/runtime/ray/cuda/Dockerfile b/images/runtime/ray/cuda/Dockerfile
@@ -1,5 +1,5 @@
 ARG PYTHON_VERSION=311
-ARG IMAGE_TAG=9.5-1744097391
+ARG IMAGE_TAG=9.6-1747333117
 
 FROM registry.access.redhat.com/ubi9/python-${PYTHON_VERSION}:${IMAGE_TAG}
 
diff --git a/images/runtime/ray/cuda/Pipfile b/images/runtime/ray/cuda/Pipfile
@@ -5,7 +5,7 @@ name = "pypi"
 
 [packages]
 numpy = {version = "<2.0.0" }
-"ray[all]" = {version = "~=2.44.0" }
+"ray[all]" = {version = "~=2.46.0" }
 
 [requires]
 python_version = "3.9"
diff --git a/images/runtime/ray/cuda/Pipfile.lock b/images/runtime/ray/cuda/Pipfile.lock
diff --git a/images/runtime/ray/cuda/README.md b/images/runtime/ray/cuda/README.md
@@ -6,4 +6,4 @@ It includes the following layers:
 * UBI 9
 * Python 3.11
 * CUDA 12.1
-* Ray 2.44.1
+* Ray 2.46.0
diff --git a/images/runtime/ray/rocm/Dockerfile b/images/runtime/ray/rocm/Dockerfile
@@ -1,5 +1,5 @@
 ARG PYTHON_VERSION=311
-ARG IMAGE_TAG=9.5-1746539270
+ARG IMAGE_TAG=9.6-1747333117
 
 FROM registry.access.redhat.com/ubi9/python-${PYTHON_VERSION}:${IMAGE_TAG}
 
diff --git a/images/runtime/ray/rocm/Pipfile b/images/runtime/ray/rocm/Pipfile
@@ -5,7 +5,7 @@ name = "pypi"
 
 [packages]
 numpy = {version = "<2.0.0" }
-"ray[all]" = {version = "~=2.44.0"}
+"ray[all]" = {version = "~=2.46.0"}
 
 [requires]
 python_version = "3.9"
diff --git a/images/runtime/ray/rocm/Pipfile.lock b/images/runtime/ray/rocm/Pipfile.lock
diff --git a/images/runtime/ray/rocm/README.md b/images/runtime/ray/rocm/README.md
@@ -6,4 +6,4 @@ It includes the following layers:
 * UBI 9
 * Python 3.11
 * ROCm 6.1
-* Ray 2.44.1
+* Ray 2.46.0
diff --git a/images/tests/Dockerfile b/images/tests/Dockerfile
@@ -6,6 +6,12 @@ ENV KUBECONFIG=/distributed-workloads/tests/.kube/config
 # Set the working directory inside the container
 WORKDIR /distributed-workloads
 
+# Download the latest OpenShift CLI binary
+RUN wget -q https://mirror.openshift.com/pub/openshift-v4/clients/ocp/stable-4.18/openshift-client-linux.tar.gz -P oc-client && \
+    tar -xf oc-client/openshift-client-linux.tar.gz -C oc-client && \
+    cp oc-client/oc /usr/local/bin && \
+    rm -rf oc-client/
+
 # Copy the go mod and sum files
 COPY go.mod go.sum ./
 
diff --git a/tests/common/environment.go b/tests/common/environment.go
@@ -31,6 +31,8 @@ const (
 	notebookUserName = "NOTEBOOK_USER_NAME"
 	// Token of the authenticated Notebook user
 	notebookUserToken = "NOTEBOOK_USER_TOKEN"
+	// Password of the authenticated Notebook user
+	notebookUserPassword = "NOTEBOOK_USER_PASSWORD"
 	// Image of the Notebook
 	notebookImage = "NOTEBOOK_IMAGE"
 	// Test tier to be invoked
@@ -77,6 +79,14 @@ func GetNotebookUserToken(t Test) string {
 	return token
 }
 
+func GetNotebookUserPassword(t Test) string {
+	password, ok := os.LookupEnv(notebookUserPassword)
+	if !ok {
+		t.T().Fatalf("Expected environment variable %s not found, please use this environment variable to specify token of the authenticated Notebook password.", notebookUserPassword)
+	}
+	return password
+}
+
 func GetNotebookImage(t Test) string {
 	notebook_image, ok := os.LookupEnv(notebookImage)
 	if !ok {
diff --git a/tests/kfto/kfto_mnist_sdk_test.go b/tests/kfto/kfto_mnist_sdk_test.go
@@ -18,6 +18,8 @@ package kfto
 
 import (
 	"fmt"
+	"os"
+	"os/exec"
 	"strings"
 	"testing"
 	"time"
@@ -48,7 +50,8 @@ func runMnistSDK(t *testing.T, trainingImage string) {
 	// Create a namespace
 	namespace := test.NewTestNamespace()
 	userName := GetNotebookUserName(test)
-	userToken := GetNotebookUserToken(test)
+	userToken := generateNotebookUserToken(test)
+
 	jupyterNotebookConfigMapFileName := "mnist_kfto.ipynb"
 	mnist := ParseAWSArgs(test, readFile(test, "resources/kfto_sdk_mnist.py"))
 
@@ -90,19 +93,8 @@ func runMnistSDK(t *testing.T, trainingImage string) {
 	defer test.Client().Kueue().KueueV1beta1().ClusterQueues().Delete(test.Ctx(), clusterQueue.Name, metav1.DeleteOptions{})
 	CreateKueueLocalQueue(test, namespace.Name, clusterQueue.Name, AsDefaultQueue)
 
-	requiredChangesInNotebook := map[string]string{
-		"${api_url}":        GetOpenShiftApiUrl(test),
-		"${password}":       userToken,
-		"${num_gpus}":       "0",
-		"${namespace}":      namespace.Name,
-		"${training_image}": trainingImage,
-	}
-
 	jupyterNotebook := string(readFile(test, "resources/mnist_kfto.ipynb"))
 	requirements := readFile(test, "resources/requirements.txt")
-	for oldValue, newValue := range requiredChangesInNotebook {
-		jupyterNotebook = strings.Replace(string(jupyterNotebook), oldValue, newValue, -1)
-	}
 
 	config := CreateConfigMap(test, namespace.Name, map[string][]byte{
 		jupyterNotebookConfigMapFileName: []byte(jupyterNotebook),
@@ -113,10 +105,10 @@ func runMnistSDK(t *testing.T, trainingImage string) {
 	notebookCommand := []string{
 		"/bin/sh",
 		"-c",
-		"pip install papermill && papermill /opt/app-root/notebooks/{{.NotebookConfigMapFileName}}" +
-			" /opt/app-root/src/mcad-out.ipynb -p namespace {{.Namespace}} -p openshift_api_url {{.OpenShiftApiUrl}}" +
-			" -p kubernetes_user_bearer_token {{.KubernetesUserBearerToken}}" +
-			" -p num_gpus {{.NumGpus}} --log-output && sleep infinity",
+		fmt.Sprintf("pip install papermill && papermill /opt/app-root/notebooks/%s"+
+			" /opt/app-root/src/mnist-kfto-out.ipynb -p namespace %s -p openshift_api_url %s"+
+			" -p token %s -p num_gpus %d -p training_image %s --log-output && sleep infinity",
+			jupyterNotebookConfigMapFileName, namespace.Name, GetOpenShiftApiUrl(test), userToken, 0, trainingImage),
 	}
 	// Create Notebook CR
 	CreateNotebook(test, namespace, userToken, notebookCommand, config.Name, jupyterNotebookConfigMapFileName, 0)
@@ -138,3 +130,43 @@ func runMnistSDK(t *testing.T, trainingImage string) {
 	// TODO: write torch job logs?
 	// time.Sleep(60 * time.Second)
 }
+
+func generateNotebookUserToken(test Test) string {
+	userName := GetNotebookUserName(test)
+	password := GetNotebookUserPassword(test)
+
+	// Use own kobeconfig file to retrieve user token to keep it separated from main test credentials
+	tempFile, err := os.CreateTemp("", "custom-kubeconfig-")
+	test.Expect(err).NotTo(HaveOccurred())
+	defer os.Remove(tempFile.Name())
+
+	// Login by oc CLI using username and password
+	cmd := exec.Command("oc", "login", "-u", userName, "-p", password, GetOpenShiftApiUrl(test), "--insecure-skip-tls-verify=true", "--kubeconfig="+tempFile.Name())
+	out, err := cmd.Output()
+	if err != nil {
+		if exitError, ok := err.(*exec.ExitError); ok {
+			test.T().Logf("Error running 'oc login' command: %v\n", exitError)
+			test.T().Logf("Output: %s\n", out)
+			test.T().Logf("Error output: %s\n", exitError.Stderr)
+		} else {
+			test.T().Logf("Error running 'oc login' command: %v\n", err)
+		}
+		test.T().FailNow()
+	}
+
+	// Use oc CLI to retrieve user token from kubeconfig
+	cmd = exec.Command("oc", "whoami", "--show-token", "--kubeconfig="+tempFile.Name())
+	out, err = cmd.Output()
+	if err != nil {
+		if exitError, ok := err.(*exec.ExitError); ok {
+			test.T().Logf("Error running 'oc whoami' command: %v\n", exitError)
+			test.T().Logf("Output: %s\n", out)
+			test.T().Logf("Error output: %s\n", exitError.Stderr)
+		} else {
+			test.T().Logf("Error running 'oc whoami' command: %v\n", err)
+		}
+		test.T().FailNow()
+	}
+
+	return strings.TrimSpace(string(out))
+}
diff --git a/tests/kfto/kfto_training_test.go b/tests/kfto/kfto_training_test.go
@@ -219,7 +219,7 @@ func createKFTOPyTorchJob(test Test, namespace string, config corev1.ConfigMap,
 		ObjectMeta: metav1.ObjectMeta{
 			GenerateName: "kfto-llm-",
 			Labels: map[string]string{
-				"kueue.x-k8s.io/default-queue": localQueue.Name,
+				"kueue.x-k8s.io/queue-name": localQueue.Name,
 			},
 		},
 		Spec: kftov1.PyTorchJobSpec{
diff --git a/tests/kfto/resources/mnist_kfto.ipynb b/tests/kfto/resources/mnist_kfto.ipynb
@@ -31,7 +31,11 @@
    "cell_type": "code",
    "execution_count": 41,
    "id": "72dd1751",
-   "metadata": {},
+   "metadata": {
+    "tags": [
+     "parameters"
+    ]
+   },
    "outputs": [],
    "source": [
     "# parameters\n",
diff --git a/tests/kfto/validating_admission_policy_test.go b/tests/kfto/validating_admission_policy_test.go
@@ -19,6 +19,7 @@ package kfto
 import (
 	"encoding/json"
 	"testing"
+	"time"
 
 	kftrainingv1 "github.com/kubeflow/training-operator/pkg/apis/kubeflow.org/v1"
 	. "github.com/onsi/gomega"
@@ -151,17 +152,29 @@ func TestValidatingAdmissionPolicy(t *testing.T) {
 		vapb.Spec.PolicyName = "none"
 		_, err = test.Client().Core().AdmissionregistrationV1().ValidatingAdmissionPolicyBindings().Update(test.Ctx(), vapb, metav1.UpdateOptions{})
 		test.Expect(err).ToNot(HaveOccurred())
+
+		// Add verification that the VAP is actually disabled
+		test.Eventually(func() (string, error) {
+			updatedVapb, err := test.Client().Core().AdmissionregistrationV1().ValidatingAdmissionPolicyBindings().Get(test.Ctx(), vapb.Name, metav1.GetOptions{})
+			if err != nil {
+				return "", err
+			}
+			return updatedVapb.Spec.PolicyName, nil
+		}).WithTimeout(10*time.Second).WithPolling(500*time.Millisecond).Should(Equal("none"), "VAP should be disabled")
+
 		defer revertVAPB(test, vapbCopy)
 
 		t.Run("PyTorchJob Tests", func(t *testing.T) {
 			t.Run("PyTorchJob should be admitted with the 'kueue.x-k8s.io/queue-name' label set", func(t *testing.T) {
-				err = createPyTorchJobWithLocalQueue(test, ns.Name, lq.Name)
-				test.Expect(err).ToNot(HaveOccurred())
+				test.Eventually(func() error {
+					return createPyTorchJobWithLocalQueue(test, ns.Name, lq.Name)
+				}).WithTimeout(10*time.Second).WithPolling(500*time.Millisecond).Should(Succeed(), "PyTorchJob with queue label should be created")
 				defer test.Client().Kubeflow().KubeflowV1().PyTorchJobs(ns.Name).Delete(test.Ctx(), pyt.Name, metav1.DeleteOptions{})
 			})
 			t.Run("PyTorchJob should be admitted without the 'kueue.x-k8s.io/queue-name' label set", func(t *testing.T) {
-				err = createPyTorchJob(test, ns.Name)
-				test.Expect(err).ToNot(HaveOccurred())
+				test.Eventually(func() error {
+					return createPyTorchJob(test, ns.Name)
+				}).WithTimeout(10*time.Second).WithPolling(500*time.Millisecond).Should(Succeed(), "PyTorchJob without queue label should be created")
 				defer test.Client().Kubeflow().KubeflowV1().PyTorchJobs(ns.Name).Delete(test.Ctx(), pyt.Name, metav1.DeleteOptions{})
 			})
 		})
@@ -178,28 +191,44 @@ func TestValidatingAdmissionPolicy(t *testing.T) {
 		vapb.Spec.MatchResources.NamespaceSelector.MatchLabels = map[string]string{"kueue.openshift.io/managed": "true"}
 		_, err = test.Client().Core().AdmissionregistrationV1().ValidatingAdmissionPolicyBindings().Update(test.Ctx(), vapb, metav1.UpdateOptions{})
 		test.Expect(err).ToNot(HaveOccurred())
+
+		// Add verification that the VAP namespace selector is updated
+		test.Eventually(func() (string, error) {
+			updatedVapb, err := test.Client().Core().AdmissionregistrationV1().ValidatingAdmissionPolicyBindings().Get(test.Ctx(), vapb.Name, metav1.GetOptions{})
+			if err != nil {
+				return "", err
+			}
+			return updatedVapb.Spec.MatchResources.NamespaceSelector.MatchLabels["kueue.openshift.io/managed"], nil
+		}).WithTimeout(10*time.Second).WithPolling(500*time.Millisecond).Should(Equal("true"), "VAP namespace selector should be updated")
+
 		defer revertVAPB(test, vapbCopy)
 
 		t.Run("PyTorchJob Tests", func(t *testing.T) {
 			t.Run("PyTorchJob should be admitted with the 'kueue.x-k8s.io/queue-name' label in a labeled namespace", func(t *testing.T) {
-				err = createPyTorchJobWithLocalQueue(test, ns.Name, lq.Name)
-				test.Expect(err).ToNot(HaveOccurred())
+				test.Eventually(func() error {
+					return createPyTorchJobWithLocalQueue(test, ns.Name, lq.Name)
+				}).WithTimeout(10*time.Second).WithPolling(500*time.Millisecond).Should(Succeed(), "PyTorchJob with queue label should be created in labeled namespace")
 				defer test.Client().Kubeflow().KubeflowV1().PyTorchJobs(ns.Name).Delete(test.Ctx(), pyt.Name, metav1.DeleteOptions{})
 			})
 			t.Run("PyTorchJob should not be admitted without the 'kueue.x-k8s.io/queue-name' label in a labeled namespace", func(t *testing.T) {
-				err = createPyTorchJob(test, ns.Name)
-				test.Expect(err).ToNot(BeNil())
-				test.Expect(err.Error()).To(ContainSubstring("The label 'kueue.x-k8s.io/queue-name' is either missing or does not have a value set"))
+				test.Eventually(func() error {
+					err = createPyTorchJob(test, ns.Name)
+					test.Expect(err).To(HaveOccurred())
+					test.Expect(err.Error()).To(ContainSubstring("The label 'kueue.x-k8s.io/queue-name' is either missing or does not have a value set"))
+					return err
+				}).WithTimeout(10 * time.Second).WithPolling(500 * time.Millisecond).Should(HaveOccurred())
 				defer test.Client().Kubeflow().KubeflowV1().PyTorchJobs(ns.Name).Delete(test.Ctx(), pyt.Name, metav1.DeleteOptions{})
 			})
 			t.Run("PyTorchJob should be admitted with the 'kueue.x-k8s.io/queue-name' label in any other namespace", func(t *testing.T) {
-				err = createPyTorchJobWithLocalQueue(test, nsNoLabel.Name, lq.Name)
-				test.Expect(err).ToNot(HaveOccurred())
+				test.Eventually(func() error {
+					return createPyTorchJobWithLocalQueue(test, nsNoLabel.Name, lq.Name)
+				}).WithTimeout(10*time.Second).WithPolling(500*time.Millisecond).Should(Succeed(), "PyTorchJob with queue label should be created in unlabeled namespace")
 				defer test.Client().Kubeflow().KubeflowV1().PyTorchJobs(ns.Name).Delete(test.Ctx(), pyt.Name, metav1.DeleteOptions{})
 			})
 			t.Run("PyTorchJob should be admitted without the 'kueue.x-k8s.io/queue-name' label in any other namespace", func(t *testing.T) {
-				err = createPyTorchJob(test, nsNoLabel.Name)
-				test.Expect(err).ToNot(HaveOccurred())
+				test.Eventually(func() error {
+					return createPyTorchJob(test, nsNoLabel.Name)
+				}).WithTimeout(10*time.Second).WithPolling(500*time.Millisecond).Should(Succeed(), "PyTorchJob without queue label should be created in unlabeled namespace")
 				defer test.Client().Kubeflow().KubeflowV1().PyTorchJobs(ns.Name).Delete(test.Ctx(), pyt.Name, metav1.DeleteOptions{})
 			})
 		})
