Merge pull request #204 from lae/fix/storage_booleans

Idempotency fixes for proxmox_storage

Author: lae

library/proxmox_role.py

@@ -186,4 +186,4 @@ def main():
     module.exit_json(**result)
 
 if __name__ == '__main__':
-    main()
+    main()

library/proxmox_storage.py

@@ -202,9 +202,9 @@ def lookup(self):
         for item in self.existing_storages:
             if item['storage'] == self.name:
                 # pvesh doesn't return the disable param value if it's false,
-                # so we set it to False.
+                # so we set it to 0, which is what PVE would normally use.
                 if item.get('disable') is None:
-                    item['disable'] = False
+                    item['disable'] = 0
                 return item
         return None
 
@@ -218,13 +218,15 @@ def prepare_storage_args(self):
         args = {}
 
         args['type'] = self.type
-        args['content'] = ','.join(self.content)
+        if self.content is not None and len(self.content) > 0:
+            args['content'] = ','.join(self.content)
+        else:
+            # PVE uses "none" to represent when no content types are selected
+            args['content'] = 'none'
         if self.nodes is not None:
             args['nodes'] = ','.join(self.nodes)
         if self.disable is not None:
-            args['disable'] = self.disable
-        else:
-            args['disable'] = False
+            args['disable'] = 1 if self.disable else 0
         if self.path is not None:
             args['path'] = self.path
         if self.pool is not None:
@@ -234,7 +236,7 @@ def prepare_storage_args(self):
         if self.username is not None:
             args['username'] = self.username
         if self.krbd is not None:
-            args['krbd'] = self.krbd
+            args['krbd'] = 1 if self.krbd else 0
         if self.maxfiles is not None:
             args['maxfiles'] = self.maxfiles
         if self.server is not None:
@@ -248,7 +250,7 @@ def prepare_storage_args(self):
         if self.thinpool is not None:
             args['thinpool'] = self.thinpool
         if self.sparse is not None:
-            args['sparse'] = self.sparse
+            args['sparse'] = 1 if self.sparse else 0
 
         if self.maxfiles is not None and 'backup' not in self.content:
             self.module.fail_json(msg="maxfiles is not allowed when there is no 'backup' in content")
@@ -275,7 +277,8 @@ def modify_storage(self):
 
         for key in new_storage:
             if key == 'content':
-                if set(self.content) != set(lookup.get('content', '').split(',')):
+                if set(new_storage['content'].split(',')) \
+                        != set(lookup.get('content', '').split(',')):
                     updated_fields.append(key)
                     staged_storage[key] = new_storage[key]
             elif key == 'monhost':

tests/vagrant/group_vars/all

@@ -58,6 +58,14 @@ pve_storages:
     type: dir
     content: [ "iso", "vztmpl", "backup" ]
     path: /testpool/zfs2
+  - name: disabled-dir
+    type: dir
+    disable: yes
+    content: [ "images" ]
+    path: /tmp/fakedir
+  - name: no-content-dir
+    type: dir
+    path: /tmp/fakedir2
 pve_zfs_create_volumes:
   - testpool/zfs2
 pve_ceph_osds: