> On Oct. 28, 2016, 10:03 a.m., Jie Yu wrote: > > src/slave/containerizer/mesos/provisioner/backends/copy.cpp, line 133 > > <https://reviews.apache.org/r/53115/diff/3/?file=1544890#file1544890line133> > > > > I am not sure if we need to pass in 'image' to backend. If we added OCI > > support, do we need to add another check here? > > > > I think we can simply assume the layer's whiteout is in aufs whiteout > > format, no matter what image type it is. I think APPC, we probably needs to > > do the same, right? > > > > You probably add a NOTE there saying that we assume all image type uses > > aufs whiteout format. > > > > In that way, no need to pass 'image' around to backend. > > Qian Zhang wrote: > The reason I passed in `image` and added this check is that previously we > have such check in `ProvisionerProcess::__provision()`: > > https://github.com/apache/mesos/blob/1.0.1/src/slave/containerizer/mesos/provisioner/provisioner.cpp#L334 > > I think one of the purposes of this check is to ensure we will NOT go > through the logic to handle whiteout files for APPC. I think this is correct > because APPC seems have a totally different way for whiteout, in its image > manifest (https://github.com/appc/spec/blob/master/spec/aci.md ), there is a > field `pathWhitelist`: > ``` > pathWhitelist (list of strings, optional): whitelist of absolute paths > that will exist in the app's rootfs after rendering. This must be a complete > and absolute set. An empty list is equivalent to an absent value and means > that all files in this image and any dependencies will be available in the > rootfs. > ``` > And there is PR in APPC to translate Docker whiteout files to > `pathWhitelist`: https://github.com/appc/docker2aci/pull/36 > > So I think if we do not pass in `image`, then copy backend will try to > handle whiteout files for APPC image which is not necessary since there is no > whiteout files to handle, instead, we should support `pathWhitelist` which > seems missed in > https://github.com/apache/mesos/blob/1.0.1/include/mesos/appc/spec.proto , > that is another issue. > > Jie Yu wrote: > If you think about that in a different way: can we convert the APPC > whiteout to be the same as OCI and Docker? I don't want each backend to have > different ways to handle whiteouts depending on the type. At the end of the > day, one needs to convert whiteout to aufs or overlayfs format, right?
So you are suggesting in APPC store, we can convert APPC whiteout (i.e., `pathWhitelist`) to AUFS whiteout (if the backend is aufs/copy) or overlay whiteout (if the backend is overlay), and then in each backend, they can just do their work regardless the image type, right? If so, then I think we should create another JIRA ticket for converting APPC whiteout to AUFS/overlay whiteout in APPC store. > On Oct. 28, 2016, 10:03 a.m., Jie Yu wrote: > > src/slave/containerizer/mesos/provisioner/backends/copy.cpp, line 213 > > <https://reviews.apache.org/r/53115/diff/3/?file=1544890#file1544890line213> > > > > Hum, IIUC, looks like this is not entirely correct. According to this: > > > > https://github.com/opencontainers/image-spec/blob/master/layer.md#whiteouts > > > > "Files that are present in the same layer as a whiteout file can only > > be hidden by whiteout files in subsequent layers." > > > > So we cannot cp first and then process whiteouts. Looks like before cp > > each layer, we need to do a pre-processing to handle whiteouts first (and > > remove the whiteouts), and then do the cp. > > Qian Zhang wrote: > Thanks for pointing this out! I think the flow should be: > 1. Delete the related files in rootfs based on the whiteout files in the > layer. > 2. Copy the layer to rootfs. > 3. Once we have done 1 and 2 for all layers, traverse rootfs to remove > all the whiteout files. > Or maybe in 2 we can do a seletively copy by `find` + `cp` to only copy > non-whiteout files from layer to rootfs? In this way, we do not need 3. > > Jie Yu wrote: > While you're doing 1, you can keep a list of paths to whiteout files as > you walk the tree. You can erase them before you do 2. Will that work? > You can erase them before you do 2. Did you mean erase the whiteout files in the layer before copy the layer to rootfs? I think we can not do it because the layer is in the store which should not be changed, otherwise other containers launched from the same image will be affected. The layer in the store should always keep the whiteout files, we just need to ensure the whiteout files got erased in container's rootfs. - Qian ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/53115/#review154084 ----------------------------------------------------------- On Oct. 25, 2016, 11:47 a.m., Qian Zhang wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/53115/ > ----------------------------------------------------------- > > (Updated Oct. 25, 2016, 11:47 a.m.) > > > Review request for mesos and Jie Yu. > > > Bugs: MESOS-6360 > https://issues.apache.org/jira/browse/MESOS-6360 > > > Repository: mesos > > > Description > ------- > > Implemented handling AUFS whiteouts for copy backend. > > > Diffs > ----- > > src/slave/containerizer/mesos/provisioner/backend.hpp > 7257d3a962ecdf87fe9d52facbd6a2619311a018 > src/slave/containerizer/mesos/provisioner/backends/aufs.hpp > a3c924195ae5eecc1caca9cd6fc0f6dc0df0a741 > src/slave/containerizer/mesos/provisioner/backends/aufs.cpp > c92c6c7174421158b9190ed0bfb00c1c97ef0f7b > src/slave/containerizer/mesos/provisioner/backends/bind.hpp > 54c88d940aa64d13114fc5d79ecbc1d474d169a6 > src/slave/containerizer/mesos/provisioner/backends/bind.cpp > 94dac40a12b6fd2e7d9733444d84763c77785402 > src/slave/containerizer/mesos/provisioner/backends/copy.hpp > 131d75521ca38afae651e8d885ebedad77d86a3e > src/slave/containerizer/mesos/provisioner/backends/copy.cpp > 9c5354e5fea488618ebdecf0aef9dd2fce555d20 > src/slave/containerizer/mesos/provisioner/backends/overlay.hpp > 4d591c5f988d87e0e905f973df5ab15a3386d676 > src/slave/containerizer/mesos/provisioner/backends/overlay.cpp > b71a31323aef376c9a28e1d52322a1802fb212ad > src/slave/containerizer/mesos/provisioner/provisioner.cpp > de2c12140652244bd3de9763ced203b144688ab2 > src/tests/containerizer/provisioner_backend_tests.cpp > 727bf645dd9337a94f098ab0a816b7e0e0651083 > > Diff: https://reviews.apache.org/r/53115/diff/ > > > Testing > ------- > > make check. > > > Thanks, > > Qian Zhang > >
