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: fix extra -t parameter #51905

Merged
merged 1 commit into from Mar 5, 2019

Conversation

@aplanas
Copy link
Contributor

commented Feb 28, 2019

If 'fstype' parameter is not set in Linux environments, salt will
build a mount command with an empty -t value, making the command
fail.

Note that this code needs to be backported to 2019.2, as this bug is also present there.

@aplanas

This comment has been minimized.

Copy link
Contributor Author

commented Feb 28, 2019

For reference, this bug was introduced here:

b701e16#diff-85764a41d31cd335e1c8d99383999e1e

@aplanas

This comment has been minimized.

Copy link
Contributor Author

commented Feb 28, 2019

Ouch, is already in 2018.X, so needs to be backported there too.

@aplanas aplanas force-pushed the aplanas:fix_mount branch from fb0ecec to 828c01d Feb 28, 2019
@aplanas aplanas changed the base branch from develop to 2018.3 Feb 28, 2019
@aplanas aplanas force-pushed the aplanas:fix_mount branch 2 times, most recently from 828c01d to b30a5d5 Feb 28, 2019
@aplanas

This comment has been minimized.

Copy link
Contributor Author

commented Feb 28, 2019

Based on IRC comment, and after reading [1] I changed the base branch from develop to 2018.3. I hope that this make the code move to 2019.2 and develop.

[1] https://docs.saltstack.com/en/develop/topics/development/contributing.html#which-salt-branch

@brejoc
brejoc approved these changes Mar 1, 2019
Copy link
Member

left a comment

lgtm

salt/modules/mount.py Outdated Show resolved Hide resolved
@aplanas aplanas force-pushed the aplanas:fix_mount branch from b30a5d5 to 273015f Mar 1, 2019
@dwoz

This comment has been minimized.

Copy link
Contributor

commented Mar 5, 2019

@aplanas Since this if fixing a bug, can you write a test for it please?

@aplanas

This comment has been minimized.

Copy link
Contributor Author

commented Mar 5, 2019

@dwoz makes sense!

@aplanas aplanas force-pushed the aplanas:fix_mount branch from 273015f to dd18bb8 Mar 5, 2019
If 'fstype' parameter is not set in Linux environments, salt will
build a mount command with an empty -t value, making the command
fail.
@aplanas aplanas force-pushed the aplanas:fix_mount branch from dd18bb8 to ac688df Mar 5, 2019
@aplanas

This comment has been minimized.

Copy link
Contributor Author

commented Mar 5, 2019

@dwoz I updated the test_mount code. I think that this test needs some refactoring, but I also think that is out of scope of this pull request

@dwoz

This comment has been minimized.

Copy link
Contributor

commented Mar 5, 2019

@aplanas I agree we should break up that monster test :D. Looks good for this PR though, thanks!

@dwoz dwoz merged commit 2c4dff6 into saltstack:2018.3 Mar 5, 2019
10 checks passed
10 checks passed
WIP Ready for review
Details
continuous-integration/jenkins/pr-merge This commit looks good
Details
jenkins/pr/docs The docs job has passed
Details
jenkins/pr/lint Python lint test has passed
Details
jenkins/pr/py2-centos-7 The py2-centos-7 job has passed
Details
jenkins/pr/py2-ubuntu-1604 The py2-ubuntu-1604 job has passed
Details
jenkins/pr/py2-windows-2016 The py2-windows-2016 job has passed
Details
jenkins/pr/py3-centos-7 The py3-centos-7 job has passed
Details
jenkins/pr/py3-ubuntu-1604 The py3-ubuntu-1604 job has passed
Details
jenkins/pr/py3-windows-2016 The py3-windows-2016 job has passed
Details
@aplanas aplanas deleted the aplanas:fix_mount branch Mar 6, 2019
@aplanas

This comment has been minimized.

Copy link
Contributor Author

commented Mar 6, 2019

Thanks for the reviews!

Do I need to port it to 2019.2 and develop? Or is there any process that will take care of it?

aplanas added a commit to aplanas/salt-1 that referenced this pull request Mar 6, 2019
* Add root and no_recommends parameters in the public interface for Zypper and RPM (lowpkg)
  saltstack/salt#50125

* Add root parameter to useradd, shadow and groupadd
  saltstack/salt#50175

* cmd: Add root parameter for wait and run states
  saltstack/salt#50302

* systemd: add optional root parameter
  saltstack/salt#50380

* Add new chroot module
  https://github.com/openSUSE/salt/pull/50418

* Add new module freezer
  saltstack/salt#50452

* btrfs: add all subvolume commands
  saltstack/salt#50541

* btrfs: add new btrfs state
  saltstack/salt#50635

* zypper: demote log from error to warning
  saltstack/salt#50671

* blkid: add search by token
  saltstack/salt#50706

* mount: add fstab_{present,absent} states
  saltstack/salt#50725

* btrfs: add option to not set subvolumes as default
  saltstack/salt#50801

* Add disk_set and disk_toggle functions, and update valid partition flags
  saltstack/salt#50834

* disk: support setting FAT size for format_
  saltstack/salt#51074

* parted: fix set_ valid flags comment.
  saltstack/salt#51704

* grains/core: ignore HOST_NOT_FOUND errno in fqdns()
  saltstack/salt#51706

* cmdmod: add 'binds' parameter in run_chroot
  saltstack/salt#51871

* mount: fix extra -t parameter
  saltstack/salt#51905

* lvm: be quiet when a pv, lv or vg is not expected
  saltstack/salt#51929

* linux_lvm: clean error in pvcreate and pvremove
  saltstack/salt#51954

* blockdev: hide blkid errors when are expected
  saltstack/salt#51956

* partially unify public functions signature for pkg and lowpkg
  saltstack/salt#51973

* extmods: add utils directories in sys.path
  saltstack/salt#52001
aplanas added a commit to aplanas/salt-1 that referenced this pull request Jun 5, 2019
* Add root and no_recommends parameters in the public interface for Zypper and RPM (lowpkg)
  saltstack/salt#50125

* Add root parameter to useradd, shadow and groupadd
  saltstack/salt#50175

* cmd: Add root parameter for wait and run states
  saltstack/salt#50302

* systemd: add optional root parameter
  saltstack/salt#50380

* Add new chroot module
  https://github.com/openSUSE/salt/pull/50418

* Add new module freezer
  saltstack/salt#50452

* btrfs: add all subvolume commands
  saltstack/salt#50541

* btrfs: add new btrfs state
  saltstack/salt#50635

* zypper: demote log from error to warning
  saltstack/salt#50671

* blkid: add search by token
  saltstack/salt#50706

* mount: add fstab_{present,absent} states
  saltstack/salt#50725

* btrfs: add option to not set subvolumes as default
  saltstack/salt#50801

* Add disk_set and disk_toggle functions, and update valid partition flags
  saltstack/salt#50834

* disk: support setting FAT size for format_
  saltstack/salt#51074

* parted: fix set_ valid flags comment.
  saltstack/salt#51704

* grains/core: ignore HOST_NOT_FOUND errno in fqdns()
  saltstack/salt#51706

* cmdmod: add 'binds' parameter in run_chroot
  saltstack/salt#51871

* mount: fix extra -t parameter
  saltstack/salt#51905

* lvm: be quiet when a pv, lv or vg is not expected
  saltstack/salt#51929

* linux_lvm: clean error in pvcreate and pvremove
  saltstack/salt#51954

* blockdev: hide blkid errors when are expected
  saltstack/salt#51956

* partially unify public functions signature for pkg and lowpkg
  saltstack/salt#51973

* extmods: add utils directories in sys.path
  saltstack/salt#52001
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.