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

load_azure_ds_dir() always returns a tuple.  Instead of saving this
tuple as ret, expand it immediately as md, userdata_raw, cfg, files.

This allows for more fine-grained passing of data before getting
expanded later.

- Update _should_reprovision methods to use cfg instead of tuple.

- Update _should_reprovision methods to remove the ovf_md guard.
 This should be a safe refactor as the OVF is not required, and the
 config is initialized to an empty dict.  In practice, a mount failure
 would have initialized ret anyways if the OVF was not found. If a
 mount failure wasn't seen and ret was None, this guard could be
 causing other failures by ignoring the PPS state that should be
 available from IMDS metadata.

Signed-off-by: Chris Patterson <cpatterson@microsoft.com>

2 files changed, 23 insertions, 32 deletions

diff --git a/cloudinit/sources/DataSourceAzure.py b/cloudinit/sources/DataSourceAzure.py
index 7134c95a..9577355c 100755
--- a/cloudinit/sources/DataSourceAzure.py
+++ b/cloudinit/sources/DataSourceAzure.py
@@ -338,7 +338,10 @@ class DataSourceAzure(sources.DataSource):
         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"
@@ -361,15 +364,17 @@ class DataSourceAzure(sources.DataSource):
                 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
@@ -383,11 +388,8 @@ class DataSourceAzure(sources.DataSource):
                     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:
@@ -414,11 +416,11 @@ class DataSourceAzure(sources.DataSource):
             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:
@@ -428,13 +430,12 @@ class DataSourceAzure(sources.DataSource):
                 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(
             {
@@ -1408,7 +1409,7 @@ class DataSourceAzure(sources.DataSource):
             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.
@@ -1421,13 +1422,10 @@ class DataSourceAzure(sources.DataSource):
         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"
@@ -1445,7 +1443,7 @@ class DataSourceAzure(sources.DataSource):
             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.
 
@@ -1456,9 +1454,6 @@ class DataSourceAzure(sources.DataSource):
         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
diff --git a/tests/unittests/sources/test_azure.py b/tests/unittests/sources/test_azure.py
index 02083184..d950e156 100644
--- a/tests/unittests/sources/test_azure.py
+++ b/tests/unittests/sources/test_azure.py
@@ -2803,9 +2803,7 @@ class TestPreprovisioningShouldReprovision(CiTestCase):
         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):
@@ -2814,9 +2812,7 @@ class TestPreprovisioningShouldReprovision(CiTestCase):
         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):
@@ -2824,7 +2820,7 @@ class TestPreprovisioningShouldReprovision(CiTestCase):
         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):
@@ -2834,14 +2830,14 @@ class TestPreprovisioningShouldReprovision(CiTestCase):
         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}}},
             )
         )