From fa4b8fb00ce199fa767e0b0b0349e6fc3eaf4b95 Mon Sep 17 00:00:00 2001 From: Steven Hardy Date: Fri, 8 Sep 2017 15:46:13 +0100 Subject: [PATCH] Handle when lists not only string in config download There was a comment about this in the code, we didn't handle when lists properly, only strings. Also fixed the test to avoid leaking temporary files every tox run. Change-Id: I24f80199ba430870af2b4aab9453387304403215 Closes-Bug: #1715871 --- .../tests/v1/overcloud_config/fakes.py | 6 +- .../overcloud_config/test_overcloud_config.py | 64 +++++++++---------- tripleoclient/v1/overcloud_config.py | 20 ++++-- 3 files changed, 49 insertions(+), 41 deletions(-) diff --git a/tripleoclient/tests/v1/overcloud_config/fakes.py b/tripleoclient/tests/v1/overcloud_config/fakes.py index 7b44ba9ff..c6b3a751d 100644 --- a/tripleoclient/tests/v1/overcloud_config/fakes.py +++ b/tripleoclient/tests/v1/overcloud_config/fakes.py @@ -50,11 +50,13 @@ FAKE_STACK = { 'upgrade_batch_tasks': [], 'upgrade_tasks': [{'name': 'Stop fake service', 'service': 'name=fake state=stopped', - 'tags': 'step1'}, + 'tags': 'step1', + 'when': 'existingcondition'}, {'name': 'Stop nova-compute service', 'service': 'name=openstack-nova-compute ' 'state=stopped', - 'tags': 'step1'}] + 'tags': 'step1', + 'when': ['existing', 'list']}] }, 'FakeController': { 'config_settings': {'tripleo::haproxy::user': 'admin'}, diff --git a/tripleoclient/tests/v1/overcloud_config/test_overcloud_config.py b/tripleoclient/tests/v1/overcloud_config/test_overcloud_config.py index 187325aa5..c39c47040 100644 --- a/tripleoclient/tests/v1/overcloud_config/test_overcloud_config.py +++ b/tripleoclient/tests/v1/overcloud_config/test_overcloud_config.py @@ -10,7 +10,9 @@ # License for the specific language governing permissions and limitations # under the License. +import fixtures import mock +import os from mock import call from mock import patch @@ -127,45 +129,41 @@ class TestOvercloudConfig(utils.TestCommand): self.cmd.take_action, parsed_args) @mock.patch('tripleoclient.utils.get_role_data', autospec=True) - @mock.patch('os.mkdir') - @mock.patch('six.moves.builtins.open') - @mock.patch('tempfile.mkdtemp', autospec=True) - def test_overcloud_config_upgrade_tasks(self, mock_tmpdir, - mock_open, - mock_mkdir, - mock_get_role_data): + def test_overcloud_config_upgrade_tasks(self, mock_get_role_data): clients = self.app.client_manager orchestration_client = clients.orchestration orchestration_client.stacks.get.return_value = fakes.create_tht_stack() - mock_tmpdir.return_value = "/tmp/tht" + self.tmp_dir = self.useFixture(fixtures.TempDir()).path fake_role = [role for role in fakes.FAKE_STACK['outputs'][1]['output_value']] - fake_tasks = {'FakeController': [{'name': 'Stop fake service', - 'service': 'name=fake ' - 'state=stopped', - 'tags': 'step1', - 'when': 'step|int == 1'}], - 'FakeCompute': [{'name': 'Stop fake service', - 'service': 'name=fake state=stopped', - 'tags': 'step1', - 'when': 'step|int == 1'}, - {'name': 'Stop nova-' - 'compute service', - 'service': - 'name=openstack-nova-' - 'compute state=stopped', - 'tags': 'step1', - 'when': 'step|int == 1'}] - } + expected_tasks = {'FakeController': [{'name': 'Stop fake service', + 'service': 'name=fake ' + 'state=stopped', + 'tags': 'step1', + 'when': 'step|int == 1'}], + 'FakeCompute': [{'name': 'Stop fake service', + 'service': + 'name=fake state=stopped', + 'tags': 'step1', + 'when': ['existingcondition', + 'step|int == 1']}, + {'name': 'Stop nova-' + 'compute service', + 'service': + 'name=openstack-nova-' + 'compute state=stopped', + 'tags': 'step1', + 'when': ['existing', + 'list', 'step|int == 1']}]} mock_get_role_data.return_value = fake_role for role in fake_role: - filepath = "/tmp/tht/%s/upgrade_tasks_playbook" % role - with mock.patch('os.fdopen'), \ - mock.patch('os.mkdir'), mock.patch('os.open') as open: - playbook_tasks = self.cmd._write_playbook_get_tasks( - fakes.FAKE_STACK['outputs'][1]['output_value'][role] - ['upgrade_tasks'], role, filepath) - self.assertEqual(fake_tasks[role], playbook_tasks) - open.assert_called() + filedir = os.path.join(self.tmp_dir, role) + os.makedirs(filedir) + filepath = os.path.join(filedir, "upgrade_tasks_playbook.yaml") + playbook_tasks = self.cmd._write_playbook_get_tasks( + fakes.FAKE_STACK['outputs'][1]['output_value'][role] + ['upgrade_tasks'], role, filepath) + self.assertTrue(os.path.isfile(filepath)) + self.assertEqual(expected_tasks[role], playbook_tasks) diff --git a/tripleoclient/v1/overcloud_config.py b/tripleoclient/v1/overcloud_config.py index f463675ef..c861fbf9d 100644 --- a/tripleoclient/v1/overcloud_config.py +++ b/tripleoclient/v1/overcloud_config.py @@ -66,13 +66,21 @@ class DownloadConfig(command.Command): match = re.search('step([0-9]+)', tag) if match: step = match.group(1) - whenline = task.get('when', None) - if whenline: # how about list of when conditionals - when_exists = re.search('step.* == [0-9]', whenline) - if when_exists: # skip if there is an existing 'step == N' + whenexpr = task.get('when', None) + if whenexpr: + # Handle when: foo and a list of when conditionals + if not isinstance(whenexpr, list): + whenexpr = [whenexpr] + for w in whenexpr: + when_exists = re.search('step|int == [0-9]', w) + if when_exists: + break + if when_exists: + # Skip to the next task, + # there is an existing 'step|int == N' continue - task['when'] = "(%s) and (step|int == %s)" % (whenline, - step) + whenexpr.append("step|int == %s" % step) + task['when'] = whenexpr else: task.update({"when": "step|int == %s" % step})