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

enhance disk role/management subsystem #1494

Closed
phillxnet opened this Issue Oct 21, 2016 · 18 comments

Comments

Projects
None yet
2 participants
@phillxnet
Member

phillxnet commented Oct 21, 2016

In order to enhance the capability and flexibility of Rockstor's disk management a disk 'role' field was introduced in cc7cfc1 . This field was initially used to identify mdraid disk members in #1164 . In pr #1314 this role field was moved to a json format, as advised by @schakrava , enhancing it's general purpose use, (see commit 4935f06 ).

Given the requirements presented by #550 (Support full disk LUKS) and #1157 (Support backup to USB devices) it is proposed that we move the existing disk identification system fully over to using the 'role' db field. Currently there is still an 'overloading' of the "parted" flag in storageadmin/views/disk.py DiskMixin _update_disk_state() to signify 'unusable' or unsupported devices or file-systems. This potentially complicates support of extended role devices such as may be used in for example external USB 'import' / 'export' type roles which may very well be partitioned. Moving over to using the role field exclusively should therefor centralise and simplify device management and classification and aid in this subsystems utility.

It is therefor proposed that the 'overloading' of the parted flag be removed and superseded by the disk role field to allow for enhanced device support. In later pull requests this role field could be user assigned via links within the disk page that take the place of the "Click to wipe it clean" cog that instead ascribes one or more compatible roles to a device as well as the existing option to wipe the device prior to the usual default whole disk btrfs device role.

@phillxnet

This comment has been minimized.

Show comment
Hide comment
@phillxnet

phillxnet Oct 21, 2016

Member

@schakrava Requesting assignee status on this issue given it's stepping stone nature towards primarily issue #550. Given time constraints and the central nature of the required code changes I see this issue and the related #550 as not being part of the current testing channel but instead as contributing to the next testing channel updates series after the currently pending stable channel release.

Member

phillxnet commented Oct 21, 2016

@schakrava Requesting assignee status on this issue given it's stepping stone nature towards primarily issue #550. Given time constraints and the central nature of the required code changes I see this issue and the related #550 as not being part of the current testing channel but instead as contributing to the next testing channel updates series after the currently pending stable channel release.

@phillxnet

This comment has been minimized.

Show comment
Hide comment
@phillxnet

phillxnet Nov 5, 2016

Member

My intention is to address this issue next; given time constraints.

Member

phillxnet commented Nov 5, 2016

My intention is to address this issue next; given time constraints.

phillxnet added a commit to phillxnet/rockstor-core that referenced this issue Nov 6, 2016

@phillxnet

This comment has been minimized.

Show comment
Hide comment
@phillxnet

phillxnet Nov 6, 2016

Member

The initial overloading of the parted flag starts in scan_disks()

disks item = Disk(name='sda3', model='QEMU HARDDISK', serial='QM00005', size=7025459, transport='sata', vendor='ATA', hctl='2:0:0:0', type='part', fstype='btrfs', label='rockstor_rockstor', btrfs_uuid='ef6737b7-6e81-4902-80a2-e450935d3031', parted=False, root=True)

Here we have a system partition that has parted=False as partitioned drives are historically disqualified drives; but in this case we flip this flag to not disqualify our root=True drive. So partially redundant flags given as of #1166 "improve root not in partition behaviour and mdraid member labeling" we can handle root not in a partition but the overloading of the parted flag here is confusing: hence the move to disk roles. This in turn should help with supporting external drives for export / import / sync roles that are likely to be partitioned.

Member

phillxnet commented Nov 6, 2016

The initial overloading of the parted flag starts in scan_disks()

disks item = Disk(name='sda3', model='QEMU HARDDISK', serial='QM00005', size=7025459, transport='sata', vendor='ATA', hctl='2:0:0:0', type='part', fstype='btrfs', label='rockstor_rockstor', btrfs_uuid='ef6737b7-6e81-4902-80a2-e450935d3031', parted=False, root=True)

Here we have a system partition that has parted=False as partitioned drives are historically disqualified drives; but in this case we flip this flag to not disqualify our root=True drive. So partially redundant flags given as of #1166 "improve root not in partition behaviour and mdraid member labeling" we can handle root not in a partition but the overloading of the parted flag here is confusing: hence the move to disk roles. This in turn should help with supporting external drives for export / import / sync roles that are likely to be partitioned.

phillxnet added a commit to phillxnet/rockstor-core that referenced this issue Nov 6, 2016

add handlebars helper to identify root disk from Disk.role #1494
To move away from overloading a devices parted flag for the root
drive / device we need a mechanism to take priority in the case of
the system / root device to avoid it being labeled as incompatible
when we no longer override roots partition flag even when it is on
a partition.
@phillxnet

This comment has been minimized.

Show comment
Hide comment
@phillxnet

phillxnet Nov 6, 2016

Member

We also overload the parted flag within scan_disks() caller disk.py _update_disk_state():

            # If attached disk has an fs and it isn't btrfs
            if (d.fstype is not None and d.fstype != 'btrfs'):
                dob.btrfs_uuid = None
                dob.parted = True  # overload use of parted as non btrfs flag.
                # N.B. this overload use may become redundant with the addition
                # of the Disk.role field.
Member

phillxnet commented Nov 6, 2016

We also overload the parted flag within scan_disks() caller disk.py _update_disk_state():

            # If attached disk has an fs and it isn't btrfs
            if (d.fstype is not None and d.fstype != 'btrfs'):
                dob.btrfs_uuid = None
                dob.parted = True  # overload use of parted as non btrfs flag.
                # N.B. this overload use may become redundant with the addition
                # of the Disk.role field.

phillxnet added a commit to phillxnet/rockstor-core that referenced this issue Nov 6, 2016

incomplete extension of disk.role field for root device #1494
Including comment updates and role removal TODOs.

phillxnet added a commit to phillxnet/rockstor-core that referenced this issue Nov 6, 2016

@phillxnet

This comment has been minimized.

Show comment
Hide comment
@phillxnet

phillxnet Nov 7, 2016

Member

Role subsystem re-write in progress: appetiser for Rockstor root drive identification:
system-drive-identification-via-role
Note the 'R' logo.

Role code should be much improved and more in keeping with the rest of _update_disk_state()

A little more time and testing and we should have an extensible disk role system in place.

Member

phillxnet commented Nov 7, 2016

Role subsystem re-write in progress: appetiser for Rockstor root drive identification:
system-drive-identification-via-role
Note the 'R' logo.

Role code should be much improved and more in keeping with the rest of _update_disk_state()

A little more time and testing and we should have an extensible disk role system in place.

phillxnet added a commit to phillxnet/rockstor-core that referenced this issue Nov 8, 2016

re-written role attribution logic with debug logging #1494
Initial commit of a simpler and cleaner variant of the role
attribution logic employing a similar pattern to that used
elsewhere in _update_disk_state().

phillxnet added a commit to phillxnet/rockstor-core that referenced this issue Nov 8, 2016

rename scan_disks collection element to uuid from btrfs_uuid #1494
As this uuid is not btrfs specific this naming makes more sense,
especially going forward were an fs or container uuid may be
used for tracking / verification purposes in 'roles' managed drives,
partitions, or LUKS containers.

phillxnet added a commit to phillxnet/rockstor-core that referenced this issue Nov 8, 2016

phillxnet added a commit to phillxnet/rockstor-core that referenced this issue Nov 8, 2016

phillxnet added a commit to phillxnet/rockstor-core that referenced this issue Nov 8, 2016

phillxnet added a commit to phillxnet/rockstor-core that referenced this issue Nov 8, 2016

todo to mirroring md serial attribution for dm devices #1494
Akin to md devices, dm devices don't have a serial: in this case
we can do as we already do with mdraid and use the udevadm
UUID field ie MD_UUID for mdraid and DM_UUID for mapped
open LUKS containers.

phillxnet added a commit to phillxnet/rockstor-core that referenced this issue Nov 9, 2016

extend get_disk_serial for LUKS mapped disks #1494
Akin to mdraid devices a /dev/mapper/open-luks device
has no hw serial so substitute DM_UUID from udevadm.

phillxnet added a commit to phillxnet/rockstor-core that referenced this issue Nov 9, 2016

phillxnet added a commit to phillxnet/rockstor-core that referenced this issue Nov 9, 2016

phillxnet added a commit to phillxnet/rockstor-core that referenced this issue Nov 9, 2016

phillxnet added a commit to phillxnet/rockstor-core that referenced this issue Nov 9, 2016

todos on stability enhancement for tracking LUKS containers #1494
If we use the udevadm approach we get also the mapper name used,
parsing this away to improve flexibility reduces simplicity of method
so might as well call cryptsetup luksUUID device-name instead as then
our serial will travel with the underlying container.

phillxnet added a commit to phillxnet/rockstor-core that referenced this issue Nov 12, 2016

todos on device partition info collection re roles interplay #1494
Proposal to use scan_disks to back propagate partition info into a
list held against the base device. Allowing role-configuration access
to device partition info without re-scanning.

phillxnet added a commit to phillxnet/rockstor-core that referenced this issue Nov 12, 2016

todos on model use for other than root on mdraid as member info #1494
Currently we use the model on mdraid devices to indicate info on
their members and raid level. This only works for root on mdraid
but could be extended to non root mdraid devices.

phillxnet added a commit to phillxnet/rockstor-core that referenced this issue Nov 12, 2016

remove scan_disks() overloading of parted flag on root device #1494
For legacy reasons we re-classified a root disk as not
partitioned even when it was. This complicates code
readability and expansion to role based drive management
and is redundant given we also have a root flag that can be
used for this specific purpose.

phillxnet added a commit to phillxnet/rockstor-core that referenced this issue Nov 12, 2016

remove _update_disk_state() overloading of parted flag #1494
In this case the parted flag was overloaded to indicate a
devices incompatibility by virtue of it's fstype. However as we
move to a role based disk management we can use the role
system instead of just the partition flag mechanism.

phillxnet added a commit to phillxnet/rockstor-core that referenced this issue Nov 12, 2016

track partition info for each base device #1494
Extend scan_disks() to back port each partition it finds
to the parent / base device. This results in a list of partition
names against each base device.

phillxnet added a commit to phillxnet/rockstor-core that referenced this issue Nov 12, 2016

simplify return value of disk partitions in scan_disks() #1494
If we only return an empty list or a non empty list then
dependant conditionals are simplified.

phillxnet added a commit to phillxnet/rockstor-core that referenced this issue Nov 12, 2016

add partitions role to _update_disk_state() #1494
As scan_disks() can now supply a list of partitions for each
device we can make this list available under a partitions
role value, hence facilitating other special roles that
must be aware of the partitions they might use to fulfil
their roles.

phillxnet added a commit to phillxnet/rockstor-core that referenced this issue Nov 14, 2016

initial commit of add_pool - disk role filter #1494
By examining the role of a disk we can rule it out as
a prospective pool member, or include it in the case of
an openLUKS device.
@phillxnet

This comment has been minimized.

Show comment
Hide comment
@phillxnet

phillxnet Nov 14, 2016

Member

We can now use a disk's role field to reject or accept it's viability as a pool member.
From the last commit and associated console logs:

add_pool ACCEPTING ROLE OF null
add_pool ACCEPTING ROLE OF openLUKS json = {"openLUKS": "dm-name-luks-8f0bfeb8-d5c2-4846-a604-ae8846217ba1"}
add_pool ACCEPTING ROLE OF null
add_pool REJECTING ROLE with json = {"LUKS": "8f0bfeb8-d5c2-4846-a604-ae8846217ba1"}
add_pool ACCEPTING ROLE OF null
add_pool REJECTING ROLE with json = {"mdraid": "linux_raid_member", "backup-task": "syncthing-backup-task"}
add_pool REJECTING ROLE with json = {"mdraid": "linux_raid_member"}
Member

phillxnet commented Nov 14, 2016

We can now use a disk's role field to reject or accept it's viability as a pool member.
From the last commit and associated console logs:

add_pool ACCEPTING ROLE OF null
add_pool ACCEPTING ROLE OF openLUKS json = {"openLUKS": "dm-name-luks-8f0bfeb8-d5c2-4846-a604-ae8846217ba1"}
add_pool ACCEPTING ROLE OF null
add_pool REJECTING ROLE with json = {"LUKS": "8f0bfeb8-d5c2-4846-a604-ae8846217ba1"}
add_pool ACCEPTING ROLE OF null
add_pool REJECTING ROLE with json = {"mdraid": "linux_raid_member", "backup-task": "syncthing-backup-task"}
add_pool REJECTING ROLE with json = {"mdraid": "linux_raid_member"}

phillxnet added a commit to phillxnet/rockstor-core that referenced this issue Nov 14, 2016

initial commit of resize pool - add disk - disk role filter #1494
By examining the role of a disk we can rule it out as
a prospective pool member, or include it in the case of
an openLUKS device. These changes enable this for the
resize pool, add disk mechanism.

phillxnet added a commit to phillxnet/rockstor-core that referenced this issue Nov 15, 2016

disable SMART on device mapped open LUKS drives #1494
This is based purely on the device mapping starting with "luks-"
but gives consistency with existing checks and adds no complexity.
These mapped names are best standardized anyway, ie
luks-uuid where the uuid is that of the base encrypted container.

phillxnet added a commit to phillxnet/rockstor-core that referenced this issue Nov 15, 2016

favour by-id names beginning with "dm-name-" + logging #1494
As we currently use dm-name-<openLUKS_dev_name> in /dev/disk/by-id
return this in preference to the usual longest rule when found.

phillxnet added a commit to phillxnet/rockstor-core that referenced this issue Nov 15, 2016

dev name informed by disk.role in _refresh_pool_state() #1494
When updating pool info a disk is selected, use this disk's
role to inform any device name changes require to account
for mapped devices.

phillxnet added a commit to phillxnet/rockstor-core that referenced this issue Nov 15, 2016

feed add_pool with role informed device names + logging #1494
Some roles affect device name mapping, ie open LUKS
containers.

phillxnet added a commit to phillxnet/rockstor-core that referenced this issue Nov 15, 2016

phillxnet added a commit to phillxnet/rockstor-core that referenced this issue Nov 15, 2016

@phillxnet

This comment has been minimized.

Show comment
Hide comment
@phillxnet

phillxnet Nov 15, 2016

Member

Some progress on roles, initially to accommodate full disk LUKS containers, openLUKS mapped disks and pool use of openLUKS mapped devices:

Container:
luks-container

Mapped open container:
open-luks-disk

Pool using above disk:
luks-pool-teaser

Pretty incomplete and mid development but in progress.

Member

phillxnet commented Nov 15, 2016

Some progress on roles, initially to accommodate full disk LUKS containers, openLUKS mapped disks and pool use of openLUKS mapped devices:

Container:
luks-container

Mapped open container:
open-luks-disk

Pool using above disk:
luks-pool-teaser

Pretty incomplete and mid development but in progress.

phillxnet added a commit to phillxnet/rockstor-core that referenced this issue Nov 16, 2016

phillxnet added a commit to phillxnet/rockstor-core that referenced this issue Nov 16, 2016

minor code simplification and logging for issue #1494
Move to single return in get_dev_byid_name to simplify
code and ease logging.

phillxnet added a commit to phillxnet/rockstor-core that referenced this issue Nov 16, 2016

move role informed disk name filter to static method #1494
Use the same method in both add pool and resize pool functions.
Improve on prior method for clarity and robustness.

phillxnet added a commit to phillxnet/rockstor-core that referenced this issue Jan 20, 2017

further clarify drive role config ui template text #1494
Also make warning text red and improve overall formatting.

phillxnet added a commit to phillxnet/rockstor-core that referenced this issue Jan 20, 2017

add additional constraint on disk/part wipe context #1494
The constrain avoids changing both the redirect and doing a
wipe simultaneously as this is both confusing and potentially
miss-leading / dangerous. Also add use feedback on 'active'
redirect setting. And don't rely on context to set default part
option when it's the default of "Whole Disk".

phillxnet added a commit to phillxnet/rockstor-core that referenced this issue Jan 20, 2017

add backend role config device wipe capability and validation #1494
The backend should refuse to honour a wipe if it is accompanied
by a redirect role request; so only wipe if no redirect changes are also
requested.

phillxnet added a commit to phillxnet/rockstor-core that referenced this issue Jan 20, 2017

add simple diagnostic logging method to pool #1494
Helper to extract list of disk names from a pool object to
assist in development by easing temporary logging tasks.

phillxnet added a commit to phillxnet/rockstor-core that referenced this issue Jan 20, 2017

fix role config for whole disk and block UI wipe of pool members #1494
Since we are moving fs wipe function to role config UI we need
to allow for hole disk (roles=None).

phillxnet added a commit to phillxnet/rockstor-core that referenced this issue Jan 20, 2017

add backend validation against wipe of pool member #1494
Include advice about pool resize as pool member wipe method.
Also update top level Exception message to reflect wipe
integration into role configuration.

phillxnet added a commit to phillxnet/rockstor-core that referenced this issue Jan 20, 2017

improve text on role config to reflect wipe capabilities #1494
Also add the eraser icon to tie in with disk page link to aid in
affordance and continuity.

phillxnet added a commit to phillxnet/rockstor-core that referenced this issue Jan 20, 2017

use role config page for device wipe links on Disks page #1494
As the new role config page integrates whole disk / partition
wipe capability along with more explanation and red capitalized
warning we use this instead of the prior popup dialog.

phillxnet added a commit to phillxnet/rockstor-core that referenced this issue Jan 20, 2017

double check name redirect shim prior to wipe actions #1494
As the wipe operation now heeds the redirect role we should
double check if the name shim is reversable and if not the abort
the wipe. We also get the partition status of our passed device
this way so can accurately update the same after the wipe.

phillxnet added a commit to phillxnet/rockstor-core that referenced this issue Jan 20, 2017

phillxnet added a commit to phillxnet/rockstor-core that referenced this issue Jan 20, 2017

phillxnet added a commit to phillxnet/rockstor-core that referenced this issue Jan 20, 2017

Remove redundant "In use" column from add remove disks table #1494
The 'In use' column, prior to disk role changes allowing partitions, would
always show False as only un partitioned disks are available post disks
filter. Post disk role changes allowing partitions this column becomes
misleading. Also no other disk table has a partitioned column.

phillxnet added a commit to phillxnet/rockstor-core that referenced this issue Jan 20, 2017

refactor root pool flag in line with it's function #1494
As this flag now works from pool role rather than hardwired
to name we refactor accordingly.

phillxnet added a commit to phillxnet/rockstor-core that referenced this issue Jan 20, 2017

use 'tags' icon in all device tables to indicate a redirect role #1494
It is useful to know if a pool member is partitioned (using a redirect
role). If so label them in a manner consistent with the main disks page
with context relevant tooltips and links to review / edit the redirect
role. Note: as the 'root' pool partition member is not managed by
a redirect role, exclude it from this iconic flagging.
@phillxnet

This comment has been minimized.

Show comment
Hide comment
@phillxnet

phillxnet Jan 20, 2017

Member

I have again re-based on master to account for changes in #1615.

To avoid complex merging I have replaced existing master files with my own variants as the referenced pr predominantly involved Flake8 compliance (code formatting). I will endeavour to re-assert those code formatting changes in this pr as there has been significant code / logic change here so I have favoured these changes over the formatting changes in #1615.

Member

phillxnet commented Jan 20, 2017

I have again re-based on master to account for changes in #1615.

To avoid complex merging I have replaced existing master files with my own variants as the referenced pr predominantly involved Flake8 compliance (code formatting). I will endeavour to re-assert those code formatting changes in this pr as there has been significant code / logic change here so I have favoured these changes over the formatting changes in #1615.

phillxnet added a commit to phillxnet/rockstor-core that referenced this issue Jan 20, 2017

restoring flake8 compliance: storageadmin/views #1494
Returning flake8 compliance after merge / rebase of new code.

phillxnet added a commit to phillxnet/rockstor-core that referenced this issue Jan 20, 2017

restoring flake8 compliance: system #1494
Returning flake8 compliance after merge / rebase of new code.

phillxnet added a commit to phillxnet/rockstor-core that referenced this issue Jan 21, 2017

abstract duplicate role assessment code on disks page #1494
As all our role assessment handlebar helpers used the same
null and json tests we can combine them.
@phillxnet

This comment has been minimized.

Show comment
Hide comment
@phillxnet

phillxnet Jan 21, 2017

Member

Almost there folks, pr should be just around the corner.

Member

phillxnet commented Jan 21, 2017

Almost there folks, pr should be just around the corner.

phillxnet added a commit to phillxnet/rockstor-core that referenced this issue Jan 22, 2017

enhance disk page user feedback resolution re btrfs in partition #1494
As we propagate btrfs partition info we can inform the user
of the btrfs in partition status and instruct on the interim step
required: ie to add a redirect role first, or wipe.

@phillxnet phillxnet changed the title from complete disk role subsystem to enhance disk role/management subsystem Jan 22, 2017

phillxnet added a commit to phillxnet/rockstor-core that referenced this issue Jan 22, 2017

minor ui tool tip improvements re bcache and luks role UI's #1494
Explain active dashboard links as pending UI features. This way
the idea of the UI jump point is established, if not yet fulfilled.

phillxnet added a commit to phillxnet/rockstor-core that referenced this issue Jan 22, 2017

phillxnet added a commit to phillxnet/rockstor-core that referenced this issue Jan 22, 2017

phillxnet added a commit to phillxnet/rockstor-core that referenced this issue Jan 22, 2017

phillxnet added a commit to phillxnet/rockstor-core that referenced this issue Jan 22, 2017

phillxnet added a commit to phillxnet/rockstor-core that referenced this issue Jan 22, 2017

phillxnet added a commit to phillxnet/rockstor-core that referenced this issue Jan 22, 2017

phillxnet added a commit to phillxnet/rockstor-core that referenced this issue Jan 22, 2017

@phillxnet

This comment has been minimized.

Show comment
Hide comment
@phillxnet

phillxnet Jan 22, 2017

Member

All initial pr preparation complete. Testing done and intended behaviour verified.
Will endeavour to write up pr tomorrow along with the testing method I employed.
Sorry about the long delays fellow developers but this one has been a little tricky.

Member

phillxnet commented Jan 22, 2017

All initial pr preparation complete. Testing done and intended behaviour verified.
Will endeavour to write up pr tomorrow along with the testing method I employed.
Sorry about the long delays fellow developers but this one has been a little tricky.

phillxnet added a commit to phillxnet/rockstor-core that referenced this issue Jan 23, 2017

phillxnet added a commit to phillxnet/rockstor-core that referenced this issue Jan 23, 2017

minor refactor and improve disks tooltip consistency #1494
Refactoring mdraid member handlebar helper improves readability
especially with the additional role helpers.

@schakrava schakrava added this to the Pinnacles milestone Feb 6, 2017

@schakrava schakrava closed this in #1622 Feb 11, 2017

schakrava added a commit that referenced this issue Feb 11, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment