summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorMina Galić <me+git@igalic.co>2020-11-19 23:19:16 +0100
committerGitHub <noreply@github.com>2020-11-19 17:19:16 -0500
commit12ef7541c2d0c6b2cd510f95fda53ca9c8333064 (patch)
tree999c8ada4b030b555f5087715a9c5380361efc2a
parent73e704e3690611625e3cda060a7a6a81492af9d2 (diff)
downloadvyos-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.py68
-rw-r--r--tests/unittests/test_handler/test_handler_resizefs.py55
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'))