Implement file binding support for host process containers #1344
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
dcantah
force-pushed
the
hpc-enhanced
branch
3 times, most recently
from
f5ff0a3
to
6bb2a8b
Compare
dcantah
commented
Apr 7, 2022
helsaawy
reviewed
Apr 12, 2022
kevpar
reviewed
Apr 13, 2022
kevpar
reviewed
Apr 13, 2022
kevpar
reviewed
Apr 13, 2022
kevpar
reviewed
Apr 13, 2022
kevpar
reviewed
Apr 13, 2022
dcantah
force-pushed
the
hpc-enhanced
branch
2 times, most recently
from
0e338df
to
f167239
Compare
helsaawy
approved these changes
Apr 13, 2022
|
@msscotb Assigning you also as you said you wanted to take a look |
kevpar
reviewed
Apr 21, 2022
kevpar
reviewed
Apr 21, 2022
kevpar
reviewed
Apr 21, 2022
kevpar
reviewed
Apr 21, 2022
kevpar
reviewed
Apr 21, 2022
kevpar
reviewed
Apr 21, 2022
marosset
reviewed
Apr 29, 2022
dcantah
commented
Apr 29, 2022
msscotb
requested changes
May 28, 2022
This change adds in file binding support for host process containers if the host has the functionality available (bindfltapi.dll exists). This makes it so mounts in the runtime spec actually show up in the container at mount.Destination instead of simply being symlinks to a relative path, as well as are completely unique per container. This is achieved by upgrading the job object to a silo and making use of silo local file bindings that the Bind Filter supports. This support additionally opens up the opportunity for the rootfs for the container to be container local and unique. So instead of the rootfs showing up to any process on the host and being located at C:\C\<containerid>, it now (by default at least) will be present at C:\payload and be unique in each container as well. In a similar fashion to the mount changes, this only takes affect if the host has file binding support available. Signed-off-by: Daniel Canter <dcanter@microsoft.com>
- Change to check for 'path=' instead of 'path' for amending the PATH env var to contain the containers rootfs path. - Make comments regarding why we need to prepend cmd /c to the commandline more clear. Fix accidentally lowercasing the env variable as well. - Change to C:\hpc as the default rootfs location instead of C:\payload. - Change to checking for file binding support in a sync.Once instead of init(). This avoids unneccessary filesystem operations for packages that include /internal/jobcontainers. Signed-off-by: Daniel Canter <dcanter@microsoft.com>
* Add back in cri-containerd tests for thios functionality. * Add back in "microsoft.com/hostprocess-rootfs-location" annotation to make the rootfs location configurable. Signed-off-by: Daniel Canter <dcanter@microsoft.com>
* Get rid of C:\payload mention and fix up comment to be more clear. * Change how I was checking the existence of a path= env var to be easier to read. Signed-off-by: Daniel Canter <dcanter@microsoft.com>
* Add comprehensive comment on what new functionality having bindflt available brings. * Fix up comment regarding why we need to prepend cmd /c to the cmdline. Signed-off-by: Daniel Canter <dcanter@microsoft.com>
* Don't stat in ApplyFileBinding and just mkdirall * Get rid of C:\C references in favor of C:\hpc Signed-off-by: Daniel Canter <dcanter@microsoft.com>
* Add a warning if the mount destination exists Signed-off-by: Daniel Canter <dcanter@microsoft.com>
* Rebased to fix a merge conflict * Removed code to additionally bind mounts under C:\hpc for backwards compatability. We can expose a way to just get the full beta behavior if needed. * Go mod vendor in /test which brings in a new file now as some of the job object surface was brought into /test with another change. Signed-off-by: Daniel Canter <dcanter@microsoft.com>
|
Given @marosset and @jsturtevant had expressed having a way to fallback to the old behavior via an annotation would be a plus, I'll add that in a quick follow up to not hold this PR up any longer. |
kiashok
pushed a commit
to kiashok/hcsshim
that referenced
this pull request
Jul 11, 2022
…#1344) * Implement file binding support for host process containers This change adds in file binding support for host process containers if the host has the functionality available (bindfltapi.dll exists). This makes it so mounts in the runtime spec actually show up in the container at mount.Destination instead of simply being symlinks to a relative path, as well as are completely unique per container. This is achieved by upgrading the job object to a silo and making use of silo local file bindings that the Bind Filter supports. This support additionally opens up the opportunity for the rootfs for the container to be container local and unique. So instead of the rootfs showing up to any process on the host and being located at C:\C\<containerid>, it now (by default at least) will be present at C:\hpc and be unique in each container as well. In a similar fashion to the mount changes, this only takes affect if the host has file binding support available. Signed-off-by: Daniel Canter <dcanter@microsoft.com>
princepereira
pushed a commit
to princepereira/hcsshim
that referenced
this pull request
Aug 29, 2024
…#1344) * Implement file binding support for host process containers This change adds in file binding support for host process containers if the host has the functionality available (bindfltapi.dll exists). This makes it so mounts in the runtime spec actually show up in the container at mount.Destination instead of simply being symlinks to a relative path, as well as are completely unique per container. This is achieved by upgrading the job object to a silo and making use of silo local file bindings that the Bind Filter supports. This support additionally opens up the opportunity for the rootfs for the container to be container local and unique. So instead of the rootfs showing up to any process on the host and being located at C:\C\<containerid>, it now (by default at least) will be present at C:\hpc and be unique in each container as well. In a similar fashion to the mount changes, this only takes affect if the host has file binding support available. Signed-off-by: Daniel Canter <dcanter@microsoft.com>
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.
This change adds in file binding support for host process containers if the host has the functionality available (bindfltapi.dll exists). This makes it so mounts in the runtime spec actually show up in the container at
mount.Destinationinstead of simply being symlinks to a relative path, as well as are completely unique per container. This is achieved by upgrading the job object to a silo and making use of silo local file bindings that the Bind Filter supports.This additionally opens up the opportunity for the rootfs for the container to be container local and unique. So instead of the rootfs showing up to any process on the host and being located at C:\C\{containerid}, it now (by default at least) will be present at C:\hpc and be unique in each container as well. In a similar fashion to the mount changes, this only takes affect if the host has file binding support available.