Merge lp:~bgh/nova/qmanager-ipam-fixes into lp:nova/essex
- qmanager-ipam-fixes
- Merge into github
Proposed by
Brad Hall
Status: | Needs review |
---|---|
Proposed branch: | lp:~bgh/nova/qmanager-ipam-fixes |
Merge into: | lp:nova/essex |
Diff against target: |
284 lines (+85/-72) 6 files modified
nova/network/quantum/client.py (+0/-2) nova/network/quantum/manager.py (+42/-22) nova/network/quantum/melange_ipam_lib.py (+25/-26) nova/network/quantum/nova_ipam_lib.py (+3/-2) nova/network/quantum/quantum_connection.py (+10/-14) nova/tests/test_quantum.py (+5/-6) |
To merge this branch: | bzr merge lp:~bgh/nova/qmanager-ipam-fixes |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Nova Core security contacts | Pending | ||
Review via email: mp+77438@code.launchpad.net |
Commit message
Description of the change
A couple fixes to the currently experimental QuantumManager (network manager)
To post a comment you must log in.
Unmerged revisions
- 1647. By Brad Hall
-
Make sure the network info structure we're returning has a label
- 1646. By Brad Hall
-
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.
Preview Diff
[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1 | === modified file 'nova/network/quantum/client.py' | |||
2 | --- nova/network/quantum/client.py 2011-09-02 20:05:24 +0000 | |||
3 | +++ nova/network/quantum/client.py 2011-09-29 19:57:23 +0000 | |||
4 | @@ -224,8 +224,6 @@ | |||
5 | 224 | type(data))) | 224 | type(data))) |
6 | 225 | 225 | ||
7 | 226 | def deserialize(self, data, status_code): | 226 | def deserialize(self, data, status_code): |
8 | 227 | if status_code == 202: | ||
9 | 228 | return data | ||
10 | 229 | return JSONSerializer().deserialize(data, self.content_type()) | 227 | return JSONSerializer().deserialize(data, self.content_type()) |
11 | 230 | 228 | ||
12 | 231 | def content_type(self, format=None): | 229 | def content_type(self, format=None): |
13 | 232 | 230 | ||
14 | === modified file 'nova/network/quantum/manager.py' | |||
15 | --- nova/network/quantum/manager.py 2011-09-02 20:05:24 +0000 | |||
16 | +++ nova/network/quantum/manager.py 2011-09-29 19:57:23 +0000 | |||
17 | @@ -201,15 +201,9 @@ | |||
18 | 201 | vifs = db.virtual_interface_get_by_instance(admin_context, | 201 | vifs = db.virtual_interface_get_by_instance(admin_context, |
19 | 202 | instance_id) | 202 | instance_id) |
20 | 203 | for vif in vifs: | 203 | for vif in vifs: |
30 | 204 | q_tenant_id = project_id | 204 | net = db.network_get(admin_context, vif['network_id']) |
31 | 205 | ipam_tenant_id = project_id | 205 | net_id = net['uuid'] |
32 | 206 | net_id, port_id = self.q_conn.get_port_by_attachment(q_tenant_id, | 206 | |
24 | 207 | vif['uuid']) | ||
25 | 208 | if not net_id: | ||
26 | 209 | q_tenant_id = FLAGS.quantum_default_tenant_id | ||
27 | 210 | ipam_tenant_id = None | ||
28 | 211 | net_id, port_id = self.q_conn.get_port_by_attachment( | ||
29 | 212 | q_tenant_id, vif['uuid']) | ||
33 | 213 | if not net_id: | 207 | if not net_id: |
34 | 214 | # TODO(bgh): We need to figure out a way to tell if we | 208 | # TODO(bgh): We need to figure out a way to tell if we |
35 | 215 | # should actually be raising this exception or not. | 209 | # should actually be raising this exception or not. |
36 | @@ -219,8 +213,19 @@ | |||
37 | 219 | # probably just log, continue, and move on. | 213 | # probably just log, continue, and move on. |
38 | 220 | raise Exception(_("No network for for virtual interface %s") % | 214 | raise Exception(_("No network for for virtual interface %s") % |
39 | 221 | vif['uuid']) | 215 | vif['uuid']) |
40 | 216 | |||
41 | 217 | ipam_tenant_id = project_id | ||
42 | 222 | (v4_subnet, v6_subnet) = self.ipam.get_subnets_by_net_id(context, | 218 | (v4_subnet, v6_subnet) = self.ipam.get_subnets_by_net_id(context, |
44 | 223 | ipam_tenant_id, net_id) | 219 | ipam_tenant_id, net_id, vif['uuid']) |
45 | 220 | # There isn't a good way of figuring out what the ipam | ||
46 | 221 | # tenant id should be before hand so we have to try both | ||
47 | 222 | # (project_id and None). | ||
48 | 223 | if not v4_subnet and not v6_subnet: | ||
49 | 224 | ipam_tenant_id = None | ||
50 | 225 | (v4_subnet, v6_subnet) = \ | ||
51 | 226 | self.ipam.get_subnets_by_net_id(context, | ||
52 | 227 | ipam_tenant_id, | ||
53 | 228 | net_id, vif['uuid']) | ||
54 | 224 | v4_ips = self.ipam.get_v4_ips_by_interface(context, | 229 | v4_ips = self.ipam.get_v4_ips_by_interface(context, |
55 | 225 | net_id, vif['uuid'], | 230 | net_id, vif['uuid'], |
56 | 226 | project_id=ipam_tenant_id) | 231 | project_id=ipam_tenant_id) |
57 | @@ -242,6 +247,7 @@ | |||
58 | 242 | 'multi_host': False} | 247 | 'multi_host': False} |
59 | 243 | 248 | ||
60 | 244 | info = { | 249 | info = { |
61 | 250 | 'label': net['label'], | ||
62 | 245 | 'gateway': v4_subnet['gateway'], | 251 | 'gateway': v4_subnet['gateway'], |
63 | 246 | 'dhcp_server': v4_subnet['gateway'], | 252 | 'dhcp_server': v4_subnet['gateway'], |
64 | 247 | 'broadcast': v4_subnet['broadcast'], | 253 | 'broadcast': v4_subnet['broadcast'], |
65 | @@ -283,20 +289,30 @@ | |||
66 | 283 | for vif_ref in vifs: | 289 | for vif_ref in vifs: |
67 | 284 | interface_id = vif_ref['uuid'] | 290 | interface_id = vif_ref['uuid'] |
68 | 285 | q_tenant_id = project_id | 291 | q_tenant_id = project_id |
73 | 286 | ipam_tenant_id = project_id | 292 | |
74 | 287 | (net_id, port_id) = self.q_conn.get_port_by_attachment(q_tenant_id, | 293 | net = db.network_get(admin_context, vif_ref['network_id']) |
75 | 288 | interface_id) | 294 | net_id = net['uuid'] |
76 | 289 | if not net_id: | 295 | |
77 | 296 | port_id = self.q_conn.get_port_by_attachment(q_tenant_id, | ||
78 | 297 | net_id, interface_id) | ||
79 | 298 | if not port_id: | ||
80 | 290 | q_tenant_id = FLAGS.quantum_default_tenant_id | 299 | q_tenant_id = FLAGS.quantum_default_tenant_id |
85 | 291 | ipam_tenant_id = None | 300 | port_id = self.q_conn.get_port_by_attachment( |
86 | 292 | (net_id, port_id) = self.q_conn.get_port_by_attachment( | 301 | q_tenant_id, net_id, interface_id) |
87 | 293 | q_tenant_id, interface_id) | 302 | |
88 | 294 | if not net_id: | 303 | if not port_id: |
89 | 295 | LOG.error("Unable to find port with attachment: %s" % | 304 | LOG.error("Unable to find port with attachment: %s" % |
90 | 296 | (interface_id)) | 305 | (interface_id)) |
94 | 297 | continue | 306 | else: |
95 | 298 | self.q_conn.detach_and_delete_port(q_tenant_id, | 307 | self.q_conn.detach_and_delete_port(q_tenant_id, |
96 | 299 | net_id, port_id) | 308 | net_id, port_id) |
97 | 309 | |||
98 | 310 | # Figure out which ipam tenant id we need here, either the project | ||
99 | 311 | # id or None (which corresponds to the provider address space). | ||
100 | 312 | if self.ipam.verify_subnet_exists(context, project_id, net_id): | ||
101 | 313 | ipam_tenant_id = project_id | ||
102 | 314 | else: | ||
103 | 315 | ipam_tenant_id = None | ||
104 | 300 | 316 | ||
105 | 301 | self.ipam.deallocate_ips_by_vif(context, ipam_tenant_id, | 317 | self.ipam.deallocate_ips_by_vif(context, ipam_tenant_id, |
106 | 302 | net_id, vif_ref) | 318 | net_id, vif_ref) |
107 | @@ -319,6 +335,10 @@ | |||
108 | 319 | 335 | ||
109 | 320 | project_id = context.project_id | 336 | project_id = context.project_id |
110 | 321 | for (net_id, _i) in networks: | 337 | for (net_id, _i) in networks: |
112 | 322 | self.ipam.verify_subnet_exists(context, project_id, net_id) | 338 | # TODO(bgh): At some point we should figure out whether or |
113 | 339 | # not we want the verify_subnet_exists call to be optional. | ||
114 | 340 | if not self.ipam.verify_subnet_exists(context, project_id, | ||
115 | 341 | net_id): | ||
116 | 342 | raise exception.NetworkNotFound(network_id=net_id) | ||
117 | 323 | if not self.q_conn.network_exists(project_id, net_id): | 343 | if not self.q_conn.network_exists(project_id, net_id): |
118 | 324 | raise exception.NetworkNotFound(network_id=net_id) | 344 | raise exception.NetworkNotFound(network_id=net_id) |
119 | 325 | 345 | ||
120 | === modified file 'nova/network/quantum/melange_ipam_lib.py' | |||
121 | --- nova/network/quantum/melange_ipam_lib.py 2011-09-02 20:05:24 +0000 | |||
122 | +++ nova/network/quantum/melange_ipam_lib.py 2011-09-29 19:57:23 +0000 | |||
123 | @@ -134,34 +134,27 @@ | |||
124 | 134 | return [(network_id, tenant_id) | 134 | return [(network_id, tenant_id) |
125 | 135 | for priority, network_id, tenant_id in priority_nets] | 135 | for priority, network_id, tenant_id in priority_nets] |
126 | 136 | 136 | ||
128 | 137 | def get_subnets_by_net_id(self, context, project_id, net_id): | 137 | def get_subnets_by_net_id(self, context, project_id, net_id, vif_id): |
129 | 138 | """Returns information about the IPv4 and IPv6 subnets | 138 | """Returns information about the IPv4 and IPv6 subnets |
130 | 139 | associated with a Quantum Network UUID. | 139 | associated with a Quantum Network UUID. |
131 | 140 | """ | 140 | """ |
132 | 141 | |||
133 | 142 | # FIXME(danwent): Melange actually returns the subnet info | ||
134 | 143 | # when we query for a particular interface. We may want to | ||
135 | 144 | # rework the ipam_manager python API to let us take advantage of | ||
136 | 145 | # this, as right now we have to get all blocks and cycle through | ||
137 | 146 | # them. | ||
138 | 147 | subnet_v4 = None | 141 | subnet_v4 = None |
139 | 148 | subnet_v6 = None | 142 | subnet_v6 = None |
140 | 149 | tenant_id = project_id or FLAGS.quantum_default_tenant_id | 143 | tenant_id = project_id or FLAGS.quantum_default_tenant_id |
156 | 150 | all_blocks = self.m_conn.get_blocks(tenant_id) | 144 | ips = self.m_conn.get_allocated_ips(net_id, vif_id, tenant_id) |
157 | 151 | for b in all_blocks['ip_blocks']: | 145 | for ip_address in ips: |
158 | 152 | if b['network_id'] == net_id: | 146 | block = ip_address['ip_block'] |
159 | 153 | subnet = {'network_id': b['network_id'], | 147 | subnet = {'network_id': block['network_id'], |
160 | 154 | 'cidr': b['cidr'], | 148 | 'cidr': block['cidr'], |
161 | 155 | 'gateway': b['gateway'], | 149 | 'gateway': block['gateway'], |
162 | 156 | 'broadcast': b['broadcast'], | 150 | 'broadcast': block['broadcast'], |
163 | 157 | 'netmask': b['netmask'], | 151 | 'netmask': block['netmask'], |
164 | 158 | 'dns1': b['dns1'], | 152 | 'dns1': block['dns1'], |
165 | 159 | 'dns2': b['dns2']} | 153 | 'dns2': block['dns2']} |
166 | 160 | 154 | if ip_address['version'] == 4: | |
167 | 161 | if IPNetwork(b['cidr']).version == 6: | 155 | subnet_v4 = subnet |
168 | 162 | subnet_v6 = subnet | 156 | else: |
169 | 163 | else: | 157 | subnet_v6 = subnet |
155 | 164 | subnet_v4 = subnet | ||
170 | 165 | return (subnet_v4, subnet_v6) | 158 | return (subnet_v4, subnet_v6) |
171 | 166 | 159 | ||
172 | 167 | def get_v4_ips_by_interface(self, context, net_id, vif_id, project_id): | 160 | def get_v4_ips_by_interface(self, context, net_id, vif_id, project_id): |
173 | @@ -179,7 +172,7 @@ | |||
174 | 179 | project_id, 6) | 172 | project_id, 6) |
175 | 180 | 173 | ||
176 | 181 | def _get_ips_by_interface(self, context, net_id, vif_id, project_id, | 174 | def _get_ips_by_interface(self, context, net_id, vif_id, project_id, |
178 | 182 | ip_version): | 175 | ip_version): |
179 | 183 | """Helper method to fetch v4 or v6 addresses for a particular | 176 | """Helper method to fetch v4 or v6 addresses for a particular |
180 | 184 | virtual interface. | 177 | virtual interface. |
181 | 185 | """ | 178 | """ |
182 | @@ -192,10 +185,16 @@ | |||
183 | 192 | """Confirms that a subnet exists that is associated with the | 185 | """Confirms that a subnet exists that is associated with the |
184 | 193 | specified Quantum Network UUID. | 186 | specified Quantum Network UUID. |
185 | 194 | """ | 187 | """ |
186 | 188 | # TODO(bgh): Would be nice if we could just do something like: | ||
187 | 189 | # GET /ipam/tenants/{tenant_id}/networks/{network_id}/ instead | ||
188 | 190 | # of searching through all the blocks. Checking for a 404 | ||
189 | 191 | # will then determine whether it exists. | ||
190 | 195 | tenant_id = project_id or FLAGS.quantum_default_tenant_id | 192 | tenant_id = project_id or FLAGS.quantum_default_tenant_id |
194 | 196 | v4_subnet, v6_subnet = self.get_subnets_by_net_id(context, tenant_id, | 193 | all_blocks = self.m_conn.get_blocks(tenant_id) |
195 | 197 | quantum_net_id) | 194 | for b in all_blocks['ip_blocks']: |
196 | 198 | return v4_subnet is not None | 195 | if b['network_id'] == quantum_net_id: |
197 | 196 | return True | ||
198 | 197 | return False | ||
199 | 199 | 198 | ||
200 | 200 | def deallocate_ips_by_vif(self, context, project_id, net_id, vif_ref): | 199 | def deallocate_ips_by_vif(self, context, project_id, net_id, vif_ref): |
201 | 201 | """Deallocate all fixed IPs associated with the specified | 200 | """Deallocate all fixed IPs associated with the specified |
202 | 202 | 201 | ||
203 | === modified file 'nova/network/quantum/nova_ipam_lib.py' | |||
204 | --- nova/network/quantum/nova_ipam_lib.py 2011-09-02 20:05:24 +0000 | |||
205 | +++ nova/network/quantum/nova_ipam_lib.py 2011-09-29 19:57:23 +0000 | |||
206 | @@ -124,7 +124,7 @@ | |||
207 | 124 | 'virtual_interface_id': vif_rec['id']} | 124 | 'virtual_interface_id': vif_rec['id']} |
208 | 125 | db.fixed_ip_update(admin_context, address, values) | 125 | db.fixed_ip_update(admin_context, address, values) |
209 | 126 | 126 | ||
211 | 127 | def get_subnets_by_net_id(self, context, tenant_id, net_id): | 127 | def get_subnets_by_net_id(self, context, tenant_id, net_id, _vif_id=None): |
212 | 128 | """Returns information about the IPv4 and IPv6 subnets | 128 | """Returns information about the IPv4 and IPv6 subnets |
213 | 129 | associated with a Quantum Network UUID. | 129 | associated with a Quantum Network UUID. |
214 | 130 | """ | 130 | """ |
215 | @@ -176,7 +176,8 @@ | |||
216 | 176 | such subnet exists. | 176 | such subnet exists. |
217 | 177 | """ | 177 | """ |
218 | 178 | admin_context = context.elevated() | 178 | admin_context = context.elevated() |
220 | 179 | db.network_get_by_uuid(admin_context, quantum_net_id) | 179 | net = db.network_get_by_uuid(admin_context, quantum_net_id) |
221 | 180 | return net is not None | ||
222 | 180 | 181 | ||
223 | 181 | def deallocate_ips_by_vif(self, context, tenant_id, net_id, vif_ref): | 182 | def deallocate_ips_by_vif(self, context, tenant_id, net_id, vif_ref): |
224 | 182 | """Deallocate all fixed IPs associated with the specified | 183 | """Deallocate all fixed IPs associated with the specified |
225 | 183 | 184 | ||
226 | === modified file 'nova/network/quantum/quantum_connection.py' | |||
227 | --- nova/network/quantum/quantum_connection.py 2011-09-02 20:05:24 +0000 | |||
228 | +++ nova/network/quantum/quantum_connection.py 2011-09-29 19:57:23 +0000 | |||
229 | @@ -98,21 +98,17 @@ | |||
230 | 98 | self.client.detach_resource(net_id, port_id, tenant=tenant_id) | 98 | self.client.detach_resource(net_id, port_id, tenant=tenant_id) |
231 | 99 | self.client.delete_port(net_id, port_id, tenant=tenant_id) | 99 | self.client.delete_port(net_id, port_id, tenant=tenant_id) |
232 | 100 | 100 | ||
236 | 101 | def get_port_by_attachment(self, tenant_id, attachment_id): | 101 | def get_port_by_attachment(self, tenant_id, net_id, attachment_id): |
237 | 102 | """Given a tenant, search for the Quantum network and port | 102 | """Given a tenant and network, search for the port UUID that |
238 | 103 | UUID that has the specified interface-id attachment. | 103 | has the specified interface-id attachment. |
239 | 104 | """ | 104 | """ |
240 | 105 | # FIXME(danwent): this will be inefficient until the Quantum | 105 | # FIXME(danwent): this will be inefficient until the Quantum |
241 | 106 | # API implements querying a port by the interface-id | 106 | # API implements querying a port by the interface-id |
250 | 107 | net_list_resdict = self.client.list_networks(tenant=tenant_id) | 107 | port_list_resdict = self.client.list_ports(net_id, tenant=tenant_id) |
251 | 108 | for n in net_list_resdict["networks"]: | 108 | for p in port_list_resdict["ports"]: |
252 | 109 | net_id = n['id'] | 109 | port_id = p["id"] |
253 | 110 | port_list_resdict = self.client.list_ports(net_id, | 110 | port_get_resdict = self.client.show_port_attachment(net_id, |
246 | 111 | tenant=tenant_id) | ||
247 | 112 | for p in port_list_resdict["ports"]: | ||
248 | 113 | port_id = p["id"] | ||
249 | 114 | port_get_resdict = self.client.show_port_attachment(net_id, | ||
254 | 115 | port_id, tenant=tenant_id) | 111 | port_id, tenant=tenant_id) |
258 | 116 | if attachment_id == port_get_resdict["attachment"]["id"]: | 112 | if attachment_id == port_get_resdict["attachment"]["id"]: |
259 | 117 | return (net_id, port_id) | 113 | return port_id |
260 | 118 | return (None, None) | 114 | return None |
261 | 119 | 115 | ||
262 | === modified file 'nova/tests/test_quantum.py' | |||
263 | --- nova/tests/test_quantum.py 2011-09-02 19:11:28 +0000 | |||
264 | +++ nova/tests/test_quantum.py 2011-09-29 19:57:23 +0000 | |||
265 | @@ -87,14 +87,13 @@ | |||
266 | 87 | "for tenant %(tenant_id)s" % locals())) | 87 | "for tenant %(tenant_id)s" % locals())) |
267 | 88 | del self.nets[net_id]['ports'][port_id] | 88 | del self.nets[net_id]['ports'][port_id] |
268 | 89 | 89 | ||
272 | 90 | def get_port_by_attachment(self, tenant_id, attachment_id): | 90 | def get_port_by_attachment(self, tenant_id, net_id, attachment_id): |
273 | 91 | for net_id, n in self.nets.items(): | 91 | for nid, n in self.nets.items(): |
274 | 92 | if n['tenant-id'] == tenant_id: | 92 | if nid == net_id and n['tenant-id'] == tenant_id: |
275 | 93 | for port_id, p in n['ports'].items(): | 93 | for port_id, p in n['ports'].items(): |
276 | 94 | if p['attachment-id'] == attachment_id: | 94 | if p['attachment-id'] == attachment_id: |
280 | 95 | return (net_id, port_id) | 95 | return port_id |
281 | 96 | 96 | return None | |
279 | 97 | return (None, None) | ||
282 | 98 | 97 | ||
283 | 99 | networks = [{'label': 'project1-net1', | 98 | networks = [{'label': 'project1-net1', |
284 | 100 | 'injected': False, | 99 | 'injected': False, |