Add support for Task Update MappedPipe operation #1865
Conversation
Adds support to the containerd shim to Add/Remove a mapped pipe to a Windows Container Silo (argon) dynamically. Signed-off-by: Justin Terry <[email protected]>
|
@katiewasnothere - FYI |
|
@kevpar - PTAL and provide your insights when you can. |
|
Any chance I could get some feedback on the approach so we can solidify how to get this in? Thanks! |
| @@ -110,6 +110,7 @@ func verifyTaskUpdateResourcesType(data interface{}) error { | |||
| case *specs.WindowsResources: | |||
| case *specs.LinuxResources: | |||
| case *ctrdtaskapi.PolicyFragment: | |||
| case *ctrdtaskapi.MappedPipe: | |||
There was a problem hiding this comment.
I know we'll want to support other types in the future. I'm not sure if we will want to directly enumerate all the different additional resources we may want to support like this or if we'll want to create a type like ExtendedWindowsContainerResources or if we'll want to add the options directly to the runtime spec's structs themselves. I'm hesitant to move forward with this especially since we could potentially accidentally break you in the future. I'd like to hear more feedback from @kevpar or @ambarve here.
There was a problem hiding this comment.
I think this is fine. Even if we add more types in the future, I don't expect it would be difficult to continue supporting this type as well for update task calls.
| @@ -0,0 +1,79 @@ | |||
| package main | |||
There was a problem hiding this comment.
if we decided that we didn't want to support the Update path, would this tool be something that we'd need to agree to maintain?
There was a problem hiding this comment.
The tool is mostly an example of it working. The proper spot to put it if we want it in a tool I think is in "shimdiag share" which already has a precedent for this ad-hoc call but isnt supported nicely via containerd. I think we should just remove the tool now that we know it will work via containerd update api. Only reason for tool I guess is non-containerd based approach, but I dont see why thats important anymore :)
Nah, it was an either or. I'm good with containerd approach, will remove tool and submit for final. Ty! |
Adds support to the containerd shim to Add/Remove a mapped pipe to a Windows Container Silo (argon) dynamically.
Hey all,
There are two approaches in this CR. We need the ability to dynamically Add/Remove named pipes to an Argon container. Unfortunately all of the v2 API and schema is internal only packages. Because we only need containerd support it seemed ok to add it as an Update API operation to the Task API of containerd shim. However, if we dont want to go down that road, I also added an example
container-pipe.exethat simply adds a cmdline support for such an operation. Please let me know which approach we would like to continue with in order to add this support and export it to callers. Thanks!!See: #1863 for background and FYI.