fix(test): provide ability to override versions used in asserts durin…

…g papermill tests Our testing framework had a fundamental issue in that it would, for certain images, naively run the test suite of parent images for a child image. In the case of `tensorflow` images, this caused problems because the (child) `tensorflow` image would **purposefully** _downgraded_ a package version given compatability issues. This fix provides the ability within a child image to specify the proper package version to assert against. The key concept here is the introduction of an (optional) `version_overrides.ini` that can be defined within a given `test/` subfolder for an image. Example format: ``` [minimal] nbdime = 3.6 [datascience] numpy = 1.26 ``` - as a best practice, key names here within a given section are intended to match the relevant python `package` name - but that is by no means a requirement While this file can be **defined** in any `test/` folder - it is presently only honored within the `minimal` and `datascience` images - as they are the "parent" images most likely susceptible to the test framework shortcomings today. For the `datascience` and `minimal` test suites - they now will attempt to load the `version_overrides.ini` file from the current working directory. This file will be copied into the running kubernetes container as part of a new `Makefile` def named `handle_test_version_overrides` **IF THE FILE EXISTS**. The relevant parent test suites will then attempt to load the version to assert against from the `configparser.ConfigParser` instance, falling back to the value we historically had hard-coded. In this manner, child images do **NOT** need to specify a `version_overrides.ini` unless required to do so. Absence of such file would then be identical to current testing behavior. - `handle_test_version_overrides` is present only called if the `validate-datascience` target is going to be invoked Some minor refactoring also was applied to the `Makefile` to reduce the use of "magic strings" - particularly since the introduction of `handle_test_version_overrides` would have required more brittle copy/pasting of hard-coded strings. As such, common values that were historically hard-coded but re-used in multiple places - have now been converted over to be variables. Related-to: https://issues.redhat.com/browse/RHOAIENG-16587
andyatmiami · Jan 17, 2025 · ee21ef4 · ee21ef4
1 parent 8a24d01
commit ee21ef4
Show file tree

Hide file tree

Showing 29 changed files with 846 additions and 380 deletions.
diff --git a/.gitignore b/.gitignore
@@ -128,6 +128,7 @@ venv/
 ENV/
 env.bak/
 venv.bak/
+.DS_store
 
 # Spyder project settings
 .spyderproject

diff --git a/Makefile b/Makefile
@@ -28,13 +28,15 @@ BUILD_DEPENDENT_IMAGES ?= yes
 PUSH_IMAGES ?= yes
 
 # OS dependant: Generate date, select appropriate cmd to locate container engine
-ifeq ($(OS), Windows_NT)
-	DATE 		?= $(shell powershell -Command "Get-Date -Format 'yyyyMMdd'")
-	WHERE_WHICH ?= where
-else
-	DATE 		?= $(shell date +'%Y%m%d')
-	WHERE_WHICH ?= which
+ifdef OS
+	ifeq ($(OS), Windows_NT)
+		DATE 		?= $(shell powershell -Command "Get-Date -Format 'yyyyMMdd'")
+		WHERE_WHICH ?= where
+	endif
 endif
+DATE 		?= $(shell date +'%Y%m%d')
+WHERE_WHICH ?= which
+
 
 # linux/amd64 or darwin/arm64
 OS_ARCH=$(shell go env GOOS)/$(shell go env GOARCH)
@@ -340,64 +342,11 @@ undeploy-c9s-%: bin/kubectl
 	$(info # Undeploying notebook from $(NOTEBOOK_DIR) directory...)
 	$(KUBECTL_BIN) delete -k $(NOTEBOOK_DIR)
 
-# Function for testing a notebook with papermill
-#   ARG 1: Notebook name
-#   ARG 1: UBI flavor
-#   ARG 1: Python kernel
-define test_with_papermill
-	$(eval PREFIX_NAME := $(subst /,-,$(1)_$(2)))
-	$(KUBECTL_BIN) exec $(FULL_NOTEBOOK_NAME) -- /bin/sh -c "python3 -m pip install papermill"
-	if ! $(KUBECTL_BIN) exec $(FULL_NOTEBOOK_NAME) -- /bin/sh -c "wget ${NOTEBOOK_REPO_BRANCH_BASE}/jupyter/$(1)/$(2)-$(3)/test/test_notebook.ipynb -O test_notebook.ipynb && python3 -m papermill test_notebook.ipynb $(PREFIX_NAME)_output.ipynb --kernel python3 --stderr-file $(PREFIX_NAME)_error.txt" ; then
-		echo "ERROR: The $(1) $(2) notebook encountered a failure. To investigate the issue, you can review the logs located in the ocp-ci cluster on 'artifacts/notebooks-e2e-tests/jupyter-$(1)-$(2)-$(3)-test-e2e' directory or run 'cat $(PREFIX_NAME)_error.txt' within your container. The make process has been aborted."
-		exit 1
-	fi
-	if $(KUBECTL_BIN) exec $(FULL_NOTEBOOK_NAME) -- /bin/sh -c "cat $(PREFIX_NAME)_error.txt | grep --quiet FAILED" ; then
-		echo "ERROR: The $(1) $(2) notebook encountered a failure. The make process has been aborted."
-		$(KUBECTL_BIN) exec $(FULL_NOTEBOOK_NAME) -- /bin/sh -c "cat $(PREFIX_NAME)_error.txt"
-		exit 1
-	fi
-endef
-
 # Verify the notebook's readiness by pinging the /api endpoint and executing the corresponding test_notebook.ipynb file in accordance with the build chain logic.
 .PHONY: test
 test-%: bin/kubectl
-	# Verify the notebook's readiness by pinging the /api endpoint
-	$(eval NOTEBOOK_NAME := $(subst .,-,$(subst cuda-,,$*)))
-	$(eval PYTHON_VERSION := $(shell echo $* | sed 's/.*-python-//'))
-	$(info # Running tests for $(NOTEBOOK_NAME) notebook...)
-	$(KUBECTL_BIN) wait --for=condition=ready pod -l app=$(NOTEBOOK_NAME) --timeout=600s
-	$(KUBECTL_BIN) port-forward svc/$(NOTEBOOK_NAME)-notebook 8888:8888 & curl --retry 5 --retry-delay 5 --retry-connrefused http://localhost:8888/notebook/opendatahub/jovyan/api ; EXIT_CODE=$$?; echo && pkill --full "^$(KUBECTL_BIN).*port-forward.*"
-	$(eval FULL_NOTEBOOK_NAME = $(shell ($(KUBECTL_BIN) get pods -l app=$(NOTEBOOK_NAME) -o custom-columns=":metadata.name" | tr -d '\n')))
-
-	# Tests notebook's functionalities
-	if echo "$(FULL_NOTEBOOK_NAME)" | grep -q "minimal-ubi9"; then
-		$(call test_with_papermill,minimal,ubi9,python-$(PYTHON_VERSION))
-	elif echo "$(FULL_NOTEBOOK_NAME)" | grep -q "intel-tensorflow-ubi9"; then
-		$(call test_with_papermill,intel/tensorflow,ubi9,python-$(PYTHON_VERSION))
-	elif echo "$(FULL_NOTEBOOK_NAME)" | grep -q "intel-pytorch-ubi9"; then
-		$(call test_with_papermill,intel/pytorch,ubi9,python-$(PYTHON_VERSION))
-	elif echo "$(FULL_NOTEBOOK_NAME)" | grep -q "datascience-ubi9"; then
-		$(MAKE) validate-ubi9-datascience PYTHON_VERSION=$(PYTHON_VERSION) -e FULL_NOTEBOOK_NAME=$(FULL_NOTEBOOK_NAME)
-	elif echo "$(FULL_NOTEBOOK_NAME)" | grep -q "pytorch-ubi9"; then
-		$(MAKE) validate-ubi9-datascience PYTHON_VERSION=$(PYTHON_VERSION) -e FULL_NOTEBOOK_NAME=$(FULL_NOTEBOOK_NAME)
-		$(call test_with_papermill,pytorch,ubi9,python-$(PYTHON_VERSION))
-	elif echo "$(FULL_NOTEBOOK_NAME)" | grep -q "tensorflow-ubi9"; then
-		$(MAKE) validate-ubi9-datascience PYTHON_VERSION=$(PYTHON_VERSION) -e FULL_NOTEBOOK_NAME=$(FULL_NOTEBOOK_NAME)
-		$(call test_with_papermill,tensorflow,ubi9,python-$(PYTHON_VERSION))
-	elif echo "$(FULL_NOTEBOOK_NAME)" | grep -q "intel-ml-ubi9"; then
-		$(call test_with_papermill,intel/ml,ubi9,python-$(PYTHON_VERSION))
-	elif echo "$(FULL_NOTEBOOK_NAME)" | grep -q "trustyai-ubi9"; then
-		$(call test_with_papermill,trustyai,ubi9,python-$(PYTHON_VERSION))
-	elif echo "$(FULL_NOTEBOOK_NAME)" | grep -q "anaconda"; then
-		echo "There is no test notebook implemented yet for Anaconda Notebook...."
-	else
-		echo "No matching condition found for $(FULL_NOTEBOOK_NAME)."
-	fi
-
-.PHONY: validate-ubi9-datascience
-validate-ubi9-datascience:
-	$(call test_with_papermill,minimal,ubi9,python-$(PYTHON_VERSION))
-	$(call test_with_papermill,datascience,ubi9,python-$(PYTHON_VERSION))
+	$(info # Running tests for $* notebook...)
+	@./scripts/test_jupyter_with_papermill.sh $*
 
 # Validate that runtime image meets minimum criteria
 # This validation is created from subset of https://github.com/elyra-ai/elyra/blob/9c417d2adc9d9f972de5f98fd37f6945e0357ab9/Makefile#L325

diff --git a/jupyter/datascience/ubi9-python-3.11/test/test_notebook.ipynb b/jupyter/datascience/ubi9-python-3.11/test/test_notebook.ipynb
@@ -7,6 +7,9 @@
    "metadata": {},
    "outputs": [],
    "source": [
+    "from pathlib import Path\n",
+    "import json\n",
+    "import re\n",
     "import unittest\n",
     "from unittest import mock\n",
     "from platform import python_version\n",
@@ -24,22 +27,35 @@
     "import kafka\n",
     "from kafka import KafkaConsumer, KafkaProducer, TopicPartition\n",
     "from kafka.producer.buffer import SimpleBufferPool\n",
-    "from kafka import KafkaConsumer\n",
     "from kafka.errors import KafkaConfigurationError\n",
     "import boto3\n",
     "\n",
     "def get_major_minor(s):\n",
     "    return '.'.join(s.split('.')[:2])\n",
     "\n",
+    "def load_expected_versions() -> dict:\n",
+    "    lock_file = Path('./expected_versions.json')\n",
+    "    data = {}\n",
+    "\n",
+    "    with open(lock_file, 'r') as file:\n",
+    "        data = json.load(file)\n",
+    "\n",
+    "    return data    \n",
+    "\n",
+    "def get_expected_version(dependency_name: str) -> str:\n",
+    "    raw_value = expected_versions.get(dependency_name)\n",
+    "    raw_version = re.sub(r'^\\D+', '', raw_value)\n",
+    "    return get_major_minor(raw_version)\n",
+    "\n",
     "class TestPythonVersion(unittest.TestCase):\n",
     "    def test_version(self):\n",
-    "        expected_major_minor = '3.11'\n",
+    "        expected_major_minor = expected_major_minor = get_expected_version('Python')\n",
     "        actual_major_minor = get_major_minor(python_version())\n",
     "        self.assertEqual(actual_major_minor, expected_major_minor, \"incorrect version\")\n",
     "\n",
     "class TestPandas(unittest.TestCase):\n",
     "    def test_version(self):\n",
-    "        expected_major_minor = '2.2'\n",
+    "        expected_major_minor = get_expected_version('Pandas')\n",
     "        actual_major_minor = get_major_minor(pd.__version__)\n",
     "        self.assertEqual(actual_major_minor, expected_major_minor, \"incorrect version\")\n",
     "\n",
@@ -94,7 +110,7 @@
     "\n",
     "class TestNumpy(unittest.TestCase):\n",
     "    def test_version(self):\n",
-    "        expected_major_minor = '2.1'\n",
+    "        expected_major_minor = get_expected_version('Numpy')\n",
     "        actual_major_minor = get_major_minor(np.__version__)\n",
     "        self.assertEqual(actual_major_minor, expected_major_minor, \"incorrect version\")\n",
     "\n",
@@ -119,7 +135,7 @@
     "\n",
     "class TestScipy(unittest.TestCase):\n",
     "    def test_version(self):\n",
-    "        expected_major_minor = '1.14'\n",
+    "        expected_major_minor = get_expected_version('Scipy')\n",
     "        actual_major_minor = get_major_minor(scipy.__version__)\n",
     "        self.assertEqual(actual_major_minor, expected_major_minor, \"incorrect version\")\n",
     "\n",
@@ -136,7 +152,7 @@
     "\n",
     "class TestSKLearn(unittest.TestCase):\n",
     "    def test_version(self):\n",
-    "        expected_major_minor = '1.5'\n",
+    "        expected_major_minor = get_expected_version('Scikit-learn')\n",
     "        actual_major_minor = get_major_minor(sklearn.__version__)\n",
     "        self.assertEqual(actual_major_minor, expected_major_minor, \"incorrect version\")\n",
     "\n",
@@ -160,7 +176,7 @@
     "class TestMatplotlib(unittest.TestCase):\n",
     "\n",
     "    def test_version(self):\n",
-    "        expected_major_minor = '3.9'\n",
+    "        expected_major_minor = get_expected_version('Matplotlib')\n",
     "        actual_major_minor = get_major_minor(matplotlib.__version__)\n",
     "        self.assertEqual(actual_major_minor, expected_major_minor, \"incorrect version\")\n",
     "\n",
@@ -171,7 +187,7 @@
     "class TestKafkaPython(unittest.TestCase):\n",
     "\n",
     "    def test_version(self):\n",
-    "        expected_major_minor = '2.2'\n",
+    "        expected_major_minor = get_expected_version('Kafka-Python-ng')\n",
     "        actual_major_minor = get_major_minor(kafka.__version__)\n",
     "        self.assertEqual(actual_major_minor, expected_major_minor, \"incorrect version\")\n",
     "\n",
@@ -193,7 +209,7 @@
     "class TestBoto3(unittest.TestCase):\n",
     "\n",
     "    def test_version(self):\n",
-    "        expected_major_minor = '1.35'\n",
+    "        expected_major_minor = get_expected_version('Boto3')\n",
     "        actual_major_minor = get_major_minor(boto3.__version__)\n",
     "        self.assertEqual(actual_major_minor, expected_major_minor, \"incorrect version\")\n",
     "\n",
@@ -212,6 +228,7 @@
     "\n",
     "        self.assertEqual(boto3.DEFAULT_SESSION, session)\n",
     "\n",
+    "expected_versions = load_expected_versions()\n",
     "unittest.main(argv=[''], verbosity=2, exit=False)"
    ]
   }
@@ -223,5 +240,5 @@
   "orig_nbformat": 4
  },
  "nbformat": 4,
- "nbformat_minor": 2
+ "nbformat_minor": 5
 }
diff --git a/jupyter/intel/ml/ubi9-python-3.11/kustomize/base/statefulset.yaml b/jupyter/intel/ml/ubi9-python-3.11/kustomize/base/statefulset.yaml
@@ -36,17 +36,19 @@ spec:
           livenessProbe:
             tcpSocket:
               port: notebook-port
-            initialDelaySeconds: 5
-            periodSeconds: 5
+            initialDelaySeconds: 15
+            periodSeconds: 10
+            timeoutSeconds: 5
             successThreshold: 1
             failureThreshold: 3
           readinessProbe:
             httpGet:
               path: /notebook/opendatahub/jovyan/api
               port: notebook-port
               scheme: HTTP
-            initialDelaySeconds: 10
-            periodSeconds: 5
+            initialDelaySeconds: 15
+            periodSeconds: 10
+            timeoutSeconds: 5
             successThreshold: 1
             failureThreshold: 3
           resources:

diff --git a/jupyter/intel/ml/ubi9-python-3.11/test/test_notebook.ipynb b/jupyter/intel/ml/ubi9-python-3.11/test/test_notebook.ipynb
@@ -3,11 +3,15 @@
   {
    "cell_type": "code",
    "execution_count": null,
+   "id": "bac7ee1b-65c4-4515-8006-7ba01e843906",
    "metadata": {
     "tags": []
    },
    "outputs": [],
    "source": [
+    "from pathlib import Path\n",
+    "import json\n",
+    "import re\n",
     "import unittest\n",
     "from unittest import mock\n",
     "from platform import python_version\n",
@@ -27,7 +31,6 @@
     "from kafka.producer.buffer import SimpleBufferPool\n",
     "from kafka import KafkaConsumer\n",
     "from kafka.errors import KafkaConfigurationError\n",
-    "import boto3\n",
     "import kfp_tekton\n",
     "import kfp\n",
     "from kfp import LocalClient, run_pipeline_func_locally\n",
@@ -37,15 +40,29 @@
     "def get_major_minor(s):\n",
     "    return '.'.join(s.split('.')[:2])\n",
     "\n",
+    "def load_expected_versions() -> dict:\n",
+    "    lock_file = Path('./expected_versions.json')\n",
+    "    data = {}\n",
+    "\n",
+    "    with open(lock_file, 'r') as file:\n",
+    "        data = json.load(file)\n",
+    "\n",
+    "    return data    \n",
+    "\n",
+    "def get_expected_version(dependency_name: str) -> str:\n",
+    "    raw_value = expected_versions.get(dependency_name)\n",
+    "    raw_version = re.sub(r'^\\D+', '', raw_value)\n",
+    "    return get_major_minor(raw_version)\n",
+    "    \n",
     "class TestPythonVersion(unittest.TestCase):\n",
     "    def test_version(self):\n",
-    "        expected_major_minor = '3.11'\n",
+    "        expected_major_minor = get_expected_version('Python')\n",
     "        actual_major_minor = get_major_minor(python_version())\n",
     "        self.assertEqual(actual_major_minor, expected_major_minor, \"incorrect version\")\n",
     "\n",
     "class TestModin(unittest.TestCase):\n",
     "    def test_version(self):\n",
-    "        expected_major_minor = '0.24'\n",
+    "        expected_major_minor = get_expected_version('Modin')\n",
     "        actual_major_minor = get_major_minor(pdm.__version__)\n",
     "        self.assertEqual(actual_major_minor, expected_major_minor, \"incorrect version\")\n",
     "\n",
@@ -61,7 +78,7 @@
     "\n",
     "class TestPandas(unittest.TestCase):\n",
     "    def test_version(self):\n",
-    "        expected_major_minor = '2.1'\n",
+    "        expected_major_minor = get_expected_version('Pandas')\n",
     "        actual_major_minor = get_major_minor(pd.__version__)\n",
     "        self.assertEqual(actual_major_minor, expected_major_minor, \"incorrect version\")\n",
     "\n",
@@ -116,7 +133,7 @@
     "\n",
     "class TestNumpy(unittest.TestCase):\n",
     "    def test_version(self):\n",
-    "        expected_major_minor = '1.24'\n",
+    "        expected_major_minor = get_expected_version('Numpy')\n",
     "        actual_major_minor = get_major_minor(np.__version__)\n",
     "        self.assertEqual(actual_major_minor, expected_major_minor, \"incorrect version\")\n",
     "\n",
@@ -141,7 +158,7 @@
     "\n",
     "class TestScipy(unittest.TestCase):\n",
     "    def test_version(self):\n",
-    "        expected_major_minor = '1.11'\n",
+    "        expected_major_minor = get_expected_version('Scipy')\n",
     "        actual_major_minor = get_major_minor(scipy.__version__)\n",
     "        self.assertEqual(actual_major_minor, expected_major_minor, \"incorrect version\")\n",
     "\n",
@@ -158,7 +175,7 @@
     "\n",
     "class TestSKLearn(unittest.TestCase):\n",
     "    def test_version(self):\n",
-    "        expected_major_minor = '1.3'\n",
+    "        expected_major_minor = get_expected_version('Scikit-learn')\n",
     "        actual_major_minor = get_major_minor(sklearn.__version__)\n",
     "        self.assertEqual(actual_major_minor, expected_major_minor, \"incorrect version\")\n",
     "\n",
@@ -182,7 +199,7 @@
     "class TestMatplotlib(unittest.TestCase):\n",
     "\n",
     "    def test_version(self):\n",
-    "        expected_major_minor = '3.6'\n",
+    "        expected_major_minor = get_expected_version('Matplotlib')\n",
     "        actual_major_minor = get_major_minor(matplotlib.__version__)\n",
     "        self.assertEqual(actual_major_minor, expected_major_minor, \"incorrect version\")\n",
     "\n",
@@ -193,7 +210,7 @@
     "class TestKafkaPython(unittest.TestCase):\n",
     "\n",
     "    def test_version(self):\n",
-    "        expected_major_minor = '2.0'\n",
+    "        expected_major_minor = get_expected_version('Kafka-Python')\n",
     "        actual_major_minor = get_major_minor(kafka.__version__)\n",
     "        self.assertEqual(actual_major_minor, expected_major_minor, \"incorrect version\")\n",
     "\n",
@@ -215,19 +232,16 @@
     "class TestKFPTekton(unittest.TestCase):\n",
     "\n",
     "    def test_version(self):\n",
-    "        expected_major_minor = '1.6.0'\n",
+    "        unsupported_version = '1.6'\n",
     "\n",
-    "        self.assertLess(kfp_tekton.__version__, expected_major_minor)\n",
+    "        expected_major_minor = get_expected_version('KFP-Tekton')\n",
     "\n",
+    "        self.assertLess(expected_major_minor, unsupported_version)\n",
+    "        self.assertLess(kfp_tekton.__version__, unsupported_version)\n",
+    "\n",
+    "expected_versions = load_expected_versions()\n",
     "unittest.main(argv=[''], verbosity=2, exit=False)"
    ]
-  },
-  {
-   "cell_type": "code",
-   "execution_count": null,
-   "metadata": {},
-   "outputs": [],
-   "source": []
   }
  ],
  "metadata": {
@@ -250,5 +264,5 @@
   }
  },
  "nbformat": 4,
- "nbformat_minor": 4
+ "nbformat_minor": 5
 }