saltstack/salt
1
0
Fork 0
mirror of https://github.com/saltstack/salt.git synced 2025-04-17 10:10:20 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 4

Merge pull request #65351 from garethgreenaway/18907_lazy_unmount_when_fails

Browse source 
[master] Add fallback for when "remounting"  NFS or FUSE initially fails
This commit is contained in:
David Murphy 2023-12-11 16:19:38 -07:00 • committed by GitHub
commit a04d09ec10
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
5 changed files with 214 additions and 57 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

1
changelog/18907.fixed.md Normal file
View file

@ -0,0 +1 @@
When an NFS or FUSE mount fails to unmount when mount options have changed, try again with a lazy umount before mounting again.

10
salt/modules/mount.py
View file

@ -1383,7 +1383,7 @@ def remount(name, device, mkmnt=False, fstype="", opts="defaults", user=None):
return mount(name, device, mkmnt, fstype, opts, user=user)
def umount(name, device=None, user=None, util="mount"):
def umount(name, device=None, user=None, util="mount", lazy=False):
"""
Attempt to unmount a device by specifying the directory it is mounted on
@ -1413,10 +1413,14 @@ def umount(name, device=None, user=None, util="mount"):
if name not in mnts:
return f"{name} does not have anything mounted"
cmd = "umount"
if lazy:
cmd = f"{cmd} -l"
if not device:
cmd = f"umount '{name}'"
cmd = f"{cmd} '{name}'"
else:
cmd = f"umount '{device}'"
cmd = f"{cmd}'{device}'"
out = __salt__["cmd.run_all"](cmd, runas=user, python_shell=False)
if out["retcode"]:
return out["stderr"]

118
salt/states/mount.py
View file

@ -71,7 +71,7 @@ def mounted(
extra_mount_translate_options=None,
hidden_opts=None,
bind_mount_copy_active_opts=True,
**kwargs
**kwargs,
):
"""
Verify that a device is mounted
@ -282,10 +282,10 @@ def mounted(
real_device = device.split("=")[1].strip('"').lower()
elif device.upper().startswith("LABEL="):
_label = device.split("=")[1]
cmd = "blkid -t LABEL={}".format(_label)
res = __salt__["cmd.run_all"]("{}".format(cmd))
cmd = f"blkid -t LABEL={_label}"
res = __salt__["cmd.run_all"](f"{cmd}")
if res["retcode"] > 0:
ret["comment"] = "Unable to find device with label {}.".format(_label)
ret["comment"] = f"Unable to find device with label {_label}."
ret["result"] = False
return ret
else:
@ -432,7 +432,7 @@ def mounted(
)
if size_match:
converted_size = _size_convert(size_match)
opt = "size={}k".format(converted_size)
opt = f"size={converted_size}k"
# make cifs option user synonym for option username which is reported by /proc/mounts
if fstype in ["cifs"] and opt.split("=")[0] == "user":
opt = "username={}".format(opt.split("=")[1])
@ -460,9 +460,9 @@ def mounted(
)
if size_match:
converted_size = _size_convert(size_match)
opt = "size={}k".format(converted_size)
opt = f"size={converted_size}k"
_active_superopts.remove(_active_opt)
_active_opt = "size={}k".format(converted_size)
_active_opt = f"size={converted_size}k"
_active_superopts.append(_active_opt)
if (
@ -504,11 +504,34 @@ def mounted(
)
ret["result"] = mount_result
else:
ret["result"] = False
ret["comment"] = "Unable to unmount {}: {}.".format(
real_name, unmount_result
# If the first attempt at unmounting fails,
# run again as a lazy umount.
ret["changes"]["umount"] = (
"Forced a lazy unmount and mount "
+ "because the previous unmount failed "
+ "and because the "
+ "options ({}) changed".format(
",".join(sorted(trigger_remount))
)
)
return ret
unmount_result = __salt__["mount.umount"](
real_name, lazy=True
)
if unmount_result is True:
mount_result = __salt__["mount.mount"](
real_name,
device,
mkmnt=mkmnt,
fstype=fstype,
opts=opts,
)
ret["result"] = mount_result
else:
ret["result"] = False
ret["comment"] = "Unable to unmount {}: {}.".format(
real_name, unmount_result
)
return ret
else:
ret["changes"]["umount"] = (
"Forced remount because "
@ -551,7 +574,7 @@ def mounted(
if fstype in ["nfs", "cvfs"] or fstype.startswith("fuse"):
ret["changes"]["umount"] = (
"Forced unmount and mount because "
+ "options ({}) changed".format(opt)
+ f"options ({opt}) changed"
)
unmount_result = __salt__["mount.umount"](real_name)
if unmount_result is True:
@ -564,15 +587,32 @@ def mounted(
)
ret["result"] = mount_result
else:
ret["result"] = False
ret["comment"] = "Unable to unmount {}: {}.".format(
real_name, unmount_result
# If the first attempt at unmounting fails,
# run again as a lazy umount.
unmount_result = __salt__["mount.umount"](
real_name, lazy=True
)
return ret
if unmount_result is True:
mount_result = __salt__["mount.mount"](
real_name,
device,
mkmnt=mkmnt,
fstype=fstype,
opts=opts,
)
ret["result"] = mount_result
else:
ret["result"] = False
ret[
"comment"
] = "Unable to unmount {}: {}.".format(
real_name, unmount_result
)
return ret
else:
ret["changes"]["umount"] = (
"Forced remount because "
+ "options ({}) changed".format(opt)
+ f"options ({opt}) changed"
)
remount_result = __salt__["mount.remount"](
real_name,
@ -638,13 +678,11 @@ def mounted(
if __opts__["test"]:
ret["result"] = None
if os.path.exists(name):
ret["comment"] = "{} would be mounted".format(name)
ret["comment"] = f"{name} would be mounted"
elif mkmnt:
ret["comment"] = "{} would be created and mounted".format(name)
ret["comment"] = f"{name} would be created and mounted"
else:
ret[
"comment"
] = "{} does not exist and would not be created".format(name)
ret["comment"] = f"{name} does not exist and would not be created"
return ret
if not os.path.exists(name) and not mkmnt:
@ -668,7 +706,7 @@ def mounted(
if __opts__["test"]:
ret["result"] = None
if mkmnt:
ret["comment"] = "{} would be created, but not mounted".format(name)
ret["comment"] = f"{name} would be created, but not mounted"
else:
ret[
"comment"
@ -677,14 +715,14 @@ def mounted(
)
elif mkmnt:
__salt__["file.mkdir"](name, user=user)
ret["comment"] = "{} was created, not mounted".format(name)
ret["comment"] = f"{name} was created, not mounted"
else:
ret["comment"] = "{} not present and not mounted".format(name)
ret["comment"] = f"{name} not present and not mounted"
else:
if __opts__["test"]:
ret["comment"] = "{} would not be mounted".format(name)
ret["comment"] = f"{name} would not be mounted"
else:
ret["comment"] = "{} not mounted".format(name)
ret["comment"] = f"{name} not mounted"
if persist:
if "/etc/fstab" == config:
@ -745,7 +783,7 @@ def mounted(
name
)
else:
comment = "The {} fstab entry must be updated.".format(name)
comment = f"The {name} fstab entry must be updated."
else:
ret["result"] = False
comment = (
@ -824,25 +862,25 @@ def swap(name, persist=True, config="/etc/fstab"):
if __salt__["file.is_link"](name):
real_swap_device = __salt__["file.readlink"](name)
if not real_swap_device.startswith("/"):
real_swap_device = "/dev/{}".format(os.path.basename(real_swap_device))
real_swap_device = f"/dev/{os.path.basename(real_swap_device)}"
else:
real_swap_device = name
if real_swap_device in on_:
ret["comment"] = "Swap {} already active".format(name)
ret["comment"] = f"Swap {name} already active"
elif __opts__["test"]:
ret["result"] = None
ret["comment"] = "Swap {} is set to be activated".format(name)
ret["comment"] = f"Swap {name} is set to be activated"
else:
__salt__["mount.swapon"](real_swap_device)
on_ = __salt__["mount.swaps"]()
if real_swap_device in on_:
ret["comment"] = "Swap {} activated".format(name)
ret["comment"] = f"Swap {name} activated"
ret["changes"] = on_[real_swap_device]
else:
ret["comment"] = "Swap {} failed to activate".format(name)
ret["comment"] = f"Swap {name} failed to activate"
ret["result"] = False
if persist:
@ -949,7 +987,7 @@ def unmounted(
# The mount is present! Unmount it
if __opts__["test"]:
ret["result"] = None
ret["comment"] = "Mount point {} is mounted but should not be".format(name)
ret["comment"] = f"Mount point {name} is mounted but should not be"
return ret
if device:
out = __salt__["mount.umount"](name, device, user=user)
@ -1041,10 +1079,10 @@ def mod_watch(name, user=None, **kwargs):
name, kwargs["device"], False, kwargs["fstype"], kwargs["opts"], user=user
)
if out:
ret["comment"] = "{} remounted".format(name)
ret["comment"] = f"{name} remounted"
else:
ret["result"] = False
ret["comment"] = "{} failed to remount: {}".format(name, out)
ret["comment"] = f"{name} failed to remount: {out}"
else:
ret["comment"] = "Watch not supported in {} at this time".format(kwargs["sfun"])
return ret
@ -1064,7 +1102,7 @@ def _convert_to(maybe_device, convert_to):
if (
not convert_to
or (convert_to == "device" and maybe_device.startswith("/"))
or maybe_device.startswith("{}=".format(convert_to.upper()))
or maybe_device.startswith(f"{convert_to.upper()}=")
):
return maybe_device
@ -1080,7 +1118,7 @@ def _convert_to(maybe_device, convert_to):
result = next(iter(blkid))
else:
key = convert_to.upper()
result = "{}={}".format(key, next(iter(blkid.values()))[key])
result = f"{key}={next(iter(blkid.values()))[key]}"
return result
@ -1232,7 +1270,7 @@ def fstab_present(
msg = "{} entry will be written in {}."
ret["comment"].append(msg.format(fs_file, config))
if mount:
msg = "Will mount {} on {}".format(name, fs_file)
msg = f"Will mount {name} on {fs_file}"
ret["comment"].append(msg)
elif out == "change":
msg = "{} entry will be updated in {}."
@ -1290,7 +1328,7 @@ def fstab_present(
ret["result"] = False
msg = "Error while mounting {}".format(out.split(":", maxsplit=1)[1])
else:
msg = "Mounted {} on {}".format(name, fs_file)
msg = f"Mounted {name} on {fs_file}"
ret["comment"].append(msg)
elif out == "change":
ret["changes"]["persist"] = out

15
tests/pytests/unit/modules/test_mount.py
View file

@ -669,6 +669,21 @@ def test_umount():
mock.assert_called_once_with("/mountpoint", disk="/path/to/my.qcow")
def test_umount_lazy_true():
"""
Attempt to lazy unmount a device by specifying the
directory it is mounted on
"""
mock_mount_active = MagicMock(return_value={"name": "name"})
with patch.object(mount, "active", mock_mount_active):
mock_cmd = MagicMock(return_value={"retcode": True, "stderr": True})
with patch.dict(mount.__salt__, {"cmd.run_all": mock_cmd}):
mount.umount("name", lazy=True)
mock_cmd.assert_called_once_with(
"umount -l 'name'", runas=None, python_shell=False
)
def test_is_fuse_exec():
"""
Returns true if the command passed is a fuse mountable application

127
tests/pytests/unit/states/test_mount.py
View file

@ -73,8 +73,8 @@ def test_mounted():
assert mount.mounted(name, device, fstype) == ret
with patch.dict(mount.__opts__, {"test": False}):
comt = "Unable to unmount {}: False.".format(name)
umount = "Forced unmount and mount because options (noowners) changed"
comt = f"Unable to unmount {name}: False."
umount = "Forced a lazy unmount and mount because the previous unmount failed and because the options (noowners) changed"
ret.update(
{
"comment": comt,
@ -114,7 +114,7 @@ def test_mounted():
with patch.dict(mount.__opts__, {"test": True}), patch(
"os.path.exists", MagicMock(return_value=False)
):
comt = "{} does not exist and would not be created".format(name)
comt = f"{name} does not exist and would not be created"
ret.update({"comment": comt, "changes": {}, "result": None})
assert mount.mounted(name, device, fstype) == ret
@ -192,7 +192,7 @@ def test_mounted():
ret.update({"comment": comt, "result": False, "changes": {}})
assert mount.mounted(name, device, fstype, mount=False) == ret
comt = "{} not present and not mounted".format(name)
comt = f"{name} not present and not mounted"
ret.update({"comment": comt, "result": True, "changes": {}})
assert mount.mounted(name, device, fstype, mount=False) == ret
@ -319,7 +319,7 @@ def test_swap():
assert mount.swap(name) == ret
with patch.dict(mount.__opts__, {"test": False}):
comt = "Swap {} already active".format(name)
comt = f"Swap {name} already active"
ret.update({"comment": comt, "result": True})
assert mount.swap(name, persist=False) == ret
@ -327,7 +327,7 @@ def test_swap():
mount.__salt__,
{"mount.fstab": mock_emt, "mount.set_fstab": mock},
):
comt = "Swap {} already active".format(name)
comt = f"Swap {name} already active"
ret.update({"comment": comt, "result": True})
assert mount.swap(name) == ret
@ -375,12 +375,12 @@ def test_swap():
},
):
with patch.dict(mount.__opts__, {"test": True}):
comt = "Swap {} already active".format(name)
comt = f"Swap {name} already active"
ret.update({"comment": comt, "result": True})
assert mount.swap(name) == ret
with patch.dict(mount.__opts__, {"test": False}):
comt = "Swap {} already active".format(name)
comt = f"Swap {name} already active"
ret.update({"comment": comt, "result": True})
assert mount.swap(name) == ret
@ -388,7 +388,7 @@ def test_swap():
mount.__salt__,
{"mount.fstab": mock_emt, "mount.set_fstab": mock},
):
comt = "Swap {} already active".format(name)
comt = f"Swap {name} already active"
ret.update({"comment": comt, "result": True})
assert mount.swap(name) == ret
@ -432,7 +432,7 @@ def test_swap():
},
):
with patch.dict(mount.__opts__, {"test": True}):
comt = "Swap {} already active".format(name)
comt = f"Swap {name} already active"
ret.update({"comment": comt, "result": True})
assert mount.swap(name) == ret
@ -494,7 +494,7 @@ def test_unmounted():
},
):
with patch.dict(mount.__opts__, {"test": True}):
comt = "Mount point {} is mounted but should not be".format(name)
comt = f"Mount point {name} is mounted but should not be"
ret.update({"comment": comt})
assert mount.unmounted(name, device) == ret
@ -535,7 +535,7 @@ def test_unmounted():
assert mount.unmounted(name, device, persist=True) == ret
with patch.dict(mount.__salt__, {"mount.filesystems": mock_dev}):
comt = "Mount point {} is mounted but should not be".format(name3)
comt = f"Mount point {name3} is mounted but should not be"
ret.update({"comment": comt, "result": None, "name": name3})
assert mount.unmounted(name3, device3, persist=True) == ret
@ -676,7 +676,7 @@ def test__convert_to_device_token():
"disk.blkid": MagicMock(return_value={"/dev/sda1": {"UUID": uuid}}),
}
with patch.dict(mount.__salt__, salt_mock):
uuid = "UUID={}".format(uuid)
uuid = f"UUID={uuid}"
assert mount._convert_to("/dev/sda1", "uuid") == uuid
salt_mock["disk.blkid"].assert_called_with("/dev/sda1")
@ -690,7 +690,7 @@ def test__convert_to_token_device():
"disk.blkid": MagicMock(return_value={"/dev/sda1": {"UUID": uuid}}),
}
with patch.dict(mount.__salt__, salt_mock):
uuid = "UUID={}".format(uuid)
uuid = f"UUID={uuid}"
assert mount._convert_to(uuid, "device") == "/dev/sda1"
salt_mock["disk.blkid"].assert_called_with(token=uuid)
@ -1431,3 +1431,102 @@ def test_bind_mount_copy_active_opts(mount_name):
"/etc/fstab",
match_on="auto",
)
def test_mount_opts_change_lazy_umount():
name = "/mnt/nfs"
device = name
active_name = name
fstype = "nfs"
opts = [
"nodev",
"noexec",
"nosuid",
"rw",
]
ret = {"name": name, "result": None, "comment": "", "changes": {}}
mock_active = MagicMock(
return_value={
active_name: {
"alt_device": "192.168.0.1:/volume2/nfs",
"device": "192.168.0.1:/volume2/nfs",
"fstype": "nfs4",
"opts": [
"rw",
"noexec",
"nodev",
"relatime",
"vers=4.1",
"rsize=131072",
"wsize=131072",
],
},
}
)
mock_read_mount_cache = MagicMock(
return_value={
"device": "192.168.0.1:/volume2/nfs",
"fstype": "nfs",
"mkmnt": True,
"opts": ["noexec", "nodev"],
}
)
mock_set_fstab = MagicMock(return_value="new")
with patch.dict(mount.__grains__, {"os": "CentOS"}), patch.dict(
mount.__salt__,
{
"mount.active": mock_active,
"mount.read_mount_cache": mock_read_mount_cache,
"mount.umount": MagicMock(side_effect=[False, True]),
"mount.mount": MagicMock(return_value=True),
"mount.set_fstab": mock_set_fstab,
"mount.write_mount_cache": MagicMock(return_value=True),
},
), patch.object(
os.path,
"realpath",
MagicMock(return_value=name),
):
with patch.dict(mount.__opts__, {"test": True}):
ret["comment"] = "Remount would be forced because options (nosuid) changed"
result = mount.mounted(
name=name,
device=device,
fstype=fstype,
opts=opts,
persist=True,
bind_mount_copy_active_opts=False,
)
assert result == ret
with patch.dict(mount.__opts__, {"test": False}):
ret["comment"] = "Target was already mounted. Added new entry to the fstab."
ret["changes"] = {
"persist": "new",
"umount": "Forced a lazy unmount and mount because the previous unmount failed and because the options (nosuid) changed",
}
ret["result"] = True
# bind_mount_copy_active_opts is off
result = mount.mounted(
name=name,
device=device,
fstype=fstype,
opts=opts,
persist=True,
)
assert result == ret
mock_set_fstab.assert_called_with(
name,
device,
fstype,
["nodev", "noexec", "nosuid", "rw"],
0,
0,
"/etc/fstab",
match_on="auto",
)