From 9e930fdd16eed3b4101f97d366b2bb26c6f8538a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Natal=20Ng=C3=A9tal?= Date: Mon, 15 Apr 2019 11:30:48 +0200 Subject: [PATCH] Remove nodes and roles options. This options was deprecated in the previous version and can be now removed. Change-Id: I29aaf4188e17355ce8c15bbedf4782d9f15fd065 Closes-Bug: #1824781 --- .../overcloud_update/test_overcloud_update.py | 62 ++----------------- .../test_overcloud_upgrade.py | 38 +++--------- tripleoclient/v1/overcloud_update.py | 39 +----------- tripleoclient/v1/overcloud_upgrade.py | 39 +----------- 4 files changed, 22 insertions(+), 156 deletions(-) diff --git a/tripleoclient/tests/v1/overcloud_update/test_overcloud_update.py b/tripleoclient/tests/v1/overcloud_update/test_overcloud_update.py index 700ee8c89..428eba919 100644 --- a/tripleoclient/tests/v1/overcloud_update/test_overcloud_update.py +++ b/tripleoclient/tests/v1/overcloud_update/test_overcloud_update.py @@ -129,11 +129,11 @@ class TestOvercloudUpdateRun(fakes.TestOvercloudUpdateRun): def test_update_with_playbook_and_user(self, mock_open, mock_execute, mock_expanduser, update_ansible): mock_expanduser.return_value = '/home/fake/' - argslist = ['--roles', 'Compute', + argslist = ['--limit', 'Compute', '--playbook', 'fake-playbook.yaml', '--ssh-user', 'tripleo-admin'] verifylist = [ - ('roles', 'Compute'), + ('limit', 'Compute'), ('static_inventory', None), ('playbook', 'fake-playbook.yaml'), ('ssh_user', 'tripleo-admin') @@ -160,12 +160,12 @@ class TestOvercloudUpdateRun(fakes.TestOvercloudUpdateRun): @mock.patch('os.path.expanduser') @mock.patch('oslo_concurrency.processutils.execute') @mock.patch('six.moves.builtins.open') - def test_update_roles_with_all_playbooks(self, mock_open, mock_execute, + def test_update_limit_with_all_playbooks(self, mock_open, mock_execute, mock_expanduser, update_ansible): mock_expanduser.return_value = '/home/fake/' - argslist = ['--roles', 'Compute', '--playbook', 'all'] + argslist = ['--limit', 'Compute', '--playbook', 'all'] verifylist = [ - ('roles', 'Compute'), + ('limit', 'Compute'), ('static_inventory', None), ('playbook', 'all') ] @@ -192,38 +192,7 @@ class TestOvercloudUpdateRun(fakes.TestOvercloudUpdateRun): @mock.patch('os.path.expanduser') @mock.patch('oslo_concurrency.processutils.execute') @mock.patch('six.moves.builtins.open') - def test_update_nodes_with_all_playbooks( - self, mock_open, mock_execute, mock_expanduser, update_ansible): - mock_expanduser.return_value = '/home/fake/' - argslist = ['--nodes', 'compute-0, compute-1'] - verifylist = [ - ('static_inventory', None), - ('playbook', 'all'), - ('nodes', 'compute-0, compute-1') - ] - parsed_args = self.check_parser(self.cmd, argslist, verifylist) - with mock.patch('os.path.exists') as mock_exists: - mock_exists.return_value = True - self.cmd.take_action(parsed_args) - for book in constants.MINOR_UPDATE_PLAYBOOKS: - update_ansible.assert_any_call( - self.app.client_manager, - nodes='compute-0, compute-1', - inventory_file=mock_open().read(), - playbook=book, - node_user='tripleo-admin', - tags='', - skip_tags='', - verbosity=1, - extra_vars=None - ) - - @mock.patch('tripleoclient.workflows.package_update.update_ansible', - autospec=True) - @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_or_limit( + def test_update_with_no_limit( self, mock_open, mock_execute, mock_expanduser, update_ansible): mock_expanduser.return_value = '/home/fake/' argslist = [] @@ -234,25 +203,6 @@ class TestOvercloudUpdateRun(fakes.TestOvercloudUpdateRun): self.assertRaises(ParserException, lambda: self.check_parser( self.cmd, argslist, verifylist)) - @mock.patch('tripleoclient.workflows.package_update.update_ansible', - autospec=True) - @mock.patch('os.path.expanduser') - @mock.patch('oslo_concurrency.processutils.execute') - @mock.patch('six.moves.builtins.open') - def test_update_with_nodes_and_roles(self, mock_open, mock_execute, - mock_expanduser, update_ansible): - mock_expanduser.return_value = '/home/fake/' - argslist = ['--roles', 'Controller', - '--nodes', 'overcloud-controller-1'] - verifylist = [ - ('roles', 'Controller'), - ('nodes', 'overcloud-controller-1'), - ('static_inventory', None), - ('playbook', 'all') - ] - self.assertRaises(ParserException, lambda: self.check_parser( - self.cmd, argslist, verifylist)) - class TestOvercloudUpdateConverge(fakes.TestOvercloudUpdateConverge): diff --git a/tripleoclient/tests/v1/overcloud_upgrade/test_overcloud_upgrade.py b/tripleoclient/tests/v1/overcloud_upgrade/test_overcloud_upgrade.py index 4754b9d14..c530c13bb 100644 --- a/tripleoclient/tests/v1/overcloud_upgrade/test_overcloud_upgrade.py +++ b/tripleoclient/tests/v1/overcloud_upgrade/test_overcloud_upgrade.py @@ -158,14 +158,14 @@ class TestOvercloudUpgradeRun(fakes.TestOvercloudUpgradeRun): @mock.patch('os.path.expanduser') @mock.patch('oslo_concurrency.processutils.execute') @mock.patch('six.moves.builtins.open') - def test_upgrade_roles_with_playbook_and_user( + def test_upgrade_limit_with_playbook_and_user( self, mock_open, mock_execute, mock_expanduser, upgrade_ansible): mock_expanduser.return_value = '/home/fake/' - argslist = ['--roles', 'Compute, Controller', + argslist = ['--limit', 'Compute, Controller', '--playbook', 'fake-playbook.yaml', '--ssh-user', 'tripleo-admin'] verifylist = [ - ('roles', 'Compute, Controller'), + ('limit', 'Compute, Controller'), ('static_inventory', None), ('playbook', 'fake-playbook.yaml') ] @@ -191,13 +191,13 @@ class TestOvercloudUpgradeRun(fakes.TestOvercloudUpgradeRun): @mock.patch('os.path.expanduser') @mock.patch('oslo_concurrency.processutils.execute') @mock.patch('six.moves.builtins.open') - def test_upgrade_role_all_playbooks_skip_validation( + def test_upgrade_limit_all_playbooks_skip_validation( self, mock_open, mock_execute, mock_expanduser, upgrade_ansible): mock_expanduser.return_value = '/home/fake/' - argslist = ['--roles', 'Compute', '--playbook', 'all', + argslist = ['--limit', 'Compute', '--playbook', 'all', '--skip-tags', 'validation'] verifylist = [ - ('roles', 'Compute'), + ('limit', 'Compute'), ('static_inventory', None), ('playbook', 'all'), ('skip_tags', 'validation') @@ -225,13 +225,13 @@ class TestOvercloudUpgradeRun(fakes.TestOvercloudUpgradeRun): @mock.patch('os.path.expanduser') @mock.patch('oslo_concurrency.processutils.execute') @mock.patch('six.moves.builtins.open') - def test_upgrade_role_all_playbooks_only_validation( + def test_upgrade_limit_all_playbooks_only_validation( self, mock_open, mock_execute, mock_expanduser, upgrade_ansible): mock_expanduser.return_value = '/home/fake/' - argslist = ['--roles', 'Compute', '--playbook', 'all', + argslist = ['--limit', 'Compute', '--playbook', 'all', '--tags', 'validation'] verifylist = [ - ('roles', 'Compute'), + ('limit', 'Compute'), ('static_inventory', None), ('playbook', 'all'), ('tags', 'validation') @@ -357,7 +357,7 @@ class TestOvercloudUpgradeRun(fakes.TestOvercloudUpgradeRun): @mock.patch('os.path.expanduser') @mock.patch('oslo_concurrency.processutils.execute') @mock.patch('six.moves.builtins.open') - def test_upgrade_with_no_nodes_or_roles_or_limit( + def test_upgrade_with_no_limit( self, mock_open, mock_execute, mock_expanduser, upgrade_ansible): mock_expanduser.return_value = '/home/fake/' argslist = [] @@ -365,24 +365,6 @@ class TestOvercloudUpgradeRun(fakes.TestOvercloudUpgradeRun): self.assertRaises(ParserException, lambda: self.check_parser( self.cmd, argslist, verifylist)) - @mock.patch('tripleoclient.workflows.package_update.update_ansible', - autospec=True) - @mock.patch('os.path.expanduser') - @mock.patch('oslo_concurrency.processutils.execute') - @mock.patch('six.moves.builtins.open') - def test_upgrade_nodes_and_roles(self, mock_open, mock_execute, - mock_expanduser, upgrade_ansible): - mock_expanduser.return_value = '/home/fake/' - argslist = ['--roles', 'Compute', '--nodes', 'overcloud-controller-1'] - verifylist = [ - ('roles', 'Compute'), - ('nodes', 'overcloud-controller-1'), - ('static_inventory', None), - ('playbook', 'all') - ] - self.assertRaises(ParserException, lambda: self.check_parser( - self.cmd, argslist, verifylist)) - @mock.patch('tripleoclient.workflows.package_update.update_ansible', autospec=True) @mock.patch('os.path.expanduser') diff --git a/tripleoclient/v1/overcloud_update.py b/tripleoclient/v1/overcloud_update.py index e770218c2..dd04e8423 100644 --- a/tripleoclient/v1/overcloud_update.py +++ b/tripleoclient/v1/overcloud_update.py @@ -78,36 +78,8 @@ class UpdateRun(command.Command): def get_parser(self, prog_name): parser = super(UpdateRun, self).get_parser(prog_name) - 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_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=_( + 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," @@ -161,12 +133,7 @@ class UpdateRun(command.Command): stack = parsed_args.stack # Run ansible: - if parsed_args.limit: - limit_hosts = parsed_args.limit - else: - roles = parsed_args.roles - nodes = parsed_args.nodes - limit_hosts = roles or nodes + limit_hosts = parsed_args.limit playbook = parsed_args.playbook inventory = oooutils.get_tripleo_ansible_inventory( diff --git a/tripleoclient/v1/overcloud_upgrade.py b/tripleoclient/v1/overcloud_upgrade.py index f88776ae7..e4f668394 100644 --- a/tripleoclient/v1/overcloud_upgrade.py +++ b/tripleoclient/v1/overcloud_upgrade.py @@ -98,36 +98,8 @@ class UpgradeRun(command.Command): def get_parser(self, prog_name): parser = super(UpgradeRun, self).get_parser(prog_name) - 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_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=_( + 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," @@ -214,12 +186,7 @@ class UpgradeRun(command.Command): stack = parsed_args.stack # Run ansible: - if parsed_args.limit: - limit_hosts = parsed_args.limit - else: - roles = parsed_args.roles - nodes = parsed_args.nodes - limit_hosts = roles or nodes + limit_hosts = parsed_args.limit playbook = parsed_args.playbook inventory = oooutils.get_tripleo_ansible_inventory(