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

State mount.fstab_present respects mount parameter #57669

Conversation

piterpunk
Copy link
Collaborator

What does this PR do?

The mount=true parameter to mount a fstab entry after created by mount.fstab_present was in documentation but there isn't code to handle it.

Also, the actual mount=true parameter was being passed to mount.set_filesystems to totally different finality, "mount" is used in AIX /etc/filesystems to mark a filesystem to be mounted by command "mount all"

  1. Changes mount.fstab_present to send fs_mount parameter to set_filesystems.
  2. Changes module mount.mount to allow only one parameter, so it can mount filesystems from fstab.
  3. Finally, added to mount.fstab_present the code to mount a new fstab entry after created.

What issues does this PR fix or reference?

Fixes: #57560

Previous Behavior

The "mount" parameter was passed to mount.set_filesystems on AIX machines marking filesystems to being mounted by "mount all" command and ignored by the others OSs.

New Behavior

The "mount" parameter is used to mount /etc/fstab entries after created by mount.fstab_present status, as indicated in documentation.

Added the "fs_mount" to be used as input to mount.set_filesystems to AIX machines. Both "mount" and "fs_mount" defaults to "True"

Merge requirements satisfied?

[NOTICE] Bug fixes or features added to Salt require tests.

  • [* ] Tests written/updated

salt-runtests-20200614142849.log

Commits signed with GPG?

No

piterpunk added 8 commits Jun 13, 2020
- mount.fstab_present have a parameter called "mount" which should
  mount an fstab entry after created.
- But this is passed to set_filesystems, which uses it as a property
  of /etc/filesystems in AIX and indicates that a entry will be
  mounted by "mount all" or "mount -a" commands.
- Added the parameter fs_mount which will be used as input to the
  mount paremeter in mount.set_filesystems module.
- Previously the mount.mount needs at least the device and the mount
  point. Using this approach it doesn't use the /etc/fstab entries.
- Change to needs only the mount point.
- You still can use the device, but now is an optional parameter.
- Added the documented but missing feature to mount a filesystem
  after added it to /etc/fstab or /etc/filesystems when parameter
  "mount" is set to "true".
- When using salt-call to apply the state, when mount.fstab_present
  calls disk.blkid the state execution never ends. Works fine when
  called from salt master.
- Workarounded by calling blkid directly in mount.py state.
This reverts commit 09b6840.

Find a better way to fix the problem with salt-call and the
neverending execution
  Still don't know what happens here. But this:

	HAS_HDPARM = salt.utils.path.which("hdparm") is not None
	HAS_IOSTAT = salt.utils.path.which("iostat") is not None

	@salt.utils.decorators.depends(HAS_HDPARM)
	...

	@salt.utils.decorators.depends(HAS_IOSTAT)
	...

  Wasn't working fine with salt-call. Removed the two HAS_SOMETHING
  lines and changed the others that were referencing this variables to:

	@salt.utils.path.which("hdparm")
	@salt.utils.path.which("iostat")

  And now the state is working fine called from salt-call and from
  salt master.
- Now when you run mount.fstab_present state in "test" mode
  it will check for "mount" parameter and emmit a message
  if it will or not try to mount the filesystem
- Includes the check for parameter mount=true or mount=false
  in fstab_present tests.
@piterpunk piterpunk requested a review from a team as a code owner Jun 14, 2020
@ghost ghost requested review from krionbsd and removed request for a team Jun 14, 2020
krionbsd
krionbsd previously approved these changes Jun 15, 2020
@sagetherage sagetherage added Bug broken, incorrect, or confusing behavior Magnesium Mg release after Na prior to Al severity-high 2nd top severity, seen by most users, causes major problems labels Jul 22, 2020
@garethgreenaway garethgreenaway self-assigned this Oct 2, 2020
@dwoz dwoz merged commit aaf663f into saltstack:master Oct 5, 2020
26 checks passed
@piterpunk piterpunk deleted the state-mount.fstab_present-respects-mount-parameter_Fix-Issue-57560 branch Oct 8, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug broken, incorrect, or confusing behavior Magnesium Mg release after Na prior to Al severity-high 2nd top severity, seen by most users, causes major problems
Projects
None yet
Development

Successfully merging this pull request may close these issues.

mount.fstab_present doesn't mount the Filesystem
5 participants