Merge "Correct floating IP extra attributes updating issues"

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