diff --git a/magnum/drivers/heat/driver.py b/magnum/drivers/heat/driver.py index afec2d4589..e27cefdd23 100755 --- a/magnum/drivers/heat/driver.py +++ b/magnum/drivers/heat/driver.py @@ -104,8 +104,7 @@ class HeatDriver(driver.Driver): def update_cluster(self, context, cluster, scale_manager=None, rollback=False): - self._update_stack(context, clients.OpenStackClients(context), cluster, - scale_manager, rollback) + self._update_stack(context, cluster, scale_manager, rollback) def delete_cluster(self, context, cluster): self._delete_stack(context, clients.OpenStackClients(context), cluster) @@ -152,26 +151,27 @@ class HeatDriver(driver.Driver): return created_stack - def _update_stack(self, context, osc, cluster, scale_manager=None, + def _update_stack(self, context, cluster, scale_manager=None, rollback=False): - template_path, heat_params, env_files = ( - self._extract_template_definition(context, cluster, - scale_manager=scale_manager)) + definition = self.get_template_definition() - tpl_files, template = template_utils.get_template_contents( - template_path) - environment_files, env_map = self._get_env_files(template_path, - env_files) - tpl_files.update(env_map) + heat_params = {} + # stack node_count parameter name + stack_nc_param = definition.get_heat_param(cluster_attr='node_count') + heat_params[stack_nc_param] = cluster.node_count + + scale_params = definition.get_scale_params(context, + cluster, + scale_manager) + heat_params.update(scale_params) fields = { 'parameters': heat_params, - 'environment_files': environment_files, - 'template': template, - 'files': tpl_files, + 'existing': True, 'disable_rollback': not rollback } + osc = clients.OpenStackClients(context) osc.heat().stacks.update(cluster.stack_id, **fields) def _delete_stack(self, context, osc, cluster): diff --git a/magnum/drivers/heat/k8s_template_def.py b/magnum/drivers/heat/k8s_template_def.py index d0e0592fc5..e5f4f74009 100644 --- a/magnum/drivers/heat/k8s_template_def.py +++ b/magnum/drivers/heat/k8s_template_def.py @@ -92,11 +92,6 @@ class K8sTemplateDefinition(template_def.BaseTemplateDefinition): def get_params(self, context, cluster_template, cluster, **kwargs): extra_params = kwargs.pop('extra_params', {}) - scale_mgr = kwargs.pop('scale_manager', None) - if scale_mgr: - hosts = self.get_output('kube_minions_private') - extra_params['minions_to_remove'] = ( - scale_mgr.get_removal_nodes(hosts)) extra_params['discovery_url'] = self.get_discovery_url(cluster) osc = self.get_osc(context) @@ -143,3 +138,11 @@ class K8sTemplateDefinition(template_def.BaseTemplateDefinition): self).get_params(context, cluster_template, cluster, extra_params=extra_params, **kwargs) + + def get_scale_params(self, context, cluster, scale_manager=None): + scale_params = dict() + if scale_manager: + hosts = self.get_output('kube_minions_private') + scale_params['minions_to_remove'] = ( + scale_manager.get_removal_nodes(hosts)) + return scale_params diff --git a/magnum/drivers/heat/template_def.py b/magnum/drivers/heat/template_def.py index b3b33148aa..3c8ba8dc3b 100755 --- a/magnum/drivers/heat/template_def.py +++ b/magnum/drivers/heat/template_def.py @@ -334,6 +334,9 @@ class BaseTemplateDefinition(TemplateDefinition): cluster.discovery_url = discovery_url return discovery_url + def get_scale_params(self, context, cluster, scale_manager=None): + return dict() + def add_lb_env_file(env_files, cluster_template): if cluster_template.master_lb_enabled: diff --git a/magnum/drivers/mesos_ubuntu_v1/template_def.py b/magnum/drivers/mesos_ubuntu_v1/template_def.py index 2a7d931112..533e3fe839 100644 --- a/magnum/drivers/mesos_ubuntu_v1/template_def.py +++ b/magnum/drivers/mesos_ubuntu_v1/template_def.py @@ -70,17 +70,19 @@ class UbuntuMesosTemplateDefinition(template_def.BaseTemplateDefinition): for label in label_list: extra_params[label] = cluster.labels.get(label) - scale_mgr = kwargs.pop('scale_manager', None) - if scale_mgr: - hosts = self.get_output('mesos_slaves_private') - extra_params['slaves_to_remove'] = ( - scale_mgr.get_removal_nodes(hosts)) - return super(UbuntuMesosTemplateDefinition, self).get_params(context, cluster_template, cluster, extra_params=extra_params, **kwargs) + def get_scale_params(self, context, cluster, scale_manager=None): + scale_params = dict() + if scale_manager: + hosts = self.get_output('mesos_slaves_private') + scale_params['slaves_to_remove'] = ( + scale_manager.get_removal_nodes(hosts)) + return scale_params + def get_env_files(self, cluster_template, cluster): env_files = [] 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 8bcc3d0d30..305e2c96e6 100644 --- a/magnum/tests/unit/conductor/handlers/test_k8s_cluster_conductor.py +++ b/magnum/tests/unit/conductor/handlers/test_k8s_cluster_conductor.py @@ -1095,13 +1095,6 @@ class TestClusterConductorWithK8s(base.TestCase): mock_get_template_contents): mock_stack_id = 'xx-xx-xx-xx' - expected_template_contents = 'template_contents' - - mock_tpl_files = {} - mock_get_template_contents.return_value = [ - mock_tpl_files, expected_template_contents] - mock_extract_template_definition.return_value = ('template/path', - {}, []) mock_heat_client = mock.MagicMock() mock_osc.return_value.heat.return_value = mock_heat_client mock_cluster = mock.MagicMock() @@ -1110,10 +1103,8 @@ class TestClusterConductorWithK8s(base.TestCase): k8s_dr.Driver().update_cluster({}, mock_cluster) expected_args = { - 'parameters': {}, - 'template': expected_template_contents, - 'files': {}, - 'environment_files': [], + 'parameters': {'number_of_minions': mock_cluster.node_count}, + 'existing': True, 'disable_rollback': True } mock_heat_client.stacks.update.assert_called_once_with(mock_stack_id, diff --git a/magnum/tests/unit/drivers/test_template_definition.py b/magnum/tests/unit/drivers/test_template_definition.py index edbf9dc85e..38a9417834 100644 --- a/magnum/tests/unit/drivers/test_template_definition.py +++ b/magnum/tests/unit/drivers/test_template_definition.py @@ -219,6 +219,18 @@ class TemplateDefinitionTestCase(base.TestCase): cmn_tdef.COMMON_ENV_PATH + 'disable_lb_floating_ip.yaml'], 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): @@ -268,6 +280,25 @@ class AtomicK8sTemplateDefinitionTestCase(BaseK8sTemplateDefinitionTestCase): def get_definition(self): return k8sa_dr.Driver().get_template_definition() + @mock.patch('magnum.common.clients.OpenStackClients') + @mock.patch('magnum.drivers.heat.template_def.TemplateDefinition' + '.get_output') + def test_k8s_get_scale_params(self, mock_get_output, + mock_osc_class): + mock_context = mock.MagicMock() + mock_cluster = mock.MagicMock() + + removal_nodes = ['node1', 'node2'] + 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, + mock_scale_manager) + expected_scale_params = {'minions_to_remove': ['node1', 'node2']} + self.assertEqual(scale_params, expected_scale_params) + @mock.patch('magnum.common.keystone.is_octavia_enabled') @mock.patch('magnum.common.clients.OpenStackClients') @mock.patch('magnum.drivers.k8s_fedora_atomic_v1.template_def' @@ -298,14 +329,11 @@ class AtomicK8sTemplateDefinitionTestCase(BaseK8sTemplateDefinitionTestCase): mock_cluster = mock.MagicMock() mock_cluster.uuid = '5d12f6fd-a196-4bf0-ae4c-1f639a523a52' del mock_cluster.stack_id - mock_scale_manager = mock.MagicMock() mock_osc = mock.MagicMock() mock_osc.magnum_url.return_value = 'http://127.0.0.1:9511/v1' mock_osc.cinder_region_name.return_value = 'RegionOne' mock_osc_class.return_value = mock_osc - removal_nodes = ['node1', 'node2'] - mock_scale_manager.get_removal_nodes.return_value = removal_nodes mock_get_discovery_url.return_value = 'fake_discovery_url' mock_context.auth_url = 'http://192.168.10.10:5000/v3' @@ -380,11 +408,9 @@ class AtomicK8sTemplateDefinitionTestCase(BaseK8sTemplateDefinitionTestCase): k8s_def = k8sa_tdef.AtomicK8sTemplateDefinition() - k8s_def.get_params(mock_context, mock_cluster_template, mock_cluster, - scale_manager=mock_scale_manager) + k8s_def.get_params(mock_context, mock_cluster_template, mock_cluster) expected_kwargs = {'extra_params': { - 'minions_to_remove': removal_nodes, 'discovery_url': 'fake_discovery_url', 'flannel_network_cidr': flannel_cidr, 'flannel_network_subnetlen': flannel_subnet, @@ -444,7 +470,6 @@ class AtomicK8sTemplateDefinitionTestCase(BaseK8sTemplateDefinitionTestCase): mock_context, mock_cluster_template, mock_cluster, - scale_manager=mock_scale_manager ) @mock.patch('magnum.common.keystone.is_octavia_enabled') @@ -477,14 +502,11 @@ class AtomicK8sTemplateDefinitionTestCase(BaseK8sTemplateDefinitionTestCase): mock_cluster = mock.MagicMock() mock_cluster.uuid = '5d12f6fd-a196-4bf0-ae4c-1f639a523a52' del mock_cluster.stack_id - mock_scale_manager = mock.MagicMock() mock_osc = mock.MagicMock() mock_osc.magnum_url.return_value = 'http://127.0.0.1:9511/v1' mock_osc.cinder_region_name.return_value mock_osc_class.return_value = mock_osc - removal_nodes = ['node1', 'node2'] - mock_scale_manager.get_removal_nodes.return_value = removal_nodes mock_get_discovery_url.return_value = 'fake_discovery_url' mock_context.auth_url = 'http://192.168.10.10:5000/v3' @@ -559,11 +581,9 @@ class AtomicK8sTemplateDefinitionTestCase(BaseK8sTemplateDefinitionTestCase): k8s_def = k8sa_tdef.AtomicK8sTemplateDefinition() - k8s_def.get_params(mock_context, mock_cluster_template, mock_cluster, - scale_manager=mock_scale_manager) + k8s_def.get_params(mock_context, mock_cluster_template, mock_cluster) expected_kwargs = {'extra_params': { - 'minions_to_remove': removal_nodes, 'discovery_url': 'fake_discovery_url', 'flannel_network_cidr': flannel_cidr, 'flannel_network_subnetlen': flannel_subnet, @@ -1315,9 +1335,8 @@ class UbuntuMesosTemplateDefinitionTestCase(base.TestCase): @mock.patch('magnum.common.clients.OpenStackClients') @mock.patch('magnum.drivers.heat.template_def.BaseTemplateDefinition' '.get_params') - @mock.patch('magnum.drivers.heat.template_def.TemplateDefinition' - '.get_output') - def test_mesos_get_params(self, mock_get_output, mock_get_params, + def test_mesos_get_params(self, + mock_get_params, mock_osc_class): mock_context = mock.MagicMock() mock_context.auth_url = 'http://192.168.10.10:5000/v3' @@ -1342,14 +1361,9 @@ class UbuntuMesosTemplateDefinitionTestCase(base.TestCase): mock_osc.cinder_region_name.return_value = 'RegionOne' mock_osc_class.return_value = mock_osc - removal_nodes = ['node1', 'node2'] - mock_scale_manager = mock.MagicMock() - mock_scale_manager.get_removal_nodes.return_value = removal_nodes - mesos_def = mesos_tdef.UbuntuMesosTemplateDefinition() - mesos_def.get_params(mock_context, mock_cluster_template, mock_cluster, - scale_manager=mock_scale_manager) + mesos_def.get_params(mock_context, mock_cluster_template, mock_cluster) expected_kwargs = {'extra_params': { 'region_name': mock_osc.cinder_region_name.return_value, @@ -1362,13 +1376,32 @@ class UbuntuMesosTemplateDefinitionTestCase(base.TestCase): 'mesos_slave_work_dir': mesos_slave_work_dir, 'mesos_slave_executor_env_variables': mesos_slave_executor_env_variables, - 'mesos_slave_image_providers': mesos_slave_image_providers, - 'slaves_to_remove': removal_nodes}} + 'mesos_slave_image_providers': mesos_slave_image_providers}} mock_get_params.assert_called_once_with(mock_context, mock_cluster_template, mock_cluster, **expected_kwargs) + @mock.patch('magnum.common.clients.OpenStackClients') + @mock.patch('magnum.drivers.heat.template_def.TemplateDefinition' + '.get_output') + def test_mesos_get_scale_params(self, mock_get_output, + mock_osc_class): + mock_context = mock.MagicMock() + mock_cluster = mock.MagicMock() + mock_cluster.uuid = '5d12f6fd-a196-4bf0-ae4c-1f639a523a52' + + removal_nodes = ['node1', 'node2'] + 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']} + self.assertEqual(scale_params, expected_scale_params) + def test_mesos_get_heat_param(self): mesos_def = mesos_tdef.UbuntuMesosTemplateDefinition() diff --git a/releasenotes/notes/fix-cluster-update-886bd2d1156bef88.yaml b/releasenotes/notes/fix-cluster-update-886bd2d1156bef88.yaml new file mode 100644 index 0000000000..ac0c3c2860 --- /dev/null +++ b/releasenotes/notes/fix-cluster-update-886bd2d1156bef88.yaml @@ -0,0 +1,8 @@ +--- +fixes: + - | + When doing a cluster update magnum is now passing the existing parameter + to heat which will use the heat templates stored in the heat db. This + change will prevent heat from replacacing all nodes when the heat + templates change, for example after an upgrade of the magnum server code. + https://storyboard.openstack.org/#!/story/1722573