Provide a temporary "scratch" volume to plugins #363
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Side-step name conflict with Docker SDK for Python.
The temporary volume allows containers-within-containers to share a writable "scratch space" which is deleted after the plugin completes. Otherwise, if a container launches another container, it would need to know what is safe to bind mount from the host (since bind mounts are always mounted from the host system).
This more closely matches the way the deployed engine container mounts the "/work" directory from the host. This is important for plugin containers that want to mount the source directory into a sub-container.
Moved the temporary named volume mount point from /work/tmp to /tmp/work. If the container mounts /work as read-only, then Docker can't mkdir /work/tmp. /tmp is a safer choice since it should always be writable. We also now pass the mount point in the temp_vol_name engine var so that if we change it again later, plugins will be able to adapt.
We now mount the target dir in the engine and inherit the volume in the plugin container. This is much closer to the deployed engine and denies the assumption that the host path matches the mount path.
The mount dir is no longer matched to the host dir.
mdfleury-wbd
approved these changes
Nov 20, 2024
Yeah, my reasoning is that the module is mostly concerned with building OCI images, which isn't necessarily specific to Docker (i.e., Docker in this case is just an implementation detail). We can still rename it later if we come up with a better name, it just needed to not conflict with |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Plugins are now provided with a temporary named volume which is automatically deleted after the plugin runs. The name of this volume is passed to the plugin via the engine var
temp_vol_name.This sets the framework for plugins which, in turn, execute a tool via a container (i.e.
docker run). These plugins can mount this named volume into the sub-container in order to pass files back and forth. For example, the plugin can prepare config files and pass them to the sub-container, then receive result files written by the sub-container.subprocess.run().dockerpackage toocito side-step the name conflict with the Docker SDK.engine-vars.json) are now treated as an error instead of a warning. This is because we need to merge thetemp_vol_nameandengine_idsettings into the provided JSON. If there's an actual need to test passing invalid JSON to the plugin, it can be done via the debug shell. :)Motivation and Context
Support for plugins which execute tools via sub-containers.
How Has This Been Tested?
Tested in non-prod environment.
Types of changes
Checklist