Skip to content

Commit

Permalink
Add more checks before running plugin configuration routines
Browse files Browse the repository at this point in the history
  • Loading branch information
phvalguima committed Feb 7, 2024
1 parent 4ab37e9 commit afbca49
Show file tree
Hide file tree
Showing 3 changed files with 51 additions and 4 deletions.
15 changes: 11 additions & 4 deletions lib/charms/opensearch/v0/opensearch_plugin_manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -135,10 +135,17 @@ def run(self) -> bool:
This method should be called at config-changed event. Returns if needed restart.
"""
if not self._charm.opensearch.is_started() or self._charm.health.apply() not in [
HealthColors.GREEN,
HealthColors.YELLOW,
]:
if not (self._charm.opensearch.is_started() and self._charm.opensearch.is_node_up()):
raise OpenSearchPluginRelationClusterNotReadyError()

if not (
len(self._charm._get_nodes(True)) == self._charm.app.planned_units()
and self._charm.health.apply()
in [
HealthColors.GREEN,
HealthColors.YELLOW,
]
):
# If the health is not green, then raise a cluster-not-ready error
# The classes above should then defer their own events in waiting.
# Defer is important as next steps to configure plugins will involve
Expand Down
10 changes: 10 additions & 0 deletions tests/unit/lib/test_ml_plugins.py
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,16 @@ def test_disable_via_config_change(
self.plugin_manager._opensearch_config.add_plugin = MagicMock()
self.charm.status = MagicMock()

self.charm._get_nodes = MagicMock(
return_value={
"1": {},
"2": {},
"3": {},
}
)
self.charm.app.planned_units = MagicMock(return_value=3)
self.charm.opensearch.is_node_up = MagicMock(return_value=True)

mock_cluster_settings.return_value = {"knn.plugin.enabled": True}

self.harness.update_config({"plugin_opensearch_knn": False})
Expand Down
30 changes: 30 additions & 0 deletions tests/unit/lib/test_plugins.py
Original file line number Diff line number Diff line change
Expand Up @@ -251,6 +251,16 @@ def test_reconfigure_and_add_keystore_plugin(
# Mock _installed_plugins to return test
mock_installed_plugins.return_value = ["test"]

self.charm._get_nodes = MagicMock(
return_value={
"1": {},
"2": {},
"3": {},
}
)
self.charm.app.planned_units = MagicMock(return_value=3)
self.charm.opensearch.is_node_up = MagicMock(return_value=True)

mock_load.return_value = {}
# run is called, but only _configure method really matter:
# Set install to false, so only _configure is evaluated
Expand Down Expand Up @@ -300,6 +310,16 @@ def test_plugin_setup_with_relation(
# Return a fake content of the relation
mock_process_relation.return_value = {"param": "tested"}

self.charm._get_nodes = MagicMock(
return_value={
"1": {},
"2": {},
"3": {},
}
)
self.charm.app.planned_units = MagicMock(return_value=3)
self.charm.opensearch.is_node_up = MagicMock(return_value=True)

# Keystore-related mocks
self.plugin_manager._keystore._add = MagicMock()
self.plugin_manager._opensearch.request = MagicMock(return_value={"status": 200})
Expand Down Expand Up @@ -371,6 +391,16 @@ def test_disable_plugin(
# Mock _installed_plugins to return test
mock_installed_plugins.return_value = ["test"]

self.charm._get_nodes = MagicMock(
return_value={
"1": {},
"2": {},
"3": {},
}
)
self.charm.app.planned_units = MagicMock(return_value=3)
self.charm.opensearch.is_node_up = MagicMock(return_value=True)

mock_get_cluster_settings.return_value = {"param": "tested"}
mock_plugin_relation.return_value = False
# plugin is initially disabled and enabled when method self._disable calls self.status
Expand Down

0 comments on commit afbca49

Please sign in to comment.