Re: Review Request 53842: Add role specific metrics for sorting runs.

[Mesos Reviewbot]

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



Patch looks great!

Reviews applied: [53840, 53841, 53842]

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 March 7, 2017, 11:41 p.m., Anindya Sinha wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53842/
> ---
> 
> (Updated March 7, 2017, 11:41 p.m.)
> 
> 
> Review request for mesos and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-6579
> https://issues.apache.org/jira/browse/MESOS-6579
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added the following 2 metrics which is maintained on a role level
> (as long as there is at least one framework of that role):
> a) allocator/mesos/frameworks/role//sort_runs: Number of framework
>level sorts (based on role) in DRF Sorter.
> b) allocator/mesos/frameworks/role//sort_run: Latency in framework
>level sorts (based on role) in DRF Sorter.
> 
> 
> Diffs
> -
> 
>   src/master/allocator/mesos/hierarchical.hpp 
> 646f66e67d9c6b8c61fc6e6558a1db976a44c126 
>   src/master/allocator/mesos/hierarchical.cpp 
> 0059ccead90f32491591990c791e7fa905a864b7 
>   src/tests/hierarchical_allocator_tests.cpp 
> cdf1f15b7802439b28405ca8f6634ce83e886630 
> 
> 
> Diff: https://reviews.apache.org/r/53842/diff/3/
> 
> 
> Testing
> ---
> 
> All tests passed.
> 
> 
> Thanks,
> 
> Anindya Sinha
> 
>

Re: Review Request 56732: Remove unnecessary perf version checks.

[haosdent huang]

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


Ship it!




Ship It!

- haosdent huang


On Feb. 15, 2017, 11:22 p.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56732/
> ---
> 
> (Updated Feb. 15, 2017, 11:22 p.m.)
> 
> 
> Review request for mesos, Mesos Reviewbot and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-7049
> https://issues.apache.org/jira/browse/MESOS-7049
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> We can remove the version check from the perf subprocess
> wrapper because we longer uses the version to figure out
> how to parse the stat output and the callers explicitly
> check perf::supported().
> 
> 
> Diffs
> -
> 
>   src/linux/perf.hpp 9c4330b6086abb18f03660fe89a6fb01d8ed 
>   src/linux/perf.cpp 3141e5ee9eee78e974625791f362dc345c682ebb 
>   src/tests/containerizer/perf_tests.cpp 
> d536ecc5cb24787bc3487efb146573cd4f3ded43 
> 
> 
> Diff: https://reviews.apache.org/r/56732/diff/1/
> 
> 
> Testing
> ---
> 
> sudo make check (Fedora 25)
> 
> 
> Thanks,
> 
> James Peach
> 
>

Re: Review Request 56731: Fix perf-stat parsing for recent perf releases.

[haosdent huang]

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


Ship it!




Ship It!

- haosdent huang


On Feb. 15, 2017, 11:24 p.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56731/
> ---
> 
> (Updated Feb. 15, 2017, 11:24 p.m.)
> 
> 
> Review request for mesos, Mesos Reviewbot and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-7049
> https://issues.apache.org/jira/browse/MESOS-7049
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> perf-stat(1) emits additional metric aggregation and value fields since
> Linux 4.6 (see kernel tree commit 92a61f6). Since these fields are not
> needed for the Sample interface, it is ok to ignore them.
> 
> 
> Diffs
> -
> 
>   src/linux/perf.cpp 3141e5ee9eee78e974625791f362dc345c682ebb 
>   src/tests/containerizer/perf_tests.cpp 
> d536ecc5cb24787bc3487efb146573cd4f3ded43 
> 
> 
> Diff: https://reviews.apache.org/r/56731/diff/1/
> 
> 
> Testing
> ---
> 
> sudo make check (Fedora 25)
> 
> 
> Thanks,
> 
> James Peach
> 
>

Review Request 57401: Removed redundant 'root' user set in containerizer::launch().

[Gilbert Song]

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

Review request for mesos, Adam B, Avinash sridharan, Jie Yu, and Timothy Chen.


Repository: mesos


Description
---

The 'user' in launch command is ignored, so it is not necessary to
explicitly set 'root' user to 'CommandInfo' in the case of command
task.


Diffs
-

  src/slave/containerizer/mesos/containerizer.cpp 
d2b4f75a55dbe4746bc2dfc180335fa831a554ef 


Diff: https://reviews.apache.org/r/57401/diff/1/


Testing
---

make check


Thanks,

Gilbert Song

Review Request 57403: Added unit test for verifying user in command task with image specified.

[Gilbert Song]

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

Review request for mesos, Adam B, Avinash sridharan, Jie Yu, and Timothy Chen.


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


Repository: mesos


Description
---

Added unit test for verifying user in command task with image specified.


Diffs
-

  src/tests/containerizer/provisioner_docker_tests.cpp 
01c06828a05bcafd5ae40191625edbd507983a99 


Diff: https://reviews.apache.org/r/57403/diff/1/


Testing
---

make check


Thanks,

Gilbert Song

Review Request 57402: Fixed command task with container image 'root' user issue.

[Gilbert Song]

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

Review request for mesos, Adam B, Avinash sridharan, Jie Yu, and Timothy Chen.


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


Repository: mesos


Description
---

This issue is command task with container image provided specific.
We used to set user as 'root' explicitly for command task with
container image. However, this would break operators who set 'user'
for FrameworkInfo/CommandInfo to any user other than 'root' because
the task cannot access all other contents owned by 'root', e.g.,
persistent volumes, stdout/stderr or any other directories/files
written by modules.

Instead of relying on each isolator/module to explicitly chown,
Mesos should set user to 'root' right before launching the command
executor, because the root privilege is only necessary for 'chroot'
in command executor launch, which should not impact on other
components.


Diffs
-

  src/slave/containerizer/mesos/containerizer.cpp 
d2b4f75a55dbe4746bc2dfc180335fa831a554ef 
  src/slave/slave.cpp 892ce1938ac695e7913aa9139536d0787f3f5ea7 


Diff: https://reviews.apache.org/r/57402/diff/1/


Testing
---

make check


Thanks,

Gilbert Song

Re: Review Request 57341: Made the usage of C++ namespaces in 'slave/http.cpp' consistent.

[Kevin Klues]

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



This patch both does what the summary says AND adds the DELETE_NESTED_CONTAINER 
API call. Should we separate these into two separate patches?

- Kevin Klues


On March 7, 2017, 5:15 p.m., Gastón Kleiman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57341/
> ---
> 
> (Updated March 7, 2017, 5:15 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Vinod Kone.
> 
> 
> Bugs: MESOS-7120
> https://issues.apache.org/jira/browse/MESOS-7120
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Followed the convention in 'master/http.cpp' and explicitly used the
> 'mesos::' prefix.
> 
> 
> Diffs
> -
> 
>   src/slave/http.cpp bc8209cb81194ebc8b604c9ba0d4a9e176cff2f6 
> 
> 
> Diff: https://reviews.apache.org/r/57341/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Gastón Kleiman
> 
>

Re: Review Request 57386: Introduced changes to the auth photos needed for DeleteNestedContainer.

[Kevin Klues]

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



I do not know enough about the auth code to do a proper review of this.

Can you fix the summary though -- s/photos/protos/

- Kevin Klues


On March 7, 2017, 5:14 p.m., Gastón Kleiman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57386/
> ---
> 
> (Updated March 7, 2017, 5:14 p.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rukletsov, Alexander Rojas, Jie 
> Yu, Kevin Klues, and Vinod Kone.
> 
> 
> Bugs: MESOS-7120
> https://issues.apache.org/jira/browse/MESOS-7120
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Introduced changes to the auth photos needed for DeleteNestedContainer.
> 
> 
> Diffs
> -
> 
>   include/mesos/authorizer/acls.proto 
> 8389917d12f9a9a3c9b4281f48e23ade14c20528 
>   include/mesos/authorizer/authorizer.proto 
> fdc4817ce74c45d792fc47f064f7909a83b1657d 
> 
> 
> Diff: https://reviews.apache.org/r/57386/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Gastón Kleiman
> 
>

Re: Review Request 57385: Introduced proto changes needed for the DeleteNestedContainer API call.

[Kevin Klues]

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


Fix it, then Ship it!





include/mesos/agent/agent.proto
Lines 186 (patched)


Can you move this up to sit underneath the `KillNestedContainer` field. 
It's OK that the proto numbers are out of order.



include/mesos/v1/agent/agent.proto
Lines 186 (patched)


ditto


- Kevin Klues


On March 7, 2017, 5:13 p.m., Gastón Kleiman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57385/
> ---
> 
> (Updated March 7, 2017, 5:13 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Jie Yu, Kevin Klues, and Vinod 
> Kone.
> 
> 
> Bugs: MESOS-7120
> https://issues.apache.org/jira/browse/MESOS-7120
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This new Agent API call will make it possible to destroy the sandbox and
> runtime directories of a terminated nested container.
> 
> 
> Diffs
> -
> 
>   include/mesos/agent/agent.proto 9149724159485ea2265e1494c1ce7ef989dad20a 
>   include/mesos/v1/agent/agent.proto 34210c30ca58f50b14ff3e5a01c54003c9705121 
> 
> 
> Diff: https://reviews.apache.org/r/57385/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Gastón Kleiman
> 
>

Re: Review Request 57384: Added `Containerizer::cleanupArtifacts`.

[Jie Yu]

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




src/slave/containerizer/containerizer.hpp
Lines 158 (patched)


can we call it 'delete'? This is more consistent with 
`DeleteNestedContainer` call?


- Jie Yu


On March 7, 2017, 5:13 p.m., Gastón Kleiman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57384/
> ---
> 
> (Updated March 7, 2017, 5:13 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Jie Yu, Kevin Klues, and Vinod 
> Kone.
> 
> 
> Bugs: MESOS-7120
> https://issues.apache.org/jira/browse/MESOS-7120
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This new method cleans up the sandbox and runtime directories of a
> terminated nested container.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/containerizer.hpp 
> f65a9b9761fc254bd0778bf13aac9b256497b22f 
>   src/slave/containerizer/mesos/containerizer.hpp 
> 09f94ccb3224c14a9324961b789455b119ec2257 
>   src/slave/containerizer/mesos/containerizer.cpp 
> b001d0265ec73822193eaac74c631930acce90c0 
>   src/tests/containerizer/nested_mesos_containerizer_tests.cpp 
> ea01fe55a28d17105157004d8cf0976202a49b7c 
> 
> 
> Diff: https://reviews.apache.org/r/57384/diff/1/
> 
> 
> Testing
> ---
> 
> Added a test and verified that it works on Linux.
> 
> 
> Thanks,
> 
> Gastón Kleiman
> 
>

Re: Review Request 57384: Added `Containerizer::cleanupArtifacts`.

[Kevin Klues]

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




src/slave/containerizer/containerizer.hpp
Lines 155 (patched)


s/Destroy/Cleanup/



src/slave/containerizer/containerizer.hpp
Lines 157 (patched)


s/destroy/cleanup/

Maybe rewrite the comment to say:
```
// NOTE: You can only cleanup artifacts from a
// nested container that has been fully destroyed.
```



src/slave/containerizer/mesos/containerizer.cpp
Lines 2482 (patched)


It's probably worth putting a TODO here that this function should check 
that recovery has completed before continuing.



src/slave/containerizer/mesos/containerizer.cpp
Lines 2486-2489 (patched)


`s/The nested container/Nested Container/`

`s/hasn't/has not/`

Also, (I know it's not consistent elsewhere) but can you put the 
containerID inside single quotes? Same for the error messages below too.



src/slave/containerizer/mesos/containerizer.cpp
Lines 2494 (patched)


s/terminated/destroyed/



src/tests/containerizer/nested_mesos_containerizer_tests.cpp
Lines 2037 (patched)


Can you just have the command here be `true`?
Then you don't need the explicit destroy below -- the container will exit 
very quickly by itself.



src/tests/containerizer/nested_mesos_containerizer_tests.cpp
Lines 2052-2054 (patched)


These two comments seem redundant.



src/tests/containerizer/nested_mesos_containerizer_tests.cpp
Lines 2052-2056 (patched)


If you follow the suggestion above, you should expect an exit status of 0 
here.



src/tests/containerizer/nested_mesos_containerizer_tests.cpp
Lines 2068 (patched)


Kill this newline



src/tests/containerizer/nested_mesos_containerizer_tests.cpp
Lines 2071-2075 (patched)


I think this check is unnecessary and it desitracts from the flow of what 
the test is trying to verify.



src/tests/containerizer/nested_mesos_containerizer_tests.cpp
Lines 2083 (patched)


kill this newline



src/tests/containerizer/nested_mesos_containerizer_tests.cpp
Lines 2089 (patched)


Can you pull this wait out of the AWAIT_READY() call in order to be 
consistent with the pattern above?


- Kevin Klues


On March 7, 2017, 5:13 p.m., Gastón Kleiman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57384/
> ---
> 
> (Updated March 7, 2017, 5:13 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Jie Yu, Kevin Klues, and Vinod 
> Kone.
> 
> 
> Bugs: MESOS-7120
> https://issues.apache.org/jira/browse/MESOS-7120
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This new method cleans up the sandbox and runtime directories of a
> terminated nested container.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/containerizer.hpp 
> f65a9b9761fc254bd0778bf13aac9b256497b22f 
>   src/slave/containerizer/mesos/containerizer.hpp 
> 09f94ccb3224c14a9324961b789455b119ec2257 
>   src/slave/containerizer/mesos/containerizer.cpp 
> b001d0265ec73822193eaac74c631930acce90c0 
>   src/tests/containerizer/nested_mesos_containerizer_tests.cpp 
> ea01fe55a28d17105157004d8cf0976202a49b7c 
> 
> 
> Diff: https://reviews.apache.org/r/57384/diff/1/
> 
> 
> Testing
> ---
> 
> Added a test and verified that it works on Linux.
> 
> 
> Thanks,
> 
> Gastón Kleiman
> 
>

Re: Review Request 56730: Kill stray tasks when tearing down test cgroups.

[Jiang Yan Xu]

> On March 2, 2017, 4:40 p.m., Jiang Yan Xu wrote:
> > Did you run into issues without this patch? cgroups::destroy() *should* 
> > kill all tasks in it.
> 
> James Peach wrote:
> On my reading of `cgroups::destroy`, processes would only be killed when 
> destroying the `freezer` cgroup, since that is the only one with the 
> `freezer.state` control that would trigger the `internal::Destroyer` to run.
> 
> The linked bug 
> [MESO-7049](https://issues.apache.org/jira/browse/MESOS-7049) shows test 
> output where the cgroup teardown fails.
> 
> Jiang Yan Xu wrote:
> Your are right. But `cgroups::kill` is asynchronous and we are not 
> waiting on it. It seems we should improve `cgroups::destroy` to optionally 
> not require the freezer for this to be reliable?
> 
> Jiang Yan Xu wrote:
> Discussed with @jpeach, seems like `cgroups::remove()` should succeed 
> after `cgroups::kill` returns sccessfully in normal cases (at least in the 
> tests) because zombie processes don't prevent removal of cgroups nor do they 
> show up in `cgroup.procs`: 
> http://man7.org/training/download/cgroups_slides.pdf
> 
> This should therefore be sufficient for test teardown. Adding such an 
> option to general `cgroups::destroy` would be overkill.
> 
> We could improve documentation to make this more clear.

I would guess we can't assume this is true for processes in D state? 
http://stackoverflow.com/questions/1475683/linux-process-states

So probably limiting this to the tests is for the best.


- Jiang Yan


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


On March 7, 2017, 2:27 p.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56730/
> ---
> 
> (Updated March 7, 2017, 2:27 p.m.)
> 
> 
> Review request for mesos, haosdent huang, Jie Yu, and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-7049
> https://issues.apache.org/jira/browse/MESOS-7049
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> If a test case fails, it may leave stray tasks in the cgroup which keeps
> us from tearing it down when the test completes. Kill any stray tasks
> before destroying the cgroup.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/cgroups_tests.cpp 
> 76fabce4530ccc0a1d685cd48d932ced5a64bc58 
>   src/tests/mesos.cpp 6a96fa51dfc2a62063c3154b256bdac707b009bb 
> 
> 
> Diff: https://reviews.apache.org/r/56730/diff/1/
> 
> 
> Testing
> ---
> 
> sudo make check (Fedora 25)
> 
> 
> Thanks,
> 
> James Peach
> 
>

Re: Review Request 56730: Kill stray tasks when tearing down test cgroups.

[Jiang Yan Xu]

> On March 2, 2017, 4:40 p.m., Jiang Yan Xu wrote:
> > Did you run into issues without this patch? cgroups::destroy() *should* 
> > kill all tasks in it.
> 
> James Peach wrote:
> On my reading of `cgroups::destroy`, processes would only be killed when 
> destroying the `freezer` cgroup, since that is the only one with the 
> `freezer.state` control that would trigger the `internal::Destroyer` to run.
> 
> The linked bug 
> [MESO-7049](https://issues.apache.org/jira/browse/MESOS-7049) shows test 
> output where the cgroup teardown fails.
> 
> Jiang Yan Xu wrote:
> Your are right. But `cgroups::kill` is asynchronous and we are not 
> waiting on it. It seems we should improve `cgroups::destroy` to optionally 
> not require the freezer for this to be reliable?

Discussed with @jpeach, seems like `cgroups::remove()` should succeed after 
`cgroups::kill` returns sccessfully in normal cases (at least in the tests) 
because zombie processes don't prevent removal of cgroups nor do they show up 
in `cgroup.procs`: http://man7.org/training/download/cgroups_slides.pdf

This should therefore be sufficient for test teardown. Adding such an option to 
general `cgroups::destroy` would be overkill.

We could improve documentation to make this more clear.


- Jiang Yan


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


On March 7, 2017, 2:27 p.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56730/
> ---
> 
> (Updated March 7, 2017, 2:27 p.m.)
> 
> 
> Review request for mesos, haosdent huang, Jie Yu, and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-7049
> https://issues.apache.org/jira/browse/MESOS-7049
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> If a test case fails, it may leave stray tasks in the cgroup which keeps
> us from tearing it down when the test completes. Kill any stray tasks
> before destroying the cgroup.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/cgroups_tests.cpp 
> 76fabce4530ccc0a1d685cd48d932ced5a64bc58 
>   src/tests/mesos.cpp 6a96fa51dfc2a62063c3154b256bdac707b009bb 
> 
> 
> Diff: https://reviews.apache.org/r/56730/diff/1/
> 
> 
> Testing
> ---
> 
> sudo make check (Fedora 25)
> 
> 
> Thanks,
> 
> James Peach
> 
>

Re: Review Request 57059: Updated default executor tests.

[Vinod Kone]

> On March 7, 2017, 11:58 p.m., Gilbert Song wrote:
> >

As discussed offline, I will do these changes in a subsequent patch since this 
patch is mainly about code movement.


- Vinod


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


On March 7, 2017, 1:17 a.m., Vinod Kone wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57059/
> ---
> 
> (Updated March 7, 2017, 1:17 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar and Gilbert Song.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Reorganized so that objects are defined closer to their usage.
> 
> 
> Diffs
> -
> 
>   src/tests/default_executor_tests.cpp 
> e4d43c8ad447577a9c5c7951207596bda1070856 
> 
> 
> Diff: https://reviews.apache.org/r/57059/diff/2/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Vinod Kone
> 
>

Review Request 57399: Expanded the critical section inside `ProcessManager::spawn`.

[Joseph Wu]

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

Review request for mesos, Benjamin Mahler and Greg Mann.


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


Repository: mesos


Description
---

This eliminates a race between `process::finalize` and
`ProcessManager::spawn` in which a managed process
(`ProcessManager::spawn(X, true)`) will reference the `gc`
process after `gc` has been destroyed.  The precise
interleaving is possible because the critical section in
`ProcessManager::spawn` only protects access to the
`processes` map.  Meaning that a spawn can pass the check
for `finalizing` and then wait until `gc` has been destroyed.

This commit expands the critical section in `ProcessManager::spawn`
to include the check for `finalizing` as well as the dereferencing
of `gc`.


Diffs
-

  3rdparty/libprocess/src/process.cpp 9eb7fe3d20aa9416db5162fa275fcf116f5d6477 


Diff: https://reviews.apache.org/r/57399/diff/1/


Testing
---

cmake .. -DENABLE_LIBEVENT=1 -DENABLE_SSL=1

make check 

src/mesos-tests 
--gtest_filter="ContentTypeAndSSLConfig/SchedulerSSLTest.RunTaskAndTeardown/1" 
--gtest_repeat=1000


Thanks,

Joseph Wu

Re: Review Request 57059: Updated default executor tests.

[Gilbert Song]

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




src/tests/default_executor_tests.cpp
Lines 153-160 (original), 142-149 (patched)


Could we use `v1::scheduler::SendSubscribe()` for these part?

We can do it once receive `connected()` at line  #119:
```
  Future connected;
  EXPECT_CALL(*scheduler, connected(_))
.WillOnce(DoAll(v1::scheduler::SendSubscribe(frameworkInfo),
FutureSatisfy()));
```

ditto to the others.



src/tests/default_executor_tests.cpp
Lines 166-167 (original), 154-161 (patched)


We could use the `v1::createExecutorInfo()` helper here:

```
  v1::FrameworkID frameworkId(subscribed->framework_id());
  v1::ExecutorInfo executorInfo = v1::createExecutorInfo(
  "test_default_executor",
  None(),
  "cpus:0.1;mem:32;disk:32",
  v1::ExecutorInfo::DEFAULT);

  // Update `executorInfo` with the subscribed `frameworkId`.
  executorInfo.mutable_framework_id()->CopyFrom(frameworkId);
```

Ideally, we should make `FrameworkId` configurable in the helper 
`createExecutorInfo()`.



src/tests/default_executor_tests.cpp
Lines 185-203 (original), 179-197 (patched)


use `v1::createCallAccept()`?



src/tests/default_executor_tests.cpp
Line 207 (original), 201 (patched)


Not yours, but should we add check for `TASK_FINISHED`?


- Gilbert Song


On March 6, 2017, 5:17 p.m., Vinod Kone wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57059/
> ---
> 
> (Updated March 6, 2017, 5:17 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar and Gilbert Song.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Reorganized so that objects are defined closer to their usage.
> 
> 
> Diffs
> -
> 
>   src/tests/default_executor_tests.cpp 
> e4d43c8ad447577a9c5c7951207596bda1070856 
> 
> 
> Diff: https://reviews.apache.org/r/57059/diff/2/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Vinod Kone
> 
>

Re: Review Request 57395: Fixed a typo in libprocess 'Socket'.

[Anand Mazumdar]

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


Ship it!




Ship It!

- Anand Mazumdar


On March 7, 2017, 10:39 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57395/
> ---
> 
> (Updated March 7, 2017, 10:39 p.m.)
> 
> 
> Review request for mesos and Anand Mazumdar.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed a typo in libprocess 'Socket' comment.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/socket.hpp 
> 0d3b6ec6645b6a06f2218ce36b148f57cc7cea85 
> 
> 
> Diff: https://reviews.apache.org/r/57395/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 57059: Updated default executor tests.

[Anand Mazumdar]

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


Ship it!




LGTM

- Anand Mazumdar


On March 7, 2017, 1:17 a.m., Vinod Kone wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57059/
> ---
> 
> (Updated March 7, 2017, 1:17 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar and Gilbert Song.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Reorganized so that objects are defined closer to their usage.
> 
> 
> Diffs
> -
> 
>   src/tests/default_executor_tests.cpp 
> e4d43c8ad447577a9c5c7951207596bda1070856 
> 
> 
> Diff: https://reviews.apache.org/r/57059/diff/2/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Vinod Kone
> 
>

Re: Review Request 53840: Metric in the allocator to track latency between allocation cycles.

[Anindya Sinha]

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

(Updated March 7, 2017, 11:41 p.m.)


Review request for mesos and Jiang Yan Xu.


Changes
---

Changed the metric to track latency between allocation cycles, as opposed to 
tracking the last allocation cycle.


Summary (updated)
-

Metric in the allocator to track latency between allocation cycles.


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


Repository: mesos


Description (updated)
---

Added a metric 'allocator/mesos/allocation_run_interval_ms' which
tracks the latency between consecutive allocation cycles.


Diffs (updated)
-

  src/master/allocator/mesos/hierarchical.cpp 
0059ccead90f32491591990c791e7fa905a864b7 
  src/master/allocator/mesos/metrics.hpp 
ce486eb079dc44bf0bbfad8668426c7ae87c97b3 
  src/master/allocator/mesos/metrics.cpp 
ec987fe0ce8605bf9e7c0ccbe3ac384deaf53be1 
  src/tests/hierarchical_allocator_tests.cpp 
cdf1f15b7802439b28405ca8f6634ce83e886630 


Diff: https://reviews.apache.org/r/53840/diff/3/

Changes: https://reviews.apache.org/r/53840/diff/2-3/


Testing
---

All tests passed.


Thanks,

Anindya Sinha

Re: Review Request 53842: Add role specific metrics for sorting runs.

[Anindya Sinha]

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

(Updated March 7, 2017, 11:41 p.m.)


Review request for mesos and Jiang Yan Xu.


Changes
---

Track latency of sort runs for all frameworks belonging to specific roles.


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


Repository: mesos


Description
---

Added the following 2 metrics which is maintained on a role level
(as long as there is at least one framework of that role):
a) allocator/mesos/frameworks/role//sort_runs: Number of framework
   level sorts (based on role) in DRF Sorter.
b) allocator/mesos/frameworks/role//sort_run: Latency in framework
   level sorts (based on role) in DRF Sorter.


Diffs (updated)
-

  src/master/allocator/mesos/hierarchical.hpp 
646f66e67d9c6b8c61fc6e6558a1db976a44c126 
  src/master/allocator/mesos/hierarchical.cpp 
0059ccead90f32491591990c791e7fa905a864b7 
  src/tests/hierarchical_allocator_tests.cpp 
cdf1f15b7802439b28405ca8f6634ce83e886630 


Diff: https://reviews.apache.org/r/53842/diff/3/

Changes: https://reviews.apache.org/r/53842/diff/2-3/


Testing
---

All tests passed.


Thanks,

Anindya Sinha

Re: Review Request 53841: Added metrics for sorting of the sorters in the allocator.

[Anindya Sinha]

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

(Updated March 7, 2017, 11:41 p.m.)


Review request for mesos and Jiang Yan Xu.


Changes
---

Track latency of sorts for role and quota sorters when the dirty bit is set 
only.


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


Repository: mesos


Description (updated)
---

Following metrics have been added:
a) allocator/mesos/roles/sort_runs: Number of role level sorts.
b) allocator/mesos/roles/sort_run: Latency in role level sorts.
c) allocator/mesos/quotas/sort_runs: Number of quota level sorts.
d) allocator/mesos/quotas/sort_run: Latency in quota level sorts.


Diffs (updated)
-

  src/master/allocator/mesos/hierarchical.hpp 
646f66e67d9c6b8c61fc6e6558a1db976a44c126 
  src/master/allocator/sorter/drf/metrics.hpp 
61568cb520826ab59d675824b212e0d3deb63764 
  src/master/allocator/sorter/drf/metrics.cpp 
15aab32db5ca1a7a14080e9bbb7c65283be3ec20 
  src/master/allocator/sorter/drf/sorter.hpp 
76329220e1115c1de7810fb69b943c78c078be59 
  src/master/allocator/sorter/drf/sorter.cpp 
ed54680cecb637931fc344fbcf8fd3b14cc24295 
  src/tests/hierarchical_allocator_tests.cpp 
cdf1f15b7802439b28405ca8f6634ce83e886630 


Diff: https://reviews.apache.org/r/53841/diff/3/

Changes: https://reviews.apache.org/r/53841/diff/2-3/


Testing
---

All tests passed.


Thanks,

Anindya Sinha

Re: Review Request 57058: Updated default executor tests to exclusively use v1 protos.

[Gilbert Song]

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


Ship it!





src/tests/default_executor_tests.cpp
Lines 215-217 (original), 215-217 (patched)


Seems like these are the `devolve`s that we cannot avoid. :(


- Gilbert Song


On March 6, 2017, 5:15 p.m., Vinod Kone wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57058/
> ---
> 
> (Updated March 6, 2017, 5:15 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar and Gilbert Song.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Now all the tests in this file use v1 protos.
> 
> 
> Diffs
> -
> 
>   src/tests/default_executor_tests.cpp 
> e4d43c8ad447577a9c5c7951207596bda1070856 
> 
> 
> Diff: https://reviews.apache.org/r/57058/diff/2/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Vinod Kone
> 
>

Re: Review Request 57358: Fixed FD leak in SSL server socket cleanup.

[Greg Mann]

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


Fix it, then Ship it!




Verified that this fixes the FD leak and all tests pass after the patch.

What do you think about adding a TODO and create a JIRA to fix this in a way 
that doesn't alter the discard semantics of the futures returned by 
`LibeventSSLSocketImpl::accept`? (Maybe investigate the ramifications of 
altering `Queue` to remove promises associated with discarded futures?) It's 
not ideal to change the discard behavior, but who knows if it will ever 
actually be an issue for us.


3rdparty/libprocess/src/libevent_ssl_socket.cpp
Lines 967 (patched)


s/an/any/


- Greg Mann


On March 7, 2017, 8:19 p.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57358/
> ---
> 
> (Updated March 7, 2017, 8:19 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Jie Yu, and Michael Park.
> 
> 
> Bugs: MESOS-6919
> https://issues.apache.org/jira/browse/MESOS-6919
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This commit deals with a specific case where the SocketImpl class
> passes a self-reference (shared_ptr) into a Future continuation and
> then discards the original Future. The behavior of `Future::discard`
> is that the `onDiscard` event is only chained to the future immediately
> following the discarded future.  i.e.
> 
> ```
> Future top;
> top
>   .onDiscard([]() { /* This gets executed. */ })
> 
>   .then([](const A&) { return Future(); })
>   .onDiscard([]() { /* This also gets executed. */ })
> 
>   .then([](const B&) { return Future(); })
>   .onDiscard([]() { /* But not this. */ });
> 
> top.discard();
> ```
> 
> When a Future is discarded, we only clear the `onDiscard` callbacks,
> which means that all other continuations will continue to reside in
> memory.  In the case of the `Socket` class, the Libevent SSL socket
> implementation had several levels of continuations:
> 
> ```
> // Each item in this queue is backed by a Promise<>::Future.
> Queue> accept_queue;
> 
> // LibeventSSLSocketImpl::accept.
> accept_queue.get()
>   .then(...)
> 
> // Socket::accept. This continuation holds a self-reference
> // to the `shared_ptr`.
>   .then([self](...) {...})
> ```
> 
> Because the second continuation is never discarded nor called, we
> end up with a dangling pointer.  For the `Socket` class, this leads
> to an FD leak.
> 
> This commit extends the future returned by the Libevent SSL Socket
> to deallocate some extra memory upon being discarded.  This will
> remove the extra reference to the SocketImpl class and thereby
> resolve the issue of a dangling pointer.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/libevent_ssl_socket.hpp 
> 9493a50243340a1c48ab1c67f6e61cc081ef24ce 
>   3rdparty/libprocess/src/libevent_ssl_socket.cpp 
> 7d493301bd5c0f24bf89e0b213f07ffe7801508b 
> 
> 
> Diff: https://reviews.apache.org/r/57358/diff/2/
> 
> 
> Testing
> ---
> 
> cmake .. -DENABLE_LIBEVENT=1 -DENABLE_SSL=1
> 
> make libprocess-tests
> 
> 3rdparty/libprocess/src/tests/libprocess-tests 
> --gtest_filter="Scheme/HTTPTest.Endpoints/0" --gtest_repeat="`ulimit -n`"
> 
> make check
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Re: Review Request 55888: Test to ensure non-authorized users cannot launch tasks on agents.

[Anindya Sinha]

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

(Updated March 7, 2017, 11:19 p.m.)


Review request for mesos, Adam B, Alexander Rojas, and Jiang Yan Xu.


Changes
---

Rebased.


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


Repository: mesos


Description
---

Test to ensure non-authorized users cannot launch tasks on agents.


Diffs (updated)
-

  src/tests/slave_authorization_tests.cpp 
61c44460eb79931dd77c5100eb33a31a182c1710 


Diff: https://reviews.apache.org/r/55888/diff/8/

Changes: https://reviews.apache.org/r/55888/diff/7-8/


Testing
---

All tests passed.


Thanks,

Anindya Sinha

Re: Review Request 55887: Check task user before allowing a task to be launched on the agent.

[Anindya Sinha]

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

(Updated March 7, 2017, 11:19 p.m.)


Review request for mesos, Adam B, Anand Mazumdar, Alexander Rojas, and Jiang 
Yan Xu.


Changes
---

A minor fix to handle the case when framework is in TERMINATING state.


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


Repository: mesos


Description
---

Added support for action `run_tasks` on the agent's flag `acl`. Based on
the ACL configured for `run_tasks`, a task to be launched on the agent
can be (dis)allowed to launch on the agent.
If a task or task group cannot be launched due to failed authorization,
a `TASK_ERROR` Status Update shall be sent with a reason code of
`REASON_TASK_UNAUTHORIZED` or `REASON_TASK_GROUP_UNAUTHORIZED` as
applicable.
Note that in case of a task group, all tasks fail if any of the tasks
within the task group encounter the authorization error.


Diffs (updated)
-

  src/slave/slave.hpp 978edd6309dfbbde1058f9c44d5fac7083ff95fb 
  src/slave/slave.cpp 4319f841fbdc7ad39eb60eb52ae2a764b133cfbd 


Diff: https://reviews.apache.org/r/55887/diff/8/

Changes: https://reviews.apache.org/r/55887/diff/7-8/


Testing
---

All tests passed.


Thanks,

Anindya Sinha

Re: Review Request 55887: Check task user before allowing a task to be launched on the agent.

[Anindya Sinha]

> On March 2, 2017, 9:26 a.m., Jiang Yan Xu wrote:
> > src/slave/slave.cpp
> > Lines 1830-1840 (original), 1818-1835 (patched)
> > 
> >
> > This results in a behavior change: if tasks are killed during 
> > unscheduling of directories, we now send TASK_DROPPED instead of 
> > TASK_KILLED. I think this is a better behavior because this is race anyways 
> > and the TASK_DROPPED here better reveals the error condition on the host.
> > 
> > Nevertheless we could check with the @anand and @vinod who last changed 
> > this.
> 
> Anindya Sinha wrote:
> I reinstated the original behavior. We can followup with @anand and 
> @vinod later.
> 
> Jiang Yan Xu wrote:
> So I checked with @anandm on slack: 
> http://mesos.slackarchive.io/dev/-/1487591716.002096/1488847311.002217/1488846253002216/
> 
> He convinced me that it makes sense to consistently send `TASK_KILLED` in 
> this case:
> 
> It takes multiple async steps to launch a task: 1) unscheudling GC, (2, 
> new) authorize task, 3) launch executor and so far if a task kill lands on 
> the agent in the other two cases it would result in a `TASK_KILLED`. We 
> should probalby send `TASK_KILLED` for 2) as well.
> 
> Agreed?

Yes, that is the case in this version.


- Anindya


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


On March 7, 2017, 12:51 a.m., Anindya Sinha wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55887/
> ---
> 
> (Updated March 7, 2017, 12:51 a.m.)
> 
> 
> Review request for mesos, Adam B, Anand Mazumdar, Alexander Rojas, and Jiang 
> Yan Xu.
> 
> 
> Bugs: MESOS-6953
> https://issues.apache.org/jira/browse/MESOS-6953
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added support for action `run_tasks` on the agent's flag `acl`. Based on
> the ACL configured for `run_tasks`, a task to be launched on the agent
> can be (dis)allowed to launch on the agent.
> If a task or task group cannot be launched due to failed authorization,
> a `TASK_ERROR` Status Update shall be sent with a reason code of
> `REASON_TASK_UNAUTHORIZED` or `REASON_TASK_GROUP_UNAUTHORIZED` as
> applicable.
> Note that in case of a task group, all tasks fail if any of the tasks
> within the task group encounter the authorization error.
> 
> 
> Diffs
> -
> 
>   src/slave/slave.hpp 857338cc54b341873c338967223c6b0138e9dc3e 
>   src/slave/slave.cpp 775f43b449ec53aa51cc5d3de448ddc7c2059bff 
> 
> 
> Diff: https://reviews.apache.org/r/55887/diff/7/
> 
> 
> Testing
> ---
> 
> All tests passed.
> 
> 
> Thanks,
> 
> Anindya Sinha
> 
>

Re: Review Request 56667: Added support for JSON Web Tokens.

[Greg Mann]

> On March 7, 2017, 9:37 p.m., Vinod Kone wrote:
> > 3rdparty/libprocess/src/jwt.cpp
> > Lines 194 (patched)
> > 
> >
> > s/JOSE/JSON/ ?

Strangely, "JOSE header" just happens to be the name of the JWT header: 
https://tools.ietf.org/html/rfc7519#section-5


- Greg


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


On March 7, 2017, 3:57 p.m., Jan Schlicht wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56667/
> ---
> 
> (Updated March 7, 2017, 3:57 p.m.)
> 
> 
> Review request for mesos, Alexander Rojas and Greg Mann.
> 
> 
> Bugs: MESOS-7001
> https://issues.apache.org/jira/browse/MESOS-7001
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> JSON Web Tokens can be used to create claim-based access tokens and is
> typically used for HTTP authentication.
> This implementation is intended for internal use, e.g. Mesos is supposed
> to only parse tokens that it also created. It doesn't fully comply with
> RFC 7519. Currently the only supported cryptographic algorithm is HMAC
> with SHA-256.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/Makefile.am 75386184108214e67a58c328258ec204099d638c 
>   3rdparty/libprocess/include/process/jwt.hpp PRE-CREATION 
>   3rdparty/libprocess/src/jwt.cpp PRE-CREATION 
>   3rdparty/libprocess/src/tests/jwt_tests.cpp PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/56667/diff/7/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jan Schlicht
> 
>

Re: Review Request 56667: Added support for JSON Web Tokens.

[Greg Mann]

> On March 6, 2017, 11:33 p.m., Greg Mann wrote:
> > 3rdparty/libprocess/src/jwt.cpp
> > Lines 169 (patched)
> > 
> >
> > Do you think `parse_component` would be a better name for this?
> 
> Jan Schlicht wrote:
> But it's also base64-decoding the component. Of course, in addition to 
> parsing JSON. How about `decode_and_parse`?
> 
> Greg Mann wrote:
> Per our discussion, we can use a name like `decode_to_json` for this.
> 
> Vinod Kone wrote:
> The fact that this is decoding to JSON is obvious from the signature 
> right? I would prefer to keep it as `decode` or use `decode_and_parse` in 
> that case.

Yea fair point, the signature makes it evident. No strong feelings here, I'd be 
fine leaving it as-is or as `decode_and_parse`.


- Greg


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


On March 7, 2017, 3:57 p.m., Jan Schlicht wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56667/
> ---
> 
> (Updated March 7, 2017, 3:57 p.m.)
> 
> 
> Review request for mesos, Alexander Rojas and Greg Mann.
> 
> 
> Bugs: MESOS-7001
> https://issues.apache.org/jira/browse/MESOS-7001
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> JSON Web Tokens can be used to create claim-based access tokens and is
> typically used for HTTP authentication.
> This implementation is intended for internal use, e.g. Mesos is supposed
> to only parse tokens that it also created. It doesn't fully comply with
> RFC 7519. Currently the only supported cryptographic algorithm is HMAC
> with SHA-256.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/Makefile.am 75386184108214e67a58c328258ec204099d638c 
>   3rdparty/libprocess/include/process/jwt.hpp PRE-CREATION 
>   3rdparty/libprocess/src/jwt.cpp PRE-CREATION 
>   3rdparty/libprocess/src/tests/jwt_tests.cpp PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/56667/diff/7/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jan Schlicht
> 
>

Review Request 57395: Fixed a typo in libprocess 'Socket'.

[Greg Mann]

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

Review request for mesos and Anand Mazumdar.


Repository: mesos


Description
---

Fixed a typo in libprocess 'Socket' comment.


Diffs
-

  3rdparty/libprocess/include/process/socket.hpp 
0d3b6ec6645b6a06f2218ce36b148f57cc7cea85 


Diff: https://reviews.apache.org/r/57395/diff/1/


Testing
---


Thanks,

Greg Mann

Re: Review Request 57388: Added documentation for the 'DeleteNestedContainer' Agent API call.

[Mesos Reviewbot]

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



Patch looks great!

Reviews applied: [57384, 57385, 57386, 57341, 57387, 57388]

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 March 7, 2017, 5:17 p.m., Gastón Kleiman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57388/
> ---
> 
> (Updated March 7, 2017, 5:17 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Jie Yu, Kevin Klues, and Vinod 
> Kone.
> 
> 
> Bugs: MESOS-7120
> https://issues.apache.org/jira/browse/MESOS-7120
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added documentation for the 'DeleteNestedContainer' Agent API call.
> 
> 
> Diffs
> -
> 
>   docs/operator-http-api.md 506251b120c6d828e7324f29dda5951f39187af2 
> 
> 
> Diff: https://reviews.apache.org/r/57388/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Gastón Kleiman
> 
>

Re: Review Request 56730: Kill stray tasks when tearing down test cgroups.

[Jiang Yan Xu]

> On March 2, 2017, 4:40 p.m., Jiang Yan Xu wrote:
> > Did you run into issues without this patch? cgroups::destroy() *should* 
> > kill all tasks in it.
> 
> James Peach wrote:
> On my reading of `cgroups::destroy`, processes would only be killed when 
> destroying the `freezer` cgroup, since that is the only one with the 
> `freezer.state` control that would trigger the `internal::Destroyer` to run.
> 
> The linked bug 
> [MESO-7049](https://issues.apache.org/jira/browse/MESOS-7049) shows test 
> output where the cgroup teardown fails.

Your are right. But `cgroups::kill` is asynchronous and we are not waiting on 
it. It seems we should improve `cgroups::destroy` to optionally not require the 
freezer for this to be reliable?


- Jiang Yan


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


On March 7, 2017, 2:27 p.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56730/
> ---
> 
> (Updated March 7, 2017, 2:27 p.m.)
> 
> 
> Review request for mesos, haosdent huang, Jie Yu, and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-7049
> https://issues.apache.org/jira/browse/MESOS-7049
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> If a test case fails, it may leave stray tasks in the cgroup which keeps
> us from tearing it down when the test completes. Kill any stray tasks
> before destroying the cgroup.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/cgroups_tests.cpp 
> 76fabce4530ccc0a1d685cd48d932ced5a64bc58 
>   src/tests/mesos.cpp 6a96fa51dfc2a62063c3154b256bdac707b009bb 
> 
> 
> Diff: https://reviews.apache.org/r/56730/diff/1/
> 
> 
> Testing
> ---
> 
> sudo make check (Fedora 25)
> 
> 
> Thanks,
> 
> James Peach
> 
>

Re: Review Request 56733: Mount all supported subsystems in the containerizer tests.

[Jiang Yan Xu]

> On March 7, 2017, 11:28 a.m., Jiang Yan Xu wrote:
> > So I was thinking about this: the code here makes sure the OS has the 
> > minimum set of subsystems required by this test suite. If the system 
> > doesn't meet the requirement, we do want to terminate here. On the other 
> > hand, although mounting more subsystems isn't harmful, if we mount 
> > everything that comes out of `cgroups::subsystems()`, we should still check 
> > whether that list is a superset of the minimumally required set. At a 
> > result we still have to add items here every time a new subsystem is added 
> > so we are not saving much?
> > 
> > Thoughts?
> 
> James Peach wrote:
> I think you are reading too much into this. Is this really a contract 
> about what subsystems the tests require? I'd argue that it is just an 
> arbitrary selection from history. Mounting everything is robust and requires 
> no future maintenance. Tests that need to verify specific subsystems are 
> present can do that.

Thanks. I think you are right. There's not a minimum set, `perf_event` probably 
shouldn't be there at all since we actually filter on it. 

I'll commit this but as a separate dicussion, I wonder if the `SetUp()` is 
needed at all? /cc @haosdent


- Jiang Yan


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


On March 6, 2017, 11:34 a.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56733/
> ---
> 
> (Updated March 6, 2017, 11:34 a.m.)
> 
> 
> Review request for mesos, Mesos Reviewbot and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-7049
> https://issues.apache.org/jira/browse/MESOS-7049
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Rather than a hard-coded list of subsystems, just mount everything we
> find that is supported.
> 
> 
> Diffs
> -
> 
>   src/tests/mesos.cpp 6a96fa51dfc2a62063c3154b256bdac707b009bb 
> 
> 
> Diff: https://reviews.apache.org/r/56733/diff/2/
> 
> 
> Testing
> ---
> 
> sudo make check (Fedora 25)
> 
> 
> Thanks,
> 
> James Peach
> 
>

Re: Review Request 54993: Modify LevelDB patch to add ARM64/AArch64 support.

[Aaron Wood via Review Board]

> On Dec. 27, 2016, 1:48 p.m., Till Toenshoff wrote:
> > This sums up the following changes on `atomic_pointer.h` done within the 
> > upstream project;
> > https://github.com/google/leveldb/commit/803d69203a62faf50f1b77897310a3a1fcae712b
> > https://github.com/google/leveldb/commit/c4c38f9c1f3bb405fe22a79c5611438f91208d09
> > https://github.com/google/leveldb/commit/ceff6f12152785a54885a47db349a6d8dfd0ce2c
> > 
> > Note that some of those changes are unrelated to ARM64 and hence not 
> > strictly within the scope
> > of this ticket -- so we do not need those changes. However, having things 
> > in sync with upstream
> > is definitely valuable and in these specific cases appears to be free of 
> > additional risks.

Should we close this now that LevelDB has been upgraded to 1.19?


- Aaron


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


On Dec. 22, 2016, 9:10 p.m., Aaron Wood wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54993/
> ---
> 
> (Updated Dec. 22, 2016, 9:10 p.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-6834
> https://issues.apache.org/jira/browse/MESOS-6834
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Mesos will not compile on ARM64/AArch64 without a patch to the version of 
> LevelDB that is used within Mesos. While the fix is already in newer versions 
> of LevelDB it's not something that Mesos pulls down.
> 
> The main issue is that the AtomicPointer header needs to be aware of other 
> architectures to provide its functionality to those architectures.
> 
> 
> Diffs
> -
> 
>   3rdparty/leveldb-1.4.patch b899f0141 
> 
> 
> Diff: https://reviews.apache.org/r/54993/diff/1/
> 
> 
> Testing
> ---
> 
> Compiled Mesos on ARM64 with no failures. Mesos also properly starts and 
> successfully runs/launches tasks with the exception of a crash when using the 
> linux_launcher and Mesos containers. That fix is addressed in 
> https://reviews.apache.org/r/54996/
> 
> 
> Thanks,
> 
> Aaron Wood
> 
>

Re: Review Request 56667: Added support for JSON Web Tokens.

[Vinod Kone]

> On March 6, 2017, 11:33 p.m., Greg Mann wrote:
> > 3rdparty/libprocess/src/jwt.cpp
> > Lines 169 (patched)
> > 
> >
> > Do you think `parse_component` would be a better name for this?
> 
> Jan Schlicht wrote:
> But it's also base64-decoding the component. Of course, in addition to 
> parsing JSON. How about `decode_and_parse`?
> 
> Greg Mann wrote:
> Per our discussion, we can use a name like `decode_to_json` for this.

The fact that this is decoding to JSON is obvious from the signature right? I 
would prefer to keep it as `decode` or use `decode_and_parse` in that case.


- Vinod


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


On March 7, 2017, 3:57 p.m., Jan Schlicht wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56667/
> ---
> 
> (Updated March 7, 2017, 3:57 p.m.)
> 
> 
> Review request for mesos, Alexander Rojas and Greg Mann.
> 
> 
> Bugs: MESOS-7001
> https://issues.apache.org/jira/browse/MESOS-7001
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> JSON Web Tokens can be used to create claim-based access tokens and is
> typically used for HTTP authentication.
> This implementation is intended for internal use, e.g. Mesos is supposed
> to only parse tokens that it also created. It doesn't fully comply with
> RFC 7519. Currently the only supported cryptographic algorithm is HMAC
> with SHA-256.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/Makefile.am 75386184108214e67a58c328258ec204099d638c 
>   3rdparty/libprocess/include/process/jwt.hpp PRE-CREATION 
>   3rdparty/libprocess/src/jwt.cpp PRE-CREATION 
>   3rdparty/libprocess/src/tests/jwt_tests.cpp PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/56667/diff/7/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jan Schlicht
> 
>

Re: Review Request 56667: Added support for JSON Web Tokens.

[Vinod Kone]

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




3rdparty/libprocess/src/jwt.cpp
Lines 120 (patched)


return JWTError("Failed to generate HMAC signature: " + hmac.error(), 
JWTError::Type::UNKNOWN);



3rdparty/libprocess/src/jwt.cpp
Lines 150-152 (patched)


the indentation looks like `generate_hmac_sha256` takes 3 args. maybe 
indent the 2nd line by couple spaces?



3rdparty/libprocess/src/jwt.cpp
Lines 155 (patched)


ditto. Add a prefix to the error message "Failed to generate HMAC 
signature: "



3rdparty/libprocess/src/jwt.cpp
Lines 162-164 (patched)


fix indentation of args.



3rdparty/libprocess/src/jwt.cpp
Lines 173 (patched)


"Failed to decode: " prefix



3rdparty/libprocess/src/jwt.cpp
Lines 179 (patched)


"Failed to parse into JSON: " prefix



3rdparty/libprocess/src/jwt.cpp
Lines 191 (patched)


s/Could not/Failed to/



3rdparty/libprocess/src/jwt.cpp
Lines 194 (patched)


s/JOSE/JSON/ ?



3rdparty/libprocess/src/jwt.cpp
Lines 202 (patched)


s/as expected//



3rdparty/libprocess/src/jwt.cpp
Lines 221 (patched)


s/as expected//



3rdparty/libprocess/src/jwt.cpp
Lines 254 (patched)


s/Could not/Failed to/



3rdparty/libprocess/src/jwt.cpp
Lines 282 (patched)


s/validate/Validate/



3rdparty/libprocess/src/tests/jwt_tests.cpp
Lines 50 (patched)


s/Broken/Invalid/



3rdparty/libprocess/src/tests/jwt_tests.cpp
Lines 61 (patched)


s/Broken/Invalid/



3rdparty/libprocess/src/tests/jwt_tests.cpp
Lines 72 (patched)


s/Wrong/Unsupported/



3rdparty/libprocess/src/tests/jwt_tests.cpp
Lines 118 (patched)


period at the end.

here and below.



3rdparty/libprocess/src/tests/jwt_tests.cpp
Lines 198 (patched)


can you do EXPECT_SOME?



3rdparty/libprocess/src/tests/jwt_tests.cpp
Lines 209 (patched)


ditto. EXPECT_SOME?



3rdparty/libprocess/src/tests/jwt_tests.cpp
Lines 233 (patched)


period at the end.

here and below.



3rdparty/libprocess/src/tests/jwt_tests.cpp
Lines 237 (patched)


EXPECT_SOME?



3rdparty/libprocess/src/tests/jwt_tests.cpp
Lines 239 (patched)


jwt->header.alg



3rdparty/libprocess/src/tests/jwt_tests.cpp
Lines 244 (patched)


jwt->signature



3rdparty/libprocess/src/tests/jwt_tests.cpp
Lines 251 (patched)


EXPECT_SOME?



3rdparty/libprocess/src/tests/jwt_tests.cpp
Lines 253 (patched)


jwt->header.alg



3rdparty/libprocess/src/tests/jwt_tests.cpp
Lines 254 (patched)


jwt->header.typ



3rdparty/libprocess/src/tests/jwt_tests.cpp
Lines 256 (patched)


jwt->payload



3rdparty/libprocess/src/tests/jwt_tests.cpp
Lines 258 (patched)


jwt->signature



3rdparty/libprocess/src/tests/jwt_tests.cpp
Lines 271 (patched)


EXPECT_SOME?


- Vinod Kone


On March 7, 2017, 3:57 p.m., Jan Schlicht wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56667/
> ---
> 
> (Updated March 7, 2017, 3:57 p.m.)
> 
> 
> Review request for mesos, Alexander Rojas and Greg Mann.
> 
> 
> Bugs: MESOS-7001
> https://issues.apache.org/jira/browse/MESOS-7001
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> JSON Web Tokens can be used to create claim-based access tokens and is
> typically 

Re: Review Request 56666: Added a HMAC SHA256 generator.

[Vinod Kone]

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




3rdparty/libprocess/src/ssl/utilities.cpp
Lines 363 (patched)


if reason is nullptr the error shows up as "HMAC failed: " which is weird.

how about

```
return Error(
   "HMAC failed" + (reason == nultpr ? "" : ": " + std::string(reason)));

```


- Vinod Kone


On March 7, 2017, 3:34 p.m., Jan Schlicht wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/5/
> ---
> 
> (Updated March 7, 2017, 3:34 p.m.)
> 
> 
> Review request for mesos, Alexander Rojas and Greg Mann.
> 
> 
> Bugs: MESOS-7001
> https://issues.apache.org/jira/browse/MESOS-7001
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> HMAC SHA256 can be used to create or verify message signatures.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/ssl/utilities.hpp 
> c2f64a91a505675d568ddf5aa081125e4e32fe17 
>   3rdparty/libprocess/src/ssl/utilities.cpp 
> 8aec613312eee3dd11d9df8c3828a5185407e073 
> 
> 
> Diff: https://reviews.apache.org/r/5/diff/5/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jan Schlicht
> 
>

Re: Review Request 56213: Added check tests for command executor.

[Alexander Rukletsov]

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

(Updated March 7, 2017, 8:39 p.m.)


Review request for mesos, Gastón Kleiman and Vinod Kone.


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


Repository: mesos


Description
---

See summary.


Diffs (updated)
-

  src/tests/check_tests.cpp f035c16920deaf559420ae0d7d881330ff65ae44 
  src/tests/mesos.hpp f39e243c2c11bc1c9c757049fda2122727d1fef9 


Diff: https://reviews.apache.org/r/56213/diff/5/

Changes: https://reviews.apache.org/r/56213/diff/4-5/


Testing
---

See https://reviews.apache.org/r/56218/


Thanks,

Alexander Rukletsov

Re: Review Request 56212: Added support for general checks to command executor.

[Alexander Rukletsov]

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

(Updated March 7, 2017, 8:39 p.m.)


Review request for mesos, Gastón Kleiman and Vinod Kone.


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


Repository: mesos


Description
---

See summary.


Diffs (updated)
-

  src/launcher/executor.cpp adcc50fb7df91b9f8dff0f583b61e0efcd6da378 


Diff: https://reviews.apache.org/r/56212/diff/5/

Changes: https://reviews.apache.org/r/56212/diff/4-5/


Testing
---

See https://reviews.apache.org/r/56213/


Thanks,

Alexander Rukletsov

Re: Review Request 57150: Simplified task id procurement in command executor.

[Alexander Rukletsov]

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

(Updated March 7, 2017, 8:39 p.m.)


Review request for mesos, Gastón Kleiman and Vinod Kone.


Repository: mesos


Description
---

See summary.


Diffs (updated)
-

  src/launcher/executor.cpp adcc50fb7df91b9f8dff0f583b61e0efcd6da378 


Diff: https://reviews.apache.org/r/57150/diff/2/

Changes: https://reviews.apache.org/r/57150/diff/1-2/


Testing
---

See https://reviews.apache.org/r/56213/


Thanks,

Alexander Rukletsov

Re: Review Request 56211: Renamed health checker in command executor for clarity.

[Alexander Rukletsov]

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

(Updated March 7, 2017, 8:39 p.m.)


Review request for mesos, Gastón Kleiman and Vinod Kone.


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


Repository: mesos


Description
---

Also validate internal invariants when health is updated.


Diffs (updated)
-

  src/launcher/executor.cpp adcc50fb7df91b9f8dff0f583b61e0efcd6da378 


Diff: https://reviews.apache.org/r/56211/diff/5/

Changes: https://reviews.apache.org/r/56211/diff/4-5/


Testing
---

See https://reviews.apache.org/r/56213/


Thanks,

Alexander Rukletsov

Re: Review Request 56210: Reused previous task status for health updates in command executor.

[Alexander Rukletsov]

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

(Updated March 7, 2017, 8:39 p.m.)


Review request for mesos, Gastón Kleiman and Vinod Kone.


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


Repository: mesos


Description
---

When a new health task status update is generated in the executor, we
have to make sure specific data is duplicated from the previous one to
avoid shadowing of those data during reconciliation.


Diffs (updated)
-

  src/launcher/executor.cpp adcc50fb7df91b9f8dff0f583b61e0efcd6da378 


Diff: https://reviews.apache.org/r/56210/diff/6/

Changes: https://reviews.apache.org/r/56210/diff/5-6/


Testing
---

See https://reviews.apache.org/r/56213/


Thanks,

Alexander Rukletsov

Re: Review Request 57149: Added a warning if command executor gets an unknown acknowledgement.

[Alexander Rukletsov]

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

(Updated March 7, 2017, 8:39 p.m.)


Review request for mesos, Gastón Kleiman and Vinod Kone.


Repository: mesos


Description
---

See summary.


Diffs (updated)
-

  src/launcher/executor.cpp adcc50fb7df91b9f8dff0f583b61e0efcd6da378 


Diff: https://reviews.apache.org/r/57149/diff/2/

Changes: https://reviews.apache.org/r/57149/diff/1-2/


Testing
---

https://reviews.apache.org/r/56213/


Thanks,

Alexander Rukletsov

Re: Review Request 56208: Updated checks library with general check support.

[Alexander Rukletsov]

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

(Updated March 7, 2017, 8:39 p.m.)


Review request for mesos, Gastón Kleiman and Vinod Kone.


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


Repository: mesos


Description
---

Add support for general checks, i.e. defined by CheckInfo, in
checking library. A general check can be either an command or
an HTTP request. The library performs the requested check at
the specified interval and sends the result to the framework
via a task status update. If the current result is the same as
the previous result, no status update is sent.


Diffs (updated)
-

  src/checks/checker.hpp dc293f3d3613dec716510d269829f8a6f406c277 
  src/checks/checker.cpp 8716e4cc684e6c4b6b76d8ca53221be06d10b2a6 
  src/checks/health_checker.hpp f1f2834b3429fb00cc49c179fa9a3de328f597b5 
  src/checks/health_checker.cpp 6c97369fd9a567ba16dd92085bf142d43f71eaf1 


Diff: https://reviews.apache.org/r/56208/diff/8/

Changes: https://reviews.apache.org/r/56208/diff/7-8/


Testing
---

https://reviews.apache.org/r/56213/


Thanks,

Alexander Rukletsov

Review Request 57394: Overloaded `<<` for `CheckInfo::Type`.

[Alexander Rukletsov]

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

Review request for mesos, Gastón Kleiman and Vinod Kone.


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


Repository: mesos


Description
---

See summary.


Diffs
-

  include/mesos/type_utils.hpp c7f86ac3a8c166512410d89678a4dd2622771bf0 
  include/mesos/v1/mesos.hpp 07007429be7fc4bee7238f8dc32197ea9c7a3a7a 
  src/common/type_utils.cpp d86d56d4e1d353d6e82ccff89357bf2abec6eded 
  src/v1/mesos.cpp 5b89603e1b760256c584a188e93be31eeaea7ce2 


Diff: https://reviews.apache.org/r/57394/diff/1/


Testing
---

https://reviews.apache.org/r/56213/


Thanks,

Alexander Rukletsov

Re: Review Request 56016: Added a note about task status updates in scheduler and internal API.

[Alexander Rukletsov]

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

(Updated March 7, 2017, 8:38 p.m.)


Review request for mesos, Gastón Kleiman and Vinod Kone.


Repository: mesos


Description
---

See summary.


Diffs (updated)
-

  include/mesos/scheduler/scheduler.proto 
d04924a49d0bf28952af6cb72d972cac61e6f781 
  include/mesos/v1/scheduler/scheduler.proto 
6e8246da0af9097b6fd2fe7c9c15fc4bdc9e4fce 
  src/messages/messages.proto 508ff5960eb70b1299ef5ec02c23852c0088d295 


Diff: https://reviews.apache.org/r/56016/diff/3/

Changes: https://reviews.apache.org/r/56016/diff/2-3/


Testing
---

None: not a functional change.


Thanks,

Alexander Rukletsov

Re: Review Request 56017: Added a helper for building a task status from an existing one.

[Alexander Rukletsov]

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

(Updated March 7, 2017, 8:38 p.m.)


Review request for mesos, Gastón Kleiman and Vinod Kone.


Repository: mesos


Description
---

See summary.


Diffs (updated)
-

  src/common/protobuf_utils.hpp 09e468c77f0cdd931302d1bdcc192370b6ce3340 
  src/common/protobuf_utils.cpp 34c14e8ebd7b575627704c7edebcbb0458eeb3b1 


Diff: https://reviews.apache.org/r/56017/diff/3/

Changes: https://reviews.apache.org/r/56017/diff/2-3/


Testing
---

make check


Thanks,

Alexander Rukletsov

Re: Review Request 56665: Added a URL-safe base64 implementation.

[Vinod Kone]

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




3rdparty/stout/include/stout/base64.hpp
Lines 178-179 (patched)


why does only this function have `param` comments and not others?



3rdparty/stout/include/stout/base64.hpp
Lines 181 (patched)


why does this function take padding as an argument but not  `encode`?

also, it's not clear to me what is the motivation for adding URL safe 
encoder? could you elaborate in the description?


- Vinod Kone


On March 3, 2017, 2 p.m., Jan Schlicht wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56665/
> ---
> 
> (Updated March 3, 2017, 2 p.m.)
> 
> 
> Review request for mesos, Alexander Rojas and Greg Mann.
> 
> 
> Bugs: MESOS-7001
> https://issues.apache.org/jira/browse/MESOS-7001
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Base64 has many variants that use different alphabets for encoding.
> "Base 64 Encoding with URL and Filename Safe Alphabet" is a variant
> described in RFC 4648.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/base64.hpp 
> 2ac04c4602bc919633a2a480dd2168b7aa301bd7 
>   3rdparty/stout/tests/base64_tests.cpp 
> 32e516861d44c7e3a36e1a29b4d1fe5960684e3b 
> 
> 
> Diff: https://reviews.apache.org/r/56665/diff/4/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jan Schlicht
> 
>

Re: Review Request 56733: Mount all supported subsystems in the containerizer tests.

[James Peach]

> On March 7, 2017, 7:28 p.m., Jiang Yan Xu wrote:
> > So I was thinking about this: the code here makes sure the OS has the 
> > minimum set of subsystems required by this test suite. If the system 
> > doesn't meet the requirement, we do want to terminate here. On the other 
> > hand, although mounting more subsystems isn't harmful, if we mount 
> > everything that comes out of `cgroups::subsystems()`, we should still check 
> > whether that list is a superset of the minimumally required set. At a 
> > result we still have to add items here every time a new subsystem is added 
> > so we are not saving much?
> > 
> > Thoughts?

I think you are reading too much into this. Is this really a contract about 
what subsystems the tests require? I'd argue that it is just an arbitrary 
selection from history. Mounting everything is robust and requires no future 
maintenance. Tests that need to verify specific subsystems are present can do 
that.


- James


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


On March 6, 2017, 7:34 p.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56733/
> ---
> 
> (Updated March 6, 2017, 7:34 p.m.)
> 
> 
> Review request for mesos, Mesos Reviewbot and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-7049
> https://issues.apache.org/jira/browse/MESOS-7049
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Rather than a hard-coded list of subsystems, just mount everything we
> find that is supported.
> 
> 
> Diffs
> -
> 
>   src/tests/mesos.cpp 6a96fa51dfc2a62063c3154b256bdac707b009bb 
> 
> 
> Diff: https://reviews.apache.org/r/56733/diff/2/
> 
> 
> Testing
> ---
> 
> sudo make check (Fedora 25)
> 
> 
> Thanks,
> 
> James Peach
> 
>

Re: Review Request 57358: Fixed FD leak in SSL server socket cleanup.

[Joseph Wu]

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

(Updated March 7, 2017, 12:19 p.m.)


Review request for mesos, Benjamin Mahler, Jie Yu, and Michael Park.


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


Repository: mesos


Description (updated)
---

This commit deals with a specific case where the SocketImpl class
passes a self-reference (shared_ptr) into a Future continuation and
then discards the original Future. The behavior of `Future::discard`
is that the `onDiscard` event is only chained to the future immediately
following the discarded future.  i.e.

```
Future top;
top
  .onDiscard([]() { /* This gets executed. */ })

  .then([](const A&) { return Future(); })
  .onDiscard([]() { /* This also gets executed. */ })

  .then([](const B&) { return Future(); })
  .onDiscard([]() { /* But not this. */ });

top.discard();
```

When a Future is discarded, we only clear the `onDiscard` callbacks,
which means that all other continuations will continue to reside in
memory.  In the case of the `Socket` class, the Libevent SSL socket
implementation had several levels of continuations:

```
// Each item in this queue is backed by a Promise<>::Future.
Queue> accept_queue;

// LibeventSSLSocketImpl::accept.
accept_queue.get()
  .then(...)

// Socket::accept. This continuation holds a self-reference
// to the `shared_ptr`.
  .then([self](...) {...})
```

Because the second continuation is never discarded nor called, we
end up with a dangling pointer.  For the `Socket` class, this leads
to an FD leak.

This commit extends the future returned by the Libevent SSL Socket
to deallocate some extra memory upon being discarded.  This will
remove the extra reference to the SocketImpl class and thereby
resolve the issue of a dangling pointer.


Diffs
-

  3rdparty/libprocess/src/libevent_ssl_socket.hpp 
9493a50243340a1c48ab1c67f6e61cc081ef24ce 
  3rdparty/libprocess/src/libevent_ssl_socket.cpp 
7d493301bd5c0f24bf89e0b213f07ffe7801508b 


Diff: https://reviews.apache.org/r/57358/diff/2/


Testing
---

cmake .. -DENABLE_LIBEVENT=1 -DENABLE_SSL=1

make libprocess-tests

3rdparty/libprocess/src/tests/libprocess-tests 
--gtest_filter="Scheme/HTTPTest.Endpoints/0" --gtest_repeat="`ulimit -n`"

make check


Thanks,

Joseph Wu

Re: Review Request 57358: Fixed FD leak in SSL server socket cleanup.

[Joseph Wu]

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

(Updated March 7, 2017, 12:18 p.m.)


Review request for mesos, Benjamin Mahler, Jie Yu, and Michael Park.


Changes
---

Different approach, which doesn't break tests, but slightly modifies the 
semantics of `Socket::accept`.  The assumption is that, when you discard the 
future from `Socket::accept`, you are preparing to clean up the socket and 
thereby do not care if the socket gives you inconsistent results.


Summary (updated)
-

Fixed FD leak in SSL server socket cleanup.


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


Repository: mesos


Description (updated)
---

Fixed FD leak in SSL server socket cleanup.

This commit deals with a specific case where the SocketImpl class
passes a self-reference (shared_ptr) into a Future continuation and
then discards the original Future. The behavior of `Future::discard`
is that the `onDiscard` event is only chained to the future immediately
following the discarded future.  i.e.

```
Future top;
top
  .onDiscard([]() { /* This gets executed. */ })

  .then([](const A&) { return Future(); })
  .onDiscard([]() { /* This also gets executed. */ })

  .then([](const B&) { return Future(); })
  .onDiscard([]() { /* But not this. */ });

top.discard();
```

When a Future is discarded, we only clear the `onDiscard` callbacks,
which means that all other continuations will continue to reside in
memory.  In the case of the `Socket` class, the Libevent SSL socket
implementation had several levels of continuations:

```
// Each item in this queue is backed by a Promise<>::Future.
Queue> accept_queue;

// LibeventSSLSocketImpl::accept.
accept_queue.get()
  .then(...)

// Socket::accept. This continuation holds a self-reference
// to the `shared_ptr`.
  .then([self](...) {...})
```

Because the second continuation is never discarded nor called, we
end up with a dangling pointer.  For the `Socket` class, this leads
to an FD leak.

This commit extends the future returned by the Libevent SSL Socket
to deallocate some extra memory upon being discarded.  This will
remove the extra reference to the SocketImpl class and thereby
resolve the issue of a dangling pointer.


Diffs (updated)
-

  3rdparty/libprocess/src/libevent_ssl_socket.hpp 
9493a50243340a1c48ab1c67f6e61cc081ef24ce 
  3rdparty/libprocess/src/libevent_ssl_socket.cpp 
7d493301bd5c0f24bf89e0b213f07ffe7801508b 


Diff: https://reviews.apache.org/r/57358/diff/2/

Changes: https://reviews.apache.org/r/57358/diff/1-2/


Testing (updated)
---

cmake .. -DENABLE_LIBEVENT=1 -DENABLE_SSL=1

make libprocess-tests

3rdparty/libprocess/src/tests/libprocess-tests 
--gtest_filter="Scheme/HTTPTest.Endpoints/0" --gtest_repeat="`ulimit -n`"

make check


Thanks,

Joseph Wu

Re: Review Request 56212: Added support for general checks to command executor.

[Alexander Rukletsov]

> On March 3, 2017, 2:10 a.m., Vinod Kone wrote:
> > src/launcher/executor.cpp
> > Lines 880 (patched)
> > 
> >
> > Can you remind me again, why this logic is different for health checks 
> > vs checks? for one, what to set into `healthy` field is passed as an 
> > argument to this function but what to set in `check_status` is calculated 
> > inside. second, whether a health check is defined or not doesn't have any 
> > bearing on the internals of this function, but check does.

Re: second, we should fix it eventually, see 
https://issues.apache.org/jira/browse/MESOS-6417. Re: first, it's not entirely 
true: another routine from `protobuf::` is used to create a task status with 
check status set, hence this function does not take it as an argument. What 
this function does is setting an "empty" task check status.


- Alexander


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


On Feb. 28, 2017, 3:54 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56212/
> ---
> 
> (Updated Feb. 28, 2017, 3:54 p.m.)
> 
> 
> Review request for mesos, Gastón Kleiman and Vinod Kone.
> 
> 
> Bugs: MESOS-6906
> https://issues.apache.org/jira/browse/MESOS-6906
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/launcher/executor.cpp adcc50fb7df91b9f8dff0f583b61e0efcd6da378 
> 
> 
> Diff: https://reviews.apache.org/r/56212/diff/4/
> 
> 
> Testing
> ---
> 
> See https://reviews.apache.org/r/56213/
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 56754: Implemented a JWT secret generator.

[Greg Mann]

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


Fix it, then Ship it!




Looking good! Just a few small comments below.


src/tests/secret_generator_tests.cpp
Lines 55 (patched)


Is the `Option::none()` necessary? I think the `value` member would 
be default-initialized to `NONE`?



src/tests/secret_generator_tests.cpp
Lines 63-66 (patched)


s/token.get()/token->/



src/tests/secret_generator_tests.cpp
Lines 68 (patched)


Could use EXPECT_SOME here.



src/tests/secret_generator_tests.cpp
Lines 72-73 (patched)


I think the `EXPECT_SOME` here is redundant since you're using 
`EXPECT_SOME_EQ`?



src/tests/secret_generator_tests.cpp
Lines 77-78 (patched)


Ditto.


- Greg Mann


On March 7, 2017, 3:40 p.m., Jan Schlicht wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56754/
> ---
> 
> (Updated March 7, 2017, 3:40 p.m.)
> 
> 
> Review request for mesos, Alexander Rojas and Greg Mann.
> 
> 
> Bugs: MESOS-7000
> https://issues.apache.org/jira/browse/MESOS-7000
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This can be used to create a 'Secret' from a 'Principal'.
> The resulting secret will provide a JWT.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am bb917c5d1b36f5106a74914fa3a864038a95e8bb 
>   src/authentication/executor/jwt_secret_generator.hpp PRE-CREATION 
>   src/authentication/executor/jwt_secret_generator.cpp PRE-CREATION 
>   src/tests/secret_generator_tests.cpp PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/56754/diff/7/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jan Schlicht
> 
>

Re: Review Request 56753: Implemented the JWT authenticator.

[Greg Mann]

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


Fix it, then Ship it!





3rdparty/libprocess/Makefile.am
Line 154 (original), 154 (patched)


We should probably keep 'authenticator.cpp', just for the Principal-related 
implementations which were recently added: `json(JSON::ObjectWriter*, const 
Principal&)` and `operator<<(std::ostream&, const Principal&)`. It seems a bit 
strange to have those in 'basic_authenticator.cpp'?



3rdparty/libprocess/include/process/authenticator.hpp
Lines 160 (patched)


s/withn/within/



3rdparty/libprocess/src/jwt_authenticator.cpp
Lines 73-78 (patched)


Need `return result;` here.



3rdparty/libprocess/src/jwt_authenticator.cpp
Lines 106 (patched)


Is the `Option::none()` necessary here? I think the `value` member 
should be default-initialized to `NONE`?



3rdparty/libprocess/src/jwt_authenticator.cpp
Lines 126-144 (patched)


Looks like a more standard practice would be to use `process_.get()` rather 
than `*process_`, in all 4 occurrences here.

I see that the `BasicAuthenticator` also uses `*process`, but using 
`.get()` will be more consistent with the rest of the codebase, as it looks 
like the `BasicAuthenticator` is the only occurrence of the `*process` syntax. 
They are functionally equivalent (modulo the presence of a `CHECK_NOTNULL` in 
the spawn case, which is unnecessary here I'd say).



3rdparty/libprocess/src/tests/http_tests.cpp
Lines 2042-2043 (patched)


Fits on one line.



3rdparty/libprocess/src/tests/http_tests.cpp
Lines 2070 (patched)


Is the `Option::none()` necessary here? I think the `value` member 
should be default-initialized to `NONE`?


- Greg Mann


On March 7, 2017, 3:36 p.m., Jan Schlicht wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56753/
> ---
> 
> (Updated March 7, 2017, 3:36 p.m.)
> 
> 
> Review request for mesos, Alexander Rojas and Greg Mann.
> 
> 
> Bugs: MESOS-7001
> https://issues.apache.org/jira/browse/MESOS-7001
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This HTTP authenticator extracts a JWT from the requests' authorization
> header using the 'Bearer' schema and validates it against a secret using
> HMAC SHA256. The 'sub' claim of the JWT is the extracted principal, all
> other claims will be additional labels of the 'Principal'.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/Makefile.am 75386184108214e67a58c328258ec204099d638c 
>   3rdparty/libprocess/include/process/authenticator.hpp 
> 00660f42cd4b707d955745bbfea5ffec73f690d6 
>   3rdparty/libprocess/src/authenticator.cpp  
>   3rdparty/libprocess/src/jwt_authenticator.cpp PRE-CREATION 
>   3rdparty/libprocess/src/tests/http_tests.cpp 
> a0e23c2300f9f6b9d1143ee1eb115bbf24adf92e 
> 
> 
> Diff: https://reviews.apache.org/r/56753/diff/5/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jan Schlicht
> 
>

Re: Review Request 56733: Mount all supported subsystems in the containerizer tests.

[Jiang Yan Xu]

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



So I was thinking about this: the code here makes sure the OS has the minimum 
set of subsystems required by this test suite. If the system doesn't meet the 
requirement, we do want to terminate here. On the other hand, although mounting 
more subsystems isn't harmful, if we mount everything that comes out of 
`cgroups::subsystems()`, we should still check whether that list is a superset 
of the minimumally required set. At a result we still have to add items here 
every time a new subsystem is added so we are not saving much?

Thoughts?

- Jiang Yan Xu


On March 6, 2017, 11:34 a.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56733/
> ---
> 
> (Updated March 6, 2017, 11:34 a.m.)
> 
> 
> Review request for mesos, Mesos Reviewbot and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-7049
> https://issues.apache.org/jira/browse/MESOS-7049
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Rather than a hard-coded list of subsystems, just mount everything we
> find that is supported.
> 
> 
> Diffs
> -
> 
>   src/tests/mesos.cpp 6a96fa51dfc2a62063c3154b256bdac707b009bb 
> 
> 
> Diff: https://reviews.apache.org/r/56733/diff/2/
> 
> 
> Testing
> ---
> 
> sudo make check (Fedora 25)
> 
> 
> Thanks,
> 
> James Peach
> 
>

Re: Review Request 53840: Added a metric in the allocator to track previous allocation cycle.

[Anindya Sinha]

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

(Updated March 7, 2017, 6:52 p.m.)


Review request for mesos and Jiang Yan Xu.


Changes
---

Removed RR 53839 from this review chain.


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


Repository: mesos


Description
---

Added a metric 'allocator/mesos/last_allocation_secs_since' which
indicates how long back (in seconds) did the previous allocation
cycle run. If no allocation cycle has run yet, it has a value of -1.


Diffs
-

  src/master/allocator/mesos/hierarchical.hpp 
d33306745a7287b750cb4a5242c7527369d58d65 
  src/master/allocator/mesos/hierarchical.cpp 
eeb44fe89d4bfd26900b11833c1182157e5c7e5c 
  src/master/allocator/mesos/metrics.hpp 
ce486eb079dc44bf0bbfad8668426c7ae87c97b3 
  src/master/allocator/mesos/metrics.cpp 
ec987fe0ce8605bf9e7c0ccbe3ac384deaf53be1 
  src/tests/hierarchical_allocator_tests.cpp 
5441fa9d1fad1ca7819038db49c6d88e40571e4a 


Diff: https://reviews.apache.org/r/53840/diff/2/


Testing
---

All tests passed.


Thanks,

Anindya Sinha

Re: Review Request 56667: Added support for JSON Web Tokens.

[Greg Mann]

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




3rdparty/libprocess/src/tests/jwt_tests.cpp
Lines 105 (patched)


Here and below, add periods to end of comments.


- Greg Mann


On March 7, 2017, 3:57 p.m., Jan Schlicht wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56667/
> ---
> 
> (Updated March 7, 2017, 3:57 p.m.)
> 
> 
> Review request for mesos, Alexander Rojas and Greg Mann.
> 
> 
> Bugs: MESOS-7001
> https://issues.apache.org/jira/browse/MESOS-7001
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> JSON Web Tokens can be used to create claim-based access tokens and is
> typically used for HTTP authentication.
> This implementation is intended for internal use, e.g. Mesos is supposed
> to only parse tokens that it also created. It doesn't fully comply with
> RFC 7519. Currently the only supported cryptographic algorithm is HMAC
> with SHA-256.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/Makefile.am 75386184108214e67a58c328258ec204099d638c 
>   3rdparty/libprocess/include/process/jwt.hpp PRE-CREATION 
>   3rdparty/libprocess/src/jwt.cpp PRE-CREATION 
>   3rdparty/libprocess/src/tests/jwt_tests.cpp PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/56667/diff/7/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jan Schlicht
> 
>

Re: Review Request 57360: Replace `.get().` in favor of `->` in tests.

[Mesos Reviewbot]

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



Patch looks great!

Reviews applied: [57269, 57360]

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 March 7, 2017, 3:39 a.m., Jay Guo wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57360/
> ---
> 
> (Updated March 7, 2017, 3:39 a.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Instead of using `get()` to fetch data, we are inclined to use
> `->` operator to direct access its methods.
> 
> 
> Diffs
> -
> 
>   src/tests/api_tests.cpp 607392ff6b714a9e812c2802f4d1465e8f71ad09 
>   src/tests/authentication_tests.cpp adbe03043dd75a0ebb0dadb2c0b218c77b58e900 
>   src/tests/cluster.cpp 04b70831ec05f715074cd93426c3645572d866ca 
>   src/tests/command_executor_tests.cpp 
> de735c62ae4be268b3e37d636f43f120f879a624 
>   src/tests/container_logger_tests.cpp 
> 54e5b29ce0668449027bde6185e37dc8b636d8c7 
>   src/tests/containerizer/appc_spec_tests.cpp 
> 840dbde1a16c6a201a65257ff207309f53f57772 
>   src/tests/containerizer/cgroups_isolator_tests.cpp 
> 971ea47ec5d2aed5e38eecf995b70ae4b0fa14a1 
>   src/tests/containerizer/cgroups_tests.cpp 
> 76fabce4530ccc0a1d685cd48d932ced5a64bc58 
>   src/tests/containerizer/cni_isolator_tests.cpp 
> cb893d3ef005a9cc60c40768fa669b27c4863020 
>   src/tests/containerizer/cpu_isolator_tests.cpp 
> f117826d9366820fd9ef5a113bb4b3d39deeca4b 
>   src/tests/containerizer/docker_containerizer_tests.cpp 
> 648453328d2f0792f7b5ba89b0351a11f6630be5 
>   src/tests/containerizer/docker_spec_tests.cpp 
> 82bddd228df3db95a00eb277ff9c380039b70b1e 
>   src/tests/containerizer/docker_tests.cpp 
> 452858d508e668fe62826de5558ea332cd4279d5 
>   src/tests/containerizer/docker_volume_isolator_tests.cpp 
> 4040d36d1dbf4968f62fa593024936858f28b4df 
>   src/tests/containerizer/fs_tests.cpp 
> f726fb06ac3e74f7f0d6d0310ac14eef60ae4fd9 
>   src/tests/containerizer/io_switchboard_tests.cpp 
> 9031815f5711cf940315ab3d8538aa099b356847 
>   src/tests/containerizer/linux_filesystem_isolator_tests.cpp 
> 51017a3a809ed0a72ea5c986a578344fff9b3276 
>   src/tests/containerizer/memory_isolator_tests.cpp 
> 388e62a9d10fc806f60d84002c10d9a072071228 
>   src/tests/containerizer/memory_pressure_tests.cpp 
> ff6c2b630bf67a679060bb8afeebc85f5c23a2c5 
>   src/tests/containerizer/mesos_containerizer_tests.cpp 
> 683807f38778095f31d37f5598d037ab87abb66b 
>   src/tests/containerizer/nested_mesos_containerizer_tests.cpp 
> ea01fe55a28d17105157004d8cf0976202a49b7c 
>   src/tests/containerizer/ns_tests.cpp 
> d6be1076a393f7959cd6ac225591c8b3aa66721d 
>   src/tests/containerizer/port_mapping_tests.cpp 
> 3e848165ace8346ead39224a46f8b85a5c4ff495 
>   src/tests/containerizer/provisioner_appc_tests.cpp 
> eb6509bc56214623fe9a37cbe5227768897f102c 
>   src/tests/containerizer/provisioner_backend_tests.cpp 
> ab7805f62767c6146588e3181954518d65076b40 
>   src/tests/containerizer/provisioner_docker_tests.cpp 
> af24b37acb265b5d301e4a5a236ac58c6f25a4a5 
>   src/tests/containerizer/routing_tests.cpp 
> e6b7eadce3eeab8f7bcb15bc1e55dff78f2a4f51 
>   src/tests/containerizer/xfs_quota_tests.cpp 
> 0fbaddd68af55c51c106962377be20afa599fb97 
>   src/tests/cram_md5_authentication_tests.cpp 
> cd8607136f963b036f6563dc0947ee0d435f5c68 
>   src/tests/credentials_tests.cpp 809f8c744b7e77c9aa42eaf8fe2d9d0ac9a52b4a 
>   src/tests/default_executor_tests.cpp 
> e4d43c8ad447577a9c5c7951207596bda1070856 
>   src/tests/disk_quota_tests.cpp d2dfc94c5f9875fc9ce05b1c86049f8e06730029 
>   src/tests/dynamic_weights_tests.cpp 
> 5571ab7b57059e4781671f45c6e59e7c3b0c42c9 
>   src/tests/environment.cpp 4c6c8f6656b62e5d8fbd8d4e8f2d7903f71e884c 
>   src/tests/exception_tests.cpp 316c329c0e6cc0aaf93c6c3ca3ab4355d7a7f2ea 
>   src/tests/executor_http_api_tests.cpp 
> 892f8f669392708968ba96e5ed22eac99ae64e18 
>   src/tests/fault_tolerance_tests.cpp 
> b13a7e2527189931b733fb4f188b1463fe1f919a 
>   src/tests/fetcher_cache_tests.cpp 85246ef610fac958c54b1655361a9b9e82c23200 
>   src/tests/fetcher_tests.cpp c4854b9fe4c520efc8ea06cdb42c79c333257eb1 
>   src/tests/files_tests.cpp d492adf71ecb22c433f0eba4d974e99f610b5dd3 
>   src/tests/gc_tests.cpp 6b6437ca0f0fa93c389e12cf2e9b23273cf23631 
>   src/tests/group_tests.cpp 193a158c4d07591584fc64b6baa41f2e90618732 
>   src/tests/health_check_tests.cpp 56e90747f2c943daee675738428d8ddeeafde36d 
>   src/tests/hook_tests.cpp 95dcfb6655709ea2e177b32ec522ceceeaff90a1 
>   src/tests/http_authentication_tests.cpp 
> 

Re: Review Request 57358: WIP: Fixed potential for circular dependencies in Future continuations.

[Joseph Wu]

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

(Updated March 7, 2017, 10:09 a.m.)


Review request for mesos, Benjamin Mahler, Jie Yu, and Michael Park.


Changes
---

Need to rethink approach, as this change "fixes" the leak, but ends up changing 
the semantics of `Future` and thereby fails a bunch of regression tests.


Summary (updated)
-

WIP: Fixed potential for circular dependencies in Future continuations.


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


Repository: mesos


Description
---

This commit deals with a specific case where an object passes a
self-reference (shared_ptr) into a Future continuation and then
discards the original Future.

The behavior of `Future::discard` is that the `onDiscard` event
is only chained to the future immediately following the discarded
future.  i.e.

```
Future top;
top
  .onDiscard([]() { /* This gets executed. */ })

  .then([](const A&) { return Future(); })
  .onDiscard([]() { /* This also gets executed. */ })

  .then([](const B&) { return Future(); })
  .onDiscard([]() { /* But not this. */ });

top.discard();
```

When a Future is discarded, we only clear the `onDiscard` callbacks,
which means that all other continuations will continue to reside in
memory.  In the case of the `Socket` class, the Libevent SSL socket
implementation had several levels of continuations:

```
// Each item in this queue is backed by a Promise<>::Future.
Queue> accept_queue;

// LibeventSSLSocketImpl::accept.
accept_queue.get()
  .then(...)

// Socket::accept. This continuation holds a self-reference
// to the `shared_ptr`.
  .then([self](...) {...})
```

Because the second continuation is never discarded nor called, we
end up with a dangling pointer.  For the `Socket` class, this leads
to an FD leak.


Diffs
-

  3rdparty/libprocess/include/process/future.hpp 
ec7ef2251947f5f42a202af0d6cb0858338e7755 


Diff: https://reviews.apache.org/r/57358/diff/1/


Testing
---

cmake .. -DENABLE_LIBEVENT=1 -DENABLE_SSL=1

make libprocess-tests

3rdparty/libprocess/src/tests/libprocess-tests 
--gtest_filter="Scheme/HTTPTest.Endpoints/0" --gtest_repeat="`ulimit -n`"


Thanks,

Joseph Wu

Re: Review Request 55790: Support the full CNI DNS specification.

[Avinash sridharan]

> On March 7, 2017, 2:22 p.m., Avinash sridharan wrote:
> > src/tests/containerizer/cni_isolator_tests.cpp
> > Lines 1112 (patched)
> > 
> >
> > Instead of setting this to the hosts IP address can we set it to the 
> > `loopback` (127.0.0.1). We want the query to this nameserver to fail before 
> > it hits the google servers. 
> > 
> > Just being a bit cautious here in case there is a DNS masquerade 
> > running on the host :).
> 
> James Peach wrote:
> dnsmasq would listen on loopback too, but sure :)

That's true. Unfortunately, __MESOS_TEST__ doesn't create a separate network 
namespace, else this would have made sense.


- Avinash


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


On March 7, 2017, 5:01 p.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55790/
> ---
> 
> (Updated March 7, 2017, 5:01 p.m.)
> 
> 
> Review request for mesos, Avinash sridharan, Jie Yu, and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-6858
> https://issues.apache.org/jira/browse/MESOS-6858
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add support for the full set of DNS resolver configuration items that
> a CNI IPAM plugin can specify. This implements updating the container's
> resolv.conf with the 'domain', 'search', and 'options' keywords.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/network/cni/cni.cpp 
> 84dc157e7d9e332a6da0f1fc33303e9ef9bdc147 
>   src/slave/containerizer/mesos/isolators/network/cni/spec.hpp 
> ccd511ec14810dcc1020dec5e1641141f3a319b4 
>   src/slave/containerizer/mesos/isolators/network/cni/spec.cpp 
> ac48159dadcea422f605e723db94a7f3bb573fa2 
>   src/tests/containerizer/cni_isolator_tests.cpp 
> cb893d3ef005a9cc60c40768fa669b27c4863020 
> 
> 
> Diff: https://reviews.apache.org/r/55790/diff/5/
> 
> 
> Testing
> ---
> 
> sudo make check (Fedora 25)
> 
> 
> Thanks,
> 
> James Peach
> 
>

Re: Review Request 57386: Introduced changes to the auth photos needed for DeleteNestedContainer.

[Gastón Kleiman]

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

(Updated March 7, 2017, 5:14 p.m.)


Review request for mesos, Adam B, Alexander Rukletsov, Alexander Rojas, Jie Yu, 
Kevin Klues, and Vinod Kone.


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


Repository: mesos


Description
---

Introduced changes to the auth photos needed for DeleteNestedContainer.


Diffs
-

  include/mesos/authorizer/acls.proto 8389917d12f9a9a3c9b4281f48e23ade14c20528 
  include/mesos/authorizer/authorizer.proto 
fdc4817ce74c45d792fc47f064f7909a83b1657d 


Diff: https://reviews.apache.org/r/57386/diff/1/


Testing
---


Thanks,

Gastón Kleiman

Review Request 57385: Introduced proto changes needed for the DeleteNestedContainer API call.

[Gastón Kleiman]

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

Review request for mesos, Alexander Rukletsov, Jie Yu, Kevin Klues, and Vinod 
Kone.


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


Repository: mesos


Description
---

This new Agent API call will make it possible to destroy the sandbox and
runtime directories of a terminated nested container.


Diffs
-

  include/mesos/agent/agent.proto 9149724159485ea2265e1494c1ce7ef989dad20a 
  include/mesos/v1/agent/agent.proto 34210c30ca58f50b14ff3e5a01c54003c9705121 


Diff: https://reviews.apache.org/r/57385/diff/1/


Testing
---


Thanks,

Gastón Kleiman

Re: Review Request 57387: Implemented the Agent API call `DeleteNestedContainer`.

[Gastón Kleiman]

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

(Updated March 7, 2017, 5:17 p.m.)


Review request for mesos, Adam B, Alexander Rukletsov, Alexander Rojas, Jie Yu, 
Kevin Klues, and Vinod Kone.


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


Repository: mesos


Description
---

This new Agent API call makes it possible to destroy the sandbox and
runtime directories of a terminated nested container.


Diffs
-

  src/authorizer/local/authorizer.cpp 2227b241eab1606815fa6464e3d6b1345624fd22 
  src/slave/http.cpp bc8209cb81194ebc8b604c9ba0d4a9e176cff2f6 
  src/slave/slave.hpp 978edd6309dfbbde1058f9c44d5fac7083ff95fb 
  src/slave/validation.cpp 3dbd0fa6ec27b38f40c7922c256859b4d29059ac 
  src/tests/authorization_tests.cpp 42edecc794b71a00ca32d26ae9b74e9f3ef97510 
  src/tests/slave_validation_tests.cpp 784528443469f68a8f2d93ebdb69dc872eef255d 


Diff: https://reviews.apache.org/r/57387/diff/1/


Testing (updated)
---

The patch contains validation and auth tests that pass on Linux.


Thanks,

Gastón Kleiman

Review Request 57388: Added documentation for the 'DeleteNestedContainer' Agent API call.

[Gastón Kleiman]

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

Review request for mesos, Alexander Rukletsov, Jie Yu, Kevin Klues, and Vinod 
Kone.


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


Repository: mesos


Description
---

Added documentation for the 'DeleteNestedContainer' Agent API call.


Diffs
-

  docs/operator-http-api.md 506251b120c6d828e7324f29dda5951f39187af2 


Diff: https://reviews.apache.org/r/57388/diff/1/


Testing
---


Thanks,

Gastón Kleiman

Review Request 57387: Implemented the Agent API call `DeleteNestedContainer`.

[Gastón Kleiman]

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

Review request for mesos, Adam B, Alexander Rukletsov, Alexander Rojas, Jie Yu, 
Kevin Klues, and Vinod Kone.


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


Repository: mesos


Description
---

This new Agent API call makes it possible to destroy the sandbox and
runtime directories of a terminated nested container.


Diffs
-

  src/authorizer/local/authorizer.cpp 2227b241eab1606815fa6464e3d6b1345624fd22 
  src/slave/http.cpp bc8209cb81194ebc8b604c9ba0d4a9e176cff2f6 
  src/slave/slave.hpp 978edd6309dfbbde1058f9c44d5fac7083ff95fb 
  src/slave/validation.cpp 3dbd0fa6ec27b38f40c7922c256859b4d29059ac 
  src/tests/authorization_tests.cpp 42edecc794b71a00ca32d26ae9b74e9f3ef97510 
  src/tests/slave_validation_tests.cpp 784528443469f68a8f2d93ebdb69dc872eef255d 


Diff: https://reviews.apache.org/r/57387/diff/1/


Testing
---


Thanks,

Gastón Kleiman

Review Request 57341: Made the usage of C++ namespaces in 'slave/http.cpp' consistent.

[Gastón Kleiman]

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

Review request for mesos, Alexander Rukletsov and Vinod Kone.


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


Repository: mesos


Description
---

Followed the convention in 'master/http.cpp' and explicitly used the
'mesos::' prefix.


Diffs
-

  src/slave/http.cpp bc8209cb81194ebc8b604c9ba0d4a9e176cff2f6 


Diff: https://reviews.apache.org/r/57341/diff/1/


Testing
---


Thanks,

Gastón Kleiman

Review Request 57386: Introduced changes to the auth photos needed for DeleteNestedContainer.

[Gastón Kleiman]

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

Review request for mesos, Adam B, Alexander Rukletsov, Alexander Rojas, Jie Yu, 
Kevin Klues, and Vinod Kone.


Repository: mesos


Description
---

Introduced changes to the auth photos needed for DeleteNestedContainer.


Diffs
-

  include/mesos/authorizer/acls.proto 8389917d12f9a9a3c9b4281f48e23ade14c20528 
  include/mesos/authorizer/authorizer.proto 
fdc4817ce74c45d792fc47f064f7909a83b1657d 


Diff: https://reviews.apache.org/r/57386/diff/1/


Testing
---


Thanks,

Gastón Kleiman

Review Request 57384: Added `Containerizer::cleanupArtifacts`.

[Gastón Kleiman]

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

Review request for mesos, Alexander Rukletsov, Jie Yu, Kevin Klues, and Vinod 
Kone.


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


Repository: mesos


Description
---

This new method cleans up the sandbox and runtime directories of a
terminated nested container.


Diffs
-

  src/slave/containerizer/containerizer.hpp 
f65a9b9761fc254bd0778bf13aac9b256497b22f 
  src/slave/containerizer/mesos/containerizer.hpp 
09f94ccb3224c14a9324961b789455b119ec2257 
  src/slave/containerizer/mesos/containerizer.cpp 
b001d0265ec73822193eaac74c631930acce90c0 
  src/tests/containerizer/nested_mesos_containerizer_tests.cpp 
ea01fe55a28d17105157004d8cf0976202a49b7c 


Diff: https://reviews.apache.org/r/57384/diff/1/


Testing
---

Added a test and verified that it works on Linux.


Thanks,

Gastón Kleiman

Re: Review Request 56667: Added support for JSON Web Tokens.

[Greg Mann]

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




3rdparty/libprocess/include/process/jwt.hpp
Lines 127-128 (patched)


Just leaving a note here to record our discussion: could you move these to 
be free functions in the implementation file? That will avoid polluting the 
header and avoid unnecessary recompilation when changing their signatures in 
the future. Your original implementation did exactly that, I apologize for 
asking you to change it before!


- Greg Mann


On March 7, 2017, 3:57 p.m., Jan Schlicht wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56667/
> ---
> 
> (Updated March 7, 2017, 3:57 p.m.)
> 
> 
> Review request for mesos, Alexander Rojas and Greg Mann.
> 
> 
> Bugs: MESOS-7001
> https://issues.apache.org/jira/browse/MESOS-7001
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> JSON Web Tokens can be used to create claim-based access tokens and is
> typically used for HTTP authentication.
> This implementation is intended for internal use, e.g. Mesos is supposed
> to only parse tokens that it also created. It doesn't fully comply with
> RFC 7519. Currently the only supported cryptographic algorithm is HMAC
> with SHA-256.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/Makefile.am 75386184108214e67a58c328258ec204099d638c 
>   3rdparty/libprocess/include/process/jwt.hpp PRE-CREATION 
>   3rdparty/libprocess/src/jwt.cpp PRE-CREATION 
>   3rdparty/libprocess/src/tests/jwt_tests.cpp PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/56667/diff/7/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jan Schlicht
> 
>

Re: Review Request 56667: Added support for JSON Web Tokens.

[Greg Mann]

> On March 6, 2017, 11:33 p.m., Greg Mann wrote:
> > 3rdparty/libprocess/src/jwt.cpp
> > Lines 169 (patched)
> > 
> >
> > Do you think `parse_component` would be a better name for this?
> 
> Jan Schlicht wrote:
> But it's also base64-decoding the component. Of course, in addition to 
> parsing JSON. How about `decode_and_parse`?

Per our discussion, we can use a name like `decode_to_json` for this.


- Greg


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


On March 7, 2017, 3:57 p.m., Jan Schlicht wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56667/
> ---
> 
> (Updated March 7, 2017, 3:57 p.m.)
> 
> 
> Review request for mesos, Alexander Rojas and Greg Mann.
> 
> 
> Bugs: MESOS-7001
> https://issues.apache.org/jira/browse/MESOS-7001
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> JSON Web Tokens can be used to create claim-based access tokens and is
> typically used for HTTP authentication.
> This implementation is intended for internal use, e.g. Mesos is supposed
> to only parse tokens that it also created. It doesn't fully comply with
> RFC 7519. Currently the only supported cryptographic algorithm is HMAC
> with SHA-256.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/Makefile.am 75386184108214e67a58c328258ec204099d638c 
>   3rdparty/libprocess/include/process/jwt.hpp PRE-CREATION 
>   3rdparty/libprocess/src/jwt.cpp PRE-CREATION 
>   3rdparty/libprocess/src/tests/jwt_tests.cpp PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/56667/diff/7/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jan Schlicht
> 
>

Review Request 57382: Removed unused lambda captures.

[Benjamin Bannier]

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

Review request for mesos and Till Toenshoff.


Repository: mesos


Description
---

This commit removes unused lambda captures of pointer and reference
types which do not extend any lifetimes.


Diffs
-

  src/master/http.cpp de2cc03e6c7a3b75224113f56ce268937ead2929 
  src/tests/api_tests.cpp 52f58a4d6b1ea75744de1c3d2f0f064d9299fe1d 
  src/tests/master_quota_tests.cpp 91219d6693fdd119ed3b0bf734eaa55da9c58b0a 


Diff: https://reviews.apache.org/r/57382/diff/1/


Testing
---


Thanks,

Benjamin Bannier

Re: Review Request 55790: Support the full CNI DNS specification.

[James Peach]

> On March 7, 2017, 2:22 p.m., Avinash sridharan wrote:
> > src/slave/containerizer/mesos/isolators/network/cni/cni.cpp
> > Lines 999 (patched)
> > 
> >
> > Do you explicitly need this? Even if you do please define this at the 
> > top.

AFAICT this is required here. I wasn't able to find a different permutation 
that would build.


> On March 7, 2017, 2:22 p.m., Avinash sridharan wrote:
> > src/tests/containerizer/cni_isolator_tests.cpp
> > Lines 1112 (patched)
> > 
> >
> > Instead of setting this to the hosts IP address can we set it to the 
> > `loopback` (127.0.0.1). We want the query to this nameserver to fail before 
> > it hits the google servers. 
> > 
> > Just being a bit cautious here in case there is a DNS masquerade 
> > running on the host :).

dnsmasq would listen on loopback too, but sure :)


- James


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


On March 1, 2017, 4:51 p.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55790/
> ---
> 
> (Updated March 1, 2017, 4:51 p.m.)
> 
> 
> Review request for mesos, Avinash sridharan, Jie Yu, and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-6858
> https://issues.apache.org/jira/browse/MESOS-6858
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add support for the full set of DNS resolver configuration items that
> a CNI IPAM plugin can specify. This implements updating the container's
> resolv.conf with the 'domain', 'search', and 'options' keywords.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/network/cni/cni.cpp 
> 84dc157e7d9e332a6da0f1fc33303e9ef9bdc147 
>   src/slave/containerizer/mesos/isolators/network/cni/spec.hpp 
> ccd511ec14810dcc1020dec5e1641141f3a319b4 
>   src/slave/containerizer/mesos/isolators/network/cni/spec.cpp 
> ac48159dadcea422f605e723db94a7f3bb573fa2 
>   src/tests/containerizer/cni_isolator_tests.cpp 
> cb893d3ef005a9cc60c40768fa669b27c4863020 
> 
> 
> Diff: https://reviews.apache.org/r/55790/diff/4/
> 
> 
> Testing
> ---
> 
> sudo make check (Fedora 25)
> 
> 
> Thanks,
> 
> James Peach
> 
>

Re: Review Request 55790: Support the full CNI DNS specification.

[James Peach]

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

(Updated March 7, 2017, 5:01 p.m.)


Review request for mesos, Avinash sridharan, Jie Yu, and Jiang Yan Xu.


Changes
---

Rebased and addressed review comments.


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


Repository: mesos


Description
---

Add support for the full set of DNS resolver configuration items that
a CNI IPAM plugin can specify. This implements updating the container's
resolv.conf with the 'domain', 'search', and 'options' keywords.


Diffs (updated)
-

  src/slave/containerizer/mesos/isolators/network/cni/cni.cpp 
84dc157e7d9e332a6da0f1fc33303e9ef9bdc147 
  src/slave/containerizer/mesos/isolators/network/cni/spec.hpp 
ccd511ec14810dcc1020dec5e1641141f3a319b4 
  src/slave/containerizer/mesos/isolators/network/cni/spec.cpp 
ac48159dadcea422f605e723db94a7f3bb573fa2 
  src/tests/containerizer/cni_isolator_tests.cpp 
cb893d3ef005a9cc60c40768fa669b27c4863020 


Diff: https://reviews.apache.org/r/55790/diff/5/

Changes: https://reviews.apache.org/r/55790/diff/4-5/


Testing
---

sudo make check (Fedora 25)


Thanks,

James Peach

Re: Review Request 56667: Added support for JSON Web Tokens.

[Greg Mann]

> On March 6, 2017, 11:33 p.m., Greg Mann wrote:
> > 3rdparty/libprocess/include/process/jwt.hpp
> > Lines 42 (patched)
> > 
> >
> > Should we also provide a link to RFC-7515 here?
> 
> Jan Schlicht wrote:
> IMO we shouldn't. While RFC 7519 and RFC 7515 are closely related, only 
> RFC 7519 covers JWT and mentions RFC 7515 where it is necessary.

SGTM


- Greg


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


On March 7, 2017, 3:57 p.m., Jan Schlicht wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56667/
> ---
> 
> (Updated March 7, 2017, 3:57 p.m.)
> 
> 
> Review request for mesos, Alexander Rojas and Greg Mann.
> 
> 
> Bugs: MESOS-7001
> https://issues.apache.org/jira/browse/MESOS-7001
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> JSON Web Tokens can be used to create claim-based access tokens and is
> typically used for HTTP authentication.
> This implementation is intended for internal use, e.g. Mesos is supposed
> to only parse tokens that it also created. It doesn't fully comply with
> RFC 7519. Currently the only supported cryptographic algorithm is HMAC
> with SHA-256.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/Makefile.am 75386184108214e67a58c328258ec204099d638c 
>   3rdparty/libprocess/include/process/jwt.hpp PRE-CREATION 
>   3rdparty/libprocess/src/jwt.cpp PRE-CREATION 
>   3rdparty/libprocess/src/tests/jwt_tests.cpp PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/56667/diff/7/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jan Schlicht
> 
>

Re: Review Request 56208: Updated checks library with general check support.

[Alexander Rukletsov]

> On March 1, 2017, 8:23 p.m., Vinod Kone wrote:
> > src/checks/checker.cpp
> > Line 424 (original), 440 (patched)
> > 
> >
> > why are we sending WEXITSTATUS and not exit code?

Because what we get from `Subprocess` is actually `pid_t`. It is unclear what 
the scheduler will do with the pid.


- Alexander


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


On March 1, 2017, 12:50 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56208/
> ---
> 
> (Updated March 1, 2017, 12:50 p.m.)
> 
> 
> Review request for mesos, Gastón Kleiman and Vinod Kone.
> 
> 
> Bugs: MESOS-6906
> https://issues.apache.org/jira/browse/MESOS-6906
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add support for general checks, i.e. defined by CheckInfo, in
> checking library. A general check can be either an command or
> an HTTP request. The library performs the requested check at
> the specified interval and sends the result to the framework
> via a task status update. If the current result is the same as
> the previous result, no status update is sent.
> 
> 
> Diffs
> -
> 
>   src/checks/checker.hpp dc293f3d3613dec716510d269829f8a6f406c277 
>   src/checks/checker.cpp 8716e4cc684e6c4b6b76d8ca53221be06d10b2a6 
>   src/checks/health_checker.hpp f1f2834b3429fb00cc49c179fa9a3de328f597b5 
>   src/checks/health_checker.cpp 6c97369fd9a567ba16dd92085bf142d43f71eaf1 
> 
> 
> Diff: https://reviews.apache.org/r/56208/diff/7/
> 
> 
> Testing
> ---
> 
> https://reviews.apache.org/r/56213/
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 56667: Added support for JSON Web Tokens.

[Jan Schlicht]

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

(Updated March 7, 2017, 4:57 p.m.)


Review request for mesos, Alexander Rojas and Greg Mann.


Changes
---

Rebased and addressed issues.


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


Repository: mesos


Description
---

JSON Web Tokens can be used to create claim-based access tokens and is
typically used for HTTP authentication.
This implementation is intended for internal use, e.g. Mesos is supposed
to only parse tokens that it also created. It doesn't fully comply with
RFC 7519. Currently the only supported cryptographic algorithm is HMAC
with SHA-256.


Diffs (updated)
-

  3rdparty/libprocess/Makefile.am 75386184108214e67a58c328258ec204099d638c 
  3rdparty/libprocess/include/process/jwt.hpp PRE-CREATION 
  3rdparty/libprocess/src/jwt.cpp PRE-CREATION 
  3rdparty/libprocess/src/tests/jwt_tests.cpp PRE-CREATION 


Diff: https://reviews.apache.org/r/56667/diff/7/

Changes: https://reviews.apache.org/r/56667/diff/6-7/


Testing
---

make check


Thanks,

Jan Schlicht

Re: Review Request 56667: Added support for JSON Web Tokens.

[Jan Schlicht]

> On March 7, 2017, 12:33 a.m., Greg Mann wrote:
> > 3rdparty/libprocess/src/jwt.cpp
> > Lines 169 (patched)
> > 
> >
> > Do you think `parse_component` would be a better name for this?

But it's also base64-decoding the component. Of course, in addition to parsing 
JSON. How about `decode_and_parse`?


- Jan


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


On March 6, 2017, 3:53 p.m., Jan Schlicht wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56667/
> ---
> 
> (Updated March 6, 2017, 3:53 p.m.)
> 
> 
> Review request for mesos, Alexander Rojas and Greg Mann.
> 
> 
> Bugs: MESOS-7001
> https://issues.apache.org/jira/browse/MESOS-7001
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> JSON Web Tokens can be used to create claim-based access tokens and is
> typically used for HTTP authentication.
> This implementation is intended for internal use, e.g. Mesos is supposed
> to only parse tokens that it also created. It doesn't fully comply with
> RFC 7519. Currently the only supported cryptographic algorithm is HMAC
> with SHA-256.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/Makefile.am 75386184108214e67a58c328258ec204099d638c 
>   3rdparty/libprocess/include/process/jwt.hpp PRE-CREATION 
>   3rdparty/libprocess/src/jwt.cpp PRE-CREATION 
>   3rdparty/libprocess/src/tests/jwt_tests.cpp PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/56667/diff/6/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jan Schlicht
> 
>

Re: Review Request 56667: Added support for JSON Web Tokens.

[Jan Schlicht]

> On March 7, 2017, 12:33 a.m., Greg Mann wrote:
> > 3rdparty/libprocess/include/process/jwt.hpp
> > Lines 42 (patched)
> > 
> >
> > Should we also provide a link to RFC-7515 here?

IMO we shouldn't. While RFC 7519 and RFC 7515 are closely related, only RFC 
7519 covers JWT and mentions RFC 7515 where it is necessary.


- Jan


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


On March 6, 2017, 3:53 p.m., Jan Schlicht wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56667/
> ---
> 
> (Updated March 6, 2017, 3:53 p.m.)
> 
> 
> Review request for mesos, Alexander Rojas and Greg Mann.
> 
> 
> Bugs: MESOS-7001
> https://issues.apache.org/jira/browse/MESOS-7001
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> JSON Web Tokens can be used to create claim-based access tokens and is
> typically used for HTTP authentication.
> This implementation is intended for internal use, e.g. Mesos is supposed
> to only parse tokens that it also created. It doesn't fully comply with
> RFC 7519. Currently the only supported cryptographic algorithm is HMAC
> with SHA-256.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/Makefile.am 75386184108214e67a58c328258ec204099d638c 
>   3rdparty/libprocess/include/process/jwt.hpp PRE-CREATION 
>   3rdparty/libprocess/src/jwt.cpp PRE-CREATION 
>   3rdparty/libprocess/src/tests/jwt_tests.cpp PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/56667/diff/6/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jan Schlicht
> 
>

Re: Review Request 56754: Implemented a JWT secret generator.

[Jan Schlicht]

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

(Updated March 7, 2017, 4:40 p.m.)


Review request for mesos, Alexander Rojas and Greg Mann.


Changes
---

Addressed issues.


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


Repository: mesos


Description
---

This can be used to create a 'Secret' from a 'Principal'.
The resulting secret will provide a JWT.


Diffs (updated)
-

  src/Makefile.am bb917c5d1b36f5106a74914fa3a864038a95e8bb 
  src/authentication/executor/jwt_secret_generator.hpp PRE-CREATION 
  src/authentication/executor/jwt_secret_generator.cpp PRE-CREATION 
  src/tests/secret_generator_tests.cpp PRE-CREATION 


Diff: https://reviews.apache.org/r/56754/diff/7/

Changes: https://reviews.apache.org/r/56754/diff/6-7/


Testing
---

make check


Thanks,

Jan Schlicht

Re: Review Request 57166: Updated role validation for hierarchical roles.

[Neil Conway]

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

(Updated March 7, 2017, 3:38 p.m.)


Review request for mesos, Benjamin Bannier and Michael Park.


Changes
---

Address review comments.


Repository: mesos


Description
---

Role names can now contain forward slashes. Each component in a role
name must now be validated separately.


Diffs (updated)
-

  src/common/roles.cpp 31774a9b8f99f5efeed35b1c3e3486e05ca00f6a 
  src/tests/role_tests.cpp 77f3d46a544a51ba71476e2f0735bb32758dd9e1 


Diff: https://reviews.apache.org/r/57166/diff/3/

Changes: https://reviews.apache.org/r/57166/diff/2-3/


Testing
---

`make check`


Thanks,

Neil Conway

Re: Review Request 57166: Updated role validation for hierarchical roles.

[Neil Conway]

> On March 7, 2017, 5:26 a.m., Michael Park wrote:
> > src/common/roles.cpp
> > Lines 72 (patched)
> > 
> >
> > For these error messages, how about we say something like this?
> > 
> > ```cpp
> > "A role cannot start with a slash, given: '" + role + "'"
> > ```
> > 
> > Here and below.

The "given" phrasing here seems confusing to me (it isn't clear who is being 
"given" what...). Personally I think the current phrasing is okay...


- Neil


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


On March 2, 2017, 6:12 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57166/
> ---
> 
> (Updated March 2, 2017, 6:12 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and Michael Park.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Role names can now contain forward slashes. Each component in a role
> name must now be validated separately.
> 
> 
> Diffs
> -
> 
>   src/common/roles.cpp 31774a9b8f99f5efeed35b1c3e3486e05ca00f6a 
>   src/tests/role_tests.cpp 77f3d46a544a51ba71476e2f0735bb32758dd9e1 
> 
> 
> Diff: https://reviews.apache.org/r/57166/diff/2/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 56754: Implemented a JWT secret generator.

[Jan Schlicht]

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

(Updated March 7, 2017, 4:36 p.m.)


Review request for mesos, Alexander Rojas and Greg Mann.


Changes
---

Rebased.


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


Repository: mesos


Description
---

This can be used to create a 'Secret' from a 'Principal'.
The resulting secret will provide a JWT.


Diffs (updated)
-

  src/Makefile.am bb917c5d1b36f5106a74914fa3a864038a95e8bb 
  src/authentication/executor/jwt_secret_generator.hpp PRE-CREATION 
  src/authentication/executor/jwt_secret_generator.cpp PRE-CREATION 
  src/tests/secret_generator_tests.cpp PRE-CREATION 


Diff: https://reviews.apache.org/r/56754/diff/6/

Changes: https://reviews.apache.org/r/56754/diff/5-6/


Testing
---

make check


Thanks,

Jan Schlicht

Re: Review Request 56753: Implemented the JWT authenticator.

[Jan Schlicht]

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

(Updated March 7, 2017, 4:36 p.m.)


Review request for mesos, Alexander Rojas and Greg Mann.


Changes
---

Rebased and addressed issues.


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


Repository: mesos


Description
---

This HTTP authenticator extracts a JWT from the requests' authorization
header using the 'Bearer' schema and validates it against a secret using
HMAC SHA256. The 'sub' claim of the JWT is the extracted principal, all
other claims will be additional labels of the 'Principal'.


Diffs (updated)
-

  3rdparty/libprocess/Makefile.am 75386184108214e67a58c328258ec204099d638c 
  3rdparty/libprocess/include/process/authenticator.hpp 
00660f42cd4b707d955745bbfea5ffec73f690d6 
  3rdparty/libprocess/src/authenticator.cpp  
  3rdparty/libprocess/src/jwt_authenticator.cpp PRE-CREATION 
  3rdparty/libprocess/src/tests/http_tests.cpp 
a0e23c2300f9f6b9d1143ee1eb115bbf24adf92e 


Diff: https://reviews.apache.org/r/56753/diff/5/

Changes: https://reviews.apache.org/r/56753/diff/4-5/


Testing
---

make check


Thanks,

Jan Schlicht

Re: Review Request 56666: Added a HMAC SHA256 generator.

[Jan Schlicht]

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

(Updated March 7, 2017, 4:34 p.m.)


Review request for mesos, Alexander Rojas and Greg Mann.


Changes
---

Fixed a typo.


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


Repository: mesos


Description
---

HMAC SHA256 can be used to create or verify message signatures.


Diffs (updated)
-

  3rdparty/libprocess/include/process/ssl/utilities.hpp 
c2f64a91a505675d568ddf5aa081125e4e32fe17 
  3rdparty/libprocess/src/ssl/utilities.cpp 
8aec613312eee3dd11d9df8c3828a5185407e073 


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

Changes: https://reviews.apache.org/r/5/diff/4-5/


Testing
---

make check


Thanks,

Jan Schlicht

Re: Review Request 57254: Updated DRFSorter to support hierarchical roles.

[Neil Conway]

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

(Updated March 7, 2017, 3:26 p.m.)


Review request for mesos, Benjamin Bannier, Benjamin Mahler, and Michael Park.


Changes
---

Rebase.


Repository: mesos


Description
---

This commit replaces the sorter's flat list of clients with a tree of
client names; this tree represents the hierarchical relationship between
sorter clients. Each node in the tree contains an (ordered) list of
pointers to child nodes. The tree might contain nodes that do not
correspond directly to sorter clients. For example, adding clients "a/b"
and "c/d" results in the following tree:

root
 -> a
   -> b
 -> c
   -> d

The `sort` member function still only returns one result for each
(active) client in the sorter. This is implemented by ensuring that each
sorter client is associated with a leaf node in the tree. Note that it
is possible for a leaf node to be transformed into an internal node by a
subsequent insertion; to handle this case, we "implicitly" create an
extra child node, which maintains the invariant that each client has a
leaf node. For example, if the client "a/b/x" is added to the tree
above, the result is:

root
 -> a
   -> b
 -> .
 -> x
 -> c
   -> d

The "." leaf node holds the allocation that has been made to the "a/b"
client itself; the "a/b" node holds the sum of all the allocations that
have been made to the subtree rooted at "a/b", which also includes
"a/b/x".


Diffs (updated)
-

  src/master/allocator/mesos/hierarchical.hpp 
646f66e67d9c6b8c61fc6e6558a1db976a44c126 
  src/master/allocator/mesos/hierarchical.cpp 
0059ccead90f32491591990c791e7fa905a864b7 
  src/master/allocator/sorter/drf/metrics.hpp 
61568cb520826ab59d675824b212e0d3deb63764 
  src/master/allocator/sorter/drf/metrics.cpp 
15aab32db5ca1a7a14080e9bbb7c65283be3ec20 
  src/master/allocator/sorter/drf/sorter.hpp 
76329220e1115c1de7810fb69b943c78c078be59 
  src/master/allocator/sorter/drf/sorter.cpp 
ed54680cecb637931fc344fbcf8fd3b14cc24295 
  src/master/allocator/sorter/sorter.hpp 
b3029fcf7342406955760da53f1ae736769f308c 
  src/tests/hierarchical_allocator_tests.cpp 
cdf1f15b7802439b28405ca8f6634ce83e886630 
  src/tests/master_allocator_tests.cpp 7b0b786f1c6c53616fd7ae1f7f765752d94a4f83 
  src/tests/sorter_tests.cpp c93d236b13256f4022a811d019990ef81521aa77 


Diff: https://reviews.apache.org/r/57254/diff/2/

Changes: https://reviews.apache.org/r/57254/diff/1-2/


Testing
---


Thanks,

Neil Conway

Re: Review Request 57364: Fixed flaky test FaultToleranceTest.FrameworkReregister.

[Neil Conway]

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



Offhand, I would think the right logic should be:

```
// Set up expectation for `registered` event

driver.start();

Clock::advance(authentication_backoff_factor);
Clock::advance(registration_backoff_factor);

AWAIT_READY(registered);

process::Time registerTime = Clock::now();
```

i.e., settling clock should be unnecessary if we wait for `registered` before 
calling `Clock::now()`, and we still want to advance the clock before waiting 
for `registered`. Does that make sense to you?


src/tests/fault_tolerance_tests.cpp
Lines 826 (patched)


With the clock paused, I believe we need to wait for 
`authentication_backoff_factor` and `registration_backoff_factor` to ensure 
that registration will occur.


- Neil Conway


On March 7, 2017, 6:48 a.m., Jay Guo wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57364/
> ---
> 
> (Updated March 7, 2017, 6:48 a.m.)
> 
> 
> Review request for mesos and Neil Conway.
> 
> 
> Bugs: MESOS-7029
> https://issues.apache.org/jira/browse/MESOS-7029
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed flaky test FaultToleranceTest.FrameworkReregister.
> 
> 
> Diffs
> -
> 
>   src/tests/fault_tolerance_tests.cpp 
> b13a7e2527189931b733fb4f188b1463fe1f919a 
> 
> 
> Diff: https://reviews.apache.org/r/57364/diff/1/
> 
> 
> Testing
> ---
> 
> ./bin/mesos-tests.sh --gtest_filter="FaultToleranceTest.FrameworkReregister" 
> --gtest_break_on_failure --gtest_repeat=-1
> 
> 
> Thanks,
> 
> Jay Guo
> 
>

Re: Review Request 56753: Implemented the JWT authenticator.

[Jan Schlicht]

> On March 7, 2017, 1:02 a.m., Greg Mann wrote:
> > 3rdparty/libprocess/src/tests/http_tests.cpp
> > Lines 2033 (patched)
> > 
> >
> > Should we do something similar here to what you're doing below, and 
> > stringify an invalid JWT (invalid 'exp' or something else) to generate this?
> > 
> > I quite liked the "forged token" test we encountered recently (where a 
> > correct token was altered to have a slightly different payload, i.e. a 
> > different 'sub'), perhaps you could use that case here, or add another case 
> > for it?

I'll just use a different secret for that. Header, payload will be the same, 
only the signature will differ. That's equivalent to altering a correct token.


- Jan


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


On March 6, 2017, 3:55 p.m., Jan Schlicht wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56753/
> ---
> 
> (Updated March 6, 2017, 3:55 p.m.)
> 
> 
> Review request for mesos, Alexander Rojas and Greg Mann.
> 
> 
> Bugs: MESOS-7001
> https://issues.apache.org/jira/browse/MESOS-7001
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This HTTP authenticator extracts a JWT from the requests' authorization
> header using the 'Bearer' schema and validates it against a secret using
> HMAC SHA256. The 'sub' claim of the JWT is the extracted principal, all
> other claims will be additional labels of the 'Principal'.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/Makefile.am 75386184108214e67a58c328258ec204099d638c 
>   3rdparty/libprocess/include/process/authenticator.hpp 
> e5489c6cb4adc8a822e7dd4515542618c36136f9 
>   3rdparty/libprocess/src/authenticator.cpp 
> cfedb6f7674e0f6690e77a633cdd1bd494c7d2c7 
>   3rdparty/libprocess/src/jwt_authenticator.cpp PRE-CREATION 
>   3rdparty/libprocess/src/tests/http_tests.cpp 
> fb4da9aecff0370d97a15269c5d8fffb30e0478f 
> 
> 
> Diff: https://reviews.apache.org/r/56753/diff/4/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jan Schlicht
> 
>

Re: Review Request 55790: Support the full CNI DNS specification.

[Avinash sridharan]

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


Fix it, then Ship it!




Ship It!


src/slave/containerizer/mesos/isolators/network/cni/cni.cpp
Lines 999 (patched)


Do you explicitly need this? Even if you do please define this at the top.



src/tests/containerizer/cni_isolator_tests.cpp
Lines 165 (patched)


s/mockPlugin/`mockPlugin`



src/tests/containerizer/cni_isolator_tests.cpp
Lines 174 (patched)


s/mockPlugin/`mockPlugin`



src/tests/containerizer/cni_isolator_tests.cpp
Lines 1048 (patched)


s/formatResolverConfig()/`formatResolverConfig()`/



src/tests/containerizer/cni_isolator_tests.cpp
Lines 1112 (patched)


Instead of setting this to the hosts IP address can we set it to the 
`loopback` (127.0.0.1). We want the query to this nameserver to fail before it 
hits the google servers. 

Just being a bit cautious here in case there is a DNS masquerade running on 
the host :).


- Avinash sridharan


On March 1, 2017, 4:51 p.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55790/
> ---
> 
> (Updated March 1, 2017, 4:51 p.m.)
> 
> 
> Review request for mesos, Avinash sridharan, Jie Yu, and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-6858
> https://issues.apache.org/jira/browse/MESOS-6858
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add support for the full set of DNS resolver configuration items that
> a CNI IPAM plugin can specify. This implements updating the container's
> resolv.conf with the 'domain', 'search', and 'options' keywords.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/network/cni/cni.cpp 
> 84dc157e7d9e332a6da0f1fc33303e9ef9bdc147 
>   src/slave/containerizer/mesos/isolators/network/cni/spec.hpp 
> ccd511ec14810dcc1020dec5e1641141f3a319b4 
>   src/slave/containerizer/mesos/isolators/network/cni/spec.cpp 
> ac48159dadcea422f605e723db94a7f3bb573fa2 
>   src/tests/containerizer/cni_isolator_tests.cpp 
> cb893d3ef005a9cc60c40768fa669b27c4863020 
> 
> 
> Diff: https://reviews.apache.org/r/55790/diff/4/
> 
> 
> Testing
> ---
> 
> sudo make check (Fedora 25)
> 
> 
> Thanks,
> 
> James Peach
> 
>