Skip to content

Commit

Permalink
growpart: Fix behaviour for ZFS datasets (#5169)
Browse files Browse the repository at this point in the history 
on FreeBSD, ZFS datasets aren't exposed directly as disks devices, so
stats will fail. Currently, this means that growpart will be skipped for
FreeBSD ZFS volumes, even tho we are perfectly capable of  resizing ZFS.

- Return fs from devent2dev(), which is now inappropriately named
- Add get_zfs_size() helper, and extend get_size() to accept an `fs`
  parameter.
- Add _call_resizer() helper, to avoid an indent on `if fs == zfs`
- Add a unittest to show the behaviour

Sponsored by:	The FreeBSD Foundation
  • Loading branch information
@igalic @holmanb
igalic authored and holmanb committed Jun 28, 2024
1 parent 272bbbb commit 8ec2edd
Show file tree
Hide file tree
Showing 2 changed files with 161 additions and 66 deletions.
146 changes: 88 additions & 58 deletions cloudinit/config/cc_growpart.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,7 @@ def available(self, devices: list) -> bool:
...

@abstractmethod
def resize(self, diskdev, partnum, partdev):
def resize(self, diskdev, partnum, partdev, fs):
...


Expand All @@ -178,8 +178,8 @@ def available(self, devices: list):
pass
return False

def resize(self, diskdev, partnum, partdev):
before = get_size(partdev)
def resize(self, diskdev, partnum, partdev, fs):
before = get_size(partdev, fs)

# growpart uses tmp dir to store intermediate states
# and may conflict with systemd-tmpfiles-clean
Expand Down Expand Up @@ -211,7 +211,7 @@ def resize(self, diskdev, partnum, partdev):
util.logexc(LOG, "Failed: growpart %s %s", diskdev, partnum)
raise ResizeFailedException(e) from e

return (before, get_size(partdev))
return (before, get_size(partdev, fs))


class ResizeGrowFS(Resizer):
Expand All @@ -231,15 +231,15 @@ def available(self, devices: list):
"""growfs only works on the root partition"""
return os.path.isfile("/etc/rc.d/growfs") and devices == ["/"]

def resize(self, diskdev, partnum, partdev):
before = get_size(partdev)
def resize(self, diskdev, partnum, partdev, fs):
before = get_size(partdev, fs)
try:
self._distro.manage_service(action="onestart", service="growfs")
except subp.ProcessExecutionError as e:
util.logexc(LOG, "Failed: service growfs onestart")
raise ResizeFailedException(e) from e

return (before, get_size(partdev))
return (before, get_size(partdev, fs))


class ResizeGpart(Resizer):
Expand All @@ -255,7 +255,7 @@ def available(self, devices: list):
pass
return False

def resize(self, diskdev, partnum, partdev):
def resize(self, diskdev, partnum, partdev, fs):
"""
GPT disks store metadata at the beginning (primary) and at the
end (secondary) of the disk. When launching an image with a
Expand All @@ -270,36 +270,49 @@ def resize(self, diskdev, partnum, partdev):
util.logexc(LOG, "Failed: gpart recover %s", diskdev)
raise ResizeFailedException(e) from e

before = get_size(partdev)
before = get_size(partdev, fs)
try:
subp.subp(["gpart", "resize", "-i", partnum, diskdev])
except subp.ProcessExecutionError as e:
util.logexc(LOG, "Failed: gpart resize -i %s %s", partnum, diskdev)
raise ResizeFailedException(e) from e

return (before, get_size(partdev))
return (before, get_size(partdev, fs))


def get_size(filename) -> Optional[int]:
def get_size(filename, fs) -> Optional[int]:
fd = None
try:
fd = os.open(filename, os.O_RDONLY)
return os.lseek(fd, 0, os.SEEK_END)
except FileNotFoundError:
if fs == "zfs":
return get_zfs_size(filename)
return None
finally:
if fd:
os.close(fd)


def get_zfs_size(dataset) -> Optional[int]:
zpool = dataset.split("/")[0]
try:
size, _ = subp.subp(["zpool", "get", "-Hpovalue", "size", zpool])
except subp.ProcessExecutionError as e:
LOG.debug("Failed: zpool get size %s: %s", zpool, e)
return None
return int(size.strip())


def devent2dev(devent):
if devent.startswith("/dev/"):
return devent
else:
result = util.get_mount_info(devent)
if not result:
raise ValueError("Could not determine device of '%s' % dev_ent")
dev = result[0]
return devent, None

result = util.get_mount_info(devent)
if not result:
raise ValueError("Could not determine device of '%s' % dev_ent")
dev = result[0]
fs = result[1]

container = util.is_container()

Expand All @@ -310,9 +323,9 @@ def devent2dev(devent):
if os.path.exists(dev):
# if /dev/root exists, but we failed to convert
# that to a "real" /dev/ path device, then return it.
return dev
return dev, None
raise ValueError("Unable to find device '/dev/root'")
return dev
return dev, fs


def is_encrypted(blockdev, partition) -> bool:
Expand Down Expand Up @@ -414,15 +427,64 @@ def resize_encrypted(blockdev, partition) -> Tuple[str, str]:
)


def _call_resizer(resizer, devent, disk, ptnum, blockdev, fs):
info = []
try:
old, new = resizer.resize(disk, ptnum, blockdev, fs)
if old == new:
info.append(
(
devent,
RESIZE.NOCHANGE,
"no change necessary (%s, %s)" % (disk, ptnum),
)
)
elif new is None or old is None:
msg = ""
if disk is not None and ptnum is None:
msg = "changed (%s, %s) size, new size is unknown" % (
disk,
ptnum,
)
else:
msg = "changed (%s) size, new size is unknown" % blockdev
info.append((devent, RESIZE.CHANGED, msg))
else:
msg = ""
if disk is not None and ptnum is None:
msg = "changed (%s, %s) from %s to %s" % (
disk,
ptnum,
old,
new,
)
else:
msg = "changed (%s) from %s to %s" % (blockdev, old, new)
info.append((devent, RESIZE.CHANGED, msg))

except ResizeFailedException as e:
info.append(
(
devent,
RESIZE.FAILED,
"failed to resize: disk=%s, ptnum=%s: %s" % (disk, ptnum, e),
)
)
return info


def resize_devices(resizer, devices, distro: Distro):
# returns a tuple of tuples containing (entry-in-devices, action, message)
devices = copy.copy(devices)
info = []

while devices:
devent = devices.pop(0)
disk = None
ptnum = None

try:
blockdev = devent2dev(devent)
blockdev, fs = devent2dev(devent)
except ValueError as e:
info.append(
(
Expand All @@ -433,6 +495,11 @@ def resize_devices(resizer, devices, distro: Distro):
)
continue

LOG.debug("growpart found fs=%s", fs)
if fs == "zfs":
info += _call_resizer(resizer, devent, disk, ptnum, blockdev, fs)
return info

try:
statret = os.stat(blockdev)
except OSError as e:
Expand Down Expand Up @@ -512,44 +579,7 @@ def resize_devices(resizer, devices, distro: Distro):
)
continue

try:
old, new = resizer.resize(disk, ptnum, blockdev)
if old == new:
info.append(
(
devent,
RESIZE.NOCHANGE,
"no change necessary (%s, %s)" % (disk, ptnum),
)
)
elif new is None or old is None:
info.append(
(
devent,
RESIZE.CHANGED,
"changed (%s, %s) size, new size is unknown"
% (disk, ptnum),
)
)
else:
info.append(
(
devent,
RESIZE.CHANGED,
"changed (%s, %s) from %s to %s"
% (disk, ptnum, old, new),
)
)

except ResizeFailedException as e:
info.append(
(
devent,
RESIZE.FAILED,
"failed to resize: disk=%s, ptnum=%s: %s"
% (disk, ptnum, e),
)
)
info += _call_resizer(resizer, devent, disk, ptnum, blockdev, fs)

return info

Expand Down
81 changes: 73 additions & 8 deletions tests/unittests/config/test_cc_growpart.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@

import pytest

from cloudinit import cloud, subp, temp_utils
from cloudinit import cloud, distros, subp, temp_utils
from cloudinit.config import cc_growpart
from cloudinit.config.schema import (
SchemaValidationError,
Expand Down Expand Up @@ -215,7 +215,7 @@ def test_force_lang_check_tempfile(self, *args, **kwargs):
diskdev = "/dev/sdb"
partnum = 1
partdev = "/dev/sdb"
ret.resize(diskdev, partnum, partdev)
ret.resize(diskdev, partnum, partdev, None)
mockobj.assert_has_calls(
[
mock.call(
Expand Down Expand Up @@ -351,8 +351,8 @@ def test_simple_devices(self):
resize_calls = []

class myresizer:
def resize(self, diskdev, partnum, partdev):
resize_calls.append((diskdev, partnum, partdev))
def resize(self, diskdev, partnum, partdev, fs):
resize_calls.append((diskdev, partnum, partdev, fs))
if partdev == "/dev/YYda2":
return (1024, 2048)
return (1024, 1024) # old size, new size
Expand Down Expand Up @@ -395,7 +395,70 @@ def find(name, res):
os.stat = real_stat


class TestResizeZFS:
def _devent2dev_side_effect(self, value):
if value.startswith("zroot"):
return value, "zfs"
raise RuntimeError(f"unexpected value {value}")

def _subp_side_effect(self, value, **kwargs):
if value[0] == "growpart":
raise subp.ProcessExecutionError()
elif value[0] == "zpool":
return ("1024\n", "")
raise subp.ProcessExecutionError()

@pytest.fixture
def common_mocks(self, mocker):
# These are all "happy path" mocks which will get overridden
# when needed
mocker.patch(
"cloudinit.config.cc_growpart.devent2dev",
side_effect=self._devent2dev_side_effect,
)
mocker.patch("cloudinit.util.is_container", return_value=False)
# Find /etc/rc.d/growfs
mocker.patch("os.path.isfile", return_value=True)
mocker.patch(
"cloudinit.config.cc_growpart.subp.subp",
side_effect=self._subp_side_effect,
)
cls = distros.fetch("freebsd")
# patch ifconfig -a
mocker.patch(
"cloudinit.distros.networking.subp.subp", return_value=("", None)
)
self.distro = cls("freebsd", {}, None)

@pytest.mark.parametrize(
"dev, expected",
[
("zroot/ROOT/changed", cc_growpart.RESIZE.CHANGED),
("zroot/ROOT/nochange", cc_growpart.RESIZE.NOCHANGE),
],
)
def test_zroot(self, dev, expected, common_mocks):
resize_calls = []

class myresizer:
def resize(self, diskdev, partnum, partdev, fs):
resize_calls.append((diskdev, partnum, partdev, fs))
if partdev == "zroot/ROOT/changed":
return (1024, 2048)
return (1024, 1024) # old size, new size

def find(name, res):
for f in res:
if f[0] == name:
return f
return None

resized = cc_growpart.resize_devices(myresizer(), [dev], self.distro)
assert expected == find(dev, resized)[1]


class TestGetSize:
# TODO: add tests for get_zfs_size()
@pytest.mark.parametrize(
"file_exists, expected",
(
Expand All @@ -408,7 +471,7 @@ def test_get_size_behaves(self, file_exists, expected, tmp_path):
tmp_file = tmp_path / "tmp.txt"
if file_exists:
tmp_file.write_bytes(b"0")
assert expected == cc_growpart.get_size(tmp_file)
assert expected == cc_growpart.get_size(tmp_file, None)


class TestEncrypted:
Expand Down Expand Up @@ -437,11 +500,13 @@ def _device_part_info_side_effect(self, value):

def _devent2dev_side_effect(self, value):
if value == "/fake_encrypted":
return "/dev/mapper/fake"
return "/dev/mapper/fake", "ext3"
elif value == "/":
return "/dev/vdz"
return "/dev/vdz", "ext4"
elif value.startswith("zroot"):
return value, "zfs"
elif value.startswith("/dev"):
return value
return value, None
raise RuntimeError(f"unexpected value {value}")

def _realpath_side_effect(self, value):
Expand Down

0 comments on commit 8ec2edd

Please sign in to comment.