From 08ab1e57915b13242056682834006f91391cece3 Mon Sep 17 00:00:00 2001 From: Orfeas Kourkakis Date: Tue, 17 Sep 2024 16:05:48 +0300 Subject: [PATCH 1/9] ci: build charms in separate GH runners --- .github/workflows/integrate.yaml | 40 ++++++++++++++--- .../kfp-api/tests/integration/test_charm.py | 17 ++++++-- charms/kfp-api/tests/integration/utils.py | 43 +++++++++++++++++++ charms/kfp-api/tox.ini | 6 ++- .../tests/integration/test_charm.py | 18 ++++++-- .../tests/integration/utils.py | 43 +++++++++++++++++++ charms/kfp-metadata-writer/tox.ini | 6 ++- .../tests/integration/test_charm.py | 17 ++++++-- .../tests/integration/utils.py | 43 +++++++++++++++++++ charms/kfp-persistence/tox.ini | 6 ++- .../tests/integration/test_charm.py | 18 ++++++-- .../tests/integration/utils.py | 43 +++++++++++++++++++ charms/kfp-profile-controller/tox.ini | 6 ++- .../tests/integration/test_charm.py | 18 ++++++-- charms/kfp-schedwf/tests/integration/utils.py | 43 +++++++++++++++++++ charms/kfp-schedwf/tox.ini | 6 ++- charms/kfp-ui/tests/integration/test_charm.py | 18 ++++++-- charms/kfp-ui/tests/integration/utils.py | 43 +++++++++++++++++++ charms/kfp-ui/tox.ini | 6 ++- .../tests/integration/test_charm.py | 18 ++++++-- charms/kfp-viewer/tests/integration/utils.py | 43 +++++++++++++++++++ charms/kfp-viewer/tox.ini | 6 ++- .../kfp-viz/tests/integration/test_charm.py | 19 ++++++-- charms/kfp-viz/tests/integration/utils.py | 43 +++++++++++++++++++ charms/kfp-viz/tox.ini | 6 ++- 25 files changed, 529 insertions(+), 46 deletions(-) create mode 100644 charms/kfp-api/tests/integration/utils.py create mode 100644 charms/kfp-metadata-writer/tests/integration/utils.py create mode 100644 charms/kfp-persistence/tests/integration/utils.py create mode 100644 charms/kfp-profile-controller/tests/integration/utils.py create mode 100644 charms/kfp-schedwf/tests/integration/utils.py create mode 100644 charms/kfp-ui/tests/integration/utils.py create mode 100644 charms/kfp-viewer/tests/integration/utils.py create mode 100644 charms/kfp-viz/tests/integration/utils.py diff --git a/.github/workflows/integrate.yaml b/.github/workflows/integrate.yaml index f310f101..6e2a2624 100644 --- a/.github/workflows/integrate.yaml +++ b/.github/workflows/integrate.yaml @@ -66,20 +66,42 @@ jobs: - run: python3 -m pip install tox - run: tox -e ${{ matrix.charm }}-unit + build: + name: Build charm + uses: canonical/data-platform-workflows/.github/workflows/build_charm.yaml@v21.0.0 + strategy: + fail-fast: false + matrix: + charm: + - kfp-api + - kfp-metadata-writer + - kfp-persistence + - kfp-profile-controller + - kfp-schedwf + - kfp-ui + - kfp-viewer + - kfp-viz + with: + cache: ${{ github.event_name == 'pull_request' }} + charmcraft-snap-channel: 2.x/edge + path-to-charm-directory: ./charms/${{ matrix.charm }} + + integration: name: Integration tests (microk8s) runs-on: ubuntu-20.04 + needs: [build] strategy: fail-fast: false matrix: charm: + - kfp-api + - kfp-metadata-writer - kfp-persistence - kfp-profile-controller - - kfp-api - kfp-schedwf - - kfp-viewer - kfp-ui - - kfp-metadata-writer + - kfp-viewer - kfp-viz steps: # Ideally we'd use self-hosted runners, but this effort is still not stable @@ -90,7 +112,7 @@ jobs: - name: Maximise GH runner space uses: jlumbroso/free-disk-space@v1.3.1 - - uses: actions/checkout@v3 + - uses: actions/checkout@v4 - name: Setup operator environment uses: charmed-kubernetes/actions-operator@main @@ -101,12 +123,20 @@ jobs: # Pinned to 3.x/stable due to https://github.com/canonical/charmcraft/issues/1845 charmcraft-channel: 3.x/stable + - name: Download packed charm(s) + id: download-charms + timeout-minutes: 5 + uses: actions/download-artifact@v4 + with: + pattern: packed-charm-cache-true-./charms/${{ matrix.charm }}-* + merge-multiple: true + - name: Integration tests run: | # Requires the model to be called kubeflow due to # https://github.com/canonical/kfp-operators/issues/389 juju add-model kubeflow - sg snap_microk8s -c "tox -e ${{ matrix.charm }}-integration -- --model kubeflow" + sg snap_microk8s -c "tox -vve ${{ matrix.charm }}-integration-using-packed-charms -- --model kubeflow" - name: Collect charm debug artifacts uses: canonical/kubeflow-ci/actions/dump-charm-debug-artifacts@main diff --git a/charms/kfp-api/tests/integration/test_charm.py b/charms/kfp-api/tests/integration/test_charm.py index 22e4374d..59bc81cd 100644 --- a/charms/kfp-api/tests/integration/test_charm.py +++ b/charms/kfp-api/tests/integration/test_charm.py @@ -2,6 +2,7 @@ # See LICENSE file for licensing details. import logging +import os from pathlib import Path import pytest @@ -15,6 +16,7 @@ get_alert_rules, ) from pytest_operator.plugin import OpsTest +from utils import get_packed_charms logger = logging.getLogger(__name__) @@ -42,11 +44,20 @@ class TestCharm: """Integration test charm""" + @pytest.fixture + def use_packed_charms() -> str: + """Return environment variable `USE_PACKED_CHARMS`. If it's not found, return `false`.""" + return os.environ.get("USE_PACKED_CHARMS", "false").replace('"', "") + @pytest.mark.abort_on_fail - async def test_build_and_deploy(self, ops_test: OpsTest): + async def test_build_and_deploy(self, ops_test: OpsTest, use_packed_charms): """Deploy kfp-api with required charms and relations.""" - built_charm_path = await ops_test.build_charm("./") - logger.info(f"Built charm {built_charm_path}") + charm_path = "." + if use_packed_charms.lower() == "true": + built_charm_path = await get_packed_charms(charm_path) + else: + built_charm_path = await ops_test.build_charm(charm_path) + logger.info(f"Built charm {built_charm_path}") image_path = METADATA["resources"]["oci-image"]["upstream-source"] resources = {"oci-image": image_path} diff --git a/charms/kfp-api/tests/integration/utils.py b/charms/kfp-api/tests/integration/utils.py new file mode 100644 index 00000000..8ff0afba --- /dev/null +++ b/charms/kfp-api/tests/integration/utils.py @@ -0,0 +1,43 @@ +# Copyright 2024 Canonical Ltd. +# See LICENSE file for licensing details. +"""Utils functions for integration tests. Should be moved to chisme.""" + +import os +import pathlib +import subprocess +import typing + + +async def get_packed_charms( + charm_path: typing.Union[str, os.PathLike], bases_index: int = None +) -> pathlib.Path: + """Simplified version of https://github.com/canonical/data-platform-workflows/blob/06f252ea079edfd055cee236ede28c237467f9b0/python/pytest_plugins/pytest_operator_cache/pytest_operator_cache/_plugin.py#L22.""" # noqa: E501 + charm_path = pathlib.Path(charm_path) + # namespace-node-affinity_ubuntu-20.04-amd64.charm + # _-.charm + architecture = subprocess.run( + ["dpkg", "--print-architecture"], + capture_output=True, + check=True, + encoding="utf-8", + ).stdout.strip() + assert architecture in ("amd64", "arm64") + packed_charms = list(charm_path.glob(f"*-{architecture}.charm")) + if len(packed_charms) == 1: + # python-libjuju's model.deploy(), juju deploy, and juju bundle files expect local charms + # to begin with `./` or `/` to distinguish them from Charmhub charms. + # Therefore, we need to return an absolute path—a relative `pathlib.Path` does not start + # with `./` when cast to a str. + # (python-libjuju model.deploy() expects a str but will cast any input to a str as a + # workaround for pytest-operator's non-compliant `build_charm` return type of + # `pathlib.Path`.) + return packed_charms[0].resolve(strict=True) + elif len(packed_charms) > 1: + message = f"More than one matching .charm file found at {charm_path=} for {architecture=}: {packed_charms}." # noqa: E501 + if bases_index is None: + message += " Specify `bases_index`" + raise ValueError(message) + else: + raise ValueError( + f"Unable to find .charm file for {architecture=} and {bases_index=} at {charm_path=}" + ) diff --git a/charms/kfp-api/tox.ini b/charms/kfp-api/tox.ini index da167271..b4da4e44 100644 --- a/charms/kfp-api/tox.ini +++ b/charms/kfp-api/tox.ini @@ -7,7 +7,7 @@ max-line-length = 100 [tox] skipsdist = True skip_missing_interpreters = True -envlist = fmt, lint, unit, integration +envlist = fmt, lint, unit, integration{-using-packed-charms,} [vars] all_path = {[vars]src_path} {[vars]tst_path} @@ -73,8 +73,10 @@ deps = -r requirements-unit.txt description = Run unit tests -[testenv:integration] +[testenv:integration{-using-packed-charms,}] commands = pytest -vv --tb native --asyncio-mode=auto {[vars]tst_path}integration --log-cli-level=INFO -s {posargs} +setenv = + using-packed-charms: USE_PACKED_CHARMS = "true" deps = -r requirements-integration.txt description = Run integration tests diff --git a/charms/kfp-metadata-writer/tests/integration/test_charm.py b/charms/kfp-metadata-writer/tests/integration/test_charm.py index 0f0d5d77..ad8dca91 100644 --- a/charms/kfp-metadata-writer/tests/integration/test_charm.py +++ b/charms/kfp-metadata-writer/tests/integration/test_charm.py @@ -2,6 +2,7 @@ # See LICENSE file for licensing details. import logging +import os from pathlib import Path import pytest @@ -12,6 +13,7 @@ deploy_and_assert_grafana_agent, ) from pytest_operator.plugin import OpsTest +from utils import get_packed_charms METADATA = yaml.safe_load(Path("./metadata.yaml").read_text()) CHARM_ROOT = "." @@ -25,10 +27,20 @@ log = logging.getLogger(__name__) +@pytest.fixture +def use_packed_charms() -> str: + """Return environment variable `USE_PACKED_CHARMS`. If it's not found, return `false`.""" + return os.environ.get("USE_PACKED_CHARMS", "false").replace('"', "") + + @pytest.mark.abort_on_fail -async def test_build_and_deploy_with_relations(ops_test: OpsTest): - built_charm_path = await ops_test.build_charm(CHARM_ROOT) - log.info(f"Built charm {built_charm_path}") +async def test_build_and_deploy_with_relations(ops_test: OpsTest, use_packed_charms): + charm_path = "." + if use_packed_charms.lower() == "true": + built_charm_path = await get_packed_charms(charm_path) + else: + built_charm_path = await ops_test.build_charm(charm_path) + log.info(f"Built charm {built_charm_path}") image_path = METADATA["resources"]["oci-image"]["upstream-source"] resources = {"oci-image": image_path} diff --git a/charms/kfp-metadata-writer/tests/integration/utils.py b/charms/kfp-metadata-writer/tests/integration/utils.py new file mode 100644 index 00000000..8ff0afba --- /dev/null +++ b/charms/kfp-metadata-writer/tests/integration/utils.py @@ -0,0 +1,43 @@ +# Copyright 2024 Canonical Ltd. +# See LICENSE file for licensing details. +"""Utils functions for integration tests. Should be moved to chisme.""" + +import os +import pathlib +import subprocess +import typing + + +async def get_packed_charms( + charm_path: typing.Union[str, os.PathLike], bases_index: int = None +) -> pathlib.Path: + """Simplified version of https://github.com/canonical/data-platform-workflows/blob/06f252ea079edfd055cee236ede28c237467f9b0/python/pytest_plugins/pytest_operator_cache/pytest_operator_cache/_plugin.py#L22.""" # noqa: E501 + charm_path = pathlib.Path(charm_path) + # namespace-node-affinity_ubuntu-20.04-amd64.charm + # _-.charm + architecture = subprocess.run( + ["dpkg", "--print-architecture"], + capture_output=True, + check=True, + encoding="utf-8", + ).stdout.strip() + assert architecture in ("amd64", "arm64") + packed_charms = list(charm_path.glob(f"*-{architecture}.charm")) + if len(packed_charms) == 1: + # python-libjuju's model.deploy(), juju deploy, and juju bundle files expect local charms + # to begin with `./` or `/` to distinguish them from Charmhub charms. + # Therefore, we need to return an absolute path—a relative `pathlib.Path` does not start + # with `./` when cast to a str. + # (python-libjuju model.deploy() expects a str but will cast any input to a str as a + # workaround for pytest-operator's non-compliant `build_charm` return type of + # `pathlib.Path`.) + return packed_charms[0].resolve(strict=True) + elif len(packed_charms) > 1: + message = f"More than one matching .charm file found at {charm_path=} for {architecture=}: {packed_charms}." # noqa: E501 + if bases_index is None: + message += " Specify `bases_index`" + raise ValueError(message) + else: + raise ValueError( + f"Unable to find .charm file for {architecture=} and {bases_index=} at {charm_path=}" + ) diff --git a/charms/kfp-metadata-writer/tox.ini b/charms/kfp-metadata-writer/tox.ini index 81868cf0..6ae0cfc6 100644 --- a/charms/kfp-metadata-writer/tox.ini +++ b/charms/kfp-metadata-writer/tox.ini @@ -7,7 +7,7 @@ max-line-length = 100 [tox] skipsdist = True skip_missing_interpreters = True -envlist = fmt, lint, unit, integration +envlist = fmt, lint, unit, integration{-using-packed-charms,} [vars] all_path = {[vars]src_path} {[vars]tst_path} @@ -73,8 +73,10 @@ deps = -r requirements-unit.txt description = Run unit tests -[testenv:integration] +[testenv:integration{-using-packed-charms,}] commands = pytest -v --tb native --asyncio-mode=auto {[vars]tst_path}integration --log-cli-level=INFO -s {posargs} +setenv = + using-packed-charms: USE_PACKED_CHARMS = "true" deps = -r requirements-integration.txt description = Run integration tests diff --git a/charms/kfp-persistence/tests/integration/test_charm.py b/charms/kfp-persistence/tests/integration/test_charm.py index bd0c3882..7c559b20 100644 --- a/charms/kfp-persistence/tests/integration/test_charm.py +++ b/charms/kfp-persistence/tests/integration/test_charm.py @@ -2,6 +2,7 @@ # See LICENSE file for licensing details. import logging +import os from pathlib import Path import pytest @@ -12,6 +13,7 @@ deploy_and_assert_grafana_agent, ) from pytest_operator.plugin import OpsTest +from utils import get_packed_charms logger = logging.getLogger(__name__) @@ -38,11 +40,20 @@ class TestCharm: """Integration test charm""" + @pytest.fixture + def use_packed_charms() -> str: + """Return environment variable `USE_PACKED_CHARMS`. If it's not found, return `false`.""" + return os.environ.get("USE_PACKED_CHARMS", "false").replace('"', "") + @pytest.mark.abort_on_fail - async def test_build_and_deploy(self, ops_test: OpsTest): + async def test_build_and_deploy(self, ops_test: OpsTest, use_packed_charms): """Deploy kfp-persistence with required charms and relations.""" - built_charm_path = await ops_test.build_charm("./") - logger.info(f"Built charm {built_charm_path}") + charm_path = "." + if use_packed_charms.lower() == "true": + built_charm_path = await get_packed_charms(charm_path) + else: + built_charm_path = await ops_test.build_charm(charm_path) + logger.info(f"Built charm {built_charm_path}") image_path = METADATA["resources"]["oci-image"]["upstream-source"] resources = {"oci-image": image_path} diff --git a/charms/kfp-persistence/tests/integration/utils.py b/charms/kfp-persistence/tests/integration/utils.py new file mode 100644 index 00000000..8ff0afba --- /dev/null +++ b/charms/kfp-persistence/tests/integration/utils.py @@ -0,0 +1,43 @@ +# Copyright 2024 Canonical Ltd. +# See LICENSE file for licensing details. +"""Utils functions for integration tests. Should be moved to chisme.""" + +import os +import pathlib +import subprocess +import typing + + +async def get_packed_charms( + charm_path: typing.Union[str, os.PathLike], bases_index: int = None +) -> pathlib.Path: + """Simplified version of https://github.com/canonical/data-platform-workflows/blob/06f252ea079edfd055cee236ede28c237467f9b0/python/pytest_plugins/pytest_operator_cache/pytest_operator_cache/_plugin.py#L22.""" # noqa: E501 + charm_path = pathlib.Path(charm_path) + # namespace-node-affinity_ubuntu-20.04-amd64.charm + # _-.charm + architecture = subprocess.run( + ["dpkg", "--print-architecture"], + capture_output=True, + check=True, + encoding="utf-8", + ).stdout.strip() + assert architecture in ("amd64", "arm64") + packed_charms = list(charm_path.glob(f"*-{architecture}.charm")) + if len(packed_charms) == 1: + # python-libjuju's model.deploy(), juju deploy, and juju bundle files expect local charms + # to begin with `./` or `/` to distinguish them from Charmhub charms. + # Therefore, we need to return an absolute path—a relative `pathlib.Path` does not start + # with `./` when cast to a str. + # (python-libjuju model.deploy() expects a str but will cast any input to a str as a + # workaround for pytest-operator's non-compliant `build_charm` return type of + # `pathlib.Path`.) + return packed_charms[0].resolve(strict=True) + elif len(packed_charms) > 1: + message = f"More than one matching .charm file found at {charm_path=} for {architecture=}: {packed_charms}." # noqa: E501 + if bases_index is None: + message += " Specify `bases_index`" + raise ValueError(message) + else: + raise ValueError( + f"Unable to find .charm file for {architecture=} and {bases_index=} at {charm_path=}" + ) diff --git a/charms/kfp-persistence/tox.ini b/charms/kfp-persistence/tox.ini index 0febc1cf..2b68a8af 100644 --- a/charms/kfp-persistence/tox.ini +++ b/charms/kfp-persistence/tox.ini @@ -7,7 +7,7 @@ max-line-length = 100 [tox] skipsdist = True skip_missing_interpreters = True -envlist = fmt, lint, unit, integration +envlist = fmt, lint, unit, integration{-using-packed-charms,} [vars] all_path = {[vars]src_path} {[vars]tst_path} @@ -73,8 +73,10 @@ deps = -r requirements-unit.txt description = Run unit tests -[testenv:integration] +[testenv:integration{-using-packed-charms,}] commands = pytest -vv --tb native --asyncio-mode=auto {[vars]tst_path}integration --log-cli-level=INFO -s {posargs} +setenv = + using-packed-charms: USE_PACKED_CHARMS = "true" deps = -r requirements-integration.txt description = Run integration tests diff --git a/charms/kfp-profile-controller/tests/integration/test_charm.py b/charms/kfp-profile-controller/tests/integration/test_charm.py index 19c0ca1b..89b3d658 100644 --- a/charms/kfp-profile-controller/tests/integration/test_charm.py +++ b/charms/kfp-profile-controller/tests/integration/test_charm.py @@ -3,6 +3,7 @@ import json import logging +import os from base64 import b64decode from pathlib import Path @@ -20,6 +21,7 @@ from lightkube.resources.core_v1 import ConfigMap, Namespace, Secret, Service, ServiceAccount from pytest_operator.plugin import OpsTest from tenacity import retry, stop_after_delay, wait_exponential +from utils import get_packed_charms logger = logging.getLogger(__name__) @@ -47,10 +49,20 @@ KUBEFLOW_PROFILES_TRUST = True +@pytest.fixture +def use_packed_charms() -> str: + """Return environment variable `USE_PACKED_CHARMS`. If it's not found, return `false`.""" + return os.environ.get("USE_PACKED_CHARMS", "false").replace('"', "") + + @pytest.mark.abort_on_fail -async def test_build_and_deploy(ops_test: OpsTest): - built_charm_path = await ops_test.build_charm("./") - logger.info(f"Built charm {built_charm_path}") +async def test_build_and_deploy(ops_test: OpsTest, use_packed_charms): + charm_path = "." + if use_packed_charms.lower() == "true": + built_charm_path = await get_packed_charms(charm_path) + else: + built_charm_path = await ops_test.build_charm(charm_path) + logger.info(f"Built charm {built_charm_path}") image_path = METADATA["resources"]["oci-image"]["upstream-source"] resources = {"oci-image": image_path} diff --git a/charms/kfp-profile-controller/tests/integration/utils.py b/charms/kfp-profile-controller/tests/integration/utils.py new file mode 100644 index 00000000..8ff0afba --- /dev/null +++ b/charms/kfp-profile-controller/tests/integration/utils.py @@ -0,0 +1,43 @@ +# Copyright 2024 Canonical Ltd. +# See LICENSE file for licensing details. +"""Utils functions for integration tests. Should be moved to chisme.""" + +import os +import pathlib +import subprocess +import typing + + +async def get_packed_charms( + charm_path: typing.Union[str, os.PathLike], bases_index: int = None +) -> pathlib.Path: + """Simplified version of https://github.com/canonical/data-platform-workflows/blob/06f252ea079edfd055cee236ede28c237467f9b0/python/pytest_plugins/pytest_operator_cache/pytest_operator_cache/_plugin.py#L22.""" # noqa: E501 + charm_path = pathlib.Path(charm_path) + # namespace-node-affinity_ubuntu-20.04-amd64.charm + # _-.charm + architecture = subprocess.run( + ["dpkg", "--print-architecture"], + capture_output=True, + check=True, + encoding="utf-8", + ).stdout.strip() + assert architecture in ("amd64", "arm64") + packed_charms = list(charm_path.glob(f"*-{architecture}.charm")) + if len(packed_charms) == 1: + # python-libjuju's model.deploy(), juju deploy, and juju bundle files expect local charms + # to begin with `./` or `/` to distinguish them from Charmhub charms. + # Therefore, we need to return an absolute path—a relative `pathlib.Path` does not start + # with `./` when cast to a str. + # (python-libjuju model.deploy() expects a str but will cast any input to a str as a + # workaround for pytest-operator's non-compliant `build_charm` return type of + # `pathlib.Path`.) + return packed_charms[0].resolve(strict=True) + elif len(packed_charms) > 1: + message = f"More than one matching .charm file found at {charm_path=} for {architecture=}: {packed_charms}." # noqa: E501 + if bases_index is None: + message += " Specify `bases_index`" + raise ValueError(message) + else: + raise ValueError( + f"Unable to find .charm file for {architecture=} and {bases_index=} at {charm_path=}" + ) diff --git a/charms/kfp-profile-controller/tox.ini b/charms/kfp-profile-controller/tox.ini index 0febc1cf..2b68a8af 100644 --- a/charms/kfp-profile-controller/tox.ini +++ b/charms/kfp-profile-controller/tox.ini @@ -7,7 +7,7 @@ max-line-length = 100 [tox] skipsdist = True skip_missing_interpreters = True -envlist = fmt, lint, unit, integration +envlist = fmt, lint, unit, integration{-using-packed-charms,} [vars] all_path = {[vars]src_path} {[vars]tst_path} @@ -73,8 +73,10 @@ deps = -r requirements-unit.txt description = Run unit tests -[testenv:integration] +[testenv:integration{-using-packed-charms,}] commands = pytest -vv --tb native --asyncio-mode=auto {[vars]tst_path}integration --log-cli-level=INFO -s {posargs} +setenv = + using-packed-charms: USE_PACKED_CHARMS = "true" deps = -r requirements-integration.txt description = Run integration tests diff --git a/charms/kfp-schedwf/tests/integration/test_charm.py b/charms/kfp-schedwf/tests/integration/test_charm.py index 64d8c229..9eb825b6 100644 --- a/charms/kfp-schedwf/tests/integration/test_charm.py +++ b/charms/kfp-schedwf/tests/integration/test_charm.py @@ -2,6 +2,7 @@ # See LICENSE file for licensing details. import logging +import os from pathlib import Path import pytest @@ -12,6 +13,7 @@ deploy_and_assert_grafana_agent, ) from pytest_operator.plugin import OpsTest +from utils import get_packed_charms METADATA = yaml.safe_load(Path("./metadata.yaml").read_text()) CHARM_ROOT = "." @@ -20,10 +22,20 @@ log = logging.getLogger(__name__) +@pytest.fixture +def use_packed_charms() -> str: + """Return environment variable `USE_PACKED_CHARMS`. If it's not found, return `false`.""" + return os.environ.get("USE_PACKED_CHARMS", "false").replace('"', "") + + @pytest.mark.abort_on_fail -async def test_build_and_deploy_with_relations(ops_test: OpsTest): - built_charm_path = await ops_test.build_charm(CHARM_ROOT) - log.info(f"Built charm {built_charm_path}") +async def test_build_and_deploy_with_relations(ops_test: OpsTest, use_packed_charms): + charm_path = "." + if use_packed_charms.lower() == "true": + built_charm_path = await get_packed_charms(charm_path) + else: + built_charm_path = await ops_test.build_charm(charm_path) + log.info(f"Built charm {built_charm_path}") image_path = METADATA["resources"]["oci-image"]["upstream-source"] resources = {"oci-image": image_path} diff --git a/charms/kfp-schedwf/tests/integration/utils.py b/charms/kfp-schedwf/tests/integration/utils.py new file mode 100644 index 00000000..8ff0afba --- /dev/null +++ b/charms/kfp-schedwf/tests/integration/utils.py @@ -0,0 +1,43 @@ +# Copyright 2024 Canonical Ltd. +# See LICENSE file for licensing details. +"""Utils functions for integration tests. Should be moved to chisme.""" + +import os +import pathlib +import subprocess +import typing + + +async def get_packed_charms( + charm_path: typing.Union[str, os.PathLike], bases_index: int = None +) -> pathlib.Path: + """Simplified version of https://github.com/canonical/data-platform-workflows/blob/06f252ea079edfd055cee236ede28c237467f9b0/python/pytest_plugins/pytest_operator_cache/pytest_operator_cache/_plugin.py#L22.""" # noqa: E501 + charm_path = pathlib.Path(charm_path) + # namespace-node-affinity_ubuntu-20.04-amd64.charm + # _-.charm + architecture = subprocess.run( + ["dpkg", "--print-architecture"], + capture_output=True, + check=True, + encoding="utf-8", + ).stdout.strip() + assert architecture in ("amd64", "arm64") + packed_charms = list(charm_path.glob(f"*-{architecture}.charm")) + if len(packed_charms) == 1: + # python-libjuju's model.deploy(), juju deploy, and juju bundle files expect local charms + # to begin with `./` or `/` to distinguish them from Charmhub charms. + # Therefore, we need to return an absolute path—a relative `pathlib.Path` does not start + # with `./` when cast to a str. + # (python-libjuju model.deploy() expects a str but will cast any input to a str as a + # workaround for pytest-operator's non-compliant `build_charm` return type of + # `pathlib.Path`.) + return packed_charms[0].resolve(strict=True) + elif len(packed_charms) > 1: + message = f"More than one matching .charm file found at {charm_path=} for {architecture=}: {packed_charms}." # noqa: E501 + if bases_index is None: + message += " Specify `bases_index`" + raise ValueError(message) + else: + raise ValueError( + f"Unable to find .charm file for {architecture=} and {bases_index=} at {charm_path=}" + ) diff --git a/charms/kfp-schedwf/tox.ini b/charms/kfp-schedwf/tox.ini index 0febc1cf..2b68a8af 100644 --- a/charms/kfp-schedwf/tox.ini +++ b/charms/kfp-schedwf/tox.ini @@ -7,7 +7,7 @@ max-line-length = 100 [tox] skipsdist = True skip_missing_interpreters = True -envlist = fmt, lint, unit, integration +envlist = fmt, lint, unit, integration{-using-packed-charms,} [vars] all_path = {[vars]src_path} {[vars]tst_path} @@ -73,8 +73,10 @@ deps = -r requirements-unit.txt description = Run unit tests -[testenv:integration] +[testenv:integration{-using-packed-charms,}] commands = pytest -vv --tb native --asyncio-mode=auto {[vars]tst_path}integration --log-cli-level=INFO -s {posargs} +setenv = + using-packed-charms: USE_PACKED_CHARMS = "true" deps = -r requirements-integration.txt description = Run integration tests diff --git a/charms/kfp-ui/tests/integration/test_charm.py b/charms/kfp-ui/tests/integration/test_charm.py index 02c59c4e..f7859827 100644 --- a/charms/kfp-ui/tests/integration/test_charm.py +++ b/charms/kfp-ui/tests/integration/test_charm.py @@ -2,6 +2,7 @@ # See LICENSE file for licensing details. import logging +import os from pathlib import Path import pytest @@ -12,6 +13,7 @@ deploy_and_assert_grafana_agent, ) from pytest_operator.plugin import OpsTest +from utils import get_packed_charms METADATA = yaml.safe_load(Path("./metadata.yaml").read_text()) CHARM_ROOT = "." @@ -21,10 +23,20 @@ log = logging.getLogger(__name__) +@pytest.fixture +def use_packed_charms() -> str: + """Return environment variable `USE_PACKED_CHARMS`. If it's not found, return `false`.""" + return os.environ.get("USE_PACKED_CHARMS", "false").replace('"', "") + + @pytest.mark.abort_on_fail -async def test_build_and_deploy_with_relations(ops_test: OpsTest): - built_charm_path = await ops_test.build_charm(CHARM_ROOT) - log.info(f"Built charm {built_charm_path}") +async def test_build_and_deploy_with_relations(ops_test: OpsTest, use_packed_charms): + charm_path = "." + if use_packed_charms.lower() == "true": + built_charm_path = await get_packed_charms(charm_path) + else: + built_charm_path = await ops_test.build_charm(charm_path) + log.info(f"Built charm {built_charm_path}") image_path = METADATA["resources"]["ml-pipeline-ui"]["upstream-source"] resources = {"ml-pipeline-ui": image_path} diff --git a/charms/kfp-ui/tests/integration/utils.py b/charms/kfp-ui/tests/integration/utils.py new file mode 100644 index 00000000..8ff0afba --- /dev/null +++ b/charms/kfp-ui/tests/integration/utils.py @@ -0,0 +1,43 @@ +# Copyright 2024 Canonical Ltd. +# See LICENSE file for licensing details. +"""Utils functions for integration tests. Should be moved to chisme.""" + +import os +import pathlib +import subprocess +import typing + + +async def get_packed_charms( + charm_path: typing.Union[str, os.PathLike], bases_index: int = None +) -> pathlib.Path: + """Simplified version of https://github.com/canonical/data-platform-workflows/blob/06f252ea079edfd055cee236ede28c237467f9b0/python/pytest_plugins/pytest_operator_cache/pytest_operator_cache/_plugin.py#L22.""" # noqa: E501 + charm_path = pathlib.Path(charm_path) + # namespace-node-affinity_ubuntu-20.04-amd64.charm + # _-.charm + architecture = subprocess.run( + ["dpkg", "--print-architecture"], + capture_output=True, + check=True, + encoding="utf-8", + ).stdout.strip() + assert architecture in ("amd64", "arm64") + packed_charms = list(charm_path.glob(f"*-{architecture}.charm")) + if len(packed_charms) == 1: + # python-libjuju's model.deploy(), juju deploy, and juju bundle files expect local charms + # to begin with `./` or `/` to distinguish them from Charmhub charms. + # Therefore, we need to return an absolute path—a relative `pathlib.Path` does not start + # with `./` when cast to a str. + # (python-libjuju model.deploy() expects a str but will cast any input to a str as a + # workaround for pytest-operator's non-compliant `build_charm` return type of + # `pathlib.Path`.) + return packed_charms[0].resolve(strict=True) + elif len(packed_charms) > 1: + message = f"More than one matching .charm file found at {charm_path=} for {architecture=}: {packed_charms}." # noqa: E501 + if bases_index is None: + message += " Specify `bases_index`" + raise ValueError(message) + else: + raise ValueError( + f"Unable to find .charm file for {architecture=} and {bases_index=} at {charm_path=}" + ) diff --git a/charms/kfp-ui/tox.ini b/charms/kfp-ui/tox.ini index 764f8028..c7262137 100644 --- a/charms/kfp-ui/tox.ini +++ b/charms/kfp-ui/tox.ini @@ -7,7 +7,7 @@ max-line-length = 100 [tox] skipsdist = True skip_missing_interpreters = True -envlist = fmt, lint, unit, integration +envlist = fmt, lint, unit, integration{-using-packed-charms,} [vars] all_path = {[vars]src_path} {[vars]tst_path} @@ -73,8 +73,10 @@ deps = -r requirements-unit.txt description = Run unit tests -[testenv:integration] +[testenv:integration{-using-packed-charms,}] commands = pytest -vv --tb native --asyncio-mode=auto {[vars]tst_path}integration --log-cli-level=INFO -s {posargs} +setenv = + using-packed-charms: USE_PACKED_CHARMS = "true" deps = -r requirements-integration.txt description = Run integration tests diff --git a/charms/kfp-viewer/tests/integration/test_charm.py b/charms/kfp-viewer/tests/integration/test_charm.py index c96f8794..39fa8995 100644 --- a/charms/kfp-viewer/tests/integration/test_charm.py +++ b/charms/kfp-viewer/tests/integration/test_charm.py @@ -2,6 +2,7 @@ # See LICENSE file for licensing details. import logging +import os from pathlib import Path import pytest @@ -12,6 +13,7 @@ deploy_and_assert_grafana_agent, ) from pytest_operator.plugin import OpsTest +from utils import get_packed_charms METADATA = yaml.safe_load(Path("./metadata.yaml").read_text()) CHARM_ROOT = "." @@ -20,10 +22,20 @@ log = logging.getLogger(__name__) +@pytest.fixture +def use_packed_charms() -> str: + """Return environment variable `USE_PACKED_CHARMS`. If it's not found, return `false`.""" + return os.environ.get("USE_PACKED_CHARMS", "false").replace('"', "") + + @pytest.mark.abort_on_fail -async def test_build_and_deploy_with_relations(ops_test: OpsTest): - built_charm_path = await ops_test.build_charm(CHARM_ROOT) - log.info(f"Built charm {built_charm_path}") +async def test_build_and_deploy_with_relations(ops_test: OpsTest, use_packed_charms): + charm_path = "." + if use_packed_charms.lower() == "true": + built_charm_path = await get_packed_charms(charm_path) + else: + built_charm_path = await ops_test.build_charm(charm_path) + log.info(f"Built charm {built_charm_path}") image_path = METADATA["resources"]["kfp-viewer-image"]["upstream-source"] resources = {"kfp-viewer-image": image_path} diff --git a/charms/kfp-viewer/tests/integration/utils.py b/charms/kfp-viewer/tests/integration/utils.py new file mode 100644 index 00000000..8ff0afba --- /dev/null +++ b/charms/kfp-viewer/tests/integration/utils.py @@ -0,0 +1,43 @@ +# Copyright 2024 Canonical Ltd. +# See LICENSE file for licensing details. +"""Utils functions for integration tests. Should be moved to chisme.""" + +import os +import pathlib +import subprocess +import typing + + +async def get_packed_charms( + charm_path: typing.Union[str, os.PathLike], bases_index: int = None +) -> pathlib.Path: + """Simplified version of https://github.com/canonical/data-platform-workflows/blob/06f252ea079edfd055cee236ede28c237467f9b0/python/pytest_plugins/pytest_operator_cache/pytest_operator_cache/_plugin.py#L22.""" # noqa: E501 + charm_path = pathlib.Path(charm_path) + # namespace-node-affinity_ubuntu-20.04-amd64.charm + # _-.charm + architecture = subprocess.run( + ["dpkg", "--print-architecture"], + capture_output=True, + check=True, + encoding="utf-8", + ).stdout.strip() + assert architecture in ("amd64", "arm64") + packed_charms = list(charm_path.glob(f"*-{architecture}.charm")) + if len(packed_charms) == 1: + # python-libjuju's model.deploy(), juju deploy, and juju bundle files expect local charms + # to begin with `./` or `/` to distinguish them from Charmhub charms. + # Therefore, we need to return an absolute path—a relative `pathlib.Path` does not start + # with `./` when cast to a str. + # (python-libjuju model.deploy() expects a str but will cast any input to a str as a + # workaround for pytest-operator's non-compliant `build_charm` return type of + # `pathlib.Path`.) + return packed_charms[0].resolve(strict=True) + elif len(packed_charms) > 1: + message = f"More than one matching .charm file found at {charm_path=} for {architecture=}: {packed_charms}." # noqa: E501 + if bases_index is None: + message += " Specify `bases_index`" + raise ValueError(message) + else: + raise ValueError( + f"Unable to find .charm file for {architecture=} and {bases_index=} at {charm_path=}" + ) diff --git a/charms/kfp-viewer/tox.ini b/charms/kfp-viewer/tox.ini index d1c301e4..39707626 100644 --- a/charms/kfp-viewer/tox.ini +++ b/charms/kfp-viewer/tox.ini @@ -7,7 +7,7 @@ max-line-length = 100 [tox] skipsdist = True skip_missing_interpreters = True -envlist = fmt, lint, unit, integration +envlist = fmt, lint, unit, integration{-using-packed-charms,} [vars] all_path = {[vars]src_path} {[vars]tst_path} @@ -73,8 +73,10 @@ deps = -r requirements-unit.txt description = Run unit tests -[testenv:integration] +[testenv:integration{-using-packed-charms,}] commands = pytest -vv --tb native --asyncio-mode=auto {[vars]tst_path}integration --log-cli-level=INFO -s {posargs} +setenv = + using-packed-charms: USE_PACKED_CHARMS = "true" deps = -r requirements-integration.txt description = Run integration tests diff --git a/charms/kfp-viz/tests/integration/test_charm.py b/charms/kfp-viz/tests/integration/test_charm.py index 7eb9b68f..bef36e02 100644 --- a/charms/kfp-viz/tests/integration/test_charm.py +++ b/charms/kfp-viz/tests/integration/test_charm.py @@ -2,6 +2,7 @@ # See LICENSE file for licensing details. import logging +import os from pathlib import Path import pytest @@ -12,6 +13,7 @@ deploy_and_assert_grafana_agent, ) from pytest_operator.plugin import OpsTest +from utils import get_packed_charms METADATA = yaml.safe_load(Path("./metadata.yaml").read_text()) CHARM_ROOT = "." @@ -20,11 +22,20 @@ log = logging.getLogger(__name__) -@pytest.mark.abort_on_fail -async def test_build_and_deploy_with_relations(ops_test: OpsTest): - built_charm_path = await ops_test.build_charm(CHARM_ROOT) - log.info(f"Built charm {built_charm_path}") +@pytest.fixture +def use_packed_charms() -> str: + """Return environment variable `USE_PACKED_CHARMS`. If it's not found, return `false`.""" + return os.environ.get("USE_PACKED_CHARMS", "false").replace('"', "") + +@pytest.mark.abort_on_fail +async def test_build_and_deploy_with_relations(ops_test: OpsTest, use_packed_charms): + charm_path = "." + if use_packed_charms.lower() == "true": + built_charm_path = await get_packed_charms(charm_path) + else: + built_charm_path = await ops_test.build_charm(charm_path) + log.info(f"Built charm {built_charm_path}") image_path = METADATA["resources"]["oci-image"]["upstream-source"] resources = {"oci-image": image_path} diff --git a/charms/kfp-viz/tests/integration/utils.py b/charms/kfp-viz/tests/integration/utils.py new file mode 100644 index 00000000..8ff0afba --- /dev/null +++ b/charms/kfp-viz/tests/integration/utils.py @@ -0,0 +1,43 @@ +# Copyright 2024 Canonical Ltd. +# See LICENSE file for licensing details. +"""Utils functions for integration tests. Should be moved to chisme.""" + +import os +import pathlib +import subprocess +import typing + + +async def get_packed_charms( + charm_path: typing.Union[str, os.PathLike], bases_index: int = None +) -> pathlib.Path: + """Simplified version of https://github.com/canonical/data-platform-workflows/blob/06f252ea079edfd055cee236ede28c237467f9b0/python/pytest_plugins/pytest_operator_cache/pytest_operator_cache/_plugin.py#L22.""" # noqa: E501 + charm_path = pathlib.Path(charm_path) + # namespace-node-affinity_ubuntu-20.04-amd64.charm + # _-.charm + architecture = subprocess.run( + ["dpkg", "--print-architecture"], + capture_output=True, + check=True, + encoding="utf-8", + ).stdout.strip() + assert architecture in ("amd64", "arm64") + packed_charms = list(charm_path.glob(f"*-{architecture}.charm")) + if len(packed_charms) == 1: + # python-libjuju's model.deploy(), juju deploy, and juju bundle files expect local charms + # to begin with `./` or `/` to distinguish them from Charmhub charms. + # Therefore, we need to return an absolute path—a relative `pathlib.Path` does not start + # with `./` when cast to a str. + # (python-libjuju model.deploy() expects a str but will cast any input to a str as a + # workaround for pytest-operator's non-compliant `build_charm` return type of + # `pathlib.Path`.) + return packed_charms[0].resolve(strict=True) + elif len(packed_charms) > 1: + message = f"More than one matching .charm file found at {charm_path=} for {architecture=}: {packed_charms}." # noqa: E501 + if bases_index is None: + message += " Specify `bases_index`" + raise ValueError(message) + else: + raise ValueError( + f"Unable to find .charm file for {architecture=} and {bases_index=} at {charm_path=}" + ) diff --git a/charms/kfp-viz/tox.ini b/charms/kfp-viz/tox.ini index 0febc1cf..2b68a8af 100644 --- a/charms/kfp-viz/tox.ini +++ b/charms/kfp-viz/tox.ini @@ -7,7 +7,7 @@ max-line-length = 100 [tox] skipsdist = True skip_missing_interpreters = True -envlist = fmt, lint, unit, integration +envlist = fmt, lint, unit, integration{-using-packed-charms,} [vars] all_path = {[vars]src_path} {[vars]tst_path} @@ -73,8 +73,10 @@ deps = -r requirements-unit.txt description = Run unit tests -[testenv:integration] +[testenv:integration{-using-packed-charms,}] commands = pytest -vv --tb native --asyncio-mode=auto {[vars]tst_path}integration --log-cli-level=INFO -s {posargs} +setenv = + using-packed-charms: USE_PACKED_CHARMS = "true" deps = -r requirements-integration.txt description = Run integration tests From 91f6c2f6dd95446977c4f4825ca0d63a36382021 Mon Sep 17 00:00:00 2001 From: Orfeas Kourkakis Date: Tue, 17 Sep 2024 16:14:39 +0300 Subject: [PATCH 2/9] try download-artifact with path:./charms/ --- .github/workflows/integrate.yaml | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/.github/workflows/integrate.yaml b/.github/workflows/integrate.yaml index 6e2a2624..89f5e99d 100644 --- a/.github/workflows/integrate.yaml +++ b/.github/workflows/integrate.yaml @@ -123,6 +123,11 @@ jobs: # Pinned to 3.x/stable due to https://github.com/canonical/charmcraft/issues/1845 charmcraft-channel: 3.x/stable + - name: echo things + run: | + echo $GITHUB_WORKSPACE + echo ${{ needs.build.outputs.artifact-prefix }} + - name: Download packed charm(s) id: download-charms timeout-minutes: 5 @@ -130,6 +135,8 @@ jobs: with: pattern: packed-charm-cache-true-./charms/${{ matrix.charm }}-* merge-multiple: true + path: ./charms/${{ matrix.charm }} + - name: Integration tests run: | From 462416301e795cc418add595d87e64fe527761c2 Mon Sep 17 00:00:00 2001 From: Orfeas Kourkakis Date: Tue, 17 Sep 2024 16:37:36 +0300 Subject: [PATCH 3/9] omit path from download-artifact + fix issue in tox --- .github/workflows/integrate.yaml | 4 ++-- tox.ini | 3 ++- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/.github/workflows/integrate.yaml b/.github/workflows/integrate.yaml index 89f5e99d..91ce911d 100644 --- a/.github/workflows/integrate.yaml +++ b/.github/workflows/integrate.yaml @@ -133,9 +133,9 @@ jobs: timeout-minutes: 5 uses: actions/download-artifact@v4 with: - pattern: packed-charm-cache-true-./charms/${{ matrix.charm }}-* + pattern: packed-charm-cache-true-.-charms-${{ matrix.charm }}-* merge-multiple: true - path: ./charms/${{ matrix.charm }} + # path: ./charms/${{ matrix.charm }} - name: Integration tests diff --git a/tox.ini b/tox.ini index fca0c2ae..9fcfa011 100644 --- a/tox.ini +++ b/tox.ini @@ -3,7 +3,7 @@ [tox] skipsdist=True skip_missing_interpreters = True -envlist = {kfp-api,kfp-metadata-writer,kfp-persistence,kfp-profile-controller,kfp-schedwf,kfp-ui,kfp-viewer,kfp-viz}-{lint,unit,integration},bundle-integration-{v1, v2} +envlist = {kfp-api,kfp-metadata-writer,kfp-persistence,kfp-profile-controller,kfp-schedwf,kfp-ui,kfp-viewer,kfp-viz}-{lint,unit},{kfp-api,kfp-metadata-writer,kfp-persistence,kfp-profile-controller,kfp-schedwf,kfp-ui,kfp-viewer,kfp-viz}-{integration}{-using-packed-charms,},bundle-integration-{v1, v2} [vars] tst_path = {toxinidir}/tests/ @@ -23,6 +23,7 @@ setenv = lint: TYPE = lint unit: TYPE = unit integration: TYPE = integration + integration-using-packed-charms: TYPE = integration-using-packed-charms commands = tox -c charms/kfp-{env:CHARM} -e {env:TYPE} -- {posargs} From 98698201012b833f38317bdfdff39b246ca03f5f Mon Sep 17 00:00:00 2001 From: Orfeas Kourkakis Date: Tue, 17 Sep 2024 16:38:40 +0300 Subject: [PATCH 4/9] use path in download-artifact --- .github/workflows/integrate.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/integrate.yaml b/.github/workflows/integrate.yaml index 91ce911d..c82f7ec6 100644 --- a/.github/workflows/integrate.yaml +++ b/.github/workflows/integrate.yaml @@ -135,7 +135,7 @@ jobs: with: pattern: packed-charm-cache-true-.-charms-${{ matrix.charm }}-* merge-multiple: true - # path: ./charms/${{ matrix.charm }} + path: ./charms/${{ matrix.charm }} - name: Integration tests From 7fb5832190036e49e002764a2a3d2de54172a0e9 Mon Sep 17 00:00:00 2001 From: Orfeas Kourkakis Date: Tue, 17 Sep 2024 18:17:34 +0300 Subject: [PATCH 5/9] ci: fix individual charm tests --- .github/workflows/integrate.yaml | 7 ------- charms/kfp-api/tests/integration/test_charm.py | 11 ++++++----- .../kfp-persistence/tests/integration/test_charm.py | 11 ++++++----- 3 files changed, 12 insertions(+), 17 deletions(-) diff --git a/.github/workflows/integrate.yaml b/.github/workflows/integrate.yaml index c82f7ec6..c95b1c03 100644 --- a/.github/workflows/integrate.yaml +++ b/.github/workflows/integrate.yaml @@ -123,11 +123,6 @@ jobs: # Pinned to 3.x/stable due to https://github.com/canonical/charmcraft/issues/1845 charmcraft-channel: 3.x/stable - - name: echo things - run: | - echo $GITHUB_WORKSPACE - echo ${{ needs.build.outputs.artifact-prefix }} - - name: Download packed charm(s) id: download-charms timeout-minutes: 5 @@ -135,8 +130,6 @@ jobs: with: pattern: packed-charm-cache-true-.-charms-${{ matrix.charm }}-* merge-multiple: true - path: ./charms/${{ matrix.charm }} - - name: Integration tests run: | diff --git a/charms/kfp-api/tests/integration/test_charm.py b/charms/kfp-api/tests/integration/test_charm.py index 59bc81cd..e8b49741 100644 --- a/charms/kfp-api/tests/integration/test_charm.py +++ b/charms/kfp-api/tests/integration/test_charm.py @@ -41,14 +41,15 @@ MYSQL_TRUST = True +@pytest.fixture +def use_packed_charms() -> str: + """Return environment variable `USE_PACKED_CHARMS`. If it's not found, return `false`.""" + return os.environ.get("USE_PACKED_CHARMS", "false").replace('"', "") + + class TestCharm: """Integration test charm""" - @pytest.fixture - def use_packed_charms() -> str: - """Return environment variable `USE_PACKED_CHARMS`. If it's not found, return `false`.""" - return os.environ.get("USE_PACKED_CHARMS", "false").replace('"', "") - @pytest.mark.abort_on_fail async def test_build_and_deploy(self, ops_test: OpsTest, use_packed_charms): """Deploy kfp-api with required charms and relations.""" diff --git a/charms/kfp-persistence/tests/integration/test_charm.py b/charms/kfp-persistence/tests/integration/test_charm.py index 7c559b20..c8c35507 100644 --- a/charms/kfp-persistence/tests/integration/test_charm.py +++ b/charms/kfp-persistence/tests/integration/test_charm.py @@ -37,14 +37,15 @@ MINIO_CONFIG = {"access-key": "minio", "secret-key": "minio-secret-key"} +@pytest.fixture +def use_packed_charms() -> str: + """Return environment variable `USE_PACKED_CHARMS`. If it's not found, return `false`.""" + return os.environ.get("USE_PACKED_CHARMS", "false").replace('"', "") + + class TestCharm: """Integration test charm""" - @pytest.fixture - def use_packed_charms() -> str: - """Return environment variable `USE_PACKED_CHARMS`. If it's not found, return `false`.""" - return os.environ.get("USE_PACKED_CHARMS", "false").replace('"', "") - @pytest.mark.abort_on_fail async def test_build_and_deploy(self, ops_test: OpsTest, use_packed_charms): """Deploy kfp-persistence with required charms and relations.""" From af7294262580c57db277f7d10cfe06f78e260db1 Mon Sep 17 00:00:00 2001 From: Orfeas Kourkakis Date: Tue, 17 Sep 2024 18:33:33 +0300 Subject: [PATCH 6/9] ci: bundle-tests use pre-packed charms --- .github/workflows/integrate.yaml | 10 ++++- tests/integration/helpers/utils.py | 43 +++++++++++++++++++++ tests/integration/test_kfp_functional_v2.py | 36 ++++++++++++----- tox.ini | 8 ++-- 4 files changed, 84 insertions(+), 13 deletions(-) create mode 100644 tests/integration/helpers/utils.py diff --git a/.github/workflows/integrate.yaml b/.github/workflows/integrate.yaml index c95b1c03..3ad02282 100644 --- a/.github/workflows/integrate.yaml +++ b/.github/workflows/integrate.yaml @@ -174,11 +174,19 @@ jobs: charmcraft-channel: 3.x/stable microk8s-addons: "dns hostpath-storage rbac metallb:10.64.140.43-10.64.140.49" + - name: Download packed charm(s) + id: download-charms + timeout-minutes: 5 + uses: actions/download-artifact@v4 + with: + pattern: packed-charm-cache-true-.-* + merge-multiple: true + - name: Run test run: | # Requires the model to be called kubeflow due to kfp-viewer juju add-model kubeflow - sg snap_microk8s -c "tox -e bundle-integration-${{ matrix.sdk }} -- --model kubeflow --bundle=./tests/integration/bundles/kfp_latest_edge.yaml.j2" + sg snap_microk8s -c "tox -e bundle-integration-${{ matrix.sdk }}-using-packed-charms -- --model kubeflow --bundle=./tests/integration/bundles/kfp_latest_edge.yaml.j2" - name: Get all run: kubectl get all -A diff --git a/tests/integration/helpers/utils.py b/tests/integration/helpers/utils.py new file mode 100644 index 00000000..8ff0afba --- /dev/null +++ b/tests/integration/helpers/utils.py @@ -0,0 +1,43 @@ +# Copyright 2024 Canonical Ltd. +# See LICENSE file for licensing details. +"""Utils functions for integration tests. Should be moved to chisme.""" + +import os +import pathlib +import subprocess +import typing + + +async def get_packed_charms( + charm_path: typing.Union[str, os.PathLike], bases_index: int = None +) -> pathlib.Path: + """Simplified version of https://github.com/canonical/data-platform-workflows/blob/06f252ea079edfd055cee236ede28c237467f9b0/python/pytest_plugins/pytest_operator_cache/pytest_operator_cache/_plugin.py#L22.""" # noqa: E501 + charm_path = pathlib.Path(charm_path) + # namespace-node-affinity_ubuntu-20.04-amd64.charm + # _-.charm + architecture = subprocess.run( + ["dpkg", "--print-architecture"], + capture_output=True, + check=True, + encoding="utf-8", + ).stdout.strip() + assert architecture in ("amd64", "arm64") + packed_charms = list(charm_path.glob(f"*-{architecture}.charm")) + if len(packed_charms) == 1: + # python-libjuju's model.deploy(), juju deploy, and juju bundle files expect local charms + # to begin with `./` or `/` to distinguish them from Charmhub charms. + # Therefore, we need to return an absolute path—a relative `pathlib.Path` does not start + # with `./` when cast to a str. + # (python-libjuju model.deploy() expects a str but will cast any input to a str as a + # workaround for pytest-operator's non-compliant `build_charm` return type of + # `pathlib.Path`.) + return packed_charms[0].resolve(strict=True) + elif len(packed_charms) > 1: + message = f"More than one matching .charm file found at {charm_path=} for {architecture=}: {packed_charms}." # noqa: E501 + if bases_index is None: + message += " Specify `bases_index`" + raise ValueError(message) + else: + raise ValueError( + f"Unable to find .charm file for {architecture=} and {bases_index=} at {charm_path=}" + ) diff --git a/tests/integration/test_kfp_functional_v2.py b/tests/integration/test_kfp_functional_v2.py index ac9db63b..17bf4a16 100644 --- a/tests/integration/test_kfp_functional_v2.py +++ b/tests/integration/test_kfp_functional_v2.py @@ -5,10 +5,12 @@ import logging import time from pathlib import Path +import os from helpers.bundle_mgmt import render_bundle, deploy_bundle from helpers.k8s_resources import apply_manifests, fetch_response from helpers.localize_bundle import get_resources_from_charm_file +from helpers.utils import get_packed_charms from kfp_globals import ( CHARM_PATH_TEMPLATE, KFP_CHARMS, @@ -66,12 +68,19 @@ def create_and_clean_experiment_v2(kfp_client: kfp.Client): kfp_client.delete_experiment(experiment_id=experiment_response.experiment_id) +@pytest.fixture +def use_packed_charms() -> str: + """Return environment variable `USE_PACKED_CHARMS`. If it's not found, return `false`.""" + return os.environ.get("USE_PACKED_CHARMS", "false").replace('"', "") @pytest.mark.abort_on_fail -async def test_build_and_deploy(ops_test: OpsTest, request, lightkube_client): +async def test_build_and_deploy(ops_test: OpsTest, request, lightkube_client, use_packed_charms): """Build and deploy kfp-operators charms.""" no_build = request.config.getoption("no_build") + if (no_build == True and use_packed_charms == "true"): + raise("Only one of `no_build` and `use_packed_charms` can be true") + # Immediately raise an error if the model name is not kubeflow if ops_test.model_name != "kubeflow": raise ValueError("kfp must be deployed to namespace kubeflow") @@ -87,14 +96,23 @@ async def test_build_and_deploy(ops_test: OpsTest, request, lightkube_client): charm: Path(CHARM_PATH_TEMPLATE.format(basedir=str(basedir), charm=charm)) for charm in KFP_CHARMS } - log.info(f"Building charms for: {charms_to_build}") - built_charms = await ops_test.build_charms(*charms_to_build.values()) - log.info(f"Built charms: {built_charms}") - - for charm, charm_file in built_charms.items(): - charm_resources = get_resources_from_charm_file(charm_file) - context.update([(f"{charm.replace('-', '_')}_resources", charm_resources)]) - context.update([(f"{charm.replace('-', '_')}", charm_file)]) + + if not use_packed_charms: + log.info(f"Building charms for: {charms_to_build}") + print(charms_to_build) + built_charms = await ops_test.build_charms(*charms_to_build.values()) + log.info(f"Built charms: {built_charms}") + + for charm, charm_file in built_charms.items(): + charm_resources = get_resources_from_charm_file(charm_file) + context.update([(f"{charm.replace('-', '_')}_resources", charm_resources)]) + context.update([(f"{charm.replace('-', '_')}", charm_file)]) + else: + for charm, charm_path in charms_to_build.items(): + charm_file = await get_packed_charms(charm_path) + charm_resources = get_resources_from_charm_file(charm_file) + context.update([(f"{charm.replace('-', '_')}_resources", charm_resources)]) + context.update([(f"{charm.replace('-', '_')}", charm_file)]) # Render kfp-operators bundle file with locally built charms and their resources rendered_bundle = render_bundle( diff --git a/tox.ini b/tox.ini index 9fcfa011..31b0ac83 100644 --- a/tox.ini +++ b/tox.ini @@ -3,7 +3,7 @@ [tox] skipsdist=True skip_missing_interpreters = True -envlist = {kfp-api,kfp-metadata-writer,kfp-persistence,kfp-profile-controller,kfp-schedwf,kfp-ui,kfp-viewer,kfp-viz}-{lint,unit},{kfp-api,kfp-metadata-writer,kfp-persistence,kfp-profile-controller,kfp-schedwf,kfp-ui,kfp-viewer,kfp-viz}-{integration}{-using-packed-charms,},bundle-integration-{v1, v2} +envlist = {kfp-api,kfp-metadata-writer,kfp-persistence,kfp-profile-controller,kfp-schedwf,kfp-ui,kfp-viewer,kfp-viz}-{lint,unit},{kfp-api,kfp-metadata-writer,kfp-persistence,kfp-profile-controller,kfp-schedwf,kfp-ui,kfp-viewer,kfp-viz}-{integration}{-using-packed-charms,},bundle-integration-{v1, v2}{-using-packed-charms,} [vars] tst_path = {toxinidir}/tests/ @@ -42,12 +42,14 @@ deps = pip-tools description = Update requirements files by executing pip-compile on all requirements*.in files, including those in subdirs. -[testenv:bundle-integration-v1] +[testenv:bundle-integration-v1{-using-packed-charms,}] commands = pytest -vv --tb=native -s {posargs} {[vars]tst_path}integration/test_kfp_functional_v1.py deps = -r requirements-integration-v1.txt -[testenv:bundle-integration-v2] +[testenv:bundle-integration-v2{-using-packed-charms,}] commands = pytest -vv --tb=native -s {posargs} {[vars]tst_path}integration/test_kfp_functional_v2.py +setenv = + using-packed-charms: USE_PACKED_CHARMS = "true" deps = -r requirements-integration-v2.txt From e11e325a8d08efc7093f934e0a947fc8e3f12f48 Mon Sep 17 00:00:00 2001 From: Orfeas Kourkakis Date: Fri, 20 Sep 2024 12:34:43 +0300 Subject: [PATCH 7/9] ci: Use release-charm.yaml in publish job --- .github/workflows/integrate.yaml | 2 +- .github/workflows/on_pull_request.yaml | 1 + .github/workflows/publish.yaml | 40 ++++++++++++++------------ 3 files changed, 24 insertions(+), 19 deletions(-) diff --git a/.github/workflows/integrate.yaml b/.github/workflows/integrate.yaml index 3ad02282..05363695 100644 --- a/.github/workflows/integrate.yaml +++ b/.github/workflows/integrate.yaml @@ -83,7 +83,7 @@ jobs: - kfp-viz with: cache: ${{ github.event_name == 'pull_request' }} - charmcraft-snap-channel: 2.x/edge + charmcraft-snap-channel: 3.x/edge path-to-charm-directory: ./charms/${{ matrix.charm }} diff --git a/.github/workflows/on_pull_request.yaml b/.github/workflows/on_pull_request.yaml index 3d7eef10..c10807f8 100644 --- a/.github/workflows/on_pull_request.yaml +++ b/.github/workflows/on_pull_request.yaml @@ -18,4 +18,5 @@ jobs: publish-charm: name: Publish Charm uses: ./.github/workflows/publish.yaml + needs: tests secrets: inherit diff --git a/.github/workflows/publish.yaml b/.github/workflows/publish.yaml index 33b09f50..1e9bf131 100644 --- a/.github/workflows/publish.yaml +++ b/.github/workflows/publish.yaml @@ -41,14 +41,12 @@ jobs: run: bash .github/workflows/get-charm-paths.sh - publish-charm: - name: Publish Charm + define-channel: + name: Define destination channel runs-on: ubuntu-20.04 needs: get-charm-paths - strategy: - fail-fast: false - matrix: - charm-path: ${{ fromJson(needs.get-charm-paths.outputs.charm_paths_list) }} + outputs: + destination-channel: ${{ steps.parse-inputs.outputs.destination_channel }} steps: - name: Checkout @@ -72,17 +70,6 @@ jobs: echo "setting output of destination_channel=$destination_channel" echo "::set-output name=destination_channel::$destination_channel" - # tag_prefix - # if charm_path = ./ --> tag_prefix = '' (null) - # if charm_path != ./some-charm (eg: a charm in a ./charms dir) --> tag_prefix = 'some-charm' - if [ ${{ matrix.charm-path }} == './' ]; then - tag_prefix='' - else - tag_prefix=$(basename ${{ matrix.charm-path }} ) - fi - echo "setting output of tag_prefix=$tag_prefix" - echo "::set-output name=tag_prefix::$tag_prefix" - - name: Upload charm to charmhub uses: canonical/charming-actions/upload-charm@2.6.2 with: @@ -92,4 +79,21 @@ jobs: channel: ${{ steps.parse-inputs.outputs.destination_channel }} tag-prefix: ${{ steps.parse-inputs.outputs.tag_prefix }} # Pinned to 3.x/stable due to https://github.com/canonical/charmcraft/issues/1845 - charmcraft-channel: 3.x/stable + charmcraft-channel: 3.x/stableZ + + release: + name: Release charm + uses: canonical/data-platform-workflows/.github/workflows/release_charm.yaml@v21.0.0 + needs: [define-channel, get-charm-paths] + strategy: + fail-fast: false + matrix: + charm-path: ${{ fromJson(needs.get-charm-paths.outputs.charm_paths_list) }} + with: + channel: ${{ needs.define-channel.outputs.destination-channel }} + artifact-prefix: packed-charm-cache-true-.-charms-${{ matrix.charm-path }} + create-github-release: ${{ github.event_name == 'push' }} + secrets: + charmhub-token: ${{ secrets.CHARMCRAFT_CREDENTIALS }} + permissions: + contents: write # Needed to create GitHub release From 7fc24884f332f13473bbcf673e33356ad9289d8d Mon Sep 17 00:00:00 2001 From: Orfeas Kourkakis Date: Fri, 20 Sep 2024 12:35:37 +0300 Subject: [PATCH 8/9] ci: Remove dependency from tests since its not needed --- .github/workflows/on_pull_request.yaml | 1 - 1 file changed, 1 deletion(-) diff --git a/.github/workflows/on_pull_request.yaml b/.github/workflows/on_pull_request.yaml index c10807f8..3d7eef10 100644 --- a/.github/workflows/on_pull_request.yaml +++ b/.github/workflows/on_pull_request.yaml @@ -18,5 +18,4 @@ jobs: publish-charm: name: Publish Charm uses: ./.github/workflows/publish.yaml - needs: tests secrets: inherit From 881748e4e02b955d8e337539b00133ae12f04a30 Mon Sep 17 00:00:00 2001 From: Orfeas Kourkakis Date: Fri, 20 Sep 2024 12:52:42 +0300 Subject: [PATCH 9/9] fix --- .github/workflows/publish.yaml | 11 ----------- 1 file changed, 11 deletions(-) diff --git a/.github/workflows/publish.yaml b/.github/workflows/publish.yaml index 1e9bf131..b7fcca83 100644 --- a/.github/workflows/publish.yaml +++ b/.github/workflows/publish.yaml @@ -70,17 +70,6 @@ jobs: echo "setting output of destination_channel=$destination_channel" echo "::set-output name=destination_channel::$destination_channel" - - name: Upload charm to charmhub - uses: canonical/charming-actions/upload-charm@2.6.2 - with: - credentials: ${{ secrets.CHARMCRAFT_CREDENTIALS }} - github-token: ${{ secrets.GITHUB_TOKEN }} - charm-path: ${{ matrix.charm-path }} - channel: ${{ steps.parse-inputs.outputs.destination_channel }} - tag-prefix: ${{ steps.parse-inputs.outputs.tag_prefix }} - # Pinned to 3.x/stable due to https://github.com/canonical/charmcraft/issues/1845 - charmcraft-channel: 3.x/stableZ - release: name: Release charm uses: canonical/data-platform-workflows/.github/workflows/release_charm.yaml@v21.0.0