From 236ff8fb830d4362896e26db9ec75eac10b5e7ec Mon Sep 17 00:00:00 2001 From: Carl Csaposs Date: Fri, 12 Apr 2024 10:28:19 +0200 Subject: [PATCH] Remove role-rebalancing See #230 Implemented in a minimal, hacky way to proceed with testing. Will be implemented fully (i.e. unnecessary code removed) by @Mehdi-Bendriss in another PR --- lib/charms/opensearch/v0/helper_cluster.py | 23 +--- tests/unit/lib/test_helper_cluster.py | 141 +++++++++++---------- 2 files changed, 76 insertions(+), 88 deletions(-) diff --git a/lib/charms/opensearch/v0/helper_cluster.py b/lib/charms/opensearch/v0/helper_cluster.py index 58bf0dbab..b7195f883 100644 --- a/lib/charms/opensearch/v0/helper_cluster.py +++ b/lib/charms/opensearch/v0/helper_cluster.py @@ -46,14 +46,8 @@ def suggest_roles(nodes: List[Node], planned_units: int) -> List[str]: — odd: "all" the nodes are cm_eligible nodes. — even: "all - 1" are cm_eligible and 1 data node. """ - max_cms = ClusterTopology.max_cluster_manager_nodes(planned_units) - - base_roles = ["data", "ingest", "ml", "coordinating_only"] - full_roles = base_roles + ["cluster_manager"] - nodes_by_roles = ClusterTopology.nodes_count_by_role(nodes) - if nodes_by_roles.get("cluster_manager", 0) == max_cms: - return base_roles - return full_roles + # TODO: remove in https://github.com/canonical/opensearch-operator/issues/230 + return ["data", "ingest", "ml", "coordinating_only", "cluster_manager"] @staticmethod def get_cluster_settings( @@ -74,6 +68,7 @@ def get_cluster_settings( @staticmethod def recompute_nodes_conf(app_name: str, nodes: List[Node]) -> Dict[str, Node]: + # TODO: remove in https://github.com/canonical/opensearch-operator/issues/230 """Recompute the configuration of all the nodes (cluster set to auto-generate roles).""" if not nodes: return {} @@ -86,19 +81,11 @@ def recompute_nodes_conf(app_name: str, nodes: List[Node]) -> Dict[str, Node]: else: # Leave node unchanged nodes_by_name[node.name] = node - base_roles = ["data", "ingest", "ml", "coordinating_only"] - full_roles = base_roles + ["cluster_manager"] - highest_unit_number = max(node.unit_number for node in current_cluster_nodes) for node in current_cluster_nodes: - # we do this in order to remove any non-default role / add any missing default role - if len(current_cluster_nodes) % 2 == 0 and node.unit_number == highest_unit_number: - roles = base_roles - else: - roles = full_roles - nodes_by_name[node.name] = Node( name=node.name, - roles=roles, + # we do this in order to remove any non-default role / add any missing default role + roles=["data", "ingest", "ml", "coordinating_only", "cluster_manager"], ip=node.ip, app_name=node.app_name, unit_number=node.unit_number, diff --git a/tests/unit/lib/test_helper_cluster.py b/tests/unit/lib/test_helper_cluster.py index 6f93dc13f..463fb805b 100644 --- a/tests/unit/lib/test_helper_cluster.py +++ b/tests/unit/lib/test_helper_cluster.py @@ -124,76 +124,77 @@ def setUp(self) -> None: self.opensearch = self.charm.opensearch - def test_topology_roles_suggestion_odd_number_of_planned_units(self): - """Test the suggestion of roles for a new node and odd numbers of planned units.""" - planned_units = 5 - cluster_5_conf = self.cluster1_5_nodes_conf() - - self.assertCountEqual(ClusterTopology.suggest_roles([], planned_units), self.cm_roles) - for start_index in range(1, 5): - self.assertCountEqual( - ClusterTopology.suggest_roles(cluster_5_conf[:start_index], planned_units), - self.cm_roles, - ) - - def test_topology_roles_suggestion_even_number_of_planned_units(self): - """Test the suggestion of roles for a new node and even numbers of planned units.""" - cluster_6_conf = self.cluster1_6_nodes_conf() - - planned_units = 6 - - self.assertCountEqual(ClusterTopology.suggest_roles([], planned_units), self.cm_roles) - for start_index in range(1, 5): - self.assertCountEqual( - ClusterTopology.suggest_roles(cluster_6_conf[:start_index], planned_units), - self.cm_roles, - ) - - self.assertCountEqual( - ClusterTopology.suggest_roles(cluster_6_conf[:-1], planned_units), - self.base_roles, - ) - - def test_auto_recompute_node_roles_in_cluster_6(self): - """Test the automatic suggestion of new roles to an existing node.""" - cluster_conf = {node.name: node for node in self.cluster1_6_nodes_conf()} - - # remove a cluster manager node - old_cluster_conf = cluster_conf.copy() - old_cluster_conf.pop("cm1") - new_cluster_conf = ClusterTopology.recompute_nodes_conf( - app_name=self.cluster1, nodes=list(old_cluster_conf.values()) - ) - assert new_cluster_conf["data1"].roles == self.cm_roles - # Assert other remaining nodes unchanged - old_cluster_conf.pop("data1") - new_cluster_conf.pop("data1") - assert old_cluster_conf == new_cluster_conf - - # remove a data node - old_cluster_conf = cluster_conf.copy() - old_cluster_conf.pop("data1") - new_cluster_conf = ClusterTopology.recompute_nodes_conf( - app_name=self.cluster1, nodes=list(old_cluster_conf.values()) - ) - # Assert all remaining nodes unchanged - assert old_cluster_conf == new_cluster_conf - - def test_auto_recompute_node_roles_in_cluster_5(self): - """Test the automatic suggestion of new roles to an existing node.""" - cluster_conf = {node.name: node for node in self.cluster1_5_nodes_conf()} - - # remove a cluster manager node - old_cluster_conf = cluster_conf.copy() - old_cluster_conf.pop("cm1") - new_cluster_conf = ClusterTopology.recompute_nodes_conf( - app_name=self.cluster1, nodes=list(old_cluster_conf.values()) - ) - assert new_cluster_conf["cm5"].roles == self.base_roles - # Assert other remaining nodes unchanged - old_cluster_conf.pop("cm5") - new_cluster_conf.pop("cm5") - assert old_cluster_conf == new_cluster_conf + # TODO: remove in https://github.com/canonical/opensearch-operator/issues/230 + # def test_topology_roles_suggestion_odd_number_of_planned_units(self): + # """Test the suggestion of roles for a new node and odd numbers of planned units.""" + # planned_units = 5 + # cluster_5_conf = self.cluster1_5_nodes_conf() + # + # self.assertCountEqual(ClusterTopology.suggest_roles([], planned_units), self.cm_roles) + # for start_index in range(1, 5): + # self.assertCountEqual( + # ClusterTopology.suggest_roles(cluster_5_conf[:start_index], planned_units), + # self.cm_roles, + # ) + # + # def test_topology_roles_suggestion_even_number_of_planned_units(self): + # """Test the suggestion of roles for a new node and even numbers of planned units.""" + # cluster_6_conf = self.cluster1_6_nodes_conf() + # + # planned_units = 6 + # + # self.assertCountEqual(ClusterTopology.suggest_roles([], planned_units), self.cm_roles) + # for start_index in range(1, 5): + # self.assertCountEqual( + # ClusterTopology.suggest_roles(cluster_6_conf[:start_index], planned_units), + # self.cm_roles, + # ) + # + # self.assertCountEqual( + # ClusterTopology.suggest_roles(cluster_6_conf[:-1], planned_units), + # self.base_roles, + # ) + # + # def test_auto_recompute_node_roles_in_cluster_6(self): + # """Test the automatic suggestion of new roles to an existing node.""" + # cluster_conf = {node.name: node for node in self.cluster1_6_nodes_conf()} + # + # # remove a cluster manager node + # old_cluster_conf = cluster_conf.copy() + # old_cluster_conf.pop("cm1") + # new_cluster_conf = ClusterTopology.recompute_nodes_conf( + # app_name=self.cluster1, nodes=list(old_cluster_conf.values()) + # ) + # assert new_cluster_conf["data1"].roles == self.cm_roles + # # Assert other remaining nodes unchanged + # old_cluster_conf.pop("data1") + # new_cluster_conf.pop("data1") + # assert old_cluster_conf == new_cluster_conf + # + # # remove a data node + # old_cluster_conf = cluster_conf.copy() + # old_cluster_conf.pop("data1") + # new_cluster_conf = ClusterTopology.recompute_nodes_conf( + # app_name=self.cluster1, nodes=list(old_cluster_conf.values()) + # ) + # # Assert all remaining nodes unchanged + # assert old_cluster_conf == new_cluster_conf + # + # def test_auto_recompute_node_roles_in_cluster_5(self): + # """Test the automatic suggestion of new roles to an existing node.""" + # cluster_conf = {node.name: node for node in self.cluster1_5_nodes_conf()} + # + # # remove a cluster manager node + # old_cluster_conf = cluster_conf.copy() + # old_cluster_conf.pop("cm1") + # new_cluster_conf = ClusterTopology.recompute_nodes_conf( + # app_name=self.cluster1, nodes=list(old_cluster_conf.values()) + # ) + # assert new_cluster_conf["cm5"].roles == self.base_roles + # # Assert other remaining nodes unchanged + # old_cluster_conf.pop("cm5") + # new_cluster_conf.pop("cm5") + # assert old_cluster_conf == new_cluster_conf def test_auto_recompute_node_roles_in_previous_non_auto_gen_cluster(self): """Test the automatic suggestion of new roles to an existing node."""