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.

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

sources/azure: always initialize _ephemeral_dhcp_ctx on unpickle #1199

Merged
merged 2 commits into from
Jan 20, 2022
Merged

sources/azure: always initialize _ephemeral_dhcp_ctx on unpickle #1199

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
f3d641d
sources/azure: always initialize _ephemeral_dhcp_ctx on unpickle
cjp256 Jan 19, 2022
9c41f4e
sources/azure: add check for expected lease value
cjp256 Jan 19, 2022
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
14 changes: 9 additions & 5 deletions cloudinit/sources/DataSourceAzure.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -299,6 +299,8 @@ def __init__(self, sys_cfg, distro, paths):

def _unpickle(self, ci_pkl_version: int) -> None:
super()._unpickle(ci_pkl_version)

self._ephemeral_dhcp_ctx = None
if not hasattr(self, "iso_dev"):
self.iso_dev = None

Expand Down Expand Up @@ -513,9 +515,11 @@ def crawl_metadata(self):

if perform_reprovision or perform_reprovision_after_nic_attach:
LOG.info("Reporting ready to Azure after getting ReprovisionData")
use_cached_ephemeral = self.distro.networking.is_up(
self.fallback_interface
) and getattr(self, "_ephemeral_dhcp_ctx", None)
use_cached_ephemeral = (
self.distro.networking.is_up(self.fallback_interface)
and self._ephemeral_dhcp_ctx
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need to check for self._ephemeral_dhcp_ctx.lease here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In practice I expect that a NoDHCPLeaseError exception would have been raised and prevented it from getting this far.

I added an explicit check and I have other PRs in queue to further improve this handling.

and self._ephemeral_dhcp_ctx.lease
)
if use_cached_ephemeral:
self._report_ready(lease=self._ephemeral_dhcp_ctx.lease)
self._ephemeral_dhcp_ctx.clean_network() # Teardown ephemeral
Expand Down Expand Up @@ -1321,8 +1325,8 @@ def _report_failure(self, description=None) -> bool:
try:
if (
self.distro.networking.is_up(self.fallback_interface)
and getattr(self, "_ephemeral_dhcp_ctx", None)
and getattr(self._ephemeral_dhcp_ctx, "lease", None)
and self._ephemeral_dhcp_ctx
and self._ephemeral_dhcp_ctx.lease
and unknown_245_key in self._ephemeral_dhcp_ctx.lease
):
report_diagnostic_event(
Expand Down