openstack
/
nova
Code Issues Proposed changes

Handle exception on adding secgroup 

If user adds security group to an instance and the instance already
has that security group, neutron will return a 400 response. Nova
should handle the 400 response properly. In before, Nova doesn't
seem to handle this case and end-user gets a 500 response. This
commit fixed it.

Conflicts:
      nova/network/security_group/neutron_driver.py

NOTE(hongbin): The conflict is due to the removal of translation of
log messages.

Closes-Bug: #1691274
Change-Id: I58b19ef6b537d690df90e542b6af3c64773ecc87
(cherry picked from commit 083bc89f99)
This commit is contained in:
Hongbin Lu 2017-05-16 20:54:25 +00:00
parent 735f484ac8
commit 1d614443a7
2 changed files with 46 additions and 3 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

10
nova/network/security_group/neutron_driver.py
View File

@ -245,7 +245,6 @@ class SecurityGroupAPI(security_group_base.SecurityGroupBase):
LOG.exception(_LE("Neutron Error: %s"), e)
self.raise_invalid_property(six.text_type(e))
else:
LOG.exception(_LE("Neutron Error:"))
six.reraise(*exc_info)
converted_rules = []
for rule in rules:
@ -443,7 +442,6 @@ class SecurityGroupAPI(security_group_base.SecurityGroupBase):
'project': context.project_id})
self.raise_not_found(msg)
else:
LOG.exception(_LE("Neutron Error:"))
six.reraise(*exc_info)
params = {'device_id': instance.uuid}
try:
@ -476,6 +474,13 @@ class SecurityGroupAPI(security_group_base.SecurityGroupBase):
{'security_group_id': security_group_id,
'port_id': port['id']})
neutron.update_port(port['id'], {'port': updated_port})
except n_exc.NeutronClientException as e:
exc_info = sys.exc_info()
if e.status_code == 400:
raise exception.SecurityGroupCannotBeApplied(
six.text_type(e))
else:
six.reraise(*exc_info)
except Exception:
with excutils.save_and_reraise_exception():
LOG.exception(_LE("Neutron Error:"))
@ -497,7 +502,6 @@ class SecurityGroupAPI(security_group_base.SecurityGroupBase):
'project': context.project_id})
self.raise_not_found(msg)
else:
LOG.exception(_LE("Neutron Error:"))
six.reraise(*exc_info)
params = {'device_id': instance.uuid}
try:

39
nova/tests/unit/network/security_group/test_neutron_driver.py
View File

@ -395,6 +395,45 @@ class TestNeutronDriver(test.NoDBTestCase):
self.mocked_client.list_ports.assert_called_once_with(
device_id=[uuids.instance])
def test_add_to_instance(self):
sg_name = 'web_server'
sg_id = '85cc3048-abc3-43cc-89b3-377341426ac5'
port_id = 1
port_list = {'ports': [{'id': port_id, 'device_id': uuids.instance,
'fixed_ips': [{'ip_address': '10.0.0.1'}],
'port_security_enabled': True, 'security_groups': []}]}
self.mocked_client.list_ports.return_value = port_list
sg_api = neutron_driver.SecurityGroupAPI()
with mock.patch.object(neutronv20, 'find_resourceid_by_name_or_id',
return_value=sg_id):
sg_api.add_to_instance(
self.context, objects.Instance(uuid=uuids.instance), sg_name)
self.mocked_client.list_ports.assert_called_once_with(
device_id=uuids.instance)
self.mocked_client.update_port.assert_called_once_with(
port_id, {'port': {'security_groups': [sg_id]}})
def test_add_to_instance_with_bad_request(self):
sg_name = 'web_server'
sg_id = '85cc3048-abc3-43cc-89b3-377341426ac5'
port_id = 1
port_list = {'ports': [{'id': port_id, 'device_id': uuids.instance,
'fixed_ips': [{'ip_address': '10.0.0.1'}],
'port_security_enabled': True, 'security_groups': []}]}
self.mocked_client.list_ports.return_value = port_list
sg_api = neutron_driver.SecurityGroupAPI()
self.mocked_client.update_port.side_effect = (
n_exc.BadRequest(message='error'))
with mock.patch.object(neutronv20, 'find_resourceid_by_name_or_id',
return_value=sg_id):
self.assertRaises(exception.SecurityGroupCannotBeApplied,
sg_api.add_to_instance, self.context,
objects.Instance(uuid=uuids.instance), sg_name)
self.mocked_client.list_ports.assert_called_once_with(
device_id=uuids.instance)
self.mocked_client.update_port.assert_called_once_with(
port_id, {'port': {'security_groups': [sg_id]}})
class TestNeutronDriverWithoutMock(test.NoDBTestCase):