self.status now considers cluster_settings

lib/charms/opensearch/v0/opensearch_plugin_manager.py

@@ -11,7 +11,7 @@
 """
 
 import logging
-from typing import Any, Dict, List, Optional
+from typing import Any, Dict, List, Optional, Tuple
 
 from charms.opensearch.v0.constants_charm import PluginConfigStart
 from charms.opensearch.v0.helper_cluster import ClusterTopology
@@ -256,6 +256,42 @@ def _disable_if_needed(self, plugin: OpenSearchPlugin) -> bool:
         except KeyError as e:
             raise OpenSearchPluginMissingConfigError(e)
 
+    def _compute_settings(
+        self, config: OpenSearchPluginConfig
+    ) -> Tuple[Dict[str, str], Dict[str, str]]:
+        """Returns the current and the new configuration."""
+        current_settings = ClusterTopology.get_cluster_settings(
+            self._charm.opensearch,
+            include_defaults=True,
+        )
+        to_remove = dict(
+            zip(config.config_entries_to_del, [None] * len(config.config_entries_to_del))
+        )
+        new_conf = {
+            **current_settings,
+            **to_remove,
+            **config.config_entries_to_add,
+        }
+        logger.debug(
+            "Difference between current and new configuration: \n"
+            + str(
+                {
+                    "in current but not in new_conf": {
+                        k: v for k, v in current_settings.items() if k not in new_conf.keys()
+                    },
+                    "in new_conf but not in current": {
+                        k: v for k, v in new_conf.items() if k not in current_settings.keys()
+                    },
+                    "in both but different values": {
+                        k: v
+                        for k, v in new_conf.items()
+                        if k in current_settings.keys() and current_settings[k] != v
+                    },
+                }
+            )
+        )
+        return current_settings, new_conf
+
     def apply_config(self, config: OpenSearchPluginConfig) -> bool:
         """Runs the configuration changes as passed via OpenSearchPluginConfig.
 
@@ -270,39 +306,7 @@ def apply_config(self, config: OpenSearchPluginConfig) -> bool:
         if config.secret_entries_to_del or config.secret_entries_to_add:
             self._keystore.reload_keystore()
 
-        # Add and remove configuration if applies
-        current_settings = ClusterTopology.get_cluster_settings(
-            self._charm.opensearch,
-            include_defaults=True,
-        )
-        to_remove = config.config_entries_to_del
-        if isinstance(config.config_entries_to_del, list):
-            # No keys should have value "None", therefore, setting them to None means
-            # the original current_settings will be changed.
-            to_remove = dict(
-                zip(config.config_entries_to_del, [None] * len(config.config_entries_to_del))
-            )
-
-        new_conf = {
-            **current_settings,
-            **to_remove,
-            **config.config_entries_to_add,
-        }
-
-        diffs = {
-            "in current but not in new_conf": {
-                k: v for k, v in current_settings.items() if k not in new_conf.keys()
-            },
-            "in new_conf but not in current": {
-                k: v for k, v in new_conf.items() if k not in current_settings.keys()
-            },
-            "in both but different values": {
-                k: v
-                for k, v in new_conf.items()
-                if k in current_settings.keys() and current_settings[k] != v
-            },
-        }
-        logger.debug(f"Difference between current and new configuration: \n{diffs}")
+        current_settings, new_conf = self._compute_settings(config)
         if current_settings == new_conf:
             # Nothing to do here
             logger.info("apply_config: nothing to do, return")
@@ -344,32 +348,28 @@ def _user_requested_to_enable(self, plugin: OpenSearchPlugin) -> bool:
     def _is_enabled(self, plugin: OpenSearchPlugin) -> bool:
         """Returns true if plugin is enabled.
 
-        The main question to answer is if we would remove the configuration from
-        opensearch.yaml and/or add some other configuration instead.
-        If yes, then we know that the service is enabled.
+        The main question to answer is if we would have it in the configuration
+        from cluster settings. If yes, then we know that the service is enabled.
 
-        The disable() method is used, as a given entry will be removed from opensearch.yml.
-
-        If disable() returns a list, then check if the keys in the stored_plugin_conf
-        matches the list values; otherwise, if a dict, then check if the keys and values match.
-        If they match in any of the cases above, then return True.
+        Check if the configuration from enable() is present or not.
         """
         try:
-            plugin_conf = plugin.disable().config_entries_to_del
-            keystore_conf = plugin.disable().secret_entries_to_del
-            stored_plugin_conf = self._opensearch_config.get_plugin(plugin_conf)
-
-            if any(k not in self._keystore.list() for k in keystore_conf):
+            current_settings, new_conf = self._compute_settings(plugin.config())
+            if current_settings != new_conf:
                 return False
 
-            # Using sets to guarantee matches; stored_plugin_conf will be a dict
-            if isinstance(plugin_conf, list):
-                return set(plugin_conf) == set(stored_plugin_conf)
-            else:  # plugin_conf is a dict
-                return plugin_conf == stored_plugin_conf
+            # Now, focus in 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):
+                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
         except (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."""

tests/integration/plugins/test_plugins.py

@@ -79,6 +79,7 @@ async def test_build_and_deploy(ops_test: OpsTest) -> None:
     assert len(ops_test.model.applications[APP_NAME].units) == 3
 
 
+@pytest.mark.abort_on_fail
 async def test_prometheus_exporter_enabled_by_default(ops_test):
     """Test that Prometheus Exporter is running before the relation is there."""
     leader_unit_ip = await get_leader_unit_ip(ops_test, app=APP_NAME)
@@ -90,6 +91,7 @@ async def test_prometheus_exporter_enabled_by_default(ops_test):
     assert len(response_str.split("\n")) > 500
 
 
+@pytest.mark.abort_on_fail
 async def test_prometheus_exporter_cos_relation(ops_test):
     await ops_test.model.deploy(COS_APP_NAME, channel="edge"),
     await ops_test.model.integrate(APP_NAME, COS_APP_NAME)
@@ -119,6 +121,7 @@ async def test_prometheus_exporter_cos_relation(ops_test):
     assert relation_data["scheme"] == "https"
 
 
+@pytest.mark.abort_on_fail
 async def test_monitoring_user_fetch_prometheus_data(ops_test):
     leader_unit_ip = await get_leader_unit_ip(ops_test, app=APP_NAME)
     endpoint = f"https://{leader_unit_ip}:9200/_prometheus/metrics"
@@ -139,6 +142,7 @@ async def test_monitoring_user_fetch_prometheus_data(ops_test):
     assert len(response_str.split("\n")) > 500
 
 
+@pytest.mark.abort_on_fail
 async def test_prometheus_monitor_user_password_change(ops_test):
     # Password change applied as expected
     leader_id = await get_leader_unit_id(ops_test, APP_NAME)
@@ -161,6 +165,7 @@ async def test_prometheus_monitor_user_password_change(ops_test):
     assert relation_data["password"] == new_password
 
 
+@pytest.mark.abort_on_fail
 async def test_knn_enabled_disabled(ops_test):
     config = await ops_test.model.applications[APP_NAME].get_config()
     assert config["plugin_opensearch_knn"]["default"] is True
@@ -177,6 +182,7 @@ async def test_knn_enabled_disabled(ops_test):
 
     config = await ops_test.model.applications[APP_NAME].get_config()
     assert config["plugin_opensearch_knn"]["value"] is True
+    await ops_test.model.wait_for_idle(apps=[APP_NAME], status="active", idle_period=45)
 
 
 @pytest.mark.abort_on_fail
@@ -218,7 +224,7 @@ async def test_knn_search_with_hnsw_faiss(ops_test: OpsTest) -> None:
     # Insert data in bulk
     await bulk_insert(ops_test, app, leader_unit_ip, payload)
     query = {"size": 2, "query": {"knn": {vector_name: {"vector": payload_list[0], "k": 2}}}}
-    docs = await search(ops_test, app, leader_unit_ip, index_name, query)
+    docs = await search(ops_test, app, leader_unit_ip, index_name, query, retries=30)
     assert len(docs) == 2
 
 
@@ -261,7 +267,7 @@ async def test_knn_search_with_hnsw_nmslib(ops_test: OpsTest) -> None:
     # Insert data in bulk
     await bulk_insert(ops_test, app, leader_unit_ip, payload)
     query = {"size": 2, "query": {"knn": {vector_name: {"vector": payload_list[0], "k": 2}}}}
-    docs = await search(ops_test, app, leader_unit_ip, index_name, query)
+    docs = await search(ops_test, app, leader_unit_ip, index_name, query, retries=30)
     assert len(docs) == 2