openstack/python-openstackclient
Code Issues Proposed changes

Merge "Port set/unset SecGroup Bug Fix"

Browse Source
This commit is contained in:
Jenkins 2017-02-27 17:18:00 +00:00 committed by Gerrit Code Review
commit 264f81f6fc
5 changed files with 67 additions and 50 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

46
openstackclient/network/v2/port.py
View File

@ -51,7 +51,6 @@ _formatters = {
'extra_dhcp_opts': utils.format_list_of_dicts, 'extra_dhcp_opts': utils.format_list_of_dicts,
'fixed_ips': utils.format_list_of_dicts, 'fixed_ips': utils.format_list_of_dicts,
'security_group_ids': utils.format_list, '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', 'binding:vnic_type': 'binding_vnic_type',
'is_admin_state_up': 'admin_state_up', 'is_admin_state_up': 'admin_state_up',
'is_port_security_enabled': 'port_security_enabled', 'is_port_security_enabled': 'port_security_enabled',
'security_group_ids': 'security_groups',
'tenant_id': 'project_id', 'tenant_id': 'project_id',
} }
return sdk_utils.get_osc_show_columns_for_sdk_resource(item, column_map) return sdk_utils.get_osc_show_columns_for_sdk_resource(item, column_map)
@ -349,7 +347,7 @@ class CreatePort(command.ShowOne):
'--security-group', '--security-group',
metavar='<security-group>', metavar='<security-group>',
action='append', action='append',
dest='security_groups', dest='security_group',
help=_("Security group to associate with this port (name or ID) " help=_("Security group to associate with this port (name or ID) "
"(repeat option to set multiple security groups)") "(repeat option to set multiple security groups)")
) )
@ -391,12 +389,13 @@ class CreatePort(command.ShowOne):
_prepare_fixed_ips(self.app.client_manager, parsed_args) _prepare_fixed_ips(self.app.client_manager, parsed_args)
attrs = _get_attrs(self.app.client_manager, parsed_args) attrs = _get_attrs(self.app.client_manager, parsed_args)
if parsed_args.security_groups: if parsed_args.security_group:
attrs['security_groups'] = [client.find_security_group( attrs['security_group_ids'] = [client.find_security_group(
sg, ignore_missing=False).id sg, ignore_missing=False).id
for sg in parsed_args.security_groups] for sg in
if parsed_args.no_security_group: parsed_args.security_group]
attrs['security_groups'] = [] elif parsed_args.no_security_group:
attrs['security_group_ids'] = []
if parsed_args.allowed_address_pairs: if parsed_args.allowed_address_pairs:
attrs['allowed_address_pairs'] = ( attrs['allowed_address_pairs'] = (
_convert_address_pairs(parsed_args)) _convert_address_pairs(parsed_args))
@ -626,7 +625,7 @@ class SetPort(command.Command):
'--security-group', '--security-group',
metavar='<security-group>', metavar='<security-group>',
action='append', action='append',
dest='security_groups', dest='security_group',
help=_("Security group to associate with this port (name or ID) " help=_("Security group to associate with this port (name or ID) "
"(repeat option to set multiple security groups)") "(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] attrs['fixed_ips'] += [ip for ip in obj.fixed_ips if ip]
elif parsed_args.no_fixed_ip: elif parsed_args.no_fixed_ip:
attrs['fixed_ips'] = [] attrs['fixed_ips'] = []
if parsed_args.security_groups and parsed_args.no_security_group:
attrs['security_groups'] = [client.find_security_group(sg, if parsed_args.security_group:
ignore_missing=False).id attrs['security_group_ids'] = [
for sg in parsed_args.security_groups] client.find_security_group(sg, ignore_missing=False).id for
elif parsed_args.security_groups: sg in parsed_args.security_group]
attrs['security_groups'] = obj.security_groups if not parsed_args.no_security_group:
for sg in parsed_args.security_groups: attrs['security_group_ids'] += obj.security_group_ids
sg_id = client.find_security_group(sg, ignore_missing=False).id
attrs['security_groups'].append(sg_id)
elif parsed_args.no_security_group: elif parsed_args.no_security_group:
attrs['security_groups'] = [] attrs['security_group_ids'] = []
if (parsed_args.allowed_address_pairs and if (parsed_args.allowed_address_pairs and
parsed_args.no_allowed_address_pair): parsed_args.no_allowed_address_pair):
@ -769,7 +767,7 @@ class UnsetPort(command.Command):
'--security-group', '--security-group',
metavar='<security-group>', metavar='<security-group>',
action='append', action='append',
dest='security_groups', dest='security_group_ids',
help=_("Security group which should be removed this port (name " help=_("Security group which should be removed this port (name "
"or ID) (repeat option to unset multiple security groups)") "or ID) (repeat option to unset multiple security groups)")
) )
@ -802,7 +800,7 @@ class UnsetPort(command.Command):
# Unset* classes # Unset* classes
tmp_fixed_ips = copy.deepcopy(obj.fixed_ips) tmp_fixed_ips = copy.deepcopy(obj.fixed_ips)
tmp_binding_profile = copy.deepcopy(obj.binding_profile) 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) tmp_addr_pairs = copy.deepcopy(obj.allowed_address_pairs)
_prepare_fixed_ips(self.app.client_manager, parsed_args) _prepare_fixed_ips(self.app.client_manager, parsed_args)
attrs = {} attrs = {}
@ -822,16 +820,16 @@ class UnsetPort(command.Command):
msg = _("Port does not contain binding-profile %s") % key msg = _("Port does not contain binding-profile %s") % key
raise exceptions.CommandError(msg) raise exceptions.CommandError(msg)
attrs['binding:profile'] = tmp_binding_profile attrs['binding:profile'] = tmp_binding_profile
if parsed_args.security_groups: if parsed_args.security_group_ids:
try: try:
for sg in parsed_args.security_groups: for sg in parsed_args.security_group_ids:
sg_id = client.find_security_group( sg_id = client.find_security_group(
sg, ignore_missing=False).id sg, ignore_missing=False).id
tmp_secgroups.remove(sg_id) tmp_secgroups.remove(sg_id)
except ValueError: except ValueError:
msg = _("Port does not contain security group %s") % sg msg = _("Port does not contain security group %s") % sg
raise exceptions.CommandError(msg) raise exceptions.CommandError(msg)
attrs['security_groups'] = tmp_secgroups attrs['security_group_ids'] = tmp_secgroups
if parsed_args.allowed_address_pairs: if parsed_args.allowed_address_pairs:
try: try:
for addr in _convert_address_pairs(parsed_args): for addr in _convert_address_pairs(parsed_args):

15
openstackclient/tests/functional/network/v2/test_port.py
View File

@ -20,6 +20,7 @@ class PortTests(base.TestCase):
"""Functional tests for port. """ """Functional tests for port. """
NAME = uuid.uuid4().hex NAME = uuid.uuid4().hex
NETWORK_NAME = uuid.uuid4().hex NETWORK_NAME = uuid.uuid4().hex
SG_NAME = uuid.uuid4().hex
@classmethod @classmethod
def setUpClass(cls): def setUpClass(cls):
@ -124,13 +125,25 @@ class PortTests(base.TestCase):
self.assertEqual('xyzpdq', json_output.get('description')) self.assertEqual('xyzpdq', json_output.get('description'))
self.assertEqual('DOWN', json_output.get('admin_state_up')) 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) self.assertOutput('', raw_output)
json_output = json.loads(self.openstack( json_output = json.loads(self.openstack(
'port show -f json ' + self.NAME 'port show -f json ' + self.NAME
)) ))
sg_id = json_output.get('security_group_ids')
self.assertEqual(self.NAME, json_output.get('name')) self.assertEqual(self.NAME, json_output.get('name'))
self.assertEqual('xyzpdq', json_output.get('description')) self.assertEqual('xyzpdq', json_output.get('description'))
self.assertEqual('UP', json_output.get('admin_state_up')) self.assertEqual('UP', json_output.get('admin_state_up'))
self.assertIsNotNone(json_output.get('mac_address')) 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'))

4
openstackclient/tests/unit/network/v2/fakes.py
View File

@ -544,7 +544,7 @@ class FakePort(object):
'name': 'port-name-' + uuid.uuid4().hex, 'name': 'port-name-' + uuid.uuid4().hex,
'network_id': 'network-id-' + uuid.uuid4().hex, 'network_id': 'network-id-' + uuid.uuid4().hex,
'port_security_enabled': True, 'port_security_enabled': True,
'security_groups': [], 'security_group_ids': [],
'status': 'ACTIVE', 'status': 'ACTIVE',
'tenant_id': 'project-id-' + uuid.uuid4().hex, '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_admin_state_up = port_attrs['admin_state_up']
port.is_port_security_enabled = port_attrs['port_security_enabled'] port.is_port_security_enabled = port_attrs['port_security_enabled']
port.project_id = port_attrs['tenant_id'] 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 return port

44
openstackclient/tests/unit/network/v2/test_port.py
View File

@ -57,7 +57,7 @@ class TestPort(network_fakes.TestNetworkV2):
'network_id', 'network_id',
'port_security_enabled', 'port_security_enabled',
'project_id', 'project_id',
'security_groups', 'security_group_ids',
'status', 'status',
) )
@ -82,7 +82,7 @@ class TestPort(network_fakes.TestNetworkV2):
fake_port.network_id, fake_port.network_id,
fake_port.port_security_enabled, fake_port.port_security_enabled,
fake_port.project_id, fake_port.project_id,
utils.format_list(fake_port.security_groups), utils.format_list(fake_port.security_group_ids),
fake_port.status, fake_port.status,
) )
@ -251,7 +251,7 @@ class TestCreatePort(TestPort):
verifylist = [ verifylist = [
('network', self._port.network_id,), ('network', self._port.network_id,),
('enable', True), ('enable', True),
('security_groups', [secgroup.id]), ('security_group', [secgroup.id]),
('name', 'test-port'), ('name', 'test-port'),
] ]
parsed_args = self.check_parser(self.cmd, arglist, verifylist) parsed_args = self.check_parser(self.cmd, arglist, verifylist)
@ -261,7 +261,7 @@ class TestCreatePort(TestPort):
self.network.create_port.assert_called_once_with(**{ self.network.create_port.assert_called_once_with(**{
'admin_state_up': True, 'admin_state_up': True,
'network_id': self._port.network_id, 'network_id': self._port.network_id,
'security_groups': [secgroup.id], 'security_group_ids': [secgroup.id],
'name': 'test-port', 'name': 'test-port',
}) })
@ -309,7 +309,7 @@ class TestCreatePort(TestPort):
verifylist = [ verifylist = [
('network', self._port.network_id,), ('network', self._port.network_id,),
('enable', True), ('enable', True),
('security_groups', [sg_1.id, sg_2.id]), ('security_group', [sg_1.id, sg_2.id]),
('name', 'test-port'), ('name', 'test-port'),
] ]
parsed_args = self.check_parser(self.cmd, arglist, verifylist) parsed_args = self.check_parser(self.cmd, arglist, verifylist)
@ -319,7 +319,7 @@ class TestCreatePort(TestPort):
self.network.create_port.assert_called_once_with(**{ self.network.create_port.assert_called_once_with(**{
'admin_state_up': True, 'admin_state_up': True,
'network_id': self._port.network_id, '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', 'name': 'test-port',
}) })
@ -346,7 +346,7 @@ class TestCreatePort(TestPort):
self.network.create_port.assert_called_once_with(**{ self.network.create_port.assert_called_once_with(**{
'admin_state_up': True, 'admin_state_up': True,
'network_id': self._port.network_id, 'network_id': self._port.network_id,
'security_groups': [], 'security_group_ids': [],
'name': 'test-port', 'name': 'test-port',
}) })
@ -590,7 +590,7 @@ class TestListPort(TestPort):
prt.mac_address, prt.mac_address,
utils.format_list_of_dicts(prt.fixed_ips), utils.format_list_of_dicts(prt.fixed_ips),
prt.status, prt.status,
utils.format_list(prt.security_groups), utils.format_list(prt.security_group_ids),
prt.device_owner, prt.device_owner,
)) ))
@ -1111,7 +1111,7 @@ class TestSetPort(TestPort):
self._port.name, self._port.name,
] ]
verifylist = [ verifylist = [
('security_groups', [sg.id]), ('security_group', [sg.id]),
('port', self._port.name), ('port', self._port.name),
] ]
@ -1119,7 +1119,7 @@ class TestSetPort(TestPort):
result = self.cmd.take_action(parsed_args) result = self.cmd.take_action(parsed_args)
attrs = { attrs = {
'security_groups': [sg.id], 'security_group_ids': [sg.id],
} }
self.network.update_port.assert_called_once_with(self._port, **attrs) self.network.update_port.assert_called_once_with(self._port, **attrs)
self.assertIsNone(result) self.assertIsNone(result)
@ -1130,7 +1130,7 @@ class TestSetPort(TestPort):
sg_3 = network_fakes.FakeSecurityGroup.create_one_security_group() sg_3 = network_fakes.FakeSecurityGroup.create_one_security_group()
self.network.find_security_group = mock.Mock(side_effect=[sg_2, sg_3]) self.network.find_security_group = mock.Mock(side_effect=[sg_2, sg_3])
_testport = network_fakes.FakePort.create_one_port( _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) self.network.find_port = mock.Mock(return_value=_testport)
arglist = [ arglist = [
'--security-group', sg_2.id, '--security-group', sg_2.id,
@ -1138,13 +1138,13 @@ class TestSetPort(TestPort):
_testport.name, _testport.name,
] ]
verifylist = [ verifylist = [
('security_groups', [sg_2.id, sg_3.id]), ('security_group', [sg_2.id, sg_3.id]),
('port', _testport.name), ('port', _testport.name),
] ]
parsed_args = self.check_parser(self.cmd, arglist, verifylist) parsed_args = self.check_parser(self.cmd, arglist, verifylist)
result = self.cmd.take_action(parsed_args) result = self.cmd.take_action(parsed_args)
attrs = { 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.network.update_port.assert_called_once_with(_testport, **attrs)
self.assertIsNone(result) self.assertIsNone(result)
@ -1163,7 +1163,7 @@ class TestSetPort(TestPort):
result = self.cmd.take_action(parsed_args) result = self.cmd.take_action(parsed_args)
attrs = { attrs = {
'security_groups': [], 'security_group_ids': [],
} }
self.network.update_port.assert_called_once_with(self._port, **attrs) self.network.update_port.assert_called_once_with(self._port, **attrs)
self.assertIsNone(result) self.assertIsNone(result)
@ -1172,7 +1172,7 @@ class TestSetPort(TestPort):
sg1 = network_fakes.FakeSecurityGroup.create_one_security_group() sg1 = network_fakes.FakeSecurityGroup.create_one_security_group()
sg2 = network_fakes.FakeSecurityGroup.create_one_security_group() sg2 = network_fakes.FakeSecurityGroup.create_one_security_group()
_testport = network_fakes.FakePort.create_one_port( _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_port = mock.Mock(return_value=_testport)
self.network.find_security_group = mock.Mock(return_value=sg2) self.network.find_security_group = mock.Mock(return_value=sg2)
arglist = [ arglist = [
@ -1181,13 +1181,13 @@ class TestSetPort(TestPort):
_testport.name, _testport.name,
] ]
verifylist = [ verifylist = [
('security_groups', [sg2.id]), ('security_group', [sg2.id]),
('no_security_group', True) ('no_security_group', True)
] ]
parsed_args = self.check_parser(self.cmd, arglist, verifylist) parsed_args = self.check_parser(self.cmd, arglist, verifylist)
result = self.cmd.take_action(parsed_args) result = self.cmd.take_action(parsed_args)
attrs = { attrs = {
'security_groups': [sg2.id], 'security_group_ids': [sg2.id],
} }
self.network.update_port.assert_called_once_with(_testport, **attrs) self.network.update_port.assert_called_once_with(_testport, **attrs)
self.assertIsNone(result) self.assertIsNone(result)
@ -1434,7 +1434,7 @@ class TestUnsetPort(TestPort):
_fake_sg1 = network_fakes.FakeSecurityGroup.create_one_security_group() _fake_sg1 = network_fakes.FakeSecurityGroup.create_one_security_group()
_fake_sg2 = network_fakes.FakeSecurityGroup.create_one_security_group() _fake_sg2 = network_fakes.FakeSecurityGroup.create_one_security_group()
_fake_port = network_fakes.FakePort.create_one_port( _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_port = mock.Mock(return_value=_fake_port)
self.network.find_security_group = mock.Mock(return_value=_fake_sg2) self.network.find_security_group = mock.Mock(return_value=_fake_sg2)
arglist = [ arglist = [
@ -1442,14 +1442,14 @@ class TestUnsetPort(TestPort):
_fake_port.name, _fake_port.name,
] ]
verifylist = [ verifylist = [
('security_groups', [_fake_sg2.id]), ('security_group_ids', [_fake_sg2.id]),
] ]
parsed_args = self.check_parser(self.cmd, arglist, verifylist) parsed_args = self.check_parser(self.cmd, arglist, verifylist)
result = self.cmd.take_action(parsed_args) result = self.cmd.take_action(parsed_args)
attrs = { attrs = {
'security_groups': [_fake_sg1.id] 'security_group_ids': [_fake_sg1.id]
} }
self.network.update_port.assert_called_once_with( self.network.update_port.assert_called_once_with(
_fake_port, **attrs) _fake_port, **attrs)
@ -1459,14 +1459,14 @@ class TestUnsetPort(TestPort):
_fake_sg1 = network_fakes.FakeSecurityGroup.create_one_security_group() _fake_sg1 = network_fakes.FakeSecurityGroup.create_one_security_group()
_fake_sg2 = network_fakes.FakeSecurityGroup.create_one_security_group() _fake_sg2 = network_fakes.FakeSecurityGroup.create_one_security_group()
_fake_port = network_fakes.FakePort.create_one_port( _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) self.network.find_security_group = mock.Mock(return_value=_fake_sg2)
arglist = [ arglist = [
'--security-group', _fake_sg2.id, '--security-group', _fake_sg2.id,
_fake_port.name, _fake_port.name,
] ]
verifylist = [ verifylist = [
('security_groups', [_fake_sg2.id]), ('security_group_ids', [_fake_sg2.id]),
] ]
parsed_args = self.check_parser(self.cmd, arglist, verifylist) parsed_args = self.check_parser(self.cmd, arglist, verifylist)

6
releasenotes/notes/bug-1656788-2f5bda2205bc0329.yaml Normal file
View File

@ -0,0 +1,6 @@
---
fixes:
- |
Fixed the ``port set`` and ``port unset`` command failures
(AttributeError) when ``--security-group`` option is included.
[Bug `1656788 <https://bugs.launchpad.net/python-openstackclient/+bug/1656788>`_]