From 1d614443a7198642ef14467ab470ce1aa41f36b9 Mon Sep 17 00:00:00 2001 From: Hongbin Lu Date: Tue, 16 May 2017 20:54:25 +0000 Subject: [PATCH] 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 083bc89f99e007c400b9f89c63ac0459da910df8) --- nova/network/security_group/neutron_driver.py | 10 +++-- .../security_group/test_neutron_driver.py | 39 +++++++++++++++++++ 2 files changed, 46 insertions(+), 3 deletions(-) diff --git a/nova/network/security_group/neutron_driver.py b/nova/network/security_group/neutron_driver.py index 5b905f8f9fa1..ecc8e303b1d6 100644 --- a/nova/network/security_group/neutron_driver.py +++ b/nova/network/security_group/neutron_driver.py @@ -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: diff --git a/nova/tests/unit/network/security_group/test_neutron_driver.py b/nova/tests/unit/network/security_group/test_neutron_driver.py index b8c7cf4cf1c3..99a9f109d2a4 100644 --- a/nova/tests/unit/network/security_group/test_neutron_driver.py +++ b/nova/tests/unit/network/security_group/test_neutron_driver.py @@ -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):