From 3a42143ce99cc4484368b16dd41f95b93627dc38 Mon Sep 17 00:00:00 2001 From: LIU Yulong Date: Thu, 5 May 2016 14:52:26 +0800 Subject: [PATCH] Correct floating IP extra attributes updating issues Updating floating IP extra attributes, for instance description, will unexpectedly disassociate it. This behavior will interrupt the user's service traffic. And this is because that user can submit an empty request dict (without port_id parameter) for the floating IP updating API, and then it will be disassociated by default. So there is no way to update the floating IP extra attributes without changing it's association. This patch will make updating floating IP extra attributes API works properly. Closes-Bug: #1607746 Change-Id: I036e473118431856550249359a22445380ef9ece --- neutron/db/l3_db.py | 11 +++++++ .../tests/tempest/api/test_floating_ips.py | 30 +++++++++++++++++++ neutron/tests/tempest/api/test_revisions.py | 2 ++ neutron/tests/unit/db/test_l3_db.py | 18 +++++++++++ 4 files changed, 61 insertions(+) diff --git a/neutron/db/l3_db.py b/neutron/db/l3_db.py index 33ca8debe46..9f022e03ee2 100644 --- a/neutron/db/l3_db.py +++ b/neutron/db/l3_db.py @@ -1158,6 +1158,17 @@ class L3_NAT_dbonly_mixin(l3.RouterPluginBase, net_id=floatingip_db['floating_network_id']) except exc.NoResultFound: pass + + if fip and 'port_id' not in fip and floatingip_db.fixed_port_id: + # NOTE(liuyulong): without the fix of bug #1610045 here could + # also let floating IP can be dissociated with an empty + # updating dict. + fip['port_id'] = floatingip_db.fixed_port_id + port_id, internal_ip_address, router_id = self._get_assoc_data( + context, fip, floatingip_db) + + # After all upper conditions, if updating API dict is submitted with + # {'port_id': null}, then the floating IP cloud also be dissociated. return port_id, internal_ip_address, router_id def _update_fip_assoc(self, context, fip, floatingip_db, external_port): diff --git a/neutron/tests/tempest/api/test_floating_ips.py b/neutron/tests/tempest/api/test_floating_ips.py index 8ccdd44c8c2..bafa54c4b29 100644 --- a/neutron/tests/tempest/api/test_floating_ips.py +++ b/neutron/tests/tempest/api/test_floating_ips.py @@ -71,3 +71,33 @@ class FloatingIPTestJSON(base.BaseNetworkTest): self.assertEqual('d2', body['floatingip']['description']) body = self.client.show_floatingip(body['floatingip']['id']) self.assertEqual('d2', body['floatingip']['description']) + # disassociate + body = self.client.update_floatingip(body['floatingip']['id'], + port_id=None) + self.assertEqual('d2', body['floatingip']['description']) + + @test.idempotent_id('fd7161e1-2167-4686-a6ff-0f3df08001bb') + @test.requires_ext(extension="standard-attr-description", + service="network") + def test_floatingip_update_extra_attributes_port_id_not_changed(self): + port_id = self.ports[1]['id'] + body = self.client.create_floatingip( + floating_network_id=self.ext_net_id, + port_id=port_id, + description='d1' + )['floatingip'] + self.assertEqual('d1', body['description']) + body = self.client.show_floatingip(body['id'])['floatingip'] + self.assertEqual(port_id, body['port_id']) + # Update description + body = self.client.update_floatingip(body['id'], description='d2') + self.assertEqual('d2', body['floatingip']['description']) + # Floating IP association is not changed. + self.assertEqual(port_id, body['floatingip']['port_id']) + body = self.client.show_floatingip(body['floatingip']['id']) + self.assertEqual('d2', body['floatingip']['description']) + self.assertEqual(port_id, body['floatingip']['port_id']) + # disassociate + body = self.client.update_floatingip(body['floatingip']['id'], + port_id=None) + self.assertEqual(None, body['floatingip']['port_id']) diff --git a/neutron/tests/tempest/api/test_revisions.py b/neutron/tests/tempest/api/test_revisions.py index b45990f5e30..ec826359e83 100644 --- a/neutron/tests/tempest/api/test_revisions.py +++ b/neutron/tests/tempest/api/test_revisions.py @@ -153,3 +153,5 @@ class TestRevisions(base.BaseAdminNetworkTest, bsg.BaseSecGroupTest): b2 = self.client.update_floatingip(body['id'], description='d2') self.assertGreater(b2['floatingip']['revision_number'], body['revision_number']) + # disassociate + self.client.update_floatingip(b2['floatingip']['id'], port_id=None) diff --git a/neutron/tests/unit/db/test_l3_db.py b/neutron/tests/unit/db/test_l3_db.py index 262298f6d43..faa9658fdd7 100644 --- a/neutron/tests/unit/db/test_l3_db.py +++ b/neutron/tests/unit/db/test_l3_db.py @@ -209,6 +209,24 @@ class TestL3_NAT_dbonly_mixin(base.BaseTestCase): context=mock.ANY, subnetpool_id='fake_id') + def test__check_and_get_fip_assoc_with_extra_association_no_change(self): + fip = {'extra_key': 'value'} + context = mock.MagicMock() + floatingip_db = l3_db.FloatingIP( + id='fake_fip_id', + floating_network_id='fake_floating_network_id', + floating_ip_address='8.8.8.8', + fixed_port_id='fake_fixed_port_id', + floating_port_id='fake_floating_port_id') + with mock.patch.object( + l3_db.L3_NAT_dbonly_mixin, + '_get_assoc_data', + return_value=('1', '2', '3')) as mock_get_assoc_data: + self.db._check_and_get_fip_assoc(context, fip, floatingip_db) + context.session.query.assert_not_called() + mock_get_assoc_data.assert_called_once_with( + mock.ANY, fip, floatingip_db) + class L3_NAT_db_mixin(base.BaseTestCase): def setUp(self):