From 2e8b375a0716c5889cbebe6b44f31ee2d4b26e39 Mon Sep 17 00:00:00 2001 From: Spyros Trigazis Date: Tue, 26 May 2020 15:51:56 +0000 Subject: [PATCH] resize: Send only nodes_to_remove and node_count When resizing a NG we should strictly send the desired node_count and the nodes_to_remove. Otherwise the stack update operation may replace/rebuild nodes or other resources. This was the functionality with: Id84e5d878b21c908021e631514c2c58b3fe8b8b0 But it was reverted with: I725413e77f5a7bdb48131e8a10e5dc884b5e066a Story: 2005266 task: 39860 master Conflicts: magnum/drivers/heat/k8s_template_def.py ussuri Conflicts: magnum/tests/unit/drivers/test_template_definition.py master (cherry-picked from 9f4c63a0df197405daa7bc3cd6939911ee3b7acf) ussuri (cherry-picked from 83ed6fe5d81abd29400e3a8279855851014f5b89) Change-Id: Ib31b6801e0e2d954c31ac91e77ae9d3ef1afebd2 Signed-off-by: Spyros Trigazis --- magnum/drivers/heat/driver.py | 57 ++++++++----------- magnum/drivers/heat/k8s_template_def.py | 5 +- .../drivers/heat/swarm_fedora_template_def.py | 6 ++ .../drivers/heat/swarm_mode_template_def.py | 6 ++ .../drivers/mesos_ubuntu_v1/template_def.py | 5 +- .../handlers/test_k8s_cluster_conductor.py | 3 +- .../unit/drivers/test_template_definition.py | 37 ++++++------ 7 files changed, 64 insertions(+), 55 deletions(-) diff --git a/magnum/drivers/heat/driver.py b/magnum/drivers/heat/driver.py index f2429ea83d..a9f0082063 100755 --- a/magnum/drivers/heat/driver.py +++ b/magnum/drivers/heat/driver.py @@ -237,60 +237,49 @@ class HeatDriver(driver.Driver): def _update_stack(self, context, cluster, scale_manager=None, rollback=False): + # update worked properly only for scaling nodes up and down + # before nodegroups. Maintain this logic until we deprecate + # and remove the command. + # Fixed behaviour Id84e5d878b21c908021e631514c2c58b3fe8b8b0 + nodegroup = cluster.default_ng_worker definition = self.get_template_definition() - - osc = clients.OpenStackClients(context) - heat_params = {} - - # Find what changed checking the stack params - # against the ones in the template_def. - stack = osc.heat().stacks.get(cluster.stack_id, - resolve_outputs=True) - stack_params = stack.parameters - definition.add_nodegroup_params(cluster) - heat_params = definition.get_stack_diff(context, stack_params, cluster) - scale_params = definition.get_scale_params(context, - cluster, - scale_manager) - heat_params.update(scale_params) + scale_params = definition.get_scale_params( + context, + cluster, + nodegroup.node_count, + scale_manager, + nodes_to_remove=None) fields = { - 'parameters': heat_params, + 'parameters': scale_params, 'existing': True, 'disable_rollback': not rollback } LOG.info('Updating cluster %s stack %s with these params: %s', - cluster.uuid, cluster.stack_id, heat_params) - osc.heat().stacks.update(cluster.stack_id, **fields) + cluster.uuid, nodegroup.stack_id, scale_params) + osc = clients.OpenStackClients(context) + osc.heat().stacks.update(nodegroup.stack_id, **fields) def _resize_stack(self, context, cluster, resize_manager, node_count, nodes_to_remove, nodegroup=None, rollback=False): definition = self.get_template_definition() - osc = clients.OpenStackClients(context) + scale_params = definition.get_scale_params( + context, + cluster, + nodegroup.node_count, + resize_manager, + nodes_to_remove=nodes_to_remove) - # Find what changed checking the stack params - # against the ones in the template_def. - stack = osc.heat().stacks.get(nodegroup.stack_id, - resolve_outputs=True) - stack_params = stack.parameters - definition.add_nodegroup_params(cluster, nodegroups=[nodegroup]) - heat_params = definition.get_stack_diff(context, stack_params, cluster) - - scale_params = definition.get_scale_params(context, - cluster, - resize_manager, - nodes_to_remove) - heat_params.update(scale_params) fields = { - 'parameters': heat_params, + 'parameters': scale_params, 'existing': True, 'disable_rollback': not rollback } LOG.info('Resizing cluster %s stack %s with these params: %s', - cluster.uuid, nodegroup.stack_id, heat_params) + cluster.uuid, nodegroup.stack_id, scale_params) osc = clients.OpenStackClients(context) osc.heat().stacks.update(nodegroup.stack_id, **fields) diff --git a/magnum/drivers/heat/k8s_template_def.py b/magnum/drivers/heat/k8s_template_def.py index 36985cccc7..230014cbf9 100644 --- a/magnum/drivers/heat/k8s_template_def.py +++ b/magnum/drivers/heat/k8s_template_def.py @@ -267,8 +267,8 @@ class K8sTemplateDefinition(template_def.BaseTemplateDefinition): extra_params=extra_params, **kwargs) - def get_scale_params(self, context, cluster, scale_manager=None, - nodes_to_remove=None): + def get_scale_params(self, context, cluster, node_count, + scale_manager=None, nodes_to_remove=None): scale_params = dict() if nodes_to_remove: scale_params['minions_to_remove'] = nodes_to_remove @@ -276,4 +276,5 @@ class K8sTemplateDefinition(template_def.BaseTemplateDefinition): hosts = self.get_output('kube_minions_private') scale_params['minions_to_remove'] = ( scale_manager.get_removal_nodes(hosts)) + scale_params['number_of_minions'] = node_count return scale_params diff --git a/magnum/drivers/heat/swarm_fedora_template_def.py b/magnum/drivers/heat/swarm_fedora_template_def.py index 6b5a5f1bfe..80bac4a204 100644 --- a/magnum/drivers/heat/swarm_fedora_template_def.py +++ b/magnum/drivers/heat/swarm_fedora_template_def.py @@ -166,3 +166,9 @@ class SwarmFedoraTemplateDefinition(template_def.BaseTemplateDefinition): template_def.add_lb_env_file(env_files, cluster_template) return env_files + + def get_scale_params(self, context, cluster, node_count, + scale_manager=None, nodes_to_remove=None): + scale_params = dict() + scale_params['number_of_nodes'] = node_count + return scale_params diff --git a/magnum/drivers/heat/swarm_mode_template_def.py b/magnum/drivers/heat/swarm_mode_template_def.py index 9b466922de..c50eff6a5d 100644 --- a/magnum/drivers/heat/swarm_mode_template_def.py +++ b/magnum/drivers/heat/swarm_mode_template_def.py @@ -193,3 +193,9 @@ class SwarmModeTemplateDefinition(template_def.BaseTemplateDefinition): template_def.add_fip_env_file(env_files, cluster_template, cluster) return env_files + + def get_scale_params(self, context, cluster, node_count, + scale_manager=None, nodes_to_remove=None): + scale_params = dict() + scale_params['number_of_nodes'] = node_count + return scale_params diff --git a/magnum/drivers/mesos_ubuntu_v1/template_def.py b/magnum/drivers/mesos_ubuntu_v1/template_def.py index e6686ea4de..bbf092cf35 100644 --- a/magnum/drivers/mesos_ubuntu_v1/template_def.py +++ b/magnum/drivers/mesos_ubuntu_v1/template_def.py @@ -111,8 +111,8 @@ class UbuntuMesosTemplateDefinition(template_def.BaseTemplateDefinition): extra_params=extra_params, **kwargs) - def get_scale_params(self, context, cluster, scale_manager=None, - nodes_to_remove=None): + def get_scale_params(self, context, cluster, node_count, + scale_manager=None, nodes_to_remove=None): scale_params = dict() if nodes_to_remove: scale_params['slaves_to_remove'] = nodes_to_remove @@ -120,6 +120,7 @@ class UbuntuMesosTemplateDefinition(template_def.BaseTemplateDefinition): hosts = self.get_output('mesos_slaves_private') scale_params['slaves_to_remove'] = ( scale_manager.get_removal_nodes(hosts)) + scale_params['number_of_slaves'] = node_count return scale_params def get_env_files(self, cluster_template, cluster, nodegroup=None): diff --git a/magnum/tests/unit/conductor/handlers/test_k8s_cluster_conductor.py b/magnum/tests/unit/conductor/handlers/test_k8s_cluster_conductor.py index cfe35754d4..6d2820f8c8 100644 --- a/magnum/tests/unit/conductor/handlers/test_k8s_cluster_conductor.py +++ b/magnum/tests/unit/conductor/handlers/test_k8s_cluster_conductor.py @@ -1273,13 +1273,14 @@ class TestClusterConductorWithK8s(base.TestCase): mock_cluster.cluster_template = mock_template self.worker_ng_dict['node_count'] = 2 worker_ng = objects.NodeGroup(self.context, **self.worker_ng_dict) + worker_ng.stack_id = mock_stack_id master_ng = objects.NodeGroup(self.context, **self.master_ng_dict) mock_objects_nodegroup_list.return_value = [master_ng, worker_ng] k8s_dr.Driver().update_cluster({}, mock_cluster) expected_args = { - 'parameters': {'number_of_minions': '2'}, + 'parameters': {'number_of_minions': 2}, 'existing': True, 'disable_rollback': True } diff --git a/magnum/tests/unit/drivers/test_template_definition.py b/magnum/tests/unit/drivers/test_template_definition.py index 5ad255ded5..ff90e05145 100644 --- a/magnum/tests/unit/drivers/test_template_definition.py +++ b/magnum/tests/unit/drivers/test_template_definition.py @@ -311,18 +311,6 @@ class TemplateDefinitionTestCase(base.TestCase): env_files ) - @mock.patch('magnum.drivers.common.driver.Driver.get_driver') - def test_base_get_scale_params(self, mock_driver): - mock_context = mock.MagicMock() - mock_cluster = mock.MagicMock() - mock_driver.return_value = swarm_v2_dr.Driver() - cluster_driver = driver.Driver.get_driver('vm', - 'fedora-atomic', - 'swarm-mode') - definition = cluster_driver.get_template_definition() - self.assertEqual(definition.get_scale_params(mock_context, - mock_cluster), {}) - @six.add_metaclass(abc.ABCMeta) class BaseK8sTemplateDefinitionTestCase(base.TestCase): @@ -401,14 +389,19 @@ class AtomicK8sTemplateDefinitionTestCase(BaseK8sTemplateDefinitionTestCase): mock_cluster = mock.MagicMock() removal_nodes = ['node1', 'node2'] + node_count = 5 mock_scale_manager = mock.MagicMock() mock_scale_manager.get_removal_nodes.return_value = removal_nodes definition = k8sa_tdef.AtomicK8sTemplateDefinition() scale_params = definition.get_scale_params(mock_context, mock_cluster, + node_count, mock_scale_manager) - expected_scale_params = {'minions_to_remove': ['node1', 'node2']} + expected_scale_params = { + 'minions_to_remove': ['node1', 'node2'], + 'number_of_minions': 5 + } self.assertEqual(scale_params, expected_scale_params) @mock.patch('magnum.common.neutron.get_fixed_network_name') @@ -1703,6 +1696,13 @@ class AtomicSwarmModeTemplateDefinitionTestCase(base.TestCase): heat_param = swarm_def.get_heat_param(cluster_attr='uuid') self.assertEqual('cluster_uuid', heat_param) + def test_swarm_get_scale_params(self): + mock_context = mock.MagicMock() + swarm_def = swarm_v2_tdef.AtomicSwarmTemplateDefinition() + self.assertEqual( + swarm_def.get_scale_params(mock_context, self.mock_cluster, 7), + {'number_of_nodes': 7}) + def test_update_outputs(self): swarm_def = swarm_v2_tdef.AtomicSwarmTemplateDefinition() @@ -2037,14 +2037,19 @@ class UbuntuMesosTemplateDefinitionTestCase(base.TestCase): mock_cluster.uuid = '5d12f6fd-a196-4bf0-ae4c-1f639a523a52' removal_nodes = ['node1', 'node2'] + node_count = 7 mock_scale_manager = mock.MagicMock() mock_scale_manager.get_removal_nodes.return_value = removal_nodes mesos_def = mesos_tdef.UbuntuMesosTemplateDefinition() - scale_params = mesos_def.get_scale_params(mock_context, mock_cluster, - mock_scale_manager) - expected_scale_params = {'slaves_to_remove': ['node1', 'node2']} + scale_params = mesos_def.get_scale_params( + mock_context, + mock_cluster, + node_count, + mock_scale_manager) + expected_scale_params = {'slaves_to_remove': ['node1', 'node2'], + 'number_of_slaves': 7} self.assertEqual(scale_params, expected_scale_params) def test_mesos_get_heat_param(self):