From 6f6b8406dd44528d96638112ea950645bde29124 Mon Sep 17 00:00:00 2001 From: Lucas Gameiro Borges Date: Mon, 5 Aug 2024 16:52:37 -0300 Subject: [PATCH 01/15] check architecture --- src/charm.py | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/src/charm.py b/src/charm.py index 7bae5a3b84..b512aecb25 100755 --- a/src/charm.py +++ b/src/charm.py @@ -13,7 +13,9 @@ from pathlib import Path from typing import Dict, List, Literal, Optional, Tuple, get_args +from ops import ErrorStatus, InstallEvent import psycopg2 +import yaml from charms.data_platform_libs.v0.data_interfaces import DataPeerData, DataPeerUnitData from charms.data_platform_libs.v0.data_models import TypedCharmBase from charms.grafana_k8s.v0.grafana_dashboard import GrafanaDashboardProvider @@ -177,6 +179,7 @@ def __init__(self, *args): self.cluster_name = f"patroni-{self._name}" self.framework.observe(self.on.config_changed, self._on_config_changed) + self.framework.observe(self.on.install, self._on_install) self.framework.observe(self.on.leader_elected, self._on_leader_elected) self.framework.observe(self.on[PEER].relation_changed, self._on_peer_relation_changed) self.framework.observe(self.on.secret_changed, self._on_peer_relation_changed) @@ -372,6 +375,16 @@ def replicas_endpoint(self) -> str: """Returns the endpoint of the replicas instances' service.""" return self._build_service_name("replicas") + @property + def architecture(self) -> Optional[str]: + """Returns the charm's architecture.""" + container = self.unit.get_container("postgresql") + if not container.can_connect(): + logger.debug("Cannot get architecture from Rock. Container inaccessible") + return + snap_meta = container.pull("/meta.charmed-postgresql/snap.yaml") + return yaml.safe_load(snap_meta)["architectures"][0] + def _build_service_name(self, service: str) -> str: """Build a full k8s service name based on the service name.""" return f"{self._name}-{service}.{self._namespace}.svc.cluster.local" @@ -407,6 +420,18 @@ def get_unit_ip(self, unit: Unit) -> Optional[str]: else: return None + def _on_install(self, event: InstallEvent) -> None: + """Handle the install event (fired on startup).""" + hw_arch = os.uname().machine + charm_arch = self.architecture + if charm_arch == "amd64" and hw_arch == "x86_64": + logger.info("Install hook succeeded: AMD64") + return + if charm_arch == "arm64" and hw_arch == "aarch64": + logger.info("Install hook succeeded: ARM64") + return + self.unit.status = ErrorStatus(f"Cannot install charm: Arch {hw_arch} not compatible with {charm_arch} charm") + def _on_peer_relation_departed(self, event: RelationDepartedEvent) -> None: """The leader removes the departing units from the list of cluster members.""" # Allow leader to update endpoints if it isn't leaving. From ebb942bb301ffefc70efbb00d7f1d1cfb7889b4c Mon Sep 17 00:00:00 2001 From: Lucas Gameiro Borges Date: Tue, 6 Aug 2024 10:37:55 -0300 Subject: [PATCH 02/15] update error message --- src/charm.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/charm.py b/src/charm.py index b512aecb25..351caae2c3 100755 --- a/src/charm.py +++ b/src/charm.py @@ -13,7 +13,6 @@ from pathlib import Path from typing import Dict, List, Literal, Optional, Tuple, get_args -from ops import ErrorStatus, InstallEvent import psycopg2 import yaml from charms.data_platform_libs.v0.data_interfaces import DataPeerData, DataPeerUnitData @@ -36,6 +35,7 @@ from lightkube.models.core_v1 import ServicePort, ServiceSpec from lightkube.models.meta_v1 import ObjectMeta from lightkube.resources.core_v1 import Endpoints, Node, Pod, Service +from ops import ErrorStatus, InstallEvent from ops.charm import ( ActionEvent, HookEvent, @@ -430,7 +430,9 @@ def _on_install(self, event: InstallEvent) -> None: if charm_arch == "arm64" and hw_arch == "aarch64": logger.info("Install hook succeeded: ARM64") return - self.unit.status = ErrorStatus(f"Cannot install charm: Arch {hw_arch} not compatible with {charm_arch} charm") + self.unit.status = ErrorStatus( + f"Cannot install: {charm_arch} charm not compatible with {hw_arch} machine" + ) def _on_peer_relation_departed(self, event: RelationDepartedEvent) -> None: """The leader removes the departing units from the list of cluster members.""" From 16627112f01eb82b2f6d79bba417bfd274f5bd46 Mon Sep 17 00:00:00 2001 From: Lucas Gameiro Borges Date: Tue, 6 Aug 2024 11:17:50 -0300 Subject: [PATCH 03/15] try checking inside init --- src/charm.py | 39 +++++++++++++-------------------------- 1 file changed, 13 insertions(+), 26 deletions(-) diff --git a/src/charm.py b/src/charm.py index 351caae2c3..5afe6bb878 100755 --- a/src/charm.py +++ b/src/charm.py @@ -35,7 +35,7 @@ from lightkube.models.core_v1 import ServicePort, ServiceSpec from lightkube.models.meta_v1 import ObjectMeta from lightkube.resources.core_v1 import Endpoints, Node, Pod, Service -from ops import ErrorStatus, InstallEvent +from ops import ErrorStatus from ops.charm import ( ActionEvent, HookEvent, @@ -146,6 +146,18 @@ class PostgresqlOperatorCharm(TypedCharmBase[CharmConfig]): def __init__(self, *args): super().__init__(*args) + hw_arch = os.uname().machine + container = self.unit.get_container("postgresql") + snap_meta = container.pull("/meta.charmed-postgresql/snap.yaml") + charm_arch = yaml.safe_load(snap_meta)["architectures"][0] + if (charm_arch == "amd64" and hw_arch != "x86_64") or ( + charm_arch == "arm64" and hw_arch != "aarch64" + ): + self.unit.status = ErrorStatus( + f"Cannot install: {charm_arch} charm not compatible with {hw_arch} machine" + ) + sys.exit(0) + # Support for disabling the operator. disable_file = Path(f"{os.environ.get('CHARM_DIR')}/disable") if disable_file.exists(): @@ -179,7 +191,6 @@ def __init__(self, *args): self.cluster_name = f"patroni-{self._name}" self.framework.observe(self.on.config_changed, self._on_config_changed) - self.framework.observe(self.on.install, self._on_install) self.framework.observe(self.on.leader_elected, self._on_leader_elected) self.framework.observe(self.on[PEER].relation_changed, self._on_peer_relation_changed) self.framework.observe(self.on.secret_changed, self._on_peer_relation_changed) @@ -375,16 +386,6 @@ def replicas_endpoint(self) -> str: """Returns the endpoint of the replicas instances' service.""" return self._build_service_name("replicas") - @property - def architecture(self) -> Optional[str]: - """Returns the charm's architecture.""" - container = self.unit.get_container("postgresql") - if not container.can_connect(): - logger.debug("Cannot get architecture from Rock. Container inaccessible") - return - snap_meta = container.pull("/meta.charmed-postgresql/snap.yaml") - return yaml.safe_load(snap_meta)["architectures"][0] - def _build_service_name(self, service: str) -> str: """Build a full k8s service name based on the service name.""" return f"{self._name}-{service}.{self._namespace}.svc.cluster.local" @@ -420,20 +421,6 @@ def get_unit_ip(self, unit: Unit) -> Optional[str]: else: return None - def _on_install(self, event: InstallEvent) -> None: - """Handle the install event (fired on startup).""" - hw_arch = os.uname().machine - charm_arch = self.architecture - if charm_arch == "amd64" and hw_arch == "x86_64": - logger.info("Install hook succeeded: AMD64") - return - if charm_arch == "arm64" and hw_arch == "aarch64": - logger.info("Install hook succeeded: ARM64") - return - self.unit.status = ErrorStatus( - f"Cannot install: {charm_arch} charm not compatible with {hw_arch} machine" - ) - def _on_peer_relation_departed(self, event: RelationDepartedEvent) -> None: """The leader removes the departing units from the list of cluster members.""" # Allow leader to update endpoints if it isn't leaving. From f6229414ec0e3c8fa7bb60838ed7ac4256ceb7e3 Mon Sep 17 00:00:00 2001 From: Lucas Gameiro Borges Date: Tue, 6 Aug 2024 14:21:08 -0300 Subject: [PATCH 04/15] try another approach --- src/charm.py | 23 ++++++++++++----------- 1 file changed, 12 insertions(+), 11 deletions(-) diff --git a/src/charm.py b/src/charm.py index 5afe6bb878..aee422ba9d 100755 --- a/src/charm.py +++ b/src/charm.py @@ -146,17 +146,18 @@ class PostgresqlOperatorCharm(TypedCharmBase[CharmConfig]): def __init__(self, *args): super().__init__(*args) - hw_arch = os.uname().machine - container = self.unit.get_container("postgresql") - snap_meta = container.pull("/meta.charmed-postgresql/snap.yaml") - charm_arch = yaml.safe_load(snap_meta)["architectures"][0] - if (charm_arch == "amd64" and hw_arch != "x86_64") or ( - charm_arch == "arm64" and hw_arch != "aarch64" - ): - self.unit.status = ErrorStatus( - f"Cannot install: {charm_arch} charm not compatible with {hw_arch} machine" - ) - sys.exit(0) + manifest_path = f"{os.environ.get('CHARM_DIR')}/manifest.yaml" + if os.path.exists(manifest_path): + with open(manifest_path, "r") as manifest: + charm_arch = yaml.safe_load(manifest)["bases"][0]["architectures"][0] + hw_arch = os.uname().machine + if (charm_arch == "amd64" and hw_arch != "x86_64") or ( + charm_arch == "arm64" and hw_arch != "aarch64" + ): + self.unit.status = ErrorStatus( + f"Cannot install: {charm_arch} charm not compatible with {hw_arch} machine" + ) + sys.exit(0) # Support for disabling the operator. disable_file = Path(f"{os.environ.get('CHARM_DIR')}/disable") From d7ef83ad227db7d080a9c5e5c5082fac7a730d64 Mon Sep 17 00:00:00 2001 From: Lucas Gameiro Borges Date: Tue, 6 Aug 2024 16:07:13 -0300 Subject: [PATCH 05/15] relocate imports --- src/charm.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/charm.py b/src/charm.py index aee422ba9d..b685fa4e42 100755 --- a/src/charm.py +++ b/src/charm.py @@ -13,7 +13,6 @@ from pathlib import Path from typing import Dict, List, Literal, Optional, Tuple, get_args -import psycopg2 import yaml from charms.data_platform_libs.v0.data_interfaces import DataPeerData, DataPeerUnitData from charms.data_platform_libs.v0.data_models import TypedCharmBase @@ -586,6 +585,8 @@ def _on_peer_relation_changed(self, event: HookEvent) -> None: # noqa: C901 def _on_config_changed(self, event) -> None: """Handle configuration changes, like enabling plugins.""" + import psycopg2 + if not self.is_cluster_initialised: logger.debug("Defer on_config_changed: cluster not initialised yet") event.defer() @@ -672,6 +673,8 @@ def enable_disable_extensions(self, database: str = None) -> None: def _handle_enable_disable_extensions(self, original_status, extensions, database) -> None: """Try enablind/disabling Postgresql extensions and handle exceptions appropriately.""" + import psycopg2 + if not isinstance(original_status, UnknownStatus): self.unit.status = WaitingStatus("Updating extensions") try: From 28586e0182a48f1bc469c00ad8e21f5d5c486386 Mon Sep 17 00:00:00 2001 From: Lucas Gameiro Borges Date: Tue, 6 Aug 2024 20:45:51 -0300 Subject: [PATCH 06/15] try fake charm approach --- src/charm.py | 48 +++++++++++++++++++++++++++++------------------- 1 file changed, 29 insertions(+), 19 deletions(-) diff --git a/src/charm.py b/src/charm.py index b685fa4e42..5cceafd5f2 100755 --- a/src/charm.py +++ b/src/charm.py @@ -13,7 +13,35 @@ from pathlib import Path from typing import Dict, List, Literal, Optional, Tuple, get_args -import yaml +try: + import psycopg2 +except ModuleNotFoundError: + import yaml + from ops.charm import CharmBase + from ops.main import main + from ops.model import BlockedStatus + + class PostgresqlOperatorCharm(CharmBase): + """Fake charm to signal wrong architecture.""" + + def __init__(self, *args): + super().__init__(*args) + + manifest_path = f"{os.environ.get('CHARM_DIR')}/manifest.yaml" + if os.path.exists(manifest_path): + with open(manifest_path, "r") as manifest: + charm_arch = yaml.safe_load(manifest)["bases"][0]["architectures"][0] + hw_arch = os.uname().machine + if (charm_arch == "amd64" and hw_arch != "x86_64") or ( + charm_arch == "arm64" and hw_arch != "aarch64" + ): + self.unit.status = BlockedStatus( + f"Cannot install: {charm_arch} charm not compatible with {hw_arch} machine" + ) + sys.exit(0) + + main(PostgresqlOperatorCharm, use_juju_for_storage=True) + from charms.data_platform_libs.v0.data_interfaces import DataPeerData, DataPeerUnitData from charms.data_platform_libs.v0.data_models import TypedCharmBase from charms.grafana_k8s.v0.grafana_dashboard import GrafanaDashboardProvider @@ -34,7 +62,6 @@ from lightkube.models.core_v1 import ServicePort, ServiceSpec from lightkube.models.meta_v1 import ObjectMeta from lightkube.resources.core_v1 import Endpoints, Node, Pod, Service -from ops import ErrorStatus from ops.charm import ( ActionEvent, HookEvent, @@ -145,19 +172,6 @@ class PostgresqlOperatorCharm(TypedCharmBase[CharmConfig]): def __init__(self, *args): super().__init__(*args) - manifest_path = f"{os.environ.get('CHARM_DIR')}/manifest.yaml" - if os.path.exists(manifest_path): - with open(manifest_path, "r") as manifest: - charm_arch = yaml.safe_load(manifest)["bases"][0]["architectures"][0] - hw_arch = os.uname().machine - if (charm_arch == "amd64" and hw_arch != "x86_64") or ( - charm_arch == "arm64" and hw_arch != "aarch64" - ): - self.unit.status = ErrorStatus( - f"Cannot install: {charm_arch} charm not compatible with {hw_arch} machine" - ) - sys.exit(0) - # Support for disabling the operator. disable_file = Path(f"{os.environ.get('CHARM_DIR')}/disable") if disable_file.exists(): @@ -585,8 +599,6 @@ def _on_peer_relation_changed(self, event: HookEvent) -> None: # noqa: C901 def _on_config_changed(self, event) -> None: """Handle configuration changes, like enabling plugins.""" - import psycopg2 - if not self.is_cluster_initialised: logger.debug("Defer on_config_changed: cluster not initialised yet") event.defer() @@ -673,8 +685,6 @@ def enable_disable_extensions(self, database: str = None) -> None: def _handle_enable_disable_extensions(self, original_status, extensions, database) -> None: """Try enablind/disabling Postgresql extensions and handle exceptions appropriately.""" - import psycopg2 - if not isinstance(original_status, UnknownStatus): self.unit.status = WaitingStatus("Updating extensions") try: From 682cd899458677218d3ad02d2378b1a801b6c45f Mon Sep 17 00:00:00 2001 From: Lucas Gameiro Borges Date: Wed, 7 Aug 2024 14:10:40 -0300 Subject: [PATCH 07/15] refactor change --- src/charm.py | 27 +++------------------------ src/fake_charm.py | 42 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 45 insertions(+), 24 deletions(-) create mode 100644 src/fake_charm.py diff --git a/src/charm.py b/src/charm.py index 5cceafd5f2..348738615f 100755 --- a/src/charm.py +++ b/src/charm.py @@ -14,33 +14,12 @@ from typing import Dict, List, Literal, Optional, Tuple, get_args try: + # First platform-specific import, will fail on wrong architecture import psycopg2 except ModuleNotFoundError: - import yaml - from ops.charm import CharmBase - from ops.main import main - from ops.model import BlockedStatus - - class PostgresqlOperatorCharm(CharmBase): - """Fake charm to signal wrong architecture.""" - - def __init__(self, *args): - super().__init__(*args) - - manifest_path = f"{os.environ.get('CHARM_DIR')}/manifest.yaml" - if os.path.exists(manifest_path): - with open(manifest_path, "r") as manifest: - charm_arch = yaml.safe_load(manifest)["bases"][0]["architectures"][0] - hw_arch = os.uname().machine - if (charm_arch == "amd64" and hw_arch != "x86_64") or ( - charm_arch == "arm64" and hw_arch != "aarch64" - ): - self.unit.status = BlockedStatus( - f"Cannot install: {charm_arch} charm not compatible with {hw_arch} machine" - ) - sys.exit(0) + from fake_charm import block_on_wrong_architecture - main(PostgresqlOperatorCharm, use_juju_for_storage=True) + block_on_wrong_architecture() from charms.data_platform_libs.v0.data_interfaces import DataPeerData, DataPeerUnitData from charms.data_platform_libs.v0.data_models import TypedCharmBase diff --git a/src/fake_charm.py b/src/fake_charm.py new file mode 100644 index 0000000000..85a20b08ef --- /dev/null +++ b/src/fake_charm.py @@ -0,0 +1,42 @@ +#!/usr/bin/env -S LD_LIBRARY_PATH=lib python3 +# Copyright 2024 Canonical Ltd. +# See LICENSE file for licensing details. + +"""Fake charm used for catching and raising architecture errors.""" + +import os +import sys + +import yaml +from ops.charm import CharmBase +from ops.main import main +from ops.model import BlockedStatus + + +class WrongArchitectureWarningCharm(CharmBase): + """A fake charm class that only signals a wrong architecture deploy.""" + + def __init__(self, *args): + super().__init__(*args) + self.unit.status = BlockedStatus( + f"Error: Charm version incompatible with {os.uname().machine} architecture" + ) + sys.exit(0) + + +def block_on_wrong_architecture() -> None: + """Checks if charm architecture is compatible with underlying hardware. + + If not, the fake charm will get deployed instead, warning the user. + """ + manifest_path = f"{os.environ.get('CHARM_DIR')}/manifest.yaml" + if not os.path.exists(manifest_path): + return + with open(manifest_path, "r") as manifest: + charm_arch = yaml.safe_load(manifest)["bases"][0]["architectures"][0] + hw_arch = os.uname().machine + + if (charm_arch == "amd64" and hw_arch != "x86_64") or ( + charm_arch == "arm64" and hw_arch != "aarch64" + ): + main(WrongArchitectureWarningCharm, use_juju_for_storage=True) From 58bb54e3d411e6f8348134504be4a7ce91cd8f6a Mon Sep 17 00:00:00 2001 From: Lucas Gameiro Borges Date: Thu, 8 Aug 2024 18:38:42 -0300 Subject: [PATCH 08/15] add unit tests + more refactor --- src/arch_utils.py | 38 ++++++++++++++++++ src/charm.py | 14 +++++-- src/fake_charm.py | 42 ------------------- tests/unit/test_arch_utils.py | 76 +++++++++++++++++++++++++++++++++++ 4 files changed, 125 insertions(+), 45 deletions(-) create mode 100644 src/arch_utils.py delete mode 100644 src/fake_charm.py create mode 100644 tests/unit/test_arch_utils.py diff --git a/src/arch_utils.py b/src/arch_utils.py new file mode 100644 index 0000000000..2abcb35192 --- /dev/null +++ b/src/arch_utils.py @@ -0,0 +1,38 @@ +# Copyright 2024 Canonical Ltd. +# See LICENSE file for licensing details. + +"""Utilities for catching and raising architecture errors.""" + +import os +import sys + +from ops.charm import CharmBase +from ops.model import BlockedStatus + + +class WrongArchitectureWarningCharm(CharmBase): + """A fake charm class that only signals a wrong architecture deploy.""" + + def __init__(self, *args): + super().__init__(*args) + self.unit.status = BlockedStatus( + f"Error: Charm version incompatible with {os.uname().machine} architecture" + ) + sys.exit(0) + + +def is_wrong_architecture() -> bool: + """Checks if charm was deployed on wrong architecture.""" + juju_charm_file = f"{os.environ.get('CHARM_DIR')}/.juju-charm" + if not os.path.exists(juju_charm_file): + return False + + with open(juju_charm_file, "r") as file: + ch_platform = file.read() + hw_arch = os.uname().machine + if ("amd64" in ch_platform and hw_arch == "x86_64") or ( + "arm64" in ch_platform and hw_arch == "aarch64" + ): + return False + + return True diff --git a/src/charm.py b/src/charm.py index 348738615f..0b2342ea6c 100755 --- a/src/charm.py +++ b/src/charm.py @@ -13,13 +13,21 @@ from pathlib import Path from typing import Dict, List, Literal, Optional, Tuple, get_args +# First platform-specific import, will fail on wrong architecture try: - # First platform-specific import, will fail on wrong architecture import psycopg2 except ModuleNotFoundError: - from fake_charm import block_on_wrong_architecture + from ops.main import main - block_on_wrong_architecture() + from arch_utils import WrongArchitectureWarningCharm, is_wrong_architecture + + # If the charm was deployed inside a host with different architecture + # (possibly due to user specifying an incompatible revision) + # then deploy an empty blocked charm with a warning. + if is_wrong_architecture() and __name__ == "__main__": + main(WrongArchitectureWarningCharm, use_juju_for_storage=True) + sys.exit(0) + raise from charms.data_platform_libs.v0.data_interfaces import DataPeerData, DataPeerUnitData from charms.data_platform_libs.v0.data_models import TypedCharmBase diff --git a/src/fake_charm.py b/src/fake_charm.py deleted file mode 100644 index 85a20b08ef..0000000000 --- a/src/fake_charm.py +++ /dev/null @@ -1,42 +0,0 @@ -#!/usr/bin/env -S LD_LIBRARY_PATH=lib python3 -# Copyright 2024 Canonical Ltd. -# See LICENSE file for licensing details. - -"""Fake charm used for catching and raising architecture errors.""" - -import os -import sys - -import yaml -from ops.charm import CharmBase -from ops.main import main -from ops.model import BlockedStatus - - -class WrongArchitectureWarningCharm(CharmBase): - """A fake charm class that only signals a wrong architecture deploy.""" - - def __init__(self, *args): - super().__init__(*args) - self.unit.status = BlockedStatus( - f"Error: Charm version incompatible with {os.uname().machine} architecture" - ) - sys.exit(0) - - -def block_on_wrong_architecture() -> None: - """Checks if charm architecture is compatible with underlying hardware. - - If not, the fake charm will get deployed instead, warning the user. - """ - manifest_path = f"{os.environ.get('CHARM_DIR')}/manifest.yaml" - if not os.path.exists(manifest_path): - return - with open(manifest_path, "r") as manifest: - charm_arch = yaml.safe_load(manifest)["bases"][0]["architectures"][0] - hw_arch = os.uname().machine - - if (charm_arch == "amd64" and hw_arch != "x86_64") or ( - charm_arch == "arm64" and hw_arch != "aarch64" - ): - main(WrongArchitectureWarningCharm, use_juju_for_storage=True) diff --git a/tests/unit/test_arch_utils.py b/tests/unit/test_arch_utils.py new file mode 100644 index 0000000000..1d9bbdb988 --- /dev/null +++ b/tests/unit/test_arch_utils.py @@ -0,0 +1,76 @@ +# Copyright 2024 Canonical Ltd. +# See LICENSE file for licensing details. +import builtins +import sys +import unittest.mock as mock +from unittest.mock import patch + +import pytest + +from arch_utils import is_wrong_architecture + +real_import = builtins.__import__ + + +def psycopg2_not_found(name, globals=None, locals=None, fromlist=(), level=0): # noqa: A002 + """Fake import function to simulate psycopg2 import error.""" + if name == "psycopg2": + raise ModuleNotFoundError(f"Mocked module not found {name}") + return real_import(name, globals=globals, locals=locals, fromlist=fromlist, level=level) + + +def test_on_module_not_found_error(monkeypatch): + """Checks if is_wrong_architecture is called on ModuleNotFoundError.""" + with patch("arch_utils.is_wrong_architecture") as _is_wrong_arch: + # If psycopg2 not there, charm should check architecture + monkeypatch.delitem(sys.modules, "psycopg2", raising=False) + monkeypatch.delitem(sys.modules, "charm", raising=False) + monkeypatch.setattr(builtins, "__import__", psycopg2_not_found) + with pytest.raises(ModuleNotFoundError): + import charm # noqa: F401 + + _is_wrong_arch.assert_called_once() + + # If no import errors, charm continues as normal + _is_wrong_arch.reset_mock() + monkeypatch.setattr(builtins, "__import__", real_import) + import charm # noqa: F401 + + _is_wrong_arch.assert_not_called() + + +def test_wrong_architecture_file_not_found(): + """Tests if the function returns False when the charm file doesn't exist.""" + with ( + patch("os.environ.get", return_value="/tmp"), + patch("os.path.exists", return_value=False), + ): + assert not is_wrong_architecture() + + +def test_wrong_architecture_amd64(): + """Tests if the function correctly identifies arch when charm is AMD.""" + with ( + patch("os.environ.get", return_value="/tmp"), + patch("os.path.exists", return_value=True), + patch("builtins.open", mock.mock_open(read_data="amd64\n")), + patch("os.uname") as _uname, + ): + _uname.return_value = mock.Mock(machine="x86_64") + assert not is_wrong_architecture() + _uname.return_value = mock.Mock(machine="aarch64") + assert is_wrong_architecture() + + +def test_wrong_architecture_arm64(): + """Tests if the function correctly identifies arch when charm is ARM.""" + with ( + patch("os.environ.get", return_value="/tmp"), + patch("os.path.exists", return_value=True), + patch("builtins.open", mock.mock_open(read_data="arm64\n")), + patch("os.uname") as _uname, + ): + _uname.return_value = mock.Mock(machine="x86_64") + assert is_wrong_architecture() + _uname.return_value = mock.Mock(machine="aarch64") + assert not is_wrong_architecture() From 253f7a882ca207dcb45a79dd5489b7f41e77b815 Mon Sep 17 00:00:00 2001 From: Lucas Gameiro Borges Date: Fri, 9 Aug 2024 09:22:15 -0300 Subject: [PATCH 09/15] try adding integration tests --- src/charm.py | 1 - tests/integration/test_wrong_arch.py | 75 ++++++++++++++++++++++++++++ 2 files changed, 75 insertions(+), 1 deletion(-) create mode 100644 tests/integration/test_wrong_arch.py diff --git a/src/charm.py b/src/charm.py index 0b2342ea6c..256f888544 100755 --- a/src/charm.py +++ b/src/charm.py @@ -26,7 +26,6 @@ # then deploy an empty blocked charm with a warning. if is_wrong_architecture() and __name__ == "__main__": main(WrongArchitectureWarningCharm, use_juju_for_storage=True) - sys.exit(0) raise from charms.data_platform_libs.v0.data_interfaces import DataPeerData, DataPeerUnitData diff --git a/tests/integration/test_wrong_arch.py b/tests/integration/test_wrong_arch.py new file mode 100644 index 0000000000..cf64d4d262 --- /dev/null +++ b/tests/integration/test_wrong_arch.py @@ -0,0 +1,75 @@ +#!/usr/bin/env python3 +# Copyright 2024 Canonical Ltd. +# See LICENSE file for licensing details. + +import logging +import time + +import pytest +from markers import amd64_only, arm64_only +from pytest_operator.plugin import OpsTest + +from .helpers import ( + CHARM_SERIES, + METADATA, +) + +logger = logging.getLogger(__name__) + +APP_NAME = METADATA["name"] + + +@pytest.mark.group(1) +@amd64_only +async def test_wrong_arch_amd(ops_test: OpsTest) -> None: + """Tries deploying an arm64 charm on amd64 host.""" + # building arm64 charm + charm = await ops_test.build_charm(".", 1) + resources = { + "postgresql-image": METADATA["resources"]["postgresql-image"]["upstream-source"], + } + await ops_test.model.deploy( + charm, + resources=resources, + application_name=APP_NAME, + trust=True, + num_units=1, + series=CHARM_SERIES, + config={"profile": "testing"}, + ) + time.sleep(10) + await ops_test.model.block_until( + lambda: all( + unit.workload_status == "blocked" + for unit in ops_test.model.applications[APP_NAME].units + ), + timeout=60, + ) + + +@pytest.mark.group(1) +@arm64_only +async def test_wrong_arch_arm(ops_test: OpsTest) -> None: + """Tries deploying an amd64 charm on arm64 host.""" + # building arm64 charm + charm = await ops_test.build_charm(".", 0) + resources = { + "postgresql-image": METADATA["resources"]["postgresql-image"]["upstream-source"], + } + await ops_test.model.deploy( + charm, + resources=resources, + application_name=APP_NAME, + trust=True, + num_units=1, + series=CHARM_SERIES, + config={"profile": "testing"}, + ) + time.sleep(10) + await ops_test.model.block_until( + lambda: all( + unit.workload_status == "blocked" + for unit in ops_test.model.applications[APP_NAME].units + ), + timeout=60, + ) From 8258f0546222e8c6739dbef4636c45397d172286 Mon Sep 17 00:00:00 2001 From: Lucas Gameiro Borges Date: Fri, 9 Aug 2024 09:51:49 -0300 Subject: [PATCH 10/15] fix markers --- tests/integration/test_wrong_arch.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/integration/test_wrong_arch.py b/tests/integration/test_wrong_arch.py index cf64d4d262..d959ac48a5 100644 --- a/tests/integration/test_wrong_arch.py +++ b/tests/integration/test_wrong_arch.py @@ -6,9 +6,9 @@ import time import pytest -from markers import amd64_only, arm64_only from pytest_operator.plugin import OpsTest +from . import markers from .helpers import ( CHARM_SERIES, METADATA, @@ -20,7 +20,7 @@ @pytest.mark.group(1) -@amd64_only +@markers.amd64_only async def test_wrong_arch_amd(ops_test: OpsTest) -> None: """Tries deploying an arm64 charm on amd64 host.""" # building arm64 charm @@ -48,7 +48,7 @@ async def test_wrong_arch_amd(ops_test: OpsTest) -> None: @pytest.mark.group(1) -@arm64_only +@markers.arm64_only async def test_wrong_arch_arm(ops_test: OpsTest) -> None: """Tries deploying an amd64 charm on arm64 host.""" # building arm64 charm From 231de014997b7c68a55f4a716141bab0ebe7c318 Mon Sep 17 00:00:00 2001 From: Lucas Gameiro Borges Date: Fri, 9 Aug 2024 11:28:43 -0300 Subject: [PATCH 11/15] try fix integration test --- tests/integration/test_wrong_arch.py | 24 ++++++++++++++++++++++-- 1 file changed, 22 insertions(+), 2 deletions(-) diff --git a/tests/integration/test_wrong_arch.py b/tests/integration/test_wrong_arch.py index d959ac48a5..9a3ee71790 100644 --- a/tests/integration/test_wrong_arch.py +++ b/tests/integration/test_wrong_arch.py @@ -3,9 +3,13 @@ # See LICENSE file for licensing details. import logging +import os +import pathlib import time +import typing import pytest +import yaml from pytest_operator.plugin import OpsTest from . import markers @@ -19,12 +23,28 @@ APP_NAME = METADATA["name"] +async def fetch_charm( + self, + charm_path: typing.Union[str, os.PathLike], + architecture: str, + bases_index: int, +) -> pathlib.Path: + charm_path = pathlib.Path(charm_path) + charmcraft_yaml = yaml.safe_load((charm_path / "charmcraft.yaml").read_text()) + assert charmcraft_yaml["type"] == "charm" + base = charmcraft_yaml["bases"][bases_index] + build_on = base.get("build-on", [base])[0] + version = build_on["channel"] + packed_charms = list(charm_path.glob(f"*{version}-{architecture}.charm")) + return packed_charms[0].resolve(strict=True) + + @pytest.mark.group(1) @markers.amd64_only async def test_wrong_arch_amd(ops_test: OpsTest) -> None: """Tries deploying an arm64 charm on amd64 host.""" # building arm64 charm - charm = await ops_test.build_charm(".", 1) + charm = await ops_test.fetch_charm(".", "arm64", 1) resources = { "postgresql-image": METADATA["resources"]["postgresql-image"]["upstream-source"], } @@ -52,7 +72,7 @@ async def test_wrong_arch_amd(ops_test: OpsTest) -> None: async def test_wrong_arch_arm(ops_test: OpsTest) -> None: """Tries deploying an amd64 charm on arm64 host.""" # building arm64 charm - charm = await ops_test.build_charm(".", 0) + charm = await ops_test.fetch_charm(".", "amd64", 0) resources = { "postgresql-image": METADATA["resources"]["postgresql-image"]["upstream-source"], } From 23876e681301392634cec8066b4823e80da7efed Mon Sep 17 00:00:00 2001 From: Lucas Gameiro Borges Date: Fri, 9 Aug 2024 11:47:54 -0300 Subject: [PATCH 12/15] fix typo --- tests/integration/test_wrong_arch.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/integration/test_wrong_arch.py b/tests/integration/test_wrong_arch.py index 9a3ee71790..c5e1c3295a 100644 --- a/tests/integration/test_wrong_arch.py +++ b/tests/integration/test_wrong_arch.py @@ -44,7 +44,7 @@ async def fetch_charm( async def test_wrong_arch_amd(ops_test: OpsTest) -> None: """Tries deploying an arm64 charm on amd64 host.""" # building arm64 charm - charm = await ops_test.fetch_charm(".", "arm64", 1) + charm = await fetch_charm(".", "arm64", 1) resources = { "postgresql-image": METADATA["resources"]["postgresql-image"]["upstream-source"], } @@ -72,7 +72,7 @@ async def test_wrong_arch_amd(ops_test: OpsTest) -> None: async def test_wrong_arch_arm(ops_test: OpsTest) -> None: """Tries deploying an amd64 charm on arm64 host.""" # building arm64 charm - charm = await ops_test.fetch_charm(".", "amd64", 0) + charm = await fetch_charm(".", "amd64", 0) resources = { "postgresql-image": METADATA["resources"]["postgresql-image"]["upstream-source"], } From cdd839a1af2278aa955102cf8ccb8c16daa63b06 Mon Sep 17 00:00:00 2001 From: Lucas Gameiro Borges Date: Fri, 9 Aug 2024 11:49:29 -0300 Subject: [PATCH 13/15] remove self --- tests/integration/test_wrong_arch.py | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/integration/test_wrong_arch.py b/tests/integration/test_wrong_arch.py index c5e1c3295a..6b489ed7f1 100644 --- a/tests/integration/test_wrong_arch.py +++ b/tests/integration/test_wrong_arch.py @@ -24,7 +24,6 @@ async def fetch_charm( - self, charm_path: typing.Union[str, os.PathLike], architecture: str, bases_index: int, From d3b5c34a05f4170bbcf8837b96238bee0f0d72f4 Mon Sep 17 00:00:00 2001 From: Lucas Gameiro Borges Date: Fri, 9 Aug 2024 12:20:46 -0300 Subject: [PATCH 14/15] use wait for idle instead of block until --- tests/integration/test_wrong_arch.py | 27 +++++++-------------------- 1 file changed, 7 insertions(+), 20 deletions(-) diff --git a/tests/integration/test_wrong_arch.py b/tests/integration/test_wrong_arch.py index 6b489ed7f1..8f7ac013b2 100644 --- a/tests/integration/test_wrong_arch.py +++ b/tests/integration/test_wrong_arch.py @@ -13,15 +13,10 @@ from pytest_operator.plugin import OpsTest from . import markers -from .helpers import ( - CHARM_SERIES, - METADATA, -) +from .helpers import CHARM_SERIES, DATABASE_APP_NAME, METADATA logger = logging.getLogger(__name__) -APP_NAME = METADATA["name"] - async def fetch_charm( charm_path: typing.Union[str, os.PathLike], @@ -50,19 +45,15 @@ async def test_wrong_arch_amd(ops_test: OpsTest) -> None: await ops_test.model.deploy( charm, resources=resources, - application_name=APP_NAME, + application_name=DATABASE_APP_NAME, trust=True, num_units=1, series=CHARM_SERIES, config={"profile": "testing"}, ) time.sleep(10) - await ops_test.model.block_until( - lambda: all( - unit.workload_status == "blocked" - for unit in ops_test.model.applications[APP_NAME].units - ), - timeout=60, + await ops_test.model.wait_for_idle( + apps=[DATABASE_APP_NAME], raise_on_error=False, status="blocked" ) @@ -78,17 +69,13 @@ async def test_wrong_arch_arm(ops_test: OpsTest) -> None: await ops_test.model.deploy( charm, resources=resources, - application_name=APP_NAME, + application_name=DATABASE_APP_NAME, trust=True, num_units=1, series=CHARM_SERIES, config={"profile": "testing"}, ) time.sleep(10) - await ops_test.model.block_until( - lambda: all( - unit.workload_status == "blocked" - for unit in ops_test.model.applications[APP_NAME].units - ), - timeout=60, + await ops_test.model.wait_for_idle( + apps=[DATABASE_APP_NAME], raise_on_error=False, status="blocked" ) From 8a94e3feab2c7a83af9929eaaf42b102bc158582 Mon Sep 17 00:00:00 2001 From: Lucas Gameiro Borges Date: Fri, 9 Aug 2024 12:53:18 -0300 Subject: [PATCH 15/15] nits --- tests/integration/test_wrong_arch.py | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/tests/integration/test_wrong_arch.py b/tests/integration/test_wrong_arch.py index 8f7ac013b2..9cfcbcbb45 100644 --- a/tests/integration/test_wrong_arch.py +++ b/tests/integration/test_wrong_arch.py @@ -5,7 +5,6 @@ import logging import os import pathlib -import time import typing import pytest @@ -23,6 +22,7 @@ async def fetch_charm( architecture: str, bases_index: int, ) -> pathlib.Path: + """Fetches packed charm from CI runner without checking for architecture.""" charm_path = pathlib.Path(charm_path) charmcraft_yaml = yaml.safe_load((charm_path / "charmcraft.yaml").read_text()) assert charmcraft_yaml["type"] == "charm" @@ -35,9 +35,8 @@ async def fetch_charm( @pytest.mark.group(1) @markers.amd64_only -async def test_wrong_arch_amd(ops_test: OpsTest) -> None: +async def test_arm_charm_on_amd_host(ops_test: OpsTest) -> None: """Tries deploying an arm64 charm on amd64 host.""" - # building arm64 charm charm = await fetch_charm(".", "arm64", 1) resources = { "postgresql-image": METADATA["resources"]["postgresql-image"]["upstream-source"], @@ -51,7 +50,7 @@ async def test_wrong_arch_amd(ops_test: OpsTest) -> None: series=CHARM_SERIES, config={"profile": "testing"}, ) - time.sleep(10) + await ops_test.model.wait_for_idle( apps=[DATABASE_APP_NAME], raise_on_error=False, status="blocked" ) @@ -59,9 +58,8 @@ async def test_wrong_arch_amd(ops_test: OpsTest) -> None: @pytest.mark.group(1) @markers.arm64_only -async def test_wrong_arch_arm(ops_test: OpsTest) -> None: +async def test_amd_charm_on_arm_host(ops_test: OpsTest) -> None: """Tries deploying an amd64 charm on arm64 host.""" - # building arm64 charm charm = await fetch_charm(".", "amd64", 0) resources = { "postgresql-image": METADATA["resources"]["postgresql-image"]["upstream-source"], @@ -75,7 +73,7 @@ async def test_wrong_arch_arm(ops_test: OpsTest) -> None: series=CHARM_SERIES, config={"profile": "testing"}, ) - time.sleep(10) + await ops_test.model.wait_for_idle( apps=[DATABASE_APP_NAME], raise_on_error=False, status="blocked" )