Skip to content

Commit f189779

Browse files
dbasunagdbasunag
committed
Add negative test cases for MC
1 parent 28070d1 commit f189779

File tree

4 files changed

+232
-38
lines changed

4 files changed

+232
-38
lines changed

‎tests/model_registry/model_catalog/test_default_model_catalog.py‎

Lines changed: 2 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,4 @@
11
import pytest
2-
import yaml
32
import random
43
from kubernetes.dynamic import DynamicClient
54
from dictdiffer import diff
@@ -17,10 +16,10 @@
1716
validate_model_catalog_enabled,
1817
execute_get_command,
1918
validate_model_catalog_resource,
20-
validate_default_catalog,
2119
get_validate_default_model_catalog_source,
2220
extract_schema_fields,
2321
get_model_catalog_pod,
22+
validate_model_catalog_configmap_data,
2423
)
2524
from tests.model_registry.utils import get_rest_headers
2625
from utilities.user_utils import UserTestSession
@@ -53,14 +52,7 @@ class TestModelCatalogGeneral:
5352
indirect=["model_catalog_config_map"],
5453
)
5554
def test_config_map_exists(self: Self, model_catalog_config_map: ConfigMap, expected_catalogs: int):
56-
# Check that model catalog configmaps is created when model registry is
57-
# enabled on data science cluster.
58-
catalogs = yaml.safe_load(model_catalog_config_map.instance.data["sources.yaml"])["catalogs"]
59-
assert len(catalogs) == expected_catalogs, (
60-
f"{model_catalog_config_map.name} should have {expected_catalogs} catalog"
61-
)
62-
if expected_catalogs:
63-
validate_default_catalog(catalogs=catalogs)
55+
validate_model_catalog_configmap_data(configmap=model_catalog_config_map, num_catalogs=expected_catalogs)
6456

6557
@pytest.mark.parametrize(
6658
"resource_name, expected_resource_count",

‎tests/model_registry/model_catalog/test_model_catalog_negative.py‎

Lines changed: 174 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,174 @@
1+
import pytest
2+
import re
3+
from kubernetes.dynamic import DynamicClient
4+
from simple_logger.logger import get_logger
5+
from typing import Self
6+
7+
from ocp_resources.config_map import ConfigMap
8+
from ocp_resources.pod import Pod
9+
from ocp_resources.resource import ResourceEditor
10+
from tests.model_registry.constants import DEFAULT_MODEL_CATALOG_CFG
11+
from tests.model_registry.model_catalog.constants import DEFAULT_CATALOGS, CATALOG_CONTAINER
12+
from tests.model_registry.model_catalog.utils import validate_model_catalog_configmap_data, is_model_catalog_ready
13+
from tests.model_registry.utils import get_model_catalog_pod
14+
from timeout_sampler import TimeoutExpiredError
15+
16+
from utilities.general import wait_for_container_status
17+
18+
LOGGER = get_logger(name=__name__)
19+
20+
pytestmark = [
21+
pytest.mark.usefixtures(
22+
"updated_dsc_component_state_scope_session",
23+
"model_registry_namespace",
24+
)
25+
]
26+
27+
28+
@pytest.fixture(scope="class")
29+
def updated_catalog_cm_negative(
30+
request: pytest.FixtureRequest,
31+
catalog_config_map: ConfigMap,
32+
admin_client: DynamicClient,
33+
model_registry_namespace: str,
34+
model_catalog_rest_url: list[str],
35+
model_registry_rest_headers: dict[str, str],
36+
) -> ConfigMap:
37+
"""Fixture for testing negative scenarios with custom catalog configmap"""
38+
patches = request.param
39+
40+
with ResourceEditor(patches={catalog_config_map: patches}):
41+
try:
42+
is_model_catalog_ready(
43+
client=admin_client, model_registry_namespace=model_registry_namespace, consecutive_try=1
44+
)
45+
except TimeoutExpiredError:
46+
LOGGER.info("Model Catalog pod is expected to be unhealthy")
47+
yield catalog_config_map
48+
49+
is_model_catalog_ready(client=admin_client, model_registry_namespace=model_registry_namespace)
50+
51+
52+
@pytest.fixture(scope="class")
53+
def model_catalog_pod_error_state(
54+
admin_client: DynamicClient,
55+
model_registry_namespace: str,
56+
) -> Pod:
57+
"""Fixture that returns a model catalog pod in error state"""
58+
model_catalog_pod = get_model_catalog_pod(
59+
client=admin_client,
60+
model_registry_namespace=model_registry_namespace,
61+
label_selector="app.kubernetes.io/name=model-catalog",
62+
)[0]
63+
assert wait_for_container_status(
64+
pod=model_catalog_pod,
65+
container_name=CATALOG_CONTAINER,
66+
expected_status=Pod.Status.CRASH_LOOPBACK_OFF,
67+
timeout=60,
68+
sleep=2,
69+
)
70+
return model_catalog_pod
71+
72+
73+
def _get_default_catalog_str() -> str:
74+
"""
75+
Create a catalog string using the first entry from DEFAULT_CATALOGS.
76+
Similar to get_catalog_str() but uses actual DEFAULT_CATALOGS values.
77+
"""
78+
# Get the first catalog ID and data from DEFAULT_CATALOGS
79+
first_catalog_id = next(iter(DEFAULT_CATALOGS))
80+
first_catalog_data = DEFAULT_CATALOGS[first_catalog_id]
81+
82+
catalog_str = f"""
83+
- name: {first_catalog_data["name"]}
84+
id: {first_catalog_id}
85+
type: {first_catalog_data["type"]}
86+
properties:
87+
yamlCatalogPath: {first_catalog_data["properties"]["yamlCatalogPath"]}
88+
"""
89+
90+
return f"""catalogs:
91+
{catalog_str}
92+
"""
93+
94+
95+
@pytest.mark.skip_must_gather
96+
class TestDefaultCatalogNegative:
97+
"""Negative tests for default catalog configuration"""
98+
99+
@pytest.mark.parametrize(
100+
"model_catalog_config_map, modified_sources_yaml",
101+
[
102+
pytest.param(
103+
{"configmap_name": DEFAULT_MODEL_CATALOG_CFG},
104+
"""
105+
catalogs:
106+
- name: Modified Catalog
107+
id: modified_catalog
108+
type: yaml
109+
properties:
110+
yamlCatalogPath: /shared-data/modified-catalog.yaml
111+
""",
112+
id="test_modify_catalog_structure",
113+
),
114+
],
115+
indirect=["model_catalog_config_map"],
116+
)
117+
def test_modify_default_catalog_configmap_reconciles(
118+
self: Self, model_catalog_config_map: ConfigMap, modified_sources_yaml: str
119+
):
120+
"""
121+
Test that attempting to modify the default catalog configmap raises an exception.
122+
This validates that the default catalog configmap is protected from modifications.
123+
"""
124+
# Attempt to modify the configmap - this should raise an exception
125+
patches = {"data": {"sources.yaml": modified_sources_yaml}}
126+
127+
with ResourceEditor(patches={model_catalog_config_map: patches}):
128+
# This block should raise an exception due to configmap protection
129+
LOGGER.info("Attempting to modify protected configmap")
130+
131+
# Verify the configmap was not actually modified
132+
validate_model_catalog_configmap_data(
133+
configmap=model_catalog_config_map, num_catalogs=len(DEFAULT_CATALOGS.keys())
134+
)
135+
136+
137+
@pytest.mark.skip_must_gather
138+
class TestCustomCatalogNegative:
139+
"""Negative tests for custom catalog configuration"""
140+
141+
@pytest.mark.parametrize(
142+
"updated_catalog_cm_negative",
143+
[
144+
pytest.param(
145+
{"data": {"sources.yaml": _get_default_catalog_str()}},
146+
id="test_default_catalog_in_custom_configmap",
147+
),
148+
],
149+
indirect=True,
150+
)
151+
def test_default_catalog_rejected_in_custom_configmap(
152+
self: Self, updated_catalog_cm_negative: ConfigMap, model_catalog_pod_error_state: Pod
153+
):
154+
"""
155+
Test that attempting to add a default catalog to custom configmap is rejected.
156+
This validates that default catalogs cannot be added to custom catalog configmap.
157+
"""
158+
# Check model catalog pod logs for the expected error
159+
pod_log = model_catalog_pod_error_state.log(container=CATALOG_CONTAINER)
160+
161+
# Look for error pattern using regex
162+
error_pattern = r"Error: error loading catalog sources: (.*): source (.*) exists from multiple origins"
163+
match = re.search(error_pattern, pod_log)
164+
165+
if match:
166+
sources_file = match.group(1)
167+
source_name = match.group(2)
168+
LOGGER.warning(
169+
f"Found expected error in pod {model_catalog_pod_error_state.name} log: "
170+
f"sources file '{sources_file}', source '{source_name}' exists from multiple origins"
171+
)
172+
else:
173+
LOGGER.info(f"Pod log is: {pod_log}")
174+
pytest.fail(f"Expected error pattern not found in pod {model_catalog_pod_error_state.name} log")

‎tests/model_registry/model_catalog/utils.py‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import json
22
from typing import Any
33
import time
4+
import yaml
45

56
from kubernetes.dynamic import DynamicClient
67
from simple_logger.logger import get_logger
@@ -9,6 +10,7 @@
910
from timeout_sampler import retry
1011

1112
from ocp_resources.pod import Pod
13+
from ocp_resources.config_map import ConfigMap
1214
from tests.model_registry.model_catalog.constants import (
1315
DEFAULT_CATALOGS,
1416
)
@@ -208,3 +210,19 @@ def _extract_properties_and_required(schema: dict[Any, Any]) -> tuple[set[str],
208210
}
209211

210212
return all_properties - excluded_fields, required_fields - excluded_fields
213+
214+
215+
def validate_model_catalog_configmap_data(configmap: ConfigMap, num_catalogs: int) -> None:
216+
"""
217+
Validate the model catalog configmap data.
218+
219+
Args:
220+
configmap: The ConfigMap object to validate
221+
num_catalogs: Expected number of catalogs in the configmap
222+
"""
223+
# Check that model catalog configmaps is created when model registry is
224+
# enabled on data science cluster.
225+
catalogs = yaml.safe_load(configmap.instance.data["sources.yaml"])["catalogs"]
226+
assert len(catalogs) == num_catalogs, f"{configmap.name} should have {num_catalogs} catalog"
227+
if num_catalogs:
228+
validate_default_catalog(catalogs=catalogs)

‎utilities/general.py‎

Lines changed: 38 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@
1111
from simple_logger.logger import get_logger
1212

1313
import utilities.infra
14-
from utilities.constants import Annotations, KServeDeploymentType, MODELMESH_SERVING, Timeout
14+
from utilities.constants import Annotations, KServeDeploymentType, MODELMESH_SERVING
1515
from utilities.exceptions import UnexpectedResourceCountError, ResourceValueMismatch
1616
from ocp_resources.resource import Resource
1717
from timeout_sampler import retry
@@ -340,19 +340,18 @@ def generate_random_name(prefix: str = "", length: int = 8) -> str:
340340
return f"{prefix}-{suffix}" if prefix else suffix
341341

342342

343-
@retry(
344-
wait_timeout=Timeout.TIMEOUT_15_SEC,
345-
sleep=1,
346-
exceptions_dict={ResourceValueMismatch: [], ResourceNotFoundError: [], NotFoundError: []},
347-
)
348-
def wait_for_container_status(pod: Pod, container_name: str, expected_status: str) -> bool:
343+
def wait_for_container_status(
344+
pod: Pod, container_name: str, expected_status: str, timeout: int = 15, sleep: int = 1
345+
) -> bool:
349346
"""
350347
Wait for a container to be in the expected status.
351348
352349
Args:
353350
pod: The pod to wait for
354351
container_name: The name of the container to wait for
355352
expected_status: The expected status
353+
timeout: The maximum time in second to wait for the container
354+
sleep: The number of seconds to sleep between checks
356355
357356
Returns:
358357
bool: True if the container is in the expected status, False otherwise
@@ -361,30 +360,41 @@ def wait_for_container_status(pod: Pod, container_name: str, expected_status: st
361360
ResourceValueMismatch: If the container is not in the expected status
362361
"""
363362

364-
container_status = None
365-
for cs in pod.instance.status.get("containerStatuses", []):
366-
if cs.name == container_name:
367-
container_status = cs
368-
break
369-
if container_status is None:
370-
raise ResourceValueMismatch(f"Container {container_name} not found in pod {pod.name}")
371-
372-
if container_status.state.waiting:
373-
reason = container_status.state.waiting.reason
374-
elif container_status.state.terminated:
375-
reason = container_status.state.terminated.reason
376-
elif container_status.state.running:
377-
# Running container does not have a reason
378-
reason = "Running"
379-
else:
363+
@retry(
364+
wait_timeout=timeout,
365+
sleep=sleep,
366+
exceptions_dict={ResourceValueMismatch: [], ResourceNotFoundError: [], NotFoundError: []},
367+
)
368+
def get_matching_container_status(_pod: Pod, _container_name: str, _expected_status: str) -> bool:
369+
container_status = None
370+
for cs in _pod.instance.status.get("containerStatuses", []):
371+
if cs.name == _container_name:
372+
container_status = cs
373+
break
374+
if container_status is None:
375+
raise ResourceValueMismatch(f"Container {_container_name} not found in pod {_pod.name}")
376+
377+
if container_status.state.waiting:
378+
reason = container_status.state.waiting.reason
379+
elif container_status.state.terminated:
380+
reason = container_status.state.terminated.reason
381+
elif container_status.state.running:
382+
# Running container does not have a reason
383+
reason = "Running"
384+
else:
385+
raise ResourceValueMismatch(
386+
f"{_container_name} in {_pod.name} is in an unrecognized or "
387+
f"transitional state: {container_status.state}"
388+
)
389+
390+
if reason == expected_status:
391+
LOGGER.info(f"Container {_container_name} is in the expected status {_expected_status}")
392+
return True
380393
raise ResourceValueMismatch(
381-
f"{container_name} in {pod.name} is in an unrecognized or transitional state: {container_status.state}"
394+
f"Container {_container_name} is not in the expected status {container_status.state}"
382395
)
383396

384-
if reason == expected_status:
385-
LOGGER.info(f"Container {container_name} is in the expected status {expected_status}")
386-
return True
387-
raise ResourceValueMismatch(f"Container {container_name} is not in the expected status {container_status.state}")
397+
return get_matching_container_status(_pod=pod, _container_name=container_name, _expected_status=expected_status)
388398

389399

390400
def get_pod_container_error_status(pod: Pod) -> str | None:

0 commit comments

Comments
 (0)