Non destructive disk test improvements

README.md

@@ -73,6 +73,9 @@ The `mount_point` specifies the directory on which the file system will be mount
 ##### `mount_options`
 The `mount_options` specifies custom mount options as a string, e.g.: 'ro'.
 
+#### `storage_safe_mode`
+When true (the default), an error will occur instead of automatically removing existing devices and/or formatting.
+
 
 Example Playbook
 ----------------

defaults/main.yml

@@ -3,6 +3,7 @@
 storage_provider: "blivet"
 storage_use_partitions: null
 storage_disklabel_type: null  # leave unset to allow the role to select an appropriate label type
+storage_safe_mode: true  # fail instead of implicitly/automatically removing devices or formatting
 
 storage_pool_defaults:
   state: "present"

library/blivet.py

@@ -31,6 +31,9 @@
     disklabel_type:
         description:
             - disklabel type string (eg: 'gpt') to use, overriding the built-in logic in blivet
+    safe_mode:
+        description:
+            - boolean indicating that we should fail rather than implicitly/automatically removing devices or formatting
 
 author:
     - David Lehman ([email protected])
@@ -116,13 +119,15 @@
 
 use_partitions = None  # create partitions on pool backing device disks?
 disklabel_type = None  # user-specified disklabel type
+safe_mode = None       # do not remove any existing devices or formatting
+packages_only = None   # only set things up enough to get a list of required packages
 
 
 class BlivetAnsibleError(Exception):
     pass
 
 
-class BlivetVolume:
+class BlivetVolume(object):
     def __init__(self, blivet_obj, volume, bpool=None):
         self._blivet = blivet_obj
         self._volume = volume
@@ -210,11 +215,16 @@ def _resize(self):
 
     def _reformat(self):
         """ Schedule actions as needed to ensure the volume is formatted as specified. """
+        global packages_only
+
         fmt = self._get_format()
         if self._device.format.type == fmt.type:
             return
 
-        if self._device.format.status:
+        if safe_mode and (self._device.format.type is not None or self._device.format.name != get_format(None).name):
+            raise BlivetAnsibleError("cannot remove existing formatting on volume '%s' in safe mode" % self._volume['name'])
+
+        if self._device.format.status and not packages_only:
             self._device.format.teardown()
         self._blivet.format_device(self._device, fmt)
 
@@ -255,6 +265,19 @@ def _get_device_id(self):
     def _type_check(self):
         return self._device.is_disk
 
+    def _look_up_device(self):
+        super(BlivetDiskVolume, self)._look_up_device()
+        if not self._get_device_id():
+            # FAIL: no disks specified for volume
+            raise BlivetAnsibleError("no disks specified for volume '%s'" % self._volume['name'])  # sure about this one?
+        elif not isinstance(self._volume['disks'], list):
+            raise BlivetAnsibleError("volume disks must be specified as a list")
+
+        if self._device is None:
+            # FAIL: failed to find the disk
+            raise BlivetAnsibleError("unable to resolve disk specified for volume '%s' (%s)" % (self._volume['name'], self._volume['disks']))
+
+
 
 class BlivetPartitionVolume(BlivetVolume):
     def _type_check(self):
@@ -346,7 +369,7 @@ def _get_blivet_volume(blivet_obj, volume, bpool=None):
     return _BLIVET_VOLUME_TYPES[volume_type](blivet_obj, volume, bpool=bpool)
 
 
-class BlivetPool:
+class BlivetPool(object):
     def __init__(self, blivet_obj, pool):
         self._blivet = blivet_obj
         self._pool = pool
@@ -428,8 +451,11 @@ def _create_members(self):
         """ Schedule actions as needed to ensure pool member devices exist. """
         members = list()
         for disk in self._disks:
-            if not disk.isleaf:
-                self._blivet.devicetree.recursive_remove(disk)
+            if not disk.isleaf or disk.format.type is not None:
+                if not safe_mode:
+                    self._blivet.devicetree.recursive_remove(disk)
+                else:
+                    raise BlivetAnsibleError("cannot remove existing formatting and/or devices on disk '%s' (pool '%s') in safe mode" % (disk.name, self._pool['name']))
 
             if use_partitions:
                 label = get_format("disklabel", device=disk.path)
@@ -490,7 +516,10 @@ def _look_up_device(self):
     def _create(self):
         if self._device.format.type != "disklabel" or \
            self._device.format.label_type != disklabel_type:
-            self._blivet.devicetree.recursive_remove(self._device, remove_device=False)
+            if not safe_mode:
+                self._blivet.devicetree.recursive_remove(self._device, remove_device=False)
+            else:
+                raise BlivetAnsibleError("cannot remove existing formatting and/or devices on disk '%s' (pool '%s') in safe mode" % (self._device.name, self._pool['name']))
 
             label = get_format("disklabel", device=self._device.path, label_type=disklabel_type)
             self._blivet.format_device(self._device, label)
@@ -524,7 +553,7 @@ def _create(self):
 
 
 _BLIVET_POOL_TYPES = {
-    "disk": BlivetPartitionPool,
+    "partition": BlivetPartitionPool,
     "lvm": BlivetLVMPool
 }
 
@@ -554,7 +583,7 @@ def manage_pool(b, pool):
         volume['_mount_id'] = bvolume._volume.get('_mount_id', '')
 
 
-class FSTab:
+class FSTab(object):
     def __init__(self, blivet_obj):
         self._blivet = blivet_obj
         self._entries = list()
@@ -660,6 +689,7 @@ def run_module():
         volumes=dict(type='list'),
         packages_only=dict(type='bool', required=False, default=False),
         disklabel_type=dict(type='str', required=False, default=None),
+        safe_mode=dict(type='bool', required=False, default=False),
         use_partitions=dict(type='bool', required=False, default=True))
 
     # seed the result dict in the object
@@ -692,6 +722,12 @@ def run_module():
     global use_partitions
     use_partitions = module.params['use_partitions']
 
+    global safe_mode
+    safe_mode = module.params['safe_mode']
+
+    global packages_only
+    packages_only = module.params['packages_only']
+
     b = Blivet()
     b.reset()
     fstab = FSTab(b)

tasks/main-blivet.yml

@@ -38,7 +38,7 @@
     _storage_vols_no_defaults: "{{ _storage_vols_no_defaults|default([]) }} + [{{ item.1 }}]"
     _storage_vol_defaults: "{{ _storage_vol_defaults|default([]) }} + [{{ storage_volume_defaults }}]"
     _storage_vol_pools: "{{ _storage_vol_pools|default([]) }} + ['{{ item.0.name }}']"
-  loop: "{{ _storage_pools|subelements('volumes') }}"
+  loop: "{{ _storage_pools|subelements('volumes', skip_missing=true) }}"
   when: storage_pools is defined
 
 - name: Apply defaults to pools and volumes [3/6]
@@ -85,6 +85,15 @@
 - debug:
     var: _storage_volumes
 
+- name: load mount facts
+  setup:
+    gather_subset: '!all,!min,mounts'
+  register: __storage_mounts_before_packages
+
+# - name: show mounts before get required packages
+#   debug:
+#     var: __storage_mounts_before_packages
+
 - name: get required packages
   blivet:
     pools: "{{ _storage_pools }}"
@@ -94,6 +103,30 @@
     packages_only: true
   register: package_info
 
+- name: load mount facts
+  setup:
+    gather_subset: '!all,!min,mounts'
+  register: __storage_mounts_after_packages
+
+- name: detect mount alteration by 'get required packages'
+  block:
+    - name: show mounts before manage the pools and volumes
+      debug:
+        var: __storage_mounts_before_packages.ansible_facts.ansible_mounts
+
+    - name: show mounts after manage the pools and volumes
+      debug:
+        var: __storage_mounts_after_packages.ansible_facts.ansible_mounts
+
+    - name: fail if mounts changed
+      fail:
+        msg: "get required packages changed mounts. Changed status is
+      {{ package_info.changed }}"
+  when:
+    - __storage_mounts_before_packages.ansible_facts.ansible_mounts |
+      count !=
+      __storage_mounts_after_packages.ansible_facts.ansible_mounts | count
+
 - name: make sure required packages are installed
   package:
     name: "{{ package_info.packages }}"
@@ -105,6 +138,7 @@
     volumes: "{{ _storage_volumes }}"
     use_partitions: "{{ storage_use_partitions }}"
     disklabel_type: "{{ storage_disklabel_type }}"
+    safe_mode: "{{ storage_safe_mode }}"
   register: blivet_output
 
 - debug:

tests/tests_change_disk_fs.yml

@@ -2,6 +2,7 @@
 - hosts: all
   become: true
   vars:
+    storage_safe_mode: false
     mount_location: '/opt/test'
     volume_size: '5g'
     fs_type_after: "{{ 'ext3' if (ansible_distribution == 'RedHat' and ansible_distribution_major_version == '6') else 'ext4' }}"

tests/tests_change_fs.yml

@@ -2,6 +2,7 @@
 - hosts: all
   become: true
   vars:
+    storage_safe_mode: false
     mount_location: '/opt/test1'
     volume_size: '5g'
     fs_after: "{{ (ansible_distribution == 'RedHat' and ansible_distribution_major_version == '6') | ternary('ext4', 'xfs') }}"

tests/tests_change_fs_use_partitions.yml

@@ -2,6 +2,7 @@
 - hosts: all
   become: true
   vars:
+    storage_safe_mode: false
     storage_use_partitions: true
     mount_location: '/opt/test1'
     volume_size: '5g'

tests/tests_create_disk_then_remove.yml

@@ -2,6 +2,7 @@
 - hosts: all
   become: true
   vars:
+    storage_safe_mode: false
     mount_location: '/opt/test1'
 
   tasks:

tests/tests_create_lvm_pool_then_remove.yml

@@ -2,6 +2,7 @@
 - hosts: all
   become: true
   vars:
+    storage_safe_mode: false
     mount_location1: '/opt/test1'
     mount_location2: '/opt/test2'
     volume_group_size: '10g'

tests/tests_create_partition_volume_then_remove.yml

@@ -2,6 +2,7 @@
 - hosts: all
   become: true
   vars:
+    storage_safe_mode: false
     mount_location: '/opt/test1'
 
   tasks:
@@ -18,7 +19,7 @@
       vars:
         storage_pools:
           - name: "{{ unused_disks[0] }}"
-            type: disk
+            type: partition
             disks: "{{ unused_disks }}"
             volumes:
               - name: test1
@@ -33,7 +34,7 @@
       vars:
         storage_pools:
           - name: "{{ unused_disks[0] }}"
-            type: disk
+            type: partition
             disks: "{{ unused_disks }}"
             volumes:
               - name: test1
@@ -48,7 +49,7 @@
       vars:
         storage_pools:
           - name:  "{{ unused_disks[0] }}"
-            type: disk
+            type: partition
             disks: "{{ unused_disks }}"
             state: absent
             volumes:
@@ -65,7 +66,7 @@
       vars:
         storage_pools:
           - name:  "{{ unused_disks[0] }}"
-            type: disk
+            type: partition
             disks: "{{ unused_disks }}"
             state: absent
             volumes: