simplified network instance association

3 files changed, 58 insertions, 67 deletions

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