Skip to content

Commit

Permalink
Remove plugin status message before cluster ready
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
phvalguima authored Mar 22, 2024
1 parent b0208dc commit c022520
Show file tree
Hide file tree
Showing 4 changed files with 11 additions and 11 deletions.
2 changes: 1 addition & 1 deletion lib/charms/opensearch/v0/constants_charm.py
Original file line number Diff line number Diff line change
Expand Up @@ -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..."
Expand Down
11 changes: 8 additions & 3 deletions lib/charms/opensearch/v0/opensearch_base_charm.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@
COSUser,
PeerRelationName,
PluginConfigChangeError,
PluginConfigStart,
PluginConfigCheck,
RequestUnitServiceOps,
SecurityIndexInitProgress,
ServiceIsStopping,
Expand Down Expand Up @@ -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)

Expand Down
8 changes: 1 addition & 7 deletions lib/charms/opensearch/v0/opensearch_plugin_manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -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 (
Expand Down Expand Up @@ -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:
Expand Down
1 change: 1 addition & 0 deletions tests/unit/lib/test_plugins.py
Original file line number Diff line number Diff line change
Expand Up @@ -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()

Expand Down

0 comments on commit c022520

Please sign in to comment.