diff options
21 files changed, 697 insertions, 106 deletions
@@ -91,6 +91,7 @@ neutron-segments: true q-metering: true q-qos: true + neutron-tag-ports-during-bulk-creation: true tox_envlist: functional - job: @@ -5,7 +5,6 @@ gcc [compile test] libc6-dev [compile test platform:dpkg] libffi-devel [platform:rpm] libffi-dev [compile test platform:dpkg] -libffi6 [platform:dpkg] libssl-dev [compile test platform:dpkg] python3-dev [compile test platform:dpkg] python3-devel [compile test platform:rpm] diff --git a/lower-constraints.txt b/lower-constraints.txt index 0a453dae..403ba4e0 100644 --- a/lower-constraints.txt +++ b/lower-constraints.txt @@ -4,7 +4,7 @@ appdirs==1.3.0 asn1crypto==0.23.0 bandit==1.1.0 cachetools==2.0.0 -cffi==1.7.0 +cffi==1.14.0 cliff==2.8.0 cmd2==0.8.0 contextlib2==0.4.0 @@ -28,7 +28,7 @@ futurist==2.1.0 gitdb==0.6.4 GitPython==1.0.1 gnocchiclient==3.3.1 -greenlet==0.4.10 +greenlet==0.4.15 hacking==2.0.0 httplib2==0.9.1 idna==2.6 diff --git a/openstackclient/compute/v2/flavor.py b/openstackclient/compute/v2/flavor.py index 42649db5..805e919e 100644 --- a/openstackclient/compute/v2/flavor.py +++ b/openstackclient/compute/v2/flavor.py @@ -402,7 +402,7 @@ class SetFlavor(command.Command): if compute_client.api_version < api_versions.APIVersion("2.55"): msg = _("--os-compute-api-version 2.55 or later is required") raise exceptions.CommandError(msg) - compute_client.flavors.update(flavor=parsed_args.flavor, + compute_client.flavors.update(flavor=flavor.id, description=parsed_args.description) diff --git a/openstackclient/compute/v2/server.py b/openstackclient/compute/v2/server.py index ff40565a..69756ae2 100644 --- a/openstackclient/compute/v2/server.py +++ b/openstackclient/compute/v2/server.py @@ -24,6 +24,7 @@ import os from novaclient import api_versions from novaclient.v2 import servers from openstack import exceptions as sdk_exceptions +from osc_lib.cli import format_columns from osc_lib.cli import parseractions from osc_lib.command import command from osc_lib import exceptions @@ -174,14 +175,14 @@ def _prep_server_detail(compute_client, image_client, server, refresh=True): if 'os-extended-volumes:volumes_attached' in info: info.update( { - 'volumes_attached': utils.format_list_of_dicts( + 'volumes_attached': format_columns.ListDictColumn( info.pop('os-extended-volumes:volumes_attached')) } ) if 'security_groups' in info: info.update( { - 'security_groups': utils.format_list_of_dicts( + 'security_groups': format_columns.ListDictColumn( info.pop('security_groups')) } ) @@ -190,9 +191,14 @@ def _prep_server_detail(compute_client, image_client, server, refresh=True): info['addresses'] = _format_servers_list_networks(server.networks) # Map 'metadata' field to 'properties' - info.update( - {'properties': utils.format_dict(info.pop('metadata'))} - ) + if not info['metadata']: + info.update( + {'properties': utils.format_dict(info.pop('metadata'))} + ) + else: + info.update( + {'properties': format_columns.DictColumn(info.pop('metadata'))} + ) # Migrate tenant_id to project_id naming if 'tenant_id' in info: @@ -2542,7 +2548,6 @@ class ShowServer(command.ShowOne): data = _prep_server_detail(compute_client, self.app.client_manager.image, server, refresh=False) - return zip(*sorted(data.items())) diff --git a/openstackclient/identity/common.py b/openstackclient/identity/common.py index e70d87d2..a75db4f8 100644 --- a/openstackclient/identity/common.py +++ b/openstackclient/identity/common.py @@ -207,7 +207,7 @@ def _find_identity_resource(identity_client_manager, name_or_id, name_or_id, **kwargs) if identity_resource is not None: return identity_resource - except exceptions.Forbidden: + except (exceptions.Forbidden, identity_exc.Forbidden): pass return resource_type(None, {'id': name_or_id, 'name': name_or_id}) diff --git a/openstackclient/identity/v3/role.py b/openstackclient/identity/v3/role.py index 980ebf11..a674564f 100644 --- a/openstackclient/identity/v3/role.py +++ b/openstackclient/identity/v3/role.py @@ -64,59 +64,61 @@ def _add_identity_and_resource_options_to_parser(parser): def _process_identity_and_resource_options(parsed_args, - identity_client_manager): + identity_client_manager, + validate_actor_existence=True): + + def _find_user(): + try: + return common.find_user( + identity_client_manager, + parsed_args.user, + parsed_args.user_domain + ).id + except exceptions.CommandError: + if not validate_actor_existence: + return parsed_args.user + raise + + def _find_group(): + try: + return common.find_group( + identity_client_manager, + parsed_args.group, + parsed_args.group_domain + ).id + except exceptions.CommandError: + if not validate_actor_existence: + return parsed_args.group + raise + kwargs = {} if parsed_args.user and parsed_args.system: - kwargs['user'] = common.find_user( - identity_client_manager, - parsed_args.user, - parsed_args.user_domain, - ).id + kwargs['user'] = _find_user() kwargs['system'] = parsed_args.system elif parsed_args.user and parsed_args.domain: - kwargs['user'] = common.find_user( - identity_client_manager, - parsed_args.user, - parsed_args.user_domain, - ).id + kwargs['user'] = _find_user() kwargs['domain'] = common.find_domain( identity_client_manager, parsed_args.domain, ).id elif parsed_args.user and parsed_args.project: - kwargs['user'] = common.find_user( - identity_client_manager, - parsed_args.user, - parsed_args.user_domain, - ).id + kwargs['user'] = _find_user() kwargs['project'] = common.find_project( identity_client_manager, parsed_args.project, parsed_args.project_domain, ).id elif parsed_args.group and parsed_args.system: - kwargs['group'] = common.find_group( - identity_client_manager, - parsed_args.group, - parsed_args.group_domain, - ).id + kwargs['group'] = _find_group() kwargs['system'] = parsed_args.system elif parsed_args.group and parsed_args.domain: - kwargs['group'] = common.find_group( - identity_client_manager, - parsed_args.group, - parsed_args.group_domain, - ).id + kwargs['group'] = _find_group() kwargs['domain'] = common.find_domain( identity_client_manager, parsed_args.domain, ).id elif parsed_args.group and parsed_args.project: - kwargs['group'] = common.find_group( - identity_client_manager, - parsed_args.group, - parsed_args.group_domain, - ).id + kwargs['group'] = _find_group() kwargs['project'] = common.find_project( identity_client_manager, parsed_args.project, @@ -340,7 +342,9 @@ class RemoveRole(command.Command): ) kwargs = _process_identity_and_resource_options( - parsed_args, self.app.client_manager.identity) + parsed_args, self.app.client_manager.identity, + validate_actor_existence=False + ) identity_client.roles.revoke(role.id, **kwargs) diff --git a/openstackclient/network/v2/port.py b/openstackclient/network/v2/port.py index a21324ae..02ab06c1 100644 --- a/openstackclient/network/v2/port.py +++ b/openstackclient/network/v2/port.py @@ -158,6 +158,16 @@ def _get_attrs(client_manager, parsed_args): parsed_args.disable_uplink_status_propagation): attrs['propagate_uplink_status'] = False + if ('numa_policy_required' in parsed_args and + parsed_args.numa_policy_required): + attrs['numa_affinity_policy'] = 'required' + elif ('numa_policy_preferred' in parsed_args and + parsed_args.numa_policy_preferred): + attrs['numa_affinity_policy'] = 'preferred' + elif ('numa_policy_legacy' in parsed_args and + parsed_args.numa_policy_legacy): + attrs['numa_affinity_policy'] = 'legacy' + return attrs @@ -265,6 +275,22 @@ def _add_updatable_args(parser): help=_("Set DNS name for this port " "(requires DNS integration extension)") ) + numa_affinity_policy_group = parser.add_mutually_exclusive_group() + numa_affinity_policy_group.add_argument( + '--numa-policy-required', + action='store_true', + help=_("NUMA affinity policy required to schedule this port") + ) + numa_affinity_policy_group.add_argument( + '--numa-policy-preferred', + action='store_true', + help=_("NUMA affinity policy preferred to schedule this port") + ) + numa_affinity_policy_group.add_argument( + '--numa-policy-legacy', + action='store_true', + help=_("NUMA affinity policy using legacy mode to schedule this port") + ) # TODO(abhiraut): Use the SDK resource mapped attribute names once the @@ -454,12 +480,23 @@ class CreatePort(command.ShowOne): if parsed_args.qos_policy: attrs['qos_policy_id'] = client.find_qos_policy( parsed_args.qos_policy, ignore_missing=False).id + + set_tags_in_post = bool( + client.find_extension('tag-ports-during-bulk-creation')) + if set_tags_in_post: + if parsed_args.no_tag: + attrs['tags'] = [] + if parsed_args.tags: + attrs['tags'] = list(set(parsed_args.tags)) + with common.check_missing_extension_if_error( self.app.client_manager.network, attrs): obj = client.create_port(**attrs) - # tags cannot be set when created, so tags need to be set later. - _tag.update_tags_for_set(client, obj, parsed_args) + if not set_tags_in_post: + # tags cannot be set when created, so tags need to be set later. + _tag.update_tags_for_set(client, obj, parsed_args) + display_columns, columns = _get_columns(obj) data = utils.get_item_properties(obj, columns, formatters=_formatters) @@ -904,6 +941,11 @@ class UnsetPort(command.Command): action='store_true', help=_("Clear existing information of data plane status") ) + parser.add_argument( + '--numa-policy', + action='store_true', + help=_("Clear existing NUMA affinity policy") + ) _tag.add_tag_option_to_parser_for_unset(parser, _('port')) @@ -959,6 +1001,8 @@ class UnsetPort(command.Command): attrs['qos_policy_id'] = None if parsed_args.data_plane_status: attrs['data_plane_status'] = None + if parsed_args.numa_policy: + attrs['numa_affinity_policy'] = None if attrs: client.update_port(obj, **attrs) diff --git a/openstackclient/tests/functional/base.py b/openstackclient/tests/functional/base.py index 08e9390e..3542a827 100644 --- a/openstackclient/tests/functional/base.py +++ b/openstackclient/tests/functional/base.py @@ -19,10 +19,6 @@ from tempest.lib import exceptions import testtools -COMMON_DIR = os.path.dirname(os.path.abspath(__file__)) -FUNCTIONAL_DIR = os.path.normpath(os.path.join(COMMON_DIR, '..')) -ROOT_DIR = os.path.normpath(os.path.join(FUNCTIONAL_DIR, '..')) -EXAMPLE_DIR = os.path.join(ROOT_DIR, 'examples') ADMIN_CLOUD = os.environ.get('OS_ADMIN_CLOUD', 'devstack-admin') diff --git a/openstackclient/tests/functional/compute/v2/test_server.py b/openstackclient/tests/functional/compute/v2/test_server.py index 3123057c..44d9c61f 100644 --- a/openstackclient/tests/functional/compute/v2/test_server.py +++ b/openstackclient/tests/functional/compute/v2/test_server.py @@ -231,7 +231,7 @@ class ServerTests(common.ComputeTestCase): )) # Really, shouldn't this be a list? self.assertEqual( - "a='b', c='d'", + {'a': 'b', 'c': 'd'}, cmd_output['properties'], ) @@ -245,7 +245,7 @@ class ServerTests(common.ComputeTestCase): name )) self.assertEqual( - "c='d'", + {'c': 'd'}, cmd_output['properties'], ) @@ -634,8 +634,8 @@ class ServerTests(common.ComputeTestCase): server_name )) volumes_attached = cmd_output['volumes_attached'] - self.assertTrue(volumes_attached.startswith('id=')) - attached_volume_id = volumes_attached.replace('id=', '') + self.assertIsNotNone(volumes_attached) + attached_volume_id = volumes_attached[0]["id"] # check the volume that attached on server cmd_output = json.loads(self.openstack( @@ -714,8 +714,8 @@ class ServerTests(common.ComputeTestCase): server_name )) volumes_attached = cmd_output['volumes_attached'] - self.assertTrue(volumes_attached.startswith('id=')) - attached_volume_id = volumes_attached.replace('id=', '') + self.assertIsNotNone(volumes_attached) + attached_volume_id = volumes_attached[0]["id"] # check the volume that attached on server cmd_output = json.loads(self.openstack( @@ -788,10 +788,12 @@ class ServerTests(common.ComputeTestCase): server_name )) volumes_attached = cmd_output['volumes_attached'] - self.assertTrue(volumes_attached.startswith('id=')) - attached_volume_id = volumes_attached.replace('id=', '') - # Don't leak the volume when the test exits. - self.addCleanup(self.openstack, 'volume delete ' + attached_volume_id) + self.assertIsNotNone(volumes_attached) + attached_volume_id = volumes_attached[0]["id"] + for vol in volumes_attached: + self.assertIsNotNone(vol['id']) + # Don't leak the volume when the test exits. + self.addCleanup(self.openstack, 'volume delete ' + vol['id']) # Since the server is volume-backed the GET /servers/{server_id} # response will have image='N/A (booted from volume)'. @@ -800,7 +802,7 @@ class ServerTests(common.ComputeTestCase): # check the volume that attached on server cmd_output = json.loads(self.openstack( 'volume show -f json ' + - attached_volume_id + volumes_attached[0]["id"] )) # The volume size should be what we specified on the command line. self.assertEqual(1, int(cmd_output['size'])) @@ -894,14 +896,21 @@ class ServerTests(common.ComputeTestCase): self.assertIsNotNone(server['id']) self.assertEqual(server_name, server['name']) - self.assertIn(str(security_group1['id']), server['security_groups']) - self.assertIn(str(security_group2['id']), server['security_groups']) + sec_grp = "" + for sec in server['security_groups']: + sec_grp += sec['name'] + self.assertIn(str(security_group1['id']), sec_grp) + self.assertIn(str(security_group2['id']), sec_grp) self.wait_for_status(server_name, 'ACTIVE') server = json.loads(self.openstack( 'server show -f json ' + server_name )) - self.assertIn(sg_name1, server['security_groups']) - self.assertIn(sg_name2, server['security_groups']) + # check if security group exists in list + sec_grp = "" + for sec in server['security_groups']: + sec_grp += sec['name'] + self.assertIn(sg_name1, sec_grp) + self.assertIn(sg_name2, sec_grp) def test_server_create_with_empty_network_option_latest(self): """Test server create with empty network option in nova 2.latest.""" diff --git a/openstackclient/tests/functional/examples/__init__.py b/openstackclient/tests/functional/examples/__init__.py deleted file mode 100644 index e69de29b..00000000 --- a/openstackclient/tests/functional/examples/__init__.py +++ /dev/null diff --git a/openstackclient/tests/functional/examples/test_examples.py b/openstackclient/tests/functional/examples/test_examples.py deleted file mode 100644 index 031f036a..00000000 --- a/openstackclient/tests/functional/examples/test_examples.py +++ /dev/null @@ -1,28 +0,0 @@ -# Licensed under the Apache License, Version 2.0 (the "License"); you may -# not use this file except in compliance with the License. You may obtain -# a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, software -# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT -# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the -# License for the specific language governing permissions and limitations -# under the License. - -from openstackclient.tests.functional import base - - -class ExampleTests(base.TestCase): - """Functional tests for running examples.""" - - def test_common(self): - # NOTE(stevemar): If an examples has a non-zero return - # code, then execute will raise an error by default. - base.execute('python', base.EXAMPLE_DIR + '/common.py --debug') - - def test_object_api(self): - base.execute('python', base.EXAMPLE_DIR + '/object_api.py --debug') - - def test_osc_lib(self): - base.execute('python', base.EXAMPLE_DIR + '/osc-lib.py --debug') diff --git a/openstackclient/tests/unit/compute/v2/test_flavor.py b/openstackclient/tests/unit/compute/v2/test_flavor.py index fe7ce174..4732cc82 100644 --- a/openstackclient/tests/unit/compute/v2/test_flavor.py +++ b/openstackclient/tests/unit/compute/v2/test_flavor.py @@ -749,6 +749,42 @@ class TestFlavorSet(TestFlavor): self.assertRaises(exceptions.CommandError, self.cmd.take_action, parsed_args) + def test_flavor_set_description_using_name_api_newer(self): + arglist = [ + '--description', 'description', + self.flavor.name, + ] + verifylist = [ + ('description', 'description'), + ('flavor', self.flavor.name), + ] + parsed_args = self.check_parser(self.cmd, arglist, verifylist) + self.app.client_manager.compute.api_version = 2.55 + with mock.patch.object(novaclient.api_versions, + 'APIVersion', + return_value=2.55): + result = self.cmd.take_action(parsed_args) + self.flavors_mock.update.assert_called_with( + flavor=self.flavor.id, description='description') + self.assertIsNone(result) + + def test_flavor_set_description_using_name_api_older(self): + arglist = [ + '--description', 'description', + self.flavor.name, + ] + verifylist = [ + ('description', 'description'), + ('flavor', self.flavor.name), + ] + parsed_args = self.check_parser(self.cmd, arglist, verifylist) + self.app.client_manager.compute.api_version = 2.54 + with mock.patch.object(novaclient.api_versions, + 'APIVersion', + return_value=2.55): + self.assertRaises(exceptions.CommandError, self.cmd.take_action, + parsed_args) + class TestFlavorShow(TestFlavor): diff --git a/openstackclient/tests/unit/compute/v2/test_server.py b/openstackclient/tests/unit/compute/v2/test_server.py index 405e05d1..2da527c6 100644 --- a/openstackclient/tests/unit/compute/v2/test_server.py +++ b/openstackclient/tests/unit/compute/v2/test_server.py @@ -5166,6 +5166,8 @@ class TestServerGeneral(TestServer): 'tenant_id': u'tenant-id-xxx', 'networks': {u'public': [u'10.20.30.40', u'2001:db8::f']}, 'links': u'http://xxx.yyy.com', + 'properties': '', + 'volumes_attached': [{"id": "6344fe9d-ef20-45b2-91a6"}], } _server = compute_fakes.FakeServer.create_one_server(attrs=server_info) find_resource.side_effect = [_server, _flavor] @@ -5182,6 +5184,7 @@ class TestServerGeneral(TestServer): 'properties': '', 'OS-EXT-STS:power_state': server._format_servers_list_power_state( getattr(_server, 'OS-EXT-STS:power_state')), + 'volumes_attached': [{"id": "6344fe9d-ef20-45b2-91a6"}], } # Call _prep_server_detail(). diff --git a/openstackclient/tests/unit/identity/v3/test_role.py b/openstackclient/tests/unit/identity/v3/test_role.py index 544da7c1..774b2c2b 100644 --- a/openstackclient/tests/unit/identity/v3/test_role.py +++ b/openstackclient/tests/unit/identity/v3/test_role.py @@ -19,6 +19,7 @@ from unittest import mock from osc_lib import exceptions from osc_lib import utils +from openstackclient.identity import common from openstackclient.identity.v3 import role from openstackclient.tests.unit import fakes from openstackclient.tests.unit.identity.v3 import fakes as identity_fakes @@ -102,6 +103,40 @@ class TestRoleAdd(TestRole): # Get the command object to test self.cmd = role.AddRole(self.app, None) + def test_role_add_user_system(self): + arglist = [ + '--user', identity_fakes.user_name, + '--system', 'all', + identity_fakes.role_name, + ] + if self._is_inheritance_testcase(): + arglist.append('--inherited') + verifylist = [ + ('user', identity_fakes.user_name), + ('group', None), + ('system', 'all'), + ('domain', None), + ('project', None), + ('role', identity_fakes.role_name), + ('inherited', self._is_inheritance_testcase()), + ] + parsed_args = self.check_parser(self.cmd, arglist, verifylist) + + result = self.cmd.take_action(parsed_args) + + # Set expected values + kwargs = { + 'user': identity_fakes.user_id, + 'system': 'all', + 'os_inherit_extension_inherited': self._is_inheritance_testcase(), + } + # RoleManager.grant(role, user=, group=, domain=, project=) + self.roles_mock.grant.assert_called_with( + identity_fakes.role_id, + **kwargs + ) + self.assertIsNone(result) + def test_role_add_user_domain(self): arglist = [ '--user', identity_fakes.user_name, @@ -168,6 +203,40 @@ class TestRoleAdd(TestRole): ) self.assertIsNone(result) + def test_role_add_group_system(self): + arglist = [ + '--group', identity_fakes.group_name, + '--system', 'all', + identity_fakes.role_name, + ] + if self._is_inheritance_testcase(): + arglist.append('--inherited') + verifylist = [ + ('user', None), + ('group', identity_fakes.group_name), + ('system', 'all'), + ('domain', None), + ('project', None), + ('role', identity_fakes.role_name), + ('inherited', self._is_inheritance_testcase()), + ] + parsed_args = self.check_parser(self.cmd, arglist, verifylist) + + result = self.cmd.take_action(parsed_args) + + # Set expected values + kwargs = { + 'group': identity_fakes.group_id, + 'system': 'all', + 'os_inherit_extension_inherited': self._is_inheritance_testcase(), + } + # RoleManager.grant(role, user=, group=, domain=, project=) + self.roles_mock.grant.assert_called_with( + identity_fakes.role_id, + **kwargs + ) + self.assertIsNone(result) + def test_role_add_group_domain(self): arglist = [ '--group', identity_fakes.group_name, @@ -744,6 +813,81 @@ class TestRoleRemove(TestRole): # Get the command object to test self.cmd = role.RemoveRole(self.app, None) + def test_role_remove_user_system(self): + arglist = [ + '--user', identity_fakes.user_name, + '--system', 'all', + identity_fakes.role_name + ] + if self._is_inheritance_testcase(): + arglist.append('--inherited') + verifylist = [ + ('user', identity_fakes.user_name), + ('group', None), + ('system', 'all'), + ('domain', None), + ('project', None), + ('role', identity_fakes.role_name), + ('inherited', self._is_inheritance_testcase()), + ] + parsed_args = self.check_parser(self.cmd, arglist, verifylist) + + result = self.cmd.take_action(parsed_args) + + # Set expected values + kwargs = { + 'user': identity_fakes.user_id, + 'system': 'all', + 'os_inherit_extension_inherited': self._is_inheritance_testcase(), + } + # RoleManager.revoke(role, user=, group=, domain=, project=) + self.roles_mock.revoke.assert_called_with( + identity_fakes.role_id, + **kwargs + ) + self.assertIsNone(result) + + @mock.patch.object(common, 'find_user') + def test_role_remove_non_existent_user_system(self, find_mock): + # Simulate the user not being in keystone, the client should gracefully + # handle this exception and send the request to remove the role since + # keystone supports removing role assignments with non-existent actors + # (e.g., users or groups). + find_mock.side_effect = exceptions.CommandError + + arglist = [ + '--user', identity_fakes.user_id, + '--system', 'all', + identity_fakes.role_name + ] + if self._is_inheritance_testcase(): + arglist.append('--inherited') + verifylist = [ + ('user', identity_fakes.user_id), + ('group', None), + ('system', 'all'), + ('domain', None), + ('project', None), + ('role', identity_fakes.role_name), + ('inherited', self._is_inheritance_testcase()), + ] + parsed_args = self.check_parser(self.cmd, arglist, verifylist) + + result = self.cmd.take_action(parsed_args) + + # Set expected values + kwargs = { + 'user': identity_fakes.user_id, + 'system': 'all', + 'os_inherit_extension_inherited': self._is_inheritance_testcase(), + } + # RoleManager.revoke(role, user=, group=, domain=, project=) + self.roles_mock.revoke.assert_called_with( + identity_fakes.role_id, + **kwargs + ) + self.assertIsNone(result) + def test_role_remove_user_domain(self): arglist = [ '--user', identity_fakes.user_name, @@ -777,6 +921,46 @@ class TestRoleRemove(TestRole): ) self.assertIsNone(result) + @mock.patch.object(common, 'find_user') + def test_role_remove_non_existent_user_domain(self, find_mock): + # Simulate the user not being in keystone, the client the gracefully + # handle this exception and send the request to remove the role since + # keystone will validate. + find_mock.side_effect = exceptions.CommandError + + arglist = [ + '--user', identity_fakes.user_id, + '--domain', identity_fakes.domain_name, + identity_fakes.role_name + ] + if self._is_inheritance_testcase(): + arglist.append('--inherited') + verifylist = [ + ('user', identity_fakes.user_id), + ('group', None), + ('system', None), + ('domain', identity_fakes.domain_name), + ('project', None), + ('role', identity_fakes.role_name), + ('inherited', self._is_inheritance_testcase()), + ] + parsed_args = self.check_parser(self.cmd, arglist, verifylist) + + result = self.cmd.take_action(parsed_args) + + # Set expected values + kwargs = { + 'user': identity_fakes.user_id, + 'domain': identity_fakes.domain_id, + 'os_inherit_extension_inherited': self._is_inheritance_testcase(), + } + # RoleManager.revoke(role, user=, group=, domain=, project=) + self.roles_mock.revoke.assert_called_with( + identity_fakes.role_id, + **kwargs + ) + self.assertIsNone(result) + def test_role_remove_user_project(self): arglist = [ '--user', identity_fakes.user_name, @@ -810,6 +994,121 @@ class TestRoleRemove(TestRole): ) self.assertIsNone(result) + @mock.patch.object(common, 'find_user') + def test_role_remove_non_existent_user_project(self, find_mock): + # Simulate the user not being in keystone, the client the gracefully + # handle this exception and send the request to remove the role since + # keystone will validate. + find_mock.side_effect = exceptions.CommandError + + arglist = [ + '--user', identity_fakes.user_id, + '--project', identity_fakes.project_name, + identity_fakes.role_name + ] + if self._is_inheritance_testcase(): + arglist.append('--inherited') + verifylist = [ + ('user', identity_fakes.user_id), + ('group', None), + ('system', None), + ('domain', None), + ('project', identity_fakes.project_name), + ('role', identity_fakes.role_name), + ('inherited', self._is_inheritance_testcase()), + ] + parsed_args = self.check_parser(self.cmd, arglist, verifylist) + + result = self.cmd.take_action(parsed_args) + + # Set expected values + kwargs = { + 'user': identity_fakes.user_id, + 'project': identity_fakes.project_id, + 'os_inherit_extension_inherited': self._is_inheritance_testcase(), + } + # RoleManager.revoke(role, user=, group=, domain=, project=) + self.roles_mock.revoke.assert_called_with( + identity_fakes.role_id, + **kwargs + ) + self.assertIsNone(result) + + def test_role_remove_group_system(self): + arglist = [ + '--group', identity_fakes.group_name, + '--system', 'all', + identity_fakes.role_name, + ] + if self._is_inheritance_testcase(): + arglist.append('--inherited') + verifylist = [ + ('user', None), + ('group', identity_fakes.group_name), + ('system', 'all'), + ('domain', None), + ('project', None), + ('role', identity_fakes.role_name), + ('role', identity_fakes.role_name), + ('inherited', self._is_inheritance_testcase()), + ] + parsed_args = self.check_parser(self.cmd, arglist, verifylist) + + result = self.cmd.take_action(parsed_args) + + # Set expected values + kwargs = { + 'group': identity_fakes.group_id, + 'system': 'all', + 'os_inherit_extension_inherited': self._is_inheritance_testcase(), + } + # RoleManager.revoke(role, user=, group=, domain=, project=) + self.roles_mock.revoke.assert_called_with( + identity_fakes.role_id, + **kwargs + ) + self.assertIsNone(result) + + @mock.patch.object(common, 'find_group') + def test_role_remove_non_existent_group_system(self, find_mock): + # Simulate the user not being in keystone, the client the gracefully + # handle this exception and send the request to remove the role since + # keystone will validate. + find_mock.side_effect = exceptions.CommandError + + arglist = [ + '--group', identity_fakes.group_id, + '--system', 'all', + identity_fakes.role_name + ] + if self._is_inheritance_testcase(): + arglist.append('--inherited') + verifylist = [ + ('user', None), + ('group', identity_fakes.group_id), + ('system', 'all'), + ('domain', None), + ('project', None), + ('role', identity_fakes.role_name), + ('inherited', self._is_inheritance_testcase()), + ] + parsed_args = self.check_parser(self.cmd, arglist, verifylist) + + result = self.cmd.take_action(parsed_args) + + # Set expected values + kwargs = { + 'group': identity_fakes.group_id, + 'system': 'all', + 'os_inherit_extension_inherited': self._is_inheritance_testcase(), + } + # RoleManager.revoke(role, user=, group=, domain=, project=) + self.roles_mock.revoke.assert_called_with( + identity_fakes.role_id, + **kwargs + ) + self.assertIsNone(result) + def test_role_remove_group_domain(self): arglist = [ '--group', identity_fakes.group_name, @@ -844,6 +1143,46 @@ class TestRoleRemove(TestRole): ) self.assertIsNone(result) + @mock.patch.object(common, 'find_group') + def test_role_remove_non_existent_group_domain(self, find_mock): + # Simulate the user not being in keystone, the client the gracefully + # handle this exception and send the request to remove the role since + # keystone will validate. + find_mock.side_effect = exceptions.CommandError + + arglist = [ + '--group', identity_fakes.group_id, + '--domain', identity_fakes.domain_name, + identity_fakes.role_name + ] + if self._is_inheritance_testcase(): + arglist.append('--inherited') + verifylist = [ + ('user', None), + ('group', identity_fakes.group_id), + ('system', None), + ('domain', identity_fakes.domain_name), + ('project', None), + ('role', identity_fakes.role_name), + ('inherited', self._is_inheritance_testcase()), + ] + parsed_args = self.check_parser(self.cmd, arglist, verifylist) + + result = self.cmd.take_action(parsed_args) + + # Set expected values + kwargs = { + 'group': identity_fakes.group_id, + 'domain': identity_fakes.domain_id, + 'os_inherit_extension_inherited': self._is_inheritance_testcase(), + } + # RoleManager.revoke(role, user=, group=, domain=, project=) + self.roles_mock.revoke.assert_called_with( + identity_fakes.role_id, + **kwargs + ) + self.assertIsNone(result) + def test_role_remove_group_project(self): arglist = [ '--group', identity_fakes.group_name, @@ -877,6 +1216,46 @@ class TestRoleRemove(TestRole): ) self.assertIsNone(result) + @mock.patch.object(common, 'find_group') + def test_role_remove_non_existent_group_project(self, find_mock): + # Simulate the user not being in keystone, the client the gracefully + # handle this exception and send the request to remove the role since + # keystone will validate. + find_mock.side_effect = exceptions.CommandError + + arglist = [ + '--group', identity_fakes.group_id, + '--project', identity_fakes.project_name, + identity_fakes.role_name + ] + if self._is_inheritance_testcase(): + arglist.append('--inherited') + verifylist = [ + ('user', None), + ('group', identity_fakes.group_id), + ('system', None), + ('domain', None), + ('project', identity_fakes.project_name), + ('role', identity_fakes.role_name), + ('inherited', self._is_inheritance_testcase()), + ] + parsed_args = self.check_parser(self.cmd, arglist, verifylist) + + result = self.cmd.take_action(parsed_args) + + # Set expected values + kwargs = { + 'group': identity_fakes.group_id, + 'project': identity_fakes.project_id, + 'os_inherit_extension_inherited': self._is_inheritance_testcase(), + } + # RoleManager.revoke(role, user=, group=, domain=, project=) + self.roles_mock.revoke.assert_called_with( + identity_fakes.role_id, + **kwargs + ) + self.assertIsNone(result) + def test_role_remove_domain_role_on_group_domain(self): self.roles_mock.get.return_value = fakes.FakeResource( None, diff --git a/openstackclient/tests/unit/network/v2/fakes.py b/openstackclient/tests/unit/network/v2/fakes.py index cef0a11c..3df4042c 100644 --- a/openstackclient/tests/unit/network/v2/fakes.py +++ b/openstackclient/tests/unit/network/v2/fakes.py @@ -634,6 +634,7 @@ class FakePort(object): 'mac_address': 'fa:16:3e:a9:4e:72', 'name': 'port-name-' + uuid.uuid4().hex, 'network_id': 'network-id-' + uuid.uuid4().hex, + 'numa_affinity_policy': 'required', 'port_security_enabled': True, 'security_group_ids': [], 'status': 'ACTIVE', diff --git a/openstackclient/tests/unit/network/v2/test_port.py b/openstackclient/tests/unit/network/v2/test_port.py index 87aea61f..f729b552 100644 --- a/openstackclient/tests/unit/network/v2/test_port.py +++ b/openstackclient/tests/unit/network/v2/test_port.py @@ -59,6 +59,7 @@ class TestPort(network_fakes.TestNetworkV2): 'mac_address', 'name', 'network_id', + 'numa_affinity_policy', 'port_security_enabled', 'project_id', 'qos_network_policy_id', @@ -90,6 +91,7 @@ class TestPort(network_fakes.TestNetworkV2): fake_port.mac_address, fake_port.name, fake_port.network_id, + fake_port.numa_affinity_policy, fake_port.port_security_enabled, fake_port.project_id, fake_port.qos_network_policy_id, @@ -119,6 +121,7 @@ class TestCreatePort(TestPort): self.network.find_network = mock.Mock(return_value=fake_net) self.fake_subnet = network_fakes.FakeSubnet.create_one_subnet() self.network.find_subnet = mock.Mock(return_value=self.fake_subnet) + self.network.find_extension = mock.Mock(return_value=[]) # Get the command object to test self.cmd = port.CreatePort(self.app, self.namespace) @@ -534,7 +537,7 @@ class TestCreatePort(TestPort): 'name': 'test-port', }) - def _test_create_with_tag(self, add_tags=True): + def _test_create_with_tag(self, add_tags=True, add_tags_in_post=True): arglist = [ '--network', self._port.network_id, 'test-port', @@ -553,28 +556,59 @@ class TestCreatePort(TestPort): else: verifylist.append(('no_tag', True)) + self.network.find_extension = mock.Mock(return_value=add_tags_in_post) + parsed_args = self.check_parser(self.cmd, arglist, verifylist) columns, data = (self.cmd.take_action(parsed_args)) - self.network.create_port.assert_called_once_with( - admin_state_up=True, - network_id=self._port.network_id, - name='test-port' - ) - if add_tags: - self.network.set_tags.assert_called_once_with( - self._port, - tests_utils.CompareBySet(['red', 'blue'])) + args = { + 'admin_state_up': True, + 'network_id': self._port.network_id, + 'name': 'test-port', + } + if add_tags_in_post: + if add_tags: + args['tags'] = sorted(['red', 'blue']) + else: + args['tags'] = [] + self.network.create_port.assert_called_once() + # Now we need to verify if arguments to call create_port are as + # expected, + # But we can't simply use assert_called_once_with() method because + # duplicates from 'tags' are removed with + # list(set(parsed_args.tags)) and that don't quarantee order of + # tags list which is used to call create_port(). + create_port_call_kwargs = self.network.create_port.call_args[1] + create_port_call_kwargs['tags'] = sorted( + create_port_call_kwargs['tags']) + self.assertDictEqual(args, create_port_call_kwargs) else: - self.assertFalse(self.network.set_tags.called) + self.network.create_port.assert_called_once_with( + admin_state_up=True, + network_id=self._port.network_id, + name='test-port' + ) + if add_tags: + self.network.set_tags.assert_called_once_with( + self._port, + tests_utils.CompareBySet(['red', 'blue'])) + else: + self.assertFalse(self.network.set_tags.called) + self.assertEqual(self.columns, columns) self.assertItemEqual(self.data, data) def test_create_with_tags(self): - self._test_create_with_tag(add_tags=True) + self._test_create_with_tag(add_tags=True, add_tags_in_post=True) def test_create_with_no_tag(self): - self._test_create_with_tag(add_tags=False) + self._test_create_with_tag(add_tags=False, add_tags_in_post=True) + + def test_create_with_tags_using_put(self): + self._test_create_with_tag(add_tags=True, add_tags_in_post=False) + + def test_create_with_no_tag_using_put(self): + self._test_create_with_tag(add_tags=False, add_tags_in_post=False) def _test_create_with_uplink_status_propagation(self, enable=True): arglist = [ @@ -655,6 +689,50 @@ class TestCreatePort(TestPort): 'name': 'test-port', }) + def _test_create_with_numa_affinity_policy(self, policy=None): + arglist = [ + '--network', self._port.network_id, + 'test-port', + ] + if policy: + arglist += ['--numa-policy-%s' % policy] + + numa_affinity_policy = None if not policy else policy + verifylist = [ + ('network', self._port.network_id,), + ('name', 'test-port'), + ] + if policy: + verifylist.append(('numa_policy_%s' % policy, True)) + + parsed_args = self.check_parser(self.cmd, arglist, verifylist) + + columns, data = (self.cmd.take_action(parsed_args)) + + create_args = { + 'admin_state_up': True, + 'network_id': self._port.network_id, + 'name': 'test-port', + } + if numa_affinity_policy: + create_args['numa_affinity_policy'] = numa_affinity_policy + self.network.create_port.assert_called_once_with(**create_args) + + self.assertEqual(self.columns, columns) + self.assertItemEqual(self.data, data) + + def test_create_with_numa_affinity_policy_required(self): + self._test_create_with_numa_affinity_policy(policy='required') + + def test_create_with_numa_affinity_policy_preferred(self): + self._test_create_with_numa_affinity_policy(policy='preferred') + + def test_create_with_numa_affinity_policy_legacy(self): + self._test_create_with_numa_affinity_policy(policy='legacy') + + def test_create_with_numa_affinity_policy_null(self): + self._test_create_with_numa_affinity_policy() + class TestDeletePort(TestPort): @@ -1668,6 +1746,32 @@ class TestSetPort(TestPort): def test_set_with_no_tag(self): self._test_set_tags(with_tags=False) + def _test_create_with_numa_affinity_policy(self, policy): + arglist = [ + '--numa-policy-%s' % policy, + self._port.id, + ] + verifylist = [ + ('numa_policy_%s' % policy, True), + ('port', self._port.id,) + ] + + parsed_args = self.check_parser(self.cmd, arglist, verifylist) + + self.cmd.take_action(parsed_args) + + self.network.update_port.assert_called_once_with( + self._port, **{'numa_affinity_policy': policy}) + + def test_create_with_numa_affinity_policy_required(self): + self._test_create_with_numa_affinity_policy('required') + + def test_create_with_numa_affinity_policy_preferred(self): + self._test_create_with_numa_affinity_policy('preferred') + + def test_create_with_numa_affinity_policy_legacy(self): + self._test_create_with_numa_affinity_policy('legacy') + class TestShowPort(TestPort): @@ -1923,3 +2027,26 @@ class TestUnsetPort(TestPort): def test_unset_with_all_tag(self): self._test_unset_tags(with_tags=False) + + def test_unset_numa_affinity_policy(self): + _fake_port = network_fakes.FakePort.create_one_port( + {'numa_affinity_policy': 'required'}) + self.network.find_port = mock.Mock(return_value=_fake_port) + arglist = [ + '--numa-policy', + _fake_port.name, + ] + verifylist = [ + ('numa_policy', True), + ('port', _fake_port.name), + ] + + parsed_args = self.check_parser(self.cmd, arglist, verifylist) + result = self.cmd.take_action(parsed_args) + + attrs = { + 'numa_affinity_policy': None, + } + + self.network.update_port.assert_called_once_with(_fake_port, **attrs) + self.assertIsNone(result) diff --git a/releasenotes/notes/add-port-numa-affinity-policy-4706b0f9485a5d4d.yaml b/releasenotes/notes/add-port-numa-affinity-policy-4706b0f9485a5d4d.yaml new file mode 100644 index 00000000..0fbc54b1 --- /dev/null +++ b/releasenotes/notes/add-port-numa-affinity-policy-4706b0f9485a5d4d.yaml @@ -0,0 +1,5 @@ +--- +features: + - | + Add NUMA affinity policy to ``port create``, ``port set`` and + ``port unset`` commands. diff --git a/releasenotes/notes/bug-2006635-3110f7a87a186e62.yaml b/releasenotes/notes/bug-2006635-3110f7a87a186e62.yaml new file mode 100644 index 00000000..ecc58c8a --- /dev/null +++ b/releasenotes/notes/bug-2006635-3110f7a87a186e62.yaml @@ -0,0 +1,7 @@ +--- +fixes: + - | + You can now remove role assignments from keystone that reference non-existent + users or groups. + + [Bug `2006635 <https://storyboard.openstack.org/#!/story/2006635>`_] diff --git a/releasenotes/notes/security-grp-json-fix.yaml-2af1f48a48034d64.yaml b/releasenotes/notes/security-grp-json-fix.yaml-2af1f48a48034d64.yaml new file mode 100644 index 00000000..3a0155a1 --- /dev/null +++ b/releasenotes/notes/security-grp-json-fix.yaml-2af1f48a48034d64.yaml @@ -0,0 +1,4 @@ +--- +fixes: + - The ``openstack server show -f json`` command was not outputting + json for security groups, volumes and properties properly. @@ -111,7 +111,6 @@ commands = [testenv:docs] deps = -c{env:UPPER_CONSTRAINTS_FILE:https://releases.openstack.org/constraints/upper/master} - -r{toxinidir}/requirements.txt -r{toxinidir}/doc/requirements.txt commands = sphinx-build -a -E -W -d doc/build/doctrees -b html doc/source doc/build/html |