From 503054d4a35b14fd441212b63a30070ff7e24bb3 Mon Sep 17 00:00:00 2001
From: Michael Johnson <johnsomor@gmail.com>
Date: Wed, 29 Apr 2020 17:33:16 -0700
Subject: [PATCH] Fix queens listener update with invalid TLS ref

On queens, updating a listener with an invalid TLS ref can lead to
the load balancer never unlocking as the transaction does not roll
back.
This is already fixed in Rocky and newer as part of the provider
driver feature.

Change-Id: I923318f7d33090543a028d7c3a20f11dab8d247e
Story: 2007608
Task: 39598
---
 octavia/api/v2/controllers/listener.py        | 40 ++++++++++---------
 .../tests/functional/api/v2/test_listener.py  |  2 +
 ...r-update-bad-tls-ref-b82190033661af12.yaml |  5 +++
 3 files changed, 28 insertions(+), 19 deletions(-)
 create mode 100644 releasenotes/notes/fix-listener-update-bad-tls-ref-b82190033661af12.yaml

diff --git a/octavia/api/v2/controllers/listener.py b/octavia/api/v2/controllers/listener.py
index 1efbb9873f..6560179940 100644
--- a/octavia/api/v2/controllers/listener.py
+++ b/octavia/api/v2/controllers/listener.py
@@ -330,27 +330,29 @@ class ListenersController(base.BaseController):
         if listener.default_pool_id:
             self._validate_pool(context.session, load_balancer_id,
                                 listener.default_pool_id, db_listener.protocol)
-        self._test_lb_and_listener_statuses(context.session, load_balancer_id,
-                                            id=id)
 
-        sni_containers = listener.sni_container_refs or []
-        tls_refs = [sni for sni in sni_containers]
-        if listener.default_tls_container_ref:
-            tls_refs.append(listener.default_tls_container_ref)
-        self._validate_tls_refs(tls_refs)
+        with db_api.get_lock_session() as lock_session:
+            self._test_lb_and_listener_statuses(lock_session, load_balancer_id,
+                                                id=id)
+
+            sni_containers = listener.sni_container_refs or []
+            tls_refs = [sni for sni in sni_containers]
+            if listener.default_tls_container_ref:
+                tls_refs.append(listener.default_tls_container_ref)
+            self._validate_tls_refs(tls_refs)
+
+            try:
+                LOG.info("Sending Update of Listener %s to handler", id)
+                self.handler.update(db_listener, listener)
+            except Exception:
+                with excutils.save_and_reraise_exception(reraise=False):
+                    self._reset_lb_status(
+                        lock_session, lb_id=db_listener.load_balancer_id)
+                    # Listener now goes to ERROR
+                    self.repositories.listener.update(
+                        lock_session, db_listener.id,
+                        provisioning_status=constants.ERROR)
 
-        try:
-            LOG.info("Sending Update of Listener %s to handler", id)
-            self.handler.update(db_listener, listener)
-        except Exception:
-            with excutils.save_and_reraise_exception(
-                    reraise=False), db_api.get_lock_session() as lock_session:
-                self._reset_lb_status(
-                    lock_session, lb_id=db_listener.load_balancer_id)
-                # Listener now goes to ERROR
-                self.repositories.listener.update(
-                    lock_session, db_listener.id,
-                    provisioning_status=constants.ERROR)
         db_listener = self._get_db_listener(context.session, id)
         result = self._convert_db_to_type(db_listener,
                                           listener_types.ListenerResponse)
diff --git a/octavia/tests/functional/api/v2/test_listener.py b/octavia/tests/functional/api/v2/test_listener.py
index 77d0578006..9b346e2557 100644
--- a/octavia/tests/functional/api/v2/test_listener.py
+++ b/octavia/tests/functional/api/v2/test_listener.py
@@ -1076,6 +1076,8 @@ class TestListener(base.BaseAPITest):
         self.assertIn(sni1, response['faultstring'])
         self.assertNotIn(sni2, response['faultstring'])
         self.assertNotIn(tls_ref, response['faultstring'])
+        self.assert_correct_lb_status(self.lb_id, constants.ONLINE,
+                                      constants.ACTIVE)
 
     def test_update_pending_update(self):
         lb = self.create_load_balancer(uuidutils.generate_uuid())
diff --git a/releasenotes/notes/fix-listener-update-bad-tls-ref-b82190033661af12.yaml b/releasenotes/notes/fix-listener-update-bad-tls-ref-b82190033661af12.yaml
new file mode 100644
index 0000000000..c0f6e749c6
--- /dev/null
+++ b/releasenotes/notes/fix-listener-update-bad-tls-ref-b82190033661af12.yaml
@@ -0,0 +1,5 @@
+---
+fixes:
+  - |
+    Fixed updating listeners with a bad TLS reference locking the load
+    balancer.