mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Qian Zhang <zhq527...@gmail.com>
Subject Re: Review Request 53115: Implemented handling AUFS whiteouts for copy backend.
Date Sat, 29 Oct 2016 13:54:32 GMT


> 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
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message