Merge ~ma-brothier/cloud-init:interfaces-order into cloud-init:master

Proposed by Marc-Aurele Brothier
Status: Merged
Approved by: Scott Moser
Approved revision: 0f705cce22f6028431b8593171369f98a98497c5
Merged at revision: 543e25cda6235d18adb6485e4266944d59e1979d
Proposed branch: ~ma-brothier/cloud-init:interfaces-order
Merge into: cloud-init:master
Diff against target: 63 lines (+26/-1)
2 files modified
cloudinit/net/__init__.py (+12/-1)
tests/unittests/test_net.py (+14/-0)
Reviewer Review Type Date Requested Status
Scott Moser Approve
Server Team CI bot continuous-integration Approve
Review via email: mp+324535@code.launchpad.net

Description of the change

Natural sort order of interfaces

The code deciding which interface to choose as the default to request the IP address through DHCP does not sort the interfaces correctly. On Ubuntu Xenial images for example, the interfaces are named ens1, ens2, ens3..., ens11, ... depending on the pci bus address. The python sorting will list 'ens11' before 'ens3' for example despite the fact that 'ens3' should be before 'ens11'.

This patch address this issue and sort the interface names according to a human sorting.

To post a comment you must log in.
Revision history for this message
Scott Moser (smoser) wrote :

Marc,

This looks good. Some comments... they're all minor, and should be explained more in HACKING.rst
http://cloudinit.readthedocs.io/en/latest/topics/hacking.html

a.) change description to be
    Summary
    <blank line>
    Description

b.) lets go ahead and just keep the function in a net/__init__.py rather than putting it in util. And name it _natural_sort_key(). At some point if someone else needs such a function we can move it to util.

c.) a unit test would be good. Just add something to tests/unittests/test_net.py to test your _natural_key_sort. Also realize you can (and do) get interfaces named 'enp0s25' (as I have on this system).

d.) You'll need to sign the contributors agreement (see HACKING.rst). Please feel free to ping me if you have any questions.

e.) Thanks!

d.) Thanks!

Revision history for this message
Scott Moser (smoser) :
review: Needs Fixing
Revision history for this message
Marc-Aurele Brothier (ma-brothier) wrote :

Thanks Scott for the comments.

a) done

b) change done

c) the tests don't run on my OSX, they got stuck in test_datasource/test_azure.py", line 333, in test_password_given. Can you give me a hint how to run only those unittests?

d) done

Dunno what's e) and f) but glad for the "thanks".
I must say the user experience on launchpad is quite an adventure to figure out things.

About the test: I created a virtualenv with py2.7 and installed the requirements. So far if I run this command I got this error:
python -m nose test_net.py
ERROR: Failure: ImportError (cannot import name renderers)

0f705cc... by Marc-Aurele Brothier

Add simple test for natural sorting

Revision history for this message
Server Team CI bot (server-team-bot) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Server Team CI bot (server-team-bot) wrote :
review: Approve (continuous-integration)
Revision history for this message
Scott Moser (smoser) wrote :

Marc,
to run the unit tests, you need to be in the top level directory (so that it can "import cloudinit").

so:
python -m nose tests/unittests/test_net.py

Revision history for this message
Scott Moser (smoser) wrote :

thanks.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1diff --git a/cloudinit/net/__init__.py b/cloudinit/net/__init__.py
2index 8c6cd05..65accbb 100644
3--- a/cloudinit/net/__init__.py
4+++ b/cloudinit/net/__init__.py
5@@ -17,6 +17,17 @@ SYS_CLASS_NET = "/sys/class/net/"
6 DEFAULT_PRIMARY_INTERFACE = 'eth0'
7
8
9+def _natural_sort_key(s, _nsre=re.compile('([0-9]+)')):
10+ """Sorting for Humans: natural sort order. Can be use as the key to sort
11+ functions.
12+ This will sort ['eth0', 'ens3', 'ens10', 'ens12', 'ens8', 'ens0'] as
13+ ['ens0', 'ens3', 'ens8', 'ens10', 'ens12', 'eth0'] instead of the simple
14+ python way which will produce ['ens0', 'ens10', 'ens12', 'ens3', 'ens8',
15+ 'eth0']."""
16+ return [int(text) if text.isdigit() else text.lower()
17+ for text in re.split(_nsre, s)]
18+
19+
20 def sys_dev_path(devname, path=""):
21 return SYS_CLASS_NET + devname + "/" + path
22
23@@ -169,7 +180,7 @@ def generate_fallback_config():
24
25 # if eth0 exists use it above anything else, otherwise get the interface
26 # that we can read 'first' (using the sorted defintion of first).
27- names = list(sorted(potential_interfaces))
28+ names = list(sorted(potential_interfaces, key=_natural_sort_key))
29 if DEFAULT_PRIMARY_INTERFACE in names:
30 names.remove(DEFAULT_PRIMARY_INTERFACE)
31 names.insert(0, DEFAULT_PRIMARY_INTERFACE)
32diff --git a/tests/unittests/test_net.py b/tests/unittests/test_net.py
33index 5169821..9993241 100644
34--- a/tests/unittests/test_net.py
35+++ b/tests/unittests/test_net.py
36@@ -1,6 +1,7 @@
37 # This file is part of cloud-init. See LICENSE file for license information.
38
39 from cloudinit import net
40+from cloudinit.net import _natural_sort_key
41 from cloudinit.net import cmdline
42 from cloudinit.net import eni
43 from cloudinit.net import netplan
44@@ -1627,6 +1628,19 @@ class TestGetInterfacesByMac(CiTestCase):
45 self.assertEqual('lo', ret[empty_mac])
46
47
48+class TestInterfacesSorting(CiTestCase):
49+
50+ def test_natural_order(self):
51+ data = ['ens5', 'ens6', 'ens3', 'ens20', 'ens13', 'ens2']
52+ self.assertEqual(
53+ sorted(data, key=_natural_sort_key),
54+ ['ens2', 'ens3', 'ens5', 'ens6', 'ens13', 'ens20'])
55+ data2 = ['enp2s0', 'enp2s3', 'enp0s3', 'enp0s13', 'enp0s8', 'enp1s2']
56+ self.assertEqual(
57+ sorted(data2, key=_natural_sort_key),
58+ ['enp0s3', 'enp0s8', 'enp0s13', 'enp1s2', 'enp2s0', 'enp2s3'])
59+
60+
61 def _gzip_data(data):
62 with io.BytesIO() as iobuf:
63 gzfp = gzip.GzipFile(mode="wb", fileobj=iobuf)

Subscribers

People subscribed via source and target branches