Re: Review Request 52787: Set `runtime_dir` to a temporary folder in `mesos-local`.

[Kevin Klues]

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



I should have looked at this more carefully before giving it a Ship it. It 
seemed reasonable at first glance though.

Did you attempt to compile it before posting the patch? Looks like there were 
errors with the flags definition.

```
In file included from ../../src/local/main.cpp:26:0:
../../src/local/flags.hpp: In constructor 
'mesos::internal::local::Flags::Flags()':
../../src/local/flags.hpp:54:10: error: 'runtime_dir' is not a member of 
'mesos::internal::local::Flags'
 add(::runtime_dir,
```

- Kevin Klues


On Oct. 12, 2016, 4:13 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52787/
> ---
> 
> (Updated Oct. 12, 2016, 4:13 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Kevin Klues.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Set `runtime_dir` to a temporary folder in `mesos-local`.
> 
> 
> Diffs
> -
> 
>   bin/mesos-local-flags.sh.in 5b4553a808dc9f34a15390e69b2f85e95761ec53 
>   src/local/flags.hpp c77eff13e7a63630ff6c9428e57f3f6707e1f30f 
> 
> Diff: https://reviews.apache.org/r/52787/diff/
> 
> 
> Testing
> ---
> 
> To void this error when launch `mesos-local` without `sudo`
> 
> ```
>   message: 'Failed to launch container: Failed to make the containerizer 
> runtime directory 
> '/var/run/mesos/containers/f2d6947f-2916-4f1a-90dc-3d137b360b9c': Permission 
> denied; Abnormal executor termination: unknown container'
> ```
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 52811: Updated 'MountInfoTableReadSorted' test to use a hashset.

[Jie Yu]

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


Ship it!




Ship It!

- Jie Yu


On Oct. 12, 2016, 11:12 p.m., Kevin Klues wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52811/
> ---
> 
> (Updated Oct. 12, 2016, 11:12 p.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-6118
> https://issues.apache.org/jira/browse/MESOS-6118
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated 'MountInfoTableReadSorted' test to use a hashset.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/fs_tests.cpp 
> 0dd212f94d2845494163a86bc2059f999e018cd0 
> 
> Diff: https://reviews.apache.org/r/52811/diff/
> 
> 
> Testing
> ---
> 
> GTEST_FILTER="*Sorted*" src/mesos-tests
> 
> 
> Thanks,
> 
> Kevin Klues
> 
>

Re: Review Request 45963: Allow tasks to set persistent volume as readonly or readwrite resource.

[Anindya Sinha]

> On Oct. 7, 2016, 5:49 p.m., Greg Mann wrote:
> > src/slave/containerizer/mesos/isolators/filesystem/posix.hpp, line 55
> > 
> >
> > If you end up removing the check for persistent volumes in this 
> > function per Yan's comment, then perhaps we could also add a comment here 
> > saying that the function assumes that its input is a persistent volume. 
> > Renaming the resource parameter to `persistentVolume` could help make this 
> > clear as well.

I moved the functionality inline since the resource being looked into is a 
persistent volume.


> On Oct. 7, 2016, 5:49 p.m., Greg Mann wrote:
> > src/examples/persistent_shared_volume_framework.cpp, line 191
> > 
> >
> > s/used/uses/
> 
> Greg Mann wrote:
> It looks like this typo is still present? Or perhaps you meant to drop 
> the issue instead of resolve?

Will fix in https://reviews.apache.org/r/45962 when I update that. Please feel 
free to add that comment in https://reviews.apache.org/r/45962 so that it is 
not forgotten.


- Anindya


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


On Oct. 13, 2016, 5:24 a.m., Anindya Sinha wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45963/
> ---
> 
> (Updated Oct. 13, 2016, 5:24 a.m.)
> 
> 
> Review request for mesos, Greg Mann and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-4324
> https://issues.apache.org/jira/browse/MESOS-4324
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Allow the task to specify the persistent volume access to be read-only
> or read-write. Note that the persistent volume is always created as
> read-write.
> If the task is the first consumer of the shared persistent volume, then
> set the ownership of the persistent volume to match that of the task.
> Otherwise, launch the task but if the task is unable to read/write the
> persistent volume, it would fail at that point of time.
> 
> 
> Diffs
> -
> 
>   src/master/validation.cpp e5da3c9bf0a1042b42522f1ab74ce798fbb1738d 
>   src/slave/containerizer/docker.cpp d71386089bf7677872bcb1bd36e07da9263dcf0d 
>   src/slave/containerizer/mesos/isolators/filesystem/linux.cpp 
> 8f62162519f12a157c28ca5f2a76502e466c1481 
>   src/slave/containerizer/mesos/isolators/filesystem/posix.cpp 
> af427c6e5691f1770ab3ebef79502eb2c2176c4a 
>   src/tests/containerizer/linux_filesystem_isolator_tests.cpp 
> eb191a32381f9d1ca84ec29adf352dde375c2f2d 
>   src/tests/master_validation_tests.cpp 
> 99e350e0587e73e9ee25ef20dd369cd146bd446a 
> 
> Diff: https://reviews.apache.org/r/45963/diff/
> 
> 
> Testing
> ---
> 
> Tests successful.
> 
> 
> Thanks,
> 
> Anindya Sinha
> 
>

Re: Review Request 45967: Added documentation for shareable resources.

[Anindya Sinha]

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

(Updated Oct. 13, 2016, 5:25 a.m.)


Review request for mesos and Jiang Yan Xu.


Changes
---

Addressed review comments.


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


Repository: mesos


Description
---

Added documentation for shareable resources.


Diffs (updated)
-

  docs/home.md ad59eb1722b49cd72454795c095ddbd1ec1eb4dd 
  docs/shared-resources.md PRE-CREATION 

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


Testing
---

Tests successful.


Thanks,

Anindya Sinha

Re: Review Request 45963: Allow tasks to set persistent volume as readonly or readwrite resource.

[Anindya Sinha]

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

(Updated Oct. 13, 2016, 5:24 a.m.)


Review request for mesos, Greg Mann and Jiang Yan Xu.


Changes
---

Addressed review comments.


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


Repository: mesos


Description (updated)
---

Allow the task to specify the persistent volume access to be read-only
or read-write. Note that the persistent volume is always created as
read-write.
If the task is the first consumer of the shared persistent volume, then
set the ownership of the persistent volume to match that of the task.
Otherwise, launch the task but if the task is unable to read/write the
persistent volume, it would fail at that point of time.


Diffs (updated)
-

  src/master/validation.cpp e5da3c9bf0a1042b42522f1ab74ce798fbb1738d 
  src/slave/containerizer/docker.cpp d71386089bf7677872bcb1bd36e07da9263dcf0d 
  src/slave/containerizer/mesos/isolators/filesystem/linux.cpp 
8f62162519f12a157c28ca5f2a76502e466c1481 
  src/slave/containerizer/mesos/isolators/filesystem/posix.cpp 
af427c6e5691f1770ab3ebef79502eb2c2176c4a 
  src/tests/containerizer/linux_filesystem_isolator_tests.cpp 
eb191a32381f9d1ca84ec29adf352dde375c2f2d 
  src/tests/master_validation_tests.cpp 
99e350e0587e73e9ee25ef20dd369cd146bd446a 

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


Testing
---

Tests successful.


Thanks,

Anindya Sinha

Re: Review Request 45963: Allow tasks to set persistent volume as readonly or readwrite resource.

[Anindya Sinha]

> On Oct. 7, 2016, 4:49 p.m., Jiang Yan Xu wrote:
> > src/master/validation.hpp, lines 72-74
> > 
> >
> > Not quite sure what `isTaskContext` is?

We want to make sure persistent volumes when CREATEd have a mode as `RW`, but 
for tasks it can be either `RO` or `RW`. As discussed, we shall not check for 
mode in this function. Instead, that check shall be contained within 
`Option validatePersistentVolume(const RepeatedPtrField& 
volumes)` which is only called when validating volumes in `CREATE` or `DESTROY` 
calls.


> On Oct. 7, 2016, 4:49 p.m., Jiang Yan Xu wrote:
> > src/master/validation.cpp, lines 426-428
> > 
> >
> > Not sure about the meaning of `isTaskContext` but for simplicty I think 
> > we can just remove this check. Previously it didn't make sense to ever 
> > allow the volume to be RO but now the situation is changed with shared 
> > persistent volumes. Even when the persistent volume is not shared, the task 
> > is just going to fail if it attempts to write to a read-only volume so it 
> > seems no harm to me that we remove this check.

See previous response.


> On Oct. 7, 2016, 4:49 p.m., Jiang Yan Xu wrote:
> > src/slave/containerizer/mesos/isolators/filesystem/linux.cpp, lines 659-660
> > 
> >
> > What's the problem with the previous formatting?

It crosses 80 chars mainly because it is not in a nested block.


> On Oct. 7, 2016, 4:49 p.m., Jiang Yan Xu wrote:
> > src/slave/containerizer/mesos/isolators/filesystem/linux.cpp, lines 698-701
> > 
> >
> > This may not work, IIRC I needed to mount twice previously:
> > 
> > 
> > https://github.com/apache/mesos/blob/4ea9651aabd01f623f2578d2823271488d924c5b/src/slave/containerizer/mesos/provisioner/backends/bind.cpp#L133-L158
> > 
> > But could you verify? i.e., a unit test?

Great catch. Apparently, this inconsistency has been fixed in util-linux v2.27. 
Prior to this version, we need to do a bind mount followed by a remount as 
read-only. Post util-linux v2.27, a single bind mount as read-only should 
suffice.
So, I moved this to a 2-stage mount.


- Anindya


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


On Oct. 13, 2016, 5:24 a.m., Anindya Sinha wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45963/
> ---
> 
> (Updated Oct. 13, 2016, 5:24 a.m.)
> 
> 
> Review request for mesos, Greg Mann and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-4324
> https://issues.apache.org/jira/browse/MESOS-4324
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Allow the task to specify the persistent volume access to be read-only
> or read-write. Note that the persistent volume is always created as
> read-write.
> If the task is the first consumer of the shared persistent volume, then
> set the ownership of the persistent volume to match that of the task.
> Otherwise, launch the task but if the task is unable to read/write the
> persistent volume, it would fail at that point of time.
> 
> 
> Diffs
> -
> 
>   src/master/validation.cpp e5da3c9bf0a1042b42522f1ab74ce798fbb1738d 
>   src/slave/containerizer/docker.cpp d71386089bf7677872bcb1bd36e07da9263dcf0d 
>   src/slave/containerizer/mesos/isolators/filesystem/linux.cpp 
> 8f62162519f12a157c28ca5f2a76502e466c1481 
>   src/slave/containerizer/mesos/isolators/filesystem/posix.cpp 
> af427c6e5691f1770ab3ebef79502eb2c2176c4a 
>   src/tests/containerizer/linux_filesystem_isolator_tests.cpp 
> eb191a32381f9d1ca84ec29adf352dde375c2f2d 
>   src/tests/master_validation_tests.cpp 
> 99e350e0587e73e9ee25ef20dd369cd146bd446a 
> 
> Diff: https://reviews.apache.org/r/45963/diff/
> 
> 
> Testing
> ---
> 
> Tests successful.
> 
> 
> Thanks,
> 
> Anindya Sinha
> 
>

Re: Review Request 49571: Added a benchmark test for allocations.

[Anindya Sinha]

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

(Updated Oct. 13, 2016, 5:24 a.m.)


Review request for mesos and Jiang Yan Xu.


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


Repository: mesos


Description
---

Allocations test has the following resource configurations:
(1) REGULAR: Offers from every slave have regular resources.
(2) SHARED: Offers from every slave include a shared resource.
(3) REGULAR: Offers from every alternate slave contain only regular
resources; and offers from every other alternate slave contains
a shared resource.

This test is parameterized based on number of agents, number of
frameworks and resource configuration.


Diffs
-

  src/tests/hierarchical_allocator_tests.cpp 
46553f6d501deb37862dd5ba48be1c114f6e6cb8 

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


Testing
---

All tests passed.

Allocations benchmark test results
==
Support of shared resources has a small impact (roughly 10%) on runtime 
performance in allocations as compared to HEAD (without shared resources). 
Also, there is no visible impact in performance when shared resources are added 
in the tests.

Following is a snapshot with 1000 agents and 200 frameworks.

With the patch (and no shared resources)

[ RUN  ] AllResources/HierarchicalAllocations_BENCHMARK_Test.Allocations/9
Using 1000 agents and 200 frameworks with resource type 0
Added 200 frameworks in 6907us
Added 1000 agents in 2.057098secs
round 0 allocate() took 1.689164secs to make 1000 offers
round 50 allocate() took 1.672373secs to make 1000 offers
round 100 allocate() took 1.680571secs to make 1000 offers
round 150 allocate() took 1.674683secs to make 1000 offers
round 199 allocate() took 1.671525secs to make 1000 offers

With the patch (and shared resources on all agents)
---
[ RUN  ] AllResources/HierarchicalAllocations_BENCHMARK_Test.Allocations/10
Using 1000 agents and 200 frameworks with resource type 1
Added 200 frameworks in 6888us
Added 1000 agents in 2.096218secs
round 0 allocate() took 1.704491secs to make 1000 offers
round 50 allocate() took 1.718623secs to make 1000 offers
round 100 allocate() took 1.716224secs to make 1000 offers
round 150 allocate() took 1.707343secs to make 1000 offers
round 199 allocate() took 1.727467secs to make 1000 offers

With the patch (and shared resources on alternate agents)
-
[ RUN  ] AllResources/HierarchicalAllocations_BENCHMARK_Test.Allocations/11
Using 1000 agents and 200 frameworks with resource type 2
Added 200 frameworks in 7304us
Added 1000 agents in 2.071009secs
round 0 allocate() took 1.689045secs to make 1000 offers
round 50 allocate() took 1.691524secs to make 1000 offers
round 100 allocate() took 1.688873secs to make 1000 offers
round 150 allocate() took 1.688713secs to make 1000 offers
round 199 allocate() took 1.691223secs to make 1000 offers

Based on HEAD, with all regular resources (no shared resources in HEAD 
supported)
-
[ RUN  ] AllResources/HierarchicalAllocations_BENCHMARK_Test.Allocations/9
Using 1000 agents and 200 frameworks with resource type 0
Added 200 frameworks in 6801us
Added 1000 agents in 1.721447secs
round 0 allocate() took 1.502953secs to make 1000 offers
round 50 allocate() took 1.520157secs to make 1000 offers
round 100 allocate() took 1.517221secs to make 1000 offers
round 150 allocate() took 1.526446secs to make 1000 offers
round 199 allocate() took 1.538005secs to make 1000 offers


Thanks,

Anindya Sinha

Re: Review Request 52810: Added tests to test usernamespaces.

[Mesos ReviewBot]

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



Bad patch!

Reviews applied: [52810, 52809]

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

Error:
2016-10-13 04:43:20 URL:https://reviews.apache.org/r/52809/diff/raw/ 
[21623/21623] -> "52809.patch" [1]
error: patch failed: src/slave/flags.hpp:67
error: src/slave/flags.hpp: patch does not apply

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

- Mesos ReviewBot


On Oct. 12, 2016, 11:01 p.m., Srinivas Brahmaroutu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52810/
> ---
> 
> (Updated Oct. 12, 2016, 11:01 p.m.)
> 
> 
> Review request for mesos, Gilbert Song and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> WIP : Added tests to test usernamespaces.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/isolator_tests.cpp 
> 8fefeef8c83ed2ab01f56a1ec572d3acb307143c 
> 
> Diff: https://reviews.apache.org/r/52810/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Srinivas Brahmaroutu
> 
>

Review Request 52814: Added notes to the getting started and upgrade docs about --runtime_dir.

[Kevin Klues]

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

Review request for mesos, Alexander Rukletsov, Jie Yu, Till Toenshoff, and 
Vinod Kone.


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


Repository: mesos


Description
---

Added notes to the getting started and upgrade docs about --runtime_dir.


Diffs
-

  docs/getting-started.md de3fba4a821ca7fa9b9e2fd692b33b40edd35b68 
  docs/upgrades.md 7250259590db322121a0f30d1cdb0f3732a7a6ee 

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


Testing
---


Thanks,

Kevin Klues

Re: Review Request 52311: Pass the user value from executor of switch_user flag is set.

[Joseph Wu]

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




src/slave/slave.cpp (line 4219)


You don't need this `if` statement here, since you check this boolean in 
each place below.



src/slave/slave.cpp (line 4230)


flags.switch_user && !customExecutor.command().has_user()


- Joseph Wu


On Oct. 11, 2016, 11:50 a.m., Sivaram Kannan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52311/
> ---
> 
> (Updated Oct. 11, 2016, 11:50 a.m.)
> 
> 
> Review request for mesos and Joseph Wu.
> 
> 
> Bugs: MESOS-5856
> https://issues.apache.org/jira/browse/MESOS-5856
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Pass the user value from executor of switch_user flag is set.
> 
> 
> Diffs
> -
> 
>   src/slave/slave.cpp 119fb36c27b25739f2a86a55d48e964ca4a84ff7 
> 
> Diff: https://reviews.apache.org/r/52311/diff/
> 
> 
> Testing
> ---
> 
> 1. Run the mesos-logrotate-logger with un-priviledged user and verify whether 
> the logs are getting rotated. 
> 2. Run the mesos-logrotate-logger as root user and verify whether the logs 
> are getting rotated.
> 
> 
> Thanks,
> 
> Sivaram Kannan
> 
>

Re: Review Request 51617: Added the `remove` and `insert` methods.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [51736, 51737, 51740, 51767, 51768, 51769, 51486, 51617]

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 Oct. 12, 2016, 8:55 p.m., Avinash sridharan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51617/
> ---
> 
> (Updated Oct. 12, 2016, 8:55 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Qian Zhang.
> 
> 
> Bugs: MESOS-6023
> https://issues.apache.org/jira/browse/MESOS-6023
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added the `remove` and `insert` methods.
> 
> 
> Diffs
> -
> 
>   
> src/slave/containerizer/mesos/isolators/network/cni/plugins/port_mapper/port_mapper.hpp
>  7fad707a240234e35828917aea1bc79f42fe130e 
>   
> src/slave/containerizer/mesos/isolators/network/cni/plugins/port_mapper/port_mapper.cpp
>  04736451481506f17c462b35fa579f6546c6d6ce 
> 
> Diff: https://reviews.apache.org/r/51617/diff/
> 
> 
> Testing
> ---
> 
> Ran the CNI plugin against a network namespace with the following JSON input:
> ```
> {
> "name": "mynet",
> "type": "port-mapper",
> "chain": "MESOS-TEST",
> "excludeDevices": ["mesos-cni0"],
> "delegate": {
>   "type" : "bridge",
>   "bridge": "cni0",
>   "isGateway": true,
>   "ipMasq": true,
>   "ipam": {
>   "type": "host-local",
>   "subnet": "192.168.37.0/24",
>   "routes": [
> { "dst": "0.0.0.0/0" }
>   ]
>   }
> },
> "args" : {
>   "org.apache.mesos" : {
> "network_info" : {
>   "port_mappings": {
> "host_port" : 8080,
> "container_port" : 9000
>   }
> }
>   }
> }
> }
> ```
> 
> Used the ADD command to test that the CNI plugin correctly invokes the 
> delegate plugin (a CNI bridge plugin in this case) and also inserts the 
> correct iptable entries for the given port mapping. After running this 
> plugin, this was the output of the `iptables -t nat -S MESOS-TEST` command:
> ```
> sudo iptables -t nat -S MESOS-TEST
> -N MESOS-TEST
> -A MESOS-TEST ! -i mesos-cni0 -p tcp -m tcp --dport 8080 -j DNAT 
> --to-destination 192.168.37.21:9000
> ```
> 
> Ran a python HTTP server in this network namespace and verified that DNAT 
> works from outside the box. Was able to connect to port 9000 of this server, 
> by connecting to port 8080 on the host.
> 
> Used the DEL command to test the CNI plugin correctly deletes the DNAT rule 
> and chain, if there are no DNAT rules exist in the chain. After running the 
> DEL command (by injecting `NetworkInfo` into the above JSON schema) verified 
> the chain and the DNAT rule is deleted from iptables.
> 
> 
> Thanks,
> 
> Avinash sridharan
> 
>

Re: Review Request 52755: Made default executor handle shutdown events while disconnected.

[Vinod Kone]

> On Oct. 12, 2016, 10:52 p.m., Vinod Kone wrote:
> > src/launcher/default_executor.cpp, line 700
> > 
> >
> > can you add a comment on how this can happen when we are in `CONNECTED` 
> > state? it's not obvious to me.
> 
> Anand Mazumdar wrote:
> Sure, will do. It can happen in `CONNECTED` state if the agent explicitly 
> asks the executor to shut down if it's not able to subscribe within the 
> re-register timeout for some reason.

i'll add it while committing.


- Vinod


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


On Oct. 12, 2016, 12:15 a.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52755/
> ---
> 
> (Updated Oct. 12, 2016, 12:15 a.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-6363
> https://issues.apache.org/jira/browse/MESOS-6363
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Previously, the default executor used to crash with a failed assertion
> when the executor library injected a shutdown event when it noticed
> a disconnection with the agent for non-checkpointed frameworks and
> upon recovery timeout for checkpointed frameworks. This change
> modifies it to commit suicide minus the failed assertion.
> 
> 
> Diffs
> -
> 
>   src/launcher/default_executor.cpp 2454bd7df608254af72af01460782f5ab78a19c1 
> 
> Diff: https://reviews.apache.org/r/52755/diff/
> 
> 
> Testing
> ---
> 
> Manual testing (It's hard to test this directly owing to the executor library 
> immediately invoking the `connected()` callback since the libprocess instance 
> initialized in tests is always active!)
> 
> Filed MESOS-6373 to address this.
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Re: Review Request 52756: Invoke the shutdown executor callback for checkpointed frameworks.

[Vinod Kone]

> On Oct. 12, 2016, 10:56 p.m., Vinod Kone wrote:
> > src/executor/executor.cpp, line 734
> > 
> >
> > I think this should call `_shutdown()` to ensure ShutdownProcess is 
> > spawned  whenever we want to shutdown.
> 
> Anand Mazumdar wrote:
> The actual plumbing happens through the `receive()` handler that first 
> invokes the callback on the executor and then invokes `_shutdown()`. All 
> other places in the code use `shutdown()` to inject the `SHUTDOWN` event.

gotcha.


- Vinod


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


On Oct. 12, 2016, 12:15 a.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52756/
> ---
> 
> (Updated Oct. 12, 2016, 12:15 a.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-6370
> https://issues.apache.org/jira/browse/MESOS-6370
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Previously, the executor library used to commit suicide after the
> recovery timeout without invoking the executor's shutdown callback.
> This behavior was not consistent with the executor driver.
> 
> 
> Diffs
> -
> 
>   src/executor/executor.cpp a0e5d8382fa3886b747ae92507dce17e75c2d749 
> 
> Diff: https://reviews.apache.org/r/52756/diff/
> 
> 
> Testing
> ---
> 
> Manual testing (It's hard to test this directly owing to the executor library 
> immediately invoking the `connected()` callback since the libprocess instance 
> initialized in tests is always active!)
> 
> Filed MESOS-6373 to address this.
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Re: Review Request 52755: Made default executor handle shutdown events while disconnected.

[Anand Mazumdar]

> On Oct. 12, 2016, 10:52 p.m., Vinod Kone wrote:
> > src/launcher/default_executor.cpp, line 700
> > 
> >
> > can you add a comment on how this can happen when we are in `CONNECTED` 
> > state? it's not obvious to me.

Sure, will do. It can happen in `CONNECTED` state if the agent explicitly asks 
the executor to shut down if it's not able to subscribe within the re-register 
timeout for some reason.


- Anand


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


On Oct. 12, 2016, 12:15 a.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52755/
> ---
> 
> (Updated Oct. 12, 2016, 12:15 a.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-6363
> https://issues.apache.org/jira/browse/MESOS-6363
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Previously, the default executor used to crash with a failed assertion
> when the executor library injected a shutdown event when it noticed
> a disconnection with the agent for non-checkpointed frameworks and
> upon recovery timeout for checkpointed frameworks. This change
> modifies it to commit suicide minus the failed assertion.
> 
> 
> Diffs
> -
> 
>   src/launcher/default_executor.cpp 2454bd7df608254af72af01460782f5ab78a19c1 
> 
> Diff: https://reviews.apache.org/r/52755/diff/
> 
> 
> Testing
> ---
> 
> Manual testing (It's hard to test this directly owing to the executor library 
> immediately invoking the `connected()` callback since the libprocess instance 
> initialized in tests is always active!)
> 
> Filed MESOS-6373 to address this.
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Re: Review Request 52756: Invoke the shutdown executor callback for checkpointed frameworks.

[Anand Mazumdar]

> On Oct. 12, 2016, 10:56 p.m., Vinod Kone wrote:
> > src/executor/executor.cpp, line 734
> > 
> >
> > I think this should call `_shutdown()` to ensure ShutdownProcess is 
> > spawned  whenever we want to shutdown.

The actual plumbing happens through the `receive()` handler that first invokes 
the callback on the executor and then invokes `_shutdown()`. All other places 
in the code use `shutdown()` to inject the `SHUTDOWN` event.


- Anand


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


On Oct. 12, 2016, 12:15 a.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52756/
> ---
> 
> (Updated Oct. 12, 2016, 12:15 a.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-6370
> https://issues.apache.org/jira/browse/MESOS-6370
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Previously, the executor library used to commit suicide after the
> recovery timeout without invoking the executor's shutdown callback.
> This behavior was not consistent with the executor driver.
> 
> 
> Diffs
> -
> 
>   src/executor/executor.cpp a0e5d8382fa3886b747ae92507dce17e75c2d749 
> 
> Diff: https://reviews.apache.org/r/52756/diff/
> 
> 
> Testing
> ---
> 
> Manual testing (It's hard to test this directly owing to the executor library 
> immediately invoking the `connected()` callback since the libprocess instance 
> initialized in tests is always active!)
> 
> Filed MESOS-6373 to address this.
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Re: Review Request 52569: Updated the nested container launch to correctly determine the user.

[Benjamin Mahler]

> On Oct. 12, 2016, 6:46 p.m., Gilbert Song wrote:
> > src/slave/http.cpp, lines 1964-1965
> > 
> >
> > Would you mind removing this `TODO`? They are no longer valid.
> 
> Gilbert Song wrote:
> I will follow up with a patch. Sorry not review on time.

Done, thanks for catching this!


- Benjamin


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


On Oct. 5, 2016, 8:45 p.m., Benjamin Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52569/
> ---
> 
> (Updated Oct. 5, 2016, 8:45 p.m.)
> 
> 
> Review request for mesos, Gilbert Song and Vinod Kone.
> 
> 
> Bugs: MESOS-6289
> https://issues.apache.org/jira/browse/MESOS-6289
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This locates the executor that corresponds to the parent container,
> and uses the user that was used to launch the executor.
> 
> 
> Diffs
> -
> 
>   src/slave/http.cpp bdafe1db9f1b22ac0761f5119e73f87b531c6a3c 
>   src/slave/slave.hpp 9beefa12283e936684d82c97ebd3ac6506475edf 
>   src/slave/slave.cpp fba089506850eec87f67a46694a1ef645cdf4ad5 
>   src/tests/api_tests.cpp 7b0ad3c18a38b5ec859a79d09f5707f6958960fb 
> 
> Diff: https://reviews.apache.org/r/52569/diff/
> 
> 
> Testing
> ---
> 
> Now that the executor is looked up, the tests have been updated to
> launch an executor and use it's container id as the parent.
> 
> 
> Thanks,
> 
> Benjamin Mahler
> 
>

Re: Review Request 51620: Removed two std::move in MountInfoTable::read.

[Kevin Klues]

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




src/linux/fs.cpp (line 145)


If you remove the `std::move()`, then you can remove this temporary 
variable reference.


- Kevin Klues


On Sept. 3, 2016, 12:25 a.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51620/
> ---
> 
> (Updated Sept. 3, 2016, 12:25 a.m.)
> 
> 
> Review request for mesos and Kevin Klues.
> 
> 
> Bugs: MESOS-6118
> https://issues.apache.org/jira/browse/MESOS-6118
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Since 'entry' is a const reference, the std::move won't reduce the
> extra copying. This patch removed this optimization.
> 
> 
> Diffs
> -
> 
>   src/linux/fs.cpp 14ae5a9089916549f691363bc2269e13c5260a14 
> 
> Diff: https://reviews.apache.org/r/51620/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 52706: Updated mesos containerizer to use new 'stout/wait.hpp' header.

[Kevin Klues]

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

(Updated Oct. 13, 2016, 12:08 a.m.)


Review request for mesos and Jie Yu.


Changes
---

Updated based on Jie and Alex's comments.


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


Repository: mesos


Description
---

Updated mesos containerizer to use new 'stout/wait.hpp' header.


Diffs (updated)
-

  src/slave/containerizer/mesos/containerizer.cpp 
e6bd9f7a8284d220be157a3db2da094e6b1b6d33 

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


Testing
---

sudo GTEST_FILTER="*Nested*" src/mesos-tests


Thanks,

Kevin Klues

Re: Review Request 52705: Added 'stout/wait.hpp' header.

[Kevin Klues]

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

(Updated Oct. 13, 2016, 12:08 a.m.)


Review request for mesos and Jie Yu.


Changes
---

Updated based on Jie and Alex's comments.


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


Repository: mesos


Description
---

This was motivated by the need for a default definition of
'W_EXITCODE' (since it is not technically POSIX compliant).


Diffs (updated)
-

  3rdparty/stout/include/Makefile.am b0b08d8e0d284a88bc8daa4570540659b94dc2d0 
  3rdparty/stout/include/stout/os/wait.hpp PRE-CREATION 

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


Testing
---

sudo GTEST_FILTER="*Nested*" src/mesos-tests


Thanks,

Kevin Klues

Re: Review Request 52705: Added 'stout/wait.hpp' header.

[Kevin Klues]

> On Oct. 11, 2016, 8:48 p.m., Alex Clemmer wrote:
> > 3rdparty/stout/include/stout/wait.hpp, line 17
> > 
> >
> > How much work is it to do this now? If we're going to have a dedicated 
> > header, I would greatly prefer to put all the definitions in one place.
> 
> Kevin Klues wrote:
> See comment below for why it probably makes sense to just leave things as 
> they are for now

See comment below for rationale.


- Kevin


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


On Oct. 10, 2016, 9:22 p.m., Kevin Klues wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52705/
> ---
> 
> (Updated Oct. 10, 2016, 9:22 p.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-6310
> https://issues.apache.org/jira/browse/MESOS-6310
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This was motivated by the need for a default definition of
> 'W_EXITCODE' (since it is not technically POSIX compliant).
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/wait.hpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/52705/diff/
> 
> 
> Testing
> ---
> 
> sudo GTEST_FILTER="*Nested*" src/mesos-tests
> 
> 
> Thanks,
> 
> Kevin Klues
> 
>

Re: Review Request 52705: Added 'stout/wait.hpp' header.

[Kevin Klues]

> On Oct. 11, 2016, 8:48 p.m., Alex Clemmer wrote:
> > 3rdparty/stout/include/stout/wait.hpp, line 28
> > 
> >
> > I've been out of the game awhile-- are we wrapping comments at 70 
> > characters these days?

I typically use a 72 character wrap for comments because it looks nicer. I only 
go up to (or near to) 80 if the line I'm commenting is around 80 characters 
itself.


> On Oct. 11, 2016, 8:48 p.m., Alex Clemmer wrote:
> > 3rdparty/stout/include/stout/wait.hpp, line 25
> > 
> >
> > Couple of questions here:
> > 
> > * This seems useful for POSIX platforms, but I don't really understand 
> > the implications of having this enabled on Windows. Windows will never 
> > support the signal semantics of POSIX, and in general my feeling is that 
> > I'd rather not define things on platforms that don't support those 
> > semantics, unless there is a particularly pertinent reason to. Thoughts?
> > * In the 2 or 3 places we do use `W_EXITCODE`, rather than defining it 
> > for Windows, we simply `#ifdef` the code out. So the precedent is to just 
> > not use the macro. Is there a compelling reason to change this?
> > 
> > 
> > Also, tiny, tiny nit question: is it style to have 2 spaces between 
> > `W_EXITCODE(ret, sig)` and `((ret) << 8 | (sig))`

To answer both your bullets at once...
If you look in `windows.hpp` all of the other `W*` macros from the posix 
standard are defined in there (even though they may be meaningless on windows). 
This is consistent with them all being defined in `sys/wait.h` on a posix 
compliant system. This is why I separated the two by a simple `#ifdef`

Adding the code below to be added to both windows and poix systems:

```
#ifndef W_EXITCODE
#define W_EXITCODE(ret, sig)  ((ret) << 8 | (sig))
#endif
```

is just to cover a corner case where some libc variants don't actually define 
this macro (because it's not technically posix compliant). Almost all libc 
variants do define it, but a bug was filed against this because (apparently) 
`musl` does not.

Given that adding this code makes both windows and any variant of libc now 
define all of the `W*` macros symetrically, I think this is likely the right 
way to do it for now. In the future when we completely strip out all `W*` 
macros from windows, we can revisit this. 

---

Regarding the formatting question. I just copy and pasted this from the glibc 
header file. I don't think we have a preferred style for this, but I tried it 
both ways just now, and it seems to be more readable as 2 spaces, so I'll leave 
it.


> On Oct. 11, 2016, 8:48 p.m., Alex Clemmer wrote:
> > 3rdparty/stout/include/stout/wait.hpp, line 17
> > 
> >
> > How much work is it to do this now? If we're going to have a dedicated 
> > header, I would greatly prefer to put all the definitions in one place.

See comment below for why it probably makes sense to just leave things as they 
are for now


- Kevin


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


On Oct. 10, 2016, 9:22 p.m., Kevin Klues wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52705/
> ---
> 
> (Updated Oct. 10, 2016, 9:22 p.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-6310
> https://issues.apache.org/jira/browse/MESOS-6310
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This was motivated by the need for a default definition of
> 'W_EXITCODE' (since it is not technically POSIX compliant).
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/wait.hpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/52705/diff/
> 
> 
> Testing
> ---
> 
> sudo GTEST_FILTER="*Nested*" src/mesos-tests
> 
> 
> Thanks,
> 
> Kevin Klues
> 
>

Re: Review Request 45962: Added a persistent volume test framework for shared volumes.

[Anindya Sinha]

> On Oct. 7, 2016, 5:20 p.m., Greg Mann wrote:
> > Given how similar the code in the new example framework is to the existing 
> > persistent volumes framework, I think it could make sense to add shared 
> > volume features to the existing framework, which could be enabled/disabled 
> > via command-line flags. Especially since you add some nice new DESTROY 
> > logic in the new framework; it would be great to have that change in the 
> > existing one, and avoid duplicating a lot of code. What do you think?

Agreed. Makes sense. Will do that in the next update.


- Anindya


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


On Sept. 28, 2016, 12:15 a.m., Anindya Sinha wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45962/
> ---
> 
> (Updated Sept. 28, 2016, 12:15 a.m.)
> 
> 
> Review request for mesos and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-4431
> https://issues.apache.org/jira/browse/MESOS-4431
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added a persistent volume test framework for shared volumes.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am fba488f9d676851dd046a8b8c7dd175b3c0d9ef0 
>   src/examples/persistent_shared_volume_framework.cpp PRE-CREATION 
>   src/tests/examples_tests.cpp 52fac33733cc86dd718c7836c5031999f1597a5c 
>   src/tests/persistent_shared_volume_framework_test.sh PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/45962/diff/
> 
> 
> Testing
> ---
> 
> New test framework for shared resources added.
> Tests successful.
> 
> 
> Thanks,
> 
> Anindya Sinha
> 
>

Re: Review Request 45967: Added documentation for shareable resources.

[Jiang Yan Xu]

> On Oct. 12, 2016, 4:13 p.m., Jiang Yan Xu wrote:
> > docs/shared-resources.md, line 175
> > 
> >
> > We can add a section for things that aren't in the doc for regular 
> > persistent volumes. i.e.,
> > 
> > ---
> > 
> > ## Unique Shared Persistent Volume Features
> > 
> > ### Launching multiple tasks on the shared persistent volume
> > 
> > ### Destroying shared persistent volumes
> >  > to be more cautious about this for shared persistent volumes>
> > 
> > ---
> > 
> > I feel these are what make a separate doc for shared persistent volumes 
> > necessary (and we can continue to add more topics). Otherwise we could just 
> > bundle everything into the persistent volume doc.

The fomatting is messed up, I meant

```
## Unique Shared Persistent Volume Features


### Launching multiple tasks on the same shared persistent volume

### Destroying shared persistent volumes

```


- Jiang Yan


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


On Sept. 27, 2016, 5:16 p.m., Anindya Sinha wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45967/
> ---
> 
> (Updated Sept. 27, 2016, 5:16 p.m.)
> 
> 
> Review request for mesos and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-4325
> https://issues.apache.org/jira/browse/MESOS-4325
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added documentation for shareable resources.
> 
> 
> Diffs
> -
> 
>   docs/home.md ad59eb1722b49cd72454795c095ddbd1ec1eb4dd 
>   docs/shared-resources.md PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/45967/diff/
> 
> 
> Testing
> ---
> 
> Tests successful.
> 
> 
> Thanks,
> 
> Anindya Sinha
> 
>

Re: Review Request 45967: Added documentation for shareable resources.

[Jiang Yan Xu]

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



High-level comments:

1. We should tailor this towards the users/operators with a brief intro and 
remove things they don't care. We can link to the Epic and your MesosCon talk 
for more technical discussions.
2. We should write the doc as if we started writing it today, explaining all 
features already checked in. So we are not proposing things, nor should we 
include things that are still in the works (add them later).


docs/home.md (line 54)


s/Shareable/Shared/

also, emphasize the usage in persistent volume? i.e.,

"allow tasks to set persistent volumes as shared"?



docs/shared-resources.md (line 5)


I think we can 

s/Shared Resources/Shared Persistent Volumes/

even though we call this document shared-resources.md (for the reasons 
below).



docs/shared-resources.md (lines 9 - 20)


Shared resources is an abstract concept and we have yet to generalize it in 
a way that it can be applied elsewhere other than the persistent volumes. 

Since the focus of this document is on using shared persistent volumes, I 
think it's more clear if we for now make it the only topic (until we can 
articulate it better to non-messo-core developers)

We should jump right into "usage" after a very brief into. We just need to 
explain some basics first:

- Access to regular persistent volumes are exclusive to one 
container/executor at a time.
- Shared persistent volumes allow multiple executor/containers access to 
the same persistent volume simultaneously.
- Simulatenous accesses to the same shared persistent volume are not 
isolated.



docs/shared-resources.md (line 11)


`RW` not relevant here?



docs/shared-resources.md (line 12)


`task` here is not precise, `executor/container` is. Shared persistent 
volume allows access from different containers, we can stick to this 
terminology so the distinction between this feature and the Pods is clear 
(without us needing to expand on that here).



docs/shared-resources.md (lines 29 - 36)


No longer a proposal. We should describe that now that this feature exsits, 
how should the user leverage it.



docs/shared-resources.md (lines 40 - 48)


This is not done and is potentially not going to be done this way 
(MESOS-6374). I think we can avoid saying too much about it for now. We can 
always add a paragraph when it MESOS-4324 is done.



docs/shared-resources.md (lines 50 - 54)


I don't think we need to emphasize this here. Multiple tasks with the same 
executor can access the same regular persistent volume and overwrite each 
other's content whereas if multiple containers strictly want to access 
different sub-directories of a persistent volume, they gain nothing by sharing 
a persistent volume.



docs/shared-resources.md (lines 58 - 65)


The doc should be written in either the user's or the "framework"'s 
perspective, not "we" as in "Mesos".



docs/shared-resources.md (lines 67 - 71)


We should assume `SharedInfo` is already a thing (not a new thing, as 
compared to e.g., `RecocableInfo`, as least the operator shouldn't care) and we 
are just informing users how to use it.

So I suggest we just skip this section jump into the usage examples.



docs/shared-resources.md (lines 73 - 75)


Here we haven't included the scheduler DESTROY API or HTTP endpoints, 
presumably because they will be the duplicate of what's in the persistent 
volume doc, which I agree we shouldn't do.

So I think we should just make it clear that we are only going to highlight 
a few things different than the regular persistent volumes in terms of usage, 
but with examples. Then we can refer the readers to the persistent volume doc 
for details.

If we are not separately explaning all APIs, we probably don't need the `## 
Framework Scheduler API` heading.



docs/shared-resources.md (lines 77 - 79)


To avoid duplicating the regular persistent volume doc, how about the 
following for using shared persistent volumes? 

---

The framework can create a shared persistent volume using the existing 
persistent 

Review Request 52811: Updated 'MountInfoTableReadSorted' test to use a hashset.

[Kevin Klues]

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

Review request for mesos and Jie Yu.


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


Repository: mesos


Description
---

Updated 'MountInfoTableReadSorted' test to use a hashset.


Diffs
-

  src/tests/containerizer/fs_tests.cpp 0dd212f94d2845494163a86bc2059f999e018cd0 

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


Testing
---

GTEST_FILTER="*Sorted*" src/mesos-tests


Thanks,

Kevin Klues

Re: Review Request 51052: Make mesos-docker-execute understand cgroups_enable_cfs.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [51052]

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 Oct. 12, 2016, 5:02 p.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51052/
> ---
> 
> (Updated Oct. 12, 2016, 5:02 p.m.)
> 
> 
> Review request for mesos, Gilbert Song, haosdent huang, and Jie Yu.
> 
> 
> Bugs: MESOS-6134
> https://issues.apache.org/jira/browse/MESOS-6134
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This fixes cpu quota for command executor (which runs outside
> of the docker container) by ensuing --cpu-quota flag to docker
> run.
> 
> Note: we have to add the boolean variable to `Docker` class
> because `Docker::run()` has reached the maximum argument length
> GMOCK can support.
> 
> 
> Diffs
> -
> 
>   src/docker/docker.hpp 9093371afc8ea792ba94f61c6875703e547ea6b0 
>   src/docker/docker.cpp 50fda393a42afefc70790a26b44911e4cf17185e 
>   src/docker/executor.hpp a49ec1b4045116741af6af08578791fb0440ad8f 
>   src/docker/executor.cpp ab3f0473fdc9105d1c425f0dbe7b81c566d541e8 
>   src/slave/containerizer/docker.cpp d71386089bf7677872bcb1bd36e07da9263dcf0d 
>   src/tests/mock_docker.cpp 6a0e613bde6889864a37ffd7ec0b454e5fe4df1c 
> 
> Diff: https://reviews.apache.org/r/51052/diff/
> 
> 
> Testing
> ---
> 
> I am now able to make docker containers launched through mesos-execute have a 
> cpu quota.
> 
> Also making sure `make check` still works on mac os for the linux only flag.
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>

Re: Review Request 52703: Added test to test corner cases with sorted 'MountInfoTable::read()'.

[Kevin Klues]

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

(Updated Oct. 12, 2016, 11:11 p.m.)


Review request for mesos and Jie Yu.


Changes
---

Addressed Jie's comments.


Summary (updated)
-

Added test to test corner cases with sorted 'MountInfoTable::read()'.


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


Repository: mesos


Description (updated)
---

We allow entries in the MountInfoTable to be out of order, as well as
parent's of themselves. This test makes sure that this functionality
is exercised.


Diffs (updated)
-

  src/tests/containerizer/fs_tests.cpp 0dd212f94d2845494163a86bc2059f999e018cd0 

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


Testing
---

GTEST_FILTER="*Sorted*" src/mesos-tests


Thanks,

Kevin Klues

Re: Review Request 52704: Refactored 'MountInfoTable::read()' into two separate functions.

[Kevin Klues]

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

(Updated Oct. 12, 2016, 11:10 p.m.)


Review request for mesos and Jie Yu.


Changes
---

Addressed Jie's comments.


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


Repository: mesos


Description
---

The original function now calls a helper which takes a string
representation of a 'MountInfoTable'. In a subsequent commit we will
use this helper to write more meaningful tests to stress the core
logic of the 'MountInfoTable::read()' functionality.


Diffs (updated)
-

  src/linux/fs.hpp 090c82616d965b1cedf9bb13ffcde2d37304a1f7 
  src/linux/fs.cpp 4b10141a49dfb3c6defdb07e295eb14cfcdd36ce 

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


Testing
---

GTEST_FILTER="*Sorted*" src/mesos-tests


Thanks,

Kevin Klues

Re: Review Request 52596: Added special case when sorting hierarchically in MountInfoTable::read.

[Kevin Klues]

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

(Updated Oct. 12, 2016, 11:09 p.m.)


Review request for mesos and Jie Yu.


Changes
---

It turns out that I can't preemptively keep a self reference out of the child 
list, otherwise the algorithm for adding entries back into the MountInfoTable 
vector doesn't work as expected.  Instead, I add the self reference into the 
child list, but don't recurse down into it once it is visited.


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


Repository: mesos


Description
---

It is legal to have entries in a `MountInfoTable` whose `entry.id` is
the same as `entry.parent`. This can happen (for example), if a system
boots from the network and then keeps the original `/` in RAM.
However, to avoid cycles when walking the mount hierarchy, we should
not treat these entries as children of their parent so we skip them.

This commit adds functionality to handle this case.


Diffs (updated)
-

  src/linux/fs.cpp 4b10141a49dfb3c6defdb07e295eb14cfcdd36ce 

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


Testing
---

GTEST_FILTER="" make -j40 check
GTEST_FILTER="FsTest.MountInfoTableReadSorted" src/mesos-tests


Thanks,

Kevin Klues

Re: Review Request 52597: Added more detailed error message when failing in MountInfoTable::read.

[Kevin Klues]

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

(Updated Oct. 12, 2016, 11:05 p.m.)


Review request for mesos and Jie Yu.


Changes
---

Updated error message with even more information (including a newline between 
the output and the mount table lines that are printed).


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


Repository: mesos


Description
---

Added more detailed error message when failing in MountInfoTable::read.


Diffs (updated)
-

  src/linux/fs.cpp 4b10141a49dfb3c6defdb07e295eb14cfcdd36ce 

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


Testing
---

make


Thanks,

Kevin Klues

Review Request 52810: Added tests to test usernamespaces.

[Srinivas Brahmaroutu]

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

Review request for mesos, Gilbert Song and Jie Yu.


Repository: mesos


Description
---

WIP : Added tests to test usernamespaces.


Diffs
-

  src/tests/containerizer/isolator_tests.cpp 
8fefeef8c83ed2ab01f56a1ec572d3acb307143c 

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


Testing
---


Thanks,

Srinivas Brahmaroutu

Review Request 52809: User Namespace implementation.

[Srinivas Brahmaroutu]

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

Review request for mesos, Gilbert Song and Jie Yu.


Repository: mesos


Description
---

WIP: User Namespace implementation.


Diffs
-

  src/Makefile.am fd01e1dfbdb04d073484ff6b7cc94b8d769f8a8e 
  src/slave/containerizer/mesos/containerizer.cpp 
32058c35ea9ca95f0a2665994c1ebccd5c840345 
  src/slave/containerizer/mesos/isolators/user/user.hpp PRE-CREATION 
  src/slave/containerizer/mesos/isolators/user/user.cpp PRE-CREATION 
  src/slave/containerizer/mesos/isolators/user/usermaps.hpp PRE-CREATION 
  src/slave/containerizer/mesos/launch.cpp 
c6b669a04c006edfc78c06560d1eb088278c2f8e 
  src/slave/flags.hpp 3952d04f6a00ac1dca1adf2bea7cc6e415620ce5 
  src/slave/flags.cpp 491d10f6a8a7ea8adbfe0a09f5fce79943bccfac 

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


Testing
---

Work in progress implementing User namespaces.
Phase 1: Create isolator and enable isolator to when Agent is run with 
"userns=true". If this flags is not set the original functionality will run the 
task as user who started the task. With User namespace the task will be run 
inside the user namespace with as a root with the user who started the task is 
mapped to outside of the container. Approriate uid and gid maps are created.
Phase 2: Provide mount point support for containers running in user namespace.


Thanks,

Srinivas Brahmaroutu

Re: Review Request 52809: User Namespace implementation.

[Srinivas Brahmaroutu]

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

(Updated Oct. 12, 2016, 11:01 p.m.)


Review request for mesos, Gilbert Song and Jie Yu.


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


Repository: mesos


Description
---

WIP: User Namespace implementation.


Diffs
-

  src/Makefile.am fd01e1dfbdb04d073484ff6b7cc94b8d769f8a8e 
  src/slave/containerizer/mesos/containerizer.cpp 
32058c35ea9ca95f0a2665994c1ebccd5c840345 
  src/slave/containerizer/mesos/isolators/user/user.hpp PRE-CREATION 
  src/slave/containerizer/mesos/isolators/user/user.cpp PRE-CREATION 
  src/slave/containerizer/mesos/isolators/user/usermaps.hpp PRE-CREATION 
  src/slave/containerizer/mesos/launch.cpp 
c6b669a04c006edfc78c06560d1eb088278c2f8e 
  src/slave/flags.hpp 3952d04f6a00ac1dca1adf2bea7cc6e415620ce5 
  src/slave/flags.cpp 491d10f6a8a7ea8adbfe0a09f5fce79943bccfac 

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


Testing
---

Work in progress implementing User namespaces.
Phase 1: Create isolator and enable isolator to when Agent is run with 
"userns=true". If this flags is not set the original functionality will run the 
task as user who started the task. With User namespace the task will be run 
inside the user namespace with as a root with the user who started the task is 
mapped to outside of the container. Approriate uid and gid maps are created.
Phase 2: Provide mount point support for containers running in user namespace.


Thanks,

Srinivas Brahmaroutu

Re: Review Request 52755: Made default executor handle shutdown events while disconnected.

[Vinod Kone]

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


Fix it, then Ship it!





src/launcher/default_executor.cpp (line 698)


can you add a comment on how this can happen when we are in `CONNECTED` 
state? it's not obvious to me.


- Vinod Kone


On Oct. 12, 2016, 12:15 a.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52755/
> ---
> 
> (Updated Oct. 12, 2016, 12:15 a.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-6363
> https://issues.apache.org/jira/browse/MESOS-6363
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Previously, the default executor used to crash with a failed assertion
> when the executor library injected a shutdown event when it noticed
> a disconnection with the agent for non-checkpointed frameworks and
> upon recovery timeout for checkpointed frameworks. This change
> modifies it to commit suicide minus the failed assertion.
> 
> 
> Diffs
> -
> 
>   src/launcher/default_executor.cpp 2454bd7df608254af72af01460782f5ab78a19c1 
> 
> Diff: https://reviews.apache.org/r/52755/diff/
> 
> 
> Testing
> ---
> 
> Manual testing (It's hard to test this directly owing to the executor library 
> immediately invoking the `connected()` callback since the libprocess instance 
> initialized in tests is always active!)
> 
> Filed MESOS-6373 to address this.
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Re: Review Request 52756: Invoke the shutdown executor callback for checkpointed frameworks.

[Vinod Kone]

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




src/executor/executor.cpp (line 731)


I think this should call `_shutdown()` to ensure ShutdownProcess is spawned 
 whenever we want to shutdown.


- Vinod Kone


On Oct. 12, 2016, 12:15 a.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52756/
> ---
> 
> (Updated Oct. 12, 2016, 12:15 a.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-6370
> https://issues.apache.org/jira/browse/MESOS-6370
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Previously, the executor library used to commit suicide after the
> recovery timeout without invoking the executor's shutdown callback.
> This behavior was not consistent with the executor driver.
> 
> 
> Diffs
> -
> 
>   src/executor/executor.cpp a0e5d8382fa3886b747ae92507dce17e75c2d749 
> 
> Diff: https://reviews.apache.org/r/52756/diff/
> 
> 
> Testing
> ---
> 
> Manual testing (It's hard to test this directly owing to the executor library 
> immediately invoking the `connected()` callback since the libprocess instance 
> initialized in tests is always active!)
> 
> Filed MESOS-6373 to address this.
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Re: Review Request 52470: Filled missing executor info in tasks when `LAUNCH_GROUP`.

[Vinod Kone]

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




include/mesos/mesos.proto 


2) `TaskInfo.executor` need not be set. If set, it should match 
`LaunchGroup.executor`.


- Vinod Kone


On Oct. 12, 2016, 5:18 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52470/
> ---
> 
> (Updated Oct. 12, 2016, 5:18 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar and Vinod Kone.
> 
> 
> Bugs: MESOS-6283
> https://issues.apache.org/jira/browse/MESOS-6283
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This fixed the navigate error in Web UI because Web UI uses the
> executor id of the task to search the corresponding sandbox directory.
> Web UI uses the task id as the executor id if the executor id of the
> task is empty when searching the sandbox directory. It works fine when
> tasks are launched by `CommandExecutor` because the executor id of the
> task is equal to the task id in this case. However, when tasks are
> launched by `DefaultExecutor`, the executor id of the task is defined
> in the framework side and may different with the task id. So we need to
> fill the `ExecutorInfo` of the `TaskInfo` when `LAUNCH_GROUP` to avoid
> the Web UI uses incorrect executor id to search sandbox directory.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 7d0201ead69c816b9d51f1a41cd64a8f922b069d 
>   include/mesos/v1/mesos.proto 301d1aedca7ac0729229ba9b05e5bf2b79f286a5 
>   src/master/master.hpp 43518b9bf1bfaa54e26acc7f2e70c4161c667a84 
>   src/master/master.cpp ad8993ae43e2508a3777f4062acaca1177ac77f2 
>   src/master/validation.cpp e5da3c9bf0a1042b42522f1ab74ce798fbb1738d 
>   src/tests/default_executor_tests.cpp 
> 9e0fd678a7f5e9c2288fbb11a60cf6f339efa24f 
>   src/tests/master_validation_tests.cpp 
> 99e350e0587e73e9ee25ef20dd369cd146bd446a 
> 
> Diff: https://reviews.apache.org/r/52470/diff/
> 
> 
> Testing
> ---
> 
> Add new test case `DefaultExecutorTest.ROOT_TaskUsesExecutor`.
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 52787: Set `runtime_dir` to a temporary folder in `mesos-local`.

[Kevin Klues]

> On Oct. 12, 2016, 10:10 p.m., Kevin Klues wrote:
> > Ship It!

Can you link this to the proper JIRA?


- Kevin


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


On Oct. 12, 2016, 4:13 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52787/
> ---
> 
> (Updated Oct. 12, 2016, 4:13 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Kevin Klues.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Set `runtime_dir` to a temporary folder in `mesos-local`.
> 
> 
> Diffs
> -
> 
>   bin/mesos-local-flags.sh.in 5b4553a808dc9f34a15390e69b2f85e95761ec53 
>   src/local/flags.hpp c77eff13e7a63630ff6c9428e57f3f6707e1f30f 
> 
> Diff: https://reviews.apache.org/r/52787/diff/
> 
> 
> Testing
> ---
> 
> To void this error when launch `mesos-local` without `sudo`
> 
> ```
>   message: 'Failed to launch container: Failed to make the containerizer 
> runtime directory 
> '/var/run/mesos/containers/f2d6947f-2916-4f1a-90dc-3d137b360b9c': Permission 
> denied; Abnormal executor termination: unknown container'
> ```
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 52787: Set `runtime_dir` to a temporary folder in `mesos-local`.

[Kevin Klues]

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


Ship it!




Ship It!

- Kevin Klues


On Oct. 12, 2016, 4:13 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52787/
> ---
> 
> (Updated Oct. 12, 2016, 4:13 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Kevin Klues.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Set `runtime_dir` to a temporary folder in `mesos-local`.
> 
> 
> Diffs
> -
> 
>   bin/mesos-local-flags.sh.in 5b4553a808dc9f34a15390e69b2f85e95761ec53 
>   src/local/flags.hpp c77eff13e7a63630ff6c9428e57f3f6707e1f30f 
> 
> Diff: https://reviews.apache.org/r/52787/diff/
> 
> 
> Testing
> ---
> 
> To void this error when launch `mesos-local` without `sudo`
> 
> ```
>   message: 'Failed to launch container: Failed to make the containerizer 
> runtime directory 
> '/var/run/mesos/containers/f2d6947f-2916-4f1a-90dc-3d137b360b9c': Permission 
> denied; Abnormal executor termination: unknown container'
> ```
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 52412: Supported logger with nested containers in Mesos Containerizer.

[Joseph Wu]

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


Ship it!




Ship It!

- Joseph Wu


On Oct. 12, 2016, 2:06 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52412/
> ---
> 
> (Updated Oct. 12, 2016, 2:06 p.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Jie Yu, Joris Van Remoortere, 
> and Joseph Wu.
> 
> 
> Bugs: MESOS-6290
> https://issues.apache.org/jira/browse/MESOS-6290
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Currently, there is an issue in mesos containerizer using logger
> for nested contaienrs:
> 
> An empty executorinfo is passed to logger when launching a nested
> container, it would potentially break some logger modules if any
> module tries to access the required proto field (e.g., executorId).
> 
> We should pass the ExecutorInfo of a nested container's top level
> parent container to the logger when launching a nested container.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/containerizer.cpp 
> 7ec6f78c739145c874fb5ee63c5034dfb838c17c 
> 
> Diff: https://reviews.apache.org/r/52412/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 52058: Fixed fetcher to not call chown on sandbox again.

[Megha Sharma]

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

(Updated Oct. 12, 2016, 9:38 p.m.)


Review request for mesos and Jiang Yan Xu.


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


Repository: mesos


Description
---

Fetcher code changes the ownership of entire sandbox directory
recursively to the taskuser and as a resut also changes the
ownership of files laid out by other entities in the sandbox.


Diffs (updated)
-

  src/slave/containerizer/fetcher.cpp 11104d66e6dd05d8eb1d37a2e3250aca19278110 
  src/tests/containerizer/mesos_containerizer_tests.cpp 
ad2070246b7fdb90aa6ed02326b5a7eb95365997 

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


Testing
---

make check on linux and macos


Thanks,

Megha Sharma

Re: Review Request 52470: Filled missing executor info in tasks when `LAUNCH_GROUP`.

[Vinod Kone]

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




src/master/validation.cpp (line 1115)


need to add a validation here to ensure taskInfo.executor() is same as 
executor for all tasks in the task group.



src/tests/master_validation_tests.cpp 


Keep this test and modify it to make sure the operation is rejected 
ifExecutorinfo inside TaskInfo doesn't match the top level Executorinfo.


- Vinod Kone


On Oct. 12, 2016, 5:18 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52470/
> ---
> 
> (Updated Oct. 12, 2016, 5:18 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar and Vinod Kone.
> 
> 
> Bugs: MESOS-6283
> https://issues.apache.org/jira/browse/MESOS-6283
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This fixed the navigate error in Web UI because Web UI uses the
> executor id of the task to search the corresponding sandbox directory.
> Web UI uses the task id as the executor id if the executor id of the
> task is empty when searching the sandbox directory. It works fine when
> tasks are launched by `CommandExecutor` because the executor id of the
> task is equal to the task id in this case. However, when tasks are
> launched by `DefaultExecutor`, the executor id of the task is defined
> in the framework side and may different with the task id. So we need to
> fill the `ExecutorInfo` of the `TaskInfo` when `LAUNCH_GROUP` to avoid
> the Web UI uses incorrect executor id to search sandbox directory.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 7d0201ead69c816b9d51f1a41cd64a8f922b069d 
>   include/mesos/v1/mesos.proto 301d1aedca7ac0729229ba9b05e5bf2b79f286a5 
>   src/master/master.hpp 43518b9bf1bfaa54e26acc7f2e70c4161c667a84 
>   src/master/master.cpp ad8993ae43e2508a3777f4062acaca1177ac77f2 
>   src/master/validation.cpp e5da3c9bf0a1042b42522f1ab74ce798fbb1738d 
>   src/tests/default_executor_tests.cpp 
> 9e0fd678a7f5e9c2288fbb11a60cf6f339efa24f 
>   src/tests/master_validation_tests.cpp 
> 99e350e0587e73e9ee25ef20dd369cd146bd446a 
> 
> Diff: https://reviews.apache.org/r/52470/diff/
> 
> 
> Testing
> ---
> 
> Add new test case `DefaultExecutorTest.ROOT_TaskUsesExecutor`.
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 40512: Libprocess Reinit: Add a test-only method to reinitialize libprocess.

[Joseph Wu]

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

(Updated Oct. 12, 2016, 2:25 p.m.)


Review request for mesos, Greg Mann, Artem Harutyunyan, Joris Van Remoortere, 
and Vinod Kone.


Changes
---

Rebase and whitespace tweak.


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


Repository: mesos


Description
---

This builds upon earlier changes to complete `process::finalize`.  
Tests which need to reconfigure libprocess, such as some SSL-related 
tests, can use `process::reinitialize` to do so.  

This method may also be useful for providing additional isolation 
between tests, such as cleaning up all processes after each test case.


Diffs (updated)
-

  3rdparty/libprocess/src/openssl.cpp 5ddb10dd48908bdae898ccc45d1741d79efe2dce 
  3rdparty/libprocess/src/process.cpp f1d746c52cfe659f5cd7da4b7a6424ff585619a3 

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


Testing
---

TODO: Define `process::reinitialize` in several tests, such as `HTTPTest`, 
`MetricsTest`, and `ReapTest` and call `process::reinitialize` before and after 
tests.


Thanks,

Joseph Wu

Re: Review Request 40411: Libprocess Reinit: Modify test to use PID.

[Joseph Wu]

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

(Updated Oct. 12, 2016, 2:25 p.m.)


Review request for mesos, Greg Mann, Artem Harutyunyan, Joris Van Remoortere, 
and Vinod Kone.


Changes
---

Renamed `metrics_singleton` to `metrics`.


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


Repository: mesos


Description
---

Updates the test's reference to the global metrics singleton.


Diffs (updated)
-

  src/tests/scheduler_driver_tests.cpp faf2e6c8ad17e07964b4340d0b340654b03f9086 
  src/tests/scheduler_tests.cpp b0ea0bbcce9d847285fda40f778caaf721804457 

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


Testing
---

Tests done in a subsequent review.


Thanks,

Joseph Wu

Re: Review Request 51049: Libprocess Reinit: Removed authorizer callback leaks.

[Joseph Wu]

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

(Updated Oct. 12, 2016, 2:25 p.m.)


Review request for mesos, Greg Mann, Artem Harutyunyan, Joris Van Remoortere, 
and Vinod Kone.


Changes
---

Rebase on the large change in the previous review.


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


Repository: mesos


Description
---

The `authorization_callbacks` were never being freed after being set
or unset.  If you run the tests in repetition, this leaks memory.


Diffs (updated)
-

  3rdparty/libprocess/src/process.cpp f1d746c52cfe659f5cd7da4b7a6424ff585619a3 

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


Testing
---

make

(More testing done later in the chain)


Thanks,

Joseph Wu

Re: Review Request 50621: Libprocess reinit: Moved HttpProxy finalization and destruction.

[Joseph Wu]

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

(Updated Oct. 12, 2016, 2:25 p.m.)


Review request for mesos, Greg Mann, Artem Harutyunyan, Joris Van Remoortere, 
and Vinod Kone.


Changes
---

Rebase on the large change in the prior-prior review.


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


Repository: mesos


Description
---

Moves the destructor code in `HttpProxy` into the `Process::finalize`
function.  And changes the `HttpProxy`s termination logic to 
terminate via `UPID` which guards against double-termination.

Removes some unused initialization code, too.


Diffs (updated)
-

  3rdparty/libprocess/src/process.cpp f1d746c52cfe659f5cd7da4b7a6424ff585619a3 

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


Testing
---

make

(More testing done later in the chain)


Thanks,

Joseph Wu

Re: Review Request 40410: Libprocess Reinit: Move MetricsProcess instantiation into process.cpp.

[Joseph Wu]

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

(Updated Oct. 12, 2016, 2:25 p.m.)


Review request for mesos, Greg Mann, Artem Harutyunyan, Joris Van Remoortere, 
and Vinod Kone.


Changes
---

Rename `metrics_singleton` to `metrics`.  Some whitespace tweaks.  Added 
`process::initialize` in a couple places.


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


Repository: mesos


Description
---

The metrics singleton must be unified with `process::initialize` so 
that it also falls under the scope of reinitialization.  The singleton 
must also not be guarded by `Once`.


Diffs (updated)
-

  3rdparty/libprocess/include/process/metrics/metrics.hpp 
54487ab2614c7f8e8df10d1e0c39880a6cf5bde3 
  3rdparty/libprocess/src/metrics/metrics.cpp 
4ab3ac7fb7688df937ff62d496d9104628324018 
  3rdparty/libprocess/src/process.cpp f1d746c52cfe659f5cd7da4b7a6424ff585619a3 
  3rdparty/libprocess/src/tests/system_tests.cpp 
0f4d0424689522337806ba2227ec4330c700e17e 

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


Testing
---

Tests done in a subsequent review.


Thanks,

Joseph Wu

Re: Review Request 40413: Libprocess Reinit: Move ReaperProcess instantiation into process.cpp.

[Joseph Wu]

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

(Updated Oct. 12, 2016, 2:25 p.m.)


Review request for mesos, Greg Mann, Artem Harutyunyan, Joris Van Remoortere, 
and Vinod Kone.


Changes
---

Renamed `reaper_singleton` to `reaper`.  Some whitespace tweaks.


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


Repository: mesos


Description
---

The reaper singleton must be unified with `process::initialize` so 
that it also falls under the scope of reinitialization.  The singleton 
must also not be guarded by `Once`.


Diffs (updated)
-

  3rdparty/libprocess/include/process/reap.hpp 
d7e0fa381df63a9ca7d438d5cea0631e1b0ec7ee 
  3rdparty/libprocess/src/process.cpp f1d746c52cfe659f5cd7da4b7a6424ff585619a3 
  3rdparty/libprocess/src/reap.cpp 5fc2a4d67a3a6fe56005fc2c2d16d4983d53b83a 

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


Testing
---

Tests done in a subsequent review.


Thanks,

Joseph Wu

Re: Review Request 52412: Supported logger with nested containers in Mesos Containerizer.

[Gilbert Song]

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

(Updated Oct. 12, 2016, 2:06 p.m.)


Review request for mesos, Artem Harutyunyan, Jie Yu, Joris Van Remoortere, and 
Joseph Wu.


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


Repository: mesos


Description
---

Currently, there is an issue in mesos containerizer using logger
for nested contaienrs:

An empty executorinfo is passed to logger when launching a nested
container, it would potentially break some logger modules if any
module tries to access the required proto field (e.g., executorId).

We should pass the ExecutorInfo of a nested container's top level
parent container to the logger when launching a nested container.


Diffs (updated)
-

  src/slave/containerizer/mesos/containerizer.cpp 
7ec6f78c739145c874fb5ee63c5034dfb838c17c 

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


Testing
---

make check


Thanks,

Gilbert Song

Re: Review Request 51617: Added the `remove` and `insert` methods.

[Avinash sridharan]

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

(Updated Oct. 12, 2016, 8:55 p.m.)


Review request for mesos, Jie Yu and Qian Zhang.


Changes
---

Rebased.


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


Repository: mesos


Description
---

Added the `remove` and `insert` methods.


Diffs (updated)
-

  
src/slave/containerizer/mesos/isolators/network/cni/plugins/port_mapper/port_mapper.hpp
 7fad707a240234e35828917aea1bc79f42fe130e 
  
src/slave/containerizer/mesos/isolators/network/cni/plugins/port_mapper/port_mapper.cpp
 04736451481506f17c462b35fa579f6546c6d6ce 

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


Testing
---

Ran the CNI plugin against a network namespace with the following JSON input:
```
{
"name": "mynet",
"type": "port-mapper",
"chain": "MESOS-TEST",
"excludeDevices": ["mesos-cni0"],
"delegate": {
  "type" : "bridge",
  "bridge": "cni0",
  "isGateway": true,
  "ipMasq": true,
  "ipam": {
  "type": "host-local",
  "subnet": "192.168.37.0/24",
  "routes": [
{ "dst": "0.0.0.0/0" }
  ]
  }
},
"args" : {
  "org.apache.mesos" : {
"network_info" : {
  "port_mappings": {
"host_port" : 8080,
"container_port" : 9000
  }
}
  }
}
}
```

Used the ADD command to test that the CNI plugin correctly invokes the delegate 
plugin (a CNI bridge plugin in this case) and also inserts the correct iptable 
entries for the given port mapping. After running this plugin, this was the 
output of the `iptables -t nat -S MESOS-TEST` command:
```
sudo iptables -t nat -S MESOS-TEST
-N MESOS-TEST
-A MESOS-TEST ! -i mesos-cni0 -p tcp -m tcp --dport 8080 -j DNAT 
--to-destination 192.168.37.21:9000
```

Ran a python HTTP server in this network namespace and verified that DNAT works 
from outside the box. Was able to connect to port 9000 of this server, by 
connecting to port 8080 on the host.

Used the DEL command to test the CNI plugin correctly deletes the DNAT rule and 
chain, if there are no DNAT rules exist in the chain. After running the DEL 
command (by injecting `NetworkInfo` into the above JSON schema) verified the 
chain and the DNAT rule is deleted from iptables.


Thanks,

Avinash sridharan

Re: Review Request 51486: Added `execute` method.

[Avinash sridharan]

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

(Updated Oct. 12, 2016, 8:55 p.m.)


Review request for mesos, Jie Yu and Qian Zhang.


Changes
---

Rebased.


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


Repository: mesos


Description
---

Added `execute` method.


Diffs (updated)
-

  
src/slave/containerizer/mesos/isolators/network/cni/plugins/port_mapper/port_mapper.hpp
 7fad707a240234e35828917aea1bc79f42fe130e 
  
src/slave/containerizer/mesos/isolators/network/cni/plugins/port_mapper/port_mapper.cpp
 04736451481506f17c462b35fa579f6546c6d6ce 

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


Testing
---

make. 

Created a network namespace using `ip route2` and used the following script to 
test the execution of the 'delegate plugin':
export CNI_COMMAND="DEL"
export CNI_CONTAINERID="00001110"
export CNI_PATH="$MESOS_INSTALL:/home/vagrant/dev/go/cni/bin"
export CNI_IFNAME="eth0"
export CNI_NETNS="/var/run/netns/$1"
$MESOS_SRC/build/src/.libs/mesos-cni-port-mapper < $2
unset CNI_COMMAND
unset CNI_CONTAINERID
unset CNI_PATH
unset CNI_IFNAME
unset CNI_NETNS

Here $1 is the name of the netns created by `ip route2` and $2 is the CNI 
configuration of the port-mapper plugin. We used the following config:
{
"name": "mynet",
"type": "port-mapper",
"chain": "MESOS",
"delegate": {
  "type" : "bridge",
  "bridge": "cni0",
  "isGateway": true,
  "ipMasq": true,
  "ipam": {
  "type": "host-local",
  "subnet": "192.168.37.0/24",
  "routes": [
{ "dst": "0.0.0.0/0" }
  ]
  }
},
"args" : {
  "org.apache.mesos" : {
"network_info" : {
  "port_mappings": {
"host_port" : 8090,
"container_port" : 80
  }
}
  }
}
}


Thanks,

Avinash sridharan

Re: Review Request 51769: Implemented `delegate` method.

[Avinash sridharan]

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

(Updated Oct. 12, 2016, 8:54 p.m.)


Review request for mesos, Jie Yu and Qian Zhang.


Changes
---

Addressed Jie's comments.


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


Repository: mesos


Description
---

Implemented `delegate` method.


Diffs (updated)
-

  
src/slave/containerizer/mesos/isolators/network/cni/plugins/port_mapper/port_mapper.cpp
 04736451481506f17c462b35fa579f6546c6d6ce 

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


Testing
---

make


Thanks,

Avinash sridharan

Re: Review Request 40266: Libprocess Reinit: Cleanup SocketManager alongside ProcessManager.

[Joseph Wu]

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

(Updated Oct. 12, 2016, 1:48 p.m.)


Review request for mesos, Greg Mann, Artem Harutyunyan, Joris Van Remoortere, 
and Vinod Kone.


Changes
---

* Added a guard in `ProcessManager::finalize` to prevent any further `spawn`s.  
This removes the need for logic in `~ProcessManager`.
* Added a bunch of comments for clarification of why some things are done in 
some order.
* Changed `ProcessManager::finalize` to terminated by UPID.
* Changed the order of `Clock::finalize`


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


Repository: mesos


Description
---

The `SocketManager` and `ProcessManager` are highly inter-dependent, 
which requires some untangling in `process::finalize`.

* Logic originally found in `~ProcessManager` has been split into 
  `ProcessManager::finalize` due to what happens during cleanup.
* The future from `__s__->accept()` must be explicitly discarded as 
  libevent does not detect a locally closed socket.
* Terminating `HttpProxy`s must close the associated socket.


Diffs (updated)
-

  3rdparty/libprocess/src/process.cpp f1d746c52cfe659f5cd7da4b7a6424ff585619a3 

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


Testing
---

`make check` (libev)
`make check` (--enable-libevent --enable-ssl)


Thanks,

Joseph Wu

Re: Review Request 52746: Changed agent to send TASK_DROPPED for task launch failures.

[Neil Conway]

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

(Updated Oct. 12, 2016, 7:35 p.m.)


Review request for mesos and Vinod Kone.


Changes
---

Rebase.


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


Repository: mesos


Description
---

If the agent cannot launch a task due to a variety of possible error
conditions, we now send TASK_DROPPED to partition-aware frameworks
rather than TASK_LOST.


Diffs (updated)
-

  src/slave/slave.cpp 6bd9b49c3bbdb973a0d03552ae8fe55b33371083 

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


Testing
---

`make check`.

Note that none of these code paths appear to have unit tests, so I couldn't 
update the unit tests to ensure that `TASK_DROPPED` is sent appropriately. We 
should add unit tests for at least some of these situations, if feasible -- but 
I'll defer that for now.


Thanks,

Neil Conway

Review Request 52803: Changed agent to send TASK_GONE.

[Neil Conway]

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

Review request for mesos and Vinod Kone.


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


Repository: mesos


Description
---

The agent previously sent TASK_LOST updates for tasks that are killed
for various reasons, such as containerizer errors or QoS preemption. The
agent now sends TASK_GONE to partition-aware frameworks instead.


Diffs
-

  src/slave/slave.cpp 6bd9b49c3bbdb973a0d03552ae8fe55b33371083 
  src/tests/oversubscription_tests.cpp b356fb62a4e068bc171a75a76001c6d0e76af92a 
  src/tests/slave_tests.cpp 93b81d3e5b90d8036409e943f598c865fe335bcc 

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


Testing
---

`make check`


Thanks,

Neil Conway

Review Request 52802: Added a new slave metric, "tasks_gone".

[Neil Conway]

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

Review request for mesos and Vinod Kone.


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


Repository: mesos


Description
---

Added a new slave metric, "tasks_gone".


Diffs
-

  src/slave/metrics.hpp d4432136590a3021f6743c79e98db64cb044118a 
  src/slave/metrics.cpp 86eb8db644227cb593e52305bfbd05444bb87a6e 
  src/slave/slave.cpp 6bd9b49c3bbdb973a0d03552ae8fe55b33371083 

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


Testing
---

`make check`


Thanks,

Neil Conway

Review Request 52801: Changed description of TASK_GONE.

[Neil Conway]

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

Review request for mesos and Vinod Kone.


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


Repository: mesos


Description
---

Changed description of TASK_GONE.


Diffs
-

  include/mesos/mesos.proto 7d0201ead69c816b9d51f1a41cd64a8f922b069d 
  include/mesos/v1/mesos.proto 301d1aedca7ac0729229ba9b05e5bf2b79f286a5 

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


Testing
---

`make check`


Thanks,

Neil Conway

Re: Review Request 52740: Refactored some code into a separate function.

[Neil Conway]

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

(Updated Oct. 12, 2016, 7:34 p.m.)


Review request for mesos and Vinod Kone.


Changes
---

Rebase.


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


Repository: mesos


Description
---

Refactored some code into a separate function.


Diffs (updated)
-

  src/slave/slave.hpp d8c5873bb7380b8449f8f344e85689519c467251 
  src/slave/slave.cpp 6bd9b49c3bbdb973a0d03552ae8fe55b33371083 

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


Testing
---

`make check`


Thanks,

Neil Conway

Re: Review Request 52798: Windows: Implemented os::execvpe with _spawnvpe.

[Jie Yu]

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


Ship it!




Ship It!

- Jie Yu


On Oct. 12, 2016, 7:20 p.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52798/
> ---
> 
> (Updated Oct. 12, 2016, 7:20 p.m.)
> 
> 
> Review request for mesos, Daniel Pravat, Artem Harutyunyan, Alex Clemmer, Jie 
> Yu, and Joris Van Remoortere.
> 
> 
> Bugs: MESOS-6323
> https://issues.apache.org/jira/browse/MESOS-6323
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/os/windows/shell.hpp 
> 0ababb4ff76d55d82b8d528f54c99bd4b231b21b 
>   3rdparty/stout/include/stout/windows/os.hpp 
> 7d6530e37e389a314185e5aaa85d8096a28b9c41 
> 
> Diff: https://reviews.apache.org/r/52798/diff/
> 
> 
> Testing
> ---
> 
> Windows: msbuild
> 
> This fixes the Windows build.
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Review Request 52798: Windows: Implemented os::execvpe with _spawnvpe.

[Joseph Wu]

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

Review request for mesos, Daniel Pravat, Artem Harutyunyan, Alex Clemmer, Jie 
Yu, and Joris Van Remoortere.


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


Repository: mesos


Description
---

See summary.


Diffs
-

  3rdparty/stout/include/stout/os/windows/shell.hpp 
0ababb4ff76d55d82b8d528f54c99bd4b231b21b 
  3rdparty/stout/include/stout/windows/os.hpp 
7d6530e37e389a314185e5aaa85d8096a28b9c41 

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


Testing
---

Windows: msbuild

This fixes the Windows build.


Thanks,

Joseph Wu

Re: Review Request 52645: Harden Mesos

[Aaron Wood]

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




src/Makefile.am (line 112)


Move these into `configure.ac`.


- Aaron Wood


On Oct. 11, 2016, 10:47 p.m., Aaron Wood wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52645/
> ---
> 
> (Updated Oct. 11, 2016, 10:47 p.m.)
> 
> 
> Review request for mesos and Michael Park.
> 
> 
> Bugs: MESOS-6229
> https://issues.apache.org/jira/browse/MESOS-6229
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Use a default set of flags to provide additional security and hardening to 
> Mesos. Additionally, check and catch more warnings/errors.
> 
> 
> Diffs
> -
> 
>   configure.ac 034bb91 
>   src/Makefile.am fd01e1d 
> 
> Diff: https://reviews.apache.org/r/52645/diff/
> 
> 
> Testing
> ---
> 
> Compared the benchmarks with and without the flags being used. Also did a 
> comparsion with the flags being used with and without optimizations and 
> without the flags being used with and without optimizations. Overall the 
> performance hit was very small with a 3-8% overhead (optimizations brings 
> this down slightly). Most benchmarks were about 5% (or less) slower.
> 
> 
> Thanks,
> 
> Aaron Wood
> 
>

Re: Review Request 52569: Updated the nested container launch to correctly determine the user.

[Gilbert Song]

> On Oct. 12, 2016, 11:46 a.m., Gilbert Song wrote:
> > src/slave/http.cpp, lines 1964-1965
> > 
> >
> > Would you mind removing this `TODO`? They are no longer valid.

I will follow up with a patch. Sorry not review on time.


- Gilbert


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


On Oct. 5, 2016, 1:45 p.m., Benjamin Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52569/
> ---
> 
> (Updated Oct. 5, 2016, 1:45 p.m.)
> 
> 
> Review request for mesos, Gilbert Song and Vinod Kone.
> 
> 
> Bugs: MESOS-6289
> https://issues.apache.org/jira/browse/MESOS-6289
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This locates the executor that corresponds to the parent container,
> and uses the user that was used to launch the executor.
> 
> 
> Diffs
> -
> 
>   src/slave/http.cpp bdafe1db9f1b22ac0761f5119e73f87b531c6a3c 
>   src/slave/slave.hpp 9beefa12283e936684d82c97ebd3ac6506475edf 
>   src/slave/slave.cpp fba089506850eec87f67a46694a1ef645cdf4ad5 
>   src/tests/api_tests.cpp 7b0ad3c18a38b5ec859a79d09f5707f6958960fb 
> 
> Diff: https://reviews.apache.org/r/52569/diff/
> 
> 
> Testing
> ---
> 
> Now that the executor is looked up, the tests have been updated to
> launch an executor and use it's container id as the parent.
> 
> 
> Thanks,
> 
> Benjamin Mahler
> 
>

Re: Review Request 52569: Updated the nested container launch to correctly determine the user.

[Gilbert Song]

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




src/slave/http.cpp (lines 1964 - 1965)


Would you mind removing this `TODO`? They are no longer valid.


- Gilbert Song


On Oct. 5, 2016, 1:45 p.m., Benjamin Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52569/
> ---
> 
> (Updated Oct. 5, 2016, 1:45 p.m.)
> 
> 
> Review request for mesos, Gilbert Song and Vinod Kone.
> 
> 
> Bugs: MESOS-6289
> https://issues.apache.org/jira/browse/MESOS-6289
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This locates the executor that corresponds to the parent container,
> and uses the user that was used to launch the executor.
> 
> 
> Diffs
> -
> 
>   src/slave/http.cpp bdafe1db9f1b22ac0761f5119e73f87b531c6a3c 
>   src/slave/slave.hpp 9beefa12283e936684d82c97ebd3ac6506475edf 
>   src/slave/slave.cpp fba089506850eec87f67a46694a1ef645cdf4ad5 
>   src/tests/api_tests.cpp 7b0ad3c18a38b5ec859a79d09f5707f6958960fb 
> 
> Diff: https://reviews.apache.org/r/52569/diff/
> 
> 
> Testing
> ---
> 
> Now that the executor is looked up, the tests have been updated to
> launch an executor and use it's container id as the parent.
> 
> 
> Thanks,
> 
> Benjamin Mahler
> 
>

Re: Review Request 52647: Fix new sign comparison errors produced by hardened flags

[Neil Conway]

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



As mentioned elsewhere, can you split stout and libprocess changes into 
separate reviews?


3rdparty/libprocess/src/decoder.hpp (line 244)


Whitespace looks wonky.



3rdparty/libprocess/src/decoder.hpp (line 438)


Whitespace looks wonky.


- Neil Conway


On Oct. 11, 2016, 10:43 p.m., Aaron Wood wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52647/
> ---
> 
> (Updated Oct. 11, 2016, 10:43 p.m.)
> 
> 
> Review request for mesos and Michael Park.
> 
> 
> Bugs: MESOS-6239
> https://issues.apache.org/jira/browse/MESOS-6239
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The hardening flags produced many new sign comparison errors that need to be 
> fixed for Mesos to compile/run.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/decoder.hpp c79296b 
>   3rdparty/libprocess/src/encoder.hpp 005d1cc 
>   3rdparty/libprocess/src/process.cpp f1d746c 
>   3rdparty/libprocess/src/tests/decoder_tests.cpp 4535614 
>   3rdparty/libprocess/src/tests/encoder_tests.cpp 9e57375 
>   3rdparty/libprocess/src/tests/http_tests.cpp 8d6c8c4 
>   3rdparty/libprocess/src/tests/io_tests.cpp b85c79f 
>   3rdparty/libprocess/src/tests/subprocess_tests.cpp c8350cf 
>   3rdparty/stout/tests/cache_tests.cpp 0950c85 
>   3rdparty/stout/tests/flags_tests.cpp 94ba915 
>   3rdparty/stout/tests/hashmap_tests.cpp 2626d67 
>   3rdparty/stout/tests/hashset_tests.cpp 66e59db 
>   3rdparty/stout/tests/ip_tests.cpp 59e69a5 
>   3rdparty/stout/tests/json_tests.cpp 2bc4c88 
>   3rdparty/stout/tests/linkedhashmap_tests.cpp 7a80769 
>   3rdparty/stout/tests/multimap_tests.cpp 488991b 
>   3rdparty/stout/tests/os/process_tests.cpp 4977d02 
>   3rdparty/stout/tests/os/sendfile_tests.cpp e221689 
>   3rdparty/stout/tests/os/systems_tests.cpp 110ba5b 
>   3rdparty/stout/tests/os_tests.cpp 6a7b836 
>   3rdparty/stout/tests/strings_tests.cpp 7dd3301 
> 
> Diff: https://reviews.apache.org/r/52647/diff/
> 
> 
> Testing
> ---
> 
> Made sure compilation, tests, and benchmarks worked with both gcc and clang. 
> Ran `make && make check && make bench`.
> 
> 
> Thanks,
> 
> Aaron Wood
> 
>

Re: Review Request 52412: Supported logger with nested containers in Mesos Containerizer.

[Gilbert Song]

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

(Updated Oct. 12, 2016, 11:39 a.m.)


Review request for mesos, Artem Harutyunyan, Jie Yu, Joris Van Remoortere, and 
Joseph Wu.


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


Repository: mesos


Description
---

Currently, there is an issue in mesos containerizer using logger
for nested contaienrs:

An empty executorinfo is passed to logger when launching a nested
container, it would potentially break some logger modules if any
module tries to access the required proto field (e.g., executorId).

We should pass the ExecutorInfo of a nested container's top level
parent container to the logger when launching a nested container.


Diffs (updated)
-

  include/mesos/slave/container_logger.hpp 
939974736f9eb744c83036e074718d2a1eba8b0a 
  src/slave/containerizer/mesos/containerizer.cpp 
7ec6f78c739145c874fb5ee63c5034dfb838c17c 

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


Testing
---

make check


Thanks,

Gilbert Song

Re: Review Request 52563: Minor clean ups in `health_check_tests.cpp` for consistency.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [52786, 52250, 52251, 52560, 52561, 52563]

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 Oct. 12, 2016, 2:38 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52563/
> ---
> 
> (Updated Oct. 12, 2016, 2:38 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Gastón Kleiman.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> * Replace `Testing` to `Tests` in comments.
> * Remove redundant `.Times(1)`.
> * Fix typos.
> 
> 
> Diffs
> -
> 
>   src/tests/health_check_tests.cpp 1d1676d7259bf52cfb1e499954fa815fe7e37522 
> 
> Diff: https://reviews.apache.org/r/52563/diff/
> 
> 
> Testing
> ---
> 
> ```
> sudo GLOG_v=1 ./bin/mesos-tests.sh --gtest_filter="*HealthCheckTest.*" 
> --gtest_also_run_disabled_tests
> [--] Global test environment tear-down
> [==] 19 tests from 1 test case ran. (35264 ms total)
> [  PASSED  ] 19 tests.
> ```
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 52774: Added a test for verifying nested container environment.

[Gilbert Song]

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


Ship it!




Ship It!

- Gilbert Song


On Oct. 11, 2016, 10:35 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52774/
> ---
> 
> (Updated Oct. 11, 2016, 10:35 p.m.)
> 
> 
> Review request for mesos and Gilbert Song.
> 
> 
> Bugs: MESOS-6323
> https://issues.apache.org/jira/browse/MESOS-6323
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added a test for verifying nested container environment.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/nested_mesos_containerizer_tests.cpp 
> 9b278e546dacc14b10dac51ee50e8fb89df86b87 
>   src/tests/mesos.hpp 5e15f5164cddebd60bb6d9856ae63ad357643cb2 
> 
> Diff: https://reviews.apache.org/r/52774/diff/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 52288: Recover resources when offer is rescinded on DESTROY of shared volume.

[Jiang Yan Xu]

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




src/tests/persistent_volume_tests.cpp (lines 1031 - 1033)


The wait on `framework1` seem to be unncessary. We don't use `framework1` 
and we wait on offers anyway.



src/tests/persistent_volume_tests.cpp (lines 1060 - 1063)


The reason given here is pretty high-level (basically the purpose of this 
test). How about narrowing it down a bit? 

```
It's not important whether the volume is used (the task is killed soon and 
its purpose is only for splitting the offer).
```



src/tests/persistent_volume_tests.cpp (lines 1096 - 1098)


The wait on `framework2` seem to be unncessary. We don't use `framework2` 
and we wait on offers anyway.



src/tests/persistent_volume_tests.cpp (line 1109)


Kill this given the comment on settling the clock before registering 
framework2.


- Jiang Yan Xu


On Oct. 7, 2016, 8:12 a.m., Anindya Sinha wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52288/
> ---
> 
> (Updated Oct. 7, 2016, 8:12 a.m.)
> 
> 
> Review request for mesos and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-6257
> https://issues.apache.org/jira/browse/MESOS-6257
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> When a framework issues a DESTROY of a shared volume, and that volume
> is not in use by a running or a pending task, we rescind the offers
> from frameworks to which the shared volume is present in pending offers
> so that the volume is not assigned to any task in a future ACCEPT call.
> At that time, we need to recover the resources as well for proper
> accounting of such resources by the allocator.
> 
> 
> Diffs
> -
> 
>   src/master/master.cpp c7e74df71aa31edb490f4f3bd95f2d5aa94b4324 
>   src/tests/persistent_volume_tests.cpp 
> e10a79e9662530e143ccaa2aa2506a4d25158364 
> 
> Diff: https://reviews.apache.org/r/52288/diff/
> 
> 
> Testing
> ---
> 
> All tests passed.
> 
> 
> Thanks,
> 
> Anindya Sinha
> 
>

Re: Review Request 51769: Implemented `delegate` method.

[Jie Yu]

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




src/slave/containerizer/mesos/isolators/network/cni/plugins/port_mapper/port_mapper.cpp
 (lines 278 - 281)


This is not necessary.



src/slave/containerizer/mesos/isolators/network/cni/plugins/port_mapper/port_mapper.cpp
 (lines 283 - 292)


you can use os::write here.



src/slave/containerizer/mesos/isolators/network/cni/plugins/port_mapper/port_mapper.cpp
 (lines 294 - 295)


hum, this is not safe. If you close the fd here. In Subprocess teardown, 
it'll try to close the fd again (which might already be allocated to others).


- Jie Yu


On Sept. 29, 2016, 3:11 a.m., Avinash sridharan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51769/
> ---
> 
> (Updated Sept. 29, 2016, 3:11 a.m.)
> 
> 
> Review request for mesos, Jie Yu and Qian Zhang.
> 
> 
> Bugs: MESOS-6023
> https://issues.apache.org/jira/browse/MESOS-6023
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Implemented `delegate` method.
> 
> 
> Diffs
> -
> 
>   
> src/slave/containerizer/mesos/isolators/network/cni/plugins/port_mapper/port_mapper.cpp
>  0ecf64f2de5fc27f208e9dd0e3608b9a6750e9a6 
> 
> Diff: https://reviews.apache.org/r/51769/diff/
> 
> 
> Testing
> ---
> 
> make
> 
> 
> Thanks,
> 
> Avinash sridharan
> 
>

Re: Review Request 52470: Filled missing executor info in tasks when `LAUNCH_GROUP`.

[haosdent huang]

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

(Updated Oct. 12, 2016, 5:18 p.m.)


Review request for mesos, Anand Mazumdar and Vinod Kone.


Changes
---

Fix failed test cases.


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


Repository: mesos


Description
---

This fixed the navigate error in Web UI because Web UI uses the
executor id of the task to search the corresponding sandbox directory.
Web UI uses the task id as the executor id if the executor id of the
task is empty when searching the sandbox directory. It works fine when
tasks are launched by `CommandExecutor` because the executor id of the
task is equal to the task id in this case. However, when tasks are
launched by `DefaultExecutor`, the executor id of the task is defined
in the framework side and may different with the task id. So we need to
fill the `ExecutorInfo` of the `TaskInfo` when `LAUNCH_GROUP` to avoid
the Web UI uses incorrect executor id to search sandbox directory.


Diffs
-

  include/mesos/mesos.proto 7d0201ead69c816b9d51f1a41cd64a8f922b069d 
  include/mesos/v1/mesos.proto 301d1aedca7ac0729229ba9b05e5bf2b79f286a5 
  src/master/master.hpp 43518b9bf1bfaa54e26acc7f2e70c4161c667a84 
  src/master/master.cpp ad8993ae43e2508a3777f4062acaca1177ac77f2 
  src/master/validation.cpp e5da3c9bf0a1042b42522f1ab74ce798fbb1738d 
  src/tests/default_executor_tests.cpp 9e0fd678a7f5e9c2288fbb11a60cf6f339efa24f 
  src/tests/master_validation_tests.cpp 
99e350e0587e73e9ee25ef20dd369cd146bd446a 

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


Testing (updated)
---

Add new test case `DefaultExecutorTest.ROOT_TaskUsesExecutor`.


Thanks,

haosdent huang

Re: Review Request 52471: Fixed the wrong sandbox directory of the tasks in Web UI.

[haosdent huang]

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

(Updated Oct. 12, 2016, 5:18 p.m.)


Review request for mesos, Anand Mazumdar and Vinod Kone.


Changes
---

Rebase.


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


Repository: mesos


Description
---

For the task launched by default-executor, its sandbox directory is
'executor_directory/tasks/task_id/'. This patch generates the
corresponding sandbox directory in Web UI for the task according to its
executor type.


Diffs (updated)
-

  src/webui/master/static/agent_executor.html 
8b83ed5acc2ecd56f20b1571878ec9f6794efbd2 
  src/webui/master/static/framework.html 
bc3c56adf22030e6cd1dcd8a2c945d44ff79aa4b 
  src/webui/master/static/home.html 179cb15140f85811df0ea0a87620dd3c90dd30c7 
  src/webui/master/static/js/app.js 400a4286d9358c699453326d87a0bd49dbb105a7 
  src/webui/master/static/js/controllers.js 
29a5a1c8754cc2fb934854750d6dfb04f1eaeae4 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 52520: Exposed the executor's type in the endpoints.

[haosdent huang]

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

(Updated Oct. 12, 2016, 5:18 p.m.)


Review request for mesos, Anand Mazumdar and Vinod Kone.


Changes
---

Rebase.


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


Repository: mesos


Description
---

Exposed the executor's type in the endpoints.


Diffs (updated)
-

  src/common/http.cpp 538330a4c780fbc2dfcdfb31537b0e75f368e3e0 
  src/slave/http.cpp 79061c3cd94d856ec695e5a82bf6792bf089d1f8 
  src/tests/default_executor_tests.cpp 9e0fd678a7f5e9c2288fbb11a60cf6f339efa24f 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 52664: Moved the `decimalFloat` filter to app.js for consistency.

[haosdent huang]

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

(Updated Oct. 12, 2016, 5:18 p.m.)


Review request for mesos, Anand Mazumdar and Vinod Kone.


Changes
---

Trigger to run test again.


Repository: mesos


Description
---

Moved the `decimalFloat` filter to app.js for consistency.


Diffs (updated)
-

  src/webui/master/static/js/app.js 400a4286d9358c699453326d87a0bd49dbb105a7 
  src/webui/master/static/js/controllers.js 
29a5a1c8754cc2fb934854750d6dfb04f1eaeae4 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 52470: Filled missing executor info in tasks when `LAUNCH_GROUP`.

[haosdent huang]

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

(Updated Oct. 12, 2016, 5:17 p.m.)


Review request for mesos, Anand Mazumdar and Vinod Kone.


Changes
---

Rebase.


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


Repository: mesos


Description
---

This fixed the navigate error in Web UI because Web UI uses the
executor id of the task to search the corresponding sandbox directory.
Web UI uses the task id as the executor id if the executor id of the
task is empty when searching the sandbox directory. It works fine when
tasks are launched by `CommandExecutor` because the executor id of the
task is equal to the task id in this case. However, when tasks are
launched by `DefaultExecutor`, the executor id of the task is defined
in the framework side and may different with the task id. So we need to
fill the `ExecutorInfo` of the `TaskInfo` when `LAUNCH_GROUP` to avoid
the Web UI uses incorrect executor id to search sandbox directory.


Diffs (updated)
-

  include/mesos/mesos.proto 7d0201ead69c816b9d51f1a41cd64a8f922b069d 
  include/mesos/v1/mesos.proto 301d1aedca7ac0729229ba9b05e5bf2b79f286a5 
  src/master/master.hpp 43518b9bf1bfaa54e26acc7f2e70c4161c667a84 
  src/master/master.cpp ad8993ae43e2508a3777f4062acaca1177ac77f2 
  src/master/validation.cpp e5da3c9bf0a1042b42522f1ab74ce798fbb1738d 
  src/tests/default_executor_tests.cpp 9e0fd678a7f5e9c2288fbb11a60cf6f339efa24f 
  src/tests/master_validation_tests.cpp 
99e350e0587e73e9ee25ef20dd369cd146bd446a 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 51768: Updated signature of `delegate` and `execute` method.

[Jie Yu]

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


Ship it!




Ship It!

- Jie Yu


On Sept. 28, 2016, 7:30 p.m., Avinash sridharan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51768/
> ---
> 
> (Updated Sept. 28, 2016, 7:30 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Qian Zhang.
> 
> 
> Bugs: MESOS-6023
> https://issues.apache.org/jira/browse/MESOS-6023
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> In case of `spec::CNI_CMD_DEL` the CNI plugin is not supposed to
> return any output. Hence, changing the signatures to return an
> `Result` and an `Option` respectively.
> 
> 
> Diffs
> -
> 
>   
> src/slave/containerizer/mesos/isolators/network/cni/plugins/port_mapper/main.cpp
>  11c03fdf8a85414d20704299103e22be7d844f80 
>   
> src/slave/containerizer/mesos/isolators/network/cni/plugins/port_mapper/port_mapper.hpp
>  85547533b0b13011615b512ec8c71b7545f33324 
>   
> src/slave/containerizer/mesos/isolators/network/cni/plugins/port_mapper/port_mapper.cpp
>  0ecf64f2de5fc27f208e9dd0e3608b9a6750e9a6 
> 
> Diff: https://reviews.apache.org/r/51768/diff/
> 
> 
> Testing
> ---
> 
> make
> 
> 
> Thanks,
> 
> Avinash sridharan
> 
>

Re: Review Request 51052: Make mesos-docker-execute understand cgroups_enable_cfs.

[Zhitao Li]

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

(Updated Oct. 12, 2016, 5:02 p.m.)


Review request for mesos, Gilbert Song, haosdent huang, and Jie Yu.


Changes
---

Rebase only.


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


Repository: mesos


Description
---

This fixes cpu quota for command executor (which runs outside
of the docker container) by ensuing --cpu-quota flag to docker
run.

Note: we have to add the boolean variable to `Docker` class
because `Docker::run()` has reached the maximum argument length
GMOCK can support.


Diffs (updated)
-

  src/docker/docker.hpp 9093371afc8ea792ba94f61c6875703e547ea6b0 
  src/docker/docker.cpp 50fda393a42afefc70790a26b44911e4cf17185e 
  src/docker/executor.hpp a49ec1b4045116741af6af08578791fb0440ad8f 
  src/docker/executor.cpp ab3f0473fdc9105d1c425f0dbe7b81c566d541e8 
  src/slave/containerizer/docker.cpp d71386089bf7677872bcb1bd36e07da9263dcf0d 
  src/tests/mock_docker.cpp 6a0e613bde6889864a37ffd7ec0b454e5fe4df1c 

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


Testing
---

I am now able to make docker containers launched through mesos-execute have a 
cpu quota.

Also making sure `make check` still works on mac os for the linux only flag.


Thanks,

Zhitao Li

Re: Review Request 51009: Collect throttle related cpu.stat for Docker Containerizer.

[Zhitao Li]

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

(Updated Oct. 12, 2016, 5 p.m.)


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


Changes
---

Rebase only.


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


Repository: mesos


Description
---

Collect throttle related cpu.stat for Docker Containerizer.


Diffs (updated)
-

  src/slave/containerizer/docker.cpp d71386089bf7677872bcb1bd36e07da9263dcf0d 

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


Testing
---

Run agent with cfs quota enabled, and observe that throttle related metrics are 
in `/containers` and `/monitoring/statistics`


Thanks,

Zhitao Li

Re: Review Request 51740: Added the 'name' and 'args' field to the 'delegate' plugin's CNI config.

[Jie Yu]

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


Ship it!




Ship It!

- Jie Yu


On Sept. 28, 2016, 7:29 p.m., Avinash sridharan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51740/
> ---
> 
> (Updated Sept. 28, 2016, 7:29 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Qian Zhang.
> 
> 
> Bugs: MESOS-6023
> https://issues.apache.org/jira/browse/MESOS-6023
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> When the port-mapper plugin invokes the 'delegate' plugin, the CNI
> config of the 'delegate' plugin needs to have the 'name' and the
> 'args' fields set. We are therefore updating the config during
> creation of the `PortMapper` object, and will use this config when
> the 'delegate' plugin is invoked.
> 
> 
> Diffs
> -
> 
>   
> src/slave/containerizer/mesos/isolators/network/cni/plugins/port_mapper/port_mapper.cpp
>  0ecf64f2de5fc27f208e9dd0e3608b9a6750e9a6 
> 
> Diff: https://reviews.apache.org/r/51740/diff/
> 
> 
> Testing
> ---
> 
> make
> 
> 
> Thanks,
> 
> Avinash sridharan
> 
>

Re: Review Request 51767: Added constants for CNI commands.

[Jie Yu]

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


Ship it!




Ship It!

- Jie Yu


On Sept. 28, 2016, 7:29 p.m., Avinash sridharan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51767/
> ---
> 
> (Updated Sept. 28, 2016, 7:29 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Qian Zhang.
> 
> 
> Bugs: MESOS-6023
> https://issues.apache.org/jira/browse/MESOS-6023
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added constants for CNI commands.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/network/cni/spec.hpp 
> 2d0187beaf0d20e87d5a9ff80f5672ce7b8cc53b 
> 
> Diff: https://reviews.apache.org/r/51767/diff/
> 
> 
> Testing
> ---
> 
> make
> 
> 
> Thanks,
> 
> Avinash sridharan
> 
>

Re: Review Request 51737: Added `PluginError` to simplify error reporting for CNI plugins.

[Jie Yu]

> On Oct. 12, 2016, 4:49 p.m., Jie Yu wrote:
> > src/slave/containerizer/mesos/isolators/network/cni/spec.hpp, line 61
> > 
> >
> > 2 lines apart

I'll fix for you


- Jie


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


On Sept. 28, 2016, 7:29 p.m., Avinash sridharan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51737/
> ---
> 
> (Updated Sept. 28, 2016, 7:29 p.m.)
> 
> 
> Review request for mesos, haosdent huang, Jie Yu, switched to 'mcypark', and 
> Qian Zhang.
> 
> 
> Bugs: MESOS-6023
> https://issues.apache.org/jira/browse/MESOS-6023
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added `PluginError` to simplify error reporting for CNI plugins.
> 
> 
> Diffs
> -
> 
>   
> src/slave/containerizer/mesos/isolators/network/cni/plugins/port_mapper/main.cpp
>  11c03fdf8a85414d20704299103e22be7d844f80 
>   
> src/slave/containerizer/mesos/isolators/network/cni/plugins/port_mapper/port_mapper.hpp
>  85547533b0b13011615b512ec8c71b7545f33324 
>   
> src/slave/containerizer/mesos/isolators/network/cni/plugins/port_mapper/port_mapper.cpp
>  0ecf64f2de5fc27f208e9dd0e3608b9a6750e9a6 
>   src/slave/containerizer/mesos/isolators/network/cni/spec.hpp 
> 2d0187beaf0d20e87d5a9ff80f5672ce7b8cc53b 
> 
> Diff: https://reviews.apache.org/r/51737/diff/
> 
> 
> Testing
> ---
> 
> make
> 
> 
> Thanks,
> 
> Avinash sridharan
> 
>

Re: Review Request 51737: Added `PluginError` to simplify error reporting for CNI plugins.

[Jie Yu]

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


Fix it, then Ship it!





src/slave/containerizer/mesos/isolators/network/cni/spec.hpp (line 61)


2 lines apart


- Jie Yu


On Sept. 28, 2016, 7:29 p.m., Avinash sridharan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51737/
> ---
> 
> (Updated Sept. 28, 2016, 7:29 p.m.)
> 
> 
> Review request for mesos, haosdent huang, Jie Yu, switched to 'mcypark', and 
> Qian Zhang.
> 
> 
> Bugs: MESOS-6023
> https://issues.apache.org/jira/browse/MESOS-6023
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added `PluginError` to simplify error reporting for CNI plugins.
> 
> 
> Diffs
> -
> 
>   
> src/slave/containerizer/mesos/isolators/network/cni/plugins/port_mapper/main.cpp
>  11c03fdf8a85414d20704299103e22be7d844f80 
>   
> src/slave/containerizer/mesos/isolators/network/cni/plugins/port_mapper/port_mapper.hpp
>  85547533b0b13011615b512ec8c71b7545f33324 
>   
> src/slave/containerizer/mesos/isolators/network/cni/plugins/port_mapper/port_mapper.cpp
>  0ecf64f2de5fc27f208e9dd0e3608b9a6750e9a6 
>   src/slave/containerizer/mesos/isolators/network/cni/spec.hpp 
> 2d0187beaf0d20e87d5a9ff80f5672ce7b8cc53b 
> 
> Diff: https://reviews.apache.org/r/51737/diff/
> 
> 
> Testing
> ---
> 
> make
> 
> 
> Thanks,
> 
> Avinash sridharan
> 
>

Re: Review Request 52638: Populated `recovered_agents` field in `GetAgents` response.

[Zhitao Li]

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

(Updated Oct. 12, 2016, 4:46 p.m.)


Review request for mesos, Anand Mazumdar, Xiaojian Huang, and Kunal Thakar.


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


Repository: mesos


Description
---

Populated `recovered_agents` field in `GetAgents` response.


Diffs (updated)
-

  src/master/http.cpp bb9c87327dfe2161a6f1fd4cded72aa9a5ffaf66 

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


Testing
---


Thanks,

Zhitao Li

Re: Review Request 52638: Populated `recovered_agents` field in `GetAgents` response.

[Zhitao Li]

> On Oct. 10, 2016, 6:53 p.m., Anand Mazumdar wrote:
> > Would you be following up with the change to `/state` in a separate review?

Done. r/52765


- Zhitao


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


On Oct. 12, 2016, 12:18 a.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52638/
> ---
> 
> (Updated Oct. 12, 2016, 12:18 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar and Xiaojian Huang.
> 
> 
> Bugs: MESOS-6177
> https://issues.apache.org/jira/browse/MESOS-6177
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Populated `recovered_agents` field in `GetAgents` response.
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp bb9c87327dfe2161a6f1fd4cded72aa9a5ffaf66 
> 
> Diff: https://reviews.apache.org/r/52638/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>

Review Request 52787: Set `runtime_dir` to a temporary folder in `mesos-local`.

[haosdent huang]

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

Review request for mesos, Jie Yu and Kevin Klues.


Repository: mesos


Description
---

Set `runtime_dir` to a temporary folder in `mesos-local`.


Diffs
-

  bin/mesos-local-flags.sh.in 5b4553a808dc9f34a15390e69b2f85e95761ec53 
  src/local/flags.hpp c77eff13e7a63630ff6c9428e57f3f6707e1f30f 

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


Testing
---

To void this error when launch `mesos-local` without `sudo`

```
  message: 'Failed to launch container: Failed to make the containerizer 
runtime directory 
'/var/run/mesos/containers/f2d6947f-2916-4f1a-90dc-3d137b360b9c': Permission 
denied; Abnormal executor termination: unknown container'
```


Thanks,

haosdent huang

Re: Review Request 52288: Recover resources when offer is rescinded on DESTROY of shared volume.

[Jiang Yan Xu]

> On Oct. 12, 2016, 8:20 a.m., Jiang Yan Xu wrote:
> > src/tests/persistent_volume_tests.cpp, lines 1125-1128
> > 
> >
> > We have reduced the number of places that advance time to one, we can 
> > just pull the Clock::pause() down here so it's grouped together.

I overlooked a place above that also relies on the paused clock (the comment I 
added below) so ignore this.


- Jiang Yan


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


On Oct. 7, 2016, 8:12 a.m., Anindya Sinha wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52288/
> ---
> 
> (Updated Oct. 7, 2016, 8:12 a.m.)
> 
> 
> Review request for mesos and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-6257
> https://issues.apache.org/jira/browse/MESOS-6257
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> When a framework issues a DESTROY of a shared volume, and that volume
> is not in use by a running or a pending task, we rescind the offers
> from frameworks to which the shared volume is present in pending offers
> so that the volume is not assigned to any task in a future ACCEPT call.
> At that time, we need to recover the resources as well for proper
> accounting of such resources by the allocator.
> 
> 
> Diffs
> -
> 
>   src/master/master.cpp c7e74df71aa31edb490f4f3bd95f2d5aa94b4324 
>   src/tests/persistent_volume_tests.cpp 
> e10a79e9662530e143ccaa2aa2506a4d25158364 
> 
> Diff: https://reviews.apache.org/r/52288/diff/
> 
> 
> Testing
> ---
> 
> All tests passed.
> 
> 
> Thanks,
> 
> Anindya Sinha
> 
>

Re: Review Request 52288: Recover resources when offer is rescinded on DESTROY of shared volume.

[Jiang Yan Xu]

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




src/tests/persistent_volume_tests.cpp (lines 1079 - 1086)


Actually, if we don't guarantee that `acceptOffer` is processed first, we 
can't guarantee that `framework2` receives an offer below even with time 
advancement (if framework2 registers before framework1 launches)

We should settle clock here and then the time advancement below 

```
AWAIT_READY(frameworkId2);

Clock::advance(masterFlags.allocation_interval);

AWAIT_READY(offers2);
```

won't be necessary.


- Jiang Yan Xu


On Oct. 7, 2016, 8:12 a.m., Anindya Sinha wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52288/
> ---
> 
> (Updated Oct. 7, 2016, 8:12 a.m.)
> 
> 
> Review request for mesos and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-6257
> https://issues.apache.org/jira/browse/MESOS-6257
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> When a framework issues a DESTROY of a shared volume, and that volume
> is not in use by a running or a pending task, we rescind the offers
> from frameworks to which the shared volume is present in pending offers
> so that the volume is not assigned to any task in a future ACCEPT call.
> At that time, we need to recover the resources as well for proper
> accounting of such resources by the allocator.
> 
> 
> Diffs
> -
> 
>   src/master/master.cpp c7e74df71aa31edb490f4f3bd95f2d5aa94b4324 
>   src/tests/persistent_volume_tests.cpp 
> e10a79e9662530e143ccaa2aa2506a4d25158364 
> 
> Diff: https://reviews.apache.org/r/52288/diff/
> 
> 
> Testing
> ---
> 
> All tests passed.
> 
> 
> Thanks,
> 
> Anindya Sinha
> 
>

Re: Review Request 52707: Updated CLI bootstrap to search for local virtualenv installations.

[Kevin Klues]

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


Ship it!




Ship It!

- Kevin Klues


On Oct. 10, 2016, 9:42 p.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52707/
> ---
> 
> (Updated Oct. 10, 2016, 9:42 p.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Kevin Klues, and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> A locally installed virtualenv does not always show up on the `PATH`,
> which fails `which virtualenv`.  This adds an extra search location
> based on the user's site package install directory.
> 
> 
> Diffs
> -
> 
>   src/cli_new/bootstrap 74a8568daef4e0cb93911352cc73c165a2fbea1d 
> 
> Diff: https://reviews.apache.org/r/52707/diff/
> 
> 
> Testing
> ---
> 
> pip uninstall virtualenv -y  # Global
> pip uninstall virtualenv -y  # Local
> 
> src/cli_new/bootstrap# Fails
> 
> pip install --user virtualenv
> 
> src/cli_new/bootstrap# Succeeds
> 
> pip uninstall virtualenv -y  # Local
> pip install virtualenv   # Global
> 
> src/cli_new/bootstrap# Succeeds
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Re: Review Request 51715: Added a parallel gtest runner.

[Benjamin Bannier]

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

(Updated Oct. 12, 2016, 5:28 p.m.)


Review request for mesos, Kevin Klues and Till Toenshoff.


Changes
---

Fixed commit message.


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


Repository: mesos


Description (updated)
---

This commits adds a driver capable of running different GoogleTest test
cases in parallel. We will add the driver to the build setup in later
commits.


Diffs
-

  support/mesos-gtest-runner.py PRE-CREATION 

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


Testing
---

Tested with e.g., `stout-tests`

$ ./support/mesos-gtest-runner.py build/3rdparty/stout/stout-tests

[PASS]


Thanks,

Benjamin Bannier

Re: Review Request 52288: Recover resources when offer is rescinded on DESTROY of shared volume.

[Jiang Yan Xu]

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


Ship it!




Committing with these cleanups.


src/tests/persistent_volume_tests.cpp (line 1117)


s/Framework 1/Framework1/ for consistency.



src/tests/persistent_volume_tests.cpp (line 1119)


s/its/their/



src/tests/persistent_volume_tests.cpp (lines 1125 - 1128)


We have reduced the number of places that advance time to one, we can just 
pull the Clock::pause() down here so it's grouped together.



src/tests/persistent_volume_tests.cpp (line 1140)


Indentation.


- Jiang Yan Xu


On Oct. 7, 2016, 8:12 a.m., Anindya Sinha wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52288/
> ---
> 
> (Updated Oct. 7, 2016, 8:12 a.m.)
> 
> 
> Review request for mesos and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-6257
> https://issues.apache.org/jira/browse/MESOS-6257
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> When a framework issues a DESTROY of a shared volume, and that volume
> is not in use by a running or a pending task, we rescind the offers
> from frameworks to which the shared volume is present in pending offers
> so that the volume is not assigned to any task in a future ACCEPT call.
> At that time, we need to recover the resources as well for proper
> accounting of such resources by the allocator.
> 
> 
> Diffs
> -
> 
>   src/master/master.cpp c7e74df71aa31edb490f4f3bd95f2d5aa94b4324 
>   src/tests/persistent_volume_tests.cpp 
> e10a79e9662530e143ccaa2aa2506a4d25158364 
> 
> Diff: https://reviews.apache.org/r/52288/diff/
> 
> 
> Testing
> ---
> 
> All tests passed.
> 
> 
> Thanks,
> 
> Anindya Sinha
> 
>

Re: Review Request 51715: Added a parallel gtest runner.

[Kevin Klues]

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


Ship it!




Ship It!

- Kevin Klues


On Oct. 11, 2016, 8:18 a.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51715/
> ---
> 
> (Updated Oct. 11, 2016, 8:18 a.m.)
> 
> 
> Review request for mesos, Kevin Klues and Till Toenshoff.
> 
> 
> Bugs: MESOS-6140
> https://issues.apache.org/jira/browse/MESOS-6140
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This adds `b814987b28cfb65a7c9635c83399545e423e690a` of
> https://github.com/bbannier/gtest-shrdr.
> 
> 
> Diffs
> -
> 
>   support/mesos-gtest-runner.py PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/51715/diff/
> 
> 
> Testing
> ---
> 
> Tested with e.g., `stout-tests`
> 
> $ ./support/mesos-gtest-runner.py build/3rdparty/stout/stout-tests
> 
> [PASS]
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 52695: Harden libprocess

[Aaron Wood]

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




3rdparty/libprocess/Makefile.am (line 29)


Only use `-fstack-protector-strong` if we have GCC >= 4.9.


- Aaron Wood


On Oct. 11, 2016, 10:47 p.m., Aaron Wood wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52695/
> ---
> 
> (Updated Oct. 11, 2016, 10:47 p.m.)
> 
> 
> Review request for mesos and Michael Park.
> 
> 
> Bugs: MESOS-6229
> https://issues.apache.org/jira/browse/MESOS-6229
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Use a default set of flags to provide additional security and hardening to 
> libprocess. Additionally, check and catch more warnings/errors.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/Makefile.am 020b0e1 
> 
> Diff: https://reviews.apache.org/r/52695/diff/
> 
> 
> Testing
> ---
> 
> Compared the benchmarks with and without the flags being used. Also did a 
> comparsion with the flags being used with and without optimizations and 
> without the flags being used with and without optimizations. Overall the 
> performance hit was very small with a 3-8% overhead (optimizations brings 
> this down slightly). Most benchmarks were about 5% (or less) slower.
> 
> 
> Thanks,
> 
> Aaron Wood
> 
>

Re: Review Request 52696: Harden stout

[Aaron Wood]

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




3rdparty/stout/Makefile.am (line 26)


Only use `-fstack-protector-strong` if we have GCC >= 4.9.


- Aaron Wood


On Oct. 11, 2016, 10:47 p.m., Aaron Wood wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52696/
> ---
> 
> (Updated Oct. 11, 2016, 10:47 p.m.)
> 
> 
> Review request for mesos and Michael Park.
> 
> 
> Bugs: MESOS-6229
> https://issues.apache.org/jira/browse/MESOS-6229
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Use a default set of flags to provide additional security and hardening to 
> stout. Additionally, check and catch more warnings/errors.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/Makefile.am fda069d 
> 
> Diff: https://reviews.apache.org/r/52696/diff/
> 
> 
> Testing
> ---
> 
> Compared the benchmarks with and without the flags being used. Also did a 
> comparsion with the flags being used with and without optimizations and 
> without the flags being used with and without optimizations. Overall the 
> performance hit was very small with a 3-8% overhead (optimizations brings 
> this down slightly). Most benchmarks were about 5% (or less) slower.
> 
> 
> Thanks,
> 
> Aaron Wood
> 
>

Re: Review Request 52561: Renamed `flags` to `agentFlags` in health check test cases.

[haosdent huang]

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

(Updated Oct. 12, 2016, 2:38 p.m.)


Review request for mesos, Alexander Rukletsov and Gastón Kleiman.


Changes
---

Use test-helper to test.


Repository: mesos


Description
---

This patch renames `slave` to `agent` in health check test cases as
well.


Diffs (updated)
-

  src/tests/health_check_tests.cpp 1d1676d7259bf52cfb1e499954fa815fe7e37522 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 52251: Added test cases for TCP health check.

[haosdent huang]

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

(Updated Oct. 12, 2016, 2:37 p.m.)


Review request for mesos, Alexander Rukletsov and Gastón Kleiman.


Changes
---

Use test-helper to test.


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


Repository: mesos


Description
---

Added test cases for TCP health check.


Diffs (updated)
-

  src/tests/health_check_tests.cpp 1d1676d7259bf52cfb1e499954fa815fe7e37522 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 52563: Minor clean ups in `health_check_tests.cpp` for consistency.

[haosdent huang]

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

(Updated Oct. 12, 2016, 2:38 p.m.)


Review request for mesos, Alexander Rukletsov and Gastón Kleiman.


Changes
---

Use test-helper to test.


Repository: mesos


Description
---

* Replace `Testing` to `Tests` in comments.
* Remove redundant `.Times(1)`.
* Fix typos.


Diffs (updated)
-

  src/tests/health_check_tests.cpp 1d1676d7259bf52cfb1e499954fa815fe7e37522 

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


Testing
---

```
sudo GLOG_v=1 ./bin/mesos-tests.sh --gtest_filter="*HealthCheckTest.*" 
--gtest_also_run_disabled_tests
[--] Global test environment tear-down
[==] 19 tests from 1 test case ran. (35264 ms total)
[  PASSED  ] 19 tests.
```


Thanks,

haosdent huang

Review Request 52786: Add the health check test helper.

[haosdent huang]

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

Review request for mesos, Alexander Rukletsov and Gastón Kleiman.


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


Repository: mesos


Description
---

This patch updates the test helper for the health check. Via starts a
simple HTTP server which listens on given IP and port, it makes testing
HTTP health check and TCP health check easier.


Diffs
-

  src/Makefile.am fd01e1dfbdb04d073484ff6b7cc94b8d769f8a8e 
  src/tests/CMakeLists.txt f5d66dc63143455506d8660674fbd9eb227625ff 
  src/tests/containerizer/health_check_test_helper.hpp PRE-CREATION 
  src/tests/containerizer/health_check_test_helper.cpp PRE-CREATION 
  src/tests/test_helper_main.cpp 129a5e427b72fb577e55c3756d071f72ced9ff14 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 52560: Avoided temporary `MockDocker` pointers in health check test cases.

[haosdent huang]

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

(Updated Oct. 12, 2016, 2:38 p.m.)


Review request for mesos, Alexander Rukletsov and Gastón Kleiman.


Changes
---

Use test-helper to test.


Repository: mesos


Description
---

Avoided temporary `MockDocker` pointers in health check test cases.


Diffs (updated)
-

  src/tests/health_check_tests.cpp 1d1676d7259bf52cfb1e499954fa815fe7e37522 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 52250: Added test cases for HTTP health check.

[haosdent huang]

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

(Updated Oct. 12, 2016, 2:37 p.m.)


Review request for mesos, Alexander Rukletsov and Gastón Kleiman.


Changes
---

Use test-helper to test.


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


Repository: mesos


Description
---

Added test cases for HTTP health check.


Diffs (updated)
-

  src/tests/health_check_tests.cpp 1d1676d7259bf52cfb1e499954fa815fe7e37522 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 52741: Added capabilities support to mesos-execute.

[Benjamin Bannier]

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

(Updated Oct. 12, 2016, 4:31 p.m.)


Review request for mesos and Jie Yu.


Changes
---

Removed accidential lines.


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


Repository: mesos


Description
---

This change introduces a flags `capabilities` to mesos-execute which
can be used to specify the capabilities a task should be able to
aquire.


Diffs (updated)
-

  src/cli/execute.cpp fcf627b0dcbedd01700cc8c9acadf7ba0dae4faa 

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


Testing
---

* `make check`, including `ROOT` tests,
* tested as root with a `ping` task with an agent where both the task and the 
agent have different or no capabilities configured.


Thanks,

Benjamin Bannier

Re: Review Request 52774: Added a test for verifying nested container environment.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [52774]

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 Oct. 12, 2016, 5:35 a.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52774/
> ---
> 
> (Updated Oct. 12, 2016, 5:35 a.m.)
> 
> 
> Review request for mesos and Gilbert Song.
> 
> 
> Bugs: MESOS-6323
> https://issues.apache.org/jira/browse/MESOS-6323
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added a test for verifying nested container environment.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/nested_mesos_containerizer_tests.cpp 
> 9b278e546dacc14b10dac51ee50e8fb89df86b87 
>   src/tests/mesos.hpp 5e15f5164cddebd60bb6d9856ae63ad357643cb2 
> 
> Diff: https://reviews.apache.org/r/52774/diff/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Review Request 52783: Added documentation for mesos-containerizer Linux capabilities support.

[Benjamin Bannier]

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

Review request for mesos and Jie Yu.


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


Repository: mesos


Description
---

Added documentation for mesos-containerizer Linux capabilities support.


Diffs
-

  docs/mesos-containerizer.md 76544625838845fe2817347fea483a60ab250f99 

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


Testing
---

Checked with local renderer.


Thanks,

Benjamin Bannier