Support for making multiple overlays in the same scratch path

[dcantah]

• The work and upper dirs were hardcoded previously so if you had tried to create more than one overlay in a mountpoint (/dev/sda / container scratch volume) it would fail. This change just adds a simple check to see if workdir and upperdir
  already exist and will create a unique name for them with a generated ID. This is the same method cri uses to generate container IDs.

Signed-off-by: Daniel Canter [email protected]

[katiewasnothere]

LGTM

[kevpar]

This is basically so that two callers can call overlay.Mount for different containers, but pass the same scratch path? It seems like it would be better for those callers to just ensure they are passing a unique path. Doing this by generating a new random path seems gross, and also makes it harder to track which directories are for which containers, if we need to debug something.

[anmaxvl]

lgtm. nit: not sure if making a utility function is worth it (to dedupe)

[kevpar]

Have concerns with this design (see my other comment)

[dcantah]

This is basically so that two callers can call overlay.Mount for different containers, but pass the same scratch path? It seems like it would be better for those callers to just ensure they are passing a unique path. Doing this by generating a new random path seems gross, and also makes it harder to track which directories are for which containers, if we need to debug something.

Correct, but by unique path do you mean where in the scratch space we would setup the upper/work dirs? Obviously we don't have anything in the CombinedLayers call to specify what we'd like the upper and work dirs to be named so I assume you mean specify a unique scratch path (Maybe just the path to the containers scratch space we're sharing/<container_id> would be good). We could verify this in the shim if the refcount on the scratch scsi disk is > 1, and then we can set the scratch path to whatever format we devise. Thoughts? @kevpar

[dcantah]

@kevpar For example, instead of this currently:

/run/gcs/c/bbe16b243025f94faae0b5c9a1cbd387a2a302b1460abec87806f308f4b1efe2 # ls  
config.json
hostname
hosts
lost+found
resolv.conf
rootfs
runc.log
sandboxMounts
upper
upper-7ab34e3c6bf1ac7024a8b7293884c3ddc28f87f6dccbf7098bf0fe46d5d5d5ff
upper-c84ecf1b8d55c0180dd2892870428fbffb0a4461a70e4363002b3e902416588a
work
work-b8dbcd37da754299807d724594f3797e8b557261ea990d8afe0513c8a0b6f2a0
work-e8ac3f60b0034ec5620721495000834e05032be55e30c1dc7eb6cbfda215ec6a

We could have:

/run/gcs/c/bbe16b243025f94faae0b5c9a1cbd387a2a302b1460abec87806f308f4b1efe2 # ls  
config.json
hostname
hosts
lost+found
resolv.conf
rootfs
runc.log
sandboxMounts
upper
<container_1_id> <- contains upper and work dirs for container 1
<container_2_id> <- contains upper and work dirs for container 2
work

[kevpar]

Yes, I think the caller of the combine layers modify operation should be responsible for passing a correct directory to use. The overlay package shouldn't have to worry about being given a path that's already been used.

As for what is the correct place to determine this, and what the paths should be, I'm not sure. As this work to re-use a scratch VHD for several containers spans several components, it might be helpful to write up an issue in Microsoft/hcsshim with your proposed flow. That would make it easier to reason about the end-to-end.

[dcantah]

@kevpar microsoft/hcsshim#891 :)