Remove unused fixtures and other minor fixes

Author: adolfo-ab

tests/model_explainability/lm_eval/conftest.py

@@ -15,27 +15,15 @@
 from ocp_resources.resource import ResourceEditor
 from pytest_testconfig import py_config
 
+from tests.model_explainability.constants import MINIO_PORT, MINIO
+from tests.model_explainability.lm_eval.utils import MINIOADMIN
 from utilities.constants import Labels, Timeout, Annotations, Protocols
 
 VLLM_EMULATOR: str = "vllm-emulator"
 VLLM_EMULATOR_PORT: int = 8000
 LMEVALJOB_NAME: str = "lmeval-test-job"
 
 
-@pytest.fixture(scope="function")
-def lmevaljob_hf_pod(admin_client: DynamicClient, lmevaljob_hf: LMEvalJob) -> Generator[Pod, Any, Any]:
-    lmeval_pod = Pod(
-        client=admin_client,
-        namespace=lmevaljob_hf.namespace,
-        name=lmevaljob_hf.name,
-    )
-
-    # TODO: Check if we can rely on LMEvalJob instead of pod
-    lmeval_pod.wait(timeout=Timeout.TIMEOUT_2MIN)
-
-    yield lmeval_pod
-
-
 @pytest.fixture(scope="function")
 def lmevaljob_hf(
     admin_client: DynamicClient, model_namespace: Namespace, patched_trustyai_operator_configmap_allow_online: ConfigMap
@@ -108,22 +96,6 @@ def lmevaljob_local_offline(
         yield job
 
 
-@pytest.fixture(scope="function")
-def lmevaljob_vllm_emulator_pod(
-    admin_client: DynamicClient, lmevaljob_vllm_emulator: LMEvalJob
-) -> Generator[Pod, Any, Any]:
-    lmeval_pod = Pod(
-        client=admin_client,
-        namespace=lmevaljob_vllm_emulator.namespace,
-        name=lmevaljob_vllm_emulator.name,
-    )
-
-    # TODO: Check if we can rely on LMEvalJob instead of pod
-    lmeval_pod.wait(timeout=Timeout.TIMEOUT_2MIN)
-
-    yield lmeval_pod
-
-
 @pytest.fixture(scope="function")
 def lmevaljob_vllm_emulator(
     admin_client: DynamicClient,
@@ -316,12 +288,8 @@ def lmeval_minio_pvc(
 ) -> Generator[PersistentVolumeClaim, Any, Any]:
     with PersistentVolumeClaim(
         client=admin_client,
-        name="minio-pvc",
+        name=f"{MINIO}-pvc",
         namespace=model_namespace.name,
-        annotations={
-            "helm.sh/hook": "pre-install",
-            "helm.sh/hook-weight": "0",
-        },
         accessmodes=PersistentVolumeClaim.AccessMode.RWO,
         size="10Gi",
     ) as pvc:
@@ -334,28 +302,25 @@ def lmeval_minio_deployment(
 ) -> Generator[Deployment, Any, Any]:
     with Deployment(
         client=admin_client,
-        name="minio",
+        name=MINIO,
         namespace=model_namespace.name,
-        annotations={
-            "helm.sh/hook": "pre-install",
-            "helm.sh/hook-weight": "0",
-        },
         replicas=1,
-        selector={"matchLabels": {"app": "minio"}},
+        selector={"matchLabels": {"app": MINIO}},
         template={
-            "metadata": {"labels": {"app": "minio"}},
+            "metadata": {"labels": {"app": MINIO}},
             "spec": {
-                "volumes": [{"name": "minio-storage", "persistentVolumeClaim": {"claimName": "minio-pvc"}}],
+                "volumes": [{"name": "minio-storage", "persistentVolumeClaim": {"claimName": lmeval_minio_pvc.name}}],
                 "containers": [
                     {
-                        "name": "minio",
-                        "image": "quay.io/minio/minio:latest",
+                        "name": MINIO,
+                        "image": "quay.io/minio/minio"
+                        "@sha256:46b3009bf7041eefbd90bd0d2b38c6ddc24d20a35d609551a1802c558c1c958f",
                         "args": ["server", "/data", "--console-address", ":9001"],
                         "env": [
-                            {"name": "MINIO_ROOT_USER", "value": "minioadmin"},
-                            {"name": "MINIO_ROOT_PASSWORD", "value": "minioadmin"},
+                            {"name": "MINIO_ROOT_USER", "value": MINIOADMIN},
+                            {"name": "MINIO_ROOT_PASSWORD", "value": MINIOADMIN},
                         ],
-                        "ports": [{"containerPort": 9000}, {"containerPort": 9001}],
+                        "ports": [{"containerPort": MINIO_PORT}, {"containerPort": 9001}],
                         "volumeMounts": [{"name": "minio-storage", "mountPath": "/data"}],
                     }
                 ],
@@ -381,7 +346,8 @@ def lmeval_minio_copy_pod(
         init_containers=[
             {
                 "name": "copy-data",
-                "image": "quay.io/ruimvieira/lmeval-assets-flan-arceasy:latest",
+                "image": "quay.io/trustyai_testing/lmeval-assets-flan-arceasy"
+                "@sha256:11cc9c2f38ac9cc26c4fab1a01a8c02db81c8f4801b5d2b2b90f90f91b97ac98",
                 "command": ["/bin/sh", "-c"],
                 "args": ["cp -r /mnt/data /shared"],
                 "volumeMounts": [{"name": "shared-data", "mountPath": "/shared"}],
@@ -390,10 +356,10 @@ def lmeval_minio_copy_pod(
         containers=[
             {
                 "name": "minio-uploader",
-                "image": "quay.io/minio/mc:latest",
+                "image": "quay.io/minio/mc@sha256:470f5546b596e16c7816b9c3fa7a78ce4076bb73c2c73f7faeec0c8043923123",
                 "command": ["/bin/sh", "-c"],
                 "args": [
-                    "mc alias set myminio http://minio:9000 minioadmin minioadmin &&\n"
+                    f"mc alias set myminio http://{minio_service.name}:{MINIO_PORT} {MINIOADMIN} {MINIOADMIN} &&\n"
                     "mc mb --ignore-existing myminio/models &&\n"
                     "mc cp --recursive /shared/data/ myminio/models"
                 ],
@@ -411,6 +377,7 @@ def lmevaljob_s3_offline(
     admin_client: DynamicClient,
     model_namespace: Namespace,
     lmeval_minio_deployment: Deployment,
+    minio_service: Service,
     lmeval_minio_copy_pod: Pod,
     minio_data_connection: Secret,
 ) -> Generator[LMEvalJob, Any, Any]:

tests/model_explainability/lm_eval/test_lm_eval.py

@@ -1,11 +1,10 @@
 import pytest
-from ocp_resources.lm_eval_job import LMEvalJob
 
 from tests.model_explainability.constants import MINIO_PORT
-from tests.model_explainability.lm_eval.utils import verify_lmevaljob_running
-from utilities.constants import Timeout
+from tests.model_explainability.lm_eval.utils import verify_lmevaljob_running, wait_for_lmevaljob_state
 from utilities.general import get_s3_secret_dict
 
+LMEVALJOB_COMPLETE_STATE: str = "Complete"
 DATA_DICT: dict[str, str] = get_s3_secret_dict(
     aws_access_key="minioadmin",
     aws_secret_access_key="minioadmin",  # pragma: allowlist secret
@@ -25,9 +24,9 @@
     indirect=True,
 )
 @pytest.mark.smoke
-def test_lmeval_huggingface_model(admin_client, model_namespace, lmevaljob_hf_pod):
+def test_lmeval_huggingface_model(admin_client, model_namespace, lmevaljob_hf):
     """Basic test that verifies that LMEval can run successfully pulling a model from HuggingFace."""
-    lmevaljob_hf_pod.wait_for_status(status=lmevaljob_hf_pod.Status.SUCCEEDED, timeout=Timeout.TIMEOUT_10MIN)
+    wait_for_lmevaljob_state(lmevaljob=lmevaljob_hf, state=LMEVALJOB_COMPLETE_STATE)
 
 
 @pytest.mark.parametrize(
@@ -96,11 +95,9 @@ def test_lmeval_local_offline_unitxt_tasks_flan_20newsgroups(
     ],
     indirect=True,
 )
-def test_lmeval_vllm_emulator(admin_client, model_namespace, lmevaljob_vllm_emulator_pod):
+def test_lmeval_vllm_emulator(admin_client, model_namespace, lmevaljob_vllm_emulator):
     """Basic test that verifies LMEval works with vLLM using a vLLM emulator for more efficient evaluation"""
-    lmevaljob_vllm_emulator_pod.wait_for_status(
-        status=lmevaljob_vllm_emulator_pod.Status.SUCCEEDED, timeout=Timeout.TIMEOUT_10MIN
-    )
+    wait_for_lmevaljob_state(lmevaljob=lmevaljob_vllm_emulator, state=LMEVALJOB_COMPLETE_STATE)
 
 
 @pytest.mark.parametrize(
@@ -116,12 +113,7 @@ def test_lmeval_vllm_emulator(admin_client, model_namespace, lmevaljob_vllm_emul
 def test_lmeval_s3_storage(
     admin_client,
     model_namespace,
-    lmeval_minio_pvc,
-    lmeval_minio_deployment,
-    minio_service,
-    lmeval_minio_copy_pod,
-    minio_data_connection,
     lmevaljob_s3_offline,
 ):
     """Test to verify that LMEval works with a model stored in a S3 bucket"""
-    lmevaljob_s3_offline.wait_for_status(status=LMEvalJob.Status.COMPLETED, timeout=Timeout.TIMEOUT_15MIN)
+    wait_for_lmevaljob_state(lmevaljob=lmevaljob_s3_offline, state=LMEVALJOB_COMPLETE_STATE)

tests/model_explainability/lm_eval/utils.py

@@ -1,14 +1,15 @@
 from kubernetes.dynamic import DynamicClient
 from ocp_resources.lm_eval_job import LMEvalJob
 from ocp_resources.pod import Pod
-
+from timeout_sampler import TimeoutSampler
 
 from utilities.constants import Timeout
 from utilities.infra import check_pod_status_in_time
 from simple_logger.logger import get_logger
 
 
 LOGGER = get_logger(name=__name__)
+MINIOADMIN: str = "minioadmin"
 
 
 def verify_lmevaljob_running(client: DynamicClient, lmevaljob: LMEvalJob) -> None:
@@ -29,3 +30,20 @@ def verify_lmevaljob_running(client: DynamicClient, lmevaljob: LMEvalJob) -> Non
     lmevaljob_pod.wait_for_status(status=lmevaljob_pod.Status.RUNNING, timeout=Timeout.TIMEOUT_10MIN)
 
     check_pod_status_in_time(pod=lmevaljob_pod, status={lmevaljob_pod.Status.RUNNING, lmevaljob_pod.Status.SUCCEEDED})
+
+
+def wait_for_lmevaljob_state(
+    lmevaljob: LMEvalJob, state: str, wait_timeout: int = Timeout.TIMEOUT_15MIN, sleep: int = 5
+) -> None:
+    LOGGER.info(f"Checking LMEvalJob state for {lmevaljob.name} to be {state}")
+
+    sampler = TimeoutSampler(
+        wait_timeout=wait_timeout,
+        sleep=sleep,
+        func=lambda: lmevaljob.instance.status.state,
+    )
+
+    for sample in sampler:
+        if sample:
+            if sample == state:
+                return