Set vif and allocated when associating fixed ip

Follow on to changes I84fa2f0926f719bff33ac43b39e161a4eb412f44 and
I8cf5483982085da57ee470fa2753b0d0aebc12b3.

This change removes a race when associating fixed IPs and pools.
Originally, the fixed IPs were associated, and
"virtual_interface_id" was updated and saved to the DB at a later time.
This value is now sent into associate(_pool) and are tunneled down
to the DB on associate, so now the value is saved to the DB at
association.

Change-Id: Ic37d96deba3a5ef51594733cc16091173a9655a4
Closes-Bug: #1249065
This commit is contained in:
Ryan Rossiter 2015-06-29 13:19:07 +00:00
parent b519516cad
commit 73a68abb28
9 changed files with 115 additions and 67 deletions

View File

@ -488,25 +488,26 @@ def migration_get_all_by_filters(context, filters):
def fixed_ip_associate(context, address, instance_uuid, network_id=None,
reserved=False):
reserved=False, virtual_interface_id=None):
"""Associate fixed ip to instance.
Raises if fixed ip is not available.
"""
return IMPL.fixed_ip_associate(context, address, instance_uuid, network_id,
reserved)
reserved, virtual_interface_id)
def fixed_ip_associate_pool(context, network_id, instance_uuid=None,
host=None):
host=None, virtual_interface_id=None):
"""Find free ip in network and associate it to instance or host.
Raises if one is not available.
"""
return IMPL.fixed_ip_associate_pool(context, network_id,
instance_uuid, host)
instance_uuid, host,
virtual_interface_id)
def fixed_ip_create(context, values):

View File

@ -1085,7 +1085,7 @@ def dnsdomain_get_all(context):
@oslo_db_api.wrap_db_retry(max_retries=5, retry_on_deadlock=True,
retry_on_request=True)
def fixed_ip_associate(context, address, instance_uuid, network_id=None,
reserved=False):
reserved=False, virtual_interface_id=None):
"""Keyword arguments:
reserved -- should be a boolean value(True or False), exact value will be
used to filter on the fixed ip address
@ -1111,9 +1111,12 @@ def fixed_ip_associate(context, address, instance_uuid, network_id=None,
raise exception.FixedIpAlreadyInUse(address=address,
instance_uuid=instance_uuid)
params = {'instance_uuid': instance_uuid}
params = {'instance_uuid': instance_uuid,
'allocated': virtual_interface_id is not None}
if not fixed_ip_ref.network_id:
params['network_id'] = network_id
if virtual_interface_id:
params['virtual_interface_id'] = virtual_interface_id
rows_updated = model_query(context, models.FixedIp, session=session,
read_deleted="no").\
@ -1135,7 +1138,7 @@ def fixed_ip_associate(context, address, instance_uuid, network_id=None,
@oslo_db_api.wrap_db_retry(max_retries=5, retry_on_deadlock=True,
retry_on_request=True)
def fixed_ip_associate_pool(context, network_id, instance_uuid=None,
host=None):
host=None, virtual_interface_id=None):
if instance_uuid and not uuidutils.is_uuid_like(instance_uuid):
raise exception.InvalidUUID(uuid=instance_uuid)
@ -1154,13 +1157,15 @@ def fixed_ip_associate_pool(context, network_id, instance_uuid=None,
if not fixed_ip_ref:
raise exception.NoMoreFixedIps(net=network_id)
params = {}
params = {'allocated': virtual_interface_id is not None}
if fixed_ip_ref['network_id'] is None:
params['network_id'] = network_id
if instance_uuid:
params['instance_uuid'] = instance_uuid
if host:
params['host'] = host
if virtual_interface_id:
params['virtual_interface_id'] = virtual_interface_id
rows_updated = model_query(context, models.FixedIp, session=session,
read_deleted="no").\

View File

@ -910,10 +910,9 @@ class NetworkManager(manager.Manager):
{'address': address, 'network': network['id'],
'cidr': network['cidr']},
instance=instance)
fip = objects.FixedIP.associate(context,
str(address),
instance_id,
network['id'])
fip = objects.FixedIP.associate(
context, str(address), instance_id, network['id'],
vif_id=vif.id)
else:
LOG.debug('Associating instance with fixed IP from pool '
'in network %(network)s on subnet %(cidr)s.' %
@ -921,14 +920,12 @@ class NetworkManager(manager.Manager):
'cidr': network['cidr']},
instance=instance)
fip = objects.FixedIP.associate_pool(
context.elevated(), network['id'], instance_id)
context.elevated(), network['id'], instance_id,
vif_id=vif.id)
LOG.debug('Associated instance with fixed IP: %s', fip,
instance=instance)
address = str(fip.address)
fip.allocated = True
fip.virtual_interface_id = vif.id
fip.save()
cleanup.append(functools.partial(fip.disassociate, context))
LOG.debug('Refreshing security group members for instance.',
@ -1998,23 +1995,21 @@ class VlanManager(RPCAllocateFixedIP, floating_ips.FloatingIP, NetworkManager):
address = network['vpn_private_address']
fip = objects.FixedIP.associate(context, str(address),
instance_id, network['id'],
reserved=True)
reserved=True,
vif_id=vif.id)
else:
address = kwargs.get('address', None)
if address:
fip = objects.FixedIP.associate(context, str(address),
instance_id,
network['id'])
network['id'],
vif_id=vif.id)
else:
fip = objects.FixedIP.associate_pool(context,
network['id'],
instance_id)
fip = objects.FixedIP.associate_pool(
context, network['id'], instance_id,
vif_id=vif.id)
address = fip.address
fip.allocated = True
fip.virtual_interface_id = vif.id
fip.save()
if not kwargs.get('vpn', None):
self._do_trigger_security_group_members_refresh_for_instance(
instance_id)

View File

@ -44,7 +44,8 @@ class FixedIP(obj_base.NovaPersistentObject, obj_base.NovaObject,
# Version 1.11: Instance 1.21
# Version 1.12: Instance 1.22, FloatingIPList 1.9
# Version 1.13: Instance 1.23, FloatingIPList 1.10
VERSION = '1.13'
# Version 1.14: Added vif_id kwarg to associate(_pool), FloatingIPList 1.11
VERSION = '1.14'
fields = {
'id': fields.IntegerField(),
@ -75,7 +76,7 @@ class FixedIP(obj_base.NovaPersistentObject, obj_base.NovaObject,
'network': [('1.0', '1.2')],
'virtual_interface': [('1.1', '1.0')],
'floating_ips': [('1.5', '1.7'), ('1.11', '1.8'), ('1.12', '1.9'),
('1.13', '1.10')],
('1.13', '1.10'), ('1.14', '1.11')],
}
def obj_make_compatible(self, primitive, target_version):
@ -153,18 +154,20 @@ class FixedIP(obj_base.NovaPersistentObject, obj_base.NovaObject,
@obj_base.remotable_classmethod
def associate(cls, context, address, instance_uuid, network_id=None,
reserved=False):
reserved=False, vif_id=None):
db_fixedip = db.fixed_ip_associate(context, address, instance_uuid,
network_id=network_id,
reserved=reserved)
reserved=reserved,
virtual_interface_id=vif_id)
return cls._from_db_object(context, cls(context), db_fixedip)
@obj_base.remotable_classmethod
def associate_pool(cls, context, network_id, instance_uuid=None,
host=None):
host=None, vif_id=None):
db_fixedip = db.fixed_ip_associate_pool(context, network_id,
instance_uuid=instance_uuid,
host=host)
host=host,
virtual_interface_id=vif_id)
return cls._from_db_object(context, cls(context), db_fixedip)
@obj_base.remotable_classmethod
@ -225,7 +228,8 @@ class FixedIPList(obj_base.ObjectListBase, obj_base.NovaObject):
# Version 1.11: FixedIP <= version 1.11
# Version 1.12: FixedIP <= version 1.12
# Version 1.13: FixedIP <= version 1.13
VERSION = '1.13'
# Version 1.14: FixedIP <= version 1.14
VERSION = '1.14'
fields = {
'objects': fields.ListOfObjectsField('FixedIP'),
@ -235,7 +239,7 @@ class FixedIPList(obj_base.ObjectListBase, obj_base.NovaObject):
('1.3', '1.3'), ('1.4', '1.4'), ('1.5', '1.5'),
('1.6', '1.6'), ('1.7', '1.7'), ('1.8', '1.8'),
('1.9', '1.9'), ('1.10', '1.10'), ('1.11', '1.11'),
('1.12', '1.12'), ('1.13', '1.13')],
('1.12', '1.12'), ('1.13', '1.13'), ('1.14', '1.14')],
}
@obj_base.remotable_classmethod

View File

@ -35,7 +35,8 @@ class FloatingIP(obj_base.NovaPersistentObject, obj_base.NovaObject,
# Version 1.7: FixedIP <= version 1.11
# Version 1.8: FixedIP <= version 1.12
# Version 1.9: FixedIP <= version 1.13
VERSION = '1.9'
# Version 1.10: FixedIP <= version 1.14
VERSION = '1.10'
fields = {
'id': fields.IntegerField(),
'address': fields.IPAddressField(),
@ -51,7 +52,8 @@ class FloatingIP(obj_base.NovaPersistentObject, obj_base.NovaObject,
obj_relationships = {
'fixed_ip': [('1.0', '1.1'), ('1.2', '1.2'), ('1.3', '1.3'),
('1.4', '1.4'), ('1.5', '1.5'), ('1.6', '1.6'),
('1.7', '1.11'), ('1.8', '1.12'), ('1.9', '1.13')],
('1.7', '1.11'), ('1.8', '1.12'), ('1.9', '1.13'),
('1.10', '1.14')],
}
@staticmethod
@ -177,6 +179,7 @@ class FloatingIPList(obj_base.ObjectListBase, obj_base.NovaObject):
# Version 1.8: FloatingIP 1.7
# Version 1.9: FloatingIP 1.8
# Version 1.10: FloatingIP 1.9
# Version 1.11: FloatingIP 1.10
fields = {
'objects': fields.ListOfObjectsField('FloatingIP'),
}
@ -184,9 +187,9 @@ class FloatingIPList(obj_base.ObjectListBase, obj_base.NovaObject):
'objects': [('1.0', '1.0'), ('1.1', '1.1'), ('1.2', '1.1'),
('1.3', '1.2'), ('1.4', '1.3'), ('1.5', '1.4'),
('1.6', '1.5'), ('1.7', '1.6'), ('1.8', '1.7'),
('1.9', '1.8'), ('1.10', '1.9')],
('1.9', '1.8'), ('1.10', '1.9'), ('1.11', '1.10')],
}
VERSION = '1.10'
VERSION = '1.11'
@obj_base.remotable_classmethod
def get_all(cls, context):

View File

@ -4532,6 +4532,28 @@ class FixedIPTestCase(BaseInstanceTypeTestCase):
self.ctxt, address, instance_uuid)
self.assertEqual(1, mock_first.call_count)
def test_fixed_ip_associate_with_vif(self):
instance_uuid = self._create_instance()
network = db.network_create_safe(self.ctxt, {})
vif = db.virtual_interface_create(self.ctxt, {})
address = self.create_fixed_ip()
fixed_ip = db.fixed_ip_associate(self.ctxt, address, instance_uuid,
network_id=network['id'],
virtual_interface_id=vif['id'])
self.assertTrue(fixed_ip['allocated'])
self.assertEqual(vif['id'], fixed_ip['virtual_interface_id'])
def test_fixed_ip_associate_not_allocated_without_vif(self):
instance_uuid = self._create_instance()
address = self.create_fixed_ip()
fixed_ip = db.fixed_ip_associate(self.ctxt, address, instance_uuid)
self.assertFalse(fixed_ip['allocated'])
self.assertIsNone(fixed_ip['virtual_interface_id'])
def test_fixed_ip_associate_pool_invalid_uuid(self):
instance_uuid = '123'
self.assertRaises(exception.InvalidUUID, db.fixed_ip_associate_pool,
@ -6143,7 +6165,7 @@ class NetworkTestCase(test.TestCase, ModelsObjectComparatorMixin):
'network_id': network.id, 'allocated': True,
'virtual_interface_id': virtual_interface.id})
db.fixed_ip_associate(self.ctxt, ip, instance.uuid,
network.id)
network.id, virtual_interface_id=virtual_interface['id'])
return network, instance
def test_network_get_associated_default_route(self):

View File

@ -683,7 +683,8 @@ class FlatNetworkTestCase(test.TestCase):
mock_associate.assert_called_once_with(self.context,
'1.2.3.4',
instance.uuid,
1)
1,
vif_id=1)
@mock.patch('nova.objects.instance.Instance.get_by_uuid')
@mock.patch('nova.objects.virtual_interface.VirtualInterface'
@ -736,11 +737,9 @@ class FlatNetworkTestCase(test.TestCase):
'.get_by_instance_and_network')
@mock.patch('nova.objects.fixed_ip.FixedIP.disassociate')
@mock.patch('nova.objects.fixed_ip.FixedIP.associate_pool')
@mock.patch('nova.objects.fixed_ip.FixedIP.save')
@mock.patch('nova.network.manager.NetworkManager._add_virtual_interface')
def test_allocate_fixed_ip_create_new_vifs(self,
mock_add,
mock_fixedip_save,
mock_fixedip_associate,
mock_fixedip_disassociate,
mock_vif_get,
@ -749,7 +748,7 @@ class FlatNetworkTestCase(test.TestCase):
fip = objects.FixedIP(instance_uuid='fake-uuid',
address=address,
virtual_interface_id=1)
virtual_interface_id=1000)
net = {'cidr': '24', 'id': 1, 'uuid': 'nosuch'}
instance = objects.Instance(context=self.context)
instance.create()
@ -881,10 +880,9 @@ class VlanNetworkTestCase(test.TestCase):
mox.IgnoreArg(),
mox.IgnoreArg(),
network_id=mox.IgnoreArg(),
reserved=True).AndReturn(fixed)
db.fixed_ip_update(mox.IgnoreArg(),
mox.IgnoreArg(),
mox.IgnoreArg())
reserved=True,
virtual_interface_id=vifs[0]['id']
).AndReturn(fixed)
db.virtual_interface_get_by_instance_and_network(mox.IgnoreArg(),
mox.IgnoreArg(), mox.IgnoreArg()).AndReturn(vifs[0])
db.instance_get_by_uuid(mox.IgnoreArg(),
@ -907,7 +905,6 @@ class VlanNetworkTestCase(test.TestCase):
'_do_trigger_security_group_members_refresh_for_instance',
lambda *a, **kw: None)
self.mox.StubOutWithMock(db, 'fixed_ip_associate_pool')
self.mox.StubOutWithMock(db, 'fixed_ip_update')
self.mox.StubOutWithMock(db,
'virtual_interface_get_by_instance_and_network')
self.mox.StubOutWithMock(db, 'instance_get_by_uuid')
@ -917,10 +914,9 @@ class VlanNetworkTestCase(test.TestCase):
db.fixed_ip_associate_pool(mox.IgnoreArg(),
mox.IgnoreArg(),
instance_uuid=mox.IgnoreArg(),
host=None).AndReturn(fixed)
db.fixed_ip_update(mox.IgnoreArg(),
mox.IgnoreArg(),
mox.IgnoreArg())
host=None,
virtual_interface_id=vifs[0]['id']
).AndReturn(fixed)
db.virtual_interface_get_by_instance_and_network(mox.IgnoreArg(),
mox.IgnoreArg(), mox.IgnoreArg()).AndReturn(vifs[0])
db.instance_get_by_uuid(mox.IgnoreArg(),
@ -945,9 +941,8 @@ class VlanNetworkTestCase(test.TestCase):
@mock.patch('nova.network.manager.VlanManager._add_virtual_interface')
@mock.patch('nova.objects.instance.Instance.get_by_uuid')
@mock.patch('nova.objects.fixed_ip.FixedIP.associate')
@mock.patch('nova.objects.fixed_ip.FixedIP.save')
@mock.patch('nova.objects.VirtualInterface.get_by_instance_and_network')
def test_allocate_fixed_ip_return_none(self, mock_get, mock_save,
def test_allocate_fixed_ip_return_none(self, mock_get,
mock_associate, mock_get_uuid, mock_add, mock_trigger,
mock_validate, mock_setup):
net = {'cidr': '24', 'id': 1, 'uuid': 'nosuch'}
@ -974,7 +969,6 @@ class VlanNetworkTestCase(test.TestCase):
mock_add.assert_called_once_with(self.context_admin, instance.uuid,
net['id'])
mock_save.assert_called_once_with()
@mock.patch('nova.objects.instance.Instance.get_by_uuid')
@mock.patch('nova.objects.fixed_ip.FixedIP.associate')
@ -992,7 +986,8 @@ class VlanNetworkTestCase(test.TestCase):
mock_associate.assert_called_once_with(self.context,
'1.2.3.4',
instance.uuid,
1)
1,
vif_id=1)
@mock.patch('nova.objects.instance.Instance.get_by_uuid')
@mock.patch('nova.objects.fixed_ip.FixedIP.associate')
@ -1011,7 +1006,8 @@ class VlanNetworkTestCase(test.TestCase):
mock_associate.assert_called_once_with(self.context,
'1.2.3.4',
instance.uuid,
1, reserved=True)
1, reserved=True,
vif_id=1)
@mock.patch.object(db, 'virtual_interface_get_by_instance_and_network',
return_value=None)
@ -1590,13 +1586,9 @@ class VlanNetworkTestCase(test.TestCase):
self.mox.StubOutWithMock(db, 'fixed_ip_associate_pool')
self.mox.StubOutWithMock(db,
'virtual_interface_get_by_instance_and_network')
self.mox.StubOutWithMock(db, 'fixed_ip_update')
self.mox.StubOutWithMock(db, 'instance_get_by_uuid')
self.mox.StubOutWithMock(self.network, 'get_instance_nw_info')
db.fixed_ip_update(mox.IgnoreArg(),
mox.IgnoreArg(),
mox.IgnoreArg())
db.virtual_interface_get_by_instance_and_network(mox.IgnoreArg(),
mox.IgnoreArg(), mox.IgnoreArg()).AndReturn(vifs[0])
@ -1605,7 +1597,9 @@ class VlanNetworkTestCase(test.TestCase):
db.fixed_ip_associate_pool(mox.IgnoreArg(),
mox.IgnoreArg(),
instance_uuid=mox.IgnoreArg(),
host=None).AndReturn(fixed)
host=None,
virtual_interface_id=vifs[0]['id']
).AndReturn(fixed)
db.network_get(mox.IgnoreArg(),
mox.IgnoreArg(),
project_only=mox.IgnoreArg()

View File

@ -163,7 +163,20 @@ class _TestFixedIPObject(object):
fixedip = fixed_ip.FixedIP.associate(self.context, '1.2.3.4',
'fake-uuid')
associate.assert_called_with(self.context, '1.2.3.4', 'fake-uuid',
network_id=None, reserved=False)
network_id=None, reserved=False,
virtual_interface_id=None)
self._compare(fixedip, fake_fixed_ip)
@mock.patch('nova.db.fixed_ip_associate')
def test_associate_with_vif(self, associate):
associate.return_value = fake_fixed_ip
fixedip = fixed_ip.FixedIP.associate(self.context, '1.2.3.4',
'fake-uuid',
vif_id=0)
associate.assert_called_with(self.context, '1.2.3.4',
'fake-uuid',
network_id=None, reserved=False,
virtual_interface_id=0)
self._compare(fixedip, fake_fixed_ip)
@mock.patch('nova.db.fixed_ip_associate_pool')
@ -173,7 +186,18 @@ class _TestFixedIPObject(object):
'fake-uuid', 'host')
associate.assert_called_with(self.context, 123,
instance_uuid='fake-uuid',
host='host')
host='host', virtual_interface_id=None)
self._compare(fixedip, fake_fixed_ip)
@mock.patch('nova.db.fixed_ip_associate_pool')
def test_associate_pool_with_vif(self, associate):
associate.return_value = fake_fixed_ip
fixedip = fixed_ip.FixedIP.associate_pool(self.context, 123,
'fake-uuid', 'host',
vif_id=0)
associate.assert_called_with(self.context, 123,
instance_uuid='fake-uuid',
host='host', virtual_interface_id=0)
self._compare(fixedip, fake_fixed_ip)
@mock.patch('nova.db.fixed_ip_disassociate')

View File

@ -1176,12 +1176,12 @@ object_data = {
'EC2InstanceMapping': '1.0-a4556eb5c5e94c045fe84f49cf71644f',
'EC2SnapshotMapping': '1.0-47e7ddabe1af966dce0cfd0ed6cd7cd1',
'EC2VolumeMapping': '1.0-5b713751d6f97bad620f3378a521020d',
'FixedIP': '1.13-b5818a33996228fc146f096d1403742c',
'FixedIPList': '1.13-87a39361c8f08f059004d6b15103cdfd',
'FixedIP': '1.14-53e1c10b539f1a82fe83b1af4720efae',
'FixedIPList': '1.14-87a39361c8f08f059004d6b15103cdfd',
'Flavor': '1.1-b6bb7a730a79d720344accefafacf7ee',
'FlavorList': '1.1-52b5928600e7ca973aa4fc1e46f3934c',
'FloatingIP': '1.9-52a67d52d85eb8b3f324a5b7935a335b',
'FloatingIPList': '1.10-7f2ba670714e1b7bab462ab3290f7159',
'FloatingIP': '1.10-52a67d52d85eb8b3f324a5b7935a335b',
'FloatingIPList': '1.11-7f2ba670714e1b7bab462ab3290f7159',
'HostMapping': '1.0-1a3390a696792a552ab7bd31a77ba9ac',
'HVSpec': '1.1-6b4f7c0f688cbd03e24142a44eb9010d',
'ImageMeta': '1.7-642d1b2eb3e880a367f37d72dd76162d',