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

Mount absent behaves erratically #525

Open
ffrank opened this issue Jun 5, 2019 · 4 comments
Open

Mount absent behaves erratically #525

ffrank opened this issue Jun 5, 2019 · 4 comments

Comments

@ffrank
Copy link
Contributor

ffrank commented Jun 5, 2019

Versions:

  • mgmt version (eg: mgmt --version): mgmt version 0.0.19-30-ge15f110

  • operating system/distribution (eg: uname -a): Linux dev-app01-ffrank 4.4.0-140-generic #166-Ubuntu SMP Wed Nov 14 20:09:47 UTC 2018 x86_64 x86_64 x86_64 GNU/Linux

  • golang version (eg: go version): go version go1.11.5 linux/amd64

Description:

I'm mounting a simple filesystem using the following script

mount "/mnt/acceptance_mount" {
	state => "exists",
	device => "/tmp/loopdevice",
	type => "ext2",
	options => {
		"rw"=>"",
		"loop"=>"",
	},
}

I'm trying to unmount it. The following script does unmount it, but leaves it in fstab:

mount "/mnt/acceptance_mount" {
        state => "absent",
        device => "/tmp/loopdevice",
}

Same when also specifying the type

mount "/mnt/acceptance_mount" {
        state => "absent",
        device => "/tmp/loopdevice",
        type => "ext2",

However, when specifying the options, the entry will be removed from fstab. But the filesystem will not be unmounted.

mount "/mnt/acceptance_mount" {
        state => "absent",
        device => "/tmp/loopdevice",
        type => "ext2",
        options => {
                "rw"=>"",
                "loop"=>"",
        },
}

Omitting the type from the final example does not change the outcome.

What will change the outcome is changing the options to a set that does not match the existing mount.

mount "/mnt/acceptance_mount" {
        state => "absent",
        device => "/tmp/loopdevice",
        type => "ext2",
        options => {
                "ro"=>"",
                "loop"=>"",
        },
}

When specifying ro rather than rw, the fstab is not touched, nor is the filesystem unmounted.

In summary: Mount resource with state=>absent will

  • unmount but not update fstab when resource specifies no options
  • update fstab but not unmount when resource does specify matching options
  • do nothing when the resource specifies non-matching options

I have not been able to come up with a script that cleanly removes the mount.

@purpleidea
Copy link
Owner

@ffrank TBH I have not properly tested the resource, so there could be bugs. @jonathangold wrote it, but maybe he'll know if this sounds like something he overlooked.

I'll have to circle back to this after I finish function values in core.

Thanks for the report! Patches and tests are welcome! (And there's existing resource test infra if you want to easily add one. Ping if you can't find it.)

@purpleidea
Copy link
Owner

@ffrank AIUI this is now fixed by your patch, so we can close, correct?

@ffrank
Copy link
Contributor Author

ffrank commented Feb 26, 2024

No my patch was for #509. I'm about to pick this back up, but honestly, I'm tempted to raise a new ticket in order to replace the fstab shenanigans with native systemd mount units (https://www.freedesktop.org/software/systemd/man/latest/systemd.mount.html).

For tooling, writing mount units should be preferred over editing /etc/fstab.

Doing so should allow us to drop the dependency on deniswernert/go-fstab (unmaintained since a decade) and should make interactions between mgmt and systemd much cleaner.

@purpleidea
Copy link
Owner

replace the fstab shenanigans with native systemd mount units

So we should definitely be doing systemd mount... Whether we also have flags to still touch legacy /etc/fstab is a good question.

FWIW I think you can wrap the svc resource to do mount things. At least one other resource does some wrapping of svc.

Feel free to open a discussion issue if you want to discuss API or design of resource changes.

ffrank added a commit to ffrank/mgmt that referenced this issue Mar 6, 2024
The mount approach was flawed. It relied on editing fstab, then restarting the
systemd service that is responsible for mounting all defined filesystems. Among
other issues, this approach made it impossible for mgmt to unmount managed
filesystems, because after removing them from fstab, they would just be ignored
by systemd.

The new approach is based on creating a systemd unit for each managed mount,
through a unit file in /etc/systemd/system. These files are managed by wrapped
File resources now. Then a wrapped Svc resource manages the lifecycle of the
mount. This works even after removing the unit definition from systemd, because
as long as the filesystem is still mounted, systemd will be able to derive a
transient unit that can be managed.

This also simplifies the logic of the MountRes a lot, because we don't have to
be so careful anymore about matching existing fstab entries to what is expected
by a given Mount resource. We just overwrite the unit definition with whatever
is specified, and rely on systemd to handle the details. Systemd will even
create mount points if they don't yet exist.

Currently this approach is not yet equipped to handle the case that a mount
could be defined via fstab already, or through a unit file that is in a
different location than /etc/systemd/system. As such, this second iteration
should be considered as a partial (if more rounded than the first one)
solution.

Fixes purpleidea#525
ffrank added a commit to ffrank/mgmt that referenced this issue Mar 6, 2024
The mount approach was flawed. It relied on editing fstab, then restarting the
systemd service that is responsible for mounting all defined filesystems. Among
other issues, this approach made it impossible for mgmt to unmount managed
filesystems, because after removing them from fstab, they would just be ignored
by systemd.

The new approach is based on creating a systemd unit for each managed mount,
through a unit file in /etc/systemd/system. These files are managed by wrapped
File resources now. Then a wrapped Svc resource manages the lifecycle of the
mount. This works even after removing the unit definition from systemd, because
as long as the filesystem is still mounted, systemd will be able to derive a
transient unit that can be managed.

This also simplifies the logic of the MountRes a lot, because we don't have to
be so careful anymore about matching existing fstab entries to what is expected
by a given Mount resource. We just overwrite the unit definition with whatever
is specified, and rely on systemd to handle the details. Systemd will even
create mount points if they don't yet exist.

Currently this approach is not yet equipped to handle the case that a mount
could be defined via fstab already, or through a unit file that is in a
different location than /etc/systemd/system. As such, this second iteration
should be considered as a partial (if more rounded than the first one)
solution.

Fixes purpleidea#525
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants