New Module: nfsexport #296

Open
wants to merge 1 commit into
from

Projects

None yet

7 participants

@randynobx

Created a new module for working with entries in /etc/exports (or nfs exports file in an otherwise specified location).

Module is nfsexport.py under the systems group.

@randynobx randynobx added nfsexport.py
updated version_added from 1.9 to 2.0
82142c9
@gregdek
Contributor
gregdek commented Jun 17, 2015

Thanks for submitting this module to Ansible Extras. Apologies that it’s taken a while to get your module reviewed.

To help facilitate reviews, we’ve broadened the number of people who can approve modules for inclusion into the Extras repository. The list of official reviewers can be found here:

https://github.com/ansible/ansible-modules-extras/blob/devel/REVIEWERS.md

Our new policy is that if a new module is reviewed and approved by at least two official module reviewers, the module will be approved for inclusion. We will be asking the community of reviewers to take a look at these modules on a regular basis.

To ensure that your module has the best chance of being approved, please double-check that you adhere to the Ansible module guidelines: http://docs.ansible.com/developing_modules.html#module-checklist

@robynbergeron robynbergeron removed the P4 label Sep 25, 2015
@robynbergeron
Contributor

Adding new process. We will be evaluating all new module PRs according to this process, effective immediately.

Thanks for submitting this new module to Ansible Extras! This module is now in community review, a process that is open to all Ansible users. In order for this module to be approved, it must gain the following votes:

“works_for_me”: If you have tested the module thoroughly, including testing of all of the module’s options, and if the module works for you, please add “works_for_me” in the comments.

“passes_guidelines”: If you have gone through the module guidelines and the module meets all of the requirements, please add “passes_guidelines” in the comments. Guidelines are available here: http://docs.ansible.com/developing_modules.html#module-checklist

“needs_revision”: If the module fails to work for you, or if it doesn’t meet guidelines, please add “needs_revision” in the comments with details about what needs to be fixed.

When a module has both “works_for_me” and “passes_guidelines” tags, we will promote the module for inclusion in Ansible Extras. At this point, you will be expected to maintain the module by fixing bugs and evaluating pull requests in a timely manner.

Thanks again for submitting your Ansible module!

@robinro
Contributor
robinro commented Dec 13, 2015

/etc/exports allows to have the same path in multiple lines with different clients, that are then combined. This module does not take this into account so far.

As far as I'm aware this is a seldomly used feature, so it might be ok to fix this later.

@Spredzy Spredzy commented on the diff May 31, 2016
system/nfsexport.py
+ description:
+ - export file to write.
+ required: false
+ default: /etc/exports
+ clients:
+ description:
+ - Address o list of addresses of the nfs clients
+ required: false
+ default: "'*', which means `all`"
+ options:
+ description:
+ - nfs export options.
+ required: false
+ default: ro,root_squash
+ aliases: ['opt', 'opts']
+examples:
@Spredzy
Spredzy May 31, 2016 Contributor

Based on the guidelines[1] this should be in its own section

[1] http://docs.ansible.com/ansible/developing_modules.html#example

@Spredzy Spredzy commented on the diff May 31, 2016
system/nfsexport.py
+import os
+
+def main():
+ module = AnsibleModule(
+ argument_spec = dict(
+ path = dict(required=True),
+ dest = dict(required=False, default='/etc/exports'),
+ clients = dict(required=False, default='*'),
+ options = dict(required=False, default='ro,root_squash', aliases=['opt', 'opts']),
+ ),
+ supports_check_mode=True
+ )
+
+ params = module.params
+ params['path'] = params['path'].strip()
+ client_list = params['clients'] \
@Spredzy
Spredzy May 31, 2016 Contributor

Can we add an example where the clients is a list in the documentation. So people know how to use this module in such a case

@Spredzy Spredzy commented on the diff May 31, 2016
system/nfsexport.py
+ fd.close()
+ module.exit_json(changed=True)
+
+ # Export path not present, proceed and add one.
+ if module.check_mode:
+ module.exit_json(changed=True)
+ try:
+ fd = open(params['dest'], 'a')
+ except IOError, err:
+ module.fail_json(msg="failed to open dest for writing: %s" % str(err))
+ fd.write(exportline)
+ fd.close()
+ module.exit_json(changed=True)
+
+from ansible.module_utils.basic import *
+main()
@Spredzy
Spredzy May 31, 2016 Contributor

It seems to be a community practice to have

if __name__ == '__main__':
    main()

Can this be added here ?

@Spredzy Spredzy commented on the diff May 31, 2016
system/nfsexport.py
+ - nfsexport: path=/nfs/share
+ - nfsexport: path=/home/user dest=/etc/nfsexports
+ - nfsexport: path=/nfs/share opts=ro,sync
+"""
+import os
+
+def main():
+ module = AnsibleModule(
+ argument_spec = dict(
+ path = dict(required=True),
+ dest = dict(required=False, default='/etc/exports'),
+ clients = dict(required=False, default='*'),
+ options = dict(required=False, default='ro,root_squash', aliases=['opt', 'opts']),
+ ),
+ supports_check_mode=True
+ )
@Spredzy
Spredzy May 31, 2016 Contributor

nit: Wrong indentation here

@Spredzy Spredzy commented on the diff May 31, 2016
system/nfsexport.py
@@ -0,0 +1,126 @@
+#!/usr/bin/env python
@Spredzy
Spredzy May 31, 2016 Contributor

The shebang should always be #!/usr/bin/python, this allows ansible_python_interpreter to work[1]

[1] http://docs.ansible.com/ansible/developing_modules.html#module-checklist

@Spredzy
Contributor
Spredzy commented May 31, 2016

Thanks @randynobx for this PR. I've tested it and it works as advertised. Few remarks:

/nfs/share 127.0.0.1(rw) 192.168.0.0/24(ro)

How would I specify that ?

Else, once the comment addressed it will look good to me.

@ansibot
ansibot commented Dec 7, 2016

This repository has been locked. All new issues and pull requests should be filed in https://github.com/ansible/ansible

Please read through the repomerge page in the dev guide. The guide contains links to tools which automatically move your issue or pull request to the ansible/ansible repo.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment