Re: Review Request 69405: Refactored createAuthorizationCallbacks into common/authorization.

2018-11-20 Thread Alexander Rojas

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


Ship it!




Thanks for the clean up work!

- Alexander Rojas


On Nov. 20, 2018, 3:35 a.m., Till Toenshoff wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69405/
> ---
> 
> (Updated Nov. 20, 2018, 3:35 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Alexander Rojas, and Greg Mann.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Moves 'createAuthorizationCallbacks' out of common/http into
> common/authorization.
> 
> 
> Diffs
> -
> 
>   src/authorizer/local/authorizer.cpp 
> 83944b9a7322707609c2d4e13b84ab3a44cf4e25 
>   src/common/authorization.hpp PRE-CREATION 
>   src/common/authorization.cpp PRE-CREATION 
>   src/common/http.hpp 6ca54a641d1ea1625c70518f8870516664741e9a 
>   src/common/http.cpp d9703f80880850b8a1ec9ddd2b1e8f125f36 
>   src/master/main.cpp 2c7b1bb492a0655dec9280e98ff942a15e2ae8f0 
>   src/slave/main.cpp e774092ff2c3941f17cdebfb26d80c05a26497c6 
>   src/tests/cluster.cpp 2b351ca70d8e80008e49722aa7d46918b5ecd9b0 
>   src/tests/logging_tests.cpp 30bcdc7f4aa9d6a39c5ef6e825357815bf4f9f19 
> 
> 
> Diff: https://reviews.apache.org/r/69405/diff/1/
> 
> 
> Testing
> ---
> 
> `make check` and internal CI
> 
> 
> Thanks,
> 
> Till Toenshoff
> 
>



Re: Review Request 69359: Rewrote test `ReconcileDroppedOperation` for `CREATE_DISK`.

2018-11-20 Thread Benjamin Bannier

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


Ship it!




Ship It!

- Benjamin Bannier


On Nov. 19, 2018, 10:12 p.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69359/
> ---
> 
> (Updated Nov. 19, 2018, 10:12 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Gastón Kleiman, and Greg Mann.
> 
> 
> Bugs: MESOS-9275
> https://issues.apache.org/jira/browse/MESOS-9275
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Previously the `ReconcileDroppedOperation` test relies on converting
> preprovisioned volumes. To adapt the new semantics for `CREATE_DISK`,
> this test is rewritten to create two disks from a storage pool, with one
> operation dropped.
> 
> 
> Diffs
> -
> 
>   src/tests/storage_local_resource_provider_tests.cpp 
> 077a46585bd56181ba199dc529e09f38f4971338 
> 
> 
> Diff: https://reviews.apache.org/r/69359/diff/2/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>



Re: Review Request 69365: Recovered disk through `CREATE_DISK` in test `AgentRegisteredWithNewId`.

2018-11-20 Thread Chun-Hung Hsiao

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




src/tests/storage_local_resource_provider_tests.cpp
Lines 1626 (patched)


This operation as well as the following one might be dropped due to the 
same race described in MESOS-9190.


- Chun-Hung Hsiao


On Nov. 21, 2018, 5:42 a.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69365/
> ---
> 
> (Updated Nov. 21, 2018, 5:42 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and Jie Yu.
> 
> 
> Bugs: MESOS-9275
> https://issues.apache.org/jira/browse/MESOS-9275
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Test `AgentRegisteredWithNewId` is now improved to exercise the code
> path for recovering disks created by the last agent through
> `CREATE_DISK`.
> 
> 
> Diffs
> -
> 
>   src/tests/storage_local_resource_provider_tests.cpp 
> 077a46585bd56181ba199dc529e09f38f4971338 
> 
> 
> Diff: https://reviews.apache.org/r/69365/diff/3/
> 
> 
> Testing
> ---
> 
> make check
> 
> This test passes iff r/69362 is applied.
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>



Re: Review Request 69365: Recovered disk through `CREATE_DISK` in test `AgentRegisteredWithNewId`.

2018-11-20 Thread Chun-Hung Hsiao

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

(Updated Nov. 21, 2018, 5:42 a.m.)


Review request for mesos, Benjamin Bannier and Jie Yu.


Changes
---

Addressed Benjamin's comment.


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


Repository: mesos


Description
---

Test `AgentRegisteredWithNewId` is now improved to exercise the code
path for recovering disks created by the last agent through
`CREATE_DISK`.


Diffs (updated)
-

  src/tests/storage_local_resource_provider_tests.cpp 
077a46585bd56181ba199dc529e09f38f4971338 


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

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


Testing
---

make check

This test passes iff r/69362 is applied.


Thanks,

Chun-Hung Hsiao



Re: Review Request 69362: Checkpointed creation parameters for CSI volumes.

2018-11-20 Thread Chun-Hung Hsiao

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

(Updated Nov. 21, 2018, 3:39 a.m.)


Review request for mesos, Benjamin Bannier and Jie Yu.


Changes
---

Addressed Benjamin's comment.


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


Repository: mesos


Description
---

The parameters of CSI volumes created by SLRPs are now checkpointed, and
used to validate volumes created from previous SLRP runs.


Diffs (updated)
-

  src/csi/state.proto 8445399ac17cfef740c2523172b34fd5439f2d3a 
  src/resource_provider/storage/provider.cpp 
c137fa4f13edc58d93c03a9dd32fdf9d38b38316 


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

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


Testing
---

make check


Thanks,

Chun-Hung Hsiao



Re: Review Request 69364: Added the `--create_parameters` flag to the test CSI plugin.

2018-11-20 Thread Chun-Hung Hsiao

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

(Updated Nov. 21, 2018, 3:43 a.m.)


Review request for mesos, Benjamin Bannier and Jie Yu.


Changes
---

Check the parameters outside the conditional.


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


Repository: mesos


Description
---

When the flag is specified, `CreateVolume` and `GetCapacity` work only
if the `parameters` argument matches this flag. This will be used to
test the checkpointing of create parameters of CSI volumes in SLRP.


Diffs (updated)
-

  src/examples/test_csi_plugin.cpp 66f4ee0e2381e94bf9110d1e6e9f79eac939f683 


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

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


Testing
---

make check


Thanks,

Chun-Hung Hsiao



Re: Review Request 69402: Fixed `CreateVolume` of the test CSI plugin.

2018-11-20 Thread Chun-Hung Hsiao

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

(Updated Nov. 21, 2018, 3:41 a.m.)


Review request for mesos, Benjamin Bannier and Jie Yu.


Changes
---

Check the volume capabilities outside the conditional.


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


Repository: mesos


Description
---

This patch makes sure that `CreateVolume` is idempotent, and check if
the specified volume capability is supported.


Diffs (updated)
-

  src/examples/test_csi_plugin.cpp 66f4ee0e2381e94bf9110d1e6e9f79eac939f683 


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

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


Testing
---

make check


Thanks,

Chun-Hung Hsiao



Re: Review Request 69365: Recovered disk through `CREATE_DISK` in test `AgentRegisteredWithNewId`.

2018-11-20 Thread Chun-Hung Hsiao


> On Nov. 20, 2018, 4:21 p.m., Benjamin Bannier wrote:
> > src/tests/storage_local_resource_provider_tests.cpp
> > Lines 275-276 (original), 278-279 (patched)
> > 
> >
> > ```
> > // Create a JSON string representing disk profile mappings containing
> > // the given profile-parameter pairs.

It's still a single mapping though ;)


- Chun-Hung


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


On Nov. 16, 2018, 12:50 a.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69365/
> ---
> 
> (Updated Nov. 16, 2018, 12:50 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and Jie Yu.
> 
> 
> Bugs: MESOS-9275
> https://issues.apache.org/jira/browse/MESOS-9275
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Test `AgentRegisteredWithNewId` is now improved to exercise the code
> path for recovering disks created by the last agent through
> `CREATE_DISK`.
> 
> 
> Diffs
> -
> 
>   src/tests/storage_local_resource_provider_tests.cpp 
> 077a46585bd56181ba199dc529e09f38f4971338 
> 
> 
> Diff: https://reviews.apache.org/r/69365/diff/2/
> 
> 
> Testing
> ---
> 
> make check
> 
> This test passes iff r/69362 is applied.
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>



Re: Review Request 69364: Added the `--create_parameters` flag to the test CSI plugin.

2018-11-20 Thread Chun-Hung Hsiao


> On Nov. 20, 2018, 1:16 p.m., Benjamin Bannier wrote:
> > src/examples/test_csi_plugin.cpp
> > Lines 367-370 (patched)
> > 
> >
> > Can we move this block and the one below outside of the conditional or 
> > do we depend on ordering/side effects?

Currently the returned `ALREADY_EXISTS` status depends on the condition. It can 
also be argued that returning `INVALID_ARGUMENT` despite the volume exists also 
conforms to the CSI spec. Let me do that instead.


- Chun-Hung


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


On Nov. 19, 2018, 11:35 p.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69364/
> ---
> 
> (Updated Nov. 19, 2018, 11:35 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and Jie Yu.
> 
> 
> Bugs: MESOS-9275
> https://issues.apache.org/jira/browse/MESOS-9275
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> When the flag is specified, `CreateVolume` and `GetCapacity` work only
> if the `parameters` argument matches this flag. This will be used to
> test the checkpointing of create parameters of CSI volumes in SLRP.
> 
> 
> Diffs
> -
> 
>   src/examples/test_csi_plugin.cpp 66f4ee0e2381e94bf9110d1e6e9f79eac939f683 
> 
> 
> Diff: https://reviews.apache.org/r/69364/diff/2/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>



Re: Review Request 69360: Rewrote test `ConvertPreExistingVolume` for `CREATE_DISK`.

2018-11-20 Thread Chun-Hung Hsiao

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

(Updated Nov. 21, 2018, 2:46 a.m.)


Review request for mesos, Benjamin Bannier, James DeFelice, and Jie Yu.


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


Repository: mesos


Description (updated)
---

Due to the changes of the `CREATE_DISK` semantics, this test is
rewritten to convert a preprovisioned volume to a profile volumes, and
then to destroy it to return the space back to the storage pool.

NOTE: The updated test will fail unless r/69361 (which implements the
new `CREATE_DISK` semantics) is also applied.


Diffs
-

  src/tests/storage_local_resource_provider_tests.cpp 
077a46585bd56181ba199dc529e09f38f4971338 


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


Testing
---

make check

This test passes iff the next patch is applied.


Thanks,

Chun-Hung Hsiao



Re: Review Request 69357: Added profiles to storage pools in tests for `CREATE_DISK`.

2018-11-20 Thread Chun-Hung Hsiao


> On Nov. 20, 2018, 3:47 p.m., Benjamin Bannier wrote:
> > src/tests/operation_reconciliation_tests.cpp
> > Lines 795-797 (original), 795-798 (patched)
> > 
> >
> > Nit: old indent was fine IMO.

Dropping this and created https://issues.apache.org/jira/browse/MESOS-9402.


> On Nov. 20, 2018, 3:47 p.m., Benjamin Bannier wrote:
> > src/tests/storage_local_resource_provider_tests.cpp
> > Line 4137 (original), 4135-4138 (patched)
> > 
> >
> > Nit: overindented, see change below.

Dropping this and created https://issues.apache.org/jira/browse/MESOS-9402.


- Chun-Hung


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


On Nov. 19, 2018, 9:09 p.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69357/
> ---
> 
> (Updated Nov. 19, 2018, 9:09 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Greg Mann, and Jan Schlicht.
> 
> 
> Bugs: MESOS-9275
> https://issues.apache.org/jira/browse/MESOS-9275
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch adds a new `targetProfile` parameter to the `CREATE_DISK`
> test helper, and add profiles to all storage pools in tests.
> 
> Tests relying on preprovisioned volumes are fixed in subsequent patches.
> 
> 
> Diffs
> -
> 
>   src/tests/api_tests.cpp fdd9f871f75617fc26a28679e2a1e41f506c6133 
>   src/tests/master_tests.cpp ac6bf379c5906cf9612284911c121c9457f648a0 
>   src/tests/mesos.hpp 576f4bde88c069ee2fa0dd33912a034437338e7e 
>   src/tests/operation_reconciliation_tests.cpp 
> 37d38b3df8c162bd1baa5ce557e54baa5c23a006 
>   src/tests/resource_provider_manager_tests.cpp 
> 5bb740edc7c3cc8698aede4f2ed57c21232fe378 
>   src/tests/storage_local_resource_provider_tests.cpp 
> 077a46585bd56181ba199dc529e09f38f4971338 
> 
> 
> Diff: https://reviews.apache.org/r/69357/diff/2/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> Tests `ReconcileDroppedOperation` and `ConvertPreExistingVolume` are fixed 
> later in chain.
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>



Re: Review Request 69357: Added profiles to storage pools in tests for `CREATE_DISK`.

2018-11-20 Thread Chun-Hung Hsiao


> On Nov. 20, 2018, 3:47 p.m., Benjamin Bannier wrote:
> > Could you update the commit message?
> > 
> > I am not sure we need to update all the tests you touched here, but there 
> > also seems nothing wrong with it. Maybe something to potentially elaborate 
> > on in the commit message as well.

We need to adhere to the requirement of `target_profile`. I'll update the 
message.


- Chun-Hung


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


On Nov. 19, 2018, 9:09 p.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69357/
> ---
> 
> (Updated Nov. 19, 2018, 9:09 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Greg Mann, and Jan Schlicht.
> 
> 
> Bugs: MESOS-9275
> https://issues.apache.org/jira/browse/MESOS-9275
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch adds a new `targetProfile` parameter to the `CREATE_DISK`
> test helper, and add profiles to all storage pools in tests.
> 
> Tests relying on preprovisioned volumes are fixed in subsequent patches.
> 
> 
> Diffs
> -
> 
>   src/tests/api_tests.cpp fdd9f871f75617fc26a28679e2a1e41f506c6133 
>   src/tests/master_tests.cpp ac6bf379c5906cf9612284911c121c9457f648a0 
>   src/tests/mesos.hpp 576f4bde88c069ee2fa0dd33912a034437338e7e 
>   src/tests/operation_reconciliation_tests.cpp 
> 37d38b3df8c162bd1baa5ce557e54baa5c23a006 
>   src/tests/resource_provider_manager_tests.cpp 
> 5bb740edc7c3cc8698aede4f2ed57c21232fe378 
>   src/tests/storage_local_resource_provider_tests.cpp 
> 077a46585bd56181ba199dc529e09f38f4971338 
> 
> 
> Diff: https://reviews.apache.org/r/69357/diff/2/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> Tests `ReconcileDroppedOperation` and `ConvertPreExistingVolume` are fixed 
> later in chain.
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>



Re: Review Request 69411: Added new interface for constructing `cluster::Master`.

2018-11-20 Thread Mesos Reviewbot

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



Bad review!

Reviews applied: []

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

- Mesos Reviewbot


On Nov. 20, 2018, 2:20 p.m., Benno Evers wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69411/
> ---
> 
> (Updated Nov. 20, 2018, 2:20 p.m.)
> 
> 
> Review request for mesos.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This review adds a new method `cluster::Master::create()` that
> allows constructing masters while only specifying the non-standard
> components required for the specific test, like this:
> 
> auto master = cluster::Master::build()
>   .withZookeeperUrl(url)
>   .withAllocator(allocator);
> 
> This is sometimes better known as "fluent interface".
> 
> 
> Diffs
> -
> 
>   src/tests/cluster.hpp ad2b80e658d2f8afcefe9969d62cd33f0c475ce9 
>   src/tests/cluster.cpp 2b351ca70d8e80008e49722aa7d46918b5ecd9b0 
> 
> 
> Diff: https://reviews.apache.org/r/69411/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Benno Evers
> 
>



Re: Review Request 69157: Fixed handling for offer operation updates.

2018-11-20 Thread Chun-Hung Hsiao

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


Fix it, then Ship it!




Could you update the commit message accordingly?


src/slave/slave.cpp
Line 8037 (original), 8037 (patched)


`operation->latest_status().state()`

It's interesting that we used `operation` here but not in the `switch` 
condition lol.


- Chun-Hung Hsiao


On Nov. 20, 2018, 5:50 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69157/
> ---
> 
> (Updated Nov. 20, 2018, 5:50 p.m.)
> 
> 
> Review request for mesos, Chun-Hung Hsiao and Jan Schlicht.
> 
> 
> Bugs: MESOS-8403
> https://issues.apache.org/jira/browse/MESOS-8403
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The handling of offer operation updates introduced in `c946615ec6d`
> made use of an update's `latest_status` without making sure that any
> value was set. This could lead to situation where an uninitialized
> enum value was switched on which would have caused a fatal error at
> runtime.
> 
> This patch replaces uses of `latest_status` with `state` which does
> contain the information we care about. We also adjust the error
> logging so we log the value that lead to the error, not some other
> value.
> 
> 
> Diffs
> -
> 
>   src/slave/slave.cpp 858b78620e1ef33f3587d0bd95a684917aaf5bbb 
> 
> 
> Diff: https://reviews.apache.org/r/69157/diff/4/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>



Re: Review Request 69157: Fixed handling for offer operation updates.

2018-11-20 Thread Benjamin Bannier

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

(Updated Nov. 20, 2018, 6:50 p.m.)


Review request for mesos, Chun-Hung Hsiao and Jan Schlicht.


Changes
---

Addressed comment from Chun.


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


Repository: mesos


Description
---

The handling of offer operation updates introduced in `c946615ec6d`
made use of an update's `latest_status` without making sure that any
value was set. This could lead to situation where an uninitialized
enum value was switched on which would have caused a fatal error at
runtime.

This patch replaces uses of `latest_status` with `state` which does
contain the information we care about. We also adjust the error
logging so we log the value that lead to the error, not some other
value.


Diffs (updated)
-

  src/slave/slave.cpp 858b78620e1ef33f3587d0bd95a684917aaf5bbb 


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

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


Testing
---

`make check`


Thanks,

Benjamin Bannier



Re: Review Request 69412: Fixed thread safety issue in jwt signature validation.

2018-11-20 Thread Mesos Reviewbot Windows

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



PASS: Mesos patch 69412 was successfully built and tested.

Reviews applied: `['69412']`

All the build artifacts available at: 
http://dcos-win.westus2.cloudapp.azure.com/artifacts/mesos-reviewbot-testing/2631/mesos-review-69412

- Mesos Reviewbot Windows


On Nov. 20, 2018, 2:51 p.m., Alexander Rojas wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69412/
> ---
> 
> (Updated Nov. 20, 2018, 2:51 p.m.)
> 
> 
> Review request for mesos, Gastón Kleiman, Greg Mann, Jan Schlicht, and Till 
> Toenshoff.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The old implementation of the OpenSSl utilities which computes an HMAC
> 256 signature makes a non thread safe call to the OpenSSL library.
> 
> This is addressed in this patch. It also includes a test which will
> cause the shared buffer to be rewritten if the non thread safe code is
> executed.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/ssl/utilities.cpp 
> 72ad07954308db3e67c75d0fc08d46095192caca 
>   3rdparty/libprocess/src/tests/jwt_tests.cpp 
> 11ada99a971e2a8c74c3a1060a25955cc0bdeac5 
> 
> 
> Diff: https://reviews.apache.org/r/69412/diff/1/
> 
> 
> Testing
> ---
> 
> make -j12 check
> 
> 
> Thanks,
> 
> Alexander Rojas
> 
>



Re: Review Request 69036: Changed the semantics of `CREATE_DISK` and `DESTROY_DISK` operations.

2018-11-20 Thread Benjamin Bannier

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


Ship it!




Ship It!

- Benjamin Bannier


On Nov. 16, 2018, 12:52 a.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69036/
> ---
> 
> (Updated Nov. 16, 2018, 12:52 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, James DeFelice, and Jie Yu.
> 
> 
> Bugs: MESOS-9275
> https://issues.apache.org/jira/browse/MESOS-9275
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The semantics of these two operations has been updated to provide
> primitives to import CSI volumes and recover CSI volumes against agent
> ID changes and metadata loss.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto f6989cd5e7a47c3fec29bdeccc38474bf0045341 
>   include/mesos/v1/mesos.proto edad35a928705f21aa1aca31f3e9cfd30810 
> 
> 
> Diff: https://reviews.apache.org/r/69036/diff/5/
> 
> 
> Testing
> ---
> 
> Test will be done later in the chain.
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>



Re: Review Request 69360: Rewrote test `ConvertPreExistingVolume` for `CREATE_DISK`.

2018-11-20 Thread Benjamin Bannier

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


Ship it!




Unless you reorder the chain, could you call out in the commit message the we 
will fix `StorageLocalResourceProviderTest.ImportPreprovisionedVolume` in a 
follow-up commit?

- Benjamin Bannier


On Nov. 19, 2018, 10:19 p.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69360/
> ---
> 
> (Updated Nov. 19, 2018, 10:19 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, James DeFelice, and Jie Yu.
> 
> 
> Bugs: MESOS-9275
> https://issues.apache.org/jira/browse/MESOS-9275
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Due to the changes of the `CREATE_DISK` semantics, this test is
> rewritten to convert a preprovisioned volume to a profile volumes, and
> then to destroy it to return the space back to the storage pool.
> 
> 
> Diffs
> -
> 
>   src/tests/storage_local_resource_provider_tests.cpp 
> 077a46585bd56181ba199dc529e09f38f4971338 
> 
> 
> Diff: https://reviews.apache.org/r/69360/diff/2/
> 
> 
> Testing
> ---
> 
> make check
> 
> This test passes iff the next patch is applied.
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>



Re: Review Request 69405: Refactored createAuthorizationCallbacks into common/authorization.

2018-11-20 Thread Alexander Rukletsov

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


Fix it, then Ship it!




Lovely!


src/common/authorization.hpp
Lines 34 (patched)


please move `stout/hashset` include here


- Alexander Rukletsov


On Nov. 20, 2018, 2:35 a.m., Till Toenshoff wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69405/
> ---
> 
> (Updated Nov. 20, 2018, 2:35 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Alexander Rojas, and Greg Mann.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Moves 'createAuthorizationCallbacks' out of common/http into
> common/authorization.
> 
> 
> Diffs
> -
> 
>   src/authorizer/local/authorizer.cpp 
> 83944b9a7322707609c2d4e13b84ab3a44cf4e25 
>   src/common/authorization.hpp PRE-CREATION 
>   src/common/authorization.cpp PRE-CREATION 
>   src/common/http.hpp 6ca54a641d1ea1625c70518f8870516664741e9a 
>   src/common/http.cpp d9703f80880850b8a1ec9ddd2b1e8f125f36 
>   src/master/main.cpp 2c7b1bb492a0655dec9280e98ff942a15e2ae8f0 
>   src/slave/main.cpp e774092ff2c3941f17cdebfb26d80c05a26497c6 
>   src/tests/cluster.cpp 2b351ca70d8e80008e49722aa7d46918b5ecd9b0 
>   src/tests/logging_tests.cpp 30bcdc7f4aa9d6a39c5ef6e825357815bf4f9f19 
> 
> 
> Diff: https://reviews.apache.org/r/69405/diff/1/
> 
> 
> Testing
> ---
> 
> `make check` and internal CI
> 
> 
> Thanks,
> 
> Till Toenshoff
> 
>



Re: Review Request 69365: Recovered disk through `CREATE_DISK` in test `AgentRegisteredWithNewId`.

2018-11-20 Thread Benjamin Bannier

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


Fix it, then Ship it!





src/tests/storage_local_resource_provider_tests.cpp
Lines 275-276 (original), 278-279 (patched)


```
// Create a JSON string representing disk profile mappings containing
// the given profile-parameter pairs.


- Benjamin Bannier


On Nov. 16, 2018, 1:50 a.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69365/
> ---
> 
> (Updated Nov. 16, 2018, 1:50 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and Jie Yu.
> 
> 
> Bugs: MESOS-9275
> https://issues.apache.org/jira/browse/MESOS-9275
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Test `AgentRegisteredWithNewId` is now improved to exercise the code
> path for recovering disks created by the last agent through
> `CREATE_DISK`.
> 
> 
> Diffs
> -
> 
>   src/tests/storage_local_resource_provider_tests.cpp 
> 077a46585bd56181ba199dc529e09f38f4971338 
> 
> 
> Diff: https://reviews.apache.org/r/69365/diff/2/
> 
> 
> Testing
> ---
> 
> make check
> 
> This test passes iff r/69362 is applied.
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>



Re: Review Request 69411: Added new interface for constructing `cluster::Master`.

2018-11-20 Thread Mesos Reviewbot Windows

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



PASS: Mesos patch 69411 was successfully built and tested.

Reviews applied: `['69411']`

All the build artifacts available at: 
http://dcos-win.westus2.cloudapp.azure.com/artifacts/mesos-reviewbot-testing/2630/mesos-review-69411

- Mesos Reviewbot Windows


On Nov. 20, 2018, 2:20 p.m., Benno Evers wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69411/
> ---
> 
> (Updated Nov. 20, 2018, 2:20 p.m.)
> 
> 
> Review request for mesos.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This review adds a new method `cluster::Master::create()` that
> allows constructing masters while only specifying the non-standard
> components required for the specific test, like this:
> 
> auto master = cluster::Master::build()
>   .withZookeeperUrl(url)
>   .withAllocator(allocator);
> 
> This is sometimes better known as "fluent interface".
> 
> 
> Diffs
> -
> 
>   src/tests/cluster.hpp ad2b80e658d2f8afcefe9969d62cd33f0c475ce9 
>   src/tests/cluster.cpp 2b351ca70d8e80008e49722aa7d46918b5ecd9b0 
> 
> 
> Diff: https://reviews.apache.org/r/69411/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Benno Evers
> 
>



Re: Review Request 69345: Made non-root containers can access SANDBOX_PATH volume of PARENT type.

2018-11-20 Thread Mesos Reviewbot Windows

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



PASS: Mesos patch 69345 was successfully built and tested.

Reviews applied: `['69342', '69344', '69345']`

All the build artifacts available at: 
http://dcos-win.westus2.cloudapp.azure.com/artifacts/mesos-reviewbot-testing/2629/mesos-review-69345

- Mesos Reviewbot Windows


On Nov. 20, 2018, 2:13 p.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69345/
> ---
> 
> (Updated Nov. 20, 2018, 2:13 p.m.)
> 
> 
> Review request for mesos, Gilbert Song, Greg Mann, Ilya Pronin, and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> If a nested container running as a non-root user tries to use a
> SANDBOX_PATH volume of PARENT type, we will make sure the volume owned
> by a unique gid allocated by the volume gid manager and the container
> process launched with that gid as its supplementary group.
> 
> 
> Diffs
> -
> 
>   include/mesos/slave/containerizer.proto 
> 5b4dcdda0f55ea3355c78d1447c7be9ca54d9dc9 
>   src/local/local.cpp 608706811486e59b9472c026876d1d84cbccc279 
>   src/slave/containerizer/containerizer.hpp 
> 66f73a306deffc51503479420531ea1948c574e1 
>   src/slave/containerizer/containerizer.cpp 
> c6b5e64a72d16b871dcbfc17c05566affea6bd44 
>   src/slave/containerizer/mesos/containerizer.hpp 
> 3102b8755c1fa3b205081d0198c6021c02d15ec6 
>   src/slave/containerizer/mesos/containerizer.cpp 
> a5cf2da55c046c5c45e0c2ca3400f64de12de62b 
>   src/slave/containerizer/mesos/isolators/volume/sandbox_path.hpp 
> 1631160236379f84c6e1ed1be1370b5f2f2fd563 
>   src/slave/containerizer/mesos/isolators/volume/sandbox_path.cpp 
> 300b3d95d74b73fbe0221096f3f3f172be745081 
>   src/slave/containerizer/mesos/launch.cpp 
> 882bcdf89e2b0cca3d3f62e6d017849a51ceaead 
>   src/slave/main.cpp e774092ff2c3941f17cdebfb26d80c05a26497c6 
>   src/slave/slave.hpp 0bd340176e2a8cefdfa7ef71e059441fb171aff6 
>   src/slave/slave.cpp 74f6fb9036a9ac4f587f53ec2df04eeb4c167bfb 
>   src/tests/cluster.cpp 2b351ca70d8e80008e49722aa7d46918b5ecd9b0 
>   src/tests/mock_slave.hpp 3c0d602a981d76dcf10f9e413851e606d835e113 
>   src/tests/mock_slave.cpp a78ca9c7911bb7928a93be6867abe62e8cd20712 
> 
> 
> Diff: https://reviews.apache.org/r/69345/diff/4/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>



Re: Review Request 69357: Added profiles to storage pools in tests for `CREATE_DISK`.

2018-11-20 Thread Benjamin Bannier

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


Fix it, then Ship it!




Could you update the commit message?

I am not sure we need to update all the tests you touched here, but there also 
seems nothing wrong with it. Maybe something to potentially elaborate on in the 
commit message as well.


src/tests/operation_reconciliation_tests.cpp
Lines 795-797 (original), 795-798 (patched)


Nit: old indent was fine IMO.



src/tests/storage_local_resource_provider_tests.cpp
Line 4137 (original), 4135-4138 (patched)


Nit: overindented, see change below.


- Benjamin Bannier


On Nov. 19, 2018, 10:09 p.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69357/
> ---
> 
> (Updated Nov. 19, 2018, 10:09 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Greg Mann, and Jan Schlicht.
> 
> 
> Bugs: MESOS-9275
> https://issues.apache.org/jira/browse/MESOS-9275
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch adds a new `targetProfile` parameter to the `CREATE_DISK`
> test helper, and add profiles to all storage pools in tests.
> 
> Tests relying on preprovisioned volumes are fixed in subsequent patches.
> 
> 
> Diffs
> -
> 
>   src/tests/api_tests.cpp fdd9f871f75617fc26a28679e2a1e41f506c6133 
>   src/tests/master_tests.cpp ac6bf379c5906cf9612284911c121c9457f648a0 
>   src/tests/mesos.hpp 576f4bde88c069ee2fa0dd33912a034437338e7e 
>   src/tests/operation_reconciliation_tests.cpp 
> 37d38b3df8c162bd1baa5ce557e54baa5c23a006 
>   src/tests/resource_provider_manager_tests.cpp 
> 5bb740edc7c3cc8698aede4f2ed57c21232fe378 
>   src/tests/storage_local_resource_provider_tests.cpp 
> 077a46585bd56181ba199dc529e09f38f4971338 
> 
> 
> Diff: https://reviews.apache.org/r/69357/diff/2/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> Tests `ReconcileDroppedOperation` and `ConvertPreExistingVolume` are fixed 
> later in chain.
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>



Re: Review Request 69361: Implemented the new `CREATE_DISK`/`DESTROY_DISK` semantics in SLRP.

2018-11-20 Thread Benjamin Bannier

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


Ship it!




Ship It!

- Benjamin Bannier


On Nov. 16, 2018, 1:42 a.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69361/
> ---
> 
> (Updated Nov. 16, 2018, 1:42 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and Jie Yu.
> 
> 
> Bugs: MESOS-9275
> https://issues.apache.org/jira/browse/MESOS-9275
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The default mount/block volume capabilities is removed from SLRP.
> Instead, `CREATE_DISK` will convert a preprovisioned RAW disk to a
> profile disk, and `DESTROY_DISK` will always deprovision a profile disk
> as long as the CSI plugin is capable of deprovisioning volumes.
> 
> 
> Diffs
> -
> 
>   src/csi/utils.hpp 5ce318e52bc39555ca09bf68dca6dedea988e5a4 
>   src/resource_provider/storage/provider.cpp 
> c137fa4f13edc58d93c03a9dd32fdf9d38b38316 
> 
> 
> Diff: https://reviews.apache.org/r/69361/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>



Review Request 69412: Fixed thread safety issue in jwt signature validation.

2018-11-20 Thread Alexander Rojas

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

Review request for mesos, Gastón Kleiman, Greg Mann, Jan Schlicht, and Till 
Toenshoff.


Repository: mesos


Description
---

The old implementation of the OpenSSl utilities which computes an HMAC
256 signature makes a non thread safe call to the OpenSSL library.

This is addressed in this patch. It also includes a test which will
cause the shared buffer to be rewritten if the non thread safe code is
executed.


Diffs
-

  3rdparty/libprocess/src/ssl/utilities.cpp 
72ad07954308db3e67c75d0fc08d46095192caca 
  3rdparty/libprocess/src/tests/jwt_tests.cpp 
11ada99a971e2a8c74c3a1060a25955cc0bdeac5 


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


Testing
---

make test


Thanks,

Alexander Rojas



Re: Review Request 69356: Added validation for `Offer.Operation.CreateDisk.target_profile`.

2018-11-20 Thread Benjamin Bannier

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


Ship it!




Ship It!

- Benjamin Bannier


On Nov. 19, 2018, 10:24 p.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69356/
> ---
> 
> (Updated Nov. 19, 2018, 10:24 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, James DeFelice, and Jie Yu.
> 
> 
> Bugs: MESOS-9275
> https://issues.apache.org/jira/browse/MESOS-9275
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added validation for `Offer.Operation.CreateDisk.target_profile`.
> 
> 
> Diffs
> -
> 
>   src/master/validation.cpp 5768ac8fe802f28855fbd7be135c75532771 
>   src/tests/master_validation_tests.cpp 
> aa7c8f70c09459be32c6c415497e95fcdc218efd 
> 
> 
> Diff: https://reviews.apache.org/r/69356/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>



Re: Review Request 69362: Checkpointed creation parameters for CSI volumes.

2018-11-20 Thread Benjamin Bannier

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


Fix it, then Ship it!





src/resource_provider/storage/provider.cpp
Line 2833 (original), 2851 (patched)


Any reason we cannot use a simpler and more familiar lambda (which captures 
`volumeAttributes`?), e.g.,
```
// ...
.then(defer(
self(),
[=](const csi::v0::ValidateVolumeCapabilitiesResponse& response)
  -> Future {
  // Body.
}));
// ...
```


- Benjamin Bannier


On Nov. 20, 2018, 12:30 a.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69362/
> ---
> 
> (Updated Nov. 20, 2018, 12:30 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and Jie Yu.
> 
> 
> Bugs: MESOS-9275
> https://issues.apache.org/jira/browse/MESOS-9275
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The parameters of CSI volumes created by SLRPs are now checkpointed, and
> used to validate volumes created from previous SLRP runs.
> 
> 
> Diffs
> -
> 
>   src/csi/state.proto 8445399ac17cfef740c2523172b34fd5439f2d3a 
>   src/resource_provider/storage/provider.cpp 
> c137fa4f13edc58d93c03a9dd32fdf9d38b38316 
> 
> 
> Diff: https://reviews.apache.org/r/69362/diff/2/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>



Re: Review Request 69345: Made non-root containers can access SANDBOX_PATH volume of PARENT type.

2018-11-20 Thread Qian Zhang

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

(Updated Nov. 20, 2018, 10:13 p.m.)


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


Summary (updated)
-

Made non-root containers can access SANDBOX_PATH volume of PARENT type.


Repository: mesos


Description (updated)
---

If a nested container running as a non-root user tries to use a
SANDBOX_PATH volume of PARENT type, we will make sure the volume owned
by a unique gid allocated by the volume gid manager and the container
process launched with that gid as its supplementary group.


Diffs (updated)
-

  include/mesos/slave/containerizer.proto 
5b4dcdda0f55ea3355c78d1447c7be9ca54d9dc9 
  src/local/local.cpp 608706811486e59b9472c026876d1d84cbccc279 
  src/slave/containerizer/containerizer.hpp 
66f73a306deffc51503479420531ea1948c574e1 
  src/slave/containerizer/containerizer.cpp 
c6b5e64a72d16b871dcbfc17c05566affea6bd44 
  src/slave/containerizer/mesos/containerizer.hpp 
3102b8755c1fa3b205081d0198c6021c02d15ec6 
  src/slave/containerizer/mesos/containerizer.cpp 
a5cf2da55c046c5c45e0c2ca3400f64de12de62b 
  src/slave/containerizer/mesos/isolators/volume/sandbox_path.hpp 
1631160236379f84c6e1ed1be1370b5f2f2fd563 
  src/slave/containerizer/mesos/isolators/volume/sandbox_path.cpp 
300b3d95d74b73fbe0221096f3f3f172be745081 
  src/slave/containerizer/mesos/launch.cpp 
882bcdf89e2b0cca3d3f62e6d017849a51ceaead 
  src/slave/main.cpp e774092ff2c3941f17cdebfb26d80c05a26497c6 
  src/slave/slave.hpp 0bd340176e2a8cefdfa7ef71e059441fb171aff6 
  src/slave/slave.cpp 74f6fb9036a9ac4f587f53ec2df04eeb4c167bfb 
  src/tests/cluster.cpp 2b351ca70d8e80008e49722aa7d46918b5ecd9b0 
  src/tests/mock_slave.hpp 3c0d602a981d76dcf10f9e413851e606d835e113 
  src/tests/mock_slave.cpp a78ca9c7911bb7928a93be6867abe62e8cd20712 


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

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


Testing
---


Thanks,

Qian Zhang



Review Request 69411: Added new interface for constructing `cluster::Master`.

2018-11-20 Thread Benno Evers

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

Review request for mesos.


Repository: mesos


Description
---

This review adds a new method `cluster::Master::create()` that
allows constructing masters while only specifying the non-standard
components required for the specific test, like this:

auto master = cluster::Master::build()
  .withZookeeperUrl(url)
  .withAllocator(allocator);

This is sometimes better known as "fluent interface".


Diffs
-

  src/tests/cluster.hpp ad2b80e658d2f8afcefe9969d62cd33f0c475ce9 
  src/tests/cluster.cpp 2b351ca70d8e80008e49722aa7d46918b5ecd9b0 


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


Testing
---


Thanks,

Benno Evers



Re: Review Request 69342: Added an agent flag `--volume_gid_range`.

2018-11-20 Thread Qian Zhang

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

(Updated Nov. 20, 2018, 10:10 p.m.)


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


Repository: mesos


Description (updated)
---

When this flag is specified, if a task running as non-root user uses a
shared persistent volume or a SANDBOX_PATH volume of PARENT type, the
volume will be owned by a gid allocated from this range and have the
`setgit` bit set, and the task process will be launched with the gid
as its supplementary group.


Diffs (updated)
-

  docs/configuration/agent.md 7a8df6852dc2af174a6c5a552dca88fa1b1c29f3 
  src/slave/flags.hpp 29d8b7985ffde57da02b5fe0d3a524e98acc27c8 
  src/slave/flags.cpp ccaf65029ec2d0e78041fc3992a0bf5ca0798686 


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

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


Testing
---


Thanks,

Qian Zhang



Re: Review Request 69344: Added volume gid manager to Mesos agent.

2018-11-20 Thread Qian Zhang

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

(Updated Nov. 20, 2018, 10:12 p.m.)


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


Repository: mesos


Description
---

Added volume gid manager to Mesos agent.


Diffs (updated)
-

  src/CMakeLists.txt 732368293049b7d9d6f62057344d433637ad44e8 
  src/Makefile.am 2d9c81b149a5764dc82593bef102f5568847daa2 
  src/slave/volume_gid_manager/volume_gid_manager.hpp PRE-CREATION 
  src/slave/volume_gid_manager/volume_gid_manager.cpp PRE-CREATION 


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

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


Testing
---


Thanks,

Qian Zhang



Re: Review Request 69364: Added the `--create_parameters` flag to the test CSI plugin.

2018-11-20 Thread Benjamin Bannier

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


Ship it!





src/examples/test_csi_plugin.cpp
Lines 367-370 (patched)


Can we move this block and the one below outside of the conditional or do 
we depend on ordering/side effects?


- Benjamin Bannier


On Nov. 20, 2018, 12:35 a.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69364/
> ---
> 
> (Updated Nov. 20, 2018, 12:35 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and Jie Yu.
> 
> 
> Bugs: MESOS-9275
> https://issues.apache.org/jira/browse/MESOS-9275
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> When the flag is specified, `CreateVolume` and `GetCapacity` work only
> if the `parameters` argument matches this flag. This will be used to
> test the checkpointing of create parameters of CSI volumes in SLRP.
> 
> 
> Diffs
> -
> 
>   src/examples/test_csi_plugin.cpp 66f4ee0e2381e94bf9110d1e6e9f79eac939f683 
> 
> 
> Diff: https://reviews.apache.org/r/69364/diff/2/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>



Re: Review Request 69409: Added Seccomp parser tests.

2018-11-20 Thread Mesos Reviewbot Windows

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



FAIL: Failed to apply the dependent review: 67844.

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

All the build artifacts available at: 
http://dcos-win.westus2.cloudapp.azure.com/artifacts/mesos-reviewbot-testing/2628/mesos-review-69409

Relevant logs:

- 
[apply-review-67844.log](http://dcos-win.westus2.cloudapp.azure.com/artifacts/mesos-reviewbot-testing/2628/mesos-review-69409/logs/apply-review-67844.log):

```
error: missing binary patch data for '3rdparty/libseccomp-2.3.3.tar.gz'
error: binary patch does not apply to '3rdparty/libseccomp-2.3.3.tar.gz'
error: 3rdparty/libseccomp-2.3.3.tar.gz: patch does not apply
```

- Mesos Reviewbot Windows


On Nov. 20, 2018, 12:21 p.m., Andrei Budnik wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69409/
> ---
> 
> (Updated Nov. 20, 2018, 12:21 p.m.)
> 
> 
> Review request for mesos, Gilbert Song, James Peach, and Qian Zhang.
> 
> 
> Bugs: MESOS-9392
> https://issues.apache.org/jira/browse/MESOS-9392
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added Seccomp parser tests.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 2d9c81b149a5764dc82593bef102f5568847daa2 
>   src/tests/CMakeLists.txt 553516ad66cab4480b7211950fc726b7d9a3869b 
>   src/tests/containerizer/linux_seccomp_parser_tests.cpp PRE-CREATION 
>   src/tests/environment.cpp 03f8f5971b1e91e62ecfbf965028e6f2388d2a7b 
> 
> 
> Diff: https://reviews.apache.org/r/69409/diff/1/
> 
> 
> Testing
> ---
> 
> ./src/mesos-tests 
> --gtest_filter=SeccompParserTest.SECCOMP_ParseInvalidSeccompProfile 
> --gtest_break_on_failure --gtest_repeat=100 --verbose
> 
> 
> Thanks,
> 
> Andrei Budnik
> 
>



Review Request 69409: Added Seccomp parser tests.

2018-11-20 Thread Andrei Budnik

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

Review request for mesos, Gilbert Song, James Peach, and Qian Zhang.


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


Repository: mesos


Description
---

Added Seccomp parser tests.


Diffs
-

  src/Makefile.am 2d9c81b149a5764dc82593bef102f5568847daa2 
  src/tests/CMakeLists.txt 553516ad66cab4480b7211950fc726b7d9a3869b 
  src/tests/containerizer/linux_seccomp_parser_tests.cpp PRE-CREATION 
  src/tests/environment.cpp 03f8f5971b1e91e62ecfbf965028e6f2388d2a7b 


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


Testing
---

./src/mesos-tests 
--gtest_filter=SeccompParserTest.SECCOMP_ParseInvalidSeccompProfile 
--gtest_break_on_failure --gtest_repeat=100 --verbose


Thanks,

Andrei Budnik



Re: Review Request 69390: Added docs describing how to use the new CLI.

2018-11-20 Thread Mesos Reviewbot Windows

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



FAIL: Some of the unit tests failed. Please check the relevant logs.

Reviews applied: `['69390']`

Failed command: `Start-MesosCITesting`

All the build artifacts available at: 
http://dcos-win.westus2.cloudapp.azure.com/artifacts/mesos-reviewbot-testing/2627/mesos-review-69390

Relevant logs:

- 
[mesos-tests.log](http://dcos-win.westus2.cloudapp.azure.com/artifacts/mesos-reviewbot-testing/2627/mesos-review-69390/logs/mesos-tests.log):

```
W1120 12:04:01.721038 51844 slave.cpp:3923] Ignoring shutdown framework 
f5a0ab59-b8b7-4998-a39b-8602a4f83156- because it is terminating
I1120 12:04:01.723042 51652 master.cpp:1273] Agent 
f5a0ab59-b8b7-4998-a39b-8602a4f83156-S0 at slave(461)@192.10.1.5:51917 
(windows-02.aa0q4n2kgcyefckmv0xukjvy4f.xx.internal.cloudapp.net) disconnected
I1120 12:04:01.723042 51652 master.cpp:3289] Disconnecting agent 
f5a0ab59-b8b7-4998-a39b-8602a4f83156-S0 at slave(461)@192.10.1.5:51917 
(windows-02.aa0q4n2kgcyefckmv0xukjvy4f.xx.internal.cloudapp.net)
I1120 12:04:01.724027 51652 master.cpp:3308] Deactivating agent 
f5a0ab59-b8b7-4998-a39b-8602a4f83156-S0 at slave(461)@192.10.1.5:51917 
(windows-02.aa0q4n2kgcyefckmv0xukjvy4f.xx.internal.cloudapp.net)
I1120 12:04:01.724027 51940 hierarchical.cpp:357] Removed framework 
f5a0ab59-b8b7-4998-a39b-8602a4f83156-
I1120 12:04:01.724027 51940 hierarchical.cpp:801] Agent 
f5a0ab59-b8b7-4998-a39b-8602a4f83156-S0 deactivated
I1120 12:04:01.726042 48620 containerizer.cpp:2463] Destroying container 
0282cb9a-00e7-4ff6-b88a-571a795801f4 in RUNNING state
I1120 12:04:01.726042 48620 containerizer.cpp:3130] Transitioning the state of 
container 0282cb9a-00e7-4ff6-b88a-571a795801f4 from RUNNING to DESTROYING
I1120 12:04:01.727033 48620 launcher.cpp:161] Asked to destroy container 
0282cb9a-00e7-4ff6-b88a-571a795801f4
W1120 12:04:01.728024 49532 process.cpp:1423] Failed to recv on socket 
WindowsFD::Type::SOCKET=7452 to peer '192.10.1.5:53721'[   OK ] 
IsolationFlag/MemoryIsolatorTest.ROOT_MemUsage/0 (670 ms)
[--] 1 test from IsolationFlag/MemoryIsolatorTest (690 ms total)

[--] Global test environment tear-down
[==] 1053 tests from 103 test cases ran. (515025 ms total)
[  PASSED  ] 1052 tests.
[  FAILED  ] 1 test, listed below:
[  FAILED  ] DockerFetcherPluginTest.INTERNET_CURL_FetchImage

 1 FAILED TEST
  YOU HAVE 231 DISABLED TESTS

: IO failed with error code: The specified network name is no longer available.

W1120 12:04:01.728024 49532 process.cpp:838] Failed to recv on socket 
WindowsFD::Type::SOCKET=8464 to peer '192.10.1.5:53722': IO failed with error 
code: The specified network name is no longer available.

I1120 12:04:01.745263 51940 containerizer.cpp:2969] Container 
0282cb9a-00e7-4ff6-b88a-571a795801f4 has exited
I1120 12:04:01.776088 50904 master.cpp:1115] Master terminating
I1120 12:04:01.777056 51844 hierarchical.cpp:643] Removed agent 
f5a0ab59-b8b7-4998-a39b-8602a4f83156-S0
I1120 12:04:02.558130 49532 process.cpp:927] Stopped the socket accept loop
```

- Mesos Reviewbot Windows


On Nov. 20, 2018, 11:12 a.m., Armand Grillet wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69390/
> ---
> 
> (Updated Nov. 20, 2018, 11:12 a.m.)
> 
> 
> Review request for mesos and Kevin Klues.
> 
> 
> Bugs: MESOS-9333
> https://issues.apache.org/jira/browse/MESOS-9333
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The documentation describes the main commands of the new CLI, how to
> activate it, how to build Mesos including this component, and how to
> write a configuration file for it.
> 
> 
> Diffs
> -
> 
>   docs/cli.md PRE-CREATION 
>   docs/home.md e05b65d55176a706c072f73904a8e0f4365a8cb2 
> 
> 
> Diff: https://reviews.apache.org/r/69390/diff/3/
> 
> 
> Testing
> ---
> 
> Checked the rendered Markdown code.
> 
> 
> Thanks,
> 
> Armand Grillet
> 
>



Re: Review Request 69311: Enabled parallel test runner to cmake build.

2018-11-20 Thread Till Toenshoff via Review Board

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


Ship it!




Ship It!

- Till Toenshoff


On Nov. 14, 2018, 1:11 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69311/
> ---
> 
> (Updated Nov. 14, 2018, 1:11 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Joseph Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Enabled parallel test runner to cmake build.
> 
> 
> Diffs
> -
> 
>   cmake/MesosConfigure.cmake de7dc08cc5ba4eaead017a97dcfeaf96bd0f4dbe 
>   src/tests/CMakeLists.txt 553516ad66cab4480b7211950fc726b7d9a3869b 
> 
> 
> Diff: https://reviews.apache.org/r/69311/diff/2/
> 
> 
> Testing
> ---
> 
> Tested as part of https://reviews.apache.org/r/69313/.
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>



Re: Review Request 69312: Enabled parallel test runner to cmake build.

2018-11-20 Thread Till Toenshoff via Review Board

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


Ship it!




Ship It!

- Till Toenshoff


On Nov. 11, 2018, 12:01 a.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69312/
> ---
> 
> (Updated Nov. 11, 2018, 12:01 a.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Joseph Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Enabled parallel test runner to cmake build.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/tests/CMakeLists.txt 
> 2bea6cf89bed85c8a590de8570abd6f3bb1c1106 
> 
> 
> Diff: https://reviews.apache.org/r/69312/diff/1/
> 
> 
> Testing
> ---
> 
> Tested as part of https://reviews.apache.org/r/69313/.
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>



Re: Review Request 69313: Enabled parallel test runner to cmake build.

2018-11-20 Thread Till Toenshoff via Review Board

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


Ship it!




- Till Toenshoff


On Nov. 11, 2018, 12:01 a.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69313/
> ---
> 
> (Updated Nov. 11, 2018, 12:01 a.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Joseph Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Enabled parallel test runner to cmake build.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/tests/CMakeLists.txt 
> ff0660707c116cae8174f2ebffb6e2a2698a2c49 
> 
> 
> Diff: https://reviews.apache.org/r/69313/diff/1/
> 
> 
> Testing
> ---
> 
> `ninja check`
> `ninja tests && ctest -j3 -V`
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>



Re: Review Request 69390: Added docs describing how to use the new CLI.

2018-11-20 Thread Armand Grillet

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

(Updated Nov. 20, 2018, 12:12 p.m.)


Review request for mesos and Kevin Klues.


Changes
---

Fixed issue.


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


Repository: mesos


Description (updated)
---

The documentation describes the main commands of the new CLI, how to
activate it, how to build Mesos including this component, and how to
write a configuration file for it.


Diffs (updated)
-

  docs/cli.md PRE-CREATION 
  docs/home.md e05b65d55176a706c072f73904a8e0f4365a8cb2 


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

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


Testing
---

Checked the rendered Markdown code.


Thanks,

Armand Grillet



Re: Review Request 69390: Added docs describing how to use the new CLI.

2018-11-20 Thread Kevin Klues

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




docs/cli.md
Lines 20 (patched)


We should move the text that says:
```
For now, the Mesos  Command Line Interface is still under development
and not built as part of a standard Mesos distribution.
```
to here
and have a better opening line for the whole document.


- Kevin Klues


On Nov. 20, 2018, 10:57 vorm., Armand Grillet wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69390/
> ---
> 
> (Updated Nov. 20, 2018, 10:57 vorm.)
> 
> 
> Review request for mesos and Kevin Klues.
> 
> 
> Bugs: MESOS-9333
> https://issues.apache.org/jira/browse/MESOS-9333
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The documentation describes the main commands of the CLI, how to enable
> it and how to build Mesos including this component.
> 
> 
> Diffs
> -
> 
>   docs/cli.md PRE-CREATION 
>   docs/home.md e05b65d55176a706c072f73904a8e0f4365a8cb2 
> 
> 
> Diff: https://reviews.apache.org/r/69390/diff/2/
> 
> 
> Testing
> ---
> 
> Checked the rendered Markdown code.
> 
> 
> Thanks,
> 
> Armand Grillet
> 
>



Re: Review Request 69390: Added docs describing how to use the new CLI.

2018-11-20 Thread Armand Grillet

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

(Updated Nov. 20, 2018, 11:57 a.m.)


Review request for mesos and Kevin Klues.


Changes
---

Fixed issues.


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


Repository: mesos


Description
---

The documentation describes the main commands of the CLI, how to enable
it and how to build Mesos including this component.


Diffs (updated)
-

  docs/cli.md PRE-CREATION 
  docs/home.md e05b65d55176a706c072f73904a8e0f4365a8cb2 


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

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


Testing
---

Checked the rendered Markdown code.


Thanks,

Armand Grillet