diff options
author | Mina Galić <me+git@igalic.co> | 2020-11-19 23:19:16 +0100 |
---|---|---|
committer | GitHub <noreply@github.com> | 2020-11-19 17:19:16 -0500 |
commit | 12ef7541c2d0c6b2cd510f95fda53ca9c8333064 (patch) | |
tree | 999c8ada4b030b555f5087715a9c5380361efc2a | |
parent | 73e704e3690611625e3cda060a7a6a81492af9d2 (diff) | |
download | vyos-cloud-init-12ef7541c2d0c6b2cd510f95fda53ca9c8333064.tar.gz vyos-cloud-init-12ef7541c2d0c6b2cd510f95fda53ca9c8333064.zip |
cc_resizefs on FreeBSD: Fix _can_skip_ufs_resize (#655)
On FreeBSD, if a UFS has trim: (-t) or MAC multilabel: (-l) flag, resize
FS fail, because the _can_skip_ufs_resize check gets tripped up by the
missing options.
This was reported at FreeBSD Bugzilla:
https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=250496 and as
LP: #1901958
Rather than fixing the parser as in the patches proposed there (and
attempted in #636) this pull-request rips out all of it, and simplifies
the code. We now use `growfs -N` and check if that returns an error. If
it returns the correct kind of error, we can skip the resize, because we
either are at the correct size, or the filesystem in question is broken
or not UFS. If it returns the wrong kind of error, we just re-raise it.
LP: #1901958
-rw-r--r-- | cloudinit/config/cc_resizefs.py | 68 | ||||
-rw-r--r-- | tests/unittests/test_handler/test_handler_resizefs.py | 55 |
2 files changed, 42 insertions, 81 deletions
diff --git a/cloudinit/config/cc_resizefs.py b/cloudinit/config/cc_resizefs.py index 978d2ee0..9afbb847 100644 --- a/cloudinit/config/cc_resizefs.py +++ b/cloudinit/config/cc_resizefs.py @@ -9,10 +9,7 @@ """Resizefs: cloud-config module which resizes the filesystem""" import errno -import getopt import os -import re -import shlex import stat from textwrap import dedent @@ -88,56 +85,23 @@ def _resize_zfs(mount_point, devpth): return ('zpool', 'online', '-e', mount_point, devpth) -def _get_dumpfs_output(mount_point): - return subp.subp(['dumpfs', '-m', mount_point])[0] - - -def _get_gpart_output(part): - return subp.subp(['gpart', 'show', part])[0] - - def _can_skip_resize_ufs(mount_point, devpth): - # extract the current fs sector size - """ - # dumpfs -m / - # newfs command for / (/dev/label/rootfs) - newfs -L rootf -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/rootf - """ - cur_fs_sz = None - frag_sz = None - dumpfs_res = _get_dumpfs_output(mount_point) - for line in dumpfs_res.splitlines(): - if not line.startswith('#'): - newfs_cmd = shlex.split(line) - opt_value = 'O:Ua:s:b:d:e:f:g:h:i:jk:m:o:L:' - optlist, _args = getopt.getopt(newfs_cmd[1:], opt_value) - for o, a in optlist: - if o == "-s": - cur_fs_sz = int(a) - if o == "-f": - frag_sz = int(a) - # check the current partition size - # Example output from `gpart show /dev/da0`: - # => 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) - expect_sz = None - m = re.search('^(/dev/.+)p([0-9])$', devpth) - gpart_res = _get_gpart_output(m.group(1)) - for line in gpart_res.splitlines(): - if re.search(r"freebsd-ufs", line): - fields = line.split() - expect_sz = int(fields[1]) - # Normalize the gpart sector size, - # because the size is not exactly the same as fs size. - normal_expect_sz = (expect_sz - expect_sz % (frag_sz / 512)) - if normal_expect_sz == cur_fs_sz: - return True - else: - return False + # possible errors cases on the code-path to growfs -N following: + # https://github.com/freebsd/freebsd/blob/HEAD/sbin/growfs/growfs.c + # This is the "good" error: + skip_start = "growfs: requested size" + skip_contain = "is not larger than the current filesystem size" + # growfs exits with 1 for almost all cases up to this one. + # This means we can't just use rcs=[0, 1] as subp parameter: + try: + subp.subp(['growfs', '-N', devpth]) + except subp.ProcessExecutionError as e: + if e.stderr.startswith(skip_start) and skip_contain in e.stderr: + # This FS is already at the desired size + return True + else: + raise e + return False # Do not use a dictionary as these commands should be able to be used diff --git a/tests/unittests/test_handler/test_handler_resizefs.py b/tests/unittests/test_handler/test_handler_resizefs.py index db9a0414..28d55072 100644 --- a/tests/unittests/test_handler/test_handler_resizefs.py +++ b/tests/unittests/test_handler/test_handler_resizefs.py @@ -6,8 +6,8 @@ from cloudinit.config.cc_resizefs import ( from collections import namedtuple import logging -import textwrap +from cloudinit.subp import ProcessExecutionError from cloudinit.tests.helpers import ( CiTestCase, mock, skipUnlessJsonSchema, util, wrap_and_call) @@ -22,44 +22,41 @@ class TestResizefs(CiTestCase): 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): + @mock.patch('cloudinit.subp.subp') + def test_skip_ufs_resize(self, m_subp): 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 = textwrap.dedent("""\ - => 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) - """) + err = ("growfs: requested size 2.0GB is not larger than the " + "current filesystem size 2.0GB\n") + exception = ProcessExecutionError(stderr=err, exit_code=1) + m_subp.side_effect = exception res = 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): + @mock.patch('cloudinit.subp.subp') + def test_cannot_skip_ufs_resize(self, m_subp): 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 = textwrap.dedent("""\ - => 34 297086 da0 GPT (145M) - 34 297086 1 freebsd-ufs (145M) - """) + m_subp.return_value = ( + ("stdout: super-block backups (for fsck_ffs -b #) at:\n\n"), + ("growfs: no room to allocate last cylinder group; " + "leaving 364KB unused\n") + ) res = can_skip_resize(fs_type, resize_what, devpth) - self.assertTrue(res) + self.assertFalse(res) + + @mock.patch('cloudinit.subp.subp') + def test_cannot_skip_ufs_growfs_exception(self, m_subp): + fs_type = "ufs" + resize_what = "/" + devpth = "/dev/da0p2" + err = "growfs: /dev/da0p2 is not clean - run fsck.\n" + exception = ProcessExecutionError(stderr=err, exit_code=1) + m_subp.side_effect = exception + with self.assertRaises(ProcessExecutionError): + can_skip_resize(fs_type, resize_what, devpth) def test_can_skip_resize_ext(self): self.assertFalse(can_skip_resize('ext', '/', '/dev/sda1')) |