-
Notifications
You must be signed in to change notification settings - Fork 98
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
hookable "storage drivers" #325
Comments
I wonder if you really want it to be on the parent of But overall, it sounds good. What you have above would allow us to easily implement a solution for project-stacker/stacker#85, at least. |
The other thing about this is: it's not really about the layer necessarily, but about the layer and everything below it. For example, since we're doing btrfs, mutations on the filesystem are cumulative. So we have some special way of computing an aggregate hash of all the layers that have been added to a particular rootfs, and we use that as the name. So we may need to pass the entire manifest to the hook. |
I'd say if the delta-list hook fails, you just abort and make people fix their bugs :) |
Yeah, the bundle directory probably makes more sense (even from a "what should we do with respect to the spirit of the runtime-spec" perspective). There might even be room for a hook which allows you to modify the generated
Riiiight, because with btrfs the overlay stuff is actually implemented as snapshots rather than individual layer directories like you'd implement it with multi-lowerdir overlayfs. Yeah we'd need to support both use-cases but I think that just boils down to providing enough information to the hook for it decide which information it cares about.
Probably not a bad idea. :D |
So one problem I see with this: we'd really like to do the decompression in parallel (in general we find that decompressing gzip is slower than our disks can write), but with these hooks, everything is all still writing content to $bundle_path/rootfs. Can you imagine a way to enable this? |
I guess something like: |
Then I would just hook |
Right now, umoci only supports plain-Jane filesystem extraction. This does make umoci much simpler and more versatile (it should work on any POSIX-compliant filesystem). However, that does have some efficiency costs -- namely when it comes to extraction and diff generation.
go-mtree
is quite a serviceable system for generating diffs, but it pales in comparison to overlay filesystems. And extracting layers is needless work if there is already a copy of that layer already extracted somewhere.However the solution most projects have come up with is to bake in support for a set of filesystems and then users are forced to use that project's layer store (disallowing layer sharing between different implementations) and implementation (disallowing users to have custom implementations of layering). This also has a significant maintenance overhead for the project itself. I propose that we have a hook-based scheme which allows users to implement their own layer store caching and overlayfs layering implementations -- but we would provide some example hooks (which we might eventually support -- since they would be external to umoci we could update them without breaking users' setups) so users can get something working out of the box.
The CLI UX of the hooks would be scripts that are executed with a given command-line format (probably configured in
~/.config/umoci/hooks.toml
or something). The Go API would be function callbacks, to allow for library users of umoci to avoid shelling out to other programs (as well as have more complicated hooks that use their own Go modules).I would currently envision a few hooks (all of these would operate on the
bundle
directory with arootfs
specified by umoci and provided to the hook):unpack.ociv1.layer.try-skip
which would ask the hook provider whether there is already an existing copy of a given layer unpacked somewhere in a usable state. If so, the hook will configure therootfs
target as though it had been unpacked (for overlayfs this would be adding it as a newlowerdir
, for other filesystems it might be more complicated). This should not be a bind-mount becausetry-skip
will be called for all layers and if you just bind-mount the last one you'll be in a bit of trouble. Iftry-skip
succeeds then the otherunpack.ociv1.layer.*
hooks are skipped because the entire layer extraction will be skipped by umoci.unpack.ociv1.layer.pre-extract
is run immediately before umoci starts extracting files, so that the hook can configure therootfs
target.unpack.ociv1.layer.post-extract
is run immediately after umoci has extracted all the files for a layer, so that the hook can do any post-extraction steps it needs.repack.ociv1.layer.delta-list
will ask the hook provider whether it has a more optimised method of generating a list of files which have changed thango-mtree
. If so, the hook will generate a stream of pathnames with an indication of whether they were added/modified or deleted (though I guess this isn't necessary since it's obvious if a path was deleted). This will be used by umoci instead ofgo-mtree
to figure out which files have changed.NOTE: Since the
delta-list
hook could fail, we might need to generate a newgo-mtree
list anyway, but this is only needed when unpacking or using--refresh-bundle
. Alternatively we could make this fallback feature optional, so that users could make sure that they don't have to pay the cost ofgo-mtree
generation if they're fine with adelta-list
hook failure meaning they can't repack images.repack.ociv1.layer.refresh-bundle
asks the hook to place therootfs
in the same state it would've been if it had been freshly extracted. This would only be called ifumoci repack
was called with--refresh-bundle
.The hook names need to be such that when we start implementing OCIv2 (#256), we can add new hooks where it makes sense but keep the old ones where the semantics should be the same (while diff generation is not needed for my current vision of OCIv2, it would be useful to know which subset of the filesystem was changed so you don't need to rescan the whole thing).
@tych0 what do you think about this idea?
The text was updated successfully, but these errors were encountered: