Merge branch 'main' into hf_negative

Author: dbasunag

tests/model_registry/model_catalog/conftest.py

@@ -44,7 +44,7 @@ def enabled_model_catalog_config_map(
     admin_client: DynamicClient,
     model_registry_namespace: str,
     current_client_token: str,
-) -> ConfigMap:
+) -> Generator[ConfigMap, None, None]:
     """
     Enable all catalogs in the default model catalog configmap
     """

tests/model_registry/model_catalog/metadata/conftest.py

@@ -0,0 +1,52 @@
+import pytest
+import yaml
+from typing import Generator
+
+from kubernetes.dynamic import DynamicClient
+from ocp_resources.config_map import ConfigMap
+from ocp_resources.resource import ResourceEditor
+
+from tests.model_registry.constants import DEFAULT_CUSTOM_MODEL_CATALOG
+from tests.model_registry.utils import is_model_catalog_ready, wait_for_model_catalog_api
+
+
+@pytest.fixture()
+def disabled_catalog_source(
+    request: pytest.FixtureRequest,
+    admin_client: DynamicClient,
+    model_registry_namespace: str,
+    model_catalog_rest_url: list[str],
+    model_registry_rest_headers: dict[str, str],
+) -> Generator[str, None, None]:
+    """
+    Disables an existing catalog source for testing.
+
+    Yields:
+        str: The catalog ID of the disabled catalog.
+    """
+    sources_cm = ConfigMap(name=DEFAULT_CUSTOM_MODEL_CATALOG, client=admin_client, namespace=model_registry_namespace)
+    current_data = yaml.safe_load(sources_cm.instance.data["sources.yaml"])
+
+    # Get catalog_id from parameter (required)
+    catalog_id = request.param
+    assert catalog_id, "catalog_id parameter is required for disabled_catalog_source fixture"
+
+    # Find catalog by ID
+    catalog_to_disable = next(
+        (catalog for catalog in current_data.get("catalogs", []) if catalog["id"] == catalog_id), None
+    )
+    assert catalog_to_disable is not None, f"Catalog '{catalog_id}' not found in ConfigMap"
+
+    # Disable the catalog
+    catalog_to_disable["enabled"] = False
+
+    patches = {"data": {"sources.yaml": yaml.dump(current_data, default_flow_style=False)}}
+
+    with ResourceEditor(patches={sources_cm: patches}):
+        is_model_catalog_ready(client=admin_client, model_registry_namespace=model_registry_namespace)
+        wait_for_model_catalog_api(url=model_catalog_rest_url[0], headers=model_registry_rest_headers)
+
+        yield catalog_id
+
+    is_model_catalog_ready(client=admin_client, model_registry_namespace=model_registry_namespace)
+    wait_for_model_catalog_api(url=model_catalog_rest_url[0], headers=model_registry_rest_headers)

tests/model_registry/model_catalog/metadata/test_sources_endpoint.py

@@ -0,0 +1,108 @@
+import pytest
+
+from ocp_resources.config_map import ConfigMap
+from simple_logger.logger import get_logger
+
+from tests.model_registry.utils import execute_get_command
+from tests.model_registry.model_catalog.metadata.utils import validate_source_status
+
+pytestmark = [pytest.mark.usefixtures("updated_dsc_component_state_scope_session", "model_registry_namespace")]
+
+LOGGER = get_logger(name=__name__)
+
+
+class TestSourcesEndpoint:
+    """Test class for the model catalog sources endpoint."""
+
+    @pytest.mark.smoke
+    def test_available_source_status(
+        self,
+        enabled_model_catalog_config_map: ConfigMap,
+        model_catalog_rest_url: list[str],
+        model_registry_rest_headers: dict[str, str],
+    ):
+        """
+        RHOAIENG-41849: Test that the sources endpoint returns no error for available sources.
+        """
+        response = execute_get_command(url=f"{model_catalog_rest_url[0]}sources", headers=model_registry_rest_headers)
+        items = response.get("items", [])
+        assert items, "Sources not found"
+        for item in items:
+            validate_source_status(catalog=item, expected_status="available")
+            error_value = item["error"]
+            assert error_value is None or error_value == "", (
+                f"Source '{item.get('id')}' should not have error, got: {error_value}"
+            )
+
+            LOGGER.info(
+                f"Available catalog verified - ID: {item.get('id')}, Status: {item.get('status')}, Error: {error_value}"
+            )
+
+    @pytest.mark.parametrize("disabled_catalog_source", ["redhat_ai_models"], indirect=True)
+    def test_disabled_source_status(
+        self,
+        enabled_model_catalog_config_map: ConfigMap,
+        disabled_catalog_source: str,
+        model_catalog_rest_url: list[str],
+        model_registry_rest_headers: dict[str, str],
+    ):
+        """
+        RHOAIENG-41849:
+        This test disables an existing catalog and verifies:
+        - status field is "disabled"
+        - error field is null or empty
+        """
+        catalog_id = disabled_catalog_source
+
+        response = execute_get_command(url=f"{model_catalog_rest_url[0]}sources", headers=model_registry_rest_headers)
+        items = response.get("items", [])
+
+        # Find the disabled catalog
+        disabled_catalog = next((item for item in items if item.get("id") == catalog_id), None)
+        assert disabled_catalog is not None, f"Disabled catalog '{catalog_id}' not found in sources"
+
+        # Validate status and error fields
+        validate_source_status(catalog=disabled_catalog, expected_status="disabled")
+        error_value = disabled_catalog["error"]
+        assert error_value is None or error_value == "", (
+            f"Source '{disabled_catalog.get('id')}' should not have error, got: {error_value}"
+        )
+
+        LOGGER.info(
+            "Disabled catalog verified - "
+            f"ID: {disabled_catalog.get('id')}, "
+            f"Status: {disabled_catalog.get('status')}, "
+            f"Error: {error_value}"
+        )
+
+    @pytest.mark.parametrize("disabled_catalog_source", ["redhat_ai_models"], indirect=True)
+    @pytest.mark.sanity
+    def test_sources_endpoint_returns_all_sources_regardless_of_enabled_field(
+        self,
+        enabled_model_catalog_config_map: ConfigMap,
+        disabled_catalog_source: str,
+        model_catalog_rest_url: list[str],
+        model_registry_rest_headers: dict[str, str],
+    ):
+        """
+        RHOAIENG-41633: Test that sources endpoint returns ALL sources regardless of enabled field value.
+        """
+        response = execute_get_command(url=f"{model_catalog_rest_url[0]}sources", headers=model_registry_rest_headers)
+        items = response.get("items", [])
+
+        assert len(items) > 1, "Expected multiple sources to be returned"
+
+        # Verify we have at least one enabled source
+        enabled_sources = [item for item in items if item.get("status") == "available"]
+        assert enabled_sources, "Expected at least one enabled source"
+
+        # Verify we have at least one disabled source
+        disabled_sources = [item for item in items if item.get("status") == "disabled"]
+        assert disabled_sources, "Expected at least one disabled source"
+
+        assert len(enabled_sources) + len(disabled_sources) == len(items), "Expected all sources to be returned"
+
+        LOGGER.info(
+            f"Sources endpoint returned {len(items)} total sources: "
+            f"{len(enabled_sources)} enabled, {len(disabled_sources)} disabled"
+        )

tests/model_registry/model_catalog/metadata/utils.py

@@ -653,3 +653,19 @@ def verify_labels_match(expected_labels: List[Dict[str, Any]], api_labels: List[
                 break
 
         assert found, f"Expected label not found in API response: {expected_label}"
+
+
+def validate_source_status(catalog: dict[str, Any], expected_status: str) -> None:
+    """
+    Validate the status field of a catalog source.
+
+    Args:
+        catalog: The catalog source dictionary from API response
+        expected_status: The expected status value (e.g., "available", "disabled", "error")
+
+    Raises:
+        AssertionError: If status field does not match expected value
+    """
+    assert catalog.get("status") == expected_status, (
+        f"Source '{catalog.get('id')}' status should be '{expected_status}', got: {catalog.get('status')}"
+    )

tests/model_registry/model_registry/negative_tests/conftest.py

@@ -119,6 +119,7 @@ def model_registry_db_deployment_negative_test(
     model_registry_db_service_for_negative_tests: Service,
 ) -> Generator[Deployment, Any, Any]:
     with Deployment(
+        client=admin_client,
         name=DB_RESOURCES_NAME_NEGATIVE,
         namespace=model_registry_namespace_for_negative_tests.name,
         annotations={
@@ -168,9 +169,9 @@ def model_registry_db_instance_pod(admin_client: DynamicClient) -> Generator[Pod
 
 
 @pytest.fixture()
-def delete_mr_deployment() -> None:
+def delete_mr_deployment(admin_client: DynamicClient) -> None:
     """Delete the model registry deployment"""
     mr_deployment = Deployment(
-        name=MR_INSTANCE_NAME, namespace=py_config["model_registry_namespace"], ensure_exists=True
+        client=admin_client, name=MR_INSTANCE_NAME, namespace=py_config["model_registry_namespace"], ensure_exists=True
     )
     mr_deployment.delete(wait=True)

tests/model_registry/model_registry/negative_tests/test_model_registry_creation_negative.py

@@ -14,6 +14,7 @@
     DB_RESOURCE_NAME,
 )
 from kubernetes.dynamic.exceptions import ForbiddenError
+from kubernetes.dynamic import DynamicClient
 
 
 LOGGER = get_logger(name=__name__)
@@ -30,6 +31,7 @@ class TestModelRegistryCreationNegative:
     @pytest.mark.sanity
     def test_registering_model_negative(
         self: Self,
+        admin_client: DynamicClient,
         current_client_token: str,
         model_registry_namespace_for_negative_tests: Namespace,
         updated_dsc_component_state_scope_session: DataScienceCluster,
@@ -50,6 +52,7 @@ def test_registering_model_negative(
             match=f"namespace must be {py_config['model_registry_namespace']}",
         ):
             with ModelRegistry(
+                client=admin_client,
                 name=MR_INSTANCE_NAME,
                 namespace=model_registry_namespace_for_negative_tests.name,
                 label={

tests/model_registry/model_registry/rbac/conftest.py

@@ -139,13 +139,16 @@ def created_role_binding_user(
 # RESOURCE FIXTURES PARMETRIZED
 # =============================================================================
 @pytest.fixture(scope="class")
-def db_secret_parametrized(request: FixtureRequest, teardown_resources: bool) -> Generator[List[Secret], Any, Any]:
+def db_secret_parametrized(
+    request: FixtureRequest, admin_client: DynamicClient, teardown_resources: bool
+) -> Generator[List[Secret], Any, Any]:
     """Create DB Secret parametrized"""
     with ExitStack() as stack:
         secrets = [
             stack.enter_context(
                 Secret(
                     **param,
+                    client=admin_client,
                     teardown=teardown_resources,
                 )
             )
@@ -156,14 +159,15 @@ def db_secret_parametrized(request: FixtureRequest, teardown_resources: bool) ->
 
 @pytest.fixture(scope="class")
 def db_pvc_parametrized(
-    request: FixtureRequest, teardown_resources: bool
+    request: FixtureRequest, admin_client: DynamicClient, teardown_resources: bool
 ) -> Generator[List[PersistentVolumeClaim], Any, Any]:
     """Create DB PVC parametrized"""
     with ExitStack() as stack:
         pvc = [
             stack.enter_context(
                 PersistentVolumeClaim(
                     **param,
+                    client=admin_client,
                     teardown=teardown_resources,
                 )
             )
@@ -173,13 +177,16 @@ def db_pvc_parametrized(
 
 
 @pytest.fixture(scope="class")
-def db_service_parametrized(request: FixtureRequest, teardown_resources: bool) -> Generator[List[Service], Any, Any]:
+def db_service_parametrized(
+    request: FixtureRequest, admin_client: DynamicClient, teardown_resources: bool
+) -> Generator[List[Service], Any, Any]:
     """Create DB Service parametrized"""
     with ExitStack() as stack:
         services = [
             stack.enter_context(
                 Service(
                     **param,
+                    client=admin_client,
                     teardown=teardown_resources,
                 )
             )
@@ -190,14 +197,15 @@ def db_service_parametrized(request: FixtureRequest, teardown_resources: bool) -
 
 @pytest.fixture(scope="class")
 def db_deployment_parametrized(
-    request: FixtureRequest, teardown_resources: bool
+    request: FixtureRequest, admin_client: DynamicClient, teardown_resources: bool
 ) -> Generator[List[Deployment], Any, Any]:
     """Create DB Deployment parametrized"""
     with ExitStack() as stack:
         deployments = [
             stack.enter_context(
                 Deployment(
                     **param,
+                    client=admin_client,
                     teardown=teardown_resources,
                 )
             )
@@ -217,7 +225,7 @@ def model_registry_instance_parametrized(
     """Create Model Registry instance parametrized"""
     with ExitStack() as stack:
         model_registry_instances = []
-        mr_instances = [stack.enter_context(ModelRegistry(**param)) for param in request.param]
+        mr_instances = [stack.enter_context(ModelRegistry(**param, client=admin_client)) for param in request.param]
         for mr_instance in mr_instances:
             # Common parameters for both ModelRegistry classes
             mr_instance.wait_for_condition(condition="Available", status="True")

tests/model_registry/model_registry/rest_api/conftest.py

@@ -155,6 +155,7 @@ def deploy_secure_mysql_and_mr(
     if "sslRootCertificateConfigMap" in param:
         mysql["sslRootCertificateConfigMap"] = param["sslRootCertificateConfigMap"]
     with ModelRegistry(
+        client=admin_client,
         name=SECURE_MR_NAME,
         namespace=model_registry_namespace,
         label=get_mr_standard_labels(resource_name=SECURE_MR_NAME),
@@ -361,12 +362,15 @@ def skip_if_not_default_db(request):
 
 @pytest.fixture()
 def model_registry_default_postgres_deployment_match_label(
-    model_registry_namespace: str, model_registry_instance: list[ModelRegistry]
+    model_registry_namespace: str, admin_client: DynamicClient, model_registry_instance: list[ModelRegistry]
 ) -> dict[str, str]:
     """
     Returns the matchLabels from the default postgres deployment for filtering pods.
     """
     deployment = Deployment(
-        namespace=model_registry_namespace, name=f"{model_registry_instance[0].name}-postgres", ensure_exists=True
+        client=admin_client,
+        namespace=model_registry_namespace,
+        name=f"{model_registry_instance[0].name}-postgres",
+        ensure_exists=True,
     )
     return deployment.instance.spec.selector.matchLabels

tests/model_registry/model_registry/rest_api/test_model_registry_rest_api.py

@@ -168,9 +168,11 @@ def test_default_postgres_db_pod_log(
 
     def test_model_registry_validate_api_version(
         self: Self,
-        model_registry_instance,
+        admin_client: DynamicClient,
+        model_registry_instance: list[ModelRegistry],
     ):
         api_version = ModelRegistry(
+            client=admin_client,
             name=model_registry_instance[0].name,
             namespace=model_registry_instance[0].namespace,
             ensure_exists=True,
@@ -181,7 +183,7 @@ def test_model_registry_validate_api_version(
 
     def test_model_registry_validate_kuberbacproxy_enabled(
         self: Self,
-        model_registry_instance,
+        model_registry_instance: list[ModelRegistry],
     ):
         model_registry_instance_spec = model_registry_instance[0].instance.spec
         LOGGER.info(f"Validating that MR is using kubeRBAC proxy {model_registry_instance_spec}")

tests/model_registry/model_registry/rest_api/test_multiple_mr.py

@@ -42,11 +42,17 @@
 class TestModelRegistryMultipleInstances:
     @pytest.mark.smoke
     def test_validate_multiple_model_registry(
-        self: Self, model_registry_instance: list[ModelRegistry], model_registry_namespace: str
+        self: Self,
+        admin_client: DynamicClient,
+        model_registry_instance: list[ModelRegistry],
+        model_registry_namespace: str,
     ):
         for num in range(0, NUM_RESOURCES["num_resources"]):
             mr = ModelRegistry(
-                name=f"{MR_INSTANCE_BASE_NAME}{num}", namespace=model_registry_namespace, ensure_exists=True
+                client=admin_client,
+                name=f"{MR_INSTANCE_BASE_NAME}{num}",
+                namespace=model_registry_namespace,
+                ensure_exists=True,
             )
             LOGGER.info(f"{mr.name} found")