[DPE-3095] Fix profile limit memory config option (#348)

* Update data-platform-workflows to v7 * Fix profile limit memory config option Signed-off-by: Marcelo Henrique Neppel <[email protected]> * Add related unit tests Signed-off-by: Marcelo Henrique Neppel <[email protected]> * Remove unused calls in unit test Signed-off-by: Marcelo Henrique Neppel <[email protected]> * Add CRAFT_SHARED_CACHE env variable to charmcraft pack wrapper Signed-off-by: Marcelo Henrique Neppel <[email protected]> --------- Signed-off-by: Marcelo Henrique Neppel <[email protected]> Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
canonical · Dec 7, 2023 · 69b2c13 · 69b2c13
1 parent 5a21d70
commit 69b2c13
Show file tree

Hide file tree

Showing 11 changed files with 175 additions and 19 deletions.
diff --git 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.3
+    uses: canonical/data-platform-workflows/.github/workflows/lint.yaml@v7.0.0
 
   unit-test:
     name: Unit test charm
@@ -45,7 +45,7 @@ jobs:
 
   build:
     name: Build charm
-    uses: canonical/data-platform-workflows/.github/workflows/build_charms_with_cache.yaml@v6.3.3
+    uses: canonical/data-platform-workflows/.github/workflows/build_charms_with_cache.yaml@v7.0.0
     permissions:
       actions: write  # Needed to manage GitHub Actions cache
 
@@ -62,7 +62,7 @@ jobs:
       - lint
       - unit-test
       - build
-    uses: canonical/data-platform-workflows/.github/workflows/integration_test_charm.yaml@v6.3.3
+    uses: canonical/data-platform-workflows/.github/workflows/integration_test_charm.yaml@v7.0.0
     with:
       artifact-name: ${{ needs.build.outputs.artifact-name }}
       cloud: microk8s

diff --git a/.github/workflows/release.yaml b/.github/workflows/release.yaml
@@ -41,14 +41,14 @@ jobs:
 
   build:
     name: Build charm
-    uses: canonical/data-platform-workflows/.github/workflows/build_charm_without_cache.yaml@v6.3.3
+    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.3
+    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
@@ -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.3
+    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
@@ -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
diff --git a/pyproject.toml b/pyproject.toml
@@ -70,10 +70,10 @@ optional = true
 [tool.poetry.group.integration.dependencies]
 lightkube = "^0.15.0"
 pytest = "^7.4.3"
-pytest-github-secrets = {git = "https://github.com/canonical/data-platform-workflows", tag = "v6.3.3", 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.31.0"
-pytest-operator-cache = {git = "https://github.com/canonical/data-platform-workflows", tag = "v6.3.3", subdirectory = "python/pytest_plugins/pytest_operator_cache"}
-pytest-operator-groups = {git = "https://github.com/canonical/data-platform-workflows", tag = "v6.3.3", 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"
 psycopg2 = {version = "^2.9.9", extras = ["binary"]}
 boto3 = "^1.33.1"

diff --git a/src/charm.py b/src/charm.py
@@ -1338,6 +1338,13 @@ 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."""
         # Retrieve PostgreSQL parameters.
+        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."
+            )
         if self.config.profile_limit_memory:
             limit_memory = self.config.profile_limit_memory * 10**6
         else:

diff --git a/src/config.py b/src/config.py
@@ -203,7 +203,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
@@ -916,3 +916,74 @@ def test_migartion(self, scope):
         secret_label = self.harness.charm.set_secret(scope, "my-secret", "blablabla")
         assert self.harness.charm.model.get_secret(label=secret_label)
         assert self.harness.charm.get_secret(scope, "my-secret") == "blablabla"
+
+    @patch(
+        "charm.PostgresqlOperatorCharm._is_workload_running",
+        new_callable=PropertyMock(return_value=False),
+    )
+    @patch(
+        "upgrade.PostgreSQLUpgrade.is_no_sync_member",
+        new_callable=PropertyMock(return_value=False),
+    )
+    @patch("charm.Patroni.render_patroni_yml_file")
+    def test_update_config(
+        self, _render_patroni_yml_file, _is_no_sync_member, _is_workload_running
+    ):
+        # Test when no config option is changed.
+        parameters = {
+            "synchronous_commit": "on",
+            "default_text_search_config": "pg_catalog.simple",
+            "password_encryption": "scram-sha-256",
+            "log_connections": False,
+            "log_disconnections": False,
+            "log_lock_waits": False,
+            "log_min_duration_statement": -1,
+            "maintenance_work_mem": 65536,
+            "max_prepared_transactions": 0,
+            "temp_buffers": 1024,
+            "work_mem": 4096,
+            "constraint_exclusion": "partition",
+            "default_statistics_target": 100,
+            "from_collapse_limit": 8,
+            "join_collapse_limit": 8,
+            "DateStyle": "ISO, MDY",
+            "standard_conforming_strings": True,
+            "TimeZone": "UTC",
+            "bytea_output": "hex",
+            "lc_monetary": "C",
+            "lc_numeric": "C",
+            "lc_time": "C",
+            "autovacuum_analyze_scale_factor": 0.1,
+            "autovacuum_analyze_threshold": 50,
+            "autovacuum_freeze_max_age": 200000000,
+            "autovacuum_vacuum_cost_delay": 2.0,
+            "autovacuum_vacuum_scale_factor": 0.2,
+            "vacuum_freeze_table_age": 150000000,
+            "shared_buffers": "0MB",
+            "effective_cache_size": "0MB",
+        }
+        self.charm.update_config()
+        _render_patroni_yml_file.assert_called_once_with(
+            connectivity=True,
+            is_creating_backup=False,
+            enable_tls=False,
+            is_no_sync_member=False,
+            backup_id=None,
+            stanza=None,
+            restore_stanza=None,
+            parameters=parameters,
+        )
+
+        # 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()