Fix FloatingIP.save() passing FixedIP object to sqlalchemy

This prevents the FloatingIP.save() method from passing the
calculated FixedIP object to the sqlalchemy floating_ip_update()
function, which would expect it to be an SA object. It also
aborts any attempt to save the object with a modified fixed_ip_id
linkage, as associate/disassociate should be used for that.

This also fixes a bug where FloatingIP expects the result of
floating_ip_update() to be a FloatingIp SA object.

Change-Id: I065caedf4d81c8583a3b390934a1d403cf2e87bd
Closes-bug: #1334164

nova/db/sqlalchemy/api.py

@@ -1050,6 +1050,7 @@ def floating_ip_update(context, address, values):
             float_ip_ref.save(session=session)
         except db_exc.DBDuplicateEntry:
             raise exception.FloatingIpExists(address=values['address'])
+        return float_ip_ref
 
 
 def _dnsdomain_get(context, session, fqdomain):

nova/objects/floating_ip.py

@@ -136,6 +136,14 @@ class FloatingIP(obj_base.NovaPersistentObject, obj_base.NovaObject):
         if 'address' in updates:
             raise exception.ObjectActionError(action='save',
                                               reason='address is not mutable')
+        if 'fixed_ip_id' in updates:
+            reason = 'fixed_ip_id is not mutable'
+            raise exception.ObjectActionError(action='save', reason=reason)
+
+        # NOTE(danms): Make sure we don't pass the calculated fixed_ip
+        # relationship to the DB update method
+        updates.pop('fixed_ip', None)
+
         db_floatingip = db.floating_ip_update(context, str(self.address),
                                               updates)
         self._from_db_object(context, self, db_floatingip)

nova/tests/db/test_db_api.py

@@ -4092,7 +4092,9 @@ class FloatingIpTestCase(test.TestCase, ModelsObjectComparatorMixin):
             'interface': 'some_interface',
             'pool': 'some_pool'
         }
-        db.floating_ip_update(self.ctxt, float_ip['address'], values)
+        floating_ref = db.floating_ip_update(self.ctxt, float_ip['address'],
+                                             values)
+        self.assertIsNot(floating_ref, None)
         updated_float_ip = db.floating_ip_get(self.ctxt, float_ip['id'])
         self._assertEqualObjects(updated_float_ip, values,
                                  ignored_keys=['id', 'address', 'updated_at',

nova/tests/objects/test_floating_ip.py

@@ -130,13 +130,32 @@ class _TestFloatingIPObject(object):
         floatingip = floating_ip.FloatingIP(context=self.context,
                                             id=123, address='1.2.3.4',
                                             host='foo')
-        self.assertRaises(exception.ObjectActionError, floatingip.save)
         floatingip.obj_reset_changes(['address', 'id'])
         floatingip.save()
         self.assertEqual(set(), floatingip.obj_what_changed())
         update.assert_called_with(self.context, '1.2.3.4',
                                   {'host': 'foo'})
 
+    def test_save_errors(self):
+        floatingip = floating_ip.FloatingIP(context=self.context,
+                                            id=123, host='foo')
+        floatingip.obj_reset_changes()
+        floating_ip.address = '1.2.3.4'
+        self.assertRaises(exception.ObjectActionError, floatingip.save)
+
+        floatingip.obj_reset_changes()
+        floatingip.fixed_ip_id = 1
+        self.assertRaises(exception.ObjectActionError, floatingip.save)
+
+    @mock.patch('nova.db.floating_ip_update')
+    def test_save_no_fixedip(self, update):
+        update.return_value = fake_floating_ip
+        floatingip = floating_ip.FloatingIP(context=self.context,
+                                            id=123)
+        floatingip.fixed_ip = objects.FixedIP(context=self.context,
+                                              id=456)
+        self.assertNotIn('fixed_ip', update.calls[1])
+
     @mock.patch('nova.db.floating_ip_get_all')
     def test_get_all(self, get):
         get.return_value = [fake_floating_ip]