Flexible manager: also accept to write directly to config files

lib/charms/opensearch/v0/opensearch_base_charm.py

@@ -634,23 +634,24 @@ def _on_config_changed(self, event: ConfigChangedEvent):  # noqa C901
             self._handle_change_to_main_orchestrator_if_needed(event, previous_deployment_desc)
 
         try:
+            if self.upgrade_in_progress:
+                logger.warning(
+                    "Changing config during an upgrade is not supported. The charm may be in a broken, unrecoverable state"
+                )
+                event.defer()
+                return
+
             if self.unit.is_leader():
                 self.status.set(MaintenanceStatus(PluginConfigCheck), app=True)
 
             if self.plugin_manager.run() and not restart_requested:
-                if self.upgrade_in_progress:
-                    logger.warning(
-                        "Changing config during an upgrade is not supported. The charm may be in a broken, unrecoverable state"
-                    )
-                    event.defer()
-                    return
                 self._restart_opensearch_event.emit()
 
         except (OpenSearchPluginError, OpenSearchKeystoreNotReadyYetError) as e:
             if self.unit.is_leader():
                 self.status.clear(PluginConfigCheck, app=True)
-            if isinstance(e, OpenSearchPluginError):
-                self.status.set(BlockedStatus(PluginConfigChangeError), app=True)
+                if isinstance(e, OpenSearchPluginError):
+                    self.status.set(BlockedStatus(PluginConfigChangeError), app=True)
             event.defer()
             return
 

lib/charms/opensearch/v0/opensearch_keystore.py

@@ -137,7 +137,7 @@ def __init__(self, charm):
         """Creates the keystore manager class."""
         super().__init__(charm)
         self._keytool = "opensearch-keystore"
-        self.keystore = charm._opensearch.paths.conf + "/opensearch.keystore"
+        self.keystore = charm.opensearch.paths.conf + "/opensearch.keystore"
 
     def add(self, entries: Dict[str, str]) -> None:
         """Adds a given key to the "opensearch" keystore."""
@@ -151,13 +151,19 @@ def add(self, entries: Dict[str, str]) -> None:
 
     def delete(self, entries: List[str]) -> None:
         """Removes a given key from "opensearch" keystore."""
+        if not os.path.exists(self.keystore):
+            raise OpenSearchKeystoreNotReadyYetError()
+
         if not entries:
             return  # no key/value to remove, no need to request reload of keystore either
         for key in entries:
             self._delete(key)
 
     def list(self, alias: str = None) -> List[str]:
         """Lists the keys available in opensearch's keystore."""
+        if not os.path.exists(self.keystore):
+            raise OpenSearchKeystoreNotReadyYetError()
+
         try:
             return self._opensearch.run_bin(self._keytool, "list").split("\n")
         except OpenSearchCmdError as e:

lib/charms/opensearch/v0/opensearch_plugin_manager.py

@@ -19,7 +19,11 @@
 from charms.opensearch.v0.opensearch_exceptions import OpenSearchCmdError
 from charms.opensearch.v0.opensearch_health import HealthColors
 from charms.opensearch.v0.opensearch_internal_data import Scope
-from charms.opensearch.v0.opensearch_keystore import OpenSearchKeystore
+from charms.opensearch.v0.opensearch_keystore import (
+    OpenSearchKeystore,
+    OpenSearchKeystoreError,
+    OpenSearchKeystoreNotReadyYetError,
+)
 from charms.opensearch.v0.opensearch_plugins import (
     OpenSearchKnn,
     OpenSearchPlugin,
@@ -56,6 +60,10 @@
 }
 
 
+class OpenSearchPluginManagerNotReadyYetError(OpenSearchPluginError):
+    """Exception when the plugin manager is not yet prepared."""
+
+
 class OpenSearchPluginManager:
     """Manages plugins."""
 
@@ -146,6 +154,7 @@ def run(self) -> bool:
 
         This method should be called at config-changed event. Returns if needed restart.
         """
+        manager_not_ready = False
         err_msgs = []
         restart_needed = False
         for plugin in self.plugins:
@@ -175,9 +184,28 @@ def run(self) -> bool:
                 # This is a more serious issue, as we are missing some input from
                 # the user. The charm should block.
                 err_msgs.append(str(e))
+
+            except OpenSearchKeystoreNotReadyYetError:
+                # Plugin manager must wait until the keystore is to finish its setup.
+                # This separated exception allows to separate this error and process
+                # it differently, once we have inserted all plugins' configs.
+                err_msgs.append("Keystore is not set yet, plugin manager not ready")
+
+                # Store the error and continue
+                # We want to apply all configuration changes to the cluster and then
+                # inform the caller this method needs to be reran later to update keystore.
+                # The keystore does not demand a restart, so we can process it later.
+                manager_not_ready = True
+
             logger.debug(f"Finished Plugin {plugin.name} status: {self.status(plugin)}")
         logger.info(f"Plugin check finished, restart needed: {restart_needed}")
 
+        if manager_not_ready:
+            # Raise a different exception, to message upper methods we still need to rerun
+            # the plugin manager later.
+            # At rerun, configurations above will not change, as they have been applied, and
+            # only the missing keystore will be set.
+            raise OpenSearchKeystoreNotReadyYetError()
         if err_msgs:
             raise OpenSearchPluginError("\n".join(err_msgs))
         return restart_needed
@@ -302,23 +330,38 @@ def apply_config(self, config: OpenSearchPluginConfig) -> bool:
         2) Add entries, if available
 
         Returns True if a configuration change was performed.
-        """
-        self._keystore.delete(config.secret_entries_to_del)
-        self._keystore.add(config.secret_entries_to_add)
-        if config.secret_entries_to_del or config.secret_entries_to_add:
-            self._keystore.reload_keystore()
 
+        Raises:
+            OpenSearchKeystoreNotReadyYetError: If the keystore is not yet ready.
+        """
         current_settings, new_conf = self._compute_settings(config)
         if current_settings and new_conf and current_settings == new_conf:
-            # Nothing to do here
+            # Settings reported data (not: (None, None)). Nothing to do here
             logger.info("apply_config: nothing to do, return")
             return False
 
-        # Update the configuration
+        # Update the configuration files
         if config.config_entries_to_del:
             self._opensearch_config.delete_plugin(config.config_entries_to_del)
         if config.config_entries_to_add:
             self._opensearch_config.add_plugin(config.config_entries_to_add)
+
+        # Now, focus on the keystore part
+        # First, check if we actually need secrets here, if not, return True
+        if not config.secret_entries_to_add and not config.secret_entries_to_del:
+            # short circuit, no secrets to add or remove
+            return any(
+                [
+                    config.config_entries_to_del,
+                    config.config_entries_to_add,
+                ]
+            )
+
+        # If security is not yet initialized, this code will throw an exception
+        self._keystore.delete(config.secret_entries_to_del)
+        self._keystore.add(config.secret_entries_to_add)
+        if config.secret_entries_to_del or config.secret_entries_to_add:
+            self._keystore.reload_keystore()
         return True
 
     def status(self, plugin: OpenSearchPlugin) -> PluginState:
@@ -367,15 +410,30 @@ def _is_enabled(self, plugin: OpenSearchPlugin) -> bool:
             if current_settings and new_conf and current_settings != new_conf:
                 return False
 
-            # Now, focus on the keystore part
-            keys_available = self._keystore.list()
-            keys_to_add = plugin.config().secret_entries_to_add
-            if any(k not in keys_available for k in keys_to_add):
+            # Avoid the keystore check as we may just be writing configuration in the files
+            # while the cluster is not up and running yet.
+            if plugin.config().secret_entries_to_add or plugin.config().secret_entries_to_del:
+                # Need to check keystore
+                # If the keystore is not yet set, then an exception will be raised here
+                keys_available = self._keystore.list()
+                keys_to_add = plugin.config().secret_entries_to_add
+                if any(k not in keys_available for k in keys_to_add):
+                    return False
+                keys_to_del = plugin.config().secret_entries_to_del
+                if any(k in keys_available for k in keys_to_del):
+                    return False
+
+            # Finally, check configuration files
+            if self._opensearch_config.get_plugin(plugin.config().config_entries_to_del):
+                # We should not have deleted entries in the configuration
                 return False
-            keys_to_del = plugin.config().secret_entries_to_del
-            if any(k in keys_available for k in keys_to_del):
+            config = self._opensearch_config.get_plugin(plugin.config().config_entries_to_add)
+            if plugin.config().config_entries_to_add and (not config or config != new_conf):
+                # Have configs that should be present but cannot find them OR they have
+                # different values than expected
                 return False
-        except (KeyError, OpenSearchPluginError) as e:
+
+        except (OpenSearchKeystoreError, KeyError, OpenSearchPluginError) as e:
             logger.warning(f"_is_enabled: error with {e}")
             return False
         return True