You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Santhosh Kumar Shanmugham <sa...@gmail.com> on 2017/02/14 17:51:43 UTC
Review Request 56657: Add support for local resolution of Docker
images.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/56657/
-----------------------------------------------------------
Review request for mesos and Jie Yu.
Bugs: MESOS-7089
https://issues.apache.org/jira/browse/MESOS-7089
Repository: mesos
Description
-------
MesosContainerizer's local Docker does not have support for
mutable tags which can point to different immutable digests. This
indirection is required when an operator wants to rollout updates
without affecting the customer's configuration.
Introduce a new optional agent flag `docker_local_resolution_mapping`
which will point to a JSON-formatted file, that contains mappings,
to be used for local resolution of docker images.
Diffs
-----
docs/configuration.md 656aaa34915eaee91d388febbc7574287b9f51b5
src/slave/containerizer/mesos/provisioner/docker/store.cpp 68ce265a5d0a61f8d9ed55dd14c630dcf893a7d2
src/slave/flags.hpp 2c4bd6ae628a272a4c6c2f02670baef011df4505
src/slave/flags.cpp 71935dea0f898d4594de9a5d8a3d1c7fc1a21606
src/tests/containerizer/provisioner_docker_tests.cpp ce57c06d50b47a150ff40412c1fde99f16892434
Diff: https://reviews.apache.org/r/56657/diff/
Testing
-------
make && make check
Thanks,
Santhosh Kumar Shanmugham
Re: Review Request 56657: Add support for local resolution of Docker
images.
Posted by Mesos Reviewbot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/56657/#review165557
-----------------------------------------------------------
Patch looks great!
Reviews applied: [56657]
Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' CONFIGURATION='--verbose' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker-build.sh
- Mesos Reviewbot
On Feb. 14, 2017, 5:51 p.m., Santhosh Kumar Shanmugham wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56657/
> -----------------------------------------------------------
>
> (Updated Feb. 14, 2017, 5:51 p.m.)
>
>
> Review request for mesos and Jie Yu.
>
>
> Bugs: MESOS-7089
> https://issues.apache.org/jira/browse/MESOS-7089
>
>
> Repository: mesos
>
>
> Description
> -------
>
> MesosContainerizer's local Docker does not have support for
> mutable tags which can point to different immutable digests. This
> indirection is required when an operator wants to rollout updates
> without affecting the customer's configuration.
>
> Introduce a new optional agent flag `docker_local_resolution_mapping`
> which will point to a JSON-formatted file, that contains mappings,
> to be used for local resolution of docker images.
>
>
> Diffs
> -----
>
> docs/configuration.md 656aaa34915eaee91d388febbc7574287b9f51b5
> src/slave/containerizer/mesos/provisioner/docker/store.cpp 68ce265a5d0a61f8d9ed55dd14c630dcf893a7d2
> src/slave/flags.hpp 2c4bd6ae628a272a4c6c2f02670baef011df4505
> src/slave/flags.cpp 71935dea0f898d4594de9a5d8a3d1c7fc1a21606
> src/tests/containerizer/provisioner_docker_tests.cpp ce57c06d50b47a150ff40412c1fde99f16892434
>
> Diff: https://reviews.apache.org/r/56657/diff/
>
>
> Testing
> -------
>
> make && make check
>
>
> Thanks,
>
> Santhosh Kumar Shanmugham
>
>
Re: Review Request 56657: Add support for local resolution of Docker
images.
Posted by Santhosh Kumar Shanmugham <sa...@gmail.com>.
> On Feb. 15, 2017, 4:31 a.m., Ilya Pronin wrote:
> > src/slave/containerizer/mesos/provisioner/docker/store.cpp, lines 224-228
> > <https://reviews.apache.org/r/56657/diff/1/?file=1633194#file1633194line224>
> >
> > Does this case really have to produce an error? What if provided image name already was in the form of `<repository>@<digest>`? I think we should try to look up an image using the original refeference if it's not found in the map.
That was my initial thought. However I changed it, since I thought it might make more sense to fail during look up. But thinking back, we can safely fallback to the image fetch failure message. I will update this to use the same `imageName` if no mapping is found.
> On Feb. 15, 2017, 4:31 a.m., Ilya Pronin wrote:
> > src/tests/containerizer/provisioner_docker_tests.cpp, line 331
> > <https://reviews.apache.org/r/56657/diff/1/?file=1633197#file1633197line331>
> >
> > Looks like this test does exactly the same as `ProvisionerDockerLocalStoreTest.MetadataManagerInitialization`. The new code is exercised in `LocalStoreTestWithMapping`. Since we don't inspect `MetadataManager` directly maybe having that test would be sufficient?
Fixed.
> On Feb. 15, 2017, 4:31 a.m., Ilya Pronin wrote:
> > src/tests/containerizer/provisioner_docker_tests.cpp, lines 236-237
> > <https://reviews.apache.org/r/56657/diff/1/?file=1633197#file1633197line236>
> >
> > The second line should be indented with 2 spaces.
Done.
- Santhosh Kumar
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/56657/#review165688
-----------------------------------------------------------
On Feb. 14, 2017, 9:51 a.m., Santhosh Kumar Shanmugham wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56657/
> -----------------------------------------------------------
>
> (Updated Feb. 14, 2017, 9:51 a.m.)
>
>
> Review request for mesos and Jie Yu.
>
>
> Bugs: MESOS-7089
> https://issues.apache.org/jira/browse/MESOS-7089
>
>
> Repository: mesos
>
>
> Description
> -------
>
> MesosContainerizer's local Docker does not have support for
> mutable tags which can point to different immutable digests. This
> indirection is required when an operator wants to rollout updates
> without affecting the customer's configuration.
>
> Introduce a new optional agent flag `docker_local_resolution_mapping`
> which will point to a JSON-formatted file, that contains mappings,
> to be used for local resolution of docker images.
>
>
> Diffs
> -----
>
> docs/configuration.md 656aaa34915eaee91d388febbc7574287b9f51b5
> src/slave/containerizer/mesos/provisioner/docker/store.cpp 68ce265a5d0a61f8d9ed55dd14c630dcf893a7d2
> src/slave/flags.hpp 2c4bd6ae628a272a4c6c2f02670baef011df4505
> src/slave/flags.cpp 71935dea0f898d4594de9a5d8a3d1c7fc1a21606
> src/tests/containerizer/provisioner_docker_tests.cpp ce57c06d50b47a150ff40412c1fde99f16892434
>
> Diff: https://reviews.apache.org/r/56657/diff/
>
>
> Testing
> -------
>
> make && make check
>
>
> Thanks,
>
> Santhosh Kumar Shanmugham
>
>
Re: Review Request 56657: Add support for local resolution of Docker
images.
Posted by Ilya Pronin <ip...@twopensource.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/56657/#review165688
-----------------------------------------------------------
src/slave/containerizer/mesos/provisioner/docker/store.cpp (lines 224 - 228)
<https://reviews.apache.org/r/56657/#comment237542>
Does this case really have to produce an error? What if provided image name already was in the form of `<repository>@<digest>`? I think we should try to look up an image using the original refeference if it's not found in the map.
src/tests/containerizer/provisioner_docker_tests.cpp (lines 236 - 237)
<https://reviews.apache.org/r/56657/#comment237545>
The second line should be indented with 2 spaces.
src/tests/containerizer/provisioner_docker_tests.cpp (line 331)
<https://reviews.apache.org/r/56657/#comment237544>
Looks like this test does exactly the same as `ProvisionerDockerLocalStoreTest.MetadataManagerInitialization`. The new code is exercised in `LocalStoreTestWithMapping`. Since we don't inspect `MetadataManager` directly maybe having that test would be sufficient?
- Ilya Pronin
On Feb. 14, 2017, 5:51 p.m., Santhosh Kumar Shanmugham wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56657/
> -----------------------------------------------------------
>
> (Updated Feb. 14, 2017, 5:51 p.m.)
>
>
> Review request for mesos and Jie Yu.
>
>
> Bugs: MESOS-7089
> https://issues.apache.org/jira/browse/MESOS-7089
>
>
> Repository: mesos
>
>
> Description
> -------
>
> MesosContainerizer's local Docker does not have support for
> mutable tags which can point to different immutable digests. This
> indirection is required when an operator wants to rollout updates
> without affecting the customer's configuration.
>
> Introduce a new optional agent flag `docker_local_resolution_mapping`
> which will point to a JSON-formatted file, that contains mappings,
> to be used for local resolution of docker images.
>
>
> Diffs
> -----
>
> docs/configuration.md 656aaa34915eaee91d388febbc7574287b9f51b5
> src/slave/containerizer/mesos/provisioner/docker/store.cpp 68ce265a5d0a61f8d9ed55dd14c630dcf893a7d2
> src/slave/flags.hpp 2c4bd6ae628a272a4c6c2f02670baef011df4505
> src/slave/flags.cpp 71935dea0f898d4594de9a5d8a3d1c7fc1a21606
> src/tests/containerizer/provisioner_docker_tests.cpp ce57c06d50b47a150ff40412c1fde99f16892434
>
> Diff: https://reviews.apache.org/r/56657/diff/
>
>
> Testing
> -------
>
> make && make check
>
>
> Thanks,
>
> Santhosh Kumar Shanmugham
>
>
Re: Review Request 56657: Add support for local resolution of Docker
images.
Posted by Santhosh Kumar Shanmugham <sa...@gmail.com>.
> On Feb. 15, 2017, 11:51 a.m., Mesos Reviewbot wrote:
> > Patch looks great!
> >
> > Reviews applied: [56657]
> >
> > Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' CONFIGURATION='--verbose' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker-build.sh
Ping!
- Santhosh Kumar
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/56657/#review165752
-----------------------------------------------------------
On Feb. 15, 2017, 10:08 a.m., Santhosh Kumar Shanmugham wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56657/
> -----------------------------------------------------------
>
> (Updated Feb. 15, 2017, 10:08 a.m.)
>
>
> Review request for mesos, Gilbert Song, Jie Yu, and Qian Zhang.
>
>
> Bugs: MESOS-7089
> https://issues.apache.org/jira/browse/MESOS-7089
>
>
> Repository: mesos
>
>
> Description
> -------
>
> MesosContainerizer's local Docker does not have support for
> mutable tags which can point to different immutable digests. This
> indirection is required when an operator wants to rollout updates
> without affecting the customer's configuration.
>
> Introduce a new optional agent flag `docker_local_resolution_mapping`
> which will point to a JSON-formatted file, that contains mappings,
> to be used for local resolution of docker images.
>
>
> Diffs
> -----
>
> docs/configuration.md 656aaa34915eaee91d388febbc7574287b9f51b5
> src/slave/containerizer/mesos/provisioner/docker/store.cpp 68ce265a5d0a61f8d9ed55dd14c630dcf893a7d2
> src/slave/flags.hpp 2c4bd6ae628a272a4c6c2f02670baef011df4505
> src/slave/flags.cpp 71935dea0f898d4594de9a5d8a3d1c7fc1a21606
> src/tests/containerizer/provisioner_docker_tests.cpp ce57c06d50b47a150ff40412c1fde99f16892434
>
> Diff: https://reviews.apache.org/r/56657/diff/
>
>
> Testing
> -------
>
> make && make check
>
>
> Thanks,
>
> Santhosh Kumar Shanmugham
>
>
Re: Review Request 56657: Add support for local resolution of Docker
images.
Posted by Mesos Reviewbot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/56657/#review165752
-----------------------------------------------------------
Patch looks great!
Reviews applied: [56657]
Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' CONFIGURATION='--verbose' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker-build.sh
- Mesos Reviewbot
On Feb. 15, 2017, 6:08 p.m., Santhosh Kumar Shanmugham wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56657/
> -----------------------------------------------------------
>
> (Updated Feb. 15, 2017, 6:08 p.m.)
>
>
> Review request for mesos, Gilbert Song, Jie Yu, and Qian Zhang.
>
>
> Bugs: MESOS-7089
> https://issues.apache.org/jira/browse/MESOS-7089
>
>
> Repository: mesos
>
>
> Description
> -------
>
> MesosContainerizer's local Docker does not have support for
> mutable tags which can point to different immutable digests. This
> indirection is required when an operator wants to rollout updates
> without affecting the customer's configuration.
>
> Introduce a new optional agent flag `docker_local_resolution_mapping`
> which will point to a JSON-formatted file, that contains mappings,
> to be used for local resolution of docker images.
>
>
> Diffs
> -----
>
> docs/configuration.md 656aaa34915eaee91d388febbc7574287b9f51b5
> src/slave/containerizer/mesos/provisioner/docker/store.cpp 68ce265a5d0a61f8d9ed55dd14c630dcf893a7d2
> src/slave/flags.hpp 2c4bd6ae628a272a4c6c2f02670baef011df4505
> src/slave/flags.cpp 71935dea0f898d4594de9a5d8a3d1c7fc1a21606
> src/tests/containerizer/provisioner_docker_tests.cpp ce57c06d50b47a150ff40412c1fde99f16892434
>
> Diff: https://reviews.apache.org/r/56657/diff/
>
>
> Testing
> -------
>
> make && make check
>
>
> Thanks,
>
> Santhosh Kumar Shanmugham
>
>
Re: Review Request 56657: Add support for local resolution of Docker
images.
Posted by Santhosh Kumar Shanmugham <sa...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/56657/
-----------------------------------------------------------
(Updated Feb. 15, 2017, 10:08 a.m.)
Review request for mesos, Gilbert Song, Jie Yu, and Qian Zhang.
Changes
-------
- Use `imageName` if not mapping is found.
Bugs: MESOS-7089
https://issues.apache.org/jira/browse/MESOS-7089
Repository: mesos
Description
-------
MesosContainerizer's local Docker does not have support for
mutable tags which can point to different immutable digests. This
indirection is required when an operator wants to rollout updates
without affecting the customer's configuration.
Introduce a new optional agent flag `docker_local_resolution_mapping`
which will point to a JSON-formatted file, that contains mappings,
to be used for local resolution of docker images.
Diffs (updated)
-----
docs/configuration.md 656aaa34915eaee91d388febbc7574287b9f51b5
src/slave/containerizer/mesos/provisioner/docker/store.cpp 68ce265a5d0a61f8d9ed55dd14c630dcf893a7d2
src/slave/flags.hpp 2c4bd6ae628a272a4c6c2f02670baef011df4505
src/slave/flags.cpp 71935dea0f898d4594de9a5d8a3d1c7fc1a21606
src/tests/containerizer/provisioner_docker_tests.cpp ce57c06d50b47a150ff40412c1fde99f16892434
Diff: https://reviews.apache.org/r/56657/diff/
Testing
-------
make && make check
Thanks,
Santhosh Kumar Shanmugham