Skip to content

sources/azure: removed unused savable PPS paths #1268

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.

Sign up for GitHub

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

Merged
TheRealFalcon merged 1 commit into from
Feb 17, 2022
Merged

sources/azure: removed unused savable PPS paths #1268

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
47 changes: 4 additions & 43 deletions cloudinit/sources/DataSourceAzure.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,6 @@
# DMI chassis-asset-tag is set static for all azure instances
AZURE_CHASSIS_ASSET_TAG = "7783-7084-3265-9085-8269-3286-77"
REPROVISION_MARKER_FILE = "/var/lib/cloud/data/poll_imds"
REPROVISION_NIC_DETACHED_MARKER_FILE = "/var/lib/cloud/data/nic_detached"
REPORTED_READY_MARKER_FILE = "/var/lib/cloud/data/reported_ready"
AGENT_SEED_DIR = "/var/lib/waagent"
DEFAULT_PROVISIONING_ISO_DEV = "/dev/sr0"
Expand Down Expand Up @@ -881,11 +880,6 @@ def _wait_for_nic_detach(self, nl_sock):
"The preprovisioned nic %s is detached" % ifname,
logger_func=LOG.warning,
)
path = REPROVISION_NIC_DETACHED_MARKER_FILE
LOG.info("Creating a marker file for nic detached: %s", path)
util.write_file(
path, "{pid}: {time}\n".format(pid=os.getpid(), time=time())
)
except AssertionError as error:
report_diagnostic_event(str(error), logger_func=LOG.error)
raise
Expand Down Expand Up @@ -1153,42 +1147,10 @@ def _wait_for_all_nics_ready(self):
nl_sock = None
try:
nl_sock = netlink.create_bound_netlink_socket()

report_ready_marker_present = bool(
os.path.isfile(REPORTED_READY_MARKER_FILE)
)

# Report ready if the marker file is not already present.
# The nic of the preprovisioned vm gets hot-detached as soon as
# we report ready. So no need to save the dhcp context.
if not os.path.isfile(REPORTED_READY_MARKER_FILE):
self._report_ready_for_pps()

has_nic_been_detached = bool(
os.path.isfile(REPROVISION_NIC_DETACHED_MARKER_FILE)
)

if not has_nic_been_detached:
LOG.info("NIC has not been detached yet.")
self._teardown_ephemeral_networking()
self._wait_for_nic_detach(nl_sock)

# If we know that the preprovisioned nic has been detached, and we
# still have a fallback nic, then it means the VM must have
# rebooted as part of customer assignment, and all the nics have
# already been attached by the Azure platform. So there is no need
# to wait for nics to be hot-attached.
if not self.fallback_interface:
self._wait_for_hot_attached_nics(nl_sock)
else:
report_diagnostic_event(
"Skipping waiting for nic attach "
"because we already have a fallback "
"interface. Report Ready marker "
"present before detaching nics: %s"
% report_ready_marker_present,
logger_func=LOG.info,
)
self._report_ready_for_pps()
self._teardown_ephemeral_networking()
self._wait_for_nic_detach(nl_sock)
self._wait_for_hot_attached_nics(nl_sock)
except netlink.NetlinkCreateSocketError as e:
report_diagnostic_event(str(e), logger_func=LOG.warning)
raise
Expand Down Expand Up @@ -1506,7 +1468,6 @@ def _cleanup_markers(self):
"""Cleanup any marker files."""
util.del_file(REPORTED_READY_MARKER_FILE)
util.del_file(REPROVISION_MARKER_FILE)
util.del_file(REPROVISION_NIC_DETACHED_MARKER_FILE)

@azure_ds_telemetry_reporter
def activate(self, cfg, is_new_instance):
Expand Down
123 changes: 11 additions & 112 deletions tests/unittests/sources/test_azure.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2942,126 +2942,32 @@ def setUp(self):
dsaz.BUILTIN_DS_CONFIG["data_dir"] = self.waagent_d
self.paths = helpers.Paths({"cloud_dir": self.tmp})

@mock.patch(
"cloudinit.sources.helpers.netlink.wait_for_nic_detach_event",
autospec=True,
)
@mock.patch(MOCKPATH + "util.write_file", autospec=True)
def test_nic_detach_writes_marker(self, m_writefile, m_detach):
"""When we detect that a nic gets detached, we write a marker for it"""
dsa = dsaz.DataSourceAzure({}, distro=None, paths=self.paths)
nl_sock = mock.MagicMock()
dsa._wait_for_nic_detach(nl_sock)
m_detach.assert_called_with(nl_sock)
self.assertEqual(1, m_detach.call_count)
m_writefile.assert_called_with(
dsaz.REPROVISION_NIC_DETACHED_MARKER_FILE, mock.ANY
)

@mock.patch(MOCKPATH + "util.write_file", autospec=True)
@mock.patch(MOCKPATH + "DataSourceAzure.fallback_interface")
@mock.patch(MOCKPATH + "DataSourceAzure._report_ready")
@mock.patch(MOCKPATH + "DataSourceAzure._wait_for_hot_attached_nics")
@mock.patch(MOCKPATH + "DataSourceAzure._wait_for_nic_detach")
def test_detect_nic_attach_reports_ready_and_waits_for_detach(
self, m_detach, m_report_ready, m_fallback_if, m_writefile
self,
m_detach,
m_wait_for_hot_attached_nics,
m_report_ready,
m_writefile,
):
"""Report ready first and then wait for nic detach"""
dsa = dsaz.DataSourceAzure({}, distro=None, paths=self.paths)
dsa._wait_for_all_nics_ready()
m_fallback_if.return_value = "Dummy interface"
self.assertEqual(1, m_report_ready.call_count)
self.assertEqual(1, m_wait_for_hot_attached_nics.call_count)
self.assertEqual(1, m_detach.call_count)
self.assertEqual(1, m_writefile.call_count)
m_writefile.assert_called_with(
dsaz.REPORTED_READY_MARKER_FILE, mock.ANY
)

@mock.patch("os.path.isfile")
@mock.patch(MOCKPATH + "DataSourceAzure.fallback_interface")
@mock.patch(MOCKPATH + "EphemeralDHCPv4", autospec=True)
@mock.patch(MOCKPATH + "DataSourceAzure._report_ready")
@mock.patch(MOCKPATH + "DataSourceAzure._wait_for_nic_detach")
def test_detect_nic_attach_skips_report_ready_when_marker_present(
self, m_detach, m_report_ready, m_dhcp, m_fallback_if, m_isfile
):
"""Skip reporting ready if we already have a marker file."""
dsa = dsaz.DataSourceAzure({}, distro=None, paths=self.paths)

def isfile(key):
return key == dsaz.REPORTED_READY_MARKER_FILE

m_isfile.side_effect = isfile
dsa._wait_for_all_nics_ready()
m_fallback_if.return_value = "Dummy interface"
self.assertEqual(0, m_report_ready.call_count)
self.assertEqual(0, m_dhcp.call_count)
self.assertEqual(1, m_detach.call_count)

@mock.patch("os.path.isfile")
@mock.patch(MOCKPATH + "DataSourceAzure.fallback_interface")
@mock.patch(MOCKPATH + "EphemeralDHCPv4")
@mock.patch(MOCKPATH + "util.write_file", autospec=True)
@mock.patch(MOCKPATH + "DataSourceAzure._report_ready")
@mock.patch(MOCKPATH + "DataSourceAzure._wait_for_nic_detach")
def test_detect_nic_attach_skips_nic_detach_when_marker_present(
self, m_detach, m_report_ready, m_dhcp, m_fallback_if, m_isfile
):
"""Skip wait for nic detach if it already happened."""
dsa = dsaz.DataSourceAzure({}, distro=None, paths=self.paths)

m_isfile.return_value = True
dsa._wait_for_all_nics_ready()
m_fallback_if.return_value = "Dummy interface"
self.assertEqual(0, m_report_ready.call_count)
self.assertEqual(0, m_dhcp.call_count)
self.assertEqual(0, m_detach.call_count)

@mock.patch(MOCKPATH + "DataSourceAzure.wait_for_link_up", autospec=True)
@mock.patch("cloudinit.sources.helpers.netlink.wait_for_nic_attach_event")
@mock.patch("cloudinit.sources.net.find_fallback_nic")
@mock.patch(MOCKPATH + "get_metadata_from_imds")
@mock.patch(MOCKPATH + "EphemeralDHCPv4", autospec=True)
@mock.patch(MOCKPATH + "DataSourceAzure._wait_for_nic_detach")
@mock.patch("os.path.isfile")
def test_wait_for_nic_attach_if_no_fallback_interface(
self,
m_isfile,
m_detach,
m_dhcpv4,
m_imds,
m_fallback_if,
m_attach,
m_link_up,
):
"""Wait for nic attach if we do not have a fallback interface"""
dsa = dsaz.DataSourceAzure({}, distro=None, paths=self.paths)
lease = {
"interface": "eth9",
"fixed-address": "192.168.2.9",
"routers": "192.168.2.1",
"subnet-mask": "255.255.255.0",
"unknown-245": "624c3620",
}

m_isfile.return_value = True
m_attach.return_value = "eth0"
dhcp_ctx = mock.MagicMock(lease=lease)
dhcp_ctx.obtain_lease.return_value = lease
m_dhcpv4.return_value = dhcp_ctx
m_imds.return_value = IMDS_NETWORK_METADATA
m_fallback_if.return_value = None

dsa._wait_for_all_nics_ready()

self.assertEqual(0, m_detach.call_count)
self.assertEqual(1, m_attach.call_count)
self.assertEqual(1, m_dhcpv4.call_count)
self.assertEqual(1, m_imds.call_count)
self.assertEqual(1, m_link_up.call_count)
m_link_up.assert_called_with(mock.ANY, "eth0")

@mock.patch(MOCKPATH + "DataSourceAzure.wait_for_link_up")
@mock.patch("cloudinit.sources.helpers.netlink.wait_for_nic_attach_event")
@mock.patch("cloudinit.sources.net.find_fallback_nic")
@mock.patch(MOCKPATH + "DataSourceAzure.get_imds_data_with_api_fallback")
@mock.patch(MOCKPATH + "EphemeralDHCPv4", autospec=True)
@mock.patch(MOCKPATH + "DataSourceAzure._wait_for_nic_detach")
Expand All @@ -3072,9 +2978,10 @@ def test_wait_for_nic_attach_multinic_attach(
m_detach,
m_dhcpv4,
m_imds,
m_fallback_if,
m_attach,
m_link_up,
m_report_ready,
m_writefile,
):
"""Wait for nic attach if we do not have a fallback interface"""
dsa = dsaz.DataSourceAzure({}, distro=None, paths=self.paths)
Expand Down Expand Up @@ -3103,11 +3010,10 @@ def test_wait_for_nic_attach_multinic_attach(
dhcp_ctx.obtain_lease.return_value = lease
m_dhcpv4.return_value = dhcp_ctx
m_imds.side_effect = [md]
m_fallback_if.return_value = None

dsa._wait_for_all_nics_ready()

self.assertEqual(0, m_detach.call_count)
self.assertEqual(1, m_detach.call_count)
self.assertEqual(2, m_attach.call_count)
# DHCP and network metadata calls will only happen on the primary NIC.
self.assertEqual(1, m_dhcpv4.call_count)
Expand Down Expand Up @@ -3976,12 +3882,8 @@ def test_savable_pps(self):
False, # /var/lib/cloud/data/poll_imds
False, # seed/azure/ovf-env.xml
False, # /var/lib/cloud/data/poll_imds
False, # /var/lib/cloud/data/reported_ready
False, # /var/lib/cloud/data/reported_ready
False, # /var/lib/cloud/data/nic_detached
True, # /var/lib/cloud/data/reported_ready
]
self.azure_ds._fallback_interface = False

self.azure_ds._get_data()

Expand All @@ -3994,9 +3896,6 @@ def test_savable_pps(self):
),
mock.call("/var/lib/cloud/data/poll_imds"),
mock.call("/var/lib/cloud/data/reported_ready"),
mock.call("/var/lib/cloud/data/reported_ready"),
mock.call("/var/lib/cloud/data/nic_detached"),
mock.call("/var/lib/cloud/data/reported_ready"),
]

assert self.mock_readurl.mock_calls == [
Expand Down