Merge lp:~bgh/nova/qmanager-rbp-trunk into lp:~hudson-openstack/nova/trunk

[Brad Hall]

Various fixes for the quantum network manager related to melange/ipam

[Vish Ishaya (vishvananda)]

This looks like a painless set of fixes for quantum related code.

review: Approve

Unmerged revisions

1542. By Brad Hall on 2011-09-06

Fix quantum/melange ipam interaction

We now query for the subnets by net_id/vif_id instead of searching through all
the blocks to find the right one. Both of the allocate and deallocate for
instance calls are now using the vif_id -> network_id mapping instead of
searching the quantum networks. get_port_by_attachment was also changed to
take a net_id so that we don't have to search through all of the quantum
networks to find the corresponding port.

=== modified file 'bin/nova-manage'
--- bin/nova-manage	2011-09-11 12:57:05 +0000
+++ bin/nova-manage	2011-09-15 18:08:49 +0000
@@ -685,7 +685,7 @@
             help='Multi host')
     @args('--dns1', dest="dns1", metavar="<DNS Address>", help='First DNS')
     @args('--dns2', dest="dns2", metavar="<DNS Address>", help='Second DNS')
-    @args('--uuid', dest="net_uuid", metavar="<network uuid>",
+    @args('--uuid', dest="uuid", metavar="<network uuid>",
       help='Network UUID')
     @args('--project_id', dest="project_id", metavar="<project id>",
       help='Project id')

=== modified file 'nova/network/quantum/client.py'
--- nova/network/quantum/client.py	2011-09-02 20:05:24 +0000
+++ nova/network/quantum/client.py	2011-09-15 18:08:49 +0000
@@ -224,8 +224,6 @@
                               type(data)))
 
     def deserialize(self, data, status_code):
-        if status_code == 202:
-            return data
         return JSONSerializer().deserialize(data, self.content_type())
 
     def content_type(self, format=None):

=== modified file 'nova/network/quantum/manager.py'
--- nova/network/quantum/manager.py	2011-09-02 20:05:24 +0000
+++ nova/network/quantum/manager.py	2011-09-15 18:08:49 +0000
@@ -201,15 +201,9 @@
         vifs = db.virtual_interface_get_by_instance(admin_context,
                                                     instance_id)
         for vif in vifs:
-            q_tenant_id = project_id
-            ipam_tenant_id = project_id
-            net_id, port_id = self.q_conn.get_port_by_attachment(q_tenant_id,
-                                                                 vif['uuid'])
-            if not net_id:
-                q_tenant_id = FLAGS.quantum_default_tenant_id
-                ipam_tenant_id = None
-                net_id, port_id = self.q_conn.get_port_by_attachment(
-                                             q_tenant_id, vif['uuid'])
+            net = db.network_get(admin_context, vif['network_id'])
+            net_id = net['uuid']
+
             if not net_id:
                 # TODO(bgh): We need to figure out a way to tell if we
                 # should actually be raising this exception or not.
@@ -219,8 +213,19 @@
                 # probably just log, continue, and move on.
                 raise Exception(_("No network for for virtual interface %s") %
                                 vif['uuid'])
+
+            ipam_tenant_id = project_id
             (v4_subnet, v6_subnet) = self.ipam.get_subnets_by_net_id(context,
-                                        ipam_tenant_id, net_id)
+                                        ipam_tenant_id, net_id, vif['uuid'])
+            # There isn't a good way of figuring out what the ipam
+            # tenant id should be before hand so we have to try both
+            # (project_id and None).
+            if not v4_subnet and not v6_subnet:
+                ipam_tenant_id = None
+                (v4_subnet, v6_subnet) = \
+                    self.ipam.get_subnets_by_net_id(context,
+                                                    ipam_tenant_id,
+                                                    net_id, vif['uuid'])
             v4_ips = self.ipam.get_v4_ips_by_interface(context,
                                         net_id, vif['uuid'],
                                         project_id=ipam_tenant_id)
@@ -283,20 +288,30 @@
         for vif_ref in vifs:
             interface_id = vif_ref['uuid']
             q_tenant_id = project_id
-            ipam_tenant_id = project_id
-            (net_id, port_id) = self.q_conn.get_port_by_attachment(q_tenant_id,
-                                            interface_id)
-            if not net_id:
+
+            net = db.network_get(admin_context, vif_ref['network_id'])
+            net_id = net['uuid']
+
+            port_id = self.q_conn.get_port_by_attachment(q_tenant_id,
+                                                         net_id, interface_id)
+            if not port_id:
                 q_tenant_id = FLAGS.quantum_default_tenant_id
-                ipam_tenant_id = None
-                (net_id, port_id) = self.q_conn.get_port_by_attachment(
-                                        q_tenant_id, interface_id)
-            if not net_id:
+                port_id = self.q_conn.get_port_by_attachment(
+                    q_tenant_id, net_id, interface_id)
+
+            if not port_id:
                 LOG.error("Unable to find port with attachment: %s" %
                           (interface_id))
-                continue
-            self.q_conn.detach_and_delete_port(q_tenant_id,
-                                               net_id, port_id)
+            else:
+                self.q_conn.detach_and_delete_port(q_tenant_id,
+                                                   net_id, port_id)
+
+            # Figure out which ipam tenant id we need here, either the project
+            # id or None (which corresponds to the provider address space).
+            if self.ipam.verify_subnet_exists(context, project_id, net_id):
+                ipam_tenant_id = project_id
+            else:
+                ipam_tenant_id = None
 
             self.ipam.deallocate_ips_by_vif(context, ipam_tenant_id,
                                             net_id, vif_ref)
@@ -319,6 +334,10 @@
 
         project_id = context.project_id
         for (net_id, _i) in networks:
-            self.ipam.verify_subnet_exists(context, project_id, net_id)
+            # TODO(bgh): At some point we should figure out whether or
+            # not we want the verify_subnet_exists call to be optional.
+            if not self.ipam.verify_subnet_exists(context, project_id,
+                                                  net_id):
+                raise exception.NetworkNotFound(network_id=net_id)
             if not self.q_conn.network_exists(project_id, net_id):
                 raise exception.NetworkNotFound(network_id=net_id)

=== modified file 'nova/network/quantum/melange_ipam_lib.py'
--- nova/network/quantum/melange_ipam_lib.py	2011-09-02 20:05:24 +0000
+++ nova/network/quantum/melange_ipam_lib.py	2011-09-15 18:08:49 +0000
@@ -134,34 +134,27 @@
         return [(network_id, tenant_id)
                 for priority, network_id, tenant_id in priority_nets]
 
-    def get_subnets_by_net_id(self, context, project_id, net_id):
+    def get_subnets_by_net_id(self, context, project_id, net_id, vif_id):
         """Returns information about the IPv4 and IPv6 subnets
            associated with a Quantum Network UUID.
         """
-
-        # FIXME(danwent):  Melange actually returns the subnet info
-        # when we query for a particular interface.  We may want to
-        # rework the ipam_manager python API to let us take advantage of
-        # this, as right now we have to get all blocks and cycle through
-        # them.
         subnet_v4 = None
         subnet_v6 = None
         tenant_id = project_id or FLAGS.quantum_default_tenant_id
-        all_blocks = self.m_conn.get_blocks(tenant_id)
-        for b in all_blocks['ip_blocks']:
-            if b['network_id'] == net_id:
-                subnet = {'network_id': b['network_id'],
-                          'cidr': b['cidr'],
-                          'gateway': b['gateway'],
-                          'broadcast': b['broadcast'],
-                          'netmask': b['netmask'],
-                          'dns1': b['dns1'],
-                          'dns2': b['dns2']}
-
-                if IPNetwork(b['cidr']).version == 6:
-                    subnet_v6 = subnet
-                else:
-                    subnet_v4 = subnet
+        ips = self.m_conn.get_allocated_ips(net_id, vif_id, tenant_id)
+        for ip_address in ips:
+            block = ip_address['ip_block']
+            subnet = {'network_id': block['network_id'],
+                      'cidr': block['cidr'],
+                      'gateway': block['gateway'],
+                      'broadcast': block['broadcast'],
+                      'netmask': block['netmask'],
+                      'dns1': block['dns1'],
+                      'dns2': block['dns2']}
+            if ip_address['version'] == 4:
+                subnet_v4 = subnet
+            else:
+                subnet_v6 = subnet
         return (subnet_v4, subnet_v6)
 
     def get_v4_ips_by_interface(self, context, net_id, vif_id, project_id):
@@ -179,7 +172,7 @@
                                                         project_id, 6)
 
     def _get_ips_by_interface(self, context, net_id, vif_id, project_id,
-                                                               ip_version):
+                              ip_version):
         """Helper method to fetch v4 or v6 addresses for a particular
            virtual interface.
         """
@@ -192,10 +185,16 @@
         """Confirms that a subnet exists that is associated with the
            specified Quantum Network UUID.
         """
+        # TODO(bgh): Would be nice if we could just do something like:
+        # GET /ipam/tenants/{tenant_id}/networks/{network_id}/ instead
+        # of searching through all the blocks.  Checking for a 404
+        # will then determine whether it exists.
         tenant_id = project_id or FLAGS.quantum_default_tenant_id
-        v4_subnet, v6_subnet = self.get_subnets_by_net_id(context, tenant_id,
-                                    quantum_net_id)
-        return v4_subnet is not None
+        all_blocks = self.m_conn.get_blocks(tenant_id)
+        for b in all_blocks['ip_blocks']:
+            if b['network_id'] == quantum_net_id:
+                return True
+        return False
 
     def deallocate_ips_by_vif(self, context, project_id, net_id, vif_ref):
         """Deallocate all fixed IPs associated with the specified

=== modified file 'nova/network/quantum/nova_ipam_lib.py'
--- nova/network/quantum/nova_ipam_lib.py	2011-09-02 20:05:24 +0000
+++ nova/network/quantum/nova_ipam_lib.py	2011-09-15 18:08:49 +0000
@@ -124,7 +124,7 @@
                       'virtual_interface_id': vif_rec['id']}
             db.fixed_ip_update(admin_context, address, values)
 
-    def get_subnets_by_net_id(self, context, tenant_id, net_id):
+    def get_subnets_by_net_id(self, context, tenant_id, net_id, _vif_id=None):
         """Returns information about the IPv4 and IPv6 subnets
            associated with a Quantum Network UUID.
         """
@@ -176,7 +176,8 @@
            such subnet exists.
         """
         admin_context = context.elevated()
-        db.network_get_by_uuid(admin_context, quantum_net_id)
+        net = db.network_get_by_uuid(admin_context, quantum_net_id)
+        return net is not None
 
     def deallocate_ips_by_vif(self, context, tenant_id, net_id, vif_ref):
         """Deallocate all fixed IPs associated with the specified

=== modified file 'nova/network/quantum/quantum_connection.py'
--- nova/network/quantum/quantum_connection.py	2011-09-02 20:05:24 +0000
+++ nova/network/quantum/quantum_connection.py	2011-09-15 18:08:49 +0000
@@ -98,21 +98,17 @@
         self.client.detach_resource(net_id, port_id, tenant=tenant_id)
         self.client.delete_port(net_id, port_id, tenant=tenant_id)
 
-    def get_port_by_attachment(self, tenant_id, attachment_id):
-        """Given a tenant, search for the Quantum network and port
-           UUID that has the specified interface-id attachment.
+    def get_port_by_attachment(self, tenant_id, net_id, attachment_id):
+        """Given a tenant and network, search for the port UUID that
+           has the specified interface-id attachment.
         """
         # FIXME(danwent): this will be inefficient until the Quantum
         # API implements querying a port by the interface-id
-        net_list_resdict = self.client.list_networks(tenant=tenant_id)
-        for n in net_list_resdict["networks"]:
-            net_id = n['id']
-            port_list_resdict = self.client.list_ports(net_id,
-                                            tenant=tenant_id)
-            for p in port_list_resdict["ports"]:
-                port_id = p["id"]
-                port_get_resdict = self.client.show_port_attachment(net_id,
+        port_list_resdict = self.client.list_ports(net_id, tenant=tenant_id)
+        for p in port_list_resdict["ports"]:
+            port_id = p["id"]
+            port_get_resdict = self.client.show_port_attachment(net_id,
                                 port_id, tenant=tenant_id)
-                if attachment_id == port_get_resdict["attachment"]["id"]:
-                    return (net_id, port_id)
-        return (None, None)
+            if attachment_id == port_get_resdict["attachment"]["id"]:
+                return port_id
+        return None

=== modified file 'nova/tests/test_quantum.py'
--- nova/tests/test_quantum.py	2011-09-02 19:11:28 +0000
+++ nova/tests/test_quantum.py	2011-09-15 18:08:49 +0000
@@ -87,14 +87,13 @@
                         "for tenant %(tenant_id)s" % locals()))
         del self.nets[net_id]['ports'][port_id]
 
-    def get_port_by_attachment(self, tenant_id, attachment_id):
-        for net_id, n in self.nets.items():
-            if n['tenant-id'] == tenant_id:
+    def get_port_by_attachment(self, tenant_id, net_id, attachment_id):
+        for nid, n in self.nets.items():
+            if nid == net_id and n['tenant-id'] == tenant_id:
                 for port_id, p in n['ports'].items():
                     if p['attachment-id'] == attachment_id:
-                        return (net_id, port_id)
-
-        return (None, None)
+                        return port_id
+        return None
 
 networks = [{'label': 'project1-net1',
              'injected': False,