You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@mesos.apache.org by Niklas Nielsen <ni...@qni.dk> on 2014/04/22 23:25:22 UTC

Review Request 20576: Added support for launching tasks by TaskInfo.

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

Review request for mesos and Benjamin Hindman.


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


Repository: mesos-git


Description
-------

Prior to containerizer API, a task could either 1) define the executor
to run one or more tasks or 2) define the command to run. In the
latter case, the slave would generate an executor info which started
the internal mesos-executor. However, this scenario got more involved
when we introduced the notion of containerizes/containers and with the
upcoming external containerizer.

Ideally, what we want is the ability to deal with five scenarios:
1) A task defines an executor to run 2) possibly within a container
image.

3) A task defines a command to run 4) possibly within a container.

5) A task defines a container image only (the container runs necessary
tasks).

As it is right now, executors are necessary to run any command and/or
container. But it should ultimately be up to the containerizer how to
do this.

This patch extends the containerizer API with one over-loaded launch
method:

Previous: Future<Nothing> launch(..., executorInfo, ...)
New:      Future<Nothing> launch(..., taskInfo, executorInfo, ...)

The latter is specifically to run the standalone tasks and provides
the generated executorInfo for convenience and is used for bookkeeping
withing the slave. Eventually, this can be obsoleted.

In the longer term, we can hopefully obsolete requirement for present
executors for standalone tasks all-together.


Diffs
-----

  src/slave/containerizer/containerizer.hpp 9c5af54 
  src/slave/containerizer/mesos_containerizer.hpp 152a6b9 
  src/slave/containerizer/mesos_containerizer.cpp 5afd26b 
  src/slave/slave.cpp b3c4285 
  src/tests/containerizer.hpp 562304c 
  src/tests/containerizer.cpp 2599727 

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


Testing
-------

make check


Thanks,

Niklas Nielsen


Re: Review Request 20576: Added support for launching tasks by TaskInfo.

Posted by Niklas Nielsen <ni...@qni.dk>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20576/
-----------------------------------------------------------

(Updated April 22, 2014, 5:50 p.m.)


Review request for mesos and Benjamin Hindman.


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


Repository: mesos-git


Description
-------

Prior to containerizer API, a task could either 1) define the executor
to run one or more tasks or 2) define the command to run. In the
latter case, the slave would generate an executor info which started
the internal mesos-executor. However, this scenario got more involved
when we introduced the notion of containerizes/containers and with the
upcoming external containerizer.

Ideally, what we want is the ability to deal with five scenarios:
1) A task defines an executor to run 2) possibly within a container
image.

3) A task defines a command to run 4) possibly within a container.

5) A task defines a container image only (the container runs necessary
tasks).

As it is right now, executors are necessary to run any command and/or
container. But it should ultimately be up to the containerizer how to
do this.

This patch extends the containerizer API with one over-loaded launch
method:

Previous: Future<Nothing> launch(..., executorInfo, ...)
New:      Future<Nothing> launch(..., taskInfo, executorInfo, ...)

The latter is specifically to run the standalone tasks and provides
the generated executorInfo for convenience and is used for bookkeeping
withing the slave. Eventually, this can be obsoleted.

In the longer term, we can hopefully obsolete requirement for present
executors for standalone tasks all-together.


Diffs
-----

  src/slave/containerizer/containerizer.hpp 9c5af54 
  src/slave/containerizer/mesos_containerizer.hpp 152a6b9 
  src/slave/containerizer/mesos_containerizer.cpp 5afd26b 
  src/slave/slave.cpp b3c4285 
  src/tests/containerizer.hpp 562304c 
  src/tests/containerizer.cpp 2599727 

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


Testing
-------

make check


Thanks,

Niklas Nielsen


Re: Review Request 20576: Added support for launching tasks by TaskInfo.

Posted by Niklas Nielsen <ni...@qni.dk>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20576/
-----------------------------------------------------------

(Updated April 22, 2014, 5:48 p.m.)


Review request for mesos and Benjamin Hindman.


Changes
-------

Addressed to Ben's comments.


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


Repository: mesos-git


Description
-------

Prior to containerizer API, a task could either 1) define the executor
to run one or more tasks or 2) define the command to run. In the
latter case, the slave would generate an executor info which started
the internal mesos-executor. However, this scenario got more involved
when we introduced the notion of containerizes/containers and with the
upcoming external containerizer.

Ideally, what we want is the ability to deal with five scenarios:
1) A task defines an executor to run 2) possibly within a container
image.

3) A task defines a command to run 4) possibly within a container.

5) A task defines a container image only (the container runs necessary
tasks).

As it is right now, executors are necessary to run any command and/or
container. But it should ultimately be up to the containerizer how to
do this.

This patch extends the containerizer API with one over-loaded launch
method:

Previous: Future<Nothing> launch(..., executorInfo, ...)
New:      Future<Nothing> launch(..., taskInfo, executorInfo, ...)

The latter is specifically to run the standalone tasks and provides
the generated executorInfo for convenience and is used for bookkeeping
withing the slave. Eventually, this can be obsoleted.

In the longer term, we can hopefully obsolete requirement for present
executors for standalone tasks all-together.


Diffs (updated)
-----

  src/slave/containerizer/containerizer.hpp 9c5af54 
  src/slave/containerizer/mesos_containerizer.hpp 152a6b9 
  src/slave/containerizer/mesos_containerizer.cpp 5afd26b 
  src/slave/slave.cpp b3c4285 
  src/tests/containerizer.hpp 562304c 
  src/tests/containerizer.cpp 2599727 

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


Testing
-------

make check


Thanks,

Niklas Nielsen


Re: Review Request 20576: Added support for launching tasks by TaskInfo.

Posted by Benjamin Hindman <be...@berkeley.edu>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20576/#review41096
-----------------------------------------------------------

Ship it!


SHIP SHIP SHIP SHIP SHIP!


src/slave/containerizer/containerizer.hpp
<https://reviews.apache.org/r/20576/#comment74506>

    Let's please s/task/taskInfo/ here and everywhere else for consistency. Thanks!



src/slave/containerizer/mesos_containerizer.cpp
<https://reviews.apache.org/r/20576/#comment74508>

    In this case we can omit the parameter name entirely since we're not using it (same for TestContainerizer below too). In fact, some compilers will even give an error for this.



src/slave/containerizer/mesos_containerizer.cpp
<https://reviews.apache.org/r/20576/#comment74507>

    +4 after parameter/argument list continuations please and thank you. ;) In the code below too.


- Benjamin Hindman


On April 22, 2014, 9:25 p.m., Niklas Nielsen wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20576/
> -----------------------------------------------------------
> 
> (Updated April 22, 2014, 9:25 p.m.)
> 
> 
> Review request for mesos and Benjamin Hindman.
> 
> 
> Bugs: MESOS-922
>     https://issues.apache.org/jira/browse/MESOS-922
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> Prior to containerizer API, a task could either 1) define the executor
> to run one or more tasks or 2) define the command to run. In the
> latter case, the slave would generate an executor info which started
> the internal mesos-executor. However, this scenario got more involved
> when we introduced the notion of containerizes/containers and with the
> upcoming external containerizer.
> 
> Ideally, what we want is the ability to deal with five scenarios:
> 1) A task defines an executor to run 2) possibly within a container
> image.
> 
> 3) A task defines a command to run 4) possibly within a container.
> 
> 5) A task defines a container image only (the container runs necessary
> tasks).
> 
> As it is right now, executors are necessary to run any command and/or
> container. But it should ultimately be up to the containerizer how to
> do this.
> 
> This patch extends the containerizer API with one over-loaded launch
> method:
> 
> Previous: Future<Nothing> launch(..., executorInfo, ...)
> New:      Future<Nothing> launch(..., taskInfo, executorInfo, ...)
> 
> The latter is specifically to run the standalone tasks and provides
> the generated executorInfo for convenience and is used for bookkeeping
> withing the slave. Eventually, this can be obsoleted.
> 
> In the longer term, we can hopefully obsolete requirement for present
> executors for standalone tasks all-together.
> 
> 
> Diffs
> -----
> 
>   src/slave/containerizer/containerizer.hpp 9c5af54 
>   src/slave/containerizer/mesos_containerizer.hpp 152a6b9 
>   src/slave/containerizer/mesos_containerizer.cpp 5afd26b 
>   src/slave/slave.cpp b3c4285 
>   src/tests/containerizer.hpp 562304c 
>   src/tests/containerizer.cpp 2599727 
> 
> Diff: https://reviews.apache.org/r/20576/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Niklas Nielsen
> 
>