Merge branch 'main' into feature/maas-model-ref-update

Author: dbasunag

pytest.ini

@@ -10,10 +10,11 @@ markers =
     parallel: marks tests that can run in parallel along with pytest-xdist
 
     # CI
-    smoke: Mark tests as smoke tests; covers core functionality of the product. Aims to ensure that the build is stable enough for further testing.
+    smoke: Mark tests as smoke tests; very high critical priority tests. Covers core functionality of the product. Aims to ensure that the build is stable enough for further testing.
     sanity: Mark tests as sanity tests. Aims to verify that specific functionality is working as expected.
-    tier1: Mark tests as tier1. Aims to cover frequently used functionality of the product and basic user flows.
-    tier2: Mark tests as tier2. Aims to cover more advanced functionality of the product.
+    tier1: Mark tests as tier1. High-priority tests.
+    tier2: Mark tests as tier2. Medium/low-priority positive tests.
+    tier3: Mark tests as tier3. Negative and destructive tests.
     slow: Mark tests which take more than 10 minutes as slow tests.
     pre_upgrade: Mark tests which should be run before upgrading the product.
     post_upgrade: Mark tests which should be run after upgrading the product.

tests/conftest.py

@@ -676,6 +676,12 @@ def cluster_sanity_scope_session(
     dsc_resource: DataScienceCluster,
     junitxml_plugin: Callable[[str, object], None],
 ) -> None:
+    # Skip cluster sanity check when running tests that have cluster_health or operator_health markers
+    selected_markers = {mark.name for item in request.session.items for mark in item.iter_markers()}
+    if {"cluster_health", "operator_health"} & selected_markers:
+        LOGGER.info("Skipping cluster sanity check because selected tests include cluster/operator health")
+        return
+
     verify_cluster_sanity(
         request=request,
         nodes=nodes,

tests/model_registry/conftest.py

@@ -4,6 +4,7 @@
 from typing import Any
 
 import pytest
+import yaml
 from kubernetes.dynamic import DynamicClient
 from ocp_resources.config_map import ConfigMap
 from ocp_resources.data_science_cluster import DataScienceCluster
@@ -25,18 +26,26 @@
 from tests.model_registry.constants import (
     DB_BASE_RESOURCES_NAME,
     DB_RESOURCE_NAME,
+    DEFAULT_CUSTOM_MODEL_CATALOG,
     KUBERBACPROXY_STR,
     MR_INSTANCE_BASE_NAME,
     MR_INSTANCE_NAME,
     MR_OPERATOR_NAME,
 )
+from tests.model_registry.mcp_servers.constants import (
+    MCP_CATALOG_API_PATH,
+    MCP_CATALOG_SOURCE,
+    MCP_SERVERS_YAML,
+)
 from tests.model_registry.utils import (
     generate_namespace_name,
     get_byoidc_user_credentials,
     get_model_registry_metadata_resources,
     get_model_registry_objects,
     get_rest_headers,
     wait_for_default_resource_cleanedup,
+    wait_for_mcp_catalog_api,
+    wait_for_model_catalog_pod_ready_after_deletion,
 )
 from utilities.constants import DscComponents, Labels
 from utilities.general import (
@@ -466,3 +475,54 @@ def model_catalog_routes(admin_client: DynamicClient, model_registry_namespace:
     return list(
         Route.get(namespace=model_registry_namespace, label_selector="component=model-catalog", client=admin_client)
     )
+
+
+@pytest.fixture(scope="class")
+def mcp_catalog_rest_urls(model_registry_namespace: str, model_catalog_routes: list[Route]) -> list[str]:
+    """Build MCP catalog REST URL from existing model catalog routes."""
+    assert model_catalog_routes, f"Model catalog routes do not exist in {model_registry_namespace}"
+    return [f"https://{route.instance.spec.host}:443{MCP_CATALOG_API_PATH}" for route in model_catalog_routes]
+
+
+@pytest.fixture(scope="class")
+def mcp_servers_configmap_patch(
+    admin_client: DynamicClient,
+    model_registry_namespace: str,
+    mcp_catalog_rest_urls: list[str],
+    model_registry_rest_headers: dict[str, str],
+) -> Generator[None]:
+    """
+    Class-scoped fixture that patches the model-catalog-sources ConfigMap
+
+    Sets two keys in the ConfigMap data:
+    - sources.yaml: catalog source definition pointing to the MCP servers YAML
+    - mcp-servers.yaml: the actual MCP server definitions
+    """
+    catalog_config_map = ConfigMap(
+        name=DEFAULT_CUSTOM_MODEL_CATALOG,
+        client=admin_client,
+        namespace=model_registry_namespace,
+    )
+
+    current_data = yaml.safe_load(catalog_config_map.instance.data.get("sources.yaml", "{}") or "{}")
+    if "mcp_catalogs" not in current_data:
+        current_data["mcp_catalogs"] = []
+    current_data["mcp_catalogs"].append(MCP_CATALOG_SOURCE)
+
+    patches = {
+        "data": {
+            "sources.yaml": yaml.dump(current_data, default_flow_style=False),
+            "mcp-servers.yaml": MCP_SERVERS_YAML,
+        }
+    }
+
+    with ResourceEditor(patches={catalog_config_map: patches}):
+        wait_for_model_catalog_pod_ready_after_deletion(
+            client=admin_client, model_registry_namespace=model_registry_namespace
+        )
+        wait_for_mcp_catalog_api(url=mcp_catalog_rest_urls[0], headers=model_registry_rest_headers)
+        yield
+
+    wait_for_model_catalog_pod_ready_after_deletion(
+        client=admin_client, model_registry_namespace=model_registry_namespace
+    )

tests/model_registry/image_validation/test_verify_rhoai_images.py

@@ -19,7 +19,7 @@
 from utilities.constants import Labels
 
 LOGGER = get_logger(name=__name__)
-pytestmark = [pytest.mark.downstream_only, pytest.mark.skip_must_gather, pytest.mark.smoke]
+pytestmark = [pytest.mark.downstream_only, pytest.mark.skip_must_gather]
 
 
 class TestAIHubResourcesImages:
@@ -28,15 +28,15 @@ class TestAIHubResourcesImages:
         [
             pytest.param(
                 {"namespace": py_config["model_registry_namespace"], "label_selector": "component=model-catalog"},
-                marks=pytest.mark.smoke,
+                marks=pytest.mark.tier1,
                 id="test_model_catalog_pods_images",
             ),
             pytest.param(
                 {
                     "namespace": py_config["applications_namespace"],
                     "label_selector": f"{Labels.OpenDataHubIo.NAME}={MR_OPERATOR_NAME}",
                 },
-                marks=pytest.mark.smoke,
+                marks=pytest.mark.tier1,
                 id="test_model_registry_operator_pods_images",
             ),
         ],
@@ -54,7 +54,7 @@ def test_verify_pod_images(
 @pytest.mark.parametrize(
     "model_registry_metadata_db_resources, model_registry_instance, model_registry_instance_pods_by_label",
     [
-        pytest.param({}, {}, {"label_selectors": [f"app={MR_INSTANCE_NAME}"]}),
+        pytest.param({}, {}, {"label_selectors": [f"app={MR_INSTANCE_NAME}"]}, marks=pytest.mark.smoke),
         pytest.param(
             {"db_name": "default"},
             {"db_name": "default"},
@@ -64,6 +64,7 @@ def test_verify_pod_images(
                     f"app.kubernetes.io/name={MR_POSTGRES_DEPLOYMENT_NAME_STR}",
                 ]
             },
+            marks=pytest.mark.tier2,
         ),
     ],
     indirect=True,

tests/model_registry/mcp_servers/conftest.py

@@ -1,57 +1,29 @@
 from collections.abc import Generator
 
 import pytest
-import requests
 import yaml
 from kubernetes.dynamic import DynamicClient
-from kubernetes.dynamic.exceptions import ResourceNotFoundError
 from ocp_resources.config_map import ConfigMap
 from ocp_resources.resource import ResourceEditor
-from ocp_resources.route import Route
 from simple_logger.logger import get_logger
-from timeout_sampler import retry
 
 from tests.model_registry.constants import DEFAULT_CUSTOM_MODEL_CATALOG
 from tests.model_registry.mcp_servers.constants import (
-    MCP_CATALOG_API_PATH,
     MCP_CATALOG_INVALID_SOURCE,
     MCP_CATALOG_SOURCE,
     MCP_CATALOG_SOURCE2,
     MCP_SERVERS_YAML,
     MCP_SERVERS_YAML2,
 )
 from tests.model_registry.utils import (
-    TransientUnauthorizedError,
-    execute_get_call,
     execute_get_command,
+    wait_for_mcp_catalog_api,
     wait_for_model_catalog_pod_ready_after_deletion,
 )
 
 LOGGER = get_logger(name=__name__)
 
 
-@pytest.fixture(scope="class")
-def mcp_catalog_rest_urls(model_registry_namespace: str, model_catalog_routes: list[Route]) -> list[str]:
-    """Build MCP catalog REST URL from existing model catalog routes."""
-    assert model_catalog_routes, f"Model catalog routes do not exist in {model_registry_namespace}"
-    return [f"https://{route.instance.spec.host}:443{MCP_CATALOG_API_PATH}" for route in model_catalog_routes]
-
-
-@retry(
-    wait_timeout=90,
-    sleep=5,
-    exceptions_dict={ResourceNotFoundError: [], TransientUnauthorizedError: []},
-)
-def wait_for_mcp_catalog_api(url: str, headers: dict[str, str]) -> requests.Response:
-    """Wait for MCP catalog API to be ready and returning MCP server data."""
-    LOGGER.info(f"Waiting for MCP catalog API at {url}mcp_servers")
-    response = execute_get_call(url=f"{url}mcp_servers", headers=headers)
-    data = response.json()
-    if not data.get("items"):
-        raise ResourceNotFoundError("MCP catalog API returned empty items, catalog data not yet loaded")
-    return response
-
-
 @pytest.fixture(scope="class")
 def mcp_servers_response(
     mcp_catalog_rest_urls: list[str],
@@ -64,50 +36,6 @@ def mcp_servers_response(
     )
 
 
-@pytest.fixture(scope="class")
-def mcp_servers_configmap_patch(
-    admin_client: DynamicClient,
-    model_registry_namespace: str,
-    mcp_catalog_rest_urls: list[str],
-    model_registry_rest_headers: dict[str, str],
-) -> Generator[None]:
-    """
-    Class-scoped fixture that patches the model-catalog-sources ConfigMap
-
-    Sets two keys in the ConfigMap data:
-    - sources.yaml: catalog source definition pointing to the MCP servers YAML
-    - mcp-servers.yaml: the actual MCP server definitions
-    """
-    catalog_config_map = ConfigMap(
-        name=DEFAULT_CUSTOM_MODEL_CATALOG,
-        client=admin_client,
-        namespace=model_registry_namespace,
-    )
-
-    current_data = yaml.safe_load(catalog_config_map.instance.data.get("sources.yaml", "{}") or "{}")
-    if "mcp_catalogs" not in current_data:
-        current_data["mcp_catalogs"] = []
-    current_data["mcp_catalogs"].append(MCP_CATALOG_SOURCE)
-
-    patches = {
-        "data": {
-            "sources.yaml": yaml.dump(current_data, default_flow_style=False),
-            "mcp-servers.yaml": MCP_SERVERS_YAML,
-        }
-    }
-
-    with ResourceEditor(patches={catalog_config_map: patches}):
-        wait_for_model_catalog_pod_ready_after_deletion(
-            client=admin_client, model_registry_namespace=model_registry_namespace
-        )
-        wait_for_mcp_catalog_api(url=mcp_catalog_rest_urls[0], headers=model_registry_rest_headers)
-        yield
-
-    wait_for_model_catalog_pod_ready_after_deletion(
-        client=admin_client, model_registry_namespace=model_registry_namespace
-    )
-
-
 @pytest.fixture(scope="class")
 def mcp_multi_source_configmap_patch(
     admin_client: DynamicClient,

tests/model_registry/mcp_servers/test_data_integrity.py

@@ -6,7 +6,6 @@
 from tests.model_registry.mcp_servers.constants import (
     EXPECTED_MCP_SERVER_NAMES,
     EXPECTED_MCP_SERVER_TIMESTAMPS,
-    EXPECTED_MCP_SERVER_TOOL_COUNTS,
     EXPECTED_MCP_SERVER_TOOLS,
 )
 from tests.model_registry.utils import execute_get_command
@@ -30,9 +29,6 @@ def test_mcp_servers_loaded(
             assert server["createTimeSinceEpoch"] == expected["createTimeSinceEpoch"]
             assert server["lastUpdateTimeSinceEpoch"] == expected["lastUpdateTimeSinceEpoch"]
 
-    @pytest.mark.xfail(
-        reason="RHOAIENG-51765: Tool name returned as {server}@{version}:{tool} instead of YAML-defined name"
-    )
     def test_mcp_server_tools_loaded(
         self: Self,
         mcp_catalog_rest_urls: list[str],
@@ -50,17 +46,3 @@ def test_mcp_server_tools_loaded(
             assert server["toolCount"] == len(expected_tool_names)
             actual_tool_names = [tool["name"] for tool in server["tools"]]
             assert sorted(actual_tool_names) == sorted(expected_tool_names)
-
-    @pytest.mark.xfail(reason="RHOAIENG-51764: toolCount is 0 when not passing includeTools=true")
-    def test_mcp_server_tool_count_without_include(
-        self: Self,
-        mcp_servers_response: dict[str, Any],
-    ):
-        """Verify that toolCount reflects actual tools even when tools are not included."""
-        for server in mcp_servers_response.get("items", []):
-            name = server["name"]
-            expected_count = EXPECTED_MCP_SERVER_TOOL_COUNTS[name]
-            actual_count = server.get("toolCount", 0)
-            assert actual_count == expected_count, (
-                f"Server '{name}': expected toolCount {expected_count}, got {actual_count}"
-            )