From 0d7a03a3689902d864568458787f1ef89245e665 Mon Sep 17 00:00:00 2001 From: Marcelo Henrique Neppel Date: Fri, 8 Dec 2023 13:55:25 -0300 Subject: [PATCH] [DPE -3096] Fix profile limit memory config option (#306) * Update data-platform-workflows to v7 * Fix profile limit memory config option Signed-off-by: Marcelo Henrique Neppel * Update related unit test Signed-off-by: Marcelo Henrique Neppel * Add CRAFT_SHARED_CACHE env variable to charmcraft pack wrapper Signed-off-by: Marcelo Henrique Neppel --------- Signed-off-by: Marcelo Henrique Neppel Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> --- .github/workflows/ci.yaml | 6 +++--- .github/workflows/release.yaml | 4 ++-- .github/workflows/sync_issue_to_jira.yaml | 2 +- config.yaml | 10 +++++++++- poetry.lock | 17 +++++++++-------- pyproject.toml | 6 +++--- src/charm.py | 8 ++++++++ src/config.py | 2 +- tests/unit/test_charm.py | 19 ++++++++++++++++++- tox.ini | 2 ++ 10 files changed, 56 insertions(+), 20 deletions(-) diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index bfcf6008fb..91a4e5668c 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -22,7 +22,7 @@ on: jobs: lint: name: Lint - uses: canonical/data-platform-workflows/.github/workflows/lint.yaml@v6.3.2 + uses: canonical/data-platform-workflows/.github/workflows/lint.yaml@v7.0.0 unit-test: name: Unit test charm @@ -42,7 +42,7 @@ jobs: build: name: Build charm - uses: canonical/data-platform-workflows/.github/workflows/build_charms_with_cache.yaml@v6.3.2 + uses: canonical/data-platform-workflows/.github/workflows/build_charms_with_cache.yaml@v7.0.0 permissions: actions: write # Needed to manage GitHub Actions cache @@ -60,7 +60,7 @@ jobs: - lint - unit-test - build - uses: canonical/data-platform-workflows/.github/workflows/integration_test_charm.yaml@v6.3.2 + uses: canonical/data-platform-workflows/.github/workflows/integration_test_charm.yaml@v7.0.0 with: artifact-name: ${{ needs.build.outputs.artifact-name }} cloud: lxd diff --git a/.github/workflows/release.yaml b/.github/workflows/release.yaml index a87f7abb69..e20f13575d 100644 --- a/.github/workflows/release.yaml +++ b/.github/workflows/release.yaml @@ -24,14 +24,14 @@ jobs: build: name: Build charm - uses: canonical/data-platform-workflows/.github/workflows/build_charm_without_cache.yaml@v6.3.2 + uses: canonical/data-platform-workflows/.github/workflows/build_charm_without_cache.yaml@v7.0.0 release: name: Release charm needs: - ci-tests - build - uses: canonical/data-platform-workflows/.github/workflows/release_charm.yaml@v6.3.2 + uses: canonical/data-platform-workflows/.github/workflows/release_charm.yaml@v7.0.0 with: channel: 14/edge artifact-name: ${{ needs.build.outputs.artifact-name }} diff --git a/.github/workflows/sync_issue_to_jira.yaml b/.github/workflows/sync_issue_to_jira.yaml index 806ac65b3e..ab1e813677 100644 --- a/.github/workflows/sync_issue_to_jira.yaml +++ b/.github/workflows/sync_issue_to_jira.yaml @@ -9,7 +9,7 @@ on: jobs: sync: name: Sync GitHub issue to Jira - uses: canonical/data-platform-workflows/.github/workflows/sync_issue_to_jira.yaml@v6.3.2 + uses: canonical/data-platform-workflows/.github/workflows/sync_issue_to_jira.yaml@v7.0.0 with: jira-base-url: https://warthogs.atlassian.net jira-project-key: DPE diff --git a/config.yaml b/config.yaml index 46bddea119..725b7860fb 100644 --- a/config.yaml +++ b/config.yaml @@ -228,11 +228,19 @@ options: type: string default: production profile-limit-memory: + type: int + description: | + [DEPRECTATED] Use profile_limit_memory instead. Amount of memory in Megabytes to limit PostgreSQL + and associated process to. If unset, this will be decided according to the default memory limit + in the selected profile. Only comes into effect when the `production` profile is selected. This + config option cannot be set at the same time as profile_limit_memory. + profile_limit_memory: type: int description: | Amount of memory in Megabytes to limit PostgreSQL and associated process to. If unset, this will be decided according to the default memory limit in the selected profile. - Only comes into effect when the `production` profile is selected. + Only comes into effect when the `production` profile is selected. This config option cannot be + set at the same time as profile-limit-memory. request_date_style: description: | Sets the display format for date and time values. Allowed formats are explained diff --git a/poetry.lock b/poetry.lock index 39534d9b57..e00c2c0a59 100644 --- a/poetry.lock +++ b/poetry.lock @@ -1,4 +1,4 @@ -# This file is automatically @generated by Poetry 1.7.0 and should not be changed by hand. +# This file is automatically @generated by Poetry 1.7.1 and should not be changed by hand. [[package]] name = "asttokens" @@ -43,6 +43,7 @@ description = "Modern password hashing for your software and your servers" optional = false python-versions = ">=3.7" files = [ + {file = "bcrypt-4.1.1-cp37-abi3-macosx_10_12_universal2.whl", hash = "sha256:196008d91201bbb1aa4e666fee5e610face25d532e433a560cabb33bfdff958b"}, {file = "bcrypt-4.1.1-cp37-abi3-macosx_13_0_universal2.whl", hash = "sha256:2e197534c884336f9020c1f3a8efbaab0aa96fc798068cb2da9c671818b7fbb0"}, {file = "bcrypt-4.1.1-cp37-abi3-manylinux_2_17_aarch64.manylinux2014_aarch64.whl", hash = "sha256:d573885b637815a7f3a3cd5f87724d7d0822da64b0ab0aa7f7c78bae534e86dc"}, {file = "bcrypt-4.1.1-cp37-abi3-manylinux_2_17_x86_64.manylinux2014_x86_64.whl", hash = "sha256:bab33473f973e8058d1b2df8d6e095d237c49fbf7a02b527541a86a5d1dc4444"}, @@ -1463,8 +1464,8 @@ develop = false [package.source] type = "git" url = "https://github.com/canonical/data-platform-workflows" -reference = "v6.3.2" -resolved_reference = "a8c3b5db3bf4c00eb2d8f1b01b7a0d3912ed356d" +reference = "v7.0.0" +resolved_reference = "450cba38a436b97f90441ef1586411f749ef1d4e" subdirectory = "python/pytest_plugins/github_secrets" [[package]] @@ -1501,8 +1502,8 @@ pyyaml = "*" [package.source] type = "git" url = "https://github.com/canonical/data-platform-workflows" -reference = "v6.3.2" -resolved_reference = "a8c3b5db3bf4c00eb2d8f1b01b7a0d3912ed356d" +reference = "v7.0.0" +resolved_reference = "450cba38a436b97f90441ef1586411f749ef1d4e" subdirectory = "python/pytest_plugins/pytest_operator_cache" [[package]] @@ -1520,8 +1521,8 @@ pytest = "*" [package.source] type = "git" url = "https://github.com/canonical/data-platform-workflows" -reference = "v6.3.2" -resolved_reference = "a8c3b5db3bf4c00eb2d8f1b01b7a0d3912ed356d" +reference = "v7.0.0" +resolved_reference = "450cba38a436b97f90441ef1586411f749ef1d4e" subdirectory = "python/pytest_plugins/pytest_operator_groups" [[package]] @@ -2061,4 +2062,4 @@ files = [ [metadata] lock-version = "2.0" python-versions = "^3.10" -content-hash = "6ee822d4d89cdd3bdb139fc3789226ed9759d35c1987a556bf0a7eaa59e27cb9" +content-hash = "32d9e58b979992d6e039044917530d8768b3806f67891388ce0ff04b1ac71ffc" diff --git a/pyproject.toml b/pyproject.toml index 3540311670..3c32a00d13 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -65,10 +65,10 @@ optional = true [tool.poetry.group.integration.dependencies] pytest = "^7.4.0" -pytest-github-secrets = {git = "https://github.com/canonical/data-platform-workflows", tag = "v6.3.2", subdirectory = "python/pytest_plugins/github_secrets"} +pytest-github-secrets = {git = "https://github.com/canonical/data-platform-workflows", tag = "v7.0.0", subdirectory = "python/pytest_plugins/github_secrets"} pytest-operator = "^0.29.0" -pytest-operator-cache = {git = "https://github.com/canonical/data-platform-workflows", tag = "v6.3.2", subdirectory = "python/pytest_plugins/pytest_operator_cache"} -pytest-operator-groups = {git = "https://github.com/canonical/data-platform-workflows", tag = "v6.3.2", subdirectory = "python/pytest_plugins/pytest_operator_groups"} +pytest-operator-cache = {git = "https://github.com/canonical/data-platform-workflows", tag = "v7.0.0", subdirectory = "python/pytest_plugins/pytest_operator_cache"} +pytest-operator-groups = {git = "https://github.com/canonical/data-platform-workflows", tag = "v7.0.0", subdirectory = "python/pytest_plugins/pytest_operator_groups"} juju = "^3.2.2" boto3 = "^1.28.70" tenacity = "^8.2.3" diff --git a/src/charm.py b/src/charm.py index bf93e46427..cd2bc90529 100755 --- a/src/charm.py +++ b/src/charm.py @@ -1369,6 +1369,14 @@ def _is_workload_running(self) -> bool: def update_config(self, is_creating_backup: bool = False) -> bool: """Updates Patroni config file based on the existence of the TLS files.""" + if ( + self.model.config.get("profile-limit-memory") is not None + and self.model.config.get("profile_limit_memory") is not None + ): + raise ValueError( + "Both profile-limit-memory and profile_limit_memory are set. Please use only one of them." + ) + enable_tls = self.is_tls_enabled limit_memory = None if self.config.profile_limit_memory: diff --git a/src/config.py b/src/config.py index b8aa96f83e..3157f56eeb 100644 --- a/src/config.py +++ b/src/config.py @@ -204,7 +204,7 @@ def profile_limit_memory_validator(cls, value: int) -> Optional[int]: if value < 128: raise ValueError("PostgreSQL Charm requires at least 128MB") if value > 9999999: - raise ValueError("`profile-limit-memory` limited to 7 digits (9999999MB)") + raise ValueError("`profile_limit_memory` limited to 7 digits (9999999MB)") return value diff --git a/tests/unit/test_charm.py b/tests/unit/test_charm.py index 29e0aa6c61..d579020fbf 100644 --- a/tests/unit/test_charm.py +++ b/tests/unit/test_charm.py @@ -1164,11 +1164,28 @@ def test_update_config( with patch.object(PostgresqlOperatorCharm, "postgresql", Mock()) as postgresql_mock: # Mock some properties. postgresql_mock.is_tls_enabled = PropertyMock(side_effect=[False, False, False, False]) - _is_workload_running.side_effect = [True, True, False, True] + _is_workload_running.side_effect = [False, False, True, True, False, True] _member_started.side_effect = [True, True, False] postgresql_mock.build_postgresql_parameters.return_value = {"test": "test"} + # Test when only one of the two config options for profile limit memory is set. + self.harness.update_config({"profile-limit-memory": 1000}) + self.charm.update_config() + + # Test when only one of the two config options for profile limit memory is set. + self.harness.update_config( + {"profile_limit_memory": 1000}, unset={"profile-limit-memory"} + ) + self.charm.update_config() + + # Test when the two config options for profile limit memory are set at the same time. + _render_patroni_yml_file.reset_mock() + self.harness.update_config({"profile-limit-memory": 1000}) + with self.assertRaises(ValueError): + self.charm.update_config() + # Test without TLS files available. + self.harness.update_config(unset={"profile-limit-memory", "profile_limit_memory"}) self.harness.update_relation_data( self.rel_id, self.charm.unit.name, {"tls": "enabled"} ) # Mock some data in the relation to test that it change. diff --git a/tox.ini b/tox.ini index 8f90888e84..6d0204a023 100644 --- a/tox.ini +++ b/tox.ini @@ -19,6 +19,8 @@ allowlist_externals = [testenv:{build,pack-wrapper}] # Wrap `charmcraft pack` +pass_env = + CRAFT_SHARED_CACHE allowlist_externals = {[testenv]allowlist_externals} charmcraft