Merge ~raharper/cloud-init:fix/net-get-devices-ignore-devs-with-master into cloud-init:master

Proposed by Ryan Harper on 2019-09-16
Status: Merged
Approved by: Chad Smith on 2019-09-17
Approved revision: 15ffe027cb53d162bc1f436e0f50b8b16c796484
Merge reported by: Server Team CI bot
Merged at revision: not available
Proposed branch: ~raharper/cloud-init:fix/net-get-devices-ignore-devs-with-master
Merge into: cloud-init:master
Diff against target: 77 lines (+25/-2)
2 files modified
cloudinit/net/__init__.py (+8/-2)
cloudinit/net/tests/test_init.py (+17/-0)
Reviewer Review Type Date Requested Status
Chad Smith 2019-09-16 Approve on 2019-09-17
Dan Watkins Approve on 2019-09-17
Server Team CI bot continuous-integration Approve on 2019-09-17
Review via email: mp+372828@code.launchpad.net

Commit message

net: add is_master check for filtering device list

Some network devices are transformed into a bond via kernel magic
and do not have the 'bonding' sysfs attribute, but like a bond they
have a duplicate MAC of other bond members. On Azure Advanced
Networking SRIOV devices are auto bonded and will have the same MAC
as the HyperV nic. We can detect this via the 'master' sysfs attribute
in the device sysfs path and this patch adds this to the list of devices
we ignore when enumerating device lists.

LP: #1844191

To post a comment you must log in.

PASSED: Continuous integration, rev:17b2058017074424fa8b87c0a1eea5547935a2a7
https://jenkins.ubuntu.com/server/job/cloud-init-ci/1143/
Executed test runs:
    SUCCESS: Checkout
    SUCCESS: Unit & Style Tests
    SUCCESS: Ubuntu LTS: Build
    SUCCESS: Ubuntu LTS: Integration
    IN_PROGRESS: Declarative: Post Actions

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/server/job/cloud-init-ci/1143//rebuild

review: Approve (continuous-integration)
Chad Smith (chad.smith) wrote :

A minor request for unit test here. otherwise +1. https://paste.ubuntu.com/p/6wMR7WGT9f/

acb65b5... by Ryan Harper on 2019-09-17

Add unittest to verify get_interfaces_by_mac skips master devs

Ryan Harper (raharper) wrote :

Thanks Chad, that's a great one.

PASSED: Continuous integration, rev:acb65b5c2344961ae04660f366aa3444b8b337cc
https://jenkins.ubuntu.com/server/job/cloud-init-ci/1144/
Executed test runs:
    SUCCESS: Checkout
    SUCCESS: Unit & Style Tests
    SUCCESS: Ubuntu LTS: Build
    SUCCESS: Ubuntu LTS: Integration
    IN_PROGRESS: Declarative: Post Actions

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/server/job/cloud-init-ci/1144//rebuild

review: Approve (continuous-integration)
15ffe02... by Ryan Harper on 2019-09-17

Better name is has_master since the interface has the 'master' sysfs attr file

PASSED: Continuous integration, rev:15ffe027cb53d162bc1f436e0f50b8b16c796484
https://jenkins.ubuntu.com/server/job/cloud-init-ci/1145/
Executed test runs:
    SUCCESS: Checkout
    SUCCESS: Unit & Style Tests
    SUCCESS: Ubuntu LTS: Build
    SUCCESS: Ubuntu LTS: Integration
    IN_PROGRESS: Declarative: Post Actions

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/server/job/cloud-init-ci/1145//rebuild

review: Approve (continuous-integration)
review: Approve
Chad Smith (chad.smith) wrote :

+1

Chad Smith (chad.smith) :
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 0eb952f..5de5c6d 100644
3--- a/cloudinit/net/__init__.py
4+++ b/cloudinit/net/__init__.py
5@@ -109,6 +109,10 @@ def is_bond(devname):
6 return os.path.exists(sys_dev_path(devname, "bonding"))
7
8
9+def has_master(devname):
10+ return os.path.exists(sys_dev_path(devname, path="master"))
11+
12+
13 def is_netfailover(devname, driver=None):
14 """ netfailover driver uses 3 nics, master, primary and standby.
15 this returns True if the device is either the primary or standby
16@@ -154,7 +158,7 @@ def is_netfail_master(devname, driver=None):
17
18 Return True if all of the above is True.
19 """
20- if os.path.exists(sys_dev_path(devname, path='master')):
21+ if has_master(devname):
22 return False
23
24 if driver is None:
25@@ -211,7 +215,7 @@ def is_netfail_standby(devname, driver=None):
26
27 Return True if all of the above is True.
28 """
29- if not os.path.exists(sys_dev_path(devname, path='master')):
30+ if not has_master(devname):
31 return False
32
33 if driver is None:
34@@ -786,6 +790,8 @@ def get_interfaces():
35 continue
36 if is_bond(name):
37 continue
38+ if has_master(name):
39+ continue
40 if is_netfailover(name):
41 continue
42 mac = get_interface_mac(name)
43diff --git a/cloudinit/net/tests/test_init.py b/cloudinit/net/tests/test_init.py
44index 7259dbe..999db98 100644
45--- a/cloudinit/net/tests/test_init.py
46+++ b/cloudinit/net/tests/test_init.py
47@@ -157,6 +157,12 @@ class TestReadSysNet(CiTestCase):
48 ensure_file(os.path.join(self.sysdir, 'eth0', 'bonding'))
49 self.assertTrue(net.is_bond('eth0'))
50
51+ def test_has_master(self):
52+ """has_master is True when /sys/net/devname/master exists."""
53+ self.assertFalse(net.has_master('enP1s1'))
54+ ensure_file(os.path.join(self.sysdir, 'enP1s1', 'master'))
55+ self.assertTrue(net.has_master('enP1s1'))
56+
57 def test_is_vlan(self):
58 """is_vlan is True when /sys/net/devname/uevent has DEVTYPE=vlan."""
59 ensure_file(os.path.join(self.sysdir, 'eth0', 'uevent'))
60@@ -424,6 +430,17 @@ class TestGetInterfaceMAC(CiTestCase):
61 expected = [('eth2', 'aa:bb:cc:aa:bb:cc', None, None)]
62 self.assertEqual(expected, net.get_interfaces())
63
64+ def test_get_interfaces_by_mac_skips_master_devs(self):
65+ """Ignore interfaces with a master device which would have dup mac."""
66+ mac1 = mac2 = 'aa:bb:cc:aa:bb:cc'
67+ write_file(os.path.join(self.sysdir, 'eth1', 'addr_assign_type'), '0')
68+ write_file(os.path.join(self.sysdir, 'eth1', 'address'), mac1)
69+ write_file(os.path.join(self.sysdir, 'eth1', 'master'), "blah")
70+ write_file(os.path.join(self.sysdir, 'eth2', 'addr_assign_type'), '0')
71+ write_file(os.path.join(self.sysdir, 'eth2', 'address'), mac2)
72+ expected = [('eth2', mac2, None, None)]
73+ self.assertEqual(expected, net.get_interfaces())
74+
75 @mock.patch('cloudinit.net.is_netfailover')
76 def test_get_interfaces_by_mac_skips_netfailvoer(self, m_netfail):
77 """Ignore interfaces if netfailover primary or standby."""

Subscribers

People subscribed via source and target branches