Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

growpart: Fix behaviour for ZFS datasets #5169

Merged
merged 2 commits into from
May 31, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
146 changes: 88 additions & 58 deletions cloudinit/config/cc_growpart.py
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):
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

might wanna rename this function now that it returns more, maybe deventinfo?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

might wanna rename this function now that it returns more, maybe deventinfo?

Agreed, how about a snake case device_entry_info() or dev_entry_info() or something?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sounds good, I'll change that tomorrow or the day after.

if devent.startswith("/dev/"):
return devent
igalic marked this conversation as resolved.
Show resolved Hide resolved
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
3 changes: 2 additions & 1 deletion cloudinit/distros/bsd.py
Original file line number Diff line number Diff line change
Expand Up @@ -169,4 +169,5 @@ def device_part_info(devpath: str) -> tuple:
if m:
return m["dev"], m["part_slice"]

return distros.Distro.device_part_info(devpath)
# the input is bogus and we need to bail
raise ValueError(f"Invalid value for devpath: '{devpath}'")
87 changes: 79 additions & 8 deletions tests/unittests/config/test_cc_growpart.py
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 Expand Up @@ -681,6 +746,12 @@ class TestDevicePartInfo:
does_not_raise(),
id="bsd_mbr_slice_and_partition",
),
pytest.param(
"zroot/ROOТ/default",
(),
pytest.raises(ValueError),
id="zfs_dataset",
),
),
)
def test_device_part_info(self, devpath, expected, raised_exception):
Expand Down
Loading