gluster / gdeploy

gdeploy - an Ansible based tool to deploy GlusterFS
GNU General Public License v3.0
91 stars 69 forks source link

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

Open ShyamsundarR opened 7 years ago

ShyamsundarR commented 7 years ago
- 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 commented 7 years ago

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 commented 7 years ago

@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