Skip to content

Commit

Permalink
comments
Browse files Browse the repository at this point in the history
  • Loading branch information
holmanb committed Jan 16, 2024
1 parent 8fbf9f3 commit fed7f30
Show file tree
Hide file tree
Showing 10 changed files with 79 additions and 57 deletions.
57 changes: 46 additions & 11 deletions cloudinit/distros/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -161,11 +161,7 @@ def __init__(self, name, cfg, paths):
self._cfg = cfg
self.name = name
self.networking: Networking = self.networking_cls()
self.dhcp_client_priority = [
dhcp.IscDhclient,
dhcp.Dhcpcd,
dhcp.Udhcpc,
]
self.dhcp_client_priority = dhcp.ALL_DHCP_CLIENTS
self.net_ops = iproute2.Iproute2
self._runner = helpers.Runners(paths)
self.package_managers: List[PackageManager] = []
Expand Down Expand Up @@ -290,13 +286,47 @@ def dhcp_client(self) -> dhcp.DhcpClient:
"""
if self._dhcp_client:
return self._dhcp_client
self._select_dhcp_client()
return self._dhcp_client

def _select_dhcp_client(self) -> dhcp.DhcpClient:
if self._dhcp_client:
return self._dhcp_client
for client in self.dhcp_client_priority:
# no client has been selected yet, so pick one
#
# set the default priority list to the distro-defined priority list
dhcp_client_priority = self.dhcp_client_priority

# if the configuration includes a network.dhcp_client_priority list
# then attempt to use it
config_priority = util.get_cfg_by_path(
self._cfg, ("network", "dhcp_client_priority"), []
)

if config_priority:
# user or image builder configured a custom dhcp client priority
# list
found_clients = []
LOG.debug(
"Using configured dhcp client priority list: %s",
config_priority,
)
for client_configured in config_priority:
for client_class in dhcp.ALL_DHCP_CLIENTS:
if client_configured == client_class.client_name:
found_clients.append(client_class)
break
else:
LOG.warning(
"Configured dhcp client %s is not supported, skipping",
client_configured,
)
# If dhcp_client_priority is defined in the configuration, but none
# of the defined clients are supported by cloud-init, then we don't
# override the distro default. If at least one client in the
# configured list exists, then we use that for our list of clients
# to check.
if found_clients:
dhcp_client_priority = found_clients

# iterate through our priority list and use the first client that is
# installed on the system
for client in dhcp_client_priority:
try:
self._dhcp_client = client()
LOG.debug("DHCP client selected: %s", client.client_name)
Expand Down Expand Up @@ -1284,6 +1314,11 @@ def fallback_interface(self):
"""Determine the network interface used during local network config."""
if self._fallback_interface is None:
self._fallback_interface = net.find_fallback_nic()
if not self._fallback_interface:
LOG.warning(
"Did not find a fallback interface on distro: %s.",
self.name,
)
return self._fallback_interface

@fallback_interface.setter
Expand Down
24 changes: 12 additions & 12 deletions cloudinit/net/dhcp.py
Original file line number Diff line number Diff line change
Expand Up @@ -174,6 +174,7 @@ def stop_service(cls, dhcp_interface: str, distro):
def parse_static_routes(routes: str) -> List[Tuple[str, str]]:
return []

@abc.abstractmethod
def get_newest_lease(self, interface: str) -> Dict[str, Any]:
return {}

Expand Down Expand Up @@ -511,14 +512,13 @@ def get_key_from_latest_lease(self, distro, key: str):
"""
lease_file = self.get_newest_lease_file_from_distro(distro)
if lease_file:
with suppress(FileNotFoundError):
content: str
content = util.load_file(lease_file) # pyright: ignore
if content:
for lease in reversed(self.parse_leases(content)):
server = lease.get(key)
if server:
return server
content: str
content = util.load_file(lease_file) # pyright: ignore
if content:
for lease in reversed(self.parse_leases(content)):
server = lease.get(key)
if server:
return server


class Dhcpcd(DhcpClient):
Expand Down Expand Up @@ -626,9 +626,6 @@ def parse_dhcpcd_lease(lease_dump: str, interface: str) -> Dict:
for source, destination in name_map.items():
if source in lease:
lease[destination] = lease.pop(source)

# nothing ever uses multiple leases in cloud-init codebase but
# for compatibility we still expect a list of leases
return lease

def get_newest_lease(self, interface: str) -> Dict[str, Any]:
Expand Down Expand Up @@ -663,7 +660,7 @@ def get_newest_lease(self, interface: str) -> Dict[str, Any]:
@staticmethod
def parse_static_routes(routes: str) -> List[Tuple[str, str]]:
"""
classless static routes as returnedd from dhcpcd --dumplease and return
classless static routes as returned from dhcpcd --dumplease and return
a list containing tuple of strings.
The tuple is composed of the network_address (including net length) and
Expand Down Expand Up @@ -788,3 +785,6 @@ def get_newest_lease(self, interface: str) -> Dict[str, Any]:
i for i in zip(static_routes[::2], static_routes[1::2])
]
return lease_json


ALL_DHCP_CLIENTS = [IscDhclient, Dhcpcd, Udhcpc]
2 changes: 1 addition & 1 deletion cloudinit/sources/DataSourceCloudStack.py
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,7 @@ def _get_domainname(self):
# cloud-init set up.
with suppress(FileNotFoundError):
latest_lease = self.distro.dhcp_client.get_newest_lease(
self.fallback_interface
self.distro.fallback_interface
)
domain_name = latest_lease.get("domain-name") or None
return domain_name
Expand Down
4 changes: 2 additions & 2 deletions cloudinit/sources/DataSourceEc2.py
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,7 @@ def _get_data(self):
try:
with EphemeralIPNetwork(
self.distro,
self.fallback_interface,
self.distro.fallback_interface,
ipv4=True,
ipv6=True,
) as netw:
Expand Down Expand Up @@ -500,7 +500,7 @@ def network_config(self):
func=self.get_data,
)

iface = self.fallback_interface
iface = self.distro.fallback_interface
net_md = self.metadata.get("network")
if isinstance(net_md, dict):
# SRU_BLOCKER: xenial, bionic and eoan should default
Expand Down
4 changes: 2 additions & 2 deletions cloudinit/sources/DataSourceGCE.py
Original file line number Diff line number Diff line change
Expand Up @@ -124,10 +124,10 @@ def _get_data(self):
except NoDHCPLeaseError:
continue
if ret["success"]:
self.fallback_interface = candidate_nic
self.distro.fallback_interface = candidate_nic
LOG.debug("Primary NIC found: %s.", candidate_nic)
break
if self.fallback_interface is None:
if self.distro.fallback_interface is None:
LOG.warning(
"Did not find a fallback interface on %s.", self.cloud_name
)
Expand Down
4 changes: 3 additions & 1 deletion cloudinit/sources/DataSourceOpenStack.py
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,9 @@ def _get_data(self):
if self.perform_dhcp_setup: # Setup networking in init-local stage.
try:

with EphemeralDHCPv4(self.distro, self.fallback_interface):
with EphemeralDHCPv4(
self.distro, self.distro.fallback_interface
):
results = util.log_time(
logfunc=LOG.debug,
msg="Crawl of metadata service",
Expand Down
8 changes: 4 additions & 4 deletions cloudinit/sources/DataSourceScaleway.py
Original file line number Diff line number Diff line change
Expand Up @@ -276,7 +276,7 @@ def _get_data(self):
# it will only reach timeout on VMs with only IPv6 addresses.
with EphemeralDHCPv4(
self.distro,
self.fallback_interface,
self.distro.fallback_interface,
) as ipv4:
util.log_time(
logfunc=LOG.debug,
Expand Down Expand Up @@ -307,7 +307,7 @@ def _get_data(self):
try:
with EphemeralIPv6Network(
self.distro,
self.fallback_interface,
self.distro.fallback_interface,
):
util.log_time(
logfunc=LOG.debug,
Expand Down Expand Up @@ -370,13 +370,13 @@ def network_config(self):
ip_cfg["routes"] += [route]
else:
ip_cfg["routes"] = [route]
netcfg[self.fallback_interface] = ip_cfg
netcfg[self.distro.fallback_interface] = ip_cfg
self._network_config = {"version": 2, "ethernets": netcfg}
else:
# Kept for backward compatibility
netcfg = {
"type": "physical",
"name": "%s" % self.fallback_interface,
"name": "%s" % self.distro.fallback_interface,
}
subnets = [{"type": "dhcp4"}]
if self.metadata["ipv6"]:
Expand Down
13 changes: 0 additions & 13 deletions cloudinit/sources/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -604,19 +604,6 @@ def get_vendordata2(self):
self.vendordata2 = self.ud_proc.process(self.get_vendordata2_raw())
return self.vendordata2

@property
def fallback_interface(self):
"""Determine the network interface used during local network config."""
if self.distro.fallback_interface is None:
LOG.warning(
"Did not find a fallback interface on %s.", self.cloud_name
)
return self.distro.fallback_interface

@fallback_interface.setter
def fallback_interface(self, value):
self.distro.fallback_interface = value

@property
def platform_type(self):
if not hasattr(self, "_platform_type"):
Expand Down
8 changes: 3 additions & 5 deletions tests/unittests/sources/test_gce.py
Original file line number Diff line number Diff line change
Expand Up @@ -405,9 +405,8 @@ def test_publish_host_keys(self, m_readurl):
autospec=True,
)
@mock.patch(M_PATH + "net.find_candidate_nics", return_value=["ens4"])
@mock.patch(M_PATH + "DataSourceGCELocal.fallback_interface")
def test_local_datasource_uses_ephemeral_dhcp(
self, _m_fallback, _m_find_candidate_nics, m_dhcp
self, _m_find_candidate_nics, m_dhcp
):
self._set_mock_metadata()
distro = mock.MagicMock()
Expand All @@ -424,9 +423,8 @@ def test_local_datasource_uses_ephemeral_dhcp(
autospec=True,
)
@mock.patch(M_PATH + "net.find_candidate_nics")
@mock.patch(M_PATH + "DataSourceGCELocal.fallback_interface")
def test_local_datasource_tries_on_multi_nic(
self, _m_fallback, m_find_candidate_nics, m_dhcp, m_read_md
self, m_find_candidate_nics, m_dhcp, m_read_md
):
self._set_mock_metadata()
distro = mock.MagicMock()
Expand Down Expand Up @@ -464,7 +462,7 @@ def test_local_datasource_tries_on_multi_nic(
mock.call(distro, iface="ens0p5"),
mock.call(distro, iface="ens0p6"),
]
assert ds.fallback_interface == "ens0p6"
assert ds.distro.fallback_interface == "ens0p6"
assert ds.metadata == "md"
assert ds.userdata_raw == "ud"

Expand Down
12 changes: 6 additions & 6 deletions tests/unittests/sources/test_init.py
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ class TestDataSource(CiTestCase):
def setUp(self):
super(TestDataSource, self).setUp()
self.sys_cfg = {"datasource": {"_undef": {"key1": False}}}
self.distro = ubuntu.Distro("", {}, {})
self.distro = ubuntu.Distro("somedistro", {}, {})
self.paths = Paths({})
self.datasource = DataSource(self.sys_cfg, self.distro, self.paths)

Expand Down Expand Up @@ -206,24 +206,24 @@ def test_datasource_get_url_uses_defaults_on_errors(self):
def test_fallback_interface_is_discovered(self, m_get_fallback_nic):
"""The fallback_interface is discovered via find_fallback_nic."""
m_get_fallback_nic.return_value = "nic9"
self.assertEqual("nic9", self.datasource.fallback_interface)
self.assertEqual("nic9", self.datasource.distro.fallback_interface)

@mock.patch("cloudinit.sources.net.find_fallback_nic")
def test_fallback_interface_logs_undiscovered(self, m_get_fallback_nic):
"""Log a warning when fallback_interface can not discover the nic."""
self.datasource._cloud_name = "MySupahCloud"
m_get_fallback_nic.return_value = None # Couldn't discover nic
self.assertIsNone(self.datasource.fallback_interface)
self.assertIsNone(self.datasource.distro.fallback_interface)
self.assertEqual(
"WARNING: Did not find a fallback interface on MySupahCloud.\n",
"WARNING: Did not find a fallback interface on distro: "
"somedistro.\n",
self.logs.getvalue(),
)

@mock.patch("cloudinit.sources.net.find_fallback_nic")
def test_wb_fallback_interface_is_cached(self, m_get_fallback_nic):
"""The fallback_interface is cached and won't be rediscovered."""
self.datasource.distro.fallback_interface = "nic10"
self.assertEqual("nic10", self.datasource.fallback_interface)
self.assertEqual("nic10", self.datasource.distro.fallback_interface)
m_get_fallback_nic.assert_not_called()

def test__get_data_unimplemented(self):
Expand Down

0 comments on commit fed7f30

Please sign in to comment.