From 173f024b59a0d25cb66e25bd4f9ca3e62d8ad9b0 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)
(cherry picked from commit 3e6f180b77f1c58265ae9351151e32bb1d215fe7)
(cherry picked from commit 81cb9d5efccdeabe67b7a79d8de43fdbb66fdd4f)
---
 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 db8d2d0e00..41e1a1117d 100644
--- a/octavia/api/v2/controllers/pool.py
+++ b/octavia/api/v2/controllers/pool.py
@@ -457,7 +457,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 f136d7ad94..38d55f4ef6 100644
--- a/octavia/tests/functional/api/v2/test_pool.py
+++ b/octavia/tests/functional/api/v2/test_pool.py
@@ -2693,3 +2693,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.