Add containerd-shim plumbing for job containers #962
Conversation
|
@dcantah can you rebase to get the new build changes? |
dcantah
force-pushed
the
job-containers-shim
branch
from
16de71e to
b74108d
Compare
|
@katiewasnothere Donezo |
dcantah
force-pushed
the
job-containers-shim
branch
2 times, most recently
from
b85da23 to
293d645
Compare
dcantah
force-pushed
the
job-containers-shim
branch
from
293d645 to
d1890da
Compare
dcantah
force-pushed
the
job-containers-shim
branch
from
d1890da to
3630bb7
Compare
dcantah
force-pushed
the
job-containers-shim
branch
2 times, most recently
from
106744f to
e2a4265
Compare
|
Anyone able to give this some eyes @microsoft/containerplat |
dcantah
force-pushed
the
job-containers-shim
branch
from
e2a4265 to
5982e53
Compare
| // | ||
| // TODO dcantah: Keep better track of the layers that are added, don't simply discard the SCSI, VSMB, etc. resource types gotten inside. | ||
| func MountContainerLayers(ctx context.Context, layerFolders []string, guestRoot string, uvm *uvmpkg.UtilityVM) (_ string, err error) { | ||
| func MountContainerLayers(ctx context.Context, layerFolders []string, guestRoot, volumeMountPath string, uvm *uvmpkg.UtilityVM) (_ string, err error) { |
There was a problem hiding this comment.
nit: IMO "volumeMountPath" is not a very clear name for what this is
katiewasnothere
left a comment
katiewasnothere
left a comment
There was a problem hiding this comment.
few comments, otherwise looks fine to me
* Add the necessary plumbing in containerd shim to be able to create a job container if asked for via the annotation. * Rework jobcontainers package a bit to return a resources struct to avoid some hacks during cleanup. This was resource cleanup for wcow/lcow is the exact same for job containers in the shim. * Change some of the layer code to handle taking in a volume mount point Signed-off-by: Daniel Canter <[email protected]>
dcantah
force-pushed
the
job-containers-shim
branch
from
5982e53 to
bbf5589
Compare
anmaxvl
left a comment
anmaxvl
left a comment
There was a problem hiding this comment.
do you need to revendor test directory here? do we have tests (which can be added later if not)?
|
@anmaxvl Tests have been in for awhile, they just won't work without this PR :P https://github.com/microsoft/hcsshim/blob/master/test/cri-containerd/jobcontainer_test.go |
oh right. I noticed that hcsshim vendored in the test folder was out of date when I reading through your branch locally. can you make sure that it's up to date? |
Related work items: microsoft#930, microsoft#962, microsoft#1004, microsoft#1008, microsoft#1039, microsoft#1045, microsoft#1046, microsoft#1047, microsoft#1052, microsoft#1053, microsoft#1054, microsoft#1057, microsoft#1058, microsoft#1060, microsoft#1061, microsoft#1063, microsoft#1064, microsoft#1068, microsoft#1069, microsoft#1070, microsoft#1071, microsoft#1074, microsoft#1078, microsoft#1079, microsoft#1081, microsoft#1082, microsoft#1083, microsoft#1084, microsoft#1088, microsoft#1090, microsoft#1091, microsoft#1093, microsoft#1094, microsoft#1096, microsoft#1098, microsoft#1099, microsoft#1102, microsoft#1103, microsoft#1105, microsoft#1106, microsoft#1108, microsoft#1109, microsoft#1115, microsoft#1116, microsoft#1122, microsoft#1123, microsoft#1126
Add containerd-shim plumbing for job containers
6 participants
if asked for via the annotation.
Signed-off-by: Daniel Canter [email protected]