From 04480b00b5c1db6bf0abe5d5768e9ff7a1dad033 Mon Sep 17 00:00:00 2001 From: phvalguima Date: Fri, 22 Sep 2023 18:50:24 +0200 Subject: [PATCH 1/2] [DPE-2636] Move to Mongodb 6/edge Add support for snap's 6/edge channel as well as migrating from older ```mongo``` CLI to ```mongosh```. Creates a constant: ```MONGO_SHELL```, that holds the value of the CLI name that is going to be used across the operator. --- .github/workflows/release.yaml | 14 ++++++-------- lib/charms/mongodb/v0/helpers.py | 5 ++--- src/config.py | 2 +- .../relation_tests/legacy_relations/helpers.py | 2 +- tests/integration/test_charm.py | 4 +++- tests/integration/tls_tests/helpers.py | 3 ++- 6 files changed, 15 insertions(+), 15 deletions(-) diff --git a/.github/workflows/release.yaml b/.github/workflows/release.yaml index 86182da37..3bfa97fd3 100644 --- a/.github/workflows/release.yaml +++ b/.github/workflows/release.yaml @@ -31,11 +31,9 @@ jobs: with: credentials: "${{ secrets.CHARMHUB_TOKEN }}" github-token: "${{ secrets.GITHUB_TOKEN }}" - # TODO - wait until track request is approved: - # https://discourse.charmhub.io/t/request-track-6-for-charmed-mongodb-operator/11900 - # - name: Upload charm to charmhub - # uses: canonical/charming-actions/upload-charm@2.2.2 - # with: - # credentials: "${{ secrets.CHARMHUB_TOKEN }}" - # github-token: "${{ secrets.GITHUB_TOKEN }}" - # channel: "6/edge" + - name: Upload charm to charmhub + uses: canonical/charming-actions/upload-charm@2.2.2 + with: + credentials: "${{ secrets.CHARMHUB_TOKEN }}" + github-token: "${{ secrets.GITHUB_TOKEN }}" + channel: "6/edge" diff --git a/lib/charms/mongodb/v0/helpers.py b/lib/charms/mongodb/v0/helpers.py index c57357403..b7aa6f1b5 100644 --- a/lib/charms/mongodb/v0/helpers.py +++ b/lib/charms/mongodb/v0/helpers.py @@ -41,6 +41,7 @@ MONGODB_COMMON_DIR = "/var/snap/charmed-mongodb/common" MONGODB_SNAP_DATA_DIR = "/var/snap/charmed-mongodb/current" +MONGO_SHELL = "charmed-mongodb.mongosh" DATA_DIR = "/var/lib/mongodb" CONF_DIR = "/etc/mongod" @@ -49,9 +50,7 @@ # noinspection GrazieInspection -def get_create_user_cmd( - config: MongoDBConfiguration, mongo_path="charmed-mongodb.mongo" -) -> List[str]: +def get_create_user_cmd(config: MongoDBConfiguration, mongo_path=MONGO_SHELL) -> List[str]: """Creates initial admin user for MongoDB. Initial admin user can be created only through localhost connection. diff --git a/src/config.py b/src/config.py index da3261864..6da269ec1 100644 --- a/src/config.py +++ b/src/config.py @@ -16,7 +16,7 @@ class Config: MONGODB_SNAP_DATA_DIR = "/var/snap/charmed-mongodb/current" MONGOD_CONF_DIR = f"{MONGODB_SNAP_DATA_DIR}/etc/mongod" MONGOD_CONF_FILE_PATH = f"{MONGOD_CONF_DIR}/mongod.conf" - SNAP_PACKAGES = [("charmed-mongodb", "5/edge", 84)] + SNAP_PACKAGES = [("charmed-mongodb", "6/edge", 87)] class Role: """Role config names for MongoDB Charm.""" diff --git a/tests/integration/relation_tests/legacy_relations/helpers.py b/tests/integration/relation_tests/legacy_relations/helpers.py index 9bb27b3fe..ccde5c01d 100644 --- a/tests/integration/relation_tests/legacy_relations/helpers.py +++ b/tests/integration/relation_tests/legacy_relations/helpers.py @@ -136,7 +136,7 @@ async def mongo_tls_command(ops_test: OpsTest) -> str: replica_set_uri = f"mongodb://{hosts}/admin?replicaSet={app}" return ( - f"charmed-mongodb.mongo '{replica_set_uri}' --eval 'rs.status()'" + f"charmed-mongodb.mongosh '{replica_set_uri}' --eval 'rs.status()'" f" --tls --tlsCAFile {EXTERNAL_CERT_PATH}" f" --tlsCertificateKeyFile {EXTERNAL_PEM_PATH}" ) diff --git a/tests/integration/test_charm.py b/tests/integration/test_charm.py index 0f75d43ce..bc71fc748 100644 --- a/tests/integration/test_charm.py +++ b/tests/integration/test_charm.py @@ -9,6 +9,7 @@ from uuid import uuid4 import pytest +from charms.mongodb.v0.helpers import MONGO_SHELL from pymongo import MongoClient from pymongo.errors import PyMongoError, ServerSelectionTimeoutError from pytest_operator.plugin import OpsTest @@ -180,7 +181,8 @@ async def test_monitor_user(ops_test: OpsTest) -> None: ] hosts = ",".join(replica_set_hosts) replica_set_uri = f"mongodb://monitor:{password}@{hosts}/admin?replicaSet=mongodb" - admin_mongod_cmd = f"charmed-mongodb.mongo '{replica_set_uri}' --eval 'rs.conf()'" + + admin_mongod_cmd = f"{MONGO_SHELL} '{replica_set_uri}' --eval 'rs.conf()'" check_monitor_cmd = f"exec --unit {unit.name} -- {admin_mongod_cmd}" return_code, _, _ = await ops_test.juju(*check_monitor_cmd.split()) assert return_code == 0, "command rs.conf() on monitor user does not work" diff --git a/tests/integration/tls_tests/helpers.py b/tests/integration/tls_tests/helpers.py index 1a4249627..0f7f8ff9e 100644 --- a/tests/integration/tls_tests/helpers.py +++ b/tests/integration/tls_tests/helpers.py @@ -4,6 +4,7 @@ from datetime import datetime import ops +from charms.mongodb.v0.helpers import MONGO_SHELL from pytest_operator.plugin import OpsTest from tenacity import RetryError, Retrying, stop_after_attempt, wait_exponential @@ -33,7 +34,7 @@ async def mongo_tls_command(ops_test: OpsTest) -> str: replica_set_uri = f"mongodb://operator:" f"{password}@" f"{hosts}/admin?replicaSet={app}" return ( - f"charmed-mongodb.mongo '{replica_set_uri}' --eval 'rs.status()'" + f"{MONGO_SHELL} '{replica_set_uri}' --eval 'rs.status()'" f" --tls --tlsCAFile {EXTERNAL_CERT_PATH}" f" --tlsCertificateKeyFile {EXTERNAL_PEM_PATH}" ) From e27b155e2208169dc2235727ea1ca1733976cc4e Mon Sep 17 00:00:00 2001 From: Dmitry Ratushnyy <132273757+dmitry-ratushnyy@users.noreply.github.com> Date: Mon, 25 Sep 2023 15:07:42 +0200 Subject: [PATCH 2/2] [DPE-2609] Remove support of Juju 2.9 (#257) The goal of this PR is to remove Juju 2.9 specific code related to secrets since we are dropping support of Juju 2.9 --- lib/charms/mongodb/v0/mongodb_backups.py | 10 +++--- src/charm.py | 35 +++---------------- tests/conftest.py | 44 ------------------------ tests/integration/test_charm.py | 17 --------- 4 files changed, 9 insertions(+), 97 deletions(-) delete mode 100644 tests/conftest.py diff --git a/lib/charms/mongodb/v0/mongodb_backups.py b/lib/charms/mongodb/v0/mongodb_backups.py index 8a9785bd4..bd30b8d49 100644 --- a/lib/charms/mongodb/v0/mongodb_backups.py +++ b/lib/charms/mongodb/v0/mongodb_backups.py @@ -523,7 +523,7 @@ def _try_to_restore(self, backup_id: str) -> None: restore_cmd = restore_cmd + remapping_args.split(" ") self.charm.run_pbm_command(restore_cmd) except (subprocess.CalledProcessError, ExecError) as e: - if type(e) == subprocess.CalledProcessError: + if type(e) is subprocess.CalledProcessError: error_message = e.output.decode("utf-8") else: error_message = str(e.stderr) @@ -560,7 +560,7 @@ def _try_to_backup(self): ) return backup_id_match.group("backup_id") if backup_id_match else "N/A" except (subprocess.CalledProcessError, ExecError) as e: - if type(e) == subprocess.CalledProcessError: + if type(e) is subprocess.CalledProcessError: error_message = e.output.decode("utf-8") else: error_message = str(e.stderr) @@ -636,13 +636,13 @@ def _get_backup_restore_operation_result(self, current_pbm_status, previous_pbm_ to contain the operation type (backup/restore) and the backup id. """ if ( - type(current_pbm_status) == type(previous_pbm_status) + type(current_pbm_status) is type(previous_pbm_status) and current_pbm_status.message == previous_pbm_status.message ): return f"Operation is still in progress: '{current_pbm_status.message}'" if ( - type(previous_pbm_status) == MaintenanceStatus + type(previous_pbm_status) is MaintenanceStatus and "backup id:" in previous_pbm_status.message ): backup_id = previous_pbm_status.message.split("backup id:")[-1].strip() @@ -679,7 +679,7 @@ def retrieve_error_message(self, pbm_status: Dict) -> str: def process_pbm_error(self, pbm_status: Optional[_StrOrBytes]) -> str: """Returns errors found in PBM status.""" - if type(pbm_status) == bytes: + if type(pbm_status) is bytes: pbm_status = pbm_status.decode("utf-8") try: diff --git a/src/charm.py b/src/charm.py index 6908864a3..4501f943a 100755 --- a/src/charm.py +++ b/src/charm.py @@ -40,7 +40,6 @@ OperatorUser, ) from charms.operator_libs_linux.v1 import snap -from ops import JujuVersion from ops.charm import ( ActionEvent, CharmBase, @@ -252,10 +251,6 @@ def db_initialised(self, value): f"'db_initialised' must be a boolean value. Proivded: {value} is of type {type(value)}" ) - @property - def _juju_has_secrets(self) -> bool: - return JujuVersion.from_environ().has_secrets - # END: properties # BEGIN: charm event handlers @@ -1006,15 +1001,7 @@ def _unit_ip(self, unit: Unit) -> str: def get_secret(self, scope: str, key: str) -> Optional[str]: """Get secret from the secret storage.""" - if self._juju_has_secrets: - return self._juju_secret_get(scope, key) - - if scope == UNIT_SCOPE: - return self.unit_peer_data.get(key, None) - elif scope == APP_SCOPE: - return self.app_peer_data.get(key, None) - else: - raise RuntimeError("Unknown secret scope.") + return self._juju_secret_get(scope, key) def set_secret(self, scope: str, key: str, value: Optional[str]) -> Optional[str]: """Set secret in the secret storage. @@ -1022,23 +1009,9 @@ def set_secret(self, scope: str, key: str, value: Optional[str]) -> Optional[str Juju versions > 3.0 use `juju secrets`, this function first checks which secret store is being used before setting the secret. """ - if self._juju_has_secrets: - if not value: - return self._juju_secret_remove(scope, key) - return self._juju_secret_set(scope, key, value) - - if scope == UNIT_SCOPE: - if not value: - del self.unit_peer_data[key] - return - self.unit_peer_data.update({key: str(value)}) - elif scope == APP_SCOPE: - if not value: - del self.app_peer_data[key] - return - self.app_peer_data.update({key: str(value)}) - else: - raise RuntimeError("Unknown secret scope.") + if not value: + return self._juju_secret_remove(scope, key) + return self._juju_secret_set(scope, key, value) def start_mongod_service(self): """Starts the mongod service and if necessary starts mongos. diff --git a/tests/conftest.py b/tests/conftest.py deleted file mode 100644 index 34259017c..000000000 --- a/tests/conftest.py +++ /dev/null @@ -1,44 +0,0 @@ -# Copyright 2023 Canonical Ltd. -# See LICENSE file for licensing details. -from importlib.metadata import version -from unittest.mock import PropertyMock - -import pytest -from ops import JujuVersion -from pytest_mock import MockerFixture - - -@pytest.fixture(autouse=True) -def juju_has_secrets(mocker: MockerFixture): - """This fixture will force the usage of secrets whenever run on Juju 3.x. - - NOTE: This is needed, as normally JujuVersion is set to 0.0.0 in tests - (i.e. not the real juju version) - """ - if version("juju") < "3": - mocker.patch.object( - JujuVersion, "has_secrets", new_callable=PropertyMock - ).return_value = False - return False - else: - mocker.patch.object( - JujuVersion, "has_secrets", new_callable=PropertyMock - ).return_value = True - return True - - -@pytest.fixture -def only_with_juju_secrets(juju_has_secrets): - """Pretty way to skip Juju 3 tests.""" - if not juju_has_secrets: - pytest.skip("Secrets test only applies on Juju 3.x") - - -@pytest.fixture -def only_without_juju_secrets(juju_has_secrets): - """Pretty way to skip Juju 2-specific tests. - - Typically: to save CI time, when the same check were executed in a Juju 3-specific way already - """ - if juju_has_secrets: - pytest.skip("Skipping legacy secrets tests") diff --git a/tests/integration/test_charm.py b/tests/integration/test_charm.py index bc71fc748..31b208ffd 100644 --- a/tests/integration/test_charm.py +++ b/tests/integration/test_charm.py @@ -206,7 +206,6 @@ async def test_only_leader_can_set_while_all_can_read_password_secret(ops_test: assert password2 == password -@pytest.mark.usefixtures("only_with_juju_secrets") async def test_reset_and_get_password_secret_same_as_cli(ops_test: OpsTest) -> None: """Test verifies that we can set and retrieve the correct password using Juju 3.x secrets.""" new_password = str(uuid4()) @@ -231,21 +230,6 @@ async def test_reset_and_get_password_secret_same_as_cli(ops_test: OpsTest) -> N assert data[secret_id]["content"]["Data"]["monitor-password"] == password -@pytest.mark.usefixtures("only_without_juju_secrets") -async def test_reset_and_get_password_no_secret(ops_test: OpsTest, mocker) -> None: - """Test verifies that we can set and retrieve the correct password using Juju 2.x.""" - new_password = str(uuid4()) - - # Re=setting existing password - leader_id = await get_leader_id(ops_test) - await set_password(ops_test, unit_id=leader_id, username="monitor", password=new_password) - - # Getting back the pw programmatically - password = await get_password(ops_test, username="monitor") - assert password == new_password - - -@pytest.mark.usefixtures("only_with_juju_secrets") async def test_empty_password(ops_test: OpsTest) -> None: """Test that the password can't be set to an empty string.""" leader_id = await get_leader_id(ops_test) @@ -258,7 +242,6 @@ async def test_empty_password(ops_test: OpsTest) -> None: assert password1 == password2 -@pytest.mark.usefixtures("only_with_juju_secrets") async def test_no_password_change_on_invalid_password(ops_test: OpsTest) -> None: """Test that in general, there is no change when password validation fails.""" leader_id = await get_leader_id(ops_test)