cloud-init-dev team mailing list archive

[Chad Smith <chad.smith@xxxxxxxxxxxxx>]

@smoser review comments addressed

Diff comments:

> diff --git a/cloudinit/config/cc_resizefs.py b/cloudinit/config/cc_resizefs.py
> index f774baa..eaa80bf 100644
> --- a/cloudinit/config/cc_resizefs.py
> +++ b/cloudinit/config/cc_resizefs.py
> @@ -242,8 +243,9 @@ def handle(name, cfg, _cloud, log, args):
>      info = "dev=%s mnt_point=%s path=%s" % (devpth, mount_point, resize_what)
>      log.debug("resize_info: %s" % info)
>  
> -    if not is_device_path_writable_block(devpth, info, log):
> -        return
> +    devpth = maybe_get_writable_device_path(devpth, info, log)
> +    if not devpth:
> +        return  # devpath was not a writabl block device

fixed.

>  
>      resizer = None
>      if can_skip_resize(fs_type, resize_what, devpth):
> diff --git a/requirements.txt b/requirements.txt
> index dd10d85..b44c742 100644
> --- a/requirements.txt
> +++ b/requirements.txt
> @@ -31,7 +31,7 @@ requests
>  jsonpatch
>  
>  # For validating cloud-config sections per schema definitions
> -jsonschema

oops, reverting.

> +#jsonschema
>  
>  # For Python 2/3 compatibility
>  six
> diff --git a/tests/unittests/test_handler/test_handler_resizefs.py b/tests/unittests/test_handler/test_handler_resizefs.py
> index 3e5d436..ff97426 100644
> --- a/tests/unittests/test_handler/test_handler_resizefs.py
> +++ b/tests/unittests/test_handler/test_handler_resizefs.py
> @@ -1,9 +1,10 @@
>  # This file is part of cloud-init. See LICENSE file for license information.
>  
>  from cloudinit.config.cc_resizefs import (
> -    can_skip_resize, handle, is_device_path_writable_block,
> +    can_skip_resize, handle, maybe_get_writable_device_path,
>      rootdev_from_cmdline)
>  
> +from collections import namedtuple

Yeah I read docs that said introduced in 2.6 to be sure. But, best to be certain.

>  import logging
>  import textwrap
>  
> @@ -234,41 +235,61 @@ class TestIsDevicePathWritableBlock(CiTestCase):
>                  'cloudinit.config.cc_resizefs',
>                  {'util.is_container': {'return_value': True},
>                   'os.stat': {'side_effect': OSError('Something unexpected')}},
> -                is_device_path_writable_block, '/I/dont/exist', info, LOG)
> +                maybe_get_writable_device_path, '/I/dont/exist', info, LOG)
>          self.assertEqual(
>              'Something unexpected', str(context_manager.exception))
>  
> -    def test_is_device_path_writable_block_non_block(self):
> +    def test_maybe_get_writable_device_path_non_block(self):
>          """When device is not a block device, emit warning return False."""
>          fake_devpath = self.tmp_path('dev/readwrite')
>          util.write_file(fake_devpath, '', mode=0o600)  # read-write
>          info = 'dev=/dev/root mnt_point=/ path={0}'.format(fake_devpath)
>  
> -        is_writable = wrap_and_call(
> +        devpath = wrap_and_call(
>              'cloudinit.config.cc_resizefs.util',
>              {'is_container': {'return_value': False}},
> -            is_device_path_writable_block, fake_devpath, info, LOG)
> -        self.assertFalse(is_writable)
> +            maybe_get_writable_device_path, fake_devpath, info, LOG)
> +        self.assertIsNone(devpath)
>          self.assertIn(
>              "WARNING: device '{0}' not a block device. cannot resize".format(
>                  fake_devpath),
>              self.logs.getvalue())
>  
> -    def test_is_device_path_writable_block_non_block_on_container(self):
> +    def test_maybe_get_writable_device_path_non_block_on_container(self):
>          """When device is non-block device in container, emit debug log."""
>          fake_devpath = self.tmp_path('dev/readwrite')
>          util.write_file(fake_devpath, '', mode=0o600)  # read-write
>          info = 'dev=/dev/root mnt_point=/ path={0}'.format(fake_devpath)
>  
> -        is_writable = wrap_and_call(
> +        devpath = wrap_and_call(
>              'cloudinit.config.cc_resizefs.util',
>              {'is_container': {'return_value': True}},
> -            is_device_path_writable_block, fake_devpath, info, LOG)
> -        self.assertFalse(is_writable)
> +            maybe_get_writable_device_path, fake_devpath, info, LOG)
> +        self.assertIsNone(devpath)
>          self.assertIn(
>              "DEBUG: device '{0}' not a block device in container."
>              ' cannot resize'.format(fake_devpath),
>              self.logs.getvalue())
>  
> +    def test_maybe_get_writable_device_path_returns_cmdline_root(self):
> +        """When root device is UUID in kernel commandline, update devpath."""
> +        FakeStat = namedtuple(

ok leaving this in for the branch and adding an XXX comment for future reference (as i know there are other tests which mock os.stat too.

> +            'FakeStat', ['st_mode', 'st_size', 'st_mtime'])  # minimal def.
> +        info = 'dev=/dev/root mnt_point=/ path=/does/not/matter'
> +        devpath = wrap_and_call(
> +            'cloudinit.config.cc_resizefs',
> +            {'util.get_cmdline': {'return_value': 'asdf root=UUID=my-uuid'},
> +             'util.is_container': False,
> +             'os.path.exists': False,  # /dev/root doesn't exist
> +             'os.stat': {
> +                 'return_value': FakeStat(25008, 0, 1)}  # char block device
> +             },
> +            maybe_get_writable_device_path, '/dev/root', info, LOG)
> +        self.assertEqual('/dev/disk/by-uuid/my-uuid', devpath)
> +        self.assertIn(
> +            "DEBUG: Converted /dev/root to '/dev/disk/by-uuid/my-uuid'"
> +            " per kernel cmdline",
> +            self.logs.getvalue())
> +
>  
>  # vi: ts=4 expandtab


-- 
https://code.launchpad.net/~chad.smith/cloud-init/+git/cloud-init/+merge/332634
Your team cloud-init commiters is requested to review the proposed merge of ~chad.smith/cloud-init:fix-device-path-from-cmdline-regression into cloud-init:master.