From 51671f58a7ec24a692be8edd2d4b01399fc8574b Mon Sep 17 00:00:00 2001 From: Markus Schade Date: Mon, 7 Feb 2022 15:00:37 +0100 Subject: [PATCH] sources/hetzner: Use EphemeralDHCPv4 instead of static configuration When the datasource was originally submitted, EphemeralDHCPv4 was not yet available. Also avoid race conditions by skipping network configuration if metadata service can be reached. Signed-off-by: Markus Schade --- cloudinit/sources/DataSourceHetzner.py | 53 +++++++++++++++---------- tests/unittests/sources/test_hetzner.py | 45 +++++++++++++-------- 2 files changed, 62 insertions(+), 36 deletions(-) diff --git a/cloudinit/sources/DataSourceHetzner.py b/cloudinit/sources/DataSourceHetzner.py index 50324cc41c7..91a6f9c9be1 100644 --- a/cloudinit/sources/DataSourceHetzner.py +++ b/cloudinit/sources/DataSourceHetzner.py @@ -9,8 +9,8 @@ import cloudinit.sources.helpers.hetzner as hc_helper from cloudinit import dmi from cloudinit import log as logging -from cloudinit import net as cloudnet -from cloudinit import sources, util +from cloudinit import net, sources, util +from cloudinit.net.dhcp import EphemeralDHCPv4, NoDHCPLeaseError LOG = logging.getLogger(__name__) @@ -45,7 +45,7 @@ def __init__(self, sys_cfg, distro, paths): self.retries = self.ds_cfg.get("retries", MD_RETRIES) self.timeout = self.ds_cfg.get("timeout", MD_TIMEOUT) self.wait_retry = self.ds_cfg.get("wait_retry", MD_WAIT_RETRY) - self._network_config = None + self._network_config = sources.UNSET self.dsmode = sources.DSMODE_NETWORK def _get_data(self): @@ -54,22 +54,28 @@ def _get_data(self): if not on_hetzner: return False - nic = cloudnet.find_fallback_nic() - with cloudnet.EphemeralIPv4Network( - nic, "169.254.0.1", 16, "169.254.255.255" - ): - md = hc_helper.read_metadata( - self.metadata_address, - timeout=self.timeout, - sec_between=self.wait_retry, - retries=self.retries, - ) - ud = hc_helper.read_userdata( - self.userdata_address, - timeout=self.timeout, - sec_between=self.wait_retry, - retries=self.retries, - ) + try: + with EphemeralDHCPv4( + iface=net.find_fallback_nic(), + connectivity_url_data={ + "url": BASE_URL_V1 + "/metadata/instance-id", + }, + ): + md = hc_helper.read_metadata( + self.metadata_address, + timeout=self.timeout, + sec_between=self.wait_retry, + retries=self.retries, + ) + ud = hc_helper.read_userdata( + self.userdata_address, + timeout=self.timeout, + sec_between=self.wait_retry, + retries=self.retries, + ) + except (NoDHCPLeaseError) as e: + LOG.error("Bailing, DHCP Exception: %s", e) + raise # Hetzner cloud does not support binary user-data. So here, do a # base64 decode of the data if we can. The end result being that a @@ -110,7 +116,14 @@ def network_config(self): migration. """ - if self._network_config: + if self._network_config is None: + LOG.warning( + "Found None as cached _network_config. Resetting to %s", + sources.UNSET, + ) + self._network_config = sources.UNSET + + if self._network_config != sources.UNSET: return self._network_config _net_config = self.metadata["network-config"] diff --git a/tests/unittests/sources/test_hetzner.py b/tests/unittests/sources/test_hetzner.py index 9e70de3448f..f80ed45f753 100644 --- a/tests/unittests/sources/test_hetzner.py +++ b/tests/unittests/sources/test_hetzner.py @@ -24,30 +24,24 @@ name: eth0 subnets: - dns_nameservers: - - 213.133.99.99 - - 213.133.100.100 - - 213.133.98.98 + - 185.12.64.1 + - 185.12.64.2 ipv4: true type: dhcp - type: physical - - name: eth0:0 - subnets: - address: 2a01:4f8:beef:beef::1/64 + dns_nameservers: + - 2a01:4ff:ff00::add:2 + - 2a01:4ff:ff00::add:1 gateway: fe80::1 ipv6: true - routes: - - gateway: fe80::1%eth0 - netmask: 0 - network: '::' - type: static type: physical version: 1 network-sysconfig: "DEVICE='eth0'\nTYPE=Ethernet\nBOOTPROTO=dhcp\n\ ONBOOT='yes'\nHWADDR=96:00:00:08:19:da\n\ IPV6INIT=yes\nIPV6ADDR=2a01:4f8:beef:beef::1/64\n\ IPV6_DEFAULTGW=fe80::1%eth0\nIPV6_AUTOCONF=no\n\ - DNS1=213.133.99.99\nDNS2=213.133.100.100\n" -public-ipv4: 192.168.0.1 + DNS1=185.12.64.1\nDNS2=185.12.64.2\n" +public-ipv4: 192.168.0.2 public-keys: - ssh-ed25519 \ AAAAC3Nzac1lZdI1NTE5AaaAIaFrcac0yVITsmRrmueq6MD0qYNKlEvW8O1Ib4nkhmWh \ @@ -77,13 +71,20 @@ def get_ds(self): ) return ds - @mock.patch("cloudinit.net.EphemeralIPv4Network") + @mock.patch("cloudinit.net.dhcp.maybe_perform_dhcp_discovery") + @mock.patch("cloudinit.sources.DataSourceHetzner.EphemeralDHCPv4") @mock.patch("cloudinit.net.find_fallback_nic") @mock.patch("cloudinit.sources.helpers.hetzner.read_metadata") @mock.patch("cloudinit.sources.helpers.hetzner.read_userdata") @mock.patch("cloudinit.sources.DataSourceHetzner.get_hcloud_data") def test_read_data( - self, m_get_hcloud_data, m_usermd, m_readmd, m_fallback_nic, m_net + self, + m_get_hcloud_data, + m_usermd, + m_readmd, + m_fallback_nic, + m_net, + m_dhcp, ): m_get_hcloud_data.return_value = ( True, @@ -92,13 +93,25 @@ def test_read_data( m_readmd.return_value = METADATA.copy() m_usermd.return_value = USERDATA m_fallback_nic.return_value = "eth0" + m_dhcp.return_value = [ + { + "interface": "eth0", + "fixed-address": "192.168.0.2", + "routers": "192.168.0.1", + "subnet-mask": "255.255.255.0", + "broadcast-address": "192.168.0.255", + } + ] ds = self.get_ds() ret = ds.get_data() self.assertTrue(ret) m_net.assert_called_once_with( - "eth0", "169.254.0.1", 16, "169.254.255.255" + iface="eth0", + connectivity_url_data={ + "url": "http://169.254.169.254/hetzner/v1/metadata/instance-id" + }, ) self.assertTrue(m_readmd.called)