From 19d4c3a6f411b3b96d4a3dffc16b9b272a01971f Mon Sep 17 00:00:00 2001 From: Ewan Mellor Date: Sun, 5 Sep 2010 05:33:56 +0100 Subject: Bug #630636: XenAPI VM destroy fails when the VM is still running When destroying a VM using the XenAPI backend, if the VM is still running (the usual case) the destroy fails. It needs to be powered-off first. --- nova/virt/xenapi.py | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) (limited to 'nova') diff --git a/nova/virt/xenapi.py b/nova/virt/xenapi.py index b44ac383a..04069e459 100644 --- a/nova/virt/xenapi.py +++ b/nova/virt/xenapi.py @@ -274,9 +274,19 @@ class XenAPIConnection(object): def destroy(self, instance): vm = yield self._lookup(instance.name) if vm is None: - raise Exception('instance not present %s' % instance.name) - task = yield self._call_xenapi('Async.VM.destroy', vm) - yield self._wait_for_task(task) + # Don't complain, just return. This lets us clean up instances + # that have already disappeared from the underlying platform. + defer.returnValue(None) + try: + task = yield self._call_xenapi('Async.VM.hard_shutdown', vm) + yield self._wait_for_task(task) + except Exception, exn: + logging.warn(exn) + try: + task = yield self._call_xenapi('Async.VM.destroy', vm) + yield self._wait_for_task(task) + except Exception, exn: + logging.warn(exn) def get_info(self, instance_id): vm = self._lookup_blocking(instance_id) -- cgit From b049c032a9f950d67bbfe709802288a7fe28bdd6 Mon Sep 17 00:00:00 2001 From: Ewan Mellor Date: Sun, 5 Sep 2010 05:56:53 +0100 Subject: Bug #630640: Duplicated power state constants Remove power state constants that have ended up duplicated following a bad merge. They were moved from nova.compute.node.Instance into nova.compute.power_state at the same time that Instance was moved into nova.compute.service. We've ended up with these constants in both places. Remove the ones from service, in favour of the ones in power_state. --- nova/compute/service.py | 8 -------- nova/tests/cloud_unittest.py | 3 ++- 2 files changed, 2 insertions(+), 9 deletions(-) (limited to 'nova') diff --git a/nova/compute/service.py b/nova/compute/service.py index e59f3fb34..3321c2c00 100644 --- a/nova/compute/service.py +++ b/nova/compute/service.py @@ -224,14 +224,6 @@ class ProductCode(object): class Instance(object): - NOSTATE = 0x00 - RUNNING = 0x01 - BLOCKED = 0x02 - PAUSED = 0x03 - SHUTDOWN = 0x04 - SHUTOFF = 0x05 - CRASHED = 0x06 - def __init__(self, conn, name, data): """ spawn an instance with a given name """ self._conn = conn diff --git a/nova/tests/cloud_unittest.py b/nova/tests/cloud_unittest.py index 900ff5a97..19aa23b9e 100644 --- a/nova/tests/cloud_unittest.py +++ b/nova/tests/cloud_unittest.py @@ -28,6 +28,7 @@ from nova import flags from nova import rpc from nova import test from nova.auth import manager +from nova.compute import power_state from nova.compute import service from nova.endpoint import api from nova.endpoint import cloud @@ -95,7 +96,7 @@ class CloudTestCase(test.BaseTestCase): rv = yield defer.succeed(time.sleep(1)) info = self.cloud._get_instance(instance['instance_id']) logging.debug(info['state']) - if info['state'] == node.Instance.RUNNING: + if info['state'] == power_state.RUNNING: break self.assert_(rv) -- cgit From f1e45e3294622e22e6044027c1d2514f107d6134 Mon Sep 17 00:00:00 2001 From: Ewan Mellor Date: Fri, 10 Sep 2010 10:56:22 +0100 Subject: Change "exn" to "exc" to fit with the common style. --- nova/virt/xenapi.py | 32 ++++++++++++++++---------------- 1 file changed, 16 insertions(+), 16 deletions(-) (limited to 'nova') diff --git a/nova/virt/xenapi.py b/nova/virt/xenapi.py index 04069e459..1c6de4403 100644 --- a/nova/virt/xenapi.py +++ b/nova/virt/xenapi.py @@ -280,13 +280,13 @@ class XenAPIConnection(object): try: task = yield self._call_xenapi('Async.VM.hard_shutdown', vm) yield self._wait_for_task(task) - except Exception, exn: - logging.warn(exn) + except Exception, exc: + logging.warn(exc) try: task = yield self._call_xenapi('Async.VM.destroy', vm) yield self._wait_for_task(task) - except Exception, exn: - logging.warn(exn) + except Exception, exc: + logging.warn(exc) def get_info(self, instance_id): vm = self._lookup_blocking(instance_id) @@ -340,9 +340,9 @@ class XenAPIConnection(object): error_info) deferred.errback(XenAPI.Failure(error_info)) #logging.debug('Polling task %s done.', task) - except Exception, exn: - logging.warn(exn) - deferred.errback(exn) + except Exception, exc: + logging.warn(exc) + deferred.errback(exc) @utils.deferredToThread def _call_xenapi(self, method, *args): @@ -368,21 +368,21 @@ class XenAPIConnection(object): def _unwrap_plugin_exceptions(func, *args, **kwargs): try: return func(*args, **kwargs) - except XenAPI.Failure, exn: - logging.debug("Got exception: %s", exn) - if (len(exn.details) == 4 and - exn.details[0] == 'XENAPI_PLUGIN_EXCEPTION' and - exn.details[2] == 'Failure'): + except XenAPI.Failure, exc: + logging.debug("Got exception: %s", exc) + if (len(exc.details) == 4 and + exc.details[0] == 'XENAPI_PLUGIN_EXCEPTION' and + exc.details[2] == 'Failure'): params = None try: - params = eval(exn.details[3]) + params = eval(exc.details[3]) except: - raise exn + raise exc raise XenAPI.Failure(params) else: raise - except xmlrpclib.ProtocolError, exn: - logging.debug("Got exception: %s", exn) + except xmlrpclib.ProtocolError, exc: + logging.debug("Got exception: %s", exc) raise -- cgit From 9003fe35cfd2a6daa49d717bf256f2229171f7c6 Mon Sep 17 00:00:00 2001 From: Vishvananda Ishaya Date: Sat, 11 Sep 2010 00:16:12 -0700 Subject: improved network error case handling for fixed ips --- nova/network/manager.py | 27 +++++++++++++++++++++++++-- nova/tests/network_unittest.py | 41 ++++++++++++++++++++++++++++++++++------- 2 files changed, 59 insertions(+), 9 deletions(-) (limited to 'nova') diff --git a/nova/network/manager.py b/nova/network/manager.py index 3212a7eab..79280384c 100644 --- a/nova/network/manager.py +++ b/nova/network/manager.py @@ -226,19 +226,42 @@ class VlanManager(NetworkManager): network_ref['vpn_private_address']) self.driver.update_dhcp(context, network_ref['id']) - def lease_fixed_ip(self, context, address): + def lease_fixed_ip(self, context, mac, address): """Called by dhcp-bridge when ip is leased""" logging.debug("Leasing IP %s", address) fixed_ip_ref = self.db.fixed_ip_get_by_address(context, address) if not fixed_ip_ref['allocated']: logging.warn("IP %s leased that was already deallocated", address) + return + instance_ref = self.db.fixed_ip_get_instance(context, address) + if not instance_ref: + raise exception.Error("IP %s leased that isn't associated" % + address) + if instance_ref['mac_address'] != mac: + raise exception.Error("IP %s leased to bad mac %s vs %s" % + (address, instance_ref['mac_address'], mac)) self.db.fixed_ip_update(context, fixed_ip_ref['str_id'], {'leased': True}) - def release_fixed_ip(self, context, address): + def release_fixed_ip(self, context, mac, address): """Called by dhcp-bridge when ip is released""" logging.debug("Releasing IP %s", address) + fixed_ip_ref = self.db.fixed_ip_get_by_address(context, address) + if not fixed_ip_ref['leased']: + logging.warn("IP %s released that was not leased", address) + return + instance_ref = self.db.fixed_ip_get_instance(context, address) + if not instance_ref: + raise exception.Error("IP %s released that isn't associated" % + address) + if instance_ref['mac_address'] != mac: + raise exception.Error("IP %s released from bad mac %s vs %s" % + (address, instance_ref['mac_address'], mac)) + if fixed_ip_ref['allocated']: + logging.warn("IP %s released that is still allocated", address) + self.db.fixed_ip_update(context, address, {'leased': False}) + return self.db.fixed_ip_update(context, address, {'allocated': False, 'leased': False}) self.db.fixed_ip_instance_disassociate(context, address) diff --git a/nova/tests/network_unittest.py b/nova/tests/network_unittest.py index 9958600e0..d8d4ec0c3 100644 --- a/nova/tests/network_unittest.py +++ b/nova/tests/network_unittest.py @@ -147,10 +147,23 @@ class NetworkTestCase(test.TrialTestCase): """Makes sure that private ips don't overlap""" first = self._create_address(0) lease_ip(first) + instance_ids = [] for i in range(1, 5): - address = self._create_address(i) - address2 = self._create_address(i) - address3 = self._create_address(i) + mac = utils.generate_mac() + instance_ref = db.instance_create(None, + {'mac_address': mac}) + instance_ids.append(instance_ref['id']) + address = self._create_address(i, instance_ref['id']) + mac = utils.generate_mac() + instance_ref = db.instance_create(None, + {'mac_address': mac}) + instance_ids.append(instance_ref['id']) + address2 = self._create_address(i, instance_ref['id']) + mac = utils.generate_mac() + instance_ref = db.instance_create(None, + {'mac_address': mac}) + instance_ids.append(instance_ref['id']) + address3 = self._create_address(i, instance_ref['id']) lease_ip(address) lease_ip(address2) lease_ip(address3) @@ -166,6 +179,8 @@ class NetworkTestCase(test.TrialTestCase): release_ip(address) release_ip(address2) release_ip(address3) + for instance_id in instance_ids: + db.instance_destroy(None, instance_id) release_ip(first) db.fixed_ip_deallocate(None, first) @@ -226,8 +241,13 @@ class NetworkTestCase(test.TrialTestCase): num_available_ips = db.network_count_available_ips(None, network['id']) addresses = [] + instance_ids = [] for i in range(num_available_ips): - address = self._create_address(0) + mac = utils.generate_mac() + instance_ref = db.instance_create(None, + {'mac_address': mac}) + instance_ids.append(instance_ref['id']) + address = self._create_address(0, instance_ref['id']) addresses.append(address) lease_ip(address) @@ -238,9 +258,10 @@ class NetworkTestCase(test.TrialTestCase): None, network['id']) - for i in range(len(addresses)): + for i in range(num_available_ips): db.fixed_ip_deallocate(None, addresses[i]) release_ip(addresses[i]) + db.instance_destroy(None, instance_ids[i]) self.assertEqual(db.network_count_available_ips(None, network['id']), num_available_ips) @@ -263,7 +284,10 @@ def binpath(script): def lease_ip(private_ip): """Run add command on dhcpbridge""" network_ref = db.fixed_ip_get_network(None, private_ip) - cmd = "%s add fake %s fake" % (binpath('nova-dhcpbridge'), private_ip) + instance_ref = db.fixed_ip_get_instance(None, private_ip) + cmd = "%s add %s %s fake" % (binpath('nova-dhcpbridge'), + instance_ref['mac_address'], + private_ip) env = {'DNSMASQ_INTERFACE': network_ref['bridge'], 'TESTING': '1', 'FLAGFILE': FLAGS.dhcpbridge_flagfile} @@ -274,7 +298,10 @@ def lease_ip(private_ip): def release_ip(private_ip): """Run del command on dhcpbridge""" network_ref = db.fixed_ip_get_network(None, private_ip) - cmd = "%s del fake %s fake" % (binpath('nova-dhcpbridge'), private_ip) + instance_ref = db.fixed_ip_get_instance(None, private_ip) + cmd = "%s del %s %s fake" % (binpath('nova-dhcpbridge'), + instance_ref['mac_address'], + private_ip) env = {'DNSMASQ_INTERFACE': network_ref['bridge'], 'TESTING': '1', 'FLAGFILE': FLAGS.dhcpbridge_flagfile} -- cgit From cb13f09d7fe886bc8340770ff8c7011b6dbab0db Mon Sep 17 00:00:00 2001 From: Soren Hansen Date: Mon, 13 Sep 2010 09:03:14 +0200 Subject: Move vol.destroy() call out of the _check method in test_multiple_volume_race_condition test and into a callback of the DeferredList. This should fix the intermittent failure of that test. I /think/ test_too_many_volumes's failure was caused by test_multiple_volume_race_condition failure, since I have not been able to reproduce its failure after fixing this one. --- nova/tests/volume_unittest.py | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) (limited to 'nova') diff --git a/nova/tests/volume_unittest.py b/nova/tests/volume_unittest.py index 2a07afe69..540b71585 100644 --- a/nova/tests/volume_unittest.py +++ b/nova/tests/volume_unittest.py @@ -128,7 +128,6 @@ class VolumeTestCase(test.TrialTestCase): volume_service.get_volume, volume_id) - @defer.inlineCallbacks def test_multiple_volume_race_condition(self): vol_size = "5" user_id = "fake" @@ -137,17 +136,28 @@ class VolumeTestCase(test.TrialTestCase): def _check(volume_id): vol = volume_service.get_volume(volume_id) shelf_blade = '%s.%s' % (vol['shelf_id'], vol['blade_id']) - self.assert_(shelf_blade not in shelf_blades) + self.assertTrue(shelf_blade not in shelf_blades, + "Same shelf/blade tuple came back twice") shelf_blades.append(shelf_blade) logging.debug("got %s" % shelf_blade) - vol.destroy() + return vol deferreds = [] for i in range(5): d = self.volume.create_volume(vol_size, user_id, project_id) d.addCallback(_check) d.addErrback(self.fail) deferreds.append(d) - yield defer.DeferredList(deferreds) + def destroy_volumes(retvals): + overall_succes = True + for success, volume in retvals: + if not success: + overall_succes = False + else: + volume.destroy() + self.assertTrue(overall_succes) + d = defer.DeferredList(deferreds) + d.addCallback(destroy_volumes) + return d def test_multi_node(self): # TODO(termie): Figure out how to test with two nodes, -- cgit From 6083273c9949b0e49a0c0af7cfc8f0fb83ea7c79 Mon Sep 17 00:00:00 2001 From: Vishvananda Ishaya Date: Sat, 11 Sep 2010 03:06:27 -0700 Subject: fix network association issue --- nova/db/sqlalchemy/api.py | 1 + nova/network/manager.py | 27 +++++++++++++++++++++------ 2 files changed, 22 insertions(+), 6 deletions(-) (limited to 'nova') diff --git a/nova/db/sqlalchemy/api.py b/nova/db/sqlalchemy/api.py index 02ebdd222..bcdea4b67 100644 --- a/nova/db/sqlalchemy/api.py +++ b/nova/db/sqlalchemy/api.py @@ -185,6 +185,7 @@ def fixed_ip_allocate(_context, network_id): ).filter_by(allocated=False ).filter_by(leased=False ).filter_by(deleted=False + ).filter_by(instance=None ).with_lockmode('update' ).first() # NOTE(vish): if with_lockmode isn't supported, as in sqlite, diff --git a/nova/network/manager.py b/nova/network/manager.py index 79280384c..18a8ec0a1 100644 --- a/nova/network/manager.py +++ b/nova/network/manager.py @@ -96,6 +96,10 @@ class NetworkManager(manager.Manager): """Gets a fixed ip from the pool""" raise NotImplementedError() + def deallocate_fixed_ip(self, context, instance_id, *args, **kwargs): + """Returns a fixed ip to the pool""" + raise NotImplementedError() + def setup_fixed_ip(self, context, address): """Sets up rules for fixed ip""" raise NotImplementedError() @@ -174,6 +178,11 @@ class FlatManager(NetworkManager): self.db.fixed_ip_instance_associate(context, address, instance_id) return address + def deallocate_fixed_ip(self, context, address, *args, **kwargs): + """Returns a fixed ip to the pool""" + self.db.fixed_ip_deallocate(context, address) + self.db.fixed_ip_instance_disassociate(context, address) + def setup_compute_network(self, context, project_id): """Network is created manually""" pass @@ -216,6 +225,14 @@ class VlanManager(NetworkManager): self.db.fixed_ip_instance_associate(context, address, instance_id) return address + def deallocate_fixed_ip(self, context, address, *args, **kwargs): + """Returns a fixed ip to the pool""" + self.db.fixed_ip_deallocate(context, address) + fixed_ip_ref = self.db.fixed_ip_get_by_address(context, address) + if not fixed_ip_ref['leased']: + self.db.fixed_ip_instance_disassociate(context, address) + + def setup_fixed_ip(self, context, address): """Sets forwarding rules and dhcp for fixed ip""" fixed_ip_ref = self.db.fixed_ip_get_by_address(context, address) @@ -258,13 +275,11 @@ class VlanManager(NetworkManager): if instance_ref['mac_address'] != mac: raise exception.Error("IP %s released from bad mac %s vs %s" % (address, instance_ref['mac_address'], mac)) - if fixed_ip_ref['allocated']: + self.db.fixed_ip_update(context, address, {'leased': False}) + if not fixed_ip_ref['allocated']: + self.db.fixed_ip_instance_disassociate(context, address) + else: logging.warn("IP %s released that is still allocated", address) - self.db.fixed_ip_update(context, address, {'leased': False}) - return - self.db.fixed_ip_update(context, address, {'allocated': False, - 'leased': False}) - self.db.fixed_ip_instance_disassociate(context, address) def allocate_network(self, context, project_id): """Set up the network""" -- cgit From 2f3a63ac73176ed91cfcf8b011a2769fbf88201a Mon Sep 17 00:00:00 2001 From: Vishvananda Ishaya Date: Sat, 11 Sep 2010 03:31:40 -0700 Subject: simplified network instance association --- nova/db/api.py | 30 ++++++++++++-------------- nova/db/sqlalchemy/api.py | 54 +++++++++++++++++++++++++---------------------- nova/network/manager.py | 41 +++++++++++++---------------------- 3 files changed, 58 insertions(+), 67 deletions(-) (limited to 'nova') diff --git a/nova/db/api.py b/nova/db/api.py index d81673fad..6a0386bad 100644 --- a/nova/db/api.py +++ b/nova/db/api.py @@ -139,12 +139,20 @@ def floating_ip_get_instance(context, address): #################### -def fixed_ip_allocate(context, network_id): - """Allocate free fixed ip and return the address. +def fixed_ip_associate(context, address, instance_id): + """Associate fixed ip to instance. + + Raises if fixed ip is not available. + """ + return IMPL.fixed_ip_allocate(context, address, instance_id) + + +def fixed_ip_associate_pool(context, network_id, instance_id): + """Find free ip in network and associate it to instance. Raises if one is not available. """ - return IMPL.fixed_ip_allocate(context, network_id) + return IMPL.fixed_ip_allocate(context, network_id, instance_id) def fixed_ip_create(context, values): @@ -152,9 +160,9 @@ def fixed_ip_create(context, values): return IMPL.fixed_ip_create(context, values) -def fixed_ip_deallocate(context, address): - """Deallocate a fixed ip by address.""" - return IMPL.fixed_ip_deallocate(context, address) +def fixed_ip_disassociate(context, address): + """Disassociate a fixed ip from an instance by address.""" + return IMPL.fixed_ip_instance_disassociate(context, address) def fixed_ip_get_by_address(context, address): @@ -172,16 +180,6 @@ def fixed_ip_get_network(context, address): return IMPL.fixed_ip_get_network(context, address) -def fixed_ip_instance_associate(context, address, instance_id): - """Associate a fixed ip to an instance by address.""" - return IMPL.fixed_ip_instance_associate(context, address, instance_id) - - -def fixed_ip_instance_disassociate(context, address): - """Disassociate a fixed ip from an instance by address.""" - return IMPL.fixed_ip_instance_disassociate(context, address) - - def fixed_ip_update(context, address, values): """Create a fixed ip from the values dictionary.""" return IMPL.fixed_ip_update(context, address, values) diff --git a/nova/db/sqlalchemy/api.py b/nova/db/sqlalchemy/api.py index bcdea4b67..485dca2b0 100644 --- a/nova/db/sqlalchemy/api.py +++ b/nova/db/sqlalchemy/api.py @@ -174,7 +174,25 @@ def floating_ip_get_instance(_context, address): ################### -def fixed_ip_allocate(_context, network_id): +def fixed_ip_associate(_context, address, instance_id): + session = get_session() + with session.begin(): + fixed_ip_ref = session.query(models.FixedIp + ).filter_by(address=address + ).filter_by(deleted=False + ).filter_by(instance=None + ).with_lockmode('update' + ).first() + # NOTE(vish): if with_lockmode isn't supported, as in sqlite, + # then this has concurrency issues + if not fixed_ip_ref: + raise db.NoMoreAddresses() + fixed_ip_ref.instance = models.Instance.find(instance_id, + session=session) + session.add(fixed_ip_ref) + + +def fixed_ip_associate_pool(_context, network_id, instance_id): session = get_session() with session.begin(): network_or_none = or_(models.FixedIp.network_id == network_id, @@ -182,8 +200,6 @@ def fixed_ip_allocate(_context, network_id): fixed_ip_ref = session.query(models.FixedIp ).filter(network_or_none ).filter_by(reserved=False - ).filter_by(allocated=False - ).filter_by(leased=False ).filter_by(deleted=False ).filter_by(instance=None ).with_lockmode('update' @@ -195,7 +211,8 @@ def fixed_ip_allocate(_context, network_id): if not fixed_ip_ref.network: fixed_ip_ref.network = models.Network.find(network_id, session=session) - fixed_ip_ref['allocated'] = True + fixed_ip_ref.instance = models.Instance.find(instance_id, + session=session) session.add(fixed_ip_ref) return fixed_ip_ref['address'] @@ -208,6 +225,14 @@ def fixed_ip_create(_context, values): return fixed_ip_ref['address'] +def fixed_ip_disassociate(_context, address): + session = get_session() + with session.begin(): + fixed_ip_ref = models.FixedIp.find_by_str(address, session=session) + fixed_ip_ref.instance = None + fixed_ip_ref.save(session=session) + + def fixed_ip_get_by_address(_context, address): return models.FixedIp.find_by_str(address) @@ -224,27 +249,6 @@ def fixed_ip_get_network(_context, address): return models.FixedIp.find_by_str(address, session=session).network -def fixed_ip_deallocate(context, address): - db.fixed_ip_update(context, address, {'allocated': False}) - - -def fixed_ip_instance_associate(_context, address, instance_id): - session = get_session() - with session.begin(): - fixed_ip_ref = models.FixedIp.find_by_str(address, session=session) - instance_ref = models.Instance.find(instance_id, session=session) - fixed_ip_ref.instance = instance_ref - fixed_ip_ref.save(session=session) - - -def fixed_ip_instance_disassociate(_context, address): - session = get_session() - with session.begin(): - fixed_ip_ref = models.FixedIp.find_by_str(address, session=session) - fixed_ip_ref.instance = None - fixed_ip_ref.save(session=session) - - def fixed_ip_update(_context, address, values): session = get_session() with session.begin(): diff --git a/nova/network/manager.py b/nova/network/manager.py index 18a8ec0a1..fbc4e2b26 100644 --- a/nova/network/manager.py +++ b/nova/network/manager.py @@ -174,14 +174,16 @@ class FlatManager(NetworkManager): def allocate_fixed_ip(self, context, instance_id, *args, **kwargs): """Gets a fixed ip from the pool""" network_ref = self.db.project_get_network(context, context.project.id) - address = self.db.fixed_ip_allocate(context, network_ref['id']) - self.db.fixed_ip_instance_associate(context, address, instance_id) + address = self.db.fixed_ip_associate_pool(context, + network_ref['id'], + instance_id) + self.db.fixed_ip_update(context, address, {'allocated': True}) return address def deallocate_fixed_ip(self, context, address, *args, **kwargs): """Returns a fixed ip to the pool""" - self.db.fixed_ip_deallocate(context, address) - self.db.fixed_ip_instance_disassociate(context, address) + self.db.fixed_ip_update(context, address, {'allocated': False}) + self.db.fixed_ip_disassociate(context, address) def setup_compute_network(self, context, project_id): """Network is created manually""" @@ -218,19 +220,21 @@ class VlanManager(NetworkManager): """Gets a fixed ip from the pool""" network_ref = self.db.project_get_network(context, context.project.id) if kwargs.get('vpn', None): - address = self._allocate_vpn_ip(context, network_ref['id']) + address = network_ref['vpn_private_address'] + self.db.fixed_ip_associate(context, address, instance_id) else: - address = self.db.fixed_ip_allocate(context, - network_ref['id']) - self.db.fixed_ip_instance_associate(context, address, instance_id) + address = self.db.fixed_ip_associate_pool(context, + network_ref['id'], + instance_id) + self.db.fixed_ip_update(context, address, {'allocated': True}) return address def deallocate_fixed_ip(self, context, address, *args, **kwargs): """Returns a fixed ip to the pool""" - self.db.fixed_ip_deallocate(context, address) + self.db.fixed_ip_update(context, address, {'allocated': False}) fixed_ip_ref = self.db.fixed_ip_get_by_address(context, address) if not fixed_ip_ref['leased']: - self.db.fixed_ip_instance_disassociate(context, address) + self.db.fixed_ip_disassociate(context, address) def setup_fixed_ip(self, context, address): @@ -277,9 +281,7 @@ class VlanManager(NetworkManager): (address, instance_ref['mac_address'], mac)) self.db.fixed_ip_update(context, address, {'leased': False}) if not fixed_ip_ref['allocated']: - self.db.fixed_ip_instance_disassociate(context, address) - else: - logging.warn("IP %s released that is still allocated", address) + self.db.fixed_ip_disassociate(context, address) def allocate_network(self, context, project_id): """Set up the network""" @@ -321,19 +323,6 @@ class VlanManager(NetworkManager): # TODO(vish): Implement this pass - @staticmethod - def _allocate_vpn_ip(context, network_id): - """Allocate vpn ip for network""" - # TODO(vish): There is a possible concurrency issue here. - network_ref = db.network_get(context, network_id) - address = network_ref['vpn_private_address'] - fixed_ip_ref = db.fixed_ip_get_by_address(context, address) - # TODO(vish): Should this be fixed_ip_is_allocated? - if fixed_ip_ref['allocated']: - raise AddressAlreadyAllocated() - db.fixed_ip_update(context, fixed_ip_ref['id'], {'allocated': True}) - return fixed_ip_ref['str_id'] - def _ensure_indexes(self, context): """Ensure the indexes for the network exist -- cgit From b574d88fd6b27ac59bc51867e824f4ec9e1f7632 Mon Sep 17 00:00:00 2001 From: Vishvananda Ishaya Date: Sat, 11 Sep 2010 04:01:44 -0700 Subject: fixed tests, added a flag for updating dhcp on disassociate --- nova/db/api.py | 6 +++--- nova/endpoint/cloud.py | 2 +- nova/network/manager.py | 14 ++++++++++++++ nova/tests/network_unittest.py | 37 ++++++++++++++++++------------------- 4 files changed, 36 insertions(+), 23 deletions(-) (limited to 'nova') diff --git a/nova/db/api.py b/nova/db/api.py index 6a0386bad..d749ae50a 100644 --- a/nova/db/api.py +++ b/nova/db/api.py @@ -144,7 +144,7 @@ def fixed_ip_associate(context, address, instance_id): Raises if fixed ip is not available. """ - return IMPL.fixed_ip_allocate(context, address, instance_id) + return IMPL.fixed_ip_associate(context, address, instance_id) def fixed_ip_associate_pool(context, network_id, instance_id): @@ -152,7 +152,7 @@ def fixed_ip_associate_pool(context, network_id, instance_id): Raises if one is not available. """ - return IMPL.fixed_ip_allocate(context, network_id, instance_id) + return IMPL.fixed_ip_associate_pool(context, network_id, instance_id) def fixed_ip_create(context, values): @@ -162,7 +162,7 @@ def fixed_ip_create(context, values): def fixed_ip_disassociate(context, address): """Disassociate a fixed ip from an instance by address.""" - return IMPL.fixed_ip_instance_disassociate(context, address) + return IMPL.fixed_ip_disassociate(context, address) def fixed_ip_get_by_address(context, address): diff --git a/nova/endpoint/cloud.py b/nova/endpoint/cloud.py index 6ca6855ca..622b4e2a4 100644 --- a/nova/endpoint/cloud.py +++ b/nova/endpoint/cloud.py @@ -613,7 +613,7 @@ class CloudController(object): # NOTE(vish): Currently, nothing needs to be done on the # network node until release. If this changes, # we will need to cast here. - db.fixed_ip_deallocate(context, address) + self.network.deallocate_fixed_ip(context, address) host = instance_ref['host'] if host: diff --git a/nova/network/manager.py b/nova/network/manager.py index fbc4e2b26..d0036c7d9 100644 --- a/nova/network/manager.py +++ b/nova/network/manager.py @@ -61,6 +61,8 @@ flags.DEFINE_integer('cnt_vpn_clients', 5, 'Number of addresses reserved for vpn clients') flags.DEFINE_string('network_driver', 'nova.network.linux_net', 'Driver to use for network creation') +flags.DEFINE_boool('update_dhcp_on_disassocate', False, + 'Whether to update dhcp when fixed_ip is disassocated') class AddressAlreadyAllocated(exception.Error): @@ -235,6 +237,12 @@ class VlanManager(NetworkManager): fixed_ip_ref = self.db.fixed_ip_get_by_address(context, address) if not fixed_ip_ref['leased']: self.db.fixed_ip_disassociate(context, address) + # NOTE(vish): dhcp server isn't updated until next setup, this + # means there will stale entries in the conf file + # the code below will update the file if necessary + if FLAGS.update_dhcp_on_disassociate: + network_ref = self.db.fixed_ip_get_network(context, address) + self.driver.update_dhcp(context, network_ref['id']) def setup_fixed_ip(self, context, address): @@ -282,6 +290,12 @@ class VlanManager(NetworkManager): self.db.fixed_ip_update(context, address, {'leased': False}) if not fixed_ip_ref['allocated']: self.db.fixed_ip_disassociate(context, address) + # NOTE(vish): dhcp server isn't updated until next setup, this + # means there will stale entries in the conf file + # the code below will update the file if necessary + if FLAGS.update_dhcp_on_disassociate: + network_ref = self.db.fixed_ip_get_network(context, address) + self.driver.update_dhcp(context, network_ref['id']) def allocate_network(self, context, project_id): """Set up the network""" diff --git a/nova/tests/network_unittest.py b/nova/tests/network_unittest.py index d8d4ec0c3..dc5277f02 100644 --- a/nova/tests/network_unittest.py +++ b/nova/tests/network_unittest.py @@ -28,6 +28,7 @@ from nova import flags from nova import test from nova import utils from nova.auth import manager +from nova.endpoint import api FLAGS = flags.FLAGS @@ -48,7 +49,7 @@ class NetworkTestCase(test.TrialTestCase): self.user = self.manager.create_user('netuser', 'netuser', 'netuser') self.projects = [] self.network = utils.import_object(FLAGS.network_manager) - self.context = None + self.context = api.APIRequestContext(None, project=None, user=self.user) for i in range(5): name = 'project%s' % i self.projects.append(self.manager.create_project(name, @@ -75,12 +76,10 @@ class NetworkTestCase(test.TrialTestCase): def _create_address(self, project_num, instance_id=None): """Create an address in given project num""" - net = db.project_get_network(None, self.projects[project_num].id) - address = db.fixed_ip_allocate(None, net['id']) if instance_id is None: instance_id = self.instance_id - db.fixed_ip_instance_associate(None, address, instance_id) - return address + self.context.project = self.projects[project_num] + return self.network.allocate_fixed_ip(self.context, instance_id) def test_public_network_association(self): """Makes sure that we can allocaate a public ip""" @@ -103,14 +102,14 @@ class NetworkTestCase(test.TrialTestCase): address = db.instance_get_floating_address(None, self.instance_id) self.assertEqual(address, None) self.network.deallocate_floating_ip(self.context, float_addr) - db.fixed_ip_deallocate(None, fix_addr) + self.network.deallocate_fixed_ip(self.context, fix_addr) def test_allocate_deallocate_fixed_ip(self): """Makes sure that we can allocate and deallocate a fixed ip""" address = self._create_address(0) self.assertTrue(is_allocated_in_project(address, self.projects[0].id)) lease_ip(address) - db.fixed_ip_deallocate(None, address) + self.network.deallocate_fixed_ip(self.context, address) # Doesn't go away until it's dhcp released self.assertTrue(is_allocated_in_project(address, self.projects[0].id)) @@ -131,14 +130,14 @@ class NetworkTestCase(test.TrialTestCase): lease_ip(address) lease_ip(address2) - db.fixed_ip_deallocate(None, address) + self.network.deallocate_fixed_ip(self.context, address) release_ip(address) self.assertFalse(is_allocated_in_project(address, self.projects[0].id)) # First address release shouldn't affect the second self.assertTrue(is_allocated_in_project(address2, self.projects[1].id)) - db.fixed_ip_deallocate(None, address2) + self.network.deallocate_fixed_ip(self.context, address2) release_ip(address2) self.assertFalse(is_allocated_in_project(address2, self.projects[1].id)) @@ -173,16 +172,16 @@ class NetworkTestCase(test.TrialTestCase): self.projects[0].id)) self.assertFalse(is_allocated_in_project(address3, self.projects[0].id)) - db.fixed_ip_deallocate(None, address) - db.fixed_ip_deallocate(None, address2) - db.fixed_ip_deallocate(None, address3) + self.network.deallocate_fixed_ip(self.context, address) + self.network.deallocate_fixed_ip(self.context, address2) + self.network.deallocate_fixed_ip(self.context, address3) release_ip(address) release_ip(address2) release_ip(address3) for instance_id in instance_ids: db.instance_destroy(None, instance_id) release_ip(first) - db.fixed_ip_deallocate(None, first) + self.network.deallocate_fixed_ip(self.context, first) def test_vpn_ip_and_port_looks_valid(self): """Ensure the vpn ip and port are reasonable""" @@ -209,12 +208,12 @@ class NetworkTestCase(test.TrialTestCase): """Makes sure that ip addresses that are deallocated get reused""" address = self._create_address(0) lease_ip(address) - db.fixed_ip_deallocate(None, address) + self.network.deallocate_fixed_ip(self.context, address) release_ip(address) address2 = self._create_address(0) self.assertEqual(address, address2) - db.fixed_ip_deallocate(None, address2) + self.network.deallocate_fixed_ip(self.context, address2) def test_available_ips(self): """Make sure the number of available ips for the network is correct @@ -254,12 +253,12 @@ class NetworkTestCase(test.TrialTestCase): self.assertEqual(db.network_count_available_ips(None, network['id']), 0) self.assertRaises(db.NoMoreAddresses, - db.fixed_ip_allocate, - None, - network['id']) + self.network.allocate_fixed_ip, + self.context, + 'foo') for i in range(num_available_ips): - db.fixed_ip_deallocate(None, addresses[i]) + self.network.deallocate_fixed_ip(self.context, addresses[i]) release_ip(addresses[i]) db.instance_destroy(None, instance_ids[i]) self.assertEqual(db.network_count_available_ips(None, -- cgit From fe78b3651c9064e527b8e3b74d7669d3d364daab Mon Sep 17 00:00:00 2001 From: Vishvananda Ishaya Date: Sat, 11 Sep 2010 04:06:22 -0700 Subject: typo fixes, add flag to nova-dhcpbridge --- nova/network/manager.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'nova') diff --git a/nova/network/manager.py b/nova/network/manager.py index d0036c7d9..7a3bcfc2f 100644 --- a/nova/network/manager.py +++ b/nova/network/manager.py @@ -61,8 +61,8 @@ flags.DEFINE_integer('cnt_vpn_clients', 5, 'Number of addresses reserved for vpn clients') flags.DEFINE_string('network_driver', 'nova.network.linux_net', 'Driver to use for network creation') -flags.DEFINE_boool('update_dhcp_on_disassocate', False, - 'Whether to update dhcp when fixed_ip is disassocated') +flags.DEFINE_bool('update_dhcp_on_disassociate', False, + 'Whether to update dhcp when fixed_ip is disassocated') class AddressAlreadyAllocated(exception.Error): -- cgit