Re: Review Request 41861: Fixed a problem of parsing v1Compatibility in Docker v2 image manifest.

[Timothy Chen]

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


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.

[Gilbert Song]

> On Jan. 4, 2016, 2:06 p.m., Timothy Chen wrote:
> > src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp, line 
> > 257
> > 
> >
> > 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.

[Timothy Chen]

> On Jan. 4, 2016, 10:06 p.m., Timothy Chen wrote:
> > src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp, line 
> > 257
> > 
> >
> > 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.

[Gilbert Song]

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

[Gilbert Song]

> On Jan. 4, 2016, 2:06 p.m., Timothy Chen wrote:
> > src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp, line 
> > 257
> > 
> >
> > 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.

[Jie Yu]

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

Re: Review Request 41861: Fixed a problem of parsing v1Compatibility in Docker v2 image manifest.

[Mesos ReviewBot]

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