cloud-init

Merge ~smoser/cloud-init:cleanup/mask2cidr into cloud-init:master

  1. Git
  2. lp:~smoser/cloud-init
  3. cleanup/mask2cidr
  4. Merge into master
Proposed by Scott Moser on 2017-05-26
Status: Merged
Merged at revision: d00da2d5b0d45db5670622a66d833d2abb907388
Proposed branch: ~smoser/cloud-init:cleanup/mask2cidr
Merge into: cloud-init:master
Diff against target: 459 lines (+215/-81)
6 files modified
cloudinit/net/eni.py (+4/-0)
cloudinit/net/netplan.py (+6/-8)
cloudinit/net/network_state.py (+194/-50)
cloudinit/net/sysconfig.py (+7/-16)
tests/unittests/test_distros/test_netconfig.py (+2/-3)
tests/unittests/test_net.py (+2/-4)
Reviewer Review Type Date Requested Status
Ryan Harper 2017-05-26 Approve on 2017-06-08
Server Team CI bot continuous-integration Approve on 2017-06-08
Chad Smith 2017-05-26 Pending
Review via email: mp+324677@code.launchpad.net

Commit Message

net: normalize data in network_state object

the network_state object's network and route keys would have different
information depending upon how the network_state object was populated.

This change cleans that up. Now,
  * address will always contain an IP address.
  * prefix will always include an integer value that is the network_prefix
    for the address.
  * netmask will be present only if the address is ipv4, and its
    value will always correlate to the 'prefix'.

    TODO: maybe this will be present also for ipv6

To post a comment you must log in.
Server Team CI bot (server-team-bot) wrote :
review: Approve (continuous-integration)
Ryan Harper (raharper) :
Server Team CI bot (server-team-bot) wrote :
review: Approve (continuous-integration)
Server Team CI bot (server-team-bot) wrote :
review: Approve (continuous-integration)
Chad Smith (chad.smith) wrote :

looks much better thanks for this. minor inlines below

Scott Moser (smoser) wrote :

responded to rharper's comments. i think he reviewed an older version though.

Scott Moser (smoser) :
Scott Moser (smoser) wrote :

thanks for the reviews.

Ryan Harper (raharper) :
Scott Moser (smoser) wrote :

bunch of fixes in tree now. i thikn i addressed all feedback.

Server Team CI bot (server-team-bot) wrote :
review: Approve (continuous-integration)
Scott Moser (smoser) wrote :

i've just rebased on master.

Server Team CI bot (server-team-bot) wrote :
review: Approve (continuous-integration)
Ryan Harper (raharper) :
Scott Moser (smoser) wrote :

fixed all that round of comments.

Server Team CI bot (server-team-bot) wrote :
review: Approve (continuous-integration)
Scott Moser (smoser) wrote :

we decided that we should add the 'netmask' field to ipv6 addresses also.
the 'ipaddress' module can help with generating this in python3, but it is not present in python2.7.
Mostly mentioning that for use in developing to make sure you're right.

Server Team CI bot (server-team-bot) wrote :
review: Approve (continuous-integration)
Ryan Harper (raharper) wrote :

I'm ok with adding ipv6 'netmask' bits later. I'm happy with this clean-up as is.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1diff --git a/cloudinit/net/eni.py b/cloudinit/net/eni.py
2index 20e19f5..98ce01e 100644
3--- a/cloudinit/net/eni.py
4+++ b/cloudinit/net/eni.py
5@@ -46,6 +46,10 @@ def _iface_add_subnet(iface, subnet):
6 'dns_nameservers',
7 ]
8 for key, value in subnet.items():
9+ if key == 'netmask':
10+ continue
11+ if key == 'address':
12+ value = "%s/%s" % (subnet['address'], subnet['prefix'])
13 if value and key in valid_map:
14 if type(value) == list:
15 value = " ".join(value)
16diff --git a/cloudinit/net/netplan.py b/cloudinit/net/netplan.py
17index a715f3b..6754330 100644
18--- a/cloudinit/net/netplan.py
19+++ b/cloudinit/net/netplan.py
20@@ -4,7 +4,7 @@ import copy
21 import os
22
23 from . import renderer
24-from .network_state import mask2cidr, subnet_is_ipv6
25+from .network_state import subnet_is_ipv6
26
27 from cloudinit import log as logging
28 from cloudinit import util
29@@ -118,10 +118,9 @@ def _extract_addresses(config, entry):
30 sn_type += '4'
31 entry.update({sn_type: True})
32 elif sn_type in ['static']:
33- addr = '%s' % subnet.get('address')
34- netmask = subnet.get('netmask')
35- if netmask and '/' not in addr:
36- addr += '/%s' % mask2cidr(netmask)
37+ addr = "%s" % subnet.get('address')
38+ if 'prefix' in subnet:
39+ addr += "/%d" % subnet.get('prefix')
40 if 'gateway' in subnet and subnet.get('gateway'):
41 gateway = subnet.get('gateway')
42 if ":" in gateway:
43@@ -138,9 +137,8 @@ def _extract_addresses(config, entry):
44 mtukey += '6'
45 entry.update({mtukey: subnet.get('mtu')})
46 for route in subnet.get('routes', []):
47- network = route.get('network')
48- netmask = route.get('netmask')
49- to_net = '%s/%s' % (network, mask2cidr(netmask))
50+ to_net = "%s/%s" % (route.get('network'),
51+ route.get('prefix'))
52 route = {
53 'via': route.get('gateway'),
54 'to': to_net,
55diff --git a/cloudinit/net/network_state.py b/cloudinit/net/network_state.py
56index 9e9c05a..87a7222 100644
57--- a/cloudinit/net/network_state.py
58+++ b/cloudinit/net/network_state.py
59@@ -289,19 +289,15 @@ class NetworkStateInterpreter(object):
60 iface.update({param: val})
61
62 # convert subnet ipv6 netmask to cidr as needed
63- subnets = command.get('subnets')
64- if subnets:
65+ subnets = _normalize_subnets(command.get('subnets'))
66+
67+ # automatically set 'use_ipv6' if any addresses are ipv6
68+ if not self.use_ipv6:
69 for subnet in subnets:
70- if subnet['type'] == 'static':
71- if ':' in subnet['address']:
72- self.use_ipv6 = True
73- if 'netmask' in subnet and ':' in subnet['address']:
74- subnet['netmask'] = mask2cidr(subnet['netmask'])
75- for route in subnet.get('routes', []):
76- if 'netmask' in route:
77- route['netmask'] = mask2cidr(route['netmask'])
78- elif subnet['type'].endswith('6'):
79+ if (subnet.get('type').endswith('6') or
80+ is_ipv6_addr(subnet.get('address'))):
81 self.use_ipv6 = True
82+ break
83
84 iface.update({
85 'name': command.get('name'),
86@@ -456,16 +452,7 @@ class NetworkStateInterpreter(object):
87
88 @ensure_command_keys(['destination'])
89 def handle_route(self, command):
90- routes = self._network_state.get('routes', [])
91- network, cidr = command['destination'].split("/")
92- netmask = cidr2mask(int(cidr))
93- route = {
94- 'network': network,
95- 'netmask': netmask,
96- 'gateway': command.get('gateway'),
97- 'metric': command.get('metric'),
98- }
99- routes.append(route)
100+ self._network_state['routes'].append(_normalize_route(command))
101
102 # V2 handlers
103 def handle_bonds(self, command):
104@@ -666,18 +653,9 @@ class NetworkStateInterpreter(object):
105
106 routes = []
107 for route in cfg.get('routes', []):
108- route_addr = route.get('to')
109- if "/" in route_addr:
110- route_addr, route_cidr = route_addr.split("/")
111- route_netmask = cidr2mask(route_cidr)
112- subnet_route = {
113- 'address': route_addr,
114- 'netmask': route_netmask,
115- 'gateway': route.get('via')
116- }
117- routes.append(subnet_route)
118- if len(routes) > 0:
119- subnet.update({'routes': routes})
120+ routes.append(_normalize_route(
121+ {'address': route.get('to'), 'gateway': route.get('via')}))
122+ subnet['routes'] = routes
123
124 if ":" in address:
125 if 'gateway6' in cfg and gateway6 is None:
126@@ -692,53 +670,219 @@ class NetworkStateInterpreter(object):
127 return subnets
128
129
130+def _normalize_subnet(subnet):
131+ # Prune all keys with None values.
132+ subnet = copy.deepcopy(subnet)
133+ normal_subnet = dict((k, v) for k, v in subnet.items() if v)
134+
135+ if subnet.get('type') in ('static', 'static6'):
136+ normal_subnet.update(
137+ _normalize_net_keys(normal_subnet, address_keys=('address',)))
138+ normal_subnet['routes'] = [_normalize_route(r)
139+ for r in subnet.get('routes', [])]
140+ return normal_subnet
141+
142+
143+def _normalize_net_keys(network, address_keys=()):
144+ """Normalize dictionary network keys returning prefix and address keys.
145+
146+ @param network: A dict of network-related definition containing prefix,
147+ netmask and address_keys.
148+ @param address_keys: A tuple of keys to search for representing the address
149+ or cidr. The first address_key discovered will be used for
150+ normalization.
151+
152+ @returns: A dict containing normalized prefix and matching addr_key.
153+ """
154+ net = dict((k, v) for k, v in network.items() if v)
155+ addr_key = None
156+ for key in address_keys:
157+ if net.get(key):
158+ addr_key = key
159+ break
160+ if not addr_key:
161+ message = (
162+ 'No config network address keys [%s] found in %s' %
163+ (','.join(address_keys), network))
164+ LOG.error(message)
165+ raise ValueError(message)
166+
167+ addr = net.get(addr_key)
168+ ipv6 = is_ipv6_addr(addr)
169+ netmask = net.get('netmask')
170+ if "/" in addr:
171+ addr_part, _, maybe_prefix = addr.partition("/")
172+ net[addr_key] = addr_part
173+ try:
174+ prefix = int(maybe_prefix)
175+ except ValueError:
176+ # this supports input of <address>/255.255.255.0
177+ prefix = mask_to_net_prefix(maybe_prefix)
178+ elif netmask:
179+ prefix = mask_to_net_prefix(netmask)
180+ elif 'prefix' in net:
181+ prefix = int(prefix)
182+ else:
183+ prefix = 64 if ipv6 else 24
184+
185+ if 'prefix' in net and str(net['prefix']) != str(prefix):
186+ LOG.warning("Overwriting existing 'prefix' with '%s' in "
187+ "network info: %s", prefix, net)
188+ net['prefix'] = prefix
189+
190+ if ipv6:
191+ # TODO: we could/maybe should add this back with the very uncommon
192+ # 'netmask' for ipv6. We need a 'net_prefix_to_ipv6_mask' for that.
193+ if 'netmask' in net:
194+ del net['netmask']
195+ else:
196+ net['netmask'] = net_prefix_to_ipv4_mask(net['prefix'])
197+
198+ return net
199+
200+
201+def _normalize_route(route):
202+ """normalize a route.
203+ return a dictionary with only:
204+ 'type': 'route' (only present if it was present in input)
205+ 'network': the network portion of the route as a string.
206+ 'prefix': the network prefix for address as an integer.
207+ 'metric': integer metric (only if present in input).
208+ 'netmask': netmask (string) equivalent to prefix iff network is ipv4.
209+ """
210+ # Prune None-value keys. Specifically allow 0 (a valid metric).
211+ normal_route = dict((k, v) for k, v in route.items()
212+ if v not in ("", None))
213+ if 'destination' in normal_route:
214+ normal_route['network'] = normal_route['destination']
215+ del normal_route['destination']
216+
217+ normal_route.update(
218+ _normalize_net_keys(
219+ normal_route, address_keys=('network', 'destination')))
220+
221+ metric = normal_route.get('metric')
222+ if metric:
223+ try:
224+ normal_route['metric'] = int(metric)
225+ except ValueError:
226+ raise TypeError(
227+ 'Route config metric {} is not an integer'.format(metric))
228+ return normal_route
229+
230+
231+def _normalize_subnets(subnets):
232+ if not subnets:
233+ subnets = []
234+ return [_normalize_subnet(s) for s in subnets]
235+
236+
237+def is_ipv6_addr(address):
238+ if not address:
239+ return False
240+ return ":" in str(address)
241+
242+
243 def subnet_is_ipv6(subnet):
244 """Common helper for checking network_state subnets for ipv6."""
245 # 'static6' or 'dhcp6'
246 if subnet['type'].endswith('6'):
247 # This is a request for DHCPv6.
248 return True
249- elif subnet['type'] == 'static' and ":" in subnet['address']:
250+ elif subnet['type'] == 'static' and is_ipv6_addr(subnet.get('address')):
251 return True
252 return False
253
254
255-def cidr2mask(cidr):
256+def net_prefix_to_ipv4_mask(prefix):
257+ """Convert a network prefix to an ipv4 netmask.
258+
259+ This is the inverse of ipv4_mask_to_net_prefix.
260+ 24 -> "255.255.255.0"
261+ Also supports input as a string."""
262+
263 mask = [0, 0, 0, 0]
264- for i in list(range(0, cidr)):
265+ for i in list(range(0, int(prefix))):
266 idx = int(i / 8)
267 mask[idx] = mask[idx] + (1 << (7 - i % 8))
268 return ".".join([str(x) for x in mask])
269
270
271-def ipv4mask2cidr(mask):
272- if '.' not in mask:
273+def ipv4_mask_to_net_prefix(mask):
274+ """Convert an ipv4 netmask into a network prefix length.
275+
276+ If the input is already an integer or a string representation of
277+ an integer, then int(mask) will be returned.
278+ "255.255.255.0" => 24
279+ str(24) => 24
280+ "24" => 24
281+ """
282+ if isinstance(mask, int):
283 return mask
284- return sum([bin(int(x)).count('1') for x in mask.split('.')])
285+ if isinstance(mask, six.string_types):
286+ try:
287+ return int(mask)
288+ except ValueError:
289+ pass
290+ else:
291+ raise TypeError("mask '%s' is not a string or int")
292
293+ if '.' not in mask:
294+ raise ValueError("netmask '%s' does not contain a '.'" % mask)
295
296-def ipv6mask2cidr(mask):
297- if ':' not in mask:
298+ toks = mask.split(".")
299+ if len(toks) != 4:
300+ raise ValueError("netmask '%s' had only %d parts" % (mask, len(toks)))
301+
302+ return sum([bin(int(x)).count('1') for x in toks])
303+
304+
305+def ipv6_mask_to_net_prefix(mask):
306+ """Convert an ipv6 netmask (very uncommon) or prefix (64) to prefix.
307+
308+ If 'mask' is an integer or string representation of one then
309+ int(mask) will be returned.
310+ """
311+
312+ if isinstance(mask, int):
313 return mask
314+ if isinstance(mask, six.string_types):
315+ try:
316+ return int(mask)
317+ except ValueError:
318+ pass
319+ else:
320+ raise TypeError("mask '%s' is not a string or int")
321+
322+ if ':' not in mask:
323+ raise ValueError("mask '%s' does not have a ':'")
324
325 bitCount = [0, 0x8000, 0xc000, 0xe000, 0xf000, 0xf800, 0xfc00, 0xfe00,
326 0xff00, 0xff80, 0xffc0, 0xffe0, 0xfff0, 0xfff8, 0xfffc,
327 0xfffe, 0xffff]
328- cidr = 0
329+ prefix = 0
330 for word in mask.split(':'):
331 if not word or int(word, 16) == 0:
332 break
333- cidr += bitCount.index(int(word, 16))
334+ prefix += bitCount.index(int(word, 16))
335+
336+ return prefix
337
338- return cidr
339
340+def mask_to_net_prefix(mask):
341+ """Return the network prefix for the netmask provided.
342
343-def mask2cidr(mask):
344- if ':' in str(mask):
345- return ipv6mask2cidr(mask)
346- elif '.' in str(mask):
347- return ipv4mask2cidr(mask)
348+ Supports ipv4 or ipv6 netmasks."""
349+ try:
350+ # if 'mask' is a prefix that is an integer.
351+ # then just return it.
352+ return int(mask)
353+ except ValueError:
354+ pass
355+ if is_ipv6_addr(mask):
356+ return ipv6_mask_to_net_prefix(mask)
357 else:
358- return mask
359+ return ipv4_mask_to_net_prefix(mask)
360+
361
362 # vi: ts=4 expandtab
363diff --git a/cloudinit/net/sysconfig.py b/cloudinit/net/sysconfig.py
364index f7d4548..5d9b3d1 100644
365--- a/cloudinit/net/sysconfig.py
366+++ b/cloudinit/net/sysconfig.py
367@@ -9,7 +9,7 @@ from cloudinit.distros.parsers import resolv_conf
368 from cloudinit import util
369
370 from . import renderer
371-from .network_state import subnet_is_ipv6
372+from .network_state import subnet_is_ipv6, net_prefix_to_ipv4_mask
373
374
375 def _make_header(sep='#'):
376@@ -26,11 +26,8 @@ def _make_header(sep='#'):
377
378
379 def _is_default_route(route):
380- if route['network'] == '::' and route['netmask'] == 0:
381- return True
382- if route['network'] == '0.0.0.0' and route['netmask'] == '0.0.0.0':
383- return True
384- return False
385+ default_nets = ('::', '0.0.0.0')
386+ return route['prefix'] == 0 and route['network'] in default_nets
387
388
389 def _quote_value(value):
390@@ -323,16 +320,10 @@ class Renderer(renderer.Renderer):
391 " " + ipv6_cidr)
392 else:
393 ipv4_index = ipv4_index + 1
394- if ipv4_index == 0:
395- iface_cfg['IPADDR'] = subnet['address']
396- if 'netmask' in subnet:
397- iface_cfg['NETMASK'] = subnet['netmask']
398- else:
399- iface_cfg['IPADDR' + str(ipv4_index)] = \
400- subnet['address']
401- if 'netmask' in subnet:
402- iface_cfg['NETMASK' + str(ipv4_index)] = \
403- subnet['netmask']
404+ suff = "" if ipv4_index == 0 else str(ipv4_index)
405+ iface_cfg['IPADDR' + suff] = subnet['address']
406+ iface_cfg['NETMASK' + suff] = \
407+ net_prefix_to_ipv4_mask(subnet['prefix'])
408
409 @classmethod
410 def _render_subnet_routes(cls, iface_cfg, route_cfg, subnets):
411diff --git a/tests/unittests/test_distros/test_netconfig.py b/tests/unittests/test_distros/test_netconfig.py
412index be9a831..83580cc 100644
413--- a/tests/unittests/test_distros/test_netconfig.py
414+++ b/tests/unittests/test_distros/test_netconfig.py
415@@ -92,10 +92,9 @@ iface lo inet loopback
416
417 auto eth0
418 iface eth0 inet static
419- address 192.168.1.5
420+ address 192.168.1.5/24
421 broadcast 192.168.1.0
422 gateway 192.168.1.254
423- netmask 255.255.255.0
424
425 auto eth1
426 iface eth1 inet dhcp
427@@ -156,7 +155,7 @@ network:
428 ethernets:
429 eth7:
430 addresses:
431- - 192.168.1.5/255.255.255.0
432+ - 192.168.1.5/24
433 gateway4: 192.168.1.254
434 eth9:
435 dhcp4: true
436diff --git a/tests/unittests/test_net.py b/tests/unittests/test_net.py
437index 0a88caf..91e5fb5 100644
438--- a/tests/unittests/test_net.py
439+++ b/tests/unittests/test_net.py
440@@ -334,17 +334,15 @@ iface lo inet loopback
441
442 auto eth0
443 iface eth0 inet static
444- address 1.2.3.12
445+ address 1.2.3.12/29
446 broadcast 1.2.3.15
447 dns-nameservers 69.9.160.191 69.9.191.4
448 gateway 1.2.3.9
449- netmask 255.255.255.248
450
451 auto eth1
452 iface eth1 inet static
453- address 10.248.2.4
454+ address 10.248.2.4/29
455 broadcast 10.248.2.7
456- netmask 255.255.255.248
457 """.lstrip()
458
459 NETWORK_CONFIGS = {

Subscribers

People subscribed via source and target branches

to all changes: