Re: Review Request 68296: Add 'handler' field to batched state requests.

[Benno Evers]

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

(Updated Aug. 13, 2018, 1:45 p.m.)


Review request for mesos, Alexander Rukletsov and Benjamin Mahler.


Summary (updated)
-

Add 'handler' field to batched state requests.


Repository: mesos


Description (updated)
---

Add 'handler' field to batched state requests.


Diffs (updated)
-

  src/master/http.cpp d43fbd689598612ec5946b46e2fa5e7f5e22cfa8 
  src/master/master.hpp 209b998db8d2bad7a3812df44f0939458f48eb11 
  src/master/master.cpp 21de9739d2891c96cd036621e3f87a160c1d7c30 


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

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


Testing
---


Thanks,

Benno Evers

Review Request 68321: Add '/state-summary' to batched master endpoints.

[Benno Evers]

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

Review request for mesos, Alexander Rukletsov and Benjamin Mahler.


Repository: mesos


Description
---

Add '/state-summary' to batched master endpoints.


Diffs
-

  src/master/http.cpp d43fbd689598612ec5946b46e2fa5e7f5e22cfa8 
  src/master/master.hpp 209b998db8d2bad7a3812df44f0939458f48eb11 


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


Testing
---


Thanks,

Benno Evers

Re: Review Request 68321: Add '/state-summary' to batched master endpoints.

[Mesos Reviewbot Windows]

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



FAIL: Failed to apply the dependent review: 68296.

Failed command: `python.exe .\support\python3\apply-reviews.py -n -r 68296`

All the build artifacts available at: 
http://dcos-win.westus.cloudapp.azure.com/artifacts/mesos-reviewbot-testing/2138/mesos-review-68321

- Mesos Reviewbot Windows


On Aug. 13, 2018, 1:46 p.m., Benno Evers wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68321/
> ---
> 
> (Updated Aug. 13, 2018, 1:46 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Benjamin Mahler.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add '/state-summary' to batched master endpoints.
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp d43fbd689598612ec5946b46e2fa5e7f5e22cfa8 
>   src/master/master.hpp 209b998db8d2bad7a3812df44f0939458f48eb11 
> 
> 
> Diff: https://reviews.apache.org/r/68321/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Benno Evers
> 
>

Re: Review Request 68054: Added networking statistics to cni isolator.

[Mesos Reviewbot]

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



Patch looks great!

Reviews applied: [68052, 68053, 68054]

Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' 
CONFIGURATION='--verbose --disable-libtool-wrappers' ENVIRONMENT='GLOG_v=1 
MESOS_VERBOSE=1'; ./support/docker-build.sh

- Mesos Reviewbot


On Aug. 10, 2018, 7:08 p.m., Sergey Urbanovich wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68054/
> ---
> 
> (Updated Aug. 10, 2018, 7:08 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Qian Zhang.
> 
> 
> Bugs: MESOS-5647
> https://issues.apache.org/jira/browse/MESOS-5647
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> On receiving a request for getting `usage` for a given container
> the `network/cni` isolator uses getifaddrs(3) glibc function. The
> function returns basic networking metrics for each networking
> interface in the container networking namespace. It should work
> right out of the box on all modern Linux-based systems.
> 
> To get more networking metrics please use Netlink Protocol Library.
> However, you will have to open NETLINK sockets in each networking
> namespace and manage them from the `network/cni` isolator.
> 
> JIRA: https://issues.apache.org/jira/browse/MESOS-5647
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/network/cni/cni.hpp 
> 31ec4ddb1049b7259b0784e5e40b002e29f6a8da 
>   src/slave/containerizer/mesos/isolators/network/cni/cni.cpp 
> f46c962d8f044092aaa113fafc536c6b25bab45c 
>   src/slave/flags.hpp eeb9708f9ec76d83b6719541f4a012544c7c0cbe 
>   src/slave/flags.cpp 58cdc0f1100fe244e5bf1036e1ccf39478d5d478 
>   src/tests/containerizer/cni_isolator_tests.cpp 
> cae2c1950c6e5218f7fc7cebd93f5b4ed687fb5a 
> 
> 
> Diff: https://reviews.apache.org/r/68054/diff/5/
> 
> 
> Testing
> ---
> 
> bin/mesos-tests.sh --verbose 
> --gtest_filter="CniIsolatorTest.ROOT_VerifyResourceStatistics" 
> --gtest_break_on_failure --gtest_repeat=100
> 
> 
> Thanks,
> 
> Sergey Urbanovich
> 
>

Re: Review Request 67932: Windows: Extracted file layers using `wclayer` from `hcsshim`.

[Mesos Reviewbot Windows]

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



PASS: Mesos patch 67932 was successfully built and tested.

Reviews applied: `['67931', '67930', '67984', '67932']`

All the build artifacts available at: 
http://dcos-win.westus.cloudapp.azure.com/artifacts/mesos-reviewbot-testing/2139/mesos-review-67932

- Mesos Reviewbot Windows


On Aug. 6, 2018, 11:46 a.m., Liangyu Zhao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67932/
> ---
> 
> (Updated Aug. 6, 2018, 11:46 a.m.)
> 
> 
> Review request for mesos, Akash Gupta, Andrew Schwartzmeyer, Gilbert Song, 
> Jie Yu, Joseph Wu, and Qian Zhang.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The `tar` command cannot work successfully on Windows, so we use
> `wclayer` instead. Note that the folder generated from extraction also
> cannot be deleted by `rmdir`, so the GC is also changed to use
> `wclayer remove`.
> 
> 
> Diffs
> -
> 
>   src/common/command_utils.hpp 525f9c1c37b74f7e4cc71fdc8d52944226998ddc 
>   src/common/command_utils.cpp 7dfcc9ff74bcf044d47b803ebc42cf63fba89d17 
>   src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp 
> a5683e3fe15dd35596122fcc0c580ae9d3adf7f2 
>   src/slave/containerizer/mesos/provisioner/docker/store.cpp 
> 60507aa1b7951666ed758d1b3800eddd67ba7be6 
> 
> 
> Diff: https://reviews.apache.org/r/67932/diff/9/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Liangyu Zhao
> 
>

Re: Review Request 68257: Fixed incorrect `mnt` namespace detection of command executor's task.

[Vinod Kone]

> On Aug. 7, 2018, 4:03 p.m., Vinod Kone wrote:
> > src/slave/containerizer/mesos/utils.cpp
> > Line 102 (original), 105 (patched)
> > 
> >
> > Are we guaranteed that there are no short-lived processes, other than 
> > the task process, at the 2nd level? If not, we will have the same issue 
> > right?
> > 
> > Modulo the above question, the change LGTM.
> 
> Andrei Budnik wrote:
> There are two types of 2nd-level processes:
> 1) the command executor's task
> 2) the nested container's task
> 
> E.g. the process tree can be like the following:
> 0. mesos-containerizer (`init`)
>1. mesos-executor (command executor)
>   2. sleep 1000 (command executor's task)
>1. mesos-containerizer (`init` of a nested container) <- enters `mnt` 
> namespace of command executor's task before forking a task
>   2. echo "echo" (nested container's task)
> 
> Since we skip 1st-level processes whose `mnt` namespace is not the same 
> as `init` process (PID 1), the algorithm doesn't iterate over their 2nd-level 
> processes. That gives a gurarantee that we only detect command executor's 
> task, but not the short-lived processes.
> 
> Alexander Rukletsov wrote:
> Let's make it explicit to the reader. I suggest to:
> 1. Collect _candidates_ (2nd level children) first.
> 2. Assert there are >= 1 candidates.
> 3. Filter all candidates except those whose (`mnt` differs from root's 
> `mnt` && parent's `mnt` equals to root's `mnt`).
> 4. Assert there are now 0 or 1 candidates left.
> 5. Return pid of that single candidate or root's pid if there are none.

@andrei Thanks for the info. IIUC, any processes launched by the mesos-executor 
or the task using `LAUNCH_NESTED_CONTAINER` API will end up with the process 
tree you showed above. I just wanted to make sure it is not possible for us to 
see a child process of command executor that is a sibling of the task process 
(e.g., task launched a process without using LNC API but it ended up as a 
sibling, command executor runs a script/binary without using LNC API etc).


- Vinod


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


On Aug. 7, 2018, 1:46 p.m., Andrei Budnik wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68257/
> ---
> 
> (Updated Aug. 7, 2018, 1:46 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Gilbert Song, Jie Yu, and 
> Kevin Klues.
> 
> 
> Bugs: MESOS-9116
> https://issues.apache.org/jira/browse/MESOS-9116
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Previously, we were walking the process tree from the container's
> `init` process to find the first process along the way whose `mnt`
> namespace differs from the `init` process. We expected this algorithm
> to always return the PID of the command executor's task. However, if
> someone launches multiple nested containers within the process tree,
> the algorithm might detect the PID of the nested container instead of
> the command executor's task. The detected PID might belong to a
> short-lived container, so the container's process might terminate at
> the moment the containerizer launcher (aka `nanny`) process tries to
> enter its `mnt` namespace. This patch fixes the detection algorithm
> so that it always returns PID of the command executor's task.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/utils.cpp 
> 30e76d1d91651975033078f5450e45f5f2fd8ba0 
> 
> 
> Diff: https://reviews.apache.org/r/68257/diff/1/
> 
> 
> Testing
> ---
> 
> 1) Internal CI with disabled 
> `ROOT_CGROUPS_LaunchNestedContainerSessionsInParallel` test (see previous 
> patch).
> 2) Fedora 25: `./src/mesos-tests 
> --gtest_filter=*AgentAPITest.LaunchNestedContainerSessionInParallel* 
> --gtest_break_on_failure --gtest_repeat=100 --verbose`
> 
> 
> Thanks,
> 
> Andrei Budnik
> 
>

Re: Review Request 68304: Fixed a bug in the agent authentication retry logic.

[Mesos Reviewbot]

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



Patch looks great!

Reviews applied: [68304]

Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' 
CONFIGURATION='--verbose --disable-libtool-wrappers' ENVIRONMENT='GLOG_v=1 
MESOS_VERBOSE=1'; ./support/docker-build.sh

- Mesos Reviewbot


On Aug. 10, 2018, 4:41 p.m., Meng Zhu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68304/
> ---
> 
> (Updated Aug. 10, 2018, 4:41 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Gastón Kleiman.
> 
> 
> Bugs: MESOS-9147
> https://issues.apache.org/jira/browse/MESOS-9147
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch fixed the backoff time calculation
> overflow bug described in MESOS-9147.
> 
> This patch also combines the authentication timeout and
> authentication retry backoff interval into a single
> wait time interval.
> 
> 
> Diffs
> -
> 
>   src/slave/slave.hpp 802d4eb9e9eba2f1175dd85e56dcc80a61e32f74 
>   src/slave/slave.cpp 78e8666f402be58af5b6e20a715da4998af2615c 
>   src/tests/authentication_tests.cpp c9a8f85951a50e278ae509f4efa7105755015ce9 
> 
> 
> Diff: https://reviews.apache.org/r/68304/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Meng Zhu
> 
>

Re: Review Request 68304: Fixed a bug in the agent authentication retry logic.

[Gastón Kleiman]

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




src/slave/slave.cpp
Line 1366 (original), 1383 (patched)


Should we multiply `maxBackoff` by `2` here? I don't see it being increased 
anywhere else.



src/slave/slave.cpp
Lines 1386-1387 (patched)


Nit: I would move this comment to before the if statement, and the note 
that is currently above to within it.


- Gastón Kleiman


On Aug. 10, 2018, 4:41 p.m., Meng Zhu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68304/
> ---
> 
> (Updated Aug. 10, 2018, 4:41 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Gastón Kleiman.
> 
> 
> Bugs: MESOS-9147
> https://issues.apache.org/jira/browse/MESOS-9147
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch fixed the backoff time calculation
> overflow bug described in MESOS-9147.
> 
> This patch also combines the authentication timeout and
> authentication retry backoff interval into a single
> wait time interval.
> 
> 
> Diffs
> -
> 
>   src/slave/slave.hpp 802d4eb9e9eba2f1175dd85e56dcc80a61e32f74 
>   src/slave/slave.cpp 78e8666f402be58af5b6e20a715da4998af2615c 
>   src/tests/authentication_tests.cpp c9a8f85951a50e278ae509f4efa7105755015ce9 
> 
> 
> Diff: https://reviews.apache.org/r/68304/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Meng Zhu
> 
>

Re: Review Request 68304: Fixed a bug in the agent authentication retry logic.

[Meng Zhu]

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

(Updated Aug. 13, 2018, 2:53 p.m.)


Review request for mesos, Benjamin Mahler and Gastón Kleiman.


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


Repository: mesos


Description
---

This patch fixed the backoff time calculation
overflow bug described in MESOS-9147.

This patch also combines the authentication timeout and
authentication retry backoff interval into a single
wait time interval.


Diffs (updated)
-

  src/slave/slave.hpp 802d4eb9e9eba2f1175dd85e56dcc80a61e32f74 
  src/slave/slave.cpp 78e8666f402be58af5b6e20a715da4998af2615c 
  src/tests/authentication_tests.cpp c9a8f85951a50e278ae509f4efa7105755015ce9 


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

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


Testing
---

make check


Thanks,

Meng Zhu

Re: Review Request 68304: Fixed a bug in the agent authentication retry logic.

[Meng Zhu]

> On Aug. 13, 2018, 2:03 p.m., Gastón Kleiman wrote:
> > src/slave/slave.cpp
> > Line 1366 (original), 1383 (patched)
> > 
> >
> > Should we multiply `maxBackoff` by `2` here? I don't see it being 
> > increased anywhere else.

It is multiplied in `__authenticate()`:

```
  if (reauthenticate || !future.isReady()) {
LOG(WARNING)
  << "Failed to authenticate with master " << master.get() << ": "
  << (reauthenticate ? "master changed" :
 (future.isFailed() ? future.failure() : "future discarded"));

authenticating = None();
reauthenticate = false;

authenticate(maxBackoff * 2); // Retry.
return;
  }

```

I feel it is better to do the multiplication at the place where the failure 
occurs and retry starts.
I renamed the `maxBackoff` in `__authenticate()` to `currentMaxBackoff`. 
Hopefully it will improve the readibility.
Dropping.


- Meng


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


On Aug. 13, 2018, 2:53 p.m., Meng Zhu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68304/
> ---
> 
> (Updated Aug. 13, 2018, 2:53 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Gastón Kleiman.
> 
> 
> Bugs: MESOS-9147
> https://issues.apache.org/jira/browse/MESOS-9147
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch fixed the backoff time calculation
> overflow bug described in MESOS-9147.
> 
> This patch also combines the authentication timeout and
> authentication retry backoff interval into a single
> wait time interval.
> 
> 
> Diffs
> -
> 
>   src/slave/slave.hpp 802d4eb9e9eba2f1175dd85e56dcc80a61e32f74 
>   src/slave/slave.cpp 78e8666f402be58af5b6e20a715da4998af2615c 
>   src/tests/authentication_tests.cpp c9a8f85951a50e278ae509f4efa7105755015ce9 
> 
> 
> Diff: https://reviews.apache.org/r/68304/diff/2/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Meng Zhu
> 
>

Review Request 68324: Added two agent flags for min/max authentication retry interval.

[Meng Zhu]

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

Review request for mesos, Benjamin Mahler and Gastón Kleiman.


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


Repository: mesos


Description
---

`flags.min_authentication_retry_interval` controls the
minimum amount of time the agent waits before retrying
authenticating with the master after a failed attempt.

`flags.max_authentication_retry_interval` controls the
maximum amount of time the agent waits before retrying
authenticating with the master after a failed attempt.


Diffs
-

  docs/configuration/agent.md 4e50b681bb956d559da6bf1d2c504099aae3cafb 
  src/slave/constants.hpp 0bd9f371ca24df66055f31bca0d57625dc7642d9 
  src/slave/flags.hpp 88c35da5fd754abbd4bd316e1fa9efa4a70a6b8c 
  src/slave/flags.cpp 54d9acc8693f53294bdc2a88183cac84a8dfbfd9 
  src/slave/slave.cpp 78e8666f402be58af5b6e20a715da4998af2615c 
  src/tests/authentication_tests.cpp c9a8f85951a50e278ae509f4efa7105755015ce9 


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


Testing
---

make check


Thanks,

Meng Zhu

Re: Review Request 68321: Add '/state-summary' to batched master endpoints.

[Mesos Reviewbot]

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



Bad patch!

Reviews applied: [68321, 68296]

Failed command: python support/apply-reviews.py -n -r 68296

Error:
The support scripts will be upgraded to Python 3 by July 1st.
Make sure to install Python 3.6 on your machine before.
2018-08-13 22:03:16 URL:https://reviews.apache.org/r/68296/diff/raw/ 
[12794/12794] -> "68296.patch" [1]
error: patch failed: src/master/http.cpp:2843
error: src/master/http.cpp: patch does not apply
error: patch failed: src/master/master.hpp:1845
error: src/master/master.hpp: patch does not apply

Full log: https://builds.apache.org/job/Mesos-Reviewbot/23063/console

- Mesos Reviewbot


On Aug. 13, 2018, 1:46 p.m., Benno Evers wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68321/
> ---
> 
> (Updated Aug. 13, 2018, 1:46 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Benjamin Mahler.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add '/state-summary' to batched master endpoints.
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp d43fbd689598612ec5946b46e2fa5e7f5e22cfa8 
>   src/master/master.hpp 209b998db8d2bad7a3812df44f0939458f48eb11 
> 
> 
> Diff: https://reviews.apache.org/r/68321/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Benno Evers
> 
>

Review Request 68327: Added tests for FUTURE_EXITED and DROP_EXITED.

[Benjamin Mahler]

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

Review request for mesos, Gastón Kleiman and Meng Zhu.


Repository: mesos


Description
---

Added tests for FUTURE_EXITED and DROP_EXITED.


Diffs
-

  3rdparty/libprocess/src/tests/process_tests.cpp 
17ed2f45c892a3c1c34040a492ad3fdd4a7552c1 


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


Testing
---

make check


Thanks,

Benjamin Mahler

Review Request 68307: Added a test for master's handling of stale authentication requests.

[Benjamin Mahler]

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

Review request for mesos.


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


Repository: mesos


Description
---

This test ensures that when the master sees a new authentication
request for a particular agent or scheduler (we just test the
scheduler case is tested here since the master does not distinguish),
the master will discard the old one and proceed with the new one.


Diffs
-

  src/internal/devolve.hpp 8007f420b3a912d7eff1fa5faa7d8502eb3e9115 
  src/internal/evolve.hpp e792ff591eff537e2a4661afe08795f00eb35843 
  src/master/master.hpp 209b998db8d2bad7a3812df44f0939458f48eb11 
  src/master/metrics.hpp df28a486ead68421970723060850de3ac32e68a7 
  src/tests/authentication_tests.cpp c9a8f85951a50e278ae509f4efa7105755015ce9 


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


Testing
---

Ran in repetition.


Thanks,

Benjamin Mahler

Review Request 68326: Introduced FUTURE_EXITED and DROP_EXITED in libprocess.

[Benjamin Mahler]

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

Review request for mesos, Gastón Kleiman and Meng Zhu.


Repository: mesos


Description
---

These extend the existing event expectations to handle `ExitedEvent`.


Diffs
-

  3rdparty/libprocess/include/process/gmock.hpp 
e30f5863d3d028fa320ca4b674a6c7a499c15991 


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


Testing
---

Added a test in a subsequent patch.


Thanks,

Benjamin Mahler

Review Request 68306: Fixed an authentication request amplification issue in the master.

[Benjamin Mahler]

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

Review request for mesos, Alexander Rojas, Gastón Kleiman, Meng Zhu, Till 
Toenshoff, and Vinod Kone.


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


Repository: mesos


Description
---

Per MESOS-9144, re-enqueuing authentication requests leads to an
amplification effect which can overwhelm the master if requests
continue to arrive rapidly on a heavily loaded master. This patch
avoids the re-enqueing and ensures the master immediately processes
a new authentication request for a client.


Diffs
-

  src/master/master.cpp 400a83e35451a3ee0ea42b5ca729357bf6c744e8 


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


Testing
---

make check


Thanks,

Benjamin Mahler

Review Request 68325: Updated libprocess filtering to take the Process UPID.

[Benjamin Mahler]

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

Review request for mesos, Gastón Kleiman and Meng Zhu.


Repository: mesos


Description
---

When `EXPECT_DISPATCH` was originally introduced in de8e10830d178a7c,
the pid was added to `DispatchEvent` rather than being passed through
the filter API. This adds a performance overhead (we must perform an
unnecessary `UPID` copy construction / destruction for every
dispatch), and it also requires that we inject the `UPID` into
additional events (for example, if we wanted to introduce
`FUTURE_EXITED`, we need to add the UPID into `ExitedEvent`).

Rather than continue this trend, we can instead pass the Process
`UPID` through the filter API so that the client knows which Process
the event belongs to. This also lets us remove the `UPID` copy
construction / destruction cost from `DispatchEvent`, which should
provide a minor performance benefit.


Diffs
-

  3rdparty/libprocess/include/process/event.hpp 
d96db74e70efa2bb824f575b0e9c965e2cd4af39 
  3rdparty/libprocess/include/process/filter.hpp 
79a19175af1964ca6b6fd415f4dfb9b9d9d0e3e0 
  3rdparty/libprocess/include/process/gmock.hpp 
e30f5863d3d028fa320ca4b674a6c7a499c15991 
  3rdparty/libprocess/src/process.cpp 23f7ce8651009f9319f4ae4681816bec0217fa31 


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


Testing
---

make check


Thanks,

Benjamin Mahler

Review Request 68305: Increased and added flag for the master's authentication timeout.

[Benjamin Mahler]

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

Review request for mesos, Alexander Rojas, Gastón Kleiman, Meng Zhu, Till 
Toenshoff, and Vinod Kone.


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


Repository: mesos


Description
---

There is not a lot of value in the master timing out a client's
authentication, other than releasing a small amount of resources.
We currently have a burned in 5 second timeout, which is largely
sufficient since most authenticators are implemented to use an
actor per session and avoid any head-of-line blocking.

Ideally, the master would know how long the client's timeout and
the master can use that for its own timeout. The current max backoff
for schedulers and agents is 1 minute, so this patch bumps the
master's timeout to be closer to that (15 seconds). We don't bump it
further because the vast majority of the timeout time is spent in
the initial trip through the master's queue, which occurs before
the master sets up its timeout.

This also adds a flag, both to allow users to tune this, as well
as to allow us to control timing in tests.


Diffs
-

  src/master/constants.hpp f3b257a8a4c492593c701af442a0acc2b3c01762 
  src/master/flags.hpp 3929c297b45e1203e5b00454e88f86988a8b1058 
  src/master/flags.cpp 8fede0d2cf2254f34134329885f47c3c78dc5846 
  src/master/master.cpp 400a83e35451a3ee0ea42b5ca729357bf6c744e8 


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


Testing
---

make check


Thanks,

Benjamin Mahler

Review Request 68329: Windows: Made `libwinio` the default option.

[Andrew Schwartzmeyer]

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

Review request for mesos, Akash Gupta, Benjamin Mahler, and Joseph Wu.


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


Repository: mesos


Description
---

Note that we now add `io_tests.cpp` unconditionally, which will
fail/hang with libevent. However, this is because libevent is buggy,
and should not be used. We would prefer the tests not pass with
libevent enabled, rather than appear to pass because the failing tests
are excluded, giving the false impression that it works correctly.


Diffs
-

  3rdparty/libprocess/src/CMakeLists.txt 
19fa9809c2298ea68649702b94a0c75d806caba3 
  3rdparty/libprocess/src/tests/CMakeLists.txt 
a03a77eb5e289b4daac0bbd414dc17c8acc848dc 


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


Testing
---


Thanks,

Andrew Schwartzmeyer

Review Request 68331: Windows: Made `libwinio` the default option.

[Andrew Schwartzmeyer]

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

Review request for mesos, Akash Gupta, Benjamin Mahler, and Joseph Wu.


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


Repository: mesos


Description
---

With no options specified, Windows will now build with the
`libwinio` (native Windows Thread API eventing library) by
default (similar to POSIX defaulting to `libev` without options). To
use `libevent` (NOT recommended), it must be explicitly enabled, and
will emit an appropriate warning. Furthermore, the tests which were
previously excluded when `libevent` was enabled are now enabled on
Windows unconditionally. This serves two reasons: (1) it simplifies
the build logic and (2) by failing with `libevent` it demonstrates why
it is not recommended (and avoids giving false impressions).


Diffs
-

  3rdparty/CMakeLists.txt 629b1968775da4d128b8d36c77d75efa303b0b7c 
  cmake/CompilationConfigure.cmake 10cacfb99e2cff1ddd2285ae441730f61182e06d 
  docs/windows.md f0f12fc81ed664528cf7496cf75e1cb11eee6013 
  src/tests/CMakeLists.txt fed072a8a761c6b9e65b2c75f2e92facd8353f7c 


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


Testing
---

Built with and without `-DENABLE_LIBEVENT` on Windows.


Thanks,

Andrew Schwartzmeyer

Review Request 68333: Made CNI isolator cleanup more robust.

[Jie Yu]

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

Review request for mesos, Deepak Goel, Qian Zhang, and Sergey Urbanovich.


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


Repository: mesos


Description
---

If the container is destroyed while in isolator preparing state, the
cleanup might fail due to missing files or directories. This patch makes
the cleanup path in CNI isolator more robust so that the cleanup does
not fail in those scenarios.


Diffs
-

  src/slave/containerizer/mesos/isolators/network/cni/cni.cpp 
f46c962d8f044092aaa113fafc536c6b25bab45c 


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


Testing
---

sudo make check


Thanks,

Jie Yu

Review Request 68335: Added a CNI test to verify destroy while preparing.

[Jie Yu]

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

Review request for mesos, Deepak Goel, Qian Zhang, and Sergey Urbanovich.


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


Repository: mesos


Description
---

This test is used to catch the regression described in MESOS-9142.


Diffs
-

  src/slave/containerizer/mesos/containerizer.hpp 
7711d463c8ed92e2580c56e88d7f372c6dfaeb2b 
  src/tests/containerizer/cni_isolator_tests.cpp 
cb22e73b4215b5b0a49ac610e5f657b73d38963b 


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


Testing
---

sudo make check


Thanks,

Jie Yu

Review Request 68334: Used state::checkpoint intead in CNI isolator.

[Jie Yu]

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

Review request for mesos, Deepak Goel, Qian Zhang, and Sergey Urbanovich.


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


Repository: mesos


Description
---

This is to ensure all or nothing semantics. We don't want to deal with a
particially written file in case agent crashes.


Diffs
-

  src/slave/containerizer/mesos/isolators/network/cni/cni.cpp 
f46c962d8f044092aaa113fafc536c6b25bab45c 


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


Testing
---

sudo make check


Thanks,

Jie Yu

Re: Review Request 68334: Used state::checkpoint instead in CNI isolator.

[Jie Yu]

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

(Updated Aug. 13, 2018, 11:03 p.m.)


Review request for mesos, Deepak Goel, Qian Zhang, and Sergey Urbanovich.


Summary (updated)
-

Used state::checkpoint instead in CNI isolator.


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


Repository: mesos


Description
---

This is to ensure all or nothing semantics. We don't want to deal with a
particially written file in case agent crashes.


Diffs (updated)
-

  src/slave/containerizer/mesos/isolators/network/cni/cni.cpp 
f46c962d8f044092aaa113fafc536c6b25bab45c 


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

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


Testing
---

sudo make check


Thanks,

Jie Yu

Re: Review Request 68334: Used state::checkpoint instead in CNI isolator.

[Jie Yu]

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

(Updated Aug. 13, 2018, 11:04 p.m.)


Review request for mesos, Deepak Goel, Qian Zhang, and Sergey Urbanovich.


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


Repository: mesos


Description
---

This is to ensure all or nothing semantics. We don't want to deal with a
particially written file in case agent crashes.


Diffs (updated)
-

  src/slave/containerizer/mesos/isolators/network/cni/cni.cpp 
f46c962d8f044092aaa113fafc536c6b25bab45c 


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

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


Testing
---

sudo make check


Thanks,

Jie Yu

Re: Review Request 68304: Fixed a bug in the agent authentication retry logic.

[Benjamin Mahler]

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



Just a quick review since you're updating the logic.

How about:

```
Fixed a backoff overflow bug in agent authentication retry logic.
```

A little more in the commit description to clarify the old vs new approach 
would be nice for posterity (e.g. why did we decide to combine them?)


src/slave/slave.cpp
Lines 1375-1376 (patched)


Can you move this to `AUTHENTICATION_RETRY_INTERVAL_MIN` in the 
`slave/constants.hpp` and put the TODO to consider the flag there?


- Benjamin Mahler


On Aug. 13, 2018, 9:53 p.m., Meng Zhu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68304/
> ---
> 
> (Updated Aug. 13, 2018, 9:53 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Gastón Kleiman.
> 
> 
> Bugs: MESOS-9147
> https://issues.apache.org/jira/browse/MESOS-9147
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch fixed the backoff time calculation
> overflow bug described in MESOS-9147.
> 
> This patch also combines the authentication timeout and
> authentication retry backoff interval into a single
> wait time interval.
> 
> 
> Diffs
> -
> 
>   src/slave/slave.hpp 802d4eb9e9eba2f1175dd85e56dcc80a61e32f74 
>   src/slave/slave.cpp 78e8666f402be58af5b6e20a715da4998af2615c 
>   src/tests/authentication_tests.cpp c9a8f85951a50e278ae509f4efa7105755015ce9 
> 
> 
> Diff: https://reviews.apache.org/r/68304/diff/2/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Meng Zhu
> 
>

Re: Review Request 68305: Increased and added flag for the master's authentication timeout.

[Benjamin Mahler]

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




src/master/flags.cpp
Lines 237-243 (patched)


Note to self: add this to configuration.md


- Benjamin Mahler


On Aug. 13, 2018, 10:22 p.m., Benjamin Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68305/
> ---
> 
> (Updated Aug. 13, 2018, 10:22 p.m.)
> 
> 
> Review request for mesos, Alexander Rojas, Gastón Kleiman, Meng Zhu, Till 
> Toenshoff, and Vinod Kone.
> 
> 
> Bugs: MESOS-9145
> https://issues.apache.org/jira/browse/MESOS-9145
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> There is not a lot of value in the master timing out a client's
> authentication, other than releasing a small amount of resources.
> We currently have a burned in 5 second timeout, which is largely
> sufficient since most authenticators are implemented to use an
> actor per session and avoid any head-of-line blocking.
> 
> Ideally, the master would know how long the client's timeout and
> the master can use that for its own timeout. The current max backoff
> for schedulers and agents is 1 minute, so this patch bumps the
> master's timeout to be closer to that (15 seconds). We don't bump it
> further because the vast majority of the timeout time is spent in
> the initial trip through the master's queue, which occurs before
> the master sets up its timeout.
> 
> This also adds a flag, both to allow users to tune this, as well
> as to allow us to control timing in tests.
> 
> 
> Diffs
> -
> 
>   src/master/constants.hpp f3b257a8a4c492593c701af442a0acc2b3c01762 
>   src/master/flags.hpp 3929c297b45e1203e5b00454e88f86988a8b1058 
>   src/master/flags.cpp 8fede0d2cf2254f34134329885f47c3c78dc5846 
>   src/master/master.cpp 400a83e35451a3ee0ea42b5ca729357bf6c744e8 
> 
> 
> Diff: https://reviews.apache.org/r/68305/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Benjamin Mahler
> 
>

Re: Review Request 68324: Added two agent flags for min/max authentication retry interval.

[Benjamin Mahler]

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




src/slave/flags.cpp
Lines 365-380 (patched)


Maybe we should start these with `authentication`? How about:

```
authentication_timeout_min
authentication_timeout_max
```

I think these will be more easily understood and they're easier to find 
alongside other "authentication" prefixed flags?



src/slave/flags.cpp
Lines 369-370 (patched)


for more details. Note that since ...


- Benjamin Mahler


On Aug. 13, 2018, 9:54 p.m., Meng Zhu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68324/
> ---
> 
> (Updated Aug. 13, 2018, 9:54 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Gastón Kleiman.
> 
> 
> Bugs: MESOS-9146
> https://issues.apache.org/jira/browse/MESOS-9146
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> `flags.min_authentication_retry_interval` controls the
> minimum amount of time the agent waits before retrying
> authenticating with the master after a failed attempt.
> 
> `flags.max_authentication_retry_interval` controls the
> maximum amount of time the agent waits before retrying
> authenticating with the master after a failed attempt.
> 
> 
> Diffs
> -
> 
>   docs/configuration/agent.md 4e50b681bb956d559da6bf1d2c504099aae3cafb 
>   src/slave/constants.hpp 0bd9f371ca24df66055f31bca0d57625dc7642d9 
>   src/slave/flags.hpp 88c35da5fd754abbd4bd316e1fa9efa4a70a6b8c 
>   src/slave/flags.cpp 54d9acc8693f53294bdc2a88183cac84a8dfbfd9 
>   src/slave/slave.cpp 78e8666f402be58af5b6e20a715da4998af2615c 
>   src/tests/authentication_tests.cpp c9a8f85951a50e278ae509f4efa7105755015ce9 
> 
> 
> Diff: https://reviews.apache.org/r/68324/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Meng Zhu
> 
>

Re: Review Request 68216: Updated `volume/image` isolator to honor volume mode.

[Gilbert Song]

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




src/slave/containerizer/mesos/isolators/volume/image.cpp
Lines 117 (patched)


Given we already parse the volume once in prepare(), should we consider 
avoid parse it again in _prepare() and pass a vector of bool along?


- Gilbert Song


On Aug. 7, 2018, 7:20 a.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68216/
> ---
> 
> (Updated Aug. 7, 2018, 7:20 a.m.)
> 
> 
> Review request for mesos, Gilbert Song, Greg Mann, Ilya Pronin, and Jie Yu.
> 
> 
> Bugs: MESOS-8814
> https://issues.apache.org/jira/browse/MESOS-8814
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated `volume/image` isolator to honor volume mode.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/volume/image.hpp 
> 706b8ff28e1b1c8d15606d54d40622bc09885667 
>   src/slave/containerizer/mesos/isolators/volume/image.cpp 
> 345772439b73b4816f71e15bb4e43a5d67c51c02 
> 
> 
> Diff: https://reviews.apache.org/r/68216/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Re: Review Request 68324: Added two agent flags for min/max authentication retry interval.

[Mesos Reviewbot Windows]

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



PASS: Mesos patch 68324 was successfully built and tested.

Reviews applied: `['68304', '68324']`

All the build artifacts available at: 
http://dcos-win.westus.cloudapp.azure.com/artifacts/mesos-reviewbot-testing/2140/mesos-review-68324

- Mesos Reviewbot Windows


On Aug. 13, 2018, 2:54 p.m., Meng Zhu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68324/
> ---
> 
> (Updated Aug. 13, 2018, 2:54 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Gastón Kleiman.
> 
> 
> Bugs: MESOS-9146
> https://issues.apache.org/jira/browse/MESOS-9146
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> `flags.min_authentication_retry_interval` controls the
> minimum amount of time the agent waits before retrying
> authenticating with the master after a failed attempt.
> 
> `flags.max_authentication_retry_interval` controls the
> maximum amount of time the agent waits before retrying
> authenticating with the master after a failed attempt.
> 
> 
> Diffs
> -
> 
>   docs/configuration/agent.md 4e50b681bb956d559da6bf1d2c504099aae3cafb 
>   src/slave/constants.hpp 0bd9f371ca24df66055f31bca0d57625dc7642d9 
>   src/slave/flags.hpp 88c35da5fd754abbd4bd316e1fa9efa4a70a6b8c 
>   src/slave/flags.cpp 54d9acc8693f53294bdc2a88183cac84a8dfbfd9 
>   src/slave/slave.cpp 78e8666f402be58af5b6e20a715da4998af2615c 
>   src/tests/authentication_tests.cpp c9a8f85951a50e278ae509f4efa7105755015ce9 
> 
> 
> Diff: https://reviews.apache.org/r/68324/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Meng Zhu
> 
>

Re: Review Request 68218: Added a test `ROOT_ImageInReadOnlyVolumeWithoutRootFilesystem`.

[Gilbert Song]

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


Ship it!




Ship It!

- Gilbert Song


On Aug. 7, 2018, 7:22 a.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68218/
> ---
> 
> (Updated Aug. 7, 2018, 7:22 a.m.)
> 
> 
> Review request for mesos, Gilbert Song, Greg Mann, Ilya Pronin, and Jie Yu.
> 
> 
> Bugs: MESOS-8814
> https://issues.apache.org/jira/browse/MESOS-8814
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added a test `ROOT_ImageInReadOnlyVolumeWithoutRootFilesystem`.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/volume_image_isolator_tests.cpp 
> b49f0f98e3c31808d8d1e9edbb9a783bfe5231ce 
> 
> 
> Diff: https://reviews.apache.org/r/68218/diff/2/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Re: Review Request 68053: Call any function in a specified namespace.

[Jie Yu]

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


Ship it!




Ship It!

- Jie Yu


On July 31, 2018, 5:30 p.m., Sergey Urbanovich wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68053/
> ---
> 
> (Updated July 31, 2018, 5:30 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Chun-Hung Hsiao, Jie Yu, and Qian 
> Zhang.
> 
> 
> Bugs: MESOS-5647
> https://issues.apache.org/jira/browse/MESOS-5647
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The NamespaceRunner runs any function in a specified namespace. To do
> that it manages a separate thread which would be re-associated with
> that namespace.
> 
> 
> Diffs
> -
> 
>   src/linux/ns.hpp 0b4136bd3cc2d3e0cfee163d89469558e699b5f2 
>   src/tests/containerizer/ns_tests.cpp 
> fa4349e29b975550e05b00a1f848a24cd8e4f0de 
> 
> 
> Diff: https://reviews.apache.org/r/68053/diff/6/
> 
> 
> Testing
> ---
> 
> bin/mesos-tests.sh --verbose --gtest_filter="NsTest*" 
> --gtest_break_on_failure --gtest_repeat=100
> 
> 
> Thanks,
> 
> Sergey Urbanovich
> 
>

Re: Review Request 68219: Updated `volume/secret` isolator to honor volume mode.

[Gilbert Song]

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


Ship it!





src/slave/containerizer/mesos/isolators/volume/secret.cpp
Lines 287 (patched)


Why not the same order as previous patches? I guess it does not hurt


- Gilbert Song


On Aug. 7, 2018, 7:22 a.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68219/
> ---
> 
> (Updated Aug. 7, 2018, 7:22 a.m.)
> 
> 
> Review request for mesos, Gilbert Song, Greg Mann, Ilya Pronin, and Jie Yu.
> 
> 
> Bugs: MESOS-8814
> https://issues.apache.org/jira/browse/MESOS-8814
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated `volume/secret` isolator to honor volume mode.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/volume/secret.cpp 
> 663aafc7173667f8f78b26bdc824a3ab2e04ed25 
> 
> 
> Diff: https://reviews.apache.org/r/68219/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Re: Review Request 68220: Updated the test `ROOT_SecretInVolumeWithRootFilesystem`.

[Gilbert Song]

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


Ship it!




Ship It!

- Gilbert Song


On Aug. 7, 2018, 7:23 a.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68220/
> ---
> 
> (Updated Aug. 7, 2018, 7:23 a.m.)
> 
> 
> Review request for mesos, Gilbert Song, Greg Mann, Ilya Pronin, and Jie Yu.
> 
> 
> Bugs: MESOS-8814
> https://issues.apache.org/jira/browse/MESOS-8814
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This test is updated to cover both read-write and read-only volume.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/volume_secret_isolator_tests.cpp 
> 11cd3b627b056d1811ab481b8aa599c346181383 
> 
> 
> Diff: https://reviews.apache.org/r/68220/diff/2/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Re: Review Request 68335: Added a CNI test to verify destroy while preparing.

[Sergey Urbanovich]

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


Ship it!




LGTM

- Sergey Urbanovich


On Aug. 13, 2018, 11 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68335/
> ---
> 
> (Updated Aug. 13, 2018, 11 p.m.)
> 
> 
> Review request for mesos, Deepak Goel, Qian Zhang, and Sergey Urbanovich.
> 
> 
> Bugs: MESOS-9142
> https://issues.apache.org/jira/browse/MESOS-9142
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This test is used to catch the regression described in MESOS-9142.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/containerizer.hpp 
> 7711d463c8ed92e2580c56e88d7f372c6dfaeb2b 
>   src/tests/containerizer/cni_isolator_tests.cpp 
> cb22e73b4215b5b0a49ac610e5f657b73d38963b 
> 
> 
> Diff: https://reviews.apache.org/r/68335/diff/1/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 68221: Updated `docker/volume` isolator to honor volume mode.

[Gilbert Song]

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




src/slave/containerizer/mesos/isolators/docker/volume/isolator.cpp
Lines 343 (patched)


ditto


- Gilbert Song


On Aug. 7, 2018, 7:23 a.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68221/
> ---
> 
> (Updated Aug. 7, 2018, 7:23 a.m.)
> 
> 
> Review request for mesos, Gilbert Song, Greg Mann, Ilya Pronin, and Jie Yu.
> 
> 
> Bugs: MESOS-8814
> https://issues.apache.org/jira/browse/MESOS-8814
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated `docker/volume` isolator to honor volume mode.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/docker/volume/isolator.hpp 
> 76f1a5243c8d5028157f795d851b547a5ce57ac9 
>   src/slave/containerizer/mesos/isolators/docker/volume/isolator.cpp 
> ab749be6234a5eedc0617a131c126129f43f8d62 
> 
> 
> Diff: https://reviews.apache.org/r/68221/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Re: Review Request 68333: Made CNI isolator cleanup more robust.

[Deepak Goel]

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


Ship it!




Ship It!

- Deepak Goel


On Aug. 13, 2018, 10:59 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68333/
> ---
> 
> (Updated Aug. 13, 2018, 10:59 p.m.)
> 
> 
> Review request for mesos, Deepak Goel, Qian Zhang, and Sergey Urbanovich.
> 
> 
> Bugs: MESOS-9142
> https://issues.apache.org/jira/browse/MESOS-9142
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> If the container is destroyed while in isolator preparing state, the
> cleanup might fail due to missing files or directories. This patch makes
> the cleanup path in CNI isolator more robust so that the cleanup does
> not fail in those scenarios.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/network/cni/cni.cpp 
> f46c962d8f044092aaa113fafc536c6b25bab45c 
> 
> 
> Diff: https://reviews.apache.org/r/68333/diff/1/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 68222: Added 2 tests for `docker/volume` isolator to cover read-only volume.

[Gilbert Song]

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




src/tests/containerizer/docker_volume_isolator_tests.cpp
Lines 1175-1179 (patched)


why do we have 1 here?



src/tests/containerizer/docker_volume_isolator_tests.cpp
Lines 1307-1311 (patched)


ditto.


- Gilbert Song


On Aug. 7, 2018, 7:24 a.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68222/
> ---
> 
> (Updated Aug. 7, 2018, 7:24 a.m.)
> 
> 
> Review request for mesos, Gilbert Song, Greg Mann, Ilya Pronin, and Jie Yu.
> 
> 
> Bugs: MESOS-8814
> https://issues.apache.org/jira/browse/MESOS-8814
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added 2 tests for `docker/volume` isolator to cover read-only volume.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/docker_volume_isolator_tests.cpp 
> c15a6fad642474765e4ad1952af6cd9ee937379e 
> 
> 
> Diff: https://reviews.apache.org/r/68222/diff/2/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Re: Review Request 68333: Made CNI isolator cleanup more robust.

[Sergey Urbanovich]

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


Ship it!




Ship It!

- Sergey Urbanovich


On Aug. 13, 2018, 10:59 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68333/
> ---
> 
> (Updated Aug. 13, 2018, 10:59 p.m.)
> 
> 
> Review request for mesos, Deepak Goel, Qian Zhang, and Sergey Urbanovich.
> 
> 
> Bugs: MESOS-9142
> https://issues.apache.org/jira/browse/MESOS-9142
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> If the container is destroyed while in isolator preparing state, the
> cleanup might fail due to missing files or directories. This patch makes
> the cleanup path in CNI isolator more robust so that the cleanup does
> not fail in those scenarios.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/network/cni/cni.cpp 
> f46c962d8f044092aaa113fafc536c6b25bab45c 
> 
> 
> Diff: https://reviews.apache.org/r/68333/diff/1/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 68335: Added a CNI test to verify destroy while preparing.

[Deepak Goel]

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


Ship it!




Ship It!

- Deepak Goel


On Aug. 13, 2018, 11 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68335/
> ---
> 
> (Updated Aug. 13, 2018, 11 p.m.)
> 
> 
> Review request for mesos, Deepak Goel, Qian Zhang, and Sergey Urbanovich.
> 
> 
> Bugs: MESOS-9142
> https://issues.apache.org/jira/browse/MESOS-9142
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This test is used to catch the regression described in MESOS-9142.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/containerizer.hpp 
> 7711d463c8ed92e2580c56e88d7f372c6dfaeb2b 
>   src/tests/containerizer/cni_isolator_tests.cpp 
> cb22e73b4215b5b0a49ac610e5f657b73d38963b 
> 
> 
> Diff: https://reviews.apache.org/r/68335/diff/1/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 68054: Added networking statistics to cni isolator.

[Jie Yu]

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



I hit this issue on my box
```
I0813 16:42:29.687569 31007 linux_launcher.cpp:503] Launching container 
fae92a9a-561f-40a5-b49e-896be36fcf8b and cloning with namespaces CLONE_NEWNS | 
CLONE_NEWUTS | CLONE_NEWNET
I0813 16:42:29.700687 31005 containerizer.cpp:3021] Transitioning the state of 
container fae92a9a-561f-40a5-b49e-896be36fcf8b from PREPARING to ISOLATING
I0813 16:42:29.702626 30978 cni.cpp:954] Bind mounted '/proc/31055/ns/net' to 
'/run/mesos/isolators/network/cni/fae92a9a-561f-40a5-b49e-896be36fcf8b/ns' for 
container fae92a9a-561f-40a5-b49e-896be36fcf8b
I0813 16:42:29.899713 30981 cni.cpp:1385] Got assigned IPv4 address 
'203.0.113.2/24' from CNI network 'veth' for container 
fae92a9a-561f-40a5-b49e-896be36fcf8b
I0813 16:42:29.900835 30982 cni.cpp:1094] Unable to find DNS nameservers for 
container fae92a9a-561f-40a5-b49e-896be36fcf8b, using host '/etc/resolv.conf'
I0813 16:42:30.102488 30996 containerizer.cpp:3021] Transitioning the state of 
container fae92a9a-561f-40a5-b49e-896be36fcf8b from ISOLATING to FETCHING
I0813 16:42:30.105840 30992 containerizer.cpp:3021] Transitioning the state of 
container fae92a9a-561f-40a5-b49e-896be36fcf8b from FETCHING to RUNNING
Marked '/' as rslave
I0813 16:42:30.236037 31088 exec.cpp:162] Version: 1.7.0
W0813 16:42:31.245690 31126 process.cpp:1449] Failed to link to 
'10.0.49.2:44965', connect: Failed to connect to 10.0.49.2:44965: No route to 
host
I0813 16:42:31.246233 31106 exec.cpp:527] Agent exited ... shutting down
I0813 16:42:31.246580 31106 v0_v1executor.cpp:172] Implicitly connecting the 
executor to shut it down
I0813 16:42:31.247351 31110 executor.cpp:182] Received SHUTDOWN event
I0813 16:42:31.248729 31110 executor.cpp:796] Shutting down
I0813 16:42:31.250615 31126 process.cpp:926] Stopped the socket accept loop
I0813 16:42:31.308015 30980 containerizer.cpp:2860] Container 
fae92a9a-561f-40a5-b49e-896be36fcf8b has exited
I0813 16:42:31.308157 30980 containerizer.cpp:2407] Destroying container 
fae92a9a-561f-40a5-b49e-896be36fcf8b in RUNNING state
I0813 16:42:31.308187 30980 containerizer.cpp:3021] Transitioning the state of 
container fae92a9a-561f-40a5-b49e-896be36fcf8b from RUNNING to DESTROYING
I0813 16:42:31.309753 31017 linux_launcher.cpp:582] Asked to destroy container 
fae92a9a-561f-40a5-b49e-896be36fcf8b
I0813 16:42:31.311748 31017 linux_launcher.cpp:629] Destroying cgroup 
'/sys/fs/cgroup/freezer/mesos/fae92a9a-561f-40a5-b49e-896be36fcf8b'
```


src/slave/containerizer/mesos/isolators/network/cni/cni.hpp
Lines 191 (patched)


nits: kill this line.



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


I would like to add the cleanup code to `CniIsolatorTest::cleanup`.

It's possible that the test is ended with sigkill or ctrl+c. And `ip link 
add name vethmesostest0 type veth peer name vethmesostestns` might fail if the 
veth hasn't been cleaned up


- Jie Yu


On Aug. 10, 2018, 7:08 p.m., Sergey Urbanovich wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68054/
> ---
> 
> (Updated Aug. 10, 2018, 7:08 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Qian Zhang.
> 
> 
> Bugs: MESOS-5647
> https://issues.apache.org/jira/browse/MESOS-5647
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> On receiving a request for getting `usage` for a given container
> the `network/cni` isolator uses getifaddrs(3) glibc function. The
> function returns basic networking metrics for each networking
> interface in the container networking namespace. It should work
> right out of the box on all modern Linux-based systems.
> 
> To get more networking metrics please use Netlink Protocol Library.
> However, you will have to open NETLINK sockets in each networking
> namespace and manage them from the `network/cni` isolator.
> 
> JIRA: https://issues.apache.org/jira/browse/MESOS-5647
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/network/cni/cni.hpp 
> 31ec4ddb1049b7259b0784e5e40b002e29f6a8da 
>   src/slave/containerizer/mesos/isolators/network/cni/cni.cpp 
> f46c962d8f044092aaa113fafc536c6b25bab45c 
>   src/slave/flags.hpp eeb9708f9ec76d83b6719541f4a012544c7c0cbe 
>   src/slave/flags.cpp 58cdc0f1100fe244e5bf1036e1ccf39478d5d478 
>   src/tests/containerizer/cni_isolator_tests.cpp 
> cae2c1950c6e5218f7fc7cebd93f5b4ed687fb5a 
> 
> 
> Diff: https://reviews.apache.org/r/68054/diff/5/
> 
> 
> Testing
> ---
> 
> bin/mesos-tests.sh --verbose 
> --gtest_filter="CniIsolatorTest.ROOT_VerifyResourceS

Re: Review Request 68307: Added a test for master's handling of stale authentication requests.

[Mesos Reviewbot Windows]

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



PASS: Mesos patch 68307 was successfully built and tested.

Reviews applied: `['68325', '68326', '68327', '68305', '68306', '68307']`

All the build artifacts available at: 
http://dcos-win.westus.cloudapp.azure.com/artifacts/mesos-reviewbot-testing/2144/mesos-review-68307

- Mesos Reviewbot Windows


On Aug. 13, 2018, 10:24 p.m., Benjamin Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68307/
> ---
> 
> (Updated Aug. 13, 2018, 10:24 p.m.)
> 
> 
> Review request for mesos, Alexander Rojas, Gastón Kleiman, Meng Zhu, Till 
> Toenshoff, and Vinod Kone.
> 
> 
> Bugs: MESOS-9144
> https://issues.apache.org/jira/browse/MESOS-9144
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This test ensures that when the master sees a new authentication
> request for a particular agent or scheduler (we just test the
> scheduler case is tested here since the master does not distinguish),
> the master will discard the old one and proceed with the new one.
> 
> 
> Diffs
> -
> 
>   src/internal/devolve.hpp 8007f420b3a912d7eff1fa5faa7d8502eb3e9115 
>   src/internal/evolve.hpp e792ff591eff537e2a4661afe08795f00eb35843 
>   src/master/master.hpp 209b998db8d2bad7a3812df44f0939458f48eb11 
>   src/master/metrics.hpp df28a486ead68421970723060850de3ac32e68a7 
>   src/tests/authentication_tests.cpp c9a8f85951a50e278ae509f4efa7105755015ce9 
> 
> 
> Diff: https://reviews.apache.org/r/68307/diff/1/
> 
> 
> Testing
> ---
> 
> Ran in repetition.
> 
> 
> Thanks,
> 
> Benjamin Mahler
> 
>

Re: Review Request 67932: Windows: Extracted file layers using `wclayer` from `hcsshim`.

[Mesos Reviewbot]

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



Patch looks great!

Reviews applied: [67931, 67930, 67984, 67932]

Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' 
CONFIGURATION='--verbose --disable-libtool-wrappers' ENVIRONMENT='GLOG_v=1 
MESOS_VERBOSE=1'; ./support/docker-build.sh

- Mesos Reviewbot


On Aug. 6, 2018, 6:46 p.m., Liangyu Zhao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67932/
> ---
> 
> (Updated Aug. 6, 2018, 6:46 p.m.)
> 
> 
> Review request for mesos, Akash Gupta, Andrew Schwartzmeyer, Gilbert Song, 
> Jie Yu, Joseph Wu, and Qian Zhang.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The `tar` command cannot work successfully on Windows, so we use
> `wclayer` instead. Note that the folder generated from extraction also
> cannot be deleted by `rmdir`, so the GC is also changed to use
> `wclayer remove`.
> 
> 
> Diffs
> -
> 
>   src/common/command_utils.hpp 525f9c1c37b74f7e4cc71fdc8d52944226998ddc 
>   src/common/command_utils.cpp 7dfcc9ff74bcf044d47b803ebc42cf63fba89d17 
>   src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp 
> a5683e3fe15dd35596122fcc0c580ae9d3adf7f2 
>   src/slave/containerizer/mesos/provisioner/docker/store.cpp 
> 60507aa1b7951666ed758d1b3800eddd67ba7be6 
> 
> 
> Diff: https://reviews.apache.org/r/67932/diff/9/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Liangyu Zhao
> 
>

Re: Review Request 68329: Windows: Made `libwinio` the default option.

[Mesos Reviewbot Windows]

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



PASS: Mesos patch 68329 was successfully built and tested.

Reviews applied: `['68329']`

All the build artifacts available at: 
http://dcos-win.westus.cloudapp.azure.com/artifacts/mesos-reviewbot-testing/2141/mesos-review-68329

- Mesos Reviewbot Windows


On Aug. 13, 2018, 10:34 p.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68329/
> ---
> 
> (Updated Aug. 13, 2018, 10:34 p.m.)
> 
> 
> Review request for mesos, Akash Gupta, Benjamin Mahler, and Joseph Wu.
> 
> 
> Bugs: MESOS-9084
> https://issues.apache.org/jira/browse/MESOS-9084
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Note that we now add `io_tests.cpp` unconditionally, which will
> fail/hang with libevent. However, this is because libevent is buggy,
> and should not be used. We would prefer the tests not pass with
> libevent enabled, rather than appear to pass because the failing tests
> are excluded, giving the false impression that it works correctly.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/CMakeLists.txt 
> 19fa9809c2298ea68649702b94a0c75d806caba3 
>   3rdparty/libprocess/src/tests/CMakeLists.txt 
> a03a77eb5e289b4daac0bbd414dc17c8acc848dc 
> 
> 
> Diff: https://reviews.apache.org/r/68329/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>

Re: Review Request 68331: Windows: Made `libwinio` the default option.

[Mesos Reviewbot Windows]

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



PASS: Mesos patch 68331 was successfully built and tested.

Reviews applied: `['68331']`

All the build artifacts available at: 
http://dcos-win.westus.cloudapp.azure.com/artifacts/mesos-reviewbot-testing/2143/mesos-review-68331

- Mesos Reviewbot Windows


On Aug. 13, 2018, 10:35 p.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68331/
> ---
> 
> (Updated Aug. 13, 2018, 10:35 p.m.)
> 
> 
> Review request for mesos, Akash Gupta, Benjamin Mahler, and Joseph Wu.
> 
> 
> Bugs: MESOS-9084
> https://issues.apache.org/jira/browse/MESOS-9084
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> With no options specified, Windows will now build with the
> `libwinio` (native Windows Thread API eventing library) by
> default (similar to POSIX defaulting to `libev` without options). To
> use `libevent` (NOT recommended), it must be explicitly enabled, and
> will emit an appropriate warning. Furthermore, the tests which were
> previously excluded when `libevent` was enabled are now enabled on
> Windows unconditionally. This serves two reasons: (1) it simplifies
> the build logic and (2) by failing with `libevent` it demonstrates why
> it is not recommended (and avoids giving false impressions).
> 
> 
> Diffs
> -
> 
>   3rdparty/CMakeLists.txt 629b1968775da4d128b8d36c77d75efa303b0b7c 
>   cmake/CompilationConfigure.cmake 10cacfb99e2cff1ddd2285ae441730f61182e06d 
>   docs/windows.md f0f12fc81ed664528cf7496cf75e1cb11eee6013 
>   src/tests/CMakeLists.txt fed072a8a761c6b9e65b2c75f2e92facd8353f7c 
> 
> 
> Diff: https://reviews.apache.org/r/68331/diff/1/
> 
> 
> Testing
> ---
> 
> Built with and without `-DENABLE_LIBEVENT` on Windows.
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>

Re: Review Request 68335: Added a CNI test to verify destroy while preparing.

[Mesos Reviewbot Windows]

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



PASS: Mesos patch 68335 was successfully built and tested.

Reviews applied: `['68333', '68334', '68335']`

All the build artifacts available at: 
http://dcos-win.westus.cloudapp.azure.com/artifacts/mesos-reviewbot-testing/2142/mesos-review-68335

- Mesos Reviewbot Windows


On Aug. 13, 2018, 11 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68335/
> ---
> 
> (Updated Aug. 13, 2018, 11 p.m.)
> 
> 
> Review request for mesos, Deepak Goel, Qian Zhang, and Sergey Urbanovich.
> 
> 
> Bugs: MESOS-9142
> https://issues.apache.org/jira/browse/MESOS-9142
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This test is used to catch the regression described in MESOS-9142.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/containerizer.hpp 
> 7711d463c8ed92e2580c56e88d7f372c6dfaeb2b 
>   src/tests/containerizer/cni_isolator_tests.cpp 
> cb22e73b4215b5b0a49ac610e5f657b73d38963b 
> 
> 
> Diff: https://reviews.apache.org/r/68335/diff/1/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 68214: Updated `volume/sandbox_path` isolator to honor volume mode.

[Qian Zhang]

> On Aug. 13, 2018, 12:15 p.m., Gilbert Song wrote:
> > src/slave/containerizer/mesos/isolators/volume/sandbox_path.cpp
> > Line 373 (original), 373 (patched)
> > 
> >
> > Not yours, but could we remove this?

I think we still need to keep it. Otherwise, the code in it may fail to compile 
on platforms other than Linux because those codes use some Linux specific 
macros, like `MS_BIND`.


- Qian


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


On Aug. 7, 2018, 10:18 p.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68214/
> ---
> 
> (Updated Aug. 7, 2018, 10:18 p.m.)
> 
> 
> Review request for mesos, Gilbert Song, Greg Mann, Ilya Pronin, and Jie Yu.
> 
> 
> Bugs: MESOS-8814
> https://issues.apache.org/jira/browse/MESOS-8814
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated `volume/sandbox_path` isolator to honor volume mode.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/volume/sandbox_path.cpp 
> 4896c6811c2c59dcf00871b7a8b6b9b50da0f062 
> 
> 
> Diff: https://reviews.apache.org/r/68214/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Re: Review Request 68219: Updated `volume/secret` isolator to honor volume mode.

[Qian Zhang]

> On Aug. 14, 2018, 7:30 a.m., Gilbert Song wrote:
> > src/slave/containerizer/mesos/isolators/volume/secret.cpp
> > Lines 287 (patched)
> > 
> >
> > Why not the same order as previous patches? I guess it does not hurt

Agree, let me make it the same order as the previous patches.


- Qian


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


On Aug. 7, 2018, 10:22 p.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68219/
> ---
> 
> (Updated Aug. 7, 2018, 10:22 p.m.)
> 
> 
> Review request for mesos, Gilbert Song, Greg Mann, Ilya Pronin, and Jie Yu.
> 
> 
> Bugs: MESOS-8814
> https://issues.apache.org/jira/browse/MESOS-8814
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated `volume/secret` isolator to honor volume mode.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/volume/secret.cpp 
> 663aafc7173667f8f78b26bdc824a3ab2e04ed25 
> 
> 
> Diff: https://reviews.apache.org/r/68219/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Re: Review Request 68054: Added networking statistics to cni isolator.

[Sergey Urbanovich]

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

(Updated Aug. 14, 2018, 2:14 a.m.)


Review request for mesos, Jie Yu and Qian Zhang.


Changes
---

Moved `vethPlugin` to `SetUp()`, fixed delete issue with veth pair (see line 
201).


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


Repository: mesos


Description
---

On receiving a request for getting `usage` for a given container
the `network/cni` isolator uses getifaddrs(3) glibc function. The
function returns basic networking metrics for each networking
interface in the container networking namespace. It should work
right out of the box on all modern Linux-based systems.

To get more networking metrics please use Netlink Protocol Library.
However, you will have to open NETLINK sockets in each networking
namespace and manage them from the `network/cni` isolator.

JIRA: https://issues.apache.org/jira/browse/MESOS-5647


Diffs (updated)
-

  src/slave/containerizer/mesos/isolators/network/cni/cni.hpp 
31ec4ddb1049b7259b0784e5e40b002e29f6a8da 
  src/slave/containerizer/mesos/isolators/network/cni/cni.cpp 
f46c962d8f044092aaa113fafc536c6b25bab45c 
  src/slave/flags.hpp 88c35da5fd754abbd4bd316e1fa9efa4a70a6b8c 
  src/slave/flags.cpp 54d9acc8693f53294bdc2a88183cac84a8dfbfd9 
  src/tests/containerizer/cni_isolator_tests.cpp 
cb22e73b4215b5b0a49ac610e5f657b73d38963b 


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

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


Testing
---

bin/mesos-tests.sh --verbose 
--gtest_filter="CniIsolatorTest.ROOT_VerifyResourceStatistics" 
--gtest_break_on_failure --gtest_repeat=100


Thanks,

Sergey Urbanovich

Re: Review Request 68334: Used state::checkpoint instead in CNI isolator.

[Sergey Urbanovich]

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


Ship it!






- Sergey Urbanovich


On Aug. 13, 2018, 11:04 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68334/
> ---
> 
> (Updated Aug. 13, 2018, 11:04 p.m.)
> 
> 
> Review request for mesos, Deepak Goel, Qian Zhang, and Sergey Urbanovich.
> 
> 
> Bugs: MESOS-9142
> https://issues.apache.org/jira/browse/MESOS-9142
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This is to ensure all or nothing semantics. We don't want to deal with a
> particially written file in case agent crashes.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/network/cni/cni.cpp 
> f46c962d8f044092aaa113fafc536c6b25bab45c 
> 
> 
> Diff: https://reviews.apache.org/r/68334/diff/3/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 68054: Added networking statistics to cni isolator.

[Mesos Reviewbot Windows]

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



PASS: Mesos patch 68054 was successfully built and tested.

Reviews applied: `['68052', '68054']`

All the build artifacts available at: 
http://dcos-win.westus.cloudapp.azure.com/artifacts/mesos-reviewbot-testing/2145/mesos-review-68054

- Mesos Reviewbot Windows


On Aug. 14, 2018, 2:14 a.m., Sergey Urbanovich wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68054/
> ---
> 
> (Updated Aug. 14, 2018, 2:14 a.m.)
> 
> 
> Review request for mesos, Jie Yu and Qian Zhang.
> 
> 
> Bugs: MESOS-5647
> https://issues.apache.org/jira/browse/MESOS-5647
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> On receiving a request for getting `usage` for a given container
> the `network/cni` isolator uses getifaddrs(3) glibc function. The
> function returns basic networking metrics for each networking
> interface in the container networking namespace. It should work
> right out of the box on all modern Linux-based systems.
> 
> To get more networking metrics please use Netlink Protocol Library.
> However, you will have to open NETLINK sockets in each networking
> namespace and manage them from the `network/cni` isolator.
> 
> JIRA: https://issues.apache.org/jira/browse/MESOS-5647
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/network/cni/cni.hpp 
> 31ec4ddb1049b7259b0784e5e40b002e29f6a8da 
>   src/slave/containerizer/mesos/isolators/network/cni/cni.cpp 
> f46c962d8f044092aaa113fafc536c6b25bab45c 
>   src/slave/flags.hpp 88c35da5fd754abbd4bd316e1fa9efa4a70a6b8c 
>   src/slave/flags.cpp 54d9acc8693f53294bdc2a88183cac84a8dfbfd9 
>   src/tests/containerizer/cni_isolator_tests.cpp 
> cb22e73b4215b5b0a49ac610e5f657b73d38963b 
> 
> 
> Diff: https://reviews.apache.org/r/68054/diff/6/
> 
> 
> Testing
> ---
> 
> bin/mesos-tests.sh --verbose 
> --gtest_filter="CniIsolatorTest.ROOT_VerifyResourceStatistics" 
> --gtest_break_on_failure --gtest_repeat=100
> 
> 
> Thanks,
> 
> Sergey Urbanovich
> 
>

Re: Review Request 68324: Added two agent flags for min/max authentication retry interval.

[Mesos Reviewbot]

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



Patch looks great!

Reviews applied: [68304, 68324]

Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' 
CONFIGURATION='--verbose --disable-libtool-wrappers' ENVIRONMENT='GLOG_v=1 
MESOS_VERBOSE=1'; ./support/docker-build.sh

- Mesos Reviewbot


On Aug. 13, 2018, 9:54 p.m., Meng Zhu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68324/
> ---
> 
> (Updated Aug. 13, 2018, 9:54 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Gastón Kleiman.
> 
> 
> Bugs: MESOS-9146
> https://issues.apache.org/jira/browse/MESOS-9146
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> `flags.min_authentication_retry_interval` controls the
> minimum amount of time the agent waits before retrying
> authenticating with the master after a failed attempt.
> 
> `flags.max_authentication_retry_interval` controls the
> maximum amount of time the agent waits before retrying
> authenticating with the master after a failed attempt.
> 
> 
> Diffs
> -
> 
>   docs/configuration/agent.md 4e50b681bb956d559da6bf1d2c504099aae3cafb 
>   src/slave/constants.hpp 0bd9f371ca24df66055f31bca0d57625dc7642d9 
>   src/slave/flags.hpp 88c35da5fd754abbd4bd316e1fa9efa4a70a6b8c 
>   src/slave/flags.cpp 54d9acc8693f53294bdc2a88183cac84a8dfbfd9 
>   src/slave/slave.cpp 78e8666f402be58af5b6e20a715da4998af2615c 
>   src/tests/authentication_tests.cpp c9a8f85951a50e278ae509f4efa7105755015ce9 
> 
> 
> Diff: https://reviews.apache.org/r/68324/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Meng Zhu
> 
>

Re: Review Request 68304: Fixed a backoff overflow bug in agent authentication retry logic.

[Meng Zhu]

> On Aug. 13, 2018, 4:06 p.m., Benjamin Mahler wrote:
> > Just a quick review since you're updating the logic.
> > 
> > How about:
> > 
> > ```
> > Fixed a backoff overflow bug in agent authentication retry logic.
> > ```
> > 
> > A little more in the commit description to clarify the old vs new approach 
> > would be nice for posterity (e.g. why did we decide to combine them?)

Done.


- Meng


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


On Aug. 13, 2018, 10:33 p.m., Meng Zhu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68304/
> ---
> 
> (Updated Aug. 13, 2018, 10:33 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Gastón Kleiman.
> 
> 
> Bugs: MESOS-9147
> https://issues.apache.org/jira/browse/MESOS-9147
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch fixed the backoff time calculation
> overflow bug described in MESOS-9147.
> 
> The old approach times out an authentication request after
> 5s and then retries after some backoff time. This is not
> optimal because, if the agent is going to backoff some time
> before retry, we might as well wait that long for the
> previous authentication request (instead of timeout early).
> 
> This patch combines the authentication timeout and
> authentication retry backoff interval into a single
> wait time interval. Now agent will timeout the previous
> authentication request after the wait time interval and
> then immediately retry.
> 
> 
> Diffs
> -
> 
>   src/slave/constants.hpp 0bd9f371ca24df66055f31bca0d57625dc7642d9 
>   src/slave/slave.hpp 802d4eb9e9eba2f1175dd85e56dcc80a61e32f74 
>   src/slave/slave.cpp 78e8666f402be58af5b6e20a715da4998af2615c 
>   src/tests/authentication_tests.cpp c9a8f85951a50e278ae509f4efa7105755015ce9 
> 
> 
> Diff: https://reviews.apache.org/r/68304/diff/3/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Meng Zhu
> 
>

Re: Review Request 68304: Fixed a backoff overflow bug in agent authentication retry logic.

[Meng Zhu]

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

(Updated Aug. 13, 2018, 10:33 p.m.)


Review request for mesos, Benjamin Mahler and Gastón Kleiman.


Summary (updated)
-

Fixed a backoff overflow bug in agent authentication retry logic.


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


Repository: mesos


Description (updated)
---

This patch fixed the backoff time calculation
overflow bug described in MESOS-9147.

The old approach times out an authentication request after
5s and then retries after some backoff time. This is not
optimal because, if the agent is going to backoff some time
before retry, we might as well wait that long for the
previous authentication request (instead of timeout early).

This patch combines the authentication timeout and
authentication retry backoff interval into a single
wait time interval. Now agent will timeout the previous
authentication request after the wait time interval and
then immediately retry.


Diffs (updated)
-

  src/slave/constants.hpp 0bd9f371ca24df66055f31bca0d57625dc7642d9 
  src/slave/slave.hpp 802d4eb9e9eba2f1175dd85e56dcc80a61e32f74 
  src/slave/slave.cpp 78e8666f402be58af5b6e20a715da4998af2615c 
  src/tests/authentication_tests.cpp c9a8f85951a50e278ae509f4efa7105755015ce9 


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

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


Testing
---

make check


Thanks,

Meng Zhu

Re: Review Request 68324: Added two agent flags for min/max authentication retry interval.

[Meng Zhu]

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

(Updated Aug. 13, 2018, 10:37 p.m.)


Review request for mesos, Benjamin Mahler and Gastón Kleiman.


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


Repository: mesos


Description (updated)
---

`flags.authentication_timeout_min` controls the
minimum amount of time the agent waits before retrying
authenticating with the master after a failed attempt.

`flags.authentication_timeout_max` controls the
maximum amount of time the agent waits before retrying
authenticating with the master after a failed attempt.


Diffs (updated)
-

  docs/configuration/agent.md 4e50b681bb956d559da6bf1d2c504099aae3cafb 
  src/slave/constants.hpp 0bd9f371ca24df66055f31bca0d57625dc7642d9 
  src/slave/flags.hpp 88c35da5fd754abbd4bd316e1fa9efa4a70a6b8c 
  src/slave/flags.cpp 54d9acc8693f53294bdc2a88183cac84a8dfbfd9 
  src/slave/slave.cpp 78e8666f402be58af5b6e20a715da4998af2615c 
  src/tests/authentication_tests.cpp c9a8f85951a50e278ae509f4efa7105755015ce9 


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

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


Testing
---

make check


Thanks,

Meng Zhu

Re: Review Request 68324: Added two agent flags for min/max authentication retry interval.

[Meng Zhu]

> On Aug. 13, 2018, 4:14 p.m., Benjamin Mahler wrote:
> > src/slave/flags.cpp
> > Lines 365-380 (patched)
> > 
> >
> > Maybe we should start these with `authentication`? How about:
> > 
> > ```
> > authentication_timeout_min
> > authentication_timeout_max
> > ```
> > 
> > I think these will be more easily understood and they're easier to find 
> > alongside other "authentication" prefixed flags?

Done.

I was thinking of being consistent with the seeming convention of putting 
min/max at the beginning (see master flags). But I agree that your suggestion 
can be better understood.


- Meng


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


On Aug. 13, 2018, 10:37 p.m., Meng Zhu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68324/
> ---
> 
> (Updated Aug. 13, 2018, 10:37 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Gastón Kleiman.
> 
> 
> Bugs: MESOS-9146
> https://issues.apache.org/jira/browse/MESOS-9146
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> `flags.authentication_timeout_min` controls the
> minimum amount of time the agent waits before retrying
> authenticating with the master after a failed attempt.
> 
> `flags.authentication_timeout_max` controls the
> maximum amount of time the agent waits before retrying
> authenticating with the master after a failed attempt.
> 
> 
> Diffs
> -
> 
>   docs/configuration/agent.md 4e50b681bb956d559da6bf1d2c504099aae3cafb 
>   src/slave/constants.hpp 0bd9f371ca24df66055f31bca0d57625dc7642d9 
>   src/slave/flags.hpp 88c35da5fd754abbd4bd316e1fa9efa4a70a6b8c 
>   src/slave/flags.cpp 54d9acc8693f53294bdc2a88183cac84a8dfbfd9 
>   src/slave/slave.cpp 78e8666f402be58af5b6e20a715da4998af2615c 
>   src/tests/authentication_tests.cpp c9a8f85951a50e278ae509f4efa7105755015ce9 
> 
> 
> Diff: https://reviews.apache.org/r/68324/diff/2/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Meng Zhu
> 
>

Re: Review Request 68216: Updated `volume/image` isolator to honor volume mode.

[Qian Zhang]

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

(Updated Aug. 14, 2018, 2:28 p.m.)


Review request for mesos, Gilbert Song, Greg Mann, Ilya Pronin, and Jie Yu.


Changes
---

Addressed review comments.


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


Repository: mesos


Description
---

Updated `volume/image` isolator to honor volume mode.


Diffs (updated)
-

  src/slave/containerizer/mesos/isolators/volume/image.hpp 
706b8ff28e1b1c8d15606d54d40622bc09885667 
  src/slave/containerizer/mesos/isolators/volume/image.cpp 
345772439b73b4816f71e15bb4e43a5d67c51c02 


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

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


Testing
---


Thanks,

Qian Zhang

Re: Review Request 68219: Updated `volume/secret` isolator to honor volume mode.

[Qian Zhang]

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

(Updated Aug. 14, 2018, 2:29 p.m.)


Review request for mesos, Gilbert Song, Greg Mann, Ilya Pronin, and Jie Yu.


Changes
---

Addressed review comments.


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


Repository: mesos


Description
---

Updated `volume/secret` isolator to honor volume mode.


Diffs (updated)
-

  src/slave/containerizer/mesos/isolators/volume/secret.cpp 
663aafc7173667f8f78b26bdc824a3ab2e04ed25 


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

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


Testing
---


Thanks,

Qian Zhang

Re: Review Request 68222: Added 2 tests for `docker/volume` isolator to cover read-only volume.

[Qian Zhang]

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

(Updated Aug. 14, 2018, 2:29 p.m.)


Review request for mesos, Gilbert Song, Greg Mann, Ilya Pronin, and Jie Yu.


Changes
---

Addressed review comments.


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


Repository: mesos


Description
---

Added 2 tests for `docker/volume` isolator to cover read-only volume.


Diffs (updated)
-

  src/tests/containerizer/docker_volume_isolator_tests.cpp 
c15a6fad642474765e4ad1952af6cd9ee937379e 


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

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


Testing
---

sudo make check


Thanks,

Qian Zhang

Re: Review Request 68221: Updated `docker/volume` isolator to honor volume mode.

[Qian Zhang]

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

(Updated Aug. 14, 2018, 2:29 p.m.)


Review request for mesos, Gilbert Song, Greg Mann, Ilya Pronin, and Jie Yu.


Changes
---

Addressed review comments.


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


Repository: mesos


Description
---

Updated `docker/volume` isolator to honor volume mode.


Diffs (updated)
-

  src/slave/containerizer/mesos/isolators/docker/volume/isolator.hpp 
76f1a5243c8d5028157f795d851b547a5ce57ac9 
  src/slave/containerizer/mesos/isolators/docker/volume/isolator.cpp 
ab749be6234a5eedc0617a131c126129f43f8d62 


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

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


Testing
---


Thanks,

Qian Zhang