From c1e86e369987158ea4fa34a3dee85dc8c214a6ab 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
(cherry picked from commit 88b661b0780ee534630c2d345ffd4545158db806)
---
 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 b10308393f43..f1c6d9aedb1d 100644
--- a/nova/api/openstack/compute/security_groups.py
+++ b/nova/api/openstack/compute/security_groups.py
@@ -439,7 +439,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 20070efcc5b4..e59b8e3feba0 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',