cloud-init-dev team mailing list archive

[Chad Smith <chad.smith@xxxxxxxxxxxxx>]

Chad Smith has proposed merging ~chad.smith/cloud-init:ubuntu/devel into cloud-init:ubuntu/devel.

Commit message:
Sync upstream snapshot for disco release. Includes an Azure pre-provisioning bugfix


Requested reviews:
  cloud-init commiters (cloud-init-dev)
Related bugs:
  Bug #1803598 in cloud-init: "Do not retry polling IMDS for reprovisiondata during timeout"
  https://bugs.launchpad.net/cloud-init/+bug/1803598

For more details, see:
https://code.launchpad.net/~chad.smith/cloud-init/+git/cloud-init/+merge/358884
-- 
Your team cloud-init commiters is requested to review the proposed merge of ~chad.smith/cloud-init:ubuntu/devel into cloud-init:ubuntu/devel.

diff --git a/cloudinit/config/cc_resizefs.py b/cloudinit/config/cc_resizefs.py
index 2edddd0..076b9d5 100644
--- a/cloudinit/config/cc_resizefs.py
+++ b/cloudinit/config/cc_resizefs.py
@@ -197,6 +197,13 @@ def maybe_get_writable_device_path(devpath, info, log):
     if devpath.startswith('gpt/'):
         log.debug('We have a gpt label - just go ahead')
         return devpath
+    # Alternatively, our device could simply be a name as returned by gpart,
+    # such as da0p3
+    if not devpath.startswith('/dev/') and not os.path.exists(devpath):
+        fulldevpath = '/dev/' + devpath.lstrip('/')
+        log.debug("'%s' doesn't appear to be a valid device path. Trying '%s'",
+                  devpath, fulldevpath)
+        devpath = fulldevpath
 
     try:
         statret = os.stat(devpath)
diff --git a/cloudinit/sources/DataSourceAzure.py b/cloudinit/sources/DataSourceAzure.py
index 9e8a1a8..2a3e567 100644
--- a/cloudinit/sources/DataSourceAzure.py
+++ b/cloudinit/sources/DataSourceAzure.py
@@ -526,6 +526,13 @@ class DataSourceAzure(sources.DataSource):
         report_ready = bool(not os.path.isfile(REPORTED_READY_MARKER_FILE))
         LOG.debug("Start polling IMDS")
 
+        def exc_cb(msg, exception):
+            if isinstance(exception, UrlError) and exception.code == 404:
+                return True
+            # If we get an exception while trying to call IMDS, we
+            # call DHCP and setup the ephemeral network to acquire the new IP.
+            return False
+
         while True:
             try:
                 # Save our EphemeralDHCPv4 context so we avoid repeated dhcp
@@ -540,7 +547,7 @@ class DataSourceAzure(sources.DataSource):
                     self._report_ready(lease=lease)
                     report_ready = False
                 return readurl(url, timeout=1, headers=headers,
-                               exception_cb=retry_on_url_exc, infinite=True,
+                               exception_cb=exc_cb, infinite=True,
                                log_req_resp=False).contents
             except UrlError:
                 # Teardown our EphemeralDHCPv4 context on failure as we retry
diff --git a/debian/changelog b/debian/changelog
index a85c8cc..f93ba9c 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -1,3 +1,12 @@
+cloud-init (18.4-24-g8f812a15-0ubuntu1) disco; urgency=medium
+
+  * New upstream snapshot.
+    - azure: _poll_imds only retry on 404. Fail on Timeout (LP: #1803598)
+    - resizefs: Prefix discovered devpath with '/dev/' when path does not
+      exist [Igor Galić]
+
+ -- Chad Smith <chad.smith@xxxxxxxxxxxxx>  Thu, 15 Nov 2018 16:11:20 -0700
+
 cloud-init (18.4-22-g6062595b-0ubuntu1) disco; urgency=medium
 
   * New upstream snapshot.
diff --git a/tests/unittests/test_datasource/test_azure.py b/tests/unittests/test_datasource/test_azure.py
index 56484b2..5ea7ae5 100644
--- a/tests/unittests/test_datasource/test_azure.py
+++ b/tests/unittests/test_datasource/test_azure.py
@@ -1687,22 +1687,44 @@ class TestPreprovisioningPollIMDS(CiTestCase):
         self.paths = helpers.Paths({'cloud_dir': self.tmp})
         dsaz.BUILTIN_DS_CONFIG['data_dir'] = self.waagent_d
 
-    @mock.patch(MOCKPATH + 'util.write_file')
-    def test_poll_imds_calls_report_ready(self, write_f, report_ready_func,
+    @mock.patch(MOCKPATH + 'EphemeralDHCPv4')
+    def test_poll_imds_re_dhcp_on_timeout(self, m_dhcpv4, report_ready_func,
                                           fake_resp, m_dhcp, m_net):
-        """The poll_imds will call report_ready after creating marker file."""
-        report_marker = self.tmp_path('report_marker', self.tmp)
+        """The poll_imds will retry DHCP on IMDS timeout."""
+        report_file = self.tmp_path('report_marker', self.tmp)
         lease = {
             'interface': 'eth9', 'fixed-address': '192.168.2.9',
             'routers': '192.168.2.1', 'subnet-mask': '255.255.255.0',
             'unknown-245': '624c3620'}
         m_dhcp.return_value = [lease]
+
+        dhcp_ctx = mock.MagicMock(lease=lease)
+        dhcp_ctx.obtain_lease.return_value = lease
+        m_dhcpv4.return_value = dhcp_ctx
+
+        self.tries = 0
+
+        def fake_timeout_once(**kwargs):
+            self.tries += 1
+            if self.tries == 1:
+                raise requests.Timeout('Fake connection timeout')
+            elif self.tries == 2:
+                response = requests.Response()
+                response.status_code = 404
+                raise requests.exceptions.HTTPError(
+                    "fake 404", response=response)
+            # Third try should succeed and stop retries or redhcp
+            return mock.MagicMock(status_code=200, text="good", content="good")
+
+        fake_resp.side_effect = fake_timeout_once
+
         dsa = dsaz.DataSourceAzure({}, distro=None, paths=self.paths)
-        mock_path = (MOCKPATH + 'REPORTED_READY_MARKER_FILE')
-        with mock.patch(mock_path, report_marker):
+        with mock.patch(MOCKPATH + 'REPORTED_READY_MARKER_FILE', report_file):
             dsa._poll_imds()
         self.assertEqual(report_ready_func.call_count, 1)
         report_ready_func.assert_called_with(lease=lease)
+        self.assertEqual(2, m_dhcpv4.call_count, 'Expected 2 DHCP calls')
+        self.assertEqual(3, self.tries, 'Expected 3 total reads from IMDS')
 
     def test_poll_imds_report_ready_false(self, report_ready_func,
                                           fake_resp, m_dhcp, m_net):
diff --git a/tests/unittests/test_handler/test_handler_resizefs.py b/tests/unittests/test_handler/test_handler_resizefs.py
index feca56c..6ebacb1 100644
--- a/tests/unittests/test_handler/test_handler_resizefs.py
+++ b/tests/unittests/test_handler/test_handler_resizefs.py
@@ -173,6 +173,38 @@ class TestResizefs(CiTestCase):
 
         self.assertEqual(('zpool', 'online', '-e', 'vmzroot', disk), ret)
 
+    @mock.patch('cloudinit.util.is_container', return_value=False)
+    @mock.patch('cloudinit.util.get_mount_info')
+    @mock.patch('cloudinit.util.get_device_info_from_zpool')
+    @mock.patch('cloudinit.util.parse_mount')
+    def test_handle_modern_zfsroot(self, mount_info, zpool_info, parse_mount,
+                                   is_container):
+        devpth = 'zroot/ROOT/default'
+        disk = 'da0p3'
+        fs_type = 'zfs'
+        mount_point = '/'
+
+        mount_info.return_value = (devpth, fs_type, mount_point)
+        zpool_info.return_value = disk
+        parse_mount.return_value = (devpth, fs_type, mount_point)
+
+        cfg = {'resize_rootfs': True}
+
+        def fake_stat(devpath):
+            if devpath == disk:
+                raise OSError("not here")
+            FakeStat = namedtuple(
+                'FakeStat', ['st_mode', 'st_size', 'st_mtime'])  # minimal stat
+            return FakeStat(25008, 0, 1)  # fake char block device
+
+        with mock.patch('cloudinit.config.cc_resizefs.do_resize') as dresize:
+            with mock.patch('cloudinit.config.cc_resizefs.os.stat') as m_stat:
+                m_stat.side_effect = fake_stat
+                handle('cc_resizefs', cfg, _cloud=None, log=LOG, args=[])
+
+        self.assertEqual(('zpool', 'online', '-e', 'zroot', '/dev/' + disk),
+                         dresize.call_args[0][0])
+
 
 class TestRootDevFromCmdline(CiTestCase):
 
@@ -246,39 +278,39 @@ class TestMaybeGetDevicePathAsWritableBlock(CiTestCase):
 
     def test_maybe_get_writable_device_path_does_not_exist(self):
         """When devpath does not exist, a warning is logged."""
-        info = 'dev=/I/dont/exist mnt_point=/ path=/dev/none'
+        info = 'dev=/dev/I/dont/exist mnt_point=/ path=/dev/none'
         devpath = wrap_and_call(
             'cloudinit.config.cc_resizefs.util',
             {'is_container': {'return_value': False}},
-            maybe_get_writable_device_path, '/I/dont/exist', info, LOG)
+            maybe_get_writable_device_path, '/dev/I/dont/exist', info, LOG)
         self.assertIsNone(devpath)
         self.assertIn(
-            "WARNING: Device '/I/dont/exist' did not exist."
+            "WARNING: Device '/dev/I/dont/exist' did not exist."
             ' cannot resize: %s' % info,
             self.logs.getvalue())
 
     def test_maybe_get_writable_device_path_does_not_exist_in_container(self):
         """When devpath does not exist in a container, log a debug message."""
-        info = 'dev=/I/dont/exist mnt_point=/ path=/dev/none'
+        info = 'dev=/dev/I/dont/exist mnt_point=/ path=/dev/none'
         devpath = wrap_and_call(
             'cloudinit.config.cc_resizefs.util',
             {'is_container': {'return_value': True}},
-            maybe_get_writable_device_path, '/I/dont/exist', info, LOG)
+            maybe_get_writable_device_path, '/dev/I/dont/exist', info, LOG)
         self.assertIsNone(devpath)
         self.assertIn(
-            "DEBUG: Device '/I/dont/exist' did not exist in container."
+            "DEBUG: Device '/dev/I/dont/exist' did not exist in container."
             ' cannot resize: %s' % info,
             self.logs.getvalue())
 
     def test_maybe_get_writable_device_path_raises_oserror(self):
         """When unexpected OSError is raises by os.stat it is reraised."""
-        info = 'dev=/I/dont/exist mnt_point=/ path=/dev/none'
+        info = 'dev=/dev/I/dont/exist mnt_point=/ path=/dev/none'
         with self.assertRaises(OSError) as context_manager:
             wrap_and_call(
                 'cloudinit.config.cc_resizefs',
                 {'util.is_container': {'return_value': True},
                  'os.stat': {'side_effect': OSError('Something unexpected')}},
-                maybe_get_writable_device_path, '/I/dont/exist', info, LOG)
+                maybe_get_writable_device_path, '/dev/I/dont/exist', info, LOG)
         self.assertEqual(
             'Something unexpected', str(context_manager.exception))