Re: Review Request 61015: Fixed cgroup device Entry.Selector major/minor number type.

[Gilbert Song]

> On July 20, 2017, 5:56 p.m., Benjamin Mahler wrote:
> > I think you'll want to reference this page in your description: 
> > http://man7.org/linux/man-pages/man3/makedev.3.html
> > 
> > The one you linked to seems to show MAJOR/MINOR returning `int` rather than 
> > `unsigned int`.

Makes sense. I will update. Thanks!


- Gilbert


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


On July 20, 2017, 5:51 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61015/
> ---
> 
> (Updated July 20, 2017, 5:51 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, haosdent huang, Jie Yu, Kevin 
> Klues, and Qian Zhang.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> A cgroup device is identified by a major and a minor number. They
> are 'unsigned int' type. A pair of  represents a
> 'dev_t'. For details:
> http://www.makelinux.net/ldd3/chp-3-sect-9
> 
> 
> Diffs
> -
> 
>   src/linux/cgroups.hpp eaf0dcad0ed38c507564624f1647e0c731b8b433 
>   src/linux/cgroups.cpp 334005abfc4ec9b20b7dc0212d852ba1f505dbb5 
> 
> 
> Diff: https://reviews.apache.org/r/61015/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 60932: Added protobuf scheme for blkio subsystem in CgroupInfo.

[Gilbert Song]

> On July 20, 2017, 1:27 a.m., Qian Zhang wrote:
> > include/mesos/v1/mesos.proto
> > Lines 2815 (patched)
> > 
> >
> > Why do we need this? I see `Entry.op` is a required field rather than 
> > an optional field, so why do we need `UNKNOWN` for a required field?
> 
> Gilbert Song wrote:
> We will use `UNKNOWN` in all protobuf enums:
> https://issues.apache.org/jira/browse/MESOS-4997
> 
> Qian Zhang wrote:
> Yeah, I saw that. But that is for backwards compatibility, and the 
> solution mentioned in that ticket is `use optional enum fields and include an 
> UNKNOWN value as the first entry in the enum`, but in this patch, the enum 
> field `Entry.op` is required.
> 
> Qian Zhang wrote:
> In the latest patch, I see you have made `Entry.op` an optional field, 
> however as you mentioned in r60933:
> > Operations are required for those repeated fields in blkio protobuf
> 
> So I think it should still be a required field.
> 
> And for the backward compatibility issue mentioned in MESOS-4997, 
> basically I think it will only happen when server (e.g., Mesos master) tries 
> to parse the serialized protobuf string from client (e.g., framework), I do 
> not think this is gonna hanppen in your case. So I would suggest to make 
> `Entry.op` an required field and remove `UNKNOWN` from the `Operation` enum. 
> There are some existing examples that we can referece: the `Image.Type`, 
> `ContainerInfo.Type` and `DiscoveryInfo.Visibility`.

After a second thought, I would still prefer `optional` since I don't think an 
optional field hurts here and they should only be set in the blkio subsystem. 
Also, considering we may upgrade to proto3 at some point, less `required` field 
may make it easier, thoughts?


- Gilbert


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


On July 20, 2017, 4:57 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60932/
> ---
> 
> (Updated July 20, 2017, 4:57 p.m.)
> 
> 
> Review request for mesos, haosdent huang, Jason Lai, Jie Yu, Qian Zhang, and 
> Zhitao Li.
> 
> 
> Bugs: MESOS-6162
> https://issues.apache.org/jira/browse/MESOS-6162
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Only statistics information for blkio in protobuf.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 8f8079bd7c2de4e8b2f8f9a56e2731b77b8e1575 
>   include/mesos/v1/mesos.proto 720f307f8d738b0787e7c47be7ee15be38b2c0d0 
> 
> 
> Diff: https://reviews.apache.org/r/60932/diff/3/
> 
> 
> Testing
> ---
> 
> make
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 60932: Added protobuf scheme for blkio subsystem in CgroupInfo.

[Gilbert Song]

> On July 20, 2017, 6:43 p.m., Qian Zhang wrote:
> > include/mesos/mesos.proto
> > Line 2843 (original), 2922 (patched)
> > 
> >
> > Why is `value` an optional field? I think there have to a value for any 
> > possible semantics, right?

The same, I don't think an `optional` field hurts here and it makes it more 
flexible. Let's chat tomorrow.


- Gilbert


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


On July 20, 2017, 4:57 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60932/
> ---
> 
> (Updated July 20, 2017, 4:57 p.m.)
> 
> 
> Review request for mesos, haosdent huang, Jason Lai, Jie Yu, Qian Zhang, and 
> Zhitao Li.
> 
> 
> Bugs: MESOS-6162
> https://issues.apache.org/jira/browse/MESOS-6162
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Only statistics information for blkio in protobuf.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 8f8079bd7c2de4e8b2f8f9a56e2731b77b8e1575 
>   include/mesos/v1/mesos.proto 720f307f8d738b0787e7c47be7ee15be38b2c0d0 
> 
> 
> Diff: https://reviews.apache.org/r/60932/diff/3/
> 
> 
> Testing
> ---
> 
> make
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Review Request 61018: Minor fix to use the more idiomatic `send` method.

[Jiang Yan Xu]

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

Review request for mesos and James Peach.


Repository: mesos


Description
---

Minor fix to use the more idiomatic `send` method.


Diffs
-

  src/log/network.hpp 635079fd7e42f3da3a595208c15f288d768424f7 


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


Testing
---

make check


Thanks,

Jiang Yan Xu

Re: Review Request 60933: Added stats/control helpers for the Blkio cgroup subsystem.

[Qian Zhang]

> On July 20, 2017, 8:39 p.m., Qian Zhang wrote:
> > src/linux/cgroups.cpp
> > Lines 1986 (patched)
> > 
> >
> > So this means there can be an entry without operation? But I think the 
> > possible semantics are:
> > ```
> > 8:0 Read 27660288
> > Total 1000
> > ```
> > So it seems not possible to have an entry without operation, instead it 
> > can be an entry without device number.
> 
> Gilbert Song wrote:
> So basically, for now we have:
> ```
> 8:0 Read 27660288
> Total 1000
> 7:0 100
> ```
> 
> Operations are required for those repeated fields in `blkio` protobuf. 
> Specifically `sectors` and `time` do not have an operation. Let me add some 
> comments here.

I see, thanks Gilbert!


- Qian


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


On July 21, 2017, 7:58 a.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60933/
> ---
> 
> (Updated July 21, 2017, 7:58 a.m.)
> 
> 
> Review request for mesos, haosdent huang, Jason Lai, Jie Yu, Qian Zhang, and 
> Zhitao Li.
> 
> 
> Bugs: MESOS-6162
> https://issues.apache.org/jira/browse/MESOS-6162
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> - Data structure for Blkio entities
> - Stats helpers for blkio.throttle.io* (generic blkio stats)
> - Stats helpers for blkio.io* (CFQ related stats)
> - Comments from the kernel blkio doc for helper functions
> 
> 
> Diffs
> -
> 
>   src/linux/cgroups.hpp eaf0dcad0ed38c507564624f1647e0c731b8b433 
>   src/linux/cgroups.cpp 334005abfc4ec9b20b7dc0212d852ba1f505dbb5 
> 
> 
> Diff: https://reviews.apache.org/r/60933/diff/3/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 60932: Added protobuf scheme for blkio subsystem in CgroupInfo.

[Qian Zhang]

> On July 20, 2017, 4:27 p.m., Qian Zhang wrote:
> > include/mesos/v1/mesos.proto
> > Lines 2815 (patched)
> > 
> >
> > Why do we need this? I see `Entry.op` is a required field rather than 
> > an optional field, so why do we need `UNKNOWN` for a required field?
> 
> Gilbert Song wrote:
> We will use `UNKNOWN` in all protobuf enums:
> https://issues.apache.org/jira/browse/MESOS-4997
> 
> Qian Zhang wrote:
> Yeah, I saw that. But that is for backwards compatibility, and the 
> solution mentioned in that ticket is `use optional enum fields and include an 
> UNKNOWN value as the first entry in the enum`, but in this patch, the enum 
> field `Entry.op` is required.

In the latest patch, I see you have made `Entry.op` an optional field, however 
as you mentioned in r60933:
> Operations are required for those repeated fields in blkio protobuf

So I think it should still be a required field.

And for the backward compatibility issue mentioned in MESOS-4997, basically I 
think it will only happen when server (e.g., Mesos master) tries to parse the 
serialized protobuf string from client (e.g., framework), I do not think this 
is gonna hanppen in your case. So I would suggest to make `Entry.op` an 
required field and remove `UNKNOWN` from the `Operation` enum. There are some 
existing examples that we can referece: the `Image.Type`, `ContainerInfo.Type` 
and `DiscoveryInfo.Visibility`.


- Qian


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


On July 21, 2017, 7:57 a.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60932/
> ---
> 
> (Updated July 21, 2017, 7:57 a.m.)
> 
> 
> Review request for mesos, haosdent huang, Jason Lai, Jie Yu, Qian Zhang, and 
> Zhitao Li.
> 
> 
> Bugs: MESOS-6162
> https://issues.apache.org/jira/browse/MESOS-6162
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Only statistics information for blkio in protobuf.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 8f8079bd7c2de4e8b2f8f9a56e2731b77b8e1575 
>   include/mesos/v1/mesos.proto 720f307f8d738b0787e7c47be7ee15be38b2c0d0 
> 
> 
> Diff: https://reviews.apache.org/r/60932/diff/3/
> 
> 
> Testing
> ---
> 
> make
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 60932: Added protobuf scheme for blkio subsystem in CgroupInfo.

[Qian Zhang]

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




include/mesos/mesos.proto
Line 2843 (original), 2922 (patched)


Why is `value` an optional field? I think there have to a value for any 
possible semantics, right?


- Qian Zhang


On July 21, 2017, 7:57 a.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60932/
> ---
> 
> (Updated July 21, 2017, 7:57 a.m.)
> 
> 
> Review request for mesos, haosdent huang, Jason Lai, Jie Yu, Qian Zhang, and 
> Zhitao Li.
> 
> 
> Bugs: MESOS-6162
> https://issues.apache.org/jira/browse/MESOS-6162
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Only statistics information for blkio in protobuf.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 8f8079bd7c2de4e8b2f8f9a56e2731b77b8e1575 
>   include/mesos/v1/mesos.proto 720f307f8d738b0787e7c47be7ee15be38b2c0d0 
> 
> 
> Diff: https://reviews.apache.org/r/60932/diff/3/
> 
> 
> Testing
> ---
> 
> make
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 61015: Fixed cgroup device Entry.Selector major/minor number type.

[Benjamin Mahler]

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


Ship it!




I think you'll want to reference this page in your description: 
http://man7.org/linux/man-pages/man3/makedev.3.html

The one you linked to seems to show MAJOR/MINOR returning `int` rather than 
`unsigned int`.

- Benjamin Mahler


On July 21, 2017, 12:51 a.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61015/
> ---
> 
> (Updated July 21, 2017, 12:51 a.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, haosdent huang, Jie Yu, Kevin 
> Klues, and Qian Zhang.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> A cgroup device is identified by a major and a minor number. They
> are 'unsigned int' type. A pair of  represents a
> 'dev_t'. For details:
> http://www.makelinux.net/ldd3/chp-3-sect-9
> 
> 
> Diffs
> -
> 
>   src/linux/cgroups.hpp eaf0dcad0ed38c507564624f1647e0c731b8b433 
>   src/linux/cgroups.cpp 334005abfc4ec9b20b7dc0212d852ba1f505dbb5 
> 
> 
> Diff: https://reviews.apache.org/r/61015/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Review Request 61015: Fixed cgroup device Entry.Selector major/minor number type.

[Gilbert Song]

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

Review request for mesos, Benjamin Mahler, haosdent huang, Jie Yu, Kevin Klues, 
and Qian Zhang.


Repository: mesos


Description
---

A cgroup device is identified by a major and a minor number. They
are 'unsigned int' type. A pair of  represents a
'dev_t'. For details:
http://www.makelinux.net/ldd3/chp-3-sect-9


Diffs
-

  src/linux/cgroups.hpp eaf0dcad0ed38c507564624f1647e0c731b8b433 
  src/linux/cgroups.cpp 334005abfc4ec9b20b7dc0212d852ba1f505dbb5 


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


Testing
---

make check


Thanks,

Gilbert Song

Re: Review Request 60915: Enabled filtering of reservations in the agent.

[Andrei Budnik]

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

(Updated July 21, 2017, 12:24 a.m.)


Review request for mesos, Alexander Rukletsov, Alexander Rojas, Benjamin 
Mahler, Greg Mann, and haosdent huang.


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


Repository: mesos


Description
---

Adds support of the `VIEW_ROLE` ACL to the results generated by the
`/state` endpoint in the agent for fields `reserved_resources_full`,
`reserved_resources` and `reserved_resources_allocated`.


Diffs
-

  src/slave/http.cpp 60640e5dbf6c3f8c709351e77ca89f287cf12fc3 


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


Testing
---

make check


Thanks,

Andrei Budnik

Re: Review Request 60933: Added stats/control helpers for the Blkio cgroup subsystem.

[Gilbert Song]

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

(Updated July 20, 2017, 4:58 p.m.)


Review request for mesos, haosdent huang, Jason Lai, Jie Yu, Qian Zhang, and 
Zhitao Li.


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


Repository: mesos


Description
---

- Data structure for Blkio entities
- Stats helpers for blkio.throttle.io* (generic blkio stats)
- Stats helpers for blkio.io* (CFQ related stats)
- Comments from the kernel blkio doc for helper functions


Diffs (updated)
-

  src/linux/cgroups.hpp eaf0dcad0ed38c507564624f1647e0c731b8b433 
  src/linux/cgroups.cpp 334005abfc4ec9b20b7dc0212d852ba1f505dbb5 


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

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


Testing
---

make check


Thanks,

Gilbert Song

Re: Review Request 60932: Added protobuf scheme for blkio subsystem in CgroupInfo.

[Gilbert Song]

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

(Updated July 20, 2017, 4:57 p.m.)


Review request for mesos, haosdent huang, Jason Lai, Jie Yu, Qian Zhang, and 
Zhitao Li.


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


Repository: mesos


Description
---

Only statistics information for blkio in protobuf.


Diffs (updated)
-

  include/mesos/mesos.proto 8f8079bd7c2de4e8b2f8f9a56e2731b77b8e1575 
  include/mesos/v1/mesos.proto 720f307f8d738b0787e7c47be7ee15be38b2c0d0 


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

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


Testing
---

make


Thanks,

Gilbert Song

Re: Review Request 60933: Added stats/control helpers for the Blkio cgroup subsystem.

[Gilbert Song]

> On July 20, 2017, 5:39 a.m., Qian Zhang wrote:
> > src/linux/cgroups.cpp
> > Lines 1986 (patched)
> > 
> >
> > So this means there can be an entry without operation? But I think the 
> > possible semantics are:
> > ```
> > 8:0 Read 27660288
> > Total 1000
> > ```
> > So it seems not possible to have an entry without operation, instead it 
> > can be an entry without device number.

So basically, for now we have:
```
8:0 Read 27660288
Total 1000
7:0 100
```

Operations are required for those repeated fields in `blkio` protobuf. 
Specifically `sectors` and `time` do not have an operation. Let me add some 
comments here.


> On July 20, 2017, 5:39 a.m., Qian Zhang wrote:
> > src/linux/cgroups.cpp
> > Lines 2005-2009 (patched)
> > 
> >
> > I think it should be:
> > ```
> >   return (s == "Total" ||
> >   s == "Read" ||
> >   s == "Write" ||
> >   s == "Sync" ||
> >   s == "Async");
> > ```
> > See `mesos::internal::protobuf::isTerminalState()` as an example.

Thanks, Qian!


- Gilbert


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


On July 19, 2017, 5:18 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60933/
> ---
> 
> (Updated July 19, 2017, 5:18 p.m.)
> 
> 
> Review request for mesos, haosdent huang, Jason Lai, Jie Yu, Qian Zhang, and 
> Zhitao Li.
> 
> 
> Bugs: MESOS-6162
> https://issues.apache.org/jira/browse/MESOS-6162
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> - Data structure for Blkio entities
> - Stats helpers for blkio.throttle.io* (generic blkio stats)
> - Stats helpers for blkio.io* (CFQ related stats)
> - Comments from the kernel blkio doc for helper functions
> 
> 
> Diffs
> -
> 
>   src/linux/cgroups.hpp eaf0dcad0ed38c507564624f1647e0c731b8b433 
>   src/linux/cgroups.cpp 334005abfc4ec9b20b7dc0212d852ba1f505dbb5 
> 
> 
> Diff: https://reviews.apache.org/r/60933/diff/2/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 60913: Adds support for OpenSSL's ECDH handshake.

[Till Toenshoff]

> On July 20, 2017, 3:51 p.m., Till Toenshoff wrote:
> > 3rdparty/libprocess/src/openssl.cpp
> > Lines 294 (patched)
> > 
> >
> > Judging from the NGINX sources, it appears that `OPENSSL_NO_ECDH` got 
> > introduced by the configuration setup of OpenSSL 0.9.8. So any version 
> > before that possibly does not set this define when the feature is missing. 
> > This means we will have to guard against that.

As the feature indicator is not available before 0.9.8, we now have two options:
- check for the openssl version via `OPENSSL_VERSION_NUMBER`
- check for the presence of the needed function symbols via `configure.ac`

Using version comparisons should always be the last line of defence, IMHO.


- Till


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


On July 20, 2017, 12:37 p.m., Alexander Rojas wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60913/
> ---
> 
> (Updated July 20, 2017, 12:37 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Till Toenshoff.
> 
> 
> Bugs: MESOS-7792
> https://issues.apache.org/jira/browse/MESOS-7792
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Support for Elliptic Curve Diffie Hellman algorithm requires extra
> configuration parameters which weren't part of Mesos.
> 
> This patch enables the extra configuration to Mesos in order to
> support ECDH algorithm, it also adds the ssl flag
> `LIBPROCESS_SSL_ECDH_CURVES` which allows for the specification of
> a specific elliptic curve.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/ssl/flags.hpp 
> 13fa7a0cc9d6d6d6849976a3ce383263c51504d7 
>   3rdparty/libprocess/src/openssl.hpp 
> 7ded2c74b2f92aacfa0f366bd27d5e0df2b8f25c 
>   3rdparty/libprocess/src/openssl.cpp 
> e6f17e4591f573186e1dc9697e1e7b60a841fe4f 
>   3rdparty/libprocess/src/tests/ssl_tests.cpp 
> 8a14dcb865dfab34fb4d0d51f42a28a913fb7ace 
> 
> 
> Diff: https://reviews.apache.org/r/60913/diff/4/
> 
> 
> Testing
> ---
> 
> ```shell
> make check
> ```
> 
> Launched Mesos with only ECDHE handshake ciphers enabled
> 
> ```shell
> LIBPROCESS_SSL_ENABLED=1 \
> LIBPROCESS_SSL_KEY_FILE=/tmp/ssl/self-signed.key \
> LIBPROCESS_SSL_CERT_FILE=/tmp/ssl/self-signed.crt \
> LIBPROCESS_SSL_CIPHERS="ECDHE-RSA-AES128-GCM-SHA256:ECDHE-RSA-AES128-SHA256:ECDHE-RSA-AES128-SHA:ECDHE-RSA-AES256-GCM-SHA384:ECDHE-RSA-AES256-SHA384:ECDHE-RSA-AES256-SHA"
>  \
> ./bin/mesos-master.sh \
> --work_dir=/tmp/mesos/master \
> --log_dir=/tmp/mesos/master/log
> ```
> 
> Then in another shell:
> 
> ```shell
> http -v --verify=no https://${MESOS_MASTER_IP}:5050/state
> 
> # Launches a browser.
> open https://${MESOS_MASTER_IP}:5050/state
> 
> # List the set of supported ciphers.
> # Expected output:
> # >  Starting Nmap 7.50 ( https://nmap.org ) at 2017-07-18 11:41 CEST
> # >  Nmap scan report for ${MESOS_MASTER_HOSTNAME} (${MESOS_MASTER_IP})
> # >  Host is up (0.13s latency).
> # >  rDNS record for ${MESOS_MASTER_IP}: ${MESOS_MASTER_HOSTNAME}
> # >  
> # >  PORT STATE SERVICE
> # >  5050/tcp open  mmcc
> # >  | ssl-enum-ciphers:
> # >  |   TLSv1.2:
> # >  | ciphers:
> # >  |   TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256 (secp256r1) - A
> # >  |   TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256 (secp256r1) - A
> # >  |   TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA (secp256r1) - A
> # >  |   TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (secp256r1) - A
> # >  |   TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA384 (secp256r1) - A
> # >  |   TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA (secp256r1) - A
> # >  | compressors:
> # >  |   NULL
> # >  | cipher preference: server
> # >  |_  least strength: A
> # >  
> # >  Nmap done: 1 IP address (1 host up) scanned in 1.87 seconds
> wget https://svn.nmap.org/nmap/scripts/ssl-enum-ciphers.nse
> nmap --script ssl-enum-ciphers.nse -p 5050 ${MESOS_MASTER_IP}
> ```
> 
> 
> Thanks,
> 
> Alexander Rojas
> 
>

Re: Review Request 60719: Add test infrastructure for src/python/lib/mesos.

[Eric Chung]

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

(Updated July 20, 2017, 6:45 p.m.)


Review request for mesos, Armand Grillet, Jason Lai, and Kevin Klues.


Changes
---

remove unused import and add tox.ini


Repository: mesos


Description (updated)
---

Part of MESOS-7310, this patch adds the test infrastructure necessary
for reliably running unit tests for the mesos package located under
src/python/lib.

setup.py is added under src/python/lib to both define the Python package.
tox.ini is added under the same dir to enable automated unit tests via the 
command `tox`,
which run tests via pytest.

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


Diffs (updated)
-

  src/python/.gitignore 0d20b6487c61e7d1bde93acf4a14b7a89083a16d 
  src/python/lib/mesos/__init__.py e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 
  src/python/lib/requirements-test.in PRE-CREATION 
  src/python/lib/requirements.in PRE-CREATION 
  src/python/lib/setup.py PRE-CREATION 
  src/python/lib/tests/__init__.py PRE-CREATION 
  src/python/lib/tests/test_mesos.py PRE-CREATION 
  src/python/lib/tox.ini PRE-CREATION 
  support/mesos-style.py 48d816f72df65d77364769f812fc06afcd6b5aec 


Diff: https://reviews.apache.org/r/60719/diff/9/

Changes: https://reviews.apache.org/r/60719/diff/8-9/


Testing (updated)
---

1. install tox
2. cd src/python/lib
3. tox


Thanks,

Eric Chung

Re: Review Request 61001: Windows: Fixed `CreateProcess` error message.

[Michael Park]

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


Ship it!




Ship It!

- Michael Park


On July 20, 2017, 10:05 a.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61001/
> ---
> 
> (Updated July 20, 2017, 10:05 a.m.)
> 
> 
> Review request for mesos, Joseph Wu and Li Li.
> 
> 
> Bugs: MESOS-7817
> https://issues.apache.org/jira/browse/MESOS-7817
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The buffer conversion of the argument string was being printed instead
> of the argument string itself, leading to an error message with a bunch
> of bytes.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/os/windows/shell.hpp 
> 0d8d45b8808df978d6a7c00a622e47384de74369 
> 
> 
> Diff: https://reviews.apache.org/r/61001/diff/1/
> 
> 
> Testing
> ---
> 
> Built and ran `stout-tests` on Windows.
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>

Review Request 61005: Added a test to check for copy assignment of `net::IP::Network`.

[Avinash sridharan]

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

Review request for mesos and Benjamin Hindman.


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


Repository: mesos


Description
---

Added a test to check for copy assignment of `net::IP::Network`.


Diffs
-

  3rdparty/stout/tests/ip_tests.cpp dba0de6f0b6483f41e10ad38fe0d87946f186b9b 


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


Testing
---

~/dev/mesosphere/mesos/build$ ./3rdparty/stout/tests/stout-tests 
--gtest_filter="NetTest.CopyIPNetwork"
Note: Google Test filter = NetTest.CopyIPNetwork-
[==] Running 1 test from 1 test case.
[--] Global test environment set-up.
[--] 1 test from NetTest
[ RUN  ] NetTest.CopyIPNetwork
[   OK ] NetTest.CopyIPNetwork (0 ms)
[--] 1 test from NetTest (0 ms total)

[--] Global test environment tear-down
[==] 1 test from 1 test case ran. (0 ms total)
[  PASSED  ] 1 test.


Thanks,

Avinash sridharan

Review Request 61004: Added a copy assignment operator to `net::IP::Network`.

[Avinash sridharan]

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

Review request for mesos and Benjamin Hindman.


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


Repository: mesos


Description
---

Added a copy assignment operator to `net::IP::Network`.


Diffs
-

  3rdparty/stout/include/stout/ip.hpp 51000b4f275466d6ed29105c15941c920af73516 


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


Testing
---

make check


Thanks,

Avinash sridharan

Re: Review Request 60719: Add test infrastructure for src/python/lib/mesos.

[Eric Chung]

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




src/python/lib/test.sh
Lines 1 (patched)


replace with tox.ini


- Eric Chung


On July 17, 2017, 5:59 p.m., Eric Chung wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60719/
> ---
> 
> (Updated July 17, 2017, 5:59 p.m.)
> 
> 
> Review request for mesos, Armand Grillet, Jason Lai, and Kevin Klues.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Part of MESOS-7310, this patch adds the test infrastructure necessary
> for reliably running unit tests for the mesos package located under
> src/python/lib.
> 
> setup.py is added under src/python/lib to both define the Python package
> and to allow tests to be run via `python setup.py test`, which delegates
> tests to pytest.
> 
> Review: https://reviews.apache.org/r/60719/
> 
> 
> Diffs
> -
> 
>   src/python/.gitignore 0d20b6487c61e7d1bde93acf4a14b7a89083a16d 
>   src/python/lib/mesos/__init__.py e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 
>   src/python/lib/requirements-test.in PRE-CREATION 
>   src/python/lib/requirements.in PRE-CREATION 
>   src/python/lib/setup.py PRE-CREATION 
>   src/python/lib/test.sh PRE-CREATION 
>   src/python/lib/tests/__init__.py PRE-CREATION 
>   src/python/lib/tests/test_mesos.py PRE-CREATION 
>   support/mesos-style.py 48d816f72df65d77364769f812fc06afcd6b5aec 
> 
> 
> Diff: https://reviews.apache.org/r/60719/diff/8/
> 
> 
> Testing
> ---
> 
> under src/python/lib, run `bash test.sh`
> 
> 
> Thanks,
> 
> Eric Chung
> 
>

Re: Review Request 60719: Add test infrastructure for src/python/lib/mesos.

[Eric Chung]

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




src/python/lib/setup.py
Lines 21 (patched)


remove


- Eric Chung


On July 17, 2017, 5:59 p.m., Eric Chung wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60719/
> ---
> 
> (Updated July 17, 2017, 5:59 p.m.)
> 
> 
> Review request for mesos, Armand Grillet, Jason Lai, and Kevin Klues.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Part of MESOS-7310, this patch adds the test infrastructure necessary
> for reliably running unit tests for the mesos package located under
> src/python/lib.
> 
> setup.py is added under src/python/lib to both define the Python package
> and to allow tests to be run via `python setup.py test`, which delegates
> tests to pytest.
> 
> Review: https://reviews.apache.org/r/60719/
> 
> 
> Diffs
> -
> 
>   src/python/.gitignore 0d20b6487c61e7d1bde93acf4a14b7a89083a16d 
>   src/python/lib/mesos/__init__.py e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 
>   src/python/lib/requirements-test.in PRE-CREATION 
>   src/python/lib/requirements.in PRE-CREATION 
>   src/python/lib/setup.py PRE-CREATION 
>   src/python/lib/test.sh PRE-CREATION 
>   src/python/lib/tests/__init__.py PRE-CREATION 
>   src/python/lib/tests/test_mesos.py PRE-CREATION 
>   support/mesos-style.py 48d816f72df65d77364769f812fc06afcd6b5aec 
> 
> 
> Diff: https://reviews.apache.org/r/60719/diff/8/
> 
> 
> Testing
> ---
> 
> under src/python/lib, run `bash test.sh`
> 
> 
> Thanks,
> 
> Eric Chung
> 
>

Re: Review Request 61001: Windows: Fixed `CreateProcess` error message.

[Andrew Schwartzmeyer]

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

(Updated July 20, 2017, 10:05 a.m.)


Review request for mesos, Joseph Wu and Li Li.


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


Repository: mesos


Description
---

The buffer conversion of the argument string was being printed instead
of the argument string itself, leading to an error message with a bunch
of bytes.


Diffs
-

  3rdparty/stout/include/stout/os/windows/shell.hpp 
0d8d45b8808df978d6a7c00a622e47384de74369 


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


Testing
---

Built and ran `stout-tests` on Windows.


Thanks,

Andrew Schwartzmeyer

Review Request 61001: Windows: Fixed `CreateProcess` error message.

[Andrew Schwartzmeyer]

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

Review request for mesos and Joseph Wu.


Repository: mesos


Description
---

The buffer conversion of the argument string was being printed instead
of the argument string itself, leading to an error message with a bunch
of bytes.


Diffs
-

  3rdparty/stout/include/stout/os/windows/shell.hpp 
0d8d45b8808df978d6a7c00a622e47384de74369 


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


Testing
---

Built and ran `stout-tests` on Windows.


Thanks,

Andrew Schwartzmeyer

Re: Review Request 60996: Adds documentation for LIBPROCESS_SSL_ECDH_CURVE environment variable.

[Till Toenshoff]

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




docs/ssl.md
Lines 78 (patched)


s/The valid values depends of OpenSSL version/Valid values depend on the 
OpenSSL version used/


- Till Toenshoff


On July 20, 2017, 12:38 p.m., Alexander Rojas wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60996/
> ---
> 
> (Updated July 20, 2017, 12:38 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Till Toenshoff.
> 
> 
> Bugs: MESOS-7792
> https://issues.apache.org/jira/browse/MESOS-7792
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This adds the adequate documentation entry for the new
> `LIBPROCESS_SSL_ECDH_CURVE` environment variable, which allow the
> configuration of ECDHE key exchange while establishing TLS sessions.
> 
> 
> Diffs
> -
> 
>   docs/ssl.md 811390f366c97b6d61cf9b3f188e3c399abb46cd 
> 
> 
> Diff: https://reviews.apache.org/r/60996/diff/1/
> 
> 
> Testing
> ---
> 
> non functional change.
> 
> 
> Thanks,
> 
> Alexander Rojas
> 
>

Re: Review Request 60913: Adds support for OpenSSL's ECDH handshake.

[Till Toenshoff]

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




3rdparty/libprocess/src/openssl.cpp
Lines 294 (patched)


Judging from the NGINX sources, it appears that `OPENSSL_NO_ECDH` got 
introduced by the configuration setup of OpenSSL 0.9.8. So any version before 
that possibly does not set this define when the feature is missing. This means 
we will have to guard against that.


- Till Toenshoff


On July 20, 2017, 12:37 p.m., Alexander Rojas wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60913/
> ---
> 
> (Updated July 20, 2017, 12:37 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Till Toenshoff.
> 
> 
> Bugs: MESOS-7792
> https://issues.apache.org/jira/browse/MESOS-7792
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Support for Elliptic Curve Diffie Hellman algorithm requires extra
> configuration parameters which weren't part of Mesos.
> 
> This patch enables the extra configuration to Mesos in order to
> support ECDH algorithm, it also adds the ssl flag
> `LIBPROCESS_SSL_ECDH_CURVES` which allows for the specification of
> a specific elliptic curve.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/ssl/flags.hpp 
> 13fa7a0cc9d6d6d6849976a3ce383263c51504d7 
>   3rdparty/libprocess/src/openssl.hpp 
> 7ded2c74b2f92aacfa0f366bd27d5e0df2b8f25c 
>   3rdparty/libprocess/src/openssl.cpp 
> e6f17e4591f573186e1dc9697e1e7b60a841fe4f 
>   3rdparty/libprocess/src/tests/ssl_tests.cpp 
> 8a14dcb865dfab34fb4d0d51f42a28a913fb7ace 
> 
> 
> Diff: https://reviews.apache.org/r/60913/diff/4/
> 
> 
> Testing
> ---
> 
> ```shell
> make check
> ```
> 
> Launched Mesos with only ECDHE handshake ciphers enabled
> 
> ```shell
> LIBPROCESS_SSL_ENABLED=1 \
> LIBPROCESS_SSL_KEY_FILE=/tmp/ssl/self-signed.key \
> LIBPROCESS_SSL_CERT_FILE=/tmp/ssl/self-signed.crt \
> LIBPROCESS_SSL_CIPHERS="ECDHE-RSA-AES128-GCM-SHA256:ECDHE-RSA-AES128-SHA256:ECDHE-RSA-AES128-SHA:ECDHE-RSA-AES256-GCM-SHA384:ECDHE-RSA-AES256-SHA384:ECDHE-RSA-AES256-SHA"
>  \
> ./bin/mesos-master.sh \
> --work_dir=/tmp/mesos/master \
> --log_dir=/tmp/mesos/master/log
> ```
> 
> Then in another shell:
> 
> ```shell
> http -v --verify=no https://${MESOS_MASTER_IP}:5050/state
> 
> # Launches a browser.
> open https://${MESOS_MASTER_IP}:5050/state
> 
> # List the set of supported ciphers.
> # Expected output:
> # >  Starting Nmap 7.50 ( https://nmap.org ) at 2017-07-18 11:41 CEST
> # >  Nmap scan report for ${MESOS_MASTER_HOSTNAME} (${MESOS_MASTER_IP})
> # >  Host is up (0.13s latency).
> # >  rDNS record for ${MESOS_MASTER_IP}: ${MESOS_MASTER_HOSTNAME}
> # >  
> # >  PORT STATE SERVICE
> # >  5050/tcp open  mmcc
> # >  | ssl-enum-ciphers:
> # >  |   TLSv1.2:
> # >  | ciphers:
> # >  |   TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256 (secp256r1) - A
> # >  |   TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256 (secp256r1) - A
> # >  |   TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA (secp256r1) - A
> # >  |   TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (secp256r1) - A
> # >  |   TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA384 (secp256r1) - A
> # >  |   TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA (secp256r1) - A
> # >  | compressors:
> # >  |   NULL
> # >  | cipher preference: server
> # >  |_  least strength: A
> # >  
> # >  Nmap done: 1 IP address (1 host up) scanned in 1.87 seconds
> wget https://svn.nmap.org/nmap/scripts/ssl-enum-ciphers.nse
> nmap --script ssl-enum-ciphers.nse -p 5050 ${MESOS_MASTER_IP}
> ```
> 
> 
> Thanks,
> 
> Alexander Rojas
> 
>

Re: Review Request 60931: Added test cases for framework streaming events.

[Mesos Reviewbot]

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



Patch looks great!

Reviews applied: [60928, 60929, 60930, 60931]

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 July 18, 2017, 12:29 a.m., Quinn Leng wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60931/
> ---
> 
> (Updated July 18, 2017, 12:29 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Greg Mann, and Zhitao Li.
> 
> 
> Bugs: MESOS-6101
> https://issues.apache.org/jira/browse/MESOS-6101
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added test cases for 'FRAMEWORK_ADDED', 'FRAMEWORK_UPDATED' and
> 'FRAMEWORK_REMOVED' events in v1 operator API.
> 
> 
> Diffs
> -
> 
>   src/tests/api_tests.cpp f22ca28c819712d8797e0c0c69dc1ebf1fe5ac1f 
> 
> 
> Diff: https://reviews.apache.org/r/60931/diff/2/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Quinn Leng
> 
>

Re: Review Request 60913: Adds support for OpenSSL's ECDH handshake.

[Till Toenshoff]

> On July 20, 2017, 3:21 p.m., Till Toenshoff wrote:
> > 3rdparty/libprocess/src/openssl.cpp
> > Lines 124 (patched)
> > 
> >
> > ```
> > "NOTE: OpenSSL versions below 1.0.2 support only..."
> > ```
> > 
> > This also needs to be part of `docs/ssl.md`.

... and this :)  sorry - I missed that RR.


- Till


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


On July 20, 2017, 12:37 p.m., Alexander Rojas wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60913/
> ---
> 
> (Updated July 20, 2017, 12:37 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Till Toenshoff.
> 
> 
> Bugs: MESOS-7792
> https://issues.apache.org/jira/browse/MESOS-7792
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Support for Elliptic Curve Diffie Hellman algorithm requires extra
> configuration parameters which weren't part of Mesos.
> 
> This patch enables the extra configuration to Mesos in order to
> support ECDH algorithm, it also adds the ssl flag
> `LIBPROCESS_SSL_ECDH_CURVES` which allows for the specification of
> a specific elliptic curve.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/ssl/flags.hpp 
> 13fa7a0cc9d6d6d6849976a3ce383263c51504d7 
>   3rdparty/libprocess/src/openssl.hpp 
> 7ded2c74b2f92aacfa0f366bd27d5e0df2b8f25c 
>   3rdparty/libprocess/src/openssl.cpp 
> e6f17e4591f573186e1dc9697e1e7b60a841fe4f 
>   3rdparty/libprocess/src/tests/ssl_tests.cpp 
> 8a14dcb865dfab34fb4d0d51f42a28a913fb7ace 
> 
> 
> Diff: https://reviews.apache.org/r/60913/diff/4/
> 
> 
> Testing
> ---
> 
> ```shell
> make check
> ```
> 
> Launched Mesos with only ECDHE handshake ciphers enabled
> 
> ```shell
> LIBPROCESS_SSL_ENABLED=1 \
> LIBPROCESS_SSL_KEY_FILE=/tmp/ssl/self-signed.key \
> LIBPROCESS_SSL_CERT_FILE=/tmp/ssl/self-signed.crt \
> LIBPROCESS_SSL_CIPHERS="ECDHE-RSA-AES128-GCM-SHA256:ECDHE-RSA-AES128-SHA256:ECDHE-RSA-AES128-SHA:ECDHE-RSA-AES256-GCM-SHA384:ECDHE-RSA-AES256-SHA384:ECDHE-RSA-AES256-SHA"
>  \
> ./bin/mesos-master.sh \
> --work_dir=/tmp/mesos/master \
> --log_dir=/tmp/mesos/master/log
> ```
> 
> Then in another shell:
> 
> ```shell
> http -v --verify=no https://${MESOS_MASTER_IP}:5050/state
> 
> # Launches a browser.
> open https://${MESOS_MASTER_IP}:5050/state
> 
> # List the set of supported ciphers.
> # Expected output:
> # >  Starting Nmap 7.50 ( https://nmap.org ) at 2017-07-18 11:41 CEST
> # >  Nmap scan report for ${MESOS_MASTER_HOSTNAME} (${MESOS_MASTER_IP})
> # >  Host is up (0.13s latency).
> # >  rDNS record for ${MESOS_MASTER_IP}: ${MESOS_MASTER_HOSTNAME}
> # >  
> # >  PORT STATE SERVICE
> # >  5050/tcp open  mmcc
> # >  | ssl-enum-ciphers:
> # >  |   TLSv1.2:
> # >  | ciphers:
> # >  |   TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256 (secp256r1) - A
> # >  |   TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256 (secp256r1) - A
> # >  |   TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA (secp256r1) - A
> # >  |   TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (secp256r1) - A
> # >  |   TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA384 (secp256r1) - A
> # >  |   TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA (secp256r1) - A
> # >  | compressors:
> # >  |   NULL
> # >  | cipher preference: server
> # >  |_  least strength: A
> # >  
> # >  Nmap done: 1 IP address (1 host up) scanned in 1.87 seconds
> wget https://svn.nmap.org/nmap/scripts/ssl-enum-ciphers.nse
> nmap --script ssl-enum-ciphers.nse -p 5050 ${MESOS_MASTER_IP}
> ```
> 
> 
> Thanks,
> 
> Alexander Rojas
> 
>

Re: Review Request 60913: Adds support for OpenSSL's ECDH handshake.

[Till Toenshoff]

> On July 20, 2017, 3:21 p.m., Till Toenshoff wrote:
> > 3rdparty/libprocess/src/openssl.cpp
> > Lines 118 (patched)
> > 
> >
> > We are still missing the documentation in `docs/ssl.md`.

Oops - drop this


- Till


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


On July 20, 2017, 12:37 p.m., Alexander Rojas wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60913/
> ---
> 
> (Updated July 20, 2017, 12:37 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Till Toenshoff.
> 
> 
> Bugs: MESOS-7792
> https://issues.apache.org/jira/browse/MESOS-7792
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Support for Elliptic Curve Diffie Hellman algorithm requires extra
> configuration parameters which weren't part of Mesos.
> 
> This patch enables the extra configuration to Mesos in order to
> support ECDH algorithm, it also adds the ssl flag
> `LIBPROCESS_SSL_ECDH_CURVES` which allows for the specification of
> a specific elliptic curve.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/ssl/flags.hpp 
> 13fa7a0cc9d6d6d6849976a3ce383263c51504d7 
>   3rdparty/libprocess/src/openssl.hpp 
> 7ded2c74b2f92aacfa0f366bd27d5e0df2b8f25c 
>   3rdparty/libprocess/src/openssl.cpp 
> e6f17e4591f573186e1dc9697e1e7b60a841fe4f 
>   3rdparty/libprocess/src/tests/ssl_tests.cpp 
> 8a14dcb865dfab34fb4d0d51f42a28a913fb7ace 
> 
> 
> Diff: https://reviews.apache.org/r/60913/diff/4/
> 
> 
> Testing
> ---
> 
> ```shell
> make check
> ```
> 
> Launched Mesos with only ECDHE handshake ciphers enabled
> 
> ```shell
> LIBPROCESS_SSL_ENABLED=1 \
> LIBPROCESS_SSL_KEY_FILE=/tmp/ssl/self-signed.key \
> LIBPROCESS_SSL_CERT_FILE=/tmp/ssl/self-signed.crt \
> LIBPROCESS_SSL_CIPHERS="ECDHE-RSA-AES128-GCM-SHA256:ECDHE-RSA-AES128-SHA256:ECDHE-RSA-AES128-SHA:ECDHE-RSA-AES256-GCM-SHA384:ECDHE-RSA-AES256-SHA384:ECDHE-RSA-AES256-SHA"
>  \
> ./bin/mesos-master.sh \
> --work_dir=/tmp/mesos/master \
> --log_dir=/tmp/mesos/master/log
> ```
> 
> Then in another shell:
> 
> ```shell
> http -v --verify=no https://${MESOS_MASTER_IP}:5050/state
> 
> # Launches a browser.
> open https://${MESOS_MASTER_IP}:5050/state
> 
> # List the set of supported ciphers.
> # Expected output:
> # >  Starting Nmap 7.50 ( https://nmap.org ) at 2017-07-18 11:41 CEST
> # >  Nmap scan report for ${MESOS_MASTER_HOSTNAME} (${MESOS_MASTER_IP})
> # >  Host is up (0.13s latency).
> # >  rDNS record for ${MESOS_MASTER_IP}: ${MESOS_MASTER_HOSTNAME}
> # >  
> # >  PORT STATE SERVICE
> # >  5050/tcp open  mmcc
> # >  | ssl-enum-ciphers:
> # >  |   TLSv1.2:
> # >  | ciphers:
> # >  |   TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256 (secp256r1) - A
> # >  |   TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256 (secp256r1) - A
> # >  |   TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA (secp256r1) - A
> # >  |   TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (secp256r1) - A
> # >  |   TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA384 (secp256r1) - A
> # >  |   TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA (secp256r1) - A
> # >  | compressors:
> # >  |   NULL
> # >  | cipher preference: server
> # >  |_  least strength: A
> # >  
> # >  Nmap done: 1 IP address (1 host up) scanned in 1.87 seconds
> wget https://svn.nmap.org/nmap/scripts/ssl-enum-ciphers.nse
> nmap --script ssl-enum-ciphers.nse -p 5050 ${MESOS_MASTER_IP}
> ```
> 
> 
> Thanks,
> 
> Alexander Rojas
> 
>

Re: Review Request 60913: Adds support for OpenSSL's ECDH handshake.

[Till Toenshoff]

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




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


We are still missing the documentation in `docs/ssl.md`.



3rdparty/libprocess/src/openssl.cpp
Lines 124 (patched)


```
"NOTE: OpenSSL versions below 1.0.2 support only..."
```

This also needs to be part of `docs/ssl.md`.



3rdparty/libprocess/src/openssl.cpp
Lines 307 (patched)


s/defined()/



3rdparty/libprocess/src/openssl.cpp
Lines 310 (patched)


Why this blank?



3rdparty/libprocess/src/openssl.cpp
Lines 344 (patched)


What is the reason we are exiting instead of returning an error like above?

Also can you please check if we could possibly use `error_string` again?



3rdparty/libprocess/src/openssl.cpp
Lines 352 (patched)


s/defined()/


- Till Toenshoff


On July 20, 2017, 12:37 p.m., Alexander Rojas wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60913/
> ---
> 
> (Updated July 20, 2017, 12:37 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Till Toenshoff.
> 
> 
> Bugs: MESOS-7792
> https://issues.apache.org/jira/browse/MESOS-7792
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Support for Elliptic Curve Diffie Hellman algorithm requires extra
> configuration parameters which weren't part of Mesos.
> 
> This patch enables the extra configuration to Mesos in order to
> support ECDH algorithm, it also adds the ssl flag
> `LIBPROCESS_SSL_ECDH_CURVES` which allows for the specification of
> a specific elliptic curve.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/ssl/flags.hpp 
> 13fa7a0cc9d6d6d6849976a3ce383263c51504d7 
>   3rdparty/libprocess/src/openssl.hpp 
> 7ded2c74b2f92aacfa0f366bd27d5e0df2b8f25c 
>   3rdparty/libprocess/src/openssl.cpp 
> e6f17e4591f573186e1dc9697e1e7b60a841fe4f 
>   3rdparty/libprocess/src/tests/ssl_tests.cpp 
> 8a14dcb865dfab34fb4d0d51f42a28a913fb7ace 
> 
> 
> Diff: https://reviews.apache.org/r/60913/diff/4/
> 
> 
> Testing
> ---
> 
> ```shell
> make check
> ```
> 
> Launched Mesos with only ECDHE handshake ciphers enabled
> 
> ```shell
> LIBPROCESS_SSL_ENABLED=1 \
> LIBPROCESS_SSL_KEY_FILE=/tmp/ssl/self-signed.key \
> LIBPROCESS_SSL_CERT_FILE=/tmp/ssl/self-signed.crt \
> LIBPROCESS_SSL_CIPHERS="ECDHE-RSA-AES128-GCM-SHA256:ECDHE-RSA-AES128-SHA256:ECDHE-RSA-AES128-SHA:ECDHE-RSA-AES256-GCM-SHA384:ECDHE-RSA-AES256-SHA384:ECDHE-RSA-AES256-SHA"
>  \
> ./bin/mesos-master.sh \
> --work_dir=/tmp/mesos/master \
> --log_dir=/tmp/mesos/master/log
> ```
> 
> Then in another shell:
> 
> ```shell
> http -v --verify=no https://${MESOS_MASTER_IP}:5050/state
> 
> # Launches a browser.
> open https://${MESOS_MASTER_IP}:5050/state
> 
> # List the set of supported ciphers.
> # Expected output:
> # >  Starting Nmap 7.50 ( https://nmap.org ) at 2017-07-18 11:41 CEST
> # >  Nmap scan report for ${MESOS_MASTER_HOSTNAME} (${MESOS_MASTER_IP})
> # >  Host is up (0.13s latency).
> # >  rDNS record for ${MESOS_MASTER_IP}: ${MESOS_MASTER_HOSTNAME}
> # >  
> # >  PORT STATE SERVICE
> # >  5050/tcp open  mmcc
> # >  | ssl-enum-ciphers:
> # >  |   TLSv1.2:
> # >  | ciphers:
> # >  |   TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256 (secp256r1) - A
> # >  |   TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256 (secp256r1) - A
> # >  |   TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA (secp256r1) - A
> # >  |   TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (secp256r1) - A
> # >  |   TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA384 (secp256r1) - A
> # >  |   TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA (secp256r1) - A
> # >  | compressors:
> # >  |   NULL
> # >  | cipher preference: server
> # >  |_  least strength: A
> # >  
> # >  Nmap done: 1 IP address (1 host up) scanned in 1.87 seconds
> wget https://svn.nmap.org/nmap/scripts/ssl-enum-ciphers.nse
> nmap --script ssl-enum-ciphers.nse -p 5050 ${MESOS_MASTER_IP}
> ```
> 
> 
> Thanks,
> 
> Alexander Rojas
> 
>

Re: Review Request 60915: Enabled filtering of reservations in the agent.

[Andrei Budnik]

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

(Updated July 20, 2017, 1:59 p.m.)


Review request for mesos, Alexander Rojas, Benjamin Mahler, and haosdent huang.


Repository: mesos


Description
---

Adds support of the `VIEW_ROLE` ACL to the results generated by the
`/state` endpoint in the agent for fields `reserved_resources_full`,
`reserved_resources` and `reserved_resources_allocated`.


Diffs
-

  src/slave/http.cpp 60640e5dbf6c3f8c709351e77ca89f287cf12fc3 


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


Testing
---

make check


Thanks,

Andrei Budnik

Re: Review Request 60915: Enabled filtering of reservations in the agent.

[Andrei Budnik]

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

(Updated July 20, 2017, 1:58 p.m.)


Review request for mesos, Alexander Rojas, Benjamin Mahler, and haosdent huang.


Repository: mesos


Description
---

Adds support of the `VIEW_ROLE` ACL to the results generated by the
`/state` endpoint in the agent for fields `reserved_resources_full`,
`reserved_resources` and `reserved_resources_allocated`.


Diffs
-

  src/slave/http.cpp 60640e5dbf6c3f8c709351e77ca89f287cf12fc3 


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


Testing
---

make check


Thanks,

Andrei Budnik

Re: Review Request 60933: Added stats/control helpers for the Blkio cgroup subsystem.

[Qian Zhang]

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




src/linux/cgroups.hpp
Lines 582-583 (patched)


Remove `along with devices and types of operations`?



src/linux/cgroups.hpp
Lines 586-587 (original), 590-591 (patched)


Ditto.



src/linux/cgroups.cpp
Line 1994 (original), 1972 (patched)


Kill `stat`.



src/linux/cgroups.cpp
Lines 1986 (patched)


So this means there can be an entry without operation? But I think the 
possible semantics are:
```
8:0 Read 27660288
Total 1000
```
So it seems not possible to have an entry without operation, instead it can 
be an entry without device number.



src/linux/cgroups.cpp
Lines 2005-2009 (patched)


I think it should be:
```
  return (s == "Total" ||
  s == "Read" ||
  s == "Write" ||
  s == "Sync" ||
  s == "Async");
```
See `mesos::internal::protobuf::isTerminalState()` as an example.


- Qian Zhang


On July 20, 2017, 8:18 a.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60933/
> ---
> 
> (Updated July 20, 2017, 8:18 a.m.)
> 
> 
> Review request for mesos, haosdent huang, Jason Lai, Jie Yu, Qian Zhang, and 
> Zhitao Li.
> 
> 
> Bugs: MESOS-6162
> https://issues.apache.org/jira/browse/MESOS-6162
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> - Data structure for Blkio entities
> - Stats helpers for blkio.throttle.io* (generic blkio stats)
> - Stats helpers for blkio.io* (CFQ related stats)
> - Comments from the kernel blkio doc for helper functions
> 
> 
> Diffs
> -
> 
>   src/linux/cgroups.hpp eaf0dcad0ed38c507564624f1647e0c731b8b433 
>   src/linux/cgroups.cpp 334005abfc4ec9b20b7dc0212d852ba1f505dbb5 
> 
> 
> Diff: https://reviews.apache.org/r/60933/diff/2/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Review Request 60996: Adds documentation for LIBPROCESS_SSL_ECDH_CURVE environment variable.

[Alexander Rojas]

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

Review request for mesos, Jie Yu and Till Toenshoff.


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


Repository: mesos


Description
---

This adds the adequate documentation entry for the new
`LIBPROCESS_SSL_ECDH_CURVE` environment variable, which allow the
configuration of ECDHE key exchange while establishing TLS sessions.


Diffs
-

  docs/ssl.md 811390f366c97b6d61cf9b3f188e3c399abb46cd 


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


Testing
---

non functional change.


Thanks,

Alexander Rojas

Re: Review Request 60913: Adds support for OpenSSL's ECDH handshake.

[Alexander Rojas]

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

(Updated July 20, 2017, 2:37 p.m.)


Review request for mesos, Jie Yu and Till Toenshoff.


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


Repository: mesos


Description
---

Support for Elliptic Curve Diffie Hellman algorithm requires extra
configuration parameters which weren't part of Mesos.

This patch enables the extra configuration to Mesos in order to
support ECDH algorithm, it also adds the ssl flag
`LIBPROCESS_SSL_ECDH_CURVES` which allows for the specification of
a specific elliptic curve.


Diffs (updated)
-

  3rdparty/libprocess/include/process/ssl/flags.hpp 
13fa7a0cc9d6d6d6849976a3ce383263c51504d7 
  3rdparty/libprocess/src/openssl.hpp 7ded2c74b2f92aacfa0f366bd27d5e0df2b8f25c 
  3rdparty/libprocess/src/openssl.cpp e6f17e4591f573186e1dc9697e1e7b60a841fe4f 
  3rdparty/libprocess/src/tests/ssl_tests.cpp 
8a14dcb865dfab34fb4d0d51f42a28a913fb7ace 


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

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


Testing
---

```shell
make check
```

Launched Mesos with only ECDHE handshake ciphers enabled

```shell
LIBPROCESS_SSL_ENABLED=1 \
LIBPROCESS_SSL_KEY_FILE=/tmp/ssl/self-signed.key \
LIBPROCESS_SSL_CERT_FILE=/tmp/ssl/self-signed.crt \
LIBPROCESS_SSL_CIPHERS="ECDHE-RSA-AES128-GCM-SHA256:ECDHE-RSA-AES128-SHA256:ECDHE-RSA-AES128-SHA:ECDHE-RSA-AES256-GCM-SHA384:ECDHE-RSA-AES256-SHA384:ECDHE-RSA-AES256-SHA"
 \
./bin/mesos-master.sh \
--work_dir=/tmp/mesos/master \
--log_dir=/tmp/mesos/master/log
```

Then in another shell:

```shell
http -v --verify=no https://${MESOS_MASTER_IP}:5050/state

# Launches a browser.
open https://${MESOS_MASTER_IP}:5050/state

# List the set of supported ciphers.
# Expected output:
# >  Starting Nmap 7.50 ( https://nmap.org ) at 2017-07-18 11:41 CEST
# >  Nmap scan report for ${MESOS_MASTER_HOSTNAME} (${MESOS_MASTER_IP})
# >  Host is up (0.13s latency).
# >  rDNS record for ${MESOS_MASTER_IP}: ${MESOS_MASTER_HOSTNAME}
# >  
# >  PORT STATE SERVICE
# >  5050/tcp open  mmcc
# >  | ssl-enum-ciphers:
# >  |   TLSv1.2:
# >  | ciphers:
# >  |   TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256 (secp256r1) - A
# >  |   TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256 (secp256r1) - A
# >  |   TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA (secp256r1) - A
# >  |   TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (secp256r1) - A
# >  |   TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA384 (secp256r1) - A
# >  |   TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA (secp256r1) - A
# >  | compressors:
# >  |   NULL
# >  | cipher preference: server
# >  |_  least strength: A
# >  
# >  Nmap done: 1 IP address (1 host up) scanned in 1.87 seconds
wget https://svn.nmap.org/nmap/scripts/ssl-enum-ciphers.nse
nmap --script ssl-enum-ciphers.nse -p 5050 ${MESOS_MASTER_IP}
```


Thanks,

Alexander Rojas

Re: Review Request 60500: Introduced `--default_container_dns` agent flag.

[Qian Zhang]

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

(Updated July 20, 2017, 8:21 p.m.)


Review request for mesos, Avinash sridharan and Jie Yu.


Changes
---

Addressed comments.


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


Repository: mesos


Description
---

Introduced `--default_container_dns` agent flag.


Diffs (updated)
-

  docs/configuration.md a06ac82e7b908601b9ba97011931276e6292102d 
  src/messages/flags.hpp 70ad58c33067a0b058deafdcc4ffc5f554b6ca72 
  src/messages/flags.proto e87075f19714739b93d9f4aa33ea8686a1bb5613 
  src/slave/flags.hpp bf9adf03847a113306c1df2d71e839b90ada7dd3 
  src/slave/flags.cpp a4c1a0c62af5506bbeb1d26d2fd9ca0ba11d8ad7 


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

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


Testing
---


Thanks,

Qian Zhang

Re: Review Request 60913: Adds support for OpenSSL's ECDH handshake.

[Alexander Rojas]

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




3rdparty/libprocess/src/openssl.cpp
Lines 297 (patched)


I don't agree this is a function which should return `Try`, this 
is kind of a continuation of `reinitialize()` and as the other one, failures 
should be fatal.


- Alexander Rojas


On July 18, 2017, 2:13 p.m., Alexander Rojas wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60913/
> ---
> 
> (Updated July 18, 2017, 2:13 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Till Toenshoff.
> 
> 
> Bugs: MESOS-7792
> https://issues.apache.org/jira/browse/MESOS-7792
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Support for Elliptic Curve Diffie Hellman algorithm requires extra
> configuration parameters which weren't part of Mesos.
> 
> This patch enables the extra configuration to Mesos in order to
> support ECDH algorithm, it also adds the ssl flag
> `LIBPROCESS_SSL_ECDH_CURVES` which allows for the specification of
> a specific elliptic curve.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/ssl/flags.hpp 
> 13fa7a0cc9d6d6d6849976a3ce383263c51504d7 
>   3rdparty/libprocess/src/openssl.cpp 
> e6f17e4591f573186e1dc9697e1e7b60a841fe4f 
>   3rdparty/libprocess/src/tests/ssl_tests.cpp 
> 8a14dcb865dfab34fb4d0d51f42a28a913fb7ace 
> 
> 
> Diff: https://reviews.apache.org/r/60913/diff/3/
> 
> 
> Testing
> ---
> 
> ```shell
> make check
> ```
> 
> Launched Mesos with only ECDHE handshake ciphers enabled
> 
> ```shell
> LIBPROCESS_SSL_ENABLED=1 \
> LIBPROCESS_SSL_KEY_FILE=/tmp/ssl/self-signed.key \
> LIBPROCESS_SSL_CERT_FILE=/tmp/ssl/self-signed.crt \
> LIBPROCESS_SSL_CIPHERS="ECDHE-RSA-AES128-GCM-SHA256:ECDHE-RSA-AES128-SHA256:ECDHE-RSA-AES128-SHA:ECDHE-RSA-AES256-GCM-SHA384:ECDHE-RSA-AES256-SHA384:ECDHE-RSA-AES256-SHA"
>  \
> ./bin/mesos-master.sh \
> --work_dir=/tmp/mesos/master \
> --log_dir=/tmp/mesos/master/log
> ```
> 
> Then in another shell:
> 
> ```shell
> http -v --verify=no https://${MESOS_MASTER_IP}:5050/state
> 
> # Launches a browser.
> open https://${MESOS_MASTER_IP}:5050/state
> 
> # List the set of supported ciphers.
> # Expected output:
> # >  Starting Nmap 7.50 ( https://nmap.org ) at 2017-07-18 11:41 CEST
> # >  Nmap scan report for ${MESOS_MASTER_HOSTNAME} (${MESOS_MASTER_IP})
> # >  Host is up (0.13s latency).
> # >  rDNS record for ${MESOS_MASTER_IP}: ${MESOS_MASTER_HOSTNAME}
> # >  
> # >  PORT STATE SERVICE
> # >  5050/tcp open  mmcc
> # >  | ssl-enum-ciphers:
> # >  |   TLSv1.2:
> # >  | ciphers:
> # >  |   TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256 (secp256r1) - A
> # >  |   TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256 (secp256r1) - A
> # >  |   TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA (secp256r1) - A
> # >  |   TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (secp256r1) - A
> # >  |   TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA384 (secp256r1) - A
> # >  |   TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA (secp256r1) - A
> # >  | compressors:
> # >  |   NULL
> # >  | cipher preference: server
> # >  |_  least strength: A
> # >  
> # >  Nmap done: 1 IP address (1 host up) scanned in 1.87 seconds
> wget https://svn.nmap.org/nmap/scripts/ssl-enum-ciphers.nse
> nmap --script ssl-enum-ciphers.nse -p 5050 ${MESOS_MASTER_IP}
> ```
> 
> 
> Thanks,
> 
> Alexander Rojas
> 
>

Re: Review Request 60932: Added protobuf scheme for blkio subsystem in CgroupInfo.

[Qian Zhang]

> On July 20, 2017, 4:27 p.m., Qian Zhang wrote:
> > include/mesos/v1/mesos.proto
> > Lines 2815 (patched)
> > 
> >
> > Why do we need this? I see `Entry.op` is a required field rather than 
> > an optional field, so why do we need `UNKNOWN` for a required field?
> 
> Gilbert Song wrote:
> We will use `UNKNOWN` in all protobuf enums:
> https://issues.apache.org/jira/browse/MESOS-4997

Yeah, I saw that. But that is for backwards compatibility, and the solution 
mentioned in that ticket is `use optional enum fields and include an UNKNOWN 
value as the first entry in the enum`, but in this patch, the enum field 
`Entry.op` is required.


- Qian


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


On July 20, 2017, 8:17 a.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60932/
> ---
> 
> (Updated July 20, 2017, 8:17 a.m.)
> 
> 
> Review request for mesos, haosdent huang, Jason Lai, Jie Yu, Qian Zhang, and 
> Zhitao Li.
> 
> 
> Bugs: MESOS-6162
> https://issues.apache.org/jira/browse/MESOS-6162
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Only statistics information for blkio in protobuf.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto ab2a372184b7cfbaf7a38e90f487cba38c3e80b8 
>   include/mesos/v1/mesos.proto 5e92e5d86023ad6edd94303fbde964bf403abf02 
> 
> 
> Diff: https://reviews.apache.org/r/60932/diff/2/
> 
> 
> Testing
> ---
> 
> make
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 60933: Added stats/control helpers for the Blkio cgroup subsystem.

[Gilbert Song]

> On July 18, 2017, 7:27 a.m., Qian Zhang wrote:
> > src/linux/cgroups.cpp
> > Lines 1944 (patched)
> > 
> >
> > Should we replace `unsigned int` with `dev_t` like the code below?
> > https://github.com/apache/mesos/blob/1.3.0/src/linux/cgroups.cpp#L2572
> > 
> > Or actually I think the above code seems wrong, we should change it 
> > from `dev_t` to `unsigned int`.
> 
> Gilbert Song wrote:
> yea, should be `unsigned int` here for major or minor.
> 
> Gilbert Song wrote:
> https://github.com/apache/mesos/blob/1.3.0/src/linux/cgroups.cpp#L2572 
> can be fixed later.
> 
> Qian Zhang wrote:
> Agree, mind to post a patch to fix that? :-)

Sure, will come up with a separte patch later.


- Gilbert


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


On July 19, 2017, 5:18 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60933/
> ---
> 
> (Updated July 19, 2017, 5:18 p.m.)
> 
> 
> Review request for mesos, haosdent huang, Jason Lai, Jie Yu, Qian Zhang, and 
> Zhitao Li.
> 
> 
> Bugs: MESOS-6162
> https://issues.apache.org/jira/browse/MESOS-6162
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> - Data structure for Blkio entities
> - Stats helpers for blkio.throttle.io* (generic blkio stats)
> - Stats helpers for blkio.io* (CFQ related stats)
> - Comments from the kernel blkio doc for helper functions
> 
> 
> Diffs
> -
> 
>   src/linux/cgroups.hpp eaf0dcad0ed38c507564624f1647e0c731b8b433 
>   src/linux/cgroups.cpp 334005abfc4ec9b20b7dc0212d852ba1f505dbb5 
> 
> 
> Diff: https://reviews.apache.org/r/60933/diff/2/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 60932: Added protobuf scheme for blkio subsystem in CgroupInfo.

[Gilbert Song]

> On July 20, 2017, 1:27 a.m., Qian Zhang wrote:
> > include/mesos/v1/mesos.proto
> > Lines 2815 (patched)
> > 
> >
> > Why do we need this? I see `Entry.op` is a required field rather than 
> > an optional field, so why do we need `UNKNOWN` for a required field?

We will use `UNKNOWN` in all protobuf enums:
https://issues.apache.org/jira/browse/MESOS-4997


- Gilbert


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


On July 19, 2017, 5:17 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60932/
> ---
> 
> (Updated July 19, 2017, 5:17 p.m.)
> 
> 
> Review request for mesos, haosdent huang, Jason Lai, Jie Yu, Qian Zhang, and 
> Zhitao Li.
> 
> 
> Bugs: MESOS-6162
> https://issues.apache.org/jira/browse/MESOS-6162
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Only statistics information for blkio in protobuf.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto ab2a372184b7cfbaf7a38e90f487cba38c3e80b8 
>   include/mesos/v1/mesos.proto 5e92e5d86023ad6edd94303fbde964bf403abf02 
> 
> 
> Diff: https://reviews.apache.org/r/60932/diff/2/
> 
> 
> Testing
> ---
> 
> make
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 60933: Added stats/control helpers for the Blkio cgroup subsystem.

[Qian Zhang]

> On July 18, 2017, 10:27 p.m., Qian Zhang wrote:
> > src/linux/cgroups.hpp
> > Lines 427 (patched)
> > 
> >
> > Why do we need this?
> 
> Gilbert Song wrote:
> Yes, so that we can use an object as a `dev_t` key in hashmap.

I see, thanks Gilbert!


> On July 18, 2017, 10:27 p.m., Qian Zhang wrote:
> > src/linux/cgroups.hpp
> > Lines 429 (patched)
> > 
> >
> > I think there is no need to have a second `public:` in this class.
> 
> Gilbert Song wrote:
> A qualifier to distinguish `inline methods` to `static Try`. A 
> style choice, seems more clear to me in this way.

+1


> On July 18, 2017, 10:27 p.m., Qian Zhang wrote:
> > src/linux/cgroups.hpp
> > Lines 457 (patched)
> > 
> >
> > Why is this an optional field? Any chance there is no device in an 
> > entry?
> 
> Gilbert Song wrote:
> Two semantics:
> ```
> 8:0 Read 27660288
> Total 1000
> ```

Thanks Gilbert! Can you please put the possible semantics somewhere in the code 
as comments? It will be much easier for others to under the code logic:-)


> On July 18, 2017, 10:27 p.m., Qian Zhang wrote:
> > src/linux/cgroups.cpp
> > Lines 1944 (patched)
> > 
> >
> > Should we replace `unsigned int` with `dev_t` like the code below?
> > https://github.com/apache/mesos/blob/1.3.0/src/linux/cgroups.cpp#L2572
> > 
> > Or actually I think the above code seems wrong, we should change it 
> > from `dev_t` to `unsigned int`.
> 
> Gilbert Song wrote:
> yea, should be `unsigned int` here for major or minor.
> 
> Gilbert Song wrote:
> https://github.com/apache/mesos/blob/1.3.0/src/linux/cgroups.cpp#L2572 
> can be fixed later.

Agree, mind to post a patch to fix that? :-)


- Qian


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


On July 20, 2017, 8:18 a.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60933/
> ---
> 
> (Updated July 20, 2017, 8:18 a.m.)
> 
> 
> Review request for mesos, haosdent huang, Jason Lai, Jie Yu, Qian Zhang, and 
> Zhitao Li.
> 
> 
> Bugs: MESOS-6162
> https://issues.apache.org/jira/browse/MESOS-6162
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> - Data structure for Blkio entities
> - Stats helpers for blkio.throttle.io* (generic blkio stats)
> - Stats helpers for blkio.io* (CFQ related stats)
> - Comments from the kernel blkio doc for helper functions
> 
> 
> Diffs
> -
> 
>   src/linux/cgroups.hpp eaf0dcad0ed38c507564624f1647e0c731b8b433 
>   src/linux/cgroups.cpp 334005abfc4ec9b20b7dc0212d852ba1f505dbb5 
> 
> 
> Diff: https://reviews.apache.org/r/60933/diff/2/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 60932: Added protobuf scheme for blkio subsystem in CgroupInfo.

[Qian Zhang]

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




include/mesos/v1/mesos.proto
Lines 2815 (patched)


Why do we need this? I see `Entry.op` is a required field rather than an 
optional field, so why do we need `UNKNOWN` for a required field?


- Qian Zhang


On July 20, 2017, 8:17 a.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60932/
> ---
> 
> (Updated July 20, 2017, 8:17 a.m.)
> 
> 
> Review request for mesos, haosdent huang, Jason Lai, Jie Yu, Qian Zhang, and 
> Zhitao Li.
> 
> 
> Bugs: MESOS-6162
> https://issues.apache.org/jira/browse/MESOS-6162
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Only statistics information for blkio in protobuf.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto ab2a372184b7cfbaf7a38e90f487cba38c3e80b8 
>   include/mesos/v1/mesos.proto 5e92e5d86023ad6edd94303fbde964bf403abf02 
> 
> 
> Diff: https://reviews.apache.org/r/60932/diff/2/
> 
> 
> Testing
> ---
> 
> make
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 58939: Filesystem isolation check for Mesos image provisioner.

[Gilbert Song]

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




src/slave/containerizer/mesos/containerizer.cpp
Lines 1112-1126 (patched)


I don't like the checks here, since we have the following case:

what if we have a task with volumes specified in its containerinfo but no 
image?

Let's add `filesystem/isolator` check at docker::store::create().



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


s/is/are/g



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


s/to create a new mount namespace/to support container images/g



src/slave/containerizer/mesos/containerizer.cpp
Lines 1118-1119 (patched)


I would prefer:

The 'filesystem/linux' isolator must be enabled for container images 
support.



src/slave/containerizer/mesos/containerizer.cpp
Lines 1123-1124 (patched)


Ditto.



src/slave/containerizer/mesos/isolators/docker/runtime.cpp
Lines 70-79 (patched)


Basically we dont add isolator dependencies inside of any isolator, nor the 
launcher since the launcher is supposed to be a component for containerizer.



src/slave/containerizer/mesos/isolators/docker/runtime.cpp
Lines 71 (patched)


"The 'filesystem/linux' isolator ..."



src/tests/containerizer/docker_volume_isolator_tests.cpp
Lines 231 (patched)


No dependency on linux filesystem isolation.



src/tests/containerizer/docker_volume_isolator_tests.cpp
Lines 386 (patched)


Ditto.



src/tests/containerizer/docker_volume_isolator_tests.cpp
Lines 487 (patched)


Ditto.



src/tests/containerizer/docker_volume_isolator_tests.cpp
Lines 685 (patched)


Ditto.



src/tests/containerizer/mesos_containerizer_tests.cpp
Lines 906-911 (original), 914-930 (patched)


No need to change this test if you do the check in docker store create().



src/tests/containerizer/mesos_containerizer_tests.cpp
Lines 997-999 (original), 1016-1029 (patched)


Ditto.



src/tests/containerizer/mesos_containerizer_tests.cpp
Lines 1087-1089 (original), 1117-1130 (patched)


Ditto.


- Gilbert Song


On May 9, 2017, 11:08 a.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58939/
> ---
> 
> (Updated May 9, 2017, 11:08 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Gilbert Song, and Jie Yu.
> 
> 
> Bugs: mesos-7374
> https://issues.apache.org/jira/browse/mesos-7374
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Checked if the 'filesystem/linux' isolator is enabled and the 'linux'
> launcher is used when launching a mesos containerizer with an image
> under Linux. This prevents the executor from messing up with the host
> filesystem. The check is in `MesosContainerizerProcess::prepare()`
> after provisioning and before launching, since provisioning itself
> does not depend on the filesystem isolator.
> 
> Also checked that the 'filesystem/linux' is enabled and the 'linux'
> launcher is used when enabling the 'docker/runtime' isolator.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/containerizer.cpp 
> 58ab74571fb14c6dbb1907151dc421f93e324bb5 
>   src/slave/containerizer/mesos/isolators/docker/runtime.cpp 
> 2a6e0b179394e0485d2495ceb4bbbcb184af08fe 
>   src/tests/containerizer/docker_volume_isolator_tests.cpp 
> b47a6b5081a63ac474ac4634701b1a572eb58137 
>   src/tests/containerizer/mesos_containerizer_tests.cpp 
> 13e0f7e603a3ffdd0965b253d7abfe6a069cd2b4 
> 
> 
> Diff: https://reviews.apache.org/r/58939/diff/6/
> 
> 
> Testing
> ---
> 
> sudo make check
> Manually tested on a simplified case of mesos-7374.
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Re: Review Request 60991: Changed Device::path to optional and introduced Device::Number.

[Qian Zhang]

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


Ship it!




Ship It!

- Qian Zhang


On July 20, 2017, 8:16 a.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60991/
> ---
> 
> (Updated July 20, 2017, 8:16 a.m.)
> 
> 
> Review request for mesos, haosdent huang, Jason Lai, Jie Yu, Qian Zhang, and 
> Zhongbo Tian.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> A device in cgroup can be represented as either a path or a major:minor
> number. Need to change the `required` Device::path to `optional` to
> add Number in the device protobuf.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto ab2a372184b7cfbaf7a38e90f487cba38c3e80b8 
>   include/mesos/v1/mesos.proto 5e92e5d86023ad6edd94303fbde964bf403abf02 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystems/devices.cpp 
> ddf2a4d661001a3a5832c21504420223ca60a753 
> 
> 
> Diff: https://reviews.apache.org/r/60991/diff/1/
> 
> 
> Testing
> ---
> 
> make
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 60791: Add fetcher cache space usage metrics.

[Jiang Yan Xu]

> On July 18, 2017, 2:16 p.m., Jiang Yan Xu wrote:
> > src/slave/containerizer/fetcher.cpp
> > Lines 272-275 (patched)
> > 
> >
> > Aside from styling/convention, would this require defer?
> 
> James Peach wrote:
> No, this is avoiding the defer on purpose.

So `tally` is concurrently modified by another thead, is the reason this is 
thread-safe that `Bytes` is a simple class with one `uint64_t` field? Even in 
this case is it always safe? Is there a definitive reference that could help me 
be convinced?

Even if it is the case I think this could be subtle. I thought the original 
intention for /r/59858/ is for constants, which would be more easy to reason 
about correctness?

In terms of the performance gains, I am not sure it's worthwhile make an 
exception here given this is on the agent and thus not a performance 
bottleneck. (I am not against something that can be proposed as a general 
recommendations which we can adopt for more than one use.)

To extend from this, is metrics collection special? So far all concurrent 
accesses to Process internals are protected by dispatches, what to do about 
those?


> On July 18, 2017, 2:16 p.m., Jiang Yan Xu wrote:
> > src/slave/containerizer/fetcher.cpp
> > Lines 293-295 (patched)
> > 
> >
> > Why await?
> 
> James Peach wrote:
> This synchronizes the metric removal so that it can't be evaluated after 
> the process has been destroyed.

This `await` doesn't block? `Future::await` would block.


- Jiang Yan


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


On July 18, 2017, 8:05 p.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60791/
> ---
> 
> (Updated July 18, 2017, 8:05 p.m.)
> 
> 
> Review request for mesos, Joseph Wu and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-7782
> https://issues.apache.org/jira/browse/MESOS-7782
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add fetcher metrics to track the (constant) size of the cache
> size and the (varying) amount of cache space use. The cache size
> is published as `containerizer/fetcher/cache_size_total_bytes`
> and the used space is `containerizer/fetcher/cache_size_used_bytes`.
> 
> 
> Diffs
> -
> 
>   docs/monitoring.md 38b8093ef683b5de65cbfa5330a6bbd1c9e10f12 
>   src/slave/containerizer/fetcher.cpp 
> 6a664e0657a19d27afac98fd5298d6a18aabe43f 
>   src/slave/containerizer/fetcher_process.hpp 
> 3ed7dc9db5b64b72881249767c0356a3bc5370e7 
>   src/tests/fetcher_cache_tests.cpp 1c654e511d2079de746ac97a2c2718e1b926768e 
> 
> 
> Diff: https://reviews.apache.org/r/60791/diff/4/
> 
> 
> Testing
> ---
> 
> make check (Fedora 26).
> 
> 
> Thanks,
> 
> James Peach
> 
>

Re: Review Request 60956: Refactored fetcher cache metrics.

[Jiang Yan Xu]

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


Ship it!




Ship It!

- Jiang Yan Xu


On July 18, 2017, 8:04 p.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60956/
> ---
> 
> (Updated July 18, 2017, 8:04 p.m.)
> 
> 
> Review request for mesos, Joseph Wu and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-7782
> https://issues.apache.org/jira/browse/MESOS-7782
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Refactored fetcher cache metrics to be more consistent with how
> metrics are handled in other processes. Added an inner "Metrics"
> class to hold the metrics, and renamed them to snake_case following
> the published metrics names.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/fetcher.cpp 
> 6a664e0657a19d27afac98fd5298d6a18aabe43f 
>   src/slave/containerizer/fetcher_process.hpp 
> 3ed7dc9db5b64b72881249767c0356a3bc5370e7 
> 
> 
> Diff: https://reviews.apache.org/r/60956/diff/1/
> 
> 
> Testing
> ---
> 
> make check (Fedora 26)
> 
> 
> Thanks,
> 
> James Peach
> 
>