From 73631dfbcf8d26063485da8f7e8925ec6ff8b884 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Natal=20Ng=C3=A9tal?= Date: Thu, 31 Jan 2019 14:57:41 +0100 Subject: [PATCH] [Core] Add limit option to manage nodes. Currently we have nodes and roles options for upgrade run and update run but it's a bit confusing. This both options it's same. Depecrated nodes and roles and a new option limit. The options nodes and roles will be remove in the future version. Closes-Bug: #1813810 Change-Id: I4d33e7e5bd4b892219cfc2067e81938e0f6a8668 --- tripleoclient/command.py | 22 +++++++ .../overcloud_update/test_overcloud_update.py | 4 +- .../test_overcloud_upgrade.py | 28 ++++----- tripleoclient/v1/overcloud_update.py | 60 ++++++++++++------- tripleoclient/v1/overcloud_upgrade.py | 31 +++++++--- 5 files changed, 100 insertions(+), 45 deletions(-) diff --git a/tripleoclient/command.py b/tripleoclient/command.py index fd25710f1..204ff9ea1 100644 --- a/tripleoclient/command.py +++ b/tripleoclient/command.py @@ -12,6 +12,7 @@ # License for the specific language governing permissions and limitations # under the License. +from argparse import _StoreAction import logging from osc_lib.command import command @@ -33,3 +34,24 @@ class Command(command.Command): class Lister(Command, command.Lister): pass + + +class DeprecatedActionStore(_StoreAction): + """To deprecated an option an store the value""" + log = logging.getLogger(__name__) + + def __call__(self, parser, namespace, values, option_string=None): + """Display the warning message""" + if len(self.option_strings) == 1: + message = 'The option {option} is deprecated, it will be removed'\ + ' in a future version'.format( + option=self.option_strings[0]) + else: + option = ', '.join(self.option_strings) + message = 'The options {option} is deprecated, it will be removed'\ + ' in a future version'.format( + option=option) + + self.log.warning(message) + super(DeprecatedActionStore, self).__call__( + parser, namespace, values, option_string) diff --git a/tripleoclient/tests/v1/overcloud_update/test_overcloud_update.py b/tripleoclient/tests/v1/overcloud_update/test_overcloud_update.py index 89eecdbc2..046e18674 100644 --- a/tripleoclient/tests/v1/overcloud_update/test_overcloud_update.py +++ b/tripleoclient/tests/v1/overcloud_update/test_overcloud_update.py @@ -220,8 +220,8 @@ class TestOvercloudUpdateRun(fakes.TestOvercloudUpdateRun): @mock.patch('os.path.expanduser') @mock.patch('oslo_concurrency.processutils.execute') @mock.patch('six.moves.builtins.open') - def test_update_with_no_nodes_or_roles(self, mock_open, mock_execute, - mock_expanduser, update_ansible): + def test_update_with_no_nodes_or_roles_or_limit( + self, mock_open, mock_execute, mock_expanduser, update_ansible): mock_expanduser.return_value = '/home/fake/' argslist = [] verifylist = [ diff --git a/tripleoclient/tests/v1/overcloud_upgrade/test_overcloud_upgrade.py b/tripleoclient/tests/v1/overcloud_upgrade/test_overcloud_upgrade.py index 125d1ea64..28c044e53 100644 --- a/tripleoclient/tests/v1/overcloud_upgrade/test_overcloud_upgrade.py +++ b/tripleoclient/tests/v1/overcloud_upgrade/test_overcloud_upgrade.py @@ -215,10 +215,10 @@ class TestOvercloudUpgradeRun(fakes.TestOvercloudUpgradeRun): def test_upgrade_nodes_with_playbook_no_skip_tags( self, mock_open, mock_execute, mock_expanduser, upgrade_ansible): mock_expanduser.return_value = '/home/fake/' - argslist = ['--nodes', 'compute-0, compute-1', + argslist = ['--limit', 'compute-0, compute-1', '--playbook', 'fake-playbook.yaml', ] verifylist = [ - ('nodes', 'compute-0, compute-1'), + ('limit', 'compute-0, compute-1'), ('static_inventory', None), ('playbook', 'fake-playbook.yaml'), ] @@ -246,9 +246,9 @@ class TestOvercloudUpgradeRun(fakes.TestOvercloudUpgradeRun): def test_upgrade_node_all_playbooks_skip_tags_default( self, mock_open, mock_execute, mock_expanduser, upgrade_ansible): mock_expanduser.return_value = '/home/fake/' - argslist = ['--nodes', 'swift-1', '--playbook', 'all'] + argslist = ['--limit', 'swift-1', '--playbook', 'all'] verifylist = [ - ('nodes', 'swift-1'), + ('limit', 'swift-1'), ('static_inventory', None), ('playbook', 'all'), ] @@ -277,10 +277,10 @@ class TestOvercloudUpgradeRun(fakes.TestOvercloudUpgradeRun): def test_upgrade_node_all_playbooks_skip_tags_all_supported( self, mock_open, mock_execute, mock_expanduser, upgrade_ansible): mock_expanduser.return_value = '/home/fake/' - argslist = ['--nodes', 'swift-1', '--playbook', 'all', + argslist = ['--limit', 'swift-1', '--playbook', 'all', '--skip-tags', 'pre-upgrade,validation'] verifylist = [ - ('nodes', 'swift-1'), + ('limit', 'swift-1'), ('static_inventory', None), ('playbook', 'all'), ('skip_tags', 'pre-upgrade,validation') @@ -307,8 +307,8 @@ class TestOvercloudUpgradeRun(fakes.TestOvercloudUpgradeRun): @mock.patch('os.path.expanduser') @mock.patch('oslo_concurrency.processutils.execute') @mock.patch('six.moves.builtins.open') - def test_upgrade_no_nodes_or_roles(self, mock_open, mock_execute, - mock_expanduser, upgrade_ansible): + def test_upgrade_with_no_nodes_or_roles_or_limit( + self, mock_open, mock_execute, mock_expanduser, upgrade_ansible): mock_expanduser.return_value = '/home/fake/' argslist = [] verifylist = [] @@ -342,10 +342,10 @@ class TestOvercloudUpgradeRun(fakes.TestOvercloudUpgradeRun): def test_upgrade_skip_tags_validations(self, mock_open, mock_execute, mock_expanduser, upgrade_ansible): mock_expanduser.return_value = '/home/fake/' - argslist = ['--nodes', 'overcloud-compute-1', + argslist = ['--limit', 'overcloud-compute-1', '--skip-tags', 'validations'] verifylist = [ - ('nodes', 'overcloud-compute-1'), + ('limit', 'overcloud-compute-1'), ('static_inventory', None), ('playbook', 'all'), ('skip_tags', 'validations'), @@ -365,10 +365,10 @@ class TestOvercloudUpgradeRun(fakes.TestOvercloudUpgradeRun): def test_upgrade_skip_tags_unsupported_validation_anything_else( self, mock_open, mock_execute, mock_expanduser, upgrade_ansible): mock_expanduser.return_value = '/home/fake/' - argslist = ['--nodes', 'overcloud-compute-1', + argslist = ['--limit', 'overcloud-compute-1', '--skip-tags', 'validation,anything-else'] verifylist = [ - ('nodes', 'overcloud-compute-1'), + ('limit', 'overcloud-compute-1'), ('static_inventory', None), ('playbook', 'all'), ('skip_tags', 'validation,anything-else'), @@ -388,10 +388,10 @@ class TestOvercloudUpgradeRun(fakes.TestOvercloudUpgradeRun): def test_upgrade_skip_tags_unsupported_pre_upgrade_anything_else( self, mock_open, mock_execute, mock_expanduser, upgrade_ansible): mock_expanduser.return_value = '/home/fake/' - argslist = ['--nodes', 'overcloud-compute-1', + argslist = ['--limit', 'overcloud-compute-1', '--skip-tags', 'pre-upgrade,anything-else'] verifylist = [ - ('nodes', 'overcloud-compute-1'), + ('limit', 'overcloud-compute-1'), ('static_inventory', None), ('playbook', 'all'), ('skip_tags', 'pre-upgrade,anything-else'), diff --git a/tripleoclient/v1/overcloud_update.py b/tripleoclient/v1/overcloud_update.py index 70787acdc..7aed493d9 100644 --- a/tripleoclient/v1/overcloud_update.py +++ b/tripleoclient/v1/overcloud_update.py @@ -78,26 +78,40 @@ class UpdateRun(command.Command): def get_parser(self, prog_name): parser = super(UpdateRun, self).get_parser(prog_name) - nodes_or_roles = parser.add_mutually_exclusive_group(required=True) - nodes_or_roles.add_argument( - '--nodes', action="store", help=_( - "A string that identifies a single node " - "or comma-separated list of nodes to be " - "updated in parallel in this minor update " - "run invocation. For example: --nodes " - "\"compute-0, compute-1, compute-5\". " - "Mutually exclusive with --roles." - "Serial is always 1, so update is always done one by one.") + nodes_or_roles_or_limit = parser.add_mutually_exclusive_group( + required=True) + nodes_or_roles_or_limit.add_argument( + '--nodes', action=command.DeprecatedActionStore, help=_( + "DEPRECATED: This option will be remove in the future release" + " Use the limit option instead." + "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: --nodes \"compute-0, compute-1, " + "compute-5\". " + "NOTE: Using this parameter with nodes of controlplane roles " + "(e.g. \"--nodes controller-1\") is NOT supported and WILL " + "end badly unless you include ALL nodes of that role as a " + "comma separated string. You should instead use the --roles " + "parameter for controlplane roles and specify the role name.") ) - nodes_or_roles.add_argument( - '--roles', action="store", help=_( - "A string that identifies the role or " - "comma-separated list of roles to be " - "updated in this minor update run " - "invocation. " - "Mutually exclusive with --nodes." - "Serial is always 1, so update is always done one by one.") + nodes_or_roles_or_limit.add_argument( + '--roles', action=command.DeprecatedActionStore, help=_( + "DEPRECATED: This option will be remove in the future release" + " Use the limit option instead." + "A string that identifies the role or comma-separated list of" + "roles to be upgraded in this upgrade run invocation. " + "NOTE: nodes of specified role(s) are upgraded in parallel. " + "This is REQUIRED for controlplane roles. For non " + "controlplane roles (e.g., \"Compute\"), you may consider " + "instead using the --nodes argument to limit the upgrade to " + "a specific node or list (comma separated string) of nodes.") ) + nodes_or_roles_or_limit.add_argument( + '--limit', action='store', 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\".")) parser.add_argument('--playbook', action="store", default="all", @@ -146,9 +160,13 @@ class UpdateRun(command.Command): stack = parsed_args.stack # Run ansible: - roles = parsed_args.roles - nodes = parsed_args.nodes - limit_hosts = roles or nodes + if parsed_args.limit: + limit_hosts = parsed_args.limit + else: + roles = parsed_args.roles + nodes = parsed_args.nodes + limit_hosts = roles or nodes + playbook = parsed_args.playbook inventory = oooutils.get_tripleo_ansible_inventory( parsed_args.static_inventory, parsed_args.ssh_user, stack) diff --git a/tripleoclient/v1/overcloud_upgrade.py b/tripleoclient/v1/overcloud_upgrade.py index 26b230a28..f2e544104 100644 --- a/tripleoclient/v1/overcloud_upgrade.py +++ b/tripleoclient/v1/overcloud_upgrade.py @@ -98,9 +98,12 @@ class UpgradeRun(command.Command): def get_parser(self, prog_name): parser = super(UpgradeRun, self).get_parser(prog_name) - nodes_or_roles = parser.add_mutually_exclusive_group(required=True) - nodes_or_roles.add_argument( - '--nodes', action="store", help=_( + nodes_or_roles_or_limit = parser.add_mutually_exclusive_group( + required=True) + nodes_or_roles_or_limit.add_argument( + '--nodes', action=command.DeprecatedActionStore, help=_( + "DEPRECATED: This option will be remove in the future release" + " Use the limit option instead." "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: --nodes \"compute-0, compute-1, " @@ -111,8 +114,10 @@ class UpgradeRun(command.Command): "comma separated string. You should instead use the --roles " "parameter for controlplane roles and specify the role name.") ) - nodes_or_roles.add_argument( - '--roles', action="store", help=_( + nodes_or_roles_or_limit.add_argument( + '--roles', action=command.DeprecatedActionStore, help=_( + "DEPRECATED: This option will be remove in the future release" + " Use the limit option instead." "A string that identifies the role or comma-separated list of" "roles to be upgraded in this upgrade run invocation. " "NOTE: nodes of specified role(s) are upgraded in parallel. " @@ -121,6 +126,12 @@ class UpgradeRun(command.Command): "instead using the --nodes argument to limit the upgrade to " "a specific node or list (comma separated string) of nodes.") ) + nodes_or_roles_or_limit.add_argument( + '--limit', action='store', 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\".")) parser.add_argument('--playbook', action="store", default="all", @@ -194,9 +205,13 @@ class UpgradeRun(command.Command): stack = parsed_args.stack # Run ansible: - roles = parsed_args.roles - nodes = parsed_args.nodes - limit_hosts = roles or nodes + if parsed_args.limit: + limit_hosts = parsed_args.limit + else: + roles = parsed_args.roles + nodes = parsed_args.nodes + limit_hosts = roles or nodes + playbook = parsed_args.playbook inventory = oooutils.get_tripleo_ansible_inventory( parsed_args.static_inventory, parsed_args.ssh_user, stack)