You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Timothy Chen <tn...@apache.org> on 2015/11/02 19:59:19 UTC

Review Request 39866: Added containerInfo support for tasks in mesos containerizer.

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

Review request for mesos, Jie Yu and Jojy Varghese.


Repository: mesos


Description
-------

Added containerInfo support for tasks in mesos containerizer.


Diffs
-----

  src/slave/containerizer/mesos/containerizer.hpp 4aad8a3be43b331efc6b8157b2fae090df16c1b4 
  src/slave/containerizer/mesos/containerizer.cpp 9fd69c1738e2300dbb843d259727010e24523cff 

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


Testing
-------

make check


Thanks,

Timothy Chen


Re: Review Request 39866: Added containerInfo support for tasks in mesos containerizer.

Posted by Jie Yu <yu...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39866/#review104845
-----------------------------------------------------------



src/slave/containerizer/mesos/containerizer.cpp 
<https://reviews.apache.org/r/39866/#comment163112>

    Any reason remove this check?


- Jie Yu


On Nov. 2, 2015, 6:59 p.m., Timothy Chen wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39866/
> -----------------------------------------------------------
> 
> (Updated Nov. 2, 2015, 6:59 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Jojy Varghese.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Added containerInfo support for tasks in mesos containerizer.
> 
> 
> Diffs
> -----
> 
>   src/slave/containerizer/mesos/containerizer.hpp 4aad8a3be43b331efc6b8157b2fae090df16c1b4 
>   src/slave/containerizer/mesos/containerizer.cpp 9fd69c1738e2300dbb843d259727010e24523cff 
> 
> Diff: https://reviews.apache.org/r/39866/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Timothy Chen
> 
>


Re: Review Request 39866: Added containerInfo support for tasks in mesos containerizer.

Posted by Kapil Arya <ka...@mesosphere.io>.

> On Nov. 3, 2015, 1:42 p.m., Kapil Arya wrote:
> > Can we merge the Tests from https://reviews.apache.org/r/39769/?
> 
> Timothy Chen wrote:
>     I thought you'd like to merge that patch seperately?

Yes, we'll do that.


- Kapil


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


On Nov. 4, 2015, 3:58 p.m., Timothy Chen wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39866/
> -----------------------------------------------------------
> 
> (Updated Nov. 4, 2015, 3:58 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Jojy Varghese.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Added containerInfo support for tasks in mesos containerizer.
> 
> 
> Diffs
> -----
> 
>   src/slave/containerizer/mesos/containerizer.hpp 4aad8a3be43b331efc6b8157b2fae090df16c1b4 
>   src/slave/containerizer/mesos/containerizer.cpp 9fd69c1738e2300dbb843d259727010e24523cff 
> 
> Diff: https://reviews.apache.org/r/39866/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Timothy Chen
> 
>


Re: Review Request 39866: Added containerInfo support for tasks in mesos containerizer.

Posted by Timothy Chen <tn...@apache.org>.

> On Nov. 3, 2015, 6:42 p.m., Kapil Arya wrote:
> > Can we merge the Tests from https://reviews.apache.org/r/39769/?

I thought you'd like to merge that patch seperately?


- Timothy


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


On Nov. 2, 2015, 6:59 p.m., Timothy Chen wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39866/
> -----------------------------------------------------------
> 
> (Updated Nov. 2, 2015, 6:59 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Jojy Varghese.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Added containerInfo support for tasks in mesos containerizer.
> 
> 
> Diffs
> -----
> 
>   src/slave/containerizer/mesos/containerizer.hpp 4aad8a3be43b331efc6b8157b2fae090df16c1b4 
>   src/slave/containerizer/mesos/containerizer.cpp 9fd69c1738e2300dbb843d259727010e24523cff 
> 
> Diff: https://reviews.apache.org/r/39866/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Timothy Chen
> 
>


Re: Review Request 39866: Added containerInfo support for tasks in mesos containerizer.

Posted by Kapil Arya <ka...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39866/#review104945
-----------------------------------------------------------


Can we merge the Tests from https://reviews.apache.org/r/39769/?

- Kapil Arya


On Nov. 2, 2015, 1:59 p.m., Timothy Chen wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39866/
> -----------------------------------------------------------
> 
> (Updated Nov. 2, 2015, 1:59 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Jojy Varghese.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Added containerInfo support for tasks in mesos containerizer.
> 
> 
> Diffs
> -----
> 
>   src/slave/containerizer/mesos/containerizer.hpp 4aad8a3be43b331efc6b8157b2fae090df16c1b4 
>   src/slave/containerizer/mesos/containerizer.cpp 9fd69c1738e2300dbb843d259727010e24523cff 
> 
> Diff: https://reviews.apache.org/r/39866/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Timothy Chen
> 
>


Re: Review Request 39866: Added containerInfo support for tasks in mesos containerizer.

Posted by Jojy Varghese <jo...@mesosphere.io>.

> On Nov. 2, 2015, 8:19 p.m., Jojy Varghese wrote:
> > src/slave/containerizer/mesos/containerizer.cpp, line 605
> > <https://reviews.apache.org/r/39866/diff/1/?file=1114138#file1114138line605>
> >
> >     Wondering if this means that we can use ContainerInfo as a predicate for "launch" inside the "composing" containerizer? Today we call "launch" on the containerizer(docker or mesos) and let the "luanch" decide if it can handle the task. Now that we have the information right away at the composing containerizer, maybe we can use that?
> 
> Timothy Chen wrote:
>     we cant since we cant distinguish between mesos containerizer and external containerizer

Ah. i thought external containerizer is to be discontinued.


- Jojy


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


On Nov. 2, 2015, 6:59 p.m., Timothy Chen wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39866/
> -----------------------------------------------------------
> 
> (Updated Nov. 2, 2015, 6:59 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Jojy Varghese.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Added containerInfo support for tasks in mesos containerizer.
> 
> 
> Diffs
> -----
> 
>   src/slave/containerizer/mesos/containerizer.hpp 4aad8a3be43b331efc6b8157b2fae090df16c1b4 
>   src/slave/containerizer/mesos/containerizer.cpp 9fd69c1738e2300dbb843d259727010e24523cff 
> 
> Diff: https://reviews.apache.org/r/39866/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Timothy Chen
> 
>


Re: Review Request 39866: Added containerInfo support for tasks in mesos containerizer.

Posted by Timothy Chen <tn...@apache.org>.

> On Nov. 2, 2015, 8:19 p.m., Jojy Varghese wrote:
> > src/slave/containerizer/mesos/containerizer.cpp, line 605
> > <https://reviews.apache.org/r/39866/diff/1/?file=1114138#file1114138line605>
> >
> >     Wondering if this means that we can use ContainerInfo as a predicate for "launch" inside the "composing" containerizer? Today we call "launch" on the containerizer(docker or mesos) and let the "luanch" decide if it can handle the task. Now that we have the information right away at the composing containerizer, maybe we can use that?
> 
> Timothy Chen wrote:
>     we cant since we cant distinguish between mesos containerizer and external containerizer
> 
> Jojy Varghese wrote:
>     Ah. i thought external containerizer is to be discontinued.

Not yet though :)


- Timothy


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


On Nov. 2, 2015, 6:59 p.m., Timothy Chen wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39866/
> -----------------------------------------------------------
> 
> (Updated Nov. 2, 2015, 6:59 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Jojy Varghese.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Added containerInfo support for tasks in mesos containerizer.
> 
> 
> Diffs
> -----
> 
>   src/slave/containerizer/mesos/containerizer.hpp 4aad8a3be43b331efc6b8157b2fae090df16c1b4 
>   src/slave/containerizer/mesos/containerizer.cpp 9fd69c1738e2300dbb843d259727010e24523cff 
> 
> Diff: https://reviews.apache.org/r/39866/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Timothy Chen
> 
>


Re: Review Request 39866: Added containerInfo support for tasks in mesos containerizer.

Posted by Timothy Chen <tn...@apache.org>.

> On Nov. 2, 2015, 8:19 p.m., Jojy Varghese wrote:
> > src/slave/containerizer/mesos/containerizer.cpp, line 605
> > <https://reviews.apache.org/r/39866/diff/1/?file=1114138#file1114138line605>
> >
> >     Wondering if this means that we can use ContainerInfo as a predicate for "launch" inside the "composing" containerizer? Today we call "launch" on the containerizer(docker or mesos) and let the "luanch" decide if it can handle the task. Now that we have the information right away at the composing containerizer, maybe we can use that?

we cant since we cant distinguish between mesos containerizer and external containerizer


- Timothy


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


On Nov. 2, 2015, 6:59 p.m., Timothy Chen wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39866/
> -----------------------------------------------------------
> 
> (Updated Nov. 2, 2015, 6:59 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Jojy Varghese.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Added containerInfo support for tasks in mesos containerizer.
> 
> 
> Diffs
> -----
> 
>   src/slave/containerizer/mesos/containerizer.hpp 4aad8a3be43b331efc6b8157b2fae090df16c1b4 
>   src/slave/containerizer/mesos/containerizer.cpp 9fd69c1738e2300dbb843d259727010e24523cff 
> 
> Diff: https://reviews.apache.org/r/39866/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Timothy Chen
> 
>


Re: Review Request 39866: Added containerInfo support for tasks in mesos containerizer.

Posted by Jojy Varghese <jo...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39866/#review104782
-----------------------------------------------------------



src/slave/containerizer/mesos/containerizer.cpp (line 578)
<https://reviews.apache.org/r/39866/#comment163022>

    Wondering if this means that we can use ContainerInfo as a predicate for "launch" inside the "composing" containerizer? Today we call "launch" on the containerizer(docker or mesos) and let the "luanch" decide if it can handle the task. Now that we have the information right away at the composing containerizer, maybe we can use that?


- Jojy Varghese


On Nov. 2, 2015, 6:59 p.m., Timothy Chen wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39866/
> -----------------------------------------------------------
> 
> (Updated Nov. 2, 2015, 6:59 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Jojy Varghese.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Added containerInfo support for tasks in mesos containerizer.
> 
> 
> Diffs
> -----
> 
>   src/slave/containerizer/mesos/containerizer.hpp 4aad8a3be43b331efc6b8157b2fae090df16c1b4 
>   src/slave/containerizer/mesos/containerizer.cpp 9fd69c1738e2300dbb843d259727010e24523cff 
> 
> Diff: https://reviews.apache.org/r/39866/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Timothy Chen
> 
>


Re: Review Request 39866: Added containerInfo support for tasks in mesos containerizer.

Posted by Kapil Arya <ka...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39866/#review105280
-----------------------------------------------------------

Ship it!


Ship It!

- Kapil Arya


On Nov. 4, 2015, 3:58 p.m., Timothy Chen wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39866/
> -----------------------------------------------------------
> 
> (Updated Nov. 4, 2015, 3:58 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Jojy Varghese.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Added containerInfo support for tasks in mesos containerizer.
> 
> 
> Diffs
> -----
> 
>   src/slave/containerizer/mesos/containerizer.hpp 4aad8a3be43b331efc6b8157b2fae090df16c1b4 
>   src/slave/containerizer/mesos/containerizer.cpp 9fd69c1738e2300dbb843d259727010e24523cff 
> 
> Diff: https://reviews.apache.org/r/39866/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Timothy Chen
> 
>


Re: Review Request 39866: Added containerInfo support for tasks in mesos containerizer.

Posted by Jie Yu <yu...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39866/#review105130
-----------------------------------------------------------

Ship it!


Ship It!

- Jie Yu


On Nov. 4, 2015, 8:58 p.m., Timothy Chen wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39866/
> -----------------------------------------------------------
> 
> (Updated Nov. 4, 2015, 8:58 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Jojy Varghese.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Added containerInfo support for tasks in mesos containerizer.
> 
> 
> Diffs
> -----
> 
>   src/slave/containerizer/mesos/containerizer.hpp 4aad8a3be43b331efc6b8157b2fae090df16c1b4 
>   src/slave/containerizer/mesos/containerizer.cpp 9fd69c1738e2300dbb843d259727010e24523cff 
> 
> Diff: https://reviews.apache.org/r/39866/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Timothy Chen
> 
>


Re: Review Request 39866: Added containerInfo support for tasks in mesos containerizer.

Posted by Timothy Chen <tn...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39866/
-----------------------------------------------------------

(Updated Nov. 4, 2015, 8:58 p.m.)


Review request for mesos, Jie Yu and Jojy Varghese.


Repository: mesos


Description
-------

Added containerInfo support for tasks in mesos containerizer.


Diffs (updated)
-----

  src/slave/containerizer/mesos/containerizer.hpp 4aad8a3be43b331efc6b8157b2fae090df16c1b4 
  src/slave/containerizer/mesos/containerizer.cpp 9fd69c1738e2300dbb843d259727010e24523cff 

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


Testing
-------

make check


Thanks,

Timothy Chen


Re: Review Request 39866: Added containerInfo support for tasks in mesos containerizer.

Posted by Timothy Chen <tn...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39866/#review105041
-----------------------------------------------------------



src/slave/containerizer/mesos/containerizer.cpp 
<https://reviews.apache.org/r/39866/#comment163408>

    I originally removed this since I added it to the ExecutorInfo command, but now we didn't go down that route I think we can put the check back.
    
    Are we planning to support custom executor with image?


- Timothy Chen


On Nov. 2, 2015, 6:59 p.m., Timothy Chen wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39866/
> -----------------------------------------------------------
> 
> (Updated Nov. 2, 2015, 6:59 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Jojy Varghese.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Added containerInfo support for tasks in mesos containerizer.
> 
> 
> Diffs
> -----
> 
>   src/slave/containerizer/mesos/containerizer.hpp 4aad8a3be43b331efc6b8157b2fae090df16c1b4 
>   src/slave/containerizer/mesos/containerizer.cpp 9fd69c1738e2300dbb843d259727010e24523cff 
> 
> Diff: https://reviews.apache.org/r/39866/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Timothy Chen
> 
>