diff --git a/tuskarclient/common/utils.py b/tuskarclient/common/utils.py index 156e00d..7163c47 100644 --- a/tuskarclient/common/utils.py +++ b/tuskarclient/common/utils.py @@ -171,13 +171,22 @@ def format_attributes(params): return attributes -def format_roles(params): +def format_roles(params, role_name_ids): """Reformat CLI roles into the structure expected by the API. The format expected by the API for roles is a list of dictionaries containing a key for the Overcloud Role (overcloud_role_id) and a key for the role count (num_nodes). Both values should be integers. + If there is no entry in role_name_ids for a given role, it is assumed + the user specified the role ID instead. + + :param params: list of key-value pairs specified in the format key=value + :type params: list of str + + :param role_name_ids: mapping of role name (str) to its ID (int) + :type role_name_ids: dict + :raises: ValidationError """ # A list of the role ID's being used so we can look out for duplicates. @@ -186,10 +195,11 @@ def format_roles(params): # The list of roles structured for the API. roles = [] - for key, value in format_key_value(params): - # TODO(dmatthew): We want to add adding roles by names, this will need - # to be changed for this. - key = int(key) + for name_or_id, value in format_key_value(params): + + # If a role with the given name is found, use it's ID. If one + # cannot be found, assume the given parameter is an ID and use that. + key = role_name_ids.get(name_or_id, name_or_id) value = int(value) if key in added_role_ids: diff --git a/tuskarclient/tests/v1/test_overclouds_shell.py b/tuskarclient/tests/v1/test_overclouds_shell.py index f3f1284..db346a8 100644 --- a/tuskarclient/tests/v1/test_overclouds_shell.py +++ b/tuskarclient/tests/v1/test_overclouds_shell.py @@ -15,6 +15,7 @@ import six from tuskarclient.openstack.common.apiclient import exceptions import tuskarclient.tests.utils as tutils +from tuskarclient.v1.overcloud_roles import OvercloudRole from tuskarclient.v1 import overclouds_shell @@ -193,7 +194,9 @@ class OvercloudRoleShellTest(BaseOvercloudShellTest): @mock.patch('tuskarclient.v1.overclouds_shell.print_overcloud_detail') def test_create_roles(self, mock_print_detail): - self.args.roles = ['1=10', '2=20', '3=30'] + self.args.roles = ['foo=10'] + self.tuskar.overcloud_roles.list.return_value =\ + [OvercloudRole(None, {'name': 'foo', 'id': 1})] self.create(self.tuskar, self.args, outfile=self.outfile) self.tuskar.overclouds.create.assert_called_with( @@ -201,8 +204,6 @@ class OvercloudRoleShellTest(BaseOvercloudShellTest): attributes={}, counts=[ {'overcloud_role_id': 1, 'num_nodes': 10}, - {'overcloud_role_id': 2, 'num_nodes': 20}, - {'overcloud_role_id': 3, 'num_nodes': 30}, ] ) mock_print_detail.assert_called_with( @@ -210,7 +211,9 @@ class OvercloudRoleShellTest(BaseOvercloudShellTest): @mock.patch('tuskarclient.v1.overclouds_shell.print_overcloud_detail') def test_create_roles_duplicate(self, mock_print_detail): - self.args.roles = ['1=10', '1=20'] + self.args.roles = ['foo=10', 'foo=20'] + self.tuskar.overcloud_roles.list.return_value =\ + [OvercloudRole(None, {'name': 'foo', 'id': 1})] self.assertRaises( exceptions.ValidationError, @@ -220,8 +223,10 @@ class OvercloudRoleShellTest(BaseOvercloudShellTest): self.tuskar.overclouds.create.assert_not_called() @mock.patch('tuskarclient.v1.overclouds_shell.print_overcloud_detail') - def test_create_roles_invalid(self, mock_print_detail): - self.args.roles = ['1=10', '120'] + def test_create_roles_invalid_structure(self, mock_print_detail): + self.args.roles = ['foo=10', '120'] + self.tuskar.overcloud_roles.list.return_value =\ + [OvercloudRole(None, {'name': 'foo', 'id': 1})] self.assertRaises( exceptions.CommandError, @@ -232,7 +237,9 @@ class OvercloudRoleShellTest(BaseOvercloudShellTest): @mock.patch('tuskarclient.v1.overclouds_shell.print_overcloud_detail') def test_create_roles_multiple_equals(self, mock_print_detail): - self.args.roles = ['1=10', '2=2=0'] + self.args.roles = ['foo=foo=0'] + self.tuskar.overcloud_roles.list.return_value =\ + [OvercloudRole(None, {'name': 'foo', 'id': 1})] self.assertRaises( ValueError, @@ -240,3 +247,20 @@ class OvercloudRoleShellTest(BaseOvercloudShellTest): self.tuskar, self.args, outfile=self.outfile) self.tuskar.overclouds.create.assert_not_called() + + @mock.patch('tuskarclient.v1.overclouds_shell.print_overcloud_detail') + def test_create_roles_with_id(self, mock_print_detail): + self.args.roles = ['12345=10'] + self.tuskar.overcloud_roles.list.return_value =\ + [OvercloudRole(None, {'name': 'foo', 'id': 1})] + + self.create(self.tuskar, self.args, outfile=self.outfile) + self.tuskar.overclouds.create.assert_called_with( + name='my_overcloud', + attributes={}, + counts=[ + {'overcloud_role_id': '12345', 'num_nodes': 10}, + ] + ) + mock_print_detail.assert_called_with( + self.tuskar.overclouds.create.return_value, outfile=self.outfile) diff --git a/tuskarclient/v1/overclouds_shell.py b/tuskarclient/v1/overclouds_shell.py index 4a48761..620f1bb 100644 --- a/tuskarclient/v1/overclouds_shell.py +++ b/tuskarclient/v1/overclouds_shell.py @@ -46,12 +46,14 @@ def do_overcloud_list(tuskar, args, outfile=sys.stdout): @utils.arg('-A', '--attribute', dest='attributes', metavar='', help='This can be specified multiple times.', action='append') -@utils.arg('-R', '--role-count', dest='roles', metavar='', +@utils.arg('-R', '--role-count', dest='roles', + metavar='', help='This can be specified multiple times.', action='append') def do_overcloud_create(tuskar, args, outfile=sys.stdout): """Create a new Overcloud.""" - overcloud_dict = create_overcloud_dict(args) + overcloud_roles = tuskar.overcloud_roles.list() + overcloud_dict = create_overcloud_dict(args, overcloud_roles) overcloud = tuskar.overclouds.create(**overcloud_dict) print_overcloud_detail(overcloud, outfile=outfile) @@ -66,13 +68,15 @@ def do_overcloud_create(tuskar, args, outfile=sys.stdout): @utils.arg('-A', '--attribute', dest='attributes', metavar='', help='This can be specified multiple times.', action='append') -@utils.arg('-R', '--role-count', dest='roles', metavar='', +@utils.arg('-R', '--role-count', dest='roles', + metavar='', help='This can be specified multiple times.', action='append') def do_overcloud_update(tuskar, args, outfile=sys.stdout): """Update an existing Overcloud by its ID.""" overcloud = utils.find_resource(tuskar.overclouds, args.id) - overcloud_dict = create_overcloud_dict(args) + overcloud_roles = tuskar.overcloud_roles.list() + overcloud_dict = create_overcloud_dict(args, overcloud_roles) updated_overcloud = tuskar.overclouds.update(overcloud.id, **overcloud_dict) print_overcloud_detail(updated_overcloud, outfile=outfile) @@ -96,8 +100,13 @@ def do_overcloud_show_template_parameters(tuskar, args, outfile=sys.stdout): fmt.print_dict(template_parameters_dict, formatters, outfile=outfile) -def create_overcloud_dict(args): - """Marshal command line arguments to an API request dict.""" +def create_overcloud_dict(args, roles): + """Marshal command line arguments to an API request dict. + + :param roles: list of OvercloudRole instances retrieved from the tuskar + client + :type roles: list of OvercloudRole + """ overcloud_dict = {} simple_fields = ['name', 'description'] for field_name in simple_fields: @@ -106,7 +115,10 @@ def create_overcloud_dict(args): overcloud_dict[field_name] = field_value overcloud_dict['attributes'] = utils.format_attributes(args.attributes) - overcloud_dict['counts'] = utils.format_roles(args.roles) + + role_name_ids = dict((r.to_dict()['name'], r.to_dict()['id']) + for r in roles) + overcloud_dict['counts'] = utils.format_roles(args.roles, role_name_ids) utils.marshal_association(args, overcloud_dict, 'resource_class') return overcloud_dict