Re: Review Request 40346: [2/4] Quota Authorization: Implemented authorization of quota requests in the authorizer.

[Jan Schlicht]

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

(Updated Dec. 18, 2015, 10:09 a.m.)


Review request for mesos, Alexander Rukletsov, Alexander Rojas, Joris Van 
Remoortere, and Till Toenshoff.


Changes
---

Rebased.


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


Repository: mesos


Description
---

Quota: Implemented authorization of quota requests in the authorizer.


Diffs (updated)
-

  include/mesos/authorizer/authorizer.hpp 
aa9bb8b556f80a7c9c0fa3db95643efb49368dd5 
  src/authorizer/local/authorizer.hpp eb95c9fdb393f69501841da0e8a2342ceb3ab7e0 
  src/authorizer/local/authorizer.cpp 46e181a907bc5248a403baf9e02200fee7fc4ba3 
  src/tests/mesos.hpp 1c6acabab9d189a6d3cc8d63359053fd230377b8 
  src/tests/mesos.cpp 50ba8c448fcc4cee4d69074472c4e0e0c6cf7c0d 

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


Testing
---

make check


Thanks,

Jan Schlicht

Re: Review Request 40345: [1/4] Quota Authorization: Added "SetQuota" message to ACL protobuf.

[Jan Schlicht]

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

(Updated Dec. 18, 2015, 10:09 a.m.)


Review request for mesos, Alexander Rukletsov, Alexander Rojas, Joris Van 
Remoortere, and Till Toenshoff.


Changes
---

Rebased.


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


Repository: mesos


Description
---

Quota: Added "SetQuota" message to ACL protobuf.


Diffs (updated)
-

  include/mesos/authorizer/authorizer.proto 
7f981e6d9cdd79a7d58b943bdc5abc81b355092f 

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


Testing
---

make check


Thanks,

Jan Schlicht

Re: Review Request 39520: Quota: Added authentication, authorization tests.

[Jan Schlicht]

> On Dec. 14, 2015, 5:18 p.m., Alexander Rukletsov wrote:
> > I would suggest to add some more test cases:
> > - Request does not contain a principal (in absence of authz quota can be 
> > set);
> > - Request does not contain a principal, ACLs allow `ANY` to set quota for 
> > role "prod" (absence of principal maps to `ANY`).

The new test fixture `AuthorizedQuotaSetRequestWithoutPrincipal` now tests for 
authorization with an empty principal if authentication is disabled.
`AuthorizationTest.SetQuota` now includes a rule where `ANY` can set quota for 
role "test" and tests that rule.


- Jan


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


On Dec. 18, 2015, 10:12 a.m., Jan Schlicht wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39520/
> ---
> 
> (Updated Dec. 18, 2015, 10:12 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Till Toenshoff.
> 
> 
> Bugs: MESOS-4082
> https://issues.apache.org/jira/browse/MESOS-4082
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Quota: Added authentication, authorization tests.
> 
> 
> Diffs
> -
> 
>   src/tests/authorization_tests.cpp dcf348acc75b143e3a854bde17e6042dc90599d5 
>   src/tests/master_quota_tests.cpp 0473869783a714766ed26fff61d7f8c56342df74 
> 
> Diff: https://reviews.apache.org/r/39520/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jan Schlicht
> 
>

Re: Review Request 41489: Provisioner: Implemented docker v1 parse serialization method in spec.

[Gilbert Song]

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

(Updated Dec. 18, 2015, 1:20 a.m.)


Review request for mesos, Artem Harutyunyan, Jie Yu, and Timothy Chen.


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


Repository: mesos


Description
---

Provisioner: Implemented docker v1 parse serialization method in spec.


Diffs (updated)
-

  src/slave/containerizer/mesos/provisioner/docker/registry_client.cpp 
e69bab43d5f5359cfd8eb4cb7c5ad4a1d22c4e05 
  src/slave/containerizer/mesos/provisioner/docker/spec.hpp 
e674b6f2e705f8b3e49770560eeab4c127473f94 
  src/slave/containerizer/mesos/provisioner/docker/spec.cpp 
1f05c75dc9473bd5e4c0d3f74fa0ef996b96a84e 
  src/tests/containerizer/provisioner_docker_tests.cpp 
3f1717b770e139c3759aab0aeda9dbcf5029b0c2 

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


Testing
---

make check


Thanks,

Gilbert Song

Re: Review Request 41490: Provisioner: Added test case for docker v1 manifest serialization.

[Gilbert Song]

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

(Updated Dec. 18, 2015, 1:21 a.m.)


Review request for mesos, Artem Harutyunyan, Jie Yu, and Timothy Chen.


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


Repository: mesos


Description
---

Provisioner: Added test case for docker v1 manifest serialization.


Diffs (updated)
-

  src/tests/containerizer/provisioner_docker_tests.cpp 
3f1717b770e139c3759aab0aeda9dbcf5029b0c2 

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


Testing
---

make check


Thanks,

Gilbert Song

Re: Review Request 41491: Unified Container: Implemented passing entrypoint in runtime config.

[Gilbert Song]

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

(Updated Dec. 18, 2015, 1:21 a.m.)


Review request for mesos, Artem Harutyunyan, Jie Yu, and Timothy Chen.


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


Repository: mesos


Description
---

Unified Container: Implemented passing entrypoint in runtime config.


Diffs (updated)
-

  src/slave/containerizer/mesos/provisioner/docker/store.cpp 
1ad7b67a94b1d9367afcb7c30a6d01fdf6b8ab6c 
  src/slave/containerizer/mesos/provisioner/store.hpp 
8d1493856420dee3210af79b628c8c770c5c8550 
  src/tests/containerizer/provisioner_docker_tests.cpp 
3f1717b770e139c3759aab0aeda9dbcf5029b0c2 

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


Testing
---

make check


Thanks,

Gilbert Song

Re: Review Request 40255: [5/7] Added framework authorization for persistent volumes.

[Greg Mann]

> On Dec. 16, 2015, 2:38 p.m., Alexander Rukletsov wrote:
> > src/tests/persistent_volume_tests.cpp, line 790
> > 
> >
> > Do you think it makes sense to extract "role1" into a constant?

Since this role name appears throughout the file, I think I'd rather follow 
these up with a patch that makes this change for all of the persistent volume 
tests. What do you think?


> On Dec. 16, 2015, 2:38 p.m., Alexander Rukletsov wrote:
> > src/master/master.cpp, line 3342
> > 
> >
> > I see that you follow the pattern here, but why does the master commit 
> > suicide if the future fails? My understanding is that this does not violate 
> > any internal invariant and should lead to a retry. Am I missing something? 
> > Do we need a comment explaining the reason?

Thanks for calling this out - I agree that it's an error and should be removed 
from the RESERVE/UNRESERVE code as well. Check out what I've put in place of 
this and see what you think. I can submit a patch for RESERVE/UNRESERVE also.


> On Dec. 16, 2015, 2:38 p.m., Alexander Rukletsov wrote:
> > src/master/master.cpp, line 3348
> > 
> >
> > You are following the pattern here, but are we sure that the framework 
> > has the principal? I also do not see any tests with frameworks without 
> > principals (nor in "reservation_tests.cpp"). It looks like an unsuccessful 
> > authorization for a framework without a principal can kill the master.

I added tests without a principal, but this code shouldn't lead to a crash of 
the master. `principal` is an optional field in `FrameworkInfo`, which means 
that if it isn't supplied, it will be initialized with the default value: an 
empty string. So if the framework has no principal, this will result in the 
logging output: "Authorization of principal '' to create persistent volumes 
failed", which seems OK to me. I'm going to drop this for now, but feel free to 
re-open if I'm missing something or if you disagree for another reason.


> On Dec. 16, 2015, 2:38 p.m., Alexander Rukletsov wrote:
> > src/master/master.cpp, lines 3348-3349
> > 
> >
> > I believe we need an extra blank line in this case. Also in other 
> > places above and below : )

I changed this in the current patch, and I'll follow up with another for 
RESERVE/UNRESERVE.


> On Dec. 16, 2015, 2:38 p.m., Alexander Rukletsov wrote:
> > src/tests/persistent_volume_tests.cpp, lines 717-719
> > 
> >
> > Could you please add a test with a framework without a principal?
> > 
> > In the same vein, do you think it makes sense to create a ticket for 
> > the same case for dynamic reservatons (even though we require the principal 
> > for now)?

Excellent idea, I've added two tests to this patch for cases with no principal 
and created a ticket for RESERVE/UNRESERVE: 
https://issues.apache.org/jira/browse/MESOS-4195


> On Dec. 16, 2015, 2:38 p.m., Alexander Rukletsov wrote:
> > src/tests/persistent_volume_tests.cpp, lines 968-970
> > 
> >
> > Do you think it makes sense to merge this test with the first one by 
> > creating a second framework with a different principal and allowing only 
> > that framework to destroy the volume?

Good idea! Done.


- Greg


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


On Dec. 18, 2015, 9:28 a.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40255/
> ---
> 
> (Updated Dec. 18, 2015, 9:28 a.m.)
> 
> 
> Review request for mesos, Jie Yu, Michael Park, and Neil Conway.
> 
> 
> Bugs: MESOS-3065
> https://issues.apache.org/jira/browse/MESOS-3065
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added framework authorization for persistent volumes.
> 
> 
> Diffs
> -
> 
>   src/master/master.cpp 0d1482279c68f2a4a27dabaf28774769a5d515c4 
>   src/tests/persistent_volume_tests.cpp 
> 01b3c13751a5558d5f06edb8f650c8644dc54486 
> 
> Diff: https://reviews.apache.org/r/40255/diff/
> 
> 
> Testing
> ---
> 
> This is the fifth in a chain of 7 patches. New tests were added to 
> `persistent_volume_tests.cpp` in order to test a framework attempting both 
> successful and failed authorizations for `CREATE` and `DESTROY` offer 
> operations. `make check` was used to test after all 

Re: Review Request 40903: Ported approximated Option CPU resource number comparison to v1 and improved the check expression for this.

[Alexander Rukletsov]

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



src/v1/resources.cpp (line 39)


Unnecessary blank line.



src/v1/resources.cpp (line 49)


I'm not sure that's the way we do it, but I would prefer to remove this 
using and write `mesos::internal::master::MIN_CPUS` explicitly to increase the 
pain and motivate ourselves remove that dependency ASAP.

Also in "src/common/resources.cpp"


- Alexander Rukletsov


On Dec. 3, 2015, 11:09 a.m., Bernd Mathiske wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40903/
> ---
> 
> (Updated Dec. 3, 2015, 11:09 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Avinash sridharan, Ben Mahler, 
> Greg Mann, Klaus Ma, Mandeep Chadha, Neil Conway, and Till Toenshoff.
> 
> 
> Bugs: MESOS-3552
> https://issues.apache.org/jira/browse/MESOS-3552
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Copied the changes from common/resources.cpp in r40767 to v1/reources.cpp, 
> also addressing BenM's post-review. Thanks to @greggomann for spotting this.
> 
> 
> Diffs
> -
> 
>   src/common/resources.cpp 5a7981744726a0544435cbbd5007487a43a01211 
>   src/v1/resources.cpp d300842576b2fb8fe75f5b18699f23c1fd324717 
> 
> Diff: https://reviews.apache.org/r/40903/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Bernd Mathiske
> 
>

Re: Review Request 40256: [6/7] Fixed handling of multiple offer operations in PersistentVolumeTest.SendingCheckpointResourcesMessage.

[Greg Mann]

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

(Updated Dec. 18, 2015, 9:38 a.m.)


Review request for mesos, Jie Yu, Michael Park, and Neil Conway.


Changes
---

Rebase.


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


Repository: mesos


Description
---

Fixed handling of multiple offer operations in 
PersistentVolumeTest.SendingCheckpointResourcesMessage.


Diffs (updated)
-

  src/tests/persistent_volume_tests.cpp 
01b3c13751a5558d5f06edb8f650c8644dc54486 

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


Testing
---

This is the sixth in a chain of 7 patches. This patch was required in order to 
fix the `PersistentVolumeTest.SendingCheckpointResourcesMessage` test, which 
was broken by the addition of authorization to the `CREATE` and `DESTROY` offer 
operations. The test was previously both creating and destroying a persistent 
volume in a single `acceptOffer` message. However, our `validate` method for 
`DESTROY` offer operations correctly enforces that in order to delete a volume, 
the volume should be present in the checkpointed resources of the relevant 
Agent. This is likely not the case if the `CREATE` and `DESTROY` operations are 
both in the same message.

`make check` was used to test after all patches were applied.


Thanks,

Greg Mann

Re: Review Request 40271: [7/7] Added documentation for RESERVE, UNRESERVE, CREATE, and DESTROY authorization.

[Greg Mann]

> On Dec. 16, 2015, 2:39 p.m., Alexander Rukletsov wrote:
> > docs/authorization.md, line 43
> > 
> >
> > As I have already mentioned in a previous review, we do not really 
> > enforce it, do we? That's why I think "valid" is a bit too strong, what do 
> > you think?
> > 
> > Same holds for create/destroy volumes.

I altered this text to be more descriptive of the current state of affairs, let 
me know if you think it should be changed further.


- Greg


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


On Dec. 18, 2015, 9:54 a.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40271/
> ---
> 
> (Updated Dec. 18, 2015, 9:54 a.m.)
> 
> 
> Review request for mesos, Jie Yu, Michael Park, and Neil Conway.
> 
> 
> Bugs: MESOS-3062 and MESOS-3065
> https://issues.apache.org/jira/browse/MESOS-3062
> https://issues.apache.org/jira/browse/MESOS-3065
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added documentation for RESERVE, UNRESERVE, CREATE, and DESTROY authorization.
> 
> 
> Diffs
> -
> 
>   docs/authorization.md 1fc0e3f9a686480ffc4deb25e49f867318d1321a 
>   docs/persistent-volume.md cf7a6bbb6044e460293dd66066df87aded0b4fb8 
>   docs/reservation.md de447664f10b1d73211805b725f2284b07c609f6 
> 
> Diff: https://reviews.apache.org/r/40271/diff/
> 
> 
> Testing
> ---
> 
> This is the last in a chain of 7 patches.
> 
> This documentation was tested by viewing with the Mesos Website Container 
> (https://github.com/mesosphere/mesos-website-container).
> 
> `make check` was run after all patches in the chain were applied.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 41444: Cleaned up Authorizer interface.

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [41444]

Passed command: export OS=ubuntu:14.04;export CONFIGURATION="--verbose";export 
COMPILER=gcc; ./support/docker_build.sh

- Mesos ReviewBot


On Dec. 16, 2015, 1:03 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41444/
> ---
> 
> (Updated Dec. 16, 2015, 1:03 p.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rojas, Greg Mann, Jan Schlicht, 
> and Till Toenshoff.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Extract a repetitive part of the function comments into a class comment. 
> Added backticks, quotes when necessary, formatted comments to avoid 
> jaggedness.
> 
> 
> Diffs
> -
> 
>   include/mesos/authorizer/authorizer.hpp 
> ec3d3e9c9bae461c8cd136f17cfda58e0cbd4e1b 
>   src/master/main.cpp e00f878770f3e0bddae5a137b50a00822d154e2c 
> 
> Diff: https://reviews.apache.org/r/41444/diff/
> 
> 
> Testing
> ---
> 
> None: not a functional change.
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 40348: [4/4] Quota Authorization: Documented quota authorization.

[Jan Schlicht]

> On Dec. 14, 2015, 11:13 p.m., Greg Mann wrote:
> > docs/authorization.md, line 30
> > 
> >
> > I wonder if there should be some mention of the behavior of 
> > `set_quotas` when no principal is set?

I added a more general description that also applies to "shutdown_frameworks", 
as the behavior is the same there.


- Jan


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


On Dec. 18, 2015, 10:55 a.m., Jan Schlicht wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40348/
> ---
> 
> (Updated Dec. 18, 2015, 10:55 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Alexander Rojas, Joris Van 
> Remoortere, and Till Toenshoff.
> 
> 
> Bugs: MESOS-3862
> https://issues.apache.org/jira/browse/MESOS-3862
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Quota: Documented quota authorization.
> 
> 
> Diffs
> -
> 
>   docs/authorization.md 1fc0e3f9a686480ffc4deb25e49f867318d1321a 
>   src/master/flags.cpp f9223edb33483cb5619e7dd75221f735306462c6 
> 
> Diff: https://reviews.apache.org/r/40348/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jan Schlicht
> 
>

Re: Review Request 40255: [5/7] Added framework authorization for persistent volumes.

[Alexander Rukletsov]

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



src/master/master.cpp (lines 3346 - 3350)


How about a TODO, that we may want to retry instead of giving up straight 
away?


- Alexander Rukletsov


On Dec. 18, 2015, 9:37 a.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40255/
> ---
> 
> (Updated Dec. 18, 2015, 9:37 a.m.)
> 
> 
> Review request for mesos, Jie Yu, Michael Park, and Neil Conway.
> 
> 
> Bugs: MESOS-3065
> https://issues.apache.org/jira/browse/MESOS-3065
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added framework authorization for persistent volumes.
> 
> 
> Diffs
> -
> 
>   src/master/master.cpp 470b542729b01f41fc6a2e601a7a6c3d0c5353d5 
>   src/tests/persistent_volume_tests.cpp 
> 01b3c13751a5558d5f06edb8f650c8644dc54486 
> 
> Diff: https://reviews.apache.org/r/40255/diff/
> 
> 
> Testing
> ---
> 
> This is the fifth in a chain of 7 patches. New tests were added to 
> `persistent_volume_tests.cpp` in order to test a framework attempting both 
> successful and failed authorizations for `CREATE` and `DESTROY` offer 
> operations. `make check` was used to test after all patches were applied.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 40255: [5/7] Added framework authorization for persistent volumes.

[Greg Mann]

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

(Updated Dec. 18, 2015, 10:10 a.m.)


Review request for mesos, Jie Yu, Michael Park, and Neil Conway.


Changes
---

Addressed comment.


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


Repository: mesos


Description
---

Added framework authorization for persistent volumes.


Diffs (updated)
-

  src/master/master.cpp 470b542729b01f41fc6a2e601a7a6c3d0c5353d5 
  src/tests/persistent_volume_tests.cpp 
01b3c13751a5558d5f06edb8f650c8644dc54486 

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


Testing
---

This is the fifth in a chain of 7 patches. New tests were added to 
`persistent_volume_tests.cpp` in order to test a framework attempting both 
successful and failed authorizations for `CREATE` and `DESTROY` offer 
operations. `make check` was used to test after all patches were applied.


Thanks,

Greg Mann

Re: Review Request 40255: [5/7] Added framework authorization for persistent volumes.

[Greg Mann]

> On Dec. 18, 2015, 9:58 a.m., Alexander Rukletsov wrote:
> > src/master/master.cpp, lines 3346-3350
> > 
> >
> > How about a TODO, that we may want to retry instead of giving up 
> > straight away?

Yea, sounds good, TODOs added!


- Greg


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


On Dec. 18, 2015, 10:10 a.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40255/
> ---
> 
> (Updated Dec. 18, 2015, 10:10 a.m.)
> 
> 
> Review request for mesos, Jie Yu, Michael Park, and Neil Conway.
> 
> 
> Bugs: MESOS-3065
> https://issues.apache.org/jira/browse/MESOS-3065
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added framework authorization for persistent volumes.
> 
> 
> Diffs
> -
> 
>   src/master/master.cpp 470b542729b01f41fc6a2e601a7a6c3d0c5353d5 
>   src/tests/persistent_volume_tests.cpp 
> 01b3c13751a5558d5f06edb8f650c8644dc54486 
> 
> Diff: https://reviews.apache.org/r/40255/diff/
> 
> 
> Testing
> ---
> 
> This is the fifth in a chain of 7 patches. New tests were added to 
> `persistent_volume_tests.cpp` in order to test a framework attempting both 
> successful and failed authorizations for `CREATE` and `DESTROY` offer 
> operations. `make check` was used to test after all patches were applied.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 41472: Disallowed dynamic reservations for roles not on the role whitelist.

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [40995, 41075, 41225, 41408, 41472]

Passed command: export OS=ubuntu:14.04;export CONFIGURATION="--verbose";export 
COMPILER=gcc; ./support/docker_build.sh

- Mesos ReviewBot


On Dec. 16, 2015, 10:48 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41472/
> ---
> 
> (Updated Dec. 16, 2015, 10:48 p.m.)
> 
> 
> Review request for mesos, Adam B and Michael Park.
> 
> 
> Bugs: MESOS-4143
> https://issues.apache.org/jira/browse/MESOS-4143
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Also added a test that dynamic reservations via the "/reserve" endpoint are
> allowed when using implicit roles.
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp 8dfd67f7d51b8395953d6beb77b2d71bc538eacd 
>   src/tests/reservation_endpoints_tests.cpp 
> b8edd6fafedd4c2221a8d19c1ebc71254071a8c7 
> 
> Diff: https://reviews.apache.org/r/41472/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 40347: [3/4] Quota Authorization: Implemented quota request authorization.

[Jan Schlicht]

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

(Updated Dec. 18, 2015, 10:10 a.m.)


Review request for mesos, Alexander Rukletsov, Alexander Rojas, Joris Van 
Remoortere, and Till Toenshoff.


Changes
---

Rebased.


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


Repository: mesos


Description
---

Quota: Implemented quota request authorization.


Diffs (updated)
-

  src/master/master.hpp 7cb0e1692644e51271588abffa832e08c536b838 
  src/master/quota_handler.cpp 11167879b2480d9c8dd6398ca39c479089ec2272 

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


Testing
---

make check


Thanks,

Jan Schlicht

Re: Review Request 40255: [5/7] Added framework authorization for persistent volumes.

[Greg Mann]

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

(Updated Dec. 18, 2015, 9:33 a.m.)


Review request for mesos, Jie Yu, Michael Park, and Neil Conway.


Changes
---

`process::Clock` -> `Clock`


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


Repository: mesos


Description
---

Added framework authorization for persistent volumes.


Diffs (updated)
-

  src/master/master.cpp 0d1482279c68f2a4a27dabaf28774769a5d515c4 
  src/tests/persistent_volume_tests.cpp 
01b3c13751a5558d5f06edb8f650c8644dc54486 

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


Testing
---

This is the fifth in a chain of 7 patches. New tests were added to 
`persistent_volume_tests.cpp` in order to test a framework attempting both 
successful and failed authorizations for `CREATE` and `DESTROY` offer 
operations. `make check` was used to test after all patches were applied.


Thanks,

Greg Mann

Re: Review Request 41444: Cleaned up Authorizer interface.

[Adam B]

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


Thank you for cleaning this up. It looked like an overwhelming amount of 
documentation for what is not really that complex of an API. It still looks a 
bit verbose/repetitive, so I've made some suggestions of what else to cut out.
I guess we're still waiting on the ACLs for create/remove persistent volumes, 
in MESOS-4179


include/mesos/authorizer/authorizer.hpp (line 40)


s/logic/interpretation/? Or "meaning"?
Or s/has the same logic/can be interpreted the same/



include/mesos/authorizer/authorizer.hpp (lines 64 - 65)


How formal. I would think you could get away with
s/the "authorizer.proto" file/"authorizor.proto"/
s|the "docs/authorization.md"|"docs/authorization.md"|



include/mesos/authorizer/authorizer.hpp (line 66)


s/on/of/



include/mesos/authorizer/authorizer.hpp (line 69)


s/autorizer/authorizer/
s/on/of/



include/mesos/authorizer/authorizer.hpp (line 75)


What is "it"? Are we removing the initialize function, the acls parameter, 
or what?

This seems very related to the first paragraph "Only relevant..." which 
should not be the first paragraph in the doxygen, since it is in no way a 
summary of the method.



include/mesos/authorizer/authorizer.hpp (lines 83 - 84)


"The `principal` and `role` parameters are packed in the request." seems 
superfluous to the summary, and is already included in the 'request' @param. 
Delete this sentence.



include/mesos/authorizer/authorizer.hpp (line 100)


Kill the "packed in" sentence for all of these. That info is in the request 
@param comment.



include/mesos/authorizer/authorizer.hpp (lines 102 - 104)


How about just "`ACL::RunTask` packing all the parameters..."? We can 
figure out that it's an instance of a protobuf message.



include/mesos/authorizer/authorizer.hpp (lines 107 - 109)


You can probably shorten this here and everywhere by just saying "A failed 
future indicates a problem processing the request; the request can be retried."



include/mesos/authorizer/authorizer.hpp (line 119)


s/RunTask/ShutdownFramework/



include/mesos/authorizer/authorizer.hpp (line 124)


s/ran/registered/



include/mesos/authorizer/authorizer.hpp (line 133)


s/reserve particular resources/reserve resources/ since the only values 
currently allowed for `resources` are ANY or NONE.



include/mesos/authorizer/authorizer.hpp (line 138)


s/reserve one or more types of resources/reserve resources/



include/mesos/authorizer/authorizer.hpp (lines 140 - 141)


s/reserve the types of resources contained in the request/reserve resources/


- Adam B


On Dec. 16, 2015, 5:03 a.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41444/
> ---
> 
> (Updated Dec. 16, 2015, 5:03 a.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rojas, Greg Mann, Jan Schlicht, 
> and Till Toenshoff.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Extract a repetitive part of the function comments into a class comment. 
> Added backticks, quotes when necessary, formatted comments to avoid 
> jaggedness.
> 
> 
> Diffs
> -
> 
>   include/mesos/authorizer/authorizer.hpp 
> ec3d3e9c9bae461c8cd136f17cfda58e0cbd4e1b 
>   src/master/main.cpp e00f878770f3e0bddae5a137b50a00822d154e2c 
> 
> Diff: https://reviews.apache.org/r/41444/diff/
> 
> 
> Testing
> ---
> 
> None: not a functional change.
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 39520: Quota: Added authentication, authorization tests.

[Jan Schlicht]

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

(Updated Dec. 18, 2015, 10:12 a.m.)


Review request for mesos, Alexander Rukletsov and Till Toenshoff.


Changes
---

Addressed issues.


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


Repository: mesos


Description
---

Quota: Added authentication, authorization tests.


Diffs (updated)
-

  src/tests/authorization_tests.cpp dcf348acc75b143e3a854bde17e6042dc90599d5 
  src/tests/master_quota_tests.cpp 0473869783a714766ed26fff61d7f8c56342df74 

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


Testing
---

make check


Thanks,

Jan Schlicht

Re: Review Request 40255: [5/7] Added framework authorization for persistent volumes.

[Greg Mann]

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

(Updated Dec. 18, 2015, 9:28 a.m.)


Review request for mesos, Jie Yu, Michael Park, and Neil Conway.


Changes
---

Addressed comments.


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


Repository: mesos


Description
---

Added framework authorization for persistent volumes.


Diffs (updated)
-

  src/master/master.cpp 0d1482279c68f2a4a27dabaf28774769a5d515c4 
  src/tests/persistent_volume_tests.cpp 
01b3c13751a5558d5f06edb8f650c8644dc54486 

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


Testing
---

This is the fifth in a chain of 7 patches. New tests were added to 
`persistent_volume_tests.cpp` in order to test a framework attempting both 
successful and failed authorizations for `CREATE` and `DESTROY` offer 
operations. `make check` was used to test after all patches were applied.


Thanks,

Greg Mann

Re: Review Request 40255: [5/7] Added framework authorization for persistent volumes.

[Greg Mann]

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

(Updated Dec. 18, 2015, 9:37 a.m.)


Review request for mesos, Jie Yu, Michael Park, and Neil Conway.


Changes
---

Rebase.


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


Repository: mesos


Description
---

Added framework authorization for persistent volumes.


Diffs (updated)
-

  src/master/master.cpp 470b542729b01f41fc6a2e601a7a6c3d0c5353d5 
  src/tests/persistent_volume_tests.cpp 
01b3c13751a5558d5f06edb8f650c8644dc54486 

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


Testing
---

This is the fifth in a chain of 7 patches. New tests were added to 
`persistent_volume_tests.cpp` in order to test a framework attempting both 
successful and failed authorizations for `CREATE` and `DESTROY` offer 
operations. `make check` was used to test after all patches were applied.


Thanks,

Greg Mann

Re: Review Request 40255: [5/7] Added framework authorization for persistent volumes.

[Alexander Rukletsov]

> On Dec. 16, 2015, 2:38 p.m., Alexander Rukletsov wrote:
> > src/master/master.cpp, line 3348
> > 
> >
> > You are following the pattern here, but are we sure that the framework 
> > has the principal? I also do not see any tests with frameworks without 
> > principals (nor in "reservation_tests.cpp"). It looks like an unsuccessful 
> > authorization for a framework without a principal can kill the master.
> 
> Greg Mann wrote:
> I added tests without a principal, but this code shouldn't lead to a 
> crash of the master. `principal` is an optional field in `FrameworkInfo`, 
> which means that if it isn't supplied, it will be initialized with the 
> default value: an empty string. So if the framework has no principal, this 
> will result in the logging output: "Authorization of principal '' to create 
> persistent volumes failed", which seems OK to me. I'm going to drop this for 
> now, but feel free to re-open if I'm missing something or if you disagree for 
> another reason.

You are right, my bad.


> On Dec. 16, 2015, 2:38 p.m., Alexander Rukletsov wrote:
> > src/tests/persistent_volume_tests.cpp, lines 717-719
> > 
> >
> > Could you please add a test with a framework without a principal?
> > 
> > In the same vein, do you think it makes sense to create a ticket for 
> > the same case for dynamic reservatons (even though we require the principal 
> > for now)?
> 
> Greg Mann wrote:
> Excellent idea, I've added two tests to this patch for cases with no 
> principal and created a ticket for RESERVE/UNRESERVE: 
> https://issues.apache.org/jira/browse/MESOS-4195

Thanks! I've noticed we usually do not test cases like "authn is off, authz is 
on, framework has a principal", "authn is off, authz if off, framework has no 
principal", though, I would say, are real-world scenarios (for test clusters 
only I hope : ) ).


> On Dec. 16, 2015, 2:38 p.m., Alexander Rukletsov wrote:
> > src/tests/persistent_volume_tests.cpp, line 790
> > 
> >
> > Do you think it makes sense to extract "role1" into a constant?
> 
> Greg Mann wrote:
> Since this role name appears throughout the file, I think I'd rather 
> follow these up with a patch that makes this change for all of the persistent 
> volume tests. What do you think?

That's fine.


- Alexander


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


On Dec. 18, 2015, 9:37 a.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40255/
> ---
> 
> (Updated Dec. 18, 2015, 9:37 a.m.)
> 
> 
> Review request for mesos, Jie Yu, Michael Park, and Neil Conway.
> 
> 
> Bugs: MESOS-3065
> https://issues.apache.org/jira/browse/MESOS-3065
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added framework authorization for persistent volumes.
> 
> 
> Diffs
> -
> 
>   src/master/master.cpp 470b542729b01f41fc6a2e601a7a6c3d0c5353d5 
>   src/tests/persistent_volume_tests.cpp 
> 01b3c13751a5558d5f06edb8f650c8644dc54486 
> 
> Diff: https://reviews.apache.org/r/40255/diff/
> 
> 
> Testing
> ---
> 
> This is the fifth in a chain of 7 patches. New tests were added to 
> `persistent_volume_tests.cpp` in order to test a framework attempting both 
> successful and failed authorizations for `CREATE` and `DESTROY` offer 
> operations. `make check` was used to test after all patches were applied.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 40348: [4/4] Quota Authorization: Documented quota authorization.

[Jan Schlicht]

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

(Updated Dec. 18, 2015, 10:55 a.m.)


Review request for mesos, Alexander Rukletsov, Alexander Rojas, Joris Van 
Remoortere, and Till Toenshoff.


Changes
---

Addressed issues.


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


Repository: mesos


Description
---

Quota: Documented quota authorization.


Diffs (updated)
-

  docs/authorization.md 1fc0e3f9a686480ffc4deb25e49f867318d1321a 
  src/master/flags.cpp f9223edb33483cb5619e7dd75221f735306462c6 

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


Testing
---

make check


Thanks,

Jan Schlicht

Re: Review Request 40271: [7/7] Added documentation for RESERVE, UNRESERVE, CREATE, and DESTROY authorization.

[Greg Mann]

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

(Updated Dec. 18, 2015, 9:54 a.m.)


Review request for mesos, Jie Yu, Michael Park, and Neil Conway.


Changes
---

Addressed comments, rebased.


Bugs: MESOS-3062 and MESOS-3065
https://issues.apache.org/jira/browse/MESOS-3062
https://issues.apache.org/jira/browse/MESOS-3065


Repository: mesos


Description
---

Added documentation for RESERVE, UNRESERVE, CREATE, and DESTROY authorization.


Diffs (updated)
-

  docs/authorization.md 1fc0e3f9a686480ffc4deb25e49f867318d1321a 
  docs/persistent-volume.md cf7a6bbb6044e460293dd66066df87aded0b4fb8 
  docs/reservation.md de447664f10b1d73211805b725f2284b07c609f6 

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


Testing
---

This is the last in a chain of 7 patches.

This documentation was tested by viewing with the Mesos Website Container 
(https://github.com/mesosphere/mesos-website-container).

`make check` was run after all patches in the chain were applied.


Thanks,

Greg Mann

Re: Review Request 40255: [5/7] Added framework authorization for persistent volumes.

[Alexander Rukletsov]

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



src/tests/persistent_volume_tests.cpp (lines 887 - 890)


It looks like this rule is not used in this test. Could you please explain 
why you have it here?



src/tests/persistent_volume_tests.cpp (line 908)


I would merge this comment with the one on L903, this way you can avoid 
inserting a missing blank line between L907 and L908 : ).

Same below.



src/tests/persistent_volume_tests.cpp (lines 978 - 981)


I think you decided to rewrite this block a bit.

Same for other EXPECT blobs with comments.



src/tests/persistent_volume_tests.cpp (lines 1134 - 1135)


Instead of "magic constants", which may lead to flaky tests, does it make 
sense to `suppressOffers` and then `reviveOffers`?

Same for the next test.



src/tests/persistent_volume_tests.cpp (lines 1185 - 1187)


Why do you think framework1 will be offered resources?


- Alexander Rukletsov


On Dec. 18, 2015, 10:10 a.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40255/
> ---
> 
> (Updated Dec. 18, 2015, 10:10 a.m.)
> 
> 
> Review request for mesos, Jie Yu, Michael Park, and Neil Conway.
> 
> 
> Bugs: MESOS-3065
> https://issues.apache.org/jira/browse/MESOS-3065
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added framework authorization for persistent volumes.
> 
> 
> Diffs
> -
> 
>   src/master/master.cpp 470b542729b01f41fc6a2e601a7a6c3d0c5353d5 
>   src/tests/persistent_volume_tests.cpp 
> 01b3c13751a5558d5f06edb8f650c8644dc54486 
> 
> Diff: https://reviews.apache.org/r/40255/diff/
> 
> 
> Testing
> ---
> 
> This is the fifth in a chain of 7 patches. New tests were added to 
> `persistent_volume_tests.cpp` in order to test a framework attempting both 
> successful and failed authorizations for `CREATE` and `DESTROY` offer 
> operations. `make check` was used to test after all patches were applied.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 40880: Fix flaky MemoryPressureMesosTest.CGROUPS_ROOT_SlaveRecovery test.

[Adam B]

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

Ship it!


Looks good to me. Testing something on the containerizer sounds more accurate 
than waiting for the slave reregistered ack to be received from the master, 
since we're really focused on the executor reregistering with the slave rather 
than the slave with the master.

- Adam B


On Dec. 3, 2015, 11:02 a.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40880/
> ---
> 
> (Updated Dec. 3, 2015, 11:02 a.m.)
> 
> 
> Review request for mesos, Bernd Mathiske, Artem Harutyunyan, and Till 
> Toenshoff.
> 
> 
> Bugs: MESOS-4047
> https://issues.apache.org/jira/browse/MESOS-4047
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> `MesosContainerizerSlaveRecoveryTest.ResourceStatistics` has very similar 
> logic for restarting an agent, re-registering the executor, and even getting 
> `ResourceStatistics`.  But 
> `MesosContainerizerSlaveRecoveryTest.ResourceStatistics` is stable.
> 
> This patch updates the flaky test's wait-for-agent-recovery logic to match 
> the stable test.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/memory_pressure_tests.cpp 
> e18b971c4df26c9b9c103ca73bdad4fd400d6c02 
> 
> Diff: https://reviews.apache.org/r/40880/diff/
> 
> 
> Testing
> ---
> 
> On Ubuntu 14:
> `make check`
> `sudo bin/mesos-tests.sh 
> --gtest_filter="*MemoryPressureMesosTest.CGROUPS_ROOT_SlaveRecovery*" 
> --gtest_repeat=-1 --gtest_break_on_failure`
> 
> ^ Ran the above until satisfied.
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Re: Review Request 40880: Fix flaky MemoryPressureMesosTest.CGROUPS_ROOT_SlaveRecovery test.

[Joerg Schad]

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

Ship it!


Ship It!

- Joerg Schad


On Dec. 3, 2015, 7:02 p.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40880/
> ---
> 
> (Updated Dec. 3, 2015, 7:02 p.m.)
> 
> 
> Review request for mesos, Bernd Mathiske, Artem Harutyunyan, and Till 
> Toenshoff.
> 
> 
> Bugs: MESOS-4047
> https://issues.apache.org/jira/browse/MESOS-4047
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> `MesosContainerizerSlaveRecoveryTest.ResourceStatistics` has very similar 
> logic for restarting an agent, re-registering the executor, and even getting 
> `ResourceStatistics`.  But 
> `MesosContainerizerSlaveRecoveryTest.ResourceStatistics` is stable.
> 
> This patch updates the flaky test's wait-for-agent-recovery logic to match 
> the stable test.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/memory_pressure_tests.cpp 
> e18b971c4df26c9b9c103ca73bdad4fd400d6c02 
> 
> Diff: https://reviews.apache.org/r/40880/diff/
> 
> 
> Testing
> ---
> 
> On Ubuntu 14:
> `make check`
> `sudo bin/mesos-tests.sh 
> --gtest_filter="*MemoryPressureMesosTest.CGROUPS_ROOT_SlaveRecovery*" 
> --gtest_repeat=-1 --gtest_break_on_failure`
> 
> ^ Ran the above until satisfied.
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Re: Review Request 40429: Report executor exit to framework schedulers.

[Adam B]

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


You've got the basics down, but you need to put your EXPECT calls before the 
action that's going to trigger the event you're expecting.


CHANGELOG (line 104)


You're modifying the 0.26 Release Notes, but this is going into 0.27 now.
If this is worth calling out as an "API Change" (probably), then you should 
add it to an "API Changes" section of `Release Notes - Mesos - Version 0.27.0 
(WIP)`.
If not, then you don't have to change this file at all. (The 0.27 release 
manager will fill in the CHANGELOG prior to release.)



docs/upgrades.md (line 24)


This needs to go in the "to 0.27.x" section now.



src/sched/sched.cpp (lines 215 - 216)


Why did you swap these? I think Vinod wanted you to swap 
ExitedExecutorMessage::slave_id and ExitedExecutorMessage::executor_id instead.



src/tests/fault_tolerance_tests.cpp (line 294)


This EXPECT should be set up before you call containerizer.destroy(), in 
case the destroy is so fast that it calls executorLost before you can set up 
the expectation.



src/tests/fault_tolerance_tests.cpp (lines 296 - 300)


If you AWAIT on the executorLost, you probably don't need the 
Clock::pause/settle/resume anymore.



src/tests/fault_tolerance_tests.cpp (line 1717)


Ditto. Move this above containerizer.destroy and consider removing the 
pause/settle/resume. Here and everywhere else.



src/tests/slave_tests.cpp (line 1149)


This executor is lost because of the EXPECT_CALL(containerizer, launch) 
which WillOnce(Return(Failure)), but that's not triggered until the 
driver.launchTasks call. Put your new EXPECT somewhere before 
driver.launchTasks.



src/tests/slave_tests.cpp (line 1391)


This expectation should go above the detector.appoint() which triggers the 
offerRescinded. I wonder why this wasn't needed before. This shouldn't be 
related to your executorLost change.



src/tests/slave_tests.cpp (line 1410)


Move above the post(ShutdownMessage), and maybe WAIT on it, rather than 
potentially stopping the driver before executorLost is delivered.



src/tests/slave_tests.cpp (line 1507)


Move above driver.killTask


- Adam B


On Dec. 14, 2015, 12:09 p.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40429/
> ---
> 
> (Updated Dec. 14, 2015, 12:09 p.m.)
> 
> 
> Review request for mesos, Adam B and Vinod Kone.
> 
> 
> Bugs: MESOS-313
> https://issues.apache.org/jira/browse/MESOS-313
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Report executor exit to framework schedulers. This is a MVP to start the work 
> of notifying scheduler on scheduler refresh.
> 
> Next step would be sending this message reliabily, and/or splitting 
> Event::FAILURE for slave failure and executor termination.
> 
> 
> Diffs
> -
> 
>   CHANGELOG dbefa5df9e9183155bee532193148988dfc1fb84 
>   docs/app-framework-development-guide.md 
> 4a43a93d080bdac37b8aee91748fea7552a1cc67 
>   docs/upgrades.md 7c1f1814680078380ca33bbc27421675ffe61d60 
>   include/mesos/scheduler.hpp 049c041286f3167e79cc5ea8a9e0bf8d42569832 
>   src/java/src/org/apache/mesos/Scheduler.java 
> 4f048830a2c47f747033c60730cc770cb2578815 
>   src/python/interface/src/mesos/interface/__init__.py 
> 4be502fd83029ad5fc798696caf9e27fd95f7482 
>   src/sched/sched.cpp 44eb4f50e8ed84297268d94a3a0320c843ff6d8c 
>   src/tests/fault_tolerance_tests.cpp 
> ba657d0e1d8515cffd1b37925bf91a84b2feaef1 
>   src/tests/gc_tests.cpp f939d27c58177fba052fbcd9d6c9a572d052df52 
>   src/tests/master_slave_reconciliation_tests.cpp 
> 9afa826006fa7129da1a9c1ac8c389c0e051f717 
>   src/tests/master_tests.cpp 865fa4a71f4bae2a218cd2c4e10873222d1ea3c4 
>   src/tests/scheduler_event_call_tests.cpp 
> 03f0332ef75bbe7c4947bd6daf55d40384570f18 
>   src/tests/slave_tests.cpp 4975bea8a7a701e0414426760692720f73dea7f5 
> 
> Diff: https://reviews.apache.org/r/40429/diff/
> 
> 
> Testing
> ---
> 
> Modified test for SchedulerDriverEventTest.Failure, which verifies that 
> MockScheduler::executorLost is invoked.
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>

Re: Review Request 39520: Quota: Added authentication, authorization tests.

[Jan Schlicht]

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

(Updated Dec. 18, 2015, 1:18 p.m.)


Review request for mesos, Alexander Rukletsov and Till Toenshoff.


Changes
---

Addressed issues.


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


Repository: mesos


Description
---

Quota: Added authentication, authorization tests.


Diffs (updated)
-

  src/tests/authorization_tests.cpp dcf348acc75b143e3a854bde17e6042dc90599d5 
  src/tests/master_quota_tests.cpp 0473869783a714766ed26fff61d7f8c56342df74 

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


Testing
---

make check


Thanks,

Jan Schlicht

Re: Review Request 39520: Quota: Added authentication, authorization tests.

[Jan Schlicht]

> On Dec. 18, 2015, 12:32 p.m., Alexander Rukletsov wrote:
> > I would like to see one more test, where both authn and authz are disabled 
> > and quota set request succeeds.

While having this test makes sense, it would test neither authentication nor 
authorization, but the actual quota functionality. Therefore it shouldn't be 
part of these tests.


- Jan


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


On Dec. 18, 2015, 1:18 p.m., Jan Schlicht wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39520/
> ---
> 
> (Updated Dec. 18, 2015, 1:18 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Till Toenshoff.
> 
> 
> Bugs: MESOS-4082
> https://issues.apache.org/jira/browse/MESOS-4082
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Quota: Added authentication, authorization tests.
> 
> 
> Diffs
> -
> 
>   src/tests/authorization_tests.cpp dcf348acc75b143e3a854bde17e6042dc90599d5 
>   src/tests/master_quota_tests.cpp 0473869783a714766ed26fff61d7f8c56342df74 
> 
> Diff: https://reviews.apache.org/r/39520/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jan Schlicht
> 
>

Re: Review Request 39520: Quota: Added authentication, authorization tests.

[Alexander Rukletsov]

> On Dec. 14, 2015, 4:18 p.m., Alexander Rukletsov wrote:
> > I would suggest to add some more test cases:
> > - Request does not contain a principal (in absence of authz quota can be 
> > set);
> > - Request does not contain a principal, ACLs allow `ANY` to set quota for 
> > role "prod" (absence of principal maps to `ANY`).
> 
> Jan Schlicht wrote:
> The new test fixture `AuthorizedQuotaSetRequestWithoutPrincipal` now 
> tests for authorization with an empty principal if authentication is disabled.
> `AuthorizationTest.SetQuota` now includes a rule where `ANY` can set 
> quota for role "test" and tests that rule.

I still do not see a tests where **both** authn and authz are disabled. Let's 
add this test.


- Alexander


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


On Dec. 18, 2015, 9:12 a.m., Jan Schlicht wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39520/
> ---
> 
> (Updated Dec. 18, 2015, 9:12 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Till Toenshoff.
> 
> 
> Bugs: MESOS-4082
> https://issues.apache.org/jira/browse/MESOS-4082
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Quota: Added authentication, authorization tests.
> 
> 
> Diffs
> -
> 
>   src/tests/authorization_tests.cpp dcf348acc75b143e3a854bde17e6042dc90599d5 
>   src/tests/master_quota_tests.cpp 0473869783a714766ed26fff61d7f8c56342df74 
> 
> Diff: https://reviews.apache.org/r/39520/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jan Schlicht
> 
>

Re: Review Request 39520: Quota: Added authentication, authorization tests.

[Alexander Rukletsov]

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


I would like to see one more test, where both authn and authz are disabled and 
quota set request succeeds.


src/tests/master_quota_tests.cpp (lines 994 - 996)


It's not the fixture which is set up to expect certain credentials, it's 
the master which is set up in the fixture in a way that only a certain 
credentials will be authenticated. Let's be explicit about it.

How about:
The master is configured so that only requests from `DEFAULT_CREDENTIAL` 
are authenticated.



src/tests/master_quota_tests.cpp (lines 1010 - 1011)


How about a one-liner:
// The absense of credentials leads to authentication failure as well.



src/tests/master_quota_tests.cpp (line 1031)


It's not the operator, but a rather specific principal. How about
// `DEFAULT_CREDENTIAL.principal()` can request quotas for `ROLE1` (mind 
backticks).



src/tests/master_quota_tests.cpp (line 1055)


quota set request;
backtick `ROLE1`
s/ that can be authorized././

Or you can kill the comment entirely.



src/tests/master_quota_tests.cpp (line 1091)


Ditto as in the previous test.



src/tests/master_quota_tests.cpp (lines 1097 - 1101)


Let's refactor for readability:
  // Disable authentication by not providing credentials.
  master::Flags masterFlags = CreateMasterFlags();
  masterFlags.acls = acls;
  masterFlags.credentials = None();



src/tests/master_quota_tests.cpp (line 1118)


See my comment in the previous test.



src/tests/master_quota_tests.cpp (line 1127)


Let's drag reader's attention that no credentials are provided.



src/tests/master_quota_tests.cpp (line 1136)


You can kill this balnk line



src/tests/master_quota_tests.cpp (line 1147)


backtick `ROLE1`, here and everywhere, please.



src/tests/master_quota_tests.cpp (line 1161)


Let's kill this comment.


- Alexander Rukletsov


On Dec. 18, 2015, 9:12 a.m., Jan Schlicht wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39520/
> ---
> 
> (Updated Dec. 18, 2015, 9:12 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Till Toenshoff.
> 
> 
> Bugs: MESOS-4082
> https://issues.apache.org/jira/browse/MESOS-4082
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Quota: Added authentication, authorization tests.
> 
> 
> Diffs
> -
> 
>   src/tests/authorization_tests.cpp dcf348acc75b143e3a854bde17e6042dc90599d5 
>   src/tests/master_quota_tests.cpp 0473869783a714766ed26fff61d7f8c56342df74 
> 
> Diff: https://reviews.apache.org/r/39520/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jan Schlicht
> 
>

Re: Review Request 39520: Quota: Added authentication, authorization tests.

[Jan Schlicht]

> On Dec. 18, 2015, 12:32 p.m., Alexander Rukletsov wrote:
> > src/tests/master_quota_tests.cpp, line 1070
> > 
> >
> > quota set request;
> > backtick `ROLE1`
> > s/ that can be authorized././
> > 
> > Or you can kill the comment entirely.

Deleted the comment.


- Jan


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


On Dec. 18, 2015, 1:18 p.m., Jan Schlicht wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39520/
> ---
> 
> (Updated Dec. 18, 2015, 1:18 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Till Toenshoff.
> 
> 
> Bugs: MESOS-4082
> https://issues.apache.org/jira/browse/MESOS-4082
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Quota: Added authentication, authorization tests.
> 
> 
> Diffs
> -
> 
>   src/tests/authorization_tests.cpp dcf348acc75b143e3a854bde17e6042dc90599d5 
>   src/tests/master_quota_tests.cpp 0473869783a714766ed26fff61d7f8c56342df74 
> 
> Diff: https://reviews.apache.org/r/39520/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jan Schlicht
> 
>

Re: Review Request 39520: Quota: Added authentication, authorization tests.

[Jan Schlicht]

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

(Updated Dec. 18, 2015, 1:27 p.m.)


Review request for mesos, Alexander Rukletsov and Till Toenshoff.


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


Repository: mesos


Description
---

Quota: Added authentication, authorization tests.


Diffs (updated)
-

  src/tests/authorization_tests.cpp dcf348acc75b143e3a854bde17e6042dc90599d5 
  src/tests/master_quota_tests.cpp 0473869783a714766ed26fff61d7f8c56342df74 

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


Testing
---

make check


Thanks,

Jan Schlicht

Re: Review Request 41510: Cleaned up STL I/O includes in external containerizer.

[Benjamin Bannier]

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

Ship it!



src/slave/containerizer/external_containerizer.cpp (line 22)


Only focussing on I/O related headers, `iomanip` is unused here as well and 
should be removed.


- Benjamin Bannier


On Dec. 17, 2015, 2:37 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41510/
> ---
> 
> (Updated Dec. 17, 2015, 2:37 p.m.)
> 
> 
> Review request for mesos, Michael Park and Till Toenshoff.
> 
> 
> Bugs: MESOS-4183
> https://issues.apache.org/jira/browse/MESOS-4183
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Remove unused I/O includes, clean up blank lines and include order.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/external_containerizer.hpp 
> a5b4079a8f89c0c144468b42787c6cec78692d63 
>   src/slave/containerizer/external_containerizer.cpp 
> 04cf31e005c2be8af0f80dbf856bb94b2ab5351a 
> 
> Diff: https://reviews.apache.org/r/41510/diff/
> 
> 
> Testing
> ---
> 
> make check on Mac OS 10.10.4.
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 41509: [libprocess] Cleaned up STL I/O includes.

[Benjamin Bannier]

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

Ship it!


- Benjamin Bannier


On Dec. 17, 2015, 2:37 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41509/
> ---
> 
> (Updated Dec. 17, 2015, 2:37 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Joris Van Remoortere, and Michael 
> Park.
> 
> 
> Bugs: MESOS-4183
> https://issues.apache.org/jira/browse/MESOS-4183
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Where possible move `operator<<` definitions and functions using streams to 
> ".cpp" files and include `` in ".hpp"s. Also remove unused I/O 
> includes and clean up `std::` prefixes.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/address.hpp 
> 79429e904546ebeb993663efb9bd9e212c1dbd63 
>   3rdparty/libprocess/include/process/future.hpp 
> 817fca2ba5352a2c1cea1cb6cf6ecc49821215e4 
>   3rdparty/libprocess/include/process/http.hpp 
> f0666f0fa48c4f3a98332d12066561a02a715236 
>   3rdparty/libprocess/include/process/pid.hpp 
> b22c160ad0051ea1dac733a39a9f833719dbcb58 
>   3rdparty/libprocess/src/http.cpp e937df6875c8024ea9c178833b9faceede990969 
>   3rdparty/libprocess/src/pid.cpp 1a9cbd1eec6aefbd9a40113ae0f4475a90011b85 
> 
> Diff: https://reviews.apache.org/r/41509/diff/
> 
> 
> Testing
> ---
> 
> make check on Mac OS 10.10.4.
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 41511: Cleaned up STL I/O includes in public header (including v1).

[Benjamin Bannier]

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

Ship it!



include/mesos/type_utils.hpp (line 26)


Looks like a forward decl is good enough here.



include/mesos/v1/mesos.hpp (line 26)


It looks like a forward decl is enough here.



src/common/type_utils.cpp (lines 493 - 494)


Not yours, but the general pattern for this here seems to be reuse the 
temp, `return stream << stringify(map)`.



src/v1/mesos.cpp (lines 488 - 489)


Not yours, but the general pattern for this here seems to be to reuse the 
temp, `return stream << stringify(map)`.



src/v1/resources.cpp (line 23)


Do you want to add an include/forward decl for RepeatedFieldPtr as well?


- Benjamin Bannier


On Dec. 17, 2015, 2:37 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41511/
> ---
> 
> (Updated Dec. 17, 2015, 2:37 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Joris Van Remoortere, and Michael 
> Park.
> 
> 
> Bugs: MESOS-4183
> https://issues.apache.org/jira/browse/MESOS-4183
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Where possible move `operator<<` definitions and functions using streams to 
> ".cpp" files and include `` in ".hpp"s. Also remove unused I/O 
> includes, add necessary includes, clean up `std::` prefixes and blank lines.
> 
> 
> Diffs
> -
> 
>   include/mesos/resources.hpp 9c8bfb0e7e1e40ca99b4cc75079e5b950630243d 
>   include/mesos/type_utils.hpp b370b5180e71ed246fa09bf0fe119dfa946d0b08 
>   include/mesos/v1/mesos.hpp 80e76040de77e65b20f10b0465124ab86c1feab6 
>   include/mesos/v1/resources.hpp db87c992e2ec466cb1d4d17775bf6423a2fb15aa 
>   include/mesos/v1/values.hpp a160bd4d49d53fd2f794f36fea69ea99a455af09 
>   include/mesos/values.hpp 58af972186d7156660eac742b41501ce420cefe9 
>   src/common/resources.cpp 5a7981744726a0544435cbbd5007487a43a01211 
>   src/common/type_utils.cpp c6c9ba7d1bfc6e6202b9e3ff94a74711e3602d6f 
>   src/v1/mesos.cpp 13a58baf086ef3aaf46abcf7f58b71b7e2639728 
>   src/v1/resources.cpp d300842576b2fb8fe75f5b18699f23c1fd324717 
> 
> Diff: https://reviews.apache.org/r/41511/diff/
> 
> 
> Testing
> ---
> 
> make check on Mac OS 10.10.4.
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 41512: Cleaned up STL I/O includes in public headers which are not part of v1.

[Benjamin Bannier]

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

Ship it!


- Benjamin Bannier


On Dec. 17, 2015, 2:37 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41512/
> ---
> 
> (Updated Dec. 17, 2015, 2:37 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Joris Van Remoortere, and Michael 
> Park.
> 
> 
> Bugs: MESOS-4183
> https://issues.apache.org/jira/browse/MESOS-4183
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Where possible move `operator<<` definitions and functions using streams to 
> ".cpp" files and include `` in ".hpp"s. Also remove unused I/O 
> includes, add necessary includes, clean up `std::` prefixes.
> 
> 
> Diffs
> -
> 
>   include/mesos/authorizer/authorizer.hpp 
> ec3d3e9c9bae461c8cd136f17cfda58e0cbd4e1b 
>   include/mesos/http.hpp 8529ed0bf5b0954a223af5f7025fcd0d54c7348d 
>   include/mesos/uri/uri.hpp aa3ab5d24bb8c501dd19e93d7563cb0afd889f23 
>   src/authorizer/authorizer.cpp 31712e511cdaac3f6d137f425d5cd430434544aa 
>   src/common/http.cpp 5198650ba6dc1ea3dab7912e5ef6e375bf9acc96 
> 
> Diff: https://reviews.apache.org/r/41512/diff/
> 
> 
> Testing
> ---
> 
> make check on Mac OS 10.10.4.
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 41513: Cleaned up STL I/O includes.

[Benjamin Bannier]

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

Ship it!


- Benjamin Bannier


On Dec. 17, 2015, 2:38 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41513/
> ---
> 
> (Updated Dec. 17, 2015, 2:38 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Joris Van Remoortere, and Michael 
> Park.
> 
> 
> Bugs: MESOS-4183
> https://issues.apache.org/jira/browse/MESOS-4183
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Move `operator<<` definitions to ".cpp" files and include `` in 
> ".hpp"s.
> 
> 
> Diffs
> -
> 
>   src/linux/perf.cpp 8ee9f381c3587bb2fa0f309a477035340bc44e15 
>   src/linux/routing/filter/ip.hpp 1a63dcca2badc041e73fe454a9994b82695117de 
>   src/linux/routing/filter/ip.cpp 158bab6cb95fe10b8dc41b22934a946b21ff88ed 
>   src/linux/routing/handle.hpp 5b03be5e70ad3910487a1867fa4fa0d8de25d11e 
>   src/linux/routing/handle.cpp e88c6eb7336ac31683a6ff394a139f6777576a3e 
>   src/messages/messages.hpp 350118399927a8d7185d87a6a8e3370b7158fe14 
>   src/messages/messages.cpp 30f55bb8ee5a57ff9517144ffc43d45df6b3de5b 
> 
> Diff: https://reviews.apache.org/r/41513/diff/
> 
> 
> Testing
> ---
> 
> make check on Mac OS 10.10.4.
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 40348: [4/4] Quota Authorization: Documented quota authorization.

[Jan Schlicht]

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

(Updated Dec. 18, 2015, 3:43 p.m.)


Review request for mesos, Alexander Rukletsov, Alexander Rojas, Bernd Mathiske, 
Joris Van Remoortere, and Till Toenshoff.


Changes
---

s/a/an/


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


Repository: mesos


Description
---

Quota: Documented quota authorization.


Diffs (updated)
-

  docs/authorization.md 1fc0e3f9a686480ffc4deb25e49f867318d1321a 
  src/master/flags.cpp f9223edb33483cb5619e7dd75221f735306462c6 

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


Testing
---

make check


Thanks,

Jan Schlicht

Re: Review Request 41554: Added documentation on using network proxy for mesos fetcher

[Shuai Lin]

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

(Updated Dec. 18, 2015, 2 p.m.)


Review request for mesos.


Repository: mesos


Description
---

just a test


Diffs (updated)
-

  docs/fetcher.md ec7372293491ed9ee4e20dda3c2def72a77a3f84 

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


Testing
---


Thanks,

Shuai Lin

Re: Review Request 41544: Quota: Verified quota requests succeed when authn and authz are disabled.

[Alexander Rukletsov]

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

(Updated Dec. 18, 2015, 4:14 p.m.)


Review request for mesos, Bernd Mathiske, Greg Mann, Jan Schlicht, and Till 
Toenshoff.


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


Repository: mesos


Description
---

See summary.


Diffs (updated)
-

  src/tests/master_quota_tests.cpp 0473869783a714766ed26fff61d7f8c56342df74 

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


Testing
---

make check on Mac OS 10.10.4


Thanks,

Alexander Rukletsov

Review Request 41554: Added documentation on using network proxy for mesos fetcher

[Shuai Lin]

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

Review request for mesos.


Repository: mesos


Description
---

just a test


Diffs
-

  docs/fetcher.md ec7372293491ed9ee4e20dda3c2def72a77a3f84 

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


Testing
---


Thanks,

Shuai Lin

Re: Review Request 40348: [4/4] Quota Authorization: Documented quota authorization.

[Jan Schlicht]

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

(Updated Dec. 18, 2015, 3:38 p.m.)


Review request for mesos, Alexander Rukletsov, Alexander Rojas, Bernd Mathiske, 
Joris Van Remoortere, and Till Toenshoff.


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


Repository: mesos


Description
---

Quota: Documented quota authorization.


Diffs (updated)
-

  docs/authorization.md 1fc0e3f9a686480ffc4deb25e49f867318d1321a 
  src/master/flags.cpp f9223edb33483cb5619e7dd75221f735306462c6 

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


Testing
---

make check


Thanks,

Jan Schlicht

Review Request 41544: Quota: Verified quota requests succeed when authn and authz are disabled.

[Alexander Rukletsov]

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

Review request for mesos, Bernd Mathiske, Greg Mann, Jan Schlicht, and Till 
Toenshoff.


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


Repository: mesos


Description
---

See summary.


Diffs
-

  src/tests/master_quota_tests.cpp 0473869783a714766ed26fff61d7f8c56342df74 

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


Testing
---

make check on Mac OS 10.10.4


Thanks,

Alexander Rukletsov

Re: Review Request 41438: Added documentation on using network proxy for mesos fetcher

[Shuai Lin]

> On Dec. 17, 2015, 11:12 a.m., Bernd Mathiske wrote:
> > docs/fetcher.md, line 244
> > 
> >
> > Did you not have to write "export" in front?
> > I know of a case where nothing worked unless you did.

It works with or without the `export` on my machine. I'll add it in the doc 
since there is no harm.


- Shuai


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


On Dec. 18, 2015, 1:43 p.m., Shuai Lin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41438/
> ---
> 
> (Updated Dec. 18, 2015, 1:43 p.m.)
> 
> 
> Review request for mesos and Bernd Mathiske.
> 
> 
> Bugs: MESOS-4130
> https://issues.apache.org/jira/browse/MESOS-4130
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added documentation on using network proxy for mesos fetcher
> 
> 
> Diffs
> -
> 
>   docs/fetcher.md ec73722 
> 
> Diff: https://reviews.apache.org/r/41438/diff/
> 
> 
> Testing
> ---
> 
> - Set `https_proxy=http://localhost:3128` (a local squid server) in 
> `/etc/default/mesos-slave`, restart mesos-slave, launch a new marathon task 
> with a `https://` uri, and through the following sysdig command I can see 
> there is data exchanging between mesos-fetcher process and port 3128.
> 
> ```
> sudo sysdig -A -c echo_fds proc.name=mesos-fetcher and fd.port=3128
> ```
> 
> - Stop the local squid server, try to restart the marathon task, the task 
> would fail repeatly, from slave logs there are error messages that fetcher 
> failed to fetch the uri.
> ```
> Dec 16 15:16:35 lin-E400 mesos-slave[24247]: E1216 15:16:35.678032 24283 
> slave.cpp:3342] Container '45c14132-c56a-4cff-a6b5-f57ba2670643' for executor 
> 'testapp_web.f0ef72d2-a3c4-11e5-af60-56847afe9799' of framework 
> 'b52179fd-8968-4
> bf8-baf0-dddc8a38c903-' failed to start: Failed to fetch all URIs for 
> container '45c14132-c56a-4cff-a6b5-f57ba2670643' with exit status: 2
> ```
> 
> - Restart the squid server, the task would start without a problem.
> 
> 
> Thanks,
> 
> Shuai Lin
> 
>

Re: Review Request 41491: Unified Container: Implemented passing entrypoint in runtime config.

[Mesos ReviewBot]

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


Bad patch!

Reviews applied: [41011, 41125, 41194, 41406, 41407]

Failed command: ./support/apply-review.sh -n -r 41407

Error:
 2015-12-18 13:43:23 URL:https://reviews.apache.org/r/41407/diff/raw/ 
[4297/4297] -> "41407.patch" [1]
error: patch failed: 
src/slave/containerizer/mesos/provisioner/docker/puller.cpp:142
error: src/slave/containerizer/mesos/provisioner/docker/puller.cpp: patch does 
not apply

- Mesos ReviewBot


On Dec. 18, 2015, 9:21 a.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41491/
> ---
> 
> (Updated Dec. 18, 2015, 9:21 a.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Jie Yu, and Timothy Chen.
> 
> 
> Bugs: MESOS-2980
> https://issues.apache.org/jira/browse/MESOS-2980
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Unified Container: Implemented passing entrypoint in runtime config.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/provisioner/docker/store.cpp 
> 1ad7b67a94b1d9367afcb7c30a6d01fdf6b8ab6c 
>   src/slave/containerizer/mesos/provisioner/store.hpp 
> 8d1493856420dee3210af79b628c8c770c5c8550 
>   src/tests/containerizer/provisioner_docker_tests.cpp 
> 3f1717b770e139c3759aab0aeda9dbcf5029b0c2 
> 
> Diff: https://reviews.apache.org/r/41491/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 41438: Added documentation on using network proxy for mesos fetcher

[Shuai Lin]

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

(Updated Dec. 18, 2015, 1:43 p.m.)


Review request for mesos and Bernd Mathiske.


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


Repository: mesos


Description
---

Added documentation on using network proxy for mesos fetcher


Diffs (updated)
-

  docs/fetcher.md ec73722 

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


Testing
---

- Set `https_proxy=http://localhost:3128` (a local squid server) in 
`/etc/default/mesos-slave`, restart mesos-slave, launch a new marathon task 
with a `https://` uri, and through the following sysdig command I can see there 
is data exchanging between mesos-fetcher process and port 3128.

```
sudo sysdig -A -c echo_fds proc.name=mesos-fetcher and fd.port=3128
```

- Stop the local squid server, try to restart the marathon task, the task would 
fail repeatly, from slave logs there are error messages that fetcher failed to 
fetch the uri.
```
Dec 16 15:16:35 lin-E400 mesos-slave[24247]: E1216 15:16:35.678032 24283 
slave.cpp:3342] Container '45c14132-c56a-4cff-a6b5-f57ba2670643' for executor 
'testapp_web.f0ef72d2-a3c4-11e5-af60-56847afe9799' of framework 'b52179fd-8968-4
bf8-baf0-dddc8a38c903-' failed to start: Failed to fetch all URIs for 
container '45c14132-c56a-4cff-a6b5-f57ba2670643' with exit status: 2
```

- Restart the squid server, the task would start without a problem.


Thanks,

Shuai Lin

Re: Review Request 40347: [3/4] Quota Authorization: Implemented quota request authorization.

[Till Toenshoff]

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

Ship it!


Ship It!

- Till Toenshoff


On Dec. 18, 2015, 11:43 a.m., Jan Schlicht wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40347/
> ---
> 
> (Updated Dec. 18, 2015, 11:43 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Alexander Rojas, Bernd 
> Mathiske, Joris Van Remoortere, and Till Toenshoff.
> 
> 
> Bugs: MESOS-3862
> https://issues.apache.org/jira/browse/MESOS-3862
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Quota: Implemented quota request authorization.
> 
> 
> Diffs
> -
> 
>   src/master/master.hpp 7cb0e1692644e51271588abffa832e08c536b838 
>   src/master/quota_handler.cpp 11167879b2480d9c8dd6398ca39c479089ec2272 
> 
> Diff: https://reviews.apache.org/r/40347/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jan Schlicht
> 
>

Re: Review Request 40271: [7/7] Added documentation for RESERVE, UNRESERVE, CREATE, and DESTROY authorization.

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [39985, 39986, 39987, 39988, 39989, 40118, 40167, 40168, 
40169, 40255, 40256, 40271]

Passed command: export OS=ubuntu:14.04;export CONFIGURATION="--verbose";export 
COMPILER=gcc; ./support/docker_build.sh

- Mesos ReviewBot


On Dec. 18, 2015, 9:54 a.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40271/
> ---
> 
> (Updated Dec. 18, 2015, 9:54 a.m.)
> 
> 
> Review request for mesos, Jie Yu, Michael Park, and Neil Conway.
> 
> 
> Bugs: MESOS-3062 and MESOS-3065
> https://issues.apache.org/jira/browse/MESOS-3062
> https://issues.apache.org/jira/browse/MESOS-3065
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added documentation for RESERVE, UNRESERVE, CREATE, and DESTROY authorization.
> 
> 
> Diffs
> -
> 
>   docs/authorization.md 1fc0e3f9a686480ffc4deb25e49f867318d1321a 
>   docs/persistent-volume.md cf7a6bbb6044e460293dd66066df87aded0b4fb8 
>   docs/reservation.md de447664f10b1d73211805b725f2284b07c609f6 
> 
> Diff: https://reviews.apache.org/r/40271/diff/
> 
> 
> Testing
> ---
> 
> This is the last in a chain of 7 patches.
> 
> This documentation was tested by viewing with the Mesos Website Container 
> (https://github.com/mesosphere/mesos-website-container).
> 
> `make check` was run after all patches in the chain were applied.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 41544: Quota: Verified quota requests succeed when authn and authz are disabled.

[Alexander Rukletsov]

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

(Updated Dec. 18, 2015, 4:23 p.m.)


Review request for mesos, Bernd Mathiske, Greg Mann, Jan Schlicht, and Till 
Toenshoff.


Changes
---

Updated the testing done section.


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


Repository: mesos


Description
---

See summary.


Diffs
-

  src/tests/master_quota_tests.cpp 0473869783a714766ed26fff61d7f8c56342df74 

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


Testing (updated)
---

make check on Mac OS 10.10.4

Additionally, the whole chain started at "/r/40345" was tested with
```
GTEST_FILTER="*MasterQuotaTest*" ./bin/mesos-tests.sh --gtest_repeat=1000 
--gtest_break_on_failure --gtest_shuffle
```


Thanks,

Alexander Rukletsov

Re: Review Request 41544: Quota: Verified quota requests succeed when authn and authz are disabled.

[Alexander Rukletsov]

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

(Updated Dec. 18, 2015, 4:54 p.m.)


Review request for mesos, Bernd Mathiske, Greg Mann, Jan Schlicht, and Till 
Toenshoff.


Changes
---

Testing updated.


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


Repository: mesos


Description
---

See summary.


Diffs
-

  src/tests/master_quota_tests.cpp 0473869783a714766ed26fff61d7f8c56342df74 

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


Testing (updated)
---

`make check` on Mac OS 10.10.4

Additionally, the whole chain starting at "/r/40345" was tested:
- on Mac OS 10.10.4 with
  `GTEST_FILTER="*MasterQuotaTest*" ./bin/mesos-tests.sh --gtest_repeat=1000 
--gtest_break_on_failure --gtest_shuffle`
- on CentOS 7.1.1503 with `make check`


Thanks,

Alexander Rukletsov

Re: Review Request 41487: Provisioner: Changed docker v2 manifest naming.

[Jie Yu]

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

Ship it!



src/slave/containerizer/mesos/provisioner/docker/registry_client.cpp (line 73)


No need to do this in this patch. Let rename this method to be 
'getImageManifest'



src/slave/containerizer/mesos/provisioner/docker/spec.hpp (line 35)


s/validateManifest/validate/



src/slave/containerizer/mesos/provisioner/docker/spec.cpp (line 33)


Ditto.


- Jie Yu


On Dec. 18, 2015, 9:19 a.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41487/
> ---
> 
> (Updated Dec. 18, 2015, 9:19 a.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Jie Yu, and Timothy Chen.
> 
> 
> Bugs: MESOS-4186
> https://issues.apache.org/jira/browse/MESOS-4186
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Provisioner: Changed docker v2 manifest naming.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 9762f8567e32d70b8df2d694a1fef5c692fc730e 
>   src/slave/containerizer/mesos/provisioner/docker/message.hpp 
> 162e4c689bba832e523ff6b7d4e1e3c8e6713803 
>   src/slave/containerizer/mesos/provisioner/docker/message.proto 
> 5c032701671b275d86c6d9276791a46df814396c 
>   src/slave/containerizer/mesos/provisioner/docker/registry_client.hpp 
> 9b02b6ff6dc5c6e8aabdc4ac0aa4df337764ef30 
>   src/slave/containerizer/mesos/provisioner/docker/registry_client.cpp 
> e69bab43d5f5359cfd8eb4cb7c5ad4a1d22c4e05 
>   src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp 
> 89f61c20e52e5eff8d8e92748f03b3b461516cd2 
>   src/slave/containerizer/mesos/provisioner/docker/spec.hpp 
> e674b6f2e705f8b3e49770560eeab4c127473f94 
>   src/slave/containerizer/mesos/provisioner/docker/spec.cpp 
> 1f05c75dc9473bd5e4c0d3f74fa0ef996b96a84e 
>   src/slave/containerizer/mesos/provisioner/docker/v2.proto PRE-CREATION 
>   src/tests/containerizer/provisioner_docker_tests.cpp 
> 3f1717b770e139c3759aab0aeda9dbcf5029b0c2 
> 
> Diff: https://reviews.apache.org/r/41487/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 41420: Added ContainerInfo to internal Task protobuf.

[Jie Yu]

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

Ship it!



src/messages/messages.proto (line 38)


You may want to adjust the comments here.


- Jie Yu


On Dec. 17, 2015, 7:59 p.m., Artem Harutyunyan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41420/
> ---
> 
> (Updated Dec. 17, 2015, 7:59 p.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan and Jie Yu.
> 
> 
> Bugs: MESOS-4064
> https://issues.apache.org/jira/browse/MESOS-4064
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added ContainerInfo to internal Task protobuf.
> 
> 
> Diffs
> -
> 
>   src/messages/messages.proto b66bac14056407140bb3cf5aa3e67ac94df1a2f8 
> 
> Diff: https://reviews.apache.org/r/41420/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Artem Harutyunyan
> 
>

Re: Review Request 41421: Modified ptotobuf::createTask to use ContainerInfo from internal Task protobuf.

[Jie Yu]

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

Ship it!


Ship It!

- Jie Yu


On Dec. 17, 2015, 7:59 p.m., Artem Harutyunyan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41421/
> ---
> 
> (Updated Dec. 17, 2015, 7:59 p.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan and Jie Yu.
> 
> 
> Bugs: MESOS-4064
> https://issues.apache.org/jira/browse/MESOS-4064
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Modified ptotobuf::createTask to use ContainerInfo from internal Task 
> protobuf.
> 
> 
> Diffs
> -
> 
>   src/common/protobuf_utils.cpp 6e1eb0b8465809d1da5dac1cd29b692b9fa6ff66 
> 
> Diff: https://reviews.apache.org/r/41421/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Artem Harutyunyan
> 
>

Re: Review Request 41556: Updated comments in quota tests for clarity.

[Till Toenshoff]

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

Ship it!


Ship It!

- Till Toenshoff


On Dec. 18, 2015, 4:15 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41556/
> ---
> 
> (Updated Dec. 18, 2015, 4:15 p.m.)
> 
> 
> Review request for mesos and Till Toenshoff.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/tests/master_quota_tests.cpp 0473869783a714766ed26fff61d7f8c56342df74 
> 
> Diff: https://reviews.apache.org/r/41556/diff/
> 
> 
> Testing
> ---
> 
> Not a functional change.
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 41487: Provisioner: Changed docker v2 manifest naming.

[Gilbert Song]

> On Dec. 18, 2015, 9:16 a.m., Jie Yu wrote:
> > src/slave/containerizer/mesos/provisioner/docker/spec.hpp, line 35
> > 
> >
> > s/validateManifest/validate/

This is fixed in https://reviews.apache.org/r/41489/


> On Dec. 18, 2015, 9:16 a.m., Jie Yu wrote:
> > src/slave/containerizer/mesos/provisioner/docker/spec.cpp, line 33
> > 
> >
> > Ditto.

Ditto.


- Gilbert


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


On Dec. 18, 2015, 1:19 a.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41487/
> ---
> 
> (Updated Dec. 18, 2015, 1:19 a.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Jie Yu, and Timothy Chen.
> 
> 
> Bugs: MESOS-4186
> https://issues.apache.org/jira/browse/MESOS-4186
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Provisioner: Changed docker v2 manifest naming.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 9762f8567e32d70b8df2d694a1fef5c692fc730e 
>   src/slave/containerizer/mesos/provisioner/docker/message.hpp 
> 162e4c689bba832e523ff6b7d4e1e3c8e6713803 
>   src/slave/containerizer/mesos/provisioner/docker/message.proto 
> 5c032701671b275d86c6d9276791a46df814396c 
>   src/slave/containerizer/mesos/provisioner/docker/registry_client.hpp 
> 9b02b6ff6dc5c6e8aabdc4ac0aa4df337764ef30 
>   src/slave/containerizer/mesos/provisioner/docker/registry_client.cpp 
> e69bab43d5f5359cfd8eb4cb7c5ad4a1d22c4e05 
>   src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp 
> 89f61c20e52e5eff8d8e92748f03b3b461516cd2 
>   src/slave/containerizer/mesos/provisioner/docker/spec.hpp 
> e674b6f2e705f8b3e49770560eeab4c127473f94 
>   src/slave/containerizer/mesos/provisioner/docker/spec.cpp 
> 1f05c75dc9473bd5e4c0d3f74fa0ef996b96a84e 
>   src/slave/containerizer/mesos/provisioner/docker/v2.proto PRE-CREATION 
>   src/tests/containerizer/provisioner_docker_tests.cpp 
> 3f1717b770e139c3759aab0aeda9dbcf5029b0c2 
> 
> Diff: https://reviews.apache.org/r/41487/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 41509: [libprocess] Cleaned up STL I/O includes.

[Joerg Schad]

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

Ship it!


Ship It!

- Joerg Schad


On Dec. 17, 2015, 2:37 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41509/
> ---
> 
> (Updated Dec. 17, 2015, 2:37 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Joris Van Remoortere, and Michael 
> Park.
> 
> 
> Bugs: MESOS-4183
> https://issues.apache.org/jira/browse/MESOS-4183
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Where possible move `operator<<` definitions and functions using streams to 
> ".cpp" files and include `` in ".hpp"s. Also remove unused I/O 
> includes and clean up `std::` prefixes.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/address.hpp 
> 79429e904546ebeb993663efb9bd9e212c1dbd63 
>   3rdparty/libprocess/include/process/future.hpp 
> 817fca2ba5352a2c1cea1cb6cf6ecc49821215e4 
>   3rdparty/libprocess/include/process/http.hpp 
> f0666f0fa48c4f3a98332d12066561a02a715236 
>   3rdparty/libprocess/include/process/pid.hpp 
> b22c160ad0051ea1dac733a39a9f833719dbcb58 
>   3rdparty/libprocess/src/http.cpp e937df6875c8024ea9c178833b9faceede990969 
>   3rdparty/libprocess/src/pid.cpp 1a9cbd1eec6aefbd9a40113ae0f4475a90011b85 
> 
> Diff: https://reviews.apache.org/r/41509/diff/
> 
> 
> Testing
> ---
> 
> make check on Mac OS 10.10.4.
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 40429: Report executor exit to framework schedulers.

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [40429]

Passed command: export OS=ubuntu:14.04;export CONFIGURATION="--verbose";export 
COMPILER=gcc; ./support/docker_build.sh

- Mesos ReviewBot


On Dec. 14, 2015, 8:09 p.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40429/
> ---
> 
> (Updated Dec. 14, 2015, 8:09 p.m.)
> 
> 
> Review request for mesos, Adam B and Vinod Kone.
> 
> 
> Bugs: MESOS-313
> https://issues.apache.org/jira/browse/MESOS-313
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Report executor exit to framework schedulers. This is a MVP to start the work 
> of notifying scheduler on scheduler refresh.
> 
> Next step would be sending this message reliabily, and/or splitting 
> Event::FAILURE for slave failure and executor termination.
> 
> 
> Diffs
> -
> 
>   CHANGELOG dbefa5df9e9183155bee532193148988dfc1fb84 
>   docs/app-framework-development-guide.md 
> 4a43a93d080bdac37b8aee91748fea7552a1cc67 
>   docs/upgrades.md 7c1f1814680078380ca33bbc27421675ffe61d60 
>   include/mesos/scheduler.hpp 049c041286f3167e79cc5ea8a9e0bf8d42569832 
>   src/java/src/org/apache/mesos/Scheduler.java 
> 4f048830a2c47f747033c60730cc770cb2578815 
>   src/python/interface/src/mesos/interface/__init__.py 
> 4be502fd83029ad5fc798696caf9e27fd95f7482 
>   src/sched/sched.cpp 44eb4f50e8ed84297268d94a3a0320c843ff6d8c 
>   src/tests/fault_tolerance_tests.cpp 
> ba657d0e1d8515cffd1b37925bf91a84b2feaef1 
>   src/tests/gc_tests.cpp f939d27c58177fba052fbcd9d6c9a572d052df52 
>   src/tests/master_slave_reconciliation_tests.cpp 
> 9afa826006fa7129da1a9c1ac8c389c0e051f717 
>   src/tests/master_tests.cpp 865fa4a71f4bae2a218cd2c4e10873222d1ea3c4 
>   src/tests/scheduler_event_call_tests.cpp 
> 03f0332ef75bbe7c4947bd6daf55d40384570f18 
>   src/tests/slave_tests.cpp 4975bea8a7a701e0414426760692720f73dea7f5 
> 
> Diff: https://reviews.apache.org/r/40429/diff/
> 
> 
> Testing
> ---
> 
> Modified test for SchedulerDriverEventTest.Failure, which verifies that 
> MockScheduler::executorLost is invoked.
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>

Re: Review Request 41489: Provisioner: Implemented docker v1 parse serialization method in spec.

[Jie Yu]

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

Ship it!


Ship It!

- Jie Yu


On Dec. 18, 2015, 9:20 a.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41489/
> ---
> 
> (Updated Dec. 18, 2015, 9:20 a.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Jie Yu, and Timothy Chen.
> 
> 
> Bugs: MESOS-4186
> https://issues.apache.org/jira/browse/MESOS-4186
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Provisioner: Implemented docker v1 parse serialization method in spec.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/provisioner/docker/registry_client.cpp 
> e69bab43d5f5359cfd8eb4cb7c5ad4a1d22c4e05 
>   src/slave/containerizer/mesos/provisioner/docker/spec.hpp 
> e674b6f2e705f8b3e49770560eeab4c127473f94 
>   src/slave/containerizer/mesos/provisioner/docker/spec.cpp 
> 1f05c75dc9473bd5e4c0d3f74fa0ef996b96a84e 
>   src/tests/containerizer/provisioner_docker_tests.cpp 
> 3f1717b770e139c3759aab0aeda9dbcf5029b0c2 
> 
> Diff: https://reviews.apache.org/r/41489/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 41487: Provisioner: Changed docker v2 manifest naming.

[Jie Yu]

> On Dec. 18, 2015, 5:16 p.m., Jie Yu wrote:
> > src/slave/containerizer/mesos/provisioner/docker/spec.hpp, line 35
> > 
> >
> > s/validateManifest/validate/
> 
> Gilbert Song wrote:
> This is fixed in https://reviews.apache.org/r/41489/

Cool, I saw you did that in the subsequent patch.


- Jie


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


On Dec. 18, 2015, 9:19 a.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41487/
> ---
> 
> (Updated Dec. 18, 2015, 9:19 a.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Jie Yu, and Timothy Chen.
> 
> 
> Bugs: MESOS-4186
> https://issues.apache.org/jira/browse/MESOS-4186
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Provisioner: Changed docker v2 manifest naming.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 9762f8567e32d70b8df2d694a1fef5c692fc730e 
>   src/slave/containerizer/mesos/provisioner/docker/message.hpp 
> 162e4c689bba832e523ff6b7d4e1e3c8e6713803 
>   src/slave/containerizer/mesos/provisioner/docker/message.proto 
> 5c032701671b275d86c6d9276791a46df814396c 
>   src/slave/containerizer/mesos/provisioner/docker/registry_client.hpp 
> 9b02b6ff6dc5c6e8aabdc4ac0aa4df337764ef30 
>   src/slave/containerizer/mesos/provisioner/docker/registry_client.cpp 
> e69bab43d5f5359cfd8eb4cb7c5ad4a1d22c4e05 
>   src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp 
> 89f61c20e52e5eff8d8e92748f03b3b461516cd2 
>   src/slave/containerizer/mesos/provisioner/docker/spec.hpp 
> e674b6f2e705f8b3e49770560eeab4c127473f94 
>   src/slave/containerizer/mesos/provisioner/docker/spec.cpp 
> 1f05c75dc9473bd5e4c0d3f74fa0ef996b96a84e 
>   src/slave/containerizer/mesos/provisioner/docker/v2.proto PRE-CREATION 
>   src/tests/containerizer/provisioner_docker_tests.cpp 
> 3f1717b770e139c3759aab0aeda9dbcf5029b0c2 
> 
> Diff: https://reviews.apache.org/r/41487/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Review Request 41556: Updated comments in quota tests for clarity.

[Alexander Rukletsov]

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

Review request for mesos and Till Toenshoff.


Repository: mesos


Description
---

See summary.


Diffs
-

  src/tests/master_quota_tests.cpp 0473869783a714766ed26fff61d7f8c56342df74 

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


Testing
---

Not a functional change.


Thanks,

Alexander Rukletsov

Re: Review Request 41544: Quota: Verified quota requests succeed when authn and authz are disabled.

[Alexander Rukletsov]

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

(Updated Dec. 18, 2015, 4:24 p.m.)


Review request for mesos, Bernd Mathiske, Greg Mann, Jan Schlicht, and Till 
Toenshoff.


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


Repository: mesos


Description
---

See summary.


Diffs
-

  src/tests/master_quota_tests.cpp 0473869783a714766ed26fff61d7f8c56342df74 

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


Testing (updated)
---

make check on Mac OS 10.10.4

Additionally, the whole chain starting at "/r/40345" was tested on Mac OS 
10.10.4 with
```
GTEST_FILTER="*MasterQuotaTest*" ./bin/mesos-tests.sh --gtest_repeat=1000 
--gtest_break_on_failure --gtest_shuffle
```


Thanks,

Alexander Rukletsov

Re: Review Request 41253: Changed ownership semantics of ssl connect socket.

[Jojy Varghese]

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

(Updated Dec. 18, 2015, 5:52 p.m.)


Review request for mesos, Joris Van Remoortere and Joseph Wu.


Changes
---

removed dependency on 41252


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


Repository: mesos


Description
---

libprocess Socket shares the ownership of the file descriptor with libevent. In
the destructor of the libprocess libevent_ssl socket, we call ssl shutdown which
is executed asynchronously. This causes the libprocess socket file descriptor to
be closed (and possibly reused) when the same file descriptor could be used by
libevent/ssl. Since we set the shutdown options as SSL_RECEIVED_SHUTDOWN, we
leave the any write operations to continue with possibly closed file descriptor.

This change solves the above issue by copying(dup) the original file descriptor
and hands over the copy to libevent ssl. The copied descriptor is then managed
by libprocess Socket.


Diffs
-

  3rdparty/libprocess/src/libevent_ssl_socket.hpp 
2669b1a1d8f275b89c75d5f12fc696be2b277220 
  3rdparty/libprocess/src/libevent_ssl_socket.cpp 
55b91dd47bb5bd5e97147d0af91c7899fd42702c 

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


Testing
---

make check.


Thanks,

Jojy Varghese

Re: Review Request 41438: Added documentation on using network proxy for mesos fetcher

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [41438]

Passed command: export OS=ubuntu:14.04;export CONFIGURATION="--verbose";export 
COMPILER=gcc; ./support/docker_build.sh

- Mesos ReviewBot


On Dec. 18, 2015, 1:45 p.m., Shuai Lin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41438/
> ---
> 
> (Updated Dec. 18, 2015, 1:45 p.m.)
> 
> 
> Review request for mesos and Bernd Mathiske.
> 
> 
> Bugs: MESOS-4130
> https://issues.apache.org/jira/browse/MESOS-4130
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added documentation on using network proxy for mesos fetcher
> 
> 
> Diffs
> -
> 
>   docs/fetcher.md ec73722 
> 
> Diff: https://reviews.apache.org/r/41438/diff/
> 
> 
> Testing
> ---
> 
> - Set `https_proxy=http://localhost:3128` (a local squid server) in 
> `/etc/default/mesos-slave`, restart mesos-slave, launch a new marathon task 
> with a `https://` uri, and through the following sysdig command I can see 
> there is data exchanging between mesos-fetcher process and port 3128.
> 
> ```
> sudo sysdig -A -c echo_fds proc.name=mesos-fetcher and fd.port=3128
> ```
> 
> - Stop the local squid server, try to restart the marathon task, the task 
> would fail repeatly, from slave logs there are error messages that fetcher 
> failed to fetch the uri.
> ```
> Dec 16 15:16:35 lin-E400 mesos-slave[24247]: E1216 15:16:35.678032 24283 
> slave.cpp:3342] Container '45c14132-c56a-4cff-a6b5-f57ba2670643' for executor 
> 'testapp_web.f0ef72d2-a3c4-11e5-af60-56847afe9799' of framework 
> 'b52179fd-8968-4
> bf8-baf0-dddc8a38c903-' failed to start: Failed to fetch all URIs for 
> container '45c14132-c56a-4cff-a6b5-f57ba2670643' with exit status: 2
> ```
> 
> - Restart the squid server, the task would start without a problem.
> 
> 
> Thanks,
> 
> Shuai Lin
> 
>

Re: Review Request 41075: Added support for implicit roles.

[Neil Conway]

> On Dec. 18, 2015, 6:04 a.m., Yongqiao Wang wrote:
> > src/master/master.cpp, line 597
> > 
> >
> > If the value of the specified weight is default value 1.0, do we still 
> > need to put it in weights hashmap and pass to allocator?

I don't think it is worth special-casing this.


- Neil


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


On Dec. 18, 2015, 1:54 a.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41075/
> ---
> 
> (Updated Dec. 18, 2015, 1:54 a.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rukletsov, Benjamin Hindman, Greg 
> Mann, and Yongqiao Wang.
> 
> 
> Bugs: MESOS-4085
> https://issues.apache.org/jira/browse/MESOS-4085
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Changed the behavior of the master when the `--roles` flag is NOT
> specified. Previously, this would allow only the `*` role to be used. Now,
> omitting `--roles` means that any role can be used. This is called "implicit
> roles". Configuring which principals can perform operations as which roles
> should be done using ACLs in the authorization system.
> 
> Note that this changes the behavior of the system when `--roles` is not
> specified. This is likely acceptable: if the operator didn't specify `--roles`
> in prior versions of Mesos, they were likely not using roles or authorization 
> at
> that time.
> 
> Another minor behavioral change is that the "/roles" endpoint will now only
> return results for currently "active" roles (those with one or more registered
> frameworks).
> 
> The `--roles` flag is now considered deprecated and will be removed in a 
> future
> version of Mesos.
> 
> 
> Diffs
> -
> 
>   include/mesos/master/allocator.hpp 619ef01c3a7d640560653cfc1838dd09046d1da0 
>   include/mesos/master/allocator.proto 
> 702f56f56c3b1331613cecf26522986f6b572f8c 
>   src/master/allocator/mesos/allocator.hpp 
> 97ee80726ad155917811265a983258b0165d3451 
>   src/master/allocator/mesos/hierarchical.hpp 
> 99c742906874c30c39c159e58a65277ade3c07fd 
>   src/master/allocator/mesos/hierarchical.cpp 
> 5da825a1d578a9ee40b4985378fddb3c5fb3b416 
>   src/master/http.cpp 8dfd67f7d51b8395953d6beb77b2d71bc538eacd 
>   src/master/master.hpp 7cb0e1692644e51271588abffa832e08c536b838 
>   src/master/master.cpp 470b542729b01f41fc6a2e601a7a6c3d0c5353d5 
>   src/master/quota_handler.cpp 11167879b2480d9c8dd6398ca39c479089ec2272 
>   src/tests/allocator.hpp c7670525765491fe931a4ee38446fa7e9d79af42 
>   src/tests/hierarchical_allocator_tests.cpp 
> e239b4746494fcc2b362a83afb634a2ce5e25f9b 
>   src/tests/master_quota_tests.cpp 0473869783a714766ed26fff61d7f8c56342df74 
> 
> Diff: https://reviews.apache.org/r/41075/diff/
> 
> 
> Testing
> ---
> 
> "make check" on OSX 10.10 and Ubuntu 15.10; `--gtest_repeat=1000` for the 
> more likely role-related tests.
> 
> TODOs:
> 
> * Add tests for allocation behavior for weights + implicit roles
> * More tests for quota + implicit roles?
> 
> Notes:
> 
> * There's two places where we use manual `new`/`delete` where a `unique_ptr` 
> would probably be nicer. I'm inclined to leave this as-is for now though 
> (making use of unique_ptr is a broader issue).
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 41420: Added ContainerInfo to internal Task protobuf.

[Artem Harutyunyan]

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

(Updated Dec. 18, 2015, 11:29 a.m.)


Review request for mesos, Artem Harutyunyan and Jie Yu.


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


Repository: mesos


Description
---

Added ContainerInfo to internal Task protobuf.


Diffs (updated)
-

  src/messages/messages.proto b66bac14056407140bb3cf5aa3e67ac94df1a2f8 

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


Testing
---


Thanks,

Artem Harutyunyan

Re: Review Request 41108: CMake: Add sasl and dl link flags, add curl link library and add protobuf library directory.

[Diana Arroyo]

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

(Updated Dec. 18, 2015, 8:25 p.m.)


Review request for mesos, Artem Harutyunyan, Alex Clemmer, Joris Van 
Remoortere, and Joseph Wu.


Changes
---

Cleaned up FindCurl based on my findings and comments of Alex's review 
regarding line number 58.


Summary (updated)
-

CMake: Add sasl and dl link flags, add curl link library and add protobuf 
library directory.


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


Repository: mesos


Description (updated)
---

See summary.


Diffs (updated)
-

  src/slave/cmake/SlaveConfigure.cmake fbdfdaa27fbd8c7429861eea5baf401a221f748b 

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


Testing (updated)
---

Tested on Ubuntu and OSX.
Tested if and else path of new logic added to FindCurl.cmake.


Thanks,

Diana Arroyo

Re: Review Request 41253: Changed ownership semantics of ssl connect socket.

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [41253]

Passed command: export OS=ubuntu:14.04;export CONFIGURATION="--verbose";export 
COMPILER=gcc; ./support/docker_build.sh

- Mesos ReviewBot


On Dec. 18, 2015, 5:52 p.m., Jojy Varghese wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41253/
> ---
> 
> (Updated Dec. 18, 2015, 5:52 p.m.)
> 
> 
> Review request for mesos, Joris Van Remoortere and Joseph Wu.
> 
> 
> Bugs: MESOS-3773
> https://issues.apache.org/jira/browse/MESOS-3773
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> libprocess Socket shares the ownership of the file descriptor with libevent. 
> In
> the destructor of the libprocess libevent_ssl socket, we call ssl shutdown 
> which
> is executed asynchronously. This causes the libprocess socket file descriptor 
> to
> be closed (and possibly reused) when the same file descriptor could be used by
> libevent/ssl. Since we set the shutdown options as SSL_RECEIVED_SHUTDOWN, we
> leave the any write operations to continue with possibly closed file 
> descriptor.
> 
> This change solves the above issue by copying(dup) the original file 
> descriptor
> and hands over the copy to libevent ssl. The copied descriptor is then managed
> by libprocess Socket.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/libevent_ssl_socket.hpp 
> 2669b1a1d8f275b89c75d5f12fc696be2b277220 
>   3rdparty/libprocess/src/libevent_ssl_socket.cpp 
> 55b91dd47bb5bd5e97147d0af91c7899fd42702c 
> 
> Diff: https://reviews.apache.org/r/41253/diff/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Jojy Varghese
> 
>

Re: Review Request 41422: Removed manual construction of Task in master.

[Jie Yu]

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

Ship it!


Ship It!

- Jie Yu


On Dec. 17, 2015, 7:58 p.m., Artem Harutyunyan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41422/
> ---
> 
> (Updated Dec. 17, 2015, 7:58 p.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan and Jie Yu.
> 
> 
> Bugs: MESOS-4064
> https://issues.apache.org/jira/browse/MESOS-4064
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Removed manual construction of Task in master.
> 
> 
> Diffs
> -
> 
>   src/master/master.cpp 0d1482279c68f2a4a27dabaf28774769a5d515c4 
> 
> Diff: https://reviews.apache.org/r/41422/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Artem Harutyunyan
> 
>

Re: Review Request 41423: Exposed partial contents of ContainerInfo through the state endpoint.

[Jie Yu]

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

Ship it!



src/tests/containerizer/docker_containerizer_tests.cpp (line 474)


Looking at surrounding code, we're not using '{}' blocks. How about just 
reusing the variable name instead?



src/tests/containerizer/docker_containerizer_tests.cpp (lines 506 - 510)


Maybe remove this in a followup patch?


- Jie Yu


On Dec. 17, 2015, 7:58 p.m., Artem Harutyunyan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41423/
> ---
> 
> (Updated Dec. 17, 2015, 7:58 p.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan and Jie Yu.
> 
> 
> Bugs: MESOS-4064
> https://issues.apache.org/jira/browse/MESOS-4064
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Exposed partial contents of ContainerInfo through the state endpoint.
> 
> 
> Diffs
> -
> 
>   src/common/http.cpp 5198650ba6dc1ea3dab7912e5ef6e375bf9acc96 
>   src/tests/containerizer/docker_containerizer_tests.cpp 
> 3f199e622dd337cdbf32d4368f4ead424c39823c 
> 
> Diff: https://reviews.apache.org/r/41423/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Artem Harutyunyan
> 
>

Review Request 41560: Change Docker::run to take Subprocess::IO instead of Option

[Joseph Wu]

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

Review request for mesos, Benjamin Hindman, Artem Harutyunyan, Jie Yu, Joris 
Van Remoortere, and Timothy Chen.


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


Repository: mesos


Description
---

This changes the FDs used by the `mesos-docker-executor` to inherit rather than 
open anew.

In the `mesos-docker-executor`, we originally passed the log file path (i.e. 
`path::join(sandboxDirectory, "stdout")`) as an argument to `Docker::run`.  In 
the executor's context, the log file is already open as `STDOUT_FILENO`.

By inheriting the FD, the docker containerizer's logging-code will path mirrors 
that of the mesos containerizer.


Diffs
-

  src/docker/docker.hpp 33d6fb3e82ff7328ad093648a45546a18ec7b8cb 
  src/docker/docker.cpp 5dc4667d93b143b54841d85606affe3e4926757a 
  src/docker/executor.cpp 4042cec0acbe03d937ea3c53ffde745cbba552d2 
  src/slave/containerizer/docker.cpp 2d2dd4e0df36207c5f3cbb4fe2c50df51c0f3e9e 
  src/tests/mesos.hpp 1c6acabab9d189a6d3cc8d63359053fd230377b8 

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


Testing
---

make check (Centos7)
sudo bin/mesos-tests.sh --gtest_filter="*ROOT_DOCKER_Logs*"


Thanks,

Joseph Wu

Re: Review Request 41490: Provisioner: Added test case for docker v1 manifest serialization.

[Gilbert Song]

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

(Updated Dec. 18, 2015, 12:11 p.m.)


Review request for mesos, Artem Harutyunyan, Jie Yu, and Timothy Chen.


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


Repository: mesos


Description
---

Provisioner: Added test case for docker v1 manifest serialization.


Diffs (updated)
-

  src/tests/containerizer/provisioner_docker_tests.cpp 
3f1717b770e139c3759aab0aeda9dbcf5029b0c2 

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


Testing
---

make check


Thanks,

Gilbert Song

Re: Review Request 41487: Provisioner: Changed docker v2 manifest naming.

[Gilbert Song]

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

(Updated Dec. 18, 2015, 12:10 p.m.)


Review request for mesos, Artem Harutyunyan, Jie Yu, and Timothy Chen.


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


Repository: mesos


Description
---

Provisioner: Changed docker v2 manifest naming.


Diffs (updated)
-

  src/Makefile.am 9762f8567e32d70b8df2d694a1fef5c692fc730e 
  src/slave/containerizer/mesos/provisioner/docker/message.hpp 
162e4c689bba832e523ff6b7d4e1e3c8e6713803 
  src/slave/containerizer/mesos/provisioner/docker/message.proto 
5c032701671b275d86c6d9276791a46df814396c 
  src/slave/containerizer/mesos/provisioner/docker/registry_client.hpp 
9b02b6ff6dc5c6e8aabdc4ac0aa4df337764ef30 
  src/slave/containerizer/mesos/provisioner/docker/registry_client.cpp 
e69bab43d5f5359cfd8eb4cb7c5ad4a1d22c4e05 
  src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp 
89f61c20e52e5eff8d8e92748f03b3b461516cd2 
  src/slave/containerizer/mesos/provisioner/docker/spec.hpp 
e674b6f2e705f8b3e49770560eeab4c127473f94 
  src/slave/containerizer/mesos/provisioner/docker/spec.cpp 
1f05c75dc9473bd5e4c0d3f74fa0ef996b96a84e 
  src/slave/containerizer/mesos/provisioner/docker/v2.proto PRE-CREATION 
  src/tests/containerizer/provisioner_docker_tests.cpp 
3f1717b770e139c3759aab0aeda9dbcf5029b0c2 

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


Testing
---

make check


Thanks,

Gilbert Song

Re: Review Request 41488: Provisioner: Added docker v1 manifest protobuf message.

[Gilbert Song]

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

(Updated Dec. 18, 2015, 10:57 a.m.)


Review request for mesos, Artem Harutyunyan, Jie Yu, and Timothy Chen.


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


Repository: mesos


Description
---

Provisioner: Added docker v1 manifest protobuf message.


Diffs (updated)
-

  src/Makefile.am 9762f8567e32d70b8df2d694a1fef5c692fc730e 
  src/slave/containerizer/mesos/provisioner/docker/message.hpp 
162e4c689bba832e523ff6b7d4e1e3c8e6713803 
  src/slave/containerizer/mesos/provisioner/docker/v1.proto PRE-CREATION 

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


Testing
---

make check


Thanks,

Gilbert Song

Re: Review Request 41489: Provisioner: Implemented docker v1 parse serialization method in spec.

[Gilbert Song]

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

(Updated Dec. 18, 2015, 10:58 a.m.)


Review request for mesos, Artem Harutyunyan, Jie Yu, and Timothy Chen.


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


Repository: mesos


Description
---

Provisioner: Implemented docker v1 parse serialization method in spec.


Diffs (updated)
-

  src/slave/containerizer/mesos/provisioner/docker/registry_client.cpp 
e69bab43d5f5359cfd8eb4cb7c5ad4a1d22c4e05 
  src/slave/containerizer/mesos/provisioner/docker/spec.hpp 
e674b6f2e705f8b3e49770560eeab4c127473f94 
  src/slave/containerizer/mesos/provisioner/docker/spec.cpp 
1f05c75dc9473bd5e4c0d3f74fa0ef996b96a84e 
  src/tests/containerizer/provisioner_docker_tests.cpp 
3f1717b770e139c3759aab0aeda9dbcf5029b0c2 

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


Testing
---

make check


Thanks,

Gilbert Song

Re: Review Request 41490: Provisioner: Added test case for docker v1 manifest serialization.

[Gilbert Song]

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

(Updated Dec. 18, 2015, 10:58 a.m.)


Review request for mesos, Artem Harutyunyan, Jie Yu, and Timothy Chen.


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


Repository: mesos


Description
---

Provisioner: Added test case for docker v1 manifest serialization.


Diffs (updated)
-

  src/tests/containerizer/provisioner_docker_tests.cpp 
3f1717b770e139c3759aab0aeda9dbcf5029b0c2 

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


Testing
---

make check


Thanks,

Gilbert Song

Re: Review Request 41408: Updated documentation for implicit roles.

[Neil Conway]

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

(Updated Dec. 18, 2015, 8 p.m.)


Review request for mesos, Adam B and Yongqiao Wang.


Changes
---

Address review comments.


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


Repository: mesos


Description
---

Updated documentation for implicit roles.


Diffs (updated)
-

  CHANGELOG 388b8b91df6fce826413649b0a1480af3f78e918 
  docs/attributes-resources.md 3c5f2776d44a050ee3ac9967dd0ba0253b9c4558 
  docs/configuration.md c75f56ce849f6960b3b6246bfa6949156a82eabb 
  docs/roles.md 459d70717f4a0bdd921bb18b6ddc2505b6fc596e 
  docs/upgrades.md 7c1f1814680078380ca33bbc27421675ffe61d60 
  src/master/flags.cpp f9223edb33483cb5619e7dd75221f735306462c6 

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


Testing
---


Thanks,

Neil Conway

Re: Review Request 41225: Added test cases for implicit roles.

[Neil Conway]

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

(Updated Dec. 18, 2015, 8:01 p.m.)


Review request for mesos, Adam B, Alexander Rukletsov, and Yongqiao Wang.


Changes
---

Address review comments.


Repository: mesos


Description
---

Added test cases for implicit roles.


Diffs (updated)
-

  src/tests/role_tests.cpp PRE-CREATION 

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


Testing
---


Thanks,

Neil Conway

Re: Review Request 40256: [6/7] Fixed handling of multiple offer operations in PersistentVolumeTest.SendingCheckpointResourcesMessage.

[Jie Yu]

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

Ship it!



src/tests/persistent_volume_tests.cpp (line 99)


Looks like `MasterFlags` is not needed anymore with implicit roles. Can you 
do a sweep to remove it in a subsequent patch?


- Jie Yu


On Dec. 18, 2015, 9:38 a.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40256/
> ---
> 
> (Updated Dec. 18, 2015, 9:38 a.m.)
> 
> 
> Review request for mesos, Jie Yu, Michael Park, and Neil Conway.
> 
> 
> Bugs: MESOS-3065
> https://issues.apache.org/jira/browse/MESOS-3065
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed handling of multiple offer operations in 
> PersistentVolumeTest.SendingCheckpointResourcesMessage.
> 
> 
> Diffs
> -
> 
>   src/tests/persistent_volume_tests.cpp 
> 01b3c13751a5558d5f06edb8f650c8644dc54486 
> 
> Diff: https://reviews.apache.org/r/40256/diff/
> 
> 
> Testing
> ---
> 
> This is the sixth in a chain of 7 patches. This patch was required in order 
> to fix the `PersistentVolumeTest.SendingCheckpointResourcesMessage` test, 
> which was broken by the addition of authorization to the `CREATE` and 
> `DESTROY` offer operations. The test was previously both creating and 
> destroying a persistent volume in a single `acceptOffer` message. However, 
> our `validate` method for `DESTROY` offer operations correctly enforces that 
> in order to delete a volume, the volume should be present in the checkpointed 
> resources of the relevant Agent. This is likely not the case if the `CREATE` 
> and `DESTROY` operations are both in the same message.
> 
> `make check` was used to test after all patches were applied.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 41472: Disallowed dynamic reservations for roles not on the role whitelist.

[Michael Park]

> On Dec. 17, 2015, 1:17 a.m., Michael Park wrote:
> > src/master/http.cpp, lines 1000-1006
> > 
> >
> > Hm, should this be added to `validation::operation::validate` instead? 
> > That way if/when we have frameworks reserving for specific roles, it'll 
> > just work?
> 
> Neil Conway wrote:
> I thought about that, but it would require passing either the role 
> whitelist or a pointer to the master itself into the 
> `validation::operation::validate()` -- both of which seemed like they would 
> be regrettable changes to make. What do you think?
> 
> Adam B wrote:
> I'm fine with it as is, but I'll wait for @mcypark to make the call.

To me, it seems natural to have to pass whatever state is necessary to perform 
validation properly.
The `offer` validation for example takes `Master*` since it needs some 
information from the master.

```cpp
// Validates the given offers.
Option validate(
const google::protobuf::RepeatedPtrField& offerIds,
Master* master,
Framework* framework);
```

Ideally, we would pass along something more constrained than "all of master", 
similar to `CreateVolume` validation.

```cpp
// Validates the CREATE operation. We need slave's checkpointed
// resources so that we can validate persistence ID uniqueness.
Option validate(
const Offer::Operation::Create& create,
const Resources& checkpointedResources);
```

Here, `checkpointedResources` is a part of the master state. Passing along the 
role whitelist would be similar to this case.

What do you think?


- Michael


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


On Dec. 16, 2015, 10:48 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41472/
> ---
> 
> (Updated Dec. 16, 2015, 10:48 p.m.)
> 
> 
> Review request for mesos, Adam B and Michael Park.
> 
> 
> Bugs: MESOS-4143
> https://issues.apache.org/jira/browse/MESOS-4143
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Also added a test that dynamic reservations via the "/reserve" endpoint are
> allowed when using implicit roles.
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp 8dfd67f7d51b8395953d6beb77b2d71bc538eacd 
>   src/tests/reservation_endpoints_tests.cpp 
> b8edd6fafedd4c2221a8d19c1ebc71254071a8c7 
> 
> Diff: https://reviews.apache.org/r/41472/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 41488: Provisioner: Added docker v1 manifest protobuf message.

[Jie Yu]

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

Ship it!


Ship It!

- Jie Yu


On Dec. 18, 2015, 6:57 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41488/
> ---
> 
> (Updated Dec. 18, 2015, 6:57 p.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Jie Yu, and Timothy Chen.
> 
> 
> Bugs: MESOS-4186
> https://issues.apache.org/jira/browse/MESOS-4186
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Provisioner: Added docker v1 manifest protobuf message.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 9762f8567e32d70b8df2d694a1fef5c692fc730e 
>   src/slave/containerizer/mesos/provisioner/docker/message.hpp 
> 162e4c689bba832e523ff6b7d4e1e3c8e6713803 
>   src/slave/containerizer/mesos/provisioner/docker/v1.proto PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/41488/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 41556: Updated comments in quota tests for clarity.

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [39211, 39018, 39102, 36913, 38059, 39285, 38110, 39288, 
40345, 40346, 40347, 40348, 39520, 41544, 41556]

Passed command: export OS=ubuntu:14.04;export CONFIGURATION="--verbose";export 
COMPILER=gcc; ./support/docker_build.sh

- Mesos ReviewBot


On Dec. 18, 2015, 4:15 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41556/
> ---
> 
> (Updated Dec. 18, 2015, 4:15 p.m.)
> 
> 
> Review request for mesos and Till Toenshoff.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/tests/master_quota_tests.cpp 0473869783a714766ed26fff61d7f8c56342df74 
> 
> Diff: https://reviews.apache.org/r/41556/diff/
> 
> 
> Testing
> ---
> 
> Not a functional change.
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 40271: [7/7] Added documentation for RESERVE, UNRESERVE, CREATE, and DESTROY authorization.

[Jie Yu]

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

Ship it!



docs/authorization.md (line 11)


Looks like we are using `_xxx_` for those variables. Can you do a 
consistency fix?


- Jie Yu


On Dec. 18, 2015, 9:54 a.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40271/
> ---
> 
> (Updated Dec. 18, 2015, 9:54 a.m.)
> 
> 
> Review request for mesos, Jie Yu, Michael Park, and Neil Conway.
> 
> 
> Bugs: MESOS-3062 and MESOS-3065
> https://issues.apache.org/jira/browse/MESOS-3062
> https://issues.apache.org/jira/browse/MESOS-3065
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added documentation for RESERVE, UNRESERVE, CREATE, and DESTROY authorization.
> 
> 
> Diffs
> -
> 
>   docs/authorization.md 1fc0e3f9a686480ffc4deb25e49f867318d1321a 
>   docs/persistent-volume.md cf7a6bbb6044e460293dd66066df87aded0b4fb8 
>   docs/reservation.md de447664f10b1d73211805b725f2284b07c609f6 
> 
> Diff: https://reviews.apache.org/r/40271/diff/
> 
> 
> Testing
> ---
> 
> This is the last in a chain of 7 patches.
> 
> This documentation was tested by viewing with the Mesos Website Container 
> (https://github.com/mesosphere/mesos-website-container).
> 
> `make check` was run after all patches in the chain were applied.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 41490: Provisioner: Added test case for docker v1 manifest serialization.

[Gilbert Song]

> On Dec. 18, 2015, 11:04 a.m., Jie Yu wrote:
> > src/tests/containerizer/provisioner_docker_tests.cpp, line 386
> > 
> >
> > s/v1DockerImageManifest/manifest/

v2 fixed as well.


- Gilbert


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


On Dec. 18, 2015, 12:11 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41490/
> ---
> 
> (Updated Dec. 18, 2015, 12:11 p.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Jie Yu, and Timothy Chen.
> 
> 
> Bugs: MESOS-4186
> https://issues.apache.org/jira/browse/MESOS-4186
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Provisioner: Added test case for docker v1 manifest serialization.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/provisioner_docker_tests.cpp 
> 3f1717b770e139c3759aab0aeda9dbcf5029b0c2 
> 
> Diff: https://reviews.apache.org/r/41490/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 41108: Add curl, sasl and dl link flags and add protobuf library directory

[Diana Arroyo]

> On Dec. 9, 2015, 5:59 p.m., Alex Clemmer wrote:
> > src/slave/cmake/SlaveConfigure.cmake, line 62
> > 
> >
> > Same comment about the include directories, but with libraries. We 
> > probably want to include `CURL_LIBS` here.
> 
> Diana Arroyo wrote:
> So I wanted to verify the CURL_LIBS label that was being built in 
> FindCurl before adding it to AGENT_LIB_DIRS.  Here is the debug message 
> output of a build:
> 
>  AGENT_LIB_DIRS = 
> /root/mesos/build/3rdparty/libprocess/3rdparty/glog-0.3.3/src/glog-0.3.3-lib/lib/lib;/root/mesos/build/3rdparty/libprocess/3rdparty/http_parser-1c3624a/src/http_parser-1c3624a-build;/root/mesos/build/3rdparty/libprocess/3rdparty/libev-4.15/src/libev-4.15-build/.libs;/root/mesos/build/3rdparty/libprocess/3rdparty/protobuf-2.5.0/src/protobuf-2.5.0-lib/lib/lib;/root/mesos/build/3rdparty/libprocess/3rdparty/glog-0.3.3/src/glog-0.3.3-lib/lib/lib;/root/mesos/build/3rdparty/zookeeper-3.4.5/src/zookeeper-3.4.5-lib/lib/lib
> 
> CURL_LIBS = /usr/lib/x86_64-linux-gnu/libcurl.so
> 
> CURL_LIB = /usr/lib/x86_64-linux-gnu/libcurl.so
> 
> There is no lib directory field built in FindCurl.  I can add this but 
> note that this would differ from the FindApr and FindSvn scripts.  Please 
> advise.

Per our conversation earlier this week regarding the comment above I did some 
additional investigation.  The result is that adding the _LIB(S) (e.g. 
CURL_LIB or APR_LIBS) variable to link_directories() (-L option) is not the 
correct way and essentially be ignored.  Adding the _LIB(S) to 
target_link_libraries() (-l option) will do the trick.  In addition I found 
that the logic exported duplicate variables for the same value so I removed one 
variable (CURL_LIBS) and kept one variable (CURL_LIB).  I will open a JIRA to 
fix APR and SVN find setup to reflex my findings above.


- Diana


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


On Dec. 8, 2015, 11:09 p.m., Diana Arroyo wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41108/
> ---
> 
> (Updated Dec. 8, 2015, 11:09 p.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Alex Clemmer, Joris Van 
> Remoortere, and Joseph Wu.
> 
> 
> Bugs: MESOS-3843
> https://issues.apache.org/jira/browse/MESOS-3843
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add curl, sasl and dl link flags and add protobuf library directory
> 
> 
> Diffs
> -
> 
>   src/slave/cmake/SlaveConfigure.cmake 
> fbdfdaa27fbd8c7429861eea5baf401a221f748b 
> 
> Diff: https://reviews.apache.org/r/41108/diff/
> 
> 
> Testing
> ---
> 
> Tested on Ubuntu and OSX.
> 
> 
> Thanks,
> 
> Diana Arroyo
> 
>

Re: Review Request 41490: Provisioner: Added test case for docker v1 manifest serialization.

[Jie Yu]

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

Ship it!



src/tests/containerizer/provisioner_docker_tests.cpp (line 302)


2 lines apart.



src/tests/containerizer/provisioner_docker_tests.cpp (line 386)


s/v1DockerImageManifest/manifest/


- Jie Yu


On Dec. 18, 2015, 6:58 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41490/
> ---
> 
> (Updated Dec. 18, 2015, 6:58 p.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Jie Yu, and Timothy Chen.
> 
> 
> Bugs: MESOS-4186
> https://issues.apache.org/jira/browse/MESOS-4186
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Provisioner: Added test case for docker v1 manifest serialization.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/provisioner_docker_tests.cpp 
> 3f1717b770e139c3759aab0aeda9dbcf5029b0c2 
> 
> Diff: https://reviews.apache.org/r/41490/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 41408: Updated documentation for implicit roles.

[Neil Conway]

> On Dec. 18, 2015, 6:25 a.m., Adam B wrote:
> > src/master/flags.cpp, lines 187-189
> > 
> >
> > Don't forget to update configuration.md to match your change to the 
> > flags.

This was already done as far as I can tell.


- Neil


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


On Dec. 15, 2015, 8:59 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41408/
> ---
> 
> (Updated Dec. 15, 2015, 8:59 p.m.)
> 
> 
> Review request for mesos, Adam B and Yongqiao Wang.
> 
> 
> Bugs: MESOS-4085
> https://issues.apache.org/jira/browse/MESOS-4085
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated documentation for implicit roles.
> 
> 
> Diffs
> -
> 
>   docs/attributes-resources.md 3c5f2776d44a050ee3ac9967dd0ba0253b9c4558 
>   docs/configuration.md c75f56ce849f6960b3b6246bfa6949156a82eabb 
>   docs/roles.md 459d70717f4a0bdd921bb18b6ddc2505b6fc596e 
>   docs/upgrades.md 7c1f1814680078380ca33bbc27421675ffe61d60 
>   src/master/flags.cpp f9223edb33483cb5619e7dd75221f735306462c6 
> 
> Diff: https://reviews.apache.org/r/41408/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 40255: [5/7] Added framework authorization for persistent volumes.

[Jie Yu]

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

Ship it!



src/master/master.cpp (line 3425)


Thanks!



src/tests/persistent_volume_tests.cpp (line 751)


If you pause the clock, looks like this is not needed?



src/tests/persistent_volume_tests.cpp (line 906)


Ditto. Using default value sounds ok to me since clock is paused.



src/tests/persistent_volume_tests.cpp (lines 969 - 970)


I would move this up right below 'acceptOffers' to have better flow (first 
volume is created, and then we receive another offer). Here, and every other 
occurances.



src/tests/persistent_volume_tests.cpp (lines 980 - 981)


Can you move this up and right below `AWAIT_READY(checkpointResources1)`? 
Here and every other occurances.



src/tests/persistent_volume_tests.cpp (line 1007)


Ditto.



src/tests/persistent_volume_tests.cpp (lines 1016 - 1018)


Ditto.



src/tests/persistent_volume_tests.cpp (line 1069)


Why this given the clock is paused.



src/tests/persistent_volume_tests.cpp (line 1138)


+1 on using suppressOffer and reviveOffer


- Jie Yu


On Dec. 18, 2015, 10:10 a.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40255/
> ---
> 
> (Updated Dec. 18, 2015, 10:10 a.m.)
> 
> 
> Review request for mesos, Jie Yu, Michael Park, and Neil Conway.
> 
> 
> Bugs: MESOS-3065
> https://issues.apache.org/jira/browse/MESOS-3065
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added framework authorization for persistent volumes.
> 
> 
> Diffs
> -
> 
>   src/master/master.cpp 470b542729b01f41fc6a2e601a7a6c3d0c5353d5 
>   src/tests/persistent_volume_tests.cpp 
> 01b3c13751a5558d5f06edb8f650c8644dc54486 
> 
> Diff: https://reviews.apache.org/r/40255/diff/
> 
> 
> Testing
> ---
> 
> This is the fifth in a chain of 7 patches. New tests were added to 
> `persistent_volume_tests.cpp` in order to test a framework attempting both 
> successful and failed authorizations for `CREATE` and `DESTROY` offer 
> operations. `make check` was used to test after all patches were applied.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 41426: Changed MergeFrom() to CopyFrom() in protobuf::createTask().

[Artem Harutyunyan]

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

(Updated Dec. 18, 2015, 3:12 p.m.)


Review request for mesos and Artem Harutyunyan.


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


Repository: mesos


Description
---

Changed MergeFrom() to CopyFrom() in protobuf::createTask().


Diffs (updated)
-

  src/common/protobuf_utils.cpp 6e1eb0b8465809d1da5dac1cd29b692b9fa6ff66 

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


Testing
---


Thanks,

Artem Harutyunyan

Re: Review Request 41423: Exposed partial contents of ContainerInfo through the state endpoint.

[Artem Harutyunyan]

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

(Updated Dec. 18, 2015, 3:12 p.m.)


Review request for mesos, Artem Harutyunyan and Jie Yu.


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


Repository: mesos


Description
---

Exposed partial contents of ContainerInfo through the state endpoint.


Diffs (updated)
-

  src/common/http.cpp 5198650ba6dc1ea3dab7912e5ef6e375bf9acc96 
  src/tests/containerizer/docker_containerizer_tests.cpp 
3f199e622dd337cdbf32d4368f4ead424c39823c 

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


Testing
---

make check


Thanks,

Artem Harutyunyan

Review Request 41573: Added DEFAULT_ALLOCATION_INTERVAL to master constants.

[Greg Mann]

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

Review request for mesos, Jie Yu and Michael Park.


Repository: mesos


Description
---

Added DEFAULT_ALLOCATION_INTERVAL to master constants.


Diffs
-

  src/master/constants.hpp cc38dfcf57ecbc8555379084acbf2bfa4b3fc759 
  src/master/constants.cpp 98ea7c8f1e7c3057cfe941f82f1b71f7731a8f32 
  src/master/flags.cpp a8861f350c2ff20022a4c15a470f37a2bf9cadfa 

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


Testing
---

`make check` on OSX.


Thanks,

Greg Mann

Re: Review Request 40255: [5/7] Added framework authorization for persistent volumes.

[Greg Mann]

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

(Updated Dec. 18, 2015, 11:12 p.m.)


Review request for mesos, Jie Yu, Michael Park, and Neil Conway.


Changes
---

Addressed comments.


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


Repository: mesos


Description
---

Added framework authorization for persistent volumes.


Diffs (updated)
-

  src/master/master.cpp 470b542729b01f41fc6a2e601a7a6c3d0c5353d5 
  src/tests/persistent_volume_tests.cpp 
01b3c13751a5558d5f06edb8f650c8644dc54486 

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


Testing
---

This is the fifth in a chain of 7 patches. New tests were added to 
`persistent_volume_tests.cpp` in order to test a framework attempting both 
successful and failed authorizations for `CREATE` and `DESTROY` offer 
operations. `make check` was used to test after all patches were applied.


Thanks,

Greg Mann

Re: Review Request 40271: [7/7] Added documentation for RESERVE, UNRESERVE, CREATE, and DESTROY authorization.

[Greg Mann]

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

(Updated Dec. 18, 2015, 11:19 p.m.)


Review request for mesos, Jie Yu, Michael Park, and Neil Conway.


Changes
---

Addressed comment.


Bugs: MESOS-3062 and MESOS-3065
https://issues.apache.org/jira/browse/MESOS-3062
https://issues.apache.org/jira/browse/MESOS-3065


Repository: mesos


Description
---

Added documentation for RESERVE, UNRESERVE, CREATE, and DESTROY authorization.


Diffs (updated)
-

  docs/authorization.md 0b108bf96cf77b9e252306c0d694e9905d29939a 
  docs/persistent-volume.md cf7a6bbb6044e460293dd66066df87aded0b4fb8 
  docs/reservation.md de447664f10b1d73211805b725f2284b07c609f6 

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


Testing
---

This is the last in a chain of 7 patches.

This documentation was tested by viewing with the Mesos Website Container 
(https://github.com/mesosphere/mesos-website-container).

`make check` was run after all patches in the chain were applied.


Thanks,

Greg Mann

Re: Review Request 41026: libevent ssl: Added check for buffer length before swapping request.

[Jojy Varghese]

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

(Updated Dec. 18, 2015, 11:53 p.m.)


Review request for mesos and Joris Van Remoortere.


Changes
---

rebased with master.


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


Repository: mesos


Description
---

recv_callback could be called from libevents receive callback and Socket::recv
for the same buffer event and different requests. There is a check for buffer
length at Socket::recv but not at libevent's receive callback. This could lead
to the incoming request for Socket::recv being swapped out even though the
buffer length is zero. This change adds a check for buffer length before
swapping out the receive request object.


Diffs (updated)
-

  3rdparty/libprocess/src/libevent_ssl_socket.cpp 
55b91dd47bb5bd5e97147d0af91c7899fd42702c 

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


Testing
---

make check


Thanks,

Jojy Varghese

Re: Review Request 41423: Exposed partial contents of ContainerInfo through the state endpoint.

[Anand Mazumdar]

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

Ship it!


LGTM

- Anand Mazumdar


On Dec. 18, 2015, 11:58 p.m., Artem Harutyunyan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41423/
> ---
> 
> (Updated Dec. 18, 2015, 11:58 p.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan and Jie Yu.
> 
> 
> Bugs: MESOS-4064
> https://issues.apache.org/jira/browse/MESOS-4064
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Exposed partial contents of ContainerInfo through the state endpoint.
> 
> 
> Diffs
> -
> 
>   src/common/http.cpp 5198650ba6dc1ea3dab7912e5ef6e375bf9acc96 
>   src/tests/containerizer/docker_containerizer_tests.cpp 
> 3f199e622dd337cdbf32d4368f4ead424c39823c 
> 
> Diff: https://reviews.apache.org/r/41423/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Artem Harutyunyan
> 
>

Re: Review Request 41575: Removed outdated "Logging and Debugging" doc page.

[Neil Conway]

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

(Updated Dec. 19, 2015, 12:14 a.m.)


Review request for mesos and Joris Van Remoortere.


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


Repository: mesos


Description
---

Removed outdated "Logging and Debugging" doc page.


Diffs
-

  docs/home.md 51c19bb9d0d74698fcdda6197d32ed8f4a57d7c9 
  docs/logging-and-debugging.md 6797d156b244e0f08dbd754c5adc80a22e247693 

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


Testing
---


Thanks,

Neil Conway

Review Request 41575: Removed outdated "Logging and Debugging" doc page.

[Neil Conway]

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

Review request for mesos and Joris Van Remoortere.


Repository: mesos


Description
---

Removed outdated "Logging and Debugging" doc page.


Diffs
-

  docs/home.md 51c19bb9d0d74698fcdda6197d32ed8f4a57d7c9 
  docs/logging-and-debugging.md 6797d156b244e0f08dbd754c5adc80a22e247693 

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


Testing
---


Thanks,

Neil Conway

Re: Review Request 41474: Added documentation for `defer` in libprocess README.

[Greg Mann]

> On Dec. 18, 2015, 10:43 p.m., Neil Conway wrote:
> > 3rdparty/libprocess/README.md, line 176
> > 
> >
> > We should be consistent (between code samples) about whether we're 
> > assuming any `using` statements are implied, or restating them at the top 
> > of each code sample.

I added `using` statements to the first code sample to make things consistent.


- Greg


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


On Dec. 19, 2015, 1:26 a.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41474/
> ---
> 
> (Updated Dec. 19, 2015, 1:26 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Ben Mahler, Joris Van Remoortere, 
> and Neil Conway.
> 
> 
> Bugs: MESOS-3996
> https://issues.apache.org/jira/browse/MESOS-3996
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added documentation for `defer` in libprocess README.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/README.md c3f309a7dc1c94882c4cc97eeaf0736c2fca0ba5 
> 
> Diff: https://reviews.apache.org/r/41474/diff/
> 
> 
> Testing
> ---
> 
> Viewed the markdown in a branch on GitHub: 
> https://github.com/mesosphere/mesos/tree/defer_docs/3rdparty/libprocess
> 
> 
> Thanks,
> 
> Greg Mann
> 
>