From 3fc324fb54d4fc5b705e60a1d6642871ebb0b2f8 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Micha=C5=82=20Dulko?= <mdulko@redhat.com>
Date: Tue, 14 Jun 2022 17:21:56 +0200
Subject: [PATCH] Do not crash on Neutron quota exceptions

We shouldn't be failing on Neutron quota exceptions as Kuryr is not in a
position to ever solve that by restarting. Anyway because bulk create
method that we implemented raised a different exception, we failed to
ignore them for health checks.

This commit makes sure bulk create method raises ConflictException, so
that Retry handler will correctly handle it. Moreover the exception
handling there is improved to make sure we're reading error code instead
of error message.

Closes-Bug: 1978701
Change-Id: I520d415a0a8091737c9a3dd4d1268b3950421d79
---
 kuryr_kubernetes/clients.py                 | 19 +++++++++++++++----
 kuryr_kubernetes/handlers/retry.py          |  9 ++++-----
 kuryr_kubernetes/tests/unit/test_clients.py |  6 +++++-
 3 files changed, 24 insertions(+), 10 deletions(-)

diff --git a/kuryr_kubernetes/clients.py b/kuryr_kubernetes/clients.py
index e72666126..ada448681 100644
--- a/kuryr_kubernetes/clients.py
+++ b/kuryr_kubernetes/clients.py
@@ -98,10 +98,7 @@ def _create_ports(self, payload):
                 port[mapping] = port.pop(key)
 
     response = self.post(os_port.Port.base_path, json=payload)
-
-    if not response.ok:
-        raise os_exc.SDKException('Error when bulk creating ports: %s' %
-                                  response.text)
+    os_exc.raise_from_response(response)
     return (os_port.Port(**item) for item in response.json()['ports'])
 
 
@@ -133,6 +130,20 @@ def _delete_trunk_subports(self, trunk, subports):
     return trunk
 
 
+def get_neutron_error_type(ex):
+    try:
+        response = ex.response.json()
+    except (ValueError, AttributeError):
+        return None
+
+    if response:
+        try:
+            return response['NeutronError']['type']
+        except KeyError:
+            pass
+    return None
+
+
 def handle_neutron_errors(method, *args, **kwargs):
     """Handle errors on openstacksdk router methods"""
     result = method(*args, **kwargs)
diff --git a/kuryr_kubernetes/handlers/retry.py b/kuryr_kubernetes/handlers/retry.py
index 1ce123a87..33ea37bc4 100644
--- a/kuryr_kubernetes/handlers/retry.py
+++ b/kuryr_kubernetes/handlers/retry.py
@@ -101,13 +101,12 @@ class Retry(base.EventHandler):
                 with excutils.save_and_reraise_exception() as ex:
                     if self._sleep(deadline, attempt, ex.value):
                         ex.reraise = False
-            except os_exc.ConflictException as ex:
-                if ex.details.startswith('Quota exceeded for resources'):
-                    with excutils.save_and_reraise_exception() as ex:
+            except os_exc.ConflictException:
+                with excutils.save_and_reraise_exception() as ex:
+                    error_type = clients.get_neutron_error_type(ex.value)
+                    if error_type == 'OverQuota':
                         if self._sleep(deadline, attempt, ex.value):
                             ex.reraise = False
-                else:
-                    raise
             except self._exceptions:
                 with excutils.save_and_reraise_exception() as ex:
                     if self._sleep(deadline, attempt, ex.value):
diff --git a/kuryr_kubernetes/tests/unit/test_clients.py b/kuryr_kubernetes/tests/unit/test_clients.py
index 46f25a872..3e14e3ad1 100644
--- a/kuryr_kubernetes/tests/unit/test_clients.py
+++ b/kuryr_kubernetes/tests/unit/test_clients.py
@@ -62,6 +62,7 @@ class TestOpenStackSDKHack(test_base.TestCase):
     def test_create_no_ports(self):
         m_response = mock.Mock()
         m_response.json.return_value = {'ports': []}
+        m_response.status_code = 201
         m_post = mock.Mock()
         m_post.return_value = m_response
         m_osdk = mock.Mock()
@@ -75,6 +76,7 @@ class TestOpenStackSDKHack(test_base.TestCase):
     def test_create_ports(self):
         m_response = mock.Mock()
         m_response.json.return_value = {'ports': []}
+        m_response.status_code = 201
         m_post = mock.Mock()
         m_post.return_value = m_response
         m_osdk = mock.Mock()
@@ -136,6 +138,8 @@ class TestOpenStackSDKHack(test_base.TestCase):
                            '"Quota exceeded for resources: [\'port\'].", '
                            '"detail": ""}}')
         m_response.ok = False
+        m_response.status_code = 409
+        m_response.headers = {'content-type': 'application/json'}
         m_post = mock.Mock()
         m_post.return_value = m_response
         m_osdk = mock.Mock()
@@ -145,7 +149,7 @@ class TestOpenStackSDKHack(test_base.TestCase):
 
         try:
             clients._create_ports(m_osdk, payload)
-        except os_exc.SDKException as ex:
+        except os_exc.ConflictException as ex:
             # no additional params passed to the exception class
             self.assertIsNone(ex.extra_data)
             # no formatting placeholders in message