From 67c49244becbd3bf9c6ccf678b1aea5f52ed647f Mon Sep 17 00:00:00 2001 From: Luke Short Date: Mon, 17 Feb 2020 03:27:51 +0000 Subject: [PATCH] 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 Co-Authored-By: Kevin Carter --- tripleoclient/tests/test_utils.py | 6 +++ .../overcloud_deploy/test_overcloud_deploy.py | 3 +- .../test_overcloud_ffwd_upgrade.py | 15 +++++++- .../test_overcloud_upgrade.py | 4 +- tripleoclient/utils.py | 26 ++++++++++++- tripleoclient/v1/overcloud_deploy.py | 30 ++++++++++++++- tripleoclient/v1/overcloud_external_update.py | 14 ++++++- .../v1/overcloud_external_upgrade.py | 13 ++++++- tripleoclient/v1/overcloud_update.py | 35 +++++++++++++----- tripleoclient/v1/overcloud_upgrade.py | 18 +++++---- tripleoclient/workflows/deployment.py | 37 ++++++------------- tripleoclient/workflows/scale.py | 7 +++- 12 files changed, 156 insertions(+), 52 deletions(-) diff --git a/tripleoclient/tests/test_utils.py b/tripleoclient/tests/test_utils.py index 142f24a81..e4968e855 100644 --- a/tripleoclient/tests/test_utils.py +++ b/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) diff --git a/tripleoclient/tests/v1/overcloud_deploy/test_overcloud_deploy.py b/tripleoclient/tests/v1/overcloud_deploy/test_overcloud_deploy.py index c6430e3d6..e5a968941 100644 --- a/tripleoclient/tests/v1/overcloud_deploy/test_overcloud_deploy.py +++ b/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() diff --git a/tripleoclient/tests/v1/overcloud_ffwd_upgrade/test_overcloud_ffwd_upgrade.py b/tripleoclient/tests/v1/overcloud_ffwd_upgrade/test_overcloud_ffwd_upgrade.py index 7de8d2859..951a8dd56 100644 --- a/tripleoclient/tests/v1/overcloud_ffwd_upgrade/test_overcloud_ffwd_upgrade.py +++ b/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): diff --git a/tripleoclient/tests/v1/overcloud_upgrade/test_overcloud_upgrade.py b/tripleoclient/tests/v1/overcloud_upgrade/test_overcloud_upgrade.py index c54b59ddb..f689c2471 100644 --- a/tripleoclient/tests/v1/overcloud_upgrade/test_overcloud_upgrade.py +++ b/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']), ] diff --git a/tripleoclient/utils.py b/tripleoclient/utils.py index b599441c9..ad0e93457 100644 --- a/tripleoclient/utils.py +++ b/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( diff --git a/tripleoclient/v1/overcloud_deploy.py b/tripleoclient/v1/overcloud_deploy.py index b6e84762d..91be7f245 100644 --- a/tripleoclient/v1/overcloud_deploy.py +++ b/tripleoclient/v1/overcloud_deploy.py @@ -955,6 +955,29 @@ class DeployOvercloud(command.Command): metavar='', 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, diff --git a/tripleoclient/v1/overcloud_external_update.py b/tripleoclient/v1/overcloud_external_update.py index dac1a4897..73cd38712 100644 --- a/tripleoclient/v1/overcloud_external_update.py +++ b/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.") diff --git a/tripleoclient/v1/overcloud_external_upgrade.py b/tripleoclient/v1/overcloud_external_upgrade.py index c05ba3fde..c22241965 100644 --- a/tripleoclient/v1/overcloud_external_upgrade.py +++ b/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.") diff --git a/tripleoclient/v1/overcloud_update.py b/tripleoclient/v1/overcloud_update.py index 3dadb5c03..eab660595 100644 --- a/tripleoclient/v1/overcloud_update.py +++ b/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=_( - "A string that identifies a single node or comma-separated" - " list of nodes to be upgraded in parallel in this upgrade" - " run invocation. For example: --limit \"compute-0," - " compute-1, compute-5\".") + '--limit', + action='store', + required=True, + 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('--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=[ - i.strip() for i in parsed_args.limit.split(',') if i - ] + limit_hosts=oooutils.playbook_limit_parse( + limit_nodes=parsed_args.limit + ), + skip_tags=parsed_args.skip_tags, + tags=parsed_args.tags ) self.log.info("Completed Overcloud Minor Update Run.") diff --git a/tripleoclient/v1/overcloud_upgrade.py b/tripleoclient/v1/overcloud_upgrade.py index ceba4b87d..0660d657d 100644 --- a/tripleoclient/v1/overcloud_upgrade.py +++ b/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=_( - "A string that identifies a single node or comma-separated" - "list of nodes to be upgraded in parallel in this upgrade" - " run invocation. For example: --limit \"compute-0," - " compute-1, compute-5\".") + '--limit', + action='store', + required=True, + 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('--playbook', nargs="*", @@ -225,9 +227,9 @@ class UpgradeRun(command.Command): ), tags=parsed_args.tags, skip_tags=parsed_args.skip_tags, - limit_list=[ - i.strip() for i in parsed_args.limit.split(',') if i - ] + limit_hosts=oooutils.playbook_limit_parse( + limit_nodes=parsed_args.limit + ) ) self.log.info("Completed Overcloud Major Upgrade Run.") diff --git a/tripleoclient/workflows/deployment.py b/tripleoclient/workflows/deployment.py index 2753df373..3bcf91f36 100644 --- a/tripleoclient/workflows/deployment.py +++ b/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. - :type limit_list: List + :param limit_hosts: String of hosts to limit the current playbook to. + :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, ) diff --git a/tripleoclient/workflows/scale.py b/tripleoclient/workflows/scale.py index b4acbf8aa..eb5d8dbe7 100644 --- a/tripleoclient/workflows/scale.py +++ b/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 )