Fix delete of session_persistence with pool update

Our LBaaSv2 database code was set to delete a pool's session_persistence
if the pool was updated and the update API request did not include
mention of the session_persistence. Expected behavior would be to not
change the session_persistence.

In addition, while editing this code it became obvious that we didn't
provide a good way to clear the session_persistence for a pool once it
was set, especially via the CLI. This has also been corrected.

Change-Id: I654b172927e1d96677a7da9e0846231b0ac48aa9
Depends-On: Idcf12e463fbaa3a61a211f13986d8472f52036d2
Closes-Bug: #1552086
Partial-Bug: #1547157

neutron_lbaas/db/loadbalancer/loadbalancer_dbv2.py

@@ -529,11 +529,14 @@ class LoadBalancerPluginDbv2(base_db.CommonDbMixin,
                         id=hmpools[0].id,
                         entity_in_use=models.HealthMonitorV2.NAME)
 
-            sp = pool.pop('session_persistence', None)
-            if sp:
-                self._update_pool_session_persistence(context, id, sp)
-            else:
-                self._delete_session_persistence(context, id)
+            # Only update or delete session persistence if it was part
+            # of the API request.
+            if 'session_persistence' in pool.keys():
+                sp = pool.pop('session_persistence')
+                if sp is None or sp == {}:
+                    self._delete_session_persistence(context, id)
+                else:
+                    self._update_pool_session_persistence(context, id, sp)
 
             # sqlalchemy cries if listeners is defined.
             listeners = pool.get('listeners')

neutron_lbaas/drivers/octavia/driver.py

@@ -306,6 +306,8 @@ class PoolManager(driver_base.BasePoolManager):
                 'type': pool.session_persistence.type,
                 'cookie_name': pool.session_persistence.cookie_name,
             }
+        else:
+            args['session_persistence'] = None
         if create:
             args['project_id'] = pool.tenant_id
             args['id'] = pool.id

neutron_lbaas/tests/unit/db/loadbalancer/test_db_loadbalancerv2.py

@@ -2420,9 +2420,10 @@ class LbaasPoolTests(PoolTestBase):
 
         with self.pool(listener_id=self.listener_id, **extras) as pool:
             pool_id = pool['pool'].get('id')
-            if 'session_persistence' in expected:
-                if not expected['session_persistence'].get('cookie_name'):
-                    expected['session_persistence']['cookie_name'] = None
+            if ('session_persistence' in expected.keys() and
+                    expected['session_persistence'] is not None and
+                    not expected['session_persistence'].get('cookie_name')):
+                expected['session_persistence']['cookie_name'] = None
             self.assertTrue(pool_id)
 
             actual = {}
@@ -2618,6 +2619,9 @@ class LbaasPoolTests(PoolTestBase):
     def test_create_pool_with_session_persistence(self):
         self.test_create_pool(session_persistence={'type': 'HTTP_COOKIE'})
 
+    def test_create_pool_with_session_persistence_none(self):
+        self.test_create_pool(session_persistence=None)
+
     def test_create_pool_with_session_persistence_with_app_cookie(self):
         sp = {'type': 'APP_COOKIE', 'cookie_name': 'sessionId'}
         self.test_create_pool(session_persistence=sp)
@@ -2672,6 +2676,26 @@ class LbaasPoolTests(PoolTestBase):
 
             self.assertIsNone(body['pool'].get('session_persistence'))
 
+    def test_update_no_change_session_persistence(self):
+        name = 'pool4'
+        sp = {'type': "HTTP_COOKIE"}
+
+        update_info = {'pool': {'lb_algorithm': 'ROUND_ROBIN'}}
+
+        with self.pool(name=name, session_persistence=sp,
+                       listener_id=self.listener_id) as pool:
+            pool_id = pool['pool']['id']
+            sp['cookie_name'] = None
+            # Ensure that pool has been created properly
+            self.assertEqual(pool['pool']['session_persistence'],
+                             sp)
+
+            # Try updating something other than session_persistence
+            resp, body = self._update_pool_api(pool_id, update_info)
+            # Make sure session_persistence is unchanged
+            self.assertEqual(pool['pool']['session_persistence'],
+                             sp)
+
     def test_update_pool_with_protocol(self):
         with self.pool(listener_id=self.listener_id) as pool:
             pool_id = pool['pool']['id']

neutron_lbaas/tests/unit/drivers/octavia/test_octavia_driver.py

@@ -207,6 +207,8 @@ class TestOctaviaDriver(BaseOctaviaDriverTest):
                 'type': pool.session_persistence.type,
                 'cookie_name': pool.session_persistence.cookie_name,
             }
+        else:
+            args['session_persistence'] = None
         m.create(pool, pool_url, args)
 
         # Test update pool.

releasenotes/notes/lbaasv2-session-persistence-update-fix-67591cbc90d7ad92.yaml

@@ -0,0 +1,23 @@
+---
+issues:
+  - |
+    CLI update does not allow clearing session_persistence.
+
+    In the process of fixing the session_persistence
+    update bug, we discovered that the CLI for updating
+    LBaaS v2 pools does not allow one to clear the
+    session_persistence for a pool once set. A fix for
+    this is being prepared, but in the mean time, the
+    following work-arounds are possible if a given
+    pool's session_persistence parameter needs to be
+    changed:
+
+    * The pool can be deleted and recreated without
+      session_persistence.
+    * A tenant can update the session persistence by
+      talking directly to the API using curl.
+fixes:
+  - |
+    session_persistence on a LBaaSv2 pool is no longer
+    deleted when other pool parameters are updated via
+    the CLI or API.