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

Proposed by Ryan Harper
Status: Merged
Approved by: Chad Smith
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 Approve
Dan Watkins Approve
Server Team CI bot continuous-integration Approve
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.
Revision history for this message
Server Team CI bot (server-team-bot) wrote :

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)
Revision history for this message
Chad Smith (chad.smith) wrote :

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

acb65b5... by Ryan Harper

Add unittest to verify get_interfaces_by_mac skips master devs

Revision history for this message
Ryan Harper (raharper) wrote :

Thanks Chad, that's a great one.

Revision history for this message
Server Team CI bot (server-team-bot) wrote :

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)
Revision history for this message
Dan Watkins (oddbloke) :
15ffe02... by Ryan Harper

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

Revision history for this message
Server Team CI bot (server-team-bot) wrote :

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)
Revision history for this message
Dan Watkins (oddbloke) :
review: Approve
Revision history for this message
Chad Smith (chad.smith) wrote :

+1

Revision history for this message
Chad Smith (chad.smith) :
review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
diff --git a/cloudinit/net/__init__.py b/cloudinit/net/__init__.py
index 0eb952f..5de5c6d 100644
--- a/cloudinit/net/__init__.py
+++ b/cloudinit/net/__init__.py
@@ -109,6 +109,10 @@ def is_bond(devname):
109 return os.path.exists(sys_dev_path(devname, "bonding"))109 return os.path.exists(sys_dev_path(devname, "bonding"))
110110
111111
112def has_master(devname):
113 return os.path.exists(sys_dev_path(devname, path="master"))
114
115
112def is_netfailover(devname, driver=None):116def is_netfailover(devname, driver=None):
113 """ netfailover driver uses 3 nics, master, primary and standby.117 """ netfailover driver uses 3 nics, master, primary and standby.
114 this returns True if the device is either the primary or standby118 this returns True if the device is either the primary or standby
@@ -154,7 +158,7 @@ def is_netfail_master(devname, driver=None):
154158
155 Return True if all of the above is True.159 Return True if all of the above is True.
156 """160 """
157 if os.path.exists(sys_dev_path(devname, path='master')):161 if has_master(devname):
158 return False162 return False
159163
160 if driver is None:164 if driver is None:
@@ -211,7 +215,7 @@ def is_netfail_standby(devname, driver=None):
211215
212 Return True if all of the above is True.216 Return True if all of the above is True.
213 """217 """
214 if not os.path.exists(sys_dev_path(devname, path='master')):218 if not has_master(devname):
215 return False219 return False
216220
217 if driver is None:221 if driver is None:
@@ -786,6 +790,8 @@ def get_interfaces():
786 continue790 continue
787 if is_bond(name):791 if is_bond(name):
788 continue792 continue
793 if has_master(name):
794 continue
789 if is_netfailover(name):795 if is_netfailover(name):
790 continue796 continue
791 mac = get_interface_mac(name)797 mac = get_interface_mac(name)
diff --git a/cloudinit/net/tests/test_init.py b/cloudinit/net/tests/test_init.py
index 7259dbe..999db98 100644
--- a/cloudinit/net/tests/test_init.py
+++ b/cloudinit/net/tests/test_init.py
@@ -157,6 +157,12 @@ class TestReadSysNet(CiTestCase):
157 ensure_file(os.path.join(self.sysdir, 'eth0', 'bonding'))157 ensure_file(os.path.join(self.sysdir, 'eth0', 'bonding'))
158 self.assertTrue(net.is_bond('eth0'))158 self.assertTrue(net.is_bond('eth0'))
159159
160 def test_has_master(self):
161 """has_master is True when /sys/net/devname/master exists."""
162 self.assertFalse(net.has_master('enP1s1'))
163 ensure_file(os.path.join(self.sysdir, 'enP1s1', 'master'))
164 self.assertTrue(net.has_master('enP1s1'))
165
160 def test_is_vlan(self):166 def test_is_vlan(self):
161 """is_vlan is True when /sys/net/devname/uevent has DEVTYPE=vlan."""167 """is_vlan is True when /sys/net/devname/uevent has DEVTYPE=vlan."""
162 ensure_file(os.path.join(self.sysdir, 'eth0', 'uevent'))168 ensure_file(os.path.join(self.sysdir, 'eth0', 'uevent'))
@@ -424,6 +430,17 @@ class TestGetInterfaceMAC(CiTestCase):
424 expected = [('eth2', 'aa:bb:cc:aa:bb:cc', None, None)]430 expected = [('eth2', 'aa:bb:cc:aa:bb:cc', None, None)]
425 self.assertEqual(expected, net.get_interfaces())431 self.assertEqual(expected, net.get_interfaces())
426432
433 def test_get_interfaces_by_mac_skips_master_devs(self):
434 """Ignore interfaces with a master device which would have dup mac."""
435 mac1 = mac2 = 'aa:bb:cc:aa:bb:cc'
436 write_file(os.path.join(self.sysdir, 'eth1', 'addr_assign_type'), '0')
437 write_file(os.path.join(self.sysdir, 'eth1', 'address'), mac1)
438 write_file(os.path.join(self.sysdir, 'eth1', 'master'), "blah")
439 write_file(os.path.join(self.sysdir, 'eth2', 'addr_assign_type'), '0')
440 write_file(os.path.join(self.sysdir, 'eth2', 'address'), mac2)
441 expected = [('eth2', mac2, None, None)]
442 self.assertEqual(expected, net.get_interfaces())
443
427 @mock.patch('cloudinit.net.is_netfailover')444 @mock.patch('cloudinit.net.is_netfailover')
428 def test_get_interfaces_by_mac_skips_netfailvoer(self, m_netfail):445 def test_get_interfaces_by_mac_skips_netfailvoer(self, m_netfail):
429 """Ignore interfaces if netfailover primary or standby."""446 """Ignore interfaces if netfailover primary or standby."""

Subscribers

People subscribed via source and target branches