Re: Review Request 58394: Setup new directory for python http client lib in src/python.

[Mesos Reviewbot]

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



Patch looks great!

Reviews applied: [58394]

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 May 11, 2017, 6:34 p.m., Eric Chung wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58394/
> ---
> 
> (Updated May 11, 2017, 6:34 p.m.)
> 
> 
> Review request for mesos, Jason Lai, Joseph Wu, and Kevin Klues.
> 
> 
> Bugs: MESOS-7310
> https://issues.apache.org/jira/browse/MESOS-7310
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Review: https://reviews.apache.org/r/58394
> 
> 
> Diffs
> -
> 
>   src/cli_new/bootstrap 6d62e9adf1d543ed00a3a2cf2484edf1c33ee443 
>   src/python/.gitignore PRE-CREATION 
>   src/python/lib/mesos/__init__.py PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/58394/diff/2/
> 
> 
> Testing
> ---
> 
> Under src/cli_new:
> 1\. ./bootstrap
> 2\. . ./activate
> 3\. python
> 4\. >>> import mesos
> 5\. >>> mesos.\_\_path\_\_
> 6\. verify that the path printed out is indeed at src/python/lib/mesos
> 
> 
> Thanks,
> 
> Eric Chung
> 
>

Re: Review Request 59001: Added volume secret isolator.

[Kapil Arya]

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

(Updated May 11, 2017, 10:20 p.m.)


Review request for mesos, Gilbert Song, Jie Yu, and Vinod Kone.


Changes
---

Addressed Jie's comment


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


Repository: mesos


Description
---

Added volume secret isolator.


Diffs (updated)
-

  src/CMakeLists.txt 40d921ee7026f5ac47efbf0243e1cafab57825f9 
  src/Makefile.am 6bb81fd49b4564a0afa993b2cef6baa9d370ee7a 
  src/slave/containerizer/mesos/containerizer.cpp 
58ab74571fb14c6dbb1907151dc421f93e324bb5 
  src/slave/containerizer/mesos/isolators/volume/secret.hpp PRE-CREATION 
  src/slave/containerizer/mesos/isolators/volume/secret.cpp PRE-CREATION 
  src/tests/CMakeLists.txt 9f2af9cdd1cf50485f4cd84ce67bcceba64b9328 
  src/tests/containerizer/rootfs.cpp fdfecc65a3fcd19d6a4dfa574320f4d1f2755322 
  src/tests/containerizer/volume_secret_isolator_tests.cpp PRE-CREATION 


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

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


Testing
---

Added new tests an ran `make check`.


Thanks,

Kapil Arya

Re: Review Request 55903: Update XFS disk isolator documentation.

[Mesos Reviewbot]

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



Patch looks great!

Reviews applied: [55896, 55895, 55897, 55903]

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 May 11, 2017, 6:20 p.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55903/
> ---
> 
> (Updated May 11, 2017, 6:20 p.m.)
> 
> 
> Review request for mesos, Gilbert Song, Jie Yu, and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-5116
> https://issues.apache.org/jira/browse/MESOS-5116
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Update XFS disk isolator documentation.
> 
> 
> Diffs
> -
> 
>   docs/mesos-containerizer.md d0b9d762ab83bff1cf2bd98ed566376b7bd566c6 
> 
> 
> Diff: https://reviews.apache.org/r/55903/diff/6/
> 
> 
> Testing
> ---
> 
> None.
> 
> 
> Thanks,
> 
> James Peach
> 
>

Re: Review Request 58224: Optionally verify the source IP address for libprocess messages.

[Benjamin Mahler]

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



Currently we require getting the peer each time we receive some requests from 
the socket. This patch can use the `Request->client` directly, without needing 
to introduce the session change, without making the performance worse.

Since the performance isn't being made worse, how about we look into the peer 
caching optimization after adding this feature?


3rdparty/libprocess/src/process.cpp
Lines 217-221 (patched)


Can we exactly match the description you added to docs/configuration.md in 
the subsequent patch? Seems nice to have them in sync, especially since it's 
not that long.



3rdparty/libprocess/src/process.cpp
Lines 586 (patched)


Can you make this a pointer? We disallow static non-POD variables.



3rdparty/libprocess/src/process.cpp
Lines 2886-2891 (patched)


At this point, you can look at `request->client` rather than 
`session->peer`. This would avoid the need for introducing session for now, and 
doesn't make the performance worse.

Separately from this feature, we could figure out how to cache the peer to 
avoid getting each each time requests are decoded, in order to improve the 
performance.


- Benjamin Mahler


On May 10, 2017, 6:06 p.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58224/
> ---
> 
> (Updated May 10, 2017, 6:06 p.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Bugs: MESOS-7401
> https://issues.apache.org/jira/browse/MESOS-7401
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> In general, libprocess is unable to validate that a peer
> is a legitimate owner of the UPID it claims in a libprocess
> message. This change adds a check that the IP address in the
> UPID matches the peer address. This makes spoofing the UPID
> harder (eg. to send authenticated messages), but also breaks
> some legitimate configurations, particularly on multihomed
> hosts.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/process.cpp 
> 96ce7dbc486a2f1d55d2238a8a102bf024b12b1c 
> 
> 
> Diff: https://reviews.apache.org/r/58224/diff/9/
> 
> 
> Testing
> ---
> 
> make check (Fedora 25). Light manual testing.
> 
> With LIBPROCESS_require_peer_address_ip_match=true, all Mesos tests pass 
> except ``ExamplesTest.DiskFullFramework``, however enabling this will 
> definitely break some libprocess APIs (though not in the way that Mesos uses 
> them) and legitimate multi-homed configurations. Note that setting 
> LIBPROCESS_ip=127.0.0.1 makes you multihomed for this purpose, which is why 
> ``ExamplesTest.DiskFullFramework`` breaks.
> 
> 
> Thanks,
> 
> James Peach
> 
>

Re: Review Request 58928: Update process tests to use a non-zero UPID.

[Benjamin Mahler]

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


Fix it, then Ship it!




Test changes look good, just noticed that this is a few independent patches, 
could you pull them into separate smaller patches?

(1) Improve the comments for server socket IP selection in libprocess 
initialization.
(2) Fix a logging message in libprocess initialization. (should be using 
strerror, not hstrerror)
(3) Add a process ID for RemoteProcess in the libprocess tests. (to make it 
easier to debug)
(4) Update the libprocess tests to use a non-zero UPID.


3rdparty/libprocess/src/process.cpp
Lines 1156-1158 (original), 1156-1158 (patched)


Would you mind pulling out the comment change here and below ("taking the 
first result") into a separate single patch that is described as clarifying the 
hostname lookup comments in libprocess intiialization? Will be able to get this 
committed for you quickly.



3rdparty/libprocess/src/process.cpp
Line 1164 (original), 1164 (patched)


Ah thanks for fixing this, this can just be:

```
PLOG(FATAL) << "Failed to initialize, gethostname";
```

Also, since its an independent change, can you make this its own patch 
about fixing the logging message here? I can get it committed for you quickly.



3rdparty/libprocess/src/tests/process_tests.cpp
Line 1295 (original), 1295 (patched)


This seems like an unrelated change? Do you want to pull it out if so?

Small independent patches are faster to review and commit :)



3rdparty/libprocess/src/tests/test_linkee.cpp
Lines 141 (patched)


```
PLOG(FATAL) << "Failed to get the local hostname";
```


- Benjamin Mahler


On May 10, 2017, 6:05 p.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58928/
> ---
> 
> (Updated May 10, 2017, 6:05 p.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Bugs: MESOS-7401
> https://issues.apache.org/jira/browse/MESOS-7401
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Some of the remote process tests were using a UPID with a zero
> IP address field. In order to enable subsequent IP address
> validation in the libprocess receiver, update these tests to
> use a UPID containing the real IP address of the sender.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/process.cpp 
> 96ce7dbc486a2f1d55d2238a8a102bf024b12b1c 
>   3rdparty/libprocess/src/tests/process_tests.cpp 
> bf90c7e78fd50ad7e16cc0a69a248ba71e2a7115 
>   3rdparty/libprocess/src/tests/test_linkee.cpp 
> 921d67695bc0e4d601e9f74fbc625d69bf36ba50 
> 
> 
> Diff: https://reviews.apache.org/r/58928/diff/3/
> 
> 
> Testing
> ---
> 
> make check (Fedora 25)
> 
> 
> Thanks,
> 
> James Peach
> 
>

Re: Review Request 59150: Document LIBPROCESS_REQUIRE_PEER_ADDRESS_IP_MATCH.

[Benjamin Mahler]

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


Ship it!





docs/configuration.md
Lines 2121 (patched)


Should we also mention that it prevents the use of proxies as well?


- Benjamin Mahler


On May 10, 2017, 6:08 p.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59150/
> ---
> 
> (Updated May 10, 2017, 6:08 p.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Bugs: MESOS-7401
> https://issues.apache.org/jira/browse/MESOS-7401
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Document LIBPROCESS_REQUIRE_PEER_ADDRESS_IP_MATCH.
> 
> 
> Diffs
> -
> 
>   docs/configuration.md c5194e7566c13514f55742aa7278e75314f69d9c 
> 
> 
> Diff: https://reviews.apache.org/r/59150/diff/2/
> 
> 
> Testing
> ---
> 
> None.
> 
> 
> Thanks,
> 
> James Peach
> 
>

Re: Review Request 58977: Add local and peer address accessors to http::Connection.

[Benjamin Mahler]

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




3rdparty/libprocess/include/process/http.hpp
Lines 960-968 (patched)


Per our offline discussion, seems like we could avoid the 'Try's here?

For `peer()`, `http::connect` takes the peer address to begin with, so we 
should just be able to carry that through here without having to call `peer()` 
on the socket.

For `address()`, see my suggestion below.



3rdparty/libprocess/include/process/http.hpp
Lines 963 (patched)


Naming wise, I can forsee confusion when I call connection.address() and I 
get the local rather than peer address, e.g. I see libraries where 'address' 
refers to the peer address:


https://android.googlesource.com/platform/libcore/+/a0d32add4376f31f35e2a50f59185f16f5cd8ccb/luni/src/main/java/libcore/net/http/HttpConnection.java#181

Also, our own http::connect function takes the peer address as just 
"address", so it seems prone to confusion that Connection "address" is the 
local address and http::connect("address") is the peer.

How about localAddress and peerAddress to be explicit?



3rdparty/libprocess/include/process/http.hpp
Lines 968 (patched)


Can these functions be const?



3rdparty/libprocess/src/http.cpp
Lines 1407-1410 (original), 1423-1426 (patched)


To deal with the case of `socket->address()` failing, this could become:

```
Try local_address = socket->address();
if (local_address.isError()) {
  return Failure("Failed to get socket's address: " + 
local_address.error());
}

return socket->connect(peer_address)
  .then([socket, local_address, peer_addresss]() {
return Connection(socket.get(), local_address.geT(), peer_address);
  }
```


- Benjamin Mahler


On May 10, 2017, 6:05 p.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58977/
> ---
> 
> (Updated May 10, 2017, 6:05 p.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Bugs: MESOS-7401
> https://issues.apache.org/jira/browse/MESOS-7401
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add local and peer address accessors to http::Connection.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/http.hpp 
> 650b9d8aaba5636e1a445abf9e27e018ff82e610 
>   3rdparty/libprocess/src/http.cpp 9789607933745f1fc4e37f47ce1be6aecb33a6e6 
> 
> 
> Diff: https://reviews.apache.org/r/58977/diff/3/
> 
> 
> Testing
> ---
> 
> make check (Fedora 25)
> 
> 
> Thanks,
> 
> James Peach
> 
>

Review Request 59218: Removed code for old agents when handling status updates.

[Neil Conway]

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

Review request for mesos and Benjamin Mahler.


Repository: mesos


Description
---

0.x agents are no longer supported.


Diffs
-

  src/master/master.cpp 4e7a161f431624bd78d3e9032eb8587687149cad 


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


Testing
---

`make check`


Thanks,

Neil Conway

Review Request 59219: Removed code for old agents in agent re-registration logic.

[Neil Conway]

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

Review request for mesos and Benjamin Mahler.


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


Repository: mesos


Description
---

0.x agents are no longer supported.


Diffs
-

  src/master/master.cpp 4e7a161f431624bd78d3e9032eb8587687149cad 


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


Testing
---

make check


Thanks,

Neil Conway

Review Request 59217: Removed outdated comments.

[Neil Conway]

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

Review request for mesos and Benjamin Mahler.


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


Repository: mesos


Description
---

0.x agents are no longer supported.


Diffs
-

  src/master/master.cpp 4e7a161f431624bd78d3e9032eb8587687149cad 


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


Testing
---

`make check`


Thanks,

Neil Conway

Review Request 59216: Fixed some inaccurate comments.

[Neil Conway]

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

Review request for mesos and Benjamin Mahler.


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


Repository: mesos


Description
---

Orphan tasks/executors are still possible in Mesos 1.2.0; they are not
possible with Mesos 1.2.1 or Mesos >= 1.3.0.


Diffs
-

  include/mesos/master/master.proto 841e207a40228dcad7210aae69cfdd2059183367 


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


Testing
---

`make check`


Thanks,

Neil Conway

Review Request 59215: Removed orphaned tasks from webui.

[Neil Conway]

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

Review request for mesos and Benjamin Mahler.


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


Repository: mesos


Description
---

Now that 0.x agents are not allowed to register with the master, orphan
tasks should no longer be possible.


Diffs
-

  src/webui/master/static/home.html 386934f4e5c568fbb1f53de50a0b144bd38290ed 
  src/webui/master/static/js/controllers.js 
a021962573d452de1581e6a7717016eac7d0cd85 


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


Testing
---

Visual inspection of webui.


Thanks,

Neil Conway

Review Request 59214: Removed code for handling missing FrameworkInfo of a running task.

[Neil Conway]

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

Review request for mesos and Benjamin Mahler.


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


Repository: mesos


Description
---

In previous versions of Mesos, the master might know about a task
running on an agent, but might not have a `FrameworkInfo` for that
task's framework. This might occur if the master fails over, the agent
re-registers, but the framework has not (yet) re-registered.

In Mesos 1.0, the agent will now supply the FrameworkInfo for all tasks
it is running when it re-registers with the master. Since we no longer
support 0.x agents, we can remove the old backward compatibility code
for handling a running task with unknown FrameworkInfo.

Note that this means that "orphan tasks", "orphan executors", and
"unregistered frameworks" are no longer possible.


Diffs
-

  src/master/http.cpp e2590a17044ac019b24a24629428d4ec8adc0c31 
  src/master/master.cpp 4e7a161f431624bd78d3e9032eb8587687149cad 


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


Testing
---

`make check`


Thanks,

Neil Conway

Review Request 59213: Cleaned up agent re-registration code in the master slightly.

[Neil Conway]

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

Review request for mesos and Benjamin Mahler.


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


Repository: mesos


Description
---

Cleaned up agent re-registration code in the master slightly.


Diffs
-

  src/master/master.cpp 4e7a161f431624bd78d3e9032eb8587687149cad 


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


Testing
---

`make check`


Thanks,

Neil Conway

Re: Review Request 58347: Introduced a filter for test cases that need thread-safety.

[Neil Conway]

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



Also please double-check that this RR builds by itself (applying the patch with 
fuzziness and compiling doesn't build).

- Neil Conway


On April 24, 2017, 11:22 a.m., Jan Schlicht wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58347/
> ---
> 
> (Updated April 24, 2017, 11:22 a.m.)
> 
> 
> Review request for mesos, Neil Conway and Till Toenshoff.
> 
> 
> Bugs: MESOS-7193
> https://issues.apache.org/jira/browse/MESOS-7193
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Instead of using asserts that would fail a test at runtime when
> thread-safety of the test environment isn't given, these tests are now
> filtered out before running the tests.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/tests/future_tests.cpp 
> f21361ed1e354778bcd0357afb71300f05d3ecfd 
>   3rdparty/libprocess/src/tests/io_tests.cpp 
> 302fadc2a1894d3fd7c4f4975af3f2cc6c3a22de 
>   3rdparty/libprocess/src/tests/limiter_tests.cpp 
> b80b1da214f97b50aa7b61b79bbf683fd01116aa 
>   3rdparty/libprocess/src/tests/main.cpp 
> 6ae772416d6a84427d82f4b25b8b26e100426933 
>   3rdparty/libprocess/src/tests/metrics_tests.cpp 
> d7fdb06060b273e16be27a263b5ee268842aa25c 
>   3rdparty/libprocess/src/tests/process_tests.cpp 
> bf90c7e78fd50ad7e16cc0a69a248ba71e2a7115 
>   3rdparty/libprocess/src/tests/reap_tests.cpp 
> 30518dee6c2fb904a607c7a457a5ec7366aab818 
> 
> 
> Diff: https://reviews.apache.org/r/58347/diff/2/
> 
> 
> Testing
> ---
> 
> libprocess-tests (using automake and CMake with macOS, Linux, Windows)
> 
> 
> Thanks,
> 
> Jan Schlicht
> 
>

Re: Review Request 58347: Introduced a filter for test cases that need thread-safety.

[Neil Conway]

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



Can you rebase this, please?

- Neil Conway


On April 24, 2017, 11:22 a.m., Jan Schlicht wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58347/
> ---
> 
> (Updated April 24, 2017, 11:22 a.m.)
> 
> 
> Review request for mesos, Neil Conway and Till Toenshoff.
> 
> 
> Bugs: MESOS-7193
> https://issues.apache.org/jira/browse/MESOS-7193
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Instead of using asserts that would fail a test at runtime when
> thread-safety of the test environment isn't given, these tests are now
> filtered out before running the tests.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/tests/future_tests.cpp 
> f21361ed1e354778bcd0357afb71300f05d3ecfd 
>   3rdparty/libprocess/src/tests/io_tests.cpp 
> 302fadc2a1894d3fd7c4f4975af3f2cc6c3a22de 
>   3rdparty/libprocess/src/tests/limiter_tests.cpp 
> b80b1da214f97b50aa7b61b79bbf683fd01116aa 
>   3rdparty/libprocess/src/tests/main.cpp 
> 6ae772416d6a84427d82f4b25b8b26e100426933 
>   3rdparty/libprocess/src/tests/metrics_tests.cpp 
> d7fdb06060b273e16be27a263b5ee268842aa25c 
>   3rdparty/libprocess/src/tests/process_tests.cpp 
> bf90c7e78fd50ad7e16cc0a69a248ba71e2a7115 
>   3rdparty/libprocess/src/tests/reap_tests.cpp 
> 30518dee6c2fb904a607c7a457a5ec7366aab818 
> 
> 
> Diff: https://reviews.apache.org/r/58347/diff/2/
> 
> 
> Testing
> ---
> 
> libprocess-tests (using automake and CMake with macOS, Linux, Windows)
> 
> 
> Thanks,
> 
> Jan Schlicht
> 
>

Re: Review Request 59198: Documented that Mesos 1.3.0, 1.2.1 don't allow old agents to register.

[Adam B]

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


Ship it!




Ship It!

- Adam B


On May 11, 2017, 2:10 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59198/
> ---
> 
> (Updated May 11, 2017, 2:10 p.m.)
> 
> 
> Review request for mesos and Adam B.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Documented that Mesos 1.3.0, 1.2.1 don't allow old agents to register.
> 
> 
> Diffs
> -
> 
>   CHANGELOG 31c21e9a69a6e11429b58a7eddecf7746a284530 
>   docs/upgrades.md 74e74642b411eefb737d575b89a1f206cae62b46 
> 
> 
> Diff: https://reviews.apache.org/r/59198/diff/4/
> 
> 
> Testing
> ---
> 
> Visual inspection, no functional change.
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 59198: Documented that Mesos 1.3.0, 1.2.1 don't allow old agents to register.

[Neil Conway]

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

(Updated May 11, 2017, 9:10 p.m.)


Review request for mesos and Adam B.


Changes
---

Address review comments.


Repository: mesos


Description
---

Documented that Mesos 1.3.0, 1.2.1 don't allow old agents to register.


Diffs (updated)
-

  CHANGELOG 31c21e9a69a6e11429b58a7eddecf7746a284530 
  docs/upgrades.md 74e74642b411eefb737d575b89a1f206cae62b46 


Diff: https://reviews.apache.org/r/59198/diff/4/

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


Testing
---

Visual inspection, no functional change.


Thanks,

Neil Conway

Re: Review Request 59202: Ported the net::mac to the Windows Platform.

[Jeff Coffler]

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

(Updated May 11, 2017, 9:10 p.m.)


Review request for mesos, Andrew Schwartzmeyer, John Kordich, Joseph Wu, and Li 
Li.


Changes
---

Clarified comments on how the interface name is dealt with on Windows.


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


Repository: mesos


Description
---

Note that, on Windows, net::mac takes a MAC address rather than a
network name. See associated changes to net::links for an explanation
of why this is the case.

Just for sanity, even though we take a MAC address as input, we go
ahead and validate the input, validating that the mac address passed
in is actually valid on the current system.


Diffs (updated)
-

  3rdparty/stout/include/stout/mac.hpp 7ed5f38ae4f93e267406254cb2acff201faffe45 
  3rdparty/stout/include/stout/posix/mac.hpp PRE-CREATION 
  3rdparty/stout/include/stout/windows/mac.hpp PRE-CREATION 


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

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


Testing
---

See final commit for tests performed.


Thanks,

Jeff Coffler

Re: Review Request 57935: Recalculate shares only when total scalar quantities have changed.

[Anindya Sinha]

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

(Updated May 11, 2017, 9:02 p.m.)


Review request for mesos and Neil Conway.


Changes
---

Rebased.


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


Repository: mesos


Description
---

In `DRFSorter::update`, we should set the `dirty` flag only when the
total scalar quantities have changed in any of the cleints in the
hierarchy, and not always.


Diffs (updated)
-

  src/master/allocator/sorter/drf/sorter.hpp 
fee58d6d1f08163e2a06a4a20c891fe535c3dcff 
  src/master/allocator/sorter/drf/sorter.cpp 
26b77f578f3235a8792c72d4575d607cdb2c7de7 


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

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


Testing
---

All tests passed.


Thanks,

Anindya Sinha

Re: Review Request 59200: Ported the net::links routine to Windows platform.

[Jeff Coffler]

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

(Updated May 11, 2017, 8:56 p.m.)


Review request for mesos, Andrew Schwartzmeyer, John Kordich, Joseph Wu, and Li 
Li.


Changes
---

Fixed comment error in 3rdparty/stout/include/stout/windows/net.hpp (noted in 
comments)


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


Repository: mesos


Description (updated)
---

Note that, rather than returning an interface name, the Windows
version will return the MAC address. The actual interface name,
on Windows, is a GUID (since there can be multiple wireless
interfaces, for example). However, it's not possible to get the
GUID from the command line (via 'ipconfig').

As a result, the MAC address of the interface (also unique) is
used to uniquely identify a particular interface on the system.


Diffs (updated)
-

  3rdparty/stout/include/stout/net.hpp b7cef53d3df8688f88211d7107927b7c8ccaed7f 
  3rdparty/stout/include/stout/posix/net.hpp 
39b89828d350d83bf1d4b0badcd3e63eb9d1a630 
  3rdparty/stout/include/stout/windows/net.hpp 
1bed115cb848332bf9c31e455b2d001c173face9 


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

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


Testing
---

See final commit for tests performed.


Thanks,

Jeff Coffler

Re: Review Request 59200: Ported the net::links routine to Windows platform.

[Jeff Coffler]

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




3rdparty/stout/include/stout/windows/net.hpp
Lines 62 (patched)


Oops, I added this comment at the last minute, neglecting to notice that I 
had the comment at line 40-41 on this same issue.

I'll post an update to this commit to get rid of this.


- Jeff Coffler


On May 11, 2017, 8:13 p.m., Jeff Coffler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59200/
> ---
> 
> (Updated May 11, 2017, 8:13 p.m.)
> 
> 
> Review request for mesos, Andrew Schwartzmeyer, John Kordich, Joseph Wu, and 
> Li Li.
> 
> 
> Bugs: MESOS-5938
> https://issues.apache.org/jira/browse/MESOS-5938
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Note that, rather than returning an interface name, the Windows
> version will return the MAC address. The actual interface name,
> on Windows, is a GUID (since there can be multiple wireless
> interfaces, for example). However, it's not possible to get the
> MAC access from the command line.
> 
> As a result, the MAC address of the interface (also unique) is
> used to uniquely identify a particular interface on the system.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/net.hpp 
> b7cef53d3df8688f88211d7107927b7c8ccaed7f 
>   3rdparty/stout/include/stout/posix/net.hpp 
> 39b89828d350d83bf1d4b0badcd3e63eb9d1a630 
>   3rdparty/stout/include/stout/windows/net.hpp 
> 1bed115cb848332bf9c31e455b2d001c173face9 
> 
> 
> Diff: https://reviews.apache.org/r/59200/diff/1/
> 
> 
> Testing
> ---
> 
> See final commit for tests performed.
> 
> 
> Thanks,
> 
> Jeff Coffler
> 
>

Re: Review Request 59198: Documented that Mesos 1.3.0, 1.2.1 don't allow old agents to register.

[Adam B]

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



Couple more nits, then we can ship-it


CHANGELOG
Line 52 (original), 52 (patched)


Still mixing `> 1.0` and `pre-1.0`, so either `s/pre-1.0/0.x/g` or 
`s/pre-1.0/< 1.0/` or `s/> 1.0/1.1+/`



docs/upgrades.md
Line 278 (original), 278 (patched)


Ditto on `> 1.0` and `pre-1.0`.



docs/upgrades.md
Lines 50 (patched)


Should we add the same thing to 1.2.x (with a special note that it only 
applies to `1.2.1+`)?


- Adam B


On May 11, 2017, 12:47 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59198/
> ---
> 
> (Updated May 11, 2017, 12:47 p.m.)
> 
> 
> Review request for mesos and Adam B.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Documented that Mesos 1.3.0, 1.2.1 don't allow old agents to register.
> 
> 
> Diffs
> -
> 
>   CHANGELOG 31c21e9a69a6e11429b58a7eddecf7746a284530 
>   docs/upgrades.md 74e74642b411eefb737d575b89a1f206cae62b46 
> 
> 
> Diff: https://reviews.apache.org/r/59198/diff/3/
> 
> 
> Testing
> ---
> 
> Visual inspection, no functional change.
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 58486: Update the allocator on a per offer operation.

[Anindya Sinha]

> On April 20, 2017, 6:17 p.m., Jiang Yan Xu wrote:
> > I feel the following may be more clear and general at high level.
> > 
> > - In `_accept()` all operations are validated, for resource operations we 
> > call `updateAllocation` for each individual operation [like we used to 
> > do](https://github.com/apache/mesos/blob/e7fcc4fbc09122480c8f2549a8af099769e888c0/src/master/master.cpp#L3770).
> >  
> > - We fail `updateAllocate` if the operation cannot be applied but leave the 
> > allocator in a good state.
> > - Individual resource operations are sent to the agent only when 
> > `updateAllocate` succeeds, otherwise the operation is dropped with no bad 
> > side effects.
> > 
> > It'll be also great if we can come up with a test?

The overall approach is as follows (based on your comments and F2F discussions):
1) `Resources::apply()` fails if a `DESTROY` is applied on a `Resources` object 
with shared count != 1 (https://reviews.apache.org/r/59194/, 
https://issues.apache.org/jira/browse/MESOS-7403).
2) `updateAllocation()` returns a failed future if any operation cannot be 
applied to `slave.allocated` (https://reviews.apache.org/r/59195/).
3) All valid operations received as a part of `ACCEPT` call 
`updateAllocation()` individually. Since `updateAllocation()` now returns a 
`Future`, we proceed to the next step in processing of `ACCEPT` once 
we get back the futures from `updateAllocation()` for each of the valid 
operations.
3a) For each of the non-failed futures (from step 3):
i) For `LAUNCH` and `LAUNCH_GROUP`: We send the `RunTaskMessage` or 
`RunTaskGroupMessage` to the agent.
ii) For all other operations, we send the `CheckpointedResourcesMessage` to 
the agent.
3b) For each of failed futures, we drop that operation.
4) Recover resources as applicable based on the successful allocation updates 
for the valid operations.

I will add tests in subsequent review in this chain.


> On April 20, 2017, 6:17 p.m., Jiang Yan Xu wrote:
> > src/master/allocator/mesos/hierarchical.cpp
> > Lines 746-749 (original), 746-749 (patched)
> > 
> >
> > Below here we can use the same mechanism as done in `updateAvailable`:
> > 
> > ```
> > // In addition to asserting the operation can be applied to the 
> > offered
> > // resources above, here we also check if it can be applied to the 
> > agent's
> > // current allocated resources. We do this because certain 
> > operations
> > // require that they are applicable to the agent's allocated 
> > resources
> > // as a whole. One such example is that a shared persistent volume 
> > can
> > // only be destroyed by a framework when it is not allocated to 
> > other
> > // frameworks (see `Resources::apply` for details). The master 
> > already does
> > // best-effort validation but this check may still fail due to 
> > additional   
> > // `allocate` runs. In a race condition an `allocate` could have 
> > been
> > // enqueued by the allocator itself just before master's request to
> > // enqueue `updateAllocation` arrives to the allocator. This is 
> > similar
> > // to the siutation in `updateAvailable`, see the diagram there.
> > Try updatedAllocated = slave.allocated.apply(operation);
> > if (updatedAllocated.isError()) {
> >   return Failure(updatedAllocated.error());
> > }
> > ```
> > 
> > We need to comment clearly on why we need this. Above I tried to state 
> > the general idea but also gave an concrete example but this is just a first 
> > draft.
> > 
> > 
> > For this to work we need to make sure `Resources::apply` fails in such 
> > a case: https://issues.apache.org/jira/browse/MESOS-7403
> > 
> > But now the failure would occur in the right place in this method (not 
> > applicable to the current allocated resources) and the error message 
> > returned by `Resources::apply` would be able to explain what exactly 
> > happened.

Fixed in https://reviews.apache.org/r/59195/.


> On April 20, 2017, 6:17 p.m., Jiang Yan Xu wrote:
> > src/master/allocator/mesos/hierarchical.cpp
> > Lines 848-851 (original)
> > 
> >
> > This being at the bottom of the method was intended to validate the 
> > post-condition. If this doesn't hold, ideally we shouldn't capture it here 
> > because we don't know how it has failed. We should capture the failure at 
> > the individual step and construct an error message/log that's explicit 
> > about it.

Updated in https://reviews.apache.org/r/59195/.


> On April 20, 2017, 6:17 p.m., Jiang Yan Xu wrote:
> > src/master/master.cpp
> > Lines 4774-4775 (patched)
> > 
> >
> > So if the failure has occurred, the allocation in the a

Re: Review Request 58486: Update the allocator on a per offer operation.

[Anindya Sinha]

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

(Updated May 11, 2017, 8:21 p.m.)


Review request for mesos, James Peach and Jiang Yan Xu.


Summary (updated)
-

Update the allocator on a per offer operation.


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


Repository: mesos


Description (updated)
---

The allocator is updated for each offer operation in the order specified
in the `ACCEPT` call. Once the allocator is updated successfully, we
handle subsequent operations as follows:

1) Launch or Launch Group: We launch the tasks or task group on the
   agent.
2) (Un)reservation or Create/Destroy of Persistent Volumes: We send
   the `CheckPointResourcesMessage` to the agent.

If allocation for any of the operations fail, those resources are not
recovered from the allocator.


Diffs (updated)
-

  src/master/master.hpp 89d0790fd5fea59e74276f462581fe0073594732 
  src/master/master.cpp 4e7a161f431624bd78d3e9032eb8587687149cad 


Diff: https://reviews.apache.org/r/58486/diff/4/

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


Testing
---

Tests passed.


Thanks,

Anindya Sinha

Review Request 59194: Validate DESTROY operation in `Resources::apply()`.

[Anindya Sinha]

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

Review request for mesos, James Peach and Jiang Yan Xu.


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


Repository: mesos


Description
---

Ensure that applying `DESTROY` operation removes this volume. This is
needed to handle `DESTROY` of shared volumes to ensure that the
resulted `Resources` object has this `Resource` removed.


Diffs
-

  src/common/resources.cpp f6f02eb0a7ca05ab8fd9a670f32428862009b7d5 
  src/tests/resources_tests.cpp 4cf320d802a749f1419ac5b9f63b6c73b0c974be 
  src/v1/resources.cpp cad069defb34d5ccc20a0e083eb88cb80f70e415 


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


Testing
---

All tests passed.


Thanks,

Anindya Sinha

Review Request 59195: Ensure that allocator can be updated before committing changes.

[Anindya Sinha]

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

Review request for mesos, James Peach and Jiang Yan Xu.


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


Repository: mesos


Description
---

In addition to `apply()` on `offeredResources`, we ensure this operation
can be applied on `slave.allocated`. It is is so because certain
operations require that they are applicable to the agent's allocated
resources as a whole.
Note that we return a failed future on the first error (and there may
be other operations which might have failed). However, we do not
modify the state of the allocator if any of the operations fail.


Diffs
-

  include/mesos/allocator/allocator.hpp 
dc34a1b6f0c2bdf0d653bd53394364cfa9873ca7 
  src/master/allocator/mesos/allocator.hpp 
119b461136123229f85ed3c3cfd41137974a6b9b 
  src/master/allocator/mesos/hierarchical.hpp 
123f97cf495bff0f822838e09df0d88818f04da6 
  src/master/allocator/mesos/hierarchical.cpp 
b75ed9a20a9a42f958cebbacd91e5e15b0d21394 
  src/tests/allocator.hpp 4ea722491f63f5ceda5a47228aafddc020f643b0 
  src/tests/hierarchical_allocator_tests.cpp 
08180b9975869de328f0c095dd3cddf0c84fbecf 


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


Testing
---

All tests passed.


Thanks,

Anindya Sinha

Review Request 59202: Ported the net::mac to the Windows Platform.

[Jeff Coffler]

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

Review request for mesos, Andrew Schwartzmeyer, John Kordich, Joseph Wu, and Li 
Li.


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


Repository: mesos


Description
---

Note that, on Windows, net::mac takes a MAC address rather than a
network name. See associated changes to net::links for an explanation
of why this is the case.

Just for sanity, even though we take a MAC address as input, we go
ahead and validate the input, validating that the mac address passed
in is actually valid on the current system.


Diffs
-

  3rdparty/stout/include/stout/mac.hpp 7ed5f38ae4f93e267406254cb2acff201faffe45 
  3rdparty/stout/include/stout/posix/mac.hpp PRE-CREATION 
  3rdparty/stout/include/stout/windows/mac.hpp PRE-CREATION 


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


Testing
---

See final commit for tests performed.


Thanks,

Jeff Coffler

Review Request 59201: Ported the IPNetwork::fromLinkDevice to the Windows Platform.

[Jeff Coffler]

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

Review request for mesos, Andrew Schwartzmeyer, John Kordich, Joseph Wu, and Li 
Li.


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


Repository: mesos


Description
---

Note that, on Windows, IPNetwork::fromLinkDevice takes a MAC address
rather than a network name. See associated chagnes to net::links for
an explanation of why this is the case.


Diffs
-

  3rdparty/stout/include/stout/ip.hpp ad2bd922158eecd12b51b272d2a003b8ec8d3550 
  3rdparty/stout/include/stout/posix/ip.hpp PRE-CREATION 
  3rdparty/stout/include/stout/windows/ip.hpp PRE-CREATION 


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


Testing
---

See final commit for tests performed.


Thanks,

Jeff Coffler

Review Request 59200: Ported the net::links routine to Windows platform.

[Jeff Coffler]

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

Review request for mesos, Andrew Schwartzmeyer, John Kordich, and Joseph Wu.


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


Repository: mesos


Description
---

Note that, rather than returning an interface name, the Windows
version will return the MAC address. The actual interface name,
on Windows, is a GUID (since there can be multiple wireless
interfaces, for example). However, it's not possible to get the
MAC access from the command line.

As a result, the MAC address of the interface (also unique) is
used to uniquely identify a particular interface on the system.


Diffs
-

  3rdparty/stout/include/stout/net.hpp b7cef53d3df8688f88211d7107927b7c8ccaed7f 
  3rdparty/stout/include/stout/posix/net.hpp 
39b89828d350d83bf1d4b0badcd3e63eb9d1a630 
  3rdparty/stout/include/stout/windows/net.hpp 
1bed115cb848332bf9c31e455b2d001c173face9 


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


Testing
---

See final commit for tests performed.


Thanks,

Jeff Coffler

Re: Review Request 59203: Enable NetTest.LinkDevice and NetTest.Mac tests on Windows platform.

[Jeff Coffler]

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

(Updated May 11, 2017, 8:09 p.m.)


Review request for mesos, Andrew Schwartzmeyer, John Kordich, Joseph Wu, and Li 
Li.


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


Repository: mesos


Description
---

Enable NetTest.LinkDevice and NetTest.Mac tests on Windows platform.


Diffs
-

  3rdparty/stout/tests/ip_tests.cpp 4c6f81bd53413360182b447ddb149a18e406870e 
  3rdparty/stout/tests/mac_tests.cpp d4560e75ae4e03d1765c599298e88842381690b9 


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


Testing
---

On Linux, did a 'make check' and insured that all tests (particularly stout 
tests, where the work was performed) passed.

On Windows, did the rough equivalent. Here's relevant output from the Windows 
stout-tests.exe program:

[--] 8 tests from NetTest
**[ RUN  ] NetTest.LinkDevice
[   OK ] NetTest.LinkDevice (4 ms)**
[ RUN  ] NetTest.ConstructIPv4
[   OK ] NetTest.ConstructIPv4 (1 ms)
[ RUN  ] NetTest.ConstructIPv6
[   OK ] NetTest.ConstructIPv6 (0 ms)
[ RUN  ] NetTest.ConstructIPv4Network
[   OK ] NetTest.ConstructIPv4Network (0 ms)
[ RUN  ] NetTest.ConstructIPv6Network
[   OK ] NetTest.ConstructIPv6Network (0 ms)
**[ RUN  ] NetTest.Mac
[   OK ] NetTest.Mac (3 ms)**
[ RUN  ] NetTest.ConstructMAC
[   OK ] NetTest.ConstructMAC (1 ms)
[ RUN  ] NetTest.ParseMAC
[   OK ] NetTest.ParseMAC (1 ms)
[--] 8 tests from NetTest (19 ms total)


Thanks,

Jeff Coffler

Review Request 59203: Enable NetTest.LinkDevice and NetTest.Mac tests on Windows platform.

[Jeff Coffler]

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

Review request for mesos.


Repository: mesos


Description
---

Enable NetTest.LinkDevice and NetTest.Mac tests on Windows platform.


Diffs
-

  3rdparty/stout/tests/ip_tests.cpp 4c6f81bd53413360182b447ddb149a18e406870e 
  3rdparty/stout/tests/mac_tests.cpp d4560e75ae4e03d1765c599298e88842381690b9 


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


Testing
---

On Linux, did a 'make check' and insured that all tests (particularly stout 
tests, where the work was performed) passed.

On Windows, did the rough equivalent. Here's relevant output from the Windows 
stout-tests.exe program:

[--] 8 tests from NetTest
**[ RUN  ] NetTest.LinkDevice
[   OK ] NetTest.LinkDevice (4 ms)**
[ RUN  ] NetTest.ConstructIPv4
[   OK ] NetTest.ConstructIPv4 (1 ms)
[ RUN  ] NetTest.ConstructIPv6
[   OK ] NetTest.ConstructIPv6 (0 ms)
[ RUN  ] NetTest.ConstructIPv4Network
[   OK ] NetTest.ConstructIPv4Network (0 ms)
[ RUN  ] NetTest.ConstructIPv6Network
[   OK ] NetTest.ConstructIPv6Network (0 ms)
**[ RUN  ] NetTest.Mac
[   OK ] NetTest.Mac (3 ms)**
[ RUN  ] NetTest.ConstructMAC
[   OK ] NetTest.ConstructMAC (1 ms)
[ RUN  ] NetTest.ParseMAC
[   OK ] NetTest.ParseMAC (1 ms)
[--] 8 tests from NetTest (19 ms total)


Thanks,

Jeff Coffler

Re: Review Request 59198: Documented that Mesos 1.3.0, 1.2.1 don't allow old agents to register.

[Neil Conway]

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

(Updated May 11, 2017, 7:47 p.m.)


Review request for mesos and Adam B.


Changes
---

Address review comments.


Repository: mesos


Description
---

Documented that Mesos 1.3.0, 1.2.1 don't allow old agents to register.


Diffs (updated)
-

  CHANGELOG 31c21e9a69a6e11429b58a7eddecf7746a284530 
  docs/upgrades.md 74e74642b411eefb737d575b89a1f206cae62b46 


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

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


Testing
---

Visual inspection, no functional change.


Thanks,

Neil Conway

Re: Review Request 59198: Documented that Mesos 1.3.0, 1.2.1 don't allow old agents to register.

[Adam B]

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



A few thoughts


CHANGELOG
Lines 51-52 (patched)


Can we refer to "pre-1.0" as `0.x`?



CHANGELOG
Lines 52 (patched)


Interoperability between 1.0 masters and 0.28 agents was supported, during 
live upgrade.



CHANGELOG
Lines 300 (patched)


s/1.3.0/1.2.1/ ?


- Adam B


On May 11, 2017, 11:58 a.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59198/
> ---
> 
> (Updated May 11, 2017, 11:58 a.m.)
> 
> 
> Review request for mesos and Adam B.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Documented that Mesos 1.3.0, 1.2.1 don't allow old agents to register.
> 
> 
> Diffs
> -
> 
>   CHANGELOG 31c21e9a69a6e11429b58a7eddecf7746a284530 
>   docs/upgrades.md 74e74642b411eefb737d575b89a1f206cae62b46 
> 
> 
> Diff: https://reviews.apache.org/r/59198/diff/2/
> 
> 
> Testing
> ---
> 
> Visual inspection, no functional change.
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 59198: Documented that Mesos 1.3.0, 1.2.1 don't allow old agents to register.

[Neil Conway]

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

(Updated May 11, 2017, 6:58 p.m.)


Review request for mesos and Adam B.


Changes
---

Update `upgrades.md`


Summary (updated)
-

Documented that Mesos 1.3.0, 1.2.1 don't allow old agents to register.


Repository: mesos


Description (updated)
---

Documented that Mesos 1.3.0, 1.2.1 don't allow old agents to register.


Diffs (updated)
-

  CHANGELOG 31c21e9a69a6e11429b58a7eddecf7746a284530 
  docs/upgrades.md 74e74642b411eefb737d575b89a1f206cae62b46 


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

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


Testing
---

Visual inspection, no functional change.


Thanks,

Neil Conway

Re: Review Request 59191: Added another test case for Docker version parsing.

[Benjamin Mahler]

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


Ship it!




Ship It!

- Benjamin Mahler


On May 11, 2017, 5:30 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59191/
> ---
> 
> (Updated May 11, 2017, 5:30 p.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added another test case for Docker version parsing.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/docker_tests.cpp 
> 4660afcb759823f51800be8809ec0f07a3288c20 
> 
> 
> Diff: https://reviews.apache.org/r/59191/diff/1/
> 
> 
> Testing
> ---
> 
> `sudo ./src/mesos-tests --gtest_filter="DockerTest.*"`
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 59190: Adjusted test expectation for change to stout's Version.

[Benjamin Mahler]

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


Ship it!




Ship It!

- Benjamin Mahler


On May 11, 2017, 5:30 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59190/
> ---
> 
> (Updated May 11, 2017, 5:30 p.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Prerelease labels are now considered for equality between versions, per
> SemVer spec.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/docker_tests.cpp 
> 4660afcb759823f51800be8809ec0f07a3288c20 
> 
> 
> Diff: https://reviews.apache.org/r/59190/diff/1/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> `sudo ./src/mesos-tests --gtest_filter="DockerTest.*"`
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Review Request 59198: Added CHANGELOG note that 1.3.0 and 1.2.1 masters ignore old agents.

[Neil Conway]

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

Review request for mesos and Adam B.


Repository: mesos


Description
---

Added CHANGELOG note that 1.3.0 and 1.2.1 masters ignore old agents.


Diffs
-

  CHANGELOG 31c21e9a69a6e11429b58a7eddecf7746a284530 


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


Testing
---

Visual inspection, no functional change.


Thanks,

Neil Conway

Re: Review Request 58394: Setup new directory for python http client lib in src/python.

[Eric Chung]

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

(Updated May 11, 2017, 6:34 p.m.)


Review request for mesos, Jason Lai, Joseph Wu, and Kevin Klues.


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


Repository: mesos


Description (updated)
---

Review: https://reviews.apache.org/r/58394


Diffs (updated)
-

  src/cli_new/bootstrap 6d62e9adf1d543ed00a3a2cf2484edf1c33ee443 
  src/python/.gitignore PRE-CREATION 
  src/python/lib/mesos/__init__.py PRE-CREATION 


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

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


Testing
---

Under src/cli_new:
1\. ./bootstrap
2\. . ./activate
3\. python
4\. >>> import mesos
5\. >>> mesos.\_\_path\_\_
6\. verify that the path printed out is indeed at src/python/lib/mesos


Thanks,

Eric Chung

Re: Review Request 58394: Setup new directory for python http client lib in src/python.

[Eric Chung]

> On May 2, 2017, 11:11 p.m., Kevin Klues wrote:
> > src/python/.gitignore
> > Lines 1 (patched)
> > 
> >
> > Why do you need .virtualenv in this folder? I wouldn't expect us to 
> > create a .virtualenv directory anywhere within this subolder (at least not 
> > yet). Once we move the cli code under this folder it might make sense. But 
> > not yet.

good point, i'll remove it


> On May 2, 2017, 11:11 p.m., Kevin Klues wrote:
> > src/python/lib/mesos/__init__.py
> > Lines 1 (patched)
> > 
> >
> > I would exclude this from this commit.
> > It is not used anywhere, therefore it should not be included until the 
> > commit where it is used.

this makes it clear that this dir is a python package, in addition to making 
git check the dir in. i would like to keep it unless there is a very strong 
reason not to.


- Eric


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


On April 12, 2017, 9:44 p.m., Eric Chung wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58394/
> ---
> 
> (Updated April 12, 2017, 9:44 p.m.)
> 
> 
> Review request for mesos, Jason Lai, Joseph Wu, and Kevin Klues.
> 
> 
> Bugs: MESOS-7310
> https://issues.apache.org/jira/browse/MESOS-7310
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Setup new directory for python http client lib in src/python.
> 
> 
> Diffs
> -
> 
>   src/cli_new/bootstrap 6d62e9adf1d543ed00a3a2cf2484edf1c33ee443 
>   src/python/.gitignore PRE-CREATION 
>   src/python/lib/mesos/__init__.py PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/58394/diff/1/
> 
> 
> Testing
> ---
> 
> Under src/cli_new:
> 1\. ./bootstrap
> 2\. . ./activate
> 3\. python
> 4\. >>> import mesos
> 5\. >>> mesos.\_\_path\_\_
> 6\. verify that the path printed out is indeed at src/python/lib/mesos
> 
> 
> Thanks,
> 
> Eric Chung
> 
>

Re: Review Request 55903: Update XFS disk isolator documentation.

[James Peach]

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

(Updated May 11, 2017, 6:20 p.m.)


Review request for mesos, Gilbert Song, Jie Yu, and Jiang Yan Xu.


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


Repository: mesos


Description
---

Update XFS disk isolator documentation.


Diffs (updated)
-

  docs/mesos-containerizer.md d0b9d762ab83bff1cf2bd98ed566376b7bd566c6 


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

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


Testing
---

None.


Thanks,

James Peach

Re: Review Request 55903: Update XFS disk isolator documentation.

[Jiang Yan Xu]

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


Fix it, then Ship it!





docs/mesos-containerizer.md
Lines 103 (patched)


s/only//


- Jiang Yan Xu


On May 11, 2017, 10:55 a.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55903/
> ---
> 
> (Updated May 11, 2017, 10:55 a.m.)
> 
> 
> Review request for mesos, Gilbert Song, Jie Yu, and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-5116
> https://issues.apache.org/jira/browse/MESOS-5116
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Update XFS disk isolator documentation.
> 
> 
> Diffs
> -
> 
>   docs/mesos-containerizer.md d0b9d762ab83bff1cf2bd98ed566376b7bd566c6 
> 
> 
> Diff: https://reviews.apache.org/r/55903/diff/5/
> 
> 
> Testing
> ---
> 
> None.
> 
> 
> Thanks,
> 
> James Peach
> 
>

Re: Review Request 58487: Fix allocation quantities when shared resources are removed.

[Anindya Sinha]

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

(Updated May 11, 2017, 6:13 p.m.)


Review request for mesos, James Peach and Jiang Yan Xu.


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


Repository: mesos


Description
---

When shared resources are removed from the `allocations` in the sorter,
we remove the scalar quantity only when the shared resource is actually
removed (i.e. shared count goes down to 0). Similarly, we increase the
scalar quantity only when this is a new shared resource that is being
added to the `allocations`.


Diffs
-

  src/master/allocator/sorter/drf/sorter.hpp 
fee58d6d1f08163e2a06a4a20c891fe535c3dcff 


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


Testing
---

Tests passed.


Thanks,

Anindya Sinha

Re: Review Request 59177: CLI: Added Config class to manage the config file.

[Mesos Reviewbot]

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



Patch looks great!

Reviews applied: [59177]

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 May 11, 2017, 1:23 p.m., Armand Grillet wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59177/
> ---
> 
> (Updated May 11, 2017, 1:23 p.m.)
> 
> 
> Review request for mesos and Kevin Klues.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This new class simplifies the management of the configuration file
> given by the user; it loads the TOML file on initialization and
> has one method for each element that the user can set.
> 
> 
> Diffs
> -
> 
>   src/cli_new/bin/main.py 397c120eaaa8f21030dedb1ed552e65c704ee7da 
>   src/cli_new/bin/settings.py 0ef07cc67e8020be0424e939e5b19475fb998ac7 
>   src/cli_new/lib/cli/__init__.py f4fc3f18af5641a4a87143adaba81e62334ccffb 
>   src/cli_new/lib/cli/config.py PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/59177/diff/1/
> 
> 
> Testing
> ---
> 
> Tested manually, PEP8 and Pylint used to make sure that the code style is 
> correct.
> 
> 
> Thanks,
> 
> Armand Grillet
> 
>

Re: Review Request 55903: Update XFS disk isolator documentation.

[James Peach]

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

(Updated May 11, 2017, 5:55 p.m.)


Review request for mesos, Gilbert Song, Jie Yu, and Jiang Yan Xu.


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


Repository: mesos


Description
---

Update XFS disk isolator documentation.


Diffs (updated)
-

  docs/mesos-containerizer.md d0b9d762ab83bff1cf2bd98ed566376b7bd566c6 


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

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


Testing
---

None.


Thanks,

James Peach

Re: Review Request 55897: Add support for not enforcing XFS quotas.

[James Peach]

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

(Updated May 11, 2017, 5:55 p.m.)


Review request for mesos, Gilbert Song, Jie Yu, and Jiang Yan Xu.


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


Repository: mesos


Description
---

Add XFS disk isolator support for not enforcing disk quotas on
containers. While there is a global filesystem configuration option
to turn off quota enforcement, we should not automatically toggle
that because we don't know why the operator might have changed that
configuration. Instead, we just apply an unlimited (0) quota, which
engages XFS space accounting without enforcing any limit.


Diffs (updated)
-

  src/slave/containerizer/mesos/isolators/xfs/disk.hpp 
52f0459421a45b01ce38b17c689633301cd97982 
  src/slave/containerizer/mesos/isolators/xfs/disk.cpp 
40f1049358ad99d3f213289e36def81c580f07f3 
  src/slave/containerizer/mesos/isolators/xfs/utils.hpp 
eddd4c37fb42339ca21ecb392dea47acf6b277bb 
  src/tests/containerizer/xfs_quota_tests.cpp 
7beb60b059910a0d4451b1ace895a35dc974a043 


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

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


Testing
---

sudo make check (Fedora 25)


Thanks,

James Peach

Re: Review Request 58874: Stripped spaces between directory elements in WebUI.

[Tomasz Janiszewski]

> On Maj 11, 2017, 8:35 rano, Tomasz Janiszewski wrote:
> > src/webui/master/static/browse.html
> > Lines 17-20 (original), 17-20 (patched)
> > 
> >
> > How about usign `` here instead of list? Then `/` will be just 
> > visible text and then copyalbe whitouth hacks with zero sized text.
> > 
> > 
> > I haven't tested this but below code is what I think it could look like.
> > ```
> > 
> >   
> >  
> > {{dir}}
> >  
> >  /
> >   
> > 
> > ```

I've tested following code. It looks similar
```diff

diff --git a/src/webui/master/static/browse.html 
b/src/webui/master/static/browse.html
index b984919..0594f81 100644
--- a/src/webui/master/static/browse.html
+++ b/src/webui/master/static/browse.html
@@ -11,14 +11,15 @@
   
 

-
-  
-
+  
+
+  /
   {{dir}}
 
-  
-
+  
+

 
   ×
diff --git a/src/webui/master/static/css/mesos.css 
b/src/webui/master/static/css/mesos.css
index 9f3de54..21e3ef1 100644
--- a/src/webui/master/static/css/mesos.css
+++ b/src/webui/master/static/css/mesos.css
@@ -41,6 +41,11 @@
   content: "/";
 }

+.dir {
+  padding: 0 5px 0 0;
+  color: #ccc;
+}
+
 /*
  * /BOOTSTRAP OVERRIDES
  */

```


- Tomasz


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


On Maj 8, 2017, 8:01 rano, haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58874/
> ---
> 
> (Updated Maj 8, 2017, 8:01 rano)
> 
> 
> Review request for mesos, Benjamin Mahler and Tomasz Janiszewski.
> 
> 
> Bugs: MESOS-7468
> https://issues.apache.org/jira/browse/MESOS-7468
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Stripped spaces between directory elements in WebUI.
> 
> 
> Diffs
> -
> 
>   src/webui/master/static/browse.html 
> b9849197227b06df348789a49348e2b5d4cfd2ae 
>   src/webui/master/static/css/mesos.css 
> 9f3de5427071fc61d3791c4bc2a660368c2cd3c2 
> 
> 
> Diff: https://reviews.apache.org/r/58874/diff/3/
> 
> 
> Testing
> ---
> 
> 
> File Attachments
> 
> 
> strip_space.gif
>   
> https://reviews.apache.org/media/uploaded/files/2017/05/03/79afd0b1-5fb1-437c-a91c-732009af8fe3__strip_space.gif
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 56681: Use glog to log EXIT() messages.

[James Peach]

> On March 20, 2017, 8:32 p.m., Benjamin Mahler wrote:
> > Have you seen the feedback 
> > [here](https://issues.apache.org/jira/browse/MESOS-3465?focusedCommentId=14944383&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-14944383)?
> >  It would be nice to make it clear in the log that the program is exiting 
> > IMHO.
> 
> Jiang Yan Xu wrote:
> I think you meant https://issues.apache.org/jira/browse/MESOS-3465
> 
> Benjamin Mahler wrote:
> Yes, this comment: 
> https://issues.apache.org/jira/browse/MESOS-3465?focusedCommentId=14944383&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-14944383

I made this change.


- James


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


On May 11, 2017, 5:36 p.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56681/
> ---
> 
> (Updated May 11, 2017, 5:36 p.m.)
> 
> 
> Review request for mesos, haosdent huang and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-7115
> https://issues.apache.org/jira/browse/MESOS-7115
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Use glog to log EXIT() messages.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/exit.hpp 
> 762864afd8f86f7e1f439ef6ea7e3965a5f147d5 
> 
> 
> Diff: https://reviews.apache.org/r/56681/diff/3/
> 
> 
> Testing
> ---
> 
> make check (Fedora 25)
> 
> **Example output after this change:**
> ```
> [jpeach@jpeach build]$ sudo ./src/mesos-agent --work_dir=/nothing/here 
> --master=phoney:5050
> WARNING: Logging before InitGoogleLogging() is written to STDERR
> I0320 11:01:10.362782   921 main.cpp:368] Build: 2017-03-20 10:43:40 by jpeach
> I0320 11:01:10.362900   921 main.cpp:369] Version: 1.3.0
> I0320 11:01:10.362912   921 main.cpp:376] Git SHA: 
> b0d82241a886e9c6239ce82c5135c4c72c44aca7
> I0320 11:01:10.381340   921 systemd.cpp:238] systemd version `231` detected
> I0320 11:01:10.381404   921 main.cpp:478] Inializing systemd state
> I0320 11:01:10.390058   921 systemd.cpp:326] Started systemd slice 
> `mesos_executors.slice`
> I0320 11:01:10.392071   921 containerizer.cpp:221] Using isolation: 
> posix/cpu,posix/mem,filesystem/posix,network/cni
> I0320 11:01:10.401108   921 linux_launcher.cpp:150] Using 
> /sys/fs/cgroup/freezer as the freezer hierarchy for the Linux launcher
> I0320 11:01:10.402873   921 provisioner.cpp:249] Using default backend 
> 'overlay'
> E0320 11:01:10.426219   921 main.cpp:507] Failed to create a master detector: 
> Failed to parse 'phoney:5050'
> ```
> 
> **Example output before this change:**
> ```
> [jpeach@jpeach build]$ sudo /opt/mesos/sbin//mesos-agent 
> --work_dir=/nothing/here --master=phoney:5050
> WARNING: Logging before InitGoogleLogging() is written to STDERR
> I0320 11:03:43.868015  1188 main.cpp:243] Build: 2017-03-10 10:49:55 by jpeach
> I0320 11:03:43.868135  1188 main.cpp:244] Version: 1.3.0
> I0320 11:03:43.868151  1188 main.cpp:251] Git SHA: 
> 487c667260ec89127e18c77b9e8c8c243cf6ec57
> I0320 11:03:43.886561  1188 systemd.cpp:238] systemd version `231` detected
> I0320 11:03:43.886631  1188 main.cpp:342] Inializing systemd state
> I0320 11:03:43.896606  1188 systemd.cpp:326] Started systemd slice 
> `mesos_executors.slice`
> I0320 11:03:43.899202  1188 containerizer.cpp:221] Using isolation: 
> posix/cpu,posix/mem,filesystem/posix,network/cni
> I0320 11:03:43.911183  1188 linux_launcher.cpp:150] Using 
> /sys/fs/cgroup/freezer as the freezer hierarchy for the Linux launcher
> I0320 11:03:43.913216  1188 provisioner.cpp:249] Using default backend 
> 'overlay'
> Failed to create a master detector: Failed to parse 'phoney:5050'
> ```
> 
> 
> Thanks,
> 
> James Peach
> 
>

Re: Review Request 56680: Use the EXIT() macro more consistently in agent startup.

[James Peach]

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

(Updated May 11, 2017, 5:36 p.m.)


Review request for mesos, haosdent huang and Jiang Yan Xu.


Changes
---

Rebased.


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


Repository: mesos


Description
---

Use the EXIT() macro more consistently in agent startup.


Diffs (updated)
-

  src/slave/slave.cpp 209aff180db1a68ae360d7c278937fe645efdb2b 


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

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


Testing
---

make check (Fedora 25)


Thanks,

James Peach

Re: Review Request 56681: Use glog to log EXIT() messages.

[James Peach]

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

(Updated May 11, 2017, 5:36 p.m.)


Review request for mesos, haosdent huang and Jiang Yan Xu.


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


Repository: mesos


Description
---

Use glog to log EXIT() messages.


Diffs (updated)
-

  3rdparty/stout/include/stout/exit.hpp 
762864afd8f86f7e1f439ef6ea7e3965a5f147d5 


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

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


Testing
---

make check (Fedora 25)

**Example output after this change:**
```
[jpeach@jpeach build]$ sudo ./src/mesos-agent --work_dir=/nothing/here 
--master=phoney:5050
WARNING: Logging before InitGoogleLogging() is written to STDERR
I0320 11:01:10.362782   921 main.cpp:368] Build: 2017-03-20 10:43:40 by jpeach
I0320 11:01:10.362900   921 main.cpp:369] Version: 1.3.0
I0320 11:01:10.362912   921 main.cpp:376] Git SHA: 
b0d82241a886e9c6239ce82c5135c4c72c44aca7
I0320 11:01:10.381340   921 systemd.cpp:238] systemd version `231` detected
I0320 11:01:10.381404   921 main.cpp:478] Inializing systemd state
I0320 11:01:10.390058   921 systemd.cpp:326] Started systemd slice 
`mesos_executors.slice`
I0320 11:01:10.392071   921 containerizer.cpp:221] Using isolation: 
posix/cpu,posix/mem,filesystem/posix,network/cni
I0320 11:01:10.401108   921 linux_launcher.cpp:150] Using 
/sys/fs/cgroup/freezer as the freezer hierarchy for the Linux launcher
I0320 11:01:10.402873   921 provisioner.cpp:249] Using default backend 'overlay'
E0320 11:01:10.426219   921 main.cpp:507] Failed to create a master detector: 
Failed to parse 'phoney:5050'
```

**Example output before this change:**
```
[jpeach@jpeach build]$ sudo /opt/mesos/sbin//mesos-agent 
--work_dir=/nothing/here --master=phoney:5050
WARNING: Logging before InitGoogleLogging() is written to STDERR
I0320 11:03:43.868015  1188 main.cpp:243] Build: 2017-03-10 10:49:55 by jpeach
I0320 11:03:43.868135  1188 main.cpp:244] Version: 1.3.0
I0320 11:03:43.868151  1188 main.cpp:251] Git SHA: 
487c667260ec89127e18c77b9e8c8c243cf6ec57
I0320 11:03:43.886561  1188 systemd.cpp:238] systemd version `231` detected
I0320 11:03:43.886631  1188 main.cpp:342] Inializing systemd state
I0320 11:03:43.896606  1188 systemd.cpp:326] Started systemd slice 
`mesos_executors.slice`
I0320 11:03:43.899202  1188 containerizer.cpp:221] Using isolation: 
posix/cpu,posix/mem,filesystem/posix,network/cni
I0320 11:03:43.911183  1188 linux_launcher.cpp:150] Using 
/sys/fs/cgroup/freezer as the freezer hierarchy for the Linux launcher
I0320 11:03:43.913216  1188 provisioner.cpp:249] Using default backend 'overlay'
Failed to create a master detector: Failed to parse 'phoney:5050'
```


Thanks,

James Peach

Review Request 59191: Added another test case for Docker version parsing.

[Neil Conway]

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

Review request for mesos and Jie Yu.


Repository: mesos


Description
---

Added another test case for Docker version parsing.


Diffs
-

  src/tests/containerizer/docker_tests.cpp 
4660afcb759823f51800be8809ec0f07a3288c20 


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


Testing
---

`sudo ./src/mesos-tests --gtest_filter="DockerTest.*"`


Thanks,

Neil Conway

Review Request 59190: Adjusted test expectation for change to stout's Version.

[Neil Conway]

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

Review request for mesos and Jie Yu.


Repository: mesos


Description
---

Prerelease labels are now considered for equality between versions, per
SemVer spec.


Diffs
-

  src/tests/containerizer/docker_tests.cpp 
4660afcb759823f51800be8809ec0f07a3288c20 


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


Testing
---

`make check`

`sudo ./src/mesos-tests --gtest_filter="DockerTest.*"`


Thanks,

Neil Conway

Re: Review Request 59185: Add ambient capability support.

[James Peach]

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

(Updated May 11, 2017, 4:45 p.m.)


Review request for mesos, Benjamin Bannier and Jie Yu.


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


Repository: mesos


Description
---

In the absence of ambient capabilities, capabilities in the
effective set do not survive across execve(2). This means
that tasks attempting to make use of the LinuxInfo capability
support also need to ensure that file capabilities are set on
the file that is ultimately executed. Supporting ambient
capabilities allows the effective capabilities to survive
execve(2), so it is now possible to launch a task with limited
privilege elevations.


Diffs
-

  src/linux/capabilities.hpp 5fa3799948f8ac4bcaeaa89f91d7d090e426c5a6 
  src/linux/capabilities.cpp 7aa8c352def644468e8c9041a2fe4319f313b09b 
  src/slave/containerizer/mesos/launch.cpp 
2835beff9dbfa7f2a1cac306a58e2b1d66c14342 
  src/tests/containerizer/capabilities_tests.cpp 
15a85cab87c28402eeb2bfbc751c8c77bf4c14f5 


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


Testing
---

make check (Fedora 25)


Thanks,

James Peach

Review Request 59186: Additional linux/capabilities isolator documention.

[James Peach]

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

Review request for mesos, Benjamin Bannier and Jie Yu.


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


Repository: mesos


Description
---

Linux capabilities are somewhat involved, so add some additional
exposition of how the linux/capabilites isolator handles them and
why an operator might care.


Diffs
-

  docs/linux_capabilities.md b588aff6842a14bbf7ff5c35931cac61f9019805 


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


Testing
---

None.


Thanks,

James Peach

Review Request 59184: Add support for explicitly setting bounding capabilities.

[James Peach]

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

Review request for mesos, Benjamin Bannier and Jie Yu.


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


Repository: mesos


Description
---

The linux/capabilities isolator implemented the --allowed_capabilities
option by granting all the allowed capabilities. This depended on the
granted capabilities being removed across exec, leaving only the changes
to the bounding set in the child process.

This change explicitly populates the bounding set in the case where
--allowed_capabilities has been set and the task itself has not been
granted any capabilities. This improves the security of tasks since it
is now possible to configure the bounding set without potentially giving
privilege to the task.


Diffs
-

  include/mesos/slave/containerizer.proto 
41f2905df690bfe88ed762f1cd1246689fa4d3ea 
  src/launcher/executor.cpp b05f73e539c22d4d40f07df76168a06373b818d4 
  src/slave/containerizer/mesos/isolators/linux/capabilities.cpp 
60d22aa877c1ab62a08222e5efe8800e337684da 
  src/slave/containerizer/mesos/launch.cpp 
2835beff9dbfa7f2a1cac306a58e2b1d66c14342 
  src/tests/containerizer/linux_capabilities_isolator_tests.cpp 
f9d2a532bb5bef4654474cb171911952218780fa 


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


Testing
---

make check (Fedora 25)


Thanks,

James Peach

Review Request 59185: Add ambient capability support.

[James Peach]

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

Review request for mesos, Benjamin Bannier and Jie Yu.


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


Repository: mesos


Description
---

In the absence of ambient capabilities, capabilities in the
effective set do not survive across execve(2). This means
that tasks attempting to make use of the LinuxInfo capability
support also need to ensure that file capabilities are set on
the file that is ultimately executed. Supporting ambient
capabilities allows the effective capabilities to survive
execve(2), so it is now possible to launch a task with limited
privilege elevations.


Diffs
-

  src/linux/capabilities.hpp 5fa3799948f8ac4bcaeaa89f91d7d090e426c5a6 
  src/linux/capabilities.cpp 7aa8c352def644468e8c9041a2fe4319f313b09b 
  src/slave/containerizer/mesos/launch.cpp 
2835beff9dbfa7f2a1cac306a58e2b1d66c14342 
  src/tests/containerizer/capabilities_tests.cpp 
15a85cab87c28402eeb2bfbc751c8c77bf4c14f5 


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


Testing
---

make check (Fedora 25)


Thanks,

James Peach

Review Request 59183: Refactor setting capabilities into a helper function.

[James Peach]

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

Review request for mesos and Jie Yu.


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


Repository: mesos


Description
---

Setting the capabilities for the launched process will become a bit more
involved in subsequent commits so hoist it into a helper function with
no semantic changes to make the code review more obvious.


Diffs
-

  src/slave/containerizer/mesos/launch.cpp 
2835beff9dbfa7f2a1cac306a58e2b1d66c14342 


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


Testing
---

make check (Fedora 25).


Thanks,

James Peach

Re: Review Request 59074: Updated all tests that use Containerizer::launch(...).

[Jie Yu]

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


Fix it, then Ship it!





src/tests/hook_tests.cpp
Lines 303 (patched)


Why you need this?


- Jie Yu


On May 11, 2017, 12:27 a.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59074/
> ---
> 
> (Updated May 11, 2017, 12:27 a.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-7304 and MESOS-7449
> https://issues.apache.org/jira/browse/MESOS-7304
> https://issues.apache.org/jira/browse/MESOS-7449
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This sweeps through the unit tests and translates all existing
> tests that use the two variants of Containerizer::launch(...)
> (i.e. nested and non-nested) and translates the arguments to the
> new interface.  Some of the fetcher interface changes are also
> addressed in this sweep.
> 
> 
> Diffs
> -
> 
>   src/tests/api_tests.cpp faf3242f9c86d866c4bb5e457fcfe47c1063cc09 
>   src/tests/check_tests.cpp ec0d5ee78a94991c68f38174d09b41e8f2e4be35 
>   src/tests/command_executor_tests.cpp 
> da4b653303d436309f6cb190ad6b600b8d934678 
>   src/tests/container_logger_tests.cpp 
> 28436b6b67c1251d877064751e695c6696725a23 
>   src/tests/containerizer/cgroups_isolator_tests.cpp 
> 4e1d027bde7f0ab51e778bfd3bac8797fc7f7f1b 
>   src/tests/containerizer/cni_isolator_tests.cpp 
> 565e58ae75e918453e4386f5e35a5a844a8b15f8 
>   src/tests/containerizer/composing_containerizer_tests.cpp 
> d7fd6216080f2f437503b898bff6b1046317933e 
>   src/tests/containerizer/cpu_isolator_tests.cpp 
> 3c6f7481de2d6d1f6abf9850bd33cea44931480a 
>   src/tests/containerizer/docker_volume_isolator_tests.cpp 
> b47a6b5081a63ac474ac4634701b1a572eb58137 
>   src/tests/containerizer/isolator_tests.cpp 
> 355e15ff69ca6bdd94821f6566fd09a280d03b47 
>   src/tests/containerizer/linux_filesystem_isolator_tests.cpp 
> 70a0dce0f0166fcb3c15a03a89151152954abbac 
>   src/tests/containerizer/memory_isolator_tests.cpp 
> 0df4aa892b531eb305eb3012862ad25527344ab6 
>   src/tests/containerizer/memory_pressure_tests.cpp 
> c4ad779906ee31d07fdd8a6fc0e51e1edd1cbe85 
>   src/tests/containerizer/port_mapping_tests.cpp 
> a528382e8b4831b9c7e8dcc877a5e242909f0cd5 
>   src/tests/containerizer/volume_image_isolator_tests.cpp 
> ad1884459e87ece767e1019adcf702a2f2ff8f1c 
>   src/tests/containerizer/xfs_quota_tests.cpp 
> 7beb60b059910a0d4451b1ace895a35dc974a043 
>   src/tests/disk_quota_tests.cpp f758bee07c448afe09dc380e99c4a81e2dd9f650 
>   src/tests/health_check_tests.cpp 6c1b9a0ead6edb34c20cf4973fffcff913c5d7ad 
>   src/tests/hook_tests.cpp 02d8f800c3eb9b1e617a14c78c2ef1e45d1c72bb 
>   src/tests/master_tests.cpp ceee2f4a5d38e0f4200f444769e058d2173de821 
>   src/tests/slave_recovery_tests.cpp 52e78b6b6280a159233b402ce2849448204d4f11 
>   src/tests/slave_tests.cpp 8c97dc6d088708d301dc3ccf90d413fd785b782f 
> 
> 
> Diff: https://reviews.apache.org/r/59074/diff/2/
> 
> 
> Testing
> ---
> 
> make check
> 
> Currently fails a few of the fetcher tests, which might need tweaking (these 
> tests assume the fetcher caches things in the current directory):
> ```
> [  FAILED  ] FetcherTest.RelativeFilePath
> [  FAILED  ] FetcherCacheTest.SimpleEviction
> [  FAILED  ] FetcherCacheTest.FallbackFromEviction
> [  FAILED  ] FetcherCacheTest.RemoveLRUCacheEntries
> ```
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Re: Review Request 59164: Updated fetcher tests to reflect changed interfaces.

[Jie Yu]

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


Ship it!




Ship It!

- Jie Yu


On May 11, 2017, 12:27 a.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59164/
> ---
> 
> (Updated May 11, 2017, 12:27 a.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-7304
> https://issues.apache.org/jira/browse/MESOS-7304
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> There are two primary changes to the tests:
>   * The arguments passed to the fetcher's methods no longer contain
> the Flags or SlaveID.  So a couple of the mock expectations need
> to be tweaked.
>   * Flags are now passed into the fetcher at construction.  Some of
> the tests modify the flags after construction, so the order of
> operations needed to be flipped.
> 
> 
> Diffs
> -
> 
>   src/tests/fetcher_cache_tests.cpp 3bd63ed0a66493829a82c542ad05ebe0f7828d1a 
>   src/tests/fetcher_tests.cpp 27ea7244c713934ef5a721ec133804a006dcb26e 
> 
> 
> Diff: https://reviews.apache.org/r/59164/diff/1/
> 
> 
> Testing
> ---
> 
> See last patch in chain.
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Re: Review Request 59073: Updated Docker containerizer tests to use the old naming scheme.

[Jie Yu]

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


Ship it!




Ship It!

- Jie Yu


On May 11, 2017, 12:26 a.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59073/
> ---
> 
> (Updated May 11, 2017, 12:26 a.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-7449
> https://issues.apache.org/jira/browse/MESOS-7449
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This changes test expectations of Docker container names back to the
> 0.22 scheme and simultaneously updates the tests to the new
> containerizer interface.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/docker_containerizer_tests.cpp 
> 4eef399b05f6e35d75c7c23992f0ccda04576277 
> 
> 
> Diff: https://reviews.apache.org/r/59073/diff/3/
> 
> 
> Testing
> ---
> 
> See last patch in chain.
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Re: Review Request 58910: Addressed a TODO about checkpointing in tests.

[Jie Yu]

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


Ship it!




Ship It!

- Jie Yu


On May 11, 2017, 12:26 a.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58910/
> ---
> 
> (Updated May 11, 2017, 12:26 a.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-7449
> https://issues.apache.org/jira/browse/MESOS-7449
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Checkpointing only plays a role in tests when the containerizer
> is destroyed and subsequently recovered.  This commit updates a
> couple of test files to the new containerizer interface and
> also turns checkpointing _off_ in tests which do not perform any
> containerizer recovery.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/io_switchboard_tests.cpp 
> b19961ee91af3c174d4377c2514465d6c99cf331 
>   src/tests/containerizer/mesos_containerizer_tests.cpp 
> 13e0f7e603a3ffdd0965b253d7abfe6a069cd2b4 
>   src/tests/containerizer/nested_mesos_containerizer_tests.cpp 
> c163b882fb2fc463537d6906c5a47b24a9a756c4 
>   src/tests/containerizer/volume_sandbox_path_isolator_tests.cpp 
> 5f4e382baf80439b9a61f22c1912cadeba1109b7 
> 
> 
> Diff: https://reviews.apache.org/r/58910/diff/3/
> 
> 
> Testing
> ---
> 
> See last patch in chain.
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Re: Review Request 58909: Added test helpers to tranlate to ContainerConfig.

[Jie Yu]

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


Ship it!




Ship It!

- Jie Yu


On May 2, 2017, 2:17 a.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58909/
> ---
> 
> (Updated May 2, 2017, 2:17 a.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-7449
> https://issues.apache.org/jira/browse/MESOS-7449
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This adds a helper that emulates the logic in the Agent's launch path.
> Now that the Containerizer takes a ContainerConfig instead of
> a TaskInfo/ExecutorInfo/Directory/SlaveID/User, tests that call
> the containerizer directly will need to translate to
> ContainerConfig.
> 
> 
> Diffs
> -
> 
>   src/tests/mesos.hpp 3c57f25ff0bec55582b8a7decef53be61a9ec6f4 
> 
> 
> Diff: https://reviews.apache.org/r/58909/diff/1/
> 
> 
> Testing
> ---
> 
> See last patch in chain.
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Re: Review Request 58908: Updated test mocks per interface changes.

[Jie Yu]

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


Ship it!




Ship It!

- Jie Yu


On May 8, 2017, 10:45 p.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58908/
> ---
> 
> (Updated May 8, 2017, 10:45 p.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-7304 and MESOS-7449
> https://issues.apache.org/jira/browse/MESOS-7304
> https://issues.apache.org/jira/browse/MESOS-7449
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This updates the mocks for the containerizers and fetcher after
> the interface changes.  For the containerizers, the two launch methods
> have been combined into a single method.
> 
> The fetcher now has an argument in its constructor and no longer
> takes some arguments (SlaveID and Flags) in its methods.
> 
> Launching nested containers via the test containerizer was
> originally a no-op, so similar behavior is implemented in the
> combined method.
> 
> 
> Diffs
> -
> 
>   src/tests/cluster.cpp a4f57e0e9c4f221c42bdbb36d8925f688b3d742f 
>   src/tests/containerizer.hpp 63fe2366d5fe79c71cbe62813a9bb0b830e70057 
>   src/tests/containerizer.cpp 548da3a8757c9ad3ab115c6b91f8a1f2f5e37144 
>   src/tests/containerizer/mock_containerizer.hpp 
> ca0ae053b1eed01d6c04f581cd08485beec0c5fb 
>   src/tests/mesos.hpp 3c57f25ff0bec55582b8a7decef53be61a9ec6f4 
>   src/tests/mesos.cpp a79ec62d6b8354a97a50533dc66e04c1afc4bef6 
>   src/tests/mock_docker.hpp f58211d93e3231bfef7afa9e495930a70aae9f2b 
>   src/tests/mock_docker.cpp 81a14ca346f0fbe6458e03b06932e1ad00a4a238 
> 
> 
> Diff: https://reviews.apache.org/r/58908/diff/2/
> 
> 
> Testing
> ---
> 
> See last patch in chain.
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Re: Review Request 58907: Refactored Docker containerizer launch path per interface changes.

[Jie Yu]

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


Fix it, then Ship it!





src/slave/containerizer/docker.hpp
Lines 353-367 (original), 329 (patched)


Why remove the CHECK and the comment here?


- Jie Yu


On May 10, 2017, 6:33 p.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58907/
> ---
> 
> (Updated May 10, 2017, 6:33 p.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-7449
> https://issues.apache.org/jira/browse/MESOS-7449
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This continues the containerizer interface change that replaced
> most "Infos" (Task, Executor, Command, Container) with a single
> `ContainerConfig` and combined the nested/non-nested container
> launch paths.
> 
> Notably, this commit also changes the fields stored in the Docker
> containerizer to match the new interface.  The somewhat ambiguously
> named `directory` field has been renamed to `containerWorkDir`.
> And the copy of `Slave::flags` in each container has been removed
> because it is not used.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/docker.hpp 44efa44586455df416fc9cdc51c92bd9027f24c3 
>   src/slave/containerizer/docker.cpp 2480f595bee032ea399d642653d618472db0b7a7 
> 
> 
> Diff: https://reviews.apache.org/r/58907/diff/2/
> 
> 
> Testing
> ---
> 
> See last patch in chain.
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Re: Review Request 59146: Added a 'UNKNOWN' field to the Update.State enumeration.

[Benjamin Bannier]

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


Ship it!




Ship It!

- Benjamin Bannier


On May 10, 2017, 5:09 p.m., Jan Schlicht wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59146/
> ---
> 
> (Updated May 10, 2017, 5:09 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Benjamin Bannier, and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   include/mesos/resource_provider/resource_provider.proto 
> 11d01df114eafb729514dab64596c5f1ecf09f26 
>   include/mesos/v1/resource_provider/resource_provider.proto 
> 97a85ce34eeecf86d14c097712867cd24624a6ec 
> 
> 
> Diff: https://reviews.apache.org/r/59146/diff/2/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jan Schlicht
> 
>

Re: Review Request 58906: Removed extranous sandbox initialization in Docker containerizer.

[Jie Yu]

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




src/slave/containerizer/docker.cpp
Line 285 (original)


Hum, can you point to me where do we mkdir for the sandbox?


- Jie Yu


On May 2, 2017, 2:06 a.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58906/
> ---
> 
> (Updated May 2, 2017, 2:06 a.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-7449
> https://issues.apache.org/jira/browse/MESOS-7449
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The Docker containerizer will create a stdout/stderr file as well
> `chown` the sandbox directory to the user specified at container
> creation time.  This logic is already performed elsewhere.
> 
> The stdout/stderr files will either be created prior to the container
> launch via the mesos-fetcher, or during launch via the ContainerLogger.
> 
> The sandbox directory will be `chown`d by the Agent when it
> starts to launch the executor.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/docker.cpp 2480f595bee032ea399d642653d618472db0b7a7 
> 
> 
> Diff: https://reviews.apache.org/r/58906/diff/1/
> 
> 
> Testing
> ---
> 
> See last patch in chain.
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Review Request 59177: CLI: Added Config class to manage the config file.

[Armand Grillet]

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

Review request for mesos and Kevin Klues.


Repository: mesos


Description
---

This new class simplifies the management of the configuration file
given by the user; it loads the TOML file on initialization and
has one method for each element that the user can set.


Diffs
-

  src/cli_new/bin/main.py 397c120eaaa8f21030dedb1ed552e65c704ee7da 
  src/cli_new/bin/settings.py 0ef07cc67e8020be0424e939e5b19475fb998ac7 
  src/cli_new/lib/cli/__init__.py f4fc3f18af5641a4a87143adaba81e62334ccffb 
  src/cli_new/lib/cli/config.py PRE-CREATION 


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


Testing
---

Tested manually, PEP8 and Pylint used to make sure that the code style is 
correct.


Thanks,

Armand Grillet

Re: Review Request 58905: Changed naming of Docker containers to the pre-0.23 scheme.

[Jie Yu]

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




src/slave/containerizer/docker.hpp
Line 318 (original), 319 (patched)


Have you considered the case where we can call 'usage' or 'update' after 
upgrade. Will the new code be able to handle old containers given the naming 
scheme has been changed?


- Jie Yu


On May 2, 2017, 2:05 a.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58905/
> ---
> 
> (Updated May 2, 2017, 2:05 a.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-7449
> https://issues.apache.org/jira/browse/MESOS-7449
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The choice of naming scheme is cosmetic.  All versions of the Docker
> containerizer will consider containers starting with "mesos-"
> as containers started by Mesos.  The Docker containerizer does not
> consider the `SlaveID` at all when recovering.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/docker.hpp 44efa44586455df416fc9cdc51c92bd9027f24c3 
>   src/slave/containerizer/docker.cpp 2480f595bee032ea399d642653d618472db0b7a7 
> 
> 
> Diff: https://reviews.apache.org/r/58905/diff/1/
> 
> 
> Testing
> ---
> 
> See last patch in chain.
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Re: Review Request 58904: Combined launch paths in ComposingContainerizer.

[Jie Yu]

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


Fix it, then Ship it!





src/slave/containerizer/composing.cpp
Lines 439 (patched)


YOu can use 'auto' here.


- Jie Yu


On May 2, 2017, 2:05 a.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58904/
> ---
> 
> (Updated May 2, 2017, 2:05 a.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-7449
> https://issues.apache.org/jira/browse/MESOS-7449
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This eliminates one of the entrypoints for launching a container
> in the `ComposingContainerizer`.  Nested and non-nested containers
> are still launched the same way, where nested containers get launched
> using their root container's containerizer.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/composing.hpp 
> 8e04bfeb76607e87ee336f4a7f519eac08b7d44c 
>   src/slave/containerizer/composing.cpp 
> 0b6c76b3d081d86df81a6062ae7a191ba8dadfde 
> 
> 
> Diff: https://reviews.apache.org/r/58904/diff/1/
> 
> 
> Testing
> ---
> 
> See last patch in chain.
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Re: Review Request 58903: Combined Mesos containerizer's launch methods.

[Jie Yu]

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


Fix it, then Ship it!





src/slave/containerizer/mesos/containerizer.cpp
Lines 984-986 (original), 931-933 (patched)


In fact, I would call the parameter `_containerConfig` here:

```
ContainerConfig containerConfig = _containerConfig;
```

And moving forward, consistently use `containerConfig` in this function.



src/slave/containerizer/mesos/containerizer.cpp
Line 1066 (original), 1037 (patched)


It's weird that you still `containerConfig` not `containerConfig_` here.

See my comments above.


- Jie Yu


On May 2, 2017, 2:04 a.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58903/
> ---
> 
> (Updated May 2, 2017, 2:04 a.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-7449
> https://issues.apache.org/jira/browse/MESOS-7449
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This simplifies the container launch path by removing combining
> the nested and non-nested container code paths into one.
> 
> The Mesos containerizer was originally translating the two
> `containerizer->launch` entrypoints into a common method (also
> called `launch`).  The previous commits moved this translation
> logic into the caller (i.e. the Agent).
> 
> The end result has some slight changes:
>   * It is now possible for the Agent to specify some more combinations
> of `ContainerConfig`.  For example, specifying a TaskInfo with
> a DEBUG-class container.  Or a nested container with Resources.
> We may need to add extra validation around this.
>   * The `bool checkpoint` argument was replaced with a `Option`
> which optionally contains an absolute path.  This allows us to
> remove the `SlaveID` field.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/containerizer.hpp 
> 29a99f33e646593127b9dc126f398f7bca88e21d 
>   src/slave/containerizer/mesos/containerizer.cpp 
> b58baed64480e22f640a4852537f85922ed382ae 
> 
> 
> Diff: https://reviews.apache.org/r/58903/diff/1/
> 
> 
> Testing
> ---
> 
> See last patch in chain.
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Re: Review Request 58902: Changed containerizer->launch callsites to new interface.

[Jie Yu]

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


Ship it!




Ship It!

- Jie Yu


On May 2, 2017, 2:02 a.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58902/
> ---
> 
> (Updated May 2, 2017, 2:02 a.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-7449
> https://issues.apache.org/jira/browse/MESOS-7449
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> As part of combining the two `containerizer->launch` code paths,
> callers now need to construct the `ContainerConfig` (instead of
> passing some fields that are then copied into a `ContainerConfig`).
> 
> 
> Diffs
> -
> 
>   src/slave/http.cpp 2ce345d9958a867ece835e47287215cefd561abf 
>   src/slave/slave.cpp 8b8078dbb656e9db2efa53cc4ec5bed2cc01d49a 
> 
> 
> Diff: https://reviews.apache.org/r/58902/diff/1/
> 
> 
> Testing
> ---
> 
> See last patch in chain.
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Re: Review Request 58899: Combined containerizer interface's launch methods.

[Jie Yu]

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


Fix it, then Ship it!





src/slave/containerizer/containerizer.hpp
Lines 85-86 (original), 85-86 (patched)


Can we make the field in the protobuf optional and add this note there as 
well?


- Jie Yu


On May 2, 2017, 2:01 a.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58899/
> ---
> 
> (Updated May 2, 2017, 2:01 a.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-7449
> https://issues.apache.org/jira/browse/MESOS-7449
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> When nested container support was added, we added a separate `launch`
> path in the containerizer because nested containers do not need
> an explicit TaskInfo/ExecutorInfo.  Nested containers basically
> only need the CommandInfo and ContainerInfo.
> 
> This commit combines the two launch methods by replacing most of the
> "Infos" (Task, Executor, Command, Container) with a `ContainerConfig`
> argument, which may contain multiple combinations of the "Infos".
> 
> The goal is to support three launch paths for containers:
>   1) When the `ContainerConfig` contains a TaskInfo/ExecutorInfo,
>  launch a task or executor.
>   2) When the `ContainerID` has a parent, launch a nested container.
>   3) (Not implemented yet) When there is no TaskInfo/ExecutorInfo or
>  parent container, launch a standalone container.
>  
> There are two other notable changes to the interface:
>   * The `SlaveID` field has been removed entirely.  The code that
> requires this (in the fetcher and Docker containerizer) will be
> addressed in a separate commit.
>   * The `checkpoint` bool has been replaced by an Option,
> which contains the path that should be used for checkpointing.
> This path includes the filename.
> This is also one of the reasons why `SlaveID` was an argument.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/containerizer.hpp 
> 4c31a1f5c853c1dc66480c7b4c867a87a1bb5c41 
> 
> 
> Diff: https://reviews.apache.org/r/58899/diff/1/
> 
> 
> Testing
> ---
> 
> See last patch in chain.
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Re: Review Request 58967: Set the working directory to parent task's for DEBUG containers.

[Alexander Rukletsov]

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

(Updated May 11, 2017, 10:07 a.m.)


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


Changes
---

Removed unicode char.


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


Repository: mesos


Description
---

A DEBUG container's working directory should be set to the parent
task's working directory. For the command executor case, even if
the task itself has a root filesystem, the executor container still
uses the host filesystem, hence
`ContainerLaunchInfo.working_directory`, pointing to the executor's
working directory in the host filesystem, may be different from the
task's working directory in the root filesystem.


Diffs (updated)
-

  include/mesos/slave/containerizer.proto 
41f2905df690bfe88ed762f1cd1246689fa4d3ea 
  src/slave/containerizer/mesos/containerizer.cpp 
dd3a0705d98c75078c1c363317def5d801844e4a 


Diff: https://reviews.apache.org/r/58967/diff/4/

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


Testing
---

make check on Mac OS and Fedora 24


Thanks,

Alexander Rukletsov

Re: Review Request 58900: Changed the default fetcher cache directory.

[Jie Yu]

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


Ship it!




Ship It!

- Jie Yu


On May 10, 2017, 8:32 p.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58900/
> ---
> 
> (Updated May 10, 2017, 8:32 p.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-7304
> https://issues.apache.org/jira/browse/MESOS-7304
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The fetcher cache directory was historically located (by default)
> in `/tmp/mesos/fetch`.  The agent flag `--fetcher_cache_dir` could
> be used to change this value.
> 
> The fetcher would create a subdirectory underneath `/tmp/mesos/fetch`
> for each `SlaveID`.  This was done because multiple agents can run on
> the same node.  If all the agents use the same default fetcher cache
> directory, they will collide and cause unpredictable results.
> As a result, the `SlaveID` needed to be passed into the fetcher
> after the agent recovers and/or registers with the master, because
> that is when the `SlaveID` is determined.
> 
> This changes the default fetcher cache directory to
> `/tmp/mesos/fetch`.  The `SlaveID` subdirectory has been removed.
> 
> This change, while techically a breaking change, is safe because of
> how the fetcher uses this directory.  Upon starting up, the fetcher
> "recovers" by clearing this directory.  Although the subdirectory
> has been removed, the fetcher still clears the fetcher cache
> on startup.
> 
> This change will only cause breakages if multiple agents are run
> with the same `--fetcher_cache_dir`.  In this case, each agent
> will delete the fetcher caches of all the other agents.
> 
> ---
> 
> With the removal of the `SlaveID` field in the fetcher's methods,
> it is no longer necessary to pass in the `SlaveID` or agent Flags
> at agent recovery time.  Instead, the flags can be passed in during
> the fetcher's construction.
> 
> Similarly, the fetcher's "recovery" (clearing the fetcher cache)
> can be done immediately upon construction, which simplifies the
> code slightly.
> 
> 
> Diffs
> -
> 
>   src/local/local.cpp e47980929db2da1f31cf899a0e1fc452070e11f3 
>   src/slave/containerizer/fetcher.hpp 
> 9e3018dc087ed55c61b2824d0105bc5339b83043 
>   src/slave/containerizer/fetcher.cpp 
> a910fea5a5556afb376524c5bb2ff98d7d84e611 
>   src/slave/flags.cpp bc63a6a4cb6115b4b4d592e67e34045f52b50d4c 
>   src/slave/main.cpp 507d59996a90f51c7cd1e0f836a34c2e9c484791 
>   src/slave/slave.cpp 209aff180db1a68ae360d7c278937fe645efdb2b 
> 
> 
> Diff: https://reviews.apache.org/r/58900/diff/5/
> 
> 
> Testing
> ---
> 
> See last patch in chain.
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Re: Review Request 58900: Changed the default fetcher cache directory.

[Jie Yu]

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



Great clean up!


src/local/local.cpp
Lines 372-374 (patched)


I'd suggest we put fetcher dir under work_dir as well for local mode so 
that all directories related to this local run have a common parent directory.

```
propagatedFlags["work_dir"] =
  path::join(flags.work_dir, "agents", stringify(i), "work_dir");
  
propagatedFlags["runtime_dir"] =
  path::join(flags.work_dir, "agents", stringify(i), "runtime_dir");
  
propagatedFlags["fetcher_cache_dir"] =
  path::join(flags.work_dir, "agents", stringify(i), "fetcher_cache_dir");
```

No need for the `runtime_dir` in local flags.


- Jie Yu


On May 10, 2017, 8:32 p.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58900/
> ---
> 
> (Updated May 10, 2017, 8:32 p.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-7304
> https://issues.apache.org/jira/browse/MESOS-7304
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The fetcher cache directory was historically located (by default)
> in `/tmp/mesos/fetch`.  The agent flag `--fetcher_cache_dir` could
> be used to change this value.
> 
> The fetcher would create a subdirectory underneath `/tmp/mesos/fetch`
> for each `SlaveID`.  This was done because multiple agents can run on
> the same node.  If all the agents use the same default fetcher cache
> directory, they will collide and cause unpredictable results.
> As a result, the `SlaveID` needed to be passed into the fetcher
> after the agent recovers and/or registers with the master, because
> that is when the `SlaveID` is determined.
> 
> This changes the default fetcher cache directory to
> `/tmp/mesos/fetch`.  The `SlaveID` subdirectory has been removed.
> 
> This change, while techically a breaking change, is safe because of
> how the fetcher uses this directory.  Upon starting up, the fetcher
> "recovers" by clearing this directory.  Although the subdirectory
> has been removed, the fetcher still clears the fetcher cache
> on startup.
> 
> This change will only cause breakages if multiple agents are run
> with the same `--fetcher_cache_dir`.  In this case, each agent
> will delete the fetcher caches of all the other agents.
> 
> ---
> 
> With the removal of the `SlaveID` field in the fetcher's methods,
> it is no longer necessary to pass in the `SlaveID` or agent Flags
> at agent recovery time.  Instead, the flags can be passed in during
> the fetcher's construction.
> 
> Similarly, the fetcher's "recovery" (clearing the fetcher cache)
> can be done immediately upon construction, which simplifies the
> code slightly.
> 
> 
> Diffs
> -
> 
>   src/local/local.cpp e47980929db2da1f31cf899a0e1fc452070e11f3 
>   src/slave/containerizer/fetcher.hpp 
> 9e3018dc087ed55c61b2824d0105bc5339b83043 
>   src/slave/containerizer/fetcher.cpp 
> a910fea5a5556afb376524c5bb2ff98d7d84e611 
>   src/slave/flags.cpp bc63a6a4cb6115b4b4d592e67e34045f52b50d4c 
>   src/slave/main.cpp 507d59996a90f51c7cd1e0f836a34c2e9c484791 
>   src/slave/slave.cpp 209aff180db1a68ae360d7c278937fe645efdb2b 
> 
> 
> Diff: https://reviews.apache.org/r/58900/diff/5/
> 
> 
> Testing
> ---
> 
> See last patch in chain.
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Re: Review Request 58967: Set the working directory to parent task's for DEBUG containers.

[Alexander Rukletsov]

> On May 10, 2017, 9:58 p.m., Vinod Kone wrote:
> > include/mesos/slave/containerizer.proto
> > Line 194 (original), 194 (patched)
> > 
> >
> > Looks like some random unicode char at the end?

Apologies, this is a letter from a belarusian layout that lives on the same key 
with english '.'.


- Alexander


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


On May 10, 2017, 1:56 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58967/
> ---
> 
> (Updated May 10, 2017, 1:56 p.m.)
> 
> 
> Review request for mesos, Gastón Kleiman, Jie Yu, and Vinod Kone.
> 
> 
> Bugs: MESOS-7433
> https://issues.apache.org/jira/browse/MESOS-7433
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> A DEBUG container's working directory should be set to the parent
> task's working directory. For the command executor case, even if
> the task itself has a root filesystem, the executor container still
> uses the host filesystem, hence
> `ContainerLaunchInfo.working_directory`, pointing to the executor's
> working directory in the host filesystem, may be different from the
> task's working directory in the root filesystem.
> 
> 
> Diffs
> -
> 
>   include/mesos/slave/containerizer.proto 
> 41f2905df690bfe88ed762f1cd1246689fa4d3ea 
>   src/slave/containerizer/mesos/containerizer.cpp 
> 3d724945812c0359ed175ce232f70886dc4401c8 
> 
> 
> Diff: https://reviews.apache.org/r/58967/diff/3/
> 
> 
> Testing
> ---
> 
> make check on Mac OS and Fedora 24
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 58874: Stripped spaces between directory elements in WebUI.

[Tomasz Janiszewski]

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




src/webui/master/static/browse.html
Lines 17-20 (original), 17-20 (patched)


How about usign `` here instead of list? Then `/` will be just 
visible text and then copyalbe whitouth hacks with zero sized text.

I haven't tested this but below code is what I think it could look like.
```

  
 
{{dir}}
 
 /
  

```


- Tomasz Janiszewski


On Maj 8, 2017, 8:01 rano, haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58874/
> ---
> 
> (Updated Maj 8, 2017, 8:01 rano)
> 
> 
> Review request for mesos, Benjamin Mahler and Tomasz Janiszewski.
> 
> 
> Bugs: MESOS-7468
> https://issues.apache.org/jira/browse/MESOS-7468
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Stripped spaces between directory elements in WebUI.
> 
> 
> Diffs
> -
> 
>   src/webui/master/static/browse.html 
> b9849197227b06df348789a49348e2b5d4cfd2ae 
>   src/webui/master/static/css/mesos.css 
> 9f3de5427071fc61d3791c4bc2a660368c2cd3c2 
> 
> 
> Diff: https://reviews.apache.org/r/58874/diff/3/
> 
> 
> Testing
> ---
> 
> 
> File Attachments
> 
> 
> strip_space.gif
>   
> https://reviews.apache.org/media/uploaded/files/2017/05/03/79afd0b1-5fb1-437c-a91c-732009af8fe3__strip_space.gif
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 56680: Use the EXIT() macro more consistently in agent startup.

[Jiang Yan Xu]

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


Ship it!




Ship It!

- Jiang Yan Xu


On May 8, 2017, 4:29 p.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56680/
> ---
> 
> (Updated May 8, 2017, 4:29 p.m.)
> 
> 
> Review request for mesos, haosdent huang and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-7115
> https://issues.apache.org/jira/browse/MESOS-7115
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Use the EXIT() macro more consistently in agent startup.
> 
> 
> Diffs
> -
> 
>   src/slave/slave.cpp a4f4a9ca80b726de8e07571fd6d93120947c278b 
> 
> 
> Diff: https://reviews.apache.org/r/56680/diff/2/
> 
> 
> Testing
> ---
> 
> make check (Fedora 25)
> 
> 
> Thanks,
> 
> James Peach
> 
>

Re: Review Request 59001: Added volume secret isolator.

[Jie Yu]

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




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


Can you do `-n` to avoid changing mtab?


- Jie Yu


On May 9, 2017, 6:11 p.m., Kapil Arya wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59001/
> ---
> 
> (Updated May 9, 2017, 6:11 p.m.)
> 
> 
> Review request for mesos, Gilbert Song, Jie Yu, and Vinod Kone.
> 
> 
> Bugs: MESOS-7418
> https://issues.apache.org/jira/browse/MESOS-7418
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added volume secret isolator.
> 
> 
> Diffs
> -
> 
>   src/CMakeLists.txt 40d921ee7026f5ac47efbf0243e1cafab57825f9 
>   src/Makefile.am 6bb81fd49b4564a0afa993b2cef6baa9d370ee7a 
>   src/slave/containerizer/mesos/containerizer.cpp 
> 58ab74571fb14c6dbb1907151dc421f93e324bb5 
>   src/slave/containerizer/mesos/isolators/volume/secret.hpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/volume/secret.cpp PRE-CREATION 
>   src/tests/CMakeLists.txt 9f2af9cdd1cf50485f4cd84ce67bcceba64b9328 
>   src/tests/containerizer/rootfs.cpp fdfecc65a3fcd19d6a4dfa574320f4d1f2755322 
>   src/tests/containerizer/volume_secret_isolator_tests.cpp PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/59001/diff/6/
> 
> 
> Testing
> ---
> 
> Added new tests an ran `make check`.
> 
> 
> Thanks,
> 
> Kapil Arya
> 
>