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

Add flag to prevent implicit removal of existing device/fs. #43

Merged
merged 7 commits into from
Nov 15, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
65 changes: 43 additions & 22 deletions library/blivet.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,10 @@
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,6 +120,8 @@

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
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This commit does not seem to fit in this PR and it seems that the added functionality is not used or tested. Not sure what it is doing here.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, I can move this out.



class BlivetAnsibleError(Exception):
Expand Down Expand Up @@ -163,7 +169,6 @@ def _get_format(self):
label=self._volume['fs_label'],
options=self._volume['fs_create_options'])
if not fmt.supported or not fmt.formattable:
# FAIL: fs type tools are not available
raise BlivetAnsibleError("required tools for file system '%s' are missing" % self._volume['fs_type'])

return fmt
Expand All @@ -189,32 +194,35 @@ def _resize(self):
try:
size = Size(self._volume['size'])
except Exception:
# FAIL: invalid size specification
raise BlivetAnsibleError("invalid size specification for volume '%s': '%s'" % (self._volume['name'], self._volume['size']))

if size and self._device.resizable and self._device.size != size:
if self._device.format.resizable:
self._device.format.update_size_info()

if not self._device.min_size <= size <= self._device.max_size:
# FAIL: resize to specified size not possible
raise BlivetAnsibleError("volume '%s' cannot be resized to '%s'" % (self._volume['name'], size))

try:
self._blivet.resize_device(self._device, size)
except ValueError as e:
# FAIL: resize not possible
raise BlivetAnsibleError("volume '%s' cannot be resized from %s to %s: %s" % (self._device.name,
self._device.size,
size, str(e)))

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) and \
not packages_only:
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 +263,17 @@ 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():
raise BlivetAnsibleError("no disks specified for volume '%s'" % self._volume['name'])
elif not isinstance(self._volume['disks'], list):
raise BlivetAnsibleError("volume disks must be specified as a list")

if self._device is None:
raise BlivetAnsibleError("unable to resolve disk specified for volume '%s' (%s)" % (self._volume['name'], self._volume['disks']))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this error meant that no disks from _volume["disks"] contains the specified volume _volume['name']?
Without the code context the error message is hard to understand IMHO.




class BlivetPartitionVolume(BlivetVolume):
def _type_check(self):
Expand All @@ -273,21 +292,18 @@ def _create(self):
parent = self._blivet.devicetree.resolve_device(self._volume['pool'])

if parent is None:
# FAIL: failed to find pool
raise BlivetAnsibleError("failed to find pool '%s' for volume '%s'" % (self._blivet_pool['name'], self._volume['name']))

size = Size("256 MiB")
try:
device = self._blivet.new_partition(parents=[parent], size=size, grow=True, fmt=self._get_format())
except Exception:
# FAIL: failed to instantiate volume device
raise BlivetAnsibleError("failed set up volume '%s'" % self._volume['name'])

self._blivet.create_device(device)
try:
do_partitioning(self._blivet)
except Exception:
# FAIL: partition allocation failed: not enough space?
raise BlivetAnsibleError("partition allocation failed for volume '%s'" % self._volume['name'])

self._device = device
Expand All @@ -303,18 +319,15 @@ def _create(self):

parent = self._blivet_pool._device
if parent is None:
# FAIL: failed to find pool
raise BlivetAnsibleError("failed to find pool '%s' for volume '%s'" % (self._blivet_pool['name'], self._volume['name']))

try:
size = Size(self._volume['size'])
except Exception:
# FAIL: invalid size specification
raise BlivetAnsibleError("invalid size '%s' specified for volume '%s'" % (self._volume['size'], self._volume['name']))

fmt = self._get_format()
if size > parent.free_space:
# FAIL: volume size greater than pool free space
raise BlivetAnsibleError("specified size for volume '%s' exceeds available space in pool '%s' (%s)" % (size,
parent.name,
parent.free_space))
Expand All @@ -323,7 +336,6 @@ def _create(self):
device = self._blivet.new_lv(name=self._volume['name'],
parents=[parent], size=size, fmt=fmt)
except Exception:
# FAIL: failed to create volume
raise BlivetAnsibleError("failed to set up volume '%s'" % self._volume['name'])

self._blivet.create_device(device)
Expand Down Expand Up @@ -391,8 +403,7 @@ def _type_check(self): # pylint: disable=no-self-use
def _look_up_disks(self):
""" Look up the pool's disks in blivet's device tree. """
if not self._pool['disks']:
# FAIL: no disks specified for pool
raise BlivetAnsibleError("no disks specified for pool '%s'" % self._pool['name']) # sure about this one?
raise BlivetAnsibleError("no disks specified for pool '%s'" % self._pool['name'])
elif not isinstance(self._pool['disks'], list):
raise BlivetAnsibleError("pool disks must be specified as a list")

Expand All @@ -403,7 +414,6 @@ def _look_up_disks(self):
disks.append(device)

if self._pool['disks'] and not disks:
# FAIL: failed to find any disks
raise BlivetAnsibleError("unable to resolve any disks specified for pool '%s' (%s)" % (self._pool['name'], self._pool['disks']))

self._disks = disks
Expand All @@ -428,8 +438,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 safe_mode and not packages_only:
raise BlivetAnsibleError("cannot remove existing formatting and/or devices on disk '%s' (pool '%s') in safe mode" % (disk.name, self._pool['name']))
else:
self._blivet.devicetree.recursive_remove(disk)

if use_partitions:
label = get_format("disklabel", device=disk.path)
Expand All @@ -446,7 +459,6 @@ def _create_members(self):
try:
do_partitioning(self._blivet)
except Exception:
# FAIL: problem allocating partitions for pool backing devices
raise BlivetAnsibleError("failed to allocation partitions for pool '%s'" % self._pool['name'])

return members
Expand Down Expand Up @@ -490,7 +502,11 @@ 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 safe_mode and not packages_only:
raise BlivetAnsibleError("cannot remove existing formatting and/or devices on disk '%s' "
"(pool '%s') in safe mode" % (self._device.name, self._pool['name']))
else:
self._blivet.devicetree.recursive_remove(self._device, remove_device=False)

label = get_format("disklabel", device=self._device.path, label_type=disklabel_type)
self._blivet.format_device(self._device, label)
Expand All @@ -503,7 +519,6 @@ def _type_check(self):
def _get_format(self):
fmt = get_format("lvmpv")
if not fmt.supported or not fmt.formattable:
# FAIL: lvm tools are not available
raise BlivetAnsibleError("required tools for managing LVM are missing")

return fmt
Expand All @@ -516,15 +531,14 @@ def _create(self):
try:
pool_device = self._blivet.new_vg(name=self._pool['name'], parents=members)
except Exception:
# FAIL: failed to instantiate pool device
raise BlivetAnsibleError("failed to set up pool '%s'" % self._pool['name'])

self._blivet.create_device(pool_device)
self._device = pool_device


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

Expand Down Expand Up @@ -660,6 +674,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=True),
use_partitions=dict(type='bool', required=False, default=True))

# seed the result dict in the object
Expand Down Expand Up @@ -692,6 +707,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
3 changes: 2 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 @@ -105,6 +105,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