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

fix: Remove partition table from disk removed from a VG #464

Merged
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
37 changes: 22 additions & 15 deletions library/blivet.py
Original file line number Diff line number Diff line change
Expand Up @@ -1841,10 +1841,10 @@
return

add_disks = [d for d in self._disks if d not in self._device.ancestors]
remove_disks = [pv for pv in self._device.pvs if not any(d in pv.ancestors for d in self._disks)]
remove_pvs = [pv for pv in self._device.pvs if not any(d in pv.ancestors for d in self._disks)]

Check warning on line 1844 in library/blivet.py

View check run for this annotation

Codecov / codecov/patch

library/blivet.py#L1844

Added line #L1844 was not covered by tests

if self._pool['grow_to_fill']:
grow_pv_candidates = [pv for pv in self._device.pvs if pv not in remove_disks and pv not in add_disks]
grow_pv_candidates = [pv for pv in self._device.pvs if pv not in remove_pvs and pv not in add_disks]

Check warning on line 1847 in library/blivet.py

View check run for this annotation

Codecov / codecov/patch

library/blivet.py#L1847

Added line #L1847 was not covered by tests

for pv in grow_pv_candidates:
if abs(self._device.size - self._device.current_size) < 2 * self._device.pe_size:
Expand All @@ -1859,12 +1859,12 @@
else:
log.warning("cannot grow/resize PV '%s', format is not resizable", pv.name)

if not (add_disks or remove_disks):
if not (add_disks or remove_pvs):

Check warning on line 1862 in library/blivet.py

View check run for this annotation

Codecov / codecov/patch

library/blivet.py#L1862

Added line #L1862 was not covered by tests
return

if remove_disks and safe_mode:
if remove_pvs and safe_mode:

Check warning on line 1865 in library/blivet.py

View check run for this annotation

Codecov / codecov/patch

library/blivet.py#L1865

Added line #L1865 was not covered by tests
raise BlivetAnsibleError("cannot remove members '%s' from pool '%s' in safe mode" %
(", ".join(d.name for d in remove_disks),
(", ".join(d.name for d in remove_pvs),
self._device.name))

if self._is_raid:
Expand All @@ -1882,26 +1882,33 @@
self._pool['name'],
str(e)))

for disk in remove_disks:
if self._device.free_space < disk.size:
raise BlivetAnsibleError("disk '%s' cannot be removed from pool '%s'" % (disk.name,
self._pool['name']))
for pv in remove_pvs:
if self._device.free_space < pv.size:
raise BlivetAnsibleError("member '%s' cannot be removed from pool '%s'" % (pv.name,

Check warning on line 1887 in library/blivet.py

View check run for this annotation

Codecov / codecov/patch

library/blivet.py#L1885-L1887

Added lines #L1885 - L1887 were not covered by tests
self._pool['name']))

try:
ac = ActionRemoveMember(self._device, disk)
ac = ActionRemoveMember(self._device, pv)

Check warning on line 1891 in library/blivet.py

View check run for this annotation

Codecov / codecov/patch

library/blivet.py#L1891

Added line #L1891 was not covered by tests
# XXX: scheduling ActionRemoveMember is currently broken, we need to execute
# the action now manually, see https://bugzilla.redhat.com/show_bug.cgi?id=2076956
# self._blivet.devicetree.actions.add(ac)
ac.apply()
ac.execute()
except Exception as e:
raise BlivetAnsibleError("failed to remove disk '%s' from pool '%s': %s" % (disk.name,
self._pool['name'],
str(e)))
raise BlivetAnsibleError("failed to remove member '%s' from pool '%s': %s" % (pv.name,

Check warning on line 1898 in library/blivet.py

View check run for this annotation

Codecov / codecov/patch

library/blivet.py#L1898

Added line #L1898 was not covered by tests
self._pool['name'],
str(e)))
# XXX workaround for https://github.com/storaged-project/blivet/pull/1040
disk.format.vg_name = None
pv.format.vg_name = None

Check warning on line 1902 in library/blivet.py

View check run for this annotation

Codecov / codecov/patch

library/blivet.py#L1902

Added line #L1902 was not covered by tests

self._blivet.devicetree.recursive_remove(pv.raw_device)

Check warning on line 1904 in library/blivet.py

View check run for this annotation

Codecov / codecov/patch

library/blivet.py#L1904

Added line #L1904 was not covered by tests

self._blivet.devicetree.recursive_remove(disk.raw_device)
# if we are using partitions remove also the disklabel from the disk (but make sure
# there aren't other partitions first)
if use_partitions and not pv.is_disk:
for disk in pv.disks:
if disk.format.type == "disklabel" and not disk.format.partitions:
self._blivet.devicetree.recursive_remove(disk)

Check warning on line 1911 in library/blivet.py

View check run for this annotation

Codecov / codecov/patch

library/blivet.py#L1908-L1911

Added lines #L1908 - L1911 were not covered by tests

def _create(self):
if not self._device:
Expand Down
18 changes: 18 additions & 0 deletions tests/tests_lvm_pool_members.yml
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,15 @@
- name: foo
disks: "{{ [unused_disks[0]] }}"

- name: Get the partition table type on disk removed from the VG
command: lsblk -o PTTYPE --noheadings --nodeps /dev/{{ unused_disks[1] }}
register: storage_test_removed_pttype
changed_when: false

- name: Verify that removing the PV from the VG also removed the partition table
assert:
that: storage_test_removed_pttype.stdout == ''

- name: Verify role results
include_tasks: verify-role-results.yml

Expand Down Expand Up @@ -279,6 +288,15 @@
- name: test
size: "{{ volume_size }}"

- name: Get the partition table type on disk removed from the VG
command: lsblk -o PTTYPE --noheadings --nodeps /dev/{{ unused_disks[0] }}
register: storage_test_removed_pttype
changed_when: false

- name: Verify that removing the PV from the VG also removed the partition table
assert:
that: storage_test_removed_pttype.stdout == ''

- name: Get UUID of the 'foo' volume group
command: vgs --noheading -o vg_uuid foo
changed_when: false
Expand Down
Loading