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

@@ -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):

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'])

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)

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):