Re: Review Request 47216: Wired up the new docker environment hook.

2016-05-27 Thread Joseph Wu 

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

(Updated May 27, 2016, 5:17 p.m.)


Review request for mesos, Adam B, Artem Harutyunyan, Jie Yu, and Kapil Arya.


Changes
---

Address `Container*` delete race.


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


Repository: mesos


Description
---

Modifies the code path for docker executors.

Docker command executors are now launched with an additional flag
that is filled in by a hook.  The --task_environment flag tells the
command executor to pass some specified mapping of environment
variables to the task.

Custom executors are launched with the environment variables directly.
It is up to custom executors to pass these variables into tasks.


Diffs (updated)
-

  src/slave/containerizer/docker.hpp fac26a239e9981e5b1c2f2f90b52785819492c2a 
  src/slave/containerizer/docker.cpp 7af14f4472283ceefd73c06dd8df60af4cf6f7e8 

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


Testing
---

sudo make check


Thanks,

Joseph Wu

Re: Review Request 47216: Wired up the new docker environment hook.

2016-05-27 Thread Jie Yu 

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


Fix it, then Ship it!





src/slave/containerizer/docker.cpp (lines 1048 - 1049)


I would put them into one line here:
```
f = HookManager::slavePreLaunchDockerEnvironmentDecorator(
taskInfo,
...);
```



src/slave/containerizer/docker.cpp (lines 1056 - 1058)


`container` might be deleted before the callback in `then` is called. You 
should:
```
.then(defer(self(), [=](...) {
  if (!containers_.contains(containerId)) {
return Failure("...");
  }
  
  ...
});
```


- Jie Yu


On May 27, 2016, 10:10 p.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47216/
> ---
> 
> (Updated May 27, 2016, 10:10 p.m.)
> 
> 
> Review request for mesos, Adam B, Artem Harutyunyan, Jie Yu, and Kapil Arya.
> 
> 
> Bugs: MESOS-5350
> https://issues.apache.org/jira/browse/MESOS-5350
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Modifies the code path for docker executors.
> 
> Docker command executors are now launched with an additional flag
> that is filled in by a hook.  The --task_environment flag tells the
> command executor to pass some specified mapping of environment
> variables to the task.
> 
> Custom executors are launched with the environment variables directly.
> It is up to custom executors to pass these variables into tasks.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/docker.hpp fac26a239e9981e5b1c2f2f90b52785819492c2a 
>   src/slave/containerizer/docker.cpp 7af14f4472283ceefd73c06dd8df60af4cf6f7e8 
> 
> Diff: https://reviews.apache.org/r/47216/diff/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Re: Review Request 47216: Wired up the new docker environment hook.

2016-05-27 Thread Mesos ReviewBot 

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



Patch looks great!

Reviews applied: [47149, 47205, 47212, 47215, 47150, 47216]

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 May 27, 2016, 10:10 p.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47216/
> ---
> 
> (Updated May 27, 2016, 10:10 p.m.)
> 
> 
> Review request for mesos, Adam B, Artem Harutyunyan, Jie Yu, and Kapil Arya.
> 
> 
> Bugs: MESOS-5350
> https://issues.apache.org/jira/browse/MESOS-5350
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Modifies the code path for docker executors.
> 
> Docker command executors are now launched with an additional flag
> that is filled in by a hook.  The --task_environment flag tells the
> command executor to pass some specified mapping of environment
> variables to the task.
> 
> Custom executors are launched with the environment variables directly.
> It is up to custom executors to pass these variables into tasks.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/docker.hpp fac26a239e9981e5b1c2f2f90b52785819492c2a 
>   src/slave/containerizer/docker.cpp 7af14f4472283ceefd73c06dd8df60af4cf6f7e8 
> 
> Diff: https://reviews.apache.org/r/47216/diff/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Re: Review Request 47216: Wired up the new docker environment hook.

2016-05-27 Thread Joseph Wu 

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

(Updated May 27, 2016, 3:10 p.m.)


Review request for mesos, Adam B, Artem Harutyunyan, Jie Yu, and Kapil Arya.


Changes
---

Added a modicum of support for custom executors.


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


Repository: mesos


Description (updated)
---

Modifies the code path for docker executors.

Docker command executors are now launched with an additional flag
that is filled in by a hook.  The --task_environment flag tells the
command executor to pass some specified mapping of environment
variables to the task.

Custom executors are launched with the environment variables directly.
It is up to custom executors to pass these variables into tasks.


Diffs (updated)
-

  src/slave/containerizer/docker.hpp fac26a239e9981e5b1c2f2f90b52785819492c2a 
  src/slave/containerizer/docker.cpp 7af14f4472283ceefd73c06dd8df60af4cf6f7e8 

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


Testing
---

sudo make check


Thanks,

Joseph Wu

Re: Review Request 47216: Wired up the new docker environment hook.

2016-05-26 Thread Jie Yu 

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




src/slave/containerizer/docker.cpp (lines 1055 - 1057)


Hum, so we don't support custom executor? what's the expection of this hook 
for custom executor? Will the environment variables returned here be included 
in the custome executor's environment?


- Jie Yu


On May 26, 2016, 2:21 a.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47216/
> ---
> 
> (Updated May 26, 2016, 2:21 a.m.)
> 
> 
> Review request for mesos, Adam B, Artem Harutyunyan, Jie Yu, and Kapil Arya.
> 
> 
> Bugs: MESOS-5350
> https://issues.apache.org/jira/browse/MESOS-5350
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Modifies the code path for docker command executors. (Custom 
> executors are not supported because they may break if exposed to an
> unfamiliar flag.)
> 
> Docker command executors are now launched with an additional flag
> that is filled in by a hook.  The --task_environment flag tells the
> command executor to pass some specified mapping of environment
> variables to the task.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/docker.hpp fac26a239e9981e5b1c2f2f90b52785819492c2a 
>   src/slave/containerizer/docker.cpp 7af14f4472283ceefd73c06dd8df60af4cf6f7e8 
> 
> Diff: https://reviews.apache.org/r/47216/diff/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Re: Review Request 47216: Wired up the new docker environment hook.

2016-05-25 Thread Mesos ReviewBot 

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



Patch looks great!

Reviews applied: [47149, 47205, 47212, 47215, 47150, 47216]

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 May 26, 2016, 2:21 a.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47216/
> ---
> 
> (Updated May 26, 2016, 2:21 a.m.)
> 
> 
> Review request for mesos, Adam B, Artem Harutyunyan, Jie Yu, and Kapil Arya.
> 
> 
> Bugs: MESOS-5350
> https://issues.apache.org/jira/browse/MESOS-5350
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Modifies the code path for docker command executors. (Custom 
> executors are not supported because they may break if exposed to an
> unfamiliar flag.)
> 
> Docker command executors are now launched with an additional flag
> that is filled in by a hook.  The --task_environment flag tells the
> command executor to pass some specified mapping of environment
> variables to the task.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/docker.hpp fac26a239e9981e5b1c2f2f90b52785819492c2a 
>   src/slave/containerizer/docker.cpp 7af14f4472283ceefd73c06dd8df60af4cf6f7e8 
> 
> Diff: https://reviews.apache.org/r/47216/diff/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Re: Review Request 47216: Wired up the new docker environment hook.

2016-05-25 Thread Joseph Wu 

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

(Updated May 25, 2016, 7:21 p.m.)


Review request for mesos, Adam B, Artem Harutyunyan, Jie Yu, and Kapil Arya.


Changes
---

Rebase and kick off tests!


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


Repository: mesos


Description
---

Modifies the code path for docker command executors. (Custom 
executors are not supported because they may break if exposed to an
unfamiliar flag.)

Docker command executors are now launched with an additional flag
that is filled in by a hook.  The --task_environment flag tells the
command executor to pass some specified mapping of environment
variables to the task.


Diffs (updated)
-

  src/slave/containerizer/docker.hpp fac26a239e9981e5b1c2f2f90b52785819492c2a 
  src/slave/containerizer/docker.cpp 7af14f4472283ceefd73c06dd8df60af4cf6f7e8 

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


Testing
---

sudo make check


Thanks,

Joseph Wu

Re: Review Request 47216: Wired up the new docker environment hook.

2016-05-24 Thread Mesos ReviewBot 

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



Patch looks great!

Reviews applied: [47149, 47205, 47212, 47213, 47214, 47215, 47150, 47216]

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 May 24, 2016, 9:05 p.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47216/
> ---
> 
> (Updated May 24, 2016, 9:05 p.m.)
> 
> 
> Review request for mesos, Adam B, Artem Harutyunyan, Jie Yu, and Kapil Arya.
> 
> 
> Bugs: MESOS-5350
> https://issues.apache.org/jira/browse/MESOS-5350
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Modifies the code path for docker command executors. (Custom 
> executors are not supported because they may break if exposed to an
> unfamiliar flag.)
> 
> Docker command executors are now launched with an additional flag
> that is filled in by a hook.  The --task_environment flag tells the
> command executor to pass some specified mapping of environment
> variables to the task.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/docker.hpp fac26a239e9981e5b1c2f2f90b52785819492c2a 
>   src/slave/containerizer/docker.cpp 7af14f4472283ceefd73c06dd8df60af4cf6f7e8 
> 
> Diff: https://reviews.apache.org/r/47216/diff/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Re: Review Request 47216: Wired up the new docker environment hook.

2016-05-24 Thread Joseph Wu 

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

(Updated May 24, 2016, 2:05 p.m.)


Review request for mesos, Adam B, Artem Harutyunyan, Jie Yu, and Kapil Arya.


Changes
---

Spacing tweak.


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


Repository: mesos


Description
---

Modifies the code path for docker command executors. (Custom 
executors are not supported because they may break if exposed to an
unfamiliar flag.)

Docker command executors are now launched with an additional flag
that is filled in by a hook.  The --task_environment flag tells the
command executor to pass some specified mapping of environment
variables to the task.


Diffs (updated)
-

  src/slave/containerizer/docker.hpp fac26a239e9981e5b1c2f2f90b52785819492c2a 
  src/slave/containerizer/docker.cpp 7af14f4472283ceefd73c06dd8df60af4cf6f7e8 

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


Testing
---

sudo make check


Thanks,

Joseph Wu

Re: Review Request 47216: Wired up the new docker environment hook.

2016-05-23 Thread Kapil Arya 

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




src/slave/containerizer/docker.cpp (line 1042)


Can we bring the RHS on the next line and adjust the rest to make it 
slightly more readable?


- Kapil Arya


On May 11, 2016, 3:30 p.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47216/
> ---
> 
> (Updated May 11, 2016, 3:30 p.m.)
> 
> 
> Review request for mesos, Adam B, Artem Harutyunyan, Jie Yu, and Kapil Arya.
> 
> 
> Bugs: MESOS-5350
> https://issues.apache.org/jira/browse/MESOS-5350
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Modifies the code path for docker command executors. (Custom 
> executors are not supported because they may break if exposed to an
> unfamiliar flag.)
> 
> Docker command executors are now launched with an additional flag
> that is filled in by a hook.  The --task_environment flag tells the
> command executor to pass some specified mapping of environment
> variables to the task.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/docker.hpp fac26a239e9981e5b1c2f2f90b52785819492c2a 
>   src/slave/containerizer/docker.cpp 7af14f4472283ceefd73c06dd8df60af4cf6f7e8 
> 
> Diff: https://reviews.apache.org/r/47216/diff/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Re: Review Request 47216: Wired up the new docker environment hook.

2016-05-11 Thread Joseph Wu 

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

(Updated May 11, 2016, 12:30 p.m.)


Review request for mesos, Adam B, Artem Harutyunyan, Jie Yu, and Kapil Arya.


Changes
---

Fixed a bug earlier in the chain.  Now fully ready for review.


Summary (updated)
-

Wired up the new docker environment hook.


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


Repository: mesos


Description (updated)
---

Modifies the code path for docker command executors. (Custom 
executors are not supported because they may break if exposed to an
unfamiliar flag.)

Docker command executors are now launched with an additional flag
that is filled in by a hook.  The --task_environment flag tells the
command executor to pass some specified mapping of environment
variables to the task.


Diffs
-

  src/slave/containerizer/docker.hpp fac26a239e9981e5b1c2f2f90b52785819492c2a 
  src/slave/containerizer/docker.cpp 7af14f4472283ceefd73c06dd8df60af4cf6f7e8 

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


Testing (updated)
---

sudo make check


Thanks,

Joseph Wu