Merge branch 'main' into must-gather

Author: dbasunag

.pre-commit-config.yaml

@@ -35,7 +35,7 @@ repos:
       - id: detect-secrets
 
   - repo: https://github.com/astral-sh/ruff-pre-commit
-    rev: v0.11.8
+    rev: v0.11.9
     hooks:
       - id: ruff
       - id: ruff-format

tests/conftest.py

@@ -14,6 +14,7 @@
 from ocp_resources.pod import Pod
 from ocp_resources.secret import Secret
 from ocp_resources.service import Service
+from ocp_utilities.monitoring import Prometheus
 from pyhelper_utils.shell import run_command
 from pytest import FixtureRequest, Config
 from kubernetes.dynamic import DynamicClient
@@ -23,6 +24,7 @@
 from pytest_testconfig import config as py_config
 from simple_logger.logger import get_logger
 
+from utilities.certificates_utils import create_ca_bundle_file
 from utilities.data_science_cluster_utils import update_components_in_dsc
 from utilities.exceptions import ClusterLoginError
 from utilities.infra import (
@@ -520,3 +522,15 @@ def cluster_sanity_scope_session(
         dsci_resource=dsci_resource,
         junitxml_property=junitxml_plugin,
     )
+
+
+@pytest.fixture(scope="session")
+def prometheus(admin_client: DynamicClient) -> Prometheus:
+    return Prometheus(
+        client=admin_client,
+        resource_name="thanos-querier",
+        verify_ssl=create_ca_bundle_file(
+            client=admin_client, ca_type="openshift"
+        ),  # TODO: Verify SSL with appropriate certs
+        bearer_token=get_openshift_token(),
+    )

tests/model_explainability/trustyai_service/drift/test_drift.py

@@ -1,3 +1,5 @@
+from functools import partial
+
 import pytest
 
 from tests.model_explainability.trustyai_service.constants import DRIFT_BASE_DATA_PATH
@@ -11,6 +13,7 @@
 )
 from utilities.constants import MinIo
 from utilities.manifests.openvino import OPENVINO_KSERVE_INFERENCE_CONFIG
+from utilities.monitoring import validate_metrics_field, get_metric_label
 
 
 @pytest.mark.parametrize(
@@ -103,6 +106,21 @@ def test_drift_metric_schedule_meanshift(
             },
         )
 
+    def test_drift_metric_prometheus(
+        self,
+        admin_client,
+        model_namespace,
+        trustyai_service_with_pvc_storage,
+        gaussian_credit_model,
+        prometheus,
+    ):
+        validate_metrics_field(
+            prometheus=prometheus,
+            metrics_query=f'trustyai_{TrustyAIServiceMetrics.Drift.MEANSHIFT}{{namespace="{model_namespace.name}"}}',
+            expected_value=TrustyAIServiceMetrics.Drift.MEANSHIFT.upper(),
+            field_getter=partial(get_metric_label, label_name="metricName"),
+        )
+
     def test_drift_metric_delete(
         self,
         admin_client,

tests/model_explainability/trustyai_service/fairness/test_fairness.py

@@ -1,3 +1,4 @@
+from functools import partial
 from typing import Any
 
 import pytest
@@ -13,6 +14,7 @@
 )
 from utilities.constants import MinIo
 from utilities.manifests.openvino import OPENVINO_KSERVE_INFERENCE_CONFIG
+from utilities.monitoring import validate_metrics_field, get_metric_label
 
 BASE_DATA_PATH: str = "./tests/model_explainability/trustyai_service/fairness/model_data"
 IS_MALE_IDENTIFYING: str = "Is Male-Identifying?"
@@ -115,6 +117,21 @@ def test_fairness_metric_schedule_spd_with_pvc_storage(
             json_data=get_fairness_request_json_data(isvc=onnx_loan_model),
         )
 
+    def test_fairness_metric_prometheus(
+        self,
+        admin_client,
+        model_namespace,
+        trustyai_service_with_pvc_storage,
+        onnx_loan_model,
+        prometheus,
+    ):
+        validate_metrics_field(
+            prometheus=prometheus,
+            metrics_query=f'trustyai_{TrustyAIServiceMetrics.Fairness.SPD}{{namespace="{model_namespace.name}"}}',
+            expected_value=TrustyAIServiceMetrics.Fairness.SPD.upper(),
+            field_getter=partial(get_metric_label, label_name="metricName"),
+        )
+
     def test_fairness_metric_delete_with_pvc_storage(
         self, admin_client, current_client_token, trustyai_service_with_pvc_storage, onnx_loan_model
     ):

tests/model_serving/model_server/conftest.py

@@ -13,7 +13,6 @@
 from ocp_resources.service_account import ServiceAccount
 from ocp_resources.serving_runtime import ServingRuntime
 from ocp_resources.storage_class import StorageClass
-from ocp_utilities.monitoring import Prometheus
 from pytest_testconfig import config as py_config
 from simple_logger.logger import get_logger
 
@@ -32,7 +31,6 @@
 )
 from utilities.inference_utils import create_isvc
 from utilities.infra import (
-    get_openshift_token,
     s3_endpoint_secret,
     update_configmap_data,
 )
@@ -394,16 +392,6 @@ def http_s3_tensorflow_model_mesh_inference_service(
         yield isvc
 
 
-@pytest.fixture(scope="session")
-def prometheus(admin_client: DynamicClient) -> Prometheus:
-    return Prometheus(
-        client=admin_client,
-        resource_name="thanos-querier",
-        verify_ssl=False,
-        bearer_token=get_openshift_token(),
-    )
-
-
 @pytest.fixture(scope="class")
 def user_workload_monitoring_config_map(
     admin_client: DynamicClient, cluster_monitoring_config: ConfigMap

tests/model_serving/model_server/inference_service_configuration/conftest.py

@@ -4,9 +4,16 @@
 from kubernetes.dynamic import DynamicClient
 from ocp_resources.inference_service import InferenceService
 from ocp_resources.pod import Pod
+from ocp_resources.namespace import Namespace
+from ocp_resources.serving_runtime import ServingRuntime
+from utilities.inference_utils import create_isvc
+from utilities.constants import KServeDeploymentType
+
 
 from tests.model_serving.model_server.inference_service_configuration.constants import (
     ISVC_ENV_VARS,
+    UPDATED_PULL_SECRET,
+    ORIGINAL_PULL_SECRET,
 )
 from tests.model_serving.model_server.inference_service_configuration.utils import (
     update_inference_service,
@@ -65,3 +72,58 @@ def patched_isvc_replicas(
         wait_for_new_pods=request.param["wait-for-new-pods"],
     ):
         yield ovms_kserve_inference_service
+
+
+@pytest.fixture(scope="class")
+def model_car_raw_inference_service_with_pull_secret(
+    request: pytest.FixtureRequest,
+    unprivileged_client: DynamicClient,
+    unprivileged_model_namespace: Namespace,
+    serving_runtime_from_template: ServingRuntime,
+) -> Generator[InferenceService, Any, Any]:
+    with create_isvc(
+        client=unprivileged_client,
+        name="model-car-raw",
+        namespace=unprivileged_model_namespace.name,
+        runtime=serving_runtime_from_template.name,
+        storage_uri=request.param["storage-uri"],
+        model_format=serving_runtime_from_template.instance.spec.supportedModelFormats[0].name,
+        deployment_mode=KServeDeploymentType.RAW_DEPLOYMENT,
+        image_pull_secrets=[ORIGINAL_PULL_SECRET],
+        wait_for_predictor_pods=False,  # Until modelcar initContainer completed, other containers may have Error status
+    ) as isvc:
+        yield isvc
+
+
+@pytest.fixture(scope="class")
+def updated_isvc_pull_secret(
+    request: pytest.FixtureRequest,
+    unprivileged_client: DynamicClient,
+    model_car_raw_inference_service_with_pull_secret: InferenceService,
+) -> Generator[InferenceService, Any, Any]:
+    with update_inference_service(
+        client=unprivileged_client,
+        isvc=model_car_raw_inference_service_with_pull_secret,
+        isvc_updated_dict={"spec": {"predictor": {"imagePullSecrets": [{"name": UPDATED_PULL_SECRET}]}}},
+    ):
+        yield model_car_raw_inference_service_with_pull_secret
+
+
+@pytest.fixture(scope="class")
+def updated_isvc_remove_pull_secret(
+    request: pytest.FixtureRequest,
+    unprivileged_client: DynamicClient,
+    model_car_raw_inference_service_with_pull_secret: InferenceService,
+) -> Generator[InferenceService, Any, Any]:
+    with update_inference_service(
+        client=unprivileged_client,
+        isvc=model_car_raw_inference_service_with_pull_secret,
+        isvc_updated_dict={
+            "spec": {
+                "predictor": {
+                    "imagePullSecrets": None  # Explicitly remove the field
+                }
+            }
+        },
+    ):
+        yield model_car_raw_inference_service_with_pull_secret

tests/model_serving/model_server/inference_service_configuration/constants.py

@@ -10,3 +10,6 @@
     {"name": "TEST_ENV_VAR1", "value": "test_value1"},
     {"name": "TEST_ENV_VAR2", "value": "test_value2"},
 ]
+
+ORIGINAL_PULL_SECRET: str = "pull-secret-1"  # pragma: allowlist-secret
+UPDATED_PULL_SECRET: str = "updated-pull-secret"  # pragma: allowlist-secret

tests/model_serving/model_server/inference_service_configuration/test_isvc_pull_secret_updates.py

@@ -0,0 +1,42 @@
+import pytest
+
+from tests.model_serving.model_server.inference_service_configuration.utils import verify_pull_secret
+from tests.model_serving.model_server.inference_service_configuration.constants import (
+    ORIGINAL_PULL_SECRET,
+    UPDATED_PULL_SECRET,
+)
+from utilities.constants import ModelFormat, ModelName, RuntimeTemplates
+
+
+@pytest.mark.parametrize(
+    "unprivileged_model_namespace, serving_runtime_from_template, model_car_raw_inference_service_with_pull_secret",
+    [
+        pytest.param(
+            {"name": f"{ModelFormat.OPENVINO}-model-car"},
+            {
+                "name": f"{ModelName.MNIST}-runtime",
+                "template-name": RuntimeTemplates.OVMS_KSERVE,
+                "multi-model": False,
+            },
+            {
+                # Using mnist-8-1 model from OCI image
+                "storage-uri": "oci://quay.io/mwaykole/test@sha256:8a3217bcfa2cc5fa3d07496cff8b234acdf2c9725dd307dc0a80401f55e1a11c"  # noqa: E501
+            },
+        )
+    ],
+    indirect=True,
+)
+class TestISVCPullSecretUpdate:
+    @pytest.mark.smoke
+    def test_initial_pull_secret_set(self, model_car_raw_inference_service_with_pull_secret):
+        """Ensure initial pull secret is correctly set in the pod"""
+        verify_pull_secret(
+            isvc=model_car_raw_inference_service_with_pull_secret, pull_secret=ORIGINAL_PULL_SECRET, secret_exists=True
+        )
+
+    def test_update_pull_secret(self, updated_isvc_pull_secret):
+        """Update the pull secret and verify it is reflected in the new pod"""
+        verify_pull_secret(isvc=updated_isvc_pull_secret, pull_secret=UPDATED_PULL_SECRET, secret_exists=True)
+
+    def test_remove_pull_secret(self, updated_isvc_remove_pull_secret):
+        verify_pull_secret(isvc=updated_isvc_remove_pull_secret, pull_secret=UPDATED_PULL_SECRET, secret_exists=False)

tests/model_serving/model_server/inference_service_configuration/utils.py

@@ -108,3 +108,32 @@ def wait_for_new_running_inference_pods(
     except TimeoutError:
         LOGGER.error(f"Timeout waiting for pods {oring_pods_names} to be replaced")
         raise
+
+
+def verify_pull_secret(isvc: InferenceService, pull_secret: str, secret_exists: bool) -> None:
+    """
+    Verify that the ImagePullSecret in the InferenceService pods match the expected values.
+
+    Args:
+        isvc (InferenceService): InferenceService object.
+        pull_secret (str): Pull secret to verify
+        secret_exists (bool): False if the pull secret should not exist in the pod.
+
+    Raises:
+        AssertionError: If the imagePullSecrets do not match the expected presence or name.
+    """
+    pod = get_pods_by_isvc_label(
+        client=isvc.client,
+        isvc=isvc,
+    )[0]
+    image_pull_secrets = pod.instance.spec.imagePullSecrets or []
+
+    secrets = [s.name for s in image_pull_secrets]
+
+    if secret_exists:
+        assert secrets, "Expected imagePullSecrets to exist, but none were found."
+        assert pull_secret in secrets, f"Expected pull secret '{pull_secret}' not found in imagePullSecrets: {secrets}"
+    else:
+        assert pull_secret not in secrets, (
+            f"Did not expect pull secret '{pull_secret}', but found in imagePullSecrets: {secrets}"
+        )

tests/model_serving/model_server/metrics/test_model_metrics.py

@@ -14,7 +14,7 @@
 )
 from utilities.inference_utils import Inference
 from utilities.manifests.caikit_tgis import CAIKIT_TGIS_INFERENCE_CONFIG
-from utilities.monitoring import get_metrics_value, validate_metrics_value
+from utilities.monitoring import get_metrics_value, validate_metrics_field
 
 pytestmark = [
     pytest.mark.serverless,
@@ -57,7 +57,7 @@ def test_model_metrics_num_success_requests(self, s3_models_inference_service, p
             model_name=ModelFormat.CAIKIT,
             use_default_query=True,
         )
-        validate_metrics_value(
+        validate_metrics_field(
             prometheus=prometheus,
             metrics_query="tgi_request_success",
             expected_value="1",
@@ -78,7 +78,7 @@ def test_model_metrics_num_total_requests(self, s3_models_inference_service, pro
             iterations=total_runs,
             run_in_parallel=True,
         )
-        validate_metrics_value(
+        validate_metrics_field(
             prometheus=prometheus,
             metrics_query="tgi_request_count",
             expected_value=str(total_runs + 1),