Expose --limit, --skip-tags, and --tags on the CLI.

This change will provide the operator the ability to better control
a given deployment or operational task while leveraging the
tripleoclient.

A utility has been added to sanitize user input. This will ensure
the parsed string is in valid ansible limit format.

Change-Id: I190f6efe8d728f124c18ce80be715ae7c5c0da01
Depends-On: I0056fdbe3d9807e6baf4a1645a632ab9eb1b2668
Signed-off-by: Luke Short <ekultails@gmail.com>
Co-Authored-By: Kevin Carter <kecarter@redhat.com>

tripleoclient/tests/test_utils.py

@@ -1740,3 +1740,9 @@ class TestGeneralUtils(base.TestCommand):
         )
         self.tc.object_store.put_object.assert_called()
         self.tc.object_store.put_container.assert_called()
+
+    def test_playbook_limit_parse(self):
+        limit_nodes = 'controller0, compute0:compute1,!compute2'
+        limit_hosts_expected = 'controller0:compute0:compute1:!compute2'
+        limit_hosts_actual = utils.playbook_limit_parse(limit_nodes)
+        self.assertEqual(limit_hosts_actual, limit_hosts_expected)

tripleoclient/tests/v1/overcloud_deploy/test_overcloud_deploy.py

@@ -1582,7 +1582,8 @@ class TestDeployOvercloud(fakes.TestDeployOvercloud):
             [mock.call(mock.ANY, mock.ANY, mock.ANY, 'ctlplane', None, None,
                        deployment_options={},
                        deployment_timeout=448,  # 451 - 3, total time left
-                       in_flight_validations=False, timeout=42, verbosity=3)],
+                       in_flight_validations=False, limit_hosts=None,
+                       skip_tags=None, tags=None, timeout=42, verbosity=3)],
             fixture.mock_config_download.mock_calls)
         fixture.mock_config_download.assert_called()
         mock_copy.assert_called_once()

tripleoclient/tests/v1/overcloud_ffwd_upgrade/test_overcloud_ffwd_upgrade.py

@@ -184,11 +184,24 @@ class TestFFWDUpgradeRun(fakes.TestFFWDUpgradeRun):
     def test_upgrade_no_nodes_or_roles(self, mock_open, mock_execute,
                                        mock_expanduser, upgrade_ansible):
         mock_expanduser.return_value = '/home/fake/'
-        argslist = ["--nodes", "controller-1", "--roles", "foo", "--yes"]
+        argslist = ["--limit", "controller-1", "--roles", "foo", "--yes"]
         verifylist = []
         self.assertRaises(ParserException, lambda: self.check_parser(
             self.cmd, argslist, verifylist))
 
+    @mock.patch('tripleoclient.utils.run_ansible_playbook',
+                autospec=True)
+    @mock.patch('os.path.expanduser')
+    @mock.patch('oslo_concurrency.processutils.execute')
+    @mock.patch('six.moves.builtins.open')
+    def test_upgrade_limit(self, mock_open, mock_execute,
+                           mock_expanduser, upgrade_ansible):
+        mock_expanduser.return_value = '/home/fake/'
+        argslist = ['--limit', 'controller1,controller2,controll3']
+        verifylist = [('limit', 'controller1,controller2,controll3')]
+        self.assertRaises(ParserException, lambda: self.check_parser(
+            self.cmd, argslist, verifylist))
+
 
 class TestFFWDUpgradeConverge(fakes.TestFFWDUpgradeConverge):
 

tripleoclient/tests/v1/overcloud_upgrade/test_overcloud_upgrade.py

@@ -216,10 +216,10 @@ class TestOvercloudUpgradeRun(fakes.TestOvercloudUpgradeRun):
             self, mock_open, mock_execute, mock_expanduser, upgrade_ansible,
             mock_run, mock_run_prepare):
         mock_expanduser.return_value = '/home/fake/'
-        argslist = ['--limit', 'compute-0, compute-1',
+        argslist = ['--limit', 'compute-0,compute-1',
                     '--playbook', 'fake-playbook.yaml', ]
         verifylist = [
-            ('limit', 'compute-0, compute-1'),
+            ('limit', 'compute-0,compute-1'),
             ('static_inventory', None),
             ('playbook', ['fake-playbook.yaml']),
         ]

tripleoclient/utils.py

@@ -35,6 +35,7 @@ import netaddr
 import os
 import os.path
 import pwd
+import re
 import shutil
 import simplejson
 import six
@@ -204,6 +205,23 @@ def makedirs(dir_path):
         return True
 
 
+def playbook_limit_parse(limit_nodes):
+    """Return a parsed string for limits.
+
+    This will sanitize user inputs so that we guarantee what is provided is
+    expected to be functional. If limit_nodes is None, this function will
+    return None.
+
+
+    :returns: String
+    """
+
+    if not limit_nodes:
+        return limit_nodes
+
+    return ':'.join([i.strip() for i in re.split(',| |:', limit_nodes) if i])
+
+
 def playbook_verbosity(self):
     """Return an integer for playbook verbosity levels.
 
@@ -403,7 +421,6 @@ def run_ansible_playbook(playbook, inventory, workdir, playbook_dir=None,
             ' multi-playbook execution: {}'
             ' Working directory: {},'
             ' Playbook directory: {}'.format(
-                playbook,
                 verified_playbooks,
                 workdir,
                 playbook_dir
@@ -415,12 +432,17 @@ def run_ansible_playbook(playbook, inventory, workdir, playbook_dir=None,
             _running_ansible_msg(playbook, timeout) +
             ' Working directory: {},'
             ' Playbook directory: {}'.format(
-                playbook,
                 workdir,
                 playbook_dir
             )
         )
 
+    if limit_hosts:
+        LOG.info(
+            'Running ansible with the following limit: {}'.format(
+                limit_hosts
+            )
+        )
     cwd = os.getcwd()
     ansible_fact_path = os.path.join(
         os.path.join(

tripleoclient/v1/overcloud_deploy.py

@@ -955,6 +955,29 @@ class DeployOvercloud(command.Command):
                             metavar='<baremetal_deployment.yaml>',
                             help=_('Configuration file describing the '
                                    'baremetal deployment'))
+        parser.add_argument(
+            '--limit',
+            action='store',
+            default=None,
+            help=_("A string that identifies a single node or comma-separated"
+                   "list of nodes the config-download Ansible playbook "
+                   "execution will be limited to. For example: --limit"
+                   " \"compute-0,compute-1,compute-5\".")
+        )
+        parser.add_argument(
+            '--tags',
+            action='store',
+            default=None,
+            help=_('A list of tags to use when running the the config-download'
+                   ' ansible-playbook command.')
+        )
+        parser.add_argument(
+            '--skip-tags',
+            action='store',
+            default=None,
+            help=_('A list of tags to skip when running the the'
+                   ' config-download ansible-playbook command.')
+        )
         return parser
 
     def take_action(self, parsed_args):
@@ -1049,7 +1072,12 @@ class DeployOvercloud(command.Command):
                     verbosity=utils.playbook_verbosity(self=self),
                     deployment_options=deployment_options,
                     in_flight_validations=parsed_args.inflight,
-                    deployment_timeout=timeout
+                    deployment_timeout=timeout,
+                    tags=parsed_args.tags,
+                    skip_tags=parsed_args.skip_tags,
+                    limit_hosts=utils.playbook_limit_parse(
+                        limit_nodes=parsed_args.limit
+                    )
                 )
                 deployment.set_deployment_status(
                     clients=self.clients,

tripleoclient/v1/overcloud_external_update.py

@@ -91,6 +91,15 @@ class ExternalUpdateRun(command.Command):
                             default=True,
                             help=_('This option no longer has any effect.')
                             )
+        parser.add_argument(
+            '--limit',
+            action='store',
+            default=None,
+            help=_("A string that identifies a single node or comma-separated"
+                   "list of nodes the config-download Ansible playbook "
+                   "execution will be limited to. For example: --limit"
+                   " \"compute-0,compute-1,compute-5\".")
+        )
 
         return parser
 
@@ -121,6 +130,9 @@ class ExternalUpdateRun(command.Command):
                 return_inventory_file_path=True
             ),
             tags=parsed_args.tags,
-            skip_tags=parsed_args.skip_tags
+            skip_tags=parsed_args.skip_tags,
+            limit_hosts=oooutils.playbook_limit_parse(
+                limit_nodes=parsed_args.limit
+            )
         )
         self.log.info("Completed Overcloud External Update Run.")

tripleoclient/v1/overcloud_external_upgrade.py

@@ -90,6 +90,15 @@ class ExternalUpgradeRun(command.Command):
                             default=True,
                             help=_('This option no longer has any effect.')
                             )
+        parser.add_argument(
+            '--limit',
+            action='store',
+            default=None,
+            help=_("A string that identifies a single node or comma-separated"
+                   "list of nodes the config-download Ansible playbook "
+                   "execution will be limited to. For example: --limit"
+                   " \"compute-0,compute-1,compute-5\".")
+        )
 
         return parser
 
@@ -117,6 +126,8 @@ class ExternalUpgradeRun(command.Command):
                 return_inventory_file_path=True
             ),
             tags=parsed_args.tags,
-            skip_tags=parsed_args.skip_tags
+            skip_tags=parsed_args.skip_tags,
+            limit_hosts=oooutils.playbook_limit_parse(
+                limit_nodes=parsed_args.limit)
         )
         self.log.info("Completed Overcloud External Upgrade Run.")

tripleoclient/v1/overcloud_update.py

@@ -89,11 +89,13 @@ class UpdateRun(command.Command):
     def get_parser(self, prog_name):
         parser = super(UpdateRun, self).get_parser(prog_name)
         parser.add_argument(
-            '--limit', action='store', required=True, help=_(
+            '--limit',
-                "A string that identifies a single node or comma-separated"
+            action='store',
-                " list of nodes to be upgraded in parallel in this upgrade"
+            required=True,
-                " run invocation. For example: --limit \"compute-0,"
+            help=_("A string that identifies a single node or comma-separated"
-                " compute-1, compute-5\".")
+                   "list of nodes the config-download Ansible playbook "
+                   "execution will be limited to. For example: --limit"
+                   " \"compute-0,compute-1,compute-5\".")
         )
         parser.add_argument('--playbook',
                             nargs="*",
@@ -134,7 +136,20 @@ class UpdateRun(command.Command):
                             default=True,
                             help=_('This option no longer has any effect.')
                             )
-
+        parser.add_argument(
+            '--tags',
+            action='store',
+            default=None,
+            help=_('A list of tags to use when running the the config-download'
+                   ' ansible-playbook command.')
+        )
+        parser.add_argument(
+            '--skip-tags',
+            action='store',
+            default=None,
+            help=_('A list of tags to skip when running the the'
+                   ' config-download ansible-playbook command.')
+        )
         return parser
 
     def take_action(self, parsed_args):
@@ -168,9 +183,11 @@ class UpdateRun(command.Command):
                 parsed_args.stack,
                 return_inventory_file_path=True
             ),
-            limit_list=[
+            limit_hosts=oooutils.playbook_limit_parse(
-                i.strip() for i in parsed_args.limit.split(',') if i
+                limit_nodes=parsed_args.limit
-            ]
+            ),
+            skip_tags=parsed_args.skip_tags,
+            tags=parsed_args.tags
         )
         self.log.info("Completed Overcloud Minor Update Run.")
 

tripleoclient/v1/overcloud_upgrade.py

@@ -113,11 +113,13 @@ class UpgradeRun(command.Command):
     def get_parser(self, prog_name):
         parser = super(UpgradeRun, self).get_parser(prog_name)
         parser.add_argument(
-            '--limit', action='store', required=True, help=_(
+            '--limit',
-                "A string that identifies a single node or comma-separated"
+            action='store',
-                "list of nodes to be upgraded in parallel in this upgrade"
+            required=True,
-                " run invocation. For example: --limit \"compute-0,"
+            help=_("A string that identifies a single node or comma-separated"
-                " compute-1, compute-5\".")
+                   "list of nodes the config-download Ansible playbook "
+                   "execution will be limited to. For example: --limit"
+                   " \"compute-0,compute-1,compute-5\".")
         )
         parser.add_argument('--playbook',
                             nargs="*",
@@ -225,9 +227,9 @@ class UpgradeRun(command.Command):
             ),
             tags=parsed_args.tags,
             skip_tags=parsed_args.skip_tags,
-            limit_list=[
+            limit_hosts=oooutils.playbook_limit_parse(
-                i.strip() for i in parsed_args.limit.split(',') if i
+                limit_nodes=parsed_args.limit
-            ]
+            )
         )
         self.log.info("Completed Overcloud Major Upgrade Run.")
 

tripleoclient/workflows/deployment.py

@@ -19,7 +19,6 @@ import yaml
 from heatclient.common import event_utils
 from heatclient import exc as heat_exc
 from openstackclient import shell
-import six
 from swiftclient import exceptions as swiftexceptions
 from tripleo_common.actions import ansible
 from tripleo_common.actions import config
@@ -265,7 +264,7 @@ def config_download(log, clients, stack, ssh_network=None,
                     timeout=600, verbosity=0, deployment_options=None,
                     in_flight_validations=False,
                     ansible_playbook_name='deploy_steps_playbook.yaml',
-                    limit_list=None, extra_vars=None, inventory_path=None,
+                    limit_hosts=None, extra_vars=None, inventory_path=None,
                     ssh_user='tripleo-admin', tags=None, skip_tags=None,
                     deployment_timeout=None):
     """Run config download.
@@ -303,8 +302,8 @@ def config_download(log, clients, stack, ssh_network=None,
     :param ansible_playbook_name: Name of the playbook to execute.
     :type ansible_playbook_name: String
 
-    :param limit_list: List of hosts to limit the current playbook to.
+    :param limit_hosts: String of hosts to limit the current playbook to.
-    :type limit_list: List
+    :type limit_hosts: String
 
     :param extra_vars: Set additional variables as a Dict or the absolute
                        path of a JSON or YAML file type.
@@ -362,15 +361,6 @@ def config_download(log, clients, stack, ssh_network=None,
         else:
             skip_tags = 'opendev-validation'
 
-    # NOTE(cloudnull): List of hosts to limit the current playbook execution
-    #                  The list is later converted into an ansible compatible
-    #                  string. Storing hosts in list format will ensure all
-    #                  entries are consistent.
-    if not limit_list:
-        limit_list = list()
-    elif isinstance(limit_list, six.string_types):
-        limit_list = [i.strip() for i in limit_list.split(',')]
-
     with utils.TempDirs() as tmp:
         utils.run_ansible_playbook(
             playbook='cli-grant-local-access.yaml',
@@ -397,9 +387,14 @@ def config_download(log, clients, stack, ssh_network=None,
     blacklist_stack_output = blacklist_show.get('output', dict())
     blacklist_stack_output_value = blacklist_stack_output.get('output_value')
     if blacklist_stack_output_value:
-        limit_list.extend(
+
-            ['!{}'.format(i) for i in blacklist_stack_output_value if i]
+        if not limit_hosts:
-        )
+            limit_hosts = ""
+
+        limit_hosts += (
+            ':'.join(['!{}'.format(i) for i in blacklist_stack_output_value
+                      if i]))
+
     _log_and_print(
         message='Retrieving configuration for stack: {}'.format(
             stack.stack_name
@@ -460,14 +455,6 @@ def config_download(log, clients, stack, ssh_network=None,
         print_msg=(verbosity == 0)
     )
 
-    # NOTE(cloudnull): Join the limit_list into an ansible compatible string.
-    #                  If it is an empty, the object will be reset to None.
-    limit_hosts = ':'.join(limit_list)
-    if not limit_hosts:
-        limit_hosts = None
-    else:
-        limit_hosts = '{}'.format(limit_hosts)
-
     if isinstance(ansible_playbook_name, list):
         playbooks = [os.path.join(stack_work_dir, p)
                      for p in ansible_playbook_name]
@@ -481,6 +468,7 @@ def config_download(log, clients, stack, ssh_network=None,
             workdir=tmp,
             playbook_dir=work_dir,
             skip_tags=skip_tags,
+            tags=tags,
             ansible_cfg=override_ansible_cfg,
             verbosity=verbosity,
             ssh_user=ssh_user,
@@ -492,7 +480,6 @@ def config_download(log, clients, stack, ssh_network=None,
                 'ANSIBLE_BECOME': True,
             },
             extra_vars=extra_vars,
-            tags=tags,
             timeout=deployment_timeout,
         )
 

tripleoclient/workflows/scale.py

@@ -60,13 +60,18 @@ def scale_down(log, clients, stack, nodes, timeout=None, verbosity=0,
         except Exception:
             limit_list.append(node)
 
+    if limit_list:
+        limit_list = ':'.join(limit_list)
+    else:
+        limit_list = None
+
     deployment.config_download(
         log=log,
         clients=clients,
         stack=stack,
         timeout=connection_timeout,
         ansible_playbook_name='scale_playbook.yaml',
-        limit_list=limit_list,
+        limit_hosts=limit_list,
         verbosity=verbosity,
         deployment_timeout=timeout
     )