Fix LB disable/enable functionality

At the moment disabling/enabling Load Balancer created with the OVN
provider will act in reverse. This is due to reffering to actual state
of the Load Balancer rather then intended state while fetching list of
VIPs that should be defined in OVN.

A mistake was made when ovn_lb.external_ids was attempted to be
updated as object is protected and update simply do not pass.

Thus, we create a copy of it and pass to the _refresh_lb_vips instead.

Closes-Bug: #2086194
Change-Id: I6271079879d6bb724092a4693c7663034f4e2b9d

Author: Dmitriy Rabotyagov

ovn_octavia_provider/helper.py

@@ -1904,14 +1904,14 @@ def lb_update(self, loadbalancer):
                 if str(ovn_lb.external_ids['enabled']) != str(lb_enabled):
                     commands = []
                     enable_info = {'enabled': str(lb_enabled)}
-                    ovn_lb.external_ids['enabled'] = str(lb_enabled)
+                    ovn_lb_external_ids = ovn_lb.external_ids
+                    ovn_lb_external_ids.update(enable_info)
                     commands.append(
                         self.ovn_nbdb_api.db_set('Load_Balancer', ovn_lb.uuid,
                                                  ('external_ids', enable_info))
                     )
                     commands.extend(
-                        self._refresh_lb_vips(ovn_lb,
-                                              ovn_lb.external_ids))
+                        self._refresh_lb_vips(ovn_lb, ovn_lb_external_ids))
                     self._execute_commands(commands)
                 if lb_enabled:
                     operating_status = constants.ONLINE

ovn_octavia_provider/tests/unit/test_helper.py

@@ -1506,29 +1506,27 @@ def test_lb_update_disabled(self, refresh_vips):
     @mock.patch.object(ovn_helper.OvnProviderHelper, '_refresh_lb_vips')
     def test_lb_update_enabled(self, refresh_vips):
         # Change the mock, its enabled by default.
-        self.ovn_lb.external_ids.update({'enabled': False})
-        self.lb['admin_state_up'] = True
+        self.lb[constants.ADMIN_STATE_UP] = False
         status = self.helper.lb_update(self.lb)
         self.assertEqual(status['loadbalancers'][0]['provisioning_status'],
                          constants.ACTIVE)
         self.assertEqual(status['loadbalancers'][0]['operating_status'],
-                         constants.ONLINE)
+                         constants.OFFLINE)
         refresh_vips.assert_called_once_with(
             self.ovn_lb, self.ovn_lb.external_ids)
         self.helper.ovn_nbdb_api.db_set.assert_called_once_with(
             'Load_Balancer', self.ovn_lb.uuid,
-            ('external_ids', {'enabled': 'True'}))
+            ('external_ids', {'enabled': 'False'}))
         # update to re-enable
-        self.ovn_lb.external_ids.update({'enabled': True})
-        self.lb['admin_state_up'] = True
+        self.lb[constants.ADMIN_STATE_UP] = True
         status = self.helper.lb_update(self.lb)
         self.assertEqual(status['loadbalancers'][0]['provisioning_status'],
                          constants.ACTIVE)
         self.assertEqual(status['loadbalancers'][0]['operating_status'],
                          constants.ONLINE)
-        refresh_vips.assert_called_once_with(
+        refresh_vips.assert_called_with(
             self.ovn_lb, self.ovn_lb.external_ids)
-        self.helper.ovn_nbdb_api.db_set.assert_called_once_with(
+        self.helper.ovn_nbdb_api.db_set.assert_called_with(
             'Load_Balancer', self.ovn_lb.uuid,
             ('external_ids', {'enabled': 'True'}))
 

releasenotes/notes/lb_enable_disable-afdfe8f08dad533c.yaml

@@ -0,0 +1,6 @@
+---
+fixes:
+  - |
+    Enabling and disabling Load Balancers with the OVN provider now works
+    properly, assigning or removing VIP definitions in the OVN Northbound
+    database according to the issued command.