Merge ~d-info-e/cloud-init:resizefs-on-zfs-root into cloud-init:master

Proposed by do3meli
Status: Merged
Approved by: Scott Moser
Approved revision: 798695437f98650cb264c03a9e931f2a3d18d469
Merge reported by: Scott Moser
Merged at revision: 20e3ddab7f55c2bf5e700c69fd24a0ac2206dbcf
Proposed branch: ~d-info-e/cloud-init:resizefs-on-zfs-root
Merge into: cloud-init:master
Diff against target: 345 lines (+214/-10)
7 files modified
cloudinit/config/ (+22/-0)
cloudinit/ (+35/-9)
tests/data/mount_parse_ext.txt (+19/-0)
tests/data/mount_parse_zfs.txt (+21/-0)
tests/data/zpool_status_simple.txt (+10/-0)
tests/unittests/test_handler/ (+57/-1)
tests/unittests/ (+50/-0)
Reviewer Review Type Date Requested Status
Scott Moser Approve
Server Team CI bot continuous-integration Approve
Review via email:

Commit message

FreeBSD: resizefs module now able to handle zfs/zpool.

Previously there was no support at all for zfs file system. With this
change it is now possible to use the resizefs module to grow a zpool to
its maximum partition size on FreeBSD.

LP: #1721243

Description of the change

this change adds the possibility to use resizefs module to extend a zfs file system on a corresponding zpool.

To post a comment you must log in.
Revision history for this message
Server Team CI bot (server-team-bot) wrote :

PASSED: Continuous integration, rev:
Executed test runs:
    SUCCESS: Checkout
    SUCCESS: Unit & Style Tests
    SUCCESS: Ubuntu LTS: Build
    SUCCESS: Ubuntu LTS: Integration
    SUCCESS: MAAS Compatability Testing
    IN_PROGRESS: Declarative: Post Actions

Click here to trigger a rebuild:

review: Approve (continuous-integration)
Revision history for this message
Scott Moser (smoser) wrote :

I'd like to see some unit tests added on this.

Definitely for the newly added path, but also for the old path.
I know that its painful to be asked to add unit tests for
code that you didnt' write, but the fact is I'm afraid that
merging these changes will break non-zfs without tests to show.

review: Needs Fixing
Revision history for this message
do3meli (d-info-e) wrote :

hi scott,
i have re factored the original merge request and added some tests. can you have a look at it again and trigger a jenkins build?
cheers dom

Revision history for this message
Server Team CI bot (server-team-bot) wrote :

FAILED: Continuous integration, rev:
Executed test runs:
    SUCCESS: Checkout
    FAILED: Unit & Style Tests

Click here to trigger a rebuild:

review: Needs Fixing (continuous-integration)
Revision history for this message
do3meli (d-info-e) wrote :

i have rebased my branch - please re-run

Revision history for this message
Server Team CI bot (server-team-bot) wrote :

PASSED: Continuous integration, rev:
Executed test runs:
    SUCCESS: Checkout
    SUCCESS: Unit & Style Tests
    SUCCESS: Ubuntu LTS: Build
    SUCCESS: Ubuntu LTS: Integration
    SUCCESS: MAAS Compatability Testing
    IN_PROGRESS: Declarative: Post Actions

Click here to trigger a rebuild:

review: Approve (continuous-integration)
Revision history for this message
do3meli (d-info-e) wrote :

any other comments? if not i will do some "manual" integration test against a FreeBSD to see if that really works or not.

Revision history for this message
Scott Moser (smoser) wrote :

lets change the commit message to indicate this is only adding
support FreeBSD zpool/zfs. As at best other distro are untested.

In curtin, we recently added zfs root support, so
we need to make sure that this doesnt raise error with zfs on zpool
at least on Ubuntu. while this is 'Experiemental', I dont want
cloud-init to show it failed on such a deployment.

^ has some files collected form a zfs root.
most useful likely is /proc/mounts (*view*/

I'm pretty sure it should not regress linux, as cc_resize_fs calls
'get_mount_info'. On linux that will take the 'parse_mount_info' path in,
and you have not modified that path here.

So I'm mostly OK with this as it seems confined to zfs on FreeBSD.

Revision history for this message
Scott Moser (smoser) :
review: Approve
Revision history for this message
do3meli (d-info-e) wrote :

the commit message has been adjusted by myself and Scott.

my "manual" cloud-init test on FreeBSD in my environment did NOT run good with this branch. i got the following log:

2018-03-26 15:41:16,159 -[WARNING]: Running module resizefs (<module 'cloudinit.config.cc_resizefs' from '/usr/local/lib/python2.7/site-packages/cloud_init-18.1-py2.7.egg/cloudinit/config/cc_resizefs.pyc'>) failed

will investigate this further tomorrow.

i am still trying to think through the curtin stuff you mentioned. will get back on this tomorrow.

7986954... by do3meli

parse_mount() now using correct if/else for FreeBSD with zpools

Revision history for this message
do3meli (d-info-e) wrote :

the last commit fixes the problem mentioned above in my environment. i haven't run into this problem with unit tests as i mocked the return value and did not follow the logic in the code. another zfs exclusion in the if/else construct fixed it now.

regarding the curtin stuff. i see it the same way as you:
the cc_resizefs module calls 'get_mount_info' which then on linux will call the 'parse_mount_info' function. as i haven't adjusted the get_mount_info nor the parse_mount_info functions it will at least not break any existing system. on the other hand as you mentioned as well it is not guaranteed to be working for zfs/zpool on ubuntu for example.

so i am good to go ahead with this now. please re-trigger a CI run.

Revision history for this message
Server Team CI bot (server-team-bot) wrote :

PASSED: Continuous integration, rev:
Executed test runs:
    SUCCESS: Checkout
    SUCCESS: Unit & Style Tests
    SUCCESS: Ubuntu LTS: Build
    SUCCESS: Ubuntu LTS: Integration
    SUCCESS: MAAS Compatability Testing
    IN_PROGRESS: Declarative: Post Actions

Click here to trigger a rebuild:

review: Approve (continuous-integration)
Revision history for this message
Scott Moser (smoser) wrote :

Thank you for your merge proposal.

Your branch has been set to 'Work in progress'.
Please set the branch back to 'Needs Review' after resolving the issues below.

Thanks again,
Your friendly neighborhood cloud-init robot.

Please fix the following issues:
Commit message lints:
 - Line #2 has 106 too many characters. Line starts with: "Previously there was"...

For more information, see commit message guidelines at

review: Needs Fixing
Revision history for this message
Scott Moser (smoser) :
review: Approve
Revision history for this message
Scott Moser (smoser) wrote :

An upstream commit landed for this bug.

To view that commit see the following URL:

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1diff --git a/cloudinit/config/ b/cloudinit/config/
2index cec22bb..c8e1752 100644
3--- a/cloudinit/config/
4+++ b/cloudinit/config/
5@@ -84,6 +84,10 @@ def _resize_ufs(mount_point, devpth):
6 return ('growfs', devpth)
9+def _resize_zfs(mount_point, devpth):
10+ return ('zpool', 'online', '-e', mount_point, devpth)
13 def _get_dumpfs_output(mount_point):
14 dumpfs_res, err = util.subp(['dumpfs', '-m', mount_point])
15 return dumpfs_res
16@@ -148,6 +152,7 @@ RESIZE_FS_PREFIXES_CMDS = [
17 ('ext', _resize_ext),
18 ('xfs', _resize_xfs),
19 ('ufs', _resize_ufs),
20+ ('zfs', _resize_zfs),
21 ]
24@@ -188,6 +193,13 @@ def maybe_get_writable_device_path(devpath, info, log):
25 log.debug("Not attempting to resize devpath '%s': %s", devpath, info)
26 return None
28+ # FreeBSD zpool can also just use gpt/<label>
29+ # with that in mind we can not do an os.stat on "gpt/whatever"
30+ # therefore return the devpath already here.
31+ if devpath.startswith('gpt/'):
32+ log.debug('We have a gpt label - just go ahead')
33+ return devpath
35 try:
36 statret = os.stat(devpath)
37 except OSError as exc:
38@@ -231,6 +243,16 @@ def handle(name, cfg, _cloud, log, args):
40 (devpth, fs_type, mount_point) = result
42+ # if we have a zfs then our device path at this point
43+ # is the zfs label. For example: vmzroot/ROOT/freebsd
44+ # we will have to get the zpool name out of this
45+ # and set the resize_what variable to the zpool
46+ # so the _resize_zfs function gets the right attribute.
47+ if fs_type == 'zfs':
48+ zpool = devpth.split('/')[0]
49+ devpth = util.get_device_info_from_zpool(zpool)
50+ resize_what = zpool
52 info = "dev=%s mnt_point=%s path=%s" % (devpth, mount_point, resize_what)
53 log.debug("resize_info: %s" % info)
55diff --git a/cloudinit/ b/cloudinit/
56index fb4ee5f..0ab2c48 100644
57--- a/cloudinit/
58+++ b/cloudinit/
59@@ -2234,7 +2234,7 @@ def get_path_dev_freebsd(path, mnt_list):
60 return path_found
63-def get_mount_info_freebsd(path, log=LOG):
64+def get_mount_info_freebsd(path):
65 (result, err) = subp(['mount', '-p', path], rcs=[0, 1])
66 if len(err):
67 # find a path if the input is not a mounting point
68@@ -2248,23 +2248,49 @@ def get_mount_info_freebsd(path, log=LOG):
69 return "/dev/" + label_part, ret[2], ret[1]
72+def get_device_info_from_zpool(zpool):
73+ (zpoolstatus, err) = subp(['zpool', 'status', zpool])
74+ if len(err):
75+ return None
76+ r = r'.*(ONLINE).*'
77+ for line in zpoolstatus.split("\n"):
78+ if, line) and zpool not in line and "state" not in line:
79+ disk = line.split()[0]
80+ LOG.debug('found zpool "%s" on disk %s', zpool, disk)
81+ return disk
84 def parse_mount(path):
85- (mountoutput, _err) = subp("mount")
86+ (mountoutput, _err) = subp(['mount'])
87 mount_locs = mountoutput.splitlines()
88+ # there are 2 types of mount outputs we have to parse therefore
89+ # the regex is a bit complex. to better understand this regex see:
90+ #
91+ #
92+ regex = r'^(/dev/[\S]+|.*zroot\S*?) on (/[\S]*) ' + \
93+ '(?=(?:type)[\s]+([\S]+)|\(([^,]*))'
94 for line in mount_locs:
95- m ='^(/dev/[\S]+) on (/.*) \((.+), .+, (.+)\)$', line)
96+ m =, line)
97 if not m:
98 continue
99+ devpth =
100+ mount_point =
101+ # above regex will either fill the fs_type in group(3)
102+ # or group(4) depending on the format we have.
103+ fs_type =
104+ if fs_type is None:
105+ fs_type =
106+ LOG.debug('found line in mount -> devpth: %s, mount_point: %s, '
107+ 'fs_type: %s', devpth, mount_point, fs_type)
108 # check whether the dev refers to a label on FreeBSD
109 # for example, if dev is '/dev/label/rootfs', we should
110 # continue finding the real device like '/dev/da0'.
111- devm ='^(/dev/.+)p([0-9])$',
112- if (not devm and is_FreeBSD()):
113+ # this is only valid for non zfs file systems as a zpool
114+ # can have gpt labels as disk.
115+ devm ='^(/dev/.+)p([0-9])$', devpth)
116+ if not devm and is_FreeBSD() and fs_type != 'zfs':
117 return get_mount_info_freebsd(path)
118- devpth =
119- mount_point =
120- fs_type =
121- if mount_point == path:
122+ elif mount_point == path:
123 return devpth, fs_type, mount_point
124 return None
126diff --git a/tests/data/mount_parse_ext.txt b/tests/data/mount_parse_ext.txt
127new file mode 100644
128index 0000000..da0c870
129--- /dev/null
130+++ b/tests/data/mount_parse_ext.txt
131@@ -0,0 +1,19 @@
132+/dev/mapper/vg00-lv_root on / type ext4 (rw,errors=remount-ro)
133+proc on /proc type proc (rw,noexec,nosuid,nodev)
134+sysfs on /sys type sysfs (rw,noexec,nosuid,nodev)
135+none on /sys/fs/cgroup type tmpfs (rw)
136+none on /sys/fs/fuse/connections type fusectl (rw)
137+none on /sys/kernel/debug type debugfs (rw)
138+none on /sys/kernel/security type securityfs (rw)
139+udev on /dev type devtmpfs (rw,mode=0755)
140+devpts on /dev/pts type devpts (rw,noexec,nosuid,gid=5,mode=0620)
141+none on /tmp type tmpfs (rw)
142+tmpfs on /run type tmpfs (rw,noexec,nosuid,size=10%,mode=0755)
143+none on /run/lock type tmpfs (rw,noexec,nosuid,nodev,size=5242880)
144+none on /run/shm type tmpfs (rw,nosuid,nodev)
145+none on /run/user type tmpfs (rw,noexec,nosuid,nodev,size=104857600,mode=0755)
146+none on /sys/fs/pstore type pstore (rw)
147+/dev/mapper/vg00-lv_var on /var type ext4 (rw)
148+rpc_pipefs on /run/rpc_pipefs type rpc_pipefs (rw)
149+systemd on /sys/fs/cgroup/systemd type cgroup (rw,noexec,nosuid,nodev,none,name=systemd)
150+ on /backup type nfs (rw,noexec,nosuid,nodev,bg,nolock,tcp,nfsvers=3,hard,addr=
151\ No newline at end of file
152diff --git a/tests/data/mount_parse_zfs.txt b/tests/data/mount_parse_zfs.txt
153new file mode 100644
154index 0000000..08af04f
155--- /dev/null
156+++ b/tests/data/mount_parse_zfs.txt
157@@ -0,0 +1,21 @@
158+vmzroot/ROOT/freebsd on / (zfs, local, nfsv4acls)
159+devfs on /dev (devfs, local, multilabel)
160+fdescfs on /dev/fd (fdescfs)
161+vmzroot/root on /root (zfs, local, nfsv4acls)
162+vmzroot/tmp on /tmp (zfs, local, nosuid, nfsv4acls)
163+vmzroot/ROOT/freebsd/usr on /usr (zfs, local, nfsv4acls)
164+vmzroot/ROOT/freebsd/usr/local on /usr/local (zfs, local, nfsv4acls)
165+vmzroot/ROOT/freebsd/var on /var (zfs, local, nfsv4acls)
166+vmzroot/ROOT/freebsd/var/cache on /var/cache (zfs, local, noexec, nosuid, nfsv4acls)
167+vmzroot/ROOT/freebsd/var/crash on /var/crash (zfs, local, noexec, nosuid, nfsv4acls)
168+vmzroot/var/cron on /var/cron (zfs, local, nosuid, nfsv4acls)
169+vmzroot/ROOT/freebsd/var/db on /var/db (zfs, local, noatime, noexec, nosuid, nfsv4acls)
170+vmzroot/ROOT/freebsd/var/empty on /var/empty (zfs, local, noexec, nosuid, read-only, nfsv4acls)
171+vmzroot/var/log on /var/log (zfs, local, noexec, nosuid, nfsv4acls)
172+vmzroot/var/log/pf on /var/log/pf (zfs, local, noexec, nosuid, nfsv4acls)
173+vmzroot/var/mail on /var/mail (zfs, local, noexec, nosuid, nfsv4acls)
174+vmzroot/ROOT/freebsd/var/run on /var/run (zfs, local, noexec, nosuid, nfsv4acls)
175+vmzroot/var/spool on /var/spool (zfs, local, noexec, nosuid, nfsv4acls)
176+vmzroot/var/tmp on /var/tmp (zfs, local, nosuid, nfsv4acls)
177+ on /mnt/test (nfs, read-only)
178+ on /mnt/test2 (nfs, nosuid)
179\ No newline at end of file
180diff --git a/tests/data/zpool_status_simple.txt b/tests/data/zpool_status_simple.txt
181new file mode 100644
182index 0000000..a2c573a
183--- /dev/null
184+++ b/tests/data/zpool_status_simple.txt
185@@ -0,0 +1,10 @@
186+ pool: vmzroot
187+ state: ONLINE
188+ scan: none requested
192+ vmzroot ONLINE 0 0 0
193+ gpt/system ONLINE 0 0 0
195+errors: No known data errors
196\ No newline at end of file
197diff --git a/tests/unittests/test_handler/ b/tests/unittests/test_handler/
198index c2a7f9f..7a7ba1f 100644
199--- a/tests/unittests/test_handler/
200+++ b/tests/unittests/test_handler/
201@@ -1,7 +1,8 @@
202 # This file is part of cloud-init. See LICENSE file for license information.
204 from cloudinit.config.cc_resizefs import (
205- can_skip_resize, handle, maybe_get_writable_device_path, _resize_btrfs)
206+ can_skip_resize, handle, maybe_get_writable_device_path, _resize_btrfs,
207+ _resize_zfs, _resize_xfs, _resize_ext, _resize_ufs)
209 from collections import namedtuple
210 import logging
211@@ -60,6 +61,9 @@ class TestResizefs(CiTestCase):
212 res = can_skip_resize(fs_type, resize_what, devpth)
213 self.assertTrue(res)
215+ def test_can_skip_resize_ext(self):
216+ self.assertFalse(can_skip_resize('ext', '/', '/dev/sda1'))
218 def test_handle_noops_on_disabled(self):
219 """The handle function logs when the configuration disables resize."""
220 cfg = {'resize_rootfs': False}
221@@ -122,6 +126,51 @@ class TestResizefs(CiTestCase):
222 logs = self.logs.getvalue()
223 self.assertIn("WARNING: Unable to find device '/dev/root'", logs)
225+ def test_resize_zfs_cmd_return(self):
226+ zpool = 'zroot'
227+ devpth = 'gpt/system'
228+ self.assertEqual(('zpool', 'online', '-e', zpool, devpth),
229+ _resize_zfs(zpool, devpth))
231+ def test_resize_xfs_cmd_return(self):
232+ mount_point = '/mnt/test'
233+ devpth = '/dev/sda1'
234+ self.assertEqual(('xfs_growfs', mount_point),
235+ _resize_xfs(mount_point, devpth))
237+ def test_resize_ext_cmd_return(self):
238+ mount_point = '/'
239+ devpth = '/dev/sdb1'
240+ self.assertEqual(('resize2fs', devpth),
241+ _resize_ext(mount_point, devpth))
243+ def test_resize_ufs_cmd_return(self):
244+ mount_point = '/'
245+ devpth = '/dev/sda2'
246+ self.assertEqual(('growfs', devpth),
247+ _resize_ufs(mount_point, devpth))
249+ @mock.patch('cloudinit.util.get_mount_info')
250+ @mock.patch('cloudinit.util.get_device_info_from_zpool')
251+ @mock.patch('cloudinit.util.parse_mount')
252+ def test_handle_zfs_root(self, mount_info, zpool_info, parse_mount):
253+ devpth = 'vmzroot/ROOT/freebsd'
254+ disk = 'gpt/system'
255+ fs_type = 'zfs'
256+ mount_point = '/'
258+ mount_info.return_value = (devpth, fs_type, mount_point)
259+ zpool_info.return_value = disk
260+ parse_mount.return_value = (devpth, fs_type, mount_point)
262+ cfg = {'resize_rootfs': True}
264+ with mock.patch('cloudinit.config.cc_resizefs.do_resize') as dresize:
265+ handle('cc_resizefs', cfg, _cloud=None, log=LOG, args=[])
266+ ret = dresize.call_args[0][0]
268+ self.assertEqual(('zpool', 'online', '-e', 'vmzroot', disk), ret)
271 class TestRootDevFromCmdline(CiTestCase):
273@@ -305,5 +354,12 @@ class TestMaybeGetDevicePathAsWritableBlock(CiTestCase):
274 ('btrfs', 'filesystem', 'resize', 'max', '/'),
275 _resize_btrfs("/", "/dev/sda1"))
277+ @mock.patch('cloudinit.util.is_FreeBSD')
278+ def test_maybe_get_writable_device_path_zfs_freebsd(self, freebsd):
279+ freebsd.return_value = True
280+ info = 'dev=gpt/system mnt_point=/ path=/'
281+ devpth = maybe_get_writable_device_path('gpt/system', info, LOG)
282+ self.assertEqual('gpt/system', devpth)
285 # vi: ts=4 expandtab
286diff --git a/tests/unittests/ b/tests/unittests/
287index 67d9607..8685b8e 100644
288--- a/tests/unittests/
289+++ b/tests/unittests/
290@@ -366,6 +366,56 @@ class TestMountinfoParsing(helpers.ResourceUsingTestCase):
291 expected = ('none', 'tmpfs', '/run/lock')
292 self.assertEqual(expected, util.parse_mount_info('/run/lock', lines))
294+ @mock.patch('cloudinit.util.subp')
295+ def test_get_device_info_from_zpool(self, zpool_output):
296+ # mock subp command from util.get_mount_info_fs_on_zpool
297+ zpool_output.return_value = (
298+ self.readResource('zpool_status_simple.txt'), ''
299+ )
300+ # save function return values and do asserts
301+ ret = util.get_device_info_from_zpool('vmzroot')
302+ self.assertEqual('gpt/system', ret)
303+ self.assertIsNotNone(ret)
305+ @mock.patch('cloudinit.util.subp')
306+ def test_get_device_info_from_zpool_on_error(self, zpool_output):
307+ # mock subp command from util.get_mount_info_fs_on_zpool
308+ zpool_output.return_value = (
309+ self.readResource('zpool_status_simple.txt'), 'error'
310+ )
311+ # save function return values and do asserts
312+ ret = util.get_device_info_from_zpool('vmzroot')
313+ self.assertIsNone(ret)
315+ @mock.patch('cloudinit.util.subp')
316+ def test_parse_mount_with_ext(self, mount_out):
317+ mount_out.return_value = (self.readResource('mount_parse_ext.txt'), '')
318+ # this one is valid and exists in mount_parse_ext.txt
319+ ret = util.parse_mount('/var')
320+ self.assertEqual(('/dev/mapper/vg00-lv_var', 'ext4', '/var'), ret)
321+ # another one that is valid and exists
322+ ret = util.parse_mount('/')
323+ self.assertEqual(('/dev/mapper/vg00-lv_root', 'ext4', '/'), ret)
324+ # this one exists in mount_parse_ext.txt
325+ ret = util.parse_mount('/sys/kernel/debug')
326+ self.assertIsNone(ret)
327+ # this one does not even exist in mount_parse_ext.txt
328+ ret = util.parse_mount('/not/existing/mount')
329+ self.assertIsNone(ret)
331+ @mock.patch('cloudinit.util.subp')
332+ def test_parse_mount_with_zfs(self, mount_out):
333+ mount_out.return_value = (self.readResource('mount_parse_zfs.txt'), '')
334+ # this one is valid and exists in mount_parse_zfs.txt
335+ ret = util.parse_mount('/var')
336+ self.assertEqual(('vmzroot/ROOT/freebsd/var', 'zfs', '/var'), ret)
337+ # this one is the root, valid and also exists in mount_parse_zfs.txt
338+ ret = util.parse_mount('/')
339+ self.assertEqual(('vmzroot/ROOT/freebsd', 'zfs', '/'), ret)
340+ # this one does not even exist in mount_parse_ext.txt
341+ ret = util.parse_mount('/not/existing/mount')
342+ self.assertIsNone(ret)
345 class TestReadDMIData(helpers.FilesystemMockingTestCase):


People subscribed via source and target branches