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

SmartOS/OmniOS - mount module fails. #14915

Closed
johngrasty opened this issue Aug 12, 2014 · 7 comments
Closed

SmartOS/OmniOS - mount module fails. #14915

johngrasty opened this issue Aug 12, 2014 · 7 comments
Labels
Bug broken, incorrect, or confusing behavior Execution-Module fixed-pls-verify fix is linked, bug author to confirm fix P3 Priority 3 Platform Relates to OS, containers, platform-based utilities like FS, system based apps severity-medium 3rd level, incorrect or bad functionality, confusing and lacks a work around
Milestone

Comments

@johngrasty
Copy link
Contributor

https://github.com/saltstack/salt/blob/develop/salt/modules/mount.py#L36

The mount module runs mount -l on line 36 (of the develop branch, 2014.1 does the same though) in the _list_mounts function. -l is an illegal option on SmartOS/OmniOS (and I assume other illumos distros.

I believe that it is used for returning the disk labels in linux. In FreeBSD, -l is used "used in conjunction with the -a option, also mount those file systems which are marked as “late”." While it does no harm, it might be best to not use it on FreeBSD as well.

Here is the error output on the minion's logs:

[ERROR   ] Command 'mount -l' failed with return code: 1
[ERROR   ] stderr: mount: illegal option -- l
Usage:
mount [-v | -p]
mount [-F FSType] [-V] [current_options] [-o specific_options]
        {special | mount_point}
mount [-F FSType] [-V] [current_options] [-o specific_options]
        special mount_point
mount -a [-F FSType ] [-V] [current_options] [-o specific_options]
        [mount_point ...]
[ERROR   ] An exception occurred in this state: Traceback (most recent call last):
  File "salt/state.py", line 1379, in call
  File "salt/states/mount.py", line 87, in mounted
  File "salt/modules/mount.py", line 108, in active
  File "salt/modules/mount.py", line 30, in _active_mountinfo
  File "salt/modules/mount.py", line 25, in _list_mounts
IndexError: list index out of rang
@basepi
Copy link
Contributor

basepi commented Aug 12, 2014

Good catch! We'll get this fixed.

@basepi basepi added this to the Approved milestone Aug 12, 2014
@ssgward
Copy link

ssgward commented Jun 30, 2015

@johngrasty - is this still an issue? This was logged long ago. Wondering if the latest Salt still shows the problem.

@ssgward ssgward added severity-medium 3rd level, incorrect or bad functionality, confusing and lacks a work around P3 Priority 3 Platform Relates to OS, containers, platform-based utilities like FS, system based apps Execution-Module and removed severity-low 4th level, cosemtic problems, work around exists labels Jun 30, 2015
@johngrasty
Copy link
Contributor Author

Yes. It appears that the code has not been changed. I think there would also be significant problems with other parts the mount module for illumos based distributions. I tested many of the ones available with 100% failure rate.

@johngrasty
Copy link
Contributor Author

This may be a start, though I am not a programmer.

def _list_mounts():
    ret = {}
    if __grains__['os'] in ['MacOS', 'Darwin']:
        mounts = __salt__['cmd.run_stdout']('mount')
    if __grains__['os_family'] in ['Solaris']:
        mounts = __salt__['cmd.run_stdout']('mount')
    else:
        mounts = __salt__['cmd.run_stdout']('mount -l')

    for line in mounts.split('\n'):
        comps = re.sub(r"\s+", " ", line).split()
        ret[comps[2]] = comps[0]
    return ret

@sjorge
Copy link
Contributor

sjorge commented Jun 21, 2016

@sjorge tagging my self, I can replicate.
Will have a look once I have a bit more free time.

rallytime pushed a commit that referenced this issue Jun 24, 2016
* mount.swaps now works on solarish like systems

* mount.swapon / mount.swapoff supported on solarish platforms

* mount.active (and internal consumers) now work on all solarish platforms

* unit-tests - mount_tests should set kernel grain
@rallytime
Copy link
Contributor

@johngrasty How does the fix in #34254 look to you?

@rallytime rallytime added the fixed-pls-verify fix is linked, bug author to confirm fix label Jun 24, 2016
gitebra pushed a commit to gitebra/salt that referenced this issue Jun 27, 2016
* upstream/develop:
  Fix pylint error
  Remove test that doesn't actually test anything
  Don't escape source before calling managed
  Fix for saltstack#14915 (saltstack#34254)
  Fixed symlinks on windows where the slashes don't match
  ipset: fix the comment containing blank
  Use 'config_dir' setting instead of CONFIG_DIR in gpg renderer
  ipset: fix commont containing blank
  Fix win_system.set_system_date_time
  win_pkg: refresh pkg database if refresh=True passed to version() or list_pkgs()
  Catch CommandExecutionError in pkg states
  some cleanup and renaming
  better way to check for openSUSE Leap
  Fix for SUSE OS grains in 2015.8
@rallytime
Copy link
Contributor

This should be fixed by #34254. Since we haven't heard back, I am closing this. If this pops up again, leave a comment and we can re-open and take another look. Thanks all!

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 Execution-Module fixed-pls-verify fix is linked, bug author to confirm fix P3 Priority 3 Platform Relates to OS, containers, platform-based utilities like FS, system based apps severity-medium 3rd level, incorrect or bad functionality, confusing and lacks a work around
Projects
None yet
Development

No branches or pull requests

5 participants