From 3e6f180b77f1c58265ae9351151e32bb1d215fe7 Mon Sep 17 00:00:00 2001
From: Gregory Thiemonge <gthiemon@redhat.com>
Date: Mon, 8 Aug 2022 18:14:45 +0200
Subject: [PATCH] Fix bug when rolling back prov and op status for some API
 calls

POST l7rule API call and PUT pool API call had a bug that might have
kept a load balancer in provisioning_status when the call was rejected
by the provider.

The code didn't use the right locked DB session to set the statuses and
then to rollback them when an exception was caught.

Story 2010210
Task 45947

Change-Id: I494ea357afc072360765d7a30a9488e08bfda52c
(cherry picked from commit fdcea400c6f591485fcb116cb47dc07c5229898d)
---
 octavia/api/v2/controllers/l7rule.py          |  2 +-
 octavia/api/v2/controllers/pool.py            |  2 +-
 .../tests/functional/api/v2/test_l7rule.py    | 16 ++++++++++++++
 octavia/tests/functional/api/v2/test_pool.py  | 21 +++++++++++++++++++
 ...E-on-provider-errors-40a03adc8ef82a54.yaml |  5 +++++
 5 files changed, 44 insertions(+), 2 deletions(-)
 create mode 100644 releasenotes/notes/fixed-PENDING_UPDATE-on-provider-errors-40a03adc8ef82a54.yaml

diff --git a/octavia/api/v2/controllers/l7rule.py b/octavia/api/v2/controllers/l7rule.py
index 56611d4541..678b60bce8 100644
--- a/octavia/api/v2/controllers/l7rule.py
+++ b/octavia/api/v2/controllers/l7rule.py
@@ -157,7 +157,7 @@ class L7RuleController(base.BaseController):
             l7rule_dict = db_prepare.create_l7rule(
                 l7rule.to_dict(render_unsets=True), self.l7policy_id)
 
-            self._test_lb_listener_policy_statuses(context.session)
+            self._test_lb_listener_policy_statuses(lock_session)
 
             db_l7rule = self._validate_create_l7rule(lock_session, l7rule_dict)
 
diff --git a/octavia/api/v2/controllers/pool.py b/octavia/api/v2/controllers/pool.py
index 1e38bb4f88..834b224cd7 100644
--- a/octavia/api/v2/controllers/pool.py
+++ b/octavia/api/v2/controllers/pool.py
@@ -460,7 +460,7 @@ class PoolsController(base.BaseController):
 
         with db_api.get_lock_session() as lock_session:
             self._test_lb_and_listener_statuses(
-                context.session, lb_id=db_pool.load_balancer_id,
+                lock_session, lb_id=db_pool.load_balancer_id,
                 listener_ids=self._get_affected_listener_ids(db_pool))
 
             # Prepare the data for the driver data model
diff --git a/octavia/tests/functional/api/v2/test_l7rule.py b/octavia/tests/functional/api/v2/test_l7rule.py
index 65bbd69164..89ab976987 100644
--- a/octavia/tests/functional/api/v2/test_l7rule.py
+++ b/octavia/tests/functional/api/v2/test_l7rule.py
@@ -1304,3 +1304,19 @@ class TestL7Rule(base.BaseAPITest):
         self.set_lb_status(self.lb_id, status=constants.DELETED)
         self.delete(self.l7rule_path.format(l7rule_id=l7rule.get('id')),
                     status=404)
+
+    @mock.patch("octavia.api.drivers.noop_driver.driver.NoopManager."
+                "l7rule_create")
+    def test_create_with_exception_in_provider_driver(self,
+                                                      l7rule_create_mock):
+        l7rule_create_mock.side_effect = Exception("Provider error")
+
+        self.create_l7rule(
+            self.l7policy_id, constants.L7RULE_TYPE_PATH,
+            constants.L7RULE_COMPARE_TYPE_STARTS_WITH,
+            '/api', status=500)
+
+        lb = self.get(self.LB_PATH.format(lb_id=self.lb_id)).json.get(
+            "loadbalancer")
+        self.assertEqual(lb[constants.PROVISIONING_STATUS],
+                         constants.ACTIVE)
diff --git a/octavia/tests/functional/api/v2/test_pool.py b/octavia/tests/functional/api/v2/test_pool.py
index 5259e2f252..45327f1405 100644
--- a/octavia/tests/functional/api/v2/test_pool.py
+++ b/octavia/tests/functional/api/v2/test_pool.py
@@ -2706,3 +2706,24 @@ class TestPool(base.BaseAPITest):
             'Invalid input for field/attribute alpn_protocols', fault)
         self.assertIn('Value should be a valid ALPN protocol ID', fault)
         self.assert_correct_status(lb_id=self.lb_id)
+
+    @mock.patch("octavia.api.drivers.noop_driver.driver.NoopManager."
+                "pool_update")
+    def test_update_with_exception_in_provider_driver(self, pool_update_mock):
+        pool_update_mock.side_effect = Exception("Provider error")
+
+        api_pool = self.create_pool(
+            self.lb_id,
+            constants.PROTOCOL_HTTP,
+            constants.LB_ALGORITHM_ROUND_ROBIN,
+            listener_id=self.listener_id).get(self.root_tag)
+        self.set_lb_status(lb_id=self.lb_id)
+
+        new_pool = {'name': 'foo'}
+        self.put(self.POOL_PATH.format(pool_id=api_pool.get('id')),
+                 self._build_body(new_pool), status=500)
+
+        lb = self.get(self.LB_PATH.format(lb_id=self.lb_id)).json.get(
+            "loadbalancer")
+        self.assertEqual(lb[constants.PROVISIONING_STATUS],
+                         constants.ACTIVE)
diff --git a/releasenotes/notes/fixed-PENDING_UPDATE-on-provider-errors-40a03adc8ef82a54.yaml b/releasenotes/notes/fixed-PENDING_UPDATE-on-provider-errors-40a03adc8ef82a54.yaml
new file mode 100644
index 0000000000..79cf904476
--- /dev/null
+++ b/releasenotes/notes/fixed-PENDING_UPDATE-on-provider-errors-40a03adc8ef82a54.yaml
@@ -0,0 +1,5 @@
+---
+fixes:
+  - |
+    Fix load balancers stuck in PENDING_UPDATE issues for some API calls (POST
+    /l7rule, PUT /pool) when a provider denied the call.