Skip to content

Commit

Permalink
Add fixes for test_plugins.py
Browse files Browse the repository at this point in the history
  • Loading branch information
phvalguima committed Jun 11, 2024
1 parent 752212c commit f580c64
Show file tree
Hide file tree
Showing 3 changed files with 24 additions and 24 deletions.
2 changes: 1 addition & 1 deletion lib/charms/opensearch/v0/opensearch_backups.py
Original file line number Diff line number Diff line change
Expand Up @@ -1091,7 +1091,7 @@ def __init__(self, charm: CharmBase):
def is_relation_set(self) -> bool:
"""Checks if the relation is set for the plugin handler."""
relation = self._charm.model.get_relation(self._relation_name)
return relation and relation.units
return relation is not None and relation.units

@property
def _relation_name(self) -> str:
Expand Down
44 changes: 22 additions & 22 deletions lib/charms/opensearch/v0/opensearch_plugin_manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -271,7 +271,10 @@ def _install_if_needed(self, plugin: OpenSearchPlugin) -> bool:
def _configure_if_needed(self, plugin: OpenSearchPlugin) -> bool:
"""Gathers all the configuration changes needed and applies them."""
try:
if self.status(plugin) != PluginState.INSTALLED:
if self.status(plugin) not in [
PluginState.INSTALLED,
PluginState.DISABLED,
] or not self._user_requested_to_enable(plugin):
# Leave this method if either user did not request to enable this plugin
# or plugin has been already enabled.
return False
Expand All @@ -283,9 +286,6 @@ def _disable_if_needed(self, plugin: OpenSearchPlugin) -> bool:
"""If disabled, removes plugin configuration or sets it to other values."""
try:
if self._user_requested_to_enable(plugin):
# Only considering "INSTALLED" or "WAITING FOR UPGRADE" status as it
# represents a plugin that has been installed but either not yet configured
# or user explicitly disabled.
return False
return self.apply_config(plugin.disable())
except KeyError as e:
Expand Down Expand Up @@ -400,7 +400,10 @@ def apply_config(self, config: OpenSearchPluginConfig) -> bool: # noqa: C901
return all(
[
(config.secret_entries_to_add or config.secret_entries_to_del),
not cluster_settings_changed,
(
(config.config_entries_to_add or config.config_entries_to_del)
and not cluster_settings_changed
),
]
)

Expand All @@ -414,10 +417,9 @@ def status(self, plugin: OpenSearchPlugin) -> PluginState:

# The _user_request_to_enable comes first, as it ensures there is a relation/config
# set, which will be used by _is_enabled to determine if we are enabled or not.
if not self._user_requested_to_enable(plugin) and not self._is_enabled(plugin):
if not self._is_enabled(plugin):
return PluginState.DISABLED

if self._is_enabled(plugin):
elif self._user_requested_to_enable(plugin):
return PluginState.ENABLED

return PluginState.INSTALLED
Expand Down Expand Up @@ -446,10 +448,6 @@ def _is_enabled(self, plugin: OpenSearchPlugin) -> bool:
OpenSearchKeystoreNotReadyYetError: If the keystore is not yet ready.
"""
try:
current_settings, new_conf = self._compute_settings(plugin.config())
if current_settings and new_conf and current_settings != new_conf:
return False

# 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:
Expand All @@ -463,20 +461,22 @@ def _is_enabled(self, plugin: OpenSearchPlugin) -> bool:
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
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
# We always check the configuration files, as we always persist data there
config = {
k: None for k in plugin.config().config_entries_to_del
} | plugin.config().config_entries_to_add
existing_setup = self._opensearch_config.get_plugin(config)
return all(
[
(k in existing_setup and config[k] == existing_setup[k])
or (k not in existing_setup and config[k] is None)
for k in config.keys()
]
)

except (OpenSearchKeystoreError, KeyError, OpenSearchPluginError) as e:
logger.warning(f"_is_enabled: error with {e}")
return False
return True

def _needs_upgrade(self, plugin: OpenSearchPlugin) -> bool:
"""Returns true if plugin needs upgrade."""
Expand Down
2 changes: 1 addition & 1 deletion tests/integration/plugins/test_plugins.py
Original file line number Diff line number Diff line change
Expand Up @@ -454,7 +454,7 @@ async def test_knn_training_search(ops_test: OpsTest) -> None:
assert await check_cluster_formation_successful(
ops_test, leader_unit_ip, get_application_unit_names(ops_test, app=APP_NAME)
), "Restart happened but cluster did not start correctly"
logger.info("Restart finished and was successful")
logger.info("Config updated and was successful")

query = {
"size": 2,
Expand Down

0 comments on commit f580c64

Please sign in to comment.