Re: Review Request 44186: Added authentication to master endpoints.

[Adam B]

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


Ship it!




Ship It!

- Adam B


On March 11, 2016, 11:12 a.m., Joerg Schad wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44186/
> ---
> 
> (Updated March 11, 2016, 11:12 a.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rojas, and Greg Mann.
> 
> 
> Bugs: MESOS-4844
> https://issues.apache.org/jira/browse/MESOS-4844
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added Authentication to master http endpoints (except version, health, 
> redirect, scheduler).
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp 54a2569ff5b10388177a9bd29c6ddc0387e5e8fd 
>   src/master/master.hpp 4fa88f1968c92c1216c0af22a476ef7aa57ae961 
>   src/master/master.cpp 249e82ffcef35aa8df3c5b9faef5b9b25d68facc 
>   src/tests/containerizer/docker_containerizer_tests.cpp 
> 6aecd912fc84b72d2b64f7a842891fddcbc469ac 
>   src/tests/fault_tolerance_tests.cpp 
> d193897e636efd0e3ef67bf67fcd6255a3de0341 
>   src/tests/health_check_tests.cpp d0fd27fd8a6b48511ef8cafab5dff59f65729d9f 
>   src/tests/master_maintenance_tests.cpp 
> 3c7024cfbd7e5bef75f092eace8d0e8ca423 
>   src/tests/master_tests.cpp 2f4d820e223a48700ce1ac3a91b7256cc836c268 
>   src/tests/persistent_volume_endpoints_tests.cpp 
> 81185a161498394020a27f1f5bf747bac5425f43 
>   src/tests/repair_tests.cpp bb104562659e135492f9857e5b452c8a0a9e97da 
>   src/tests/role_tests.cpp fc3a72894631279460ee7971a4627d73c3d8c351 
>   src/tests/status_update_manager_tests.cpp 
> d64d3b8c96270478f6b681c038de77c3a9eb68fe 
> 
> Diff: https://reviews.apache.org/r/44186/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Joerg Schad
> 
>

Review Request 44760: Windows: Fixed non-blocking connect.

[Daniel Pravat]

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

Review request for mesos, Alex Naparu, Daniel Pravat, Artem Harutyunyan, Joris 
Van Remoortere, Michael Park, M Lawindi, and Yi Sun.


Repository: mesos


Description
---

Windows: Fixed non-blocking connect.


Diffs
-

  3rdparty/libprocess/include/process/network.hpp 
7d203f0ff1cdb3145bc2b914f8bd606203878f09 
  3rdparty/libprocess/src/poll_socket.cpp 
6e6634b4b352e3723096521843546cf56ec6dd8b 

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


Testing
---

OSX: make check
Windows: build/run


Thanks,

Daniel Pravat

Re: Review Request 44089: Windows: Used os::read/write from Stout for proper OS isolation.

[Daniel Pravat]

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

(Updated March 13, 2016, 7:28 a.m.)


Review request for mesos, Alex Naparu, Daniel Pravat, Artem Harutyunyan, Joris 
Van Remoortere, Michael Park, M Lawindi, and Yi Sun.


Repository: mesos


Description
---

Windows: Used os::read/write from Stout for proper OS isolation.


Diffs (updated)
-

  3rdparty/libprocess/src/io.cpp 4a58d6dff3787ca262ee2ade2cbea7578ad27e95 

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


Testing (updated)
---

OSX: make check
Windows: build/run


Thanks,

Daniel Pravat

Re: Review Request 44090: Windows: Changed the calling parameters for Windows API.

[Daniel Pravat]

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

(Updated March 13, 2016, 7:27 a.m.)


Review request for mesos, Alex Naparu, Daniel Pravat, Artem Harutyunyan, Joris 
Van Remoortere, Michael Park, M Lawindi, and Yi Sun.


Summary (updated)
-

Windows: Changed the calling parameters for Windows API.


Repository: mesos


Description (updated)
---

Windows: Changed the calling parameters for Windows API.


Diffs (updated)
-

  3rdparty/libprocess/src/libevent.cpp c4a8da8a70b97dd575b1256179c4f43742131a1e 
  3rdparty/libprocess/src/poll_socket.cpp 
6e6634b4b352e3723096521843546cf56ec6dd8b 

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


Testing (updated)
---

OSX: make check
Windows: build/run


Thanks,

Daniel Pravat

Re: Review Request 44570: Added an owner field to TaskInfo and ExecutorInfo.

[Adam B]

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



Sorry for the misunderstanding. As far as (sandbox) access-control is 
concerned, we can only isolate containers from each other, so each container 
can only have a single owner. With the default executor, the TaskInfo is 1:1 
with the container. But with a custom executor, the ExecutorInfo is 1:1 with 
the container.


include/mesos/mesos.proto (lines 440 - 442)


Owner of the executor...



src/tests/master_validation_tests.cpp (lines 1184 - 1185)


Not true. If an executorInfo is specified, then TaskInfo.owner is 
irrelevant, since the executor is the unit of containerization, ownership, and 
access control.
The only reason we need a TaskInfo.owner is because there is no 
ExecutorInfo for the default command executor. Same holds true for CommandInfo.


- Adam B


On March 10, 2016, 4:49 a.m., Jan Schlicht wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44570/
> ---
> 
> (Updated March 10, 2016, 4:49 a.m.)
> 
> 
> Review request for mesos, Adam B and Joerg Schad.
> 
> 
> Bugs: MESOS-4772
> https://issues.apache.org/jira/browse/MESOS-4772
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> To be able to authenticate HTTP requests for tasks, the authorizer has to
> determine who owns the tasks. This is achieved by adding an owner field to
> TaskInfo and ExecutorInfo.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 3d22ec32655dca741169e1f0e382303e061c38b7 
>   include/mesos/v1/mesos.proto 31960a52061f70d80528fb8326522ae1d6f75b2c 
>   src/master/validation.hpp 29dbdf1da540db4966ff9e86037badde06e69a4b 
>   src/master/validation.cpp 820a9faee6fde09a98317854b181fe897167e2ca 
>   src/tests/master_validation_tests.cpp 
> c9bc38ce604d2d44d6e6b1286507d1c45e5e9e25 
> 
> Diff: https://reviews.apache.org/r/44570/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jan Schlicht
> 
>

Re: Review Request 44570: Added an owner field to TaskInfo and ExecutorInfo.

[Adam B]

> On March 9, 2016, 10:31 p.m., Adam B wrote:
> > src/master/validation.cpp, line 381
> > 
> >
> > It may be sufficient to only check `if (task.has_owner() && 
> > task.has_executor())` since a custom executor should set the owner on the 
> > ExecutorInfo not the TaskInfo.
> 
> Jan Schlicht wrote:
> That is if the `ExecutorInfo` is reused in a `TaskInfo` it's sufficient 
> to check for that? That makes sense because the `ExecutorInfo` validation 
> compares the `ExecutorId` against the ones of existing `ExecutorInfo`s. But 
> how'd we make sure that the owner is set in `ExecutorInfo` if that executor 
> isn't existing yet?

Well, we can't enforce that the owner field has to be set, since that would 
break backwards compatibility. It's an optional field, so even if a custom 
ExecutorInfo is provided, the owner field may not be set. Here's what's allowed:
a) !task.has_executor() && (task.has_owner() || !task.has_owner()) // Default 
executor, may or may not have owner set.
b) task.has_executor() && !task.has_owner() && (task.executor().has_owner() || 
!task.executor().has_owner()) // Custom executor, may or may not have owner 
set; taskInfo does not have an owner set.
Here's what's not allowed:
x) task.has_executor() && task.has_owner() && task.executor().has_owner() // 
How can the task have a different owner than the executor?
y) task.has_executor() && task.has_owner() && !task.executor().has_owner() // 
Why would the task have an owner but the executor doesn't?


- Adam


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


On March 10, 2016, 4:49 a.m., Jan Schlicht wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44570/
> ---
> 
> (Updated March 10, 2016, 4:49 a.m.)
> 
> 
> Review request for mesos, Adam B and Joerg Schad.
> 
> 
> Bugs: MESOS-4772
> https://issues.apache.org/jira/browse/MESOS-4772
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> To be able to authenticate HTTP requests for tasks, the authorizer has to
> determine who owns the tasks. This is achieved by adding an owner field to
> TaskInfo and ExecutorInfo.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 3d22ec32655dca741169e1f0e382303e061c38b7 
>   include/mesos/v1/mesos.proto 31960a52061f70d80528fb8326522ae1d6f75b2c 
>   src/master/validation.hpp 29dbdf1da540db4966ff9e86037badde06e69a4b 
>   src/master/validation.cpp 820a9faee6fde09a98317854b181fe897167e2ca 
>   src/tests/master_validation_tests.cpp 
> c9bc38ce604d2d44d6e6b1286507d1c45e5e9e25 
> 
> Diff: https://reviews.apache.org/r/44570/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jan Schlicht
> 
>

Re: Review Request 44758: Upgrade to clang-format-3.8 (MESOS-4906).

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [44758]

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

- Mesos ReviewBot


On March 13, 2016, 4:15 a.m., Yong Tang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44758/
> ---
> 
> (Updated March 13, 2016, 4:15 a.m.)
> 
> 
> Review request for mesos and Michael Park.
> 
> 
> Bugs: MESOS-4906
> https://issues.apache.org/jira/browse/MESOS-4906
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Upgrade to clang-format-3.8 (MESOS-4906).
> 
> 
> Diffs
> -
> 
>   docs/clang-format.md 7f1c1dfd70e1fe9bfa186df1bdda7bdcf867db04 
>   support/clang-format 499d0e749e14e50256ae649afa0ced2b04589a0e 
> 
> Diff: https://reviews.apache.org/r/44758/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Yong Tang
> 
>

Re: Review Request 44672: Added normalize method to registry puller.

[haosdent huang]

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


Ship it!




Ship It!

- haosdent huang


On March 10, 2016, 11:49 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44672/
> ---
> 
> (Updated March 10, 2016, 11:49 p.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Jie Yu, and Timothy Chen.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added normalize method to registry puller.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp 
> 6d637ed14f35feb554c8fcc63a7a7e046aaca574 
> 
> Diff: https://reviews.apache.org/r/44672/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> sudo ./bin/mesos-test.sh 
> --gtest_filter="*ProvisionerDockerRegistryPullerTest*"
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 44439: Added device support in cgroups abstraction.

[Kevin Klues]

> On March 9, 2016, 1:58 a.m., Ben Mahler wrote:
> > src/linux/cgroups.hpp, lines 635-654
> > 
> >
> > The naming convention in this file is to mirror the cgroups controls, 
> > so these would be:
> > 
> > ```
> > cgroups::devices::allow(...)
> > cgroups::devices::deny(...)
> > cgroups::devices::list(...)
> > ```
> > 
> > The other reason we've added these helpers is to provide better types 
> > to make the code more readable, notice how the memory controls and cpu 
> > controls in this file use Bytes and Duration, respectively.
> > 
> > Here, I think we need to do something a bit more involved. To be more 
> > specific, if I read the caller code:
> > 
> > ```
> > cgroups::devices::allow(
> > hierarchy,
> > cgroup, 
> > "c 1:3 rm");
> > ```
> > 
> > It's not easy for the reader to understand what `"c 1:3 rm"` means. So 
> > adding types here would help, for example:
> > 
> > ```
> > // This is /dev/null.
> > cgroups::device::Selector selector;
> > devices.type = CHARACTER;
> > devices.major = 1;
> > devices.minor = ALL;
> > 
> > cgroups::device::Access access;
> > access.read = true;
> > access.write = true;
> > access.mknod = true;
> > 
> > Try allow = cgroups::devices::allow(
> > hierarchy,
> > cgroup,
> > selector,
> > access);
> > ```
> > 
> > For cgroups::devices::list, Kevin and I played around and weren't able 
> > to see the contents of the file change at all, did you have any luck 
> > getting different results out of the devices.list file?
> 
> Abhishek Dasgupta wrote:
> So, here you are suggesting to parse deviceEntry like "c 1:3 rm" into say 
> Selector class and use the Selector object as argument to 
> cgroups::devices::allow. Right?

The idea is to represent that string as two objects (Selector and Access) 
instead of just as a string.  When actually writing to the allow and deny 
files, you can construct the string from these objects.  When reading from the 
the list file to perform a check, you will need to parse the  string and 
compare it against the Selector / Access objects that are passed in.


- Kevin


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


On March 7, 2016, 6:27 a.m., Abhishek Dasgupta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44439/
> ---
> 
> (Updated March 7, 2016, 6:27 a.m.)
> 
> 
> Review request for mesos, Ben Mahler, Felix Abecassis, Kevin Klues, and 
> Niklas Nielsen.
> 
> 
> Bugs: MESOS-3368
> https://issues.apache.org/jira/browse/MESOS-3368
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> There are some helper methods added to support device cgroups in cgroups 
> abstraction to aid isolators controlling access to devices.
> 
> 
> Diffs
> -
> 
>   src/linux/cgroups.hpp 51ccefd1fd2d0989b9bd31342d3d1e3701f88ed2 
>   src/linux/cgroups.cpp df18ed46a2a96871f67c7eb4233c3b4c27b7aa1c 
>   src/tests/containerizer/cgroups_tests.cpp 
> acaed9b3f8a04964092cef413133834d0cf5a145 
> 
> Diff: https://reviews.apache.org/r/44439/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Abhishek Dasgupta
> 
>

Review Request 44758: Upgrade to clang-format-3.8 (MESOS-4906).

[Yong Tang]

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

Review request for mesos and Michael Park.


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


Repository: mesos


Description
---

Upgrade to clang-format-3.8 (MESOS-4906).


Diffs
-

  docs/clang-format.md 7f1c1dfd70e1fe9bfa186df1bdda7bdcf867db04 
  support/clang-format 499d0e749e14e50256ae649afa0ced2b04589a0e 

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


Testing
---

make check


Thanks,

Yong Tang

Re: Review Request 44719: Avoided external linkage for slave constants.

[Neil Conway]

> On March 12, 2016, 2:28 a.m., Ben Mahler wrote:
> > src/slave/constants.hpp, lines 116-118
> > 
> >
> > Can this be in-line or is there a circular dependency issue that 
> > warrants the .cpp file?

This can be in the header file, but it would require having the slave's 
`constants.hpp` include the master's `constants.hpp`, which seemed like 
something we'd want to avoid. Happy to make this change if you think it isn't a 
problem though.


- Neil


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


On March 11, 2016, 6:32 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44719/
> ---
> 
> (Updated March 11, 2016, 6:32 p.m.)
> 
> 
> Review request for mesos and Ben Mahler.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Avoided external linkage for slave constants.
> 
> 
> Diffs
> -
> 
>   src/slave/constants.hpp bcbb1401aa8f9f04c4f9256bb4f560e18d8994e0 
>   src/slave/constants.cpp 0f0d8e4b079d136d250f83cfc768de8c98b8bee2 
> 
> Diff: https://reviews.apache.org/r/44719/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 44191: Avoided external linkage for master constants.

[Neil Conway]

> On March 12, 2016, 2:21 a.m., Ben Mahler wrote:
> > src/master/constants.hpp, lines 127-128
> > 
> >
> > Any reason this Duration constant wasn't changed to constexpr?

This was an oversight on my part. Thanks for catching!


- Neil


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


On March 11, 2016, 6:31 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44191/
> ---
> 
> (Updated March 11, 2016, 6:31 p.m.)
> 
> 
> Review request for mesos and Ben Mahler.
> 
> 
> Bugs: MESOS-4790
> https://issues.apache.org/jira/browse/MESOS-4790
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> We were only using external linkage to workaround bugs in ancient
> (4.1.x) of GCC. We can also mark most of these constants `constexpr`,
> which should be both slightly more efficient (in theory) and
> safer (see MESOS-1023).
> 
> 
> Diffs
> -
> 
>   src/CMakeLists.txt e9f7c3a447aad79ab19d53879888a413a587408f 
>   src/Makefile.am b24f0f58fa188c16770fe6a3c23ec06262cb0955 
>   src/master/constants.hpp 2c3299bc1aaa0888f7e47a71965c56ada8ecc21f 
>   src/master/constants.cpp e316f9772d880b694faeee6d001dc56bc088c118 
> 
> Diff: https://reviews.apache.org/r/44191/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> Note that I left a few string constants as `const std::string`; you might 
> arguably want them to be `constexpr char[]` (per Google Style Guide).
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 44456: Added Appc provisioner integration test.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [44755, 44456]

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

- Mesos ReviewBot


On March 12, 2016, 5:14 p.m., Jojy Varghese wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44456/
> ---
> 
> (Updated March 12, 2016, 5:14 p.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-4818
> https://issues.apache.org/jira/browse/MESOS-4818
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added Appc provisioner integration test.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/provisioner_appc_tests.cpp 
> 069af8a0b5e031d9032fd13154cee250f279b404 
> 
> Diff: https://reviews.apache.org/r/44456/diff/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Jojy Varghese
> 
>

Re: Review Request 44748: Stout: Added implementation of `read` that works on Windows.

[Alex Clemmer]

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

(Updated March 12, 2016, 6:14 p.m.)


Review request for mesos, Alex Naparu, Daniel Pravat, Artem Harutyunyan, Joris 
Van Remoortere, Michael Park, M Lawindi, and Yi Sun.


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


Repository: mesos


Description
---

Stout: Added implementation of `read` that works on Windows.


Diffs (updated)
-

  3rdparty/libprocess/3rdparty/stout/include/Makefile.am 
7bd4bfbc2ec5922879dcefddc12137336b11be52 
  3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/read.hpp 
PRE-CREATION 
  3rdparty/libprocess/3rdparty/stout/include/stout/os/read.hpp 
d494cbf8a2a24c88b0569634cfcbf29de0784797 
  3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/read.hpp 
PRE-CREATION 

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


Testing
---


Thanks,

Alex Clemmer

Re: Review Request 44747: Stout: Added implementation of `write` that works on Windows.

[Alex Clemmer]

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

(Updated March 12, 2016, 6:14 p.m.)


Review request for mesos, Alex Naparu, Daniel Pravat, Artem Harutyunyan, Joris 
Van Remoortere, Michael Park, M Lawindi, and Yi Sun.


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


Repository: mesos


Description
---

Stout: Added implementation of `write` that works on Windows.


Diffs (updated)
-

  3rdparty/libprocess/3rdparty/stout/include/Makefile.am 
7bd4bfbc2ec5922879dcefddc12137336b11be52 
  3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/write.hpp 
PRE-CREATION 
  3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/stat.hpp 
a2a5d0b7af673fee89f66e2722846af7b7fd059a 
  3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/write.hpp 
PRE-CREATION 
  3rdparty/libprocess/3rdparty/stout/include/stout/os/write.hpp 
1715bf5e6c590e7d14f59d517b30a281346365be 

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


Testing
---


Thanks,

Alex Clemmer

Re: Review Request 44456: Added Appc provisioner integration test.

[Jojy Varghese]

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

(Updated March 12, 2016, 5:14 p.m.)


Review request for mesos and Jie Yu.


Changes
---

review addressed.


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


Repository: mesos


Description
---

Added Appc provisioner integration test.


Diffs (updated)
-

  src/tests/containerizer/provisioner_appc_tests.cpp 
069af8a0b5e031d9032fd13154cee250f279b404 

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


Testing
---

make check.


Thanks,

Jojy Varghese

Review Request 44755: Added getAppcImage for Appc provisioning tests.

[Jojy Varghese]

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

Review request for mesos and Jie Yu.


Repository: mesos


Description
---

Added helper function to get an Appc image protobuf object with given
parameters.


Diffs
-

  src/tests/containerizer/provisioner_appc_tests.cpp 
069af8a0b5e031d9032fd13154cee250f279b404 

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


Testing
---

make check.


Thanks,

Jojy Varghese

Re: Review Request 44555: Implemented the framework and create() method of "network/cni" isolator.

[Qian Zhang]

> On March 12, 2016, 3:28 a.m., Jie Yu wrote:
> > src/slave/containerizer/mesos/isolators/network/cni.cpp, line 48
> > 
> >
> > Let's remove this 'empty' check here since you hae the os::exists check 
> > below.

They are for different purposes, `os::exist` is used to check if the directory 
exists or not, and here `flags.network_cni_plugins_dir.get().empty()` is used 
to check if operator does not specify the value of `--network_cni_plugins_dir` 
(e.g., "`--network_cni_plugins_dir=`") when starting agent.


> On March 12, 2016, 3:28 a.m., Jie Yu wrote:
> > src/slave/containerizer/mesos/isolators/network/cni.cpp, line 53
> > 
> >
> > Ditto on removing this check.

Ditto.


> On March 12, 2016, 3:28 a.m., Jie Yu wrote:
> > src/slave/containerizer/mesos/isolators/network/cni.cpp, lines 73-77
> > 
> >
> > I think the CNI plugin should also support the case where the user does 
> > not specify a name for the network. In other words,  the CNI network 
> > isolator should support host network as well. So it's OK to have no plugins.

I have two questions:
1. How are we going to define the design behavior of supporting host network in 
CNI network isolator? I mean if user does not specify name in NetworkInfo when 
launching task, what should CNI network isolator do in `prepare()` and 
`isolate()`? And what if user even does not specify the whole NetworkInfo in 
ContainerInfo? My thinking is, CNI network isolator should not do anything in 
this case.
2. In the code below, I check whether the plugin binary specified in each 
network config file exists under `--network_cni_plugins` or not, if here we 
allow the directory specified by `--network_cni_plugins` to be empty, then I 
should remove that check too, right? But that way, we can not bail out agent in 
`create()` to let operator know the issue which will be left to launching task.


- Qian


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


On March 9, 2016, 2:01 p.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44555/
> ---
> 
> (Updated March 9, 2016, 2:01 p.m.)
> 
> 
> Review request for mesos, Avinash sridharan, Gilbert Song, and Jie Yu.
> 
> 
> Bugs: MESOS-4759
> https://issues.apache.org/jira/browse/MESOS-4759
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Implemented the framework and create() method of "network/cni" isolator.
> 
> 
> Diffs
> -
> 
>   src/CMakeLists.txt 8f57a5701073bf1eaaa223383e928cf5db8f8ae4 
>   src/Makefile.am a41e95ddeb838fdebf4ced953c4a29181916e261 
>   src/slave/containerizer/mesos/isolators/network/cni.hpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/network/cni.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/44555/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>