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

backend_reset from a play does not work as intended? #427

Open
ShyamsundarR opened this issue Aug 15, 2017 · 2 comments
Open

backend_reset from a play does not work as intended? #427

ShyamsundarR opened this issue Aug 15, 2017 · 2 comments

Comments

@ShyamsundarR
Copy link

- hosts: <myhost>
  tasks:
    - name: Cleanup disks
      backend_reset: pvs="{{ item }}"
                     unmount=yes
      with_items:
        - /dev/sdb

The above play does not remove the said PV, when the PV is present (additional VG,LV, mounts being present or not does not make a difference.

Changing the with_items as follows also does not help,

- '/dev/sdb'
- "'/dev/sdb'"
- "'/dev/sdb' '/dev/sdc'"

or changing pvs to,

backend_reset: pvs={{ item }}

also fails

The issue seems to stem from the literav_eval as pointed in the issue #247 which evaluates the end self.pvs to None, and hence nothing is done on the system, and further the play returns as a success.

The diff as follows, helps the situation and works for the above play, as well as for plays that pass in a list of devices,

diff --git a/modules/backend_reset.py b/modules/backend_reset.py
index e2e71cd..546b5eb 100644
--- a/modules/backend_reset.py
+++ b/modules/backend_reset.py
@@ -31,13 +31,10 @@ class BackendReset(object):
     def __init__(self, module):
         self.output = []
         self.module = module
-        try:
-            self.pvs = literal_eval(self.validated_params('pvs'))
-        except:
-            self.pvs = None
+        self.pvs = self.module.params.get('pvs') or None
         self.vgs = self.module.params.get('vgs') or None
         self.lvs = self.module.params.get('lvs') or None
-        self.unmount = self.module.params.get('unmount') or 'no'
+        self.unmount = self.module.params.get('unmount') or False
         self.mountpoints = self.module.params.get('mountpoints') or None
         self.remove_lvs()
         self.remove_vgs()
@@ -93,7 +90,7 @@ class BackendReset(object):
         self.output.append([rc, out, err])
 
     def umount_bricks(self):
-        if not self.unmount or literal_eval(self.unmount)[0].lower() != 'yes':
+        if not self.unmount:
             return
         if not self.mountpoints:
             return
@@ -157,10 +154,10 @@ class BackendReset(object):
 if __name__ == '__main__':
     module = AnsibleModule(
         argument_spec=dict(
-            pvs=dict(),
+            pvs=dict(type='list'),
             vgs=dict(),
             lvs=dict(type='str'),
-            unmount=dict(),
+            unmount=dict(type='bool', default='no'),
             mountpoints=dict(),
         ),
     )

The above is not submitted as a PR as I assume gdeploy may not function given the changes, and as this is being used from a play and that works, it is left here for consideration when making backend_reset a module for ansible.

@sac
Copy link
Member

sac commented Aug 17, 2017

Hi @ShyamsundarR the backend-rest works for me. These are the variables used in group_vars/all :

lvs:
- GLUSTER_lv1
- GLUSTER_lv2
master: 10.70.42.122
mountpoints:
- /gluster/brick1
- /gluster/brick2
pvs:
- /dev/vdb
- /dev/vdc
unmount:
- 'yes'
vgs:
- GLUSTER_vg1
- GLUSTER_vg2 

And this is the playbook:

---
- hosts: gluster_servers
  remote_user: root
  gather_facts: no

  tasks:
  - name: Cleans up backend
    backend_reset: pvs="{{ pvs }}"
                   vgs="{{ vgs }}"
                   lvs="{{ lvs }}"
                   unmount="{{ unmount }}"
                   mountpoints="{{ mountpoints }}"

@sac
Copy link
Member

sac commented Aug 17, 2017

@ShyamsundarR this worked too:

---
- hosts: gluster_servers
  remote_user: root
  gather_facts: no

  tasks:
  - name: Cleans up backend
    backend_reset: pvs="{{ item }}"
                   unmount="yes"
    with_items:
      - /dev/vdb
      - /dev/vdc

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants