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

Non destructive disk test improvements #54

Closed
Show file tree
Hide file tree
Changes from 28 commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
939ebeb
Add flag to prevent implicit removal of existing device/fs.
dwlehman Sep 25, 2019
55767dc
Improve accuracy of safe mode description.
dwlehman Sep 25, 2019
5d1645d
Do not protect resize ops in safe mode.
dwlehman Oct 1, 2019
1392ad6
Enable safe mode by default.
dwlehman Oct 1, 2019
efa0cdb
Document safe mode.
dwlehman Oct 1, 2019
26496e8
Actually set safe_mode var in python.
dwlehman Oct 3, 2019
7c9e6ee
Update tests to adjust safe mode as needed.
dwlehman Oct 3, 2019
8fac471
Special case for format destroy on unformatted device.
dwlehman Oct 3, 2019
f0617b4
Fix logic of safe mode guard on reformat.
dwlehman Oct 4, 2019
3f6b84c
Fix key for partition pool class lookup.
dwlehman Oct 15, 2019
e66688d
Update partition pool type in tests.
dwlehman Oct 15, 2019
1e3f2c9
Use correct array syntax for an invalid disk
pcahyna Oct 15, 2019
611553a
Test that the role fails, not that it sets blivet_output.failed
pcahyna Oct 15, 2019
a597f0f
Create a file in the test fs to actually detect data loss
pcahyna Oct 15, 2019
2fd8c33
Do not use ignore_errors: yes in a test block
pcahyna Oct 15, 2019
d2f3d97
Remove last instance of ignore_errors: yes
pcahyna Oct 15, 2019
0494d06
Turn off safe mode in cleanup
pcahyna Oct 15, 2019
afd45f8
Assert that "get required packages" has not changed mounts
pcahyna Oct 16, 2019
57dd5cc
Merge remote-tracking branch 'pcahyna/mount-assert' into non-destructive
dwlehman Oct 18, 2019
0d2ab64
Don't unmount anything if only getting package list.
dwlehman Oct 18, 2019
6d512fd
Add proper checking of specified disk for disk volumes.
dwlehman Oct 18, 2019
5f910b7
Fix error in exception raising when clearing disk for disklabel.
dwlehman Oct 18, 2019
b488658
Don't crash when a pool contains no volumes.
dwlehman Oct 18, 2019
d937f5b
Adjust names of tests to match reality.
dwlehman Oct 18, 2019
dd695e8
Make all classes new-style (derive from object)
pcahyna Oct 21, 2019
09972bf
Safe mode should now be the default, account for this
pcahyna Oct 21, 2019
7f02139
Check that safe mode works even when the filesystem is unmounted.
pcahyna Oct 21, 2019
9a25938
Verify that replacing a fs by a LVM pool fails in safe mode
pcahyna Oct 21, 2019
da350a9
Cleanup properly.
pcahyna Oct 22, 2019
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
----------------
Expand Down
1 change: 1 addition & 0 deletions defaults/main.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
52 changes: 44 additions & 8 deletions library/blivet.py
Original file line number Diff line number Diff line change
Expand Up @@ -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])
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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)

Expand Down Expand Up @@ -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):
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -524,7 +553,7 @@ def _create(self):


_BLIVET_POOL_TYPES = {
"disk": BlivetPartitionPool,
"partition": BlivetPartitionPool,
"lvm": BlivetLVMPool
}

Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand Down
36 changes: 35 additions & 1 deletion tasks/main-blivet.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down Expand Up @@ -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 }}"
Expand All @@ -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 }}"
Expand All @@ -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:
Expand Down
1 change: 1 addition & 0 deletions tests/tests_change_disk_fs.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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' }}"
Expand Down
1 change: 1 addition & 0 deletions tests/tests_change_fs.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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') }}"
Expand Down
1 change: 1 addition & 0 deletions tests/tests_change_fs_use_partitions.yml
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
- hosts: all
become: true
vars:
storage_safe_mode: false
storage_use_partitions: true
mount_location: '/opt/test1'
volume_size: '5g'
Expand Down
1 change: 1 addition & 0 deletions tests/tests_create_disk_then_remove.yml
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
- hosts: all
become: true
vars:
storage_safe_mode: false
mount_location: '/opt/test1'

tasks:
Expand Down
1 change: 1 addition & 0 deletions tests/tests_create_lvm_pool_then_remove.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down
9 changes: 5 additions & 4 deletions tests/tests_create_partition_volume_then_remove.yml
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
- hosts: all
become: true
vars:
storage_safe_mode: false
mount_location: '/opt/test1'

tasks:
Expand All @@ -18,7 +19,7 @@
vars:
storage_pools:
- name: "{{ unused_disks[0] }}"
type: disk
type: partition
disks: "{{ unused_disks }}"
volumes:
- name: test1
Expand All @@ -33,7 +34,7 @@
vars:
storage_pools:
- name: "{{ unused_disks[0] }}"
type: disk
type: partition
disks: "{{ unused_disks }}"
volumes:
- name: test1
Expand All @@ -48,7 +49,7 @@
vars:
storage_pools:
- name: "{{ unused_disks[0] }}"
type: disk
type: partition
disks: "{{ unused_disks }}"
state: absent
volumes:
Expand All @@ -65,7 +66,7 @@
vars:
storage_pools:
- name: "{{ unused_disks[0] }}"
type: disk
type: partition
disks: "{{ unused_disks }}"
state: absent
volumes:
Expand Down
Loading