cloud-init

Merge ~oddbloke/cloud-init/+git/cloud-init:ubuntu/xenial into cloud-init:ubuntu/xenial

  1. Git
  2. lp:~oddbloke/cloud-init/+git/cloud-init
  3. ubuntu/xenial
  4. Merge into ubuntu/xenial
Proposed by Dan Watkins
Status: Merged
Merged at revision: d6e008acc2f79e8bae486c4fdcba70256ebe6e87
Proposed branch: ~oddbloke/cloud-init/+git/cloud-init:ubuntu/xenial
Merge into: cloud-init:ubuntu/xenial
Diff against target: 180 lines (+158/-0)
3 files modified
debian/changelog (+7/-0)
debian/patches/cpick-a7d8d032-get_interfaces-don-t-exclude-bridge-and-bond-members (+150/-0)
debian/patches/series (+1/-0)
Reviewer Review Type Date Requested Status
Server Team CI bot continuous-integration Needs Fixing
Chad Smith Approve
Review via email: mp+373660@code.launchpad.net
To post a comment you must log in.
Revision history for this message
Chad Smith (chad.smith) wrote :

Only deb/changelog metadata diffs https://paste.ubuntu.com/p/PpzdPqrpNS/
tox works, package builds, contains the right content.

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

No commit message was specified in the merge proposal. Click on the following link and set the commit message (if you want jenkins to rebuild you need to trigger it yourself):
https://code.launchpad.net/~daniel-thewatkins/cloud-init/+git/cloud-init/+merge/373660/+edit-commit-message
https://jenkins.ubuntu.com/server/job/cloud-init-ci/1196/
Executed test runs:
    SUCCESS: Checkout
    FAILED: Unit & Style Tests

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

review: Needs Fixing (continuous-integration)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
diff --git a/debian/changelog b/debian/changelog
index 5b03045..ecd454e 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -1,3 +1,10 @@
1cloud-init (19.2-36-g059d049c-0ubuntu2~16.04.1) xenial; urgency=medium
2
3 * cherry-pick a7d8d032: get_interfaces: don't exclude bridge and bond
4 members (LP: #1846535)
5
6 -- Daniel Watkins <oddbloke@ubuntu.com> Fri, 04 Oct 2019 12:01:19 -0400
7
1cloud-init (19.2-36-g059d049c-0ubuntu1~16.04.1) xenial; urgency=medium8cloud-init (19.2-36-g059d049c-0ubuntu1~16.04.1) xenial; urgency=medium
29
3 * New upstream snapshot. (LP: #1844334)10 * New upstream snapshot. (LP: #1844334)
diff --git a/debian/patches/cpick-a7d8d032-get_interfaces-don-t-exclude-bridge-and-bond-members b/debian/patches/cpick-a7d8d032-get_interfaces-don-t-exclude-bridge-and-bond-members
4new file mode 10064411new file mode 100644
index 0000000..4415fcb
--- /dev/null
+++ b/debian/patches/cpick-a7d8d032-get_interfaces-don-t-exclude-bridge-and-bond-members
@@ -0,0 +1,150 @@
1From a7d8d032a65e807007f1467a9220b7f161625668 Mon Sep 17 00:00:00 2001
2From: Daniel Watkins <oddbloke@ubuntu.com>
3Date: Fri, 4 Oct 2019 15:34:41 +0000
4Subject: [PATCH] get_interfaces: don't exclude bridge and bond members
5
6The change that introduced this issue was handling interfaces that are
7bonded in the kernel, in a way that doesn't present as "a bond" to
8userspace in the normal way. Both members of this "bond" will share a
9MAC address, so we filter one of them out to avoid incorrect MAC address
10collision warnings.
11
12Unfortunately, the matching condition was too broad, so that change also
13affected normal bonds and bridges. This change specifically excludes
14bonds and bridges from that determination, to address that regression.
15
16LP: #1846535
17---
18 cloudinit/net/__init__.py | 24 ++++++++++---
19 cloudinit/net/tests/test_init.py | 59 +++++++++++++++++++++++++++++---
20 2 files changed, 73 insertions(+), 10 deletions(-)
21
22--- a/cloudinit/net/__init__.py
23+++ b/cloudinit/net/__init__.py
24@@ -109,8 +109,22 @@ def is_bond(devname):
25 return os.path.exists(sys_dev_path(devname, "bonding"))
26
27
28-def has_master(devname):
29- return os.path.exists(sys_dev_path(devname, path="master"))
30+def get_master(devname):
31+ """Return the master path for devname, or None if no master"""
32+ path = sys_dev_path(devname, path="master")
33+ if os.path.exists(path):
34+ return path
35+ return None
36+
37+
38+def master_is_bridge_or_bond(devname):
39+ """Return a bool indicating if devname's master is a bridge or bond"""
40+ master_path = get_master(devname)
41+ if master_path is None:
42+ return False
43+ bonding_path = os.path.join(master_path, "bonding")
44+ bridge_path = os.path.join(master_path, "bridge")
45+ return (os.path.exists(bonding_path) or os.path.exists(bridge_path))
46
47
48 def is_netfailover(devname, driver=None):
49@@ -158,7 +172,7 @@ def is_netfail_master(devname, driver=No
50
51 Return True if all of the above is True.
52 """
53- if has_master(devname):
54+ if get_master(devname) is not None:
55 return False
56
57 if driver is None:
58@@ -215,7 +229,7 @@ def is_netfail_standby(devname, driver=N
59
60 Return True if all of the above is True.
61 """
62- if not has_master(devname):
63+ if get_master(devname) is None:
64 return False
65
66 if driver is None:
67@@ -790,7 +804,7 @@ def get_interfaces():
68 continue
69 if is_bond(name):
70 continue
71- if has_master(name):
72+ if get_master(name) is not None and not master_is_bridge_or_bond(name):
73 continue
74 if is_netfailover(name):
75 continue
76--- a/cloudinit/net/tests/test_init.py
77+++ b/cloudinit/net/tests/test_init.py
78@@ -157,11 +157,40 @@ class TestReadSysNet(CiTestCase):
79 ensure_file(os.path.join(self.sysdir, 'eth0', 'bonding'))
80 self.assertTrue(net.is_bond('eth0'))
81
82- def test_has_master(self):
83- """has_master is True when /sys/net/devname/master exists."""
84- self.assertFalse(net.has_master('enP1s1'))
85- ensure_file(os.path.join(self.sysdir, 'enP1s1', 'master'))
86- self.assertTrue(net.has_master('enP1s1'))
87+ def test_get_master(self):
88+ """get_master returns the path when /sys/net/devname/master exists."""
89+ self.assertIsNone(net.get_master('enP1s1'))
90+ master_path = os.path.join(self.sysdir, 'enP1s1', 'master')
91+ ensure_file(master_path)
92+ self.assertEqual(master_path, net.get_master('enP1s1'))
93+
94+ def test_master_is_bridge_or_bond(self):
95+ bridge_mac = 'aa:bb:cc:aa:bb:cc'
96+ bond_mac = 'cc:bb:aa:cc:bb:aa'
97+
98+ # No master => False
99+ write_file(os.path.join(self.sysdir, 'eth1', 'address'), bridge_mac)
100+ write_file(os.path.join(self.sysdir, 'eth2', 'address'), bond_mac)
101+
102+ self.assertFalse(net.master_is_bridge_or_bond('eth1'))
103+ self.assertFalse(net.master_is_bridge_or_bond('eth2'))
104+
105+ # masters without bridge/bonding => False
106+ write_file(os.path.join(self.sysdir, 'br0', 'address'), bridge_mac)
107+ write_file(os.path.join(self.sysdir, 'bond0', 'address'), bond_mac)
108+
109+ os.symlink('../br0', os.path.join(self.sysdir, 'eth1', 'master'))
110+ os.symlink('../bond0', os.path.join(self.sysdir, 'eth2', 'master'))
111+
112+ self.assertFalse(net.master_is_bridge_or_bond('eth1'))
113+ self.assertFalse(net.master_is_bridge_or_bond('eth2'))
114+
115+ # masters with bridge/bonding => True
116+ write_file(os.path.join(self.sysdir, 'br0', 'bridge'), '')
117+ write_file(os.path.join(self.sysdir, 'bond0', 'bonding'), '')
118+
119+ self.assertTrue(net.master_is_bridge_or_bond('eth1'))
120+ self.assertTrue(net.master_is_bridge_or_bond('eth2'))
121
122 def test_is_vlan(self):
123 """is_vlan is True when /sys/net/devname/uevent has DEVTYPE=vlan."""
124@@ -461,6 +490,26 @@ class TestGetInterfaceMAC(CiTestCase):
125 expected = [('ens3', mac, None, None)]
126 self.assertEqual(expected, net.get_interfaces())
127
128+ def test_get_interfaces_does_not_skip_phys_members_of_bridges_and_bonds(
129+ self
130+ ):
131+ bridge_mac = 'aa:bb:cc:aa:bb:cc'
132+ bond_mac = 'cc:bb:aa:cc:bb:aa'
133+ write_file(os.path.join(self.sysdir, 'br0', 'address'), bridge_mac)
134+ write_file(os.path.join(self.sysdir, 'br0', 'bridge'), '')
135+
136+ write_file(os.path.join(self.sysdir, 'bond0', 'address'), bond_mac)
137+ write_file(os.path.join(self.sysdir, 'bond0', 'bonding'), '')
138+
139+ write_file(os.path.join(self.sysdir, 'eth1', 'address'), bridge_mac)
140+ os.symlink('../br0', os.path.join(self.sysdir, 'eth1', 'master'))
141+
142+ write_file(os.path.join(self.sysdir, 'eth2', 'address'), bond_mac)
143+ os.symlink('../bond0', os.path.join(self.sysdir, 'eth2', 'master'))
144+
145+ interface_names = [interface[0] for interface in net.get_interfaces()]
146+ self.assertEqual(['eth1', 'eth2'], sorted(interface_names))
147+
148
149 class TestInterfaceHasOwnMAC(CiTestCase):
150
diff --git a/debian/patches/series b/debian/patches/series
index 5d6995e..ace99df 100644
--- a/debian/patches/series
+++ b/debian/patches/series
@@ -5,3 +5,4 @@ openstack-no-network-config.patch
5azure-apply-network-config-false.patch5azure-apply-network-config-false.patch
6ec2-classic-dont-reapply-networking.patch6ec2-classic-dont-reapply-networking.patch
7ubuntu-advantage-revert-tip.patch7ubuntu-advantage-revert-tip.patch
8cpick-a7d8d032-get_interfaces-don-t-exclude-bridge-and-bond-members

Subscribers

People subscribed via source and target branches