cri-resource-manager icon indicating copy to clipboard operation
cri-resource-manager copied to clipboard

resmgr: make sysfs/other hostfs pseudoroot directory configurable on the command line.

Open klihub opened this issue 3 years ago • 4 comments

Allow overriding the default sysfs mount point using the newly introduced --sysfs-root $dir command line option. When used, the real host sysfs should be available/mounted as $dir/sys.

Target usage for this is to allow an NRI-enabled cri-resmgr to run as a DaemonSet inside a container, yet be able to do proper HW-discovery using the full sysfs of the host.

klihub avatar Oct 11 '22 08:10 klihub

When making this configurable, wouldn't it be even more future-proof, if one could give sysfs mountpoint, defaulting to /sys, rather than giving a root directory and expecting the mountpoint to be called sys in it?

askervin avatar Oct 11 '22 12:10 askervin

When making this configurable, wouldn't it be even more future-proof, if one could give sysfs mountpoint, defaulting to /sys, rather than giving a root directory and expecting the mountpoint to be called sys in it?

I see what you mean. And it was my first idea, too. But when I looked into pkg/topology I realized that it has some correct but hard-coded assumptions about how an LFS-like sysfs hierarchy looks like, with configurability supported only via an optional extra prefix dir (initially added for mocking sysfs for tests). Since it was the fastest/most trivial for me to modify configurability in pkg/sysfs to do the same, I opted to go that route. If you know of a setup where this would not work, please let me know and I can take another look.

klihub avatar Oct 11 '22 16:10 klihub

I think the current approach is actually better wrt. what @askervin suggested, as this is more general. It could be used for other system directories, too, if needed in the future. I'd just rename the option something else to reflect this. E.g. --sys-root or --host-root or smth. WDYT?

I think this PR looks good to be promoted out of draft state

marquiz avatar Oct 13 '22 11:10 marquiz

I think the current approach is actually better wrt. what @askervin suggested, as this is more general. It could be used for other system directories, too, if needed in the future. I'd just rename the option something else to reflect this. E.g. --sys-root or --host-root or smth. WDYT?

I think this PR looks good to be promoted out of draft state

Good idea to change the option name. Updated accordingly, and marked ready for review.

klihub avatar Oct 13 '22 18:10 klihub