HF basic validation tests (#941)

* HF basic validation tests

* rename constant based on review comment

Author: dbasunag

tests/model_registry/model_catalog/conftest.py

@@ -128,6 +128,11 @@ def expected_catalog_values(request: pytest.FixtureRequest) -> dict[str, str]:
     return request.param
 
 
+@pytest.fixture(scope="class")
+def is_huggingface(request: pytest.FixtureRequest) -> dict[str, str]:
+    return request.param
+
+
 @pytest.fixture(scope="function")
 def update_configmap_data_add_model(
     request: pytest.FixtureRequest,
@@ -325,3 +330,27 @@ def labels_configmap_patch(admin_client: DynamicClient, model_registry_namespace
 
     with ResourceEditor(patches={sources_cm: patches}):
         yield patches
+
+
+@pytest.fixture()
+def skip_on_huggingface_source(is_huggingface: bool) -> None:
+    if is_huggingface:
+        pytest.skip(reason="Huggingface models does not support artifacts endpoints")
+
+
+@pytest.fixture()
+def updated_catalog_config_map_scope_function(
+    pytestconfig: pytest.Config,
+    request: pytest.FixtureRequest,
+    catalog_config_map: ConfigMap,
+    model_registry_namespace: str,
+    admin_client: DynamicClient,
+    model_catalog_rest_url: list[str],
+    model_registry_rest_headers: dict[str, str],
+) -> Generator[ConfigMap, None, None]:
+    patches = {"data": {"sources.yaml": request.param}}
+    with ResourceEditor(patches={catalog_config_map: 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_config_map
+    is_model_catalog_ready(client=admin_client, model_registry_namespace=model_registry_namespace)

tests/model_registry/model_catalog/constants.py

@@ -40,3 +40,20 @@
 MODEL_ARTIFACT_TYPE: str = "model-artifact"
 METRICS_ARTIFACT_TYPE: str = "metrics-artifact"
 PERFORMANCE_DATA_DIR: str = "/shared-benchmark-data"
+HF_SOURCE_ID: str = "huggingface_mixed"
+HF_MODEL_NAME: str = "ibm-granite/granite-speech-3.2-8b"
+HF_MODELS: dict[str, Any] = {
+    "mixed": ["ibm-granite/granite-4.0-h-1b", "microsoft/phi-2", "meta-llama/Llama-3.1-8B-Instruct"],
+    "granite": [
+        "ibm-granite/granite-4.0-h-small",
+        "ibm-granite/granite-4.0-micro",
+        "ibm-granite/granite-4.0-h-350m",
+        "ibm-granite/granite-4.0-micro-base",
+        "ibm-granite/granite-4.0-h-micro",
+    ],
+}
+EXPECTED_HF_CATALOG_VALUES: list[dict[str, str]] = [{"id": HF_SOURCE_ID, "model_name": HF_MODELS["mixed"][0]}]
+EXPECTED_MULTIPLE_HF_CATALOG_VALUES: list[dict[str, str]] = [
+    {"id": HF_SOURCE_ID, "model_name": HF_MODELS["mixed"][0]},
+    {"id": "huggingface_granite", "model_name": HF_MODELS["granite"][0]},
+]

tests/model_registry/model_catalog/test_custom_model_catalog.py

@@ -4,6 +4,8 @@
     SAMPLE_MODEL_NAME2,
     MULTIPLE_CUSTOM_CATALOG_VALUES,
     SAMPLE_MODEL_NAME3,
+    EXPECTED_HF_CATALOG_VALUES,
+    EXPECTED_MULTIPLE_HF_CATALOG_VALUES,
 )
 from tests.model_registry.constants import SAMPLE_MODEL_NAME1, CUSTOM_CATALOG_ID1
 from ocp_resources.config_map import ConfigMap
@@ -12,6 +14,7 @@
 from typing import Self
 from kubernetes.dynamic.exceptions import ResourceNotFoundError
 
+from tests.model_registry.model_catalog.utils import get_hf_catalog_str
 from tests.model_registry.utils import (
     execute_get_command,
     get_sample_yaml_str,
@@ -23,14 +26,33 @@
 
 
 @pytest.mark.parametrize(
-    "updated_catalog_config_map, expected_catalog_values",
+    "updated_catalog_config_map, expected_catalog_values, is_huggingface",
     [
+        pytest.param(
+            {
+                "sources_yaml": get_hf_catalog_str(ids=["mixed"]),
+            },
+            EXPECTED_HF_CATALOG_VALUES,
+            True,
+            id="test_HF_test_catalog",
+            marks=(pytest.mark.install),
+        ),
+        pytest.param(
+            {
+                "sources_yaml": get_hf_catalog_str(ids=["mixed", "granite"]),
+            },
+            EXPECTED_MULTIPLE_HF_CATALOG_VALUES,
+            True,
+            id="test_HF_test_catalog",
+            marks=(pytest.mark.install),
+        ),
         pytest.param(
             {
                 "sources_yaml": get_catalog_str(ids=[CUSTOM_CATALOG_ID1]),
                 "sample_yaml": {"sample-custom-catalog1.yaml": get_sample_yaml_str(models=[SAMPLE_MODEL_NAME1])},
             },
             EXPECTED_CUSTOM_CATALOG_VALUES,
+            False,
             id="test_file_test_catalog",
             marks=(pytest.mark.pre_upgrade, pytest.mark.post_upgrade, pytest.mark.install),
         ),
@@ -43,6 +65,7 @@
                 },
             },
             MULTIPLE_CUSTOM_CATALOG_VALUES,
+            False,
             id="test_file_test_catalog_multiple_sources",
         ),
     ],
@@ -59,6 +82,7 @@ def test_model_custom_catalog_list_sources(
         model_catalog_rest_url: list[str],
         model_registry_rest_headers: dict[str, str],
         expected_catalog_values: dict[str, str],
+        is_huggingface: bool,
     ):
         """
         Validate sources api for model catalog
@@ -75,6 +99,7 @@ def test_model_custom_catalog_get_models_by_source(
         model_catalog_rest_url: list[str],
         model_registry_rest_headers: dict[str, str],
         expected_catalog_values: dict[str, str],
+        is_huggingface: bool,
     ):
         """
         Validate models api for model catalog associated with a specific source
@@ -93,6 +118,7 @@ def test_model_custom_catalog_get_model_by_name(
         model_catalog_rest_url: list[str],
         model_registry_rest_headers: dict[str, str],
         expected_catalog_values: dict[str, str],
+        is_huggingface: bool,
     ):
         """
         Get Model by name associated with a specific source
@@ -112,10 +138,12 @@ def test_model_custom_catalog_get_model_artifact(
         model_catalog_rest_url: list[str],
         model_registry_rest_headers: dict[str, str],
         expected_catalog_values: dict[str, str],
+        skip_on_huggingface_source: None,
     ):
         """
         Get Model artifacts for model associated with specific source
         """
+
         for expected_entry in expected_catalog_values:
             model_name = expected_entry["model_name"]
             url = f"{model_catalog_rest_url[0]}sources/{expected_entry['id']}/models/{model_name}/artifacts"
@@ -134,6 +162,8 @@ def test_model_custom_catalog_add_model(
         model_catalog_rest_url: list[str],
         model_registry_rest_headers: dict[str, str],
         expected_catalog_values: dict[str, str],
+        is_huggingface: bool,
+        skip_on_huggingface_source: None,
         update_configmap_data_add_model: dict[str, str],
     ):
         """
@@ -147,20 +177,19 @@ def test_model_custom_catalog_add_model(
         assert result["name"] == SAMPLE_MODEL_NAME3
 
     @pytest.mark.dependency(depends=["test_model_custom_catalog_add_model"])
-    @pytest.mark.xfail(reason="RHOAIENG-38653")
     def test_model_custom_catalog_remove_model(
         self: Self,
         model_catalog_rest_url: list[str],
         model_registry_rest_headers: dict[str, str],
         expected_catalog_values: dict[str, str],
+        is_huggingface: bool,
     ):
         """
         Ensure models are removed from the catalog
         """
         url = f"{model_catalog_rest_url[0]}sources/{CUSTOM_CATALOG_ID1}/models/{SAMPLE_MODEL_NAME3}"
         with pytest.raises(ResourceNotFoundError):
-            result = execute_get_command(
+            execute_get_command(
                 url=url,
                 headers=model_registry_rest_headers,
             )
-            LOGGER.info(f"URL: {url} Result: {result}")

tests/model_registry/model_catalog/test_model_catalog_negative.py

@@ -7,7 +7,7 @@
 from tests.model_registry.constants import DEFAULT_MODEL_CATALOG_CM
 from tests.model_registry.model_catalog.constants import DEFAULT_CATALOGS
 from tests.model_registry.model_catalog.utils import validate_model_catalog_configmap_data
-
+from tests.model_registry.utils import execute_get_command
 
 LOGGER = get_logger(name=__name__)
 
@@ -59,3 +59,65 @@ def test_modify_default_catalog_configmap_reconciles(
         validate_model_catalog_configmap_data(
             configmap=model_catalog_config_map, num_catalogs=len(DEFAULT_CATALOGS.keys())
         )
+
+    @pytest.mark.parametrize(
+        "updated_catalog_config_map_scope_function, expected_error_message",
+        [
+            pytest.param(
+                """
+catalogs:
+  - name: Modified Catalog
+    id: error_catalog
+    type: yaml
+    properties:
+      yamlCatalogPath: non-existent-catalog.yaml
+""",
+                "non-existent-catalog.yaml: no such file or directory",
+                id="test_source_error_invalid_path",
+            ),
+            pytest.param(
+                """
+catalogs:
+  - name: HuggingFace Hub
+    id: error_catalog
+    type: hf
+    enabled: true
+""",
+                "includedModels cannot be empty for HuggingFace catalog",
+                id="test_hf_source_no_include_model",
+            ),
+            pytest.param(
+                """
+catalogs:
+  - name: HuggingFace Hub
+    id: error_catalog
+    type: hf
+    enabled: true
+    includedModels:
+    - ibm-granite/*
+""",
+                "HuggingFace API returned status 401",
+                id="test_hf_source_no_include_model",
+                marks=pytest.mark.xfail(reason="RHOAIENG-42213 is causing this failure"),
+            ),
+        ],
+        indirect=["updated_catalog_config_map_scope_function"],
+    )
+    def test_source_error_state(
+        self: Self,
+        updated_catalog_config_map_scope_function: ConfigMap,
+        model_catalog_rest_url: list[str],
+        model_registry_rest_headers: dict[str, str],
+        expected_error_message,
+    ):
+        results = execute_get_command(
+            url=f"{model_catalog_rest_url[0]}sources",
+            headers=model_registry_rest_headers,
+        )["items"]
+        # pick the relevant source first by id:
+        matched_source = [result for result in results if result["id"] == "error_catalog"]
+        assert matched_source, f"Matched expected source not found: {results}"
+        assert matched_source[0]["status"] == "error"
+        assert expected_error_message in matched_source[0]["error"], (
+            f"Expected error: {expected_error_message} not found in {matched_source[0]['error']}"
+        )

tests/model_registry/model_catalog/utils.py

@@ -8,7 +8,7 @@
 from ocp_resources.pod import Pod
 from ocp_resources.config_map import ConfigMap
 from ocp_resources.route import Route
-from tests.model_registry.model_catalog.constants import DEFAULT_CATALOGS
+from tests.model_registry.model_catalog.constants import DEFAULT_CATALOGS, HF_MODELS
 from tests.model_registry.model_catalog.db_constants import (
     SEARCH_MODELS_DB_QUERY,
     SEARCH_MODELS_WITH_SOURCE_ID_DB_QUERY,
@@ -29,10 +29,6 @@
 LOGGER = get_logger(name=__name__)
 
 
-class ResourceNotFoundError(Exception):
-    pass
-
-
 @retry(wait_timeout=60, sleep=5, exceptions_dict={AssertionError: []})
 def get_catalog_url_and_headers(
     admin_client: DynamicClient,
@@ -1086,3 +1082,47 @@ 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 get_hf_catalog_str(ids):
+    """
+    Generate a HuggingFace catalog configuration string in YAML format.
+    Similar to get_catalog_str() but for HuggingFace catalogs.
+
+    Args:
+        ids (list): List of model set identifiers that correspond to keys in MODELS dict
+
+    Returns:
+        str: YAML formatted catalog configuration with multiple catalog entries
+    """
+    catalog_entries = ""
+
+    for source_id in ids:
+        if source_id not in HF_MODELS:
+            raise ValueError(f"Model ID '{source_id}' not found in MODELS dictionary")
+        name = f"HuggingFace Source {source_id}"
+        # Build catalog entry
+        catalog_entries += f"""
+- name: {name}
+  id: huggingface_{source_id}
+  type: "hf"
+  enabled: true
+  includedModels:
+  {get_included_model_str(models=HF_MODELS[source_id])}
+  labels:
+  - {name}
+"""
+
+    # Combine all catalog entries
+    return f"""catalogs:
+    {catalog_entries}
+    """
+
+
+def get_included_model_str(models: list[str]) -> str:
+    included_models: str = ""
+    for model_name in models:
+        included_models += f"""
+    - {model_name}
+"""
+    return included_models

tests/model_registry/utils.py

@@ -760,6 +760,7 @@ def validate_model_catalog_sources(
     LOGGER.info(f"Model catalog sources: {results}")
     ids_from_query = [result_entry["id"] for result_entry in results]
     ids_expected = [expected_entry["id"] for expected_entry in expected_catalog_values]
+    LOGGER.info(f"IDs expected: {ids_expected}, IDs found: {ids_from_query}")
     assert set(ids_expected).issubset(set(ids_from_query)), f"Expected: {expected_catalog_values}. Actual: {results}"