hostapp: add kernel version ABI extension filtering#24
Conversation
Extensions can contain two labels (container labels or image labels, with container labels taking precedence): * io.balena.image.kernel-version: user space ABI in the form M.m.p revision * io.balena.image.kernel-abi-id: kernel space ABI as a sha256 of the Modules.symver file. Change-type: patch Signed-off-by: Alex Gonzalez <alexg@balena.io>
alexgg
force-pushed
the
alexgg/oci-runtime
branch
from
64dde61 to
230faf1
Compare
| } | ||
| hostABIID := hostapp.ParseHostKernelABIID(string(cmdline)) | ||
|
|
||
| containers = hostapp.SelectMountable(containers, release, hostABIID) |
There was a problem hiding this comment.
I'm wondering, would it be more straight forward to do it in one pass?
Like merge the filtering with the mounting line 146.
We would in a function mount the hostapp-extensions one by one and check if they are kernel compatible, if they are not we would umount them.
What do you think of something like this:
3eeecd5
Outside of this, SelectMountable name is a bit broad for me as it actually only check for kernel compatibility in the hostapp, what do you think of changing the name to something like umountIncompatibleKernelExtensions ?
There was a problem hiding this comment.
thanks for the comments @ycardaillac
On merging filter into the mount loop, my preference is always small pure functions that are easy to test and reuse, instead of folding two logical actions into one. In this case, merging doesn't actually save the mounting, which is the cost that matters, the ABI check has to read Module.symvers from inside the mount, so both versions mount every candidate and unmount the rejects. "One pass" only drops a cheap in-memory loop, and to get it we'd thread a callback through initializeContainers and swap a pure, testable FilterByKernelVersion for an I/O coupled fucntion.
On the name change, it is actually called SelectMountable intentionally as it already composes two independent filters, FilterByKernelVersion (userspace ABI) and FilterByKernelABIID (kernel ABI), and the plan is to add more (for example libc as I mentioned in my presentation). The comment says "compatible with the running kernel," which reads as kernel-only so I could redo the comment, but it's not really important I think until more non-kernel filters are added.
2 participants
Extensions can contain two labels (container labels or image labels, with container labels taking precedence):
Change-type: patch