From 16c423fa175779c19ba93ce4a21597071dbb7194 Mon Sep 17 00:00:00 2001 From: Stephen Balukoff Date: Tue, 1 Mar 2016 22:14:57 -0800 Subject: [PATCH] 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 --- .../db/loadbalancer/loadbalancer_dbv2.py | 13 ++++---- neutron_lbaas/drivers/octavia/driver.py | 2 ++ .../db/loadbalancer/test_db_loadbalancerv2.py | 30 +++++++++++++++++-- .../drivers/octavia/test_octavia_driver.py | 2 ++ ...rsistence-update-fix-67591cbc90d7ad92.yaml | 23 ++++++++++++++ 5 files changed, 62 insertions(+), 8 deletions(-) create mode 100644 releasenotes/notes/lbaasv2-session-persistence-update-fix-67591cbc90d7ad92.yaml diff --git a/neutron_lbaas/db/loadbalancer/loadbalancer_dbv2.py b/neutron_lbaas/db/loadbalancer/loadbalancer_dbv2.py index e5949e85c..c254ec72e 100644 --- a/neutron_lbaas/db/loadbalancer/loadbalancer_dbv2.py +++ b/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') diff --git a/neutron_lbaas/drivers/octavia/driver.py b/neutron_lbaas/drivers/octavia/driver.py index cb629dc45..44f806cdb 100644 --- a/neutron_lbaas/drivers/octavia/driver.py +++ b/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 diff --git a/neutron_lbaas/tests/unit/db/loadbalancer/test_db_loadbalancerv2.py b/neutron_lbaas/tests/unit/db/loadbalancer/test_db_loadbalancerv2.py index 34c72643b..9da91dd63 100644 --- a/neutron_lbaas/tests/unit/db/loadbalancer/test_db_loadbalancerv2.py +++ b/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'] diff --git a/neutron_lbaas/tests/unit/drivers/octavia/test_octavia_driver.py b/neutron_lbaas/tests/unit/drivers/octavia/test_octavia_driver.py index 82a2126a1..701841e75 100644 --- a/neutron_lbaas/tests/unit/drivers/octavia/test_octavia_driver.py +++ b/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. diff --git a/releasenotes/notes/lbaasv2-session-persistence-update-fix-67591cbc90d7ad92.yaml b/releasenotes/notes/lbaasv2-session-persistence-update-fix-67591cbc90d7ad92.yaml new file mode 100644 index 000000000..f8738fa43 --- /dev/null +++ b/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.