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.

Sign up for GitHub

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

Jump to bottom

(Backport 50725) mount: add fstab_{present,absent} states #54965

Merged
merged 1 commit into from
Dec 5, 2019
Merged

(Backport 50725) mount: add fstab_{present,absent} states #54965

merged 1 commit into from
Dec 5, 2019

Conversation

aplanas
Copy link
Contributor

@aplanas aplanas commented Oct 11, 2019 •
edited
Loading

What does this PR do?

Add two new states in mount:

  • fstab_present: Makes sure that a fstab entry is present, without mounting the device.
  • fstab_absent: Makes sure that a fstab entry is absent, without unmounting the device.

The code is adjusted to work on Linux, MacOS and AIX.

Tests written?

Yes

(backport #50725, already merged in develop)

@aplanas aplanas changed the title (Backport 50725) mount: add fstab_{present,absent} states (Backport 50725, 52962) mount: add fstab_{present,absent} states Oct 14, 2019
@waynew waynew added this to PR needs merge to master in PRs to port to master Oct 17, 2019
@dwoz dwoz requested a review from a team as a code owner November 20, 2019 07:39
@ghost ghost requested a review from xeacott November 20, 2019 07:39
dhiltonp
dhiltonp suggested changes Dec 4, 2019
View reviewed changes
Copy link
Contributor

@dhiltonp dhiltonp left a comment

Choose a reason for hiding this comment

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

Thank you so much for this PR with these 2 backports. Could you split them up in to two separate PRs?

The functionality of these 2 PRs overlaps and I'm not sure both should go in.

@aplanas
Copy link
Contributor Author

aplanas commented Dec 4, 2019

@dhiltonp they overlap, as the second one is build as a change for the first one. You can see it, for example, here: ae34159#diff-ba2bfb2cb504d0deb9fc5af6584c9032L994

@dhiltonp
Copy link
Contributor

dhiltonp commented Dec 4, 2019

Yeah. It is annoying having PRs that depend on each other.

I certainly see why the 52962 port PR would include the 50725 commits, but there is value in also having an individual port PR for 50725.

@aplanas aplanas mentioned this pull request Dec 5, 2019
Merged
@aplanas aplanas changed the title (Backport 50725, 52962) mount: add fstab_{present,absent} states (Backport 50725) mount: add fstab_{present,absent} states Dec 5, 2019
@aplanas
Copy link
Contributor Author

aplanas commented Dec 5, 2019

@dhiltonp very well. I split the PR in two, the other backport is living here in #55516

@aplanas aplanas requested a review from dhiltonp December 5, 2019 09:56
@dwoz dwoz merged commit a68ddcb into saltstack:master Dec 5, 2019
PRs to port to master automation moved this from PR needs merge to master to PR merged Dec 5, 2019
@aplanas aplanas deleted the backport_50725 branch December 9, 2019 09:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dwoz dwoz dwoz approved these changes

@xeacott xeacott Awaiting requested review from xeacott

@dhiltonp dhiltonp Awaiting requested review from dhiltonp

Assignees

@dhiltonp dhiltonp

Labels
master-port
Projects
PRs to port to master
  
PR merged
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@aplanas @dhiltonp @dwoz @garethgreenaway