From 6afab5749c60b2ec37c04701e7edf67599ccb611 Mon Sep 17 00:00:00 2001 From: dbasunag Date: Mon, 7 Apr 2025 18:37:48 -0400 Subject: [PATCH 1/2] updates to test_registering_model() based on previous review comments --- .../test_model_registry_creation.py | 27 ++++++++++--------- 1 file changed, 14 insertions(+), 13 deletions(-) diff --git a/tests/model_registry/test_model_registry_creation.py b/tests/model_registry/test_model_registry_creation.py index 21f1f777c..5b12a0d89 100644 --- a/tests/model_registry/test_model_registry_creation.py +++ b/tests/model_registry/test_model_registry_creation.py @@ -67,19 +67,20 @@ def test_registering_model( registered_model: RegisteredModel, ): model = model_registry_client.get_registered_model(MODEL_NAME) - errors = [] - if not registered_model.id == model.id: - errors.append(f"Unexpected id, received {model.id}") - if not registered_model.name == model.name: - errors.append(f"Unexpected name, received {model.name}") - if not registered_model.description == model.description: - errors.append(f"Unexpected description, received {model.description}") - if not registered_model.owner == model.owner: - errors.append(f"Unexpected owner, received {model.owner}") - if not registered_model.state == model.state: - errors.append(f"Unexpected state, received {model.state}") - - assert not errors, "errors found in model registry response validation:\n{}".format("\n".join(errors)) + expected_attrs = { + "id": registered_model.id, + "name": registered_model.name, + "description": registered_model.description, + "owner": registered_model.owner, + "state": registered_model.state, + } + errors = [ + f"Unexpected {attr} expected: {expected}, received {getattr(model, attr)}" + for attr, expected in expected_attrs.items() + if getattr(model, attr) != expected + ] + if errors: + pytest.fail("errors found in model registry response validation:\n{}".format("\n".join(errors))) def test_model_registry_operator_env( self, From cba3a8b1e5c6ba881486553eaf18ecac1cf4921f Mon Sep 17 00:00:00 2001 From: dbasunag Date: Mon, 14 Apr 2025 14:48:04 -0400 Subject: [PATCH 2/2] update namespace code and rearrange tests --- tests/model_registry/conftest.py | 184 ++++-------------- tests/model_registry/constants.py | 12 +- .../model_registry/negative_tests/__init__.py | 0 .../model_registry/negative_tests/conftest.py | 121 ++++++++++++ .../negative_tests/constants.py | 1 + .../test_model_registry_creation_negative.py | 24 +-- .../model_registry/python_client/__init__.py | 0 .../test_model_registry_creation.py | 28 +-- .../scc/test_model_registry_scc.py | 23 +-- .../model_registry/test_rest_api_stateful.py | 5 +- tests/model_registry/utils.py | 117 ++++++++++- 11 files changed, 317 insertions(+), 198 deletions(-) create mode 100644 tests/model_registry/negative_tests/__init__.py create mode 100644 tests/model_registry/negative_tests/conftest.py create mode 100644 tests/model_registry/negative_tests/constants.py rename tests/model_registry/{ => negative_tests}/test_model_registry_creation_negative.py (75%) create mode 100644 tests/model_registry/python_client/__init__.py rename tests/model_registry/{ => python_client}/test_model_registry_creation.py (77%) diff --git a/tests/model_registry/conftest.py b/tests/model_registry/conftest.py index 6fd613246..6323c03e8 100644 --- a/tests/model_registry/conftest.py +++ b/tests/model_registry/conftest.py @@ -25,49 +25,39 @@ from pytest_testconfig import config as py_config from model_registry.types import RegisteredModel from tests.model_registry.constants import ( - MR_NAMESPACE, MR_OPERATOR_NAME, MR_INSTANCE_NAME, ISTIO_CONFIG_DICT, DB_RESOURCES_NAME, - MR_DB_IMAGE_DIGEST, + MODEL_REGISTRY_DB_SECRET_STR_DATA, + MODEL_REGISTRY_DB_SECRET_ANNOTATIONS, +) +from tests.model_registry.utils import ( + get_endpoint_from_mr_service, + get_mr_service_by_label, + get_model_registry_deployment_template_dict, + get_model_registry_db_label_dict, ) -from tests.model_registry.utils import get_endpoint_from_mr_service, get_mr_service_by_label -from utilities.infra import create_ns from utilities.constants import Annotations, Protocols, DscComponents from model_registry import ModelRegistry as ModelRegistryClient LOGGER = get_logger(name=__name__) -DEFAULT_LABEL_DICT_DB: dict[str, str] = { - Annotations.KubernetesIo.NAME: DB_RESOURCES_NAME, - Annotations.KubernetesIo.INSTANCE: DB_RESOURCES_NAME, - Annotations.KubernetesIo.PART_OF: DB_RESOURCES_NAME, -} - @pytest.fixture(scope="class") -def model_registry_namespace(request: FixtureRequest, admin_client: DynamicClient) -> Generator[Namespace, Any, Any]: - # TODO: model_registry_namespace fixture should basically be doing this 1) check the ns exists, 2) it is in ACTIVE - # state and return. But it should not create the ns. If DSC manages registriesNamespace, and namespace was not - # created when mr is updated to Managed state, it would be a bug and we should catch it - # To be handled in upcoming PR. - with create_ns( - name=request.param.get("namespace_name", MR_NAMESPACE), - admin_client=admin_client, - ) as ns: - yield ns +def model_registry_namespace(updated_dsc_component_state_scope_class: DataScienceCluster) -> str: + return updated_dsc_component_state_scope_class.instance.spec.components.modelregistry.registriesNamespace @pytest.fixture(scope="class") def model_registry_db_service( - admin_client: DynamicClient, model_registry_namespace: Namespace + admin_client: DynamicClient, model_registry_namespace: str ) -> Generator[Service, Any, Any]: with Service( client=admin_client, name=DB_RESOURCES_NAME, - namespace=model_registry_namespace.name, + namespace=model_registry_namespace, ports=[ { "name": "mysql", @@ -81,7 +71,7 @@ def model_registry_db_service( selector={ "name": DB_RESOURCES_NAME, }, - label=DEFAULT_LABEL_DICT_DB, + label=get_model_registry_db_label_dict(db_resource_name=DB_RESOURCES_NAME), annotations={ "template.openshift.io/expose-uri": r"mysql://{.spec.clusterIP}:{.spec.ports[?(.name==\mysql\)].port}", }, @@ -91,16 +81,15 @@ def model_registry_db_service( @pytest.fixture(scope="class") def model_registry_db_pvc( - admin_client: DynamicClient, - model_registry_namespace: Namespace, + admin_client: DynamicClient, model_registry_namespace: str ) -> Generator[PersistentVolumeClaim, Any, Any]: with PersistentVolumeClaim( accessmodes="ReadWriteOnce", name=DB_RESOURCES_NAME, - namespace=model_registry_namespace.name, + namespace=model_registry_namespace, client=admin_client, size="5Gi", - label=DEFAULT_LABEL_DICT_DB, + label=get_model_registry_db_label_dict(db_resource_name=DB_RESOURCES_NAME), ) as pvc: yield pvc @@ -108,23 +97,15 @@ def model_registry_db_pvc( @pytest.fixture(scope="class") def model_registry_db_secret( admin_client: DynamicClient, - model_registry_namespace: Namespace, + model_registry_namespace: str, ) -> Generator[Secret, Any, Any]: with Secret( client=admin_client, name=DB_RESOURCES_NAME, - namespace=model_registry_namespace.name, - string_data={ - "database-name": "model_registry", - "database-password": "TheBlurstOfTimes", # pragma: allowlist secret - "database-user": "mlmduser", # pragma: allowlist secret - }, - label=DEFAULT_LABEL_DICT_DB, - annotations={ - "template.openshift.io/expose-database_name": "'{.data[''database-name'']}'", - "template.openshift.io/expose-password": "'{.data[''database-password'']}'", - "template.openshift.io/expose-username": "'{.data[''database-user'']}'", - }, + namespace=model_registry_namespace, + string_data=MODEL_REGISTRY_DB_SECRET_STR_DATA, + label=get_model_registry_db_label_dict(db_resource_name=DB_RESOURCES_NAME), + annotations=MODEL_REGISTRY_DB_SECRET_ANNOTATIONS, ) as mr_db_secret: yield mr_db_secret @@ -132,122 +113,25 @@ def model_registry_db_secret( @pytest.fixture(scope="class") def model_registry_db_deployment( admin_client: DynamicClient, - model_registry_namespace: Namespace, + model_registry_namespace: str, model_registry_db_secret: Secret, model_registry_db_pvc: PersistentVolumeClaim, model_registry_db_service: Service, ) -> Generator[Deployment, Any, Any]: with Deployment( name=DB_RESOURCES_NAME, - namespace=model_registry_namespace.name, + namespace=model_registry_namespace, annotations={ "template.alpha.openshift.io/wait-for-ready": "true", }, - label=DEFAULT_LABEL_DICT_DB, + label=get_model_registry_db_label_dict(db_resource_name=DB_RESOURCES_NAME), replicas=1, revision_history_limit=0, selector={"matchLabels": {"name": DB_RESOURCES_NAME}}, strategy={"type": "Recreate"}, - template={ - "metadata": { - "labels": { - "name": DB_RESOURCES_NAME, - "sidecar.istio.io/inject": "false", - } - }, - "spec": { - "containers": [ - { - "env": [ - { - "name": "MYSQL_USER", - "valueFrom": { - "secretKeyRef": { - "key": "database-user", - "name": f"{model_registry_db_secret.name}", - } - }, - }, - { - "name": "MYSQL_PASSWORD", - "valueFrom": { - "secretKeyRef": { - "key": "database-password", - "name": f"{model_registry_db_secret.name}", - } - }, - }, - { - "name": "MYSQL_ROOT_PASSWORD", - "valueFrom": { - "secretKeyRef": { - "key": "database-password", - "name": f"{model_registry_db_secret.name}", - } - }, - }, - { - "name": "MYSQL_DATABASE", - "valueFrom": { - "secretKeyRef": { - "key": "database-name", - "name": f"{model_registry_db_secret.name}", - } - }, - }, - ], - "args": [ - "--datadir", - "/var/lib/mysql/datadir", - "--default-authentication-plugin=mysql_native_password", - ], - "image": MR_DB_IMAGE_DIGEST, - "imagePullPolicy": "IfNotPresent", - "livenessProbe": { - "exec": { - "command": [ - "/bin/bash", - "-c", - "mysqladmin -u${MYSQL_USER} -p${MYSQL_ROOT_PASSWORD} ping", - ] - }, - "initialDelaySeconds": 15, - "periodSeconds": 10, - "timeoutSeconds": 5, - }, - "name": "mysql", - "ports": [{"containerPort": 3306, "protocol": "TCP"}], - "readinessProbe": { - "exec": { - "command": [ - "/bin/bash", - "-c", - 'mysql -D ${MYSQL_DATABASE} -u${MYSQL_USER} -p${MYSQL_ROOT_PASSWORD} -e "SELECT 1"', - ] - }, - "initialDelaySeconds": 10, - "timeoutSeconds": 5, - }, - "securityContext": {"capabilities": {}, "privileged": False}, - "terminationMessagePath": "/dev/termination-log", - "volumeMounts": [ - { - "mountPath": "/var/lib/mysql", - "name": f"{DB_RESOURCES_NAME}-data", - } - ], - } - ], - "dnsPolicy": "ClusterFirst", - "restartPolicy": "Always", - "volumes": [ - { - "name": f"{DB_RESOURCES_NAME}-data", - "persistentVolumeClaim": {"claimName": DB_RESOURCES_NAME}, - } - ], - }, - }, + template=get_model_registry_deployment_template_dict( + secret_name=model_registry_db_secret.name, resource_name=DB_RESOURCES_NAME + ), wait_for_resource=True, ) as mr_db_deployment: mr_db_deployment.wait_for_replicas(deployed=True) @@ -257,14 +141,14 @@ def model_registry_db_deployment( @pytest.fixture(scope="class") def model_registry_instance( admin_client: DynamicClient, - model_registry_namespace: Namespace, + model_registry_namespace: str, model_registry_db_deployment: Deployment, model_registry_db_secret: Secret, model_registry_db_service: Service, ) -> Generator[ModelRegistry, Any, Any]: with ModelRegistry( name=MR_INSTANCE_NAME, - namespace=model_registry_namespace.name, + namespace=model_registry_namespace, label={ Annotations.KubernetesIo.NAME: MR_INSTANCE_NAME, Annotations.KubernetesIo.INSTANCE: MR_INSTANCE_NAME, @@ -291,11 +175,11 @@ def model_registry_instance( @pytest.fixture(scope="class") def model_registry_instance_service( admin_client: DynamicClient, - model_registry_namespace: Namespace, + model_registry_namespace: str, model_registry_instance: ModelRegistry, ) -> Service: return get_mr_service_by_label( - client=admin_client, ns=model_registry_namespace, mr_instance=model_registry_instance + client=admin_client, ns=Namespace(name=model_registry_namespace), mr_instance=model_registry_instance ) @@ -341,13 +225,17 @@ def after_call(self, response: Response, case: Case) -> None: @pytest.fixture(scope="class") def updated_dsc_component_state_scope_class( request: FixtureRequest, - model_registry_namespace: Namespace, dsc_resource: DataScienceCluster, ) -> Generator[DataScienceCluster, Any, Any]: original_components = dsc_resource.instance.spec.components with ResourceEditor(patches={dsc_resource: {"spec": {"components": request.param["component_patch"]}}}): for component_name in request.param["component_patch"]: dsc_resource.wait_for_condition(condition=DscComponents.COMPONENT_MAPPING[component_name], status="True") + if request.param["component_patch"].get(DscComponents.MODELREGISTRY): + namespace = Namespace( + name=dsc_resource.instance.spec.components.modelregistry.registriesNamespace, ensure_exists=True + ) + namespace.wait_for_status(status=Namespace.Status.ACTIVE) yield dsc_resource for component_name, value in request.param["component_patch"].items(): diff --git a/tests/model_registry/constants.py b/tests/model_registry/constants.py index 6e3f8b0f8..0f54d4a22 100644 --- a/tests/model_registry/constants.py +++ b/tests/model_registry/constants.py @@ -1,5 +1,5 @@ from typing import Any - +from ocp_resources.resource import Resource from utilities.constants import ModelFormat @@ -35,3 +35,13 @@ class ModelRegistryEndpoints: MR_DB_IMAGE_DIGEST: str = ( "public.ecr.aws/docker/library/mysql@sha256:9de9d54fecee6253130e65154b930978b1fcc336bcc86dfd06e89b72a2588ebe" ) +MODEL_REGISTRY_DB_SECRET_STR_DATA = { + "database-name": "model_registry", + "database-password": "TheBlurstOfTimes", # pragma: allowlist secret + "database-user": "mlmduser", # pragma: allowlist secret +} +MODEL_REGISTRY_DB_SECRET_ANNOTATIONS = { + f"{Resource.ApiGroup.TEMPLATE_OPENSHIFT_IO}/expose-database_name": "'{.data[''database-name'']}'", + f"{Resource.ApiGroup.TEMPLATE_OPENSHIFT_IO}/expose-password": "'{.data[''database-password'']}'", + f"{Resource.ApiGroup.TEMPLATE_OPENSHIFT_IO}/expose-username": "'{.data[''database-user'']}'", +} diff --git a/tests/model_registry/negative_tests/__init__.py b/tests/model_registry/negative_tests/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/tests/model_registry/negative_tests/conftest.py b/tests/model_registry/negative_tests/conftest.py new file mode 100644 index 000000000..a57ea9a35 --- /dev/null +++ b/tests/model_registry/negative_tests/conftest.py @@ -0,0 +1,121 @@ +import pytest +from typing import Generator, Any +from ocp_resources.secret import Secret +from ocp_resources.namespace import Namespace +from ocp_resources.service import Service +from ocp_resources.persistent_volume_claim import PersistentVolumeClaim +from ocp_resources.deployment import Deployment + +from pytest import FixtureRequest +from kubernetes.dynamic import DynamicClient + + +from tests.model_registry.constants import ( + MODEL_REGISTRY_DB_SECRET_STR_DATA, + MODEL_REGISTRY_DB_SECRET_ANNOTATIONS, +) +from tests.model_registry.negative_tests.constants import CUSTOM_NEGATIVE_NS +from tests.model_registry.utils import get_model_registry_deployment_template_dict, get_model_registry_db_label_dict +from utilities.infra import create_ns + +DB_RESOURCES_NAME_NEGATIVE = "db-model-registry-negative" + + +@pytest.fixture(scope="class") +def model_registry_namespace_for_negative_tests( + request: FixtureRequest, admin_client: DynamicClient +) -> Generator[Namespace, Any, Any]: + with create_ns( + name=request.param.get("namespace_name", CUSTOM_NEGATIVE_NS), + admin_client=admin_client, + ) as ns: + yield ns + + +@pytest.fixture(scope="class") +def model_registry_db_service_for_negative_tests( + admin_client: DynamicClient, model_registry_namespace_for_negative_tests: Namespace +) -> Generator[Service, Any, Any]: + with Service( + client=admin_client, + name=DB_RESOURCES_NAME_NEGATIVE, + namespace=model_registry_namespace_for_negative_tests.name, + ports=[ + { + "name": "mysql", + "nodePort": 0, + "port": 3306, + "protocol": "TCP", + "appProtocol": "tcp", + "targetPort": 3306, + } + ], + selector={ + "name": DB_RESOURCES_NAME_NEGATIVE, + }, + label=get_model_registry_db_label_dict(db_resource_name=DB_RESOURCES_NAME_NEGATIVE), + annotations={ + "template.openshift.io/expose-uri": r"mysql://{.spec.clusterIP}:{.spec.ports[?(.name==\mysql\)].port}", + }, + ) as mr_db_service: + yield mr_db_service + + +@pytest.fixture(scope="class") +def model_registry_db_pvc_for_negative_tests( + admin_client: DynamicClient, + model_registry_namespace_for_negative_tests: Namespace, +) -> Generator[PersistentVolumeClaim, Any, Any]: + with PersistentVolumeClaim( + accessmodes="ReadWriteOnce", + name=DB_RESOURCES_NAME_NEGATIVE, + namespace=model_registry_namespace_for_negative_tests.name, + client=admin_client, + size="5Gi", + label=get_model_registry_db_label_dict(db_resource_name=DB_RESOURCES_NAME_NEGATIVE), + ) as pvc: + yield pvc + + +@pytest.fixture(scope="class") +def model_registry_db_secret_negative_test( + admin_client: DynamicClient, + model_registry_namespace_for_negative_tests: Namespace, +) -> Generator[Secret, Any, Any]: + with Secret( + client=admin_client, + name=DB_RESOURCES_NAME_NEGATIVE, + namespace=model_registry_namespace_for_negative_tests.name, + string_data=MODEL_REGISTRY_DB_SECRET_STR_DATA, + label=get_model_registry_db_label_dict(db_resource_name=DB_RESOURCES_NAME_NEGATIVE), + annotations=MODEL_REGISTRY_DB_SECRET_ANNOTATIONS, + ) as mr_db_secret: + yield mr_db_secret + + +@pytest.fixture(scope="class") +def model_registry_db_deployment_negative_test( + admin_client: DynamicClient, + model_registry_namespace_for_negative_tests: Namespace, + model_registry_db_secret_negative_test: Secret, + model_registry_db_pvc_for_negative_tests: PersistentVolumeClaim, + model_registry_db_service_for_negative_tests: Service, +) -> Generator[Deployment, Any, Any]: + with Deployment( + name=DB_RESOURCES_NAME_NEGATIVE, + namespace=model_registry_namespace_for_negative_tests.name, + annotations={ + "template.alpha.openshift.io/wait-for-ready": "true", + }, + label=get_model_registry_db_label_dict(db_resource_name=DB_RESOURCES_NAME_NEGATIVE), + replicas=1, + revision_history_limit=0, + selector={"matchLabels": {"name": DB_RESOURCES_NAME_NEGATIVE}}, + strategy={"type": "Recreate"}, + template=get_model_registry_deployment_template_dict( + secret_name=model_registry_db_secret_negative_test.name, resource_name=DB_RESOURCES_NAME_NEGATIVE + ), + wait_for_resource=True, + ) as mr_db_deployment: + mr_db_deployment.wait_for_replicas(deployed=True) + yield mr_db_deployment diff --git a/tests/model_registry/negative_tests/constants.py b/tests/model_registry/negative_tests/constants.py new file mode 100644 index 000000000..df66cb7b4 --- /dev/null +++ b/tests/model_registry/negative_tests/constants.py @@ -0,0 +1 @@ +CUSTOM_NEGATIVE_NS = "model-registry-negative-ns" diff --git a/tests/model_registry/test_model_registry_creation_negative.py b/tests/model_registry/negative_tests/test_model_registry_creation_negative.py similarity index 75% rename from tests/model_registry/test_model_registry_creation_negative.py rename to tests/model_registry/negative_tests/test_model_registry_creation_negative.py index dc5c82421..be23b7184 100644 --- a/tests/model_registry/test_model_registry_creation_negative.py +++ b/tests/model_registry/negative_tests/test_model_registry_creation_negative.py @@ -1,12 +1,13 @@ import pytest from typing import Self from simple_logger.logger import get_logger - from ocp_resources.data_science_cluster import DataScienceCluster from ocp_resources.deployment import Deployment from ocp_resources.model_registry import ModelRegistry + from ocp_resources.namespace import Namespace from ocp_resources.secret import Secret +from tests.model_registry.negative_tests.constants import CUSTOM_NEGATIVE_NS from utilities.constants import DscComponents, Annotations from tests.model_registry.constants import ( MR_NAMESPACE, @@ -17,12 +18,12 @@ ) from kubernetes.dynamic.exceptions import UnprocessibleEntityError + LOGGER = get_logger(name=__name__) -CUSTOM_NEGATIVE_NS = "model-registry-negative-ns" @pytest.mark.parametrize( - "model_registry_namespace, updated_dsc_component_state_scope_class, expected_namespace", + "model_registry_namespace_for_negative_tests, updated_dsc_component_state_scope_class, expected_namespace", [ pytest.param( {"namespace_name": CUSTOM_NEGATIVE_NS}, @@ -49,25 +50,26 @@ CUSTOM_NEGATIVE_NS, ), ], - indirect=["model_registry_namespace", "updated_dsc_component_state_scope_class"], + indirect=["model_registry_namespace_for_negative_tests", "updated_dsc_component_state_scope_class"], ) class TestModelRegistryCreationNegative: def test_registering_model_negative( self: Self, current_client_token: str, - model_registry_namespace: Namespace, + model_registry_namespace_for_negative_tests: Namespace, updated_dsc_component_state_scope_class: DataScienceCluster, - model_registry_db_deployment: Deployment, - model_registry_db_secret: Secret, + model_registry_db_secret_negative_test: Secret, + model_registry_db_deployment_negative_test: Deployment, expected_namespace: str, ): my_sql_dict: dict[str, str] = { - "host": f"{model_registry_db_deployment.name}.{model_registry_db_deployment.namespace}.svc.cluster.local", - "database": model_registry_db_secret.string_data["database-name"], + "host": f"{model_registry_db_deployment_negative_test.name}." + f"{model_registry_db_deployment_negative_test.namespace}.svc.cluster.local", + "database": model_registry_db_secret_negative_test.string_data["database-name"], "passwordSecret": {"key": "database-password", "name": DB_RESOURCES_NAME}, "port": 3306, "skipDBCreation": False, - "username": model_registry_db_secret.string_data["database-user"], + "username": model_registry_db_secret_negative_test.string_data["database-user"], } with pytest.raises( UnprocessibleEntityError, @@ -75,7 +77,7 @@ def test_registering_model_negative( ): with ModelRegistry( name=MR_INSTANCE_NAME, - namespace=model_registry_namespace.name, + namespace=model_registry_namespace_for_negative_tests.name, label={ Annotations.KubernetesIo.NAME: MR_INSTANCE_NAME, Annotations.KubernetesIo.INSTANCE: MR_INSTANCE_NAME, diff --git a/tests/model_registry/python_client/__init__.py b/tests/model_registry/python_client/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/tests/model_registry/test_model_registry_creation.py b/tests/model_registry/python_client/test_model_registry_creation.py similarity index 77% rename from tests/model_registry/test_model_registry_creation.py rename to tests/model_registry/python_client/test_model_registry_creation.py index 5b12a0d89..83ff33cfc 100644 --- a/tests/model_registry/test_model_registry_creation.py +++ b/tests/model_registry/python_client/test_model_registry_creation.py @@ -2,14 +2,12 @@ from typing import Self from simple_logger.logger import get_logger -from ocp_resources.data_science_cluster import DataScienceCluster from ocp_resources.pod import Pod from ocp_resources.namespace import Namespace from utilities.constants import DscComponents -from tests.model_registry.constants import MR_NAMESPACE, MODEL_NAME, MODEL_DICT +from tests.model_registry.constants import MODEL_NAME, MODEL_DICT, MR_NAMESPACE from model_registry import ModelRegistry as ModelRegistryClient from model_registry.types import RegisteredModel -from kubernetes.dynamic import DynamicClient LOGGER = get_logger(name=__name__) @@ -17,10 +15,9 @@ @pytest.mark.parametrize( - "model_registry_namespace, updated_dsc_component_state_scope_class", + "updated_dsc_component_state_scope_class, registered_model", [ pytest.param( - {"namespace_name": CUSTOM_NAMESPACE}, { "component_patch": { DscComponents.MODELREGISTRY: { @@ -29,9 +26,9 @@ }, } }, + MODEL_DICT, ), pytest.param( - {"namespace_name": MR_NAMESPACE}, { "component_patch": { DscComponents.MODELREGISTRY: { @@ -40,11 +37,12 @@ }, }, }, + MODEL_DICT, ), ], indirect=True, ) -@pytest.mark.usefixtures("model_registry_namespace", "updated_dsc_component_state_scope_class") +@pytest.mark.usefixtures("updated_dsc_component_state_scope_class", "registered_model") class TestModelRegistryCreation: """ Tests the creation of a model registry. If the component is set to 'Removed' it will be switched to 'Managed' @@ -52,15 +50,6 @@ class TestModelRegistryCreation: """ @pytest.mark.smoke - @pytest.mark.parametrize( - "registered_model", - [ - pytest.param( - MODEL_DICT, - ) - ], - indirect=True, - ) def test_registering_model( self: Self, model_registry_client: ModelRegistryClient, @@ -84,15 +73,14 @@ def test_registering_model( def test_model_registry_operator_env( self, - admin_client: DynamicClient, - model_registry_namespace: Namespace, - updated_dsc_component_state_scope_class: DataScienceCluster, + updated_dsc_component_state_scope_class: Namespace, + model_registry_namespace: str, model_registry_operator_pod: Pod, ): namespace_env = [] for container in model_registry_operator_pod.instance.spec.containers: for env in container.env: - if env.name == "REGISTRIES_NAMESPACE" and env.value == model_registry_namespace.name: + if env.name == "REGISTRIES_NAMESPACE" and env.value == model_registry_namespace: namespace_env.append({container.name: env}) if not namespace_env: pytest.fail("Missing environment variable REGISTRIES_NAMESPACE") diff --git a/tests/model_registry/scc/test_model_registry_scc.py b/tests/model_registry/scc/test_model_registry_scc.py index 9f355e1f0..cf3d08c17 100644 --- a/tests/model_registry/scc/test_model_registry_scc.py +++ b/tests/model_registry/scc/test_model_registry_scc.py @@ -6,7 +6,6 @@ from ocp_resources.namespace import Namespace from ocp_resources.pod import Pod from ocp_resources.deployment import Deployment -from ocp_resources.resource import NamespacedResource from tests.model_registry.scc.utils import ( get_uid_from_namespace, validate_pod_security_context, @@ -23,8 +22,8 @@ @pytest.fixture(scope="class") -def model_registry_scc_namespace(model_registry_namespace: Namespace): - mr_annotations = model_registry_namespace.instance.metadata.annotations +def model_registry_scc_namespace(model_registry_namespace: str): + mr_annotations = Namespace(name=model_registry_namespace).instance.metadata.annotations return { "seLinuxOptions": mr_annotations.get("openshift.io/sa.scc.mcs"), "uid-range": mr_annotations.get("openshift.io/sa.scc.uid-range"), @@ -33,13 +32,13 @@ def model_registry_scc_namespace(model_registry_namespace: Namespace): @pytest.fixture(scope="class") def model_registry_resource( - request: FixtureRequest, admin_client: DynamicClient, model_registry_namespace: Namespace -) -> NamespacedResource: + request: FixtureRequest, admin_client: DynamicClient, model_registry_namespace: str +) -> Deployment | Pod: if request.param["kind"] == Deployment: - return Deployment(name=MR_INSTANCE_NAME, namespace=model_registry_namespace.name, ensure_exists=True) + return Deployment(name=MR_INSTANCE_NAME, namespace=model_registry_namespace, ensure_exists=True) elif request.param["kind"] == Pod: pods = get_pods_by_name_prefix( - client=admin_client, pod_prefix=MR_INSTANCE_NAME, namespace=model_registry_namespace.name + client=admin_client, pod_prefix=MR_INSTANCE_NAME, namespace=model_registry_namespace ) if len(pods) != 1: pytest.fail( @@ -51,10 +50,9 @@ def model_registry_resource( @pytest.mark.parametrize( - "model_registry_namespace, updated_dsc_component_state_scope_class, registered_model", + "updated_dsc_component_state_scope_class, registered_model", [ pytest.param( - {"namespace_name": MR_NAMESPACE}, { "component_patch": { DscComponents.MODELREGISTRY: { @@ -68,7 +66,7 @@ def model_registry_resource( ], indirect=True, ) -@pytest.mark.usefixtures("model_registry_namespace", "updated_dsc_component_state_scope_class", "registered_model") +@pytest.mark.usefixtures("updated_dsc_component_state_scope_class", "registered_model") class TestModelRegistrySecurityContextValidation: @pytest.mark.parametrize( "model_registry_resource", @@ -79,8 +77,7 @@ class TestModelRegistrySecurityContextValidation: ) def test_model_registry_deployment_security_context_validation( self: Self, - model_registry_resource: NamespacedResource, - model_registry_namespace: Namespace, + model_registry_resource: Deployment, ): """ Validate that model registry deployment does not set runAsUser/runAsGroup @@ -105,7 +102,7 @@ def test_model_registry_deployment_security_context_validation( ) def test_model_registry_pod_security_context_validation( self: Self, - model_registry_resource: NamespacedResource, + model_registry_resource: Pod, model_registry_scc_namespace: dict[str, str], ): """ diff --git a/tests/model_registry/test_rest_api_stateful.py b/tests/model_registry/test_rest_api_stateful.py index 4487ce4be..50712c84d 100644 --- a/tests/model_registry/test_rest_api_stateful.py +++ b/tests/model_registry/test_rest_api_stateful.py @@ -8,10 +8,9 @@ @pytest.mark.fuzzer @pytest.mark.parametrize( - "model_registry_namespace, updated_dsc_component_state_scope_class", + "updated_dsc_component_state_scope_class", [ pytest.param( - {"namespace_name": MR_NAMESPACE}, { "component_patch": { DscComponents.MODELREGISTRY: { @@ -24,7 +23,7 @@ ], indirect=True, ) -@pytest.mark.usefixtures("model_registry_namespace", "updated_dsc_component_state_scope_class") +@pytest.mark.usefixtures("updated_dsc_component_state_scope_class") class TestRestAPIStateful: def test_mr_api_stateful(self, state_machine): """Launches stateful tests against the Model Registry API endpoints defined in its openAPI yaml spec file""" diff --git a/tests/model_registry/utils.py b/tests/model_registry/utils.py index 74e43b433..4282bbd6c 100644 --- a/tests/model_registry/utils.py +++ b/tests/model_registry/utils.py @@ -1,12 +1,14 @@ +from typing import Any + from kubernetes.dynamic import DynamicClient from ocp_resources.namespace import Namespace from ocp_resources.service import Service from ocp_resources.model_registry import ModelRegistry from kubernetes.dynamic.exceptions import ResourceNotFoundError +from tests.model_registry.constants import MR_DB_IMAGE_DIGEST from utilities.exceptions import ProtocolNotSupportedError, TooManyServicesError -from utilities.constants import Protocols - +from utilities.constants import Protocols, Annotations ADDRESS_ANNOTATION_PREFIX: str = "routing.opendatahub.io/external-address-" @@ -43,3 +45,114 @@ def get_endpoint_from_mr_service(client: DynamicClient, svc: Service, protocol: return svc.instance.metadata.annotations[f"{ADDRESS_ANNOTATION_PREFIX}{protocol}"] else: raise ProtocolNotSupportedError(protocol) + + +def get_model_registry_deployment_template_dict(secret_name: str, resource_name: str) -> dict[str, Any]: + return { + "metadata": { + "labels": { + "name": resource_name, + "sidecar.istio.io/inject": "false", + } + }, + "spec": { + "containers": [ + { + "env": [ + { + "name": "MYSQL_USER", + "valueFrom": { + "secretKeyRef": { + "key": "database-user", + "name": secret_name, + } + }, + }, + { + "name": "MYSQL_PASSWORD", + "valueFrom": { + "secretKeyRef": { + "key": "database-password", + "name": secret_name, + } + }, + }, + { + "name": "MYSQL_ROOT_PASSWORD", + "valueFrom": { + "secretKeyRef": { + "key": "database-password", + "name": secret_name, + } + }, + }, + { + "name": "MYSQL_DATABASE", + "valueFrom": { + "secretKeyRef": { + "key": "database-name", + "name": secret_name, + } + }, + }, + ], + "args": [ + "--datadir", + "/var/lib/mysql/datadir", + "--default-authentication-plugin=mysql_native_password", + ], + "image": MR_DB_IMAGE_DIGEST, + "imagePullPolicy": "IfNotPresent", + "livenessProbe": { + "exec": { + "command": [ + "/bin/bash", + "-c", + "mysqladmin -u${MYSQL_USER} -p${MYSQL_ROOT_PASSWORD} ping", + ] + }, + "initialDelaySeconds": 15, + "periodSeconds": 10, + "timeoutSeconds": 5, + }, + "name": "mysql", + "ports": [{"containerPort": 3306, "protocol": "TCP"}], + "readinessProbe": { + "exec": { + "command": [ + "/bin/bash", + "-c", + 'mysql -D ${MYSQL_DATABASE} -u${MYSQL_USER} -p${MYSQL_ROOT_PASSWORD} -e "SELECT 1"', + ] + }, + "initialDelaySeconds": 10, + "timeoutSeconds": 5, + }, + "securityContext": {"capabilities": {}, "privileged": False}, + "terminationMessagePath": "/dev/termination-log", + "volumeMounts": [ + { + "mountPath": "/var/lib/mysql", + "name": f"{resource_name}-data", + } + ], + } + ], + "dnsPolicy": "ClusterFirst", + "restartPolicy": "Always", + "volumes": [ + { + "name": f"{resource_name}-data", + "persistentVolumeClaim": {"claimName": resource_name}, + } + ], + }, + } + + +def get_model_registry_db_label_dict(db_resource_name: str) -> dict[str, str]: + return { + Annotations.KubernetesIo.NAME: db_resource_name, + Annotations.KubernetesIo.INSTANCE: db_resource_name, + Annotations.KubernetesIo.PART_OF: db_resource_name, + }