From 13da73d41425dcbaaeec560df4f118314ed7998c Mon Sep 17 00:00:00 2001 From: "Gareth J. Greenaway" Date: Wed, 4 Oct 2023 16:03:34 -0700 Subject: [PATCH] When an NFS or FUSE mount fails to unmount when mount options have changed, try again with a lazy umount before mounting again. --- salt/modules/mount.py | 115 +++++++++++---------- salt/states/mount.py | 118 +++++++++++++-------- tests/pytests/unit/modules/test_mount.py | 15 +++ tests/pytests/unit/states/test_mount.py | 125 ++++++++++++++++++++--- 4 files changed, 262 insertions(+), 111 deletions(-) diff --git a/salt/modules/mount.py b/salt/modules/mount.py index 869c6f2016d..4faa0d0e347 100644 --- a/salt/modules/mount.py +++ b/salt/modules/mount.py @@ -197,7 +197,7 @@ def _active_mounts_openbsd(ret): comps = re.sub(r"\s+", " ", line).split() parens = re.findall(r"\((.*?)\)", line, re.DOTALL) if len(parens) > 1: - nod = __salt__["cmd.run_stdout"]("ls -l {}".format(comps[0])) + nod = __salt__["cmd.run_stdout"](f"ls -l {comps[0]}") nod = " ".join(nod.split()).split(" ") ret[comps[3]] = { "device": comps[0], @@ -306,7 +306,7 @@ class _fstab_entry: @classmethod def dict_from_line(cls, line, keys=fstab_keys): if len(keys) != 6: - raise ValueError("Invalid key array: {}".format(keys)) + raise ValueError(f"Invalid key array: {keys}") if line.startswith("#"): raise cls.ParseError("Comment!") @@ -523,12 +523,12 @@ class _FileSystemsEntry: @classmethod def dict_from_lines(cls, lines, keys=filesystems_keys): if len(lines) < 2: - raise ValueError("Invalid number of lines: {}".format(lines)) + raise ValueError(f"Invalid number of lines: {lines}") if not keys: # if empty force default filesystems_keys keys = _FileSystemsEntry.filesystems_keys elif len(keys) < 6: - raise ValueError("Invalid key name array: {}".format(keys)) + raise ValueError(f"Invalid key name array: {keys}") blk_lines = lines orddict = OrderedDict() @@ -546,9 +546,7 @@ class _FileSystemsEntry: if key_name in keys: orddict[key_name] = comps[1].strip() else: - raise ValueError( - "Invalid name for use in filesystems: {}".format(key_name) - ) + raise ValueError(f"Invalid name for use in filesystems: {key_name}") return orddict @@ -575,7 +573,7 @@ class _FileSystemsEntry: strg_out = entry["name"] + ":" + os.linesep for k, v in entry.items(): if "name" not in k: - strg_out += "\t{}\t\t= {}".format(k, v) + os.linesep + strg_out += f"\t{k}\t\t= {v}" + os.linesep strg_out += os.linesep return str(strg_out) @@ -586,7 +584,7 @@ class _FileSystemsEntry: list_out.append(str(entry["name"] + ":" + os.linesep)) for k, v in entry.items(): if "name" not in k: - list_out.append(str("\t{}\t\t= {}".format(k, v) + os.linesep)) + list_out.append(str(f"\t{k}\t\t= {v}" + os.linesep)) list_out.append(str(os.linesep)) return list_out @@ -793,7 +791,7 @@ def set_fstab( test=False, match_on="auto", not_change=False, - **kwargs + **kwargs, ): """ Verify that this mount is represented in the fstab, change the mount @@ -869,12 +867,12 @@ def set_fstab( filterFn = lambda key: key not in _fstab_entry.fstab_keys invalid_keys = filter(filterFn, match_on) - msg = 'Unrecognized keys in match_on: "{}"'.format(invalid_keys) + msg = f'Unrecognized keys in match_on: "{invalid_keys}"' raise CommandExecutionError(msg) # parse file, use ret to cache status if not os.path.isfile(config): - raise CommandExecutionError('Bad config file "{}"'.format(config)) + raise CommandExecutionError(f'Bad config file "{config}"') try: with salt.utils.files.fopen(config, "r") as ifile: @@ -930,7 +928,7 @@ def set_vfstab( test=False, match_on="auto", not_change=False, - **kwargs + **kwargs, ): """ .. versionadded:: 2016.3.2 @@ -999,12 +997,12 @@ def set_vfstab( filterFn = lambda key: key not in _vfstab_entry.vfstab_keys invalid_keys = filter(filterFn, match_on) - msg = 'Unrecognized keys in match_on: "{}"'.format(invalid_keys) + msg = f'Unrecognized keys in match_on: "{invalid_keys}"' raise CommandExecutionError(msg) # parse file, use ret to cache status if not os.path.isfile(config): - raise CommandExecutionError('Bad config file "{}"'.format(config)) + raise CommandExecutionError(f'Bad config file "{config}"') try: with salt.utils.files.fopen(config, "r") as ifile: @@ -1117,7 +1115,7 @@ def set_automaster( config="/etc/auto_salt", test=False, not_change=False, - **kwargs + **kwargs, ): """ Verify that this mount is represented in the auto_salt, change the mount @@ -1139,11 +1137,11 @@ def set_automaster( if not os.path.isfile(config): __salt__["file.touch"](config) - __salt__["file.append"](automaster_file, "/-\t\t\t{}".format(config)) + __salt__["file.append"](automaster_file, f"/-\t\t\t{config}") - name = "/..{}".format(name) - device_fmt = "{}:{}".format(fstype, device) - type_opts = "-fstype={},{}".format(fstype, opts) + name = f"/..{name}" + device_fmt = f"{fstype}:{device}" + type_opts = f"-fstype={fstype},{opts}" if fstype == "smbfs": device_fmt = device_fmt.replace(fstype, "") @@ -1185,7 +1183,7 @@ def set_automaster( "auto_master entry for mount point %s needs to be updated", name, ) - newline = "{}\t{}\t{}\n".format(name, type_opts, device_fmt) + newline = f"{name}\t{type_opts}\t{device_fmt}\n" lines.append(newline) else: lines.append(line) @@ -1212,14 +1210,14 @@ def set_automaster( else: if not salt.utils.args.test_mode(test=test, **kwargs): # The entry is new, add it to the end of the fstab - newline = "{}\t{}\t{}\n".format(name, type_opts, device_fmt) + newline = f"{name}\t{type_opts}\t{device_fmt}\n" lines.append(newline) try: with salt.utils.files.fopen(config, "wb") as ofile: # The line was changed, commit it! ofile.writelines(salt.utils.data.encode(lines)) except OSError: - raise CommandExecutionError("File not writable {}".format(config)) + raise CommandExecutionError(f"File not writable {config}") return "new" @@ -1280,7 +1278,7 @@ def mount( if not mnt: return False first = next(iter(mnt.keys())) - __context__["img.mnt_{}".format(first)] = mnt + __context__[f"img.mnt_{first}"] = mnt return first return False @@ -1297,24 +1295,24 @@ def mount( args = "" if opts is not None: lopts = ",".join(opts) - args = "-o {}".format(lopts) + args = f"-o {lopts}" if fstype: # use of fstype on AIX differs from typical Linux use of -t # functionality AIX uses -v vfsname, -t fstype mounts all with # fstype in /etc/filesystems if "AIX" in __grains__["os"]: - args += " -v {}".format(fstype) + args += f" -v {fstype}" elif "solaris" in __grains__["os"].lower(): - args += " -F {}".format(fstype) + args += f" -F {fstype}" else: - args += " -t {}".format(fstype) + args += f" -t {fstype}" cmd = "mount " if device: - cmd += "{} '{}' '{}' ".format(args, device, name) + cmd += f"{args} '{device}' '{name}' " else: - cmd += "'{}' ".format(name) + cmd += f"'{name}' " out = __salt__["cmd.run_all"](cmd, runas=user, python_shell=False) if out["retcode"]: return out["stderr"] @@ -1360,23 +1358,23 @@ def remount(name, device, mkmnt=False, fstype="", opts="defaults", user=None): args = "" if opts: lopts = ",".join(opts) - args = "-o {}".format(lopts) + args = f"-o {lopts}" if fstype: # use of fstype on AIX differs from typical Linux use of # -t functionality AIX uses -v vfsname, -t fstype mounts # all with fstype in /etc/filesystems if "AIX" in __grains__["os"]: - args += " -v {}".format(fstype) + args += f" -v {fstype}" elif "solaris" in __grains__["os"].lower(): - args += " -F {}".format(fstype) + args += f" -F {fstype}" else: - args += " -t {}".format(fstype) + args += f" -t {fstype}" if __grains__["os"] not in ["OpenBSD", "MacOS", "Darwin"] or force_mount: - cmd = "mount {} '{}' '{}' ".format(args, device, name) + cmd = f"mount {args} '{device}' '{name}' " else: - cmd = "mount -u {} '{}' '{}' ".format(args, device, name) + cmd = f"mount -u {args} '{device}' '{name}' " out = __salt__["cmd.run_all"](cmd, runas=user, python_shell=False) if out["retcode"]: return out["stderr"] @@ -1385,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 @@ -1407,18 +1405,21 @@ def umount(name, device=None, user=None, util="mount"): elif util == "qemu_nbd": # This functionality used to live in img.umount_image if "qemu_nbd.clear" in __salt__: - if "img.mnt_{}".format(name) in __context__: - __salt__["qemu_nbd.clear"](__context__["img.mnt_{}".format(name)]) + if f"img.mnt_{name}" in __context__: + __salt__["qemu_nbd.clear"](__context__[f"img.mnt_{name}"]) return mnts = active() if name not in mnts: - return "{} does not have anything mounted".format(name) + return f"{name} does not have anything mounted" + cmd = "umount" + if lazy: + cmd = f"{cmd} -l" if not device: - cmd = "umount '{}'".format(name) + cmd = f"{cmd} '{name}'" else: - cmd = "umount '{}'".format(device) + cmd = f"{cmd}'{device}'" out = __salt__["cmd.run_all"](cmd, runas=user, python_shell=False) if out["retcode"]: return out["stderr"] @@ -1443,7 +1444,7 @@ def is_fuse_exec(cmd): elif not salt.utils.path.which("ldd"): raise CommandNotFoundError("ldd") - out = __salt__["cmd.run"]("ldd {}".format(cmd_path), python_shell=False) + out = __salt__["cmd.run"](f"ldd {cmd_path}", python_shell=False) return "libfuse" in out @@ -1535,13 +1536,13 @@ def swapon(name, priority=None): if __grains__["kernel"] == "SunOS": if __grains__["virtual"] != "zone": - __salt__["cmd.run"]("swap -a '{}'".format(name), python_shell=False) + __salt__["cmd.run"](f"swap -a '{name}'", python_shell=False) else: return False else: - cmd = "swapon '{}'".format(name) + cmd = f"swapon '{name}'" if priority and "AIX" not in __grains__["kernel"]: - cmd += " -p {}".format(priority) + cmd += f" -p {priority}" __salt__["cmd.run"](cmd, python_shell=False) on_ = swaps() @@ -1569,13 +1570,13 @@ def swapoff(name): if name in on_: if __grains__["kernel"] == "SunOS": if __grains__["virtual"] != "zone": - __salt__["cmd.run"]("swap -a '{}'".format(name), python_shell=False) + __salt__["cmd.run"](f"swap -a '{name}'", python_shell=False) else: return False elif __grains__["os"] != "OpenBSD": - __salt__["cmd.run"]("swapoff '{}'".format(name), python_shell=False) + __salt__["cmd.run"](f"swapoff '{name}'", python_shell=False) else: - __salt__["cmd.run"]("swapctl -d '{}'".format(name), python_shell=False) + __salt__["cmd.run"](f"swapctl -d '{name}'", python_shell=False) on_ = swaps() if name in on_: return False @@ -1779,7 +1780,7 @@ def set_filesystems( test=False, match_on="auto", not_change=False, - **kwargs + **kwargs, ): """ .. versionadded:: 2018.3.3 @@ -1880,13 +1881,11 @@ def set_filesystems( except KeyError: filterFn = lambda key: key not in _FileSystemsEntry.compatibility_keys invalid_keys = filter(filterFn, match_on) - raise CommandExecutionError( - 'Unrecognized keys in match_on: "{}"'.format(invalid_keys) - ) + raise CommandExecutionError(f'Unrecognized keys in match_on: "{invalid_keys}"') # parse file, use ret to cache status if not os.path.isfile(config): - raise CommandExecutionError('Bad config file "{}"'.format(config)) + raise CommandExecutionError(f'Bad config file "{config}"') # read in block of filesystem, block starts with '/' till empty line try: @@ -1904,7 +1903,7 @@ def set_filesystems( view_lines.append(fsys_view) except OSError as exc: - raise CommandExecutionError("Couldn't read from {}: {}".format(config, exc)) + raise CommandExecutionError(f"Couldn't read from {config}: {exc}") # add line if not present or changed if ret is None: @@ -1922,7 +1921,7 @@ def set_filesystems( ofile.writelines(salt.utils.data.encode(list_strgs)) except OSError: - raise CommandExecutionError("File not writable {}".format(config)) + raise CommandExecutionError(f"File not writable {config}") except Exception as exc: raise CommandExecutionError("set_filesystems error exception {exc}") @@ -1961,7 +1960,7 @@ def rm_filesystems(name, device, config="/etc/filesystems"): view_lines.append(fsys_view) except OSError as exc: - raise CommandExecutionError("Couldn't read from {}: {}".format(config, exc)) + raise CommandExecutionError(f"Couldn't read from {config}: {exc}") if modified: try: @@ -1971,7 +1970,7 @@ def rm_filesystems(name, device, config="/etc/filesystems"): list_strgs = _FileSystemsEntry.dict_to_list_lines(entry) ofile.writelines(salt.utils.data.encode(list_strgs)) except OSError as exc: - raise CommandExecutionError("Couldn't write to {}: {}".format(config, exc)) + raise CommandExecutionError(f"Couldn't write to {config}: {exc}") except Exception as exc: raise CommandExecutionError("rm_filesystems error exception {exc}") diff --git a/salt/states/mount.py b/salt/states/mount.py index 218644a8590..8f608bf265b 100644 --- a/salt/states/mount.py +++ b/salt/states/mount.py @@ -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 diff --git a/tests/pytests/unit/modules/test_mount.py b/tests/pytests/unit/modules/test_mount.py index 3a4e1d544e3..b93017cb5e8 100644 --- a/tests/pytests/unit/modules/test_mount.py +++ b/tests/pytests/unit/modules/test_mount.py @@ -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 diff --git a/tests/pytests/unit/states/test_mount.py b/tests/pytests/unit/states/test_mount.py index 1eb806d2a4a..a98709b2963 100644 --- a/tests/pytests/unit/states/test_mount.py +++ b/tests/pytests/unit/states/test_mount.py @@ -73,7 +73,7 @@ def test_mounted(): assert mount.mounted(name, device, fstype) == ret with patch.dict(mount.__opts__, {"test": False}): - comt = "Unable to unmount {}: False.".format(name) + comt = f"Unable to unmount {name}: False." umount = "Forced unmount and mount because options (noowners) changed" ret.update( { @@ -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", + )