Minor fixes to opensearch_backup, following up updates in the plugin-…

…manager

lib/charms/opensearch/v0/opensearch_backups.py

@@ -467,7 +467,7 @@ def _query_backup_status(self, backup_id=None) -> BackupServiceState:
             return BackupServiceState.RESPONSE_FAILED_NETWORK
         return self.get_service_status(output)
 
-    def _on_s3_credentials_changed(self, event: EventBase) -> None:
+    def _on_s3_credentials_changed(self, event: EventBase) -> None:  # noqa: C901
         """Calls the plugin manager config handler.
 
         This method will iterate over the s3 relation and check:
@@ -489,8 +489,14 @@ def _on_s3_credentials_changed(self, event: EventBase) -> None:
         self.charm.status.set(MaintenanceStatus(BackupSetupStart))
 
         try:
+            if not self.charm.plugin_manager.check_plugin_manager_ready():
+                raise OpenSearchNotFullyReadyError()
+
             plugin = self.charm.plugin_manager.get_plugin(OpenSearchBackupPlugin)
             if self.charm.plugin_manager.status(plugin) == PluginState.ENABLED:
+                # We need to explicitly disable the plugin before reconfiguration
+                # That happens because, differently from the actual configs, we cannot
+                # retrieve the key values and check if they changed.
                 self.charm.plugin_manager.apply_config(plugin.disable())
             self.charm.plugin_manager.apply_config(plugin.config())
         except OpenSearchError as e:
@@ -507,6 +513,7 @@ def _on_s3_credentials_changed(self, event: EventBase) -> None:
             PluginState.ENABLED,
             PluginState.WAITING_FOR_UPGRADE,
         ]:
+            logger.warning("_on_s3_credentials_changed: plugin is not enabled.")
             event.defer()
             return
 

lib/charms/opensearch/v0/opensearch_plugin_manager.py

@@ -132,11 +132,8 @@ def _extra_conf(self, plugin_data: Dict[str, Any]) -> Optional[Dict[str, Any]]:
             }
         return {**self._charm_config, "opensearch-version": self._opensearch.version}
 
-    def run(self) -> bool:
-        """Runs a check on each plugin: install, execute config changes or remove.
-
-        This method should be called at config-changed event. Returns if needed restart.
-        """
+    def check_plugin_manager_ready(self) -> bool:
+        """Checks if the plugin manager is ready to run."""
         if not (self._charm.opensearch.is_started() and self._charm.opensearch.is_node_up()):
             raise OpenSearchNotFullyReadyError()
 
@@ -153,6 +150,15 @@ def run(self) -> bool:
             # Defer is important as next steps to configure plugins will involve
             # calls to the APIs of the cluster.
             logger.info("Cluster not ready, wait for the next event...")
+            return False
+        return True
+
+    def run(self) -> bool:
+        """Runs a check on each plugin: install, execute config changes or remove.
+
+        This method should be called at config-changed event. Returns if needed restart.
+        """
+        if not self.check_plugin_manager_ready():
             raise OpenSearchNotFullyReadyError()
 
         err_msgs = []

lib/charms/opensearch/v0/opensearch_plugins.py

@@ -481,11 +481,6 @@ def config(self) -> OpenSearchPluginConfig:
             logger.error("Missing AWS access-key and secret-key configuration")
             return OpenSearchPluginConfig()
         return OpenSearchPluginConfig(
-            # Try to remove the previous values
-            secret_entries_to_del=[
-                "s3.client.default.access_key",
-                "s3.client.default.secret_key",
-            ],
             secret_entries_to_add={
                 # Remove any entries with None value
                 k: v

tests/integration/ha/test_backups.py

@@ -147,8 +147,6 @@ def microceph():
                 uceph,
                 "-c",
                 "latest/edge",
-                "-d",
-                "/dev/sdc",
                 "-a",
                 "accesskey",
                 "-s",

tests/unit/lib/test_backups.py

@@ -490,12 +490,18 @@ def test_get_endpoint_protocol(self) -> None:
         assert self.charm.backup._get_endpoint_protocol("https://10.0.0.2:8000") == "https"
         assert self.charm.backup._get_endpoint_protocol("test.not-valid-url") == "https"
 
+    @patch(
+        "charms.opensearch.v0.opensearch_plugin_manager.OpenSearchPluginManager.check_plugin_manager_ready"
+    )
     @patch("charms.opensearch.v0.opensearch_plugin_manager.OpenSearchPluginManager.status")
     @patch("charms.opensearch.v0.opensearch_backups.OpenSearchBackup.apply_api_config_if_needed")
     @patch("charms.opensearch.v0.opensearch_plugin_manager.OpenSearchPluginManager.apply_config")
     @patch("charms.opensearch.v0.opensearch_distro.OpenSearchDistribution.version")
-    def test_00_update_relation_data(self, __, mock_apply_config, _, mock_status) -> None:
+    def test_00_update_relation_data(
+        self, __, mock_apply_config, _, mock_status, mock_pm_ready
+    ) -> None:
         """Tests if new relation without data returns."""
+        mock_pm_ready.return_value = True
         mock_status.return_value = PluginState.INSTALLED
         self.harness.update_relation_data(
             self.s3_rel_id,
@@ -513,10 +519,6 @@ def test_00_update_relation_data(self, __, mock_apply_config, _, mock_status) ->
         assert (
             mock_apply_config.call_args[0][0].__dict__
             == OpenSearchPluginConfig(
-                secret_entries_to_del=[
-                    "s3.client.default.access_key",
-                    "s3.client.default.secret_key",
-                ],
                 secret_entries_to_add={
                     "s3.client.default.access_key": "aaaa",
                     "s3.client.default.secret_key": "bbbb",

tests/unit/lib/test_plugins.py

@@ -451,10 +451,6 @@ def test_config_with_valid_keys(self):
             "secret-key": "SECRET_KEY",
         }
         expected_config = OpenSearchPluginConfig(
-            secret_entries_to_del=[
-                "s3.client.default.access_key",
-                "s3.client.default.secret_key",
-            ],
             secret_entries_to_add={
                 "s3.client.default.access_key": "ACCESS_KEY",
                 "s3.client.default.secret_key": "SECRET_KEY",