[rhcos-4.16] kola/testiso: Adapt test for mpath name variations #4355
Conversation
We adjust test to be compatible with both mpath friendly and non-friendly names.
|
Hi @openshift-cherrypick-robot. Thanks for your PR. I'm waiting for a github.com member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
|
/ok-to-test |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request adapts a test for multipath device name variations by replacing a hardcoded device name with a dynamic check. This is a good improvement. However, the new command has a potential issue where it can produce false positives if no multipath devices are configured. I've suggested a more robust and simpler alternative that directly checks the properties of the root device.
| Type=oneshot | ||
| RemainAfterExit=yes | ||
| ExecStart=/bin/bash -c '[[ $(findmnt -nvro SOURCE /sysroot) == /dev/mapper/mpatha4 ]]' | ||
| ExecStart=/bin/bash -c 'lsblk -pno NAME "/dev/mapper/$(multipath -l -v 1)" | grep -qw "$(findmnt -nvr /sysroot -o SOURCE)"' |
There was a problem hiding this comment.
This is a good improvement to remove the hardcoded multipath device name. However, this new command has a potential issue. If multipath -l -v 1 fails or returns an empty string (e.g., when no multipath devices are configured), the command effectively becomes lsblk -pno NAME "/dev/mapper/". This will list all devices under /dev/mapper/, and if the root filesystem is on any other type of device-mapper device (like an LVM volume), the grep will succeed, leading to a false positive.
A simpler and more robust approach would be to use udevadm to check the properties of the root device directly. This is also consistent with how multipath devices are verified in other tests in this repository (see mantle/kola/tests/misc/multipath.go).
| ExecStart=/bin/bash -c 'lsblk -pno NAME "/dev/mapper/$(multipath -l -v 1)" | grep -qw "$(findmnt -nvr /sysroot -o SOURCE)"' | |
| ExecStart=/bin/bash -c 'udevadm info "$(findmnt -nvr /sysroot -o SOURCE)" | grep -q -e "dm-uuid-mpath-" -e "DM_MPATH"' |
|
/override continuous-integration/jenkins/pr-merge For some reason the worker pod is terminated consistently when cheking format with gofmt |
|
@joelcapitao: Overrode contexts on behalf of joelcapitao: continuous-integration/jenkins/pr-merge In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
2 participants
This is an automated cherry-pick of #4151
/assign joelcapitao