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

Verify that disk volumes are mounted by UUID in fstab #50

Open
wants to merge 9 commits into
base: main
Choose a base branch
from

Conversation

pcahyna
Copy link
Member

@pcahyna pcahyna commented Oct 18, 2019

Note that disk volumes mean filesystems directly on disks.

Currently the test fails: on first invocation the filesystem gets mounted by device name and only on the second by UUID. This should be also caught by a real idempotence test.

XXX this should be part of the existing verification task lists: there is already a task called "Verify that the device identifier appears in /etc/fstab", but it does not check for this condition.

@pcahyna
Copy link
Member Author

pcahyna commented Oct 18, 2019

here's what happens:
One invocation has created a fstab entry which mounts the filesystem by block device path:

TASK [storage : manage mounts to match the specified state] ********************
changed: [/home/pcahyna/linux-system-roles-testing/local/rhel-guest-image-7.7-200.x86_64.qcow2] => (item={'path': '/opt/test', 'state': 'absent'}) => {"ansible_loop_var": "mount_info", "changed": true, "dump": "0", "fstab": "/etc/fstab", "mount_info": {"path": "/opt/test", "state": "absent"}, "name": "/opt/test", "opts": "defaults", "passno": "0"}
changed: [/home/pcahyna/linux-system-roles-testing/local/rhel-guest-image-7.7-200.x86_64.qcow2] => (item={'src': '/dev/vdc', 'dump': 0, 'passno': 0, 'fstype': 'ext4', 'state': 'mounted', 'path': '/opt/test', 'opts': 'defaults'}) => {"ansible_loop_var": "mount_info", "changed": true, "dump": "0", "fstab": "/etc/fstab", "fstype": "ext4", "mount_info": {"dump": 0, "fstype": "ext4", "opts": "defaults", "passno": 0, "path": "/opt/test", "src": "/dev/vdc", "state": "mounted"}, "name": "/opt/test", "opts": "defaults", "passno": "0", "src": "/dev/vdc"}

...

TASK [Read the /etc/fstab file for volume existence] ***************************
ok: [/home/pcahyna/linux-system-roles-testing/local/rhel-guest-image-7.7-200.x86_64.qcow2] => {"changed": false, "cmd": ["cat", "/etc/fstab"], "delta": "0:00:00.003845", "end": "2019-10-15 13:43:52.449118", "rc": 0, "start": "2019-10-15 13:43:52.445273", "stderr": "", "stderr_lines": [], "stdout_lines": ["/dev/vdc /opt/test ext4 defaults 0 0"]}

and the repeated invocation changes it to mount by UUID:

TASK [storage : manage mounts to match the specified state] ********************
changed: [/home/pcahyna/linux-system-roles-testing/local/rhel-guest-image-7.7-200.x86_64.qcow2] => (item={'src': 'UUID=26bddec8-c250-416c-b8f1-7d8bfc070d06', 'dump': 0, 'passno': 0, 'fstype': 'ext4', 'state': 'mounted', 'path': '/opt/test', 'opts': 'defaults'}) => {"ansible_loop_var": "mount_info", "changed": true, "dump": "0", "fstab": "/etc/fstab", "fstype": "ext4", "mount_info": {"dump": 0, "fstype": "ext4", "opts": "defaults", "passno": 0, "path": "/opt/test", "src": "UUID=26bddec8-c250-416c-b8f1-7d8bfc070d06", "state": "mounted"}, "name": "/opt/test", "opts": "defaults", "passno": "0", "src": "UUID=26bddec8-c250-416c-b8f1-7d8bfc070d06"}

...

TASK [Read the /etc/fstab file for volume existence] ***************************
ok: [/home/pcahyna/linux-system-roles-testing/local/rhel-guest-image-7.7-200.x86_64.qcow2] => {"changed": false, "cmd": ["cat", "/etc/fstab"], "delta": "0:00:00.004238", "end": "2019-10-15 13:45:30.693829", "rc": 0, "start": "2019-10-15 13:45:30.689591", "stderr": "", "stderr_lines": [], "stdout_lines": ["UUID=26bddec8-c250-416c-b8f1-7d8bfc070d06 /opt/test ext4 defaults 0 0"]}

(copied from 36da735#r35511810 )

@dwlehman
Copy link
Collaborator

dwlehman commented Jun 2, 2020

The rules are a bit more complicated. Device-mapper devices are generally listed by path instead of by UUID since that path can only change as a direct result of the user changing it, as opposed to partitions or disks which can change arbitrarily depending on device detection ordering.

I am definitely open to verifying that the correct identifier is used, but it will have to be a bit more complex than this.

This probably merits an explanation. In blivet, the backing device
will have a format of type 'luks'. This is the encrypted, or
backing, device. The next layer out is a LUKSDevice, which
represents the (decrypted/open) device-mapper device. Because the
LUKS layer is optional and can be effectively toggled, there are
many occasions on which it is convenient to look past the LUKS
layer directly to the backing device. For this purpose, all of
blivet's StorageDevice classes have a 'raw_device' property. For
unexcrypted leaf devices, the raw device is the same as the actual
device. For encrypted leaf devices, the raw device points to the
backing device.

In other words, adding _raw_device establishes a line between the
raw/backing/encrypted device and the decrypted/mapped/open device
which makes test validation quite a bit easier.
Doing so for all formats can trigger deactivation of device stacks
that needlessly complicates things.
Relying on _reformat to create the formatting worked because a
DiskDevice always exists. Now that self._device can be an
optional, non-existent, LUKS layer on top of the disk we have to
make the disk volume class behave more like the other volume
classes -- namely, it has to create its format as part of _create
since we will not always call _reformat (eg: when we've set up,
but not yet created, a new LUKS layer on the disk).
Note that disk volumes mean filesystems directly on disks.

Currently the test fails: on first invocation the filesystem gets mounted by
device name and only on the second by UUID. This should be also aught by a real
idempotence test.

XXX this should be part of the existing verification task lists:
there is already a task called
"Verify that the device identifier appears in /etc/fstab",
but it does not check for this condition.
@@ -6,7 +6,7 @@
mount_location: '/opt/test'
volume_size: '5g'
fs_type_after: "{{ 'ext3' if (ansible_distribution == 'RedHat' and ansible_distribution_major_version == '6') else 'ext4' }}"

pat: "{{ '^([^#\\s]+)\\s+' + mount_location + '\\s.*' }}"
tasks:
- include_role:
name: storage
Copy link
Contributor

Choose a reason for hiding this comment

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

please use name: linux-system-roles.storage


tasks:
- include_role:
name: storage
Copy link
Contributor

Choose a reason for hiding this comment

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

please use name: linux-system-roles.storage

block:
- name: Create an encrypted disk volume w/ default fs
include_role:
name: storage
Copy link
Contributor

Choose a reason for hiding this comment

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

please use name: linux-system-roles.storage

# encrypted disk volume
- name: Create an encrypted disk volume w/ default fs
include_role:
name: storage
Copy link
Contributor

Choose a reason for hiding this comment

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

please use name: linux-system-roles.storage


- name: Remove the encryption layer
include_role:
name: storage
Copy link
Contributor

Choose a reason for hiding this comment

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

please use name: linux-system-roles.storage


- name: Add encryption to the volume
include_role:
name: storage
Copy link
Contributor

Choose a reason for hiding this comment

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

please use name: linux-system-roles.storage

block:
- name: Create an encrypted partition volume w/ default fs
include_role:
name: storage
Copy link
Contributor

Choose a reason for hiding this comment

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

please use name: linux-system-roles.storage


- name: Create an encrypted partition volume w/ default fs
include_role:
name: storage
Copy link
Contributor

Choose a reason for hiding this comment

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

please use name: linux-system-roles.storage


- name: Remove the encryption layer
include_role:
name: storage
Copy link
Contributor

Choose a reason for hiding this comment

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

please use name: linux-system-roles.storage


- name: Add encryption to the volume
include_role:
name: storage
Copy link
Contributor

Choose a reason for hiding this comment

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

please use name: linux-system-roles.storage

block:
- name: Create an encrypted lvm volume w/ default fs
include_role:
name: storage
Copy link
Contributor

Choose a reason for hiding this comment

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

please use name: linux-system-roles.storage


- name: Create an encrypted lvm volume w/ default fs
include_role:
name: storage
Copy link
Contributor

Choose a reason for hiding this comment

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

please use name: linux-system-roles.storage


- name: Remove the encryption layer
include_role:
name: storage
Copy link
Contributor

Choose a reason for hiding this comment

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

please use name: linux-system-roles.storage


- name: Add encryption to the volume
include_role:
name: storage
Copy link
Contributor

Choose a reason for hiding this comment

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

please use name: linux-system-roles.storage

@richm
Copy link
Contributor

richm commented Mar 13, 2021

should we close this in favor of #104 ?

@richm
Copy link
Contributor

richm commented May 4, 2021

[citest pending]

@richm
Copy link
Contributor

richm commented May 4, 2021

[citest bad]

@richm
Copy link
Contributor

richm commented May 4, 2021

[citest pending]


- name: Clean up
include_role:
name: storage
Copy link
Contributor

Choose a reason for hiding this comment

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

One more?

please use name: linux-system-roles.storage

@richm
Copy link
Contributor

richm commented Oct 11, 2021

[citest pending]

2 similar comments
@richm
Copy link
Contributor

richm commented Oct 11, 2021

[citest pending]

@richm
Copy link
Contributor

richm commented Oct 20, 2021

[citest pending]

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants