OpenStack Compute (nova)

Merge lp:~zulcss/nova/nova-convert-ipy-to-netaddr into lp:~hudson-openstack/nova/trunk

  1. nova-convert-ipy-to-netaddr
  2. Merge into trunk
Proposed by Chuck Short
Status: Merged
Approved by: Vish Ishaya
Approved revision: 1169
Merged at revision: 1211
Proposed branch: lp:~zulcss/nova/nova-convert-ipy-to-netaddr
Merge into: lp:~hudson-openstack/nova/trunk
Diff against target: 346 lines (+46/-48)
11 files modified
bin/nova-manage (+3/-3)
nova/api/ec2/cloud.py (+2/-2)
nova/network/linux_net.py (+2/-2)
nova/network/manager.py (+23/-23)
nova/tests/network/base.py (+1/-1)
nova/tests/test_flat_network.py (+3/-3)
nova/tests/test_network.py (+1/-1)
nova/tests/test_vlan_network.py (+3/-3)
nova/virt/libvirt/connection.py (+1/-2)
nova/virt/libvirt/netutils.py (+7/-7)
tools/pip-requires (+0/-1)
To merge this branch: bzr merge lp:~zulcss/nova/nova-convert-ipy-to-netaddr
Reviewer Review Type Date Requested Status
Brian Waldon (community) Approve
Chuck Short (community) Needs Resubmitting
Soren Hansen (community) Approve
Devin Carlen (community) Approve
Dan Prince (community) Needs Fixing
Review via email: mp+63600@code.launchpad.net

Commit message

Removes the usage of the IPy module in favor of the netaddr module.

Description of the change

This branch removes the usage of the ipy module in favor of the netaddr module. I have run unit tests for this branch and it has passed.

To post a comment you must log in.
Revision history for this message
Jesse Andrews (anotherjesse) wrote :

Missed removing an unused "import IPy" from:

nova/tests/network/base.py
nova/tests/test_network.py

Also we can remove IPy from tools/pip-requires?

Revision history for this message
Jesse Andrews (anotherjesse) wrote :

otherwise it is all :)

Revision history for this message
Brian Waldon (bcwaldon) wrote :

Good work, Chuck. I love seeing this kind of refactoring. I've got a couple of questions for you, though:

1) Why did you pick netaddr over IPy?

2) Jesse beat me to the point of removing the few remaining places IPy is still imported.

review: Needs Information
Revision history for this message
Chuck Short (zulcss) wrote :

> Missed removing an unused "import IPy" from:
>
> nova/tests/network/base.py
> nova/tests/test_network.py
>
> Also we can remove IPy from tools/pip-requires?

Fixed

Revision history for this message
Chuck Short (zulcss) wrote :

> Good work, Chuck. I love seeing this kind of refactoring. I've got a couple of
> questions for you, though:
>
> 1) Why did you pick netaddr over IPy?

It looked more sensible in my opinnon and seems to be easier to support down the line.

>
> 2) Jesse beat me to the point of removing the few remaining places IPy is
> still imported.

fixed

Revision history for this message
Dan Prince (dan-prince) wrote :

Hi Chuck,

This appears the break (or at least change) the existing nova-manage call that creates floating IPs. I get the following error:

---

nova-manage floating create nova1 172.20.0.0/24
Possible wrong number of arguments supplied
floating create: Creates floating ips for host by range
        arguments: host ip_range

---

Looking at pydoc...

It looks like the calls for IPy.IP(range) and netaddr.IPRange(range) aren't an exact match.

The IPy.IP call wants: ('172.20.0.0/24')

Where the netaddr.IPRange wants: (start, end)

---

I like the idea of consolidating the imports but we should probably try to adhere to the existing ( and documented ) nova-manage command arguments.

review: Needs Fixing
Revision history for this message
Brian Waldon (bcwaldon) wrote :

> > Good work, Chuck. I love seeing this kind of refactoring. I've got a couple
> of
> > questions for you, though:
> >
> > 1) Why did you pick netaddr over IPy?
>
> It looked more sensible in my opinnon and seems to be easier to support down
> the line.
>
> >
> > 2) Jesse beat me to the point of removing the few remaining places IPy is
> > still imported.
>
> fixed

Great. Once Dan's concerns are addressed, this is good to go.

review: Approve
Revision history for this message
Chuck Short (zulcss) wrote :

> Hi Chuck,
>
> This appears the break (or at least change) the existing nova-manage call that
> creates floating IPs. I get the following error:
>
> ---
>
> nova-manage floating create nova1 172.20.0.0/24
> Possible wrong number of arguments supplied
> floating create: Creates floating ips for host by range
> arguments: host ip_range
>
> ---
>
> Looking at pydoc...
>
> It looks like the calls for IPy.IP(range) and netaddr.IPRange(range) aren't an
> exact match.
>
> The IPy.IP call wants: ('172.20.0.0/24')
>
> Where the netaddr.IPRange wants: (start, end)
>
> ---
>
> I like the idea of consolidating the imports but we should probably try to
> adhere to the existing ( and documented ) nova-manage command arguments.

I think I fixed this now.

Regards
chuck

Revision history for this message
Devin Carlen (devcamcar) wrote :

nice cleanup, lgtm

review: Approve
Revision history for this message
Brian Waldon (bcwaldon) wrote :

There's a conflict that needs to be resolved.

review: Needs Fixing
Revision history for this message
Brian Waldon (bcwaldon) wrote :

Just a quick note, Chuck: "Commit Message" is what will show up in nova/trunk when this gets merged in. I think the comment you left would be more appropriate down here. Typically the commit message is a more condensed version of the "Description" field, if not just the same thing altogether.

Revision history for this message
Soren Hansen (soren) wrote :

I took the liberty of adjusting the commit message.

Revision history for this message
Soren Hansen (soren) wrote :

lgtm

review: Approve
Revision history for this message
Brian Waldon (bcwaldon) wrote :

Looks like there may have been a bad merge. When i try to run the tests, I get this syntax error:

Traceback (most recent call last):
  File "run_tests.py", line 70, in <module>
    from nova.tests import fake_flags
  File "/Users/brian.waldon/valhalla/nova/nova-convert-ipy-to-netaddr/nova/tests/fake_flags.py", line 29, in <module>
    flags.DECLARE('network_size', 'nova.network.manager')
  File "/Users/brian.waldon/valhalla/nova/nova-convert-ipy-to-netaddr/nova/flags.py", line 245, in DECLARE
    __import__(module_string, globals(), locals())
  File "/Users/brian.waldon/valhalla/nova/nova-convert-ipy-to-netaddr/nova/network/manager.py", line 331
    net['gateway_v6'] = str(list(gateway_v6)[1]))
                                                ^
SyntaxError: invalid syntax

Revision history for this message
Chuck Short (zulcss) wrote :

Doh! typo has been fixed and re-ran the testsuite.

review: Needs Resubmitting
Revision history for this message
Brian Waldon (bcwaldon) wrote :

Looks good, Chuck. Thanks!

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'bin/nova-manage'
2--- bin/nova-manage 2011-06-22 19:22:25 +0000
3+++ bin/nova-manage 2011-06-23 13:59:36 +0000
4@@ -56,11 +56,11 @@
5 import gettext
6 import glob
7 import json
8+import netaddr
9 import os
10 import sys
11 import time
12
13-import IPy
14
15 # If ../nova/__init__.py exists, add ../ to Python search path, so that
16 # it will override what happens to be installed in /usr/(local/)lib/python...
17@@ -513,7 +513,7 @@
18 def create(self, host, range):
19 """Creates floating ips for host by range
20 arguments: host ip_range"""
21- for address in IPy.IP(range):
22+ for address in netaddr.IPNetwork(range):
23 db.floating_ip_create(context.get_admin_context(),
24 {'address': str(address),
25 'host': host})
26@@ -521,7 +521,7 @@
27 def delete(self, ip_range):
28 """Deletes floating ips by range
29 arguments: range"""
30- for address in IPy.IP(ip_range):
31+ for address in netaddr.IPNetwork(ip_range):
32 db.floating_ip_destroy(context.get_admin_context(),
33 str(address))
34
35
36=== modified file 'nova/api/ec2/cloud.py'
37--- nova/api/ec2/cloud.py 2011-06-17 23:52:22 +0000
38+++ nova/api/ec2/cloud.py 2011-06-23 13:59:36 +0000
39@@ -23,7 +23,7 @@
40 """
41
42 import base64
43-import IPy
44+import netaddr
45 import os
46 import urllib
47 import tempfile
48@@ -452,7 +452,7 @@
49 elif cidr_ip:
50 # If this fails, it throws an exception. This is what we want.
51 cidr_ip = urllib.unquote(cidr_ip).decode()
52- IPy.IP(cidr_ip)
53+ netaddr.IPNetwork(cidr_ip)
54 values['cidr'] = cidr_ip
55 else:
56 values['cidr'] = '0.0.0.0/0'
57
58=== modified file 'nova/network/linux_net.py'
59--- nova/network/linux_net.py 2011-05-21 07:00:58 +0000
60+++ nova/network/linux_net.py 2011-06-23 13:59:36 +0000
61@@ -20,6 +20,7 @@
62
63 import calendar
64 import inspect
65+import netaddr
66 import os
67
68 from nova import db
69@@ -27,7 +28,6 @@
70 from nova import flags
71 from nova import log as logging
72 from nova import utils
73-from IPy import IP
74
75
76 LOG = logging.getLogger("nova.linux_net")
77@@ -700,7 +700,7 @@
78 '--listen-address=%s' % net['gateway'],
79 '--except-interface=lo',
80 '--dhcp-range=%s,static,120s' % net['dhcp_start'],
81- '--dhcp-lease-max=%s' % IP(net['cidr']).len(),
82+ '--dhcp-lease-max=%s' % len(netaddr.IPNetwork(net['cidr'])),
83 '--dhcp-hostsfile=%s' % _dhcp_file(net['bridge'], 'conf'),
84 '--dhcp-script=%s' % FLAGS.dhcpbridge,
85 '--leasefile-ro']
86
87=== modified file 'nova/network/manager.py'
88--- nova/network/manager.py 2011-06-07 15:47:14 +0000
89+++ nova/network/manager.py 2011-06-23 13:59:36 +0000
90@@ -45,10 +45,9 @@
91
92 import datetime
93 import math
94+import netaddr
95 import socket
96
97-import IPy
98-
99 from nova import context
100 from nova import db
101 from nova import exception
102@@ -295,8 +294,8 @@
103 def create_networks(self, context, cidr, num_networks, network_size,
104 cidr_v6, gateway_v6, label, *args, **kwargs):
105 """Create networks based on parameters."""
106- fixed_net = IPy.IP(cidr)
107- fixed_net_v6 = IPy.IP(cidr_v6)
108+ fixed_net = netaddr.IPNetwork(cidr)
109+ fixed_net_v6 = netaddr.IPNetwork(cidr_v6)
110 significant_bits_v6 = 64
111 network_size_v6 = 1 << 64
112 count = 1
113@@ -305,15 +304,15 @@
114 start_v6 = index * network_size_v6
115 significant_bits = 32 - int(math.log(network_size, 2))
116 cidr = '%s/%s' % (fixed_net[start], significant_bits)
117- project_net = IPy.IP(cidr)
118+ project_net = netaddr.IPNetwork(cidr)
119 net = {}
120 net['bridge'] = FLAGS.flat_network_bridge
121 net['dns'] = FLAGS.flat_network_dns
122 net['cidr'] = cidr
123- net['netmask'] = str(project_net.netmask())
124- net['gateway'] = str(project_net[1])
125- net['broadcast'] = str(project_net.broadcast())
126- net['dhcp_start'] = str(project_net[2])
127+ net['netmask'] = str(project_net.netmask)
128+ net['gateway'] = str(list(project_net)[1])
129+ net['broadcast'] = str(project_net.broadcast)
130+ net['dhcp_start'] = str(list(project_net)[2])
131 if num_networks > 1:
132 net['label'] = '%s_%d' % (label, count)
133 else:
134@@ -324,15 +323,16 @@
135 cidr_v6 = '%s/%s' % (fixed_net_v6[start_v6],
136 significant_bits_v6)
137 net['cidr_v6'] = cidr_v6
138- project_net_v6 = IPy.IP(cidr_v6)
139+
140+ project_net_v6 = netaddr.IPNetwork(cidr_v6)
141
142 if gateway_v6:
143 # use a pre-defined gateway if one is provided
144- net['gateway_v6'] = str(gateway_v6)
145+ net['gateway_v6'] = str(list(gateway_v6)[1])
146 else:
147- net['gateway_v6'] = str(project_net_v6[1])
148+ net['gateway_v6'] = str(list(project_net_v6)[1])
149
150- net['netmask_v6'] = str(project_net_v6.prefixlen())
151+ net['netmask_v6'] = str(project_net_v6._prefixlen)
152
153 network_ref = self.db.network_create_safe(context, net)
154
155@@ -356,7 +356,7 @@
156 # to properties of the manager class?
157 bottom_reserved = self._bottom_reserved_ips
158 top_reserved = self._top_reserved_ips
159- project_net = IPy.IP(network_ref['cidr'])
160+ project_net = netaddr.IPNetwork(network_ref['cidr'])
161 num_ips = len(project_net)
162 for index in range(num_ips):
163 address = str(project_net[index])
164@@ -546,13 +546,13 @@
165 ' the vlan start cannot be greater'
166 ' than 4094'))
167
168- fixed_net = IPy.IP(cidr)
169- if fixed_net.len() < num_networks * network_size:
170+ fixed_net = netaddr.IPNetwork(cidr)
171+ if len(fixed_net) < num_networks * network_size:
172 raise ValueError(_('The network range is not big enough to fit '
173 '%(num_networks)s. Network size is %(network_size)s' %
174 locals()))
175
176- fixed_net_v6 = IPy.IP(cidr_v6)
177+ fixed_net_v6 = netaddr.IPNetwork(cidr_v6)
178 network_size_v6 = 1 << 64
179 significant_bits_v6 = 64
180 for index in range(num_networks):
181@@ -561,14 +561,14 @@
182 start_v6 = index * network_size_v6
183 significant_bits = 32 - int(math.log(network_size, 2))
184 cidr = "%s/%s" % (fixed_net[start], significant_bits)
185- project_net = IPy.IP(cidr)
186+ project_net = netaddr.IPNetwork(cidr)
187 net = {}
188 net['cidr'] = cidr
189- net['netmask'] = str(project_net.netmask())
190- net['gateway'] = str(project_net[1])
191- net['broadcast'] = str(project_net.broadcast())
192- net['vpn_private_address'] = str(project_net[2])
193- net['dhcp_start'] = str(project_net[3])
194+ net['netmask'] = str(project_net.netmask)
195+ net['gateway'] = str(list(project_net)[1])
196+ net['broadcast'] = str(project_net.broadcast)
197+ net['vpn_private_address'] = str(list(project_net)[2])
198+ net['dhcp_start'] = str(list(project_net)[3])
199 net['vlan'] = vlan
200 net['bridge'] = 'br%s' % vlan
201 if(FLAGS.use_ipv6):
202
203=== modified file 'nova/tests/network/base.py'
204--- nova/tests/network/base.py 2011-05-16 18:55:46 +0000
205+++ nova/tests/network/base.py 2011-06-23 13:59:36 +0000
206@@ -18,7 +18,7 @@
207 """
208 Base class of Unit Tests for all network models
209 """
210-import IPy
211+import netaddr
212 import os
213
214 from nova import context
215
216=== modified file 'nova/tests/test_flat_network.py'
217--- nova/tests/test_flat_network.py 2011-03-16 21:13:57 +0000
218+++ nova/tests/test_flat_network.py 2011-06-23 13:59:36 +0000
219@@ -18,7 +18,7 @@
220 """
221 Unit Tests for flat network code
222 """
223-import IPy
224+import netaddr
225 import os
226 import unittest
227
228@@ -45,8 +45,8 @@
229
230 self.context._project = self.projects[0]
231 self.context.project_id = self.projects[0].id
232- pubnet = IPy.IP(flags.FLAGS.floating_range)
233- address = str(pubnet[0])
234+ pubnet = netaddr.IPRange(flags.FLAGS.floating_range)
235+ address = str(list(pubnet)[0])
236 try:
237 db.floating_ip_get_by_address(context.get_admin_context(), address)
238 except exception.NotFound:
239
240=== modified file 'nova/tests/test_network.py'
241--- nova/tests/test_network.py 2011-03-23 05:29:32 +0000
242+++ nova/tests/test_network.py 2011-06-23 13:59:36 +0000
243@@ -18,7 +18,7 @@
244 """
245 Unit Tests for network code
246 """
247-import IPy
248+import netaddr
249 import os
250
251 from nova import test
252
253=== modified file 'nova/tests/test_vlan_network.py'
254--- nova/tests/test_vlan_network.py 2011-03-12 00:36:39 +0000
255+++ nova/tests/test_vlan_network.py 2011-06-23 13:59:36 +0000
256@@ -18,7 +18,7 @@
257 """
258 Unit Tests for vlan network code
259 """
260-import IPy
261+import netaddr
262 import os
263
264 from nova import context
265@@ -44,8 +44,8 @@
266 # TODO(vish): better way of adding floating ips
267 self.context._project = self.projects[0]
268 self.context.project_id = self.projects[0].id
269- pubnet = IPy.IP(flags.FLAGS.floating_range)
270- address = str(pubnet[0])
271+ pubnet = netaddr.IPNetwork(flags.FLAGS.floating_range)
272+ address = str(list(pubnet)[0])
273 try:
274 db.floating_ip_get_by_address(context.get_admin_context(), address)
275 except exception.NotFound:
276
277=== modified file 'nova/virt/libvirt/connection.py'
278--- nova/virt/libvirt/connection.py 2011-06-15 16:46:24 +0000
279+++ nova/virt/libvirt/connection.py 2011-06-23 13:59:36 +0000
280@@ -38,6 +38,7 @@
281
282 import hashlib
283 import multiprocessing
284+import netaddr
285 import os
286 import random
287 import re
288@@ -53,8 +54,6 @@
289 from eventlet import greenthread
290 from eventlet import tpool
291
292-import IPy
293-
294 from nova import context
295 from nova import db
296 from nova import exception
297
298=== modified file 'nova/virt/libvirt/netutils.py'
299--- nova/virt/libvirt/netutils.py 2011-05-19 20:25:57 +0000
300+++ nova/virt/libvirt/netutils.py 2011-06-23 13:59:36 +0000
301@@ -21,7 +21,7 @@
302 """Network-releated utilities for supporting libvirt connection code."""
303
304
305-import IPy
306+import netaddr
307
308 from nova import context
309 from nova import db
310@@ -34,18 +34,18 @@
311
312
313 def get_net_and_mask(cidr):
314- net = IPy.IP(cidr)
315- return str(net.net()), str(net.netmask())
316+ net = netaddr.IPNetwork(cidr)
317+ return str(net.ip), str(net.netmask)
318
319
320 def get_net_and_prefixlen(cidr):
321- net = IPy.IP(cidr)
322- return str(net.net()), str(net.prefixlen())
323+ net = netaddr.IPNetwork(cidr)
324+ return str(net.ip), str(net._prefixlen)
325
326
327 def get_ip_version(cidr):
328- net = IPy.IP(cidr)
329- return int(net.version())
330+ net = netaddr.IPNetwork(cidr)
331+ return int(net.version)
332
333
334 def get_network_info(instance):
335
336=== modified file 'tools/pip-requires'
337--- tools/pip-requires 2011-06-21 18:42:57 +0000
338+++ tools/pip-requires 2011-06-23 13:59:36 +0000
339@@ -1,7 +1,6 @@
340 SQLAlchemy==0.6.3
341 pep8==0.5.0
342 pylint==0.19
343-IPy==0.70
344 Cheetah==2.4.4
345 M2Crypto==0.20.2
346 amqplib==0.6.1