Re: Review Request 51871: Modified the `isolate` method to be nesting aware.

2016-09-17 Thread Avinash sridharan

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

(Updated Sept. 18, 2016, 5:40 a.m.)


Review request for mesos, Gilbert Song, Jie Yu, Joseph Wu, and Qian Zhang.


Changes
---

Rebased, in order to use `getRootContainerDir`.


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


Repository: mesos


Description
---

The network file setup in the `network/cni` isolator is now nesting
aware. Since the children share the network and UTS namespace with the
parent, the network files need to be created only for the parent
container. For the child containers, the network files will be simply
a symlink to a parents network files.


Diffs (updated)
-

  src/slave/containerizer/mesos/isolators/network/cni/cni.cpp 
359479083894e887647a694a1a133dce44817073 

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


Testing
---

make
make check
sudo ./bin/mesos-tests.sh


Thanks,

Avinash sridharan



Re: Review Request 51978: Refactored commandScheduler in cli/execute.cpp to take TaskInfo.

2016-09-17 Thread Abhishek Dasgupta

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

(Updated Sept. 18, 2016, 5:16 a.m.)


Review request for mesos and Vinod Kone.


Changes
---

Rebased.


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


Repository: mesos


Description (updated)
---

Refactored commandScheduler in cli/execute.cpp to take TaskInfo.


Diffs (updated)
-

  src/cli/execute.cpp 525c89803ad1b29328420c5925a3e6045e487645 

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


Testing
---

Manually Ran:
mesos-execute --master=127.0.0.1:5050 --name="Hello1" --command="echo hello" 
--docker_image="ubuntu" --containerizer="docker"


Thanks,

Abhishek Dasgupta



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

2016-09-17 Thread Guangya Liu

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




src/cli/execute.cpp (line 270)


s/taskgroup/taskGroup



src/cli/execute.cpp (line 302)


s/_taskgroup/_taskGroup


- Guangya Liu


On 九月 17, 2016, 8:56 p.m., Abhishek Dasgupta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51623/
> ---
> 
> (Updated 九月 17, 2016, 8:56 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
> ---
> 
> Updated mesos-execute to support task groups.
> 
> 
> 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
> 
> 
> Thanks,
> 
> Abhishek Dasgupta
> 
>



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

2016-09-17 Thread Guangya Liu


> On 九月 18, 2016, 1:33 a.m., Guangya Liu wrote:
> >

Can you please also append your file:///home/abhishek/taskgroup.txt to the 
`Testing Done` section?


- Guangya


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


On 九月 17, 2016, 8:56 p.m., Abhishek Dasgupta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51623/
> ---
> 
> (Updated 九月 17, 2016, 8:56 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
> ---
> 
> Updated mesos-execute to support task groups.
> 
> 
> 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
> 
> 
> Thanks,
> 
> Abhishek Dasgupta
> 
>



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

2016-09-17 Thread Guangya Liu

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




src/cli/execute.cpp (lines 73 - 74)


adjust the order here



src/cli/execute.cpp (lines 389 - 395)


How about:

```
CHECK_NE(task.isSome(), taskGroup.isSome())
  << "Either task or task group should be set but not both";

vector tasks;
if (task.isSome()) {
  requiredResources = Resources(task.get().resources());
} else {
  foreach (const TaskInfo& _task, taskgroup.get().tasks()) {
requiredResources += Resources(_task.resources());
  }
}
```

Ditto for others



src/cli/execute.cpp (lines 406 - 409)


I'd like a `CHECK_SOME` here for flatten.

```
// Takes resources first from the specified role, then from '*'.
Try flattened =
  Resources(task.get().resources()).flatten(frameworkInfo.role());

// `frameworkInfo.role()` must be valid as it's allowed to register.
CHECK_SOME(flattened);
Option resources = offered.find(flattened.get());
```



src/cli/execute.cpp (line 414)


This can be updated to `else {}` if you `CHECK_NE` above.

Ditto for others.



src/cli/execute.cpp (lines 419 - 422)


I'd like a `CHECK_SOME` here for flatten.

```
// Takes resources first from the specified role, then from '*'.
Try flattened =
  Resources(_task.resources()).flatten(frameworkInfo.role());

// `frameworkInfo.role()` must be valid as it's allowed to register.
CHECK_SOME(flattened);
Option resources = offered.find(flattened.get());
```



src/cli/execute.cpp (line 444)


s/if(/if (



src/cli/execute.cpp (line 452)


4 spaces



src/cli/execute.cpp (line 456)


4 spaces



src/cli/execute.cpp (line 468)


s/taskgroup/task group



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


How about

```
Submitted task group with tasks [task1, task2] to agent 'agetname'
```



src/cli/execute.cpp (line 547)


break here?



src/cli/execute.cpp (lines 771 - 777)


Move this to line 758 to fail fast.



src/cli/execute.cpp (line 774)


s/Either of task or taskgroup can be run at one time/Either task or task 
group should be set but not both



src/cli/execute.cpp (line 929)


s/Option /Option


- Guangya Liu


On 九月 17, 2016, 8:56 p.m., Abhishek Dasgupta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51623/
> ---
> 
> (Updated 九月 17, 2016, 8:56 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
> ---
> 
> Updated mesos-execute to support task groups.
> 
> 
> 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
> 
> 
> Thanks,
> 
> Abhishek Dasgupta
> 
>



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

2016-09-17 Thread Mesos ReviewBot

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



Bad patch!

Reviews applied: [51623, 51991, 51990, 51978]

Failed command: ./support/apply-review.sh -n -r 51978

Error:
2016-09-18 00:26:46 URL:https://reviews.apache.org/r/51978/diff/raw/ 
[13606/13606] -> "51978.patch" [1]
error: patch failed: src/cli/execute.cpp:355
error: src/cli/execute.cpp: patch does not apply

Full log: https://builds.apache.org/job/mesos-reviewbot/15295/console

- Mesos ReviewBot


On Sept. 17, 2016, 8:56 p.m., Abhishek Dasgupta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51623/
> ---
> 
> (Updated Sept. 17, 2016, 8:56 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
> ---
> 
> Updated mesos-execute to support task groups.
> 
> 
> 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
> 
> 
> Thanks,
> 
> Abhishek Dasgupta
> 
>



Re: Review Request 51978: Refactored commandScheduler in cli/execute.cpp to take TaskInfo.

2016-09-17 Thread Guangya Liu

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



A rebase is needed.


src/cli/execute.cpp (line 512)


Since you are doing a refactor, it is better to also set this parameter as 
`const Option& volumes` as the `volumes` is also an option 
value. With updating here, you may also want to update Line 764 as well.


- Guangya Liu


On 九月 17, 2016, 4:23 p.m., Abhishek Dasgupta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51978/
> ---
> 
> (Updated 九月 17, 2016, 4:23 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-6096
> https://issues.apache.org/jira/browse/MESOS-6096
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This refactoring was needed for better accomodating pod support
> for mesos-execute.
> 
> 
> Diffs
> -
> 
>   src/cli/execute.cpp c9f56af7f37d5b79b51f350d6c533714c170e889 
> 
> Diff: https://reviews.apache.org/r/51978/diff/
> 
> 
> Testing
> ---
> 
> Manually Ran:
> mesos-execute --master=127.0.0.1:5050 --name="Hello1" --command="echo hello" 
> --docker_image="ubuntu" --containerizer="docker"
> 
> 
> Thanks,
> 
> Abhishek Dasgupta
> 
>



Re: Review Request 51985: Waited for agent finish registering in test case.

2016-09-17 Thread Mesos ReviewBot

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



Patch looks great!

Reviews applied: [51985]

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. 17, 2016, 3:41 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51985/
> ---
> 
> (Updated Sept. 17, 2016, 3:41 p.m.)
> 
> 
> Review request for mesos, Gilbert Song, Greg Mann, Jie Yu, Joseph Wu, and 
> Qian Zhang.
> 
> 
> Bugs: MESOS-6180
> https://issues.apache.org/jira/browse/MESOS-6180
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Waited for agent finish registering in test case.
> 
> 
> Diffs
> -
> 
>   src/tests/slave_recovery_tests.cpp 23cf9d8cd76d568fa841817d6f4279c4aa2fdc75 
> 
> Diff: https://reviews.apache.org/r/51985/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>



Re: Review Request 51992: Fixed a bug in getRootContainerId due to protobuf copying issue.

2016-09-17 Thread Kevin Klues

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


Ship it!




Ship It!

- Kevin Klues


On Sept. 17, 2016, 9:25 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51992/
> ---
> 
> (Updated Sept. 17, 2016, 9:25 p.m.)
> 
> 
> Review request for mesos, Gilbert Song and Kevin Klues.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> It looks like protobuf is not so great dealing with nesting messages
> when doing merge or copy. This patch uses an extra copy to bypass that
> issue in the protobuf.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/utils.hpp 
> 2bb55c19bcbfcdfff15930d0f9e437a502ce6624 
> 
> Diff: https://reviews.apache.org/r/51992/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>



Review Request 51992: Fixed a bug in getRootContainerId due to protobuf copying issue.

2016-09-17 Thread Jie Yu

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

Review request for mesos, Gilbert Song and Kevin Klues.


Repository: mesos


Description
---

It looks like protobuf is not so great dealing with nesting messages
when doing merge or copy. This patch uses an extra copy to bypass that
issue in the protobuf.


Diffs
-

  src/slave/containerizer/mesos/utils.hpp 
2bb55c19bcbfcdfff15930d0f9e437a502ce6624 

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


Testing
---

make check


Thanks,

Jie Yu



Re: Review Request 51984: Cleaned up initialization of atomic fields in ProcessManager.

2016-09-17 Thread Mesos ReviewBot

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



Patch looks great!

Reviews applied: [51984]

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. 17, 2016, 3:25 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51984/
> ---
> 
> (Updated Sept. 17, 2016, 3:25 p.m.)
> 
> 
> Review request for mesos and Joseph Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> `std::atomic` has a trivial default constructor, which means
> `std::atomic` values are left uninitialized by default. `ProcessManager`
> uses two atomic fields that are default-constructed; although we `store`
> to them before reading from them, it would be cleaner and safer to
> initialize them to a well-defined value in the first place.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/process.cpp 
> 1e48fd5269d1a94c2217e8826af54b9b42ec4b23 
> 
> Diff: https://reviews.apache.org/r/51984/diff/
> 
> 
> Testing
> ---
> 
> `make check` on OSX.
> 
> 
> Thanks,
> 
> Neil Conway
> 
>



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

2016-09-17 Thread Abhishek Dasgupta

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

(Updated Sept. 17, 2016, 8:56 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
---

Updated mesos-execute to support task groups.


Diffs (updated)
-

  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


Thanks,

Abhishek Dasgupta



Review Request 51991: Added parse function in flags namespace for v1::TaskGroupInfo protobuf.

2016-09-17 Thread Abhishek Dasgupta

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

Review request for mesos and Vinod Kone.


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


Repository: mesos


Description
---

Added parse function in flags namespace for v1::TaskGroupInfo protobuf.


Diffs
-

  src/common/parse.hpp 51582a4f8943296799d71d0a8d7812787fd1b2c2 

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


Testing
---


Thanks,

Abhishek Dasgupta



Review Request 51990: Added << operator to stringify v1::TaskGroupInfo protobuf.

2016-09-17 Thread Abhishek Dasgupta

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

Review request for mesos and Vinod Kone.


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


Repository: mesos


Description
---

Added << operator to stringify v1::TaskGroupInfo protobuf.


Diffs
-

  include/mesos/v1/mesos.hpp 936af77f95ccbdc333452a1946187a81222e480f 
  src/v1/mesos.cpp c6a8799648af488d11a48e0c8841b98313c7dff1 

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


Testing
---


Thanks,

Abhishek Dasgupta



Re: Review Request 51988: Updated 'cgroups' isolator 'watch' to be properly nested aware.

2016-09-17 Thread Jie Yu

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


Ship it!




Ship It!

- Jie Yu


On Sept. 17, 2016, 7:47 p.m., Kevin Klues wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51988/
> ---
> 
> (Updated Sept. 17, 2016, 7:47 p.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-6186
> https://issues.apache.org/jira/browse/MESOS-6186
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> In a previous iteration of attempting to make this isolator nested
> aware, we (incorrectly) decided to return a 'Failure()' in 'watch()'
> for nested containers. However, we shouldn't just blindly fail this
> call for nested containers. Instead we should just return a 'Future'
> that will never be satisfied. This way it is safe to call 'watch()'
> for nested containers, without any unwanted side effects.
> 
> This commit fixes this issue.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp 
> ed7150a9b53c577fc248f8af2be55c6b12f64303 
> 
> Diff: https://reviews.apache.org/r/51988/diff/
> 
> 
> Testing
> ---
> 
> $ GTEST_FILTER="" make -j check
> $ sudo src/mesos-tests
> 
> 
> Thanks,
> 
> Kevin Klues
> 
>



Re: Review Request 51988: Updated 'cgroups' isolator 'watch' to be properly nested aware.

2016-09-17 Thread Gilbert Song

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


Ship it!




Ship It!

- Gilbert Song


On Sept. 17, 2016, 12:47 p.m., Kevin Klues wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51988/
> ---
> 
> (Updated Sept. 17, 2016, 12:47 p.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-6186
> https://issues.apache.org/jira/browse/MESOS-6186
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> In a previous iteration of attempting to make this isolator nested
> aware, we (incorrectly) decided to return a 'Failure()' in 'watch()'
> for nested containers. However, we shouldn't just blindly fail this
> call for nested containers. Instead we should just return a 'Future'
> that will never be satisfied. This way it is safe to call 'watch()'
> for nested containers, without any unwanted side effects.
> 
> This commit fixes this issue.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp 
> ed7150a9b53c577fc248f8af2be55c6b12f64303 
> 
> Diff: https://reviews.apache.org/r/51988/diff/
> 
> 
> Testing
> ---
> 
> $ GTEST_FILTER="" make -j check
> $ sudo src/mesos-tests
> 
> 
> Thanks,
> 
> Kevin Klues
> 
>



Re: Review Request 51981: Marked the pid namespace isolator as nesting aware.

2016-09-17 Thread Kevin Klues

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


Ship it!




Ship It!

- Kevin Klues


On Sept. 17, 2016, 6:07 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51981/
> ---
> 
> (Updated Sept. 17, 2016, 6:07 p.m.)
> 
> 
> Review request for mesos, Gilbert Song and Kevin Klues.
> 
> 
> Bugs: MESOS-6189
> https://issues.apache.org/jira/browse/MESOS-6189
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Marked the pid namespace isolator as nesting aware.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/namespaces/pid.hpp 
> ccb525adbcf0944acf01f0e94126e8d14e1e95e8 
>   src/slave/containerizer/mesos/isolators/namespaces/pid.cpp 
> e330bb64df8c3dbcd9cb217e40010cbf017d6d4e 
> 
> Diff: https://reviews.apache.org/r/51981/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>



Re: Review Request 51989: Made the 'disk/du' isolator nesting aware.

2016-09-17 Thread Kevin Klues

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


Ship it!




Ship It!

- Kevin Klues


On Sept. 17, 2016, 7:33 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51989/
> ---
> 
> (Updated Sept. 17, 2016, 7:33 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Gilbert Song, and Kevin Klues.
> 
> 
> Bugs: MESOS-6194
> https://issues.apache.org/jira/browse/MESOS-6194
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The disk space check will be done at the top container level. All
> nested container will not have separate disk space check for now.
> 
> In theory, this change is not necessary for now as we can simply mark
> the isolator as nesting unaware. However, in the future, we might want
> to do separate disk check for each nested container and report
> limitations for them. Therefore, we include this patch as a starting
> point.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/posix/disk.hpp 
> 3998251248ecef2d174f0ea68e0493d09b012952 
>   src/slave/containerizer/mesos/isolators/posix/disk.cpp 
> 5b9d5aaeb008e9b102e86736c9312db0b3266d34 
> 
> Diff: https://reviews.apache.org/r/51989/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>



Re: Review Request 51988: Updated 'cgroups' isolator 'watch' to be properly nested aware.

2016-09-17 Thread Kevin Klues

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

(Updated Sept. 17, 2016, 7:47 p.m.)


Review request for mesos and Jie Yu.


Changes
---

Updated the comment


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


Repository: mesos


Description
---

In a previous iteration of attempting to make this isolator nested
aware, we (incorrectly) decided to return a 'Failure()' in 'watch()'
for nested containers. However, we shouldn't just blindly fail this
call for nested containers. Instead we should just return a 'Future'
that will never be satisfied. This way it is safe to call 'watch()'
for nested containers, without any unwanted side effects.

This commit fixes this issue.


Diffs (updated)
-

  src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp 
ed7150a9b53c577fc248f8af2be55c6b12f64303 

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


Testing
---

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


Thanks,

Kevin Klues



Re: Review Request 51981: Marked the pid namespace isolator as nesting aware.

2016-09-17 Thread Gilbert Song

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


Ship it!




Ship It!

- Gilbert Song


On Sept. 17, 2016, 11:07 a.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51981/
> ---
> 
> (Updated Sept. 17, 2016, 11:07 a.m.)
> 
> 
> Review request for mesos, Gilbert Song and Kevin Klues.
> 
> 
> Bugs: MESOS-6189
> https://issues.apache.org/jira/browse/MESOS-6189
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Marked the pid namespace isolator as nesting aware.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/namespaces/pid.hpp 
> ccb525adbcf0944acf01f0e94126e8d14e1e95e8 
>   src/slave/containerizer/mesos/isolators/namespaces/pid.cpp 
> e330bb64df8c3dbcd9cb217e40010cbf017d6d4e 
> 
> Diff: https://reviews.apache.org/r/51981/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>



Review Request 51988: Updated 'cgroups' isolator 'watch' to be properly nested aware.

2016-09-17 Thread Kevin Klues

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

Review request for mesos and Jie Yu.


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


Repository: mesos


Description
---

In a previous iteration of attempting to make this isolator nested
aware, we (incorrectly) decided to return a 'Failure()' in 'watch()'
for nested containers. However, we shouldn't just blindly fail this
call for nested containers. Instead we should just return a 'Future'
that will never be satisfied. This way it is safe to call 'watch()'
for nested containers, without any unwanted side effects.

This commit fixes this issue.


Diffs
-

  src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp 
ecfa5dbd538c967961e983abaeeefddf273a6e9e 

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


Testing
---

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


Thanks,

Kevin Klues



Re: Review Request 45491: Refactored subprocess options [1/2].

2016-09-17 Thread Jie Yu


> On Sept. 17, 2016, 4:11 a.m., Jie Yu wrote:
> > 3rdparty/libprocess/include/process/subprocess_base.hpp, lines 402-403
> > 
> >
> > Hum, i don't like ChildHook::None(). Have you tried if ```const 
> > vector& child_hooks = {}``` works or not?
> 
> Joerg Schad wrote:
> This is currently consistent with Hook::None(). Do you have a strong 
> opinion?

Yeah, I'd like to cleanup Hook::None() as well. This caused some confusion to 
me before. You can clean up the Hook::None() in a separate patch.


- Jie


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


On Sept. 11, 2016, 8:17 p.m., Joerg Schad wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45491/
> ---
> 
> (Updated Sept. 11, 2016, 8:17 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Jie Yu.
> 
> 
> Bugs: MESOS-5070
> https://issues.apache.org/jira/browse/MESOS-5070
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Previously the subprocess interface supported a several options for the
> child process such as setsid. In order to make the interface more
> flexible we refactored such options into a vector of ChildHooks.
> In order not to allow arbitrary code inside a ChildHook it has to be
> constructed via pre-defined factory methods.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/posix/subprocess.hpp 
> 57f71d15e6fbb032c8b6c5d0c73a93751022e7e7 
>   3rdparty/libprocess/include/process/ssl/gtest.hpp 
> 2ad623a5ea3b3286983e895010af756f14f51b64 
>   3rdparty/libprocess/include/process/subprocess_base.hpp 
> acd23c99162f164c02a7781abeb95b3c220ead12 
>   3rdparty/libprocess/src/subprocess.cpp 
> 8ca61f77083ac4b77aa1aec22806e1be43dbfa6c 
>   3rdparty/libprocess/src/tests/subprocess_tests.cpp 
> 0ff3945d2c722ebc1529265427397c5dfba6854e 
> 
> Diff: https://reviews.apache.org/r/45491/diff/
> 
> 
> Testing
> ---
> 
> Tested entire chain see https://reviews.apache.org/r/45492.
> 
> 
> Thanks,
> 
> Joerg Schad
> 
>



Review Request 51989: Made the 'disk/du' isolator nesting aware.

2016-09-17 Thread Jie Yu

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

Review request for mesos, Benjamin Hindman, Gilbert Song, and Kevin Klues.


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


Repository: mesos


Description
---

The disk space check will be done at the top container level. All
nested container will not have separate disk space check for now.

In theory, this change is not necessary for now as we can simply mark
the isolator as nesting unaware. However, in the future, we might want
to do separate disk check for each nested container and report
limitations for them. Therefore, we include this patch as a starting
point.


Diffs
-

  src/slave/containerizer/mesos/isolators/posix/disk.hpp 
3998251248ecef2d174f0ea68e0493d09b012952 
  src/slave/containerizer/mesos/isolators/posix/disk.cpp 
5b9d5aaeb008e9b102e86736c9312db0b3266d34 

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


Testing
---

make check


Thanks,

Jie Yu



Re: Review Request 45491: Refactored subprocess options [1/2].

2016-09-17 Thread Joerg Schad


> On Sept. 17, 2016, 4:11 a.m., Jie Yu wrote:
> > 3rdparty/libprocess/include/process/subprocess_base.hpp, line 314
> > 
> >
> > I suggest we rename Hook to ParentHook. You can do that in a separate 
> > patch.

Sound good. Will be done in a seperate patch.


- Joerg


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


On Sept. 11, 2016, 8:17 p.m., Joerg Schad wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45491/
> ---
> 
> (Updated Sept. 11, 2016, 8:17 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Jie Yu.
> 
> 
> Bugs: MESOS-5070
> https://issues.apache.org/jira/browse/MESOS-5070
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Previously the subprocess interface supported a several options for the
> child process such as setsid. In order to make the interface more
> flexible we refactored such options into a vector of ChildHooks.
> In order not to allow arbitrary code inside a ChildHook it has to be
> constructed via pre-defined factory methods.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/posix/subprocess.hpp 
> 57f71d15e6fbb032c8b6c5d0c73a93751022e7e7 
>   3rdparty/libprocess/include/process/ssl/gtest.hpp 
> 2ad623a5ea3b3286983e895010af756f14f51b64 
>   3rdparty/libprocess/include/process/subprocess_base.hpp 
> acd23c99162f164c02a7781abeb95b3c220ead12 
>   3rdparty/libprocess/src/subprocess.cpp 
> 8ca61f77083ac4b77aa1aec22806e1be43dbfa6c 
>   3rdparty/libprocess/src/tests/subprocess_tests.cpp 
> 0ff3945d2c722ebc1529265427397c5dfba6854e 
> 
> Diff: https://reviews.apache.org/r/45491/diff/
> 
> 
> Testing
> ---
> 
> Tested entire chain see https://reviews.apache.org/r/45492.
> 
> 
> Thanks,
> 
> Joerg Schad
> 
>



Re: Review Request 45491: Refactored subprocess options [1/2].

2016-09-17 Thread Joerg Schad


> On Sept. 17, 2016, 4:11 a.m., Jie Yu wrote:
> > 3rdparty/libprocess/include/process/subprocess_base.hpp, lines 402-403
> > 
> >
> > Hum, i don't like ChildHook::None(). Have you tried if ```const 
> > vector& child_hooks = {}``` works or not?

This is currently consistent with Hook::None(). Do you have a strong opinion?


- Joerg


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


On Sept. 11, 2016, 8:17 p.m., Joerg Schad wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45491/
> ---
> 
> (Updated Sept. 11, 2016, 8:17 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Jie Yu.
> 
> 
> Bugs: MESOS-5070
> https://issues.apache.org/jira/browse/MESOS-5070
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Previously the subprocess interface supported a several options for the
> child process such as setsid. In order to make the interface more
> flexible we refactored such options into a vector of ChildHooks.
> In order not to allow arbitrary code inside a ChildHook it has to be
> constructed via pre-defined factory methods.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/posix/subprocess.hpp 
> 57f71d15e6fbb032c8b6c5d0c73a93751022e7e7 
>   3rdparty/libprocess/include/process/ssl/gtest.hpp 
> 2ad623a5ea3b3286983e895010af756f14f51b64 
>   3rdparty/libprocess/include/process/subprocess_base.hpp 
> acd23c99162f164c02a7781abeb95b3c220ead12 
>   3rdparty/libprocess/src/subprocess.cpp 
> 8ca61f77083ac4b77aa1aec22806e1be43dbfa6c 
>   3rdparty/libprocess/src/tests/subprocess_tests.cpp 
> 0ff3945d2c722ebc1529265427397c5dfba6854e 
> 
> Diff: https://reviews.apache.org/r/45491/diff/
> 
> 
> Testing
> ---
> 
> Tested entire chain see https://reviews.apache.org/r/45492.
> 
> 
> Thanks,
> 
> Joerg Schad
> 
>



Re: Review Request 51983: Avoided passing large object by value.

2016-09-17 Thread Mesos ReviewBot

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



Patch looks great!

Reviews applied: [51983]

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. 17, 2016, 3:05 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51983/
> ---
> 
> (Updated Sept. 17, 2016, 3:05 p.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Avoided passing large object by value.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/network/cni/cni.hpp 
> eea80355d9a12f7b9571c55194ef3ab4931e6aed 
> 
> Diff: https://reviews.apache.org/r/51983/diff/
> 
> 
> Testing
> ---
> 
> `make check` on OSX and Linux.
> 
> 
> Thanks,
> 
> Neil Conway
> 
>



Re: Review Request 51981: Marked the pid namespace isolator as nesting aware.

2016-09-17 Thread Jie Yu

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

(Updated Sept. 17, 2016, 6:07 p.m.)


Review request for mesos, Gilbert Song and Kevin Klues.


Summary (updated)
-

Marked the pid namespace isolator as nesting aware.


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


Repository: mesos


Description (updated)
---

Marked the pid namespace isolator as nesting aware.


Diffs (updated)
-

  src/slave/containerizer/mesos/isolators/namespaces/pid.hpp 
ccb525adbcf0944acf01f0e94126e8d14e1e95e8 
  src/slave/containerizer/mesos/isolators/namespaces/pid.cpp 
e330bb64df8c3dbcd9cb217e40010cbf017d6d4e 

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


Testing
---

make check


Thanks,

Jie Yu



Re: Review Request 51978: Refactored commandScheduler in cli/execute.cpp to take TaskInfo.

2016-09-17 Thread Vinod Kone

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


Ship it!




Ship It!

- Vinod Kone


On Sept. 17, 2016, 4:23 p.m., Abhishek Dasgupta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51978/
> ---
> 
> (Updated Sept. 17, 2016, 4:23 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-6096
> https://issues.apache.org/jira/browse/MESOS-6096
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This refactoring was needed for better accomodating pod support
> for mesos-execute.
> 
> 
> Diffs
> -
> 
>   src/cli/execute.cpp c9f56af7f37d5b79b51f350d6c533714c170e889 
> 
> Diff: https://reviews.apache.org/r/51978/diff/
> 
> 
> Testing
> ---
> 
> Manually Ran:
> mesos-execute --master=127.0.0.1:5050 --name="Hello1" --command="echo hello" 
> --docker_image="ubuntu" --containerizer="docker"
> 
> 
> Thanks,
> 
> Abhishek Dasgupta
> 
>



Re: Review Request 51983: Avoided passing large object by value.

2016-09-17 Thread Jie Yu

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


Ship it!




Ship It!

- Jie Yu


On Sept. 17, 2016, 3:05 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51983/
> ---
> 
> (Updated Sept. 17, 2016, 3:05 p.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Avoided passing large object by value.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/network/cni/cni.hpp 
> eea80355d9a12f7b9571c55194ef3ab4931e6aed 
> 
> Diff: https://reviews.apache.org/r/51983/diff/
> 
> 
> Testing
> ---
> 
> `make check` on OSX and Linux.
> 
> 
> Thanks,
> 
> Neil Conway
> 
>



Re: Review Request 51958: Removed use of "--registry_strict" master flag.

2016-09-17 Thread Mesos ReviewBot

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



Patch looks great!

Reviews applied: [50235, 50416, 50417, 50418, 50422, 50699, 50700, 50701, 
50702, 50703, 50704, 50705, 50706, 50707, 50844, 50845, 50846, 51020, 51371, 
51374, 51375, 51376, 51377, 51021, 51706, 51653, 51707, 51805, 51845, 51891, 
51909, 51913, 51953, 51954, 51955, 51956, 51957, 51958]

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. 17, 2016, 2:23 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51958/
> ---
> 
> (Updated Sept. 17, 2016, 2:23 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-5951
> https://issues.apache.org/jira/browse/MESOS-5951
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The flag will remain for a few more months per the deprecation policy,
> but we no longer need to set it in the unit tests. Providing an
> extensive usage summary in "--help" output also doesn't seem useful.
> 
> 
> Diffs
> -
> 
>   src/master/flags.cpp 19ae6c1c30a1054b64a9585f325bd0bf943af218 
>   src/tests/cluster.cpp b04653af97d17aaa9d0d3ee872169b66cd67126b 
>   src/tests/mesos.cpp 07a64f0ff49a753ec26260cdf859d0584c3f935a 
> 
> Diff: https://reviews.apache.org/r/51958/diff/
> 
> 
> Testing
> ---
> 
> `make check` on OSX and Linux.
> 
> 
> Thanks,
> 
> Neil Conway
> 
>



Re: Review Request 51871: Modified the `isolate` method to be nesting aware.

2016-09-17 Thread Avinash sridharan

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

(Updated Sept. 17, 2016, 4:34 p.m.)


Review request for mesos, Gilbert Song, Jie Yu, Joseph Wu, and Qian Zhang.


Changes
---

Modified the logic for isolate, based on Jie's comments. We are not using the 
top-level parent container's network files to setup the child container's 
network files.


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


Repository: mesos


Description
---

The network file setup in the `network/cni` isolator is now nesting
aware. Since the children share the network and UTS namespace with the
parent, the network files need to be created only for the parent
container. For the child containers, the network files will be simply
a symlink to a parents network files.


Diffs (updated)
-

  src/slave/containerizer/mesos/isolators/network/cni/cni.cpp 
822f11eab5b00c014563322a8c3b2c14cb440e0b 

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


Testing
---

make
make check
sudo ./bin/mesos-tests.sh


Thanks,

Avinash sridharan



Re: Review Request 51978: Refactored commandScheduler in cli/execute.cpp to take TaskInfo.

2016-09-17 Thread Abhishek Dasgupta

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

(Updated Sept. 17, 2016, 4:23 p.m.)


Review request for mesos and Vinod Kone.


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


Repository: mesos


Description (updated)
---

This refactoring was needed for better accomodating pod support
for mesos-execute.


Diffs
-

  src/cli/execute.cpp c9f56af7f37d5b79b51f350d6c533714c170e889 

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


Testing
---

Manually Ran:
mesos-execute --master=127.0.0.1:5050 --name="Hello1" --command="echo hello" 
--docker_image="ubuntu" --containerizer="docker"


Thanks,

Abhishek Dasgupta



Re: Review Request 51978: Refactored commandScheduler in cli/execute.cpp to take TaskInfo.

2016-09-17 Thread Abhishek Dasgupta

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

(Updated Sept. 17, 2016, 4:22 p.m.)


Review request for mesos and Vinod Kone.


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


Repository: mesos


Description (updated)
---

Refactored commandScheduler in cli/execute.cpp to take TaskInfo.


Diffs (updated)
-

  src/cli/execute.cpp c9f56af7f37d5b79b51f350d6c533714c170e889 

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


Testing (updated)
---

Manually Ran:
mesos-execute --master=127.0.0.1:5050 --name="Hello1" --command="echo hello" 
--docker_image="ubuntu" --containerizer="docker"


Thanks,

Abhishek Dasgupta



Review Request 51985: Waited for agent finish registering in test case.

2016-09-17 Thread haosdent huang

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

Review request for mesos, Gilbert Song, Greg Mann, Jie Yu, Joseph Wu, and Qian 
Zhang.


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


Repository: mesos


Description
---

Waited for agent finish registering in test case.


Diffs
-

  src/tests/slave_recovery_tests.cpp 23cf9d8cd76d568fa841817d6f4279c4aa2fdc75 

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


Testing
---


Thanks,

haosdent huang



Review Request 51984: Cleaned up initialization of atomic fields in ProcessManager.

2016-09-17 Thread Neil Conway

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

Review request for mesos and Joseph Wu.


Repository: mesos


Description
---

`std::atomic` has a trivial default constructor, which means
`std::atomic` values are left uninitialized by default. `ProcessManager`
uses two atomic fields that are default-constructed; although we `store`
to them before reading from them, it would be cleaner and safer to
initialize them to a well-defined value in the first place.


Diffs
-

  3rdparty/libprocess/src/process.cpp 1e48fd5269d1a94c2217e8826af54b9b42ec4b23 

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


Testing
---

`make check` on OSX.


Thanks,

Neil Conway



Review Request 51983: Avoided passing large object by value.

2016-09-17 Thread Neil Conway

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

Review request for mesos and Jie Yu.


Repository: mesos


Description
---

Avoided passing large object by value.


Diffs
-

  src/slave/containerizer/mesos/isolators/network/cni/cni.hpp 
eea80355d9a12f7b9571c55194ef3ab4931e6aed 

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


Testing
---

`make check` on OSX and Linux.


Thanks,

Neil Conway



Re: Review Request 51958: Removed use of "--registry_strict" master flag.

2016-09-17 Thread Neil Conway

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

(Updated Sept. 17, 2016, 2:23 p.m.)


Review request for mesos and Vinod Kone.


Changes
---

Per review comment: keep flag for now, remove some uses of it.


Summary (updated)
-

Removed use of "--registry_strict" master flag.


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


Repository: mesos


Description (updated)
---

The flag will remain for a few more months per the deprecation policy,
but we no longer need to set it in the unit tests. Providing an
extensive usage summary in "--help" output also doesn't seem useful.


Diffs (updated)
-

  src/master/flags.cpp 19ae6c1c30a1054b64a9585f325bd0bf943af218 
  src/tests/cluster.cpp b04653af97d17aaa9d0d3ee872169b66cd67126b 
  src/tests/mesos.cpp 07a64f0ff49a753ec26260cdf859d0584c3f935a 

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


Testing
---

`make check` on OSX and Linux.


Thanks,

Neil Conway



Re: Review Request 45492: Used ChildHooks in Mesos [2/2].

2016-09-17 Thread Qian Zhang

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


Ship it!




Ship It!

- Qian Zhang


On Sept. 12, 2016, 9:05 p.m., Joerg Schad wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45492/
> ---
> 
> (Updated Sept. 12, 2016, 9:05 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Jie Yu.
> 
> 
> Bugs: MESOS-5070
> https://issues.apache.org/jira/browse/MESOS-5070
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> We now use the new ChildHooks instead of explicit options such
> as setsid.
> 
> 
> Diffs
> -
> 
>   src/docker/docker.cpp 16abf64f98cc833ea4bee28bc08951eb2ff6d995 
>   src/health-check/health_checker.cpp 
> f373df19fc8af8e9650be61e3b101e89362a67cd 
>   src/launcher/posix/executor.cpp 7c40ebebb983b7977526fee123bf07c86fbb5e50 
>   src/linux/perf.cpp 556dc8dbb4170101ae9154afd4935df891db136f 
>   src/slave/container_loggers/lib_logrotate.cpp 
> 1c170e5d11ef31d468b200c2c4cbd27abeeb418a 
>   src/slave/containerizer/docker.cpp 110a1eb41b1ff7cb94f3630a1843d9f01efbe09c 
>   src/slave/containerizer/fetcher.cpp 
> 4d3fc521bf8a7c438c48e31c549f108ac3602b3f 
>   src/slave/containerizer/mesos/isolators/docker/volume/driver.cpp 
> ed3aa1d4c7213acfde2f63ee719edab553c9bd79 
>   src/slave/containerizer/mesos/isolators/network/cni/cni.cpp 
> 822f11eab5b00c014563322a8c3b2c14cb440e0b 
>   src/slave/containerizer/mesos/isolators/network/port_mapping.cpp 
> 55a06cfe1edbb7699c04101f78cc4979176e33f6 
>   src/slave/containerizer/mesos/isolators/posix/disk.cpp 
> 5b9d5aaeb008e9b102e86736c9312db0b3266d34 
>   src/slave/containerizer/mesos/launcher.cpp 
> 8183a1cd511909e5680b793e1adbf8d26afa9ce5 
>   src/slave/containerizer/mesos/linux_launcher.cpp 
> d0f92856780f087908731ec0601692c78339d8a7 
>   src/tests/containerizer/capabilities_tests.cpp 
> 3ba46404c389385e02d2a1789e33dab3e8f15902 
>   src/tests/containerizer/launch_tests.cpp 
> 76d71b3088a7bec3b3f8546c197f65557f464028 
>   src/tests/containerizer/ns_tests.cpp 
> e52ebbe93cd80236ca865a137de4251f25d55b05 
>   src/tests/containerizer/port_mapping_tests.cpp 
> 7fac6ca1550acf54616fb4cef2eab1335f5e9760 
> 
> Diff: https://reviews.apache.org/r/45492/diff/
> 
> 
> Testing
> ---
> 
> sudo Make check + internal ci
> 
> 
> Thanks,
> 
> Joerg Schad
> 
>



Re: Review Request 51978: Refactored commandScheduler in cli/execute.cpp to take TaskInfo.

2016-09-17 Thread Mesos ReviewBot

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



Bad patch!

Reviews applied: [51978]

Failed command: ./support/apply-review.sh -n -r 51978

Error:
2016-09-17 10:48:31 URL:https://reviews.apache.org/r/51978/diff/raw/ 
[13751/13751] -> "51978.patch" [1]
error: patch failed: src/cli/execute.cpp:355
error: src/cli/execute.cpp: patch does not apply

Full log: https://builds.apache.org/job/mesos-reviewbot/15288/console

- Mesos ReviewBot


On Sept. 17, 2016, 6:38 a.m., Abhishek Dasgupta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51978/
> ---
> 
> (Updated Sept. 17, 2016, 6:38 a.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-6096
> https://issues.apache.org/jira/browse/MESOS-6096
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This refactoring was needed for better accomodating pod support
> for mesos-execute.
> 
> 
> Diffs
> -
> 
>   src/cli/execute.cpp c9f56af7f37d5b79b51f350d6c533714c170e889 
> 
> Diff: https://reviews.apache.org/r/51978/diff/
> 
> 
> Testing
> ---
> 
> Manually Ran:
> mesos-execute --master=127.0.0.1:5050 --name="Hello1" --command="echo hello" 
> --docker_image="ubuntu"
> 
> 
> Thanks,
> 
> Abhishek Dasgupta
> 
>



Re: Review Request 51621: WIP: Made recovered resource allocated as soon as possible.

2016-09-17 Thread Mesos ReviewBot

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



Patch looks great!

Reviews applied: [51028, 51027, 51621]

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. 17, 2016, 1:29 a.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51621/
> ---
> 
> (Updated Sept. 17, 2016, 1:29 a.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Jacob Janco, and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-3078
> https://issues.apache.org/jira/browse/MESOS-3078
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Please do not review for now as it is WIP.
> 
> Enable the `recoverResources` logic to call `ensureAllocation`,
> this can make sure the `allocationCandidates` can be updated
> as soon as possible and allocate the resources as soon as possible
> if there are no request in the message queue.
> 
> 
> Diffs
> -
> 
>   src/master/allocator/mesos/hierarchical.cpp 
> 2d56bd011f2c87c67a02d0ae467a4a537d36867e 
>   src/tests/hierarchical_allocator_tests.cpp 
> 46553f6d501deb37862dd5ba48be1c114f6e6cb8 
>   src/tests/master_allocator_tests.cpp 
> b4ecd461d522503264ad2e25b2bf0cfc4a0ba12a 
>   src/tests/persistent_volume_tests.cpp 
> c38d8484fbd62b36bf124a96d17c8cae5e51e594 
>   src/tests/reservation_endpoints_tests.cpp 
> 28b497eccd5fadd03dc24e73e11e59ce36d78d32 
>   src/tests/reservation_tests.cpp 6c28ab4557f342134efce0ad7cb174a5adb4dc10 
>   src/tests/slave_tests.cpp 696e0f71190238133f29b6380bc58b994a556e69 
> 
> Diff: https://reviews.apache.org/r/51621/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> ```
> GLOG_v=1 ./bin/mesos-tests.sh 
> --gtest_filter="*PersistentVolumeTest.BadACL*/*" --verbose --gtest_repeat=20
> GLOG_v=1 ./bin/mesos-tests.sh 
> --gtest_filter="*PersistentVolumeTest.GoodACL*/*" --verbose --gtest_repeat=20
> GLOG_v=1 ./bin/mesos-tests.sh 
> --gtest_filter="ReservationEndpointsTest.UnreserveAvailableAndOfferedResources"
>   --gtest_repeat=20
> GLOG_v=1 ./bin/mesos-tests.sh 
> --gtest_filter="ReservationEndpointsTest.ReserveAvailableAndOfferedResources" 
>  --gtest_repeat=20
> GLOG_v=1 ./bin/mesos-tests.sh 
> --gtest_filter="ReservationTest.ACLMultipleOperations" --verbose  
> --gtest_repeat=20
> GLOG_v=1 ./bin/mesos-tests.sh 
> --gtest_filter="MasterAllocatorTest/1.RebalancedForUpdatedWeights" 
> --gtest_repeat=30
> GLOG_v=1 ./bin/mesos-tests.sh 
> --gtest_filter="HierarchicalAllocatorTest.UpdateWeight" --verbose 
> --gtest_repeat=20
> GLOG_v=1 ./bin/mesos-tests.sh 
> --gtest_filter="MasterAllocatorTest/*.SlaveLost" --gtest_repeat=20
> GLOG_v=1 ./bin/mesos-tests.sh 
> --gtest_filter="SlaveTest.ExecutorShutdownGracePeriod" --gtest_repeat=20
> ```
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>



Re: Review Request 51968: Simplified usage of `os::pagesize` in Mesos.

2016-09-17 Thread Michael Park

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


Ship it!




Ship It!

- Michael Park


On Sept. 16, 2016, 7:02 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51968/
> ---
> 
> (Updated Sept. 16, 2016, 7:02 p.m.)
> 
> 
> Review request for mesos and Michael Park.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Simplified usage of `os::pagesize` in Mesos.
> 
> 
> Diffs
> -
> 
>   src/slave/container_loggers/lib_logrotate.hpp 
> 9f8ec2f2258fbd5d70f8db466bc71856c568a58d 
>   src/slave/container_loggers/logrotate.hpp 
> 96697d46ca71e7f62119f2fe669230cf5a04242f 
> 
> Diff: https://reviews.apache.org/r/51968/diff/
> 
> 
> Testing
> ---
> 
> `make check` on OSX and Linux.
> 
> 
> Thanks,
> 
> Neil Conway
> 
>



Re: Review Request 51967: Changed return type of `os::pagesize` in stout.

2016-09-17 Thread Michael Park

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


Ship it!




Ship It!

- Michael Park


On Sept. 16, 2016, 7:02 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51967/
> ---
> 
> (Updated Sept. 16, 2016, 7:02 p.m.)
> 
> 
> Review request for mesos and Michael Park.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Previously, `os::pagesize` returned a signed integer (`int`), where a
> negative value indicated an error. This is problematic: several callers
> neglected to check for errors and most callers ultimately want an
> unsigned value anyway.
> 
> We could make this return `Try`, but that seems heavyweight: the
> Win32 implementation of this function will never fail and the POSIX
> implementation (using `sysconf(3)`) seems unlikely to fail in
> practice. Hence, this commit changes `os::pagesize` to return `size_t`.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/os/freebsd.hpp 
> 3f5142c81f4ea363238d6b546130e51518fd9daa 
>   3rdparty/stout/include/stout/os/linux.hpp 
> 33825280eb1404bcd89324f8ab5949f735b2d130 
>   3rdparty/stout/include/stout/os/osx.hpp 
> 54d34121d8cde434fca60679e19eae507b4048ba 
>   3rdparty/stout/include/stout/os/posix/pagesize.hpp 
> f3ae69adf096d558e083615dfcf848c94e017e6e 
>   3rdparty/stout/include/stout/os/windows/pagesize.hpp 
> 6112e9781a9d42f7ec1ae0832c0c877d1915b09b 
> 
> Diff: https://reviews.apache.org/r/51967/diff/
> 
> 
> Testing
> ---
> 
> `make check` on OSX and Linux.
> 
> 
> Thanks,
> 
> Neil Conway
> 
>



Re: Review Request 51981: Marke the pid namespace isolator as nesting aware.

2016-09-17 Thread Mesos ReviewBot

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



Patch looks great!

Reviews applied: [51981]

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. 17, 2016, 1:26 a.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51981/
> ---
> 
> (Updated Sept. 17, 2016, 1:26 a.m.)
> 
> 
> Review request for mesos, Gilbert Song and Kevin Klues.
> 
> 
> Bugs: MESOS-6189
> https://issues.apache.org/jira/browse/MESOS-6189
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Marke the pid namespace isolator as nesting aware.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/namespaces/pid.hpp 
> ccb525adbcf0944acf01f0e94126e8d14e1e95e8 
>   src/slave/containerizer/mesos/isolators/namespaces/pid.cpp 
> e330bb64df8c3dbcd9cb217e40010cbf017d6d4e 
> 
> Diff: https://reviews.apache.org/r/51981/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>



Re: Review Request 45491: Refactored subprocess options [1/2].

2016-09-17 Thread Qian Zhang

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




3rdparty/libprocess/include/process/posix/subprocess.hpp (line 214)


s/std::vector/vector/



3rdparty/libprocess/include/process/posix/subprocess.hpp (line 312)


Ditto.



3rdparty/libprocess/src/subprocess.cpp (line 132)


Ditto.


- Qian Zhang


On Sept. 12, 2016, 4:17 a.m., Joerg Schad wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45491/
> ---
> 
> (Updated Sept. 12, 2016, 4:17 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Jie Yu.
> 
> 
> Bugs: MESOS-5070
> https://issues.apache.org/jira/browse/MESOS-5070
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Previously the subprocess interface supported a several options for the
> child process such as setsid. In order to make the interface more
> flexible we refactored such options into a vector of ChildHooks.
> In order not to allow arbitrary code inside a ChildHook it has to be
> constructed via pre-defined factory methods.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/posix/subprocess.hpp 
> 57f71d15e6fbb032c8b6c5d0c73a93751022e7e7 
>   3rdparty/libprocess/include/process/ssl/gtest.hpp 
> 2ad623a5ea3b3286983e895010af756f14f51b64 
>   3rdparty/libprocess/include/process/subprocess_base.hpp 
> acd23c99162f164c02a7781abeb95b3c220ead12 
>   3rdparty/libprocess/src/subprocess.cpp 
> 8ca61f77083ac4b77aa1aec22806e1be43dbfa6c 
>   3rdparty/libprocess/src/tests/subprocess_tests.cpp 
> 0ff3945d2c722ebc1529265427397c5dfba6854e 
> 
> Diff: https://reviews.apache.org/r/45491/diff/
> 
> 
> Testing
> ---
> 
> Tested entire chain see https://reviews.apache.org/r/45492.
> 
> 
> Thanks,
> 
> Joerg Schad
> 
>



Re: Review Request 51869: Export "reserved_resources" in the agent HTTP endpoint.

2016-09-17 Thread Anindya Sinha

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


Ship it!




Ship It!

- Anindya Sinha


On Sept. 16, 2016, 6:24 p.m., Jiang Yan Xu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51869/
> ---
> 
> (Updated Sept. 16, 2016, 6:24 p.m.)
> 
> 
> Review request for mesos, Anindya Sinha and Neil Conway.
> 
> 
> Bugs: MESOS-6085
> https://issues.apache.org/jira/browse/MESOS-6085
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Also the 'resource' field now exposes the total resources, which is
> consistent with the same field in the master's /slaves endpoint.
> 
> 
> Diffs
> -
> 
>   src/slave/http.cpp 67463105d7fa38b2158a64f6994e3dd353a9fcc7 
>   src/slave/slave.cpp 7f99e4610d06ebadbef48ce314fec6ad04acb307 
> 
> Diff: https://reviews.apache.org/r/51869/diff/
> 
> 
> Testing
> ---
> 
> make check. A new test is added in https://reviews.apache.org/r/51870/.
> 
> 
> Thanks,
> 
> Jiang Yan Xu
> 
>



Re: Review Request 51868: Expose full reservation info in the agent's http endpoint.

2016-09-17 Thread Anindya Sinha

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


Ship it!




Ship It!

- Anindya Sinha


On Sept. 14, 2016, 6:21 p.m., Jiang Yan Xu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51868/
> ---
> 
> (Updated Sept. 14, 2016, 6:21 p.m.)
> 
> 
> Review request for mesos, Anindya Sinha and Neil Conway.
> 
> 
> Bugs: MESOS-4668
> https://issues.apache.org/jira/browse/MESOS-4668
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Expose full reservation info in the agent's http endpoint.
> 
> 
> Diffs
> -
> 
>   src/slave/http.cpp 67463105d7fa38b2158a64f6994e3dd353a9fcc7 
> 
> Diff: https://reviews.apache.org/r/51868/diff/
> 
> 
> Testing
> ---
> 
> make check. A new test is added in https://reviews.apache.org/r/51870/.
> 
> 
> Thanks,
> 
> Jiang Yan Xu
> 
>



Re: Review Request 51868: Expose full reservation info in the agent's http endpoint.

2016-09-17 Thread Anindya Sinha


> On Sept. 14, 2016, 11:56 p.m., Anindya Sinha wrote:
> > src/slave/http.cpp, line 986
> > 
> >
> > Add this new field in the output of help, i.e. `string 
> > Slave::Http::STATE_HELP()`?
> 
> Jiang Yan Xu wrote:
> Should we punt on this until MESOS-3568 is addressed?

Sounds good.


- Anindya


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


On Sept. 14, 2016, 6:21 p.m., Jiang Yan Xu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51868/
> ---
> 
> (Updated Sept. 14, 2016, 6:21 p.m.)
> 
> 
> Review request for mesos, Anindya Sinha and Neil Conway.
> 
> 
> Bugs: MESOS-4668
> https://issues.apache.org/jira/browse/MESOS-4668
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Expose full reservation info in the agent's http endpoint.
> 
> 
> Diffs
> -
> 
>   src/slave/http.cpp 67463105d7fa38b2158a64f6994e3dd353a9fcc7 
> 
> Diff: https://reviews.apache.org/r/51868/diff/
> 
> 
> Testing
> ---
> 
> make check. A new test is added in https://reviews.apache.org/r/51870/.
> 
> 
> Thanks,
> 
> Jiang Yan Xu
> 
>



Re: Review Request 51871: Modified the `isolate` method to be nesting aware.

2016-09-17 Thread Avinash Sridharan
Ah, that simplifies things. In that case don't think there should be any
change to `status`.

On Fri, Sep 16, 2016 at 9:02 PM, Jie Yu  wrote:

> In fact, in MVP, 'status' won't be called for nested containers.
> Containerizer will enforce that.
>
> On Fri, Sep 16, 2016 at 9:01 PM, Avinash Sridharan 
> wrote:
>
>>
>>
>> On Fri, Sep 16, 2016 at 8:53 PM, Jie Yu  wrote:
>>
>>> containers should be able to operate even if the parent container goes
 away
>>>
>>>
>>> This should never happen. One of the nesting feature is: if the parent
>>> container goes way, all its child containers will go away too. The
>>> containerizer will enforce it.
>>>
>>> Reason being that during recovery the containers need to be re-populated
 in the `infos` structure. Otherwise there would be descripancy between the
 containers present in `infos` before and after recovery.
>>>
>>>
>>> Ok, in that case its my bad. I misunderstood, thinking that in future
>> the kill policy might allow reincarnation of the parents without killing
>> the children. So using the parents network files directly should just work.
>>
>>> We already have that discrepancy. If a container joining the host
>>> network has a rootfs, we'll construct an Info, but we will not recover it.
>>> I think we just need to follow the same.
>>>
>>> In retrospect, we probably should not create 'Info' for that case, we
>>> probably should use a different map. We can clean that up later.
>>>
>>> Ok, for MVP, I will keep the current behavior (with the discrepancy).
>> Won't create a containerDir for this child containers. Will need to
>> implement some extra checks in `status` to get correct `ContainerStatus`
>> for containers not in `infos`, but that should be trivial.
>>
>>> - Jie
>>>
>>> On Fri, Sep 16, 2016 at 8:01 PM, Avinash sridharan <
>>> avin...@mesosphere.io> wrote:
>>>


 > On Sept. 17, 2016, 1 a.m., Jie Yu wrote:
 > > src/slave/containerizer/mesos/isolators/network/cni/cni.cpp, lines
 816-820
 > > 
 > >
 > > Any reason we need to copy the network files, instead of just
 using it? Do we really need to have a containerDir for nested container? I
 like the invariant that we only have a containerDir if we actually create
 network for the container.

 That is a good point. The only reason I was thinking about maintaining
 a copy of each of the network files is that the containers should be able
 to operate even if the parent container goes away. This is not an issue for
 the MVP, but I was thinking this might be an issue later on when we have
 different kill policies enforced.

 That said, after seeing you comment, I am realizing that just copying
 the network files is not enough, we need to copy the entire parentDir if we
 want to maintain the container operation in the abscene of the parent
 container. Which gets complicated. So if we shouldn't worry too much about
 this case, then I agree we can just do without using the parent files, and
 keep it much simpler.

 Irrespective of whether we use parent's network files, or copy over the
 files, I think we still need a `containerDir` for each container. Reason
 being that during recovery the containers need to be re-populated in the
 `infos` structure. Otherwise there would be descripancy between the
 containers present in `infos` before and after recovery. This, I think,
 would convolute the logic we would have in `status`, which is critical to
 retrieve the container's IP address.


 - Avinash


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


 On Sept. 16, 2016, 11:31 p.m., Avinash sridharan wrote:
 >
 > ---
 > This is an automatically generated e-mail. To reply, visit:
 > https://reviews.apache.org/r/51871/
 > ---
 >
 > (Updated Sept. 16, 2016, 11:31 p.m.)
 >
 >
 > Review request for mesos, Gilbert Song, Jie Yu, Joseph Wu, and Qian
 Zhang.
 >
 >
 > Bugs: MESOS-6156
 > https://issues.apache.org/jira/browse/MESOS-6156
 >
 >
 > Repository: mesos
 >
 >
 > Description
 > ---
 >
 > The network file setup in the `network/cni` isolator is now nesting
 > aware. Since the children share the network and UTS namespace with the
 > parent, the network files need to be created only for the parent
 > container. For the child containers, the network files will be simply
 > a symlink to a 

Re: Review Request 51978: Refactored commandScheduler in cli/execute.cpp to take TaskInfo.

2016-09-17 Thread Abhishek Dasgupta

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

(Updated Sept. 17, 2016, 6:38 a.m.)


Review request for mesos and Vinod Kone.


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


Repository: mesos


Description (updated)
---

This refactoring was needed for better accomodating pod support
for mesos-execute.


Diffs
-

  src/cli/execute.cpp c9f56af7f37d5b79b51f350d6c533714c170e889 

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


Testing
---

Manually Ran:
mesos-execute --master=127.0.0.1:5050 --name="Hello1" --command="echo hello" 
--docker_image="ubuntu"


Thanks,

Abhishek Dasgupta



Re: Review Request 51978: Refactored commandScheduler in cli/execute.cpp to take TaskInfo.

2016-09-17 Thread Abhishek Dasgupta

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

(Updated Sept. 17, 2016, 6:37 a.m.)


Review request for mesos and Vinod Kone.


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


Repository: mesos


Description (updated)
---

Refactored commandScheduler in cli/execute.cpp to take TaskInfo.


Diffs (updated)
-

  src/cli/execute.cpp c9f56af7f37d5b79b51f350d6c533714c170e889 

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


Testing (updated)
---

Manually Ran:
mesos-execute --master=127.0.0.1:5050 --name="Hello1" --command="echo hello" 
--docker_image="ubuntu"


Thanks,

Abhishek Dasgupta