From 0a48b7bfb639279c2cd5e37e590ec9773735ef3b 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

Conflicts:
	octavia/tests/functional/api/v2/test_pool.py

Change-Id: I494ea357afc072360765d7a30a9488e08bfda52c
(cherry picked from commit fdcea400c6f591485fcb116cb47dc07c5229898d)
(cherry picked from commit 3e6f180b77f1c58265ae9351151e32bb1d215fe7)
(cherry picked from commit 81cb9d5efccdeabe67b7a79d8de43fdbb66fdd4f)
(cherry picked from commit 173f024b59a0d25cb66e25bd4f9ca3e62d8ad9b0)
(cherry picked from commit ab39cc3881a9a05147e620df61d3865b43418ce5)
(cherry picked from commit 64abc34c7325c695e84d87bab3d187e83e2cbd5d)
---
 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 380957cd5b..908cacdb64 100644
--- a/octavia/api/v2/controllers/l7rule.py
+++ b/octavia/api/v2/controllers/l7rule.py
@@ -149,7 +149,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 e2565b7ea2..20f3c1e110 100644
--- a/octavia/api/v2/controllers/pool.py
+++ b/octavia/api/v2/controllers/pool.py
@@ -397,7 +397,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 cee15e5b1a..6b28967917 100644
--- a/octavia/tests/functional/api/v2/test_l7rule.py
+++ b/octavia/tests/functional/api/v2/test_l7rule.py
@@ -1293,3 +1293,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 94cc5cd8c9..f1fd133031 100644
--- a/octavia/tests/functional/api/v2/test_pool.py
+++ b/octavia/tests/functional/api/v2/test_pool.py
@@ -2427,3 +2427,24 @@ class TestPool(base.BaseAPITest):
                                     status=400, expect_errors=True)
                     self.assertEqual(expect_error_msg, res.json['faultstring'])
                     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.