Revert "Remove defaults from custom configmaps (#699)"

This reverts commit 56f4250.

Author: lugi0

tests/model_registry/constants.py

@@ -64,5 +64,4 @@ class ModelRegistryEndpoints:
     "mysql": 3306,
 }
 MODEL_REGISTRY_POD_FILTER: str = "component=model-registry"
-DEFAULT_CUSTOM_MODEL_CATALOG: str = "model-catalog-sources"
-DEFAULT_MODEL_CATALOG_CFG: str = "model-catalog-default-sources"
+DEFAULT_MODEL_CATALOG: str = "model-catalog-sources"

tests/model_registry/model_catalog/conftest.py

@@ -12,20 +12,21 @@
 
 from ocp_resources.route import Route
 from ocp_resources.service_account import ServiceAccount
-from tests.model_registry.constants import DEFAULT_CUSTOM_MODEL_CATALOG
+from tests.model_registry.constants import DEFAULT_MODEL_CATALOG
 from tests.model_registry.model_catalog.constants import (
     SAMPLE_MODEL_NAME3,
     CUSTOM_CATALOG_ID1,
+    DEFAULT_CATALOG_ID,
     DEFAULT_CATALOG_FILE,
     CATALOG_CONTAINER,
-    REDHAT_AI_CATALOG_ID,
 )
 from tests.model_registry.model_catalog.utils import (
     is_model_catalog_ready,
     wait_for_model_catalog_api,
     get_model_str,
     execute_get_command,
     get_model_catalog_pod,
+    get_default_model_catalog_yaml,
 )
 from tests.model_registry.utils import get_rest_headers
 from utilities.infra import get_openshift_token, login_with_user_password, create_inference_token
@@ -37,7 +38,7 @@
 
 @pytest.fixture(scope="class")
 def catalog_config_map(admin_client: DynamicClient, model_registry_namespace: str) -> ConfigMap:
-    return ConfigMap(name=DEFAULT_CUSTOM_MODEL_CATALOG, client=admin_client, namespace=model_registry_namespace)
+    return ConfigMap(name=DEFAULT_MODEL_CATALOG, client=admin_client, namespace=model_registry_namespace)
 
 
 @pytest.fixture(scope="class")
@@ -79,7 +80,15 @@ def updated_catalog_config_map(
     model_catalog_rest_url: list[str],
     model_registry_rest_headers: dict[str, str],
 ) -> Generator[ConfigMap, None, None]:
-    patches = {"data": {"sources.yaml": request.param["sources_yaml"]}}
+    defaults = yaml.dump(
+        get_default_model_catalog_yaml(config_map=catalog_config_map),
+        default_flow_style=False,
+    )
+    catalog_str = f"""catalogs:
+{defaults}
+{request.param["sources_yaml"]}
+    """
+    patches = {"data": {"sources.yaml": catalog_str}}
     if "sample_yaml" in request.param:
         for key in request.param["sample_yaml"]:
             patches["data"][key] = request.param["sample_yaml"][key]
@@ -150,7 +159,7 @@ def user_token_for_api_calls(
 def randomly_picked_model_from_default_catalog(
     model_catalog_rest_url: list[str], user_token_for_api_calls: str
 ) -> dict[Any, Any]:
-    url = f"{model_catalog_rest_url[0]}models?source={REDHAT_AI_CATALOG_ID}&pageSize=100"
+    url = f"{model_catalog_rest_url[0]}models?source={DEFAULT_CATALOG_ID}&pageSize=100"
     result = execute_get_command(
         url=url,
         headers=get_rest_headers(token=user_token_for_api_calls),
@@ -172,7 +181,7 @@ def default_catalog_api_response(
 ) -> dict[Any, Any]:
     """Fetch all models from default catalog API (used for data validation tests)"""
     return execute_get_command(
-        url=f"{model_catalog_rest_url[0]}models?source={REDHAT_AI_CATALOG_ID}&pageSize=100",
+        url=f"{model_catalog_rest_url[0]}models?source={DEFAULT_CATALOG_ID}&pageSize=100",
         headers=model_registry_rest_headers,
     )
 

tests/model_registry/model_catalog/constants.py

@@ -1,5 +1,3 @@
-from typing import Any
-
 CUSTOM_CATALOG_ID1: str = "sample_custom_catalog1"
 CUSTOM_CATALOG_ID2: str = "sample_custom_catalog2"
 SAMPLE_MODEL_NAME1 = "mistralai/Mistral-7B-Instruct-v0.3"
@@ -10,20 +8,9 @@
     {"id": CUSTOM_CATALOG_ID1, "model_name": SAMPLE_MODEL_NAME1},
     {"id": CUSTOM_CATALOG_ID2, "model_name": SAMPLE_MODEL_NAME2},
 ]
-
+DEFAULT_CATALOG_NAME: str = "Default Catalog"
+DEFAULT_CATALOG_ID: str = "default_catalog"
+CATALOG_TYPE: str = "yaml"
+DEFAULT_CATALOG_FILE: str = "/shared-data/default-catalog.yaml"
 SAMPLE_MODEL_NAME3 = "mistralai/Ministral-8B-Instruct-2410"
 CATALOG_CONTAINER: str = "catalog"
-DEFAULT_CATALOGS: dict[str, Any] = {
-    "redhat_ai_models": {
-        "name": "Red Hat AI models",
-        "type": "yaml",
-        "properties": {"yamlCatalogPath": "/shared-data/models-catalog.yaml"},
-    },
-    "redhat_ai_validated_models": {
-        "name": "Red Hat AI validated models",
-        "type": "yaml",
-        "properties": {"yamlCatalogPath": "/shared-data/validated-models-catalog.yaml"},
-    },
-}
-REDHAT_AI_CATALOG_ID: str = next(iter(DEFAULT_CATALOGS))
-DEFAULT_CATALOG_FILE: str = DEFAULT_CATALOGS[REDHAT_AI_CATALOG_ID]["properties"]["yamlCatalogPath"]

tests/model_registry/model_catalog/test_custom_model_catalog.py

@@ -6,6 +6,7 @@
     SAMPLE_MODEL_NAME2,
     MULTIPLE_CUSTOM_CATALOG_VALUES,
     SAMPLE_MODEL_NAME3,
+    DEFAULT_CATALOG_ID,
 )
 from ocp_resources.config_map import ConfigMap
 import pytest
@@ -67,11 +68,12 @@ def test_model_custom_catalog_list_sources(
             url=url,
             headers=model_registry_rest_headers,
         )["items"]
+        # since we expect one default catalog
+        assert len(results) == len(expected_catalog_values) + 1
         ids_from_query = [result_entry["id"] for result_entry in results]
         ids_expected = [expected_entry["id"] for expected_entry in expected_catalog_values]
-        assert set(ids_expected).issubset(set(ids_from_query)), (
-            f"Expected model catalogs: {expected_catalog_values}. Actual model catalogs: {results}"
-        )
+        ids_expected.append(DEFAULT_CATALOG_ID)
+        assert sorted(ids_from_query) == sorted(ids_expected), f"Expected: {expected_catalog_values}. Actual: {results}"
 
     def test_model_custom_catalog_get_models_by_source(
         self: Self,

tests/model_registry/model_catalog/test_default_model_catalog.py

@@ -11,8 +11,7 @@
 from ocp_resources.config_map import ConfigMap
 from ocp_resources.route import Route
 from ocp_resources.service import Service
-from tests.model_registry.constants import DEFAULT_CUSTOM_MODEL_CATALOG, DEFAULT_MODEL_CATALOG_CFG
-from tests.model_registry.model_catalog.constants import REDHAT_AI_CATALOG_ID, CATALOG_CONTAINER
+from tests.model_registry.model_catalog.constants import DEFAULT_CATALOG_ID, CATALOG_CONTAINER
 from tests.model_registry.model_catalog.utils import (
     validate_model_catalog_enabled,
     execute_get_command,
@@ -37,30 +36,26 @@
 @pytest.mark.skip_must_gather
 class TestModelCatalogGeneral:
     @pytest.mark.parametrize(
-        "model_catalog_config_map, expected_catalogs",
+        "model_catalog_config_map",
         [
             pytest.param(
-                {"configmap_name": DEFAULT_CUSTOM_MODEL_CATALOG},
-                0,
+                {"configmap_name": "model-catalog-sources"},
                 id="test_model_catalog_sources_configmap",
             ),
             pytest.param(
-                {"configmap_name": DEFAULT_MODEL_CATALOG_CFG},
-                2,
+                {"configmap_name": "model-catalog-default-sources"},
                 id="test_model_catalog_default_sources_configmap",
             ),
         ],
         indirect=["model_catalog_config_map"],
     )
-    def test_config_map_exists(self: Self, model_catalog_config_map: ConfigMap, expected_catalogs: int):
+    def test_config_map_exists(self: Self, model_catalog_config_map: ConfigMap):
         # Check that model catalog configmaps is created when model registry is
         # enabled on data science cluster.
         catalogs = yaml.safe_load(model_catalog_config_map.instance.data["sources.yaml"])["catalogs"]
-        assert len(catalogs) == expected_catalogs, (
-            f"{model_catalog_config_map.name} should have {expected_catalogs} catalog"
-        )
-        if expected_catalogs:
-            validate_default_catalog(catalogs=catalogs)
+        assert catalogs
+        assert len(catalogs) == 1, f"{model_catalog_config_map.name} should have 1 catalog"
+        validate_default_catalog(default_catalog=catalogs[0])
 
     @pytest.mark.parametrize(
         "resource_name, expected_resource_count",
@@ -175,7 +170,7 @@ def test_model_default_catalog_get_model_by_name(
         """
         model_name = randomly_picked_model_from_default_catalog["name"]
         result = execute_get_command(
-            url=f"{model_catalog_rest_url[0]}sources/{REDHAT_AI_CATALOG_ID}/models/{model_name}",
+            url=f"{model_catalog_rest_url[0]}sources/{DEFAULT_CATALOG_ID}/models/{model_name}",
             headers=get_rest_headers(token=user_token_for_api_calls),
         )
         differences = list(diff(randomly_picked_model_from_default_catalog, result))
@@ -192,7 +187,7 @@ def test_model_default_catalog_get_model_artifact(
         """
         model_name = randomly_picked_model_from_default_catalog["name"]
         result = execute_get_command(
-            url=f"{model_catalog_rest_url[0]}sources/{REDHAT_AI_CATALOG_ID}/models/{model_name}/artifacts",
+            url=f"{model_catalog_rest_url[0]}sources/{DEFAULT_CATALOG_ID}/models/{model_name}/artifacts",
             headers=get_rest_headers(token=user_token_for_api_calls),
         )["items"]
         assert result, f"No artifacts found for {model_name}"
@@ -290,25 +285,24 @@ def test_model_default_catalog_random_artifact(
         LOGGER.info(f"Required artifact fields from OpenAPI schema: {required_artifact_fields}")
 
         random_model = random.choice(seq=default_model_catalog_yaml_content.get("models", []))
-        model_name = random_model["name"]
-        LOGGER.info(f"Random model: {model_name}")
+        LOGGER.info(f"Random model: {random_model['name']}")
 
         api_model_artifacts = execute_get_command(
-            url=f"{model_catalog_rest_url[0]}sources/{REDHAT_AI_CATALOG_ID}/models/{model_name}/artifacts",
+            url=f"{model_catalog_rest_url[0]}sources/{DEFAULT_CATALOG_ID}/models/{random_model['name']}/artifacts",
             headers=model_registry_rest_headers,
         )["items"]
 
         yaml_artifacts = random_model.get("artifacts", [])
-        assert api_model_artifacts, f"No artifacts found in API for {model_name}"
-        assert yaml_artifacts, f"No artifacts found in YAML for {model_name}"
+        assert api_model_artifacts, f"No artifacts found in API for {random_model['name']}"
+        assert yaml_artifacts, f"No artifacts found in YAML for {random_model['name']}"
 
         # Validate all required fields are present in both YAML and API artifact
         # FAILS artifactType is not in YAML nor in API until https://issues.redhat.com/browse/RHOAIENG-35569 is fixed
         for field in required_artifact_fields:
             for artifact in yaml_artifacts:
-                assert field in artifact, f"YAML artifact for {model_name} missing REQUIRED field: {field}"
+                assert field in artifact, f"YAML artifact for {random_model['name']} missing REQUIRED field: {field}"
             for artifact in api_model_artifacts:
-                assert field in artifact, f"API artifact for {model_name} missing REQUIRED field: {field}"
+                assert field in artifact, f"API artifact for {random_model['name']} missing REQUIRED field: {field}"
 
         # Filter artifacts to only include schema-defined fields for comparison
         yaml_artifacts_filtered = [
@@ -319,5 +313,5 @@ def test_model_default_catalog_random_artifact(
         ]
 
         differences = list(diff(yaml_artifacts_filtered, api_artifacts_filtered))
-        assert not differences, f"Artifacts mismatch for {model_name}: {differences}"
+        assert not differences, f"Artifacts mismatch for {random_model['name']}: {differences}"
         LOGGER.info("Artifacts match")

tests/model_registry/model_catalog/utils.py

@@ -1,16 +1,21 @@
 import json
 from typing import Any
 import time
+import yaml
 
 from kubernetes.dynamic import DynamicClient
 from simple_logger.logger import get_logger
 
 import requests
 from timeout_sampler import retry
 
+from ocp_resources.config_map import ConfigMap
 from ocp_resources.pod import Pod
 from tests.model_registry.model_catalog.constants import (
-    DEFAULT_CATALOGS,
+    DEFAULT_CATALOG_NAME,
+    DEFAULT_CATALOG_ID,
+    CATALOG_TYPE,
+    DEFAULT_CATALOG_FILE,
 )
 from tests.model_registry.utils import get_model_catalog_pod, get_rest_headers
 from utilities.general import wait_for_pods_running
@@ -90,16 +95,11 @@ def validate_model_catalog_resource(
     )
 
 
-def validate_default_catalog(catalogs: list[dict[Any, Any]]) -> None:
-    errors = ""
-    for catalog in catalogs:
-        expected_catalog = DEFAULT_CATALOGS.get(catalog["id"])
-        assert expected_catalog, f"Unexpected catalog: {catalog}"
-        for field in ["type", "name", "properties"]:
-            if catalog[field] != expected_catalog[field]:
-                errors += f"For {catalog['id']} expected {field}={expected_catalog[field]}, but got {catalog[field]}"
-
-    assert not errors, errors
+def validate_default_catalog(default_catalog) -> None:
+    assert default_catalog["name"] == DEFAULT_CATALOG_NAME
+    assert default_catalog["id"] == DEFAULT_CATALOG_ID
+    assert default_catalog["type"] == CATALOG_TYPE
+    assert default_catalog["properties"].get("yamlCatalogPath") == DEFAULT_CATALOG_FILE
 
 
 def get_catalog_str(ids: list[str]) -> str:
@@ -113,9 +113,7 @@ def get_catalog_str(ids: list[str]) -> str:
   properties:
     yamlCatalogPath: {id.replace("_", "-")}.yaml
 """
-    return f"""catalogs:
-{catalog_str}
-"""
+    return catalog_str
 
 
 def get_sample_yaml_str(models: list[str]) -> str:
@@ -156,11 +154,14 @@ def get_validate_default_model_catalog_source(token: str, model_catalog_url: str
         headers=get_rest_headers(token=token),
     )["items"]
     assert result
-    assert len(result) == 2, f"Expected no custom models to be present. Actual: {result}"
-    ids_actual = [entry["id"] for entry in result]
-    assert sorted(ids_actual) == sorted(DEFAULT_CATALOGS.keys()), (
-        f"Actual default catalog entries: {ids_actual},Expected: {DEFAULT_CATALOGS.keys()}"
-    )
+    assert len(result) == 1, f"Expected no custom models to be present. Actual: {result}"
+    assert result[0]["id"] == DEFAULT_CATALOG_ID
+    assert result[0]["name"] == DEFAULT_CATALOG_NAME
+    assert str(result[0]["enabled"]) == "True", result[0]["enabled"]
+
+
+def get_default_model_catalog_yaml(config_map: ConfigMap) -> str:
+    return yaml.safe_load(config_map.instance.data["sources.yaml"])["catalogs"]
 
 
 def extract_schema_fields(openapi_schema: dict[Any, Any], schema_name: str) -> tuple[set[str], set[str]]:

tests/model_registry/rest_api/test_multiple_mr.py

@@ -6,12 +6,7 @@
 from ocp_resources.model_registry_modelregistry_opendatahub_io import ModelRegistry
 from ocp_resources.pod import Pod
 
-from tests.model_registry.constants import (
-    MR_INSTANCE_BASE_NAME,
-    NUM_RESOURCES,
-    DEFAULT_CUSTOM_MODEL_CATALOG,
-    DEFAULT_MODEL_CATALOG_CFG,
-)
+from tests.model_registry.constants import MR_INSTANCE_BASE_NAME, NUM_RESOURCES, DEFAULT_MODEL_CATALOG
 from tests.model_registry.rest_api.utils import (
     validate_resource_attributes,
     get_register_model_data,
@@ -55,25 +50,26 @@ def test_validate_one_model_catalog_configmap(
         self: Self, admin_client: DynamicClient, model_registry_namespace: str
     ):
         """
-        Validate that when multiple MR exists on a cluster, only two model catalog configmaps are created
+        Validate that when multiple MR exists on a cluster, only one model catalog configmap is created
         """
         config_map_names: list[str] = []
-        expected_number_config_maps: int = 2
+        expected_number_config_maps: int = 1
         for config_map in list(ConfigMap.get(namespace=model_registry_namespace, dyn_client=admin_client)):
-            if config_map.name.startswith(tuple([DEFAULT_CUSTOM_MODEL_CATALOG, DEFAULT_MODEL_CATALOG_CFG])):
+            if config_map.name.startswith(DEFAULT_MODEL_CATALOG):
                 config_map_names.append(config_map.name)
         assert len(config_map_names) == expected_number_config_maps, (
-            f"Expected {expected_number_config_maps} model catalog sources, found: {config_map_names}"
+            f"Expected {expected_number_config_maps} modelcatalog sources, found: {config_map_names}"
         )
 
-    def test_validate_model_catalog_pods(self: Self, admin_client: DynamicClient, model_registry_namespace: str):
+    # FAILS until https://github.com/opendatahub-io/model-registry-operator/pull/298/ is merged downstream
+    def test_validate_one_model_catalog_pod(self: Self, admin_client: DynamicClient, model_registry_namespace: str):
         """
-        Validate that even when multiple MR exists on a cluster, only two model catalog pods are created
+        Validate that even when multiple MR exists on a cluster, only one model catalog pod is created
         """
         catalog_pods: list[Pod] = get_model_catalog_pod(
             client=admin_client, model_registry_namespace=model_registry_namespace
         )
-        expected_number_pods: int = 2
+        expected_number_pods: int = 1
 
         assert len(catalog_pods) == expected_number_pods, (
             f"Expected {expected_number_pods} model catalog pods, found: {[pod.name for pod in catalog_pods]}"