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 2016/01/03 08:02:29 UTC
Review Request 41861: Fixed a problem of parsing v1Compatibility in
Docker v2 image manifest.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41861/
-----------------------------------------------------------
Review request for mesos, Gilbert Song, Jojy Varghese, and Timothy Chen.
Repository: mesos
Description
-------
Fixed a problem of parsing v1Compatibility in Docker v2 image manifest.
According to Docker v2 image manifest spec, v1Compatibility is a string containing a raw JSON object. The raw JSON object is v1::ImageManifest. This patch fixed the bug in the original code.
Diffs
-----
src/docker/spec.cpp 5bbd98cb1c5f3aefd9050859d066b43360e6eb75
src/docker/v2.proto fdf159d003f8e5718de0f4d8bf9a2b1374cc11a2
src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp 049d46cad5cf94a3fb5d74cbfe649850311d35ad
src/tests/containerizer/docker_spec_tests.cpp PRE-CREATION
src/tests/containerizer/provisioner_docker_tests.cpp d51f342dabf386fb618ef72ce3e36a8bd8c82b5f
Diff: https://reviews.apache.org/r/41861/diff/
Testing
-------
make check
Thanks,
Jie Yu
Re: Review Request 41861: Fixed a problem of parsing v1Compatibility
in Docker v2 image manifest.
Posted by Gilbert Song <so...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41861/#review112655
-----------------------------------------------------------
Ship it!
Tested and verified that this patch works well.
- Gilbert Song
On Jan. 3, 2016, 10:33 a.m., Jie Yu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41861/
> -----------------------------------------------------------
>
> (Updated Jan. 3, 2016, 10:33 a.m.)
>
>
> Review request for mesos, Gilbert Song, Jojy Varghese, and Timothy Chen.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Fixed a problem of parsing v1Compatibility in Docker v2 image manifest.
>
> According to Docker v2 image manifest spec, v1Compatibility is a string containing a raw JSON object. The raw JSON object is v1::ImageManifest. This patch fixed the bug in the original code.
>
>
> Diffs
> -----
>
> src/docker/spec.cpp 5bbd98cb1c5f3aefd9050859d066b43360e6eb75
> src/docker/v2.proto fdf159d003f8e5718de0f4d8bf9a2b1374cc11a2
> src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp 049d46cad5cf94a3fb5d74cbfe649850311d35ad
> src/tests/containerizer/docker_spec_tests.cpp PRE-CREATION
> src/tests/containerizer/provisioner_docker_tests.cpp d51f342dabf386fb618ef72ce3e36a8bd8c82b5f
>
> Diff: https://reviews.apache.org/r/41861/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Jie Yu
>
>
Re: Review Request 41861: Fixed a problem of parsing v1Compatibility
in Docker v2 image manifest.
Posted by Gilbert Song <so...@gmail.com>.
> On Jan. 4, 2016, 2:06 p.m., Timothy Chen wrote:
> > src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp, line 257
> > <https://reviews.apache.org/r/41861/diff/2/?file=1180580#file1180580line257>
> >
> > Should we also CHECK here fslayers_size() == history size?
Actually it is validated spec::v2::parse in getManifest before we call downloadLayers.
- Gilbert
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41861/#review112657
-----------------------------------------------------------
On Jan. 3, 2016, 10:33 a.m., Jie Yu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41861/
> -----------------------------------------------------------
>
> (Updated Jan. 3, 2016, 10:33 a.m.)
>
>
> Review request for mesos, Gilbert Song, Jojy Varghese, and Timothy Chen.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Fixed a problem of parsing v1Compatibility in Docker v2 image manifest.
>
> According to Docker v2 image manifest spec, v1Compatibility is a string containing a raw JSON object. The raw JSON object is v1::ImageManifest. This patch fixed the bug in the original code.
>
>
> Diffs
> -----
>
> src/docker/spec.cpp 5bbd98cb1c5f3aefd9050859d066b43360e6eb75
> src/docker/v2.proto fdf159d003f8e5718de0f4d8bf9a2b1374cc11a2
> src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp 049d46cad5cf94a3fb5d74cbfe649850311d35ad
> src/tests/containerizer/docker_spec_tests.cpp PRE-CREATION
> src/tests/containerizer/provisioner_docker_tests.cpp d51f342dabf386fb618ef72ce3e36a8bd8c82b5f
>
> Diff: https://reviews.apache.org/r/41861/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Jie Yu
>
>
Re: Review Request 41861: Fixed a problem of parsing v1Compatibility
in Docker v2 image manifest.
Posted by Timothy Chen <tn...@apache.org>.
> On Jan. 4, 2016, 10:06 p.m., Timothy Chen wrote:
> > src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp, line 257
> > <https://reviews.apache.org/r/41861/diff/2/?file=1180580#file1180580line257>
> >
> > Should we also CHECK here fslayers_size() == history size?
>
> Gilbert Song wrote:
> Actually it is validated spec::v2::parse in getManifest before we call downloadLayers.
I know it is, that's why I thought we can have a CHECK as it should be validated.
- Timothy
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41861/#review112657
-----------------------------------------------------------
On Jan. 3, 2016, 6:33 p.m., Jie Yu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41861/
> -----------------------------------------------------------
>
> (Updated Jan. 3, 2016, 6:33 p.m.)
>
>
> Review request for mesos, Gilbert Song, Jojy Varghese, and Timothy Chen.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Fixed a problem of parsing v1Compatibility in Docker v2 image manifest.
>
> According to Docker v2 image manifest spec, v1Compatibility is a string containing a raw JSON object. The raw JSON object is v1::ImageManifest. This patch fixed the bug in the original code.
>
>
> Diffs
> -----
>
> src/docker/spec.cpp 5bbd98cb1c5f3aefd9050859d066b43360e6eb75
> src/docker/v2.proto fdf159d003f8e5718de0f4d8bf9a2b1374cc11a2
> src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp 049d46cad5cf94a3fb5d74cbfe649850311d35ad
> src/tests/containerizer/docker_spec_tests.cpp PRE-CREATION
> src/tests/containerizer/provisioner_docker_tests.cpp d51f342dabf386fb618ef72ce3e36a8bd8c82b5f
>
> Diff: https://reviews.apache.org/r/41861/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Jie Yu
>
>
Re: Review Request 41861: Fixed a problem of parsing v1Compatibility
in Docker v2 image manifest.
Posted by Gilbert Song <so...@gmail.com>.
> On Jan. 4, 2016, 2:06 p.m., Timothy Chen wrote:
> > src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp, line 257
> > <https://reviews.apache.org/r/41861/diff/2/?file=1180580#file1180580line257>
> >
> > Should we also CHECK here fslayers_size() == history size?
>
> Gilbert Song wrote:
> Actually it is validated spec::v2::parse in getManifest before we call downloadLayers.
>
> Timothy Chen wrote:
> I know it is, that's why I thought we can have a CHECK as it should be validated.
Gotcha. Thanks for explaining. :)
- Gilbert
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41861/#review112657
-----------------------------------------------------------
On Jan. 3, 2016, 10:33 a.m., Jie Yu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41861/
> -----------------------------------------------------------
>
> (Updated Jan. 3, 2016, 10:33 a.m.)
>
>
> Review request for mesos, Gilbert Song, Jojy Varghese, and Timothy Chen.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Fixed a problem of parsing v1Compatibility in Docker v2 image manifest.
>
> According to Docker v2 image manifest spec, v1Compatibility is a string containing a raw JSON object. The raw JSON object is v1::ImageManifest. This patch fixed the bug in the original code.
>
>
> Diffs
> -----
>
> src/docker/spec.cpp 5bbd98cb1c5f3aefd9050859d066b43360e6eb75
> src/docker/v2.proto fdf159d003f8e5718de0f4d8bf9a2b1374cc11a2
> src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp 049d46cad5cf94a3fb5d74cbfe649850311d35ad
> src/tests/containerizer/docker_spec_tests.cpp PRE-CREATION
> src/tests/containerizer/provisioner_docker_tests.cpp d51f342dabf386fb618ef72ce3e36a8bd8c82b5f
>
> Diff: https://reviews.apache.org/r/41861/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Jie Yu
>
>
Re: Review Request 41861: Fixed a problem of parsing v1Compatibility
in Docker v2 image manifest.
Posted by Timothy Chen <tn...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41861/#review112657
-----------------------------------------------------------
Ship it!
Ship It!
src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp (line 257)
<https://reviews.apache.org/r/41861/#comment173168>
Should we also CHECK here fslayers_size() == history size?
- Timothy Chen
On Jan. 3, 2016, 6:33 p.m., Jie Yu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41861/
> -----------------------------------------------------------
>
> (Updated Jan. 3, 2016, 6:33 p.m.)
>
>
> Review request for mesos, Gilbert Song, Jojy Varghese, and Timothy Chen.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Fixed a problem of parsing v1Compatibility in Docker v2 image manifest.
>
> According to Docker v2 image manifest spec, v1Compatibility is a string containing a raw JSON object. The raw JSON object is v1::ImageManifest. This patch fixed the bug in the original code.
>
>
> Diffs
> -----
>
> src/docker/spec.cpp 5bbd98cb1c5f3aefd9050859d066b43360e6eb75
> src/docker/v2.proto fdf159d003f8e5718de0f4d8bf9a2b1374cc11a2
> src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp 049d46cad5cf94a3fb5d74cbfe649850311d35ad
> src/tests/containerizer/docker_spec_tests.cpp PRE-CREATION
> src/tests/containerizer/provisioner_docker_tests.cpp d51f342dabf386fb618ef72ce3e36a8bd8c82b5f
>
> Diff: https://reviews.apache.org/r/41861/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Jie Yu
>
>
Re: Review Request 41861: Fixed a problem of parsing v1Compatibility
in Docker v2 image manifest.
Posted by Mesos ReviewBot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41861/#review112475
-----------------------------------------------------------
Patch looks great!
Reviews applied: [41839, 41840, 41841, 41842, 41843, 41861]
Passed command: export OS=ubuntu:14.04;export CONFIGURATION="--verbose";export COMPILER=gcc; ./support/docker_build.sh
- Mesos ReviewBot
On Jan. 3, 2016, 6:33 p.m., Jie Yu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41861/
> -----------------------------------------------------------
>
> (Updated Jan. 3, 2016, 6:33 p.m.)
>
>
> Review request for mesos, Gilbert Song, Jojy Varghese, and Timothy Chen.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Fixed a problem of parsing v1Compatibility in Docker v2 image manifest.
>
> According to Docker v2 image manifest spec, v1Compatibility is a string containing a raw JSON object. The raw JSON object is v1::ImageManifest. This patch fixed the bug in the original code.
>
>
> Diffs
> -----
>
> src/docker/spec.cpp 5bbd98cb1c5f3aefd9050859d066b43360e6eb75
> src/docker/v2.proto fdf159d003f8e5718de0f4d8bf9a2b1374cc11a2
> src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp 049d46cad5cf94a3fb5d74cbfe649850311d35ad
> src/tests/containerizer/docker_spec_tests.cpp PRE-CREATION
> src/tests/containerizer/provisioner_docker_tests.cpp d51f342dabf386fb618ef72ce3e36a8bd8c82b5f
>
> Diff: https://reviews.apache.org/r/41861/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Jie Yu
>
>
Re: Review Request 41861: Fixed a problem of parsing v1Compatibility
in Docker v2 image manifest.
Posted by Jie Yu <yu...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41861/
-----------------------------------------------------------
(Updated Jan. 3, 2016, 6:33 p.m.)
Review request for mesos, Gilbert Song, Jojy Varghese, and Timothy Chen.
Changes
-------
Rebased.
Repository: mesos
Description
-------
Fixed a problem of parsing v1Compatibility in Docker v2 image manifest.
According to Docker v2 image manifest spec, v1Compatibility is a string containing a raw JSON object. The raw JSON object is v1::ImageManifest. This patch fixed the bug in the original code.
Diffs (updated)
-----
src/docker/spec.cpp 5bbd98cb1c5f3aefd9050859d066b43360e6eb75
src/docker/v2.proto fdf159d003f8e5718de0f4d8bf9a2b1374cc11a2
src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp 049d46cad5cf94a3fb5d74cbfe649850311d35ad
src/tests/containerizer/docker_spec_tests.cpp PRE-CREATION
src/tests/containerizer/provisioner_docker_tests.cpp d51f342dabf386fb618ef72ce3e36a8bd8c82b5f
Diff: https://reviews.apache.org/r/41861/diff/
Testing
-------
make check
Thanks,
Jie Yu