Code review comment for ~redriver/cloud-init:frbsd-azure-branch

Hongjiang Zhang (redriver) wrote :

> diff --git a/cloudinit/util.py b/cloudinit/util.py index
> 3301957..ef84e32 100644
> --- a/cloudinit/util.py
> +++ b/cloudinit/util.py
> @@ -2089,6 +2144,8 @@ def get_mount_info(path, log=LOG):
> #
> # So use /proc/$$/mountinfo to find the device underlying the
> # input path.
> + if is_FreeBSD_on_hyperv():

>but why "freebsd on hyperv"
>why are you not just checking "is this freebsd".
>what is specific to Azure here?

FreeBSD on Azure used a special label on disk. If we want to
get the /dev/daXXX from mount information, we need to handle
it specially.

> + return get_mount_info_freebsd_on_Azure(path, log)
> mountinfo_path = '/proc/%s/mountinfo' % os.getpid()
> if os.path.exists(mountinfo_path):
> lines = load_file(mountinfo_path).splitlines()
> diff --git a/tests/unittests/test_handler/test_handler_resizefs.py
> b/tests/unittests/test_handler/test_handler_resizefs.py
> new file mode 100644
> index 0000000..b5384e4
> --- /dev/null
> +++ b/tests/unittests/test_handler/test_handler_resizefs.py
> @@ -0,0 +1,73 @@
> +# This file is part of cloud-init. See LICENSE file for license information.
> +
> +from cloudinit.config import cc_resizefs
> +
> +import unittest
> +
> +try:
> + from unittest import mock
> +except ImportError:
> + import mock
> +
> +
> +class TestResizefs(unittest.TestCase):
> + def setUp(self):
> + super(TestResizefs, self).setUp()
> + self.name = "resizefs"
> +
> + @mock.patch('cloudinit.config.cc_resizefs._get_dumpfs_output')
> + @mock.patch('cloudinit.config.cc_resizefs._get_gpart_output')
> + def test_skip_ufs_resize(self, gpart_out, dumpfs_out):
> + fs_type = "ufs"
> + resize_what = "/"
> + devpth = "/dev/da0p2"
> + dumpfs_out.return_value = "# newfs command for / "\
> + "(/dev/label/rootfs)\n" \
> + "newfs -O 2 -U -a 4 -b "\
> + "32768 -d 32768 -e 4096 "\
> + "-f 4096 -g 16384 -h 64 "\
> + "-i 8192 -j -k 6408 -m 8 "\
> + "-o time -s 58719232 "\
> + "/dev/label/rootfs\n"
> + gpart_out.return_value = """
> +=> 40 62914480 da0 GPT (30G)
> + 40 1024 1 freebsd-boot (512K)
> + 1064 58719232 2 freebsd-ufs (28G)
> + 58720296 3145728 3 freebsd-swap (1.5G)
> + 61866024 1048496 - free - (512M)
> +"""
> + res = cc_resizefs.can_skip_resize(fs_type,
> + resize_what,
> + devpth)
> + self.assertTrue(res)
> +
> + @mock.patch('cloudinit.config.cc_resizefs._get_dumpfs_output')
> + @mock.patch('cloudinit.config.cc_resizefs._get_gpart_output')
> + def test_skip_ufs_resize_roundup(self, gpart_out, dumpfs_out):
> + fs_type = "ufs"
> + resize_what = "/"
> + devpth = "/dev/da0p2"
> + dumpfs_out.return_value = "# newfs command for / "\
> + "(/dev/label/rootfs)\n" \
> + "newfs -O 2 -U -a 4 -b "\
> + "32768 -d 32768 -e 4096 "\
> + "-f 4096 -g 16384 -h 64 "\
> + "-i 8192 -j -k 368 -m 8 "\
> + "-o time -s 297080 "\
> + "/dev/label/rootfs\n"
> + gpart_out.return_value = """
> +=> 34 297086 da0 GPT (145M)
> + 34 297086 1 freebsd-ufs (145M)
> +"""
> + res = cc_resizefs.can_skip_resize(fs_type,
> + resize_what,
> + devpth)
> + self.assertTrue(res)
> +
> +
> +class Bunch(object):

>this seems not necessary.

Ok.

> + def __init__(self, **kwds):
> + self.__dict__.update(kwds)
> +
> +
> +# vi: ts=4 expandtab
> diff --git a/tests/unittests/test_net.py b/tests/unittests/test_net.py
> index 4f07d80..bd22759 100644
> --- a/tests/unittests/test_net.py
> +++ b/tests/unittests/test_net.py
> @@ -1044,6 +1044,9 @@ class TestNetplanPostcommands(CiTestCase):
> @mock.patch.object(netplan, "get_devicelist")
> @mock.patch('cloudinit.util.subp')
> def test_netplan_postcmds(self, mock_subp, mock_devlist):
> + # FreeBSD does not have 'netplan' cmd

>this should not be necessary. netplan is never called (it is mocked).

Ok. Let me double check it.

> + if util.is_FreeBSD():
> + return
> mock_devlist.side_effect = [['lo']]
> tmp_dir = self.tmp_dir()
> ns = network_state.parse_net_config_data(self.mycfg,

--
https://na01.safelinks.protection.outlook.com/?url=https:%2F%2Fcode.launchpad.net%2F~redriver%2Fcloud-init%2F%2Bgit%2Fcloud-init%2F%2Bmerge%2F314895&data=02%7C01%7Chonzhan%40microsoft.com%7Cf2afe4f90815495139aa08d481d2ac90%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C636276189228313859&sdata=XdfR50OqWiiw%2BYeJLmyMfagkHlBWlNSBfQFPWwIHmp4%3D&reserved=0
You are the owner of ~redriver/cloud-init:frbsd-azure-branch.

« Back to merge proposal