Repository: ansible/ansible
Pull request: Fix incorrect command-line parameters for Solaris
##### SUMMARY
<!--- Describe the change, including rationale and design decisions -->
Solaris 'mount' command does not have flag to point to fstab file.
So no such flags should be passed to the command. 
Fixes #22135
<!---
If you are fixing an existing issue, please include "Fixes #nnn" in your
commit message and your description; but you should still explain what
the change does.
-->

##### ISSUE TYPE
<!--- Pick one below and delete the rest: -->
 - Bugfix Pull Request


##### COMPONENT NAME
<!--- Name of the module/plugin/module/task -->
mount module

##### ANSIBLE VERSION
<!--- Paste verbatim output from “ansible --version” between quotes below -->
```
ansible 2.2.1.0 and later 

```


##### ADDITIONAL INFORMATION
<!---
Include additional information to help people understand the change here.
For bugs that don't have a linked bug report, a step-by-step reproduction
of the problem is helpful.
  -->

<!--- Paste verbatim command output below, e.g. before and after your change -->
```

```


Problem statement:
Mount  module: incorrect command-line arguments used for Solaris.
<!---
Verify first that your issue/request is not already reported on GitHub.
Also test if the latest release, and master branch are affected too.
-->

##### ISSUE TYPE
<!--- Pick one below and delete the rest: -->
 - Bug Report

##### COMPONENT NAME
<!--- Name of the module/plugin/task/feature -->
mount module 

##### ANSIBLE VERSION
<!--- Paste verbatim output from “ansible --version” between quotes below -->
```
ansible 2.2.1.0
```

##### CONFIGURATION
<!---
Mention any settings you have changed/added/removed in ansible.cfg
(or using the ANSIBLE_* environment variables).
-->

##### OS / ENVIRONMENT
<!---
Mention the OS you are running Ansible from, and the OS you are
managing, or say “N/A” for anything that is not platform-specific.
-->
Running ansible on ubuntu linux 16.04.1
Controlling hosts with Solaris 11.2/11.3

##### SUMMARY
<!--- Explain the problem briefly -->
mount module uses incorrect command-line arguments for SunOS,
"mount" command on Solaris does not support pointing to fstab file, 
but _set_fstab_args(args)  do not check if task is running against SunOS and in this case adds "-T  path_to_fstab..." to command-line arguments. 

##### STEPS TO REPRODUCE
<!---
For bugs, show exactly how to reproduce the problem, using a minimal test-case.
For new features, show how the feature would be used.
-->
Just try any mount task with state "mounted" against Solaris.
<!--- Paste example playbooks or commands between quotes below -->
cat roles/nfs_client/defaults/main.yml 
```yaml
---
nfs_shares:
  - dst:        "/opt/storage"
    dst_owner:  "support"
    dst_mode:   "0755"
    dst_group:  "support"
    src:        "tribune:/opt/storage"

  - dst: "/opt/homes"
    src: "tribune:/opt/nfs/homes"
```
tail  roles/nfs_client/tasks/solaris.yml 
```yaml
  mount:	
    fstype:		"nfs"
    name:		"{{ share.dst }}"
    src:		"{{ share.src }}"
    state:		"mounted"

  with_items:		"{{ nfs_shares }}"
  loop_control:		
    loop_var:		"share"

```

<!--- You can also paste gist.github.com links for larger files -->

##### EXPECTED RESULTS
<!--- What did you expect to happen when running the steps above? -->
According to these var and task, two NFS shares mounted on destination server. 

##### ACTUAL RESULTS
<!--- What actually happened? If possible run with extra verbosity (-vvvv) -->


<!--- Paste verbatim command output between quotes below -->
```
failed: [sulla] (item={u'dst_owner': u'support', u'src': u'tribune:/opt/storage', u'dst': u'/opt/storage', u'dst_group': u'support'}) => {"failed": true, "msg": "Error mounting /opt/storage: /sbin/mount: illegal option -- T\nUsage:\nmount [-v | -p]\nmount [-F FSType] [-V] [current_options] [-o specific_options]\n\t{special | mount_point}\nmount [-F FSType] [-V] [current_options] [-o specific_options]\n\tspecial mount_point\nmount -a [-F FSType ] [-V] [current_options] [-o specific_options]\n\t[mount_point ...]\n", "share": {"dst": "/opt/storage", "dst_group": "support", "dst_owner": "support", "src": "tribune:/opt/storage"}}
```

##### POTENTIAL FIX 
```
ansible@nero:~$ diff ./library/mount.py ./lib/ansible/modules/core/system/mount.py  
313c313
<     if 'fstab' in args and args['fstab'] != '/etc/fstab' and get_platform().lower() != 'sunos' :
---
>     if 'fstab' in args and args['fstab'] != '/etc/fstab':
ansible@nero:~$ 
```

Hints from issue discussion:
@AlexZolotarenko would you be able to submit a pullrequest for that patch?

needs_info
@AlexZolotarenko This issue is waiting for your response. Please respond or the issue will be closed.

[click here for bot help](https://github.com/ansible/ansibullbot/blob/master/ISSUE_HELP.md)
<!--- boilerplate: needs_info_base --->

Commit message: Fix incorrect command-line parameters for Solaris

Patch to review:
diff --git a/lib/ansible/modules/system/mount.py b/lib/ansible/modules/system/mount.py
index e775e8fdabe979..4573dfbcdcbf26 100644
--- a/lib/ansible/modules/system/mount.py
+++ b/lib/ansible/modules/system/mount.py
@@ -327,7 +327,7 @@ def unset_mount(module, args):
 def _set_fstab_args(fstab_file):
     result = []
 
-    if fstab_file and fstab_file != '/etc/fstab':
+    if fstab_file and fstab_file != '/etc/fstab' and get_platform().lower() != 'sunos':
         if get_platform().lower().endswith('bsd'):
             result.append('-F')
         else:


Code review comments:
diff_hunk: @@ -327,7 +327,7 @@ def unset_mount(module, args):
 def _set_fstab_args(fstab_file):
     result = []
 
-    if fstab_file and fstab_file != '/etc/fstab':
+    if fstab_file and fstab_file != '/etc/fstab' and get_platform().lower() != 'sunos':
path: lib/ansible/modules/system/mount.py
text: @user1:
Please could you change it to:

```
    if (
            fstab_file and
            fstab_file != '/etc/fstab' and
            get_platform().lower() != 'sunos'):
```

@author:
Not sure if I did it correct, but it's updated according your request.

@user1:
It looks good!