diff options
| author | Christian Breunig <christian@breunig.cc> | 2026-10-31 11:51:09 +0100 |
|---|---|---|
| committer | GitHub <noreply@github.com> | 2026-10-31 11:51:09 +0100 |
| commit | dfd2b46ff41af4edf2fd4a2a1b9fae1efe48c145 (patch) | |
| tree | 9c5538ea145c2a869bdf379398219aa74730bd86 /src/tests | |
| parent | c15665309e2991e6de5f99bf375de83d167c4248 (diff) | |
| parent | 6bef88ed9c11210855689bd78a39ee032fb27731 (diff) | |
| download | vyos-1x-dfd2b46ff41af4edf2fd4a2a1b9fae1efe48c145.tar.gz vyos-1x-dfd2b46ff41af4edf2fd4a2a1b9fae1efe48c145.zip | |
Merge pull request #5479 from c-po/T3871-ifname-store
T3871: bind interface names to hardware slots instead of hw-id
Diffstat (limited to 'src/tests')
| -rw-r--r-- | src/tests/test_ifname_store.py | 1317 | ||||
| -rw-r--r-- | src/tests/test_migration_interfaces_34_to_35.py | 196 | ||||
| -rw-r--r-- | src/tests/test_net_name_resolve.py | 1738 |
3 files changed, 1761 insertions, 1490 deletions
diff --git a/src/tests/test_ifname_store.py b/src/tests/test_ifname_store.py new file mode 100644 index 000000000..40a975ba5 --- /dev/null +++ b/src/tests/test_ifname_store.py @@ -0,0 +1,1317 @@ +# Copyright VyOS maintainers and contributors <maintainers@vyos.io> +# +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License version 2 or later as +# published by the Free Software Foundation. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see <http://www.gnu.org/licenses/>. +# +# Coverage for T3871: slot-anchored persistent interface naming. +# +# The regression classes at the bottom replay, verbatim, the field +# reproductions from the review of vyos-1x PR #5417 - the cases where the +# config-tree-driven resolver bound a configured address to the wrong +# physical wire. Under the store-based design none of them have a code path +# to travel, and these tests exist to keep it that way. + +import json +import os +import shutil +import tempfile +import unittest +from pathlib import Path +from unittest import mock + +from vyos.ifconfig.ifname_store import STORE_VERSION +from vyos.ifconfig.ifname_store import canonical_sort_key +from vyos.ifconfig.ifname_store import config_dir_is_mounted +from vyos.ifconfig.ifname_store import device_matches +from vyos.ifconfig.ifname_store import ambiguous_keys +from vyos.ifconfig.ifname_store import hardware_key +from vyos.ifconfig.ifname_store import discover_devices +from vyos.ifconfig.ifname_store import empty_store +from vyos.ifconfig.ifname_store import load_store +from vyos.ifconfig.ifname_store import port_label_index +from vyos.ifconfig.ifname_store import is_vmware_nic +from vyos.ifconfig.ifname_store import _LABEL_UNKNOWN +from vyos.ifconfig.ifname_store import permanent_mac +from vyos.ifconfig.ifname_store import render_link +from vyos.ifconfig.ifname_store import resolve +from vyos.ifconfig.ifname_store import sync_link_files +from vyos.ifconfig.ifname_store import save_store + + +def dev(name, path, mac, wireless=False, bus='pci', label=None): + """A device sitting at 'path'. Its key is what udev would report for it - + here the raw ID_PATH, which is what a plain virtio or e1000e NIC with no + firmware slot index ends up keyed on.""" + properties = {'ID_PATH': path, 'ID_BUS': bus} if path else {} + device = {'name': name, 'mac': mac, 'wireless': wireless, + 'properties': properties, + 'key': hardware_key(properties)} + if label is not None: + device['label_index'] = label + return device + + +def store_of(mapping, hardware=None): + """mapping: {name: path} - the slot each name is recorded against""" + return { + 'version': STORE_VERSION, + 'interfaces': {n: f'ID_PATH={p}' for n, p in mapping.items()}, + 'hardware': dict(hardware or {}), + } + + +class TestCanonicalOrder(unittest.TestCase): + def test_pci_sorted_numerically_not_lexically(self): + # 0000:09:00.0 must precede 0000:10:00.0 - string order gets this wrong + a = dev('x', 'pci-0000:00:09.0', 'aa:00:00:00:00:01') + b = dev('y', 'pci-0000:00:10.0', 'aa:00:00:00:00:02') + self.assertLess(canonical_sort_key(a), canonical_sort_key(b)) + + def test_pci_before_usb(self): + pci = dev('x', 'pci-0000:00:1f.6', 'aa:00:00:00:00:01') + usb = dev('y', 'usb-0:1:1.0', 'aa:00:00:00:00:00', bus='usb') + self.assertLess(canonical_sort_key(pci), canonical_sort_key(usb)) + + def test_order_is_independent_of_mac(self): + # the whole point: a numerically higher MAC in a lower slot still wins + low_slot = dev('x', 'pci-0000:00:02.0', 'ff:ff:ff:ff:ff:ff') + high_slot = dev('y', 'pci-0000:00:05.0', '00:00:00:00:00:01') + self.assertLess(canonical_sort_key(low_slot), canonical_sort_key(high_slot)) + + +class TestHypervisorAdapterOrder(unittest.TestCase): + """ESXi does not lay its adapters out in PCI address order. With four or + more it puts the fourth on a root port which sorts ahead of the first + three, so naming by PCI address alone made Network adapter 4 eth0. On the + OVA that moves the WAN: cloud-init takes eth0 from the OVF properties. + Reported against ESXi 8.0.3, vmx-21. + """ + + # the two interfaces from the report, verbatim + ADAPTER_4 = ('pci-0000:04:00.0', '00:0c:29:60:0e:5c', 3) # Ethernet3 + ADAPTER_1 = ('pci-0000:0b:00.0', '00:0c:29:60:0e:3e', 0) # Ethernet0 + + def test_the_first_adapter_sorts_first_despite_its_pci_address(self): + a4 = dev('eth0', self.ADAPTER_4[0], self.ADAPTER_4[1], label=self.ADAPTER_4[2]) + a1 = dev('eth1', self.ADAPTER_1[0], self.ADAPTER_1[1], label=self.ADAPTER_1[2]) + self.assertEqual(sorted([a4, a1], key=canonical_sort_key)[0]['mac'], + self.ADAPTER_1[1]) + + def test_four_adapters_are_named_in_adapter_order(self): + devices = [dev('?', 'pci-0000:04:00.0', 'aa:03', label=3), + dev('?', 'pci-0000:0b:00.0', 'aa:00', label=0), + dev('?', 'pci-0000:0c:00.0', 'aa:01', label=1), + dev('?', 'pci-0000:0d:00.0', 'aa:02', label=2)] + _, store, report = resolve(devices, empty_store()) + # assert on the hardware, not just that four names exist + self.assertEqual(report['bootstrapped'], + {'eth0': 'aa:00', 'eth1': 'aa:01', + 'eth2': 'aa:02', 'eth3': 'aa:03'}) + self.assertEqual(store['interfaces']['eth3'], 'ID_PATH=pci-0000:04:00.0') + + def test_a_labelled_and_an_unlabelled_device_can_be_compared(self): + # the sentinel has to be an int - sort keys are tuples compared + # element-wise, and None against int raises + labelled = dev('eth0', 'pci-0000:0b:00.0', 'aa:00', label=0) + plain = dev('eth1', 'pci-0000:03:00.0', 'bb:00') + self.assertEqual(sorted([plain, labelled], key=canonical_sort_key)[0]['mac'], + 'aa:00') + + def test_unlabelled_hardware_never_takes_the_first_name(self): + # a passthrough NIC is not in the OVF network mapping + devices = [dev('?', 'pci-0000:03:00.0', 'bb:00'), + dev('?', 'pci-0000:0b:00.0', 'aa:00', label=0)] + _, _, report = resolve(devices, empty_store()) + self.assertEqual(report['bootstrapped']['eth0'], 'aa:00') + + def test_adding_unlabelled_hardware_does_not_reshuffle_the_adapters(self): + adapters = [dev('?', 'pci-0000:0b:00.0', 'aa:00', label=0), + dev('?', 'pci-0000:04:00.0', 'aa:01', label=1)] + plain = dev('?', 'pci-0000:03:00.0', 'bb:00') + before = [d['mac'] for d in sorted(adapters, key=canonical_sort_key)] + after = [d['mac'] for d in sorted(adapters + [plain], key=canonical_sort_key)] + self.assertEqual(after[:len(before)], before) + + def test_the_order_survives_into_the_store(self): + # the names close the gap: two adapters labelled 0 and 3 become + # eth0 and eth1, the lowest free names in adapter order + devices = [dev('enp4s0', 'pci-0000:04:00.0', 'aa:03', label=3), + dev('enp11s0', 'pci-0000:0b:00.0', 'aa:00', label=0)] + plan, store, _ = resolve(devices, empty_store()) + self.assertEqual(plan, {'enp11s0': 'eth0', 'enp4s0': 'eth1'}) + + # the next boot sees them under the names it just gave them + for device in devices: + device['name'] = plan[device['name']] + + plan, _, report = resolve(devices, store) + self.assertEqual(plan, {}) + self.assertEqual(report['bootstrapped'], {}) + + def test_hardware_added_later_takes_the_next_name(self): + devices = [dev('?', 'pci-0000:0b:00.0', 'aa:00', label=0)] + _, store, _ = resolve(devices, empty_store()) + # a second adapter, labelled lower, must not displace the first + devices.append(dev('?', 'pci-0000:04:00.0', 'aa:09', label=9)) + _, store, report = resolve(devices, store) + self.assertEqual(report['bootstrapped'], {'eth1': 'aa:09'}) + + def test_the_label_is_ordering_only_never_identity(self): + # it must not reach hardware_key(), or a relabelled adapter would read + # as different hardware + properties = {'ID_PATH': 'pci-0000:0b:00.0', 'ID_BUS': 'pci'} + self.assertNotIn('Ethernet', hardware_key(properties)) + self.assertNotIn('label', hardware_key(properties)) + + +def hv(name, mac): + """A Hyper-V NIC as udev reports it on Azure: every one shares the ACPI + VMBus node as its ID_PATH and has no slot, so its MAC is its key.""" + return { + 'name': name, + 'mac': mac, + 'wireless': False, + 'properties': { + 'ID_PATH': 'acpi-MSFT1000:00', + 'ID_NET_NAME_MAC': 'enx' + mac.replace(':', ''), + 'ID_NET_DRIVER': 'hv_netvsc', + }, + } + + +def keyed(devices): + """Keys as discover_devices() computes them, shared ID_PATH passed over""" + ambiguous = ambiguous_keys([d['properties'] for d in devices]) + for device in devices: + device['key'] = hardware_key(device['properties'], ambiguous) + return devices + + +class TestHyperV(unittest.TestCase): + """Azure, four NICs, measured on a fresh deployment of this series: every + Hyper-V NIC shares one ID_PATH and has no slot, so its MAC is its key. + The order Azure lists the NICs in is cloud-init's to record; what is left + here is a NIC deleted and created again on the same VM, where cloud-init + does not run again. + """ + + AZURE = [ + ('eth0', '00:0d:3a:47:38:1d'), # WAN, the primary NIC + ('eth1', '00:0d:3a:47:3b:29'), + ('eth2', '00:0d:3a:47:33:ad'), + ('eth3', '00:0d:3a:47:3b:f8'), + ] + + def nics(self): + return keyed([hv(name, mac) for name, mac in self.AZURE]) + + def stored(self): + """the map as cloud-init records it on Azure: the names it gave""" + store = empty_store() + for name, mac in self.AZURE: + store['interfaces'][name] = 'ID_NET_NAME_MAC=enx' + mac.replace(':', '') + store['hardware'][name] = mac + plan, store, report = resolve(self.nics(), store) + assert plan == {} and len(report['matched']) == 4 + return store + + def test_each_nic_is_keyed_on_its_own_mac(self): + store = self.stored() + self.assertEqual( + sorted(store['interfaces'].values()), + sorted( + 'ID_NET_NAME_MAC=enx' + mac.replace(':', '') for _, mac in self.AZURE + ), + ) + + def test_a_recreated_nic_takes_the_name_of_the_one_it_replaced(self): + # LAN3's NIC deleted and created again: new MAC. It came up as eth4, + # eth3 stayed reserved and the boot commit failed on it every boot. + store = self.stored() + devices = self.nics() + devices[3] = keyed([hv('eth3', '00:0d:3a:47:aa:01')] + devices[:3])[0] + plan, store, report = resolve(devices, store) + self.assertEqual(report.get('adopted', {}), {'eth3': '00:0d:3a:47:aa:01'}) + self.assertEqual(report['bootstrapped'], {}) + self.assertEqual(plan, {}) + self.assertEqual(store['interfaces']['eth3'], 'ID_NET_NAME_MAC=enx000d3a47aa01') + + def test_two_recreated_nics_are_not_paired_by_guess(self): + # nothing here says which new NIC replaced which, and pairing them by + # the order they came up in can swap two subnets' configuration + store = self.stored() + devices = self.nics()[:2] + keyed( + [hv('eth3', '00:0d:3a:99:00:11'), hv('eth2', '00:0d:3a:99:00:33')] + ) + _, store, report = resolve(devices, store) + self.assertEqual(report.get('adopted', {}), {}) + self.assertEqual(set(report['bootstrapped']), {'eth4', 'eth5'}) + self.assertIn('eth2', store['interfaces']) + self.assertIn('eth3', store['interfaces']) + + def test_two_vacancies_and_one_new_nic_are_not_paired(self): + # which of two gone NICs the one new NIC replaced is not known + store = self.stored() + devices = self.nics()[:2] + keyed( + [hv('eth3', '00:0d:3a:99:00:11')] + self.nics()[:2])[:1] + _, store, report = resolve(devices, store) + self.assertEqual(report.get('adopted', {}), {}) + self.assertEqual(report['bootstrapped'], {'eth4': '00:0d:3a:99:00:11'}) + self.assertEqual(store.get('absent'), ['eth2', 'eth3']) + + def test_a_new_nic_with_no_vacancy_takes_a_new_name(self): + store = self.stored() + devices = self.nics() + keyed([hv('eth4', '00:0d:3a:48:00:09')]) + _, _, report = resolve(devices, store) + self.assertEqual(report.get('adopted', {}), {}) + self.assertEqual(report['bootstrapped'], {'eth4': '00:0d:3a:48:00:09'}) + + def test_one_vacancy_and_two_new_nics_are_not_paired(self): + store = self.stored() + devices = self.nics()[:3] + keyed( + [hv('eth3', '00:0d:3a:48:00:01'), hv('eth4', '00:0d:3a:48:00:02')] + ) + _, _, report = resolve(devices, store) + self.assertEqual(report.get('adopted', {}), {}) + + def test_a_disk_on_all_new_nics_is_not_handed_over_here(self): + # every recorded NIC gone at once: the platform's order is cloud-init's + # to supply (it runs again on a new instance); guessing it here could + # put the WAN's configuration on a LAN + store = self.stored() + new = keyed([hv(f'eth{i}', f'00:0d:3a:48:00:0{i}') for i in range(4)]) + _, _, report = resolve(new, store) + self.assertEqual(report.get('adopted', {}), {}) + + def test_a_slot_keyed_name_is_never_adopted(self): + # a PCI card has a slot to come back to; its name stays reserved + devices = [dev('eth0', 'pci-0000:00:03.0', 'aa:00')] + _, store, _ = resolve(devices, empty_store()) + other = keyed([hv('eth1', '00:0d:3a:48:00:09')]) + _, store, report = resolve(other, store) + self.assertEqual(report.get('adopted', {}), {}) + self.assertEqual(report['bootstrapped'], {'eth1': '00:0d:3a:48:00:09'}) + + def test_a_nic_added_after_one_was_removed_is_not_its_replacement(self): + # P8, measured on Azure: LAN3's NIC deleted for good (one boot), a NIC + # on another subnet added later (the next boot). Taking the vacant + # name put LAN3's configuration on the other subnet. + store = self.stored() + devices = self.nics()[:3] + _, store, report = resolve(devices, store) + self.assertEqual(report.get('adopted', {}), {}) + later = devices + keyed([hv('eth4', '38:33:c5:cc:a3:3e')]) + _, store, report = resolve(later, store) + self.assertEqual(report.get('adopted', {}), {}) + self.assertEqual(report['bootstrapped'], {'eth4': '38:33:c5:cc:a3:3e'}) + self.assertEqual(store.get('absent'), ['eth3']) + + def test_a_nic_replaced_on_a_running_system_is_not_adopted(self): + # measured on Hyper-V: a NIC hot-added after another was hot-removed + # took its name at once, while the configuration under that name + # stayed unapplied until the next boot + store = self.stored() + devices = self.nics() + devices[3] = keyed([hv('eth3', '00:0d:3a:47:aa:01')] + devices[:3])[0] + _, store, report = resolve(devices, store, adopt=False) + self.assertEqual(report.get('adopted', {}), {}) + self.assertEqual(report['bootstrapped'], {'eth4': '00:0d:3a:47:aa:01'}) + self.assertEqual(store.get('absent'), ['eth3']) + # nor at the next boot: the name was already missing at the last pass + _, store, report = resolve(devices, store) + self.assertEqual(report.get('adopted', {}), {}) + self.assertEqual(report['matched'].get('eth4'), '00:0d:3a:47:aa:01') + + def test_a_name_whose_nic_was_not_hyperv_is_not_handed_over(self): + # a MAC-keyed name can belong to other hardware sharing the VMBus + # path; only a Hyper-V NIC's name goes to a Hyper-V replacement + other = hv('eth1', '00:0d:3a:47:3b:29') + other['properties']['ID_NET_DRIVER'] = 'mlx5_core' + devices = keyed([hv('eth0', '00:0d:3a:47:38:1d'), other]) + _, store, _ = resolve(devices, empty_store()) + self.assertEqual(store.get('hyperv'), ['eth0']) + later = keyed([hv('eth0', '00:0d:3a:47:38:1d'), + hv('eth1', '00:0d:3a:48:00:09')]) + _, _, report = resolve(later, store) + self.assertEqual(report.get('adopted', {}), {}) + self.assertEqual(report['bootstrapped'], {'eth2': '00:0d:3a:48:00:09'}) + + def test_only_a_hyperv_nic_takes_a_vacant_name(self): + store = self.stored() + new = hv('eth3', '00:0d:3a:48:00:09') + new['properties']['ID_NET_DRIVER'] = 'virtio_net' + devices = self.nics()[:3] + keyed([new] + self.nics()[:3])[:1] + _, _, report = resolve(devices, store) + self.assertEqual(report.get('adopted', {}), {}) + self.assertEqual(report['bootstrapped'], {'eth4': '00:0d:3a:48:00:09'}) + + def test_a_new_nic_takes_no_name_of_another_namespace(self): + store = self.stored() + new = keyed([hv('wlan0', '00:0d:3a:48:00:09')] + self.nics()[:3])[0] + new['wireless'] = True + _, _, report = resolve(self.nics()[:3] + [new], store) + self.assertEqual(report.get('adopted', {}), {}) + self.assertEqual(report['bootstrapped'], {'wlan0': '00:0d:3a:48:00:09'}) + + def test_a_store_that_does_not_say_which_nics_were_hyperv_hands_nothing_over(self): + # written by code that did not record it: whether the vacant name + # was a Hyper-V NIC's, or how long it has been vacant, is unknown + store = self.stored() + store.pop('hyperv') + devices = self.nics() + devices[3] = keyed([hv('eth3', '00:0d:3a:47:aa:01')] + devices[:3])[0] + _, store, report = resolve(devices, store) + self.assertEqual(report.get('adopted', {}), {}) + self.assertEqual(report['bootstrapped'], {'eth4': '00:0d:3a:47:aa:01'}) + self.assertEqual(store.get('absent'), ['eth3']) + + def test_the_hyperv_record_survives_a_load_and_follows_the_nics(self): + store = self.stored() + self.assertEqual(store['hyperv'], ['eth0', 'eth1', 'eth2', 'eth3']) + with tempfile.TemporaryDirectory() as d: + path = Path(d) / 'map.json' + with mock.patch('vyos.ifconfig.ifname_store.config_dir_is_mounted', + return_value=True), \ + mock.patch('vyos.ifconfig.ifname_store.get_cfg_group_id', + return_value=0): + save_store(store, path) + self.assertEqual(load_store(path)['hyperv'], store['hyperv']) + # an absent NIC keeps its record, so its replacement is recognised + _, store, _ = resolve(self.nics()[:3], store) + self.assertEqual(store['hyperv'], ['eth0', 'eth1', 'eth2', 'eth3']) + + def test_a_lone_hyperv_nic_keyed_on_its_path_still_pins_its_driver(self): + # one NIC has the VMBus path to itself, so it is keyed on it; its VF + # must not match its file any more than with a MAC key + devices = keyed([hv('eth0', '00:0d:3a:47:38:1d')]) + _, store, _ = resolve(devices, empty_store()) + self.assertTrue(store['interfaces']['eth0'].startswith('ID_PATH=')) + with tempfile.TemporaryDirectory() as d: + sync_link_files(store, link_dir=Path(d), devices=devices) + body = (Path(d) / '10-vyos-eth0.link').read_text() + self.assertIn('Driver=hv_netvsc\n', body) + + def test_a_store_without_the_record_still_adopts(self): + # written before names missing at a pass were recorded: nothing was + # missing then, as far as it can say, so a one-step replacement holds + store = self.stored() + store.pop('absent', None) + devices = self.nics() + devices[3] = keyed([hv('eth3', '00:0d:3a:47:aa:01')] + devices[:3])[0] + _, _, report = resolve(devices, store) + self.assertEqual(report.get('adopted', {}), {'eth3': '00:0d:3a:47:aa:01'}) + + def test_the_record_clears_when_the_nic_is_back(self): + store = self.stored() + _, store, _ = resolve(self.nics()[:3], store) + self.assertEqual(store.get('absent'), ['eth3']) + _, store, report = resolve(self.nics(), store) + self.assertNotIn('absent', store) + self.assertEqual(report['matched']['eth3'], self.AZURE[3][1]) + + def test_the_record_survives_a_load(self): + tmp = Path(tempfile.mkdtemp()) + self.addCleanup(shutil.rmtree, tmp) + path = tmp / 'interface-mapping.json' + store = self.stored() + _, store, _ = resolve(self.nics()[:3], store) + path.write_text(json.dumps(store)) + self.assertEqual(load_store(path).get('absent'), ['eth3']) + # a name the map no longer has is dropped from it + store['absent'] = ['eth3', 'eth9'] + path.write_text(json.dumps(store)) + self.assertEqual(load_store(path).get('absent'), ['eth3']) + + def test_link_file_pins_the_driver_for_a_mac_key(self): + # the accelerated-networking VF carries its NIC's MAC + body = render_link('eth0', 'ID_NET_NAME_MAC=enx000d3a47381d', 'hv_netvsc') + self.assertIn( + 'Property=ID_NET_NAME_MAC=enx000d3a47381d\nDriver=hv_netvsc\n', body + ) + + def test_link_file_without_a_driver_is_unchanged(self): + self.assertNotIn('Driver=', render_link('eth0', 'ID_PATH=pci-0000:00:03.0')) + + def link_dir(self): + tmp = Path(tempfile.mkdtemp()) + self.addCleanup(shutil.rmtree, tmp) + return tmp + + def test_sync_pins_the_driver_for_hyperv_only(self): + devices = self.nics() + usb = { + 'name': 'eth4', + 'mac': 'aa:00:00:00:00:04', + 'wireless': False, + 'properties': { + 'ID_NET_NAME_MAC': 'enxaa0000000004', + 'ID_NET_DRIVER': 'r8152', + }, + } + devices += keyed([usb]) + _, store, _ = resolve(devices, empty_store()) + tmp = self.link_dir() + sync_link_files(store, link_dir=tmp, devices=devices) + name_of = {key: name for name, key in store['interfaces'].items()} + wan = name_of['ID_NET_NAME_MAC=enx000d3a47381d'] + other = name_of['ID_NET_NAME_MAC=enxaa0000000004'] + self.assertIn('Driver=hv_netvsc', (tmp / f'10-vyos-{wan}.link').read_text()) + self.assertNotIn('Driver=', (tmp / f'10-vyos-{other}.link').read_text()) + + def test_an_absent_nic_keeps_its_driver_pin(self): + # a boot without the NIC used to rewrite its file without the pin, and + # on its return its VF matched the file again + devices = self.nics() + _, store, _ = resolve(devices, empty_store()) + tmp = self.link_dir() + sync_link_files(store, link_dir=tmp, devices=devices) + sync_link_files(store, link_dir=tmp, devices=devices[:3]) + self.assertIn('Driver=hv_netvsc', (tmp / '10-vyos-eth3.link').read_text()) + + +class TestReplacedHardware(unittest.TestCase): + """A slot keeps its name when its card is swapped - which hands that + interface's addresses to different hardware. resolve() reports the change + so vyos-router can warn instead of letting it happen silently. + """ + + PATH = 'pci-0000:00:12.0' + + def _device(self, mac): + return dev('eth0', self.PATH, mac) + + def test_same_card_reports_nothing(self): + store = store_of({'eth0': self.PATH}, {'eth0': 'aa:aa:aa:aa:aa:01'}) + _, _, report = resolve([self._device('aa:aa:aa:aa:aa:01')], store) + self.assertEqual(report['replaced'], {}) + + def test_swapped_card_is_reported_with_its_new_mac(self): + store = store_of({'eth0': self.PATH}, {'eth0': 'aa:aa:aa:aa:aa:01'}) + _, new_store, report = resolve([self._device('bb:bb:bb:bb:bb:02')], store) + self.assertEqual(report['replaced'], {'eth0': 'bb:bb:bb:bb:bb:02'}) + # the name is still the slot's, and the new card is now what is on record + self.assertEqual(new_store['interfaces']['eth0'], f'ID_PATH={self.PATH}') + self.assertEqual(new_store['hardware']['eth0'], 'bb:bb:bb:bb:bb:02') + + def test_reported_once_not_on_every_later_boot(self): + store = store_of({'eth0': self.PATH}, {'eth0': 'aa:aa:aa:aa:aa:01'}) + _, store, _ = resolve([self._device('bb:bb:bb:bb:bb:02')], store) + _, _, report = resolve([self._device('bb:bb:bb:bb:bb:02')], store) + self.assertEqual(report['replaced'], {}) + + def test_absent_card_keeps_its_record_so_a_later_swap_is_caught(self): + store = store_of({'eth0': self.PATH}, {'eth0': 'aa:aa:aa:aa:aa:01'}) + _, store, _ = resolve([], store) + self.assertEqual(store['hardware']['eth0'], 'aa:aa:aa:aa:aa:01') + _, _, report = resolve([self._device('bb:bb:bb:bb:bb:02')], store) + self.assertEqual(report['replaced'], {'eth0': 'bb:bb:bb:bb:bb:02'}) + + def test_new_hardware_is_not_a_replacement(self): + _, _, report = resolve([self._device('aa:aa:aa:aa:aa:01')], empty_store()) + self.assertEqual(report['replaced'], {}) + self.assertIn('eth0', report['bootstrapped']) + + def test_store_written_before_this_field_reports_nothing(self): + # nothing to compare against yet - and no interface may be renamed + legacy = {'version': STORE_VERSION, + 'interfaces': {'eth0': f'ID_PATH={self.PATH}'}} + plan, new_store, report = resolve([self._device('aa:aa:aa:aa:aa:01')], + legacy) + self.assertEqual(report['replaced'], {}) + self.assertEqual(plan, {}) + self.assertEqual(new_store['interfaces']['eth0'], f'ID_PATH={self.PATH}') + + +class TestMovedHardware(unittest.TestCase): + """A card carries its name with it. Matching on the slot alone would give + the name - and the address configured under it - to whatever took the old + position, which is how a card added for another purpose ended up on + another segment's address. + """ + + A = 'aa:aa:aa:aa:aa:01' + B = 'bb:bb:bb:bb:bb:02' + C = 'cc:cc:cc:cc:cc:03' + + def setUp(self): + self.store = store_of({'eth0': 'pci-0000:00:02.0', + 'eth1': 'pci-0000:00:03.0'}, + {'eth0': self.A, 'eth1': self.B}) + + @staticmethod + def _at(slot, mac): + return dev('?', f'pci-0000:00:{slot:02x}.0', mac) + + def test_card_moved_to_another_slot_keeps_its_name(self): + _, store, report = resolve([self._at(4, self.A), self._at(3, self.B)], + self.store) + self.assertEqual(report['moved'], {'eth0': self.A}) + self.assertEqual(report['replaced'], {}) + self.assertEqual(store['interfaces']['eth0'], 'ID_PATH=pci-0000:00:04.0') + + def test_a_new_card_in_the_vacated_slot_does_not_inherit_the_name(self): + # the shape reported from the field, with the original card still in + # the box: it keeps eth0, and the newcomer is named as new hardware + _, store, report = resolve( + [self._at(4, self.A), self._at(3, self.B), self._at(2, self.C)], + self.store) + self.assertEqual(report['moved'], {'eth0': self.A}) + self.assertEqual(report['bootstrapped'], {'eth2': self.C}) + self.assertEqual(report['replaced'], {}) + self.assertEqual(store['interfaces']['eth0'], 'ID_PATH=pci-0000:00:04.0') + + def test_two_known_cards_swapping_slots_stay_with_their_slots(self): + # Both cards are on record, so this is a rearrangement within the set + # the store already describes - and the store is what decides names. + # The same path covers an operator swapping two entries by hand, which + # has to take effect or the map would not be authoritative at all. + _, store, report = resolve([self._at(3, self.A), self._at(2, self.B)], + self.store) + self.assertEqual(report['moved'], {}) + self.assertEqual(report['matched'], {}) + self.assertEqual(report['reassigned'], {'eth0': self.B, 'eth1': self.A}) + self.assertEqual(store['interfaces']['eth0'], 'ID_PATH=pci-0000:00:02.0') + self.assertEqual(store['hardware']['eth0'], self.B) + + def test_editing_the_slot_map_by_hand_takes_effect(self): + # what `make test-interface-naming` does: swap the two entries in 'interfaces' + # and leave 'hardware' alone. Following the recorded address here + # would quietly undo the edit. + store = store_of({'eth0': 'pci-0000:00:03.0', + 'eth1': 'pci-0000:00:02.0'}, + {'eth0': self.A, 'eth1': self.B}) + _, _, report = resolve([self._at(2, self.A), self._at(3, self.B)], + store) + self.assertEqual(report['matched'], {}) + self.assertEqual(report['reassigned'], {'eth0': self.B, 'eth1': self.A}) + self.assertEqual(report['moved'], {}) + + def test_identity_wins_over_the_slot(self): + # both rules could fire: the card that owns the name sits elsewhere + # while another card occupies its recorded slot. The card wins. + _, _, report = resolve( + [self._at(4, self.A), self._at(2, self.C), self._at(3, self.B)], + self.store) + self.assertIn('eth0', report['moved']) + self.assertNotIn('eth0', report['replaced']) + + def test_gone_card_still_falls_back_to_the_slot_and_is_reported(self): + # the original is nowhere to be found, so nothing distinguishes an + # in-place replacement from an unrelated card - the slot decides, loudly + _, _, report = resolve([self._at(2, self.C), self._at(3, self.B)], + self.store) + self.assertEqual(report['replaced'], {'eth0': self.C}) + self.assertEqual(report['moved'], {}) + + +class TestWwanNaming(unittest.TestCase): + """A modem is addressed as wwanN by the CLI - 'interfaces wwan' looks at + nothing else. Naming it ethN like any other port left an HP lt4132 bound, + working and completely invisible on a DUT. + """ + + WWP = 'wwp0s20f0u2u4c3' + MODEM_MAC = 'b6:53:dd:66:c2:98' + + def _modem(self, name='eth6'): + # no address: discovery keeps none for a modem + properties = {'ID_NET_NAME_PATH': self.WWP, 'ID_BUS': 'usb'} + return {'name': name, 'mac': '', 'wireless': False, + 'wwan': True, 'properties': properties, + 'key': f'ID_NET_NAME_PATH={self.WWP}'} + + def _nic(self, name='eth0', path='enp3s0', mac='00:f0:cb:fe:ba:99'): + properties = {'ID_NET_NAME_PATH': path, 'ID_BUS': 'pci'} + return {'name': name, 'mac': mac, 'wireless': False, 'wwan': False, + 'properties': properties, 'key': f'ID_NET_NAME_PATH={path}'} + + def test_a_modem_is_named_in_its_own_namespace(self): + _, store, _ = resolve([self._nic(), self._modem()], empty_store()) + self.assertEqual(store['interfaces']['wwan0'], + f'ID_NET_NAME_PATH={self.WWP}') + # and it must not have eaten an ethernet number on the way + self.assertEqual(store['interfaces']['eth0'], + 'ID_NET_NAME_PATH=enp3s0') + self.assertNotIn('eth1', store['interfaces']) + + def test_the_link_file_names_the_type_the_device_is(self): + # a .link naming a type the device is not simply never applies + body = render_link('wwan0', f'ID_NET_NAME_PATH={self.WWP}') + self.assertIn('Type=wwan', body) + self.assertNotIn('Type=ether', body) + + def test_a_modem_filed_under_an_ethernet_name_is_reclaimed(self): + # what an earlier version left on the DUT: the entry still matches the + # modem by path, so without dropping it the modem is captured again and + # stays invisible to 'interfaces wwan' + store = {'version': STORE_VERSION, + 'interfaces': {'eth0': 'ID_NET_NAME_PATH=enp3s0', + 'eth6': f'ID_NET_NAME_PATH={self.WWP}'}, + 'hardware': {'eth0': '00:f0:cb:fe:ba:99', + 'eth6': self.MODEM_MAC}} + plan, new_store, _ = resolve([self._nic(), self._modem()], store) + + self.assertNotIn('eth6', new_store['interfaces']) + self.assertNotIn('eth6', new_store['hardware']) + self.assertEqual(new_store['interfaces']['wwan0'], + f'ID_NET_NAME_PATH={self.WWP}') + self.assertEqual(plan, {'eth6': 'wwan0'}) + + def test_the_stale_link_file_goes_with_it(self): + store = {'version': STORE_VERSION, + 'interfaces': {'eth6': f'ID_NET_NAME_PATH={self.WWP}'}, + 'hardware': {'eth6': self.MODEM_MAC}} + _, new_store, _ = resolve([self._modem()], store) + with tempfile.TemporaryDirectory() as d: + (Path(d) / '10-vyos-eth6.link').write_text('left over') + sync_link_files(new_store, Path(d)) + names = sorted(p.name for p in Path(d).iterdir()) + self.assertEqual(names, ['10-vyos-wwan0.link']) + + def test_a_nic_in_the_modem_s_socket_does_not_take_its_name(self): + # the modem is gone and an ordinary USB NIC is in the port it used. + # The slot matches, but wwan0 is a name 'interfaces wwan' reads - give + # it to a NIC and the modem's configuration is applied to a card that + # cannot carry it. Leave the name reserved and name the NIC as a NIC. + store = {'version': STORE_VERSION, + 'interfaces': {'wwan0': f'ID_NET_NAME_PATH={self.WWP}'}, + 'hardware': {}} + nic = self._nic('eth0', self.WWP, 'aa:bb:cc:dd:ee:07') + _, new_store, report = resolve([nic], store) + + self.assertEqual(report['replaced'], {}) + self.assertIn('eth0', new_store['interfaces']) + self.assertEqual(new_store['interfaces']['wwan0'], + f'ID_NET_NAME_PATH={self.WWP}') + + def test_a_modem_is_not_reported_as_replaced(self): + # its address changes at every probe, so comparing it reported the + # modem as a different card in the same socket on every boot + store = {'version': STORE_VERSION, + 'interfaces': {'wwan0': f'ID_NET_NAME_PATH={self.WWP}'}, + 'hardware': {'wwan0': self.MODEM_MAC}} + plan, new_store, report = resolve([self._modem('wwan0')], store) + + self.assertEqual(plan, {}) + self.assertEqual(report['replaced'], {}) + self.assertIn('wwan0', report['matched']) + # and an address an earlier version recorded is dropped, so a store + # already carrying one settles on the next pass + self.assertNotIn('wwan0', new_store['hardware']) + + def test_a_card_whose_address_changed_is_still_replaced(self): + # the modem must not have weakened this: a card in a recorded slot + # with a different address is the case worth warning about + store = {'version': STORE_VERSION, + 'interfaces': {'eth0': 'ID_NET_NAME_PATH=enp3s0'}, + 'hardware': {'eth0': '00:f0:cb:fe:ba:99'}} + nic = self._nic('eth0', 'enp3s0', 'aa:bb:cc:dd:ee:07') + _, _, report = resolve([nic], store) + + self.assertIn('eth0', report['replaced']) + + def test_an_ordinary_port_on_the_usb_bus_is_not_a_modem(self): + # the key alone cannot tell them apart - a device falling back to + # ID_PATH reads usb-... whether it is a modem or not + usb_nic = self._nic('eth1', 'enp0s20f0u3', 'aa:bb:cc:dd:ee:01') + usb_nic['properties'] = {'ID_PATH': 'usb-0:2:1.0', 'ID_BUS': 'usb'} + usb_nic['key'] = 'ID_PATH=usb-0:2:1.0' + _, store, _ = resolve([usb_nic], empty_store()) + self.assertIn('eth0', store['interfaces']) + self.assertNotIn('wwan0', store['interfaces']) + + +class TestStoreIO(unittest.TestCase): + @staticmethod + def _config_dir(d): + """An ordinary, unencrypted config directory - the common case.""" + return Path(d) / 'interface-mapping.json' + + def test_roundtrip(self): + with tempfile.TemporaryDirectory() as d, \ + mock.patch('vyos.ifconfig.ifname_store.encrypted_config_volume', + return_value=False): + p = self._config_dir(d) + s = store_of({'eth0': 'pci-0000:00:12.0'}) + self.assertTrue(save_store(s, p)) + self.assertEqual(load_store(p), s) + + def test_refuses_to_write_when_the_encrypted_volume_is_not_mounted(self): + # TPM/LUKS unlock failed: vyos-router continues, but persisting here + # would write a shadow store that the real volume masks later + with tempfile.TemporaryDirectory() as d, \ + mock.patch('vyos.ifconfig.ifname_store.encrypted_config_volume', + return_value=True): + p = Path(d) / 'interface-mapping.json' # a plain dir, not a mount + self.assertFalse(save_store(store_of({'eth0': 'x'}), p)) + self.assertFalse(p.exists()) + + def test_writes_on_the_first_boot_of_a_prebuilt_image(self): + # T3871: the check used to key on '.vyatta_config', which an + # activation script writes only AFTER interface naming has run. On a + # qcow2 that marker is absent on the first boot, so no store was ever + # written and the next boot named the hardware from scratch - pull a + # card and every name below it moved onto the wrong port. + with tempfile.TemporaryDirectory() as d, \ + mock.patch('vyos.ifconfig.ifname_store.encrypted_config_volume', + return_value=False): + p = Path(d) / 'interface-mapping.json' + self.assertFalse((Path(d) / '.vyatta_config').exists()) + self.assertTrue(save_store(store_of({'eth0': 'pci-0000:00:12.0'}), p)) + self.assertTrue(p.exists()) + + def test_unencrypted_config_needs_no_mount_point(self): + # the config directory of an ordinary install is not a mount of its + # own - only an encrypted one is + with tempfile.TemporaryDirectory() as d, \ + mock.patch('vyos.ifconfig.ifname_store.encrypted_config_volume', + return_value=False): + self.assertTrue(config_dir_is_mounted( + Path(d) / 'interface-mapping.json')) + + def test_missing_file_is_empty_store(self): + with tempfile.TemporaryDirectory() as d: + self.assertEqual(load_store(Path(d) / 'nope.json'), empty_store()) + + def test_corrupt_file_is_empty_store(self): + with tempfile.TemporaryDirectory() as d: + p = self._config_dir(d) + p.write_text('{ this is not json') + self.assertEqual(load_store(p), empty_store()) + + def test_write_is_atomic_no_leftovers(self): + with tempfile.TemporaryDirectory() as d, \ + mock.patch('vyos.ifconfig.ifname_store.encrypted_config_volume', + return_value=False): + p = self._config_dir(d) + save_store(store_of({'eth0': 'pci-0000:00:12.0'}), p) + self.assertEqual(sorted(f.name for f in Path(d).iterdir()), + ['interface-mapping.json']) + + +def final_names(report): + """Which name each MAC ended up with, read back off the resolve report.""" + out = {} + for bucket in report.values(): + for name, mac in bucket.items(): + out[mac] = name + return out + + +class TestDeviceMatches(unittest.TestCase): + """A stored key names the property it was taken from, so matching a + device is comparing that one property - never a guess about which + identifier was meant.""" + + def test_matches_the_named_property(self): + self.assertTrue(device_matches({'ID_NET_NAME_SLOT': 'ens18'}, + 'ID_NET_NAME_SLOT=ens18')) + + def test_same_value_under_a_different_property_does_not_match(self): + self.assertFalse(device_matches({'ID_NET_NAME_PATH': 'ens18'}, + 'ID_NET_NAME_SLOT=ens18')) + + def test_device_without_the_property_does_not_match(self): + self.assertFalse(device_matches({'ID_PATH': 'pci-0000:00:12.0'}, + 'ID_NET_NAME_SLOT=ens18')) + + def test_empty_or_malformed_key_never_matches(self): + self.assertFalse(device_matches({'ID_NET_NAME_SLOT': 'ens18'}, '')) + self.assertFalse(device_matches({'ID_NET_NAME_SLOT': 'ens18'}, 'ens18')) + + +class TestResolveBasics(unittest.TestCase): + def test_first_boot_bootstraps_in_slot_order(self): + devices = [ + dev('eth0', 'pci-0000:00:15.0', '00:00:5e:00:53:03'), + dev('eth1', 'pci-0000:00:12.0', '00:00:5e:00:53:00'), + dev('eth2', 'pci-0000:00:13.0', '00:00:5e:00:53:01'), + ] + _, new_store, _ = resolve(devices, empty_store()) + self.assertEqual(new_store['interfaces']['eth0'], 'ID_PATH=pci-0000:00:12.0') + self.assertEqual(new_store['interfaces']['eth1'], 'ID_PATH=pci-0000:00:13.0') + self.assertEqual(new_store['interfaces']['eth2'], 'ID_PATH=pci-0000:00:15.0') + + def test_established_names_are_stable_regardless_of_probe_order(self): + store = store_of({'eth0': 'pci-0000:00:12.0', 'eth1': 'pci-0000:00:13.0'}) + # drivers finished in the opposite order this boot + devices = [ + dev('eth0', 'pci-0000:00:13.0', '00:00:5e:00:53:01'), + dev('eth1', 'pci-0000:00:12.0', '00:00:5e:00:53:00'), + ] + plan, _, _ = resolve(devices, store) + self.assertEqual(plan, {'eth0': 'eth1', 'eth1': 'eth0'}) + + def test_replacement_nic_in_same_slot_inherits_name(self): + # the slot is the identity, so this needs no special rule: whatever + # card is in eth1's slot is eth1, along with its configuration + store = store_of({'eth1': 'pci-0000:00:13.0'}) + devices = [dev('eth0', 'pci-0000:00:13.0', 'bb:bb:bb:bb:bb:bb')] + plan, new_store, report = resolve(devices, store) + self.assertEqual(plan, {'eth0': 'eth1'}) + self.assertEqual(report['matched'], {'eth1': 'bb:bb:bb:bb:bb:bb'}) + self.assertEqual(new_store['interfaces']['eth1'], 'ID_PATH=pci-0000:00:13.0') + + def test_a_card_moved_to_another_slot_takes_that_slot_s_name(self): + # the consequence of slot identity, stated plainly: move a card and it + # becomes whatever the new slot is called, not what it used to be + store = store_of({'eth1': 'pci-0000:00:13.0', 'eth2': 'pci-0000:00:14.0'}) + devices = [dev('eth9', 'pci-0000:00:14.0', 'aa:aa:aa:aa:aa:aa')] + plan, _, report = resolve(devices, store) + self.assertEqual(plan, {'eth9': 'eth2'}) + self.assertEqual(final_names(report)['aa:aa:aa:aa:aa:aa'], 'eth2') + + def test_absent_hardware_keeps_its_name_reserved(self): + store = store_of({'eth0': 'pci-0000:00:12.0', 'eth1': 'pci-0000:00:13.0'}) + # eth0's slot is empty; a brand new card appeared elsewhere + devices = [ + dev('eth0', 'pci-0000:00:13.0', '00:00:5e:00:53:01'), + dev('eth1', 'pci-0000:00:1c.0', 'cc:cc:cc:cc:cc:cc'), + ] + _, new_store, report = resolve(devices, store) + # the new card must not inherit eth0's name, and with it eth0's config + self.assertNotEqual(final_names(report)['cc:cc:cc:cc:cc:cc'], 'eth0') + self.assertEqual(new_store['interfaces']['eth0'], 'ID_PATH=pci-0000:00:12.0') + + def test_hardware_with_no_usable_path_falls_back_to_its_mac(self): + # some paravirtual buses give udev nothing to build a path from; the + # MAC-derived name is the last step of the key chain so the interface + # is still stable instead of being renamed every boot + properties = {'ID_NET_NAME_MAC': 'enxaabbccddeeff'} + device = {'name': 'eth0', 'mac': 'aa:bb:cc:dd:ee:ff', 'wireless': False, + 'properties': properties, 'key': hardware_key(properties)} + _, new_store, _ = resolve([device], empty_store()) + self.assertEqual(new_store['interfaces']['eth0'], + 'ID_NET_NAME_MAC=enxaabbccddeeff') + + def test_a_path_two_devices_share_is_not_used_to_identify_either(self): + # a modem can put several interfaces on one parent device, and they + # then report the same path. Keying both on it would give them one + # name between them, so fall through to something which tells them + # apart. + first = {'ID_PATH': 'pci-0000:00:12.0', + 'ID_NET_NAME_MAC': 'enxaabbccddee01'} + second = {'ID_PATH': 'pci-0000:00:12.0', + 'ID_NET_NAME_MAC': 'enxaabbccddee02'} + ambiguous = ambiguous_keys([first, second]) + self.assertEqual(hardware_key(first, ambiguous), + 'ID_NET_NAME_MAC=enxaabbccddee01') + self.assertEqual(hardware_key(second, ambiguous), + 'ID_NET_NAME_MAC=enxaabbccddee02') + + def test_a_path_only_one_device_has_is_still_used(self): + first = {'ID_PATH': 'pci-0000:00:12.0'} + second = {'ID_PATH': 'pci-0000:00:13.0'} + ambiguous = ambiguous_keys([first, second]) + self.assertEqual(hardware_key(first, ambiguous), + 'ID_PATH=pci-0000:00:12.0') + + def test_firmware_slot_is_preferred_over_a_bare_pci_path(self): + # a firmware slot index survives PCI renumbering, a bus address does not + properties = {'ID_NET_NAME_SLOT': 'ens18', + 'ID_NET_NAME_PATH': 'enp0s18', + 'ID_PATH': 'pci-0000:00:12.0'} + self.assertEqual(hardware_key(properties), 'ID_NET_NAME_SLOT=ens18') + + def test_a_radio_recorded_by_an_older_version_is_dropped(self): + # Wireless is no longer named here: every interface on a phy carries + # that phy's address, so an entry recorded against one of them would + # be applied to whichever appeared next - renaming an interface the + # operator had just created by name. Drop what an older version wrote + # so the entry, and the .link file made from it, go away on upgrade. + with tempfile.TemporaryDirectory() as d: + p = Path(d) / 'interface-mapping.json' + p.write_text(json.dumps({ + 'version': STORE_VERSION, + 'interfaces': {'eth0': 'ID_PATH=pci-0000:00:12.0', + 'wlan0': 'ID_NET_NAME_MAC=wlx020000000000'}, + 'hardware': {'eth0': 'aa:aa:aa:aa:aa:aa', + 'wlan0': '02:00:00:00:00:00'}})) + store = load_store(p) + self.assertEqual(list(store['interfaces']), ['eth0']) + self.assertNotIn('wlan0', store['hardware']) + + +class TestPR5417Regressions(unittest.TestCase): + """Each test replays a reproduction from the review of PR #5417, where the + config-tree-driven resolver put a configured address on the wrong wire. + The assertions are about physical correctness - which MAC ends up behind + which name - not about log output. + """ + + def test_multi_nic_cloud_init_first_boot(self): + """2026-08-25 #1: three NICs, cloud-init synthesized a hw-id for eth0 + only. eth1/eth2 were pushed to eth3/eth4 and both data addresses were + unreachable, persisting across reboot. + """ + devices = [ + dev('eth0', 'pci-0000:00:03.0', '52:54:00:00:00:10'), + dev('eth1', 'pci-0000:00:04.0', '52:54:00:ff:00:21'), + dev('eth2', 'pci-0000:00:05.0', '52:54:00:00:00:22'), + ] + _, new_store, report = resolve(devices, empty_store()) + names = final_names(report) + self.assertEqual(names['52:54:00:00:00:10'], 'eth0') + self.assertEqual(names['52:54:00:ff:00:21'], 'eth1') + self.assertEqual(names['52:54:00:00:00:22'], 'eth2') + self.assertEqual(set(new_store['interfaces']), {'eth0', 'eth1', 'eth2'}) + + def test_names_follow_slots_not_mac_magnitude(self): + """2026-08-25 #2: the resolver swapped eth1/eth2 by MAC order, so each + address landed on the other's physical network, and the swap was + persisted with no warning. eth1 sits in the lower slot but has the + numerically HIGHER MAC. + """ + devices = [ + dev('eth1', 'pci-0000:00:04.0', '52:54:00:ff:00:21'), + dev('eth2', 'pci-0000:00:05.0', '52:54:00:00:00:22'), + ] + _, _, report = resolve(devices, empty_store()) + names = final_names(report) + self.assertEqual(names['52:54:00:ff:00:21'], 'eth0') + self.assertEqual(names['52:54:00:00:00:22'], 'eth1') + + def test_box_named_out_of_slot_order_is_not_reordered(self): + """2026-09-02 / 2026-09-09: a box first named by an earlier release in + MAC order - eth1 on the slot-5 NIC, eth4 on the slot-2 NIC. Deleting + hw-id entries made the resolver re-derive names in PCI slot order and + put eth1 and eth4 on each other's cable, silently and permanently. + + The store records the slot each name was actually on, so nothing is + ever re-derived and the box keeps the names it had. + """ + store = store_of({ + 'eth0': 'pci-0000:00:01.0', + 'eth1': 'pci-0000:00:05.0', + 'eth2': 'pci-0000:00:03.0', + 'eth3': 'pci-0000:00:04.0', + 'eth4': 'pci-0000:00:02.0', + }) + devices = [ + dev('eth0', 'pci-0000:00:01.0', '00:00:5e:00:54:01'), + dev('eth1', 'pci-0000:00:05.0', '00:00:5e:00:54:12'), + dev('eth2', 'pci-0000:00:03.0', '00:00:5e:00:54:13'), + dev('eth3', 'pci-0000:00:04.0', '00:00:5e:00:54:14'), + dev('eth4', 'pci-0000:00:02.0', '00:00:5e:00:54:19'), + ] + plan, _, report = resolve(devices, store) + self.assertEqual(plan, {}, 'nothing may be renamed') + names = final_names(report) + self.assertEqual(names['00:00:5e:00:54:12'], 'eth1') + self.assertEqual(names['00:00:5e:00:54:19'], 'eth4') + + def test_spare_nic_does_not_displace_configured_interfaces(self): + """2026-09-02 #2: a fourth, unconfigured NIC was enough to make the + three configured interfaces fail to get their hardware - eth1/eth2 + stayed bare while eth3/eth4/eth5 appeared holding the NICs. + """ + devices = [ + dev('eth0', 'pci-0000:00:03.0', '00:00:5e:00:54:31'), + dev('eth1', 'pci-0000:00:04.0', '00:00:5e:00:54:39'), + dev('eth2', 'pci-0000:00:05.0', '00:00:5e:00:54:32'), + dev('eth3', 'pci-0000:00:06.0', '00:00:5e:00:54:3c'), + ] + _, new_store, report = resolve(devices, empty_store()) + self.assertEqual(set(new_store['interfaces']), + {'eth0', 'eth1', 'eth2', 'eth3'}) + names = final_names(report) + self.assertEqual(names['00:00:5e:00:54:31'], 'eth0') + self.assertEqual(names['00:00:5e:00:54:39'], 'eth1') + self.assertEqual(names['00:00:5e:00:54:32'], 'eth2') + + def test_config_node_without_hardware_does_not_block_the_others(self): + """2026-09-09: eth0/eth1/eth2 configured with no hw-id on a two-NIC + VM. Every interface was left pending, both NICs parked on eth3/eth4, + and the box was unreachable from the network. + """ + devices = [ + dev('eth0', 'pci-0000:00:03.0', '00:00:5e:00:54:a1'), + dev('eth1', 'pci-0000:00:04.0', '00:00:5e:00:54:a9'), + ] + plan, new_store, report = resolve(devices, empty_store()) + # the two NICs present take eth0 and eth1; a configured eth2 node + # simply has no hardware, which is a reporting matter not a naming one + self.assertEqual(plan, {}) + names = final_names(report) + self.assertEqual(names['00:00:5e:00:54:a1'], 'eth0') + self.assertEqual(names['00:00:5e:00:54:a9'], 'eth1') + self.assertNotIn('eth3', new_store['interfaces']) + + +class TestLinkFiles(unittest.TestCase): + """The .link layer is what removes the rename window: udev applies the + name as the device is added, so no probe-order name is ever observable. + """ + + def test_matches_the_same_property_the_store_keyed_on(self): + # udev must apply exactly the mapping recorded in the store - if the + # .link matched on anything else the two could disagree about what + # identifies an interface + body = render_link('eth1', 'ID_NET_NAME_SLOT=ens18') + self.assertIn('Property=ID_NET_NAME_SLOT=ens18', body) + self.assertIn('Name=eth1', body) + self.assertIn('Type=ether', body) + + def test_the_address_policy_is_spelled_out(self): + # only the first matching file is applied, so this one shadows the + # policy the distribution ships. Without it a modem keeps the random + # address its driver gives it, fresh on every boot. + for name in ('eth1', 'wwan0'): + body = render_link(name, 'ID_NET_NAME_PATH=wwp0s20f0u2u4c3') + self.assertIn('MACAddressPolicy=persistent', body) + + def test_a_file_without_the_policy_is_rewritten(self): + # the upgrade path: a box already carrying files from before only + # picks this up if an existing one is replaced + store = {'version': STORE_VERSION, + 'interfaces': {'wwan0': 'ID_NET_NAME_PATH=wwp0s20f0u2u4c3'}, + 'hardware': {}} + with tempfile.TemporaryDirectory() as d: + path = Path(d) / '10-vyos-wwan0.link' + path.write_text('[Match]\nType=wwan\n\n[Link]\nName=wwan0\n') + sync_link_files(store, Path(d)) + self.assertIn('MACAddressPolicy=persistent', path.read_text()) + + def test_no_link_file_is_written_for_a_radio(self): + # a .link keyed on a phy's address matches every interface created on + # that phy, so writing one renamed the interface the operator had just + # asked for. Wireless is named from the configuration instead. + store = {'version': STORE_VERSION, + 'interfaces': {'eth0': 'ID_PATH=pci-0000:00:12.0', + 'wlan0': 'ID_NET_NAME_MAC=wlx020000000000'}, + 'hardware': {}} + with tempfile.TemporaryDirectory() as d: + written = sync_link_files(store, Path(d)) + names = [p.name for p in written] + self.assertEqual(names, ['10-vyos-eth0.link']) + + def test_one_file_per_interface(self): + with tempfile.TemporaryDirectory() as d: + written = sync_link_files( + store_of({'eth1': 'pci-0000:00:13.0'}), Path(d)) + self.assertEqual([p.name for p in written], ['10-vyos-eth1.link']) + + def test_stale_files_are_removed(self): + with tempfile.TemporaryDirectory() as d: + link_dir = Path(d) + sync_link_files(store_of({'eth1': 'p1', + 'eth2': 'p2'}), link_dir) + sync_link_files(store_of({'eth1': 'p1'}), link_dir) + remaining = sorted(p.name for p in link_dir.glob('*.link')) + self.assertEqual(remaining, ['10-vyos-eth1.link']) + + def test_operator_files_are_left_alone(self): + with tempfile.TemporaryDirectory() as d: + link_dir = Path(d) + custom = link_dir / '99-my-tuning.link' + custom.write_text('[Match]\nType=ether\n\n[Link]\nMTUBytes=9000\n') + sync_link_files(store_of({'eth1': 'p1'}), link_dir) + self.assertTrue(custom.exists()) + + +class TestPortLabelGate(unittest.TestCase): + """The adapter label only orders hardware the hypervisor presented. + Physical servers label their ports too - "NIC1", "Embedded LOM 1 Port 1" - + and reordering those would be the same bug on bare metal. + """ + + def setUp(self): + self.tmp = tempfile.mkdtemp() + self.addCleanup(shutil.rmtree, self.tmp, ignore_errors=True) + self.dmi = os.path.join(self.tmp, 'sys_vendor') + patch = mock.patch('vyos.ifconfig.ifname_store._DMI_SYS_VENDOR', + Path(self.dmi)) + patch.start() + self.addCleanup(patch.stop) + self._dmi('Dell Inc.') + + def _dmi(self, vendor): + with open(self.dmi, 'w') as f: + f.write(vendor + '\n') + + def _make(self, name, label=None, vendor=None, subsystem_vendor=None): + device = os.path.join(self.tmp, name, 'device') + os.makedirs(device) + for attr, value in (('label', label), ('vendor', vendor), + ('subsystem_vendor', subsystem_vendor)): + if value is not None: + with open(os.path.join(device, attr), 'w') as f: + f.write(value + '\n') + return name + + def test_label_index_is_parsed(self): + self._make('eth0', label='Ethernet0') + self._make('eth1', label='Ethernet10') + self.assertEqual(port_label_index('eth0', self.tmp), 0) + # not lexical - Ethernet10 must sort after Ethernet9 + self.assertEqual(port_label_index('eth1', self.tmp), 10) + + def test_labels_physical_servers_use_are_not_adapter_numbers(self): + for i, label in enumerate(['NIC1', 'Integrated NIC 1', 'LAN1', + 'Embedded LOM 1 Port 1', + 'PCIe Slot 1 Port 1', 'Ethernet', + 'Ethernet0 Port 2', '']): + name = self._make(f'eth{i}', label=label) + self.assertEqual(port_label_index(name, self.tmp), _LABEL_UNKNOWN, + f'{label!r} was taken for an adapter number') + + def test_no_label_at_all(self): + self._make('eth0') + self.assertEqual(port_label_index('eth0', self.tmp), _LABEL_UNKNOWN) + + def test_a_vmware_nic_is_recognised_by_its_pci_vendor_alone(self): + # DMI says Dell - smbios.reflectHost does exactly this, and a gate on + # DMI alone would switch itself off + self._make('eth0', vendor='0x15ad') # vmxnet3 + self._make('eth1', subsystem_vendor='0x15ad') # emulated e1000 + self.assertTrue(is_vmware_nic('eth0', self.tmp)) + self.assertTrue(is_vmware_nic('eth1', self.tmp)) + + def test_dmi_still_catches_a_model_whose_pci_ids_are_unknown(self): + self._make('eth0', vendor='0x1af4') + self._dmi('VMware, Inc.') + self.assertTrue(is_vmware_nic('eth0', self.tmp)) + + def test_a_physical_nic_is_left_alone(self): + self._make('eth0', label='Ethernet0', + vendor='0x8086', subsystem_vendor='0x8086') + self.assertFalse(is_vmware_nic('eth0', self.tmp)) + + +class TestPermanentMac(unittest.TestCase): + """Reading the hardware's own address matters wherever a custom MAC may + be in effect: the current address is then the custom one. This used to + come from the hw-id node, which could also simply be stale. + """ + + def test_ethtool_permanent_address(self): + with mock.patch('vyos.ifconfig.ifname_store.rc_cmd', + return_value=(0, 'Permanent address: aa:bb:cc:dd:ee:ff\n')): + self.assertEqual(permanent_mac('eth0'), 'aa:bb:cc:dd:ee:ff') + + def test_zero_address_falls_back_to_sysfs(self): + # some drivers report an all-zero "unsupported" permanent address + # instead of failing outright + with mock.patch('vyos.ifconfig.ifname_store.rc_cmd', + return_value=(0, 'Permanent address: 00:00:00:00:00:00\n')), \ + mock.patch('pathlib.Path.read_text', return_value='11:22:33:44:55:66\n'): + self.assertEqual(permanent_mac('eth0'), '11:22:33:44:55:66') + + def test_ethtool_unsupported_falls_back_to_sysfs(self): + with mock.patch('vyos.ifconfig.ifname_store.rc_cmd', + return_value=(1, 'Operation not supported')), \ + mock.patch('pathlib.Path.read_text', return_value='11:22:33:44:55:66\n'): + self.assertEqual(permanent_mac('eth0'), '11:22:33:44:55:66') + + def test_no_mac_available_returns_empty(self): + with mock.patch('vyos.ifconfig.ifname_store.rc_cmd', return_value=(1, '')), \ + mock.patch('pathlib.Path.read_text', side_effect=OSError): + self.assertEqual(permanent_mac('eth0'), '') + + +class TestDiscoverDevices(unittest.TestCase): + """Only real, independently-addressable hardware is a naming candidate. + Virtual interfaces have no bus device; SR-IOV VFs and hypervisor VF + datapaths are acceleration children sharing another interface's MAC, so + naming them would invent a second interface for one physical port. Being + a member of a bridge, bond or VRF says nothing about the hardware. + """ + + def setUp(self): + self.tmp = tempfile.mkdtemp() + self.addCleanup(shutil.rmtree, self.tmp, ignore_errors=True) + + for patch in (mock.patch('vyos.ifconfig.ifname_store.read_properties', + return_value={}), + mock.patch('vyos.ifconfig.ifname_store.rc_cmd', + return_value=(1, ''))): + patch.start() + self.addCleanup(patch.stop) + + def make(name, mac=None, has_device=True, enslaved=False, is_vf=False, + master_is_device=False, wwan=False): + path = os.path.join(self.tmp, name) + os.mkdir(path) + if has_device: + os.mkdir(os.path.join(path, 'device')) + if is_vf: + pf = os.path.join(self.tmp, f'{name}_pf') + os.mkdir(pf) + os.symlink(pf, os.path.join(path, 'device', 'physfn')) + if enslaved: + master = os.path.join(self.tmp, f'{name}_master') + os.mkdir(master) + if master_is_device: + os.mkdir(os.path.join(master, 'device')) + os.symlink(master, os.path.join(path, 'master')) + if mac: + with open(os.path.join(path, 'address'), 'w') as f: + f.write(mac + '\n') + if wwan: + with open(os.path.join(path, 'uevent'), 'w') as f: + f.write('DEVTYPE=wwan\n') + + self.make = make + make('eth0', mac='aa:aa:aa:aa:aa:00') + make('eth1', mac='aa:aa:aa:aa:aa:01') + make('br0', mac='aa:aa:aa:aa:aa:99', has_device=False) + make('lo', has_device=False) + make('eth8', mac='aa:aa:aa:aa:aa:08', enslaved=True) + make('eth7', mac='aa:aa:aa:aa:aa:07', enslaved=True, + master_is_device=True) + make('eth9', mac='aa:aa:aa:aa:aa:09', is_vf=True) + make('vf_eth0', mac='aa:aa:aa:aa:aa:00') + + def _names(self): + return {d['name'] for d in discover_devices(self.tmp)} + + def test_only_physical_independent_interfaces(self): + self.assertEqual(self._names(), {'eth0', 'eth1', 'eth8'}) + + def test_a_modem_is_discovered_without_an_address(self): + # a modem is given a random address at each probe, so recording it + # would make the same modem look like different hardware every boot + self.make('wwan0', mac='b6:53:dd:66:c2:98', wwan=True) + devices = {d['name']: d for d in discover_devices(self.tmp)} + self.assertIn('wwan0', devices) + self.assertEqual(devices['wwan0']['mac'], '') + self.assertTrue(devices['wwan0']['wwan']) + + def test_a_card_with_no_address_is_still_skipped(self): + # only a modem is kept without one + self.make('eth5') + self.assertNotIn('eth5', self._names()) + + def test_virtual_interfaces_excluded(self): + self.assertNotIn('br0', self._names()) + self.assertNotIn('lo', self._names()) + + def test_bridge_or_bond_member_still_discovered(self): + # a member is still its own card, in its own slot. Dropping it here + # would report the hardware as missing for as long as it is in use. + self.assertIn('eth8', self._names()) + + def test_vf_datapath_enslaved_to_its_parent_excluded(self): + # the accelerated path is enslaved to the interface it belongs to, + # which - unlike a bridge or a bond - is a device itself + self.assertNotIn('eth7', self._names()) + + def test_sriov_virtual_function_excluded(self): + # the general marker, independent of hypervisor - the previous + # exclusion relied on udev rules scoped to a Microsoft DMI vendor + # string and would miss a bare-metal Mellanox VF + self.assertNotIn('eth9', self._names()) + + def test_hypervisor_vf_datapath_excluded(self): + # parked as vf_* by 63-hyperv-vf-net.rules; shares its synthetic + # parent's MAC and must not be taken for a second interface + self.assertNotIn('vf_eth0', self._names()) diff --git a/src/tests/test_migration_interfaces_34_to_35.py b/src/tests/test_migration_interfaces_34_to_35.py new file mode 100644 index 000000000..ad83de424 --- /dev/null +++ b/src/tests/test_migration_interfaces_34_to_35.py @@ -0,0 +1,196 @@ +# Copyright VyOS maintainers and contributors <maintainers@vyos.io> +# +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License version 2 or later as +# published by the Free Software Foundation. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see <http://www.gnu.org/licenses/>. +# +# Coverage for the interfaces 34-to-35 migration (T3871): hw-id leaves the +# CLI, and its bindings are carried into the interface mapping store so that +# no interface changes name across the upgrade. + +import os +import unittest +from unittest import mock + +from helper import prepare_module +from vyos.configtree import ConfigTree +from vyos.ifconfig.ifname_store import STORE_VERSION + +_here = os.path.dirname(__file__) + +migration = prepare_module( + os.path.join(_here, '../migration-scripts/interfaces/34-to-35'), + 'interfaces_34_to_35') + + +def device(name, mac, path): + properties = {'ID_PATH': path, 'ID_BUS': 'pci'} + return {'name': name, 'mac': mac, 'wireless': False, + 'properties': properties, 'key': f'ID_PATH={path}'} + + +CONFIG = """interfaces { + ethernet eth0 { + address "192.0.2.1/24" + hw-id "00:00:5e:00:53:01" + } + ethernet eth1 { + description "uplink" + hw-id "00:00:5e:00:53:02" + } +} +""" + + +class TestMigration34to35(unittest.TestCase): + def setUp(self): + self.config = ConfigTree(CONFIG) + self.saved = [] + + patches = [ + mock.patch.object(migration, 'load_store', + return_value={'version': STORE_VERSION, + 'interfaces': {}}), + mock.patch.object(migration, 'save_store', + side_effect=lambda s: self.saved.append(s) or True), + mock.patch.object(migration, 'sync_link_files'), + ] + for p in patches: + p.start() + self.addCleanup(p.stop) + + def _devices(self, devices): + return mock.patch.object(migration, 'discover_devices', + return_value=devices) + + def test_hwid_nodes_are_removed(self): + with self._devices([]): + migration.migrate(self.config) + self.assertFalse(self.config.exists( + ['interfaces', 'ethernet', 'eth0', 'hw-id'])) + self.assertFalse(self.config.exists( + ['interfaces', 'ethernet', 'eth1', 'hw-id'])) + + def test_other_settings_are_untouched(self): + with self._devices([]): + migration.migrate(self.config) + self.assertEqual( + self.config.return_value(['interfaces', 'ethernet', 'eth0', 'address']), + '192.0.2.1/24') + self.assertEqual( + self.config.return_value(['interfaces', 'ethernet', 'eth1', 'description']), + 'uplink') + + def test_matching_hardware_is_recorded_with_its_slot(self): + devices = [ + device('eth0', '00:00:5e:00:53:01', 'pci-0000:00:04.0'), + device('eth1', '00:00:5e:00:53:02', 'pci-0000:00:05.0'), + ] + with self._devices(devices): + migration.migrate(self.config) + + self.assertEqual(len(self.saved), 1) + entries = self.saved[0]['interfaces'] + self.assertEqual(entries['eth0'], 'ID_PATH=pci-0000:00:04.0') + self.assertEqual(entries['eth1'], 'ID_PATH=pci-0000:00:05.0') + + def test_the_link_files_are_written_from_the_hardware_present(self): + # the files pin a Hyper-V NIC's driver, which only the device says + devices = [ + device('eth0', '00:00:5e:00:53:01', 'pci-0000:00:04.0'), + device('eth1', '00:00:5e:00:53:02', 'pci-0000:00:05.0'), + ] + with self._devices(devices): + migration.migrate(self.config) + migration.sync_link_files.assert_called_once() + self.assertEqual(migration.sync_link_files.call_args.kwargs['devices'], + devices) + + def test_names_are_preserved_even_out_of_slot_order(self): + # a box named by an earlier release in MAC order: eth0 sits in the + # HIGHER slot. The migration must record what is, not what slot + # order would suggest, or the upgrade renames its interfaces. + devices = [ + device('eth0', '00:00:5e:00:53:01', 'pci-0000:00:09.0'), + device('eth1', '00:00:5e:00:53:02', 'pci-0000:00:03.0'), + ] + with self._devices(devices): + migration.migrate(self.config) + entries = self.saved[0]['interfaces'] + self.assertEqual(entries['eth0'], 'ID_PATH=pci-0000:00:09.0') + self.assertEqual(entries['eth1'], 'ID_PATH=pci-0000:00:03.0') + + def test_foreign_config_writes_nothing(self): + # migrations also run against configuration files from other + # machines (config tests, a config copied off another box). Nothing + # matches, so nothing may be written - otherwise this would reserve + # names for hardware that is not here. + with self._devices([device('eth0', 'ff:ff:ff:ff:ff:fe', 'pci-0000:00:04.0')]): + migration.migrate(self.config) + self.assertEqual(self.saved, []) + + def test_existing_store_entry_is_never_overwritten(self): + existing = {'version': STORE_VERSION, + 'interfaces': {'eth0': 'ID_PATH=pci-0000:00:11.0'}} + devices = [ + device('eth0', '00:00:5e:00:53:01', 'pci-0000:00:04.0'), + device('eth1', '00:00:5e:00:53:02', 'pci-0000:00:05.0'), + ] + with mock.patch.object(migration, 'load_store', return_value=existing), \ + self._devices(devices): + migration.migrate(self.config) + entries = self.saved[0]['interfaces'] + self.assertEqual(entries['eth0'], 'ID_PATH=pci-0000:00:11.0') + self.assertEqual(entries['eth1'], 'ID_PATH=pci-0000:00:05.0') + + def test_slot_already_named_does_not_gain_a_second_name(self): + # the store already names this slot. Recording the config's name for + # it too would leave two names on one slot: resolve() gives the slot + # to the first, so the second reserves a name nothing can ever fill, + # and sync_link_files() would point two .link files at one device. + existing = {'version': STORE_VERSION, + 'interfaces': {'eth0': 'ID_PATH=pci-0000:00:04.0'}} + config = ConfigTree('interfaces {\n ethernet eth9 {\n' + ' hw-id "00:00:5e:00:53:01"\n }\n}\n') + with mock.patch.object(migration, 'load_store', return_value=existing), \ + self._devices([device('eth0', '00:00:5e:00:53:01', + 'pci-0000:00:04.0')]): + migration.migrate(config) + + # nothing new to record, so nothing is written + self.assertEqual(self.saved, []) + self.assertNotIn('eth9', existing['interfaces']) + + def test_one_hwid_on_two_nodes_records_only_the_first(self): + # a malformed config binding the same hw-id twice must not put that + # slot in the store under both names. + config = ConfigTree('interfaces {\n' + ' ethernet eth0 {\n' + ' hw-id "00:00:5e:00:53:01"\n }\n' + ' ethernet eth1 {\n' + ' hw-id "00:00:5e:00:53:01"\n }\n}\n') + with self._devices([device('eth0', '00:00:5e:00:53:01', + 'pci-0000:00:04.0')]): + migration.migrate(config) + + entries = self.saved[0]['interfaces'] + self.assertEqual(entries, {'eth0': 'ID_PATH=pci-0000:00:04.0'}) + self.assertEqual(len(set(entries.values())), len(entries)) + + def test_config_without_hwid_is_a_no_op(self): + config = ConfigTree('interfaces {\n ethernet eth0 {\n }\n}\n') + with self._devices([device('eth0', '00:00:5e:00:53:01', 'p')]): + migration.migrate(config) + self.assertEqual(self.saved, []) + + +if __name__ == '__main__': + unittest.main() diff --git a/src/tests/test_net_name_resolve.py b/src/tests/test_net_name_resolve.py index e1530eba7..b529c7164 100644 --- a/src/tests/test_net_name_resolve.py +++ b/src/tests/test_net_name_resolve.py @@ -12,19 +12,18 @@ # You should have received a copy of the GNU General Public License # along with this program. If not, see <http://www.gnu.org/licenses/>. # -# Coverage for T3871 (boot race on hw-id based interface naming): the -# MAC-driven, collision-safe rename plan that replaces the old per-uevent -# udev decision, the find_available() crash it used to hit, the -# ethtool/sysfs permanent-MAC fallback, and the config.boot-availability -# ordering corner case (vyos-net-name-resolve must run from inside -# vyos-router, after the config directory is mounted/decrypted, not as an -# independently-scheduled early-boot unit). +# Coverage for T3871: the boot-time half of persistent interface naming - +# settling, renaming and boot ordering. +# +# The naming decision and hardware discovery both live in +# vyos.ifconfig.ifname_store and are covered by test_ifname_store.py. import json import os import shutil import tempfile import unittest +from pathlib import Path from unittest import mock from helper import prepare_module @@ -34,9 +33,6 @@ _here = os.path.dirname(__file__) resolver = prepare_module( os.path.join(_here, '../system/vyos-net-name-resolve.py'), 'vyos_net_name_resolve') -vyos_net_name = prepare_module( - os.path.join(_here, '../udev/vyos_net_name'), - 'vyos_net_name') _container_patcher = None @@ -56,704 +52,63 @@ def tearDownModule(): _container_patcher.stop() -class TestGetPendingHwidNodes(unittest.TestCase): - """A node that exists under interfaces/{ethernet,wireless} but has no - hw-id (e.g. after `delete interfaces ethernet eth1 hw-id`, the - documented "NIC replaced" remediation) must be surfaced separately - from the MAC-keyed configured dict, since it has no MAC to key by yet - - this is what lets main() try to reclaim it for the exact NIC that - vacated it, instead of silently orphaning its other settings. - """ - - def setUp(self): - self.tmp = tempfile.mkdtemp() - self.addCleanup(shutil.rmtree, self.tmp, ignore_errors=True) - self.config_path = os.path.join(self.tmp, 'config.boot') - self._orig_config_path = resolver.config_path - resolver.config_path = self.config_path - self.addCleanup(setattr, resolver, 'config_path', self._orig_config_path) - - def _write_config(self, text): - with open(self.config_path, 'w') as f: - f.write(text) - - def test_node_with_hwid_not_pending(self): - self._write_config( - 'interfaces {\n' - ' ethernet eth0 {\n' - ' hw-id 00:11:22:33:44:55\n' - ' }\n' - '}\n' - ) - self.assertEqual(resolver.get_pending_hwid_nodes(), - {'ethernet': set(), 'wireless': set()}) - - def test_node_without_hwid_is_pending(self): - self._write_config( - 'interfaces {\n' - ' ethernet eth1 {\n' - ' address 10.99.1.1/24\n' - ' }\n' - '}\n' - ) - self.assertEqual(resolver.get_pending_hwid_nodes(), - {'ethernet': {'eth1'}, 'wireless': set()}) - - def test_mixed_ethernet_and_wireless(self): - self._write_config( - 'interfaces {\n' - ' ethernet eth0 {\n' - ' hw-id 00:11:22:33:44:55\n' - ' }\n' - ' ethernet eth1 {\n' - ' address 10.99.1.1/24\n' - ' }\n' - ' wireless wlan0 {\n' - ' ssid example\n' - ' }\n' - '}\n' - ) - self.assertEqual(resolver.get_pending_hwid_nodes(), - {'ethernet': {'eth1'}, 'wireless': {'wlan0'}}) - - def test_no_interfaces_section(self): - self._write_config('system {\n host-name vyos\n}\n') - self.assertEqual(resolver.get_pending_hwid_nodes(), - {'ethernet': set(), 'wireless': set()}) - - def test_no_config_boot_at_all(self): - # livecd/ISO case - config_path doesn't exist - self.assertEqual(resolver.get_pending_hwid_nodes(), - {'ethernet': set(), 'wireless': set()}) - - -class TestFindAvailableCrashFix(unittest.TestCase): - """find_available() used to raise IndexError on an empty candidate set - (index_list[0]) and ValueError on a name with no trailing digit - (int('')) - both are realistic inputs on a box's first boot, or when a - biosdevname/predefined name has no numeric suffix. - """ - - def test_resolver_empty_set(self): - self.assertEqual(resolver.find_available(set(), 'eth'), 'eth0') - - def test_resolver_no_prefix_match(self): - self.assertEqual(resolver.find_available({'wlan3'}, 'eth'), 'eth0') - - def test_resolver_contiguous(self): - names = {'eth0', 'eth1', 'eth2'} - self.assertEqual(resolver.find_available(names, 'eth'), 'eth3') - - def test_resolver_fills_hole(self): - self.assertEqual(resolver.find_available({'eth2', 'eth5'}, 'eth'), 'eth3') - - def test_resolver_no_hole_appends(self): - self.assertEqual(resolver.find_available({'eth2', 'eth3'}, 'eth'), 'eth4') - - def test_vyos_net_name_empty_dict(self): - self.assertEqual(vyos_net_name.find_available({}, 'eth'), 'eth0') - - def test_vyos_net_name_no_digit_suffix(self): - self.assertEqual( - vyos_net_name.find_available({'m': 'wlan3'}, 'eth'), 'eth0') - - def test_vyos_net_name_contiguous(self): - intfs = {'a': 'eth0', 'b': 'eth1'} - self.assertEqual(vyos_net_name.find_available(intfs, 'eth'), 'eth2') - - def test_vyos_net_name_is_available(self): - self.assertTrue(vyos_net_name.is_available({'a': 'eth0'}, 'eth1')) - self.assertFalse(vyos_net_name.is_available({'a': 'eth0'}, 'eth0')) - - def test_vyos_net_name_mod_ifname(self): - self.assertEqual(vyos_net_name.mod_ifname('e5'), 'eth3') - self.assertEqual(vyos_net_name.mod_ifname('e2'), 'eth0') - self.assertEqual(vyos_net_name.mod_ifname('e1'), 'eth1') - self.assertEqual(vyos_net_name.mod_ifname('wlan0'), 'wlan0') - - -class TestUnmatchedCandidates(unittest.TestCase): - """Field report: an unconfigured NIC that happens to be squatting on a - DIFFERENT interface's configured hw-id slot this boot (e.g. due to - probe-order scrambling) was excluded here just because - compute_rename_plan() already scheduled it for eviction. That skipped - it past match_pending_nodes() entirely, so a squatter that was really - the exact NIC a pending node should reclaim - or one that made a - reclaim genuinely ambiguous - fell through to ordinary bootstrap - naming and got a permanent hw-id with no ambiguity check at all. - Squatters must stay in this pool; only their eviction destination - (still computed by compute_rename_plan()) is a fallback, not a reason - to hide them from reclaim/ambiguity handling. - """ - - def test_squatter_is_still_a_candidate(self): - configured = {'m0': 'eth1'} - current = {'eth1': 'squatter-mac', 'eth9': 'm0'} - existing_plan = {'eth1': 'eth3', 'eth9': 'eth1'} - candidates = resolver.unmatched_candidates(configured, current, existing_plan) - self.assertEqual(candidates, [('squatter-mac', 'eth1')]) - - def test_rightful_owner_still_excluded(self): - configured = {'m0': 'eth0'} - current = {'eth0': 'm0'} - candidates = resolver.unmatched_candidates(configured, current, {}) - self.assertEqual(candidates, []) - - -class TestMatchPendingNodes(unittest.TestCase): - """A pending node's hw-id is unknown by construction - there's no MAC - to check a candidate against - so matching must only ever happen in - the unambiguous exactly-one-pending/exactly-one-candidate case per - type. Confirmed in the field: a naive deterministic fill (sorting - pending nodes and candidates together like numeric gaps) silently - bound a configured node's address to a different physical NIC than - its own, because an already-provisioned box's existing names have no - relationship to PCIe/MAC sort order. Guessing in any other - cardinality risks the same mistake; leaving the node unresolved and - reported is safer. - """ - - def setUp(self): - patcher = mock.patch.object(resolver, 'is_wireless_interface', - return_value=False) - self.is_wireless = patcher.start() - self.addCleanup(patcher.stop) - - def test_unambiguous_single_pending_single_candidate_matches(self): - pending = {'ethernet': {'eth1'}, 'wireless': set()} - candidates = [('m1', 'eth9')] - matched = resolver.match_pending_nodes(pending, candidates) - self.assertEqual(matched, {'m1': 'eth1'}) - - def test_two_pending_one_candidate_no_match(self): - # can't tell which of the two pending nodes this one candidate - # belongs to - neither is matched. - pending = {'ethernet': {'eth1', 'eth4'}, 'wireless': set()} - candidates = [('m1', 'eth9')] - matched = resolver.match_pending_nodes(pending, candidates) - self.assertEqual(matched, {}) - - def test_one_pending_two_candidates_no_match(self): - # the field-reported shape: an unrelated second candidate (e.g. - # freed by a different interface's config being fully deleted in - # the same boot) is enough to make this genuinely ambiguous, even - # though it isn't itself pending anything. - pending = {'ethernet': {'eth1'}, 'wireless': set()} - candidates = [('m1', 'eth9'), ('m2', 'eth8')] - matched = resolver.match_pending_nodes(pending, candidates) - self.assertEqual(matched, {}) - - def test_ethernet_and_wireless_matched_independently(self): - self.is_wireless.side_effect = lambda name: name == 'radio0' - pending = {'ethernet': {'eth1'}, 'wireless': {'wlan2'}} - candidates = [('m1', 'ifaceB'), ('m2', 'radio0')] - matched = resolver.match_pending_nodes(pending, candidates) - self.assertEqual(matched, {'m1': 'eth1', 'm2': 'wlan2'}) - - def test_no_pending_returns_empty(self): - pending = {'ethernet': set(), 'wireless': set()} - candidates = [('m1', 'eth9')] - matched = resolver.match_pending_nodes(pending, candidates) - self.assertEqual(matched, {}) - - -class TestComputeRenamePlan(unittest.TestCase): - """The authoritative rename plan is what fixes the multi-vendor NIC - boot race: it is driven purely by MAC address, independent of - ifindex/probe order, and must never produce a colliding assignment. - """ - - def test_noop_when_already_correct(self): - configured = {'m0': 'eth0', 'm1': 'eth1'} - current = {'eth0': 'm0', 'eth1': 'm1'} - self.assertEqual(resolver.compute_rename_plan(configured, current), {}) - - def test_rename_independent_of_probe_order(self): - # m0/m1 landed on unrelated names this boot (e.g. a different - # vendor driver won the probe race) - the plan must still point - # them at their configured targets regardless. - configured = {'m0': 'eth0', 'm1': 'eth1'} - current = {'eth5': 'm0', 'eth3': 'm1'} - plan = resolver.compute_rename_plan(configured, current) - self.assertEqual(plan, {'eth5': 'eth0', 'eth3': 'eth1'}) - - def test_full_swap_cycle(self): - # eth0 and eth1 need to trade names - a naive from->to rename - # would collide; the two-phase scratch-name staging must not. - configured = {'m0': 'eth1', 'm1': 'eth0'} - current = {'eth0': 'm0', 'eth1': 'm1'} - plan = resolver.compute_rename_plan(configured, current) - self.assertEqual(plan, {'eth0': 'eth1', 'eth1': 'eth0'}) - - def test_squatter_relocated(self): - # an unconfigured interface currently sits on a name a configured - # hw-id needs - it must be moved out of the way, not left there. - configured = {'m0': 'eth0'} - current = {'eth0': 'unconfigured-mac', 'eth5': 'm0'} - plan = resolver.compute_rename_plan(configured, current) - self.assertEqual(plan['eth5'], 'eth0') - self.assertNotEqual(plan['eth0'], 'eth0') - self.assertTrue(plan['eth0'].startswith('eth')) - - def test_squatter_never_relocated_onto_a_missing_hwid_target(self): - # macMissing's hardware hasn't shown up this boot (removed/faulty/ - # slow driver), so its reserved target 'eth1' never appears in - # `current` and the main hw-id loop skips it entirely (source is - # None) - the squatter being relocated off of 'eth0' must still not - # be handed 'eth1', or it would need relocating again the moment - # that hardware actually appears. - configured = {'macA': 'eth0', 'macMissing': 'eth1'} - current = {'eth0': 'squatterMac', 'eth2': 'macA'} - plan = resolver.compute_rename_plan(configured, current) - self.assertEqual(plan.get('eth2'), 'eth0') - self.assertNotEqual(plan.get('eth0'), 'eth1') - - def test_squatter_never_relocated_onto_a_pending_hwid_less_node(self): - # 'eth1' has no hw-id at all (e.g. `delete interfaces ethernet - # eth1 hw-id`, the documented remediation for a replaced NIC) but - # its config node - and settings like address - still exist. A - # squatter being relocated off of 'eth0' must not be handed 'eth1' - # either, or it would permanently steal the exact slot the - # original NIC needs to reclaim. - configured = {'macA': 'eth0'} - pending = {'ethernet': {'eth1'}, 'wireless': set()} - current = {'eth0': 'squatterMac', 'eth2': 'macA'} - plan = resolver.compute_rename_plan(configured, current, pending) - self.assertEqual(plan.get('eth2'), 'eth0') - self.assertNotEqual(plan.get('eth0'), 'eth1') - - def test_missing_hardware_no_crash_no_entry(self): - # hw-id configured but the NIC never showed up (missing/faulty) - - # must be skipped, not crash or incorrectly assign another interface. - configured = {'m0': 'eth0', 'missing-mac': 'eth9'} - current = {'eth5': 'm0'} - plan = resolver.compute_rename_plan(configured, current) - self.assertEqual(plan, {'eth5': 'eth0'}) - - def test_rename_introducing_a_gap(self): - # user edits the CLI so m2's node is renamed eth2 -> eth10 while - # keeping the same hw-id, leaving eth5..eth9 unused. - configured = { - 'm0': 'eth0', 'm1': 'eth1', 'm2': 'eth10', - 'm3': 'eth3', 'm4': 'eth4', - } - current = { - 'eth0': 'm0', 'eth1': 'm1', 'eth2': 'm2', - 'eth3': 'm3', 'eth4': 'm4', - } - plan = resolver.compute_rename_plan(configured, current) - self.assertEqual(plan, {'eth2': 'eth10'}) - - def test_rename_introducing_a_gap_with_scrambled_fastpath(self): - # same as above, but this boot's cosmetic udev fast-path lands m2 - # on an unrelated name and coincidentally puts a different NIC on - # the now-meaningless old 'eth2'. - configured = { - 'm0': 'eth0', 'm1': 'eth1', 'm2': 'eth10', - 'm3': 'eth3', 'm4': 'eth4', - } - current = { - 'eth0': 'm0', 'eth1': 'm1', 'eth7': 'm2', - 'eth2': 'm3', 'eth4': 'm4', - } - plan = resolver.compute_rename_plan(configured, current) - self.assertEqual(plan.get('eth7'), 'eth10') - self.assertEqual(plan.get('eth2'), 'eth3') - self.assertNotIn('eth2', configured.values()) +def device(name, mac, path='', wireless=False): + return {'name': name, 'mac': mac, 'path': path, 'wireless': wireless, + 'properties': {}} class TestWaitForSettle(unittest.TestCase): - """There is no specific MAC to wait for when bootstrap-naming hardware - without a configured hw-id, so settling is detected as N consecutive - identical discover_physical_interfaces() snapshots instead. + """There is no specific MAC to wait for when naming hardware the store + has never seen, so settling is detected as N consecutive identical + discover_physical_interfaces() snapshots instead. """ def test_returns_after_n_consecutive_stable_polls(self): - snapshots = [ - {'eth0': 'm0'}, # still changing - {'eth0': 'm0', 'eth1': 'm1'}, # still changing - {'eth0': 'm0', 'eth1': 'm1'}, # 1st stable repeat - {'eth0': 'm0', 'eth1': 'm1'}, # 2nd stable repeat -> settled - ] - with mock.patch.object(resolver, 'discover_physical_interfaces', - side_effect=snapshots): - result = resolver.wait_for_settle( - {}, timeout=5, poll=0, stable_polls=3) - self.assertEqual(result, {'eth0': 'm0', 'eth1': 'm1'}) + one = [device('eth0', 'm0')] + two = [device('eth0', 'm0'), device('eth1', 'm1')] + snapshots = [one, two, two, two] + with mock.patch.object(resolver, 'discover_devices', + side_effect=snapshots): + result = resolver.wait_for_settle([], timeout=5, poll=0, + stable_polls=3) + self.assertEqual(resolver._snapshot(result), + {('eth0', 'm0'), ('eth1', 'm1')}) def test_bounded_by_timeout_when_never_stable(self): counter = iter(range(1000)) def ever_changing(*_a, **_kw): - return {'eth0': f'm{next(counter)}'} + return [device('eth0', f'm{next(counter)}')] clock = iter([0, 0.1, 20, 20]) - with mock.patch.object(resolver, 'discover_physical_interfaces', - side_effect=ever_changing), \ + with mock.patch.object(resolver, 'discover_devices', + side_effect=ever_changing), \ mock.patch('time.monotonic', side_effect=lambda: next(clock)): - result = resolver.wait_for_settle( - {}, timeout=10, poll=0, stable_polls=3) + result = resolver.wait_for_settle([], timeout=10, poll=0, + stable_polls=3) # must return SOMETHING (the last snapshot seen), not hang or crash - self.assertIn('eth0', result) + self.assertEqual(len(result), 1) def test_stable_polls_of_one_returns_on_initial_snapshot(self): - with mock.patch.object(resolver, 'discover_physical_interfaces') as m: - result = resolver.wait_for_settle( - {'eth0': 'm0'}, timeout=5, poll=0, stable_polls=1) + initial = [device('eth0', 'm0')] + with mock.patch.object(resolver, 'discover_devices') as m: + result = resolver.wait_for_settle(initial, timeout=5, poll=0, + stable_polls=1) m.assert_not_called() - self.assertEqual(result, {'eth0': 'm0'}) - - -class TestIsWirelessInterface(unittest.TestCase): - """Wireless detection must be race-free - it has to be correct even - when the interface's current name is a leftover from the cosmetic - udev-time fast path and doesn't look like a wlan name at all. - """ - - def setUp(self): - self.tmp = tempfile.mkdtemp() - self.addCleanup(shutil.rmtree, self.tmp, ignore_errors=True) - # phy80211 is always a symlink to a real sysfs node in practice - - # exists() follows symlinks, so the test target must actually exist - self.phy_target = os.path.join(self.tmp, 'phy0') - os.mkdir(self.phy_target) - - def test_phy80211_present_is_wireless(self): - path = os.path.join(self.tmp, 'wlan0') - os.mkdir(path) - os.symlink(self.phy_target, os.path.join(path, 'phy80211')) - self.assertTrue(resolver.is_wireless_interface('wlan0', self.tmp)) - - def test_no_phy80211_is_not_wireless(self): - os.mkdir(os.path.join(self.tmp, 'eth0')) - self.assertFalse(resolver.is_wireless_interface('eth0', self.tmp)) - - def test_detection_independent_of_misleading_name(self): - # named like a leftover cosmetic ethernet guess, but it's a real - # wifi card - must still be classified wireless - path = os.path.join(self.tmp, 'eth7') - os.mkdir(path) - os.symlink(self.phy_target, os.path.join(path, 'phy80211')) - self.assertTrue(resolver.is_wireless_interface('eth7', self.tmp)) - - -class TestPcieDistance(unittest.TestCase): - """PCIe hop-depth is what lets bootstrap naming reflect physical slot - position instead of raw MAC magnitude - a NIC wired straight to the - root complex must not sort after an add-in card just because its MAC - happens to be numerically higher. - """ - - def setUp(self): - self.tmp = tempfile.mkdtemp() - self.addCleanup(shutil.rmtree, self.tmp, ignore_errors=True) - # fake /sys/devices tree, separate from the fake /sys/class/net - # entries, so symlink targets can point at realistic PCI-bus-shaped - # paths the way a real /sys/class/net/<if>/device symlink does - self.devices_root = os.path.join(self.tmp, 'devices') - os.makedirs(self.devices_root) - - def _make_iface_pointing_at(self, name, *path_segments): - """Create <tmp>/<name>/device as a symlink to a fake sysfs device - node at devices_root/<path_segments...>. - """ - target = os.path.join(self.devices_root, *path_segments) - os.makedirs(target, exist_ok=True) - iface_path = os.path.join(self.tmp, name) - os.mkdir(iface_path) - os.symlink(target, os.path.join(iface_path, 'device')) - - def test_shallow_device_one_pci_segment(self): - # e.g. a NIC directly under the root complex: pci0000:00/0000:00:1f.6 - self._make_iface_pointing_at('eth0', 'pci0000:00', '0000:00:1f.6') - self.assertEqual(resolver.pcie_distance('eth0', self.tmp), 1) - - def test_deep_device_behind_bridges(self): - # e.g. an add-in card behind two cascaded PCIe bridges - self._make_iface_pointing_at( - 'eth1', 'pci0000:00', '0000:00:1c.0', - '0000:01:00.0', '0000:02:04.0') - self.assertEqual(resolver.pcie_distance('eth1', self.tmp), 3) - - def test_virtio_indirection_not_miscounted(self): - # verified-real virtio_net layout: device -> virtioN node, whose - # OWN parent is the true PCI BDF - the virtioN segment itself must - # be skipped, not counted as a hop. - self._make_iface_pointing_at( - 'eth2', 'pci0000:00', '0000:00:12.0', 'virtio2', 'net') - self.assertEqual(resolver.pcie_distance('eth2', self.tmp), 1) - - def test_missing_device_symlink_returns_sentinel(self): - os.mkdir(os.path.join(self.tmp, 'eth3')) # no 'device' entry at all - self.assertEqual(resolver.pcie_distance('eth3', self.tmp), - resolver.PCIE_DISTANCE_UNKNOWN) - - def test_broken_device_symlink_returns_sentinel(self): - iface_path = os.path.join(self.tmp, 'eth4') - os.mkdir(iface_path) - os.symlink(os.path.join(self.devices_root, 'does-not-exist'), - os.path.join(iface_path, 'device')) - self.assertEqual(resolver.pcie_distance('eth4', self.tmp), - resolver.PCIE_DISTANCE_UNKNOWN) - - def test_no_pci_segment_at_all_returns_sentinel(self): - # simulated USB NIC: resolved path has no PCI BDF component - self._make_iface_pointing_at('eth5', 'usb1', '1-1', '1-1:1.0') - self.assertEqual(resolver.pcie_distance('eth5', self.tmp), - resolver.PCIE_DISTANCE_UNKNOWN) - - -class TestComputeBootstrapPlan(unittest.TestCase): - """Bootstrap naming is what makes a box's very first boot (before any - hw-id exists) deterministic, the same way hw-id makes every boot after - that deterministic - it must depend only on PCIe topology and MAC - rank, never on whatever name the racy cosmetic fast-path happened to - assign. pcie_distance is held constant by default in these tests, so - they exercise pure MAC-rank ordering (see TestPcieDistance for the - topology-ordering cases specifically). - """ - - def setUp(self): - patcher = mock.patch.object(resolver, 'is_wireless_interface', - return_value=False) - self.is_wireless = patcher.start() - self.addCleanup(patcher.stop) - - distance_patcher = mock.patch.object(resolver, 'pcie_distance', - return_value=0) - self.pcie_distance = distance_patcher.start() - self.addCleanup(distance_patcher.stop) - - def test_sorted_by_mac_independent_of_current_names(self): - # names are in the OPPOSITE order of their MACs - current = {'eth5': 'bb', 'eth2': 'aa'} - plan = resolver.compute_bootstrap_plan({}, current, {}) - self.assertEqual(plan, {'eth2': 'eth0', 'eth5': 'eth1'}) - - def test_noop_when_already_in_canonical_rank_order(self): - current = {'eth0': 'aa', 'eth1': 'bb'} - plan = resolver.compute_bootstrap_plan({}, current, {}) - self.assertEqual(plan, {}) - - def test_configured_targets_are_never_overwritten(self): - configured = {'m0': 'eth0'} - current = {'eth0': 'm0', 'eth9': 'm2', 'eth8': 'm1'} - plan = resolver.compute_bootstrap_plan(configured, current, {}) - self.assertNotIn('eth0', plan.values()) - self.assertEqual(plan.get('eth8'), 'eth1') - self.assertEqual(plan.get('eth9'), 'eth2') - - def test_new_interface_backfills_a_gap_with_no_pending_node(self): - # eth1 is a gap between two hw-id'd interfaces, but nothing in - # config.boot reserves it (no hw-id, no pending node either - the - # interface was fully deleted at some point, or never existed) - - # a new, unrelated NIC discovered this boot is free to take it, - # the same as it would on a system with no config for it at all. - configured = {'m0': 'eth0', 'm2': 'eth2'} - current = {'eth0': 'm0', 'eth2': 'm2', 'eth9': 'new-mac'} - plan = resolver.compute_bootstrap_plan(configured, current, {}) - self.assertEqual(plan, {'eth9': 'eth1'}) - - def test_multiple_new_interfaces_backfill_gaps_in_order(self): - configured = {'m0': 'eth0', 'm2': 'eth2'} - current = { - 'eth0': 'm0', 'eth2': 'm2', - 'ethB': 'bb', 'ethA': 'aa', - } - plan = resolver.compute_bootstrap_plan(configured, current, {}) - # lower mac 'aa' takes the gap at eth1 first, 'bb' continues to - # the next free slot, eth3 - not "skip past the gap to eth3/eth4" - self.assertEqual(plan, {'ethA': 'eth1', 'ethB': 'eth3'}) - - def test_ethernet_and_wireless_numbered_independently(self): - self.is_wireless.side_effect = lambda name: name == 'radio0' - current = {'ifaceB': 'bb', 'radio0': 'aa'} - plan = resolver.compute_bootstrap_plan({}, current, {}) - self.assertEqual(plan.get('radio0'), 'wlan0') - self.assertEqual(plan.get('ifaceB'), 'eth0') - - def test_squatter_already_in_existing_plan_excluded(self): - # 'eth0' is a squatter the hw-id plan already decided to move away - # (existing_plan key) - it IS still bootstrap candidacy (see - # unmatched_candidates()), but must never be handed a second, - # conflicting assignment on top of its existing_plan eviction. - existing_plan = {'eth0': 'eth3'} - current = {'eth0': 'unconfigured-mac', 'eth7': 'm0'} - plan = resolver.compute_bootstrap_plan({'m0': 'eth1'}, current, - existing_plan) - self.assertNotIn('eth0', plan) - - def test_bootstrap_targets_never_collide_with_existing_plan_values(self): - # existing_plan says an interface currently named 'oldname' is - # moving to 'eth0' - nothing else in `current` is named 'eth0' or - # 'eth*' at all, so if that target isn't reserved, find_available() - # would freely hand 'eth0' to the bootstrap candidate too. - existing_plan = {'oldname': 'eth0'} - current = {'oldname': 'm0', 'newnic': 'unconfigured-mac'} - plan = resolver.compute_bootstrap_plan({'m0': 'eth0'}, current, - existing_plan) - self.assertNotEqual(plan.get('newnic'), 'eth0') - - def test_smaller_pcie_distance_wins_over_higher_mac(self): - # 'bb' has the numerically higher MAC but sits closer to the root - # complex - it must be named first despite losing on MAC alone. - current = {'eth5': 'bb', 'eth2': 'aa'} - self.pcie_distance.side_effect = lambda name: {'eth5': 0, 'eth2': 3}[name] - plan = resolver.compute_bootstrap_plan({}, current, {}) - self.assertEqual(plan, {'eth5': 'eth0', 'eth2': 'eth1'}) - - def test_same_pcie_distance_falls_back_to_mac_order(self): - current = {'eth5': 'bb', 'eth2': 'aa'} - self.pcie_distance.side_effect = lambda name: 2 # tie for both - plan = resolver.compute_bootstrap_plan({}, current, {}) - self.assertEqual(plan, {'eth2': 'eth0', 'eth5': 'eth1'}) - - def test_unknown_pcie_distance_sorts_last(self): - # 'aa' has the numerically lowest MAC but an undeterminable bus - # position (e.g. a USB NIC) - it must not jump the queue. - current = {'eth9': 'cc', 'eth5': 'bb', 'ethX': 'aa'} - self.pcie_distance.side_effect = lambda name: { - 'eth9': 1, 'eth5': 2, 'ethX': resolver.PCIE_DISTANCE_UNKNOWN, - }[name] - plan = resolver.compute_bootstrap_plan({}, current, {}) - self.assertEqual(plan, {'eth9': 'eth0', 'eth5': 'eth1', 'ethX': 'eth2'}) - - def test_pcie_distance_ordering_independent_of_ethernet_wireless_split(self): - # distance-based ordering applies within each type group separately, - # same as MAC does today - a wlan candidate's distance must not - # affect eth numbering or vice versa. - self.is_wireless.side_effect = lambda name: name == 'radio0' - current = {'ifaceB': 'bb', 'radio0': 'aa'} - self.pcie_distance.side_effect = lambda name: { - 'ifaceB': 5, 'radio0': 0, - }[name] - plan = resolver.compute_bootstrap_plan({}, current, {}) - self.assertEqual(plan.get('radio0'), 'wlan0') - self.assertEqual(plan.get('ifaceB'), 'eth0') - - -class TestComputeBootstrapPlanGapFill(unittest.TestCase): - """compute_bootstrap_plan() fills ordinary numeric gaps (no config - trace at all) deterministically by PCIe distance then MAC. A pending - node (hw-id deleted, node kept - see get_pending_hwid_nodes()) is a - separate concern here: passing `pending` reserves its name so an - unrelated candidate can never squat on it and inherit its settings - - only match_pending_nodes() may fill it, and only when unambiguous. - """ - - def setUp(self): - patcher = mock.patch.object(resolver, 'is_wireless_interface', - return_value=False) - self.is_wireless = patcher.start() - self.addCleanup(patcher.stop) - - distance_patcher = mock.patch.object(resolver, 'pcie_distance', - return_value=0) - self.pcie_distance = distance_patcher.start() - self.addCleanup(distance_patcher.stop) - - def test_open_name_filled_like_an_ordinary_gap(self): - # 'eth1' has nothing configured for it and no pending reservation - # - it is simply the lowest open name. - configured = {'m0': 'eth0'} - current = {'eth0': 'm0', 'eth9': 'new-mac'} - plan = resolver.compute_bootstrap_plan(configured, current, {}) - self.assertEqual(plan.get('eth9'), 'eth1') - - def test_ascending_fill_has_no_preference_between_open_names(self): - # two open names (eth1, eth4 - the reason either is open makes no - # difference) and two candidates - the lower-sorted candidate - # takes the lower-numbered open name, unconditionally. - configured = {'m0': 'eth0', 'm2': 'eth2', 'm3': 'eth3'} - current = {'eth0': 'm0', 'eth2': 'm2', 'eth3': 'm3', - 'eth9': 'bb', 'eth8': 'aa'} - plan = resolver.compute_bootstrap_plan(configured, current, {}) - self.assertEqual(plan.get('eth8'), 'eth1') - self.assertEqual(plan.get('eth9'), 'eth4') - - def test_two_plain_gaps_fill_in_ascending_order(self): - # two fully-deleted interfaces (no hw-id, no pending node either) - # - both ordinary gaps, filled by the same ascending PCIe/MAC - # sort as any other unconfigured hardware. - configured = { - '00:00:5e:00:53:00': 'eth0', '00:00:5e:00:53:01': 'eth1', - '00:00:5e:00:53:03': 'eth3', '00:00:5e:00:53:05': 'eth5', - '00:00:5e:00:53:06': 'eth6', '00:00:5e:00:53:07': 'eth7', - } - current = {name: mac for mac, name in configured.items()} - current.update({'eth9': '00:00:5e:00:53:02', - 'eth8': '00:00:5e:00:53:04'}) - plan = resolver.compute_bootstrap_plan(configured, current, {}) - self.assertEqual(plan.get('eth9'), 'eth2') - self.assertEqual(plan.get('eth8'), 'eth4') - - def test_open_names_currently_squatted_by_a_rightful_mover_are_not_taken(self): - # real boot reproduction (via the resolver's own status file, - # captured mid-failure): the cosmetic fast-path this boot had - # every configured mac sitting on some OTHER configured mac's - # target name (a full rotation), including the two plain gaps - # (eth1, eth6) both currently squatted by macs that are about to - # move elsewhere via a rightful-owner move. existing_plan's - # SOURCE names looked "taken" at the exact moment this function - # ran, but safe_bulk_rename()'s two-phase staging vacates every - # source before any target is claimed - so they must not count - # as taken, or the two real candidates get needlessly pushed to - # fresh eth8/eth9-style slots instead of their own open names. - configured = { - '00:00:5e:00:53:00': 'eth0', '00:00:5e:00:53:02': 'eth2', - '00:00:5e:00:53:03': 'eth3', '00:00:5e:00:53:04': 'eth4', - '00:00:5e:00:53:05': 'eth5', '00:00:5e:00:53:07': 'eth7', - } - current = { - 'eth4': '00:00:5e:00:53:00', 'eth6': '00:00:5e:00:53:02', - 'eth7': '00:00:5e:00:53:03', 'eth2': '00:00:5e:00:53:04', - 'eth3': '00:00:5e:00:53:05', 'eth1': '00:00:5e:00:53:07', - 'eth0': '00:00:5e:00:53:06', # unconfigured - squats on eth0 - 'eth5': '00:00:5e:00:53:01', # unconfigured - squats on eth5 - } - existing_plan = resolver.compute_rename_plan(configured, current) - plan = resolver.compute_bootstrap_plan(configured, current, - existing_plan) - self.assertEqual(plan.get('eth5'), 'eth1') - self.assertEqual(plan.get('eth0'), 'eth6') - - def test_pending_name_reserved_against_unrelated_bootstrap_candidate(self): - # 'eth1' is pending (no hw-id) - two unrelated new NICs discovered - # this boot must not land on it just because sequential numbering - # would otherwise reach it as the second assignment. - configured = {} - pending = {'ethernet': {'eth1'}, 'wireless': set()} - current = {'eth9': 'aa', 'eth8': 'bb'} - plan = resolver.compute_bootstrap_plan(configured, current, {}, - pending=pending) - self.assertEqual(plan.get('eth9'), 'eth0') - self.assertEqual(plan.get('eth8'), 'eth2') - self.assertNotEqual(plan.get('eth9'), 'eth1') - self.assertNotEqual(plan.get('eth8'), 'eth1') - - def test_reclaimed_candidate_excluded_from_ordinary_bootstrap(self): - # main() already matched this candidate to a pending node via - # match_pending_nodes() - it must not also be assigned a fresh - # bootstrap name here, even if its current name differs from the - # reclaimed target (main() handles the actual rename itself). - configured = {} - current = {'eth9': 'm1', 'eth8': 'm2'} - plan = resolver.compute_bootstrap_plan(configured, current, {}, - reclaimed_macs={'m1'}) - self.assertNotIn('eth9', plan) - self.assertIn('eth8', plan) + self.assertEqual(result, initial) class TestSafeBulkRename(unittest.TestCase): - """The two-phase rename must accurately report what actually happened - - a phase-2 (scratch -> final target) failure must not be reported as a + """Only hardware the store has never seen should reach this on a settled + system, but when it does the two-phase rename must report accurately: a + phase-2 (scratch -> final target) failure must not be reported as a successful rename, and must not leave the interface down indefinitely under a name nothing else knows about. """ def setUp(self): patcher = mock.patch.object(resolver, 'get_ifindex', - side_effect=lambda name: name) + side_effect=lambda name: name) patcher.start() self.addCleanup(patcher.stop) @@ -763,8 +118,6 @@ class TestSafeBulkRename(unittest.TestCase): self.assertEqual(applied, {'eth5': 'eth0'}) def test_phase_two_failure_is_not_reported_as_applied(self): - # 'ip link set dev vyetheth5 name eth0' fails (exit 1); everything - # else (down, the phase-1 rename to scratch) succeeds def fake_run(command, *_a, **_kw): return 1 if command == 'ip link set dev vyetheth5 name eth0' else 0 @@ -772,7 +125,6 @@ class TestSafeBulkRename(unittest.TestCase): applied = resolver.safe_bulk_rename({'eth5': 'eth0'}) self.assertNotIn('eth5', applied) - self.assertNotEqual(applied.get('eth5'), 'eth0') def test_phase_two_failure_brings_scratch_name_back_up(self): calls = [] @@ -792,850 +144,255 @@ class TestSafeBulkRename(unittest.TestCase): return 1 if command == 'ip link set dev vyetheth5 name eth0' else 0 with mock.patch.object(resolver, 'run', side_effect=fake_run): - applied = resolver.safe_bulk_rename({'eth5': 'eth0', 'eth9': 'eth1'}) + applied = resolver.safe_bulk_rename({'eth5': 'eth0', + 'eth9': 'eth1'}) self.assertNotIn('eth5', applied) self.assertEqual(applied.get('eth9'), 'eth1') -class TestGetPermanentMac(unittest.TestCase): - """Part of the original race is reading a MAC before the driver has - programmed the permanent address. Verify the ethtool-first, - sysfs-fallback behaviour, including drivers that report an all-zero - 'unsupported' permanent address instead of failing outright. - """ - - def test_ethtool_permanent_address(self): - with mock.patch.object( - resolver, 'rc_cmd', - return_value=(0, 'Permanent address: aa:bb:cc:dd:ee:ff\n')): - self.assertEqual(resolver.get_permanent_mac('eth0'), - 'aa:bb:cc:dd:ee:ff') - - def test_ethtool_zero_address_falls_back_to_sysfs(self): - with mock.patch.object( - resolver, 'rc_cmd', - return_value=(0, 'Permanent address: 00:00:00:00:00:00\n')), \ - mock.patch('pathlib.Path.read_text', - return_value='11:22:33:44:55:66\n'): - self.assertEqual(resolver.get_permanent_mac('eth0'), - '11:22:33:44:55:66') - - def test_ethtool_unsupported_falls_back_to_sysfs(self): - with mock.patch.object( - resolver, 'rc_cmd', - return_value=(1, 'Operation not supported')), \ - mock.patch('pathlib.Path.read_text', - return_value='11:22:33:44:55:66\n'): - self.assertEqual(resolver.get_permanent_mac('eth0'), - '11:22:33:44:55:66') - - def test_no_mac_available_returns_empty(self): - with mock.patch.object(resolver, 'rc_cmd', return_value=(1, '')), \ - mock.patch('pathlib.Path.read_text', side_effect=OSError): - self.assertEqual(resolver.get_permanent_mac('eth0'), '') - - -class TestDiscoverPhysicalInterfaces(unittest.TestCase): - """Only interfaces with a real bus 'device' link are candidates - - bridges/bonds/VLANs/veth must be excluded regardless of name, since - this replaces the racy udev DRIVERS=="?*" check with a stable, - post-settle sysfs read. +class TestRefuseRenamesInUse(unittest.TestCase): + """A name the running system answers to must not move. The map is only + free to change a name while nothing is using it, which at boot is every + interface and afterwards is only a card which has just arrived. """ def setUp(self): self.tmp = tempfile.mkdtemp() self.addCleanup(shutil.rmtree, self.tmp, ignore_errors=True) - def make_iface(name, mac=None, has_device=True): - path = os.path.join(self.tmp, name) - os.mkdir(path) - if has_device: - os.mkdir(os.path.join(path, 'device')) - if mac: - with open(os.path.join(path, 'address'), 'w') as f: - f.write(mac + '\n') - - make_iface('eth0', mac='aa:aa:aa:aa:aa:00') - make_iface('eth1', mac='aa:aa:aa:aa:aa:01') - make_iface('br0', mac='aa:aa:aa:aa:aa:99', has_device=False) - make_iface('lo', has_device=False) - - def test_only_physical_interfaces_with_mac(self): - with mock.patch.object(resolver, 'rc_cmd', return_value=(1, '')): - found = resolver.discover_physical_interfaces(self.tmp) - self.assertEqual(found, { - 'eth0': 'aa:aa:aa:aa:aa:00', - 'eth1': 'aa:aa:aa:aa:aa:01', - }) - self.assertNotIn('br0', found) - self.assertNotIn('lo', found) - - -class TestSyncRescanHints(unittest.TestCase): - """New/unconfigured hardware must still be surfaced to - vyos-interface-rescan.py, already-configured interfaces must not, and - hints left under a pre-rename name must not linger once the resolver - has moved that interface to its hw-id target. - """ - - def setUp(self): - self.tmp = tempfile.mkdtemp() - self.addCleanup(shutil.rmtree, self.tmp, ignore_errors=True) - self._orig_dir = resolver.vyos_udev_dir - resolver.vyos_udev_dir = self.tmp - self.addCleanup(setattr, resolver, 'vyos_udev_dir', self._orig_dir) - - def test_writes_hint_for_unconfigured_interface(self): - resolver.sync_rescan_hints({'eth5': 'new-mac'}, configured={}) - hint = os.path.join(self.tmp, 'eth5') - self.assertTrue(os.path.isfile(hint)) - with open(hint) as f: - self.assertEqual(f.read(), 'new-mac') - - def test_no_hint_for_configured_interface(self): - resolver.sync_rescan_hints({'eth0': 'm0'}, configured={'m0': 'eth0'}) - self.assertFalse(os.path.exists(os.path.join(self.tmp, 'eth0'))) - - def test_stale_hint_removed_after_rename(self): - # udev's cosmetic fast-path wrote a hint under the pre-rename name; - # the resolver then moved that interface to its hw-id target. - stale = os.path.join(self.tmp, 'eth7') - with open(stale, 'w') as f: - f.write('m2') - resolver.sync_rescan_hints({'eth10': 'm2'}, configured={'m2': 'eth10'}, - stale_names={'eth7'}) - self.assertFalse(os.path.exists(stale)) - - - -class TestMainFirstBootBootstrap(unittest.TestCase): - """Direct regression guard for the gap this feature closes: main() - used to leave a fully-unconfigured system's interfaces at whatever - racy cosmetic name they already had, with no settle wait at all, and - freeze that into the rescan hints vyos-interface-rescan.py persists. + def _make(self, name, up=False, enslaved=False): + path = os.path.join(self.tmp, name) + os.mkdir(path) + # IFF_UP is bit 0; 0x1002 is a down interface with IFF_BROADCAST + with open(os.path.join(path, 'flags'), 'w') as f: + f.write('0x1003\n' if up else '0x1002\n') + if enslaved: + master = os.path.join(self.tmp, f'{name}_master') + os.mkdir(master) + os.symlink(master, os.path.join(path, 'master')) + + def _split(self, plan): + return resolver.refuse_renames_in_use(plan, self.tmp) + + def test_a_card_which_just_arrived_is_renamed(self): + self._make('eth8') + allowed, refused = self._split({'eth8': 'eth2'}) + self.assertEqual(allowed, {'eth8': 'eth2'}) + self.assertEqual(refused, {}) + + def test_an_interface_carrying_traffic_is_left_alone(self): + # what an edited map plus an unrelated hot-plug used to do: rename a + # running port out from under the configuration still naming it + self._make('eth1', up=True) + allowed, refused = self._split({'eth1': 'eth23'}) + self.assertEqual(allowed, {}) + self.assertEqual(refused, {'eth1': 'eth23'}) + + def test_a_bridge_member_is_left_alone(self): + self._make('eth1', enslaved=True) + allowed, refused = self._split({'eth1': 'eth23'}) + self.assertEqual(refused, {'eth1': 'eth23'}) + + def test_one_refusal_does_not_hold_up_the_rest(self): + self._make('eth1', up=True) + self._make('eth8') + allowed, refused = self._split({'eth1': 'eth23', 'eth8': 'eth2'}) + self.assertEqual(allowed, {'eth8': 'eth2'}) + self.assertEqual(refused, {'eth1': 'eth23'}) + + +class TestStatusAdopted(unittest.TestCase): + """A Hyper-V NIC re-created with a new MAC takes the vacant name. That is + neither a card found in its recorded slot nor anything the slot says, so + it is reported under its own key - vyos-router warns about it from there. """ - def setUp(self): - self.udev_dir = tempfile.mkdtemp() - self.addCleanup(shutil.rmtree, self.udev_dir, ignore_errors=True) - self._orig_udev_dir = resolver.vyos_udev_dir - resolver.vyos_udev_dir = self.udev_dir - self.addCleanup(setattr, resolver, 'vyos_udev_dir', self._orig_udev_dir) - - status_dir = tempfile.mkdtemp() - self.addCleanup(shutil.rmtree, status_dir, ignore_errors=True) - self._orig_status_file = resolver.status_file - resolver.status_file = resolver.Path(status_dir) / 'status.json' - self.addCleanup(setattr, resolver, 'status_file', self._orig_status_file) - - def test_empty_configfile_bootstrap_renames_and_hints_new_names(self): - # a tiny fake kernel: name -> mac, mutated by simulated 'ip link' - # calls so discover_physical_interfaces() reflects renames for real - state = {'eth5': 'bb', 'eth2': 'aa'} # cosmetic fast-path leftover - - def fake_discover(*_a, **_kw): - return dict(state) - - def fake_run(command, *_a, **_kw): - parts = command.split() - if 'name' in parts: - old = parts[parts.index('dev') + 1] - new = parts[parts.index('name') + 1] - if old in state: - state[new] = state.pop(old) - return 0 - - with mock.patch.object(resolver, 'get_configfile_interfaces', - return_value={}), \ - mock.patch.object(resolver, 'get_pending_hwid_nodes', - return_value={'ethernet': set(), 'wireless': set()}), \ - mock.patch.object(resolver, 'discover_physical_interfaces', - side_effect=fake_discover), \ - mock.patch.object(resolver, 'is_wireless_interface', - return_value=False), \ - mock.patch.object(resolver, 'run', side_effect=fake_run), \ - mock.patch('time.sleep'): - resolver.main() - - hints = set(os.listdir(self.udev_dir)) - # renamed into MAC-rank order: lower mac 'aa' -> eth0, higher 'bb' -> eth1 - self.assertEqual(hints, {'eth0', 'eth1'}) - with open(os.path.join(self.udev_dir, 'eth0')) as f: - self.assertEqual(f.read(), 'aa') - with open(os.path.join(self.udev_dir, 'eth1')) as f: - self.assertEqual(f.read(), 'bb') - - def test_pending_node_still_settles_when_its_hardware_is_slow_to_appear(self): - # field report: a pending node ended up permanently bound to a - # fresh eth8/eth9-style name despite its own hardware genuinely - # being present - because that hardware (a slower driver, exactly - # the multi-vendor-NIC race this whole feature addresses) simply - # had not shown up in the FIRST, unsettled discover_physical_ - # interfaces() snapshot wait_for_hardware() returns as soon as all - # already-CONFIGURED macs are found. Since that snapshot showed - # nothing unconfigured yet, main()'s gate into wait_for_settle() - # never even ran, giving the slow hardware zero extra time to - # appear before the boot gave up. A pending node in play must - # force wait_for_settle() to run regardless of what the early - # snapshot shows, so slower hardware still gets its chance. - configured = {'m0': 'eth0'} - pending = {'ethernet': {'eth1'}, 'wireless': set()} - - calls = {'n': 0} - - def fake_discover(*_a, **_kw): - calls['n'] += 1 - if calls['n'] <= 2: - return {'eth0': 'm0'} # m1's hardware not visible yet - return {'eth0': 'm0', 'eth9': 'm1'} # now it has appeared - - state = {'eth0': 'm0', 'eth9': 'm1'} - - def fake_run(command, *_a, **_kw): - parts = command.split() - if 'name' in parts: - old = parts[parts.index('dev') + 1] - new = parts[parts.index('name') + 1] - if old in state: - state[new] = state.pop(old) - return 0 - - with mock.patch.object(resolver, 'get_configfile_interfaces', - return_value=configured), \ - mock.patch.object(resolver, 'get_pending_hwid_nodes', - return_value=pending), \ - mock.patch.object(resolver, 'discover_physical_interfaces', - side_effect=fake_discover), \ - mock.patch.object(resolver, 'is_wireless_interface', - return_value=False), \ - mock.patch.object(resolver, 'pcie_distance', return_value=0), \ - mock.patch.object(resolver, 'run', side_effect=fake_run), \ - mock.patch('time.sleep'): - resolver.main() - - self.assertEqual(state.get('eth1'), 'm1') - - status = json.loads(resolver.status_file.read_text()) - self.assertEqual(status['reclaimed'], {'m1': 'eth1'}) - self.assertEqual(status['pending_unresolved'], []) - - def test_reported_reproduction_end_to_end(self): - # exact reported scenario: eth0/eth2 keep their hw-id, eth1's hw-id - # was deleted (the documented "NIC replaced" remediation) but its - # node - and an address setting - stay in config.boot. The same - # physical NIC (mac 'm1') must come back as 'eth1', not bootstrap - # past the floor to 'eth3', and must still get a rescan hint under - # 'eth1' so vyos-interface-rescan.py can refill its hw-id. - configured = {'m0': 'eth0', 'm2': 'eth2'} - pending = {'ethernet': {'eth1'}, 'wireless': set()} - # cosmetic fast-path already placed everything at its final - # hw-id'd name except the replaced NIC, which landed on a racy name - state = {'eth0': 'm0', 'eth2': 'm2', 'eth9': 'm1'} - - def fake_discover(*_a, **_kw): - return dict(state) - - def fake_run(command, *_a, **_kw): - parts = command.split() - if 'name' in parts: - old = parts[parts.index('dev') + 1] - new = parts[parts.index('name') + 1] - if old in state: - state[new] = state.pop(old) - return 0 - - with mock.patch.object(resolver, 'get_configfile_interfaces', - return_value=configured), \ - mock.patch.object(resolver, 'get_pending_hwid_nodes', - return_value=pending), \ - mock.patch.object(resolver, 'discover_physical_interfaces', - side_effect=fake_discover), \ - mock.patch.object(resolver, 'is_wireless_interface', - return_value=False), \ - mock.patch.object(resolver, 'run', side_effect=fake_run), \ - mock.patch('time.sleep'): - resolver.main() - - # the NIC is back at 'eth1', not bootstrapped past the floor to 'eth3' - self.assertEqual(state.get('eth1'), 'm1') - self.assertNotIn('eth3', state) - - # still gets a rescan hint under its reclaimed name, so - # vyos-interface-rescan.py can write the real hw-id into the - # existing 'eth1' node (preserving its other settings, e.g. address) - hints = set(os.listdir(self.udev_dir)) - self.assertEqual(hints, {'eth1'}) - with open(os.path.join(self.udev_dir, 'eth1')) as f: - self.assertEqual(f.read(), 'm1') - - status = json.loads(resolver.status_file.read_text()) - self.assertEqual(status['reclaimed'], {'m1': 'eth1'}) - self.assertEqual(status['pending_unresolved'], []) - - def test_fully_deleted_interfaces_backfill_their_gaps_end_to_end(self): - # reported scenario: eth3, eth6 and eth7 were fully deleted (not - # just their hw-id - the whole node, so get_pending_hwid_nodes() - # has nothing for them either) while eth0,1,2,4,5,8,9 stay - # configured. The three now-unconfigured NICs must backfill the - # gaps at 3/6/7 in MAC order, not bootstrap past the highest - # configured index (9) to 10/11/12. - configured = { - 'm0': 'eth0', 'm1': 'eth1', 'm2': 'eth2', 'm4': 'eth4', - 'm5': 'eth5', 'm8': 'eth8', 'm9': 'eth9', - } - pending = {'ethernet': set(), 'wireless': set()} - state = {name: mac for mac, name in configured.items()} - # realistic macs, deliberately NOT alphabetically matching their - # old slot, so the test can't pass by coincidence - state.update({'ethX': 'ff:ff:ff:ff:ff:07', - 'ethY': 'aa:aa:aa:aa:aa:03', - 'ethZ': 'cc:cc:cc:cc:cc:06'}) - - def fake_discover(*_a, **_kw): - return dict(state) - - def fake_run(command, *_a, **_kw): - parts = command.split() - if 'name' in parts: - old = parts[parts.index('dev') + 1] - new = parts[parts.index('name') + 1] - if old in state: - state[new] = state.pop(old) - return 0 - - with mock.patch.object(resolver, 'get_configfile_interfaces', - return_value=configured), \ - mock.patch.object(resolver, 'get_pending_hwid_nodes', - return_value=pending), \ - mock.patch.object(resolver, 'discover_physical_interfaces', - side_effect=fake_discover), \ - mock.patch.object(resolver, 'is_wireless_interface', - return_value=False), \ - mock.patch.object(resolver, 'pcie_distance', return_value=0), \ - mock.patch.object(resolver, 'run', side_effect=fake_run), \ - mock.patch('time.sleep'): - resolver.main() - - # the three gaps got backfilled in ascending MAC order, not pushed - # past the highest configured index to eth10/eth11/eth12 - self.assertEqual(state.get('eth3'), 'aa:aa:aa:aa:aa:03') - self.assertEqual(state.get('eth6'), 'cc:cc:cc:cc:cc:06') - self.assertEqual(state.get('eth7'), 'ff:ff:ff:ff:ff:07') - self.assertNotIn('eth10', state) - self.assertNotIn('eth11', state) - self.assertNotIn('eth12', state) - - def test_stray_leftover_interface_makes_reclaim_ambiguous(self): - # field report: deleting only eth7's hw-id and rebooting produced - # "could not be safely auto-matched" instead of a clean reclaim, - # because a stray interface left over from an earlier, unrelated - # boot (e.g. a scratch vyethN name stuck after a failed rename) - # was also present, making this a 1 pending/2 candidate case. - # There is no way to tell which candidate was "really" eth7's own - # past hardware once its hw-id is gone, so eth7 stays unresolved; - # both candidates still get a real, settings-free hw-id via - # ordinary bootstrap naming instead of being lost. - configured = { - 'm0': 'eth0', 'm1': 'eth1', 'm2': 'eth2', 'm4': 'eth4', - 'm5': 'eth5', 'm6': 'eth6', - } - pending = {'ethernet': {'eth7'}, 'wireless': set()} - state = {name: mac for mac, name in configured.items()} - state.update({'eth9': 'aa:bb:cc:dd:ee:07', # eth7's own hardware - 'vyeth13': 'ff:ff:ff:ff:ff:99'}) # unrelated leftover - - def fake_discover(*_a, **_kw): - return dict(state) - - def fake_run(command, *_a, **_kw): - parts = command.split() - if 'name' in parts: - old = parts[parts.index('dev') + 1] - new = parts[parts.index('name') + 1] - if old in state: - state[new] = state.pop(old) - return 0 - - with mock.patch.object(resolver, 'get_configfile_interfaces', - return_value=configured), \ - mock.patch.object(resolver, 'get_pending_hwid_nodes', - return_value=pending), \ - mock.patch.object(resolver, 'discover_physical_interfaces', - side_effect=fake_discover), \ - mock.patch.object(resolver, 'is_wireless_interface', - return_value=False), \ - mock.patch.object(resolver, 'pcie_distance', return_value=0), \ - mock.patch.object(resolver, 'run', side_effect=fake_run), \ - mock.patch('time.sleep'): - resolver.main() - - # 'eth7' stays pending rather than being guessed at; both - # candidates still land on real, settings-free names (the lone - # ordinary gap, then the next fresh slot after that) - self.assertNotIn('eth7', state) - self.assertEqual(state.get('eth3'), 'aa:bb:cc:dd:ee:07') - self.assertEqual(state.get('eth8'), 'ff:ff:ff:ff:ff:99') - - hints = set(os.listdir(self.udev_dir)) - self.assertEqual(hints, {'eth3', 'eth8'}) - - status = json.loads(resolver.status_file.read_text()) - self.assertEqual(status['pending_unresolved'], ['eth7']) - self.assertEqual(status['reclaimed'], {}) - - def test_second_pending_node_makes_reclaim_ambiguous(self): - # two pending nodes (eth1 and eth7), but only one candidate showed - # up this boot - which one it belongs to can't be told, so - # neither is matched; the candidate still gets a real, - # settings-free name via ordinary bootstrap naming. - configured = {'m0': 'eth0'} - pending = {'ethernet': {'eth1', 'eth7'}, 'wireless': set()} - state = {'eth0': 'm0', 'eth9': 'aa:bb:cc:dd:ee:07'} - - def fake_discover(*_a, **_kw): - return dict(state) - - def fake_run(command, *_a, **_kw): - parts = command.split() - if 'name' in parts: - old = parts[parts.index('dev') + 1] - new = parts[parts.index('name') + 1] - if old in state: - state[new] = state.pop(old) - return 0 - - with mock.patch.object(resolver, 'get_configfile_interfaces', - return_value=configured), \ - mock.patch.object(resolver, 'get_pending_hwid_nodes', - return_value=pending), \ - mock.patch.object(resolver, 'discover_physical_interfaces', - side_effect=fake_discover), \ - mock.patch.object(resolver, 'is_wireless_interface', - return_value=False), \ - mock.patch.object(resolver, 'pcie_distance', return_value=0), \ - mock.patch.object(resolver, 'run', side_effect=fake_run), \ - mock.patch('time.sleep'): - resolver.main() - - self.assertNotIn('eth1', state) - self.assertNotIn('eth7', state) - self.assertEqual(state.get('eth2'), 'aa:bb:cc:dd:ee:07') - - status = json.loads(resolver.status_file.read_text()) - self.assertEqual(status['pending_unresolved'], ['eth1', 'eth7']) - self.assertEqual(status['reclaimed'], {}) - - def test_ambiguous_candidates_never_get_a_permanent_wrong_binding(self): - # field report: deleting eth2's hw-id (leaving its node in place) - # produced "0 unconfigured candidates" on a later boot, because an - # EARLIER ambiguous boot had already permanently bound the wrong - # candidate elsewhere. One pending node with two candidates this - # boot must not be guessed at either way - it stays pending, and - # both candidates still get a real, settings-free hw-id via - # ordinary bootstrap naming (the one open gap, then a fresh slot). - configured = {'m0': 'eth0'} - pending = {'ethernet': {'eth2'}, 'wireless': set()} - state = { - 'eth0': 'm0', - 'racyA': 'aa:bb:cc:dd:ee:02', # eth2's own hardware - 'racyB': 'ff:ff:ff:ff:ff:99', # unrelated leftover candidate - } - - def fake_discover(*_a, **_kw): - return dict(state) - - def fake_run(command, *_a, **_kw): - parts = command.split() - if 'name' in parts: - old = parts[parts.index('dev') + 1] - new = parts[parts.index('name') + 1] - if old in state: - state[new] = state.pop(old) - return 0 - - with mock.patch.object(resolver, 'get_configfile_interfaces', - return_value=configured), \ - mock.patch.object(resolver, 'get_pending_hwid_nodes', - return_value=pending), \ - mock.patch.object(resolver, 'discover_physical_interfaces', - side_effect=fake_discover), \ - mock.patch.object(resolver, 'is_wireless_interface', - return_value=False), \ - mock.patch.object(resolver, 'pcie_distance', return_value=0), \ - mock.patch.object(resolver, 'run', side_effect=fake_run), \ - mock.patch('time.sleep'): - resolver.main() - - # 'eth2' stays pending; both candidates land on real, settings- - # free names instead (the one open gap, then a fresh slot) - self.assertNotIn('eth2', state) - self.assertEqual(state.get('eth1'), 'aa:bb:cc:dd:ee:02') - self.assertEqual(state.get('eth3'), 'ff:ff:ff:ff:ff:99') - - hints = set(os.listdir(self.udev_dir)) - self.assertEqual(hints, {'eth1', 'eth3'}) - - status = json.loads(resolver.status_file.read_text()) - self.assertEqual(status['pending_unresolved'], ['eth2']) - self.assertEqual(status['reclaimed'], {}) - - def test_simultaneous_ambiguity_holds_back_every_candidate(self): - # two NICs' hw-id were deleted before the same reboot (eth0 and - # eth2, nodes left in place), and both came back racily named. - # Each candidate's MAC happens to obviously "belong" to one - # specific pending node (racyA's mac ends in :00, matching what - # eth0 used to be; racyB's mac ends in :02, matching eth2) - but - # there is no way to actually verify that pairing, so with 2 - # pending nodes and 2 candidates present at once, neither may be - # auto-matched: guessing the "obvious" pairing risks silently - # binding a configured node's settings to the wrong physical NIC - # if the guess is ever wrong. Both stay pending and reported; - # both candidates still get a real, settings-free hw-id via - # ordinary bootstrap naming. - configured = { - 'm1': 'eth1', 'm3': 'eth3', 'm4': 'eth4', - 'm5': 'eth5', 'm6': 'eth6', 'm7': 'eth7', + def test_an_adoption_is_reported_apart_from_slot_matches(self): + tmp = tempfile.mkdtemp() + self.addCleanup(shutil.rmtree, tmp) + status = os.path.join(tmp, 'status.json') + store = { + 'version': 1, + 'hardware': {}, + 'interfaces': {'eth3': 'ID_NET_NAME_MAC=enx000d3a47aa01'}, } - pending = {'ethernet': {'eth0', 'eth2'}, 'wireless': set()} - state = {name: mac for mac, name in configured.items()} - state.update({ - 'racyA': 'aa:bb:cc:dd:ee:00', # looks like eth0's old hardware - 'racyB': 'aa:bb:cc:dd:ee:02', # looks like eth2's old hardware - }) - - def fake_discover(*_a, **_kw): - return dict(state) - - def fake_run(command, *_a, **_kw): - parts = command.split() - if 'name' in parts: - old = parts[parts.index('dev') + 1] - new = parts[parts.index('name') + 1] - if old in state: - state[new] = state.pop(old) - return 0 - - with mock.patch.object(resolver, 'get_configfile_interfaces', - return_value=configured), \ - mock.patch.object(resolver, 'get_pending_hwid_nodes', - return_value=pending), \ - mock.patch.object(resolver, 'discover_physical_interfaces', - side_effect=fake_discover), \ - mock.patch.object(resolver, 'is_wireless_interface', - return_value=False), \ - mock.patch.object(resolver, 'pcie_distance', return_value=0), \ - mock.patch.object(resolver, 'run', side_effect=fake_run), \ - mock.patch('time.sleep'): - resolver.main() - - # neither pending node is guessed at - both candidates land on - # fresh, settings-free bootstrap names instead (no ordinary gaps - # exist here, so both go beyond the highest configured index) - self.assertNotIn('eth0', state) - self.assertNotIn('eth2', state) - self.assertEqual(state.get('eth8'), 'aa:bb:cc:dd:ee:00') - self.assertEqual(state.get('eth9'), 'aa:bb:cc:dd:ee:02') - - hints = set(os.listdir(self.udev_dir)) - self.assertEqual(hints, {'eth8', 'eth9'}) - - status = json.loads(resolver.status_file.read_text()) - self.assertEqual(status['pending_unresolved'], ['eth0', 'eth2']) - self.assertEqual(status['reclaimed'], {}) - - def test_squatting_pending_candidate_still_reclaims_correctly(self): - # field report: deleting eth7's hw-id (node left in place) produced - # a "0 unconfigured candidates" boot even though eth7's real - # hardware was present, and it later turned up permanently bound - # to a fresh eth8/eth9-style name. Root cause: eth7's hardware - # (mac07) happened to be squatting on eth2's configured hw-id slot - # this boot (probe-order scrambling) - compute_rename_plan() - # already scheduled it for eviction, which used to make - # unmatched_candidates() skip it entirely, so it never reached the - # ascending fill and fell through to a stale, unrelated fallback - # destination instead. A squatting candidate must still reclaim - # its pending node exactly like a non-squatting one would, when it - # is the only open name and the only candidate this boot. - configured = { - 'm0': 'eth0', 'm1': 'eth1', 'm2': 'eth2', 'm3': 'eth3', - 'm4': 'eth4', 'm5': 'eth5', 'm6': 'eth6', - } - pending = {'ethernet': {'eth7'}, 'wireless': set()} - state = {name: mac for mac, name in configured.items()} - state.update({ - 'eth2': 'aa:bb:cc:dd:ee:07', # eth7's hardware squats on eth2 - 'eth9': 'm2', # m2's rightful NIC, racy this boot - }) - - def fake_discover(*_a, **_kw): - return dict(state) - - def fake_run(command, *_a, **_kw): - parts = command.split() - if 'name' in parts: - old = parts[parts.index('dev') + 1] - new = parts[parts.index('name') + 1] - if old in state: - state[new] = state.pop(old) - return 0 - - with mock.patch.object(resolver, 'get_configfile_interfaces', - return_value=configured), \ - mock.patch.object(resolver, 'get_pending_hwid_nodes', - return_value=pending), \ - mock.patch.object(resolver, 'discover_physical_interfaces', - side_effect=fake_discover), \ - mock.patch.object(resolver, 'is_wireless_interface', - return_value=False), \ - mock.patch.object(resolver, 'pcie_distance', return_value=0), \ - mock.patch.object(resolver, 'run', side_effect=fake_run), \ - mock.patch('time.sleep'): - resolver.main() - - # m2 still lands on its own configured slot, and the squatter - # reclaims 'eth7' - not some unrelated fresh bootstrap slot - self.assertEqual(state.get('eth2'), 'm2') - self.assertEqual(state.get('eth7'), 'aa:bb:cc:dd:ee:07') - self.assertNotIn('eth8', state) - self.assertNotIn('eth9', state) - - hints = set(os.listdir(self.udev_dir)) - self.assertEqual(hints, {'eth7'}) - - status = json.loads(resolver.status_file.read_text()) - self.assertEqual(status['reclaimed'], {'aa:bb:cc:dd:ee:07': 'eth7'}) - self.assertEqual(status['pending_unresolved'], []) - - def test_squatting_candidate_and_extra_candidate_makes_reclaim_ambiguous(self): - # same squatter-eviction shape as the test above, but with a - # second, unrelated candidate also present this boot - genuinely - # ambiguous, exactly like a non-squatting extra candidate would - # be. The pending node stays unresolved; the squatter still gets - # evicted off of the configured slot it's sitting on (so the - # rightful owner isn't blocked) and, like the stray, lands on a - # real, settings-free bootstrap name instead of the pending one. - configured = { - 'm0': 'eth0', 'm1': 'eth1', 'm2': 'eth2', - 'm3': 'eth3', 'm4': 'eth4', 'm5': 'eth5', - } - pending = {'ethernet': {'eth7'}, 'wireless': set()} - state = {name: mac for mac, name in configured.items()} - state.update({ - 'eth3': 'aa:bb:cc:dd:ee:07', # squats on m3's configured slot - 'eth9': 'm3', # m3's rightful NIC, racy this boot - 'racyC': 'ff:ff:ff:ff:ff:99', # unrelated leftover candidate - }) - - def fake_discover(*_a, **_kw): - return dict(state) - - def fake_run(command, *_a, **_kw): - parts = command.split() - if 'name' in parts: - old = parts[parts.index('dev') + 1] - new = parts[parts.index('name') + 1] - if old in state: - state[new] = state.pop(old) - return 0 - - with mock.patch.object(resolver, 'get_configfile_interfaces', - return_value=configured), \ - mock.patch.object(resolver, 'get_pending_hwid_nodes', - return_value=pending), \ - mock.patch.object(resolver, 'discover_physical_interfaces', - side_effect=fake_discover), \ - mock.patch.object(resolver, 'is_wireless_interface', - return_value=False), \ - mock.patch.object(resolver, 'pcie_distance', return_value=0), \ - mock.patch.object(resolver, 'run', side_effect=fake_run), \ - mock.patch('time.sleep'): - resolver.main() - - # m3 still lands on its own configured slot; 'eth7' stays pending - # rather than being guessed at; the squatter and the stray both - # land on real, settings-free names instead - self.assertEqual(state.get('eth3'), 'm3') - self.assertNotIn('eth7', state) - self.assertEqual(state.get('eth6'), 'aa:bb:cc:dd:ee:07') - self.assertEqual(state.get('eth8'), 'ff:ff:ff:ff:ff:99') - - hints = set(os.listdir(self.udev_dir)) - self.assertEqual(hints, {'eth6', 'eth8'}) - - status = json.loads(resolver.status_file.read_text()) - self.assertEqual(status['pending_unresolved'], ['eth7']) - self.assertEqual(status['reclaimed'], {}) - - def test_gap_backfill_and_pending_node_coexisting_stays_ambiguous(self): - # the exact vyos-build check-qemu-install --ifnametest shape: one - # interface's whole config node is fully deleted (a free numeric - # gap) while a DIFFERENT interface's hw-id alone is deleted (a - # pending node) in the very same reboot - # (del_idx, hwid_idx = random.sample(range(8), 2)). This is - # genuinely ambiguous for the pending node (eth2): its own - # hardware and the gap's freed hardware are two indistinguishable - # candidates of the same type, and guessing risks silently - # binding eth2's settings to the wrong physical NIC - confirmed - # in the field. eth2 stays pending; both candidates still get a - # real, settings-free hw-id via ordinary bootstrap naming (the - # actual gap, then a fresh slot) - nothing is lost, just not - # auto-attributed to the right name. - configured = { - 'm0': 'eth0', 'm1': 'eth1', 'm3': 'eth3', 'm4': 'eth4', - 'm6': 'eth6', 'm8': 'eth8', 'm9': 'eth9', - } - pending = {'ethernet': {'eth2'}, 'wireless': set()} - state = {name: mac for mac, name in configured.items()} - state.update({ - 'eth7': 'aa:bb:cc:dd:ee:02', # eth2's real hardware, racy this boot - 'eth5': 'ff:ff:ff:ff:ff:05', # eth5 fully deleted, its own hardware freed - }) - - def fake_discover(*_a, **_kw): - return dict(state) - - def fake_run(command, *_a, **_kw): - parts = command.split() - if 'name' in parts: - old = parts[parts.index('dev') + 1] - new = parts[parts.index('name') + 1] - if old in state: - state[new] = state.pop(old) - return 0 - - with mock.patch.object(resolver, 'get_configfile_interfaces', - return_value=configured), \ - mock.patch.object(resolver, 'get_pending_hwid_nodes', - return_value=pending), \ - mock.patch.object(resolver, 'discover_physical_interfaces', - side_effect=fake_discover), \ - mock.patch.object(resolver, 'is_wireless_interface', - return_value=False), \ - mock.patch.object(resolver, 'pcie_distance', return_value=0), \ - mock.patch.object(resolver, 'run', side_effect=fake_run), \ - mock.patch('time.sleep'): - resolver.main() - - self.assertNotIn('eth2', state) - self.assertEqual(state.get('eth5'), 'aa:bb:cc:dd:ee:02') - self.assertEqual(state.get('eth7'), 'ff:ff:ff:ff:ff:05') - - status = json.loads(resolver.status_file.read_text()) - self.assertEqual(status['pending_unresolved'], ['eth2']) - self.assertEqual(status['reclaimed'], {}) + report = {'adopted': {'eth3': '00:0d:3a:47:aa:01'}} + devices = [device('eth3', '00:0d:3a:47:aa:01')] + devices[0]['properties'] = {'ID_NET_NAME_MAC': 'enx000d3a47aa01'} + with mock.patch.object(resolver, 'status_file', Path(status)): + resolver.write_status(store, devices, {}, report, True) + with open(status) as f: + data = json.load(f) + self.assertEqual(data['adopted'], {'eth3': '00:0d:3a:47:aa:01'}) + self.assertEqual(data['matched_by_path'], {}) + self.assertIn('eth3', data['matched']) class TestMainInContainer(unittest.TestCase): - """A container owns no NIC: its interfaces are runtime-created veth - pairs with no backing bus device in sysfs and a host-assigned MAC that - changes on every start. The naming pass can therefore never resolve a - pending node there - it only spent both bounded hardware waits and then - told the user to bind an hw-id that would not survive a restart. + """A container owns no NIC: its interfaces are runtime-created veth pairs + with no backing bus device in sysfs and a host-assigned MAC that changes + on every start. There is nothing to name and nothing worth persisting. """ def setUp(self): - self.udev_dir = tempfile.mkdtemp() - self.addCleanup(shutil.rmtree, self.udev_dir, ignore_errors=True) - self._orig_udev_dir = resolver.vyos_udev_dir - resolver.vyos_udev_dir = self.udev_dir - self.addCleanup(setattr, resolver, 'vyos_udev_dir', self._orig_udev_dir) - status_dir = tempfile.mkdtemp() self.addCleanup(shutil.rmtree, status_dir, ignore_errors=True) self._orig_status_file = resolver.status_file resolver.status_file = resolver.Path(status_dir) / 'status.json' - self.addCleanup(setattr, resolver, 'status_file', self._orig_status_file) + self.addCleanup(setattr, resolver, 'status_file', + self._orig_status_file) def test_naming_pass_is_skipped_entirely(self): with mock.patch.object(resolver, 'is_running_as_container', - return_value=True), \ - mock.patch.object(resolver, 'get_configfile_interfaces') as configured, \ - mock.patch.object(resolver, 'get_pending_hwid_nodes') as pending, \ - mock.patch.object(resolver, 'discover_physical_interfaces') as discover, \ + return_value=True), \ + mock.patch.object(resolver, 'load_store') as load, \ + mock.patch.object(resolver, 'discover_devices') as discover, \ + mock.patch.object(resolver, 'save_store') as save, \ mock.patch.object(resolver, 'run') as run: resolver.main() - configured.assert_not_called() - pending.assert_not_called() + load.assert_not_called() discover.assert_not_called() + save.assert_not_called() run.assert_not_called() - # no status file means vyos-router's warn_missing_interface_hardware() - # stays silent instead of warning about an unbindable pending node self.assertFalse(resolver.status_file.exists()) - self.assertEqual(os.listdir(self.udev_dir), []) -class TestWriteStatus(unittest.TestCase): - """A pending node that couldn't be safely auto-matched must be - reported so vyos-router can surface a boot-time warning - it must - never regress into another silent, buried-syslog-only failure like - the one this whole fix closes. +class TestHotplugPass(unittest.TestCase): + """A pass for hardware added to a running system hands no vacant name + over: the configuration under it would not be applied until the next boot. """ - def setUp(self): + def _unit(self, name): + with open(os.path.join(_here, '../systemd', name)) as f: + return f.read() + + def test_only_the_hotplug_unit_says_so(self): + self.assertIn('vyos-net-name-resolve.py --hotplug\n', + self._unit('vyos-net-name-resolve-hotplug.service')) + exec_start = [line for line in + self._unit('vyos-net-name-resolve.service').splitlines() + if line.startswith('ExecStart=')] + self.assertEqual(len(exec_start), 1) + self.assertNotIn('--hotplug', exec_start[0]) + + def _main(self, hotplug): status_dir = tempfile.mkdtemp() self.addCleanup(shutil.rmtree, status_dir, ignore_errors=True) - self._orig_status_file = resolver.status_file - resolver.status_file = resolver.Path(status_dir) / 'status.json' - self.addCleanup(setattr, resolver, 'status_file', self._orig_status_file) - - def _read_status(self): - return json.loads(resolver.status_file.read_text()) - - def test_pending_unresolved_reported_when_no_match(self): - pending = {'ethernet': {'eth1', 'eth4'}, 'wireless': set()} - resolver.write_status({}, {}, set(), {}, pending=pending, reclaimed={}) - status = self._read_status() - self.assertEqual(status['pending_unresolved'], ['eth1', 'eth4']) - - def test_reclaimed_not_reported_as_unresolved(self): - pending = {'ethernet': {'eth1'}, 'wireless': set()} - reclaimed = {'m1': 'eth1'} - resolver.write_status({}, {}, set(), {}, pending=pending, - reclaimed=reclaimed) - status = self._read_status() - self.assertEqual(status['pending_unresolved'], []) - self.assertEqual(status['reclaimed'], {'m1': 'eth1'}) - - def test_no_pending_no_crash(self): - resolver.write_status({}, {}, set(), {}) - status = self._read_status() - self.assertEqual(status['pending_unresolved'], []) - self.assertEqual(status['reclaimed'], {}) - - def test_unconfigured_candidates_reported_for_diagnosis(self): - # the full unmatched_candidates() list from main(), so an - # ambiguous reclaim (zero or multiple candidates for a pending - # node) is diagnosable from this one file alone - which physical - # interfaces were actually competing - without needing a separate - # `ip -br link show` or `show configuration commands` to - # reconstruct the same picture by hand. - pending = {'ethernet': {'eth7'}, 'wireless': set()} - candidates = [('aa:bb:cc:dd:ee:07', 'eth9'), ('aa:bb:cc:dd:ee:99', 'vyeth13')] - resolver.write_status({}, {}, set(), {}, pending=pending, - reclaimed={}, candidates=candidates) - status = self._read_status() - self.assertEqual(status['pending_unresolved'], ['eth7']) - self.assertEqual(status['unconfigured_candidates'], { - 'eth9': 'aa:bb:cc:dd:ee:07', - 'vyeth13': 'aa:bb:cc:dd:ee:99', - }) - - def test_unconfigured_candidates_defaults_empty(self): - resolver.write_status({}, {}, set(), {}) - status = self._read_status() - self.assertEqual(status['unconfigured_candidates'], {}) + store = {'interfaces': {}, 'hardware': {}} + with mock.patch.object(resolver, 'is_running_as_container', + return_value=False), \ + mock.patch.object(resolver, 'status_file', + resolver.Path(status_dir) / 'status.json'), \ + mock.patch.object(resolver, 'load_store', return_value=store), \ + mock.patch.object(resolver, 'wait_for_hardware', + return_value=([], set())), \ + mock.patch.object(resolver, 'wait_for_settle', return_value=[]), \ + mock.patch.object(resolver, 'resolve', + return_value=({}, store, {})) as resolve, \ + mock.patch.object(resolver, 'save_store', return_value=False): + resolver.main(hotplug=hotplug) + return resolve.call_args + + def test_the_flag_is_read_from_the_command_line(self): + self.assertTrue(resolver.hotplug_requested(['--hotplug'])) + self.assertFalse(resolver.hotplug_requested([])) + + def test_a_hotplug_pass_keeps_the_boot_adoption_on_record(self): + # the boot pass gave eth2 to a new NIC; a later VF re-add starts a + # hotplug pass, which must neither forget nor repeat that + status_dir = tempfile.mkdtemp() + self.addCleanup(shutil.rmtree, status_dir, ignore_errors=True) + status = resolver.Path(status_dir) / 'status.json' + status.write_text(json.dumps({'adopted': {'eth2': '60:45:bd:00:00:01', + 'eth3': '60:45:bd:00:00:02'}})) + store = {'interfaces': {'eth2': 'ID_NET_NAME_MAC=enx6045bd000001', + 'eth3': 'ID_NET_NAME_MAC=enx6045bd000002'}, + 'hardware': {'eth2': '60:45:bd:00:00:01', + 'eth3': '60:45:bd:00:00:09'}} + with mock.patch.object(resolver, 'is_running_as_container', + return_value=False), \ + mock.patch.object(resolver, 'status_file', status), \ + mock.patch.object(resolver, 'load_store', return_value=store), \ + mock.patch.object(resolver, 'wait_for_hardware', + return_value=([], set())), \ + mock.patch.object(resolver, 'wait_for_settle', return_value=[]), \ + mock.patch.object(resolver, 'resolve', + return_value=({}, store, {})), \ + mock.patch.object(resolver, 'save_store', return_value=False), \ + mock.patch.object(resolver.logger, 'warning') as warning: + resolver.main(hotplug=True) + # eth3's NIC changed since: that record no longer holds + self.assertEqual(json.loads(status.read_text())['adopted'], + {'eth2': '60:45:bd:00:00:01'}) + self.assertFalse(any('had no NIC this boot' in str(c) + for c in warning.call_args_list)) + + def test_the_boot_pass_reports_only_its_own_adoptions(self): + # a status left from an earlier pass is not carried by a boot pass + status_dir = tempfile.mkdtemp() + self.addCleanup(shutil.rmtree, status_dir, ignore_errors=True) + status = resolver.Path(status_dir) / 'status.json' + status.write_text(json.dumps({'adopted': {'eth2': '60:45:bd:00:00:01'}})) + store = {'interfaces': {'eth2': 'ID_NET_NAME_MAC=enx6045bd000001'}, + 'hardware': {'eth2': '60:45:bd:00:00:01'}, 'hyperv': ['eth2']} + with mock.patch.object(resolver, 'is_running_as_container', + return_value=False), \ + mock.patch.object(resolver, 'status_file', status), \ + mock.patch.object(resolver, 'load_store', return_value=store), \ + mock.patch.object(resolver, 'wait_for_hardware', + return_value=([], set())), \ + mock.patch.object(resolver, 'wait_for_settle', return_value=[]), \ + mock.patch.object(resolver, 'resolve', + return_value=({}, store, {})), \ + mock.patch.object(resolver, 'save_store', return_value=False): + resolver.main(hotplug=False) + self.assertEqual(json.loads(status.read_text())['adopted'], {}) + + def _unrecorded_map_logs(self, present): + store = {'interfaces': {'eth0': 'ID_NET_NAME_MAC=enx000d3a000001'}, + 'hardware': {'eth0': '00:0d:3a:00:00:01'}} + devices = [{'properties': {'ID_NET_NAME_MAC': 'enx000d3a000001'}, + 'mac': '00:0d:3a:00:00:01', 'name': 'eth0'}] if present else [] + status_dir = tempfile.mkdtemp() + self.addCleanup(shutil.rmtree, status_dir, ignore_errors=True) + with mock.patch.object(resolver, 'is_running_as_container', + return_value=False), \ + mock.patch.object(resolver, 'status_file', + resolver.Path(status_dir) / 'status.json'), \ + mock.patch.object(resolver, 'load_store', return_value=store), \ + mock.patch.object(resolver, 'wait_for_hardware', + return_value=(devices, set())), \ + mock.patch.object(resolver, 'wait_for_settle', + return_value=devices), \ + mock.patch.object(resolver, 'resolve', + return_value=({}, store, {})), \ + mock.patch.object(resolver, 'save_store', return_value=False), \ + mock.patch.object(resolver.logger, 'info') as info: + resolver.main(hotplug=False) + return any('does not say which interfaces' in str(c) + for c in info.call_args_list) + + def test_an_unrecorded_map_is_mentioned_only_when_a_name_is_vacant(self): + # a fresh Azure deployment: cloud-init wrote the map, every NIC is + # there - nothing is held back, so nothing is said (5479.0.9 G1) + self.assertFalse(self._unrecorded_map_logs(present=True)) + self.assertTrue(self._unrecorded_map_logs(present=False)) + + def test_the_boot_pass_may_adopt(self): + self.assertTrue(self._main(False).kwargs['adopt']) + + def test_the_hotplug_pass_does_not(self): + self.assertFalse(self._main(True).kwargs['adopt']) class TestBootOrdering(unittest.TestCase): - """Regression guard for the config.boot availability corner case: - /opt/vyatta/etc/config/config.boot does not exist until vyos-router - mounts (and, for an encrypted config volume, decrypts) the config - directory itself. vyos-net-name-resolve must be started explicitly - from inside vyos-router after that point, and must not be - reintroduced as an independently-scheduled unit ordered before - vyos-router/network-pre.target/cloud-init. + """The store lives in the config directory, which does not exist until + vyos-router mounts (and, for an encrypted config volume, decrypts) it. So + the pass must be started explicitly from inside vyos-router after that + point, and must not be reintroduced as an independently-scheduled unit + ordered before it. """ def setUp(self): @@ -1654,15 +411,16 @@ class TestBootOrdering(unittest.TestCase): migrate = self._first_index('migrate_bootfile || overall_status=1') resolve = self._first_index( 'systemctl start vyos-net-name-resolve.service') - update_iface = self._first_index('update_interface_config || overall_status=1') + update_iface = self._first_index( + 'update_interface_config || overall_status=1') load_boot = self._first_index('disabled configure || load_bootfile') self.assertLess(migrate, resolve, - 'resolver must run after config.boot is migrated/current') + 'resolver must run after config.boot is current') self.assertLess(resolve, update_iface, - 'resolver must run before new-hardware rescan consumes its hints') + 'resolver must write the store before the rescan reads it') self.assertLess(resolve, load_boot, - 'resolver must run before the CLI config is applied') + 'resolver must run before the CLI config is applied') def test_service_unit_not_independently_ordered_before_vyos_router(self): path = os.path.join(_here, '../systemd/vyos-net-name-resolve.service') |