Re: Review Request 52164: Added `ContainerID` field to the executor `SUBSCRIBED` event.

[Anand Mazumdar]

> On Sept. 23, 2016, 5:07 a.m., Vinod Kone wrote:
> > src/slave/slave.cpp, line 3087
> > 
> >
> > weren't you going to remove the corresponding evolve method?

hmm, it's a bit non trivial to do owing to the fact that `send()` is templated 
on the message type. We invoke `evolve()` if it's an HTTP based executor inside 
`send()`. Due to the method being statically typed on `Message`, `send()` still 
expects the `evolve()` handler to be present for that type i.e. 
`ExecutorRegisteredMessage`.


- Anand


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


On Sept. 23, 2016, 3:24 a.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52164/
> ---
> 
> (Updated Sept. 23, 2016, 3:24 a.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-6227
> https://issues.apache.org/jira/browse/MESOS-6227
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This change adds the field `ContainerID` to the executor `SUBSCRIBED`
> event. This allows the default executor/other custom executors to
> then launch nested containers. This is needed as the executors need
> to let the agent know of the parent container ID when they make the
> `LAUNCH_NESTED_CONTAINER` call. This change also implements the
> logic on the agent to send the event to the executor.
> 
> 
> Diffs
> -
> 
>   include/mesos/executor/executor.proto 
> 0d31406a70d6b7d9030c1f157a7869a2e8f1879f 
>   include/mesos/v1/executor/executor.proto 
> f661b71cecb96a9971c687f117625ef242749394 
>   src/slave/slave.cpp 11e9c8af87aa5153f72f2a20cc578fe3d729b153 
>   src/tests/executor_http_api_tests.cpp 
> fd322ee267b2f0d4bf250db15027d61f1fc74a57 
> 
> Diff: https://reviews.apache.org/r/52164/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Re: Review Request 52177: Added support for CNI networks for `--task_group`.

[Vinod Kone]

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


Ship it!




Ship It!

- Vinod Kone


On Sept. 22, 2016, 9:48 p.m., Avinash sridharan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52177/
> ---
> 
> (Updated Sept. 22, 2016, 9:48 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Vinod Kone.
> 
> 
> Bugs: MESOS-6096
> https://issues.apache.org/jira/browse/MESOS-6096
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added support for CNI networks for `--task_group`.
> 
> 
> Diffs
> -
> 
>   src/cli/execute.cpp 79091d6dd3e09e354f6965b0da45662621297bee 
> 
> Diff: https://reviews.apache.org/r/52177/diff/
> 
> 
> Testing
> ---
> 
> make
> 
> Ran `mesos-execute` with the --task_group and --network option with the 
> following POD definition:
> {
>   "tasks":[
> {
>   "name" : "nginx",
>   "task_id" : {"value":"nginx"},
>   "agent_id": {"value" : ""},
>   "resources": [{
> "name": "cpus",
> "type": "SCALAR",
> "scalar": {
>   "value": 0.1
> },
> "role": "*"
>   },
>   {
> "name": "mem",
> "type": "SCALAR",
> "scalar": {
>   "value": 128
> },
> "role": "*"
>   }],
>   "container" : {
> "type" : "MESOS",
> "mesos" : {
>   "image" : {
> "type" : "DOCKER",
> "docker" : {
>   "name" : "nginx"
> }
>   }
> }
>   }
> },
> {
>   "name" : "redis",
>   "task_id" : {"value":"redis"},
>   "agent_id": {"value" : ""},
>   "resources": [{
> "name": "cpus",
> "type": "SCALAR",
> "scalar": {
>   "value": 0.1
> },
> "role": "*"
>   },
>   {
> "name": "mem",
> "type": "SCALAR",
> "scalar": {
>   "value": 128
> },
> "role": "*"
>   }],
>   "container" : {
> "type" : "MESOS",
> "mesos" : {
>   "image" : {
> "type" : "DOCKER",
> "docker" : {
>   "name" : "redis"
> }
>   }
> }
>   }
> }
>   ]
> }
> 
> The pod definition doesn't do much (mesos can't launch docker containers in 
> PODS right now), but the pod itself is attached to a CNI network and it gets 
> an IP address and a network name space of its own.
> 
> 
> Thanks,
> 
> Avinash sridharan
> 
>

Re: Review Request 52177: Added support for CNI networks for `--task_group`.

[Vinod Kone]

> On Sept. 23, 2016, 5:16 a.m., Vinod Kone wrote:
> > src/cli/execute.cpp, lines 468-478
> > 
> >
> > I don't follow this change. Can the TaskGroupInfo JSON not contain the 
> > NetworkInfos? Why do have to specify it via `--networks`?
> 
> Avinash sridharan wrote:
> The proto definitions explicitly state that we should not be setting the 
> `NetworkInfo` in indvidual tasks in a task group:
> 
> https://github.com/apache/mesos/blob/29236068f23b6cfbd19d7a4b5b96be852818a356/include/mesos/v1/mesos.proto#L1433
> 
> Also, its stated here that we should be setting the `NetworkInfo` in 
> `ExecutorInfo` to set networks for all tasks in a task group:
> 
> https://github.com/apache/mesos/blob/29236068f23b6cfbd19d7a4b5b96be852818a356/include/mesos/v1/mesos.proto#L1290
> 
> 
> Hence the setting of network in `ExecutorInfo`.
> 
> Avinash sridharan wrote:
> I believe this has to do with the fact that all tasks within a taskgroup 
> (currently) have to share the same network namespace, and hence this rule?

sorry my bad. totally forgot that we don't allow setting ExecutorInfo via 
command line yet. ignore me.


- Vinod


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


On Sept. 22, 2016, 9:48 p.m., Avinash sridharan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52177/
> ---
> 
> (Updated Sept. 22, 2016, 9:48 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Vinod Kone.
> 
> 
> Bugs: MESOS-6096
> https://issues.apache.org/jira/browse/MESOS-6096
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added support for CNI networks for `--task_group`.
> 
> 
> Diffs
> -
> 
>   src/cli/execute.cpp 79091d6dd3e09e354f6965b0da45662621297bee 
> 
> Diff: https://reviews.apache.org/r/52177/diff/
> 
> 
> Testing
> ---
> 
> make
> 
> Ran `mesos-execute` with the --task_group and --network option with the 
> following POD definition:
> {
>   "tasks":[
> {
>   "name" : "nginx",
>   "task_id" : {"value":"nginx"},
>   "agent_id": {"value" : ""},
>   "resources": [{
> "name": "cpus",
> "type": "SCALAR",
> "scalar": {
>   "value": 0.1
> },
> "role": "*"
>   },
>   {
> "name": "mem",
> "type": "SCALAR",
> "scalar": {
>   "value": 128
> },
> "role": "*"
>   }],
>   "container" : {
> "type" : "MESOS",
> "mesos" : {
>   "image" : {
> "type" : "DOCKER",
> "docker" : {
>   "name" : "nginx"
> }
>   }
> }
>   }
> },
> {
>   "name" : "redis",
>   "task_id" : {"value":"redis"},
>   "agent_id": {"value" : ""},
>   "resources": [{
> "name": "cpus",
> "type": "SCALAR",
> "scalar": {
>   "value": 0.1
> },
> "role": "*"
>   },
>   {
> "name": "mem",
> "type": "SCALAR",
> "scalar": {
>   "value": 128
> },
> "role": "*"
>   }],
>   "container" : {
> "type" : "MESOS",
> "mesos" : {
>   "image" : {
> "type" : "DOCKER",
> "docker" : {
>   "name" : "redis"
> }
>   }
> }
>   }
> }
>   ]
> }
> 
> The pod definition doesn't do much (mesos can't launch docker containers in 
> PODS right now), but the pod itself is attached to a CNI network and it gets 
> an IP address and a network name space of its own.
> 
> 
> Thanks,
> 
> Avinash sridharan
> 
>

Re: Review Request 52177: Added support for CNI networks for `--task_group`.

[Avinash sridharan]

> On Sept. 23, 2016, 5:16 a.m., Vinod Kone wrote:
> > src/cli/execute.cpp, lines 468-478
> > 
> >
> > I don't follow this change. Can the TaskGroupInfo JSON not contain the 
> > NetworkInfos? Why do have to specify it via `--networks`?
> 
> Avinash sridharan wrote:
> The proto definitions explicitly state that we should not be setting the 
> `NetworkInfo` in indvidual tasks in a task group:
> 
> https://github.com/apache/mesos/blob/29236068f23b6cfbd19d7a4b5b96be852818a356/include/mesos/v1/mesos.proto#L1433
> 
> Also, its stated here that we should be setting the `NetworkInfo` in 
> `ExecutorInfo` to set networks for all tasks in a task group:
> 
> https://github.com/apache/mesos/blob/29236068f23b6cfbd19d7a4b5b96be852818a356/include/mesos/v1/mesos.proto#L1290
> 
> 
> Hence the setting of network in `ExecutorInfo`.

I believe this has to do with the fact that all tasks within a taskgroup 
(currently) have to share the same network namespace, and hence this rule?


- Avinash


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


On Sept. 22, 2016, 9:48 p.m., Avinash sridharan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52177/
> ---
> 
> (Updated Sept. 22, 2016, 9:48 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Vinod Kone.
> 
> 
> Bugs: MESOS-6096
> https://issues.apache.org/jira/browse/MESOS-6096
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added support for CNI networks for `--task_group`.
> 
> 
> Diffs
> -
> 
>   src/cli/execute.cpp 79091d6dd3e09e354f6965b0da45662621297bee 
> 
> Diff: https://reviews.apache.org/r/52177/diff/
> 
> 
> Testing
> ---
> 
> make
> 
> Ran `mesos-execute` with the --task_group and --network option with the 
> following POD definition:
> {
>   "tasks":[
> {
>   "name" : "nginx",
>   "task_id" : {"value":"nginx"},
>   "agent_id": {"value" : ""},
>   "resources": [{
> "name": "cpus",
> "type": "SCALAR",
> "scalar": {
>   "value": 0.1
> },
> "role": "*"
>   },
>   {
> "name": "mem",
> "type": "SCALAR",
> "scalar": {
>   "value": 128
> },
> "role": "*"
>   }],
>   "container" : {
> "type" : "MESOS",
> "mesos" : {
>   "image" : {
> "type" : "DOCKER",
> "docker" : {
>   "name" : "nginx"
> }
>   }
> }
>   }
> },
> {
>   "name" : "redis",
>   "task_id" : {"value":"redis"},
>   "agent_id": {"value" : ""},
>   "resources": [{
> "name": "cpus",
> "type": "SCALAR",
> "scalar": {
>   "value": 0.1
> },
> "role": "*"
>   },
>   {
> "name": "mem",
> "type": "SCALAR",
> "scalar": {
>   "value": 128
> },
> "role": "*"
>   }],
>   "container" : {
> "type" : "MESOS",
> "mesos" : {
>   "image" : {
> "type" : "DOCKER",
> "docker" : {
>   "name" : "redis"
> }
>   }
> }
>   }
> }
>   ]
> }
> 
> The pod definition doesn't do much (mesos can't launch docker containers in 
> PODS right now), but the pod itself is attached to a CNI network and it gets 
> an IP address and a network name space of its own.
> 
> 
> Thanks,
> 
> Avinash sridharan
> 
>

Re: Review Request 52177: Added support for CNI networks for `--task_group`.

[Avinash sridharan]

> On Sept. 23, 2016, 5:16 a.m., Vinod Kone wrote:
> > src/cli/execute.cpp, lines 468-478
> > 
> >
> > I don't follow this change. Can the TaskGroupInfo JSON not contain the 
> > NetworkInfos? Why do have to specify it via `--networks`?

The proto definitions explicitly state that we should not be setting the 
`NetworkInfo` in indvidual tasks in a task group:
https://github.com/apache/mesos/blob/29236068f23b6cfbd19d7a4b5b96be852818a356/include/mesos/v1/mesos.proto#L1433

Also, its stated here that we should be setting the `NetworkInfo` in 
`ExecutorInfo` to set networks for all tasks in a task group:
https://github.com/apache/mesos/blob/29236068f23b6cfbd19d7a4b5b96be852818a356/include/mesos/v1/mesos.proto#L1290


Hence the setting of network in `ExecutorInfo`.


- Avinash


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


On Sept. 22, 2016, 9:48 p.m., Avinash sridharan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52177/
> ---
> 
> (Updated Sept. 22, 2016, 9:48 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Vinod Kone.
> 
> 
> Bugs: MESOS-6096
> https://issues.apache.org/jira/browse/MESOS-6096
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added support for CNI networks for `--task_group`.
> 
> 
> Diffs
> -
> 
>   src/cli/execute.cpp 79091d6dd3e09e354f6965b0da45662621297bee 
> 
> Diff: https://reviews.apache.org/r/52177/diff/
> 
> 
> Testing
> ---
> 
> make
> 
> Ran `mesos-execute` with the --task_group and --network option with the 
> following POD definition:
> {
>   "tasks":[
> {
>   "name" : "nginx",
>   "task_id" : {"value":"nginx"},
>   "agent_id": {"value" : ""},
>   "resources": [{
> "name": "cpus",
> "type": "SCALAR",
> "scalar": {
>   "value": 0.1
> },
> "role": "*"
>   },
>   {
> "name": "mem",
> "type": "SCALAR",
> "scalar": {
>   "value": 128
> },
> "role": "*"
>   }],
>   "container" : {
> "type" : "MESOS",
> "mesos" : {
>   "image" : {
> "type" : "DOCKER",
> "docker" : {
>   "name" : "nginx"
> }
>   }
> }
>   }
> },
> {
>   "name" : "redis",
>   "task_id" : {"value":"redis"},
>   "agent_id": {"value" : ""},
>   "resources": [{
> "name": "cpus",
> "type": "SCALAR",
> "scalar": {
>   "value": 0.1
> },
> "role": "*"
>   },
>   {
> "name": "mem",
> "type": "SCALAR",
> "scalar": {
>   "value": 128
> },
> "role": "*"
>   }],
>   "container" : {
> "type" : "MESOS",
> "mesos" : {
>   "image" : {
> "type" : "DOCKER",
> "docker" : {
>   "name" : "redis"
> }
>   }
> }
>   }
> }
>   ]
> }
> 
> The pod definition doesn't do much (mesos can't launch docker containers in 
> PODS right now), but the pod itself is attached to a CNI network and it gets 
> an IP address and a network name space of its own.
> 
> 
> Thanks,
> 
> Avinash sridharan
> 
>

Re: Review Request 52146: Made `killTask()` invoke `shutdown()`.

[Vinod Kone]

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


Fix it, then Ship it!





src/launcher/default_executor.cpp (line 72)


s/killed/shuttingDown/


- Vinod Kone


On Sept. 22, 2016, 7:33 p.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52146/
> ---
> 
> (Updated Sept. 22, 2016, 7:33 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This change gets rid of the trivial `killTask()` implementation
> and makes it call `shutdown()`. It also gets rid of the existing
> `taskGroup` member variable. Eventually, later in the review chain
> we would populate these functions to make the actual calls to the
> agent.
> 
> 
> Diffs
> -
> 
>   src/launcher/default_executor.cpp 2102fe8d70f0960fed669e1c4f0d6b6cd4af261c 
> 
> Diff: https://reviews.apache.org/r/52146/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Re: Review Request 52145: Added support for reading agent API URL in default executor.

[Vinod Kone]

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


Ship it!




Ship It!

- Vinod Kone


On Sept. 22, 2016, 7:15 p.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52145/
> ---
> 
> (Updated Sept. 22, 2016, 7:15 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-6227
> https://issues.apache.org/jira/browse/MESOS-6227
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This change adds support for populating the agent API URL needed
> for making the calls for launching/waiting/killing
> child containers. Ideally, we should have used the
> `MESOS_AGENT_ENDPOINT` variable but that is a bit hard
> to do with our testcase setup that generates the
> agent process ID.
> 
> 
> Diffs
> -
> 
>   src/launcher/default_executor.cpp 2102fe8d70f0960fed669e1c4f0d6b6cd4af261c 
> 
> Diff: https://reviews.apache.org/r/52145/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Re: Review Request 52177: Added support for CNI networks for `--task_group`.

[Vinod Kone]

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




src/cli/execute.cpp (lines 468 - 478)


I don't follow this change. Can the TaskGroupInfo JSON not contain the 
NetworkInfos? Why do have to specify it via `--networks`?



src/cli/execute.cpp (line 788)


Infact `--networks` should not be specified if `--task_group` is specified. 
We need to add `flags.networks` here.


- Vinod Kone


On Sept. 22, 2016, 9:48 p.m., Avinash sridharan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52177/
> ---
> 
> (Updated Sept. 22, 2016, 9:48 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Vinod Kone.
> 
> 
> Bugs: MESOS-6096
> https://issues.apache.org/jira/browse/MESOS-6096
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added support for CNI networks for `--task_group`.
> 
> 
> Diffs
> -
> 
>   src/cli/execute.cpp 79091d6dd3e09e354f6965b0da45662621297bee 
> 
> Diff: https://reviews.apache.org/r/52177/diff/
> 
> 
> Testing
> ---
> 
> make
> 
> Ran `mesos-execute` with the --task_group and --network option with the 
> following POD definition:
> {
>   "tasks":[
> {
>   "name" : "nginx",
>   "task_id" : {"value":"nginx"},
>   "agent_id": {"value" : ""},
>   "resources": [{
> "name": "cpus",
> "type": "SCALAR",
> "scalar": {
>   "value": 0.1
> },
> "role": "*"
>   },
>   {
> "name": "mem",
> "type": "SCALAR",
> "scalar": {
>   "value": 128
> },
> "role": "*"
>   }],
>   "container" : {
> "type" : "MESOS",
> "mesos" : {
>   "image" : {
> "type" : "DOCKER",
> "docker" : {
>   "name" : "nginx"
> }
>   }
> }
>   }
> },
> {
>   "name" : "redis",
>   "task_id" : {"value":"redis"},
>   "agent_id": {"value" : ""},
>   "resources": [{
> "name": "cpus",
> "type": "SCALAR",
> "scalar": {
>   "value": 0.1
> },
> "role": "*"
>   },
>   {
> "name": "mem",
> "type": "SCALAR",
> "scalar": {
>   "value": 128
> },
> "role": "*"
>   }],
>   "container" : {
> "type" : "MESOS",
> "mesos" : {
>   "image" : {
> "type" : "DOCKER",
> "docker" : {
>   "name" : "redis"
> }
>   }
> }
>   }
> }
>   ]
> }
> 
> The pod definition doesn't do much (mesos can't launch docker containers in 
> PODS right now), but the pod itself is attached to a CNI network and it gets 
> an IP address and a network name space of its own.
> 
> 
> Thanks,
> 
> Avinash sridharan
> 
>

Re: Review Request 52198: Fixed warning in `statistics_tests`.

[Daniel Pravat]

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

(Updated Sept. 23, 2016, 5:09 a.m.)


Review request for mesos, Alex Clemmer and Joseph Wu.


Repository: mesos


Description
---

Fixed warning in `statistics_tests`.


Diffs
-

  3rdparty/libprocess/src/tests/statistics_tests.cpp 
54849a0a95be334a7203d7725185a402947755e5 

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


Testing (updated)
---

Windows: build
OSX: make check


Thanks,

Daniel Pravat

Review Request 52198: Fixed warning in `statistics_tests`.

[Daniel Pravat]

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

Review request for mesos, Alex Clemmer and Joseph Wu.


Repository: mesos


Description
---

Fixed warning in `statistics_tests`.


Diffs
-

  3rdparty/libprocess/src/tests/statistics_tests.cpp 
54849a0a95be334a7203d7725185a402947755e5 

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


Testing
---


Thanks,

Daniel Pravat

Re: Review Request 52164: Added `ContainerID` field to the executor `SUBSCRIBED` event.

[Vinod Kone]

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


Fix it, then Ship it!





src/slave/slave.cpp (line 3087)


weren't you going to remove the corresponding evolve method?


- Vinod Kone


On Sept. 23, 2016, 3:24 a.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52164/
> ---
> 
> (Updated Sept. 23, 2016, 3:24 a.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-6227
> https://issues.apache.org/jira/browse/MESOS-6227
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This change adds the field `ContainerID` to the executor `SUBSCRIBED`
> event. This allows the default executor/other custom executors to
> then launch nested containers. This is needed as the executors need
> to let the agent know of the parent container ID when they make the
> `LAUNCH_NESTED_CONTAINER` call. This change also implements the
> logic on the agent to send the event to the executor.
> 
> 
> Diffs
> -
> 
>   include/mesos/executor/executor.proto 
> 0d31406a70d6b7d9030c1f157a7869a2e8f1879f 
>   include/mesos/v1/executor/executor.proto 
> f661b71cecb96a9971c687f117625ef242749394 
>   src/slave/slave.cpp 11e9c8af87aa5153f72f2a20cc578fe3d729b153 
>   src/tests/executor_http_api_tests.cpp 
> fd322ee267b2f0d4bf250db15027d61f1fc74a57 
> 
> Diff: https://reviews.apache.org/r/52164/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Review Request 52199: Fixed warning in `stout_tests`.

[Daniel Pravat]

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

Review request for mesos, Alex Clemmer and Joseph Wu.


Repository: mesos


Description
---

Fixed warning in `stout_tests`.


Diffs
-

  3rdparty/stout/tests/os/filesystem_tests.cpp 
b747c168687af48427af167941a11a45aa0cd352 
  3rdparty/stout/tests/os/process_tests.cpp 
1e26877c7aee67d629789205639ad7c35df1d9c7 

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


Testing
---

Windows: build
OSX: make check


Thanks,

Daniel Pravat

Review Request 52195: Used functions from net:: namespace.

[Daniel Pravat]

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

Review request for mesos, Alex Clemmer and Joseph Wu.


Repository: mesos


Description
---

Used functions from net:: namespace.


Diffs
-

  3rdparty/libprocess/include/process/network.hpp 
0dd1b6d14e66a74172d1b077353dcc3391950b08 
  3rdparty/libprocess/src/poll_socket.cpp 
1cc5e8831dbcb096c135b4cb35d1df5efcf4f622 

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


Testing
---

OSX: make check
Windows: build


Thanks,

Daniel Pravat

Review Request 52196: Fixed warnings in `read/write` functions.

[Daniel Pravat]

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

Review request for mesos, Alex Clemmer and Joseph Wu.


Repository: mesos


Description
---

Fixed warnings in `read/write` functions.


Diffs
-

  3rdparty/stout/include/stout/os/windows/read.hpp 
cb0abf70307f0dbba0b8f68e884df199b0359186 
  3rdparty/stout/include/stout/os/windows/sendfile.hpp 
4c5178e1f3696906aa1cf93b8ca450f61ba79355 
  3rdparty/stout/include/stout/os/windows/write.hpp 
705ad03ee58f9cc822ec1ed25fd10f39059717d4 

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


Testing
---

OSX: make check
Windows: build


Thanks,

Daniel Pravat

Review Request 52194: Added `socket` functions to translate parameters.

[Daniel Pravat]

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

Review request for mesos, Alex Clemmer and Joseph Wu.


Repository: mesos


Description
---

Added `socket` functions to translate parameters.


Diffs
-

  3rdparty/stout/include/stout/os/posix/socket.hpp 
ab5f62a67c3adfe4d0c4c3532f9c2b4d43336c48 
  3rdparty/stout/include/stout/os/windows/socket.hpp 
d564a2bcc78570beb76a71cf35f3902333cd99ea 

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


Testing
---

Windows: build
OSX: make check


Thanks,

Daniel Pravat

Re: Review Request 52144: Populated the timestamp field when sending status updates.

[Vinod Kone]

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


Ship it!




Would be nice to update an existing test to check for the presence of timestamp 
in an executor generated update.

- Vinod Kone


On Sept. 22, 2016, 7:15 p.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52144/
> ---
> 
> (Updated Sept. 22, 2016, 7:15 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The command executor/default executor were not populating
> the timestamp for the status update. Previously, the executor
> driver used to do it underneath but that won't work with using
> the HTTP command executor.
> 
> 
> Diffs
> -
> 
>   src/launcher/default_executor.cpp 2102fe8d70f0960fed669e1c4f0d6b6cd4af261c 
>   src/launcher/executor.cpp 5370634ef9e6f3ac9717fed71f6a77707026a16a 
> 
> Diff: https://reviews.apache.org/r/52144/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Re: Review Request 52143: Added evolve handler for `agent::Call` to `v1::agent::Call`.

[Vinod Kone]

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


Ship it!




Ship It!

- Vinod Kone


On Sept. 22, 2016, 7:14 p.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52143/
> ---
> 
> (Updated Sept. 22, 2016, 7:14 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added evolve handler for `agent::Call` to `v1::agent::Call`.
> 
> 
> Diffs
> -
> 
>   src/internal/evolve.hpp 52a1302cf24f34ec676244b3f81cbf4a5e0a4805 
>   src/internal/evolve.cpp 9ae720cdc22c6bcdd56bc6bb391419c52df446f3 
> 
> Diff: https://reviews.apache.org/r/52143/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Review Request 52193: Fixed VC warnings in windows.hpp.

[Daniel Pravat]

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

Review request for mesos, Alex Clemmer and Joseph Wu.


Repository: mesos


Description
---

Fixed VC warnings in windows.hpp.


Diffs
-

  3rdparty/stout/include/stout/windows.hpp 
5cca853c4b090c4516ad4ec04c6e5830a4e7ccfa 

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


Testing
---

Windows: build
OSX: make check


Thanks,

Daniel Pravat

Review Request 52200: Fixed warnings in system.hpp.

[Daniel Pravat]

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

Review request for mesos, Alex Clemmer and Joseph Wu.


Repository: mesos


Description
---

Fixed warnings in system.hpp.


Diffs
-

  3rdparty/libprocess/include/process/system.hpp 
2a61b8e7fcd797a22ada6322932baefc485b7d56 

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


Testing
---

Windows: build
OSX: make build


Thanks,

Daniel Pravat

Re: Review Request 52175: Tweaked test comments.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [52174, 52175]

Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' 
CONFIGURATION='--verbose' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; 
./support/docker_build.sh

- Mesos ReviewBot


On Sept. 22, 2016, 9:12 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52175/
> ---
> 
> (Updated Sept. 22, 2016, 9:12 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Tweaked test comments.
> 
> 
> Diffs
> -
> 
>   src/tests/partition_tests.cpp 7c38f0efa414447e6292b2d6b334fb9879c92eb5 
> 
> Diff: https://reviews.apache.org/r/52175/diff/
> 
> 
> Testing
> ---
> 
> `make check` on OSX.
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Review Request 52197: Used intermediate functions.

[Daniel Pravat]

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

Review request for mesos, Alex Clemmer and Joseph Wu.


Repository: mesos


Description
---

Used `socket` intermediate functions.


Diffs
-

  3rdparty/libprocess/src/io.cpp d9c60754b1eaf83b1f5626a220f431cadeca541a 

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


Testing
---

Windows: build
OSX: make check


Thanks,

Daniel Pravat

Review Request 52201: Fixed warnings in timer.hpp.

[Daniel Pravat]

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

Review request for mesos, Alex Clemmer and Joseph Wu.


Repository: mesos


Description
---

Fixed warnings in timer.hpp.


Diffs
-

  3rdparty/libprocess/include/process/timer.hpp 
d60b9aec78210e30cce4c053939f6fecf36ba938 

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


Testing
---

Windows: build
OSX: make check


Thanks,

Daniel Pravat

reviews@mesos.apache.org

[Daniel Pravat]

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

Review request for mesos, Alex Clemmer and Joseph Wu.


Repository: mesos


Description
---

Fixed warnings in encoder&decoder.hpp.


Diffs
-

  3rdparty/libprocess/src/decoder.hpp ccf12ac8ae05141b4324ef2b4599a6bd7461c4ec 
  3rdparty/libprocess/src/encoder.hpp af083d196aa67d08f1b3299b0a15fbc7d1fd7cd5 

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


Testing
---

Windows: build
OSX: make check


Thanks,

Daniel Pravat

Review Request 52203: Fixed warnings in metrics.hpp.

[Daniel Pravat]

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

Review request for mesos, Alex Clemmer and Joseph Wu.


Repository: mesos


Description
---

Fixed warnings in metrics.hpp.


Diffs
-

  3rdparty/libprocess/src/metrics/metrics.cpp 
ac1544e70f5884f946ce3c31c5430c6a2c1f9dd1 

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


Testing
---

Windows: build
OSX: make check


Thanks,

Daniel Pravat

Review Request 52204: Fixed warnings in socket.hpp.

[Daniel Pravat]

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

Review request for mesos, Alex Clemmer and Joseph Wu.


Repository: mesos


Description
---

Fixed warnings in socket.hpp.


Diffs
-

  3rdparty/libprocess/src/socket.cpp 1e495181f3e43d4a2d6cdf8aa8c9605dfd52c81b 

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


Testing
---

OSX: make check
Windows: build


Thanks,

Daniel Pravat

Review Request 52205: Fixed warnings in flags.hpp.

[Daniel Pravat]

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

Review request for mesos, Alex Clemmer and Joseph Wu.


Repository: mesos


Description
---

Fixed warnings in flags.hpp.


Diffs
-

  3rdparty/stout/include/stout/flags/flags.hpp 
4ca6c69aeb1e3343e9b0ae2562c450c4c645890c 

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


Testing
---

Windows: build
OSX: make check


Thanks,

Daniel Pravat

Review Request 52206: Fixed warnings in os.hpp.

[Daniel Pravat]

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

Review request for mesos, Alex Clemmer and Joseph Wu.


Repository: mesos


Description
---

Fixed warnings in os.hpp.


Diffs
-

  3rdparty/stout/include/stout/windows/os.hpp 
fb341348d842513f7479a9f00880c8b63dd49fb6 

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


Testing
---

Windows: build
OSX: make check


Thanks,

Daniel Pravat

Review Request 52207: Fixed warnings in fs.hpp.

[Daniel Pravat]

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

Review request for mesos, Alex Clemmer and Joseph Wu.


Repository: mesos


Description
---

Fixed warnings in fs.hpp.


Diffs
-

  3rdparty/stout/include/stout/windows/fs.hpp 
99887b1047aedf1d814690cbf826469cb0e4fefa 

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


Testing
---

Windows: build
OSX: make check


Thanks,

Daniel Pravat

Review Request 52208: Fixed warnings in health_checker.cpp.

[Daniel Pravat]

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

Review request for mesos, Alex Clemmer and Joseph Wu.


Repository: mesos


Description
---

Fixed warnings in health_checker.cpp.


Diffs
-

  src/health-check/health_checker.cpp 758cbb382b08743f518022ba66eaafbea7615592 

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


Testing
---

Windows: buils
OSX: make check


Thanks,

Daniel Pravat

Re: Review Request 52209: Fixed warnings in slave.cpp.

[Daniel Pravat]

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

(Updated Sept. 23, 2016, 4:49 a.m.)


Review request for mesos, Alex Clemmer and Joseph Wu.


Repository: mesos


Description
---

Fixed warnings in slave.cpp.


Diffs
-

  src/slave/slave.hpp 3e7832c18acf876e8139fd0758204d8af1d91a04 

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


Testing (updated)
---

Windows: build
OSX: make check


Thanks,

Daniel Pravat

Review Request 52209: Fixed warnings in slave.cpp.

[Daniel Pravat]

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

Review request for mesos, Alex Clemmer and Joseph Wu.


Repository: mesos


Description
---

Fixed warnings in slave.cpp.


Diffs
-

  src/slave/slave.hpp 3e7832c18acf876e8139fd0758204d8af1d91a04 

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


Testing
---


Thanks,

Daniel Pravat

Review Request 52210: Fixed warnings in getcwd.hpp.

[Daniel Pravat]

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

Review request for mesos, Alex Clemmer and Joseph Wu.


Repository: mesos


Description
---

Fixed warnings in getcwd.hpp.


Diffs
-

  3rdparty/stout/include/stout/os/getcwd.hpp 
63ecc98326a4ba9d78eb0bcec18407c3f4a76de3 

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


Testing
---


Thanks,

Daniel Pravat

Review Request 52192: Fixed VC warnings in bytes.hpp.

[Daniel Pravat]

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

Review request for mesos, Alex Clemmer and Joseph Wu.


Repository: mesos


Description
---

Fixed VC warnings in bytes.hpp.


Diffs
-

  3rdparty/stout/include/stout/bytes.hpp 
9debe2f5579b9b2d67933ecc2bfcc40c2730f7f1 

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


Testing
---


Thanks,

Daniel Pravat

Re: Review Request 52062: Fixed warnings in `numify.hpp`.

[Daniel Pravat]

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

(Updated Sept. 23, 2016, 4:46 a.m.)


Review request for mesos, Alex Naparu, Artem Harutyunyan, Alex Clemmer, Joseph 
Wu, and Michael Park.


Repository: mesos


Description
---

When the template is instatiated for unsigned scalars
the unary negation generates warnings.


Diffs (updated)
-

  3rdparty/stout/include/stout/numify.hpp 
c174fcb8cb9d809f443e44058f07b58751bed9dd 

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


Testing
---

Windows: build


Thanks,

Daniel Pravat

Re: Review Request 52188: Added MESOS-6075 to 1.0.2 CHANGELOG.

[Kevin Klues]

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


Ship it!




Ship It!

- Kevin Klues


On Sept. 23, 2016, 4:19 a.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52188/
> ---
> 
> (Updated Sept. 23, 2016, 4:19 a.m.)
> 
> 
> Review request for mesos, Kevin Klues and Till Toenshoff.
> 
> 
> Bugs: MESOS-6075
> https://issues.apache.org/jira/browse/MESOS-6075
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added MESOS-6075 to 1.0.2 CHANGELOG.
> 
> 
> Diffs
> -
> 
>   CHANGELOG 4dc1446c80650267bd663e83c98f6870717b81b9 
> 
> Diff: https://reviews.apache.org/r/52188/diff/
> 
> 
> Testing
> ---
> 
> TRIVIAL
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 52187: Added MESOS-6235 to 1.0.2 CHANGELOG.

[Kevin Klues]

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


Fix it, then Ship it!





CHANGELOG (line 46)


I would mention that it's called `os::spawn()`


- Kevin Klues


On Sept. 23, 2016, 4:19 a.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52187/
> ---
> 
> (Updated Sept. 23, 2016, 4:19 a.m.)
> 
> 
> Review request for mesos, Kevin Klues and Till Toenshoff.
> 
> 
> Bugs: MESOS-6235
> https://issues.apache.org/jira/browse/MESOS-6235
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added MESOS-6235 to 1.0.2 CHANGELOG.
> 
> 
> Diffs
> -
> 
>   CHANGELOG 4dc1446c80650267bd663e83c98f6870717b81b9 
> 
> Diff: https://reviews.apache.org/r/52187/diff/
> 
> 
> Testing
> ---
> 
> TRIVIAL
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Review Request 52188: Added MESOS-6075 to 1.0.2 CHANGELOG.

[Jie Yu]

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

Review request for mesos, Kevin Klues and Till Toenshoff.


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


Repository: mesos


Description
---

Added MESOS-6075 to 1.0.2 CHANGELOG.


Diffs
-

  CHANGELOG 4dc1446c80650267bd663e83c98f6870717b81b9 

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


Testing
---

TRIVIAL


Thanks,

Jie Yu

Review Request 52187: Added MESOS-6235 to 1.0.2 CHANGELOG.

[Jie Yu]

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

Review request for mesos, Kevin Klues and Till Toenshoff.


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


Repository: mesos


Description
---

Added MESOS-6235 to 1.0.2 CHANGELOG.


Diffs
-

  CHANGELOG 4dc1446c80650267bd663e83c98f6870717b81b9 

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


Testing
---

TRIVIAL


Thanks,

Jie Yu

Re: Review Request 52148: Added support for killing child containers to the default executor.

[Anand Mazumdar]

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

(Updated Sept. 23, 2016, 3:28 a.m.)


Review request for mesos and Vinod Kone.


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


Repository: mesos


Description
---

This change implements support for killing child containers via
the `KILL_NESTED_CONTAINER` call on the Agent API. This is
triggered when the executor receives a `KILL`/`SHUTDOWN`
event. Currently, only SIGKILL is supported. Also, the specifying
custom kill policies is not yet supported.


Diffs (updated)
-

  src/launcher/default_executor.cpp 2102fe8d70f0960fed669e1c4f0d6b6cd4af261c 

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


Testing
---

make check


Thanks,

Anand Mazumdar

Re: Review Request 52149: Added support for waiting on child containers to the default executor.

[Anand Mazumdar]

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

(Updated Sept. 23, 2016, 3:28 a.m.)


Review request for mesos and Vinod Kone.


Changes
---

Review comments


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


Repository: mesos


Description
---

This change adds support for waiting on child containers via the
`WAIT_NESTED_CONTAINER` call on the Agent API. If the connection
fails due to a temporary network blip, it reconnects with the
agent.


Diffs (updated)
-

  src/launcher/default_executor.cpp 2102fe8d70f0960fed669e1c4f0d6b6cd4af261c 

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


Testing
---

make check


Thanks,

Anand Mazumdar

Re: Review Request 52164: Added `ContainerID` field to the executor `SUBSCRIBED` event.

[Anand Mazumdar]

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

(Updated Sept. 23, 2016, 3:24 a.m.)


Review request for mesos and Vinod Kone.


Changes
---

Review comments


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


Repository: mesos


Description
---

This change adds the field `ContainerID` to the executor `SUBSCRIBED`
event. This allows the default executor/other custom executors to
then launch nested containers. This is needed as the executors need
to let the agent know of the parent container ID when they make the
`LAUNCH_NESTED_CONTAINER` call. This change also implements the
logic on the agent to send the event to the executor.


Diffs (updated)
-

  include/mesos/executor/executor.proto 
0d31406a70d6b7d9030c1f157a7869a2e8f1879f 
  include/mesos/v1/executor/executor.proto 
f661b71cecb96a9971c687f117625ef242749394 
  src/slave/slave.cpp 11e9c8af87aa5153f72f2a20cc578fe3d729b153 
  src/tests/executor_http_api_tests.cpp 
fd322ee267b2f0d4bf250db15027d61f1fc74a57 

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


Testing
---

make check


Thanks,

Anand Mazumdar

Re: Review Request 52174: Fixed bug with unreachable tasks and disconnected frameworks.

[Vinod Kone]

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




src/master/master.cpp (line 5925)


No need for this hack. You can look in the `recovered` map for the 
FrameworkInfo of a recovered framework that has not resubscribed yet.


- Vinod Kone


On Sept. 22, 2016, 9:12 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52174/
> ---
> 
> (Updated Sept. 22, 2016, 9:12 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-6226
> https://issues.apache.org/jira/browse/MESOS-6226
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> We previously assumed that when marking a task unreachable, we would
> have access to the `FrameworkInfo` for that task's framework. However,
> that is not the case if the master has failed over and the framework has
> not yet reregistered with the new master.
> 
> In that situation, we don't have the framework's FrameworkInfo, and
> hence we cannot determine if the framework is partition-aware. For now,
> we assume the framework is partition-aware, which isn't great behavior
> (MESOS-6232).
> 
> 
> Diffs
> -
> 
>   src/master/master.cpp 66a672f6d16233e96b29e330a9e6c474546fa851 
>   src/tests/partition_tests.cpp 7c38f0efa414447e6292b2d6b334fb9879c92eb5 
> 
> Diff: https://reviews.apache.org/r/52174/diff/
> 
> 
> Testing
> ---
> 
> `make check` on OSX.
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 51124: Support more layers through symlink for overlay backend.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [51124]

Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' 
CONFIGURATION='--verbose' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; 
./support/docker_build.sh

- Mesos ReviewBot


On Sept. 22, 2016, 8:52 p.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51124/
> ---
> 
> (Updated Sept. 22, 2016, 8:52 p.m.)
> 
> 
> Review request for mesos, Xiaojian Huang, Gilbert Song, and Jie Yu.
> 
> 
> Bugs: MESOS-6000
> https://issues.apache.org/jira/browse/MESOS-6000
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Support more layers through symlink for overlay backend.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/provisioner/backend.hpp 
> 93819c887f2f801f06aae7020084b56cc81ce818 
>   src/slave/containerizer/mesos/provisioner/backends/aufs.hpp 
> a5ac13bc17b8f291472b622ebfdd0cd1dd072273 
>   src/slave/containerizer/mesos/provisioner/backends/aufs.cpp 
> 4c5cdb660dea205aea29d217ba80d845d1108fdf 
>   src/slave/containerizer/mesos/provisioner/backends/bind.hpp 
> 19d44c6a9a2cd5f8de79f33c76e9d79900ab003f 
>   src/slave/containerizer/mesos/provisioner/backends/bind.cpp 
> 62ccaec1a5cfb466e929f4db7bf7e5376f2a0c2d 
>   src/slave/containerizer/mesos/provisioner/backends/copy.hpp 
> 5bd6a5279d8dd6d088afcd58e4839bf6087ccd1e 
>   src/slave/containerizer/mesos/provisioner/backends/copy.cpp 
> e57bb3d2fb4e67e9caae416824a6a748db1460a1 
>   src/slave/containerizer/mesos/provisioner/backends/overlay.hpp 
> 387f28a331813c75a509b4a31dbbdc764080b8c1 
>   src/slave/containerizer/mesos/provisioner/backends/overlay.cpp 
> e20cd48463a78ecd8a652a4d5ac923dc02cca4d9 
>   src/slave/containerizer/mesos/provisioner/provisioner.cpp 
> f37c45ccfa572876dfbba6a0797c223896db5a7f 
>   src/tests/containerizer/provisioner_backend_tests.cpp 
> 7c9a7cd5c733f74e10316fc1234115e6820cc2cb 
> 
> Diff: https://reviews.apache.org/r/51124/diff/
> 
> 
> Testing
> ---
> 
> 1. Make sure `OverlayBackendTest.*` passes;
> 2. Use mesos-execute to provision a container using overlay backend. Observed 
> following log lines:
> 
> ```
> I0816 01:04:27.823420 46584 overlay.cpp:167] Created tempLink at 
> '/var/lib/mesos/provisioner/containers/fd574bbf-4bc0-4538-9ce5-8c2cc93b94c7/backends/overlay/links'
>  to '/tmp/NcmRZt'
> I0816 01:04:27.824834 46584 overlay.cpp:191] Provisioning image rootfs with 
> overlayfs: 
> 'lowerdir=/tmp/NcmRZt/42:/tmp/NcmRZt/41:/tmp/NcmRZt/40:/tmp/NcmRZt/39:/tmp/NcmRZt/38:/tmp/NcmRZt/37:/tmp/NcmRZt/36:/tmp/NcmRZt/35:/tmp/NcmRZt/34:/tmp/NcmRZt/33:/tmp/NcmRZt/32:/tmp/NcmRZt/31:/tmp/NcmRZt/30:/tmp/NcmRZt/29:/tmp/NcmRZt/28:/tmp/NcmRZt/27:/tmp/NcmRZt/26:/tmp/NcmRZt/25:/tmp/NcmRZt/24:/tmp/NcmRZt/23:/tmp/NcmRZt/22:/tmp/NcmRZt/21:/tmp/NcmRZt/20:/tmp/NcmRZt/19:/tmp/NcmRZt/18:/tmp/NcmRZt/17:/tmp/NcmRZt/16:/tmp/NcmRZt/15:/tmp/NcmRZt/14:/tmp/NcmRZt/13:/tmp/NcmRZt/12:/tmp/NcmRZt/11:/tmp/NcmRZt/10:/tmp/NcmRZt/9:/tmp/NcmRZt/8:/tmp/NcmRZt/7:/tmp/NcmRZt/6:/tmp/NcmRZt/5:/tmp/NcmRZt/4:/tmp/NcmRZt/3:/tmp/NcmRZt/2:/tmp/NcmRZt/1:/tmp/NcmRZt/0,upperdir=/var/lib/mesos/provisioner/containers/fd574bbf-4bc0-4538-9ce5-8c2cc93b94c7/backends/overlay/scratch/25d9b6e4-fd0e-401e-bb9e-754dbaec4f55/upperdir,workdir=/var/lib/mesos/provisioner/containers/fd574bbf-4bc0-4538-9ce5-8c2cc93b94c7/backends/overlay/scratch/2
 5d9b6e4-fd0e-401e-bb9e-754dbaec4f55/workdir'
> ...
> (after executor exited)
> I0816 01:04:34.859851 46584 overlay.cpp:281] Removed temporary directory 
> '/tmp/NcmRZt' pointed by 
> '/var/lib/mesos/provisioner/containers/fd574bbf-4bc0-4538-9ce5-8c2cc93b94c7/backends/overlay/links'
> ```
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>

Re: Review Request 52011: Updated launch helper to avoid initializing libprocess.

[Jie Yu]

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


Fix it, then Ship it!





src/slave/containerizer/mesos/launch.cpp (line 248)


space before 'value'


- Jie Yu


On Sept. 23, 2016, 2:07 a.m., Kevin Klues wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52011/
> ---
> 
> (Updated Sept. 23, 2016, 2:07 a.m.)
> 
> 
> Review request for mesos, Gilbert Song and Jie Yu.
> 
> 
> Bugs: MESOS-6075
> https://issues.apache.org/jira/browse/MESOS-6075
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Previously, we used 'process::subprocess()' to run all of our pre-exec
> commands. However, doing so causes us to (unnecesssarily) initialize
> all of libprocess (and subsequently creating a whole bunch of unused
> threads, etc.) just to run a simple script.
> 
> To avoid this, we now use `os::system()` and the new `os::spawn()`
> functions to give us our shell/non-shell variant of commands we want
> to launch.
> In the past, we used 'os::system()' alone to avoid initializing
> libprocess, but this caused security issues with allowing arbitrary
> shell commands to be appended to root-level pre-exec commands that
> take strings as their last argument (e.g. mount --bind  ,
> where target is user supplied and is set to "target_dir; rm -rf /").
> We now handle this case by using `os::spawn()` instead.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/launch.cpp 
> 48ec3707d772ec68e34acfc5adb47e25336ae8d3 
> 
> Diff: https://reviews.apache.org/r/52011/diff/
> 
> 
> Testing
> ---
> 
> $ GTEST_FILTER="" make -j check
> $ src/mesos-tests
> $ sudo src/mesos-tests
> 
> 
> Thanks,
> 
> Kevin Klues
> 
>

Re: Review Request 52184: Added 'os::spawn()' to stout as 'argv' counterpart to 'os::system()'.

[Jie Yu]

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


Ship it!




Ship It!

- Jie Yu


On Sept. 23, 2016, 2:03 a.m., Kevin Klues wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52184/
> ---
> 
> (Updated Sept. 23, 2016, 2:03 a.m.)
> 
> 
> Review request for mesos, Gilbert Song and Jie Yu.
> 
> 
> Bugs: MESOS-6235
> https://issues.apache.org/jira/browse/MESOS-6235
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added 'os::spawn()' to stout as 'argv' counterpart to 'os::system()'.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/os/posix/shell.hpp 
> df8fb4c94d44067038eacd8dc3bb4d566c8df836 
>   3rdparty/stout/include/stout/os/windows/shell.hpp 
> 8fb48c1c4178485cac2934833b5c440d873e4fc4 
> 
> Diff: https://reviews.apache.org/r/52184/diff/
> 
> 
> Testing
> ---
> 
> make check
> sudo make check
> 
> I also had someone make sure it at least compiled for windows.
> 
> 
> Thanks,
> 
> Kevin Klues
> 
>

Re: Review Request 52185: Do not need to erase task if it not in pendingTasks.

[Andy Pang]

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

(Updated 九月 23, 2016, 2:43 a.m.)


Review request for mesos, Jie Yu and Vinod Kone.


Repository: mesos


Description
---

If task has not been in hashmap pendingTasks, indicate it has been erase, so 
don't need to erase twice.


Diffs (updated)
-

  src/master/master.cpp 66a672f 

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


Testing
---

make check


Thanks,

Andy Pang

Re: Review Request 52186: Fixed build error for HTTPCommandExecutorTest.TerminateWithACK.

[Joseph Wu]

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


Ship it!




Ship It!

- Joseph Wu


On Sept. 22, 2016, 7:34 p.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52186/
> ---
> 
> (Updated Sept. 22, 2016, 7:34 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed build error for HTTPCommandExecutorTest.TerminateWithACK.
> 
> 
> Diffs
> -
> 
>   src/tests/command_executor_tests.cpp 
> 2bc466761d48b3c8024b754cc561e1e11fe7fe1f 
>   src/tests/mock_slave.hpp 39e836b7671012930ce46e16f40b248803cbb999 
> 
> Diff: https://reviews.apache.org/r/52186/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> ```
> ./bin/mesos-tests.sh --gtest_filter="HTTPCommandExecutorTest.TerminateWithACK"
> [==] Running 1 test from 1 test case.
> [--] Global test environment set-up.
> [--] 1 test from HTTPCommandExecutorTest
> [ RUN  ] HTTPCommandExecutorTest.TerminateWithACK
> [   OK ] HTTPCommandExecutorTest.TerminateWithACK (1566 ms)
> [--] 1 test from HTTPCommandExecutorTest (1567 ms total)
> 
> [--] Global test environment tear-down
> [==] 1 test from 1 test case ran. (1736 ms total)
> [  PASSED  ] 1 test.
> 
> ```
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 51027: WIP: Track allocation candidates to bound allocator.

[Guangya Liu]

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



It is really weired that the performance of 
`SlaveAndFrameworkCount/HierarchicalAllocator_BENCHMARK_Test.DeclineOffers/7` 
does not improve much when calling `addSlave`, need check more for why 
`addSlave` was same? Without fix, the `addSlave` will call `allocate` for each 
agent, but with the fix, only one `allocate` will be called

```
without fix:
[==] Running 1 test from 1 test case.
[--] Global test environment set-up.
[--] 1 test from 
SlaveAndFrameworkCount/HierarchicalAllocator_BENCHMARK_Test
[ RUN  ] 
SlaveAndFrameworkCount/HierarchicalAllocator_BENCHMARK_Test.DeclineOffers/7
Using 1000 agents and 6000 frameworks
Added 6000 frameworks in 122268us
Added 1000 agents in 42.037104secs

With fix:
[==] Running 1 test from 1 test case.
[--] Global test environment set-up.
[--] 1 test from 
SlaveAndFrameworkCount/HierarchicalAllocator_BENCHMARK_Test
[ RUN  ] 
SlaveAndFrameworkCount/HierarchicalAllocator_BENCHMARK_Test.DeclineOffers/7
Using 1000 agents and 6000 frameworks
Added 6000 frameworks in 116107us
Added 1000 agents in 41.615396secs
```

- Guangya Liu


On 九月 21, 2016, 5:42 p.m., Jacob Janco wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51027/
> ---
> 
> (Updated 九月 21, 2016, 5:42 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Guangya Liu, James Peach, Klaus 
> Ma, and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-3157
> https://issues.apache.org/jira/browse/MESOS-3157
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> - Triggered allocations dispatch allocate() only
>   if there is no pending allocation in the queue.
> - Allocation candidates are accumulated and only
>   cleared when enqueued allocations are processed.
> - Batched allocations are handled synchronously.
> 
> 
> Diffs
> -
> 
>   src/master/allocator/mesos/hierarchical.hpp 
> 2c31471ee0f5d6836393bf87ff9ecfd8df835013 
>   src/master/allocator/mesos/hierarchical.cpp 
> 2d56bd011f2c87c67a02d0ae467a4a537d36867e 
> 
> Diff: https://reviews.apache.org/r/51027/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> note: check without filters depends on https://reviews.apache.org/r/51028
> 
> With new benchmark https://reviews.apache.org/r/49617: 
> Sample output without 51027:
> [ RUN  ] 
> SlaveAndFrameworkCount/HierarchicalAllocator_BENCHMARK_Test.FrameworkFailover/22
> Using 1 agents and 3000 frameworks
> Added 3000 frameworks in 57251us
> Added 1 agents in 3.2134535333mins
> allocator settled after  1.6123603833mins
> [   OK ] 
> SlaveAndFrameworkCount/HierarchicalAllocator_BENCHMARK_Test.FrameworkFailover/22
>  (290578 ms)
> 
> Sample output with 51027:
> [ RUN  ] 
> SlaveAndFrameworkCount/HierarchicalAllocator_BENCHMARK_Test.FrameworkFailover/22
> Using 1 agents and 3000 frameworks
> Added 3000 frameworks in 39817us
> Added 1 agents in 3.2286054167mins
> allocator settled after  25.525654secs
> [   OK ] 
> SlaveAndFrameworkCount/HierarchicalAllocator_BENCHMARK_Test.FrameworkFailover/22
>  (220137 ms)
> 
> 
> Thanks,
> 
> Jacob Janco
> 
>

Review Request 52186: Fixed build error for HTTPCommandExecutorTest.TerminateWithACK.

[Guangya Liu]

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

Review request for mesos and Vinod Kone.


Repository: mesos


Description
---

Fixed build error for HTTPCommandExecutorTest.TerminateWithACK.


Diffs
-

  src/tests/command_executor_tests.cpp 2bc466761d48b3c8024b754cc561e1e11fe7fe1f 
  src/tests/mock_slave.hpp 39e836b7671012930ce46e16f40b248803cbb999 

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


Testing
---

make check

```
./bin/mesos-tests.sh --gtest_filter="HTTPCommandExecutorTest.TerminateWithACK"
[==] Running 1 test from 1 test case.
[--] Global test environment set-up.
[--] 1 test from HTTPCommandExecutorTest
[ RUN  ] HTTPCommandExecutorTest.TerminateWithACK
[   OK ] HTTPCommandExecutorTest.TerminateWithACK (1566 ms)
[--] 1 test from HTTPCommandExecutorTest (1567 ms total)

[--] Global test environment tear-down
[==] 1 test from 1 test case ran. (1736 ms total)
[  PASSED  ] 1 test.

```


Thanks,

Guangya Liu

Review Request 52185: Do not need to erase task if it not in pendingTasks.

[Andy Pang]

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

Review request for mesos, Jie Yu and Vinod Kone.


Repository: mesos


Description
---

If task has not been in hashmap pendingTasks, indicate it has been erase, so 
don't need to erase twice.


Diffs
-

  src/master/master.cpp 66a672f 

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


Testing
---

make check


Thanks,

Andy Pang

Re: Review Request 51277: Added an 'ns::enter' helper.

[haosdent huang]

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




src/linux/ns.hpp (lines 79 - 130)


It would be better that we move this to ``, so that we could 
reuse these in ``.



src/linux/ns.hpp (lines 230 - 282)


Move this to `ns::enter` in ``, than subprocess could reuse 
this part as well.


- haosdent huang


On Aug. 22, 2016, 4:54 a.m., Benjamin Hindman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51277/
> ---
> 
> (Updated Aug. 22, 2016, 4:54 a.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added an 'ns::enter' helper.
> 
> 
> Diffs
> -
> 
>   src/linux/ns.hpp 2d6c359ed24d6e964882f34df60d8182491a27c9 
> 
> Diff: https://reviews.apache.org/r/51277/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Benjamin Hindman
> 
>

Re: Review Request 52011: Updated launch helper to avoid initializing libprocess.

[Kevin Klues]

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

(Updated Sept. 23, 2016, 2:07 a.m.)


Review request for mesos, Gilbert Song and Jie Yu.


Changes
---

Updated to use the new `os::spawn()` functionality in stout.


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


Repository: mesos


Description (updated)
---

Previously, we used 'process::subprocess()' to run all of our pre-exec
commands. However, doing so causes us to (unnecesssarily) initialize
all of libprocess (and subsequently creating a whole bunch of unused
threads, etc.) just to run a simple script.

To avoid this, we now use `os::system()` and the new `os::spawn()`
functions to give us our shell/non-shell variant of commands we want
to launch.
In the past, we used 'os::system()' alone to avoid initializing
libprocess, but this caused security issues with allowing arbitrary
shell commands to be appended to root-level pre-exec commands that
take strings as their last argument (e.g. mount --bind  ,
where target is user supplied and is set to "target_dir; rm -rf /").
We now handle this case by using `os::spawn()` instead.


Diffs (updated)
-

  src/slave/containerizer/mesos/launch.cpp 
48ec3707d772ec68e34acfc5adb47e25336ae8d3 

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


Testing
---

$ GTEST_FILTER="" make -j check
$ src/mesos-tests
$ sudo src/mesos-tests


Thanks,

Kevin Klues

Review Request 52184: Added 'os::spawn()' to stout as 'argv' counterpart to 'os::system()'.

[Kevin Klues]

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

Review request for mesos, Gilbert Song and Jie Yu.


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


Repository: mesos


Description
---

Added 'os::spawn()' to stout as 'argv' counterpart to 'os::system()'.


Diffs
-

  3rdparty/stout/include/stout/os/posix/shell.hpp 
df8fb4c94d44067038eacd8dc3bb4d566c8df836 
  3rdparty/stout/include/stout/os/windows/shell.hpp 
8fb48c1c4178485cac2934833b5c440d873e4fc4 

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


Testing
---

make check
sudo make check

I also had someone make sure it at least compiled for windows.


Thanks,

Kevin Klues

Re: Review Request 52075: Added the test `HTTPCommandExecutorTest.TerminateWithACK`.

[Kevin Klues]

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



This test is causing segfaults for me on master.

- Kevin Klues


On Sept. 23, 2016, 12:17 a.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52075/
> ---
> 
> (Updated Sept. 23, 2016, 12:17 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar and Vinod Kone.
> 
> 
> Bugs: MESOS-5276
> https://issues.apache.org/jira/browse/MESOS-5276
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added the test `HTTPCommandExecutorTest.TerminateWithACK`.
> 
> 
> Diffs
> -
> 
>   src/slave/slave.hpp 265032e0dece9ba99cf53c9fbafe4a5cbb25d9f7 
>   src/tests/command_executor_tests.cpp 
> 07e5eb4d7c2ace2b6714fbe02f29d41663152556 
>   src/tests/mock_slave.hpp 53e1c9ee1ae5150fc2e6deebb574c9c86e08df7f 
> 
> Diff: https://reviews.apache.org/r/52075/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Re: Review Request 46187: Terminate when receiving the ACK of terminal status update.

[Qian Zhang]

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

(Updated Sept. 23, 2016, 8:17 a.m.)


Review request for mesos, Anand Mazumdar and Vinod Kone.


Changes
---

Rebased


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


Repository: mesos


Description
---

Terminate when receiving the ACK of terminal status update.


Diffs (updated)
-

  src/launcher/executor.cpp 5370634ef9e6f3ac9717fed71f6a77707026a16a 

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


Testing
---

make check


Thanks,

Qian Zhang

Re: Review Request 52075: Added the test `HTTPCommandExecutorTest.TerminateWithACK`.

[Qian Zhang]

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

(Updated Sept. 23, 2016, 8:17 a.m.)


Review request for mesos, Anand Mazumdar and Vinod Kone.


Changes
---

Rebased and addressed Vinod's comments.


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


Repository: mesos


Description
---

Added the test `HTTPCommandExecutorTest.TerminateWithACK`.


Diffs (updated)
-

  src/slave/slave.hpp 265032e0dece9ba99cf53c9fbafe4a5cbb25d9f7 
  src/tests/command_executor_tests.cpp 07e5eb4d7c2ace2b6714fbe02f29d41663152556 
  src/tests/mock_slave.hpp 53e1c9ee1ae5150fc2e6deebb574c9c86e08df7f 

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


Testing
---


Thanks,

Qian Zhang

Re: Review Request 52105: Prevent `/redirect/foo` loop.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [52105]

Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' 
CONFIGURATION='--verbose' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; 
./support/docker_build.sh

- Mesos ReviewBot


On Sept. 22, 2016, 8:47 p.m., Charles Allen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52105/
> ---
> 
> (Updated Sept. 22, 2016, 8:47 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-6210
> https://issues.apache.org/jira/browse/MESOS-6210
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Prevent `/redirect/foo` loop.
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp 9005e7c308d5f57c6f5c573951d468a3ba730740 
> 
> Diff: https://reviews.apache.org/r/52105/diff/
> 
> 
> Testing
> ---
> 
> Single Node started with `./src/mesos-master --work_dir=/tmp/mesos`
> 
> 
> ```
> Charless-MacBook-Pro:~ charlesallen$ curl -v http://127.0.0.1:5050/redirect
> *   Trying 127.0.0.1...
> * Connected to 127.0.0.1 (127.0.0.1) port 5050 (#0)
> > GET /redirect HTTP/1.1
> > Host: 127.0.0.1:5050
> > User-Agent: curl/7.43.0
> > Accept: */*
> >
> < HTTP/1.1 307 Temporary Redirect
> < Date: Thu, 22 Sep 2016 20:35:18 GMT
> < Location: //10.17.97.185:5050
> < Content-Length: 0
> <
> * Connection #0 to host 127.0.0.1 left intact
> Charless-MacBook-Pro:~ charlesallen$ curl -v 
> http://127.0.0.1:5050/master/redirect
> *   Trying 127.0.0.1...
> * Connected to 127.0.0.1 (127.0.0.1) port 5050 (#0)
> > GET /master/redirect HTTP/1.1
> > Host: 127.0.0.1:5050
> > User-Agent: curl/7.43.0
> > Accept: */*
> >
> < HTTP/1.1 307 Temporary Redirect
> < Date: Thu, 22 Sep 2016 20:35:23 GMT
> < Location: //10.17.97.185:5050
> < Content-Length: 0
> <
> * Connection #0 to host 127.0.0.1 left intact
> Charless-MacBook-Pro:~ charlesallen$ curl -v 
> http://127.0.0.1:5050/master/redirect/
> *   Trying 127.0.0.1...
> * Connected to 127.0.0.1 (127.0.0.1) port 5050 (#0)
> > GET /master/redirect/ HTTP/1.1
> > Host: 127.0.0.1:5050
> > User-Agent: curl/7.43.0
> > Accept: */*
> >
> < HTTP/1.1 404 Not Found
> < Date: Thu, 22 Sep 2016 20:35:29 GMT
> < Content-Length: 0
> <
> * Connection #0 to host 127.0.0.1 left intact
> Charless-MacBook-Pro:~ charlesallen$ curl -v 
> http://127.0.0.1:5050/master/redirect/foo
> *   Trying 127.0.0.1...
> * Connected to 127.0.0.1 (127.0.0.1) port 5050 (#0)
> > GET /master/redirect/foo HTTP/1.1
> > Host: 127.0.0.1:5050
> > User-Agent: curl/7.43.0
> > Accept: */*
> >
> < HTTP/1.1 307 Temporary Redirect
> < Date: Thu, 22 Sep 2016 20:35:31 GMT
> < Location: //10.17.97.185:5050
> < Content-Length: 0
> <
> * Connection #0 to host 127.0.0.1 left intact
> Charless-MacBook-Pro:~ charlesallen$ curl -v 
> http://127.0.0.1:5050/master/redirect/foo/bar
> *   Trying 127.0.0.1...
> * Connected to 127.0.0.1 (127.0.0.1) port 5050 (#0)
> > GET /master/redirect/foo/bar HTTP/1.1
> > Host: 127.0.0.1:5050
> > User-Agent: curl/7.43.0
> > Accept: */*
> >
> < HTTP/1.1 307 Temporary Redirect
> < Date: Thu, 22 Sep 2016 20:35:34 GMT
> < Location: //10.17.97.185:5050
> < Content-Length: 0
> <
> * Connection #0 to host 127.0.0.1 left intact
> Charless-MacBook-Pro:~ charlesallen$ curl -v 
> http://127.0.0.1:5050/redirect/foo/bar
> *   Trying 127.0.0.1...
> * Connected to 127.0.0.1 (127.0.0.1) port 5050 (#0)
> > GET /redirect/foo/bar HTTP/1.1
> > Host: 127.0.0.1:5050
> > User-Agent: curl/7.43.0
> > Accept: */*
> >
> < HTTP/1.1 307 Temporary Redirect
> < Date: Thu, 22 Sep 2016 20:35:41 GMT
> < Location: //10.17.97.185:5050
> < Content-Length: 0
> <
> * Connection #0 to host 127.0.0.1 left intact
> Charless-MacBook-Pro:~ charlesallen$ curl -v 
> http://127.0.0.1:5050/redirectNOTFOUND/foo/bar
> *   Trying 127.0.0.1...
> * Connected to 127.0.0.1 (127.0.0.1) port 5050 (#0)
> > GET /redirectNOTFOUND/foo/bar HTTP/1.1
> > Host: 127.0.0.1:5050
> > User-Agent: curl/7.43.0
> > Accept: */*
> >
> < HTTP/1.1 404 Not Found
> < Date: Thu, 22 Sep 2016 20:35:47 GMT
> < Content-Length: 0
> <
> * Connection #0 to host 127.0.0.1 left intact
> ```
> 
> It is worth noting that in this PR /master/redirect/  and /redirect/ return 
> 404, not 307. This behavior is consistent with master.
> 
> 
> Thanks,
> 
> Charles Allen
> 
>

Re: Review Request 52118: Added support for opaque whiteout file `.wh..wh..opq`.

[Qian Zhang]

> On Sept. 23, 2016, 1:39 a.m., Jie Yu wrote:
> > I'll commit this first. Please DO follow up with tests on this.

Sure, I will follow up with a test.


- Qian


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


On Sept. 22, 2016, 10:29 a.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52118/
> ---
> 
> (Updated Sept. 22, 2016, 10:29 a.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-6215
> https://issues.apache.org/jira/browse/MESOS-6215
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added support for opaque whiteout file `.wh..wh..opq`.
> 
> 
> Diffs
> -
> 
>   include/mesos/docker/spec.hpp 92367cf942c7ee4c23b1265b659f01d427212f62 
>   src/slave/containerizer/mesos/provisioner/provisioner.cpp 
> f37c45ccfa572876dfbba6a0797c223896db5a7f 
> 
> Diff: https://reviews.apache.org/r/52118/diff/
> 
> 
> Testing
> ---
> 
> make check on Ubuntu 14.04.
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Re: Review Request 52154: Avoided modifying process environment.

[Joris Van Remoortere]

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



Why this solution as opposed to ensuring we initialize at the beginning of 
processes like we do with `process::initialize()`?

- Joris Van Remoortere


On Sept. 22, 2016, 12:06 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52154/
> ---
> 
> (Updated Sept. 22, 2016, 12:06 p.m.)
> 
> 
> Review request for mesos, Joris Van Remoortere and Till Toenshoff.
> 
> 
> Bugs: MESOS-6216
> https://issues.apache.org/jira/browse/MESOS-6216
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This code modified the process environment in order to support upgrading
> on the fly from old-style libprocess SSL variables `SSL_` to
> `LIBPROCESS_SSL_`. Modifying the process environment at this point is
> not safe as other actors might concurrently read out that same
> environment.
> 
> Instead avoid changing the process environment altogether since flags
> can just as well be read from a map.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/openssl.cpp 
> c09cdc89509e4e4ca4c8a0f4fb0a57156a3a6091 
>   src/master/allocator/mesos/hierarchical.cpp 
> 3f51f4194c1ba7c1e4f08c3dd623281ca5754d39 
>   src/master/master.hpp 48011eabda03986df3dfac124506645a398eaff4 
> 
> Diff: https://reviews.apache.org/r/52154/diff/
> 
> 
> Testing
> ---
> 
> Tested on various platforms in internal CI.
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 51880: Added unit tests to determine disk size for MOUNT disks.

[Guangya Liu]

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




src/tests/containerizer/common_containerizer_tests.cpp (lines 159 - 161)


Using `EXPECT_SOME_EQ` may be more accurate. 

I saw that you only updated #149 - #151, but here and elsewhere should also 
be updated.


- Guangya Liu


On 九月 21, 2016, 4:15 a.m., Anindya Sinha wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51880/
> ---
> 
> (Updated 九月 21, 2016, 4:15 a.m.)
> 
> 
> Review request for mesos and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-6062
> https://issues.apache.org/jira/browse/MESOS-6062
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added unit tests to determine disk size for MOUNT disks.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 478fb5ae01dbdf1a735680edf7c5f30867726e07 
>   src/tests/containerizer/common_containerizer_tests.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/51880/diff/
> 
> 
> Testing
> ---
> 
> All tests including the additional tests in this RR passed.
> 
> 
> Thanks,
> 
> Anindya Sinha
> 
>

Re: Review Request 52153: Fixed openssl CA location logging.

[Benjamin Bannier]

> On Sept. 22, 2016, 9:58 p.m., Joseph Wu wrote:
> > 3rdparty/libprocess/src/openssl.cpp, line 493
> > 
> >
> > I recall that we prefer `.at(...)` even when there's an existence check 
> > directly preceeding the map access.

I was briefly wondering why that would be prefered (terminate if key does not 
exist?), but `map::at` being `const` sold me.


- Benjamin


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


On Sept. 22, 2016, 11:43 a.m., Till Toenshoff wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52153/
> ---
> 
> (Updated Sept. 22, 2016, 11:43 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Joris Van Remoortere, and Joseph 
> Wu.
> 
> 
> Bugs: MESOS-5320
> https://issues.apache.org/jira/browse/MESOS-5320
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> see summary.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/openssl.cpp 
> c09cdc89509e4e4ca4c8a0f4fb0a57156a3a6091 
> 
> Diff: https://reviews.apache.org/r/52153/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Till Toenshoff
> 
>

Re: Review Request 52153: Fixed openssl CA location logging.

[Benjamin Bannier]

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


Ship it!




modulo `map::operator[]` -> `map::at`.

- Benjamin Bannier


On Sept. 22, 2016, 11:43 a.m., Till Toenshoff wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52153/
> ---
> 
> (Updated Sept. 22, 2016, 11:43 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Joris Van Remoortere, and Joseph 
> Wu.
> 
> 
> Bugs: MESOS-5320
> https://issues.apache.org/jira/browse/MESOS-5320
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> see summary.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/openssl.cpp 
> c09cdc89509e4e4ca4c8a0f4fb0a57156a3a6091 
> 
> Diff: https://reviews.apache.org/r/52153/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Till Toenshoff
> 
>

Re: Review Request 51623: Updated mesos-execute to support task groups.

[Guangya Liu]

> On 九月 22, 2016, 10:02 p.m., Zhitao Li wrote:
> > src/cli/execute.cpp, line 270
> > 
> >
> > Does this work when `--task_group` is not specified? It seems to cause 
> > segfault.

Can you show more detail for your command which caused the segfault? I think 
that we alraedy have some check here 
https://github.com/apache/mesos/blob/master/src/cli/execute.cpp#L767-L792


- Guangya


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


On 九月 19, 2016, 7:45 p.m., Abhishek Dasgupta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51623/
> ---
> 
> (Updated 九月 19, 2016, 7:45 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar and Vinod Kone.
> 
> 
> Bugs: MESOS-6096
> https://issues.apache.org/jira/browse/MESOS-6096
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Through this patch, mesos-execute supports running
> task groups. Single task provided through command
> line option is supported as well for backward
> compatibility.
> 
> 
> Diffs
> -
> 
>   src/cli/execute.cpp 525c89803ad1b29328420c5925a3e6045e487645 
> 
> Diff: https://reviews.apache.org/r/51623/diff/
> 
> 
> Testing
> ---
> 
> On Ubuntu 16.04:
> sudo make -j4
> 
> and manually ran this command:
> **Successful**
> mesos-execute --master=127.0.0.1:5050 
> --taskgroup=file:///home/abhishek/taskgroup.txt
> 
> `/home/abhishek/taskgroup.txt:`
> {
>   "tasks": [{
>   "name": "sigmtdroid",
>   "task_id": {
>   "value": "sigmoid"
>   },
>   "agent_id": {
>   "value": ""
>   },
>   "resources": [{
>   "name": "cpus",
>   "type": "SCALAR",
>   "scalar": {
>   "value": 1
>   },
>   "role": "*"
>   }, {
>   "name": "mem",
>   "type": "SCALAR",
>   "scalar": {
>   "value": 32
>   },
>   "role": "*"
>   }],
>   "command": {
>   "value": "sleep 1000"
>   },
>   "volumes": [{
>   "container_path": "/mnt/volume",
>   "mode": "RW",
>   "source": {
>   "docker_volume": {
>   "driver": "volume_driver",
>   "docker_options": {
>   "parameter": [{
>   "key": "key",
>   "value": "value"
>   }]
>   },
>   "name": "volume_name"
>   },
>   "type": "DOCKER_VOLUME"
>   }
>   }]
>   }, {
>   "name": "sigmteroid",
>   "task_id": {
>   "value": "sigmoid2"
>   },
>   "agent_id": {
>   "value": ""
>   },
>   "resources": [{
>   "name": "cpus",
>   "type": "SCALAR",
>   "scalar": {
>   "value": 2
>   },
>   "role": "*"
>   }, {
>   "name": "mem",
>   "type": "SCALAR",
>   "scalar": {
>   "value": 32
>   },
>   "role": "*"
>   }],
>   "command": {
>   "value": "sleep 1000"
>   },
>   "volumes": [{
>   "container_path": "/mnt/volume",
>   "mode": "RW",
>   "source": {
>   "docker_volume": {
>   "driver": "volume_driver",
>   "docker_options": {
>   "parameter": [{
>   "key": "key",
>   "value": "value"
>   }]
>   },
>   "name": 

Re: Review Request 52033: Escalated some openssl logs from VLOG to INFO.

[Benjamin Bannier]

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


Ship it!




- Benjamin Bannier


On Sept. 22, 2016, 11:44 a.m., Till Toenshoff wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52033/
> ---
> 
> (Updated Sept. 22, 2016, 11:44 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Joris Van Remoortere, and Joseph 
> Wu.
> 
> 
> Bugs: MESOS-5320
> https://issues.apache.org/jira/browse/MESOS-5320
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The logging messages in question prove to be very useful for debugging
> the cluster setup and hence we decided they should be generally
> available as their helpfulnes by far outwages their noise given that
> we call these only during initializing.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/openssl.cpp 
> c09cdc89509e4e4ca4c8a0f4fb0a57156a3a6091 
> 
> Diff: https://reviews.apache.org/r/52033/diff/
> 
> 
> Testing
> ---
> 
> make check && functional testing
> 
> 
> Thanks,
> 
> Till Toenshoff
> 
>

Re: Review Request 51999: Refactor parsing of resources.

[Guangya Liu]

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




include/mesos/resources.hpp (lines 179 - 213)


I think we do not intend to expose those two APIs, so `private` them?



include/mesos/resources.hpp (line 196)


This was only called by `Try parse` and the `Try 
parse` already defined the default value as `defaultRole` as star role, so we 
do not need to set the parameter `defaultRole` default to star role. But adding 
a default value may make the code more easy to understand.



include/mesos/resources.hpp (line 213)


ditto



src/common/resources.cpp (line 580)


Why not 

```
Resources resources;
```



src/common/resources.cpp (line 670)


I think that we should still use `internal::validateCommandLineResources` 
as the command line resources should not have dynamic resevations etc.

But there is a bug in `internal::validateCommandLineResources` now, as when 
I set a resource string as `cpus:45.55;ports:[1-2, 
3-5];disks:{-xx}`, there will be no erros if using 
`internal::validateCommandLineResources` to validate.

So I would suggest the following:
1) Continue use `internal::validateCommandLineResources` to validate 
resource.
2) Add `role` validation in `internal::validateCommandLineResources`.


- Guangya Liu


On 九月 21, 2016, 4:14 a.m., Anindya Sinha wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51999/
> ---
> 
> (Updated 九月 21, 2016, 4:14 a.m.)
> 
> 
> Review request for mesos and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-6062
> https://issues.apache.org/jira/browse/MESOS-6062
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Refactored `Resources::parse()` into 2 separate static functions:
> 1. Resources::fromJSONString() to parse JSON representation of
>resources.
> 2. Resources::fromSimpleString() to parse text representation of
>resources.
> 
> Since these 2 new functions return a `Try>`, the
> existing `Resources::parse()` implicitly converts that to a
> `Resources` object. This refactor is done to retrieve all resources
> (include empty resources) required for auto detection of root
> and MOUNT disks.
> 
> 
> Diffs
> -
> 
>   include/mesos/resources.hpp 3ef8cacee529addc745b4aeb6398d7606c61b749 
>   include/mesos/v1/resources.hpp ef56b4960b103a3efd916fab64796aa334ba44c6 
>   src/common/resources.cpp 4602bff22ec7ff77f069056085ad92bfa04595f3 
>   src/tests/resources_tests.cpp 3e493007d6d1d8194d07035aaa1cde28dedf2b5a 
>   src/v1/resources.cpp 7c4db44d4c31a8295cc16b0ce1ef5adf314e6ac6 
> 
> Diff: https://reviews.apache.org/r/51999/diff/
> 
> 
> Testing
> ---
> 
> All tests passed.
> 
> 
> Thanks,
> 
> Anindya Sinha
> 
>

Re: Review Request 51623: Updated mesos-execute to support task groups.

[Zhitao Li]

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




src/cli/execute.cpp (line 270)


Does this work when `--task_group` is not specified? It seems to cause 
segfault.


- Zhitao Li


On Sept. 19, 2016, 7:45 p.m., Abhishek Dasgupta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51623/
> ---
> 
> (Updated Sept. 19, 2016, 7:45 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar and Vinod Kone.
> 
> 
> Bugs: MESOS-6096
> https://issues.apache.org/jira/browse/MESOS-6096
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Through this patch, mesos-execute supports running
> task groups. Single task provided through command
> line option is supported as well for backward
> compatibility.
> 
> 
> Diffs
> -
> 
>   src/cli/execute.cpp 525c89803ad1b29328420c5925a3e6045e487645 
> 
> Diff: https://reviews.apache.org/r/51623/diff/
> 
> 
> Testing
> ---
> 
> On Ubuntu 16.04:
> sudo make -j4
> 
> and manually ran this command:
> **Successful**
> mesos-execute --master=127.0.0.1:5050 
> --taskgroup=file:///home/abhishek/taskgroup.txt
> 
> `/home/abhishek/taskgroup.txt:`
> {
>   "tasks": [{
>   "name": "sigmtdroid",
>   "task_id": {
>   "value": "sigmoid"
>   },
>   "agent_id": {
>   "value": ""
>   },
>   "resources": [{
>   "name": "cpus",
>   "type": "SCALAR",
>   "scalar": {
>   "value": 1
>   },
>   "role": "*"
>   }, {
>   "name": "mem",
>   "type": "SCALAR",
>   "scalar": {
>   "value": 32
>   },
>   "role": "*"
>   }],
>   "command": {
>   "value": "sleep 1000"
>   },
>   "volumes": [{
>   "container_path": "/mnt/volume",
>   "mode": "RW",
>   "source": {
>   "docker_volume": {
>   "driver": "volume_driver",
>   "docker_options": {
>   "parameter": [{
>   "key": "key",
>   "value": "value"
>   }]
>   },
>   "name": "volume_name"
>   },
>   "type": "DOCKER_VOLUME"
>   }
>   }]
>   }, {
>   "name": "sigmteroid",
>   "task_id": {
>   "value": "sigmoid2"
>   },
>   "agent_id": {
>   "value": ""
>   },
>   "resources": [{
>   "name": "cpus",
>   "type": "SCALAR",
>   "scalar": {
>   "value": 2
>   },
>   "role": "*"
>   }, {
>   "name": "mem",
>   "type": "SCALAR",
>   "scalar": {
>   "value": 32
>   },
>   "role": "*"
>   }],
>   "command": {
>   "value": "sleep 1000"
>   },
>   "volumes": [{
>   "container_path": "/mnt/volume",
>   "mode": "RW",
>   "source": {
>   "docker_volume": {
>   "driver": "volume_driver",
>   "docker_options": {
>   "parameter": [{
>   "key": "key",
>   "value": "value"
>   }]
>   },
>   "name": "volume_name"
>   },
>   "type": "DOCKER_VOLUME"
>   }
>   }]
>   }]
> }
> 
> 
> Thanks,
> 
> Abhishek Dasgupta
> 
>

Re: Review Request 50271: Created an isolator for Linux capabilities.

[Benjamin Bannier]

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

(Updated Sept. 22, 2016, 11:48 p.m.)


Review request for mesos, Jay Guo and Jie Yu.


Changes
---

Addressed Jie's comments,

* only build capabilities isolator under Linux,
* do not yet reject tasks with empty requested capability set when no agent 
capability isolation active.


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


Repository: mesos


Description
---

This isolator evaluates agent allowed capabilities and passes net
capabilities on to `mesos-containerizer` which enforces the
capabilities.

Capability information is passed via a new field in
`ContainerLaunchInfo`.


Diffs (updated)
-

  include/mesos/slave/containerizer.proto 
20db010ea158a813034b41ce9cddac7d8317 
  src/CMakeLists.txt 42c52b60cc850901f2eff1545cf7900f4a65ca81 
  src/Makefile.am bfdb66a6969a35660d545210c1c6951926117ef3 
  src/slave/containerizer/mesos/containerizer.cpp 
144b0db501d40d4e0bba12672723616bedd76e7e 
  src/slave/containerizer/mesos/isolators/linux/capabilities.hpp PRE-CREATION 
  src/slave/containerizer/mesos/isolators/linux/capabilities.cpp PRE-CREATION 
  src/slave/containerizer/mesos/launch.hpp 
859990cb85e9e8c06400397256cfc512f0811800 
  src/slave/containerizer/mesos/launch.cpp 
48ec3707d772ec68e34acfc5adb47e25336ae8d3 
  src/slave/flags.hpp 3952d04f6a00ac1dca1adf2bea7cc6e415620ce5 
  src/tests/containerizer/isolator_tests.cpp 
b4d25e57df7f0e157769c9ae4f7847657c505e78 

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


Testing (updated)
---

`make check` and `sudo make check` (Debian jessie, gcc-4.9.2, w/o optimizations)
`make` (OS X, clang-4.0 w/o optimizations)


Thanks,

Benjamin Bannier

Review Request 52177: Added support for CNI networks for `--task_group`.

[Avinash sridharan]

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

Review request for mesos, Jie Yu and Vinod Kone.


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


Repository: mesos


Description
---

Added support for CNI networks for `--task_group`.


Diffs
-

  src/cli/execute.cpp 79091d6dd3e09e354f6965b0da45662621297bee 

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


Testing
---

make

Ran `mesos-execute` with the --task_group and --network option with the 
following POD definition:
{
  "tasks":[
{
  "name" : "nginx",
  "task_id" : {"value":"nginx"},
  "agent_id": {"value" : ""},
  "resources": [{
"name": "cpus",
"type": "SCALAR",
"scalar": {
  "value": 0.1
},
"role": "*"
  },
  {
"name": "mem",
"type": "SCALAR",
"scalar": {
  "value": 128
},
"role": "*"
  }],
  "container" : {
"type" : "MESOS",
"mesos" : {
  "image" : {
"type" : "DOCKER",
"docker" : {
  "name" : "nginx"
}
  }
}
  }
},
{
  "name" : "redis",
  "task_id" : {"value":"redis"},
  "agent_id": {"value" : ""},
  "resources": [{
"name": "cpus",
"type": "SCALAR",
"scalar": {
  "value": 0.1
},
"role": "*"
  },
  {
"name": "mem",
"type": "SCALAR",
"scalar": {
  "value": 128
},
"role": "*"
  }],
  "container" : {
"type" : "MESOS",
"mesos" : {
  "image" : {
"type" : "DOCKER",
"docker" : {
  "name" : "redis"
}
  }
}
  }
}
  ]
}

The pod definition doesn't do much (mesos can't launch docker containers in 
PODS right now), but the pod itself is attached to a CNI network and it gets an 
IP address and a network name space of its own.


Thanks,

Avinash sridharan

Re: Review Request 52165: Disabled default executor tests for now.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [52143, 52144, 52164, 52145, 52146, 52147, 52148, 52149, 52165]

Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' 
CONFIGURATION='--verbose' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; 
./support/docker_build.sh

- Mesos ReviewBot


On Sept. 22, 2016, 7:33 p.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52165/
> ---
> 
> (Updated Sept. 22, 2016, 7:33 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-6227
> https://issues.apache.org/jira/browse/MESOS-6227
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This is just a temporary set up to make the ReviewBot happy.
> Eventually, when the launcher changes land and we are able to
> launch child containers these tests would pass!
> 
> Please note that it is currently a bit hard to unit test
> the default executor as we don't have the equivalent of
> a `MockMesosContainerizer` that actually launches an
> executor. `TestContainerizer` never launches an executor though
> we can refactor it to suport v1 executors.
> 
> 
> Diffs
> -
> 
>   src/tests/default_executor_tests.cpp 
> 786fbe3baac3c1542b318a26363213a4a1945ddd 
> 
> Diff: https://reviews.apache.org/r/52165/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Review Request 52175: Tweaked test comments.

[Neil Conway]

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

Review request for mesos and Vinod Kone.


Repository: mesos


Description
---

Tweaked test comments.


Diffs
-

  src/tests/partition_tests.cpp 7c38f0efa414447e6292b2d6b334fb9879c92eb5 

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


Testing
---

`make check` on OSX.


Thanks,

Neil Conway

Review Request 52174: Fixed bug with unreachable tasks and disconnected frameworks.

[Neil Conway]

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

Review request for mesos and Vinod Kone.


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


Repository: mesos


Description
---

We previously assumed that when marking a task unreachable, we would
have access to the `FrameworkInfo` for that task's framework. However,
that is not the case if the master has failed over and the framework has
not yet reregistered with the new master.

In that situation, we don't have the framework's FrameworkInfo, and
hence we cannot determine if the framework is partition-aware. For now,
we assume the framework is partition-aware, which isn't great behavior
(MESOS-6232).


Diffs
-

  src/master/master.cpp 66a672f6d16233e96b29e330a9e6c474546fa851 
  src/tests/partition_tests.cpp 7c38f0efa414447e6292b2d6b334fb9879c92eb5 

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


Testing
---

`make check` on OSX.


Thanks,

Neil Conway

Re: Review Request 52145: Added support for reading agent API URL in default executor.

[Guangya Liu]

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




src/launcher/default_executor.cpp (line 356)


How about add a INFO level log for the agent URL? I saw that the 
`executor.cpp` also do not have the URL in log message, but I think it make 
sense to adding such debug log.


- Guangya Liu


On 九月 22, 2016, 7:15 p.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52145/
> ---
> 
> (Updated 九月 22, 2016, 7:15 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-6227
> https://issues.apache.org/jira/browse/MESOS-6227
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This change adds support for populating the agent API URL needed
> for making the calls for launching/waiting/killing
> child containers. Ideally, we should have used the
> `MESOS_AGENT_ENDPOINT` variable but that is a bit hard
> to do with our testcase setup that generates the
> agent process ID.
> 
> 
> Diffs
> -
> 
>   src/launcher/default_executor.cpp 2102fe8d70f0960fed669e1c4f0d6b6cd4af261c 
> 
> Diff: https://reviews.apache.org/r/52145/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Re: Review Request 51124: Support more layers through symlink for overlay backend.

[Zhitao Li]

> On Sept. 21, 2016, 6:26 p.m., Jie Yu wrote:
> > src/tests/containerizer/provisioner_backend_tests.cpp, lines 116-131
> > 
> >
> > I'd suggest we create a separate test. Also, i suggest we construct a 
> > lot of layers (using a for loop), and make sure the test fail without 
> > applying this patch and succeed after applying this patch. This better 
> > captures the regression.

+1 for the for loop based test w/ many layers.
For the comment of "create a separate test", do you mean move these link 
related comparisons to that test?


- Zhitao


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


On Sept. 22, 2016, 8:52 p.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51124/
> ---
> 
> (Updated Sept. 22, 2016, 8:52 p.m.)
> 
> 
> Review request for mesos, Xiaojian Huang, Gilbert Song, and Jie Yu.
> 
> 
> Bugs: MESOS-6000
> https://issues.apache.org/jira/browse/MESOS-6000
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Support more layers through symlink for overlay backend.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/provisioner/backend.hpp 
> 93819c887f2f801f06aae7020084b56cc81ce818 
>   src/slave/containerizer/mesos/provisioner/backends/aufs.hpp 
> a5ac13bc17b8f291472b622ebfdd0cd1dd072273 
>   src/slave/containerizer/mesos/provisioner/backends/aufs.cpp 
> 4c5cdb660dea205aea29d217ba80d845d1108fdf 
>   src/slave/containerizer/mesos/provisioner/backends/bind.hpp 
> 19d44c6a9a2cd5f8de79f33c76e9d79900ab003f 
>   src/slave/containerizer/mesos/provisioner/backends/bind.cpp 
> 62ccaec1a5cfb466e929f4db7bf7e5376f2a0c2d 
>   src/slave/containerizer/mesos/provisioner/backends/copy.hpp 
> 5bd6a5279d8dd6d088afcd58e4839bf6087ccd1e 
>   src/slave/containerizer/mesos/provisioner/backends/copy.cpp 
> e57bb3d2fb4e67e9caae416824a6a748db1460a1 
>   src/slave/containerizer/mesos/provisioner/backends/overlay.hpp 
> 387f28a331813c75a509b4a31dbbdc764080b8c1 
>   src/slave/containerizer/mesos/provisioner/backends/overlay.cpp 
> e20cd48463a78ecd8a652a4d5ac923dc02cca4d9 
>   src/slave/containerizer/mesos/provisioner/provisioner.cpp 
> f37c45ccfa572876dfbba6a0797c223896db5a7f 
>   src/tests/containerizer/provisioner_backend_tests.cpp 
> 7c9a7cd5c733f74e10316fc1234115e6820cc2cb 
> 
> Diff: https://reviews.apache.org/r/51124/diff/
> 
> 
> Testing
> ---
> 
> 1. Make sure `OverlayBackendTest.*` passes;
> 2. Use mesos-execute to provision a container using overlay backend. Observed 
> following log lines:
> 
> ```
> I0816 01:04:27.823420 46584 overlay.cpp:167] Created tempLink at 
> '/var/lib/mesos/provisioner/containers/fd574bbf-4bc0-4538-9ce5-8c2cc93b94c7/backends/overlay/links'
>  to '/tmp/NcmRZt'
> I0816 01:04:27.824834 46584 overlay.cpp:191] Provisioning image rootfs with 
> overlayfs: 
> 'lowerdir=/tmp/NcmRZt/42:/tmp/NcmRZt/41:/tmp/NcmRZt/40:/tmp/NcmRZt/39:/tmp/NcmRZt/38:/tmp/NcmRZt/37:/tmp/NcmRZt/36:/tmp/NcmRZt/35:/tmp/NcmRZt/34:/tmp/NcmRZt/33:/tmp/NcmRZt/32:/tmp/NcmRZt/31:/tmp/NcmRZt/30:/tmp/NcmRZt/29:/tmp/NcmRZt/28:/tmp/NcmRZt/27:/tmp/NcmRZt/26:/tmp/NcmRZt/25:/tmp/NcmRZt/24:/tmp/NcmRZt/23:/tmp/NcmRZt/22:/tmp/NcmRZt/21:/tmp/NcmRZt/20:/tmp/NcmRZt/19:/tmp/NcmRZt/18:/tmp/NcmRZt/17:/tmp/NcmRZt/16:/tmp/NcmRZt/15:/tmp/NcmRZt/14:/tmp/NcmRZt/13:/tmp/NcmRZt/12:/tmp/NcmRZt/11:/tmp/NcmRZt/10:/tmp/NcmRZt/9:/tmp/NcmRZt/8:/tmp/NcmRZt/7:/tmp/NcmRZt/6:/tmp/NcmRZt/5:/tmp/NcmRZt/4:/tmp/NcmRZt/3:/tmp/NcmRZt/2:/tmp/NcmRZt/1:/tmp/NcmRZt/0,upperdir=/var/lib/mesos/provisioner/containers/fd574bbf-4bc0-4538-9ce5-8c2cc93b94c7/backends/overlay/scratch/25d9b6e4-fd0e-401e-bb9e-754dbaec4f55/upperdir,workdir=/var/lib/mesos/provisioner/containers/fd574bbf-4bc0-4538-9ce5-8c2cc93b94c7/backends/overlay/scratch/2
 5d9b6e4-fd0e-401e-bb9e-754dbaec4f55/workdir'
> ...
> (after executor exited)
> I0816 01:04:34.859851 46584 overlay.cpp:281] Removed temporary directory 
> '/tmp/NcmRZt' pointed by 
> '/var/lib/mesos/provisioner/containers/fd574bbf-4bc0-4538-9ce5-8c2cc93b94c7/backends/overlay/links'
> ```
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>

Re: Review Request 51124: Support more layers through symlink for overlay backend.

[Zhitao Li]

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

(Updated Sept. 22, 2016, 8:52 p.m.)


Review request for mesos, Xiaojian Huang, Gilbert Song, and Jie Yu.


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


Repository: mesos


Description
---

Support more layers through symlink for overlay backend.


Diffs (updated)
-

  src/slave/containerizer/mesos/provisioner/backend.hpp 
93819c887f2f801f06aae7020084b56cc81ce818 
  src/slave/containerizer/mesos/provisioner/backends/aufs.hpp 
a5ac13bc17b8f291472b622ebfdd0cd1dd072273 
  src/slave/containerizer/mesos/provisioner/backends/aufs.cpp 
4c5cdb660dea205aea29d217ba80d845d1108fdf 
  src/slave/containerizer/mesos/provisioner/backends/bind.hpp 
19d44c6a9a2cd5f8de79f33c76e9d79900ab003f 
  src/slave/containerizer/mesos/provisioner/backends/bind.cpp 
62ccaec1a5cfb466e929f4db7bf7e5376f2a0c2d 
  src/slave/containerizer/mesos/provisioner/backends/copy.hpp 
5bd6a5279d8dd6d088afcd58e4839bf6087ccd1e 
  src/slave/containerizer/mesos/provisioner/backends/copy.cpp 
e57bb3d2fb4e67e9caae416824a6a748db1460a1 
  src/slave/containerizer/mesos/provisioner/backends/overlay.hpp 
387f28a331813c75a509b4a31dbbdc764080b8c1 
  src/slave/containerizer/mesos/provisioner/backends/overlay.cpp 
e20cd48463a78ecd8a652a4d5ac923dc02cca4d9 
  src/slave/containerizer/mesos/provisioner/provisioner.cpp 
f37c45ccfa572876dfbba6a0797c223896db5a7f 
  src/tests/containerizer/provisioner_backend_tests.cpp 
7c9a7cd5c733f74e10316fc1234115e6820cc2cb 

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


Testing
---

1. Make sure `OverlayBackendTest.*` passes;
2. Use mesos-execute to provision a container using overlay backend. Observed 
following log lines:

```
I0816 01:04:27.823420 46584 overlay.cpp:167] Created tempLink at 
'/var/lib/mesos/provisioner/containers/fd574bbf-4bc0-4538-9ce5-8c2cc93b94c7/backends/overlay/links'
 to '/tmp/NcmRZt'
I0816 01:04:27.824834 46584 overlay.cpp:191] Provisioning image rootfs with 
overlayfs: 
'lowerdir=/tmp/NcmRZt/42:/tmp/NcmRZt/41:/tmp/NcmRZt/40:/tmp/NcmRZt/39:/tmp/NcmRZt/38:/tmp/NcmRZt/37:/tmp/NcmRZt/36:/tmp/NcmRZt/35:/tmp/NcmRZt/34:/tmp/NcmRZt/33:/tmp/NcmRZt/32:/tmp/NcmRZt/31:/tmp/NcmRZt/30:/tmp/NcmRZt/29:/tmp/NcmRZt/28:/tmp/NcmRZt/27:/tmp/NcmRZt/26:/tmp/NcmRZt/25:/tmp/NcmRZt/24:/tmp/NcmRZt/23:/tmp/NcmRZt/22:/tmp/NcmRZt/21:/tmp/NcmRZt/20:/tmp/NcmRZt/19:/tmp/NcmRZt/18:/tmp/NcmRZt/17:/tmp/NcmRZt/16:/tmp/NcmRZt/15:/tmp/NcmRZt/14:/tmp/NcmRZt/13:/tmp/NcmRZt/12:/tmp/NcmRZt/11:/tmp/NcmRZt/10:/tmp/NcmRZt/9:/tmp/NcmRZt/8:/tmp/NcmRZt/7:/tmp/NcmRZt/6:/tmp/NcmRZt/5:/tmp/NcmRZt/4:/tmp/NcmRZt/3:/tmp/NcmRZt/2:/tmp/NcmRZt/1:/tmp/NcmRZt/0,upperdir=/var/lib/mesos/provisioner/containers/fd574bbf-4bc0-4538-9ce5-8c2cc93b94c7/backends/overlay/scratch/25d9b6e4-fd0e-401e-bb9e-754dbaec4f55/upperdir,workdir=/var/lib/mesos/provisioner/containers/fd574bbf-4bc0-4538-9ce5-8c2cc93b94c7/backends/overlay/scratch/25d
 9b6e4-fd0e-401e-bb9e-754dbaec4f55/workdir'
...
(after executor exited)
I0816 01:04:34.859851 46584 overlay.cpp:281] Removed temporary directory 
'/tmp/NcmRZt' pointed by 
'/var/lib/mesos/provisioner/containers/fd574bbf-4bc0-4538-9ce5-8c2cc93b94c7/backends/overlay/links'
```


Thanks,

Zhitao Li

Re: Review Request 52105: Prevent `/redirect/foo` loop.

[Charles Allen]

> On Sept. 21, 2016, 3:18 p.m., haosdent huang wrote:
> > Hi, @drcrallen Thanks a lot for your patch!
> > 
> > I saw your patch try to redirect 
> > `http://SOME_MASTER:5050/master/redirect/master/frameworks` to 
> > `http://LEADER_MASTER:5050/master/frameworks`. Could you elaborate the user 
> > cases about this? Because we have handled the endpoint redirection 
> > automatically internal. For example, when you request 
> > `http://NON_LEADING_MASTER:5050/master/frameworks`, mesos master would 
> > redirect to `http://LEADING_MASTER:5050/master/frameworks`.
> 
> Charles Allen wrote:
> That comes from two things:
> 
> 1. 
> http://mesos.apache.org/documentation/latest/endpoints/master/redirect/ 
> suggests using redirect as a UI bookmarking mechanism, as such, it seemed 
> reasonable for other calls to be able to redirect through the same mechanism. 
> This might be a left-over suggestion from pre-1.0 though. An alternative 
> thing here would be to update the redirect docs to remove the suggestion for 
> UI bookmarking if it is no longer required.
> 2. That's how some other internal discovery mechanism are setup here, so 
> I kind of expected http://SOME_MASTER:5050/master/redirect/state to redirect 
> to the master and get the cluster state. But if the non-leading masters 
> should redirect to the leading masters then I don't think that is helpful 
> anymore, and I think that behavior changed with 1.0?
> 
> haosdent huang wrote:
> Hi, @drcrallen
> 
> >1. ... suggests using redirect as a UI bookmarking mechanism
> 
> I think you may misunderstand this. We suggest to bookmark 
> `http://SOME_MASTER:5050/master/redirect` to open the leading master quickly. 
> And it has never work for `/master/redirect/xxx` before.
> 
> > 2. xxx so I kind of expected 
> http://SOME_MASTER:5050/master/redirect/state to redirect to the master and 
> get the cluster state.
> 
> I think just need to access `http://SOME_MASTER:5050/master/state` here. 
> If `SOME_MASTER` is not the leading master, it would redirct to 
> `http://LEADING_MASTER/master/state` automatically. This hehaviour never 
> change with 1.0
> 
> Charles Allen wrote:
> Allright, I'll simplify the patch to only do the string starts with check.

@Haosdent simplified scope, and updated master comment testing info with new 
results.


- Charles


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


On Sept. 22, 2016, 8:47 p.m., Charles Allen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52105/
> ---
> 
> (Updated Sept. 22, 2016, 8:47 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-6210
> https://issues.apache.org/jira/browse/MESOS-6210
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Prevent `/redirect/foo` loop.
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp 9005e7c308d5f57c6f5c573951d468a3ba730740 
> 
> Diff: https://reviews.apache.org/r/52105/diff/
> 
> 
> Testing
> ---
> 
> Single Node started with `./src/mesos-master --work_dir=/tmp/mesos`
> 
> 
> ```
> Charless-MacBook-Pro:~ charlesallen$ curl -v http://127.0.0.1:5050/redirect
> *   Trying 127.0.0.1...
> * Connected to 127.0.0.1 (127.0.0.1) port 5050 (#0)
> > GET /redirect HTTP/1.1
> > Host: 127.0.0.1:5050
> > User-Agent: curl/7.43.0
> > Accept: */*
> >
> < HTTP/1.1 307 Temporary Redirect
> < Date: Thu, 22 Sep 2016 20:35:18 GMT
> < Location: //10.17.97.185:5050
> < Content-Length: 0
> <
> * Connection #0 to host 127.0.0.1 left intact
> Charless-MacBook-Pro:~ charlesallen$ curl -v 
> http://127.0.0.1:5050/master/redirect
> *   Trying 127.0.0.1...
> * Connected to 127.0.0.1 (127.0.0.1) port 5050 (#0)
> > GET /master/redirect HTTP/1.1
> > Host: 127.0.0.1:5050
> > User-Agent: curl/7.43.0
> > Accept: */*
> >
> < HTTP/1.1 307 Temporary Redirect
> < Date: Thu, 22 Sep 2016 20:35:23 GMT
> < Location: //10.17.97.185:5050
> < Content-Length: 0
> <
> * Connection #0 to host 127.0.0.1 left intact
> Charless-MacBook-Pro:~ charlesallen$ curl -v 
> http://127.0.0.1:5050/master/redirect/
> *   Trying 127.0.0.1...
> * Connected to 127.0.0.1 (127.0.0.1) port 5050 (#0)
> > GET /master/redirect/ HTTP/1.1
> > Host: 127.0.0.1:5050
> > User-Agent: curl/7.43.0
> > Accept: */*
> >
> < HTTP/1.1 404 Not Found
> < Date: Thu, 22 Sep 2016 20:35:29 GMT
> < Content-Length: 0
> <
> * Connection #0 to host 127.0.0.1 left intact
> Charless-MacBook-Pro:~ charlesallen$ curl -v 
> http://127.0.0.1:5050/master/redirect/foo
> *   Trying 127.0.0.1...
> * Connected to 127.0.0.1 (127.0.0.1) port 5050 (#0)
> > GET /master/redirect/foo HTTP/1.1
> > Host: 127.0.0.1:5050
> > User-Agent: curl/7.43.0
> > A

Re: Review Request 52105: Prevent `/redirect/foo` loop.

[Charles Allen]

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

(Updated Sept. 22, 2016, 8:47 p.m.)


Review request for mesos and Vinod Kone.


Changes
---

Updated test case


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


Repository: mesos


Description
---

Prevent `/redirect/foo` loop.


Diffs
-

  src/master/http.cpp 9005e7c308d5f57c6f5c573951d468a3ba730740 

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


Testing (updated)
---

Single Node started with `./src/mesos-master --work_dir=/tmp/mesos`


```
Charless-MacBook-Pro:~ charlesallen$ curl -v http://127.0.0.1:5050/redirect
*   Trying 127.0.0.1...
* Connected to 127.0.0.1 (127.0.0.1) port 5050 (#0)
> GET /redirect HTTP/1.1
> Host: 127.0.0.1:5050
> User-Agent: curl/7.43.0
> Accept: */*
>
< HTTP/1.1 307 Temporary Redirect
< Date: Thu, 22 Sep 2016 20:35:18 GMT
< Location: //10.17.97.185:5050
< Content-Length: 0
<
* Connection #0 to host 127.0.0.1 left intact
Charless-MacBook-Pro:~ charlesallen$ curl -v 
http://127.0.0.1:5050/master/redirect
*   Trying 127.0.0.1...
* Connected to 127.0.0.1 (127.0.0.1) port 5050 (#0)
> GET /master/redirect HTTP/1.1
> Host: 127.0.0.1:5050
> User-Agent: curl/7.43.0
> Accept: */*
>
< HTTP/1.1 307 Temporary Redirect
< Date: Thu, 22 Sep 2016 20:35:23 GMT
< Location: //10.17.97.185:5050
< Content-Length: 0
<
* Connection #0 to host 127.0.0.1 left intact
Charless-MacBook-Pro:~ charlesallen$ curl -v 
http://127.0.0.1:5050/master/redirect/
*   Trying 127.0.0.1...
* Connected to 127.0.0.1 (127.0.0.1) port 5050 (#0)
> GET /master/redirect/ HTTP/1.1
> Host: 127.0.0.1:5050
> User-Agent: curl/7.43.0
> Accept: */*
>
< HTTP/1.1 404 Not Found
< Date: Thu, 22 Sep 2016 20:35:29 GMT
< Content-Length: 0
<
* Connection #0 to host 127.0.0.1 left intact
Charless-MacBook-Pro:~ charlesallen$ curl -v 
http://127.0.0.1:5050/master/redirect/foo
*   Trying 127.0.0.1...
* Connected to 127.0.0.1 (127.0.0.1) port 5050 (#0)
> GET /master/redirect/foo HTTP/1.1
> Host: 127.0.0.1:5050
> User-Agent: curl/7.43.0
> Accept: */*
>
< HTTP/1.1 307 Temporary Redirect
< Date: Thu, 22 Sep 2016 20:35:31 GMT
< Location: //10.17.97.185:5050
< Content-Length: 0
<
* Connection #0 to host 127.0.0.1 left intact
Charless-MacBook-Pro:~ charlesallen$ curl -v 
http://127.0.0.1:5050/master/redirect/foo/bar
*   Trying 127.0.0.1...
* Connected to 127.0.0.1 (127.0.0.1) port 5050 (#0)
> GET /master/redirect/foo/bar HTTP/1.1
> Host: 127.0.0.1:5050
> User-Agent: curl/7.43.0
> Accept: */*
>
< HTTP/1.1 307 Temporary Redirect
< Date: Thu, 22 Sep 2016 20:35:34 GMT
< Location: //10.17.97.185:5050
< Content-Length: 0
<
* Connection #0 to host 127.0.0.1 left intact
Charless-MacBook-Pro:~ charlesallen$ curl -v 
http://127.0.0.1:5050/redirect/foo/bar
*   Trying 127.0.0.1...
* Connected to 127.0.0.1 (127.0.0.1) port 5050 (#0)
> GET /redirect/foo/bar HTTP/1.1
> Host: 127.0.0.1:5050
> User-Agent: curl/7.43.0
> Accept: */*
>
< HTTP/1.1 307 Temporary Redirect
< Date: Thu, 22 Sep 2016 20:35:41 GMT
< Location: //10.17.97.185:5050
< Content-Length: 0
<
* Connection #0 to host 127.0.0.1 left intact
Charless-MacBook-Pro:~ charlesallen$ curl -v 
http://127.0.0.1:5050/redirectNOTFOUND/foo/bar
*   Trying 127.0.0.1...
* Connected to 127.0.0.1 (127.0.0.1) port 5050 (#0)
> GET /redirectNOTFOUND/foo/bar HTTP/1.1
> Host: 127.0.0.1:5050
> User-Agent: curl/7.43.0
> Accept: */*
>
< HTTP/1.1 404 Not Found
< Date: Thu, 22 Sep 2016 20:35:47 GMT
< Content-Length: 0
<
* Connection #0 to host 127.0.0.1 left intact
```

It is worth noting that in this PR /master/redirect/  and /redirect/ return 
404, not 307. This behavior is consistent with master.


Thanks,

Charles Allen

Re: Review Request 52105: Prevent `/redirect/foo` loop.

[Charles Allen]

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

(Updated Sept. 22, 2016, 8:46 p.m.)


Review request for mesos and Vinod Kone.


Changes
---

Now with simple loop prevention


Summary (updated)
-

Prevent `/redirect/foo` loop.


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


Repository: mesos


Description (updated)
---

Prevent `/redirect/foo` loop.


Diffs (updated)
-

  src/master/http.cpp 9005e7c308d5f57c6f5c573951d468a3ba730740 

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


Testing
---

Single Node started with `./src/mesos-master --work_dir=/tmp/mesos`


```
Charless-MacBook-Pro:~ charlesallen$ curl -v http://127.0.0.1:5050/redirect
*   Trying 127.0.0.1...
* Connected to 127.0.0.1 (127.0.0.1) port 5050 (#0)
> GET /redirect HTTP/1.1
> Host: 127.0.0.1:5050
> User-Agent: curl/7.43.0
> Accept: */*
>
< HTTP/1.1 307 Temporary Redirect
< Date: Tue, 20 Sep 2016 23:12:24 GMT
< Location: //10.17.97.185:5050
< Content-Length: 0
<
* Connection #0 to host 127.0.0.1 left intact
Charless-MacBook-Pro:~ charlesallen$ curl -v http://127.0.0.1:5050/redirect/foo
*   Trying 127.0.0.1...
* Connected to 127.0.0.1 (127.0.0.1) port 5050 (#0)
> GET /redirect/foo HTTP/1.1
> Host: 127.0.0.1:5050
> User-Agent: curl/7.43.0
> Accept: */*
>
< HTTP/1.1 307 Temporary Redirect
< Date: Tue, 20 Sep 2016 23:12:35 GMT
< Location: //10.17.97.185:5050/foo
< Content-Length: 0
<
* Connection #0 to host 127.0.0.1 left intact
Charless-MacBook-Pro:~ charlesallen$ curl -v 
http://127.0.0.1:5050/master/redirect/test
*   Trying 127.0.0.1...
* Connected to 127.0.0.1 (127.0.0.1) port 5050 (#0)
> GET /master/redirect/test HTTP/1.1
> Host: 127.0.0.1:5050
> User-Agent: curl/7.43.0
> Accept: */*
>
< HTTP/1.1 307 Temporary Redirect
< Date: Tue, 20 Sep 2016 23:12:49 GMT
< Location: //10.17.97.185:5050/test
< Content-Length: 0
<
* Connection #0 to host 127.0.0.1 left intact
Charless-MacBook-Pro:~ charlesallen$ curl -v 
http://127.0.0.1:5050/master/redirectNOTFOUND/test
*   Trying 127.0.0.1...
* Connected to 127.0.0.1 (127.0.0.1) port 5050 (#0)
> GET /master/redirectNOTFOUND/test HTTP/1.1
> Host: 127.0.0.1:5050
> User-Agent: curl/7.43.0
> Accept: */*
>
< HTTP/1.1 404 Not Found
< Date: Tue, 20 Sep 2016 23:12:57 GMT
< Content-Length: 0
<
* Connection #0 to host 127.0.0.1 left intact
Charless-MacBook-Pro:~ charlesallen$ curl -v 
http://127.0.0.1:5050/master/redirect/
*   Trying 127.0.0.1...
* Connected to 127.0.0.1 (127.0.0.1) port 5050 (#0)
> GET /master/redirect/ HTTP/1.1
> Host: 127.0.0.1:5050
> User-Agent: curl/7.43.0
> Accept: */*
>
< HTTP/1.1 404 Not Found
< Date: Tue, 20 Sep 2016 23:13:32 GMT
< Content-Length: 0
<
* Connection #0 to host 127.0.0.1 left intact
Charless-MacBook-Pro:~ charlesallen$ curl -v 
http://127.0.0.1:5050/master/redirect/test
*   Trying 127.0.0.1...
* Connected to 127.0.0.1 (127.0.0.1) port 5050 (#0)
> GET /master/redirect/test HTTP/1.1
> Host: 127.0.0.1:5050
> User-Agent: curl/7.43.0
> Accept: */*
>
< HTTP/1.1 307 Temporary Redirect
< Date: Tue, 20 Sep 2016 23:13:43 GMT
< Location: //10.17.97.185:5050/test
< Content-Length: 0
<
* Connection #0 to host 127.0.0.1 left intact
```

It is worth noting that in this PR /master/redirect/  and /redirect/ return 
404, not 307. This behavior is consistent with master.


Thanks,

Charles Allen

Re: Review Request 51930: Introduced Linux capabilities support for Mesos executor.

[Benjamin Bannier]

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

(Updated Sept. 22, 2016, 10:24 p.m.)


Review request for mesos and Jie Yu.


Changes
---

Addressed Jie's comment.


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


Repository: mesos


Description
---

This change introduces Linux capability-based security the Mesos
exector. A new flag `capabilities` is introduced to optionally specify
the capabilities tasks launched by the Mesos executor are allowed to
use.


Diffs (updated)
-

  src/launcher/executor.cpp 5370634ef9e6f3ac9717fed71f6a77707026a16a 
  src/launcher/posix/executor.hpp 9e467263dcc0623fce83b18887896547db9aa16b 
  src/launcher/posix/executor.cpp 0f3fed3f117d150a1020a3b2987f9763c6a343b9 

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


Testing
---

Tested as part of https://reviews.apache.org/r/50271/ on ubuntu-14.04 w/ 
gcc-4.8.4 w/o optimizations.


Thanks,

Benjamin Bannier

Re: Review Request 52144: Populated the timestamp field when sending status updates.

[Joseph Wu]

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


Ship it!




Ship It!

- Joseph Wu


On Sept. 22, 2016, 12:15 p.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52144/
> ---
> 
> (Updated Sept. 22, 2016, 12:15 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The command executor/default executor were not populating
> the timestamp for the status update. Previously, the executor
> driver used to do it underneath but that won't work with using
> the HTTP command executor.
> 
> 
> Diffs
> -
> 
>   src/launcher/default_executor.cpp 2102fe8d70f0960fed669e1c4f0d6b6cd4af261c 
>   src/launcher/executor.cpp 5370634ef9e6f3ac9717fed71f6a77707026a16a 
> 
> Diff: https://reviews.apache.org/r/52144/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Re: Review Request 52153: Fixed openssl CA location logging.

[Joseph Wu]

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


Fix it, then Ship it!





3rdparty/libprocess/src/openssl.cpp (line 493)


I recall that we prefer `.at(...)` even when there's an existence check 
directly preceeding the map access.



3rdparty/libprocess/src/openssl.cpp (line 501)


Ditto here.


- Joseph Wu


On Sept. 22, 2016, 2:43 a.m., Till Toenshoff wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52153/
> ---
> 
> (Updated Sept. 22, 2016, 2:43 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Joris Van Remoortere, and Joseph 
> Wu.
> 
> 
> Bugs: MESOS-5320
> https://issues.apache.org/jira/browse/MESOS-5320
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> see summary.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/openssl.cpp 
> c09cdc89509e4e4ca4c8a0f4fb0a57156a3a6091 
> 
> Diff: https://reviews.apache.org/r/52153/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Till Toenshoff
> 
>

Re: Review Request 52135: Implemented the *_NESTED_CONTAINER calls in the agent API.

[Benjamin Mahler]

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

(Updated Sept. 22, 2016, 7:49 p.m.)


Review request for mesos and Vinod Kone.


Changes
---

Updated per feedback:
- Split apart the tests.
- Avoid inconsistent quoting of ContainerID (no need to quote but I left in one 
case of quoting).
- Updated a log message for consistency.
- Explained why the test needs to expect a call to `Containerizer::containers`.


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


Repository: mesos


Description
---

This patch adds the wiring for the *_NESTED_CONTAINER calls,
including validation and calling into the containerizer.


Diffs (updated)
-

  src/slave/http.cpp 73135be5651300e1fe7a20428aeb034392f915ed 
  src/tests/api_tests.cpp 26f99f7c337fbbc5278d1b30d3d5c8f659ddf5ca 

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


Testing
---

Added a test that mocks out the containerizer.


Thanks,

Benjamin Mahler

Re: Review Request 52134: Pulled out the mock containerizer into a header for reuse.

[Benjamin Mahler]

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

(Updated Sept. 22, 2016, 7:46 p.m.)


Review request for mesos and Vinod Kone.


Changes
---

Removed an unnecessary include.


Repository: mesos


Description
---

This also adds a mock of the newly introduce nested container
`launch()` member function.


Diffs (updated)
-

  src/Makefile.am 239e5af89d3f57054d0c86ed79b9d40b158e6c19 
  src/tests/containerizer/composing_containerizer_tests.cpp 
6772c7e1682da6652742ef552cc0e4ee9fc61b8f 
  src/tests/containerizer/mock_containerizer.hpp PRE-CREATION 

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


Testing
---

make check


Thanks,

Benjamin Mahler

Re: Review Request 52135: Implemented the *_NESTED_CONTAINER calls in the agent API.

[Benjamin Mahler]

> On Sept. 22, 2016, 5:23 p.m., Vinod Kone wrote:
> > src/tests/api_tests.cpp, lines 3235-3280
> > 
> >
> > Hmm. I would put them in their own separate tests since the setup for 
> > them is easy enough.

Sounds good.


> On Sept. 22, 2016, 5:23 p.m., Vinod Kone wrote:
> > src/slave/http.cpp, lines 1977-1979
> > 
> >
> > I'm wondering if we should include the whole ContainerTermination proto 
> > in the response instead of just the status.  For example, the "message" and 
> > "reasons" might be useful if the container OOMs.
> > 
> > Thoughts?

Agreed, I'll punt on this for a separate follow-up.

My thinking for omitting them was that TaskState.Reason seemed unfortunately 
wide (many values are not applicable here). So, I was hoping we could introduce 
a container specific termination reason (OOM, OOD, etc). Note that right now 
the resources are not used, so it won't be useful just yet.

For message, we could add it but I'd like to add it ideally after we have the 
Reason in place (OOM, OOD, etc), to avoid users parsing / relying on the 
message format (that might change underneath them).


- Benjamin


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


On Sept. 22, 2016, 6:06 a.m., Benjamin Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52135/
> ---
> 
> (Updated Sept. 22, 2016, 6:06 a.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-2449
> https://issues.apache.org/jira/browse/MESOS-2449
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch adds the wiring for the *_NESTED_CONTAINER calls,
> including validation and calling into the containerizer.
> 
> 
> Diffs
> -
> 
>   src/slave/http.cpp 0aae14583faf95fee678b3a3424c96cdba531968 
>   src/tests/api_tests.cpp 26f99f7c337fbbc5278d1b30d3d5c8f659ddf5ca 
> 
> Diff: https://reviews.apache.org/r/52135/diff/
> 
> 
> Testing
> ---
> 
> Added a test that mocks out the containerizer.
> 
> 
> Thanks,
> 
> Benjamin Mahler
> 
>

Re: Review Request 52147: Added support for launching child containers to the default executor.

[Anand Mazumdar]

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

(Updated Sept. 22, 2016, 7:33 p.m.)


Review request for mesos and Vinod Kone.


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


Repository: mesos


Description
---

This change adds support for sending the `LAUNCH_NESTED_CONTAINER`
call to the agent to launch a child container.


Diffs (updated)
-

  src/launcher/default_executor.cpp 2102fe8d70f0960fed669e1c4f0d6b6cd4af261c 

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


Testing
---

make check


Thanks,

Anand Mazumdar

Re: Review Request 52148: Added support for killing child containers to the default executor.

[Anand Mazumdar]

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

(Updated Sept. 22, 2016, 7:33 p.m.)


Review request for mesos and Vinod Kone.


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


Repository: mesos


Description (updated)
---

This change implements support for killing child containers via
the `KILL_NESTED_CONTAINER` call on the Agent API. This is
triggered when the executor receives a `KILL`/`SHUTDOWN`
event. Currently, only SIGKILL is supported. Also, the specifying
custom kill policies is not yet supported.


Diffs (updated)
-

  src/launcher/default_executor.cpp 2102fe8d70f0960fed669e1c4f0d6b6cd4af261c 

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


Testing (updated)
---

make check


Thanks,

Anand Mazumdar

Re: Review Request 52146: Made `killTask()` invoke `shutdown()`.

[Anand Mazumdar]

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

(Updated Sept. 22, 2016, 7:33 p.m.)


Review request for mesos and Vinod Kone.


Repository: mesos


Description
---

This change gets rid of the trivial `killTask()` implementation
and makes it call `shutdown()`. It also gets rid of the existing
`taskGroup` member variable. Eventually, later in the review chain
we would populate these functions to make the actual calls to the
agent.


Diffs (updated)
-

  src/launcher/default_executor.cpp 2102fe8d70f0960fed669e1c4f0d6b6cd4af261c 

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


Testing
---


Thanks,

Anand Mazumdar

Re: Review Request 52149: Added support for waiting on child containers to the default executor.

[Anand Mazumdar]

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

(Updated Sept. 22, 2016, 7:33 p.m.)


Review request for mesos and Vinod Kone.


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


Repository: mesos


Description
---

This change adds support for waiting on child containers via the
`WAIT_NESTED_CONTAINER` call on the Agent API. If the connection
fails due to a temporary network blip, it reconnects with the
agent.


Diffs (updated)
-

  src/launcher/default_executor.cpp 2102fe8d70f0960fed669e1c4f0d6b6cd4af261c 

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


Testing
---

make check


Thanks,

Anand Mazumdar

Re: Review Request 52165: Disabled default executor tests for now.

[Anand Mazumdar]

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

(Updated Sept. 22, 2016, 7:33 p.m.)


Review request for mesos and Vinod Kone.


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


Repository: mesos


Description (updated)
---

This is just a temporary set up to make the ReviewBot happy.
Eventually, when the launcher changes land and we are able to
launch child containers these tests would pass!

Please note that it is currently a bit hard to unit test
the default executor as we don't have the equivalent of
a `MockMesosContainerizer` that actually launches an
executor. `TestContainerizer` never launches an executor though
we can refactor it to suport v1 executors.


Diffs (updated)
-

  src/tests/default_executor_tests.cpp 786fbe3baac3c1542b318a26363213a4a1945ddd 

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


Testing
---

make check


Thanks,

Anand Mazumdar

Re: Review Request 52105: Add smarter master redirects.

[Charles Allen]

> On Sept. 21, 2016, 3:18 p.m., haosdent huang wrote:
> > Hi, @drcrallen Thanks a lot for your patch!
> > 
> > I saw your patch try to redirect 
> > `http://SOME_MASTER:5050/master/redirect/master/frameworks` to 
> > `http://LEADER_MASTER:5050/master/frameworks`. Could you elaborate the user 
> > cases about this? Because we have handled the endpoint redirection 
> > automatically internal. For example, when you request 
> > `http://NON_LEADING_MASTER:5050/master/frameworks`, mesos master would 
> > redirect to `http://LEADING_MASTER:5050/master/frameworks`.
> 
> Charles Allen wrote:
> That comes from two things:
> 
> 1. 
> http://mesos.apache.org/documentation/latest/endpoints/master/redirect/ 
> suggests using redirect as a UI bookmarking mechanism, as such, it seemed 
> reasonable for other calls to be able to redirect through the same mechanism. 
> This might be a left-over suggestion from pre-1.0 though. An alternative 
> thing here would be to update the redirect docs to remove the suggestion for 
> UI bookmarking if it is no longer required.
> 2. That's how some other internal discovery mechanism are setup here, so 
> I kind of expected http://SOME_MASTER:5050/master/redirect/state to redirect 
> to the master and get the cluster state. But if the non-leading masters 
> should redirect to the leading masters then I don't think that is helpful 
> anymore, and I think that behavior changed with 1.0?
> 
> haosdent huang wrote:
> Hi, @drcrallen
> 
> >1. ... suggests using redirect as a UI bookmarking mechanism
> 
> I think you may misunderstand this. We suggest to bookmark 
> `http://SOME_MASTER:5050/master/redirect` to open the leading master quickly. 
> And it has never work for `/master/redirect/xxx` before.
> 
> > 2. xxx so I kind of expected 
> http://SOME_MASTER:5050/master/redirect/state to redirect to the master and 
> get the cluster state.
> 
> I think just need to access `http://SOME_MASTER:5050/master/state` here. 
> If `SOME_MASTER` is not the leading master, it would redirct to 
> `http://LEADING_MASTER/master/state` automatically. This hehaviour never 
> change with 1.0

Allright, I'll simplify the patch to only do the string starts with check.


- Charles


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


On Sept. 21, 2016, 3:03 a.m., Charles Allen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52105/
> ---
> 
> (Updated Sept. 21, 2016, 3:03 a.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-6210
> https://issues.apache.org/jira/browse/MESOS-6210
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add smarter master redirects.
> 
> 
> Diffs
> -
> 
>   docs/endpoints/master/redirect.md 3387b64c32fe394949863cbcb3e2b58be1b00bd0 
>   src/master/http.cpp 9005e7c308d5f57c6f5c573951d468a3ba730740 
> 
> Diff: https://reviews.apache.org/r/52105/diff/
> 
> 
> Testing
> ---
> 
> Single Node started with `./src/mesos-master --work_dir=/tmp/mesos`
> 
> 
> ```
> Charless-MacBook-Pro:~ charlesallen$ curl -v http://127.0.0.1:5050/redirect
> *   Trying 127.0.0.1...
> * Connected to 127.0.0.1 (127.0.0.1) port 5050 (#0)
> > GET /redirect HTTP/1.1
> > Host: 127.0.0.1:5050
> > User-Agent: curl/7.43.0
> > Accept: */*
> >
> < HTTP/1.1 307 Temporary Redirect
> < Date: Tue, 20 Sep 2016 23:12:24 GMT
> < Location: //10.17.97.185:5050
> < Content-Length: 0
> <
> * Connection #0 to host 127.0.0.1 left intact
> Charless-MacBook-Pro:~ charlesallen$ curl -v 
> http://127.0.0.1:5050/redirect/foo
> *   Trying 127.0.0.1...
> * Connected to 127.0.0.1 (127.0.0.1) port 5050 (#0)
> > GET /redirect/foo HTTP/1.1
> > Host: 127.0.0.1:5050
> > User-Agent: curl/7.43.0
> > Accept: */*
> >
> < HTTP/1.1 307 Temporary Redirect
> < Date: Tue, 20 Sep 2016 23:12:35 GMT
> < Location: //10.17.97.185:5050/foo
> < Content-Length: 0
> <
> * Connection #0 to host 127.0.0.1 left intact
> Charless-MacBook-Pro:~ charlesallen$ curl -v 
> http://127.0.0.1:5050/master/redirect/test
> *   Trying 127.0.0.1...
> * Connected to 127.0.0.1 (127.0.0.1) port 5050 (#0)
> > GET /master/redirect/test HTTP/1.1
> > Host: 127.0.0.1:5050
> > User-Agent: curl/7.43.0
> > Accept: */*
> >
> < HTTP/1.1 307 Temporary Redirect
> < Date: Tue, 20 Sep 2016 23:12:49 GMT
> < Location: //10.17.97.185:5050/test
> < Content-Length: 0
> <
> * Connection #0 to host 127.0.0.1 left intact
> Charless-MacBook-Pro:~ charlesallen$ curl -v 
> http://127.0.0.1:5050/master/redirectNOTFOUND/test
> *   Trying 127.0.0.1...
> * Connected to 127.0.0.1 (127.0.0.1) port 5050 (#0)
> > GET /master/redirectNOTFOUND/test HTTP/1.1
> > Host: 127.0.0.1:5050

Re: Review Request 52146: Made `killTask()` invoke `shutdown()`.

[Anand Mazumdar]

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

(Updated Sept. 22, 2016, 7:23 p.m.)


Review request for mesos and Vinod Kone.


Repository: mesos


Description
---

This change gets rid of the trivial `killTask()` implementation
and makes it call `shutdown()`. It also gets rid of the existing
`taskGroup` member variable. Eventually, later in the review chain
we would populate these functions to make the actual calls to the
agent.


Diffs (updated)
-

  src/launcher/default_executor.cpp 2102fe8d70f0960fed669e1c4f0d6b6cd4af261c 

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


Testing
---


Thanks,

Anand Mazumdar

Re: Review Request 52153: Fixed openssl CA location logging.

[Joris Van Remoortere]

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


Ship it!




Ship It!

- Joris Van Remoortere


On Sept. 22, 2016, 9:43 a.m., Till Toenshoff wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52153/
> ---
> 
> (Updated Sept. 22, 2016, 9:43 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Joris Van Remoortere, and Joseph 
> Wu.
> 
> 
> Bugs: MESOS-5320
> https://issues.apache.org/jira/browse/MESOS-5320
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> see summary.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/openssl.cpp 
> c09cdc89509e4e4ca4c8a0f4fb0a57156a3a6091 
> 
> Diff: https://reviews.apache.org/r/52153/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Till Toenshoff
> 
>

Re: Review Request 52031: Added openssl error string output to initializing failures.

[Joris Van Remoortere]

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


Ship it!




Ship It!

- Joris Van Remoortere


On Sept. 22, 2016, 9:27 a.m., Till Toenshoff wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52031/
> ---
> 
> (Updated Sept. 22, 2016, 9:27 a.m.)
> 
> 
> Review request for mesos, Joris Van Remoortere and Joseph Wu.
> 
> 
> Bugs: MESOS-5320
> https://issues.apache.org/jira/browse/MESOS-5320
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Adds the human readable openssl error messages for failure cases. Also
> fixes a spacing nit in one of the existing messages.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/openssl.cpp 
> c09cdc89509e4e4ca4c8a0f4fb0a57156a3a6091 
> 
> Diff: https://reviews.apache.org/r/52031/diff/
> 
> 
> Testing
> ---
> 
> make check && functional testing
> 
> 
> Thanks,
> 
> Till Toenshoff
> 
>

Re: Review Request 52146: Made `killTask()` invoke `shutdown()`.

[Anand Mazumdar]

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

(Updated Sept. 22, 2016, 7:16 p.m.)


Review request for mesos and Vinod Kone.


Repository: mesos


Description
---

This change gets rid of the trivial `killTask()` implementation
and makes it call `shutdown()`. It also gets rid of the existing
`taskGroup` member variable. Eventually, later in the review chain
we would populate these functions to make the actual calls to the
agent.


Diffs (updated)
-

  src/launcher/default_executor.cpp 2102fe8d70f0960fed669e1c4f0d6b6cd4af261c 

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


Testing
---


Thanks,

Anand Mazumdar

Re: Review Request 52145: Added support for reading agent API URL in default executor.

[Anand Mazumdar]

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

(Updated Sept. 22, 2016, 7:15 p.m.)


Review request for mesos and Vinod Kone.


Summary (updated)
-

Added support for reading agent API URL in default executor.


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


Repository: mesos


Description (updated)
---

This change adds support for populating the agent API URL needed
for making the calls for launching/waiting/killing
child containers. Ideally, we should have used the
`MESOS_AGENT_ENDPOINT` variable but that is a bit hard
to do with our testcase setup that generates the
agent process ID.


Diffs (updated)
-

  src/launcher/default_executor.cpp 2102fe8d70f0960fed669e1c4f0d6b6cd4af261c 

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


Testing
---

make check


Thanks,

Anand Mazumdar

Re: Review Request 52164: Added `ContainerID` field to the executor `SUBSCRIBED` event.

[Anand Mazumdar]

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

(Updated Sept. 22, 2016, 7:15 p.m.)


Review request for mesos and Vinod Kone.


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


Repository: mesos


Description
---

This change adds the field `ContainerID` to the executor `SUBSCRIBED`
event. This allows the default executor/other custom executors to
then launch nested containers. This is needed as the executors need
to let the agent know of the parent container ID when they make the
`LAUNCH_NESTED_CONTAINER` call. This change also implements the
logic on the agent to send the event to the executor.


Diffs (updated)
-

  include/mesos/executor/executor.proto 
0d31406a70d6b7d9030c1f157a7869a2e8f1879f 
  include/mesos/v1/executor/executor.proto 
f661b71cecb96a9971c687f117625ef242749394 
  src/slave/slave.cpp 11e9c8af87aa5153f72f2a20cc578fe3d729b153 

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


Testing
---

make check


Thanks,

Anand Mazumdar

Re: Review Request 52144: Populated the timestamp field when sending status updates.

[Anand Mazumdar]

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

(Updated Sept. 22, 2016, 7:15 p.m.)


Review request for mesos and Vinod Kone.


Repository: mesos


Description
---

The command executor/default executor were not populating
the timestamp for the status update. Previously, the executor
driver used to do it underneath but that won't work with using
the HTTP command executor.


Diffs (updated)
-

  src/launcher/default_executor.cpp 2102fe8d70f0960fed669e1c4f0d6b6cd4af261c 
  src/launcher/executor.cpp 5370634ef9e6f3ac9717fed71f6a77707026a16a 

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


Testing
---

make check


Thanks,

Anand Mazumdar

Re: Review Request 52143: Added evolve handler for `agent::Call` to `v1::agent::Call`.

[Anand Mazumdar]

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

(Updated Sept. 22, 2016, 7:14 p.m.)


Review request for mesos and Vinod Kone.


Repository: mesos


Description
---

Added evolve handler for `agent::Call` to `v1::agent::Call`.


Diffs (updated)
-

  src/internal/evolve.hpp 52a1302cf24f34ec676244b3f81cbf4a5e0a4805 
  src/internal/evolve.cpp 9ae720cdc22c6bcdd56bc6bb391419c52df446f3 

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


Testing
---

make check


Thanks,

Anand Mazumdar

Re: Review Request 52033: Escalated some openssl logs from VLOG to INFO.

[Joris Van Remoortere]

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


Ship it!




Ship It!

- Joris Van Remoortere


On Sept. 22, 2016, 9:44 a.m., Till Toenshoff wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52033/
> ---
> 
> (Updated Sept. 22, 2016, 9:44 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Joris Van Remoortere, and Joseph 
> Wu.
> 
> 
> Bugs: MESOS-5320
> https://issues.apache.org/jira/browse/MESOS-5320
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The logging messages in question prove to be very useful for debugging
> the cluster setup and hence we decided they should be generally
> available as their helpfulnes by far outwages their noise given that
> we call these only during initializing.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/openssl.cpp 
> c09cdc89509e4e4ca4c8a0f4fb0a57156a3a6091 
> 
> Diff: https://reviews.apache.org/r/52033/diff/
> 
> 
> Testing
> ---
> 
> make check && functional testing
> 
> 
> Thanks,
> 
> Till Toenshoff
> 
>

Re: Review Request 50271: Created an isolator for Linux capabilities.

[Jie Yu]

> On Sept. 20, 2016, 2:38 a.m., Jie Yu wrote:
> > src/Makefile.am, line 834
> > 
> >
> > This should belong linux files below?
> 
> Benjamin Bannier wrote:
> This isolator has no hard dependency on Linux so we can build it on all 
> platforms. I think it would be great to have as much code as possible not 
> behind `ifdef`s. What do you think?

I don't see any way to impl that on mac. All the capability constants we 
defined in header is linux specific. I still suggest we make it linux specific.


- Jie


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


On Sept. 20, 2016, 11:40 a.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50271/
> ---
> 
> (Updated Sept. 20, 2016, 11:40 a.m.)
> 
> 
> Review request for mesos, Jay Guo and Jie Yu.
> 
> 
> Bugs: MESOS-5275
> https://issues.apache.org/jira/browse/MESOS-5275
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This isolator evaluates agent allowed capabilities and passes net
> capabilities on to `mesos-containerizer` which enforces the
> capabilities.
> 
> Capability information is passed via a new field in
> `ContainerLaunchInfo`.
> 
> 
> Diffs
> -
> 
>   include/mesos/slave/containerizer.proto 
> 20db010ea158a813034b41ce9cddac7d8317 
>   src/CMakeLists.txt 42c52b60cc850901f2eff1545cf7900f4a65ca81 
>   src/Makefile.am 478fb5ae01dbdf1a735680edf7c5f30867726e07 
>   src/slave/containerizer/mesos/containerizer.cpp 
> e54169ba00f6e0cdd7043075b4cdd12d714c99e3 
>   src/slave/containerizer/mesos/isolators/linux/capabilities.hpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/linux/capabilities.cpp PRE-CREATION 
>   src/slave/containerizer/mesos/launch.hpp 
> 859990cb85e9e8c06400397256cfc512f0811800 
>   src/slave/containerizer/mesos/launch.cpp 
> fc51e04ec1572679e6a48ff4f0fa31ef2dfd6ec3 
>   src/slave/flags.hpp 3952d04f6a00ac1dca1adf2bea7cc6e415620ce5 
>   src/tests/containerizer/isolator_tests.cpp 
> 93ce75180520d382f63ce7323be844fe43c6594e 
> 
> Diff: https://reviews.apache.org/r/50271/diff/
> 
> 
> Testing
> ---
> 
> `make check` and `sudo make check` (Debian jessie, gcc-4.9.2, w/o 
> optimizations)
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 50271: Created an isolator for Linux capabilities.

[Jie Yu]

> On Sept. 20, 2016, 2:38 a.m., Jie Yu wrote:
> > src/slave/containerizer/mesos/containerizer.cpp, lines 1242-1251
> > 
> >
> > I understand why you want to do this check, but we usually do not do 
> > such checks in containerizer. We rely on operator to properly configure the 
> > agent.
> > 
> > The correct solution I believe is to advertise agent isolation 
> > capabilities to the master and let master reject those tasks.
> > 
> > I would simply remove this check here for now.
> 
> Benjamin Bannier wrote:
> Like you probably know I added this to satisfy a requirement from the 
> design doc where we specified for the case when `--isolation=capabilities` is 
> absent:
> 
> a) ContainerInfo::CapabilityInfo not provided:
> All capabilities for the user. (no isolation)
> b) ContainerInfo::CapabilityInfo provided, ContainerInfo::CapabilityInfo 
> is empty:
> Not supported. Agent will fail the task with appropriate reasoning.
> c) ContainerInfo::CapabilityInfo is NOT empty:
> Not supported. Agent will fail the task with appropriate reasoning.   
>  
> 
> Do you mean the design doc is wrong when it says this work should be 
> performed in the agent (instead the master should be fail the task)? In that 
> case I wonder if we wouldn't still need to check in the agent, e.g., to allow 
> restarting agents with changed capabilities.

yeah, I feel this is the wrong way to solve the issue. If we need to do this, 
we need to do a lot for other isolators as well (e.g., gpu).

For instance, maybe we should still invoke those some validation method even if 
the isolator is not enabled. In that way, the checks can be distributed to each 
isolator, instead of accumulating in containerizer. I just feel that adding 
isolator specific checks in the containerizer like that is not going to scale.

For now, i'll just punt on that and clearly document that in the doc. Sounds 
good?


- Jie


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


On Sept. 20, 2016, 11:40 a.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50271/
> ---
> 
> (Updated Sept. 20, 2016, 11:40 a.m.)
> 
> 
> Review request for mesos, Jay Guo and Jie Yu.
> 
> 
> Bugs: MESOS-5275
> https://issues.apache.org/jira/browse/MESOS-5275
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This isolator evaluates agent allowed capabilities and passes net
> capabilities on to `mesos-containerizer` which enforces the
> capabilities.
> 
> Capability information is passed via a new field in
> `ContainerLaunchInfo`.
> 
> 
> Diffs
> -
> 
>   include/mesos/slave/containerizer.proto 
> 20db010ea158a813034b41ce9cddac7d8317 
>   src/CMakeLists.txt 42c52b60cc850901f2eff1545cf7900f4a65ca81 
>   src/Makefile.am 478fb5ae01dbdf1a735680edf7c5f30867726e07 
>   src/slave/containerizer/mesos/containerizer.cpp 
> e54169ba00f6e0cdd7043075b4cdd12d714c99e3 
>   src/slave/containerizer/mesos/isolators/linux/capabilities.hpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/linux/capabilities.cpp PRE-CREATION 
>   src/slave/containerizer/mesos/launch.hpp 
> 859990cb85e9e8c06400397256cfc512f0811800 
>   src/slave/containerizer/mesos/launch.cpp 
> fc51e04ec1572679e6a48ff4f0fa31ef2dfd6ec3 
>   src/slave/flags.hpp 3952d04f6a00ac1dca1adf2bea7cc6e415620ce5 
>   src/tests/containerizer/isolator_tests.cpp 
> 93ce75180520d382f63ce7323be844fe43c6594e 
> 
> Diff: https://reviews.apache.org/r/50271/diff/
> 
> 
> Testing
> ---
> 
> `make check` and `sudo make check` (Debian jessie, gcc-4.9.2, w/o 
> optimizations)
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>