From 88b661b0780ee534630c2d345ffd4545158db806 Mon Sep 17 00:00:00 2001
From: Rajesh Tailor <ratailor@redhat.com>
Date: Sat, 20 Apr 2024 15:37:50 +0530
Subject: [PATCH] Handle neutron-client conflict

When user tries to add stateless and stateful security
groups on same port, neutron raises SecurityGroupConflict (409),
but nova doesnot handle it and raises InternalServerError (500).

As it appears to be invalid operation from user, so user should get
the message that they are doing wrong.

This changes catches SecurityGroupConflict from neutron
client and raises newly added nova exception
SecurityGroupConnectionStateConflict with 409 error code.

Closes-Bug: #2056195
Change-Id: Ifad28fdd536ff0a4b30e786b2fcbc5a55987a13a
---
 nova/api/openstack/compute/security_groups.py |  3 +-
 nova/exception.py                             |  6 ++++
 nova/network/security_group_api.py            |  2 ++
 .../tests/unit/network/test_security_group.py | 32 +++++++++++++++++++
 4 files changed, 42 insertions(+), 1 deletion(-)

diff --git a/nova/api/openstack/compute/security_groups.py b/nova/api/openstack/compute/security_groups.py
index 6ee5ee528a95..37caf1b7469f 100644
--- a/nova/api/openstack/compute/security_groups.py
+++ b/nova/api/openstack/compute/security_groups.py
@@ -441,7 +441,8 @@ class SecurityGroupActionController(wsgi.Controller):
         except (exception.SecurityGroupNotFound,
                 exception.InstanceNotFound) as exp:
             raise exc.HTTPNotFound(explanation=exp.format_message())
-        except exception.NoUniqueMatch as exp:
+        except (exception.NoUniqueMatch,
+                exception.SecurityGroupConnectionStateConflict) as exp:
             raise exc.HTTPConflict(explanation=exp.format_message())
         except exception.SecurityGroupCannotBeApplied as exp:
             raise exc.HTTPBadRequest(explanation=exp.format_message())
diff --git a/nova/exception.py b/nova/exception.py
index 801033b33e7f..79246864212c 100644
--- a/nova/exception.py
+++ b/nova/exception.py
@@ -1056,6 +1056,12 @@ class SecurityGroupCannotBeApplied(Invalid):
                 " in order to apply security groups.")
 
 
+class SecurityGroupConnectionStateConflict(Invalid):
+    code = 409
+    msg_fmt = _("Cannot apply both stateful and stateless security groups on "
+                "the same port.")
+
+
 class NoUniqueMatch(NovaException):
     msg_fmt = _("No Unique Match Found.")
     code = 409
diff --git a/nova/network/security_group_api.py b/nova/network/security_group_api.py
index b36d96473da5..ab76ab8e67cd 100644
--- a/nova/network/security_group_api.py
+++ b/nova/network/security_group_api.py
@@ -649,6 +649,8 @@ def add_to_instance(context, instance, security_group_name):
         except n_exc.NeutronClientException as e:
             if e.status_code == 400:
                 raise exception.SecurityGroupCannotBeApplied(str(e))
+            elif e.status_code == 409:
+                raise exception.SecurityGroupConnectionStateConflict(str(e))
             else:
                 raise e
         except Exception:
diff --git a/nova/tests/unit/network/test_security_group.py b/nova/tests/unit/network/test_security_group.py
index a76dd4bf3c98..8c1a4a1eb4c4 100644
--- a/nova/tests/unit/network/test_security_group.py
+++ b/nova/tests/unit/network/test_security_group.py
@@ -373,6 +373,38 @@ class TestNeutronDriver(test.NoDBTestCase):
         self.mocked_client.update_port.assert_called_once_with(
             port_id, {'port': {'security_groups': [sg_id]}})
 
+    def test_add_to_instance_with_conflicting_sg(self):
+        sg1_name = 'sg-stateful'
+        sg1_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
+        with mock.patch.object(neutronv20, 'find_resourceid_by_name_or_id',
+                               return_value=sg1_id):
+            sg_api.add_to_instance(
+                self.context, objects.Instance(uuid=uuids.instance), sg1_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': [sg1_id]}})
+
+        sg2_name = 'sg-stateless'
+        sg2_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
+        self.mocked_client.update_port.side_effect = (
+            n_exc.Conflict(message='error'))
+        with mock.patch.object(neutronv20, 'find_resourceid_by_name_or_id',
+                               return_value=sg2_id):
+            self.assertRaises(exception.SecurityGroupConnectionStateConflict,
+                              sg_api.add_to_instance, self.context,
+                              objects.Instance(uuid=uuids.instance), sg2_name)
+
     def test_add_to_instance_duplicate_sg_name(self):
         sg_name = 'web_server'
         with mock.patch.object(neutronv20, 'find_resourceid_by_name_or_id',