From 9d946f0f45c83c5677e9dd2688830c45cb6a24af Mon Sep 17 00:00:00 2001 From: Ankur Gupta Date: Mon, 6 Feb 2017 22:37:46 -0800 Subject: [PATCH] Port set/unset SecGroup Bug Fix Throwing error 'Port' object has no attribute 'security_groups' Fix for set and unset. Change-Id: I1a0625b5a432c7a91cf40249ce4f7c883f53d704 Closes-Bug: #1656788 --- openstackclient/network/v2/port.py | 48 +++++++++---------- .../tests/functional/network/v2/test_port.py | 15 +++++- .../tests/unit/network/v2/fakes.py | 4 +- .../tests/unit/network/v2/test_port.py | 44 ++++++++--------- .../notes/bug-1656788-2f5bda2205bc0329.yaml | 6 +++ 5 files changed, 67 insertions(+), 50 deletions(-) create mode 100644 releasenotes/notes/bug-1656788-2f5bda2205bc0329.yaml diff --git a/openstackclient/network/v2/port.py b/openstackclient/network/v2/port.py index 7283dc0706..6117175e9e 100644 --- a/openstackclient/network/v2/port.py +++ b/openstackclient/network/v2/port.py @@ -51,7 +51,6 @@ _formatters = { 'extra_dhcp_opts': utils.format_list_of_dicts, 'fixed_ips': utils.format_list_of_dicts, 'security_group_ids': utils.format_list, - 'security_groups': utils.format_list, } @@ -64,7 +63,6 @@ def _get_columns(item): 'binding:vnic_type': 'binding_vnic_type', 'is_admin_state_up': 'admin_state_up', 'is_port_security_enabled': 'port_security_enabled', - 'security_group_ids': 'security_groups', 'tenant_id': 'project_id', } return sdk_utils.get_osc_show_columns_for_sdk_resource(item, column_map) @@ -349,7 +347,7 @@ class CreatePort(command.ShowOne): '--security-group', metavar='', action='append', - dest='security_groups', + dest='security_group', help=_("Security group to associate with this port (name or ID) " "(repeat option to set multiple security groups)") ) @@ -391,12 +389,13 @@ class CreatePort(command.ShowOne): _prepare_fixed_ips(self.app.client_manager, parsed_args) attrs = _get_attrs(self.app.client_manager, parsed_args) - if parsed_args.security_groups: - attrs['security_groups'] = [client.find_security_group( - sg, ignore_missing=False).id - for sg in parsed_args.security_groups] - if parsed_args.no_security_group: - attrs['security_groups'] = [] + if parsed_args.security_group: + attrs['security_group_ids'] = [client.find_security_group( + sg, ignore_missing=False).id + for sg in + parsed_args.security_group] + elif parsed_args.no_security_group: + attrs['security_group_ids'] = [] if parsed_args.allowed_address_pairs: attrs['allowed_address_pairs'] = ( _convert_address_pairs(parsed_args)) @@ -626,7 +625,7 @@ class SetPort(command.Command): '--security-group', metavar='', action='append', - dest='security_groups', + dest='security_group', help=_("Security group to associate with this port (name or ID) " "(repeat option to set multiple security groups)") ) @@ -694,17 +693,16 @@ class SetPort(command.Command): attrs['fixed_ips'] += [ip for ip in obj.fixed_ips if ip] elif parsed_args.no_fixed_ip: attrs['fixed_ips'] = [] - if parsed_args.security_groups and parsed_args.no_security_group: - attrs['security_groups'] = [client.find_security_group(sg, - ignore_missing=False).id - for sg in parsed_args.security_groups] - elif parsed_args.security_groups: - attrs['security_groups'] = obj.security_groups - for sg in parsed_args.security_groups: - sg_id = client.find_security_group(sg, ignore_missing=False).id - attrs['security_groups'].append(sg_id) + + if parsed_args.security_group: + attrs['security_group_ids'] = [ + client.find_security_group(sg, ignore_missing=False).id for + sg in parsed_args.security_group] + if not parsed_args.no_security_group: + attrs['security_group_ids'] += obj.security_group_ids + elif parsed_args.no_security_group: - attrs['security_groups'] = [] + attrs['security_group_ids'] = [] if (parsed_args.allowed_address_pairs and parsed_args.no_allowed_address_pair): @@ -769,7 +767,7 @@ class UnsetPort(command.Command): '--security-group', metavar='', action='append', - dest='security_groups', + dest='security_group_ids', help=_("Security group which should be removed this port (name " "or ID) (repeat option to unset multiple security groups)") ) @@ -802,7 +800,7 @@ class UnsetPort(command.Command): # Unset* classes tmp_fixed_ips = copy.deepcopy(obj.fixed_ips) tmp_binding_profile = copy.deepcopy(obj.binding_profile) - tmp_secgroups = copy.deepcopy(obj.security_groups) + tmp_secgroups = copy.deepcopy(obj.security_group_ids) tmp_addr_pairs = copy.deepcopy(obj.allowed_address_pairs) _prepare_fixed_ips(self.app.client_manager, parsed_args) attrs = {} @@ -822,16 +820,16 @@ class UnsetPort(command.Command): msg = _("Port does not contain binding-profile %s") % key raise exceptions.CommandError(msg) attrs['binding:profile'] = tmp_binding_profile - if parsed_args.security_groups: + if parsed_args.security_group_ids: try: - for sg in parsed_args.security_groups: + for sg in parsed_args.security_group_ids: sg_id = client.find_security_group( sg, ignore_missing=False).id tmp_secgroups.remove(sg_id) except ValueError: msg = _("Port does not contain security group %s") % sg raise exceptions.CommandError(msg) - attrs['security_groups'] = tmp_secgroups + attrs['security_group_ids'] = tmp_secgroups if parsed_args.allowed_address_pairs: try: for addr in _convert_address_pairs(parsed_args): diff --git a/openstackclient/tests/functional/network/v2/test_port.py b/openstackclient/tests/functional/network/v2/test_port.py index 818076d675..78c572730d 100644 --- a/openstackclient/tests/functional/network/v2/test_port.py +++ b/openstackclient/tests/functional/network/v2/test_port.py @@ -20,6 +20,7 @@ class PortTests(base.TestCase): """Functional tests for port. """ NAME = uuid.uuid4().hex NETWORK_NAME = uuid.uuid4().hex + SG_NAME = uuid.uuid4().hex @classmethod def setUpClass(cls): @@ -124,13 +125,25 @@ class PortTests(base.TestCase): self.assertEqual('xyzpdq', json_output.get('description')) self.assertEqual('DOWN', json_output.get('admin_state_up')) - raw_output = self.openstack('port set ' + '--enable ' + self.NAME) + raw_output = self.openstack( + 'port set ' + '--enable ' + self.NAME) self.assertOutput('', raw_output) json_output = json.loads(self.openstack( 'port show -f json ' + self.NAME )) + sg_id = json_output.get('security_group_ids') + self.assertEqual(self.NAME, json_output.get('name')) self.assertEqual('xyzpdq', json_output.get('description')) self.assertEqual('UP', json_output.get('admin_state_up')) self.assertIsNotNone(json_output.get('mac_address')) + + raw_output = self.openstack( + 'port unset --security-group ' + sg_id + ' ' + id1) + self.assertOutput('', raw_output) + + json_output = json.loads(self.openstack( + 'port show -f json ' + self.NAME + )) + self.assertEqual('', json_output.get('security_group_ids')) diff --git a/openstackclient/tests/unit/network/v2/fakes.py b/openstackclient/tests/unit/network/v2/fakes.py index 1125289e28..612dcab003 100644 --- a/openstackclient/tests/unit/network/v2/fakes.py +++ b/openstackclient/tests/unit/network/v2/fakes.py @@ -544,7 +544,7 @@ class FakePort(object): 'name': 'port-name-' + uuid.uuid4().hex, 'network_id': 'network-id-' + uuid.uuid4().hex, 'port_security_enabled': True, - 'security_groups': [], + 'security_group_ids': [], 'status': 'ACTIVE', 'tenant_id': 'project-id-' + uuid.uuid4().hex, } @@ -564,7 +564,7 @@ class FakePort(object): port.is_admin_state_up = port_attrs['admin_state_up'] port.is_port_security_enabled = port_attrs['port_security_enabled'] port.project_id = port_attrs['tenant_id'] - port.security_group_ids = port_attrs['security_groups'] + port.security_group_ids = port_attrs['security_group_ids'] return port diff --git a/openstackclient/tests/unit/network/v2/test_port.py b/openstackclient/tests/unit/network/v2/test_port.py index bfffc5c062..80eba3a875 100644 --- a/openstackclient/tests/unit/network/v2/test_port.py +++ b/openstackclient/tests/unit/network/v2/test_port.py @@ -57,7 +57,7 @@ class TestPort(network_fakes.TestNetworkV2): 'network_id', 'port_security_enabled', 'project_id', - 'security_groups', + 'security_group_ids', 'status', ) @@ -82,7 +82,7 @@ class TestPort(network_fakes.TestNetworkV2): fake_port.network_id, fake_port.port_security_enabled, fake_port.project_id, - utils.format_list(fake_port.security_groups), + utils.format_list(fake_port.security_group_ids), fake_port.status, ) @@ -251,7 +251,7 @@ class TestCreatePort(TestPort): verifylist = [ ('network', self._port.network_id,), ('enable', True), - ('security_groups', [secgroup.id]), + ('security_group', [secgroup.id]), ('name', 'test-port'), ] parsed_args = self.check_parser(self.cmd, arglist, verifylist) @@ -261,7 +261,7 @@ class TestCreatePort(TestPort): self.network.create_port.assert_called_once_with(**{ 'admin_state_up': True, 'network_id': self._port.network_id, - 'security_groups': [secgroup.id], + 'security_group_ids': [secgroup.id], 'name': 'test-port', }) @@ -309,7 +309,7 @@ class TestCreatePort(TestPort): verifylist = [ ('network', self._port.network_id,), ('enable', True), - ('security_groups', [sg_1.id, sg_2.id]), + ('security_group', [sg_1.id, sg_2.id]), ('name', 'test-port'), ] parsed_args = self.check_parser(self.cmd, arglist, verifylist) @@ -319,7 +319,7 @@ class TestCreatePort(TestPort): self.network.create_port.assert_called_once_with(**{ 'admin_state_up': True, 'network_id': self._port.network_id, - 'security_groups': [sg_1.id, sg_2.id], + 'security_group_ids': [sg_1.id, sg_2.id], 'name': 'test-port', }) @@ -346,7 +346,7 @@ class TestCreatePort(TestPort): self.network.create_port.assert_called_once_with(**{ 'admin_state_up': True, 'network_id': self._port.network_id, - 'security_groups': [], + 'security_group_ids': [], 'name': 'test-port', }) @@ -590,7 +590,7 @@ class TestListPort(TestPort): prt.mac_address, utils.format_list_of_dicts(prt.fixed_ips), prt.status, - utils.format_list(prt.security_groups), + utils.format_list(prt.security_group_ids), prt.device_owner, )) @@ -1111,7 +1111,7 @@ class TestSetPort(TestPort): self._port.name, ] verifylist = [ - ('security_groups', [sg.id]), + ('security_group', [sg.id]), ('port', self._port.name), ] @@ -1119,7 +1119,7 @@ class TestSetPort(TestPort): result = self.cmd.take_action(parsed_args) attrs = { - 'security_groups': [sg.id], + 'security_group_ids': [sg.id], } self.network.update_port.assert_called_once_with(self._port, **attrs) self.assertIsNone(result) @@ -1130,7 +1130,7 @@ class TestSetPort(TestPort): sg_3 = network_fakes.FakeSecurityGroup.create_one_security_group() self.network.find_security_group = mock.Mock(side_effect=[sg_2, sg_3]) _testport = network_fakes.FakePort.create_one_port( - {'security_groups': [sg_1.id]}) + {'security_group_ids': [sg_1.id]}) self.network.find_port = mock.Mock(return_value=_testport) arglist = [ '--security-group', sg_2.id, @@ -1138,13 +1138,13 @@ class TestSetPort(TestPort): _testport.name, ] verifylist = [ - ('security_groups', [sg_2.id, sg_3.id]), + ('security_group', [sg_2.id, sg_3.id]), ('port', _testport.name), ] parsed_args = self.check_parser(self.cmd, arglist, verifylist) result = self.cmd.take_action(parsed_args) attrs = { - 'security_groups': [sg_1.id, sg_2.id, sg_3.id], + 'security_group_ids': [sg_2.id, sg_3.id, sg_1.id], } self.network.update_port.assert_called_once_with(_testport, **attrs) self.assertIsNone(result) @@ -1163,7 +1163,7 @@ class TestSetPort(TestPort): result = self.cmd.take_action(parsed_args) attrs = { - 'security_groups': [], + 'security_group_ids': [], } self.network.update_port.assert_called_once_with(self._port, **attrs) self.assertIsNone(result) @@ -1172,7 +1172,7 @@ class TestSetPort(TestPort): sg1 = network_fakes.FakeSecurityGroup.create_one_security_group() sg2 = network_fakes.FakeSecurityGroup.create_one_security_group() _testport = network_fakes.FakePort.create_one_port( - {'security_groups': [sg1.id]}) + {'security_group_ids': [sg1.id]}) self.network.find_port = mock.Mock(return_value=_testport) self.network.find_security_group = mock.Mock(return_value=sg2) arglist = [ @@ -1181,13 +1181,13 @@ class TestSetPort(TestPort): _testport.name, ] verifylist = [ - ('security_groups', [sg2.id]), + ('security_group', [sg2.id]), ('no_security_group', True) ] parsed_args = self.check_parser(self.cmd, arglist, verifylist) result = self.cmd.take_action(parsed_args) attrs = { - 'security_groups': [sg2.id], + 'security_group_ids': [sg2.id], } self.network.update_port.assert_called_once_with(_testport, **attrs) self.assertIsNone(result) @@ -1434,7 +1434,7 @@ class TestUnsetPort(TestPort): _fake_sg1 = network_fakes.FakeSecurityGroup.create_one_security_group() _fake_sg2 = network_fakes.FakeSecurityGroup.create_one_security_group() _fake_port = network_fakes.FakePort.create_one_port( - {'security_groups': [_fake_sg1.id, _fake_sg2.id]}) + {'security_group_ids': [_fake_sg1.id, _fake_sg2.id]}) self.network.find_port = mock.Mock(return_value=_fake_port) self.network.find_security_group = mock.Mock(return_value=_fake_sg2) arglist = [ @@ -1442,14 +1442,14 @@ class TestUnsetPort(TestPort): _fake_port.name, ] verifylist = [ - ('security_groups', [_fake_sg2.id]), + ('security_group_ids', [_fake_sg2.id]), ] parsed_args = self.check_parser(self.cmd, arglist, verifylist) result = self.cmd.take_action(parsed_args) attrs = { - 'security_groups': [_fake_sg1.id] + 'security_group_ids': [_fake_sg1.id] } self.network.update_port.assert_called_once_with( _fake_port, **attrs) @@ -1459,14 +1459,14 @@ class TestUnsetPort(TestPort): _fake_sg1 = network_fakes.FakeSecurityGroup.create_one_security_group() _fake_sg2 = network_fakes.FakeSecurityGroup.create_one_security_group() _fake_port = network_fakes.FakePort.create_one_port( - {'security_groups': [_fake_sg1.id]}) + {'security_group_ids': [_fake_sg1.id]}) self.network.find_security_group = mock.Mock(return_value=_fake_sg2) arglist = [ '--security-group', _fake_sg2.id, _fake_port.name, ] verifylist = [ - ('security_groups', [_fake_sg2.id]), + ('security_group_ids', [_fake_sg2.id]), ] parsed_args = self.check_parser(self.cmd, arglist, verifylist) diff --git a/releasenotes/notes/bug-1656788-2f5bda2205bc0329.yaml b/releasenotes/notes/bug-1656788-2f5bda2205bc0329.yaml new file mode 100644 index 0000000000..1a5fd2c103 --- /dev/null +++ b/releasenotes/notes/bug-1656788-2f5bda2205bc0329.yaml @@ -0,0 +1,6 @@ +--- +fixes: + - | + Fixed the ``port set`` and ``port unset`` command failures + (AttributeError) when ``--security-group`` option is included. + [Bug `1656788 `_]