openstack
/
python-tripleoclient
Code Issues Proposed changes

Remove nodes and roles options. 

This options was deprecated in the previous version and can be now
removed.

Change-Id: I29aaf4188e17355ce8c15bbedf4782d9f15fd065
Closes-Bug: #1824781
This commit is contained in:
Natal Ngétal 2019-04-15 11:30:48 +02:00
parent 9f3f6c8028
commit 9e930fdd16
4 changed files with 22 additions and 156 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

62
tripleoclient/tests/v1/overcloud_update/test_overcloud_update.py
View File

@ -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):

38
tripleoclient/tests/v1/overcloud_upgrade/test_overcloud_upgrade.py
View File

@ -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')

39
tripleoclient/v1/overcloud_update.py
View File

@ -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(

39
tripleoclient/v1/overcloud_upgrade.py
View File

@ -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(