Skip to content

sources/azure: unpack ret tuple in crawl_metadata() #1194

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
Jan 20, 2022
+23 −32
Merged

sources/azure: unpack ret tuple in crawl_metadata() #1194

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
39 changes: 17 additions & 22 deletions cloudinit/sources/DataSourceAzure.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -338,7 +338,10 @@ def crawl_metadata(self):
ovf_is_accessible = False
reprovision_after_nic_attach = False
metadata_source = None
ret = None
md = {}
userdata_raw = ""
cfg = {}
files = {}
if os.path.isfile(REPROVISION_MARKER_FILE):
reprovision = True
metadata_source = "IMDS"
Expand All @@ -361,15 +364,17 @@ def crawl_metadata(self):
try:
if src.startswith("/dev/"):
if util.is_FreeBSD():
ret = util.mount_cb(
md, userdata_raw, cfg, files = util.mount_cb(
src, load_azure_ds_dir, mtype="udf"
)
else:
ret = util.mount_cb(src, load_azure_ds_dir)
md, userdata_raw, cfg, files = util.mount_cb(
src, load_azure_ds_dir
)
# save the device for ejection later
self.iso_dev = src
else:
ret = load_azure_ds_dir(src)
md, userdata_raw, cfg, files = load_azure_ds_dir(src)
ovf_is_accessible = True
metadata_source = src
break
Expand All @@ -383,11 +388,8 @@ def crawl_metadata(self):
report_diagnostic_event(
"%s was not mountable" % src, logger_func=LOG.debug
)
empty_md = {"local-hostname": ""}
empty_cfg = dict(
system_info=dict(default_user=dict(name=""))
)
ret = (empty_md, "", empty_cfg, {})
md = {"local-hostname": ""}
cfg = {"system_info": {"default_user": {"name": ""}}}
metadata_source = "IMDS"
continue
except BrokenAzureDataSource as exc:
Expand All @@ -414,11 +416,11 @@ def crawl_metadata(self):
raise sources.InvalidMetaDataException(msg)

perform_reprovision = reprovision or self._should_reprovision(
ret, imds_md
cfg, imds_md
)
perform_reprovision_after_nic_attach = (
reprovision_after_nic_attach
or self._should_reprovision_after_nic_attach(ret, imds_md)
or self._should_reprovision_after_nic_attach(cfg, imds_md)
)

if perform_reprovision or perform_reprovision_after_nic_attach:
Expand All @@ -428,13 +430,12 @@ def crawl_metadata(self):
raise sources.InvalidMetaDataException(msg)
if perform_reprovision_after_nic_attach:
self._wait_for_all_nics_ready()
ret = self._reprovision()
md, userdata_raw, cfg, files = self._reprovision()
# fetch metadata again as it has changed after reprovisioning
imds_md = self.get_imds_data_with_api_fallback(
self.fallback_interface, retries=10
)

(md, userdata_raw, cfg, files) = ret
self.seed = metadata_source
crawled_data.update(
{
Expand Down Expand Up @@ -1408,7 +1409,7 @@ def _ppstype_from_imds(self, imds_md: dict = None) -> str:
return None

def _should_reprovision_after_nic_attach(
self, ovf_md, imds_md=None
self, cfg: dict, imds_md=None
) -> bool:
"""Whether or not we should wait for nic attach and then poll
IMDS for reprovisioning data. Also sets a marker file to poll IMDS.
Expand All @@ -1421,13 +1422,10 @@ def _should_reprovision_after_nic_attach(
the ISO, thus cloud-init needs to have a way of knowing that it should
jump back into the waiting mode in order to retrieve the ovf_env.

@param ovf_md: Metadata obtained from reading ovf-env.
@param cfg: OVF cfg.
@param imds_md: Metadata obtained from IMDS
@return: Whether to reprovision after waiting for nics to be attached.
"""
if not ovf_md:
return False
(_md, _userdata_raw, cfg, _files) = ovf_md
path = REPROVISION_NIC_ATTACH_MARKER_FILE
if (
cfg.get("PreprovisionedVMType", None) == "Savable"
Expand All @@ -1445,7 +1443,7 @@ def _should_reprovision_after_nic_attach(
return True
return False

def _should_reprovision(self, ovf_md, imds_md=None):
def _should_reprovision(self, cfg: dict, imds_md=None):
"""Whether or not we should poll IMDS for reprovisioning data.
Also sets a marker file to poll IMDS.

Expand All @@ -1456,9 +1454,6 @@ def _should_reprovision(self, ovf_md, imds_md=None):
However, since the VM reports ready to the Fabric, we will not attach
the ISO, thus cloud-init needs to have a way of knowing that it should
jump back into the polling loop in order to retrieve the ovf_env."""
if not ovf_md:
return False
(_md, _userdata_raw, cfg, _files) = ovf_md
path = REPROVISION_MARKER_FILE
if (
cfg.get("PreprovisionedVm") is True
Expand Down
16 changes: 6 additions & 10 deletions tests/unittests/sources/test_azure.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2803,9 +2803,7 @@ def test__should_reprovision_with_true_cfg(self, isfile, write_f):
isfile.return_value = False
dsa = dsaz.DataSourceAzure({}, distro=mock.Mock(), paths=self.paths)
self.assertTrue(
dsa._should_reprovision(
(None, None, {"PreprovisionedVm": True}, None)
)
dsa._should_reprovision({"PreprovisionedVm": True}, None)
)

def test__should_reprovision_with_file_existing(self, isfile):
Expand All @@ -2814,17 +2812,15 @@ def test__should_reprovision_with_file_existing(self, isfile):
isfile.return_value = True
dsa = dsaz.DataSourceAzure({}, distro=mock.Mock(), paths=self.paths)
self.assertTrue(
dsa._should_reprovision(
(None, None, {"preprovisionedvm": False}, None)
)
dsa._should_reprovision({"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=mock.Mock(), paths=self.paths)
self.assertFalse(dsa._should_reprovision((None, None, {}, None)))
self.assertFalse(dsa._should_reprovision({}))

@mock.patch(MOCKPATH + "util.write_file", autospec=True)
def test__should_reprovision_uses_imds_md(self, write_file, isfile):
Expand All @@ -2834,14 +2830,14 @@ def test__should_reprovision_uses_imds_md(self, write_file, isfile):
dsa = dsaz.DataSourceAzure({}, distro=mock.Mock(), paths=self.paths)
self.assertTrue(
dsa._should_reprovision(
(None, None, {}, None),
{},
{"extended": {"compute": {"ppsType": "Running"}}},
)
)
self.assertFalse(dsa._should_reprovision((None, None, {}, None), {}))
self.assertFalse(dsa._should_reprovision({}, {}))
self.assertFalse(
dsa._should_reprovision(
(None, None, {}, None),
{},
{"extended": {"compute": {"hasCustomData": False}}},
)
)
Expand Down