Merge branch 'main' into clusterIP

Author: mwaykole

.github/workflows/push-container-on-comment.yml

@@ -14,8 +14,10 @@ jobs:
     if: contains(github.event.comment.body, '/build-push-pr-image')
     runs-on: ubuntu-latest
     steps:
-      - name: Checkout Repository
+      - name: Checkout pull request
         uses: actions/checkout@v4
+        with:
+          ref: refs/pull/${{ github.event.issue.number }}/head
       - name: Install uv
         uses: astral-sh/setup-uv@v6
 

.github/workflows/workflow-review.yml

@@ -1,3 +1,4 @@
+# https://stackoverflow.com/questions/67247752/how-to-use-secret-in-pull-request-review-similar-to-pull-request-target
 name: Dummy Workflow on review
 
 on:

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}"
+        )

utilities/inference_utils.py

@@ -534,6 +534,7 @@ def create_isvc(
     resources: dict[str, Any] | None = None,
     volumes: dict[str, Any] | None = None,
     volumes_mounts: dict[str, Any] | None = None,
+    image_pull_secrets: list[str] | None = None,
     model_version: str | None = None,
     wait_for_predictor_pods: bool = True,
     autoscaler_mode: str | None = None,
@@ -608,6 +609,8 @@ def create_isvc(
         predictor_dict["model"]["storage"] = {"key": storage_key, "path": storage_path}
     if model_service_account:
         predictor_dict["serviceAccountName"] = model_service_account
+    if image_pull_secrets:
+        predictor_dict["imagePullSecrets"] = [{"name": name} for name in image_pull_secrets]
 
     if min_replicas:
         predictor_dict["minReplicas"] = min_replicas