From 9ad39cb80be2386ce74de1f8ea08f4cb6eb3d595 Mon Sep 17 00:00:00 2001 From: Isabella Basso Date: Wed, 16 Oct 2024 05:28:18 -0300 Subject: [PATCH] use deployed MR for python e2e tests (#447) * use deployed MR for python tests Signed-off-by: Alessio Pragliola Co-authored-by: Alessio Pragliola Signed-off-by: Isabella do Amaral * scripts: add clarifications Signed-off-by: Matteo Mortari Co-authored-by: Matteo Mortari Signed-off-by: Isabella do Amaral * script: make deploy_on_kind idempotent Signed-off-by: Alessio Pragliola Co-authored-by: Alessio Pragliola Signed-off-by: Isabella do Amaral * Update scripts/deploy_on_kind.sh Co-authored-by: Alessio Pragliola <83355398+Al-Pragliola@users.noreply.github.com> Signed-off-by: Isabella Basso --------- Signed-off-by: Alessio Pragliola Signed-off-by: Isabella do Amaral Signed-off-by: Matteo Mortari Signed-off-by: Isabella Basso Co-authored-by: Alessio Pragliola Co-authored-by: Matteo Mortari Co-authored-by: Alessio Pragliola <83355398+Al-Pragliola@users.noreply.github.com> --- .github/workflows/build-image-pr.yml | 26 ++--- .github/workflows/python-tests.yml | 149 +++++++++++++++++++++--- clients/python/Makefile | 10 +- clients/python/noxfile.py | 45 ++++--- clients/python/poetry.lock | 1 - clients/python/tests/conftest.py | 61 ++-------- clients/python/tests/regression_test.py | 2 +- clients/python/tests/test_client.py | 3 + scripts/cleanup.sh | 35 ++++++ scripts/deploy_on_kind.sh | 38 ++++++ 10 files changed, 263 insertions(+), 107 deletions(-) create mode 100755 scripts/cleanup.sh create mode 100755 scripts/deploy_on_kind.sh diff --git a/.github/workflows/build-image-pr.yml b/.github/workflows/build-image-pr.yml index 14ff73f5..01b79ca4 100644 --- a/.github/workflows/build-image-pr.yml +++ b/.github/workflows/build-image-pr.yml @@ -1,5 +1,6 @@ name: Test container image build and deployment on: + workflow_dispatch: pull_request: paths-ignore: - "LICENSE*" @@ -9,7 +10,7 @@ on: - ".github/ISSUE_TEMPLATE/**" - ".github/dependabot.yml" - "docs/**" - - "clients/python/docs/**" + - "clients/python/**" env: IMG_ORG: kubeflow IMG_REPO: model-registry @@ -30,8 +31,8 @@ jobs: - name: Build Image shell: bash env: - VERSION: ${{ steps.tags.outputs.tag }} - run: ./scripts/build_deploy.sh + IMG_VERSION: ${{ steps.tags.outputs.tag }} + run: make image/build - name: Start Kind Cluster uses: helm/kind-action@v1.10.0 with: @@ -41,25 +42,12 @@ jobs: IMG: "${{ env.IMG_ORG }}/${{ env.IMG_REPO }}:${{ steps.tags.outputs.tag }}" run: | kind load docker-image -n chart-testing ${IMG} - - name: Create Test Registry + - name: Deploy Model Registry using manifests env: IMG: "${{ env.IMG_ORG }}/${{ env.IMG_REPO }}:${{ steps.tags.outputs.tag }}" + run: ./scripts/deploy_on_kind.sh + - name: Deployment logs run: | - echo "Download kustomize 5.2.1" - mkdir $GITHUB_WORKSPACE/kustomize - curl -s "https://raw.githubusercontent.com/kubernetes-sigs/kustomize/master/hack/install_kustomize.sh" | bash -s "5.2.1" "$GITHUB_WORKSPACE/kustomize" - PATH=$GITHUB_WORKSPACE/kustomize:$PATH - echo "Display Kustomize version" - kustomize version - echo "Deploying Model Registry using Manifests; branch ${BRANCH}" - kubectl create namespace kubeflow - cd manifests/kustomize/overlays/db - kustomize edit set image kubeflow/model-registry:latest $IMG - kustomize build | kubectl apply -f - - - name: Wait for Test Registry Deployment - run: | - kubectl wait --for=condition=available -n kubeflow deployment/model-registry-db --timeout=5m - kubectl wait --for=condition=available -n kubeflow deployment/model-registry-deployment --timeout=5m kubectl logs -n kubeflow deployment/model-registry-deployment - name: Set up Python uses: actions/setup-python@v5 diff --git a/.github/workflows/python-tests.yml b/.github/workflows/python-tests.yml index 750a3b5f..90cc4482 100644 --- a/.github/workflows/python-tests.yml +++ b/.github/workflows/python-tests.yml @@ -3,6 +3,7 @@ on: push: branches: - "main" + workflow_dispatch: pull_request: paths-ignore: - "LICENSE*" @@ -12,26 +13,19 @@ on: - ".github/ISSUE_TEMPLATE/**" - ".github/dependabot.yml" - "docs/**" + jobs: - tests: - name: ${{ matrix.session }} ${{ matrix.python }} + lint: + name: ${{ matrix.session }} runs-on: ubuntu-latest strategy: fail-fast: false matrix: python: ["3.12"] - session: [lint, tests, mypy, docs-build] - include: - - python: "3.9" - session: tests - - python: "3.10" - session: tests - - python: "3.11" - session: tests + session: [lint, mypy] env: NOXSESSION: ${{ matrix.session }} FORCE_COLOR: "1" - PRE_COMMIT_COLOR: "always" steps: - name: Check out the repository uses: actions/checkout@v4 @@ -61,27 +55,144 @@ jobs: pipx install --pip-args=--constraint=${{ github.workspace }}/.github/workflows/constraints.txt nox pipx inject --pip-args=--constraint=${{ github.workspace }}/.github/workflows/constraints.txt nox nox-poetry nox --version - - name: Run Nox + - name: Nox lint working-directory: clients/python run: | - if [[ ${{ matrix.session }} == "tests" ]]; then - make build-mr - nox --python=${{ matrix.python }} -- --cov-report=xml - poetry build - elif [[ ${{ matrix.session }} == "mypy" ]]; then + if [[ ${{ matrix.session }} == "mypy" ]]; then nox --python=${{ matrix.python }} ||\ echo "::error title='mypy failure'::Check the logs for more details" else nox --python=${{ matrix.python }} fi + + test: + name: Test against Py ${{ matrix.python }} + runs-on: ubuntu-latest + strategy: + fail-fast: false + matrix: + python: ["3.12", "3.11", "3.10", "3.9"] + env: + FORCE_COLOR: "1" + IMG_ORG: kubeflow + IMG_REPO: model-registry + steps: + - name: Check out the repository + uses: actions/checkout@v4 + - name: Set up Python ${{ matrix.python }} + uses: actions/setup-python@v5 + with: + python-version: ${{ matrix.python }} + - name: Upgrade pip + run: | + pip install --constraint=.github/workflows/constraints.txt pip + pip --version + - name: Upgrade pip in virtual environments + shell: python + run: | + import os + import pip + + with open(os.environ["GITHUB_ENV"], mode="a") as io: + print(f"VIRTUALENV_PIP={pip.__version__}", file=io) + - name: Install Poetry + # use absolute path as recommended with: https://github.com/pypa/pipx/issues/1331 + run: | + pipx install --pip-args=--constraint=${{ github.workspace }}/.github/workflows/constraints.txt poetry + poetry --version + - name: Install Nox + run: | + pipx install --pip-args=--constraint=${{ github.workspace }}/.github/workflows/constraints.txt nox + pipx inject --pip-args=--constraint=${{ github.workspace }}/.github/workflows/constraints.txt nox nox-poetry + nox --version + - name: Nox test + working-directory: clients/python + run: | + nox --python=${{ matrix.python }} --session=tests + - name: Generate Tag + shell: bash + id: tags + run: | + commit_sha=${{ github.event.after }} + tag=main-${commit_sha:0:7} + echo "tag=${tag}" >> $GITHUB_OUTPUT + - name: Build Image + shell: bash + env: + IMG_VERSION: ${{ steps.tags.outputs.tag }} + run: make image/build + - name: Start Kind Cluster + uses: helm/kind-action@v1.10.0 + with: + node_image: "kindest/node:v1.27.11" + - name: Load Local Registry Test Image + if: matrix.session == 'tests' + env: + IMG: "docker.io/${{ env.IMG_ORG }}/${{ env.IMG_REPO }}:${{ steps.tags.outputs.tag }}" + run: | + kind load docker-image -n chart-testing ${IMG} + - name: Deploy Model Registry using manifests + env: + IMG: "docker.io/${{ env.IMG_ORG }}/${{ env.IMG_REPO }}:${{ steps.tags.outputs.tag }}" + run: ./scripts/deploy_on_kind.sh + - name: Nox test end-to-end + working-directory: clients/python + run: | + kubectl port-forward -n kubeflow service/model-registry-service 8080:8080 & + sleep 2 + nox --python=${{ matrix.python }} --session=e2e -- --cov-report=xml + + docs-build: + name: ${{ matrix.session }} + runs-on: ubuntu-latest + strategy: + fail-fast: false + matrix: + python: ["3.12"] + session: [docs-build] + env: + NOXSESSION: ${{ matrix.session }} + FORCE_COLOR: "1" + steps: + - name: Check out the repository + uses: actions/checkout@v4 + - name: Set up Python ${{ matrix.python }} + uses: actions/setup-python@v5 + with: + python-version: ${{ matrix.python }} + - name: Upgrade pip + run: | + pip install --constraint=.github/workflows/constraints.txt pip + pip --version + - name: Upgrade pip in virtual environments + shell: python + run: | + import os + import pip + + with open(os.environ["GITHUB_ENV"], mode="a") as io: + print(f"VIRTUALENV_PIP={pip.__version__}", file=io) + - name: Install Poetry + # use absolute path as recommended with: https://github.com/pypa/pipx/issues/1331 + run: | + pipx install --pip-args=--constraint=${{ github.workspace }}/.github/workflows/constraints.txt poetry + poetry --version + - name: Install Nox + run: | + pipx install --pip-args=--constraint=${{ github.workspace }}/.github/workflows/constraints.txt nox + pipx inject --pip-args=--constraint=${{ github.workspace }}/.github/workflows/constraints.txt nox nox-poetry + nox --version + - name: Run Nox + working-directory: clients/python + run: | + nox --python=${{ matrix.python }} + poetry build - name: Upload dist - if: matrix.session == 'tests' && matrix.python == '3.12' uses: actions/upload-artifact@v4 with: name: py-dist path: clients/python/dist - name: Upload documentation - if: matrix.session == 'docs-build' uses: actions/upload-artifact@v4 with: name: py-docs diff --git a/clients/python/Makefile b/clients/python/Makefile index e5412a9b..dde6612a 100644 --- a/clients/python/Makefile +++ b/clients/python/Makefile @@ -1,6 +1,5 @@ all: install tidy -IMG_REGISTRY ?= docker.io IMG_VERSION ?= latest .PHONY: install @@ -14,12 +13,13 @@ install: clean: rm -rf src/mr_openapi -.PHONY: build-mr -build-mr: - cd ../../ && IMG_REGISTRY=${IMG_REGISTRY} IMG_VERSION=${IMG_VERSION} make image/build +.PHONY: deploy-latest-mr +deploy-latest-mr: + cd ../../ && IMG_VERSION=${IMG_VERSION} make image/build && LOCAL=1 ./scripts/deploy_on_kind.sh + kubectl port-forward -n kubeflow services/model-registry-service 8080:8080 & .PHONY: test-e2e -test-e2e: build-mr +test-e2e: deploy-latest-mr poetry run pytest --e2e -s .PHONY: test diff --git a/clients/python/noxfile.py b/clients/python/noxfile.py index 195631e5..ae44d702 100644 --- a/clients/python/noxfile.py +++ b/clients/python/noxfile.py @@ -41,8 +41,8 @@ def lint(session: Session) -> None: @session(python=python_versions) def mypy(session: Session) -> None: """Type check using mypy.""" - session.install(".") session.install( + ".", "mypy", "types-python-dateutil", ) @@ -53,21 +53,31 @@ def mypy(session: Session) -> None: @session(python=python_versions) def tests(session: Session) -> None: """Run the test suite.""" - session.install(".") session.install( - "coverage[toml]", + ".", + "requests", + "pytest", + "pytest-asyncio", + ) + session.run( + "pytest", + *session.posargs, + ) + + +@session(name="e2e", python=python_versions) +def e2e_tests(session: Session) -> None: + """Run the test suite.""" + session.install( + ".", + "requests", "pytest", "pytest-asyncio", - "nest-asyncio", + "coverage[toml]", "pytest-cov", - "pygments", "huggingface-hub", ) try: - session.run( - "pytest", - *session.posargs, - ) session.run( "pytest", "--e2e", @@ -101,8 +111,12 @@ def docs_build(session: Session) -> None: if not session.posargs and "FORCE_COLOR" in os.environ: args.insert(0, "--color") - session.install(".") - session.install("sphinx", "myst-parser[linkify]", "furo") + session.install( + ".", + "sphinx", + "myst-parser[linkify]", + "furo", + ) build_dir = Path("docs", "_build") if build_dir.exists(): @@ -115,8 +129,13 @@ def docs_build(session: Session) -> None: def docs(session: Session) -> None: """Build and serve the documentation with live reloading on file changes.""" args = session.posargs or ["--open-browser", "docs", "docs/_build"] - session.install(".") - session.install("sphinx", "myst-parser[linkify]", "furo", "sphinx-autobuild") + session.install( + ".", + "sphinx", + "myst-parser[linkify]", + "furo", + "sphinx-autobuild", + ) build_dir = Path("docs", "_build") if build_dir.exists(): diff --git a/clients/python/poetry.lock b/clients/python/poetry.lock index ff41eb7f..b31414c2 100644 --- a/clients/python/poetry.lock +++ b/clients/python/poetry.lock @@ -1460,7 +1460,6 @@ files = [ {file = "PyYAML-6.0.1-cp311-cp311-win_amd64.whl", hash = "sha256:bf07ee2fef7014951eeb99f56f39c9bb4af143d8aa3c21b1677805985307da34"}, {file = "PyYAML-6.0.1-cp312-cp312-macosx_10_9_x86_64.whl", hash = "sha256:855fb52b0dc35af121542a76b9a84f8d1cd886ea97c84703eaa6d88e37a2ad28"}, {file = "PyYAML-6.0.1-cp312-cp312-macosx_11_0_arm64.whl", hash = "sha256:40df9b996c2b73138957fe23a16a4f0ba614f4c0efce1e9406a184b6d07fa3a9"}, - {file = "PyYAML-6.0.1-cp312-cp312-manylinux_2_17_aarch64.manylinux2014_aarch64.whl", hash = "sha256:a08c6f0fe150303c1c6b71ebcd7213c2858041a7e01975da3a99aed1e7a378ef"}, {file = "PyYAML-6.0.1-cp312-cp312-manylinux_2_17_x86_64.manylinux2014_x86_64.whl", hash = "sha256:6c22bec3fbe2524cde73d7ada88f6566758a8f7227bfbf93a408a9d86bcc12a0"}, {file = "PyYAML-6.0.1-cp312-cp312-musllinux_1_1_x86_64.whl", hash = "sha256:8d4e9c88387b0f5c7d5f281e55304de64cf7f9c0021a3525bd3b1c542da3b0e4"}, {file = "PyYAML-6.0.1-cp312-cp312-win32.whl", hash = "sha256:d483d2cdf104e7c9fa60c544d92981f12ad66a457afae824d146093b8c294c54"}, diff --git a/clients/python/tests/conftest.py b/clients/python/tests/conftest.py index b480379c..d92628ef 100644 --- a/clients/python/tests/conftest.py +++ b/clients/python/tests/conftest.py @@ -6,7 +6,7 @@ import time from contextlib import asynccontextmanager from pathlib import Path -from time import sleep +from urllib.parse import urlparse import pytest import requests @@ -29,13 +29,14 @@ def pytest_collection_modifyitems(config, items): continue -REGISTRY_HOST = "http://localhost" -REGISTRY_PORT = 8080 -REGISTRY_URL = f"{REGISTRY_HOST}:{REGISTRY_PORT}" -COMPOSE_FILE = "docker-compose.yaml" -MAX_POLL_TIME = 1200 # the first build is extremely slow if using docker-compose-*local*.yaml for bootstrap of builder image +REGISTRY_URL = os.environ.get("MR_URL", "http://localhost:8080") +parsed = urlparse(REGISTRY_URL) +host, port = parsed.netloc.split(":") +REGISTRY_HOST = f"{parsed.scheme}://{host}" +REGISTRY_PORT = int(port) + +MAX_POLL_TIME = 10 POLL_INTERVAL = 1 -DOCKER = os.getenv("DOCKER", "docker") start_time = time.time() @@ -64,38 +65,8 @@ def poll_for_ready(): time.sleep(POLL_INTERVAL) -@pytest.fixture(scope="session") -def _compose_mr(root): - print("Assuming this is the Model Registry root directory:", root) - shared_volume = root / "test/config/ml-metadata" - sqlite_db_file = shared_volume / "metadata.sqlite.db" - if sqlite_db_file.exists(): - msg = f"The file {sqlite_db_file} already exists; make sure to cancel it before running these tests." - raise FileExistsError(msg) - print(f" Starting Docker Compose in folder {root}") - p = subprocess.Popen( # noqa: S602 - f"{DOCKER} compose -f {COMPOSE_FILE} up", - shell=True, - cwd=root, - ) - yield - - p.kill() - print(f" Closing Docker Compose in folder {root}") - subprocess.call( # noqa: S602 - f"{DOCKER} compose -f {COMPOSE_FILE} down", - shell=True, - cwd=root, - ) - try: - os.remove(sqlite_db_file) - print(f"Removed {sqlite_db_file} successfully.") - except Exception as e: - print(f"An error occurred while removing {sqlite_db_file}: {e}") - - def cleanup(client): - async def yield_and_restart(_compose_mr, root): + async def yield_and_restart(root): poll_for_ready() if inspect.iscoroutinefunction(client) or inspect.isasyncgenfunction(client): async with asynccontextmanager(client)() as async_client: @@ -103,18 +74,9 @@ async def yield_and_restart(_compose_mr, root): else: yield client() - sqlite_db_file = root / "test/config/ml-metadata/metadata.sqlite.db" - try: - os.remove(sqlite_db_file) - print(f"Removed {sqlite_db_file} successfully.") - except Exception as e: - print(f"An error occurred while removing {sqlite_db_file}: {e}") - # we have to wait to make sure the server restarts after the file is gone - sleep(1) - - print("Restarting model-registry...") + print("Cleaning DB...") subprocess.call( # noqa: S602 - f"{DOCKER} compose -f {COMPOSE_FILE} restart model-registry", + "./scripts/cleanup.sh", shell=True, cwd=root, ) @@ -135,6 +97,7 @@ def event_loop(): def client() -> ModelRegistry: return ModelRegistry(REGISTRY_HOST, REGISTRY_PORT, author="author", is_secure=False) + @pytest.fixture(scope="module") def setup_env_user_token(): with tempfile.NamedTemporaryFile(delete=False) as token_file: diff --git a/clients/python/tests/regression_test.py b/clients/python/tests/regression_test.py index 035a2257..0310f387 100644 --- a/clients/python/tests/regression_test.py +++ b/clients/python/tests/regression_test.py @@ -26,7 +26,7 @@ def test_create_tagged_version(client: ModelRegistry): @pytest.mark.e2e -def test_get_model_without_user_token(setup_env_user_token, client): +def test_get_model_without_user_token(setup_env_user_token, client: ModelRegistry): """Test regression for using client methods without an user_token in the init arguments. Reported on: https://github.com/kubeflow/model-registry/issues/340 diff --git a/clients/python/tests/test_client.py b/clients/python/tests/test_client.py index bb958cf5..4cd93764 100644 --- a/clients/python/tests/test_client.py +++ b/clients/python/tests/test_client.py @@ -393,6 +393,9 @@ def test_get_model_versions(client: ModelRegistry): @pytest.mark.e2e +@pytest.mark.xfail( + reason="MLMD issue tracked on: https://github.com/kubeflow/model-registry/issues/358" +) def test_get_model_versions_order_by(client: ModelRegistry): name = "test_model" models = 5 diff --git a/scripts/cleanup.sh b/scripts/cleanup.sh new file mode 100755 index 00000000..a0f7208d --- /dev/null +++ b/scripts/cleanup.sh @@ -0,0 +1,35 @@ +#!/usr/bin/env bash + +set -e + +MR_NAMESPACE="${MR_NAMESPACE:-kubeflow}" +TEST_DB_NAME="${TEST_DB_NAME:-metadb}" + +# transaction start commands are different between sqlite and mysql +PARTIAL_SQL_CMD=$( + cat <