Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

net: add the ability to blacklist network interfaces based on driver during enumeration of physical network devices #591

Merged
merged 8 commits into from
Oct 13, 2020
6 changes: 5 additions & 1 deletion cloudinit/distros/networking.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,9 @@ class Networking(metaclass=abc.ABCMeta):
Hierarchy" in HACKING.rst for full details.
"""

def __init__(self):
self.blacklist_drivers = None

def _get_current_rename_info(self) -> dict:
return net._get_current_rename_info()

Expand Down Expand Up @@ -68,7 +71,8 @@ def get_interfaces(self) -> list:
return net.get_interfaces()

def get_interfaces_by_mac(self) -> dict:
return net.get_interfaces_by_mac()
return net.get_interfaces_by_mac(
blacklist_drivers=self.blacklist_drivers)

def get_master(self, devname: DeviceName):
return net.get_master(devname)
Expand Down
35 changes: 23 additions & 12 deletions cloudinit/net/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -746,18 +746,22 @@ def get_ib_interface_hwaddr(ifname, ethernet_format):
return mac


def get_interfaces_by_mac():
def get_interfaces_by_mac(blacklist_drivers=None) -> dict:
if util.is_FreeBSD():
return get_interfaces_by_mac_on_freebsd()
return get_interfaces_by_mac_on_freebsd(
blacklist_drivers=blacklist_drivers)
elif util.is_NetBSD():
return get_interfaces_by_mac_on_netbsd()
return get_interfaces_by_mac_on_netbsd(
blacklist_drivers=blacklist_drivers)
elif util.is_OpenBSD():
return get_interfaces_by_mac_on_openbsd()
return get_interfaces_by_mac_on_openbsd(
blacklist_drivers=blacklist_drivers)
else:
return get_interfaces_by_mac_on_linux()
return get_interfaces_by_mac_on_linux(
blacklist_drivers=blacklist_drivers)


def get_interfaces_by_mac_on_freebsd():
def get_interfaces_by_mac_on_freebsd(blacklist_drivers=None) -> dict():
(out, _) = subp.subp(['ifconfig', '-a', 'ether'])

# flatten each interface block in a single line
Expand All @@ -784,7 +788,7 @@ def find_mac(flat_list):
return results


def get_interfaces_by_mac_on_netbsd():
def get_interfaces_by_mac_on_netbsd(blacklist_drivers=None) -> dict():
ret = {}
re_field_match = (
r"(?P<ifname>\w+).*address:\s"
Expand All @@ -800,7 +804,7 @@ def get_interfaces_by_mac_on_netbsd():
return ret


def get_interfaces_by_mac_on_openbsd():
def get_interfaces_by_mac_on_openbsd(blacklist_drivers=None) -> dict():
ret = {}
re_field_match = (
r"(?P<ifname>\w+).*lladdr\s"
Expand All @@ -815,12 +819,13 @@ def get_interfaces_by_mac_on_openbsd():
return ret


def get_interfaces_by_mac_on_linux():
def get_interfaces_by_mac_on_linux(blacklist_drivers=None) -> dict:
"""Build a dictionary of tuples {mac: name}.

Bridges and any devices that have a 'stolen' mac are excluded."""
ret = {}
for name, mac, _driver, _devid in get_interfaces():
for name, mac, _driver, _devid in get_interfaces(
blacklist_drivers=blacklist_drivers):
if mac in ret:
raise RuntimeError(
"duplicate mac found! both '%s' and '%s' have mac '%s'" %
Expand All @@ -838,11 +843,13 @@ def get_interfaces_by_mac_on_linux():
return ret


def get_interfaces():
def get_interfaces(blacklist_drivers=None) -> list:
"""Return list of interface tuples (name, mac, driver, device_id)

Bridges and any devices that have a 'stolen' mac are excluded."""
ret = []
if blacklist_drivers is None:
blacklist_drivers = []
devs = get_devicelist()
# 16 somewhat arbitrarily chosen. Normally a mac is 6 '00:' tokens.
zero_mac = ':'.join(('00',) * 16)
Expand All @@ -866,7 +873,11 @@ def get_interfaces():
# skip nics that have no mac (00:00....)
if name != 'lo' and mac == zero_mac[:len(mac)]:
continue
ret.append((name, mac, device_driver(name), device_devid(name)))
# skip nics that have drivers blacklisted
driver = device_driver(name)
if driver in blacklist_drivers:
continue
ret.append((name, mac, driver, device_devid(name)))
return ret


Expand Down
36 changes: 23 additions & 13 deletions cloudinit/sources/DataSourceAzure.py
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,25 @@
'/run/network/interfaces.ephemeral.d',
]

# This list is used to blacklist devices that will be considered
# for renaming or fallback interfaces.
anhvoms marked this conversation as resolved.
Show resolved Hide resolved
#
# On Azure network devices using these drivers are automatically
# configured by the platform and should not be configured by
# cloud-init's network configuration.
#
# Note:
# Azure Dv4 and Ev4 series VMs always have mlx5 hardware.
# https://docs.microsoft.com/en-us/azure/virtual-machines/dv4-dsv4-series
# https://docs.microsoft.com/en-us/azure/virtual-machines/ev4-esv4-series
# Earlier D and E series VMs (such as Dv2, Dv3, and Ev3 series VMs)
# can have either mlx4 or mlx5 hardware, with the older series VMs
# having a higher chance of coming with mlx4 hardware.
# https://docs.microsoft.com/en-us/azure/virtual-machines/dv2-dsv2-series
# https://docs.microsoft.com/en-us/azure/virtual-machines/dv3-dsv3-series
# https://docs.microsoft.com/en-us/azure/virtual-machines/ev3-esv3-series
BLACKLIST_DRIVERS = ['mlx4_core', 'mlx5_core']


def find_storvscid_from_sysctl_pnpinfo(sysctl_out, deviceid):
# extract the 'X' from dev.storvsc.X. if deviceid matches
Expand Down Expand Up @@ -529,6 +548,8 @@ def _get_data(self):
except Exception as e:
LOG.warning("Failed to get system information: %s", e)

self.distro.networking.blacklist_drivers = BLACKLIST_DRIVERS

try:
crawled_data = util.log_time(
logfunc=LOG.debug, msg='Crawl of metadata service',
Expand Down Expand Up @@ -1468,23 +1489,12 @@ def _generate_network_config_from_imds_metadata(imds_metadata) -> dict:

@azure_ds_telemetry_reporter
def _generate_network_config_from_fallback_config() -> dict:
anhvoms marked this conversation as resolved.
Show resolved Hide resolved
"""Generate fallback network config excluding mlx4_core & mlx5_core devices.
"""Generate fallback network config excluding blacklisted devices.

@return: Dictionary containing network version 2 standard configuration.
"""
# Azure Dv4 and Ev4 series VMs always have mlx5 hardware.
# https://docs.microsoft.com/en-us/azure/virtual-machines/dv4-dsv4-series
# https://docs.microsoft.com/en-us/azure/virtual-machines/ev4-esv4-series
# Earlier D and E series VMs (such as Dv2, Dv3, and Ev3 series VMs)
# can have either mlx4 or mlx5 hardware, with the older series VMs
# having a higher chance of coming with mlx4 hardware.
# https://docs.microsoft.com/en-us/azure/virtual-machines/dv2-dsv2-series
# https://docs.microsoft.com/en-us/azure/virtual-machines/dv3-dsv3-series
# https://docs.microsoft.com/en-us/azure/virtual-machines/ev3-esv3-series
blacklist = ['mlx4_core', 'mlx5_core']
# generate a network config, blacklist picking mlx4_core and mlx5_core devs
return net.generate_fallback_config(
blacklist_drivers=blacklist, config_driver=True)
blacklist_drivers=BLACKLIST_DRIVERS, config_driver=True)


@azure_ds_telemetry_reporter
Expand Down
49 changes: 35 additions & 14 deletions tests/unittests/test_datasource/test_azure.py
Original file line number Diff line number Diff line change
Expand Up @@ -526,7 +526,7 @@ def _get_mockds(self):
])
return dsaz

def _get_ds(self, data, agent_command=None, distro=None,
def _get_ds(self, data, agent_command=None, distro='ubuntu',
anhvoms marked this conversation as resolved.
Show resolved Hide resolved
apply_network=None):

def dsdevs():
Expand Down Expand Up @@ -576,7 +576,7 @@ def _dmi_mocks(key):
side_effect=_wait_for_files)),
])

if distro is not None:
if isinstance(distro, str):
distro_cls = distros.fetch(distro)
distro = distro_cls(distro, data.get('sys_cfg', {}), self.paths)
dsrc = dsaz.DataSourceAzure(
Expand Down Expand Up @@ -638,7 +638,7 @@ def test_call_is_platform_viable_seed(self, m_is_platform_viable):
# Return a non-matching asset tag value
m_is_platform_viable.return_value = False
dsrc = dsaz.DataSourceAzure(
{}, distro=None, paths=self.paths)
{}, distro=mock.Mock(), paths=self.paths)
self.assertFalse(dsrc.get_data())
m_is_platform_viable.assert_called_with(dsrc.seed_dir)

Expand Down Expand Up @@ -1311,6 +1311,28 @@ def test_fallback_network_config(self, mock_fallback, mock_dd,
blacklist_drivers=['mlx4_core', 'mlx5_core'],
config_driver=True)

@mock.patch(MOCKPATH + 'net.get_interfaces')
@mock.patch(MOCKPATH + 'util.is_FreeBSD')
def test_blacklist_through_distro(
self, m_is_freebsd, m_net_get_interfaces):
"""Verify Azure DS updates blacklist drivers in the distro's
networking object."""
odata = {'HostName': "myhost", 'UserName': "myuser"}
data = {'ovfcontent': construct_valid_ovf_env(data=odata),
'sys_cfg': {}}

distro_cls = distros.fetch('ubuntu')
distro = distro_cls('ubuntu', {}, self.paths)
dsrc = self._get_ds(data, distro=distro)
dsrc.get_data()
self.assertEqual(distro.networking.blacklist_drivers,
dsaz.BLACKLIST_DRIVERS)

m_is_freebsd.return_value = False
distro.networking.get_interfaces_by_mac()
m_net_get_interfaces.assert_called_with(
blacklist_drivers=dsaz.BLACKLIST_DRIVERS)

@mock.patch(MOCKPATH + 'subp.subp')
def test_get_hostname_with_no_args(self, m_subp):
dsaz.get_hostname()
Expand Down Expand Up @@ -1421,8 +1443,7 @@ def _get_ds(self, ovfcontent=None, agent_command=None):
if ovfcontent is not None:
populate_dir(os.path.join(self.paths.seed_dir, "azure"),
{'ovf-env.xml': ovfcontent})
dsrc = dsaz.DataSourceAzure(
{}, distro=None, paths=self.paths)
dsrc = dsaz.DataSourceAzure({}, distro=mock.Mock(), paths=self.paths)
if agent_command is not None:
dsrc.ds_cfg['agent_command'] = agent_command
return dsrc
Expand Down Expand Up @@ -1906,7 +1927,7 @@ def test_clear_cached_attrs_clears_imds(self):
tmp = self.tmp_dir()
paths = helpers.Paths(
{'cloud_dir': tmp, 'run_dir': tmp})
dsrc = dsaz.DataSourceAzure({}, distro=None, paths=paths)
dsrc = dsaz.DataSourceAzure({}, distro=mock.Mock(), paths=paths)
clean_values = [dsrc.metadata, dsrc.userdata, dsrc._metadata_imds]
dsrc.metadata = 'md'
dsrc.userdata = 'ud'
Expand Down Expand Up @@ -1970,23 +1991,23 @@ def test__should_reprovision_with_true_cfg(self, isfile, write_f):
"""The _should_reprovision method should return true with config
flag present."""
isfile.return_value = False
dsa = dsaz.DataSourceAzure({}, distro=None, paths=self.paths)
dsa = dsaz.DataSourceAzure({}, distro=mock.Mock(), paths=self.paths)
self.assertTrue(dsa._should_reprovision(
(None, None, {'PreprovisionedVm': True}, None)))

def test__should_reprovision_with_file_existing(self, isfile):
"""The _should_reprovision method should return True if the sentinal
exists."""
isfile.return_value = True
dsa = dsaz.DataSourceAzure({}, distro=None, paths=self.paths)
dsa = dsaz.DataSourceAzure({}, distro=mock.Mock(), paths=self.paths)
self.assertTrue(dsa._should_reprovision(
(None, None, {'preprovisionedvm': False}, None)))

def test__should_reprovision_returns_false(self, isfile):
"""The _should_reprovision method should return False
if config and sentinal are not present."""
isfile.return_value = False
dsa = dsaz.DataSourceAzure({}, distro=None, paths=self.paths)
dsa = dsaz.DataSourceAzure({}, distro=mock.Mock(), paths=self.paths)
self.assertFalse(dsa._should_reprovision((None, None, {}, None)))

@mock.patch(MOCKPATH + 'DataSourceAzure._poll_imds')
Expand All @@ -1997,7 +2018,7 @@ def test_reprovision_calls__poll_imds(self, _poll_imds, isfile):
username = "myuser"
odata = {'HostName': hostname, 'UserName': username}
_poll_imds.return_value = construct_valid_ovf_env(data=odata)
dsa = dsaz.DataSourceAzure({}, distro=None, paths=self.paths)
dsa = dsaz.DataSourceAzure({}, distro=mock.Mock(), paths=self.paths)
dsa._reprovision()
_poll_imds.assert_called_with()

Expand Down Expand Up @@ -2051,7 +2072,7 @@ def fake_timeout_once(**kwargs):

fake_resp.side_effect = fake_timeout_once

dsa = dsaz.DataSourceAzure({}, distro=None, paths=self.paths)
dsa = dsaz.DataSourceAzure({}, distro=mock.Mock(), paths=self.paths)
with mock.patch(MOCKPATH + 'REPORTED_READY_MARKER_FILE', report_file):
dsa._poll_imds()
self.assertEqual(report_ready_func.call_count, 1)
Expand All @@ -2071,7 +2092,7 @@ def test_poll_imds_report_ready_false(self,
'routers': '192.168.2.1', 'subnet-mask': '255.255.255.0',
'unknown-245': '624c3620'}]
m_media_switch.return_value = None
dsa = dsaz.DataSourceAzure({}, distro=None, paths=self.paths)
dsa = dsaz.DataSourceAzure({}, distro=mock.Mock(), paths=self.paths)
with mock.patch(MOCKPATH + 'REPORTED_READY_MARKER_FILE', report_file):
dsa._poll_imds()
self.assertEqual(report_ready_func.call_count, 0)
Expand Down Expand Up @@ -2109,7 +2130,7 @@ def test_poll_imds_returns_ovf_env(self, fake_resp,
full_url = url.format(host)
fake_resp.return_value = mock.MagicMock(status_code=200, text="ovf",
content="ovf")
dsa = dsaz.DataSourceAzure({}, distro=None, paths=self.paths)
dsa = dsaz.DataSourceAzure({}, distro=mock.Mock(), paths=self.paths)
self.assertTrue(len(dsa._poll_imds()) > 0)
self.assertEqual(fake_resp.call_args_list,
[mock.call(allow_redirects=True,
Expand Down Expand Up @@ -2146,7 +2167,7 @@ def test__reprovision_calls__poll_imds(self, fake_resp,
content = construct_valid_ovf_env(data=odata)
fake_resp.return_value = mock.MagicMock(status_code=200, text=content,
content=content)
dsa = dsaz.DataSourceAzure({}, distro=None, paths=self.paths)
dsa = dsaz.DataSourceAzure({}, distro=mock.Mock(), paths=self.paths)
md, _ud, cfg, _d = dsa._reprovision()
self.assertEqual(md['local-hostname'], hostname)
self.assertEqual(cfg['system_info']['default_user']['name'], username)
Expand Down