From c0225203320a412c3492ac827ac47b3b5b763787 Mon Sep 17 00:00:00 2001 From: phvalguima Date: Fri, 22 Mar 2024 13:22:24 +0100 Subject: [PATCH] Remove plugin status message before cluster ready This PR proposes to move the cluster ready check that originally happens within `plugin_manager.run()` to be executed at `_on_config_changed`. If the cluster is ready, then we proceed to inform the user we will start the plugin configuration. It also updates the message to inform we are checking the plugin configs instead of applying anything. --- lib/charms/opensearch/v0/constants_charm.py | 2 +- lib/charms/opensearch/v0/opensearch_base_charm.py | 11 ++++++++--- lib/charms/opensearch/v0/opensearch_plugin_manager.py | 8 +------- tests/unit/lib/test_plugins.py | 1 + 4 files changed, 11 insertions(+), 11 deletions(-) diff --git a/lib/charms/opensearch/v0/constants_charm.py b/lib/charms/opensearch/v0/constants_charm.py index 4456d80cf..c54088683 100644 --- a/lib/charms/opensearch/v0/constants_charm.py +++ b/lib/charms/opensearch/v0/constants_charm.py @@ -74,7 +74,7 @@ WaitingForOtherUnitServiceOps = "Waiting for other units to complete the ops on their service." NewIndexRequested = "new index {index} requested" RestoreInProgress = "Restore in progress..." -PluginConfigStart = "Plugin configuration started." +PluginConfigCheck = "Plugin configuration check." BackupSetupStart = "Backup setup started." BackupConfigureStart = "Configuring backup service..." BackupInDisabling = "Disabling backup service..." diff --git a/lib/charms/opensearch/v0/opensearch_base_charm.py b/lib/charms/opensearch/v0/opensearch_base_charm.py index 31745a681..d47f033e5 100644 --- a/lib/charms/opensearch/v0/opensearch_base_charm.py +++ b/lib/charms/opensearch/v0/opensearch_base_charm.py @@ -21,7 +21,7 @@ COSUser, PeerRelationName, PluginConfigChangeError, - PluginConfigStart, + PluginConfigCheck, RequestUnitServiceOps, SecurityIndexInitProgress, ServiceIsStopping, @@ -484,18 +484,23 @@ def _on_config_changed(self, event: ConfigChangedEvent): return try: - self.status.set(MaintenanceStatus(PluginConfigStart)) + if not self.plugin_manager.check_plugin_manager_ready(): + raise OpenSearchNotFullyReadyError() + self.status.set(MaintenanceStatus(PluginConfigCheck)) if self.plugin_manager.run(): self.on[self.service_manager.name].acquire_lock.emit( callback_override="_restart_opensearch" ) - self.status.clear(PluginConfigStart) + self.status.clear(PluginConfigCheck) except (OpenSearchNotFullyReadyError, OpenSearchPluginError) as e: if isinstance(e, OpenSearchNotFullyReadyError): logger.warning("Plugin management: cluster not ready yet at config changed") else: self.status.set(BlockedStatus(PluginConfigChangeError)) event.defer() + # Decided to defer the event. We can clean up the status and reset it once the + # config-changed is called again. + self.status.clear(PluginConfigCheck) return self.status.clear(PluginConfigChangeError) diff --git a/lib/charms/opensearch/v0/opensearch_plugin_manager.py b/lib/charms/opensearch/v0/opensearch_plugin_manager.py index 7dc737f13..f3563c51e 100644 --- a/lib/charms/opensearch/v0/opensearch_plugin_manager.py +++ b/lib/charms/opensearch/v0/opensearch_plugin_manager.py @@ -16,10 +16,7 @@ from typing import Any, Dict, List, Optional, Tuple from charms.opensearch.v0.helper_cluster import ClusterTopology -from charms.opensearch.v0.opensearch_exceptions import ( - OpenSearchCmdError, - OpenSearchNotFullyReadyError, -) +from charms.opensearch.v0.opensearch_exceptions import OpenSearchCmdError from charms.opensearch.v0.opensearch_health import HealthColors from charms.opensearch.v0.opensearch_keystore import OpenSearchKeystore from charms.opensearch.v0.opensearch_plugins import ( @@ -154,9 +151,6 @@ def run(self) -> bool: This method should be called at config-changed event. Returns if needed restart. """ - if not self.check_plugin_manager_ready(): - raise OpenSearchNotFullyReadyError() - err_msgs = [] restart_needed = False for plugin in self.plugins: diff --git a/tests/unit/lib/test_plugins.py b/tests/unit/lib/test_plugins.py index dd9b31242..362c3849c 100644 --- a/tests/unit/lib/test_plugins.py +++ b/tests/unit/lib/test_plugins.py @@ -209,6 +209,7 @@ def test_check_plugin_called_on_config_changed(self, mock_version, deployment_de self.plugin_manager.run = MagicMock(return_value=False) self.charm.opensearch_config.update_host_if_needed = MagicMock(return_value=False) self.charm.opensearch.is_started = MagicMock(return_value=True) + self.plugin_manager.check_plugin_manager_ready = MagicMock(return_value=True) self.harness.update_config({}) self.plugin_manager.run.assert_called()