-
Notifications
You must be signed in to change notification settings - Fork 9
Find k3s device from volume ID instead of trusting openstack #941
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
base: main
Are you sure you want to change the base?
Find k3s device from volume ID instead of trusting openstack #941
Conversation
I wasn't sure why |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the patch - its a good idea and we should've done it this way from the start.
General comment on the implementation - I would value the view of @JohnGarbutt and @sd109 here too.
/dev/{{ | ||
candidate_device_paths_stat.results | | ||
selectattr("stat.exists") | | ||
map(attribute = "stat.lnk_source") | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thinking more about this, I think we want the actual /dev/disk/by-id
path in /etc/fstab
(which happens in the mount filesytem task below), otherwise I think we could end up in a situation like this:
- Attach a volume, it is
/dev/disk/by-id/SOME-UUID
=>/dev/vdb
- Ansible here adds
/dev/vdb
to/etc/fstab
- Reboot the machine, libvirt shuffles the cards and now
/dev/disk/by-id/SOME-UUID
=>/dev/vdc
- Machine never boots because its missing a device it expects to see in
/etc/fstab
. - (Machine miraculously boots, but then we run the ansible again and we have
/dev/vdb
and/dev/vdc
in/etc/fstab
.
I think this task should look more like:
- name: Set volume block device name
ansible.builtin.set_fact:
k3s_storage_device: >-
{{
candidate_device_paths_stat.results |
selectattr("stat.exists") |
map(attribute = "stat.path") |
first |
default("") |
}}
then k3s_storage_device
will end up with the /dev/disk/by-id
path, and we should be fully immune from libvirt shuffling.
If you go this way, then you can set when: k3s_storage_device == ""
in the next task, which is somewhat cleaner I think.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note to self - if we move to this method, we should do it in a release where we change the base image, because that way we don't have to worry about the oldstyle (/dev/vdb
) and newstyle (/dev/disk/by-id/
) coexisting in /etc/fstab
, because we'll be rolling the image and will have a fresh fstab.
I don't have enough time to adopt this change myself right now, can this PR be taken over @m-bull? |
openstack_compute_volume_attach_v2.data_volume_attach.device
is often wrong at provision time and should not be trustedInstead, the same approach used in azimuth-images should be used, finding the device from the volume ID.
This avoids errors like:
when the hypervisor chooses the mount the volume in a different mount than Terraform/OpenStack is expecting.