Re: Review Request 51505: Replace http::get with http::request in mesos project.

[Yongqiao Wang]

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

(Updated Aug. 30, 2016, 5:36 a.m.)


Review request for mesos.


Changes
---

Code refactor.


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


Repository: mesos


Description
---

Replace http::get with http::request in mesos project.


Diffs (updated)
-

  src/tests/containerizer/docker_containerizer_tests.cpp 
28cd3fa66886dbdbae3fdeca77707147faafcb7a 
  src/tests/executor_http_api_tests.cpp 
fd322ee267b2f0d4bf250db15027d61f1fc74a57 
  src/tests/fault_tolerance_tests.cpp 5a9944cf459ab688907d95bbda09f464b37efd1e 
  src/tests/files_tests.cpp 78719da5938ff0de722f8411174817f708fd41f2 
  src/tests/gc_tests.cpp f3aec4a95ff231bed1283ef1408101c012cadea1 
  src/tests/health_check_tests.cpp 50a252b5267cbc3b21ddc75e1da86c4975f6faf1 
  src/tests/logging_tests.cpp 886dcd0d6d6ccf509c98578e8b86b1c6ecd5775f 
  src/tests/master_authorization_tests.cpp 
a6399f1b958705b87418b40d3771b25a37d49682 
  src/tests/master_maintenance_tests.cpp 
0820e637b0da69ebcfc8776c223c3bc8a4f1bc94 
  src/tests/master_quota_tests.cpp 48be7406181646c8cc1d169b82a4a4ca71cdf03b 
  src/tests/master_tests.cpp 6cde15fcd6ca8ec40438c75aed980e83f8de9b86 
  src/tests/metrics_tests.cpp 27fc35f3bc47fc2905f4ce400b1051307dcacb2e 
  src/tests/persistent_volume_endpoints_tests.cpp 
266c2a0ff4a99baa96a7c4980f076755603256a9 
  src/tests/registrar_tests.cpp b04fc92c66449f1094212a534899e16d5f6f01bd 
  src/tests/role_tests.cpp 162c9414a70723a212cfd39ac5c0b8325c3f3b5d 
  src/tests/scheduler_driver_tests.cpp faf2e6c8ad17e07964b4340d0b340654b03f9086 
  src/tests/scheduler_http_api_tests.cpp 
80a2ef0af9a4c67deaef40e1f36343868ee4428f 
  src/tests/slave_authorization_tests.cpp 
6f120b6ab5d86c3737e979cdcdb5b6cca008d1ff 
  src/tests/slave_tests.cpp 701fab20091f9f7df020290b9f0a9db1dec92647 
  src/tests/status_update_manager_tests.cpp 
38d8913a5b33aa5325d0bc632c0a1d80480eddf8 
  src/tests/utils.cpp cc5259a720bb6451714a100b0451b473395e3650 

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


Testing
---

make & make check successfully!


Thanks,

Yongqiao Wang

Re: Review Request 51505: Replace http::get with http::request in mesos project.

[Yongqiao Wang]

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

(Updated Aug. 30, 2016, 5:28 a.m.)


Review request for mesos.


Summary (updated)
-

Replace http::get with http::request in mesos project.


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


Repository: mesos


Description (updated)
---

Replace http::get with http::request in mesos project.


Diffs (updated)
-

  src/tests/containerizer/docker_containerizer_tests.cpp 
28cd3fa66886dbdbae3fdeca77707147faafcb7a 
  src/tests/executor_http_api_tests.cpp 
fd322ee267b2f0d4bf250db15027d61f1fc74a57 
  src/tests/fault_tolerance_tests.cpp 5a9944cf459ab688907d95bbda09f464b37efd1e 
  src/tests/files_tests.cpp 78719da5938ff0de722f8411174817f708fd41f2 
  src/tests/gc_tests.cpp f3aec4a95ff231bed1283ef1408101c012cadea1 
  src/tests/health_check_tests.cpp 50a252b5267cbc3b21ddc75e1da86c4975f6faf1 
  src/tests/logging_tests.cpp 886dcd0d6d6ccf509c98578e8b86b1c6ecd5775f 
  src/tests/master_authorization_tests.cpp 
a6399f1b958705b87418b40d3771b25a37d49682 
  src/tests/master_maintenance_tests.cpp 
0820e637b0da69ebcfc8776c223c3bc8a4f1bc94 
  src/tests/master_quota_tests.cpp 48be7406181646c8cc1d169b82a4a4ca71cdf03b 
  src/tests/master_tests.cpp 6cde15fcd6ca8ec40438c75aed980e83f8de9b86 
  src/tests/metrics_tests.cpp 27fc35f3bc47fc2905f4ce400b1051307dcacb2e 
  src/tests/persistent_volume_endpoints_tests.cpp 
266c2a0ff4a99baa96a7c4980f076755603256a9 
  src/tests/registrar_tests.cpp b04fc92c66449f1094212a534899e16d5f6f01bd 
  src/tests/role_tests.cpp 162c9414a70723a212cfd39ac5c0b8325c3f3b5d 
  src/tests/scheduler_driver_tests.cpp faf2e6c8ad17e07964b4340d0b340654b03f9086 
  src/tests/scheduler_http_api_tests.cpp 
80a2ef0af9a4c67deaef40e1f36343868ee4428f 
  src/tests/slave_authorization_tests.cpp 
6f120b6ab5d86c3737e979cdcdb5b6cca008d1ff 
  src/tests/slave_tests.cpp 701fab20091f9f7df020290b9f0a9db1dec92647 
  src/tests/status_update_manager_tests.cpp 
38d8913a5b33aa5325d0bc632c0a1d80480eddf8 
  src/tests/utils.cpp cc5259a720bb6451714a100b0451b473395e3650 

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


Testing
---

make & make check successfully!


Thanks,

Yongqiao Wang

Re: Review Request 51495: Replace http::get with http::request in libprocess project.

[Yongqiao Wang]

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

(Updated Aug. 30, 2016, 5:27 a.m.)


Review request for mesos.


Summary (updated)
-

Replace http::get with http::request in libprocess project.


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


Repository: mesos


Description (updated)
---

Replace http::get with http::request in libprocess project.


Diffs (updated)
-

  3rdparty/libprocess/src/tests/benchmarks.cpp 
945007c38f96a66212f5ebdd700a117a4902a736 
  3rdparty/libprocess/src/tests/http_tests.cpp 
24b266df5f17e28e0c95326f5d1ea451952500e8 
  3rdparty/libprocess/src/tests/metrics_tests.cpp 
88526e67a20b7b58a6e14034215faa3ae9879fd0 
  3rdparty/libprocess/src/tests/process_tests.cpp 
b9feec7e34cffe19e49035f8865b150f79258f54 
  3rdparty/libprocess/src/tests/profiler_tests.cpp 
995bd02f6ecce484cd9b2aca355c2707d73d40b2 
  3rdparty/libprocess/src/tests/system_tests.cpp 
0f4d0424689522337806ba2227ec4330c700e17e 

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


Testing
---

make & make check successfully!


Thanks,

Yongqiao Wang

Re: Review Request 51096: Added the `mesos-cni-port-mapper` binary.

[Avinash sridharan]

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

(Updated Aug. 30, 2016, 3:54 a.m.)


Review request for mesos, Jie Yu and Qian Zhang.


Changes
---

Address Qian's comments.


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


Repository: mesos


Description
---

The `mesos-cni-port-mapper` is a CNI plugin that can be used to install
port-forwarding rules for a container. The `mesos-cni-port-mapper` is a
wrapper CNI plugin that should always be used in conjunction with
other CNI plugins.


Diffs (updated)
-

  src/Makefile.am 120a715eeeb7fb222d1169500950b5c7643df554 
  
src/slave/containerizer/mesos/isolators/network/cni/plugins/port_mapper/main.cpp
 PRE-CREATION 

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


Testing
---

make


Thanks,

Avinash sridharan

Re: Review Request 51511: Added check in FileEncoder's destructor.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [51509, 51511]

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 Aug. 30, 2016, 1 a.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51511/
> ---
> 
> (Updated Aug. 30, 2016, 1 a.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Artem Harutyunyan, and Joris Van 
> Remoortere.
> 
> 
> Bugs: MESOS-6104
> https://issues.apache.org/jira/browse/MESOS-6104
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This check will force libprocess to fail fast if a file descriptor
> is closed underneath it.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/encoder.hpp 
> 9cd0d3f55f2e9c9dc9ebb97b34a23375f8d4e07f 
> 
> Diff: https://reviews.apache.org/r/51511/diff/
> 
> 
> Testing
> ---
> 
> Re-ran the steps in the previous review for several minutes.  Saw that the 
> master did not CHECK fail.
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Review Request 51511: Added check in FileEncoder's destructor.

[Joseph Wu]

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

Review request for mesos, Benjamin Mahler, Artem Harutyunyan, and Joris Van 
Remoortere.


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


Repository: mesos


Description
---

This check will force libprocess to fail fast if a file descriptor
is closed underneath it.


Diffs
-

  3rdparty/libprocess/src/encoder.hpp 9cd0d3f55f2e9c9dc9ebb97b34a23375f8d4e07f 

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


Testing
---

Re-ran the steps in the previous review for several minutes.  Saw that the 
master did not CHECK fail.


Thanks,

Joseph Wu

Re: Review Request 51509: Fixed potential FD double close in the libevent socket.

[Joseph Wu]

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

(Updated Aug. 29, 2016, 5:51 p.m.)


Review request for mesos, Benjamin Mahler, Artem Harutyunyan, and Joris Van 
Remoortere.


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


Repository: mesos


Description
---

`evbuffer_add_file` will take ownership of the file descriptor passed
into it.  Normally, this file descriptor is owned by the `FileEncoder`
in the libprocess's `SocketManager`.  Since there are two owners, one
of the owners may close the file descriptor when it has been re-used.

In this case, when multiple threads access the master's web UI at once
with SSL enabled, the master may CHECK-fail due to a bad (closed)
file descriptor.


Diffs
-

  3rdparty/libprocess/src/libevent_ssl_socket.cpp 
99a12b81eea9c37278b6db0bfedf4b151ff8ed50 

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


Testing
---

The master will CHECK fail under the following:

1) Paste lots of text (16KB or more) of text into 
`src/webui/master/static/home.html`.  The more text, the more reliable the 
repro.

2) Start the master with SSL enabled:
```
LIBPROCESS_SSL_ENABLED=true LIBPROCESS_SSL_KEY_FILE=key.pem 
LIBPROCESS_SSL_CERT_FILE=cert.pem bin/mesos-master.sh --work_dir=/tmp/master
```

3) Run two instances of this python script repeatedly:
```
import socket
import ssl

s = ssl.wrap_socket(socket.socket())
s.connect(("localhost", 5050))

s.sendall("""GET /static/home.html HTTP/1.1
User-Agent: foobar
Host: localhost:5050
Accept: */*
Connection: Keep-Alive

""")

# The HTTP part of the response
print s.recv(1000)
```

i.e. 
```
while python test.py; do :; done & while python test.py; do :; done
```


Thanks,

Joseph Wu

Review Request 51509: Fixed potential FD double close in the libevent socket.

[Joseph Wu]

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

Review request for mesos, Benjamin Mahler, Artem Harutyunyan, and Joris Van 
Remoortere.


Repository: mesos


Description
---

`evbuffer_add_file` will take ownership of the file descriptor passed
into it.  Normally, this file descriptor is owned by the `FileEncoder`
in the libprocess's `SocketManager`.  Since there are two owners, one
of the owners may close the file descriptor when it has been re-used.

In this case, when multiple threads access the master's web UI at once
with SSL enabled, the master may CHECK-fail due to a bad (closed)
file descriptor.


Diffs
-

  3rdparty/libprocess/src/libevent_ssl_socket.cpp 
99a12b81eea9c37278b6db0bfedf4b151ff8ed50 

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


Testing
---

The master will CHECK fail under the following:

1) Paste lots of text (16KB or more) of text into 
`src/webui/master/static/home.html`.  The more text, the more reliable the 
repro.

2) Start the master with SSL enabled:
```
LIBPROCESS_SSL_ENABLED=true LIBPROCESS_SSL_KEY_FILE=key.pem 
LIBPROCESS_SSL_CERT_FILE=cert.pem bin/mesos-master.sh --work_dir=/tmp/master
```

3) Run two instances of this python script repeatedly:
```
import socket
import ssl

s = ssl.wrap_socket(socket.socket())
s.connect(("localhost", 5050))

s.sendall("""GET /static/home.html HTTP/1.1
User-Agent: foobar
Host: localhost:5050
Accept: */*
Connection: Keep-Alive

""")

# The HTTP part of the response
print s.recv(1000)
```

i.e. 
```
while python test.py; do :; done & while python test.py; do :; done
```


Thanks,

Joseph Wu

Re: Review Request 51508: Remove the useless http::get method.

[Mesos ReviewBot]

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



Bad review!

Reviews applied: []

Error:
No reviewers specified. Please find a reviewer by asking on JIRA or the mailing 
list.

- Mesos ReviewBot


On Aug. 29, 2016, 11:41 p.m., Yongqiao Wang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51508/
> ---
> 
> (Updated Aug. 29, 2016, 11:41 p.m.)
> 
> 
> Review request for mesos.
> 
> 
> Bugs: MESOS-4440
> https://issues.apache.org/jira/browse/MESOS-4440
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Remove the useless http::get method.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/http.hpp 
> 404196bb198c1ff958b55d72fb29c5fe92dba429 
>   3rdparty/libprocess/src/http.cpp 298bd460afdeccd18e201d8a505b961fd4cf3d3e 
> 
> Diff: https://reviews.apache.org/r/51508/diff/
> 
> 
> Testing
> ---
> 
> make & make check successfully!
> 
> 
> Thanks,
> 
> Yongqiao Wang
> 
>

Review Request 51508: Remove the useless http::get method.

[Yongqiao Wang]

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

Review request for mesos.


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


Repository: mesos


Description
---

Remove the useless http::get method.


Diffs
-

  3rdparty/libprocess/include/process/http.hpp 
404196bb198c1ff958b55d72fb29c5fe92dba429 
  3rdparty/libprocess/src/http.cpp 298bd460afdeccd18e201d8a505b961fd4cf3d3e 

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


Testing
---


Thanks,

Yongqiao Wang

Re: Review Request 51508: Remove the useless http::get method.

[Yongqiao Wang]

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

(Updated Aug. 29, 2016, 11:41 p.m.)


Review request for mesos.


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


Repository: mesos


Description
---

Remove the useless http::get method.


Diffs
-

  3rdparty/libprocess/include/process/http.hpp 
404196bb198c1ff958b55d72fb29c5fe92dba429 
  3rdparty/libprocess/src/http.cpp 298bd460afdeccd18e201d8a505b961fd4cf3d3e 

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


Testing (updated)
---

make & make check successfully!


Thanks,

Yongqiao Wang

Re: Review Request 51505: [2]Replace http::get with http::request.

[Yongqiao Wang]

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

(Updated Aug. 29, 2016, 11:39 p.m.)


Review request for mesos.


Summary (updated)
-

[2]Replace http::get with http::request.


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


Repository: mesos


Description
---

Replace http::get with http::request.


Diffs
-

  src/tests/containerizer/docker_containerizer_tests.cpp 
28cd3fa66886dbdbae3fdeca77707147faafcb7a 
  src/tests/executor_http_api_tests.cpp 
fd322ee267b2f0d4bf250db15027d61f1fc74a57 
  src/tests/fault_tolerance_tests.cpp 5a9944cf459ab688907d95bbda09f464b37efd1e 
  src/tests/files_tests.cpp 78719da5938ff0de722f8411174817f708fd41f2 
  src/tests/gc_tests.cpp f3aec4a95ff231bed1283ef1408101c012cadea1 
  src/tests/health_check_tests.cpp 50a252b5267cbc3b21ddc75e1da86c4975f6faf1 
  src/tests/logging_tests.cpp 886dcd0d6d6ccf509c98578e8b86b1c6ecd5775f 
  src/tests/master_authorization_tests.cpp 
a6399f1b958705b87418b40d3771b25a37d49682 
  src/tests/master_maintenance_tests.cpp 
0820e637b0da69ebcfc8776c223c3bc8a4f1bc94 
  src/tests/master_quota_tests.cpp 48be7406181646c8cc1d169b82a4a4ca71cdf03b 
  src/tests/master_tests.cpp 6cde15fcd6ca8ec40438c75aed980e83f8de9b86 
  src/tests/metrics_tests.cpp 27fc35f3bc47fc2905f4ce400b1051307dcacb2e 
  src/tests/persistent_volume_endpoints_tests.cpp 
266c2a0ff4a99baa96a7c4980f076755603256a9 
  src/tests/registrar_tests.cpp b04fc92c66449f1094212a534899e16d5f6f01bd 
  src/tests/role_tests.cpp 162c9414a70723a212cfd39ac5c0b8325c3f3b5d 
  src/tests/scheduler_driver_tests.cpp faf2e6c8ad17e07964b4340d0b340654b03f9086 
  src/tests/scheduler_http_api_tests.cpp 
80a2ef0af9a4c67deaef40e1f36343868ee4428f 
  src/tests/slave_authorization_tests.cpp 
6f120b6ab5d86c3737e979cdcdb5b6cca008d1ff 
  src/tests/slave_tests.cpp 701fab20091f9f7df020290b9f0a9db1dec92647 
  src/tests/status_update_manager_tests.cpp 
38d8913a5b33aa5325d0bc632c0a1d80480eddf8 
  src/tests/utils.cpp cc5259a720bb6451714a100b0451b473395e3650 

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


Testing
---

make & make check successfully!


Thanks,

Yongqiao Wang

Re: Review Request 51495: [1]Replace http::get with http::request.

[Yongqiao Wang]

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

(Updated Aug. 29, 2016, 11:39 p.m.)


Review request for mesos.


Summary (updated)
-

[1]Replace http::get with http::request.


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


Repository: mesos


Description
---

Replace http::get with http::request.


Diffs
-

  3rdparty/libprocess/src/tests/benchmarks.cpp 
945007c38f96a66212f5ebdd700a117a4902a736 
  3rdparty/libprocess/src/tests/http_tests.cpp 
24b266df5f17e28e0c95326f5d1ea451952500e8 
  3rdparty/libprocess/src/tests/metrics_tests.cpp 
88526e67a20b7b58a6e14034215faa3ae9879fd0 
  3rdparty/libprocess/src/tests/process_tests.cpp 
b9feec7e34cffe19e49035f8865b150f79258f54 
  3rdparty/libprocess/src/tests/profiler_tests.cpp 
995bd02f6ecce484cd9b2aca355c2707d73d40b2 
  3rdparty/libprocess/src/tests/system_tests.cpp 
0f4d0424689522337806ba2227ec4330c700e17e 

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


Testing
---

make & make check successfully!


Thanks,

Yongqiao Wang

Re: Review Request 51421: Added provisioner appc unit test for recovering nested container.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [51323, 51343, 51358, 51359, 51392, 51393, 51402, 51503, 
51420, 51421]

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 Aug. 29, 2016, 9:20 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51421/
> ---
> 
> (Updated Aug. 29, 2016, 9:20 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Artem Harutyunyan, Jie Yu, Joseph 
> Wu, and Kevin Klues.
> 
> 
> Bugs: MESOS-6067
> https://issues.apache.org/jira/browse/MESOS-6067
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added provisioner appc unit test for recovering nested container.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/provisioner_appc_tests.cpp 
> b3ba176e506a6d1528290c07a8a0555b12c8cf70 
> 
> Diff: https://reviews.apache.org/r/51421/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 51097: Added a `PortMapper` class.

[Qian Zhang]

> On Aug. 29, 2016, 9:59 a.m., Qian Zhang wrote:
> > src/slave/containerizer/mesos/isolators/network/cni/spec.cpp, line 63
> > 
> >
> > I would suggest to switch `msg` and `code`.
> 
> Avinash sridharan wrote:
> You mean the parameters right? The default values should always be at the 
> end. May be I didn't understand the comment?

Sorry, my bad, I forgot `code` has default value.


- Qian


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


On Aug. 30, 2016, midnight, Avinash sridharan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51097/
> ---
> 
> (Updated Aug. 30, 2016, midnight)
> 
> 
> Review request for mesos, Jie Yu and Qian Zhang.
> 
> 
> Bugs: MESOS-6023
> https://issues.apache.org/jira/browse/MESOS-6023
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This class will embody the logic for implementing the CNI port-mapper
> plugin.
> 
> Also added helper functions in `cni::spec` to be able to log CNI
> plugin error as a JSON formatted string of `spec::error`.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 120a715eeeb7fb222d1169500950b5c7643df554 
>   
> src/slave/containerizer/mesos/isolators/network/cni/plugins/port_mapper/main.cpp
>  PRE-CREATION 
>   
> src/slave/containerizer/mesos/isolators/network/cni/plugins/port_mapper/port_mapper.hpp
>  PRE-CREATION 
>   
> src/slave/containerizer/mesos/isolators/network/cni/plugins/port_mapper/port_mapper.cpp
>  PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/network/cni/spec.hpp 
> e6967415a689184f13d65a986f13b0af54364fb2 
>   src/slave/containerizer/mesos/isolators/network/cni/spec.cpp 
> 938a9f3bff2a95c8591450f0edafaddb01833e59 
> 
> Diff: https://reviews.apache.org/r/51097/diff/
> 
> 
> Testing
> ---
> 
> Tested the port-mapper with the following CNI config:
> {
> "name": "mynet",
> "type": "port-mapper",
> "chain": "MESOS",
> "delegate": {
>   "type" : "bridge",
>   "bridge": "cni0",
>   "isGateway": true,
>   "ipMasq": true,
>   "ipam": {
>   "type": "host-local",
>   "subnet": "10.22.0.0/16",
>   "routes": [
> { "dst": "0.0.0.0/0" }
>   ]
>   }
> },
> "args" : {
>   "org.apache.mesos" : {
> "network_info" : {
>   "port_mappings": {
> "host_port" : 80,
> "container_port" : 80
>   }
> }
>   }
> }
> }
> 
> and the following environment variables:
> export CNI_COMMAND="ADD"
> export CNI_CONTAINERID="00001110"
> export CNI_PATH="$MESOS_INSTALL:/home/vagrant/dev/go/cni/bin"
> export CNI_IFNAME="eth0"
> export CNI_NETNS="/etc/netns"
> 
> If we remove fields from the above CNI config, or remove certain environment 
> variables the creation of the `PortMapper` correctly fails. However, if 
> config and environment variables are passed as is it will create the 
> `PortMapper` correctly.
> 
> 
> Thanks,
> 
> Avinash sridharan
> 
>

Review Request 51505: Replace http::get with http::request.

[Yongqiao Wang]

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

Review request for mesos.


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


Repository: mesos


Description
---

Replace http::get with http::request.


Diffs
-

  src/tests/containerizer/docker_containerizer_tests.cpp 
28cd3fa66886dbdbae3fdeca77707147faafcb7a 
  src/tests/executor_http_api_tests.cpp 
fd322ee267b2f0d4bf250db15027d61f1fc74a57 
  src/tests/fault_tolerance_tests.cpp 5a9944cf459ab688907d95bbda09f464b37efd1e 
  src/tests/files_tests.cpp 78719da5938ff0de722f8411174817f708fd41f2 
  src/tests/gc_tests.cpp f3aec4a95ff231bed1283ef1408101c012cadea1 
  src/tests/health_check_tests.cpp 50a252b5267cbc3b21ddc75e1da86c4975f6faf1 
  src/tests/logging_tests.cpp 886dcd0d6d6ccf509c98578e8b86b1c6ecd5775f 
  src/tests/master_authorization_tests.cpp 
a6399f1b958705b87418b40d3771b25a37d49682 
  src/tests/master_maintenance_tests.cpp 
0820e637b0da69ebcfc8776c223c3bc8a4f1bc94 
  src/tests/master_quota_tests.cpp 48be7406181646c8cc1d169b82a4a4ca71cdf03b 
  src/tests/master_tests.cpp 6cde15fcd6ca8ec40438c75aed980e83f8de9b86 
  src/tests/metrics_tests.cpp 27fc35f3bc47fc2905f4ce400b1051307dcacb2e 
  src/tests/persistent_volume_endpoints_tests.cpp 
266c2a0ff4a99baa96a7c4980f076755603256a9 
  src/tests/registrar_tests.cpp b04fc92c66449f1094212a534899e16d5f6f01bd 
  src/tests/role_tests.cpp 162c9414a70723a212cfd39ac5c0b8325c3f3b5d 
  src/tests/scheduler_driver_tests.cpp faf2e6c8ad17e07964b4340d0b340654b03f9086 
  src/tests/scheduler_http_api_tests.cpp 
80a2ef0af9a4c67deaef40e1f36343868ee4428f 
  src/tests/slave_authorization_tests.cpp 
6f120b6ab5d86c3737e979cdcdb5b6cca008d1ff 
  src/tests/slave_tests.cpp 701fab20091f9f7df020290b9f0a9db1dec92647 
  src/tests/status_update_manager_tests.cpp 
38d8913a5b33aa5325d0bc632c0a1d80480eddf8 
  src/tests/utils.cpp cc5259a720bb6451714a100b0451b473395e3650 

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


Testing
---

make & make check successfully!


Thanks,

Yongqiao Wang

Review Request 51495: Replace http::get with http::request.

[Yongqiao Wang]

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

Review request for mesos.


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


Repository: mesos


Description
---

Replace http::get with http::request.


Diffs
-

  3rdparty/libprocess/src/tests/benchmarks.cpp 
945007c38f96a66212f5ebdd700a117a4902a736 
  3rdparty/libprocess/src/tests/http_tests.cpp 
24b266df5f17e28e0c95326f5d1ea451952500e8 
  3rdparty/libprocess/src/tests/metrics_tests.cpp 
88526e67a20b7b58a6e14034215faa3ae9879fd0 
  3rdparty/libprocess/src/tests/process_tests.cpp 
b9feec7e34cffe19e49035f8865b150f79258f54 
  3rdparty/libprocess/src/tests/profiler_tests.cpp 
995bd02f6ecce484cd9b2aca355c2707d73d40b2 
  3rdparty/libprocess/src/tests/system_tests.cpp 
0f4d0424689522337806ba2227ec4330c700e17e 

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


Testing
---

make & make check successfully!


Thanks,

Yongqiao Wang

Review Request 51493: Add the query parameter in createRequest.

[Yongqiao Wang]

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

Review request for mesos.


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


Repository: mesos


Description
---

Add the new 'query' parameter in createRequest method, which will be used by 
Http GET method later.


Diffs
-

  3rdparty/libprocess/include/process/http.hpp 
404196bb198c1ff958b55d72fb29c5fe92dba429 
  3rdparty/libprocess/src/http.cpp 298bd460afdeccd18e201d8a505b961fd4cf3d3e 

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


Testing
---

make & make check successfully.


Thanks,

Yongqiao Wang

Re: Review Request 51501: Exposed metrics in scheduler library.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [51501]

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 Aug. 29, 2016, 7:21 p.m., Abhishek Dasgupta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51501/
> ---
> 
> (Updated Aug. 29, 2016, 7:21 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar and Vinod Kone.
> 
> 
> Bugs: MESOS-6080
> https://issues.apache.org/jira/browse/MESOS-6080
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Exposed metrics scheduler/event_queue_messages(size of
> message queue) and scheduler/event_queue_dispatches
> (size of dispatch queue) in scheduler library.
> 
> 
> Diffs
> -
> 
>   src/scheduler/scheduler.cpp 276ed10bd439c4a7830447bec5053292fb2ca4e7 
>   src/tests/scheduler_tests.cpp 931c1857f1ab454c67eece2f34c4649a426178de 
> 
> Diff: https://reviews.apache.org/r/51501/diff/
> 
> 
> Testing
> ---
> 
> On Ubuntu 16.04
> sudo GTEST_FILTER="*SchedulerTest.MetricsEndpoint*" make -j4 check
> 
> 
> Thanks,
> 
> Abhishek Dasgupta
> 
>

Re: Review Request 51421: Added provisioner appc unit test for recovering nested container.

[Gilbert Song]

> On Aug. 25, 2016, 5:58 p.m., Jie Yu wrote:
> > src/tests/containerizer/provisioner_appc_tests.cpp, lines 538-543
> > 
> >
> > Hum, I don't think legitimate case. ContainerState will only container 
> > top level executor containers.
> 
> Gilbert Song wrote:
> I have a different thought on this. According to the prvisioner::recover()
> ```
>   virtual process::Future recover(
>   const std::list& states,
>   const hashset& orphans) const;
> ```
> 
> We are assuming the `orphans` includes all orphaned containers (parent 
> and its sub-containers). Correspondingly, the `state` should do the same. It 
> would depend on more changes in MesosContainerizer::recover() to check the 
> `taskState`. Otherwise, all sub-containers will be destroyed as unknown 
> orphans. There is no way to recognize alive sub-containers.

Let's keep this open. I need some more time to think about it.


- Gilbert


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


On Aug. 29, 2016, 2:20 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51421/
> ---
> 
> (Updated Aug. 29, 2016, 2:20 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Artem Harutyunyan, Jie Yu, Joseph 
> Wu, and Kevin Klues.
> 
> 
> Bugs: MESOS-6067
> https://issues.apache.org/jira/browse/MESOS-6067
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added provisioner appc unit test for recovering nested container.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/provisioner_appc_tests.cpp 
> b3ba176e506a6d1528290c07a8a0555b12c8cf70 
> 
> Diff: https://reviews.apache.org/r/51421/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 51402: Added nested container check in provisioner destroy.

[Gilbert Song]

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

(Updated Aug. 29, 2016, 2:20 p.m.)


Review request for mesos, Benjamin Hindman, Artem Harutyunyan, Jie Yu, Joseph 
Wu, and Kevin Klues.


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


Repository: mesos


Description
---

Added nested container check in provisioner destroy.


Diffs (updated)
-

  src/slave/containerizer/mesos/provisioner/provisioner.cpp 
8e35ff49ec99a242e764095dcfbb541c5e41ec71 

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


Testing
---

make check


Thanks,

Gilbert Song

Re: Review Request 51421: Added provisioner appc unit test for recovering nested container.

[Gilbert Song]

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

(Updated Aug. 29, 2016, 2:20 p.m.)


Review request for mesos, Benjamin Hindman, Artem Harutyunyan, Jie Yu, Joseph 
Wu, and Kevin Klues.


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


Repository: mesos


Description
---

Added provisioner appc unit test for recovering nested container.


Diffs (updated)
-

  src/tests/containerizer/provisioner_appc_tests.cpp 
b3ba176e506a6d1528290c07a8a0555b12c8cf70 

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


Testing
---

make check


Thanks,

Gilbert Song

Re: Review Request 51421: Added provisioner appc unit test for recovering nested container.

[Gilbert Song]

> On Aug. 25, 2016, 5:58 p.m., Jie Yu wrote:
> > src/tests/containerizer/provisioner_appc_tests.cpp, lines 538-543
> > 
> >
> > Hum, I don't think legitimate case. ContainerState will only container 
> > top level executor containers.

I have a different thought on this. According to the prvisioner::recover()
```
  virtual process::Future recover(
  const std::list& states,
  const hashset& orphans) const;
```

We are assuming the `orphans` includes all orphaned containers (parent and its 
sub-containers). Correspondingly, the `state` should do the same. It would 
depend on more changes in MesosContainerizer::recover() to check the 
`taskState`. Otherwise, all sub-containers will be destroyed as unknown 
orphans. There is no way to recognize alive sub-containers.


- Gilbert


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


On Aug. 29, 2016, 2:20 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51421/
> ---
> 
> (Updated Aug. 29, 2016, 2:20 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Artem Harutyunyan, Jie Yu, Joseph 
> Wu, and Kevin Klues.
> 
> 
> Bugs: MESOS-6067
> https://issues.apache.org/jira/browse/MESOS-6067
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added provisioner appc unit test for recovering nested container.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/provisioner_appc_tests.cpp 
> b3ba176e506a6d1528290c07a8a0555b12c8cf70 
> 
> Diff: https://reviews.apache.org/r/51421/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 51470: Add the 'resources' field back to DRFSorter::Total.

[Jiang Yan Xu]

> On Aug. 29, 2016, 12:26 a.m., Anindya Sinha wrote:
> > I think we should also bring back this function:
> > `const hashmap& total()`.
> 
> Jiang Yan Xu wrote:
> Why? The total is now fully hidden and we only need to reply on it within 
> DRFSorter for shared resources.

*rely


- Jiang Yan


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


On Aug. 26, 2016, 3:53 p.m., Jiang Yan Xu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51470/
> ---
> 
> (Updated Aug. 26, 2016, 3:53 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Anindya Sinha, Michael Park, 
> and Neil Conway.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This is a partial reversal of 5a63f2a5d2a5fe0e5315a5b8f79b75b99a6a5893.
> We used to have this field but it turned out to be unnecessary for the
> current use cases so it was removed. We however will need it for the
> ongoing work to support tasks using shared resources.
> 
> The details can be found in https://reviews.apache.org/r/45961 but in
> essence, we now know of a case where the removal of resource quantities
> from the sorter depends on the identity. e.g., To determine if the
> quantity of a shared resource can be removed from the sorter we need to
> know if there are no longer instances of **the same** (hence identity)
> shared resource in the sorter.
> 
> 
> Diffs
> -
> 
>   src/master/allocator/sorter/drf/sorter.hpp 
> bc6bfb2d5d3b32d55be055a0514861b4e7d889bb 
>   src/master/allocator/sorter/drf/sorter.cpp 
> ac85b327fc33d34246788e6a8c8bf5a486c61434 
> 
> Diff: https://reviews.apache.org/r/51470/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jiang Yan Xu
> 
>

Re: Review Request 51470: Add the 'resources' field back to DRFSorter::Total.

[Jiang Yan Xu]

> On Aug. 29, 2016, 12:26 a.m., Anindya Sinha wrote:
> > I think we should also bring back this function:
> > `const hashmap& total()`.

Why? The total is now fully hidden and we only need to reply on it within 
DRFSorter for shared resources.


- Jiang Yan


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


On Aug. 26, 2016, 3:53 p.m., Jiang Yan Xu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51470/
> ---
> 
> (Updated Aug. 26, 2016, 3:53 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Anindya Sinha, Michael Park, 
> and Neil Conway.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This is a partial reversal of 5a63f2a5d2a5fe0e5315a5b8f79b75b99a6a5893.
> We used to have this field but it turned out to be unnecessary for the
> current use cases so it was removed. We however will need it for the
> ongoing work to support tasks using shared resources.
> 
> The details can be found in https://reviews.apache.org/r/45961 but in
> essence, we now know of a case where the removal of resource quantities
> from the sorter depends on the identity. e.g., To determine if the
> quantity of a shared resource can be removed from the sorter we need to
> know if there are no longer instances of **the same** (hence identity)
> shared resource in the sorter.
> 
> 
> Diffs
> -
> 
>   src/master/allocator/sorter/drf/sorter.hpp 
> bc6bfb2d5d3b32d55be055a0514861b4e7d889bb 
>   src/master/allocator/sorter/drf/sorter.cpp 
> ac85b327fc33d34246788e6a8c8bf5a486c61434 
> 
> Diff: https://reviews.apache.org/r/51470/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jiang Yan Xu
> 
>

Re: Review Request 51420: Added provisioner appc unit test for provisioning nested container.

[Gilbert Song]

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

(Updated Aug. 29, 2016, 2:20 p.m.)


Review request for mesos, Benjamin Hindman, Artem Harutyunyan, Jie Yu, Joseph 
Wu, and Kevin Klues.


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


Repository: mesos


Description
---

Added provisioner appc unit test for provisioning nested container.


Diffs (updated)
-

  src/tests/containerizer/provisioner_appc_tests.cpp 
b3ba176e506a6d1528290c07a8a0555b12c8cf70 

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


Testing
---

make check


Thanks,

Gilbert Song

Re: Review Request 51358: Implemented recursive helper method findContainerDir for provisioner.

[Gilbert Song]

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

(Updated Aug. 29, 2016, 2:20 p.m.)


Review request for mesos, Benjamin Hindman, Artem Harutyunyan, Jie Yu, Joseph 
Wu, and Kevin Klues.


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


Repository: mesos


Description
---

Implemented recursive helper method findContainerDir for provisioner.


Diffs (updated)
-

  src/slave/containerizer/mesos/provisioner/paths.hpp 
9829d6b52c8547ae22297a5bc47852ce5a219e4c 
  src/slave/containerizer/mesos/provisioner/paths.cpp 
86a45f30c22dc1e41f4779c0ce8c11d02dcc46bb 

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


Testing
---

make check


Thanks,

Gilbert Song

Re: Review Request 51393: Added unit test for provisioner recursive listContainers().

[Gilbert Song]

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

(Updated Aug. 29, 2016, 2:20 p.m.)


Review request for mesos, Benjamin Hindman, Artem Harutyunyan, Jie Yu, Joseph 
Wu, and Kevin Klues.


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


Repository: mesos


Description
---

Added unit test for provisioner recursive listContainers().


Diffs (updated)
-

  src/tests/containerizer/provisioner_paths_tests.cpp PRE-CREATION 

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


Testing
---

make check


Thanks,

Gilbert Song

Re: Review Request 51343: Refactored the redundant logic in provisioner recover().

[Gilbert Song]

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

(Updated Aug. 29, 2016, 2:20 p.m.)


Review request for mesos, Benjamin Hindman, Artem Harutyunyan, Jie Yu, Joseph 
Wu, and Kevin Klues.


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


Repository: mesos


Description
---

This patch removed the unnecessary loop to construct an identical
hashmap for info->rootfses.


Diffs (updated)
-

  src/slave/containerizer/mesos/provisioner/provisioner.cpp 
8e35ff49ec99a242e764095dcfbb541c5e41ec71 

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


Testing
---

make check


Thanks,

Gilbert Song

Review Request 51503: Fixed appc provisioner tests to use absolute work directory.

[Gilbert Song]

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

Review request for mesos, Benjamin Hindman, Artem Harutyunyan, Jie Yu, Joseph 
Wu, and Kevin Klues.


Repository: mesos


Description
---

Fixed appc provisioner tests to use absolute work directory.


Diffs
-

  src/tests/containerizer/provisioner_appc_tests.cpp 
b3ba176e506a6d1528290c07a8a0555b12c8cf70 

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


Testing
---

make check


Thanks,

Gilbert Song

Re: Review Request 51359: Added unit test for provisioner helper findContainerDir.

[Gilbert Song]

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

(Updated Aug. 29, 2016, 2:20 p.m.)


Review request for mesos, Benjamin Hindman, Artem Harutyunyan, Jie Yu, Joseph 
Wu, and Kevin Klues.


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


Repository: mesos


Description
---

Added unit test for provisioner helper findContainerDir.


Diffs (updated)
-

  src/Makefile.am 8dc4175c60e4a9776ddb8ad21774fa4b30c28d00 
  src/tests/containerizer/provisioner_paths_tests.cpp PRE-CREATION 

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


Testing
---

make check


Thanks,

Gilbert Song

Re: Review Request 51392: Supported provisioner listContainers() to be recursive.

[Gilbert Song]

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

(Updated Aug. 29, 2016, 2:20 p.m.)


Review request for mesos, Benjamin Hindman, Artem Harutyunyan, Jie Yu, Joseph 
Wu, and Kevin Klues.


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


Repository: mesos


Description
---

This patch supports collecting all containerIds (all containers in the
nested hierarchy) recursively.


Diffs (updated)
-

  src/slave/containerizer/mesos/provisioner/paths.cpp 
86a45f30c22dc1e41f4779c0ce8c11d02dcc46bb 

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


Testing
---

make check


Thanks,

Gilbert Song

Re: Review Request 51323: Supported provisioner provision() and destroy() to be nested aware.

[Gilbert Song]

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

(Updated Aug. 29, 2016, 2:20 p.m.)


Review request for mesos, Benjamin Hindman, Artem Harutyunyan, Jie Yu, Joseph 
Wu, and Kevin Klues.


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


Repository: mesos


Description
---

This patch makes the provisioner provision() and destory() methods
to be multi-level nested aware, by simply change the helper function
getContainerDir() to be recursive:

1. In provisioner provision(), just need to make sure the rootfs
   returned from getContainerRootfsDir() and the backend dir
   returned from getBackendDir() are correctly nested.
2. In provisioner destroy(), need to simply make sure the rootfs
   from getContainerRootfsDir() is correctly nested.

Both getContainerRootfsDir() and getBackendDir() rely on
the helper method getContainerDir(). So we can simply make them
nested aware by change getContainerDir() to be recursive.


Diffs (updated)
-

  src/slave/containerizer/mesos/provisioner/paths.hpp 
9829d6b52c8547ae22297a5bc47852ce5a219e4c 
  src/slave/containerizer/mesos/provisioner/paths.cpp 
86a45f30c22dc1e41f4779c0ce8c11d02dcc46bb 

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


Testing (updated)
---

make check


Thanks,

Gilbert Song

Re: Review Request 51497: Avoided blocking calls in `Docker::validateVersion`.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [39939, 51497]

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 Aug. 29, 2016, 5:05 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51497/
> ---
> 
> (Updated Aug. 29, 2016, 5:05 p.m.)
> 
> 
> Review request for mesos and Zhitao Li.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch added a `version` member variable in `Docker` class to avoid
> the blocking command line call in `Docker::validateVersion`.
> 
> 
> Diffs
> -
> 
>   src/docker/docker.hpp 9093371afc8ea792ba94f61c6875703e547ea6b0 
>   src/docker/docker.cpp 16abf64f98cc833ea4bee28bc08951eb2ff6d995 
>   src/tests/container_logger_tests.cpp 
> e8f934106510fe02b8b92be19c918a1e5c0b78fd 
>   src/tests/containerizer/docker_containerizer_tests.cpp 
> 28cd3fa66886dbdbae3fdeca77707147faafcb7a 
>   src/tests/containerizer/docker_tests.cpp 
> e9a214a8973b3dfac69d59e90ce08473baa7eba4 
>   src/tests/health_check_tests.cpp 50a252b5267cbc3b21ddc75e1da86c4975f6faf1 
>   src/tests/hook_tests.cpp d864ef3d6153dedab8cc47403b1b9f76d75aeb5c 
>   src/tests/mesos.hpp 4cae54b4df906d4b7e8fe8d40d5b0ad59d260e6f 
>   src/tests/mesos.cpp f5034f9f8de7040182e10f51be125a87b29fdd24 
> 
> Diff: https://reviews.apache.org/r/51497/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Review Request 51501: Exposed metrics in scheduler library.

[Abhishek Dasgupta]

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

Review request for mesos, Anand Mazumdar and Vinod Kone.


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


Repository: mesos


Description
---

Exposed metrics scheduler/event_queue_messages(size of
message queue) and scheduler/event_queue_dispatches
(size of dispatch queue) in scheduler library.


Diffs
-

  src/scheduler/scheduler.cpp 276ed10bd439c4a7830447bec5053292fb2ca4e7 
  src/tests/scheduler_tests.cpp 931c1857f1ab454c67eece2f34c4649a426178de 

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


Testing
---

On Ubuntu 16.04
sudo GTEST_FILTER="*SchedulerTest.MetricsEndpoint*" make -j4 check


Thanks,

Abhishek Dasgupta

Re: Review Request 51412: Add offeredResources to Allocator::updateAllocation() API.

[Jiang Yan Xu]

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


Ship it!




Will commit with the minor edit.


src/master/master.hpp (line 853)


This method also "updates the slave's resources by applying the given 
operation."


- Jiang Yan Xu


On Aug. 29, 2016, 12:20 a.m., Anindya Sinha wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51412/
> ---
> 
> (Updated Aug. 29, 2016, 12:20 a.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-4431
> https://issues.apache.org/jira/browse/MESOS-4431
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This is being added to allow for handling of ACCEPT call within the
> allocator in the future. For now, we expose the offered resources
> in this API to account for additional copies of shared resources in
> the allocator when there are more tasks being launched in a single
> ACCEPT call for a single copy of shared resource being offered.
> 
> 
> Diffs
> -
> 
>   include/mesos/allocator/allocator.hpp 
> 41a9d457286e30431490ca626e680d85684b48d6 
>   src/master/allocator/mesos/allocator.hpp 
> 69639be9c14b83157df29f767e819db719588053 
>   src/master/allocator/mesos/hierarchical.hpp 
> bdbc6d3b5b959990538f4e3b7b1a3b031d9aea05 
>   src/master/allocator/mesos/hierarchical.cpp 
> 234ef98529964a0b6d3f132426a6c8ccbb1263ee 
>   src/master/master.hpp b118293823d37bba9b751aac12e0403318ce5736 
>   src/master/master.cpp 2b4aff80385d02c71dc3eeff4ddda326c9824ede 
>   src/tests/allocator.hpp 5ca3057265eb2c00c965132ac7922019e1cc77b8 
>   src/tests/hierarchical_allocator_tests.cpp 
> ddd48694e5c2d3d01fbff20558a46c1256b3dbc3 
>   src/tests/reservation_tests.cpp 6181c6f8abbf254b45ea77f33c48dee918106ef0 
> 
> Diff: https://reviews.apache.org/r/51412/diff/
> 
> 
> Testing
> ---
> 
> All tests passed.
> 
> 
> Thanks,
> 
> Anindya Sinha
> 
>

Re: Review Request 51097: Added a `PortMapper` class.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [51095, 51096, 51097]

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 Aug. 29, 2016, 4 p.m., Avinash sridharan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51097/
> ---
> 
> (Updated Aug. 29, 2016, 4 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Qian Zhang.
> 
> 
> Bugs: MESOS-6023
> https://issues.apache.org/jira/browse/MESOS-6023
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This class will embody the logic for implementing the CNI port-mapper
> plugin.
> 
> Also added helper functions in `cni::spec` to be able to log CNI
> plugin error as a JSON formatted string of `spec::error`.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 120a715eeeb7fb222d1169500950b5c7643df554 
>   
> src/slave/containerizer/mesos/isolators/network/cni/plugins/port_mapper/main.cpp
>  PRE-CREATION 
>   
> src/slave/containerizer/mesos/isolators/network/cni/plugins/port_mapper/port_mapper.hpp
>  PRE-CREATION 
>   
> src/slave/containerizer/mesos/isolators/network/cni/plugins/port_mapper/port_mapper.cpp
>  PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/network/cni/spec.hpp 
> e6967415a689184f13d65a986f13b0af54364fb2 
>   src/slave/containerizer/mesos/isolators/network/cni/spec.cpp 
> 938a9f3bff2a95c8591450f0edafaddb01833e59 
> 
> Diff: https://reviews.apache.org/r/51097/diff/
> 
> 
> Testing
> ---
> 
> Tested the port-mapper with the following CNI config:
> {
> "name": "mynet",
> "type": "port-mapper",
> "chain": "MESOS",
> "delegate": {
>   "type" : "bridge",
>   "bridge": "cni0",
>   "isGateway": true,
>   "ipMasq": true,
>   "ipam": {
>   "type": "host-local",
>   "subnet": "10.22.0.0/16",
>   "routes": [
> { "dst": "0.0.0.0/0" }
>   ]
>   }
> },
> "args" : {
>   "org.apache.mesos" : {
> "network_info" : {
>   "port_mappings": {
> "host_port" : 80,
> "container_port" : 80
>   }
> }
>   }
> }
> }
> 
> and the following environment variables:
> export CNI_COMMAND="ADD"
> export CNI_CONTAINERID="00001110"
> export CNI_PATH="$MESOS_INSTALL:/home/vagrant/dev/go/cni/bin"
> export CNI_IFNAME="eth0"
> export CNI_NETNS="/etc/netns"
> 
> If we remove fields from the above CNI config, or remove certain environment 
> variables the creation of the `PortMapper` correctly fails. However, if 
> config and environment variables are passed as is it will create the 
> `PortMapper` correctly.
> 
> 
> Thanks,
> 
> Avinash sridharan
> 
>

Re: Review Request 50868: Using `OfferedResources` for benchmark test of `SuppressOffers`.

[Jiang Yan Xu]

> On Aug. 25, 2016, 3:56 p.m., Jiang Yan Xu wrote:
> > I don't know. Making such distinction feels like splitting hairs to me. 
> > Yeah there are subtle differences between the two concepts but to me:
> > 
> > - `offerCallback` may not be the best name what we could have come up with 
> > even though in `Master::offer()` we do construct offers.
> > - When offer objects are constructed, we call the resources in them 
> > "offered resources". 
> > - When we parse resources from offer objects, we call them "offered 
> > resources".
> > - In this test, namely, the "Allocator" tests, we are indeed only 
> > allocating resources. (I don't see "Offer" objects being created).
> > 
> > Later if the allocator actually hands out offer objects, then we probably 
> > need to change the tests to use offers directly but right now I think 1) 
> > "allocation" is most correct, 2) it's not a big deal if people treat them 
> > as synonymous terms. 
> > 
> > That said, I think reusing the "Allocation" defined at the top of the test 
> > makes sense.
> 
> Guangya Liu wrote:
> Thanks Jiang Yan, there are acutally some discussion here 
> https://reviews.apache.org/r/50677/ , the reason that I want to update this 
> is because:
>  
> 1) Make the code consistent, in the allocator test cases, some benchmark 
> test using `OfferedResources` while some using `Allocation`, when people want 
> to add new benchmark test, they will be confused: Which one shall I use, 
> `OfferedResources` or `Allocation`, so here we need to make the code 
> consistent with each other for better reading, easy to maintain and 
> consistent.
> 
> 2) Seems here using `OfferedResources` maybe better, as here we are 
> actually constructing the `offers` on each agent; But the `Allocation` is an 
> allocator concept and it means the resources for a specified framework. Also 
> all of the log messages are highlighting `offer` in benchmark test such as 
> here 
> https://github.com/apache/mesos/blob/master/src/tests/hierarchical_allocator_tests.cpp#L3885
> 
> 3) The benchmark test is focusing on `offers` even though the allocator 
> know nothing about `offer`, but here we are just simulating `offer 
> operations` in benchmark test. For other unit test cases other than benchmark 
> test, they are focusing on `allocations` for each framework, so it is using 
> `Allocation`. 
> 
> What do you think?

Thanks for the reply. I've listed a few critia that I think could explain their 
subtle differences but I try to see if making such a distinction leads to 
simplity and clarity of the code. i.e., would everyone agree with the way we 
make such distinction and would it help readers understand the code better. If 
it's controversial, then perhaps we should aim for consistently. :)

In terms of code in this file, yes I agree there's inconsistency and we should 
address it. To that end I think:

To 1): The question to me is to choose between *OfferedResources -> Allocation* 
or *Allocation -> OfferedResources*. I am leaning towards *OfferedResources -> 
Allocation* and FWIW `Allocation` is a more established usage than 
`OfferedResources` in this file.

To 2) and 3): sorry I failed to see the fundamental difference between the 
benchmarks and the unit tests here in terms of `offers` vs `allocations`. The 
unit tests, espcially later-added ones, follow basically the same pattern as 
the benchmarks. The benchmarks are just different in the use of expectations 
vs. measurements and the number of iterations. It's not convincing to me that 
in some cases we need to call them OfferedResources and in others we should 
call them Allocations. 

So overall I think it's better to change to use the Allocation struct defined 
at the top to achieve consistency. It looks to me that all occurrences of 
`offers` in the log messages can be changed to `allocations` as well.

Let me know what you think.


- Jiang Yan


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


On Aug. 5, 2016, 2:50 p.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50868/
> ---
> 
> (Updated Aug. 5, 2016, 2:50 p.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The current benchmark test of `SuppressOffers` is using `Allocation`
> to specify the resources from an `OfferCallback`, but here seems
> using `OfferedResources` maybe better, as here we are actually
> constructing the offers on each agent, but the `Allocation` is an
> allocator concept and it means the resources allocated for a specified
> framework.
> 
> 
> Diffs
> -
> 
>   

Re: Review Request 51497: Avoided blocking calls in `Docker::validateVersion`.

[Zhitao Li]

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




src/docker/docker.hpp (line 223)


Can we make tihs a non-static virtual function so that we can simply 
override it in `MockDocker`?



src/docker/docker.cpp (lines 111 - 122)


Does this mean if docker daemon is not available, Mesos agent will not 
start either because DockerContainerizer will fail to initialize?

This seems okay behavior to me, but we might want to document this behavior 
in UPGRADE and/or containerizer.md



src/tests/mesos.cpp (lines 652 - 669)


Is there a way to avoid actually calling docker daemon to prepare the mock?

I think we can either override the `getVersion()` call, or override the 
`_version()` call by directly returning the `version` in this class.


- Zhitao Li


On Aug. 29, 2016, 5:05 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51497/
> ---
> 
> (Updated Aug. 29, 2016, 5:05 p.m.)
> 
> 
> Review request for mesos and Zhitao Li.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch added a `version` member variable in `Docker` class to avoid
> the blocking command line call in `Docker::validateVersion`.
> 
> 
> Diffs
> -
> 
>   src/docker/docker.hpp 9093371afc8ea792ba94f61c6875703e547ea6b0 
>   src/docker/docker.cpp 16abf64f98cc833ea4bee28bc08951eb2ff6d995 
>   src/tests/container_logger_tests.cpp 
> e8f934106510fe02b8b92be19c918a1e5c0b78fd 
>   src/tests/containerizer/docker_containerizer_tests.cpp 
> 28cd3fa66886dbdbae3fdeca77707147faafcb7a 
>   src/tests/containerizer/docker_tests.cpp 
> e9a214a8973b3dfac69d59e90ce08473baa7eba4 
>   src/tests/health_check_tests.cpp 50a252b5267cbc3b21ddc75e1da86c4975f6faf1 
>   src/tests/hook_tests.cpp d864ef3d6153dedab8cc47403b1b9f76d75aeb5c 
>   src/tests/mesos.hpp 4cae54b4df906d4b7e8fe8d40d5b0ad59d260e6f 
>   src/tests/mesos.cpp f5034f9f8de7040182e10f51be125a87b29fdd24 
> 
> Diff: https://reviews.apache.org/r/51497/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Review Request 51497: Avoided blocking calls in `Docker::validateVersion`.

[haosdent huang]

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

Review request for mesos and Zhitao Li.


Repository: mesos


Description
---

This patch added a `version` member variable in `Docker` class to avoid
the blocking command line call in `Docker::validateVersion`.


Diffs
-

  src/docker/docker.hpp 9093371afc8ea792ba94f61c6875703e547ea6b0 
  src/docker/docker.cpp 16abf64f98cc833ea4bee28bc08951eb2ff6d995 
  src/tests/container_logger_tests.cpp e8f934106510fe02b8b92be19c918a1e5c0b78fd 
  src/tests/containerizer/docker_containerizer_tests.cpp 
28cd3fa66886dbdbae3fdeca77707147faafcb7a 
  src/tests/containerizer/docker_tests.cpp 
e9a214a8973b3dfac69d59e90ce08473baa7eba4 
  src/tests/health_check_tests.cpp 50a252b5267cbc3b21ddc75e1da86c4975f6faf1 
  src/tests/hook_tests.cpp d864ef3d6153dedab8cc47403b1b9f76d75aeb5c 
  src/tests/mesos.hpp 4cae54b4df906d4b7e8fe8d40d5b0ad59d260e6f 
  src/tests/mesos.cpp f5034f9f8de7040182e10f51be125a87b29fdd24 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 39939: Made docker_socket option support different protocols.

[haosdent huang]

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

(Updated Aug. 29, 2016, 5:04 p.m.)


Review request for mesos, Gilbert Song, Jie Yu, Klaus Ma, and Timothy Chen.


Changes
---

Rebase.


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


Repository: mesos


Description
---

Made docker_socket option support different protocols.


Diffs (updated)
-

  docs/configuration.md 20906723be45bd43da0ef605ddb0742c56994675 
  docs/docker-containerizer.md 1750b0692301c6adfcfbd3d56137ce00e41c2721 
  src/docker/docker.hpp 9093371afc8ea792ba94f61c6875703e547ea6b0 
  src/docker/docker.cpp 16abf64f98cc833ea4bee28bc08951eb2ff6d995 
  src/slave/containerizer/docker.cpp 110a1eb41b1ff7cb94f3630a1843d9f01efbe09c 
  src/slave/flags.cpp 0f2be1700f41b74da4ea1ce699a81ec33cf92a9a 

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


Testing
---

make check


Thanks,

haosdent huang

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

[haosdent huang]

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


Fix it, then Ship it!




Ship It!


src/docker/docker.cpp (line 526)


I suggest to add an extra line above.



src/docker/docker.cpp (line 528)


An extra line below to match mesos style.



src/docker/docker.cpp (line 530)


Should be 
```
argv.push_back(stringify(static_cast(quota.us(;
```
here?



src/docker/executor.hpp (lines 91 - 96)


Should we put `bool cgroups_enable_cfs;` after `task_environment`?

For example:

```
Option task_environment;
bool cgroups_enable_cfs;

// TOD.
```


- haosdent huang


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

Re: Review Request 51097: Added a `PortMapper` class.

[Avinash sridharan]

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

(Updated Aug. 29, 2016, 4 p.m.)


Review request for mesos, Jie Yu and Qian Zhang.


Changes
---

Fixed a comment from Qian.


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


Repository: mesos


Description
---

This class will embody the logic for implementing the CNI port-mapper
plugin.

Also added helper functions in `cni::spec` to be able to log CNI
plugin error as a JSON formatted string of `spec::error`.


Diffs (updated)
-

  src/Makefile.am 120a715eeeb7fb222d1169500950b5c7643df554 
  
src/slave/containerizer/mesos/isolators/network/cni/plugins/port_mapper/main.cpp
 PRE-CREATION 
  
src/slave/containerizer/mesos/isolators/network/cni/plugins/port_mapper/port_mapper.hpp
 PRE-CREATION 
  
src/slave/containerizer/mesos/isolators/network/cni/plugins/port_mapper/port_mapper.cpp
 PRE-CREATION 
  src/slave/containerizer/mesos/isolators/network/cni/spec.hpp 
e6967415a689184f13d65a986f13b0af54364fb2 
  src/slave/containerizer/mesos/isolators/network/cni/spec.cpp 
938a9f3bff2a95c8591450f0edafaddb01833e59 

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


Testing
---

Tested the port-mapper with the following CNI config:
{
"name": "mynet",
"type": "port-mapper",
"chain": "MESOS",
"delegate": {
  "type" : "bridge",
  "bridge": "cni0",
  "isGateway": true,
  "ipMasq": true,
  "ipam": {
  "type": "host-local",
  "subnet": "10.22.0.0/16",
  "routes": [
{ "dst": "0.0.0.0/0" }
  ]
  }
},
"args" : {
  "org.apache.mesos" : {
"network_info" : {
  "port_mappings": {
"host_port" : 80,
"container_port" : 80
  }
}
  }
}
}

and the following environment variables:
export CNI_COMMAND="ADD"
export CNI_CONTAINERID="00001110"
export CNI_PATH="$MESOS_INSTALL:/home/vagrant/dev/go/cni/bin"
export CNI_IFNAME="eth0"
export CNI_NETNS="/etc/netns"

If we remove fields from the above CNI config, or remove certain environment 
variables the creation of the `PortMapper` correctly fails. However, if config 
and environment variables are passed as is it will create the `PortMapper` 
correctly.


Thanks,

Avinash sridharan

Re: Review Request 51097: Added a `PortMapper` class.

[Avinash sridharan]

> On Aug. 29, 2016, 1:59 a.m., Qian Zhang wrote:
> > src/slave/containerizer/mesos/isolators/network/cni/plugins/port_mapper/port_mapper.cpp,
> >  line 49
> > 
> >
> > The indent should be 4 spaces, please see the following code as a 
> > reference.
> > 
> > https://github.com/apache/mesos/blob/1.0.0/src/slave/containerizer/mesos/containerizer.cpp#L179:L180
> > 
> > Ditto for all other places.

Yeah, I saw that. The problem is that vim is doing this indentation, since it 
sees two function calls, I think it autoindents to 8 spaces. Not sure if that 
is the right thing. Let me check with Jie or one of the other shepherds. Its a 
bit of a pain to do this manually.


> On Aug. 29, 2016, 1:59 a.m., Qian Zhang wrote:
> > src/slave/containerizer/mesos/isolators/network/cni/spec.cpp, line 63
> > 
> >
> > I would suggest to switch `msg` and `code`.

You mean the parameters right? The default values should always be at the end. 
May be I didn't understand the comment?


- Avinash


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


On Aug. 28, 2016, 7:14 p.m., Avinash sridharan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51097/
> ---
> 
> (Updated Aug. 28, 2016, 7:14 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Qian Zhang.
> 
> 
> Bugs: MESOS-6023
> https://issues.apache.org/jira/browse/MESOS-6023
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This class will embody the logic for implementing the CNI port-mapper
> plugin.
> 
> Also added helper functions in `cni::spec` to be able to log CNI
> plugin error as a JSON formatted string of `spec::error`.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 120a715eeeb7fb222d1169500950b5c7643df554 
>   
> src/slave/containerizer/mesos/isolators/network/cni/plugins/port_mapper/main.cpp
>  PRE-CREATION 
>   
> src/slave/containerizer/mesos/isolators/network/cni/plugins/port_mapper/port_mapper.hpp
>  PRE-CREATION 
>   
> src/slave/containerizer/mesos/isolators/network/cni/plugins/port_mapper/port_mapper.cpp
>  PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/network/cni/spec.hpp 
> e6967415a689184f13d65a986f13b0af54364fb2 
>   src/slave/containerizer/mesos/isolators/network/cni/spec.cpp 
> 938a9f3bff2a95c8591450f0edafaddb01833e59 
> 
> Diff: https://reviews.apache.org/r/51097/diff/
> 
> 
> Testing
> ---
> 
> Tested the port-mapper with the following CNI config:
> {
> "name": "mynet",
> "type": "port-mapper",
> "chain": "MESOS",
> "delegate": {
>   "type" : "bridge",
>   "bridge": "cni0",
>   "isGateway": true,
>   "ipMasq": true,
>   "ipam": {
>   "type": "host-local",
>   "subnet": "10.22.0.0/16",
>   "routes": [
> { "dst": "0.0.0.0/0" }
>   ]
>   }
> },
> "args" : {
>   "org.apache.mesos" : {
> "network_info" : {
>   "port_mappings": {
> "host_port" : 80,
> "container_port" : 80
>   }
> }
>   }
> }
> }
> 
> and the following environment variables:
> export CNI_COMMAND="ADD"
> export CNI_CONTAINERID="00001110"
> export CNI_PATH="$MESOS_INSTALL:/home/vagrant/dev/go/cni/bin"
> export CNI_IFNAME="eth0"
> export CNI_NETNS="/etc/netns"
> 
> If we remove fields from the above CNI config, or remove certain environment 
> variables the creation of the `PortMapper` correctly fails. However, if 
> config and environment variables are passed as is it will create the 
> `PortMapper` correctly.
> 
> 
> Thanks,
> 
> Avinash sridharan
> 
>

Re: Review Request 51021: Added GC of unreachable agent metadata from the registry.

[Neil Conway]

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

(Updated Aug. 29, 2016, 9:52 a.m.)


Review request for mesos and Vinod Kone.


Changes
---

Rebase.


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


Repository: mesos


Description
---

Information about agents that have been marked unreachable is stored
in the registry. Since the number of unreachable agents can grow without
bound, this commit implements a garbage collection scheme. The current
leading master will periodically examine the registry and discard
information about unreachable agents according to two criteria:
`registry_max_agent_age` and `registry_max_agent_count`. The frequency
with which the master examines the registry is controlled by a third
parameter, `registry_gc_interval`.


Diffs (updated)
-

  src/master/constants.hpp cd80dace80968a1f67a8de5b2c112fb1396e26aa 
  src/master/flags.hpp c6e85303f60387f42b5e187eaedb6a01000f948f 
  src/master/flags.cpp 19ae6c1c30a1054b64a9585f325bd0bf943af218 
  src/master/master.hpp b118293823d37bba9b751aac12e0403318ce5736 
  src/master/master.cpp 2b4aff80385d02c71dc3eeff4ddda326c9824ede 
  src/master/registry.proto 03c896c39458b54b3966ce32c45eaa33c890ed28 
  src/tests/partition_tests.cpp f3142ad8d50daafcdb70ad9dbb2772f8ba30db00 
  src/tests/registrar_tests.cpp b04fc92c66449f1094212a534899e16d5f6f01bd 

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


Testing
---

make check


Thanks,

Neil Conway

Re: Review Request 51374: Change registry update order on removal, mark-unreachable.

[Neil Conway]

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

(Updated Aug. 29, 2016, 9:52 a.m.)


Review request for mesos and Vinod Kone.


Changes
---

Rebase.


Bugs: MESOS-4049 and MESOS-6090
https://issues.apache.org/jira/browse/MESOS-4049
https://issues.apache.org/jira/browse/MESOS-6090


Repository: mesos


Description
---

This commit changes the master so that we always update the registry
for an important change (e.g., removing an agent or marking an agent
unreachable), before updating the important parts of the master's
in-memory state (e.g., the in-memory list of registered agents).
Previously, the registry was updated first for registration and
reregistration, but second for removal and marking unreachable.
Updating the registry first is simpler, and also avoids possibly
leaking inaccurate information (e.g., via HTTP endpoints) if the
registry operation fails.


Diffs (updated)
-

  src/master/master.hpp b118293823d37bba9b751aac12e0403318ce5736 
  src/master/master.cpp 2b4aff80385d02c71dc3eeff4ddda326c9824ede 
  src/tests/master_tests.cpp 6cde15fcd6ca8ec40438c75aed980e83f8de9b86 
  src/tests/partition_tests.cpp f3142ad8d50daafcdb70ad9dbb2772f8ba30db00 
  src/tests/slave_tests.cpp 701fab20091f9f7df020290b9f0a9db1dec92647 

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


Testing
---

make check


Thanks,

Neil Conway

Re: Review Request 51371: Fixed flakiness in MasterAuthorizationTest.SlaveDisconnected.

[Neil Conway]

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

(Updated Aug. 29, 2016, 9:51 a.m.)


Review request for mesos and Vinod Kone.


Changes
---

Rebase, fixes.


Summary (updated)
-

Fixed flakiness in MasterAuthorizationTest.SlaveDisconnected.


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


Repository: mesos


Description (updated)
---

The goal of this test is to check this interleaving:

  task launch: block on authorization
  slave terminates gracefully: removed by master
  auth succeeds: we expect the task launch to fail

However, the test neglected to ensure that slave removal was completed
before allowing authorization to succeed.


Diffs (updated)
-

  src/tests/master_authorization_tests.cpp 
a6399f1b958705b87418b40d3771b25a37d49682 

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


Testing
---

make check


Thanks,

Neil Conway

Re: Review Request 50845: Added `unreachable_time` to TaskStatus.

[Neil Conway]

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

(Updated Aug. 29, 2016, 9:50 a.m.)


Review request for mesos and Vinod Kone.


Changes
---

Rebase, fix bug.


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


Repository: mesos


Description
---

This field contains the time at which the master marked an agent as
unreachable. It will only be set for status updates (including
reconciliation updates) that describe tasks running on unreachable
(e.g., partitioned) agents.

The intent is to help frameworks decide how to handle tasks running on
partitioned agents: since the framework might have missed the initial
TASK_LOST/TASK_UNREACHABLE status update (e.g., due to framework
downtime/failover), it might otherwise be difficult for frameworks to
determine how long an agent has been partitioned.

This unreachable timestamp is stored in the registry and loaded as part
of master recovery.


Diffs (updated)
-

  include/mesos/mesos.proto 7fbcdf005718ba4d7ec3b02cf70a3fc53d4f4818 
  include/mesos/v1/mesos.proto 60ec0cce35e5e2d2f4907efadc7ab92680566ef0 
  src/common/protobuf_utils.hpp 3324838f0a54491024b3e26ddf38afbfad31b8e4 
  src/common/protobuf_utils.cpp ed3ac7fcdd19bd517c30ee64d4746ea7c01628f1 
  src/master/master.hpp b118293823d37bba9b751aac12e0403318ce5736 
  src/master/master.cpp 2b4aff80385d02c71dc3eeff4ddda326c9824ede 
  src/tests/partition_tests.cpp f3142ad8d50daafcdb70ad9dbb2772f8ba30db00 
  src/tests/reconciliation_tests.cpp 8e438bfbce508a074f0d54513cd752344238e3f2 
  src/tests/registrar_tests.cpp b04fc92c66449f1094212a534899e16d5f6f01bd 

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


Testing
---

make check


Thanks,

Neil Conway

Re: Review Request 50846: Added more assertions to the master.

[Neil Conway]

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

(Updated Aug. 29, 2016, 9:50 a.m.)


Review request for mesos and Vinod Kone.


Changes
---

Rebase.


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


Repository: mesos


Description
---

Added more assertions to the master.


Diffs (updated)
-

  src/master/master.hpp b118293823d37bba9b751aac12e0403318ce5736 
  src/master/master.cpp 2b4aff80385d02c71dc3eeff4ddda326c9824ede 

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


Testing
---

make check


Thanks,

Neil Conway

Re: Review Request 50705: Changed master to allow partitioned slaves to reregister.

[Neil Conway]

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

(Updated Aug. 29, 2016, 9:48 a.m.)


Review request for mesos and Vinod Kone.


Changes
---

Various changes per review comments.


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


Repository: mesos


Description
---

The previous behavior was to shutdown partitioned agents that attempt to
reregister---unless the master has failed over, in which case the
reregistration is allowed (when running in "non-strict" mode).

The new behavior is always to allow partitioned agents to reregister.
This is part of a longer-term project to allow frameworks to define
their own policies for handling tasks running on partitioned agents.

In particular, if a framework has the PARTITION_AWARE capability, any
tasks running on the partitioned agent will continue to run after
reregistration. If the framework is not PARTITION_AWARE, any tasks that
were running on such an agent will be killed after the agent reregisters
(unless the master has failed over). This is for backward compatibility
with the previous ("non-strict") behavior. Note that regardless of the
PARTITION_AWARE capability, the agent will not be shutdown, which is a
change from the previous Mesos behavior.

This commit also changes the master so that if an agent is removed and
then the master receives a message from that agent, the master will no
longer attempt to shutdown the agent. This is consistent with the goal
of getting the master out of the business of shutting down agents that
we suspect are unhealthy. Such an agent will eventually realize it is
not registered with the master (e.g., because it won't receive any pings
from the master), which will cause it to reregister.


Diffs (updated)
-

  src/master/master.hpp b118293823d37bba9b751aac12e0403318ce5736 
  src/master/master.cpp 2b4aff80385d02c71dc3eeff4ddda326c9824ede 
  src/tests/master_tests.cpp 6cde15fcd6ca8ec40438c75aed980e83f8de9b86 
  src/tests/partition_tests.cpp f3142ad8d50daafcdb70ad9dbb2772f8ba30db00 

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


Testing
---

make check


Thanks,

Neil Conway

Re: Review Request 50706: Added test cases for PARTITION_AWARE behavior.

[Neil Conway]

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

(Updated Aug. 29, 2016, 9:49 a.m.)


Review request for mesos and Vinod Kone.


Changes
---

Simplify test code.


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


Repository: mesos


Description
---

Added test cases for PARTITION_AWARE behavior.


Diffs (updated)
-

  src/tests/partition_tests.cpp f3142ad8d50daafcdb70ad9dbb2772f8ba30db00 

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


Testing
---

make check


Thanks,

Neil Conway

Re: Review Request 50705: Changed master to allow partitioned slaves to reregister.

[Neil Conway]

> On Aug. 23, 2016, 12:56 a.m., Vinod Kone wrote:
> > src/tests/partition_tests.cpp, line 169
> > 
> >
> > This test is huge. Can you split PARTITION_AWARE and 
> > non-PARTITION_AWARE into separate tests?
> 
> Neil Conway wrote:
> It's only ~200 lines of code. I think there is some value in keeping the 
> PARTITION_AWARE and non-PARTITION_AWARE cases in the same test: in most 
> situations, both cases should behave the same, so we avoid redundancy; it 
> also makes the situations where different behavior is expected more clear. It 
> would be easy for the two cases to get out of sync if they are written 
> separately.

Per discussion, I split this into two separate tests.


- Neil


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


On Aug. 25, 2016, 8:23 a.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50705/
> ---
> 
> (Updated Aug. 25, 2016, 8:23 a.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-4049
> https://issues.apache.org/jira/browse/MESOS-4049
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The previous behavior was to shutdown partitioned agents that attempt to
> reregister---unless the master has failed over, in which case the
> reregistration is allowed (when running in "non-strict" mode).
> 
> The new behavior is always to allow partitioned agents to reregister.
> This is part of a longer-term project to allow frameworks to define
> their own policies for handling tasks running on partitioned agents.
> 
> In particular, if a framework has the PARTITION_AWARE capability, any
> tasks running on the partitioned agent will continue to run after
> reregistration. If the framework is not PARTITION_AWARE, any tasks that
> were running on such an agent will be killed after the agent reregisters
> (unless the master has failed over). This is for backward compatibility
> with the previous ("non-strict") behavior. Note that regardless of the
> PARTITION_AWARE capability, the agent will not be shutdown, which is a
> change from the previous Mesos behavior.
> 
> This commit also changes the master so that if an agent is removed and
> then the master receives a message from that agent, the master will no
> longer attempt to shutdown the agent. This is consistent with the goal
> of getting the master out of the business of shutting down agents that
> we suspect are unhealthy. Such an agent will eventually realize it is
> not registered with the master (e.g., because it won't receive any pings
> from the master), which will cause it to reregister.
> 
> 
> Diffs
> -
> 
>   src/master/master.hpp 6decff6f4b9c3434de030fd5c06df4c683a7abad 
>   src/master/master.cpp 789fb9760a5ea60ce3b7b08f17e36cec69349c60 
>   src/tests/master_tests.cpp 398164d09b8ef14f916122ed8780023c4a3cd0f6 
>   src/tests/partition_tests.cpp 0a72b345538ca3b9510fccf38ceb68ac71c2b473 
> 
> Diff: https://reviews.apache.org/r/50705/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 51470: Add the 'resources' field back to DRFSorter::Total.

[Anindya Sinha]

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



I think we should also bring back this function:
`const hashmap& total()`.

- Anindya Sinha


On Aug. 26, 2016, 10:53 p.m., Jiang Yan Xu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51470/
> ---
> 
> (Updated Aug. 26, 2016, 10:53 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Anindya Sinha, Michael Park, 
> and Neil Conway.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This is a partial reversal of 5a63f2a5d2a5fe0e5315a5b8f79b75b99a6a5893.
> We used to have this field but it turned out to be unnecessary for the
> current use cases so it was removed. We however will need it for the
> ongoing work to support tasks using shared resources.
> 
> The details can be found in https://reviews.apache.org/r/45961 but in
> essence, we now know of a case where the removal of resource quantities
> from the sorter depends on the identity. e.g., To determine if the
> quantity of a shared resource can be removed from the sorter we need to
> know if there are no longer instances of **the same** (hence identity)
> shared resource in the sorter.
> 
> 
> Diffs
> -
> 
>   src/master/allocator/sorter/drf/sorter.hpp 
> bc6bfb2d5d3b32d55be055a0514861b4e7d889bb 
>   src/master/allocator/sorter/drf/sorter.cpp 
> ac85b327fc33d34246788e6a8c8bf5a486c61434 
> 
> Diff: https://reviews.apache.org/r/51470/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jiang Yan Xu
> 
>

Re: Review Request 45961: Support sharing of resources through reference counting of resources.

[Anindya Sinha]

> On Aug. 25, 2016, 3:22 a.m., Jiang Yan Xu wrote:
> > src/tests/sorter_tests.cpp, lines 773-779
> > 
> >
> > Doesn't look like this block with `e` add much to the test coverage, 
> > remove it?

I removed 'd' and renamed 'e' as 'd'. I think we can keep 'd' (originally 'e') 
as it allows to show comparison between 3 clients when 1 of the clients is 
deactivated.


- Anindya


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


On Aug. 29, 2016, 7:21 a.m., Anindya Sinha wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45961/
> ---
> 
> (Updated Aug. 29, 2016, 7:21 a.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Joris Van Remoortere, Michael 
> Park, and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-4431
> https://issues.apache.org/jira/browse/MESOS-4431
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> o Each shared resource is accouted via its share count. This count is
>   updated based on the resource operations (such as add and subtract)
>   in various scenarios such as task launch and terminate at multiple
>   modules such as master, allocator, sorter, etc.
> o Only allow DESTROY if there are no running or pending tasks using
>   the volume. However, if the volume is in a pending offer to one or
>   more frameworks, rescind that offer before processing the DESTROY.
> o To allow multiple tasks to be launched in the same ACCEPT call
>   using the same shared resource, we update the allocator and
>   sorter with additional copies of shared resources to reflect the
>   true shared count of allocated shared resources.
> 
> 
> Diffs
> -
> 
>   src/common/resources.cpp a5f5902d8f7f2757e3aee35619bff5cc3a52f29b 
>   src/common/resources_utils.cpp 8e881a09500b0966b577e514077460f724868a8d 
>   src/master/allocator/mesos/hierarchical.hpp 
> bdbc6d3b5b959990538f4e3b7b1a3b031d9aea05 
>   src/master/allocator/mesos/hierarchical.cpp 
> 234ef98529964a0b6d3f132426a6c8ccbb1263ee 
>   src/master/allocator/sorter/drf/sorter.hpp 
> bc6bfb2d5d3b32d55be055a0514861b4e7d889bb 
>   src/master/allocator/sorter/drf/sorter.cpp 
> ac85b327fc33d34246788e6a8c8bf5a486c61434 
>   src/master/http.cpp 525ef6cd7545d25f3ac89a6325ace6e6d768262a 
>   src/master/master.hpp b118293823d37bba9b751aac12e0403318ce5736 
>   src/master/master.cpp 2b4aff80385d02c71dc3eeff4ddda326c9824ede 
>   src/master/validation.hpp 3205ee1e699d0fb7ca16ced96a07be3a07cf8956 
>   src/master/validation.cpp 15e2ecc2de99d6bed522f45f855ba686bf19c008 
>   src/tests/master_validation_tests.cpp 
> e440ff3f6d04f797e65874b9f610ed63d9f28e0e 
>   src/tests/sorter_tests.cpp 821e30d6574b045d25d4de4f7c3b8ac5346d3002 
>   src/v1/resources.cpp 172217505d80d66cb7e10b3635dc273229313601 
> 
> Diff: https://reviews.apache.org/r/45961/diff/
> 
> 
> Testing
> ---
> 
> Tests successful.
> 
> 
> Thanks,
> 
> Anindya Sinha
> 
>

Re: Review Request 45964: Add unit tests for sharing of resources.

[Anindya Sinha]

> On Aug. 25, 2016, 3:11 p.m., Jiang Yan Xu wrote:
> > src/tests/mesos.hpp, line 754
> > 
> >
> > The original comment was intended for the following three methods. Do 
> > you think we need to comment on each one? If so at least do it for LAUNCH 
> > as well and change plurals to singulars.

There are separate comments that exist for `RESERVE` and `UNRESERVE`, so let me 
add them for all the offer operations.


> On Aug. 25, 2016, 3:11 p.m., Jiang Yan Xu wrote:
> > src/tests/resources_tests.cpp, line 2871
> > 
> >
> > This test came up before IIRC. I don't think we need this as the 
> > existing tests already have "multiple consumers".

This test adds the test for the `shared()` API. I moved that to 
`TEST(SharedResourcesTest, ScalarAdditionShared)`, and also added check for 
`nonShared()`.


> On Aug. 25, 2016, 3:11 p.m., Jiang Yan Xu wrote:
> > src/tests/persistent_volume_tests.cpp, line 547
> > 
> >
> > It's unfortunate that we duplicate all the code from 
> > `PreparePersistentVolume` with only two lines of change.
> > 
> > This applies to `AccessSharedPersistentVolume` as well. 
> > 
> > If we want to test whether shared volume works on the slave, we should 
> > create a test for the more interesting case: multiple tasks accessing the 
> > same persistent volume. This test would be both necessary and also would 
> > cover both "preparing" and "accessing" the shared persistent volume 
> > therefore we don't need to test them separately.

Removed `PreparePersistentVolume `. Modified `AccessSharedPersistentVolume` 
with testing 2 tasks being able to write to the same shared persistent volume 
simultaneously.


> On Aug. 25, 2016, 3:11 p.m., Jiang Yan Xu wrote:
> > src/tests/resources_tests.cpp, line 2390
> > 
> >
> > Also verify `nonShared()`?
> > 
> > Plus, there is no operation (therefore shouldn't be in 
> > ResourcesOperationTest)?

Moved this test to `SharedResourcesTest.Filter`. Added check for `nonShared()` 
in addition to `shared()`.


- Anindya


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


On Aug. 29, 2016, 7:22 a.m., Anindya Sinha wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45964/
> ---
> 
> (Updated Aug. 29, 2016, 7:22 a.m.)
> 
> 
> Review request for mesos and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-4324
> https://issues.apache.org/jira/browse/MESOS-4324
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add unit tests for sharing of resources.
> 
> 
> Diffs
> -
> 
>   src/tests/hierarchical_allocator_tests.cpp 
> ddd48694e5c2d3d01fbff20558a46c1256b3dbc3 
>   src/tests/master_validation_tests.cpp 
> e440ff3f6d04f797e65874b9f610ed63d9f28e0e 
>   src/tests/mesos.hpp 4cae54b4df906d4b7e8fe8d40d5b0ad59d260e6f 
>   src/tests/persistent_volume_tests.cpp 
> a6f97c4bb5fb29d610c01255036095e2b30c44c5 
>   src/tests/resources_tests.cpp 4932d95f4e78cae764b89472373e13527b4354a2 
> 
> Diff: https://reviews.apache.org/r/45964/diff/
> 
> 
> Testing
> ---
> 
> Tests for shared resources added for allocator, resources and sorter.
> Tests successful.
> 
> 
> Thanks,
> 
> Anindya Sinha
> 
>

Re: Review Request 45961: Support sharing of resources through reference counting of resources.

[Anindya Sinha]

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

(Updated Aug. 29, 2016, 7:21 a.m.)


Review request for mesos, Benjamin Mahler, Joris Van Remoortere, Michael Park, 
and Jiang Yan Xu.


Changes
---

updateAllocation() for all offer operations.


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


Repository: mesos


Description
---

o Each shared resource is accouted via its share count. This count is
  updated based on the resource operations (such as add and subtract)
  in various scenarios such as task launch and terminate at multiple
  modules such as master, allocator, sorter, etc.
o Only allow DESTROY if there are no running or pending tasks using
  the volume. However, if the volume is in a pending offer to one or
  more frameworks, rescind that offer before processing the DESTROY.
o To allow multiple tasks to be launched in the same ACCEPT call
  using the same shared resource, we update the allocator and
  sorter with additional copies of shared resources to reflect the
  true shared count of allocated shared resources.


Diffs (updated)
-

  src/common/resources.cpp a5f5902d8f7f2757e3aee35619bff5cc3a52f29b 
  src/common/resources_utils.cpp 8e881a09500b0966b577e514077460f724868a8d 
  src/master/allocator/mesos/hierarchical.hpp 
bdbc6d3b5b959990538f4e3b7b1a3b031d9aea05 
  src/master/allocator/mesos/hierarchical.cpp 
234ef98529964a0b6d3f132426a6c8ccbb1263ee 
  src/master/allocator/sorter/drf/sorter.hpp 
bc6bfb2d5d3b32d55be055a0514861b4e7d889bb 
  src/master/allocator/sorter/drf/sorter.cpp 
ac85b327fc33d34246788e6a8c8bf5a486c61434 
  src/master/http.cpp 525ef6cd7545d25f3ac89a6325ace6e6d768262a 
  src/master/master.hpp b118293823d37bba9b751aac12e0403318ce5736 
  src/master/master.cpp 2b4aff80385d02c71dc3eeff4ddda326c9824ede 
  src/master/validation.hpp 3205ee1e699d0fb7ca16ced96a07be3a07cf8956 
  src/master/validation.cpp 15e2ecc2de99d6bed522f45f855ba686bf19c008 
  src/tests/master_validation_tests.cpp 
e440ff3f6d04f797e65874b9f610ed63d9f28e0e 
  src/tests/sorter_tests.cpp 821e30d6574b045d25d4de4f7c3b8ac5346d3002 
  src/v1/resources.cpp 172217505d80d66cb7e10b3635dc273229313601 

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


Testing
---

Tests successful.


Thanks,

Anindya Sinha

Re: Review Request 45964: Add unit tests for sharing of resources.

[Anindya Sinha]

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

(Updated Aug. 29, 2016, 7:22 a.m.)


Review request for mesos and Jiang Yan Xu.


Changes
---

Addressed review comments, and rebased.


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


Repository: mesos


Description
---

Add unit tests for sharing of resources.


Diffs (updated)
-

  src/tests/hierarchical_allocator_tests.cpp 
ddd48694e5c2d3d01fbff20558a46c1256b3dbc3 
  src/tests/master_validation_tests.cpp 
e440ff3f6d04f797e65874b9f610ed63d9f28e0e 
  src/tests/mesos.hpp 4cae54b4df906d4b7e8fe8d40d5b0ad59d260e6f 
  src/tests/persistent_volume_tests.cpp 
a6f97c4bb5fb29d610c01255036095e2b30c44c5 
  src/tests/resources_tests.cpp 4932d95f4e78cae764b89472373e13527b4354a2 

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


Testing
---

Tests for shared resources added for allocator, resources and sorter.
Tests successful.


Thanks,

Anindya Sinha

Re: Review Request 51412: Add offeredResources to Allocator::updateAllocation() API.

[Anindya Sinha]

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

(Updated Aug. 29, 2016, 7:20 a.m.)


Review request for mesos, Benjamin Mahler and Jiang Yan Xu.


Changes
---

Addressed review comments, and rebased.


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


Repository: mesos


Description
---

This is being added to allow for handling of ACCEPT call within the
allocator in the future. For now, we expose the offered resources
in this API to account for additional copies of shared resources in
the allocator when there are more tasks being launched in a single
ACCEPT call for a single copy of shared resource being offered.


Diffs (updated)
-

  include/mesos/allocator/allocator.hpp 
41a9d457286e30431490ca626e680d85684b48d6 
  src/master/allocator/mesos/allocator.hpp 
69639be9c14b83157df29f767e819db719588053 
  src/master/allocator/mesos/hierarchical.hpp 
bdbc6d3b5b959990538f4e3b7b1a3b031d9aea05 
  src/master/allocator/mesos/hierarchical.cpp 
234ef98529964a0b6d3f132426a6c8ccbb1263ee 
  src/master/master.hpp b118293823d37bba9b751aac12e0403318ce5736 
  src/master/master.cpp 2b4aff80385d02c71dc3eeff4ddda326c9824ede 
  src/tests/allocator.hpp 5ca3057265eb2c00c965132ac7922019e1cc77b8 
  src/tests/hierarchical_allocator_tests.cpp 
ddd48694e5c2d3d01fbff20558a46c1256b3dbc3 
  src/tests/reservation_tests.cpp 6181c6f8abbf254b45ea77f33c48dee918106ef0 

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


Testing
---

All tests passed.


Thanks,

Anindya Sinha