Re: Review Request 47485: Added utility for parsing ld.so.cache on linux.

[Kevin Klues]

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

(Updated May 19, 2016, 5:38 a.m.)


Review request for mesos and Benjamin Mahler.


Changes
---

Updated to only compile ldcache_tests.cpp when compiling for Nvidia GPU support 
(because of an external dependence on libelf headers).  We also added a check 
in configure.ac to make sure we have these headers installed when building with 
Nvidia GPU support in general.


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


Repository: mesos


Description
---

Added utility for parsing ld.so.cache on linux.


Diffs (updated)
-

  configure.ac 63ea028fa89ba8e164e98226fc7ddcffd8b045c8 
  src/Makefile.am 571d2a5cca1f78ed55131fe830e8b17f2bcf0471 
  src/linux/ldcache.hpp PRE-CREATION 
  src/linux/ldcache.cpp PRE-CREATION 
  src/tests/ldcache_tests.cpp PRE-CREATION 

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


Testing
---

GTEST_FILTER="LdcacheTest.Parse" make check -j


Thanks,

Kevin Klues

Re: Review Request 47400: Documentation quota authorization changes in 0.29.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [47400]

Passed command: export OS='ubuntu:14.04' CONFIGURATION='--verbose' 
COMPILER='gcc' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker_build.sh

- Mesos ReviewBot


On May 19, 2016, 4:34 a.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47400/
> ---
> 
> (Updated May 19, 2016, 4:34 a.m.)
> 
> 
> Review request for mesos, Adam B and Alexander Rukletsov.
> 
> 
> Bugs: MESOS-5155 and MESOS-5336
> https://issues.apache.org/jira/browse/MESOS-5155
> https://issues.apache.org/jira/browse/MESOS-5336
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Documentation quota authorization changes in 0.29.
> 
> 
> Diffs
> -
> 
>   CHANGELOG 6bd59d191a8ca59436fa8a1953587119aeb3e256 
>   docs/authorization.md 9a359dc1c5576e9871c0747d02852bde35d67a3e 
>   docs/quota.md 797e134605381ae576d9aa93875e0314889ab047 
>   docs/upgrades.md 59274362fa6ec1e5857176fd5f8fd78c381f0d52 
> 
> Diff: https://reviews.apache.org/r/47400/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>

Re: Review Request 47400: Documentation quota authorization changes in 0.29.

[Zhitao Li]

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

(Updated May 19, 2016, 4:34 a.m.)


Review request for mesos, Adam B and Alexander Rukletsov.


Changes
---

Review comments from Guangya.


Bugs: MESOS-5155 and MESOS-5336
https://issues.apache.org/jira/browse/MESOS-5155
https://issues.apache.org/jira/browse/MESOS-5336


Repository: mesos


Description
---

Documentation quota authorization changes in 0.29.


Diffs (updated)
-

  CHANGELOG 6bd59d191a8ca59436fa8a1953587119aeb3e256 
  docs/authorization.md 9a359dc1c5576e9871c0747d02852bde35d67a3e 
  docs/quota.md 797e134605381ae576d9aa93875e0314889ab047 
  docs/upgrades.md 59274362fa6ec1e5857176fd5f8fd78c381f0d52 

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


Testing
---


Thanks,

Zhitao Li

Re: Review Request 47400: Documentation quota authorization changes in 0.29.

[Zhitao Li]

> On May 19, 2016, 3:25 a.m., Guangya Liu wrote:
> > CHANGELOG, line 59
> > 
> >
> > s/set_quotas/`set_quotas`
> > s/remove_quotas/`remove_quotas`
> > 
> > s/in/in favor
> > 
> > Do we need to mention `get_quotas` here?

I don't think we need to mention get_quotas.


> On May 19, 2016, 3:25 a.m., Guangya Liu wrote:
> > CHANGELOG, line 61
> > 
> >
> > I think ` This is applicable to both local authorizer as well as any 
> > custom authorizer module.` can be removed.
> > 
> > The CHANGELOG only need to clarify the update

Hmm, I'd like to explicitly call it out, unless you see harm here.


> On May 19, 2016, 3:25 a.m., Guangya Liu wrote:
> > docs/upgrades.md, line 175
> > 
> >
> > Why moving this part to quota section?

I'm keeping all bulletpoints with an anchor before ones without.


- Zhitao


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


On May 18, 2016, 4:53 p.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47400/
> ---
> 
> (Updated May 18, 2016, 4:53 p.m.)
> 
> 
> Review request for mesos, Adam B and Alexander Rukletsov.
> 
> 
> Bugs: MESOS-5155 and MESOS-5336
> https://issues.apache.org/jira/browse/MESOS-5155
> https://issues.apache.org/jira/browse/MESOS-5336
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Documentation quota authorization changes in 0.29.
> 
> 
> Diffs
> -
> 
>   CHANGELOG 6bd59d191a8ca59436fa8a1953587119aeb3e256 
>   docs/authorization.md 9a359dc1c5576e9871c0747d02852bde35d67a3e 
>   docs/quota.md 797e134605381ae576d9aa93875e0314889ab047 
>   docs/upgrades.md 59274362fa6ec1e5857176fd5f8fd78c381f0d52 
> 
> Diff: https://reviews.apache.org/r/47400/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>

Review Request 47577: Agent: Added minor changes to various .cpp files to support Windows.

[Alex Clemmer]

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

Review request for mesos, Daniel Pravat, Artem Harutyunyan, Joris Van 
Remoortere, and Michael Park.


Repository: mesos


Description
---

Agent: Added minor changes to various .cpp files to support Windows.


Diffs
-

  src/docker/docker.cpp 19cf424dfd5748ae66a7023840aa2b0652e8f2c0 
  src/docker/executor.cpp d60addcbe4a1869945ce42f4bb4b1e80e3f29f19 
  src/files/files.cpp 4e916101b378b0e9032a08a3f6c73e195b2a08a1 
  src/health-check/main.cpp 98ea5d3675f088e3a341037dcee92695e4857999 
  src/logging/logging.cpp 20d2f6341bd39fc5d056f1046d258d006fc602e4 
  src/uri/fetcher.hpp 8af2c9122e0b15fd54f7d3a84779540e7186f566 
  src/uri/fetcher.cpp aa9df5d0256a26b8684934c2bd37b82a069088f7 
  src/uri/fetchers/copy.cpp 2180adfba1f33852d11069eed9d9bca72e2e3b6f 
  src/uri/fetchers/curl.cpp c4420623d718d87776f2eb8e13faf02ef5edb335 
  src/zookeeper/group.cpp 01680899778e554af70b176db82498ca92b51b60 

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


Testing
---


Thanks,

Alex Clemmer

Re: Review Request 47484: Added -lelf to default dynamic libraries on Linux.

[Benjamin Mahler]

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


Ship it!





configure.ac (line 580)


```
# For stout/elf.hpp.
```


- Benjamin Mahler


On May 17, 2016, 7:17 p.m., Kevin Klues wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47484/
> ---
> 
> (Updated May 17, 2016, 7:17 p.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Bugs: MESOS-5400
> https://issues.apache.org/jira/browse/MESOS-5400
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added -lelf to default dynamic libraries on Linux.
> 
> 
> Diffs
> -
> 
>   configure.ac 63ea028fa89ba8e164e98226fc7ddcffd8b045c8 
> 
> Diff: https://reviews.apache.org/r/47484/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Kevin Klues
> 
>

Re: Review Request 47483: Added -lelf to default dynamic libraries for libprocess on Linux.

[Benjamin Mahler]

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


Ship it!





3rdparty/libprocess/configure.ac (line 342)


```
# For stout/elf.hpp.
```


- Benjamin Mahler


On May 17, 2016, 7:18 p.m., Kevin Klues wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47483/
> ---
> 
> (Updated May 17, 2016, 7:18 p.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Bugs: MESOS-5400
> https://issues.apache.org/jira/browse/MESOS-5400
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added -lelf to default dynamic libraries for libprocess on Linux.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/configure.ac c8fcd35241c10829e7b2fa582491898589f0576f 
> 
> Diff: https://reviews.apache.org/r/47483/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Kevin Klues
> 
>

Re: Review Request 47482: Added preliminary support for parsing ELF files in stout.

[Benjamin Mahler]

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


Fix it, then Ship it!





3rdparty/stout/configure.ac (line 154)


s/posix//



3rdparty/stout/include/stout/elf.hpp (lines 20 - 33)


Looks like we need to audit these?



3rdparty/stout/include/stout/elf.hpp (line 69)


Hm.. could we move this down towards where it is first used?



3rdparty/stout/include/stout/elf.hpp (line 71)


Can we document what this does?



3rdparty/stout/include/stout/elf.hpp (line 80)


We should avoid logging the caller-available information here as it leads 
to redundant logging.



3rdparty/stout/include/stout/elf.hpp (line 91)


Ditto on excluding the path here.



3rdparty/stout/include/stout/elf.hpp (line 94)


`s/ */* /` here and elsewhere



3rdparty/stout/include/stout/elf.hpp (lines 94 - 106)


Hm.. could we use some unabbreviated variable names here and elsewhere? 
E.g. 'section', 'section_header', 'section_type', etc. Since the type names are 
opaque that would help the reader.



3rdparty/stout/include/stout/elf.hpp (line 104)


Hm.. emplace here seems a bit odd since there's no need to avoid copying 
for pointer types. Let's avoid emplace for now overall since we're not doing 
any large copying here, we can optimize it later.



3rdparty/stout/include/stout/elf.hpp (line 134)


const for these member functions?



3rdparty/stout/include/stout/elf.hpp (line 137)


Why cast instead of using the Class enum value?



3rdparty/stout/include/stout/elf.hpp (line 138)


Let's do an audit to match the names in the log messages: 'gelf_getclass' 
here for example.



3rdparty/stout/include/stout/elf.hpp (line 156)


We've started to use snake_case in stout and libprocess (we used to, then 
started using camelCase, and now we've moved back), it's unfortunately 
inconsistent so I would be fine either way here. If you don't mind we can 
switch all the variables here to use snake_case.



3rdparty/stout/include/stout/elf.hpp (line 169)


Looks like we should make this an Option to catch the case where the loop 
completes without finding a pointer.


- Benjamin Mahler


On May 18, 2016, 8:20 p.m., Kevin Klues wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47482/
> ---
> 
> (Updated May 18, 2016, 8:20 p.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Bugs: MESOS-5400
> https://issues.apache.org/jira/browse/MESOS-5400
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Right now we are able to parse ELF formatted shared libraries and
> extract their canonical SONAME and external library dependencies. In
> the future, we should add support for fully parsing an ELf file for
> easy access to all of its contents.
> 
> The current implementation relies on libelf. We should probably remove
> this dependency in future versions (mostly since the headers for
> libelf are not installed on a standard Linux distribution by default).
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/configure.ac ada1e22c72875fe9d557f07e4846128be0bcef13 
>   3rdparty/stout/include/stout/elf.hpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/47482/diff/
> 
> 
> Testing
> ---
> 
> The test for this is actually in a follow-on patch for testing ld.so.cache 
> parsing. The test itself is run with:
> ```
> GTEST_FILTER="LdcacheTest.Parse" make check -j
> ```
> 
> 
> Thanks,
> 
> Kevin Klues
> 
>

Re: Review Request 45474: MESOS-1739: Allow slave reconfiguration on restart, Phase 1.

[Guangya Liu]

> On March 31, 2016, 2:03 a.m., Jie Yu wrote:
> > This is a high level question: I am now sure if adding attributes is safe 
> > or not. For instance, my framework has the following rule: only schedule 
> > tasks to agents that do not have attribute "not_safe". Now, say agent A is 
> > initially without that attribute. My framework lands several tasks on that 
> > agent. Later, when agent restarts, the operator adds the new attribute 
> > "not_safe". Suddently, i have tasks running on unsafe boxes. oops.
> 
> Deshi Xiao wrote:
> I think so, this is very common case. it need more detail discussion with 
> team.

It's difficult to handle such issue as the operation are issued by 
administrator, but the tasks are launched by framework developers. I can see 
for most of such cases, just left the old tasks running on the agent till 
finished but the new tasks will launch on other agents other than this agent.


- Guangya


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


On March 30, 2016, 8:13 a.m., Deshi Xiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45474/
> ---
> 
> (Updated March 30, 2016, 8:13 a.m.)
> 
> 
> Review request for mesos, Adam B, Greg Mann, haosdent huang, and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-1739
> https://issues.apache.org/jira/browse/MESOS-1739
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Phase 1
> Make SlaveInfo mutable throughout the stack, and allow for expansion of 
> resources and attributes only (Which allows testing to make sure it 
> propagates to the allocator, shows up in offers, etc). Ensure there is 
> unified checking for incompatibilities in both the slave and master (the 
> slave should validate the config, the master should validate that all 
> operations the slave takes are legal).
> 
> it derived from another PR(https://reviews.apache.org/r/25525/)
> 
> 
> Diffs
> -
> 
>   src/tests/slave_tests.cpp 1f1a31020096efa5db698e86ac74e61dfdb4b94a 
> 
> Diff: https://reviews.apache.org/r/45474/diff/
> 
> 
> Testing
> ---
> 
> make check on localhost
> 
> 
> Thanks,
> 
> Deshi Xiao
> 
>

Re: Review Request 47400: Documentation quota authorization changes in 0.29.

[Guangya Liu]

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




CHANGELOG (line 59)


s/set_quotas/`set_quotas`
s/remove_quotas/`remove_quotas`

s/in/in favor

Do we need to mention `get_quotas` here?



CHANGELOG (line 60)


s/update_quotas/`update_quotas`



CHANGELOG (line 61)


I think ` This is applicable to both local authorizer as well as any custom 
authorizer module.` can be removed.

The CHANGELOG only need to clarify the update



docs/authorization.md (lines 128 - 129)


Can you please link the `quota` keyword to quota document?



docs/authorization.md (line 524)


blank line above



docs/authorization.md (line 551)


blank line above



docs/upgrades.md (line 175)


Does this belong to your patch? Why copy here?



docs/upgrades.md (line 175)


Why moving this part to quota section?


- Guangya Liu


On May 18, 2016, 4:53 p.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47400/
> ---
> 
> (Updated May 18, 2016, 4:53 p.m.)
> 
> 
> Review request for mesos, Adam B and Alexander Rukletsov.
> 
> 
> Bugs: MESOS-5155 and MESOS-5336
> https://issues.apache.org/jira/browse/MESOS-5155
> https://issues.apache.org/jira/browse/MESOS-5336
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Documentation quota authorization changes in 0.29.
> 
> 
> Diffs
> -
> 
>   CHANGELOG 6bd59d191a8ca59436fa8a1953587119aeb3e256 
>   docs/authorization.md 9a359dc1c5576e9871c0747d02852bde35d67a3e 
>   docs/quota.md 797e134605381ae576d9aa93875e0314889ab047 
>   docs/upgrades.md 59274362fa6ec1e5857176fd5f8fd78c381f0d52 
> 
> Diff: https://reviews.apache.org/r/47400/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>

Re: Review Request 47576: Agent: Add Windows support to the containerizer.

[Mesos ReviewBot]

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



Bad patch!

Reviews applied: [47576, 47536, 47472, 47471, 47470, 47469, 47468, 47412, 
47411, 47410, 47409, 47404, 47403, 47391, 47390, 47389, 47388, 47387, 47386, 
47169, 47168, 41632, 47054, 47221, 47053, 47052]

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

Error:
2016-05-19 03:14:04 URL:https://reviews.apache.org/r/47471/diff/raw/ 
[5804/5804] -> "47471.patch" [1]
error: patch failed: src/exec/exec.cpp:595
error: src/exec/exec.cpp: patch does not apply
error: patch failed: src/executor/executor.cpp:154
error: src/executor/executor.cpp: patch does not apply
error: patch failed: src/launcher/fetcher.cpp:443
error: src/launcher/fetcher.cpp: patch does not apply

Full log: https://builds.apache.org/job/mesos-reviewbot/13160/console

- Mesos ReviewBot


On May 19, 2016, 2:47 a.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47576/
> ---
> 
> (Updated May 19, 2016, 2:47 a.m.)
> 
> 
> Review request for mesos, Daniel Pravat, Artem Harutyunyan, Joris Van 
> Remoortere, and Michael Park.
> 
> 
> Bugs: MESOS-3617, MESOS-3618, MESOS-3619, MESOS-3622, MESOS-3623, MESOS-3624, 
> MESOS-3681, MESOS-3682, and MESOS-3684
> https://issues.apache.org/jira/browse/MESOS-3617
> https://issues.apache.org/jira/browse/MESOS-3618
> https://issues.apache.org/jira/browse/MESOS-3619
> https://issues.apache.org/jira/browse/MESOS-3622
> https://issues.apache.org/jira/browse/MESOS-3623
> https://issues.apache.org/jira/browse/MESOS-3624
> https://issues.apache.org/jira/browse/MESOS-3681
> https://issues.apache.org/jira/browse/MESOS-3682
> https://issues.apache.org/jira/browse/MESOS-3684
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Agent: Add Windows support to the containerizer.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/docker.cpp 7af14f4472283ceefd73c06dd8df60af4cf6f7e8 
>   src/slave/containerizer/external_containerizer.cpp 
> cf4384cce44172a028c890f52f71ceb8ae109383 
>   src/slave/containerizer/mesos/containerizer.cpp 
> 75e5a32a3e70ec60a6800e21a621673184ea0956 
>   src/slave/containerizer/mesos/launcher.hpp 
> 5977c30c0aacc569019f7b34bb0c6577823ec887 
>   src/slave/containerizer/mesos/launcher.cpp 
> a5c8c31b72773d0bd10b9d02675a01f1d641d41c 
> 
> Diff: https://reviews.apache.org/r/47576/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>

Re: Review Request 47536: Agent: Added Windows isolators.

[Alex Clemmer]

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

(Updated May 19, 2016, 2:50 a.m.)


Review request for mesos, Daniel Pravat, Artem Harutyunyan, Joris Van 
Remoortere, and Michael Park.


Bugs: MESOS-3619, MESOS-3620 and MESOS-3683
https://issues.apache.org/jira/browse/MESOS-3619
https://issues.apache.org/jira/browse/MESOS-3620
https://issues.apache.org/jira/browse/MESOS-3683


Repository: mesos


Description
---

Agent: Added Windows isolators.


Diffs
-

  src/CMakeLists.txt e0c538d9e6542fbe279bfbf6f20172e4c611c859 
  src/Makefile.am 447bc2ab511ad173d3d911be10992be0974f4584 
  src/slave/containerizer/mesos/isolators/filesystem/posix.hpp 
c6cea98e16f2bdea2da0220c235468080bbcd17b 
  src/slave/containerizer/mesos/isolators/filesystem/posix.cpp 
01c0ad6dbb6d509e62e769365586b3d23dcb240d 
  src/slave/containerizer/mesos/isolators/filesystem/windows.hpp PRE-CREATION 
  src/slave/containerizer/mesos/isolators/filesystem/windows.cpp PRE-CREATION 
  src/slave/containerizer/mesos/isolators/posix.hpp 
227505a70a440f15e68ac001878bcf25610db45f 
  src/slave/containerizer/mesos/isolators/windows.hpp PRE-CREATION 
  src/usage/main.cpp 731acb69900b6fc2bb7bd19cccd78aafb0cc 
  src/usage/usage.hpp 2e1996a78c617c2559ef882ebede5c8aab6f899f 
  src/usage/usage.cpp 3b19682e67372b81484eacddbab78c2e5eda3c5b 

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


Testing
---


Thanks,

Alex Clemmer

Review Request 47576: Agent: Add Windows support to the containerizer.

[Alex Clemmer]

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

Review request for mesos, Daniel Pravat, Artem Harutyunyan, Joris Van 
Remoortere, and Michael Park.


Bugs: MESOS-3617, MESOS-3618, MESOS-3619, MESOS-3622, MESOS-3623, MESOS-3624, 
MESOS-3681, MESOS-3682, and MESOS-3684
https://issues.apache.org/jira/browse/MESOS-3617
https://issues.apache.org/jira/browse/MESOS-3618
https://issues.apache.org/jira/browse/MESOS-3619
https://issues.apache.org/jira/browse/MESOS-3622
https://issues.apache.org/jira/browse/MESOS-3623
https://issues.apache.org/jira/browse/MESOS-3624
https://issues.apache.org/jira/browse/MESOS-3681
https://issues.apache.org/jira/browse/MESOS-3682
https://issues.apache.org/jira/browse/MESOS-3684


Repository: mesos


Description
---

Agent: Add Windows support to the containerizer.


Diffs
-

  src/slave/containerizer/docker.cpp 7af14f4472283ceefd73c06dd8df60af4cf6f7e8 
  src/slave/containerizer/external_containerizer.cpp 
cf4384cce44172a028c890f52f71ceb8ae109383 
  src/slave/containerizer/mesos/containerizer.cpp 
75e5a32a3e70ec60a6800e21a621673184ea0956 
  src/slave/containerizer/mesos/launcher.hpp 
5977c30c0aacc569019f7b34bb0c6577823ec887 
  src/slave/containerizer/mesos/launcher.cpp 
a5c8c31b72773d0bd10b9d02675a01f1d641d41c 

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


Testing
---


Thanks,

Alex Clemmer

Re: Review Request 47511: Added documentation for `docker/volume` isolator.

[Mesos ReviewBot]

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



Bad patch!

Reviews applied: [47511]

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

Error:
2016-05-19 02:13:04 URL:https://reviews.apache.org/r/47511/diff/raw/ 
[11902/11902] -> "47511.patch" [1]
error: missing binary patch data for 'docs/images/docker-volume-isolator.png'
error: binary patch does not apply to 'docs/images/docker-volume-isolator.png'
error: docs/images/docker-volume-isolator.png: patch does not apply

Full log: https://builds.apache.org/job/mesos-reviewbot/13158/console

- Mesos ReviewBot


On May 19, 2016, 1:38 a.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47511/
> ---
> 
> (Updated May 19, 2016, 1:38 a.m.)
> 
> 
> Review request for mesos, Gilbert Song and Jie Yu.
> 
> 
> Bugs: MESOS-5216
> https://issues.apache.org/jira/browse/MESOS-5216
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added documentation for `docker/volume` isolator.
> 
> 
> Diffs
> -
> 
>   docs/docker-volume-isolator.md PRE-CREATION 
>   docs/images/docker-volume-isolator.png PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/47511/diff/
> 
> 
> Testing
> ---
> 
> You can review the document here: 
> https://github.com/jay-lau/mesos/blob/master/docs/docker-volume-isolator.md
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 46369: Added capabilities support in ContanerInfo protobuf.

[Jie Yu]

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


Fix it, then Ship it!





include/mesos/mesos.proto (line 1734)


Remove the extra space before the text.



include/mesos/mesos.proto (line 1739)


Could you please add a NOTE here about why we want to start the tag number 
from 1000? Also, can you add an INVALID=0?


- Jie Yu


On May 11, 2016, 4:27 a.m., Jojy Varghese wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46369/
> ---
> 
> (Updated May 11, 2016, 4:27 a.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-5232
> https://issues.apache.org/jira/browse/MESOS-5232
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added capabilities support in ContanerInfo protobuf.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 9a180304996895e2e003085690a7dff9ec561e9c 
>   include/mesos/v1/mesos.proto 44b4f8a059f9dfdcbf02f0c30c1b859898c2e617 
> 
> Diff: https://reviews.apache.org/r/46369/diff/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Jojy Varghese
> 
>

Re: Review Request 47511: Added documentation for `docker/volume` isolator.

[Guangya Liu]

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

(Updated May 19, 2016, 1:38 a.m.)


Review request for mesos, Gilbert Song and Jie Yu.


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


Repository: mesos


Description
---

Added documentation for `docker/volume` isolator.


Diffs (updated)
-

  docs/docker-volume-isolator.md PRE-CREATION 
  docs/images/docker-volume-isolator.png PRE-CREATION 

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


Testing
---

You can review the document here: 
https://github.com/jay-lau/mesos/blob/master/docs/docker-volume-isolator.md


Thanks,

Guangya Liu

Re: Review Request 47511: Added documentation for `docker/volume` isolator.

[Guangya Liu]

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

(Updated May 19, 2016, 1:37 a.m.)


Review request for mesos, Gilbert Song and Jie Yu.


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


Repository: mesos


Description
---

Added documentation for `docker/volume` isolator.


Diffs (updated)
-

  docs/.mesos-containerizer.md.swp PRE-CREATION 
  docs/docker-volume-isolator.md PRE-CREATION 
  docs/images/docker-volume-isolator.png PRE-CREATION 

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


Testing
---

You can review the document here: 
https://github.com/jay-lau/mesos/blob/master/docs/docker-volume-isolator.md


Thanks,

Guangya Liu

Re: Review Request 47481: Rewrote os::read() to be friendlier to reading binary data.

[Benjamin Mahler]

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


Ship it!





3rdparty/stout/include/stout/os/read.hpp (line 108)


Not yours, but we should avoid including caller-available information here 
as it leads to redundant information when error messages are composed.



3rdparty/stout/include/stout/os/read.hpp (line 120)


true instead of 1?



3rdparty/stout/include/stout/os/read.hpp (line 124)


Perhaps we should mention that ferror does not modify errno if the file is 
valid.



3rdparty/stout/include/stout/os/read.hpp (line 137)


Perhaps we should assert feof here?


- Benjamin Mahler


On May 18, 2016, 3:25 a.m., Kevin Klues wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47481/
> ---
> 
> (Updated May 18, 2016, 3:25 a.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Joris Van Remoortere, and switched 
> to 'mcypark'.
> 
> 
> Bugs: MESOS-5398
> https://issues.apache.org/jira/browse/MESOS-5398
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The previous read() implementation was based on calling getline() to
> read in chunks of data from a file. This is fine for text-based files,
> but is a little strange for binary files.
> 
> The new implementation reads in chunks of raw bytes into a stack
> allocated buffer before copying them into their final location. I
> usually don't like stack allocated buffers because of their potential
> security implications when it comes to stack buffer overflows, but I
> took extra care here to make sure there won't be any.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/os/read.hpp 
> e1e97c1bcb7493a734fc77721a83c230b1a23724 
> 
> Diff: https://reviews.apache.org/r/47481/diff/
> 
> 
> Testing
> ---
> 
> make check -j
> 
> 
> Thanks,
> 
> Kevin Klues
> 
>

Re: Review Request 46613: Introduced filtering relevant actions and acls.

[Joerg Schad]

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

(Updated May 18, 2016, 10:16 p.m.)


Review request for mesos, Adam B, Alexander Rojas, and Michael Park.


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


Repository: mesos


Description (updated)
---

In order to allow for framework and task level filtering we introduce
the following authorizer actions:
* FILTER_FRAMEWORK_WITH_INFO
* FILTER_TASK_WITH_EXECUTOR_INFO = 13;
* FILTER_TASK_WITH_COMMAND_INFO = 14;
* FILTER_TASK_WITH_TASK = 15;

Note that we need different actions for authorizing a tasks
based on the object being authorized.

We also introduce the following acls for the local authorizer:
* ViewFrameworks  (giving access to frameworks running  
   under a specific OS user
* ViewTasks view_tasks (giving access to Tasks run under a
specific OS user)


Diffs (updated)
-

  docs/authorization.md 0db5c345b3239814b3b9d2e8a87601ff69d0f869 
  include/mesos/authorizer/acls.proto 9adae8c2a2e1b2ee4b9068ea50fcebb0544f2e5d 
  include/mesos/authorizer/authorizer.proto 
32492a59ad95df3bb673ec42321518f86c11af59 
  src/authorizer/local/authorizer.cpp aa1a9d8e5c7fb86b6310015d93aeacb466a307ef 

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


Testing
---

make check (OSX)


Thanks,

Joerg Schad

Re: Review Request 47530: Added authorization to agent's '/containers' endpoint.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [47530]

Passed command: export OS='ubuntu:14.04' CONFIGURATION='--verbose' 
COMPILER='gcc' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker_build.sh

- Mesos ReviewBot


On May 18, 2016, 8 p.m., Abhishek Dasgupta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47530/
> ---
> 
> (Updated May 18, 2016, 8 p.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rukletsov, Greg Mann, Jan 
> Schlicht, and Till Toenshoff.
> 
> 
> Bugs: MESOS-5317
> https://issues.apache.org/jira/browse/MESOS-5317
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Used GET_ENDPOINT_WITH_PATH coarse-grained authz on agent's 
> '/containers' endpoint to enable authorization on this endpoint.
> Updated docs and testcases as well.
> 
> 
> Diffs
> -
> 
>   docs/endpoints/slave/containers.md 959f40b9db4de4b6cea456ecf7bcb402f7a94f05 
>   src/slave/http.cpp fb48ec61e2fe0c83f80d3b8aa4c2ef5a96b748ae 
>   src/slave/slave.hpp 209f071448e3c52d16d3366d564003ee36b1d2e0 
>   src/tests/slave_authorization_tests.cpp 
> 843cf1c631e0a25125ca1c0c0028ad1a920c2c2f 
> 
> Diff: https://reviews.apache.org/r/47530/diff/
> 
> 
> Testing
> ---
> 
> ## Unit tests.
> 
> On ubuntu 16.04:
> `sudo GTEST_FILTER="*SlaveEndpointTest*.*" make -j2 check`
> 
> ## Manual testing.
> 
> 1. Ran master with:
> ```
> sudo  ./bin/mesos-master.sh --ip=127.0.0.1 --work_dir=/var/lib/mesos
> ```
> 
> 2. ACL File: 
> ```
>   {
> "get_endpoints": [
>   {
> "principals": { "type": "NONE" },
> "paths": { "values": ["/flags", "/monitor/statistics", "/containers"] 
> }
>   }
> ]
>   } 
> ```
> 
> 3. Ran slave with: 
> ```
> sudo ./bin/mesos-slave.sh --master=127.0.0.1:5050 --ip=0.0.0.0 
> --acls=file:///home/abhishek/testAcl
> ```
> 
> 4. Ran toy-framework with: 
> ```
> sudo ./no-executor-framework --master=master@127.0.0.1:5050 --command="echo 
> hello"
> ```
> 
> 5. Output while hitting "http://127.0.0.1:5051/slave(1)/containers" - HTTP 
> error 403: Forbidden
> 
> 6. Changed ACL to: 
> ```
>   {
> "get_endpoints": [
>   {
> "principals": { "type": "ANY" },
> "paths": { "values": ["/flags", "/monitor/statistics", "/containers"] 
> }
>   }
> ]
>   }
> ```
> 
> 7. Ran slave and framework again.
> 
> 8. Output:
> ```
> 
> [{"container_id":"9b8a6a51-68be-4763-9c7d-b67e85fccb4a","executor_id":"42","executor_name":"Command
>  Executor (Task: 42) (Command: sh -c 'echo hello')","framework_id":"52...
> ```
> 
> 
> Thanks,
> 
> Abhishek Dasgupta
> 
>

Re: Review Request 47400: Documentation quota authorization changes in 0.29.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [47400]

Passed command: export OS='ubuntu:14.04' CONFIGURATION='--verbose' 
COMPILER='gcc' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker_build.sh

- Mesos ReviewBot


On May 18, 2016, 4:53 p.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47400/
> ---
> 
> (Updated May 18, 2016, 4:53 p.m.)
> 
> 
> Review request for mesos, Adam B and Alexander Rukletsov.
> 
> 
> Bugs: MESOS-5155 and MESOS-5336
> https://issues.apache.org/jira/browse/MESOS-5155
> https://issues.apache.org/jira/browse/MESOS-5336
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Documentation quota authorization changes in 0.29.
> 
> 
> Diffs
> -
> 
>   CHANGELOG 6bd59d191a8ca59436fa8a1953587119aeb3e256 
>   docs/authorization.md 9a359dc1c5576e9871c0747d02852bde35d67a3e 
>   docs/quota.md 797e134605381ae576d9aa93875e0314889ab047 
>   docs/upgrades.md 59274362fa6ec1e5857176fd5f8fd78c381f0d52 
> 
> Diff: https://reviews.apache.org/r/47400/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>

Re: Review Request 47485: Added utility for parsing ld.so.cache on linux.

[Kevin Klues]

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

(Updated May 18, 2016, 8:21 p.m.)


Review request for mesos and Benjamin Mahler.


Changes
---

Updated the test based on changes to the ELF parsing library.


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


Repository: mesos


Description
---

Added utility for parsing ld.so.cache on linux.


Diffs (updated)
-

  src/Makefile.am ce5245883f3d2661812272702c0d2060513b6d88 
  src/linux/ldcache.hpp PRE-CREATION 
  src/linux/ldcache.cpp PRE-CREATION 
  src/tests/ldcache_tests.cpp PRE-CREATION 

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


Testing
---

GTEST_FILTER="LdcacheTest.Parse" make check -j


Thanks,

Kevin Klues

Re: Review Request 47530: Added authorization to agent's '/containers' endpoint.

[Abhishek Dasgupta]

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

(Updated May 18, 2016, 8 p.m.)


Review request for mesos, Adam B, Alexander Rukletsov, Greg Mann, Jan Schlicht, 
and Till Toenshoff.


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


Repository: mesos


Description (updated)
---

Used GET_ENDPOINT_WITH_PATH coarse-grained authz on agent's 
'/containers' endpoint to enable authorization on this endpoint.
Updated docs and testcases as well.


Diffs
-

  docs/endpoints/slave/containers.md 959f40b9db4de4b6cea456ecf7bcb402f7a94f05 
  src/slave/http.cpp fb48ec61e2fe0c83f80d3b8aa4c2ef5a96b748ae 
  src/slave/slave.hpp 209f071448e3c52d16d3366d564003ee36b1d2e0 
  src/tests/slave_authorization_tests.cpp 
843cf1c631e0a25125ca1c0c0028ad1a920c2c2f 

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


Testing
---

## Unit tests.

On ubuntu 16.04:
`sudo GTEST_FILTER="*SlaveEndpointTest*.*" make -j2 check`

## Manual testing.

1. Ran master with:
```
sudo  ./bin/mesos-master.sh --ip=127.0.0.1 --work_dir=/var/lib/mesos
```

2. ACL File: 
```
  {
"get_endpoints": [
  {
"principals": { "type": "NONE" },
"paths": { "values": ["/flags", "/monitor/statistics", "/containers"] }
  }
]
  } 
```

3. Ran slave with: 
```
sudo ./bin/mesos-slave.sh --master=127.0.0.1:5050 --ip=0.0.0.0 
--acls=file:///home/abhishek/testAcl
```

4. Ran toy-framework with: 
```
sudo ./no-executor-framework --master=master@127.0.0.1:5050 --command="echo 
hello"
```

5. Output while hitting "http://127.0.0.1:5051/slave(1)/containers" - HTTP 
error 403: Forbidden

6. Changed ACL to: 
```
  {
"get_endpoints": [
  {
"principals": { "type": "ANY" },
"paths": { "values": ["/flags", "/monitor/statistics", "/containers"] }
  }
]
  }
```

7. Ran slave and framework again.

8. Output:
```

[{"container_id":"9b8a6a51-68be-4763-9c7d-b67e85fccb4a","executor_id":"42","executor_name":"Command
 Executor (Task: 42) (Command: sh -c 'echo hello')","framework_id":"52...
```


Thanks,

Abhishek Dasgupta

Re: Review Request 47530: Added authorization to agent's '/containers' endpoint.

[Abhishek Dasgupta]

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

(Updated May 18, 2016, 7:59 p.m.)


Review request for mesos, Adam B, Alexander Rukletsov, Greg Mann, Jan Schlicht, 
and Till Toenshoff.


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


Repository: mesos


Description (updated)
---

Added authorization to agent's '/containers' endpoint.


Diffs (updated)
-

  docs/endpoints/slave/containers.md 959f40b9db4de4b6cea456ecf7bcb402f7a94f05 
  src/slave/http.cpp fb48ec61e2fe0c83f80d3b8aa4c2ef5a96b748ae 
  src/slave/slave.hpp 209f071448e3c52d16d3366d564003ee36b1d2e0 
  src/tests/slave_authorization_tests.cpp 
843cf1c631e0a25125ca1c0c0028ad1a920c2c2f 

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


Testing
---

## Unit tests.

On ubuntu 16.04:
`sudo GTEST_FILTER="*SlaveEndpointTest*.*" make -j2 check`

## Manual testing.

1. Ran master with:
```
sudo  ./bin/mesos-master.sh --ip=127.0.0.1 --work_dir=/var/lib/mesos
```

2. ACL File: 
```
  {
"get_endpoints": [
  {
"principals": { "type": "NONE" },
"paths": { "values": ["/flags", "/monitor/statistics", "/containers"] }
  }
]
  } 
```

3. Ran slave with: 
```
sudo ./bin/mesos-slave.sh --master=127.0.0.1:5050 --ip=0.0.0.0 
--acls=file:///home/abhishek/testAcl
```

4. Ran toy-framework with: 
```
sudo ./no-executor-framework --master=master@127.0.0.1:5050 --command="echo 
hello"
```

5. Output while hitting "http://127.0.0.1:5051/slave(1)/containers" - HTTP 
error 403: Forbidden

6. Changed ACL to: 
```
  {
"get_endpoints": [
  {
"principals": { "type": "ANY" },
"paths": { "values": ["/flags", "/monitor/statistics", "/containers"] }
  }
]
  }
```

7. Ran slave and framework again.

8. Output:
```

[{"container_id":"9b8a6a51-68be-4763-9c7d-b67e85fccb4a","executor_id":"42","executor_name":"Command
 Executor (Task: 42) (Command: sh -c 'echo hello')","framework_id":"52...
```


Thanks,

Abhishek Dasgupta

Re: Review Request 47482: Added preliminary support for parsing ELF files in stout.

[Kevin Klues]

> On May 18, 2016, 5:35 p.m., Jojy Varghese wrote:
> > 3rdparty/stout/include/stout/elf.hpp, line 56
> > 
> >
> > Wondering why the implementation is in the header file.

For better or worse, stout is a header-only library.  The implementation of all 
classes / functions are in header files.


> On May 18, 2016, 5:35 p.m., Jojy Varghese wrote:
> > 3rdparty/stout/include/stout/elf.hpp, line 63
> > 
> >
> > Its recommended to use a smart pointer here. They provide the RAII 
> > property that you need here. Also, the pattern in Mesos is to use factory 
> > methods that return a `Owned`.

`Owner` doesn't exist in stout. As I mentioned in response to your comment 
above about "Dont we have to close the open file in error cases?", I tried 
using a `std::unique_ptr` here, but it doesn't play nicely with `Try<>`. If you 
have any other suggestions, I'd love to hear them.  Maybe it's worth adding a 
comment in the code somehwere about the limitations I encountered.


> On May 18, 2016, 5:35 p.m., Jojy Varghese wrote:
> > 3rdparty/stout/include/stout/elf.hpp, line 188
> > 
> >
> > The advantage of emplace_back here would be that the `string` object 
> > will be created only once as opposed to being first created and then copied.

OK, good to know.


- Kevin


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


On May 18, 2016, 3:16 a.m., Kevin Klues wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47482/
> ---
> 
> (Updated May 18, 2016, 3:16 a.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Bugs: MESOS-5400
> https://issues.apache.org/jira/browse/MESOS-5400
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Right now we are able to parse ELF formatted shared libraries and
> extract their canonical SONAME and external library dependencies. In
> the future, we should add support for fully parsing an ELf file for
> easy access to all of its contents.
> 
> The current implementation relies on libelf. We should probably remove
> this dependency in future versions (mostly since the headers for
> libelf are not installed on a standard Linux distribution by default).
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/configure.ac ada1e22c72875fe9d557f07e4846128be0bcef13 
>   3rdparty/stout/include/stout/elf.hpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/47482/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Kevin Klues
> 
>

Re: Review Request 47530: Added authorization to agent's '/containers' endpoint.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [47530]

Passed command: export OS='ubuntu:14.04' CONFIGURATION='--verbose' 
COMPILER='gcc' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker_build.sh

- Mesos ReviewBot


On May 18, 2016, 12:59 p.m., Abhishek Dasgupta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47530/
> ---
> 
> (Updated May 18, 2016, 12:59 p.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rukletsov, Greg Mann, Jan 
> Schlicht, and Till Toenshoff.
> 
> 
> Bugs: MESOS-5317
> https://issues.apache.org/jira/browse/MESOS-5317
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Used GET_ENDPOINT_WITH_PATH coarse-grained authz on agent's 
>  '/containers' endpoint to enable authorization on this endpoint.
>  Updated docs and testcases as well.
> 
> 
> Diffs
> -
> 
>   docs/endpoints/slave/containers.md 959f40b9db4de4b6cea456ecf7bcb402f7a94f05 
>   src/slave/http.cpp fb48ec61e2fe0c83f80d3b8aa4c2ef5a96b748ae 
>   src/slave/slave.hpp 209f071448e3c52d16d3366d564003ee36b1d2e0 
>   src/tests/slave_authorization_tests.cpp 
> 843cf1c631e0a25125ca1c0c0028ad1a920c2c2f 
> 
> Diff: https://reviews.apache.org/r/47530/diff/
> 
> 
> Testing
> ---
> 
> ## Unit tests.
> 
> On ubuntu 16.04:
> `sudo GTEST_FILTER="*SlaveEndpointTest*.*" make -j2 check`
> 
> ## Manual testing.
> 
> 1. Ran master with:
> ```
> sudo  ./bin/mesos-master.sh --ip=127.0.0.1 --work_dir=/var/lib/mesos
> ```
> 
> 2. ACL File: 
> ```
>   {
> "get_endpoints": [
>   {
> "principals": { "type": "NONE" },
> "paths": { "values": ["/flags", "/monitor/statistics", "/containers"] 
> }
>   }
> ]
>   } 
> ```
> 
> 3. Ran slave with: 
> ```
> sudo ./bin/mesos-slave.sh --master=127.0.0.1:5050 --ip=0.0.0.0 
> --acls=file:///home/abhishek/testAcl
> ```
> 
> 4. Ran toy-framework with: 
> ```
> sudo ./no-executor-framework --master=master@127.0.0.1:5050 --command="echo 
> hello"
> ```
> 
> 5. Output while hitting "http://127.0.0.1:5051/slave(1)/containers" - HTTP 
> error 403: Forbidden
> 
> 6. Changed ACL to: 
> ```
>   {
> "get_endpoints": [
>   {
> "principals": { "type": "ANY" },
> "paths": { "values": ["/flags", "/monitor/statistics", "/containers"] 
> }
>   }
> ]
>   }
> ```
> 
> 7. Ran slave and framework again.
> 
> 8. Output:
> ```
> 
> [{"container_id":"9b8a6a51-68be-4763-9c7d-b67e85fccb4a","executor_id":"42","executor_name":"Command
>  Executor (Task: 42) (Command: sh -c 'echo hello')","framework_id":"52...
> ```
> 
> 
> Thanks,
> 
> Abhishek Dasgupta
> 
>

Re: Review Request 45474: MESOS-1739: Allow slave reconfiguration on restart, Phase 1.

[Adam B]

> On March 30, 2016, 2:08 a.m., Adam B wrote:
> > src/tests/slave_tests.cpp, lines 3541-3545
> > 
> >
> > Here you shutdown the slave and wait (you'll probably want to advance 
> > the clock rather than wait for 90s) for the slave to be declared 
> > SLAVE_LOST. Once this occurs, the master will no longer allow the slave to 
> > reregister with the same slaveId, and the slave will be told to kill all 
> > running tasks. The slave will do so and then restart and register as a new 
> > slaveId. 
> > This is what is meant by the quote from the design doc: "Currently this 
> > can only be handled by stopping / draining a mesos slave entirely (Killing 
> > all of its running jobs), removing it from the cluster, then bringing it 
> > back up as a brand new slave."
> > 
> > To truly observe this behavior, you should start a task on the slave 
> > before you shut it down. Then you will see a TASK_LOST and the task will be 
> > killed.
> 
> Deshi Xiao wrote:
> Thanks Adam, i will udpate the test case.
> 
> Deshi Xiao wrote:
> @Adam B
> Here i have a confuse,need your guide. use test case to track the 
> TASK_LOST in restart slave. do we expect keep the slave_id is not outdate?

Desired behavior: Operator can kill a slave process and restart it with new 
--attributes. Existing tasks will continue to run. No TASK_LOST or SLAVE_LOST 
message is sent. The slaveId remains the same. Outstanding offers from that 
slave will be rescinded, and those offers will be remade with the updated 
attributes.
Current behavior 1: Operator shuts down a slave process, and restarts with 
--recover=cleanup, which kills all its tasks, clears the work_dir, and notifies 
the master that the old slaveId is "shutdown" and will never be reused again 
(SLAVE_LOST, offers rescinded, TASK_KILLED/LOST). Operator then restarts the 
slave with new --attributes, it gets a new slaveId, and new offers will be made 
with the new slaveId and updated attributes.
Current behavior 2: Slave process dies/killed and tries to restart with new 
--attributes. Errors on recovery.
Current behavior 3: Slave process dies/killed and doesn't reregister in 
`slave_ping_timeout*max_slave_ping_timeouts` (90s). Master considers it gone, 
sends SLAVE_LOST, TASK_LOST. Future attempts to reregister with the same 
slaveId fail. Slave must be cleaned up (tasks killed, work_dir removed) so it 
can register with a new slaveId (and new attributes).


- Adam


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


On March 30, 2016, 1:13 a.m., Deshi Xiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45474/
> ---
> 
> (Updated March 30, 2016, 1:13 a.m.)
> 
> 
> Review request for mesos, Adam B, Greg Mann, haosdent huang, and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-1739
> https://issues.apache.org/jira/browse/MESOS-1739
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Phase 1
> Make SlaveInfo mutable throughout the stack, and allow for expansion of 
> resources and attributes only (Which allows testing to make sure it 
> propagates to the allocator, shows up in offers, etc). Ensure there is 
> unified checking for incompatibilities in both the slave and master (the 
> slave should validate the config, the master should validate that all 
> operations the slave takes are legal).
> 
> it derived from another PR(https://reviews.apache.org/r/25525/)
> 
> 
> Diffs
> -
> 
>   src/tests/slave_tests.cpp 1f1a31020096efa5db698e86ac74e61dfdb4b94a 
> 
> Diff: https://reviews.apache.org/r/45474/diff/
> 
> 
> Testing
> ---
> 
> make check on localhost
> 
> 
> Thanks,
> 
> Deshi Xiao
> 
>

Re: Review Request 47463: Added documentation for `network/cni` isolator.

[Avinash sridharan]

> On May 18, 2016, 6:39 p.m., Vinod Kone wrote:
> > docs/cni.md, line 5
> > 
> >
> > Should we make it explicit that this is not supported for 
> > DockerContainerizer?

I don't think we should be explicit about the `DockerContainerizer` here. I 
mention in the introduction that the `network/cni` isolator is for the 
`MesosContainerizer`. The title also states that this is for Mesos containers. 
We are also going to have a separate page for `Networking with 
DockerContainerizer`. Was also thinking that in the home page we can link this 
page using the title "Networking with MesosContainerizer".


> On May 18, 2016, 6:39 p.m., Vinod Kone wrote:
> > docs/cni.md, lines 100-101
> > 
> >
> > What happens if a network that is in use (by containers) is removed 
> > from the config and agent restarted? What if the network config is updated? 
> > Is that all safe?

Thanks !! This is an important point that I missed. Currently we are not 
checkpointing the CNI configuration for a given container. This implies that if 
the current CNI config is modified or deleted, it won't affect container 
operation, or even Agent restart, but it will impact deletion of containers. 
Since the plugin information in the CNI config might have changed, resulting in 
the plugin throwing an error when it tries to delete the veth and release IP 
address from the container network namespace.

I will add a `### Limiations` section describing this behavior. We have 
https://issues.apache.org/jira/browse/MESOS-5310 to address this issue.


> On May 18, 2016, 6:39 p.m., Vinod Kone wrote:
> > docs/cni.md, lines 114-119
> > 
> >
> > Do all these configs result in the same networking behavior?
> > 
> > -- network/cni flag is disabled
> > -- NetworkInfo is not set and flag is enabled
> > --- NetworkInfo is not set and flag is disabled
> > -- NetworkInfo is set but name is not and flag is enabled
> > -- NetworkInfo is set but name is not and flag is disabled
> > 
> > -- all the above options with 0.28.0; without the flag ofcourse

Yes. In all the above cases the behavior will be that the container will use 
the host network namespace, effectively joining the host naetwork.


- Avinash


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


On May 18, 2016, 1:31 p.m., Avinash sridharan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47463/
> ---
> 
> (Updated May 18, 2016, 1:31 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Jie Yu, Kevin Klues, Neil Conway, 
> Qian Zhang, and Vinod Kone.
> 
> 
> Bugs: MESOS-4771
> https://issues.apache.org/jira/browse/MESOS-4771
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added documentation for `network/cni` isolator.
> 
> 
> Diffs
> -
> 
>   docs/cni.md PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/47463/diff/
> 
> 
> Testing
> ---
> 
> Build the documentation website and verified the rendering.
> 
> You can review a rendering of the markdown on my github:
> https://github.com/asridharan/mesos/blob/MESOS-4771/docs/cni.md
> 
> 
> Thanks,
> 
> Avinash sridharan
> 
>

Re: Review Request 47364: Set env variable used to toggle executor implementation.

[Anand Mazumdar]

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

(Updated May 18, 2016, 6:55 p.m.)


Review request for mesos and Vinod Kone.


Changes
---

Updated JIRA link


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


Repository: mesos


Description
---

This change adds the environment variable that is set by
the agent allowing the command executor to toggle between
implementations.


Diffs
-

  src/slave/containerizer/containerizer.cpp 
d0cae79834e451594d7675f00c5f7d2d2cd3a264 

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


Testing
---

make check


Thanks,

Anand Mazumdar

Re: Review Request 47463: Added documentation for `network/cni` isolator.

[Vinod Kone]

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


Fix it, then Ship it!




LGTM. I'm assuming Jie already verified the technical correctness.


docs/cni.md (line 5)


Should we make it explicit that this is not supported for 
DockerContainerizer?



docs/cni.md (line 36)


s/the users/users/



docs/cni.md (lines 84 - 85)


The second sentence "The operator..." seems redundant? I suggest to kill it.



docs/cni.md (line 94)


s/An important point to note is/Note/



docs/cni.md (lines 100 - 101)


What happens if a network that is in use (by containers) is removed from 
the config and agent restarted? What if the network config is updated? Is that 
all safe?



docs/cni.md (lines 114 - 119)


Do all these configs result in the same networking behavior?

-- network/cni flag is disabled
-- NetworkInfo is not set and flag is enabled
--- NetworkInfo is not set and flag is disabled
-- NetworkInfo is set but name is not and flag is enabled
-- NetworkInfo is set but name is not and flag is disabled

-- all the above options with 0.28.0; without the flag ofcourse



docs/cni.md (line 197)


s/provisioner/provider/ ?



docs/cni.md (line 200)


s/of frameworks/of a framework/



docs/cni.md (line 247)


and vice versa unless the executor uses HTTP API?



docs/cni.md (line 255)


s/existing// ?


- Vinod Kone


On May 18, 2016, 1:31 p.m., Avinash sridharan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47463/
> ---
> 
> (Updated May 18, 2016, 1:31 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Jie Yu, Kevin Klues, Neil Conway, 
> Qian Zhang, and Vinod Kone.
> 
> 
> Bugs: MESOS-4771
> https://issues.apache.org/jira/browse/MESOS-4771
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added documentation for `network/cni` isolator.
> 
> 
> Diffs
> -
> 
>   docs/cni.md PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/47463/diff/
> 
> 
> Testing
> ---
> 
> Build the documentation website and verified the rendering.
> 
> You can review a rendering of the markdown on my github:
> https://github.com/asridharan/mesos/blob/MESOS-4771/docs/cni.md
> 
> 
> Thanks,
> 
> Avinash sridharan
> 
>

Re: Review Request 47510: Adjusted style to make website mobile friendly.

[Vinod Kone]

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


Ship it!




Ship It!

- Vinod Kone


On May 18, 2016, 6:04 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47510/
> ---
> 
> (Updated May 18, 2016, 6:04 p.m.)
> 
> 
> Review request for mesos, Freddy Ayuso-Henson, Tomasz Janiszewski, Tim 
> Anderegg, and Vinod Kone.
> 
> 
> Bugs: MESOS-3690
> https://issues.apache.org/jira/browse/MESOS-3690
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Adjusted style to make website mobile friendly.
> 
> 
> Diffs
> -
> 
>   site/source/assets/css/main.css 73ca8946091a638a359b1d769961879a283fa099 
>   site/source/index.html.md 7b4bdaee61687f487423c1d90a674c78fdf002a4 
>   site/source/layouts/layout.erb 2bf6967faad45644647b732be0fa3c410b9951c5 
> 
> Diff: https://reviews.apache.org/r/47510/diff/
> 
> 
> Testing
> ---
> 
> # Note
> @vinodkone, this should credited to @fayusohenson and @janisz when submit.
> I pick the necessary changes for mobile friendly from 
> https://github.com/apache/mesos/pull/75 .
> 
> And record some simple videos under mobile, tablet and pc to show this patch.
> 
> ![home_mobile.gif](https://issues.apache.org/jira/secure/attachment/12804724/home_mobile.gif)
> ![home_tablet.gif](https://issues.apache.org/jira/secure/attachment/12804726/home_tablet.gif)
> ![home_pc.gif](https://issues.apache.org/jira/secure/attachment/12804725/home_pc.gif)
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 47510: Adjusted style to make website mobile friendly.

[Vinod Kone]

> On May 18, 2016, 6:05 p.m., Vinod Kone wrote:
> > site/source/layouts/layout.erb, lines 66-83
> > 
> >
> > what does this do?
> > 
> > also, can you attach before and after screen shots?
> 
> Tim Anderegg wrote:
> @vinodkone That section collapes the menu for mobile devices, which 
> addresses the comment I left on the other patch.  Without it, the menu takes 
> up a good portion of the screen until the user scrolls down, with this the 
> menu is hidden unless the user hits the new menu button.  The new menu button 
> is only displayed on mobile.

Gotcha. Thanks!


- Vinod


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


On May 18, 2016, 6:04 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47510/
> ---
> 
> (Updated May 18, 2016, 6:04 p.m.)
> 
> 
> Review request for mesos, Freddy Ayuso-Henson, Tomasz Janiszewski, Tim 
> Anderegg, and Vinod Kone.
> 
> 
> Bugs: MESOS-3690
> https://issues.apache.org/jira/browse/MESOS-3690
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Adjusted style to make website mobile friendly.
> 
> 
> Diffs
> -
> 
>   site/source/assets/css/main.css 73ca8946091a638a359b1d769961879a283fa099 
>   site/source/index.html.md 7b4bdaee61687f487423c1d90a674c78fdf002a4 
>   site/source/layouts/layout.erb 2bf6967faad45644647b732be0fa3c410b9951c5 
> 
> Diff: https://reviews.apache.org/r/47510/diff/
> 
> 
> Testing
> ---
> 
> # Note
> @vinodkone, this should credited to @fayusohenson and @janisz when submit.
> I pick the necessary changes for mobile friendly from 
> https://github.com/apache/mesos/pull/75 .
> 
> And record some simple videos under mobile, tablet and pc to show this patch.
> 
> ![home_mobile.gif](https://issues.apache.org/jira/secure/attachment/12804724/home_mobile.gif)
> ![home_tablet.gif](https://issues.apache.org/jira/secure/attachment/12804726/home_tablet.gif)
> ![home_pc.gif](https://issues.apache.org/jira/secure/attachment/12804725/home_pc.gif)
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 47510: Adjusted style to make website mobile friendly.

[Tim Anderegg]

> On May 18, 2016, 6:05 p.m., Vinod Kone wrote:
> > site/source/layouts/layout.erb, lines 66-83
> > 
> >
> > what does this do?
> > 
> > also, can you attach before and after screen shots?

@vinodkone That section collapes the menu for mobile devices, which addresses 
the comment I left on the other patch.  Without it, the menu takes up a good 
portion of the screen until the user scrolls down, with this the menu is hidden 
unless the user hits the new menu button.  The new menu button is only 
displayed on mobile.


- Tim


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


On May 18, 2016, 6:04 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47510/
> ---
> 
> (Updated May 18, 2016, 6:04 p.m.)
> 
> 
> Review request for mesos, Freddy Ayuso-Henson, Tomasz Janiszewski, Tim 
> Anderegg, and Vinod Kone.
> 
> 
> Bugs: MESOS-3690
> https://issues.apache.org/jira/browse/MESOS-3690
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Adjusted style to make website mobile friendly.
> 
> 
> Diffs
> -
> 
>   site/source/assets/css/main.css 73ca8946091a638a359b1d769961879a283fa099 
>   site/source/index.html.md 7b4bdaee61687f487423c1d90a674c78fdf002a4 
>   site/source/layouts/layout.erb 2bf6967faad45644647b732be0fa3c410b9951c5 
> 
> Diff: https://reviews.apache.org/r/47510/diff/
> 
> 
> Testing
> ---
> 
> # Note
> @vinodkone, this should credited to @fayusohenson and @janisz when submit.
> I pick the necessary changes for mobile friendly from 
> https://github.com/apache/mesos/pull/75 .
> 
> And record some simple videos under mobile, tablet and pc to show this patch.
> 
> ![home_mobile.gif](https://issues.apache.org/jira/secure/attachment/12804724/home_mobile.gif)
> ![home_tablet.gif](https://issues.apache.org/jira/secure/attachment/12804726/home_tablet.gif)
> ![home_pc.gif](https://issues.apache.org/jira/secure/attachment/12804725/home_pc.gif)
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 47510: Adjusted style to make website mobile friendly.

[haosdent huang]

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

(Updated May 18, 2016, 6:04 p.m.)


Review request for mesos, Freddy Ayuso-Henson, Tomasz Janiszewski, Tim 
Anderegg, and Vinod Kone.


Changes
---

Add record gifs.


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


Repository: mesos


Description
---

Adjusted style to make website mobile friendly.


Diffs
-

  site/source/assets/css/main.css 73ca8946091a638a359b1d769961879a283fa099 
  site/source/index.html.md 7b4bdaee61687f487423c1d90a674c78fdf002a4 
  site/source/layouts/layout.erb 2bf6967faad45644647b732be0fa3c410b9951c5 

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


Testing (updated)
---

# Note
@vinodkone, this should credited to @fayusohenson and @janisz when submit.
I pick the necessary changes for mobile friendly from 
https://github.com/apache/mesos/pull/75 .

And record some simple videos under mobile, tablet and pc to show this patch.

![home_mobile.gif](https://issues.apache.org/jira/secure/attachment/12804724/home_mobile.gif)
![home_tablet.gif](https://issues.apache.org/jira/secure/attachment/12804726/home_tablet.gif)
![home_pc.gif](https://issues.apache.org/jira/secure/attachment/12804725/home_pc.gif)


Thanks,

haosdent huang

Re: Review Request 47510: Adjusted style to make website mobile friendly.

[Vinod Kone]

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




site/source/layouts/layout.erb (lines 65 - 82)


what does this do?

also, can you attach before and after screen shots?


- Vinod Kone


On May 18, 2016, 6:04 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47510/
> ---
> 
> (Updated May 18, 2016, 6:04 p.m.)
> 
> 
> Review request for mesos, Freddy Ayuso-Henson, Tomasz Janiszewski, Tim 
> Anderegg, and Vinod Kone.
> 
> 
> Bugs: MESOS-3690
> https://issues.apache.org/jira/browse/MESOS-3690
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Adjusted style to make website mobile friendly.
> 
> 
> Diffs
> -
> 
>   site/source/assets/css/main.css 73ca8946091a638a359b1d769961879a283fa099 
>   site/source/index.html.md 7b4bdaee61687f487423c1d90a674c78fdf002a4 
>   site/source/layouts/layout.erb 2bf6967faad45644647b732be0fa3c410b9951c5 
> 
> Diff: https://reviews.apache.org/r/47510/diff/
> 
> 
> Testing
> ---
> 
> # Note
> @vinodkone, this should credited to @fayusohenson and @janisz when submit.
> I pick the necessary changes for mobile friendly from 
> https://github.com/apache/mesos/pull/75 .
> 
> And record some simple videos under mobile, tablet and pc to show this patch.
> 
> ![home_mobile.gif](https://issues.apache.org/jira/secure/attachment/12804724/home_mobile.gif)
> ![home_tablet.gif](https://issues.apache.org/jira/secure/attachment/12804726/home_tablet.gif)
> ![home_pc.gif](https://issues.apache.org/jira/secure/attachment/12804725/home_pc.gif)
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 46761: Slave/Agent Rename Phase I - Update terms in WebUI.

[Vinod Kone]

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


Ship it!




Ship It!

- Vinod Kone


On May 18, 2016, 2:16 a.m., zhou xing wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46761/
> ---
> 
> (Updated May 18, 2016, 2:16 a.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Kevin Klues, Maxim Khutornenko, 
> and Vinod Kone.
> 
> 
> Bugs: mesos-3779
> https://issues.apache.org/jira/browse/mesos-3779
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch did the following changes in WebUI:
> 1. Slave/Agent Rename on web page
> 2. Slave/Agent Rename in web url
> 3. Slave/Agent Rename in JS class/method/attribute
> 4. Slave/Agent Rename in comments
> 
> For the JSON data returned from server that contains
> term 'slave', the change is not included.
> 
> 
> Diffs
> -
> 
>   CHANGELOG 6bd59d191a8ca59436fa8a1953587119aeb3e256 
>   src/Makefile.am ce5245883f3d2661812272702c0d2060513b6d88 
>   src/webui/master/static/browse.html 
> 04a4600f0c762a2412ddee078ba2c173d595aa8d 
>   src/webui/master/static/framework.html 
> 041513b0e005e8b54ca9723741b21b136ff61ca6 
>   src/webui/master/static/home.html 4b201d72f9dfd787133008b8105a225ffb2747aa 
>   src/webui/master/static/index.html ec2f5792d21bf7efb479e87be3812b06bfbe98dc 
>   src/webui/master/static/js/app.js 543fe9efb9618b311c7f21b7771a251738b01a91 
>   src/webui/master/static/js/controllers.js 
> 8d9021cc89e54ae3a4151ff7f399733f5a7376dd 
>   src/webui/master/static/js/services.js 
> fa5cc35c1ef0e8ec149ed88852837058ec6ab13c 
>   src/webui/master/static/offers.html 
> ec32a649239da48270a1ad1d5bf195326c31ff9d 
>   src/webui/master/static/slave.html c908511df85141128599ad5edc40d4b567437822 
>   src/webui/master/static/slave_executor.html 
> 99b23ed9e85011a66bad780fb2d3076e946821a6 
>   src/webui/master/static/slave_framework.html 
> 176e7e9fa7878f31268bd5aa06dfc8789f3e7edd 
>   src/webui/master/static/slaves.html 
> 063031771cef8b9f45723869198bad3460591936 
> 
> Diff: https://reviews.apache.org/r/46761/diff/
> 
> 
> Testing
> ---
> 
> cd mesos
> ./bootstrap
> mkdir build
> cd build
> ../configure --prefix=${HOME}/install/mesos
> make
> make check
> make install
> 
> start mesos master/agent and run a sample framework, then open browser and 
> goto : to check all the strings on web page and url
> 
> Tested the following slave relevant urls in browser, and it redirect to 
> corresponding agent urls:
> 
>   '/slaves', redirectTo: '/agents'
> 
>   '/slaves/:agent_id', redirectTo: '/agents/:agent_id'
> 
>   '/slaves/:agent_id/frameworks/:framework_id', redirectTo: 
> '/agents/:agent_id/frameworks/:framework_id'
> 
>   '/slaves/:agent_id/frameworks/:framework_id/executors/:executor_id', 
> redirectTo: 
> '/agents/:agent_id/frameworks/:framework_id/executors/:executor_id'
> 
>   '/slaves/:agent_id/frameworks/:framework_id/executors/:executor_id/browse', 
> redirectTo: 
> '/agents/:agent_id/frameworks/:framework_id/executors/:executor_id/browse'
> 
>   '/slaves/:agent_id/browse', redirectTo: '/agents/:agent_id/browse'
> 
> 
> Thanks,
> 
> zhou xing
> 
>

Re: Review Request 47536: Agent: Added Windows isolators.

[Mesos ReviewBot]

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



Bad patch!

Reviews applied: [47536, 47472, 47471, 47470, 47469, 47468, 47412, 47411, 
47410, 47409, 47404, 47403, 47391, 47390, 47389, 47388, 47387, 47386, 47169, 
47168, 41632, 47054, 47221, 47053, 47052]

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

Error:
2016-05-18 18:07:23 URL:https://reviews.apache.org/r/47471/diff/raw/ 
[5804/5804] -> "47471.patch" [1]
error: patch failed: src/exec/exec.cpp:595
error: src/exec/exec.cpp: patch does not apply
error: patch failed: src/executor/executor.cpp:154
error: src/executor/executor.cpp: patch does not apply
error: patch failed: src/launcher/fetcher.cpp:443
error: src/launcher/fetcher.cpp: patch does not apply

Full log: https://builds.apache.org/job/mesos-reviewbot/13151/console

- Mesos ReviewBot


On May 18, 2016, 1:41 p.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47536/
> ---
> 
> (Updated May 18, 2016, 1:41 p.m.)
> 
> 
> Review request for mesos, Daniel Pravat, Artem Harutyunyan, Joris Van 
> Remoortere, and Michael Park.
> 
> 
> Bugs: MESOS-3619 and MESOS-3620
> https://issues.apache.org/jira/browse/MESOS-3619
> https://issues.apache.org/jira/browse/MESOS-3620
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Agent: Added Windows isolators.
> 
> 
> Diffs
> -
> 
>   src/CMakeLists.txt e0c538d9e6542fbe279bfbf6f20172e4c611c859 
>   src/Makefile.am 447bc2ab511ad173d3d911be10992be0974f4584 
>   src/slave/containerizer/mesos/isolators/filesystem/posix.hpp 
> c6cea98e16f2bdea2da0220c235468080bbcd17b 
>   src/slave/containerizer/mesos/isolators/filesystem/posix.cpp 
> 01c0ad6dbb6d509e62e769365586b3d23dcb240d 
>   src/slave/containerizer/mesos/isolators/filesystem/windows.hpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/filesystem/windows.cpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/posix.hpp 
> 227505a70a440f15e68ac001878bcf25610db45f 
>   src/slave/containerizer/mesos/isolators/windows.hpp PRE-CREATION 
>   src/usage/main.cpp 731acb69900b6fc2bb7bd19cccd78aafb0cc 
>   src/usage/usage.hpp 2e1996a78c617c2559ef882ebede5c8aab6f899f 
>   src/usage/usage.cpp 3b19682e67372b81484eacddbab78c2e5eda3c5b 
> 
> Diff: https://reviews.apache.org/r/47536/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>

Re: Review Request 47482: Added preliminary support for parsing ELF files in stout.

[Jojy Varghese]

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




3rdparty/stout/include/stout/elf.hpp (line 56)


Wondering why the implementation is in the header file.



3rdparty/stout/include/stout/elf.hpp (line 63)


Its recommended to use a smart pointer here. They provide the RAII property 
that you need here. Also, the pattern in Mesos is to use factory methods that 
return a `Owned`.



3rdparty/stout/include/stout/elf.hpp (line 188)


The advantage of emplace_back here would be that the `string` object will 
be created only once as opposed to being first created and then copied.


- Jojy Varghese


On May 18, 2016, 3:16 a.m., Kevin Klues wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47482/
> ---
> 
> (Updated May 18, 2016, 3:16 a.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Bugs: MESOS-5400
> https://issues.apache.org/jira/browse/MESOS-5400
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Right now we are able to parse ELF formatted shared libraries and
> extract their canonical SONAME and external library dependencies. In
> the future, we should add support for fully parsing an ELf file for
> easy access to all of its contents.
> 
> The current implementation relies on libelf. We should probably remove
> this dependency in future versions (mostly since the headers for
> libelf are not installed on a standard Linux distribution by default).
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/configure.ac ada1e22c72875fe9d557f07e4846128be0bcef13 
>   3rdparty/stout/include/stout/elf.hpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/47482/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Kevin Klues
> 
>

Re: Review Request 47511: Added documentation for `docker/volume` isolator.

[Mesos ReviewBot]

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



Bad patch!

Reviews applied: [47511]

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

Error:
2016-05-18 17:34:03 URL:https://reviews.apache.org/r/47511/diff/raw/ 
[7/7] -> "47511.patch" [1]
error: missing binary patch data for 'docs/images/docker-volume-isolator.png'
error: binary patch does not apply to 'docs/images/docker-volume-isolator.png'
error: docs/images/docker-volume-isolator.png: patch does not apply

Full log: https://builds.apache.org/job/mesos-reviewbot/13150/console

- Mesos ReviewBot


On May 18, 2016, 9:43 a.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47511/
> ---
> 
> (Updated May 18, 2016, 9:43 a.m.)
> 
> 
> Review request for mesos, Gilbert Song and Jie Yu.
> 
> 
> Bugs: MESOS-5216
> https://issues.apache.org/jira/browse/MESOS-5216
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added documentation for `docker/volume` isolator.
> 
> 
> Diffs
> -
> 
>   docs/docker-volume-isolator.md PRE-CREATION 
>   docs/images/docker-volume-isolator.png PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/47511/diff/
> 
> 
> Testing
> ---
> 
> You can review the document here: 
> https://github.com/jay-lau/mesos/blob/master/docs/docker-volume-isolator.md
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 47509: Fixed authorization::Request initializings.

[Mesos ReviewBot]

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



Bad patch!

Reviews applied: [47509]

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

Error:
2016-05-18 17:07:47 URL:https://reviews.apache.org/r/47509/diff/raw/ 
[13117/13117] -> "47509.patch" [1]
error: patch failed: src/slave/http.cpp:845
error: src/slave/http.cpp: patch does not apply

Full log: https://builds.apache.org/job/mesos-reviewbot/13149/console

- Mesos ReviewBot


On May 18, 2016, 3:10 a.m., Till Toenshoff wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47509/
> ---
> 
> (Updated May 18, 2016, 3:10 a.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rukletsov, Alexander Rojas, and 
> Vinod Kone.
> 
> 
> Bugs: MESOS-5405
> https://issues.apache.org/jira/browse/MESOS-5405
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixes a number of a authorization::Request initializings that were
> missing required fields. Also adds a CHECK to the LocalAuthorizer
> helping us to catch and prevent these problems in the future.
> 
> 
> Diffs
> -
> 
>   src/authorizer/local/authorizer.cpp 
> e95435327bb3b6f447e814b8657bce8084535346 
>   src/common/http.cpp ad6a4b44af3ec847a3a6c0839a88fba18cda5011 
>   src/master/http.cpp 6cefcc8b6ddcfe4098521e838bfdc95936fe7476 
>   src/master/master.cpp 35b428b0f7dee5954514d8860cfc498271ccf267 
>   src/master/quota_handler.cpp 6590519d8440f352f5bf00fda805414a5aad725c 
>   src/master/weights_handler.cpp 4bc060fdb015df6658194eef92fe11b14aa15c79 
>   src/slave/http.cpp 0fb9b81c4a87250e454e3380c61cf9037454810b 
>   src/tests/authorization_tests.cpp f50ac69c32d0551a63391d97f31559ff4f414011 
> 
> Diff: https://reviews.apache.org/r/47509/diff/
> 
> 
> Testing
> ---
> 
> make check (OSX)
> 
> 
> Thanks,
> 
> Till Toenshoff
> 
>

Re: Review Request 47400: Documentation quota authorization changes in 0.29.

[Zhitao Li]

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

(Updated May 18, 2016, 4:53 p.m.)


Review request for mesos, Adam B and Alexander Rukletsov.


Changes
---

Rebase and review comments.


Bugs: MESOS-5155 and MESOS-5336
https://issues.apache.org/jira/browse/MESOS-5155
https://issues.apache.org/jira/browse/MESOS-5336


Repository: mesos


Description
---

Documentation quota authorization changes in 0.29.


Diffs (updated)
-

  CHANGELOG 6bd59d191a8ca59436fa8a1953587119aeb3e256 
  docs/authorization.md 9a359dc1c5576e9871c0747d02852bde35d67a3e 
  docs/quota.md 797e134605381ae576d9aa93875e0314889ab047 
  docs/upgrades.md 59274362fa6ec1e5857176fd5f8fd78c381f0d52 

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


Testing
---


Thanks,

Zhitao Li

Re: Review Request 47516: Added creator principal to tests.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [47515, 47528, 47519, 47520, 47521, 47522, 47516]

Passed command: export OS='ubuntu:14.04' CONFIGURATION='--verbose' 
COMPILER='gcc' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker_build.sh

- Mesos ReviewBot


On May 18, 2016, 8:06 a.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47516/
> ---
> 
> (Updated May 18, 2016, 8:06 a.m.)
> 
> 
> Review request for mesos, Bernd Mathiske, Joerg Schad, and Neil Conway.
> 
> 
> Bugs: MESOS-5005
> https://issues.apache.org/jira/browse/MESOS-5005
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch adds a creator principal to the persistent
> volumes used in various tests.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/docker_containerizer_tests.cpp 
> f43165388f29513ab8be6594ab6647e8f9eb5a93 
>   src/tests/containerizer/filesystem_isolator_tests.cpp 
> 4293416ac8434e9eb7e80724480a54936a2fe24a 
>   src/tests/disk_quota_tests.cpp 1564f70854ff5630da1d7348a034f726d67b1757 
>   src/tests/reservation_tests.cpp 2d7fb21e2fe153c2b62dfd60bbaccb350a157391 
>   src/tests/role_tests.cpp 24959d6e0f83ef7b62b0586be18661aa3cac91dd 
> 
> Diff: https://reviews.apache.org/r/47516/diff/
> 
> 
> Testing
> ---
> 
> `make check` was used to test on OSX at the end of this review chain.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 47399: New update_quotas ACL for both set and remove cases.

[Alexander Rukletsov]

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


Fix it, then Ship it!




I'll fix the last issues and commit shortly.


include/mesos/authorizer/acls.proto (line 134)


s/operator/Operator



include/mesos/authorizer/acls.proto (lines 141 - 144)


- Blank line
- Wrap type into backticks
- Comment fits two line



include/mesos/authorizer/acls.proto (lines 153 - 156)


Ditto



include/mesos/authorizer/acls.proto (line 182)


Rebase artifact?



include/mesos/authorizer/acls.proto (line 220)


s/on/with



include/mesos/authorizer/authorizer.proto (lines 61 - 63)


```
  // TODO(zhitao): Remove the following two actions at the end of
  // the deprecation cycle started with 0.29. They will be fully
  // replaced by `UPDATE_QUOTA_WITH_ROLE`.
```



src/authorizer/local/authorizer.cpp (line 198)


Comma after `QuotaHandler`



src/authorizer/local/authorizer.cpp (lines 200 - 201)


// TODO(zhitao): Remove this special case at the end
// of the deprecation cycle which started with 0.29.



src/authorizer/local/authorizer.cpp (line 429)


s/valid/validationError



src/authorizer/local/authorizer.cpp (line 462)


We usually use `Option` for validation routines.



src/master/master.hpp (lines 1019 - 1021)


Move above after `authorizeGetQuota`.



src/master/quota_handler.cpp (lines 341 - 344)


We can say it shorter:
```
list authorizeRequests = {
authorizeSetQuota(principal, quotaInfo.role()),
authorizeUpdateQuota(principal, quotaInfo.role())};
```



src/master/quota_handler.cpp (line 348)


const &?



src/master/quota_handler.cpp (line 468)


const &?



src/tests/authorization_tests.cpp (line 1046)


I've discovered this is a general issue: MESOS-5405. I'm calling back my 
suggestion, let's remove this line for now, we need a proper fix instead of 
band aids.


- Alexander Rukletsov


On May 17, 2016, 9:11 p.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47399/
> ---
> 
> (Updated May 17, 2016, 9:11 p.m.)
> 
> 
> Review request for mesos, Adam B and Alexander Rukletsov.
> 
> 
> Bugs: MESOS-5155
> https://issues.apache.org/jira/browse/MESOS-5155
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> New update_quotas ACL for both set and remove cases.
> 
> 
> Diffs
> -
> 
>   include/mesos/authorizer/acls.proto 
> b170330b236b83611d8477c0e45e520ef69aed9b 
>   include/mesos/authorizer/authorizer.proto 
> 7d4aa32f42de538864508a0ba481d875917d9ab9 
>   src/authorizer/local/authorizer.hpp 
> d15d3a6c9f3d7c432e593cfe78cc48d672848d02 
>   src/authorizer/local/authorizer.cpp 
> bd47f4f02238e5052d9ab6caf338a2b51ddb752d 
>   src/master/master.hpp 22a1538b8c2ca85cb8316aafe3f9a4476df833fc 
>   src/master/quota_handler.cpp 86a2e175a3e1932961682c2a2d0cfe8210ee5fd0 
>   src/tests/authorization_tests.cpp f50ac69c32d0551a63391d97f31559ff4f414011 
>   src/tests/master_quota_tests.cpp 9bfa6ca58d5a92b857e8f2ce5cb835ddf18b44e6 
> 
> Diff: https://reviews.apache.org/r/47399/diff/
> 
> 
> Testing
> ---
> 
> 1. Unit tests;
> 2. Manually tested cases: authorized and forbidden under both deprecated 
> set_quotas/remove_quotas and new update_quotas, as well as the case that 
> specifying both triggers master crash.
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>

Re: Review Request 47518: Added test case for no volume and no checkpoint.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [46428, 36383, 47518]

Passed command: export OS='ubuntu:14.04' CONFIGURATION='--verbose' 
COMPILER='gcc' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker_build.sh

- Mesos ReviewBot


On May 18, 2016, 5:59 a.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47518/
> ---
> 
> (Updated May 18, 2016, 5:59 a.m.)
> 
> 
> Review request for mesos, Gilbert Song and Jie Yu.
> 
> 
> Bugs: MESOS-5266
> https://issues.apache.org/jira/browse/MESOS-5266
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added test case for no volume and no checkpoint.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/docker_volume_isolator_tests.cpp 
> c524f42743bf08ee54f1cbb083d0d3c85a8b70c9 
> 
> Diff: https://reviews.apache.org/r/47518/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> [ RUN  ] DockerVolumeIsolatorTest.ROOT_CommandTaskNoRootfsWithoutVolumes
> [   OK ] DockerVolumeIsolatorTest.ROOT_CommandTaskNoRootfsWithoutVolumes 
> (384 ms)
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 47515: Enforced a constraint on `DiskInfo.Persistence.principal`.

[Bernd Mathiske]

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




src/master/master.cpp (line 3750)


Although there does not seem to be a rule in the style guide for this, I'd 
suggest:
{code}
  Option principal = framework->info.has_principal() ? 
framework->info.principal() :
Option::none();
{code}



src/master/validation.cpp (line 823)


was -> has been

same below


- Bernd Mathiske


On May 18, 2016, 1:01 a.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47515/
> ---
> 
> (Updated May 18, 2016, 1:01 a.m.)
> 
> 
> Review request for mesos, Bernd Mathiske, Joerg Schad, and Neil Conway.
> 
> 
> Bugs: MESOS-5005
> https://issues.apache.org/jira/browse/MESOS-5005
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch enforces the constraint that the principal
> found in `DiskInfo.Persistence` should equal that of
> the framework or operator responsible for creating
> the volume.
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp c4ca343baa318fbb05ba2cbc3ed892c16478dbcc 
>   src/master/master.cpp b8c732a6178777544f0d09708177d9c68ab0532b 
>   src/master/validation.hpp f29f9753c75e5abc3402ed23381edcea26517ab3 
>   src/master/validation.cpp f490b899758bdac9676a6f6939918efa6ac52781 
> 
> Diff: https://reviews.apache.org/r/47515/diff/
> 
> 
> Testing
> ---
> 
> `make check` was used to test on OSX at the end of this review chain.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 46373: Request /files/read.json with a negative length value causes error.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [46373]

Passed command: export OS='ubuntu:14.04' CONFIGURATION='--verbose' 
COMPILER='gcc' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker_build.sh

- Mesos ReviewBot


On May 18, 2016, 4:13 a.m., zhou xing wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46373/
> ---
> 
> (Updated May 18, 2016, 4:13 a.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Greg Mann.
> 
> 
> Bugs: mesos-5060
> https://issues.apache.org/jira/browse/mesos-5060
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> [MESOS-5060]
> The patch did the following changes:
> 1. Fix the length logic in files.cpp.
> 2. Add some tests to test the /files/read.json endponit with
> negative length.
> 
> 
> Diffs
> -
> 
>   src/files/files.cpp e4b0ada00aabba6553810391f4015a896f8a69a5 
>   src/tests/files_tests.cpp 5d6620d13babaf0bb7f9c888bb1b4fa2228b6ccd 
> 
> Diff: https://reviews.apache.org/r/46373/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> request 'files/read.json' endpoint with negative offset or length argument
> 
> 
> Thanks,
> 
> zhou xing
> 
>

Re: Review Request 47530: Added authorization to agent's '/containers' endpoint.

[Alexander Rukletsov]

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




src/slave/http.cpp (line 788)


4 spaces indent, please


- Alexander Rukletsov


On May 18, 2016, 12:59 p.m., Abhishek Dasgupta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47530/
> ---
> 
> (Updated May 18, 2016, 12:59 p.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rukletsov, Greg Mann, Jan 
> Schlicht, and Till Toenshoff.
> 
> 
> Bugs: MESOS-5317
> https://issues.apache.org/jira/browse/MESOS-5317
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Used GET_ENDPOINT_WITH_PATH coarse-grained authz on agent's 
>  '/containers' endpoint to enable authorization on this endpoint.
>  Updated docs and testcases as well.
> 
> 
> Diffs
> -
> 
>   docs/endpoints/slave/containers.md 959f40b9db4de4b6cea456ecf7bcb402f7a94f05 
>   src/slave/http.cpp fb48ec61e2fe0c83f80d3b8aa4c2ef5a96b748ae 
>   src/slave/slave.hpp 209f071448e3c52d16d3366d564003ee36b1d2e0 
>   src/tests/slave_authorization_tests.cpp 
> 843cf1c631e0a25125ca1c0c0028ad1a920c2c2f 
> 
> Diff: https://reviews.apache.org/r/47530/diff/
> 
> 
> Testing
> ---
> 
> ## Unit tests.
> 
> On ubuntu 16.04:
> `sudo GTEST_FILTER="*SlaveEndpointTest*.*" make -j2 check`
> 
> ## Manual testing.
> 
> 1. Ran master with:
> ```
> sudo  ./bin/mesos-master.sh --ip=127.0.0.1 --work_dir=/var/lib/mesos
> ```
> 
> 2. ACL File: 
> ```
>   {
> "get_endpoints": [
>   {
> "principals": { "type": "NONE" },
> "paths": { "values": ["/flags", "/monitor/statistics", "/containers"] 
> }
>   }
> ]
>   } 
> ```
> 
> 3. Ran slave with: 
> ```
> sudo ./bin/mesos-slave.sh --master=127.0.0.1:5050 --ip=0.0.0.0 
> --acls=file:///home/abhishek/testAcl
> ```
> 
> 4. Ran toy-framework with: 
> ```
> sudo ./no-executor-framework --master=master@127.0.0.1:5050 --command="echo 
> hello"
> ```
> 
> 5. Output while hitting "http://127.0.0.1:5051/slave(1)/containers" - HTTP 
> error 403: Forbidden
> 
> 6. Changed ACL to: 
> ```
>   {
> "get_endpoints": [
>   {
> "principals": { "type": "ANY" },
> "paths": { "values": ["/flags", "/monitor/statistics", "/containers"] 
> }
>   }
> ]
>   }
> ```
> 
> 7. Ran slave and framework again.
> 
> 8. Output:
> ```
> 
> [{"container_id":"9b8a6a51-68be-4763-9c7d-b67e85fccb4a","executor_id":"42","executor_name":"Command
>  Executor (Task: 42) (Command: sh -c 'echo hello')","framework_id":"52...
> ```
> 
> 
> Thanks,
> 
> Abhishek Dasgupta
> 
>

Re: Review Request 47530: Added authorization to agent's '/containers' endpoint.

[Alexander Rukletsov]

> On May 18, 2016, 12:55 p.m., Jan Schlicht wrote:
> > src/slave/http.cpp, line 787
> > 
> >
> > Please call `authorizeEndpoint` as soon as possible, i.e. after the 
> > endpoint has been extracted from the URL.
> > 
> > While I like the idea of doing work in parallel, by requesting the 
> > containerizer statuses prior to the authorization, this work should only be 
> > done after the authorization was successful. Hence this part should be in 
> > the `_containers` continuation.

This will also allow us to avoid the tuple-induced mess in the header.


- Alexander


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


On May 18, 2016, 12:59 p.m., Abhishek Dasgupta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47530/
> ---
> 
> (Updated May 18, 2016, 12:59 p.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rukletsov, Greg Mann, Jan 
> Schlicht, and Till Toenshoff.
> 
> 
> Bugs: MESOS-5317
> https://issues.apache.org/jira/browse/MESOS-5317
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Used GET_ENDPOINT_WITH_PATH coarse-grained authz on agent's 
>  '/containers' endpoint to enable authorization on this endpoint.
>  Updated docs and testcases as well.
> 
> 
> Diffs
> -
> 
>   docs/endpoints/slave/containers.md 959f40b9db4de4b6cea456ecf7bcb402f7a94f05 
>   src/slave/http.cpp fb48ec61e2fe0c83f80d3b8aa4c2ef5a96b748ae 
>   src/slave/slave.hpp 209f071448e3c52d16d3366d564003ee36b1d2e0 
>   src/tests/slave_authorization_tests.cpp 
> 843cf1c631e0a25125ca1c0c0028ad1a920c2c2f 
> 
> Diff: https://reviews.apache.org/r/47530/diff/
> 
> 
> Testing
> ---
> 
> ## Unit tests.
> 
> On ubuntu 16.04:
> `sudo GTEST_FILTER="*SlaveEndpointTest*.*" make -j2 check`
> 
> ## Manual testing.
> 
> 1. Ran master with:
> ```
> sudo  ./bin/mesos-master.sh --ip=127.0.0.1 --work_dir=/var/lib/mesos
> ```
> 
> 2. ACL File: 
> ```
>   {
> "get_endpoints": [
>   {
> "principals": { "type": "NONE" },
> "paths": { "values": ["/flags", "/monitor/statistics", "/containers"] 
> }
>   }
> ]
>   } 
> ```
> 
> 3. Ran slave with: 
> ```
> sudo ./bin/mesos-slave.sh --master=127.0.0.1:5050 --ip=0.0.0.0 
> --acls=file:///home/abhishek/testAcl
> ```
> 
> 4. Ran toy-framework with: 
> ```
> sudo ./no-executor-framework --master=master@127.0.0.1:5050 --command="echo 
> hello"
> ```
> 
> 5. Output while hitting "http://127.0.0.1:5051/slave(1)/containers" - HTTP 
> error 403: Forbidden
> 
> 6. Changed ACL to: 
> ```
>   {
> "get_endpoints": [
>   {
> "principals": { "type": "ANY" },
> "paths": { "values": ["/flags", "/monitor/statistics", "/containers"] 
> }
>   }
> ]
>   }
> ```
> 
> 7. Ran slave and framework again.
> 
> 8. Output:
> ```
> 
> [{"container_id":"9b8a6a51-68be-4763-9c7d-b67e85fccb4a","executor_id":"42","executor_name":"Command
>  Executor (Task: 42) (Command: sh -c 'echo hello')","framework_id":"52...
> ```
> 
> 
> Thanks,
> 
> Abhishek Dasgupta
> 
>

Review Request 47536: Agent: Added Windows isolators.

[Alex Clemmer]

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

Review request for mesos, Daniel Pravat, Artem Harutyunyan, Joris Van 
Remoortere, and Michael Park.


Bugs: MESOS-3619 and MESOS-3620
https://issues.apache.org/jira/browse/MESOS-3619
https://issues.apache.org/jira/browse/MESOS-3620


Repository: mesos


Description
---

Agent: Added Windows isolators.


Diffs
-

  src/CMakeLists.txt e0c538d9e6542fbe279bfbf6f20172e4c611c859 
  src/Makefile.am 447bc2ab511ad173d3d911be10992be0974f4584 
  src/slave/containerizer/mesos/isolators/filesystem/posix.hpp 
c6cea98e16f2bdea2da0220c235468080bbcd17b 
  src/slave/containerizer/mesos/isolators/filesystem/posix.cpp 
01c0ad6dbb6d509e62e769365586b3d23dcb240d 
  src/slave/containerizer/mesos/isolators/filesystem/windows.hpp PRE-CREATION 
  src/slave/containerizer/mesos/isolators/filesystem/windows.cpp PRE-CREATION 
  src/slave/containerizer/mesos/isolators/posix.hpp 
227505a70a440f15e68ac001878bcf25610db45f 
  src/slave/containerizer/mesos/isolators/windows.hpp PRE-CREATION 
  src/usage/main.cpp 731acb69900b6fc2bb7bd19cccd78aafb0cc 
  src/usage/usage.hpp 2e1996a78c617c2559ef882ebede5c8aab6f899f 
  src/usage/usage.cpp 3b19682e67372b81484eacddbab78c2e5eda3c5b 

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


Testing
---


Thanks,

Alex Clemmer

Re: Review Request 47532: Updated release guide to recommend regenerating endpoint help.

[Alexander Rukletsov]

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


Ship it!




Ship It!

- Alexander Rukletsov


On May 18, 2016, 1:15 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47532/
> ---
> 
> (Updated May 18, 2016, 1:15 p.m.)
> 
> 
> Review request for mesos and Alexander Rukletsov.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated release guide to recommend regenerating endpoint help.
> 
> 
> Diffs
> -
> 
>   docs/release-guide.md d0f228f1cc234082df55285a691a1f048be20905 
> 
> Diff: https://reviews.apache.org/r/47532/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 47499: Make Mesos site responsive.

[Tomasz Janiszewski]

> On May 18, 2016, 3 a.m., haosdent huang wrote:
> > Hi, @janisz May you merge changes from 
> > https://reviews.apache.org/r/47510/diff/1#index_header . It looks better 
> > according my test.

I checked 47510 and indeed it looks better than this. I'll close this PR, in 
favour of 47510


- Tomasz


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


On May 18, 2016, 12:24 a.m., Tomasz Janiszewski wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47499/
> ---
> 
> (Updated May 18, 2016, 12:24 a.m.)
> 
> 
> Review request for mesos, Tim Anderegg and Vinod Kone.
> 
> 
> Bugs: MESOS-5402
> https://issues.apache.org/jira/browse/MESOS-5402
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch wrap lines on small screens,
> so the page is more friendly to view on
> mobile.
> 
> 
> Diffs
> -
> 
>   site/source/assets/css/main.css 73ca8946091a638a359b1d769961879a283fa099 
>   site/source/index.html.md 7b4bdaee61687f487423c1d90a674c78fdf002a4 
> 
> Diff: https://reviews.apache.org/r/47499/diff/
> 
> 
> Testing
> ---
> 
> Manually checked some random pages in different sizes.
> 
> * PC
> ![PC](https://s3-eu-west-1.amazonaws.com/uploads-eu.hipchat.com/48981/330344/pT87vto4CHMMaG2/old_pc.png)
> ![PC](https://s3-eu-west-1.amazonaws.com/uploads-eu.hipchat.com/48981/330344/idXPLIM8t0LxRWA/new_pc.png)
> * iPad
> ![iPad](https://s3-eu-west-1.amazonaws.com/uploads-eu.hipchat.com/48981/330344/Wt3g8bknstahXSC/old_iPad_landscape.png)
> ![iPad](https://s3-eu-west-1.amazonaws.com/uploads-eu.hipchat.com/48981/330344/TBTjlj1xT3YFrOf/new_iPad_landscape.png)
> ![iPad](https://s3-eu-west-1.amazonaws.com/uploads-eu.hipchat.com/48981/330344/sMct2b54lfCzrAu/old_iPad.png)
> ![iPad](https://s3-eu-west-1.amazonaws.com/uploads-eu.hipchat.com/48981/330344/qp2hj1UCNOfGLIX/new_iPad.png)
> * iPhone
> ![iPhone](https://s3-eu-west-1.amazonaws.com/uploads-eu.hipchat.com/48981/330344/mCTMOx8FLFxANeo/old_iPhone.png)
> ![iPad](https://s3-eu-west-1.amazonaws.com/uploads-eu.hipchat.com/48981/330344/5hlKDsOKqdJaztw/new_iPhone.png)
> 
> 
> Thanks,
> 
> Tomasz Janiszewski
> 
>

Re: Review Request 47491: Added `environment` field to `Task` protobuf message.

[Joerg Schad]

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

(Updated May 18, 2016, 1:18 p.m.)


Review request for mesos and Michael Park.


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


Repository: mesos


Description
---

An Authorizer might use environment variables for
authorization of tasks, so we add it to the \`Task\` protobuf
message. Note that the master stores \`Task\` (as opposed
to \`TaskInfo\`) for running and completed tasks.


Diffs
-

  include/mesos/mesos.proto 887ffe9c5f04e25539a4c6b3d52ce5299c65e8d3 
  src/common/protobuf_utils.cpp 4f4711d54c471922f1a103310d4d360e41a99870 

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


Testing (updated)
---

make check OS-X + (sudo) make check on a variety of Linux systems on internal 
CI.


Thanks,

Joerg Schad

Re: Review Request 47510: Adjusted style to make website mobile friendly.

[Tomasz Janiszewski]

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


Ship it!




Ship It!

- Tomasz Janiszewski


On May 18, 2016, 2:48 a.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47510/
> ---
> 
> (Updated May 18, 2016, 2:48 a.m.)
> 
> 
> Review request for mesos, Freddy Ayuso-Henson, Tomasz Janiszewski, Tim 
> Anderegg, and Vinod Kone.
> 
> 
> Bugs: MESOS-3690
> https://issues.apache.org/jira/browse/MESOS-3690
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Adjusted style to make website mobile friendly.
> 
> 
> Diffs
> -
> 
>   site/source/assets/css/main.css 73ca8946091a638a359b1d769961879a283fa099 
>   site/source/index.html.md 7b4bdaee61687f487423c1d90a674c78fdf002a4 
>   site/source/layouts/layout.erb 2bf6967faad45644647b732be0fa3c410b9951c5 
> 
> Diff: https://reviews.apache.org/r/47510/diff/
> 
> 
> Testing
> ---
> 
> I pick the necessary changes for mobile friendly from 
> https://github.com/apache/mesos/pull/75 .
> @vinodkone, this should credited to @fayusohenson when submit.
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Review Request 47532: Updated release guide to recommend regenerating endpoint help.

[Neil Conway]

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

Review request for mesos and Alexander Rukletsov.


Repository: mesos


Description
---

Updated release guide to recommend regenerating endpoint help.


Diffs
-

  docs/release-guide.md d0f228f1cc234082df55285a691a1f048be20905 

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


Testing
---


Thanks,

Neil Conway

Re: Review Request 47530: Added authorization to agent's '/containers' endpoint.

[Abhishek Dasgupta]

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

(Updated May 18, 2016, 12:59 p.m.)


Review request for mesos, Adam B, Alexander Rukletsov, Greg Mann, Jan Schlicht, 
and Till Toenshoff.


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


Repository: mesos


Description
---

Used GET_ENDPOINT_WITH_PATH coarse-grained authz on agent's 
 '/containers' endpoint to enable authorization on this endpoint.
 Updated docs and testcases as well.


Diffs
-

  docs/endpoints/slave/containers.md 959f40b9db4de4b6cea456ecf7bcb402f7a94f05 
  src/slave/http.cpp fb48ec61e2fe0c83f80d3b8aa4c2ef5a96b748ae 
  src/slave/slave.hpp 209f071448e3c52d16d3366d564003ee36b1d2e0 
  src/tests/slave_authorization_tests.cpp 
843cf1c631e0a25125ca1c0c0028ad1a920c2c2f 

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


Testing (updated)
---

## Unit tests.

On ubuntu 16.04:
`sudo GTEST_FILTER="*SlaveEndpointTest*.*" make -j2 check`

## Manual testing.

1. Ran master with:
```
sudo  ./bin/mesos-master.sh --ip=127.0.0.1 --work_dir=/var/lib/mesos
```

2. ACL File: 
```
  {
"get_endpoints": [
  {
"principals": { "type": "NONE" },
"paths": { "values": ["/flags", "/monitor/statistics", "/containers"] }
  }
]
  } 
```

3. Ran slave with: 
```
sudo ./bin/mesos-slave.sh --master=127.0.0.1:5050 --ip=0.0.0.0 
--acls=file:///home/abhishek/testAcl
```

4. Ran toy-framework with: 
```
sudo ./no-executor-framework --master=master@127.0.0.1:5050 --command="echo 
hello"
```

5. Output while hitting "http://127.0.0.1:5051/slave(1)/containers" - HTTP 
error 403: Forbidden

6. Changed ACL to: 
```
  {
"get_endpoints": [
  {
"principals": { "type": "ANY" },
"paths": { "values": ["/flags", "/monitor/statistics", "/containers"] }
  }
]
  }
```

7. Ran slave and framework again.

8. Output:
```

[{"container_id":"9b8a6a51-68be-4763-9c7d-b67e85fccb4a","executor_id":"42","executor_name":"Command
 Executor (Task: 42) (Command: sh -c 'echo hello')","framework_id":"52...
```


Thanks,

Abhishek Dasgupta

Re: Review Request 47530: Added authorization to agent's '/containers' endpoint.

[Jan Schlicht]

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




src/slave/http.cpp (line 756)


Please make sure that `request.method` is "GET" when this function is 
called with enabled authorization. See `Slave::Http::flags` for an example.

Currently, for many existing endpoints, the request method isn't checked 
which can lead to problems with authorization. We plan to change that later, 
see [MESOS-5346](https://issues.apache.org/jira/browse/MESOS-5346).



src/slave/http.cpp (line 786)


Please call `authorizeEndpoint` as soon as possible, i.e. after the 
endpoint has been extracted from the URL.

While I like the idea of doing work in parallel, by requesting the 
containerizer statuses prior to the authorization, this work should only be 
done after the authorization was successful. Hence this part should be in the 
`_containers` continuation.



src/slave/slave.hpp (lines 96 - 97)


Please don't do this in a header. The `using namespace process;` above is a 
bad example and probably shouldn't even be there.


- Jan Schlicht


On May 18, 2016, 12:06 p.m., Abhishek Dasgupta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47530/
> ---
> 
> (Updated May 18, 2016, 12:06 p.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rukletsov, Greg Mann, Jan 
> Schlicht, and Till Toenshoff.
> 
> 
> Bugs: MESOS-5317
> https://issues.apache.org/jira/browse/MESOS-5317
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Used GET_ENDPOINT_WITH_PATH coarse-grained authz on agent's 
>  '/containers' endpoint to enable authorization on this endpoint.
>  Updated docs and testcases as well.
> 
> 
> Diffs
> -
> 
>   docs/endpoints/slave/containers.md 959f40b9db4de4b6cea456ecf7bcb402f7a94f05 
>   src/slave/http.cpp fb48ec61e2fe0c83f80d3b8aa4c2ef5a96b748ae 
>   src/slave/slave.hpp 209f071448e3c52d16d3366d564003ee36b1d2e0 
>   src/tests/slave_authorization_tests.cpp 
> 843cf1c631e0a25125ca1c0c0028ad1a920c2c2f 
> 
> Diff: https://reviews.apache.org/r/47530/diff/
> 
> 
> Testing
> ---
> 
> sudo GTEST_FILTER="*SlaveEndpointTest*.*" make -j2 check
> 
> On ubuntu 16.04.
> 
> Ran manual testing as well.
> 
> 
> Thanks,
> 
> Abhishek Dasgupta
> 
>

Re: Review Request 47499: Make Mesos site responsive.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [47499]

Passed command: export OS='ubuntu:14.04' CONFIGURATION='--verbose' 
COMPILER='gcc' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker_build.sh

- Mesos ReviewBot


On May 18, 2016, 12:24 a.m., Tomasz Janiszewski wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47499/
> ---
> 
> (Updated May 18, 2016, 12:24 a.m.)
> 
> 
> Review request for mesos, Tim Anderegg and Vinod Kone.
> 
> 
> Bugs: MESOS-5402
> https://issues.apache.org/jira/browse/MESOS-5402
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch wrap lines on small screens,
> so the page is more friendly to view on
> mobile.
> 
> 
> Diffs
> -
> 
>   site/source/assets/css/main.css 73ca8946091a638a359b1d769961879a283fa099 
>   site/source/index.html.md 7b4bdaee61687f487423c1d90a674c78fdf002a4 
> 
> Diff: https://reviews.apache.org/r/47499/diff/
> 
> 
> Testing
> ---
> 
> Manually checked some random pages in different sizes.
> 
> * PC
> ![PC](https://s3-eu-west-1.amazonaws.com/uploads-eu.hipchat.com/48981/330344/pT87vto4CHMMaG2/old_pc.png)
> ![PC](https://s3-eu-west-1.amazonaws.com/uploads-eu.hipchat.com/48981/330344/idXPLIM8t0LxRWA/new_pc.png)
> * iPad
> ![iPad](https://s3-eu-west-1.amazonaws.com/uploads-eu.hipchat.com/48981/330344/Wt3g8bknstahXSC/old_iPad_landscape.png)
> ![iPad](https://s3-eu-west-1.amazonaws.com/uploads-eu.hipchat.com/48981/330344/TBTjlj1xT3YFrOf/new_iPad_landscape.png)
> ![iPad](https://s3-eu-west-1.amazonaws.com/uploads-eu.hipchat.com/48981/330344/sMct2b54lfCzrAu/old_iPad.png)
> ![iPad](https://s3-eu-west-1.amazonaws.com/uploads-eu.hipchat.com/48981/330344/qp2hj1UCNOfGLIX/new_iPad.png)
> * iPhone
> ![iPhone](https://s3-eu-west-1.amazonaws.com/uploads-eu.hipchat.com/48981/330344/mCTMOx8FLFxANeo/old_iPhone.png)
> ![iPad](https://s3-eu-west-1.amazonaws.com/uploads-eu.hipchat.com/48981/330344/5hlKDsOKqdJaztw/new_iPhone.png)
> 
> 
> Thanks,
> 
> Tomasz Janiszewski
> 
>

Re: Review Request 47400: Documentation quota authorization changes in 0.29.

[Alexander Rukletsov]

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



I've committed https://reviews.apache.org/r/46501/ , could you please update 
this review?

- Alexander Rukletsov


On May 17, 2016, 7:15 p.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47400/
> ---
> 
> (Updated May 17, 2016, 7:15 p.m.)
> 
> 
> Review request for mesos, Adam B and Alexander Rukletsov.
> 
> 
> Bugs: MESOS-5155 and MESOS-5336
> https://issues.apache.org/jira/browse/MESOS-5155
> https://issues.apache.org/jira/browse/MESOS-5336
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Documentation quota authorization changes in 0.29.
> 
> 
> Diffs
> -
> 
>   CHANGELOG 825366d771f3b1bf26cec2ec1cb87619e82f9c8f 
>   docs/authorization.md 0db5c345b3239814b3b9d2e8a87601ff69d0f869 
>   docs/quota.md 797e134605381ae576d9aa93875e0314889ab047 
>   docs/upgrades.md 4f0c86db4c8d51f70487c03b2e75c1a4189b67b9 
> 
> Diff: https://reviews.apache.org/r/47400/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>

Re: Review Request 46501: Updated authorization.md to reflect current changes.

[Alexander Rukletsov]

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


Fix it, then Ship it!




I've wrapped the text and did a final pass. I'll fix the outstanding issues and 
commit this for you shortly.


docs/authorization.md (line 10)


s/, but/.



docs/authorization.md (line 12)


s/but to programmers/but at engineers



docs/authorization.md (line 18)


The analogy is not precise, killing that extra.



docs/authorization.md (line 24)


* The master consults the local authorizer, which in turn...

s/, so the framework/, the framework



docs/authorization.md (line 36)


s/no ACL apply/no ACL applies



docs/authorization.md (lines 46 - 57)


According to 
https://mesos.apache.org/documentation/latest/markdown-style-guide/ , we should 
use HTML tables. Could you please follow-up with the fix? Feel free to publish 
a review without a JIRA ticket. Thanks!



docs/authorization.md (line 57)


We should add an entry for `get_endpoints`



docs/authorization.md (lines 486 - 500)


It's hard to remember that this doc should be updated if changes to the 
proto are made. Can we reference the code instead? Could you please follow up 
with a fix?



docs/authorization.md (line 502)


Remove trailing space.


- Alexander Rukletsov


On May 17, 2016, 1:11 p.m., Alexander Rojas wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46501/
> ---
> 
> (Updated May 17, 2016, 1:11 p.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rukletsov, Greg Mann, and Neil 
> Conway.
> 
> 
> Bugs: MESOS-4785
> https://issues.apache.org/jira/browse/MESOS-4785
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The API of the authorization has been changing constantly over the
> last few versions. This patch attempts to update the documentation to
> the those changes into account.
> 
> 
> Diffs
> -
> 
>   docs/authorization.md 0db5c345b3239814b3b9d2e8a87601ff69d0f869 
> 
> Diff: https://reviews.apache.org/r/46501/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Alexander Rojas
> 
>

Review Request 47530: Added authorization to agent's '/containers' endpoint.

[Abhishek Dasgupta]

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

Review request for mesos, Adam B, Alexander Rukletsov, Greg Mann, Jan Schlicht, 
and Till Toenshoff.


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


Repository: mesos


Description
---

Used GET_ENDPOINT_WITH_PATH coarse-grained authz on agent's 
 '/containers' endpoint to enable authorization on this endpoint.
 Updated docs and testcases as well.


Diffs
-

  docs/endpoints/slave/containers.md 959f40b9db4de4b6cea456ecf7bcb402f7a94f05 
  src/slave/http.cpp fb48ec61e2fe0c83f80d3b8aa4c2ef5a96b748ae 
  src/slave/slave.hpp 209f071448e3c52d16d3366d564003ee36b1d2e0 
  src/tests/slave_authorization_tests.cpp 
843cf1c631e0a25125ca1c0c0028ad1a920c2c2f 

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


Testing
---

sudo GTEST_FILTER="*SlaveEndpointTest*.*" make -j2 check

On ubuntu 16.04.

Ran manual testing as well.


Thanks,

Abhishek Dasgupta

Review Request 47511: Added documentation for `docker/volume` isolator.

[Guangya Liu]

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

Review request for mesos, Gilbert Song and Jie Yu.


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


Repository: mesos


Description
---

Added documentation for `docker/volume` isolator.


Diffs
-

  docs/docker-volume-isolator.md PRE-CREATION 
  docs/images/docker-volume-isolator.png PRE-CREATION 

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


Testing
---

You can review the document here: 
https://github.com/jay-lau/mesos/blob/master/docs/docker-volume-isolator.md


Thanks,

Guangya Liu

Re: Review Request 47505: Updated authorizer.proto Subject, Object and Request.

[Alexander Rukletsov]

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



Also you would need to update "authorization.md". However, it looks strange we 
paste the proto definition there, since it's hard to track updates.

- Alexander Rukletsov


On May 18, 2016, 12:35 a.m., Till Toenshoff wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47505/
> ---
> 
> (Updated May 18, 2016, 12:35 a.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rukletsov, Alexander Rojas, and 
> Vinod Kone.
> 
> 
> Bugs: MESOS-5405
> https://issues.apache.org/jira/browse/MESOS-5405
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Makes `value` required for clearity, to make sure that we do not
> introduce two ways of expressing `ANY`.
> 
> 
> Diffs
> -
> 
>   include/mesos/authorizer/authorizer.proto 
> 7d4aa32f42de538864508a0ba481d875917d9ab9 
> 
> Diff: https://reviews.apache.org/r/47505/diff/
> 
> 
> Testing
> ---
> 
> make check (OSX)
> 
> 
> Thanks,
> 
> Till Toenshoff
> 
>

Re: Review Request 47510: Adjusted style to make website mobile friendly.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [47510]

Passed command: export OS='ubuntu:14.04' CONFIGURATION='--verbose' 
COMPILER='gcc' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker_build.sh

- Mesos ReviewBot


On May 18, 2016, 2:48 a.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47510/
> ---
> 
> (Updated May 18, 2016, 2:48 a.m.)
> 
> 
> Review request for mesos, Freddy Ayuso-Henson, Tomasz Janiszewski, Tim 
> Anderegg, and Vinod Kone.
> 
> 
> Bugs: MESOS-3690
> https://issues.apache.org/jira/browse/MESOS-3690
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Adjusted style to make website mobile friendly.
> 
> 
> Diffs
> -
> 
>   site/source/assets/css/main.css 73ca8946091a638a359b1d769961879a283fa099 
>   site/source/index.html.md 7b4bdaee61687f487423c1d90a674c78fdf002a4 
>   site/source/layouts/layout.erb 2bf6967faad45644647b732be0fa3c410b9951c5 
> 
> Diff: https://reviews.apache.org/r/47510/diff/
> 
> 
> Testing
> ---
> 
> I pick the necessary changes for mobile friendly from 
> https://github.com/apache/mesos/pull/75 .
> @vinodkone, this should credited to @fayusohenson when submit.
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 47505: Updated authorizer.proto Subject, Object and Request.

[Alexander Rukletsov]

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



Additionally, you should also update the local authorizer implementation, 
specifically
```
  Future authorized(
  const authorization::Request& request,
  const vector& acls)
```
Currently, we do not call `has_*()`, meaning we use default instances, which do 
the job in our case, but explicit calls to `has_*()` are cleaner, especially if 
we are going to upgrade the messages in the future.


include/mesos/authorizer/authorizer.proto (line 28)


As per comment on MESOS-5405, I think it is fine that we have two ways of 
expressing {{ANY}}. We are doing it already in our code, illegally ; ). Please 
change it back to optional.

Also, please return the comment back, with a minor change: 
s/unknown/unspecified.



include/mesos/authorizer/authorizer.proto (line 38)


Ditto.



include/mesos/authorizer/authorizer.proto (line 67)


s/unknown/unspecified


- Alexander Rukletsov


On May 18, 2016, 12:35 a.m., Till Toenshoff wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47505/
> ---
> 
> (Updated May 18, 2016, 12:35 a.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rukletsov, Alexander Rojas, and 
> Vinod Kone.
> 
> 
> Bugs: MESOS-5405
> https://issues.apache.org/jira/browse/MESOS-5405
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Makes `value` required for clearity, to make sure that we do not
> introduce two ways of expressing `ANY`.
> 
> 
> Diffs
> -
> 
>   include/mesos/authorizer/authorizer.proto 
> 7d4aa32f42de538864508a0ba481d875917d9ab9 
> 
> Diff: https://reviews.apache.org/r/47505/diff/
> 
> 
> Testing
> ---
> 
> make check (OSX)
> 
> 
> Thanks,
> 
> Till Toenshoff
> 
>

Re: Review Request 46761: Slave/Agent Rename Phase I - Update terms in WebUI.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [46761]

Passed command: export OS='ubuntu:14.04' CONFIGURATION='--verbose' 
COMPILER='gcc' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker_build.sh

- Mesos ReviewBot


On May 18, 2016, 2:16 a.m., zhou xing wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46761/
> ---
> 
> (Updated May 18, 2016, 2:16 a.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Kevin Klues, Maxim Khutornenko, 
> and Vinod Kone.
> 
> 
> Bugs: mesos-3779
> https://issues.apache.org/jira/browse/mesos-3779
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch did the following changes in WebUI:
> 1. Slave/Agent Rename on web page
> 2. Slave/Agent Rename in web url
> 3. Slave/Agent Rename in JS class/method/attribute
> 4. Slave/Agent Rename in comments
> 
> For the JSON data returned from server that contains
> term 'slave', the change is not included.
> 
> 
> Diffs
> -
> 
>   CHANGELOG 6bd59d191a8ca59436fa8a1953587119aeb3e256 
>   src/Makefile.am ce5245883f3d2661812272702c0d2060513b6d88 
>   src/webui/master/static/browse.html 
> 04a4600f0c762a2412ddee078ba2c173d595aa8d 
>   src/webui/master/static/framework.html 
> 041513b0e005e8b54ca9723741b21b136ff61ca6 
>   src/webui/master/static/home.html 4b201d72f9dfd787133008b8105a225ffb2747aa 
>   src/webui/master/static/index.html ec2f5792d21bf7efb479e87be3812b06bfbe98dc 
>   src/webui/master/static/js/app.js 543fe9efb9618b311c7f21b7771a251738b01a91 
>   src/webui/master/static/js/controllers.js 
> 8d9021cc89e54ae3a4151ff7f399733f5a7376dd 
>   src/webui/master/static/js/services.js 
> fa5cc35c1ef0e8ec149ed88852837058ec6ab13c 
>   src/webui/master/static/offers.html 
> ec32a649239da48270a1ad1d5bf195326c31ff9d 
>   src/webui/master/static/slave.html c908511df85141128599ad5edc40d4b567437822 
>   src/webui/master/static/slave_executor.html 
> 99b23ed9e85011a66bad780fb2d3076e946821a6 
>   src/webui/master/static/slave_framework.html 
> 176e7e9fa7878f31268bd5aa06dfc8789f3e7edd 
>   src/webui/master/static/slaves.html 
> 063031771cef8b9f45723869198bad3460591936 
> 
> Diff: https://reviews.apache.org/r/46761/diff/
> 
> 
> Testing
> ---
> 
> cd mesos
> ./bootstrap
> mkdir build
> cd build
> ../configure --prefix=${HOME}/install/mesos
> make
> make check
> make install
> 
> start mesos master/agent and run a sample framework, then open browser and 
> goto : to check all the strings on web page and url
> 
> Tested the following slave relevant urls in browser, and it redirect to 
> corresponding agent urls:
> 
>   '/slaves', redirectTo: '/agents'
> 
>   '/slaves/:agent_id', redirectTo: '/agents/:agent_id'
> 
>   '/slaves/:agent_id/frameworks/:framework_id', redirectTo: 
> '/agents/:agent_id/frameworks/:framework_id'
> 
>   '/slaves/:agent_id/frameworks/:framework_id/executors/:executor_id', 
> redirectTo: 
> '/agents/:agent_id/frameworks/:framework_id/executors/:executor_id'
> 
>   '/slaves/:agent_id/frameworks/:framework_id/executors/:executor_id/browse', 
> redirectTo: 
> '/agents/:agent_id/frameworks/:framework_id/executors/:executor_id/browse'
> 
>   '/slaves/:agent_id/browse', redirectTo: '/agents/:agent_id/browse'
> 
> 
> Thanks,
> 
> zhou xing
> 
>

Review Request 47516: Added creator principal to tests.

[Greg Mann]

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

Review request for mesos, Bernd Mathiske, Joerg Schad, and Neil Conway.


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


Repository: mesos


Description
---

This patch adds a creator principal to the persistent
volumes used in various tests.


Diffs
-

  src/tests/containerizer/docker_containerizer_tests.cpp 
f43165388f29513ab8be6594ab6647e8f9eb5a93 
  src/tests/containerizer/filesystem_isolator_tests.cpp 
4293416ac8434e9eb7e80724480a54936a2fe24a 
  src/tests/disk_quota_tests.cpp 1564f70854ff5630da1d7348a034f726d67b1757 
  src/tests/reservation_tests.cpp 2d7fb21e2fe153c2b62dfd60bbaccb350a157391 
  src/tests/role_tests.cpp 24959d6e0f83ef7b62b0586be18661aa3cac91dd 

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


Testing
---

`make check` was used to test on OSX at the end of this review chain.


Thanks,

Greg Mann

Re: Review Request 47509: Fixed authorization::Request initializings.

[Alexander Rukletsov]

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



As per comment on MESOS-5405, I'd rather go the other path, r/47505/

- Alexander Rukletsov


On May 18, 2016, 3:10 a.m., Till Toenshoff wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47509/
> ---
> 
> (Updated May 18, 2016, 3:10 a.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rukletsov, Alexander Rojas, and 
> Vinod Kone.
> 
> 
> Bugs: MESOS-5405
> https://issues.apache.org/jira/browse/MESOS-5405
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixes a number of a authorization::Request initializings that were
> missing required fields. Also adds a CHECK to the LocalAuthorizer
> helping us to catch and prevent these problems in the future.
> 
> 
> Diffs
> -
> 
>   src/authorizer/local/authorizer.cpp 
> e95435327bb3b6f447e814b8657bce8084535346 
>   src/common/http.cpp ad6a4b44af3ec847a3a6c0839a88fba18cda5011 
>   src/master/http.cpp 6cefcc8b6ddcfe4098521e838bfdc95936fe7476 
>   src/master/master.cpp 35b428b0f7dee5954514d8860cfc498271ccf267 
>   src/master/quota_handler.cpp 6590519d8440f352f5bf00fda805414a5aad725c 
>   src/master/weights_handler.cpp 4bc060fdb015df6658194eef92fe11b14aa15c79 
>   src/slave/http.cpp 0fb9b81c4a87250e454e3380c61cf9037454810b 
>   src/tests/authorization_tests.cpp f50ac69c32d0551a63391d97f31559ff4f414011 
> 
> Diff: https://reviews.apache.org/r/47509/diff/
> 
> 
> Testing
> ---
> 
> make check (OSX)
> 
> 
> Thanks,
> 
> Till Toenshoff
> 
>

Review Request 47522: Added creator principal to persistent volume tests.

[Greg Mann]

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

Review request for mesos, Bernd Mathiske, Joerg Schad, and Neil Conway.


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


Repository: mesos


Description
---

This patch adds a creator principal to the
persistent volumes used in the PersistentVolumeTests.


Diffs
-

  src/tests/persistent_volume_tests.cpp 
d246f35046fff469b847c908de2b305ae629212f 

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


Testing
---

`make check` was used to test on OSX at the end of this review chain.


Thanks,

Greg Mann

Review Request 47521: Added creator principal to '/create-volumes' tests.

[Greg Mann]

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

Review request for mesos, Bernd Mathiske, Joerg Schad, and Neil Conway.


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


Repository: mesos


Description
---

A creator principal is added to the persistent volumes
used in the PersistentVolumeEndpointsTests.


Diffs
-

  src/tests/persistent_volume_endpoints_tests.cpp 
a57461d881b2bf0175f83b50b0a46167acd5bd3e 

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


Testing
---

`make check` was used to test on OSX.


Thanks,

Greg Mann

Review Request 47520: Updated test helpers with creator principal.

[Greg Mann]

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

Review request for mesos, Bernd Mathiske, Joerg Schad, and Neil Conway.


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


Repository: mesos


Description
---

This patch updates the `createPersistentVolume` and
`createDiskInfo` test helper functions to accept an
argument specifying the creator principal to be included
in `DiskInfo.Persistence`.


Diffs
-

  src/tests/mesos.hpp 79bf1ff16412ce2a510a9b75ab1ac91c1c182653 

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


Testing
---

`make check` was done on OSX at the end of this review chain.


Thanks,

Greg Mann

Review Request 47519: Updated an example framework to specify its principal.

[Greg Mann]

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

Review request for mesos, Bernd Mathiske, Joerg Schad, and Neil Conway.


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


Repository: mesos


Description
---

This patch adds the creator principal into the
persistent volumes created by the persistent volume
example framework.


Diffs
-

  src/examples/persistent_volume_framework.cpp 
53a6f6f82101ebb75abdc8f586fb23ab13298979 

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


Testing
---

Tested manually with the following commands:
`bin/mesos-master.sh --ip=127.0.0.1 --work_dir="$HOME/var/mesos1" 
--authenticate=false`
`bin/mesos-slave.sh --master=127.0.0.1:5050 --work_dir="$HOME/var/mesos2" 
--"resources=disk(test):2048;mem(test):2048;cpu(test):4"`
`src/persistent-volume-framework --master=127.0.0.1:5050`


Thanks,

Greg Mann

Review Request 47528: Updated validation tests with creator principal.

[Greg Mann]

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

Review request for mesos, Bernd Mathiske, Joerg Schad, and Neil Conway.


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


Repository: mesos


Description
---

The master validation tests are updated to include the
new `validate()` signature, and a new test is added,
`CreateOperationValidationTest.NonMatchingPrincipal`,
to ensure that a non-matching creator principal will
be invalidated.


Diffs
-

  src/tests/master_validation_tests.cpp 
ca4442aa1ef0087a7d058d1b3aa430a1dbc16960 

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


Testing
---

`make check` was used to test on OSX at the end of this review chain.


Thanks,

Greg Mann

Review Request 47515: Enforced a constraint on `DiskInfo.Persistence.principal`.

[Greg Mann]

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

Review request for mesos, Bernd Mathiske, Joerg Schad, and Neil Conway.


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


Repository: mesos


Description
---

This patch enforces the constraint that the principal
found in `DiskInfo.Persistence` should equal that of
the framework or operator responsible for creating
the volume.


Diffs
-

  src/master/http.cpp c4ca343baa318fbb05ba2cbc3ed892c16478dbcc 
  src/master/master.cpp b8c732a6178777544f0d09708177d9c68ab0532b 
  src/master/validation.hpp f29f9753c75e5abc3402ed23381edcea26517ab3 
  src/master/validation.cpp f490b899758bdac9676a6f6939918efa6ac52781 

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


Testing
---

`make check` was used to test on OSX at the end of this review chain.


Thanks,

Greg Mann

Re: Review Request 45474: MESOS-1739: Allow slave reconfiguration on restart, Phase 1.

[Deshi Xiao]

> On 三月 30, 2016, 9:08 a.m., Adam B wrote:
> > src/tests/slave_tests.cpp, lines 3541-3545
> > 
> >
> > Here you shutdown the slave and wait (you'll probably want to advance 
> > the clock rather than wait for 90s) for the slave to be declared 
> > SLAVE_LOST. Once this occurs, the master will no longer allow the slave to 
> > reregister with the same slaveId, and the slave will be told to kill all 
> > running tasks. The slave will do so and then restart and register as a new 
> > slaveId. 
> > This is what is meant by the quote from the design doc: "Currently this 
> > can only be handled by stopping / draining a mesos slave entirely (Killing 
> > all of its running jobs), removing it from the cluster, then bringing it 
> > back up as a brand new slave."
> > 
> > To truly observe this behavior, you should start a task on the slave 
> > before you shut it down. Then you will see a TASK_LOST and the task will be 
> > killed.
> 
> Deshi Xiao wrote:
> Thanks Adam, i will udpate the test case.

@Adam B
Here i have a confuse,need your guide. use test case to track the TASK_LOST in 
restart slave. do we expect keep the slave_id is not outdate?


- Deshi


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


On 三月 30, 2016, 8:13 a.m., Deshi Xiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45474/
> ---
> 
> (Updated 三月 30, 2016, 8:13 a.m.)
> 
> 
> Review request for mesos, Adam B, Greg Mann, haosdent huang, and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-1739
> https://issues.apache.org/jira/browse/MESOS-1739
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Phase 1
> Make SlaveInfo mutable throughout the stack, and allow for expansion of 
> resources and attributes only (Which allows testing to make sure it 
> propagates to the allocator, shows up in offers, etc). Ensure there is 
> unified checking for incompatibilities in both the slave and master (the 
> slave should validate the config, the master should validate that all 
> operations the slave takes are legal).
> 
> it derived from another PR(https://reviews.apache.org/r/25525/)
> 
> 
> Diffs
> -
> 
>   src/tests/slave_tests.cpp 1f1a31020096efa5db698e86ac74e61dfdb4b94a 
> 
> Diff: https://reviews.apache.org/r/45474/diff/
> 
> 
> Testing
> ---
> 
> make check on localhost
> 
> 
> Thanks,
> 
> Deshi Xiao
> 
>

Re: Review Request 47505: Updated authorizer.proto Subject, Object and Request.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [47505]

Passed command: export OS='ubuntu:14.04' CONFIGURATION='--verbose' 
COMPILER='gcc' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker_build.sh

- Mesos ReviewBot


On May 18, 2016, 12:35 a.m., Till Toenshoff wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47505/
> ---
> 
> (Updated May 18, 2016, 12:35 a.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rukletsov, Alexander Rojas, and 
> Vinod Kone.
> 
> 
> Bugs: MESOS-5405
> https://issues.apache.org/jira/browse/MESOS-5405
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Makes `value` required for clearity, to make sure that we do not
> introduce two ways of expressing `ANY`.
> 
> 
> Diffs
> -
> 
>   include/mesos/authorizer/authorizer.proto 
> 7d4aa32f42de538864508a0ba481d875917d9ab9 
> 
> Diff: https://reviews.apache.org/r/47505/diff/
> 
> 
> Testing
> ---
> 
> make check (OSX)
> 
> 
> Thanks,
> 
> Till Toenshoff
> 
>