Merge ~minagalic/cloud-init:fix/fbsd-resizefs into cloud-init:master

[Mina Galić]

resizefs: Prefix discovered devpath with '/dev/' when path does not exist

In some environments, like FreeBSD, gpart can return the device basename
instead of the full path. If this discovered devpath does not exist and
is missing the '/dev/' prefix, add that prefix in an attempt to find the
device.

If the device name doesn't start with `/dev/` check there for its existence!

[Scott Moser (smoser)]

Hi,
Thank you for contributing to cloud-init.

To contribute, you must sign the Canonical Contributor License Agreement (CLA) [1].

If you have already signed it as an individual, your Launchpad user will be listed in the contributor-agreement-canonical launchpad group [2]. Unfortunately there is no easy way to check if an organization or company you are doing work for has signed. If you are unsure or have questions, email <email address hidden> or ping smoser in #cloud-init channel via freenode.

For information on how to sign, please see the HACKING document [3].

Thanks again, and please feel free to reach out with any questions.

–
[1] http://www.canonical.com/contributors
[2] https://launchpad.net/~contributor-agreement-canonical/+members
[3] http://cloudinit.readthedocs.io/en/latest/topics/hacking.html

[Mina Galić (minagalic)]

> Hi,
> Thank you for contributing to cloud-init.
>
> To contribute, you must sign the Canonical Contributor License Agreement (CLA)

done… i think!

[Mina Galić (minagalic)]

> > Hi,
> > Thank you for contributing to cloud-init.
> >
> > To contribute, you must sign the Canonical Contributor License Agreement
> (CLA)
>
> done… i think!

https://launchpad.net/%7Econtributor-agreement-canonical/+members?active_batch=75&active_memo=300&active_start=300

indeed!

[Chad Smith (chad.smith)]

Thank you Igor,

To me it feels like this should be a fallback option only if the provided devpath doesn't exist.

I've added a comment inline and set this to "Work in progress" to let you know it is off our radar until you have a chance to respond to the review comments and would like us to re-review this branch.

Please set this branch top-level 'status' back to 'Needs review' when you have had a chance to respond to the comments or fixes.

thanks again!

review: Needs Fixing

[Mina Galić (minagalic)]

Thanks for the review Chad. I've now fixed up the code as per your suggestions, but the test i added is failing for reasons i cannot quite explain yet.

review: Needs Resubmitting

[Chad Smith (chad.smith)]

thanks for adding the unit test. I added a patch on top to fixup the unit test and account for my inline comments.
hopefully that works and passes tox for us.

[Server Team CI bot (server-team-bot)]

FAILED: Continuous integration, rev:0acd2f9898ac8bc5fb58a9bafbfe36b272abfb99
https://jenkins.ubuntu.com/server/job/cloud-init-ci/442/
Executed test runs:
    SUCCESS: Checkout
    FAILED: Unit & Style Tests

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/server/job/cloud-init-ci/442/rebuild

review: Needs Fixing (continuous-integration)

7a42186... by =?utf-8?q?Igor_Gali=C4=87?= <email address hidden> on 2018-11-14

Fix style

[Server Team CI bot (server-team-bot)]

PASSED: Continuous integration, rev:7a42186003638aa69821ca6e9b30f736aee1632f
https://jenkins.ubuntu.com/server/job/cloud-init-ci/443/
Executed test runs:
    SUCCESS: Checkout
    SUCCESS: Unit & Style Tests
    SUCCESS: Ubuntu LTS: Build
    SUCCESS: Ubuntu LTS: Integration
    IN_PROGRESS: Declarative: Post Actions

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/server/job/cloud-init-ci/443/rebuild

review: Approve (continuous-integration)

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/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))