Skip interfaces not in sys_interfaces
Change I30ce988fdb9459267b565b67e8bbce136fcd0249 introduced a subtle change to these loops where interfaces that aren't in sys_interfaces are not skipped. It resulted in falling through and causing KeyError's later when the non-existent MAC is looked up. It seems the idea is that if none of the raw_macs for the interface we are looking at (which, for a single, non-bonded, interface is just it's MAC) are in the sys_interfaces list, we should skip configuration. TBH, it is still not totally clear to me what the interaction between raw_macs and sys_interfaces is, particularly if you do something like restrict sys_interfaces to just one interface of a bonded interface or somethinglike that (the answer probably is -- don't do that). This problem particularly arises under systemd were we are called for each interface. A test-case is added to exercise that path. (Based on pabelanger's I049eeac6845692d20c6f18d3c2fec3fcb8e4ec34) Change-Id: I998655808b74eb840181764ee347f2f3cad1a7e3
This commit is contained in:
parent
770164e922
commit
80b98f8ae5
24
glean/cmd.py
24
glean/cmd.py
|
@ -129,10 +129,12 @@ def write_redhat_interfaces(interfaces, sys_interfaces):
|
|||
interfaces.items(), key=lambda x: x[1]['id']):
|
||||
if interface['type'] == 'ipv6':
|
||||
continue
|
||||
# sys_interfaces is pruned by --interface; if one of the
|
||||
# raw_macs (or, *the* MAC for single interfaces) does not
|
||||
# match as one of the interfaces we want configured, skip
|
||||
raw_macs = interface.get('raw_macs', [interface['mac_address']])
|
||||
for mac in raw_macs:
|
||||
if mac not in sys_interfaces:
|
||||
continue
|
||||
if not set(sys_interfaces).intersection(set(raw_macs)):
|
||||
continue
|
||||
|
||||
if 'vlan_id' in interface:
|
||||
# raw_macs will have a single entry if the vlan device is a
|
||||
|
@ -286,10 +288,12 @@ def write_gentoo_interfaces(interfaces, sys_interfaces):
|
|||
interfaces.items(), key=lambda x: x[1]['id']):
|
||||
if interface['type'] == 'ipv6':
|
||||
continue
|
||||
# sys_interfaces is pruned by --interface; if one of the
|
||||
# raw_macs (or, *the* MAC for single interfaces) does not
|
||||
# match as one of the interfaces we want configured, skip
|
||||
raw_macs = interface.get('raw_macs', [interface['mac_address']])
|
||||
for mac in raw_macs:
|
||||
if mac not in sys_interfaces:
|
||||
continue
|
||||
if not set(sys_interfaces).intersection(set(raw_macs)):
|
||||
continue
|
||||
|
||||
if 'bond_mode' in interface:
|
||||
interface['slaves'] = [
|
||||
|
@ -357,10 +361,12 @@ def write_debian_interfaces(interfaces, sys_interfaces):
|
|||
files_to_write[eni_path] += "source /etc/network/interfaces.d/*.cfg\n"
|
||||
# Sort the interfaces by id so that we'll have consistent output order
|
||||
for iname, interface in interfaces.items():
|
||||
# sys_interfaces is pruned by --interface; if one of the
|
||||
# raw_macs (or, *the* MAC for single interfaces) does not
|
||||
# match as one of the interfaces we want configured, skip
|
||||
raw_macs = interface.get('raw_macs', [interface['mac_address']])
|
||||
for mac in raw_macs:
|
||||
if mac not in sys_interfaces:
|
||||
continue
|
||||
if not set(sys_interfaces).intersection(set(raw_macs)):
|
||||
continue
|
||||
|
||||
vlan_raw_device = None
|
||||
if 'vlan_id' in interface:
|
||||
|
|
|
@ -153,8 +153,11 @@ class TestGlean(base.BaseTestCase):
|
|||
|
||||
self.useFixture(fixtures.MonkeyPatch('platform.dist', fake_distro))
|
||||
|
||||
def _assert_distro_provider(self, distro, provider):
|
||||
self._patch_argv(['--hostname'])
|
||||
def _assert_distro_provider(self, distro, provider, interface=None):
|
||||
argv = ['--hostname']
|
||||
if interface:
|
||||
argv.append('--interface=%s' % interface)
|
||||
self._patch_argv(argv)
|
||||
self._patch_files(provider)
|
||||
self._patch_distro(distro)
|
||||
|
||||
|
@ -188,6 +191,8 @@ class TestGlean(base.BaseTestCase):
|
|||
write_blocks.append((write_dest, write_content))
|
||||
|
||||
for dest, content in write_blocks:
|
||||
if interface and interface not in dest:
|
||||
continue
|
||||
self.assertNotIn("eth2", dest)
|
||||
self.assertIn(dest, self.file_handle_mocks)
|
||||
write_handle = self.file_handle_mocks[dest].write
|
||||
|
@ -225,3 +230,10 @@ class TestGlean(base.BaseTestCase):
|
|||
def test_glean(self):
|
||||
with mock.patch('glean.systemlock.Lock'):
|
||||
self._assert_distro_provider(self.distro, self.style)
|
||||
|
||||
# In the systemd case, we are a templated unit file
|
||||
# (glean@.service) so we get called once for each interface that
|
||||
# comes up with "--interface". This simulates that.
|
||||
def test_glean_systemd(self):
|
||||
with mock.patch('glean.systemlock.Lock'):
|
||||
self._assert_distro_provider(self.distro, self.style, 'eth0')
|
||||
|
|
Loading…
Reference in New Issue