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

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'},

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)

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})