Fix: storage_utils: Check multipath devices on all nodes - #2173
Fix: storage_utils: Check multipath devices on all nodes#2173liangxin1300 wants to merge 2 commits into
Conversation
2ba533a to
fcba122
Compare
Codecov Report❌ Patch coverage is
Additional details and impacted files
Flags with carried forward coverage won't be shown. Click here to find out more.
☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
| for node in node_list or [utils.this_node()]: | ||
| inspector = cls(dev, node) | ||
| if inspector._is_under_multipath(): | ||
| error_msg = f"Device {dev} is under multipath on {node}, please provide the multipath device instead" |
There was a problem hiding this comment.
please provide the multipath device instead
I am not sure if this suggestion is accurate. If a device is only under multipath on some of the nodes, it is probably a different problem, for example, inconsistent device file names in different node. I incline to report only the error without providing suggestions blindly.
There was a problem hiding this comment.
inconsistent device file names in different node
This is the prerequisite to configuring disk-based SBD, see https://documentation.suse.com/sle-ha/15-SP7/html/SLE-HA-all/cha-ha-storage-protect.html#sec-ha-storage-protect-req
There was a problem hiding this comment.
Even though consistent device naming is a prerequisite, validation logic should detect and report when this prerequisite is unsatisfied rather than assuming it is met. Suggesting users provide the multipath device when names are actually inconsistent is misleading and should be avoided.
0a335fe to
ea455cf
Compare
| resolved = Path(dev).resolve() | ||
| cmd = f"lsblk -dn -o PKNAME {shlex.quote(str(resolved))}" | ||
| _, out, _ = self._shell.get_rc_stdout_stderr_without_input(None, cmd) | ||
| _, out, _ = self._shell.get_rc_stdout_stderr_without_input(self._peer, cmd) |
There was a problem hiding this comment.
It looks problematic to resolve symlink on localhost and run lsblk on a remote host.
Moreover, PKNAME returns the kernel device name, which will possibly negate the benefits of using stable names like /dev/disk/by-id/.... I don't know what kinds of symlink you have tested here. It looks quite complex when combining iscsi, device mapper, multipath and partitions together.
Multipath validation for SBD and cluster filesystems only checked the local node. This change runs the multipath inspection on each target node and reports where the invalid device was found. - Add peer-aware MultipathInspector checks - Pass node lists from SBD and cluster FS validation - Log multipath errors with the failing node - Add unit tests for multi-node checks
MultipathInspector can inspect devices on remote cluster nodes. Resolving device symlinks locally can produce paths that do not exist or have different meaning on the target node, causing incorrect multipath validation. - Query lsblk on the target node - Preserve the original device path for lsblk - Parse PKNAME/KNAME pair output - Use KNAME for whole-disk devices - Raise an error when no kernel name is found - Cover remote-safe and failure cases in tests
ea455cf to
39045e5
Compare
2 participants
Multipath validation for SBD and cluster filesystems only checked the local node. This change runs the multipath inspection on each target node and reports where the invalid device was found.