You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Jie Yu <yu...@gmail.com> on 2018/08/15 20:50:25 UTC
Re: Review Request 67931: Windows: Added support for Docker Image
Manifest Version 2 Schema 2.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/67931/#review207353
-----------------------------------------------------------
what’s the filesystem layout for the cache when v2 schema 2 is used?
The layout for schema 1 is here:
https://github.com/apache/mesos/blob/master/src/slave/containerizer/mesos/provisioner/docker/paths.hpp#L31-L46
as far as I know, layer_id does not apply to schema 2. What do you use for the key?
- Jie Yu
On Aug. 6, 2018, 6:45 p.m., Liangyu Zhao wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67931/
> -----------------------------------------------------------
>
> (Updated Aug. 6, 2018, 6:45 p.m.)
>
>
> Review request for mesos, Akash Gupta, Andrew Schwartzmeyer, Gilbert Song, Jie Yu, Joseph Wu, and Qian Zhang.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added support for parsing schema 2 and fetching blob with multiple
> URLs as specified in schema 2. Also updated `curl` version to 7.61.0
> because of a bug encountered in version 7.57.0.
>
>
> Diffs
> -----
>
> 3rdparty/CMakeLists.txt 629b1968775da4d128b8d36c77d75efa303b0b7c
> 3rdparty/cmake/Versions.cmake c65d7b3d2d48ce9d0b4777aeb12fdf5b605803ac
> include/mesos/docker/spec.hpp 2879414dc42ffe633ac74b51e1bb116698c41162
> include/mesos/docker/v2_2.hpp PRE-CREATION
> include/mesos/docker/v2_2.proto PRE-CREATION
> include/mesos/uri/fetcher.hpp 760d6b33234d8efdc533c0c6f05e83a5c7d1f56b
> src/CMakeLists.txt a80b011bbd5c418ae66eb8dd7697d070168462bb
> src/Makefile.am cf0cf22b3f582d3d4427f13288af3845aef45263
> src/docker/spec.cpp 96fbf1f9cf1c2c4b2383607a97990f3a9156e6d9
> src/slave/containerizer/mesos/containerizer.cpp 98129d006cda9b65804b518619b6addc8990410a
> src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp a5683e3fe15dd35596122fcc0c580ae9d3adf7f2
> src/uri/fetcher.hpp fbf64c6767dea3aa0798f68db8322ce47cd8ad36
> src/uri/fetcher.cpp 489c7ce0198ee6803dcc8eb9710b292fa743a0e8
> src/uri/fetchers/docker.hpp 7a6193dd75dc10846f52cb039f6a162cc8805c16
> src/uri/fetchers/docker.cpp 55ca118660872a933a2dc186723bec6a39ee80f7
>
>
> Diff: https://reviews.apache.org/r/67931/diff/6/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Liangyu Zhao
>
>
Re: Review Request 67931: Windows: Added support for Docker Image
Manifest Version 2 Schema 2.
Posted by Jie Yu <yu...@gmail.com>.
> On Aug. 15, 2018, 8:50 p.m., Jie Yu wrote:
> > what’s the filesystem layout for the cache when v2 schema 2 is used?
> > The layout for schema 1 is here:
> > https://github.com/apache/mesos/blob/master/src/slave/containerizer/mesos/provisioner/docker/paths.hpp#L31-L46
> >
> > as far as I know, layer_id does not apply to schema 2. What do you use for the key?
>
> Jie Yu wrote:
> I think the layout we're currently using in the cache is actually v1. The reason for that is legacy (because we want to also support local puller, and at that time, the layout is v1).
>
> And we wanted to switch to use v2 instead for a long time. But I don't think we should mix v1 and v2 in the same `layers` dir.
>
> I'd rather see v2 being a totally separate cache that's content addressible.
>
> I think @gilbert has some thoughts on the v2 (or unified artifacts store) cache layout, @gilbert, can you share it here?
>
> Liangyu Zhao wrote:
> The filesystem layout is in fact not changed. What we are doing right now is pulling both schema 1 and schema 2 manifests. All the previous schema 1 logic including filesystem layout is unchanged, and schema 2 manifest is only used to support features not supported by schema 1, for example, pulling layers with urls in this patch. According to the backward compatibility of [schema 2](https://docs.docker.com/registry/spec/manifest-v2-2/#backward-compatibility), schema 1 is always present, but schema 2 may not be present. Therefore, it is reasonable to keep the schema 1 logic, and use schema 2 only for additional support.
Can you describe in the description that why windows need the feature of "pulling layers with urls"? It's not clear to me.
ok, so basically, you need information from both v2.1 and v2.2 manifest so that you can re-use the code we have for v2.1. This is indeed very differnt than what Ilya does in https://reviews.apache.org/r/53850/
I would suggest we have a sync with all stakeholders see if this is the long term solution we want to support.
- Jie
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/67931/#review207353
-----------------------------------------------------------
On Aug. 6, 2018, 6:45 p.m., Liangyu Zhao wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67931/
> -----------------------------------------------------------
>
> (Updated Aug. 6, 2018, 6:45 p.m.)
>
>
> Review request for mesos, Akash Gupta, Andrew Schwartzmeyer, Gilbert Song, Jie Yu, Joseph Wu, and Qian Zhang.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added support for parsing schema 2 and fetching blob with multiple
> URLs as specified in schema 2. Also updated `curl` version to 7.61.0
> because of a bug encountered in version 7.57.0.
>
>
> Diffs
> -----
>
> 3rdparty/CMakeLists.txt 629b1968775da4d128b8d36c77d75efa303b0b7c
> 3rdparty/cmake/Versions.cmake c65d7b3d2d48ce9d0b4777aeb12fdf5b605803ac
> include/mesos/docker/spec.hpp 2879414dc42ffe633ac74b51e1bb116698c41162
> include/mesos/docker/v2_2.hpp PRE-CREATION
> include/mesos/docker/v2_2.proto PRE-CREATION
> include/mesos/uri/fetcher.hpp 760d6b33234d8efdc533c0c6f05e83a5c7d1f56b
> src/CMakeLists.txt a80b011bbd5c418ae66eb8dd7697d070168462bb
> src/Makefile.am cf0cf22b3f582d3d4427f13288af3845aef45263
> src/docker/spec.cpp 96fbf1f9cf1c2c4b2383607a97990f3a9156e6d9
> src/slave/containerizer/mesos/containerizer.cpp 98129d006cda9b65804b518619b6addc8990410a
> src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp a5683e3fe15dd35596122fcc0c580ae9d3adf7f2
> src/uri/fetcher.hpp fbf64c6767dea3aa0798f68db8322ce47cd8ad36
> src/uri/fetcher.cpp 489c7ce0198ee6803dcc8eb9710b292fa743a0e8
> src/uri/fetchers/docker.hpp 7a6193dd75dc10846f52cb039f6a162cc8805c16
> src/uri/fetchers/docker.cpp 55ca118660872a933a2dc186723bec6a39ee80f7
>
>
> Diff: https://reviews.apache.org/r/67931/diff/6/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Liangyu Zhao
>
>
Re: Review Request 67931: Windows: Added support for Docker Image
Manifest Version 2 Schema 2.
Posted by Jie Yu <yu...@gmail.com>.
> On Aug. 15, 2018, 8:50 p.m., Jie Yu wrote:
> > what’s the filesystem layout for the cache when v2 schema 2 is used?
> > The layout for schema 1 is here:
> > https://github.com/apache/mesos/blob/master/src/slave/containerizer/mesos/provisioner/docker/paths.hpp#L31-L46
> >
> > as far as I know, layer_id does not apply to schema 2. What do you use for the key?
I think the layout we're currently using in the cache is actually v1. The reason for that is legacy (because we want to also support local puller, and at that time, the layout is v1).
And we wanted to switch to use v2 instead for a long time. But I don't think we should mix v1 and v2 in the same `layers` dir.
I'd rather see v2 being a totally separate cache that's content addressible.
I think @gilbert has some thoughts on the v2 (or unified artifacts store) cache layout, @gilbert, can you share it here?
- Jie
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/67931/#review207353
-----------------------------------------------------------
On Aug. 6, 2018, 6:45 p.m., Liangyu Zhao wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67931/
> -----------------------------------------------------------
>
> (Updated Aug. 6, 2018, 6:45 p.m.)
>
>
> Review request for mesos, Akash Gupta, Andrew Schwartzmeyer, Gilbert Song, Jie Yu, Joseph Wu, and Qian Zhang.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added support for parsing schema 2 and fetching blob with multiple
> URLs as specified in schema 2. Also updated `curl` version to 7.61.0
> because of a bug encountered in version 7.57.0.
>
>
> Diffs
> -----
>
> 3rdparty/CMakeLists.txt 629b1968775da4d128b8d36c77d75efa303b0b7c
> 3rdparty/cmake/Versions.cmake c65d7b3d2d48ce9d0b4777aeb12fdf5b605803ac
> include/mesos/docker/spec.hpp 2879414dc42ffe633ac74b51e1bb116698c41162
> include/mesos/docker/v2_2.hpp PRE-CREATION
> include/mesos/docker/v2_2.proto PRE-CREATION
> include/mesos/uri/fetcher.hpp 760d6b33234d8efdc533c0c6f05e83a5c7d1f56b
> src/CMakeLists.txt a80b011bbd5c418ae66eb8dd7697d070168462bb
> src/Makefile.am cf0cf22b3f582d3d4427f13288af3845aef45263
> src/docker/spec.cpp 96fbf1f9cf1c2c4b2383607a97990f3a9156e6d9
> src/slave/containerizer/mesos/containerizer.cpp 98129d006cda9b65804b518619b6addc8990410a
> src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp a5683e3fe15dd35596122fcc0c580ae9d3adf7f2
> src/uri/fetcher.hpp fbf64c6767dea3aa0798f68db8322ce47cd8ad36
> src/uri/fetcher.cpp 489c7ce0198ee6803dcc8eb9710b292fa743a0e8
> src/uri/fetchers/docker.hpp 7a6193dd75dc10846f52cb039f6a162cc8805c16
> src/uri/fetchers/docker.cpp 55ca118660872a933a2dc186723bec6a39ee80f7
>
>
> Diff: https://reviews.apache.org/r/67931/diff/6/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Liangyu Zhao
>
>
Re: Review Request 67931: Windows: Added support for Docker Image
Manifest Version 2 Schema 2.
Posted by Liangyu Zhao via Review Board <no...@reviews.apache.org>.
> On Aug. 15, 2018, 8:50 p.m., Jie Yu wrote:
> > what’s the filesystem layout for the cache when v2 schema 2 is used?
> > The layout for schema 1 is here:
> > https://github.com/apache/mesos/blob/master/src/slave/containerizer/mesos/provisioner/docker/paths.hpp#L31-L46
> >
> > as far as I know, layer_id does not apply to schema 2. What do you use for the key?
>
> Jie Yu wrote:
> I think the layout we're currently using in the cache is actually v1. The reason for that is legacy (because we want to also support local puller, and at that time, the layout is v1).
>
> And we wanted to switch to use v2 instead for a long time. But I don't think we should mix v1 and v2 in the same `layers` dir.
>
> I'd rather see v2 being a totally separate cache that's content addressible.
>
> I think @gilbert has some thoughts on the v2 (or unified artifacts store) cache layout, @gilbert, can you share it here?
The filesystem layout is in fact not changed. What we are doing right now is pulling both schema 1 and schema 2 manifests. All the previous schema 1 logic including filesystem layout is unchanged, and schema 2 manifest is only used to support features not supported by schema 1, for example, pulling layers with urls in this patch. According to the backward compatibility of [schema 2](https://docs.docker.com/registry/spec/manifest-v2-2/#backward-compatibility), schema 1 is always present, but schema 2 may not be present. Therefore, it is reasonable to keep the schema 1 logic, and use schema 2 only for additional support.
- Liangyu
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/67931/#review207353
-----------------------------------------------------------
On Aug. 6, 2018, 6:45 p.m., Liangyu Zhao wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67931/
> -----------------------------------------------------------
>
> (Updated Aug. 6, 2018, 6:45 p.m.)
>
>
> Review request for mesos, Akash Gupta, Andrew Schwartzmeyer, Gilbert Song, Jie Yu, Joseph Wu, and Qian Zhang.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added support for parsing schema 2 and fetching blob with multiple
> URLs as specified in schema 2. Also updated `curl` version to 7.61.0
> because of a bug encountered in version 7.57.0.
>
>
> Diffs
> -----
>
> 3rdparty/CMakeLists.txt 629b1968775da4d128b8d36c77d75efa303b0b7c
> 3rdparty/cmake/Versions.cmake c65d7b3d2d48ce9d0b4777aeb12fdf5b605803ac
> include/mesos/docker/spec.hpp 2879414dc42ffe633ac74b51e1bb116698c41162
> include/mesos/docker/v2_2.hpp PRE-CREATION
> include/mesos/docker/v2_2.proto PRE-CREATION
> include/mesos/uri/fetcher.hpp 760d6b33234d8efdc533c0c6f05e83a5c7d1f56b
> src/CMakeLists.txt a80b011bbd5c418ae66eb8dd7697d070168462bb
> src/Makefile.am cf0cf22b3f582d3d4427f13288af3845aef45263
> src/docker/spec.cpp 96fbf1f9cf1c2c4b2383607a97990f3a9156e6d9
> src/slave/containerizer/mesos/containerizer.cpp 98129d006cda9b65804b518619b6addc8990410a
> src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp a5683e3fe15dd35596122fcc0c580ae9d3adf7f2
> src/uri/fetcher.hpp fbf64c6767dea3aa0798f68db8322ce47cd8ad36
> src/uri/fetcher.cpp 489c7ce0198ee6803dcc8eb9710b292fa743a0e8
> src/uri/fetchers/docker.hpp 7a6193dd75dc10846f52cb039f6a162cc8805c16
> src/uri/fetchers/docker.cpp 55ca118660872a933a2dc186723bec6a39ee80f7
>
>
> Diff: https://reviews.apache.org/r/67931/diff/6/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Liangyu Zhao
>
>