You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Kapil Arya <ka...@mesosphere.io> on 2015/10/27 22:54:37 UTC

Review Request 39703: [WIP] Exposed container-id via TaskStatus updates.

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39703/
-----------------------------------------------------------

Review request for mesos and Jie Yu.


Bugs: MESOS-3688
    https://issues.apache.org/jira/browse/MESOS-3688


Repository: mesos


Description
-------

The container-id is mesos-specific and so it's not quite clear what good could it do. However, for Docker containers, one can use the "docker container id" for various things such as "docker ps". This particular patch doesn't expose docker container ids yet.


Diffs
-----

  include/mesos/mesos.proto 94004343ea615d87d7c7d63a6a81aaaa1cf8002e 
  src/common/http.cpp f56d8a178e1f61d91adfeaad32b47718adbc4379 
  src/slave/slave.cpp 5f9b52b41eaab0c24965f28e192074340e00bde5 
  src/tests/master_tests.cpp ee2473997ccbd1c50d0cbf65d1259ea2dfe82971 

Diff: https://reviews.apache.org/r/39703/diff/


Testing
-------

make check with updated MasterTest.TaskStatusContainerStatus test.


Thanks,

Kapil Arya


Re: Review Request 39703: [WIP] Exposed container-id via TaskStatus updates.

Posted by Mesos ReviewBot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39703/#review104240
-----------------------------------------------------------


Patch looks great!

Reviews applied: [39702, 39703]

All tests passed.

- Mesos ReviewBot


On Oct. 27, 2015, 9:54 p.m., Kapil Arya wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39703/
> -----------------------------------------------------------
> 
> (Updated Oct. 27, 2015, 9:54 p.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-3688
>     https://issues.apache.org/jira/browse/MESOS-3688
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> The container-id is mesos-specific and so it's not quite clear what good could it do. However, for Docker containers, one can use the "docker container id" for various things such as "docker ps". This particular patch doesn't expose docker container ids yet.
> 
> 
> Diffs
> -----
> 
>   include/mesos/mesos.proto 94004343ea615d87d7c7d63a6a81aaaa1cf8002e 
>   src/common/http.cpp f56d8a178e1f61d91adfeaad32b47718adbc4379 
>   src/slave/slave.cpp 5f9b52b41eaab0c24965f28e192074340e00bde5 
>   src/tests/master_tests.cpp ee2473997ccbd1c50d0cbf65d1259ea2dfe82971 
> 
> Diff: https://reviews.apache.org/r/39703/diff/
> 
> 
> Testing
> -------
> 
> make check with updated MasterTest.TaskStatusContainerStatus test.
> 
> 
> Thanks,
> 
> Kapil Arya
> 
>


Re: Review Request 39703: [WIP] Exposed container-id via TaskStatus updates.

Posted by Ben Mahler <be...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39703/#review105203
-----------------------------------------------------------



include/mesos/mesos.proto (lines 1475 - 1484)
<https://reviews.apache.org/r/39703/#comment163624>

    Just noticing this now, do we want to be sending this inside reconciliation updates? The fact that health was added seems to suggest container status should be added as well.


- Ben Mahler


On Oct. 27, 2015, 9:54 p.m., Kapil Arya wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39703/
> -----------------------------------------------------------
> 
> (Updated Oct. 27, 2015, 9:54 p.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-3688
>     https://issues.apache.org/jira/browse/MESOS-3688
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> The container-id is mesos-specific and so it's not quite clear what good could it do. However, for Docker containers, one can use the "docker container id" for various things such as "docker ps". This particular patch doesn't expose docker container ids yet.
> 
> 
> Diffs
> -----
> 
>   include/mesos/mesos.proto 94004343ea615d87d7c7d63a6a81aaaa1cf8002e 
>   src/common/http.cpp f56d8a178e1f61d91adfeaad32b47718adbc4379 
>   src/slave/slave.cpp 5f9b52b41eaab0c24965f28e192074340e00bde5 
>   src/tests/master_tests.cpp ee2473997ccbd1c50d0cbf65d1259ea2dfe82971 
> 
> Diff: https://reviews.apache.org/r/39703/diff/
> 
> 
> Testing
> -------
> 
> make check with updated MasterTest.TaskStatusContainerStatus test.
> 
> 
> Thanks,
> 
> Kapil Arya
> 
>


Re: Review Request 39703: [WIP] Exposed container-id via TaskStatus updates.

Posted by Guangya Liu <gy...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39703/#review104528
-----------------------------------------------------------


Kapil, does it make sense to create another patch handle the container id issues for docker and leave this as it is now?

- Guangya Liu


On Oct. 27, 2015, 9:54 p.m., Kapil Arya wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39703/
> -----------------------------------------------------------
> 
> (Updated Oct. 27, 2015, 9:54 p.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-3688
>     https://issues.apache.org/jira/browse/MESOS-3688
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> The container-id is mesos-specific and so it's not quite clear what good could it do. However, for Docker containers, one can use the "docker container id" for various things such as "docker ps". This particular patch doesn't expose docker container ids yet.
> 
> 
> Diffs
> -----
> 
>   include/mesos/mesos.proto 94004343ea615d87d7c7d63a6a81aaaa1cf8002e 
>   src/common/http.cpp f56d8a178e1f61d91adfeaad32b47718adbc4379 
>   src/slave/slave.cpp 5f9b52b41eaab0c24965f28e192074340e00bde5 
>   src/tests/master_tests.cpp ee2473997ccbd1c50d0cbf65d1259ea2dfe82971 
> 
> Diff: https://reviews.apache.org/r/39703/diff/
> 
> 
> Testing
> -------
> 
> make check with updated MasterTest.TaskStatusContainerStatus test.
> 
> 
> Thanks,
> 
> Kapil Arya
> 
>