Re: Review Request 58137: CLI: Added 'mesos config show' command to display the config file.

[Mesos Reviewbot]

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



Patch looks great!

Reviews applied: [57896, 57951, 58381, 57952, 58137]

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

- Mesos Reviewbot


On May 4, 2017, 9:39 p.m., Armand Grillet wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58137/
> ---
> 
> (Updated May 4, 2017, 9:39 p.m.)
> 
> 
> Review request for mesos, Joseph Wu and Kevin Klues.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This command displays the contents of the user-defined config.toml file.
> 
> 
> Diffs
> -
> 
>   src/cli_new/lib/cli/plugins/config/main.py 
> d95a36f4a66c66b4477c6816b7fa5a721f9212f7 
> 
> 
> Diff: https://reviews.apache.org/r/58137/diff/7/
> 
> 
> Testing
> ---
> 
> Tested manually, PEP8 and Pylint used to make sure that the code style is 
> correct.
> 
> 
> Thanks,
> 
> Armand Grillet
> 
>

Re: Review Request 57816: Add a scheduler flag `suppress_offers_on_registration`.

[Anindya Sinha]

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

(Updated May 5, 2017, 2:21 a.m.)


Review request for mesos, James Peach, Vinod Kone, and Jiang Yan Xu.


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


Repository: mesos


Description
---

If the framework sets this flag to indicate that it wants offers
suppressed on successful registration to master, we pass this
information to the master in the `SUBSCRIBE` call.


Diffs
-

  src/sched/flags.hpp d19d20bad7dba48c8792783c73115affa1430cbe 
  src/sched/sched.cpp ef73c1dccfd736b79f40a057951f022df7f60644 


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


Testing
---

All tests passed.


Thanks,

Anindya Sinha

Re: Review Request 58998: Fixed extra newline in webui for re-registered frameworks.

[haosdent huang]

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




src/webui/master/static/framework.html
Lines 40-41 (original), 40-41 (patched)


I think change `ng-show` to `ng-if` should be enough.

```
-

...
```


- haosdent huang


On May 4, 2017, 7:04 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58998/
> ---
> 
> (Updated May 4, 2017, 7:04 p.m.)
> 
> 
> Review request for mesos and haosdent huang.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed extra newline in webui for re-registered frameworks.
> 
> 
> Diffs
> -
> 
>   src/webui/master/static/framework.html 
> 79dd1a7fa8c41c1e09141c273cc5d2e261e578b3 
> 
> 
> Diff: https://reviews.apache.org/r/58998/diff/1/
> 
> 
> Testing
> ---
> 
> Visual inspection.
> 
> Screenshot before: http://www.neilconway.org/tmp/screenshot_before.png
> 
> Screenshot after: http://www.neilconway.org/tmp/screenshot_after.png
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 58998: Fixed extra newline in webui for re-registered frameworks.

[Mesos Reviewbot]

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



Patch looks great!

Reviews applied: [58998]

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

- Mesos Reviewbot


On May 4, 2017, 7:04 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58998/
> ---
> 
> (Updated May 4, 2017, 7:04 p.m.)
> 
> 
> Review request for mesos and haosdent huang.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed extra newline in webui for re-registered frameworks.
> 
> 
> Diffs
> -
> 
>   src/webui/master/static/framework.html 
> 79dd1a7fa8c41c1e09141c273cc5d2e261e578b3 
> 
> 
> Diff: https://reviews.apache.org/r/58998/diff/1/
> 
> 
> Testing
> ---
> 
> Visual inspection.
> 
> Screenshot before: http://www.neilconway.org/tmp/screenshot_before.png
> 
> Screenshot after: http://www.neilconway.org/tmp/screenshot_after.png
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 57817: Offers not sent for deactivated roles as indicated in `SUBSCRIBE`.

[Anindya Sinha]

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

(Updated May 5, 2017, 12:34 a.m.)


Review request for mesos, James Peach, Vinod Kone, and Jiang Yan Xu.


Changes
---

Modifications based on discussion in Slack.


Summary (updated)
-

Offers not sent for deactivated roles as indicated in `SUBSCRIBE`.


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


Repository: mesos


Description (updated)
---

If the framework indicates a subset of roles to be deactivated, the
allocator does not offer resources for those roles to such frameworks.
In addition, the master validates the deactivated roles to be a
subset of all the roles being (re)registered by the framework.


Diffs (updated)
-

  src/common/protobuf_utils.hpp be2325f05b81b847fa592eff65175cbc99764fd6 
  src/common/protobuf_utils.cpp 3fcaf786b29a00f003c10b0f1614a2c7eddc725d 
  src/master/allocator/mesos/hierarchical.hpp 
123f97cf495bff0f822838e09df0d88818f04da6 
  src/master/allocator/mesos/hierarchical.cpp 
b75ed9a20a9a42f958cebbacd91e5e15b0d21394 
  src/master/master.cpp 4e7a161f431624bd78d3e9032eb8587687149cad 
  src/tests/hierarchical_allocator_tests.cpp 
ebc4868a6b7e2a04cc202a74a4633969ade40aca 


Diff: https://reviews.apache.org/r/57817/diff/6/

Changes: https://reviews.apache.org/r/57817/diff/5-6/


Testing
---

All tests passed.


Thanks,

Anindya Sinha

Re: Review Request 57818: Added unit tests to verify offers are suppressed based on registration.

[Anindya Sinha]

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

(Updated May 5, 2017, 12:34 a.m.)


Review request for mesos, James Peach, Vinod Kone, and Jiang Yan Xu.


Changes
---

Modifications based on discussion in Slack.


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


Repository: mesos


Description
---

Added unit tests to verify offers are suppressed based on registration.


Diffs (updated)
-

  src/tests/master_tests.cpp ceee2f4a5d38e0f4200f444769e058d2173de821 
  src/tests/scheduler_tests.cpp 0f5d9ada6eb880379baf5f106fd2d5b12e9738db 


Diff: https://reviews.apache.org/r/57818/diff/6/

Changes: https://reviews.apache.org/r/57818/diff/5-6/


Testing
---

All tests passed.


Thanks,

Anindya Sinha

Re: Review Request 57815: Added `deactivated_roles` field in `FrameworkInfo`.

[Anindya Sinha]

> On April 17, 2017, 6:28 p.m., Vinod Kone wrote:
> > include/mesos/scheduler/scheduler.proto
> > Lines 249-250 (original), 249-250 (patched)
> > 
> >
> > Hmm, I was hoping that we could re-use the `Call::Suppress` message 
> > here instead of re-defining it. That way any changes to suppress would be 
> > in sync.
> 
> Anindya Sinha wrote:
> Currently, `Call::Suppress` is:
> ```
>   message Suppress {
> optional string role = 1;
>   }
> ```
> 
> So, the `SUPPRESS` call currently allows suppressing offers for a 
> framework on a per-role basis (or all roles if `role` is not set). Since we 
> now support multiple roles for a frameework, and if we want to suppress 
> offers for a subset of roles at the time of `SUBSCRIBE`, using 
> `Call::Suppress` would not satisfy that use case. It would only satisfy 
> suppressing for all roles (i.e. `Suppress::role` is not set) or for a 
> specific role.
> 
> Is there any plans of extending `SUPPRESS` to suppress offers for a 
> subset of roles (>0, and < all roles) in a single call? Do you think we 
> should consider that also?
> 
> One such approach could be to have the following:
> ```
>   message Suppress {
> repeated string roles = 1;
>   }
> 
>   optional Suppress suppress;
> ```
> 
> If suppress is not set, then we do not suppress for any roles. Otherwise, 
> we suppress for the set of roles specified in `suppress.roles`. If `suppress` 
> is set but `suppress.roles` is empty, we suppress for all roles of the 
> framework.
> 
> Thoughts?

So based on discussion in the Slack channel, this modified review chain does 
the following:

1. Add `repeated string deactivated_roles` to `FrameworkInfo` which represents 
a subset of roles which are deactivated. Offers pertaining to the deactivated 
roles shall not be sent out.
2. `SUPPRESS` and `REVIVE` calls will toggle the (de)activation mode of the 
roles.
3. Allocator's `activateFramework()` call will activate all roles of the 
framework which are not in `FrameworkInfo::deactivated+roles`. Similarly, in 
`deactivateFramework()` call will deactivate all roles that are not deactivated 
already.


- Anindya


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


On May 5, 2017, 12:34 a.m., Anindya Sinha wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57815/
> ---
> 
> (Updated May 5, 2017, 12:34 a.m.)
> 
> 
> Review request for mesos, James Peach, Vinod Kone, and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-7015
> https://issues.apache.org/jira/browse/MESOS-7015
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This field is a subset of roles the framework registered as for which
> the framework does not want any resources offere to.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 1935f47a52840f6b395ecb2d2829551fa691 
>   include/mesos/v1/mesos.proto c7f0bec5c96f2f41344d4261d0696f9fe0421db7 
> 
> 
> Diff: https://reviews.apache.org/r/57815/diff/4/
> 
> 
> Testing
> ---
> 
> All tests passed.
> 
> 
> Thanks,
> 
> Anindya Sinha
> 
>

Re: Review Request 57815: Added `deactivated_roles` field in `FrameworkInfo`.

[Anindya Sinha]

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

(Updated May 5, 2017, 12:34 a.m.)


Review request for mesos, James Peach, Vinod Kone, and Jiang Yan Xu.


Changes
---

Addressed review comments based on discussion on Slack.


Summary (updated)
-

Added `deactivated_roles` field in `FrameworkInfo`.


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


Repository: mesos


Description (updated)
---

This field is a subset of roles the framework registered as for which
the framework does not want any resources offere to.


Diffs (updated)
-

  include/mesos/mesos.proto 1935f47a52840f6b395ecb2d2829551fa691 
  include/mesos/v1/mesos.proto c7f0bec5c96f2f41344d4261d0696f9fe0421db7 


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

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


Testing
---

All tests passed.


Thanks,

Anindya Sinha

Re: Review Request 58759: Introduced SecretFetcher module interface.

[Kapil Arya]

> On May 4, 2017, 6:25 p.m., Jie Yu wrote:
> > include/mesos/module/secretfetcher.hpp
> > Lines 17 (patched)
> > 
> >
> > secret_fetcher.hpp
> > 
> > (we have secret_generator.hpp). Let's make sure it's consistent.

I'll update it to secret_fetcher.hpp. However, please note that there is still 
some inconsistency here: the module interface is called `SecretGenerator` but 
the file is named `secret_generator.hpp`.


> On May 4, 2017, 6:25 p.m., Jie Yu wrote:
> > include/mesos/secret/fetcher.hpp
> > Lines 17 (patched)
> > 
> >
> > I'd put the file to include/mesos/fetcher/secret_fetcher.hpp
> 
> Kapil Arya wrote:
> I am not sure so sure about it because there is nothing in common between 
> `Fetcher` and `SecretFetcher`. One can think of `SecretFetcher` as more like 
> secret-resolver.
> 
> Perhaps, next to authentication/secret_generator.hpp would be slightly 
> better, but then it doesn't have much to do with authentication either.
> 
> Jie Yu wrote:
> OK, i am fine with secret/fetcher.hpp. Make sure it's consistent with the 
> cpp file.

I am not sure what you mean by "Make sure it's consistent with the cpp file"?


- Kapil


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


On May 4, 2017, 4:06 p.m., Kapil Arya wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58759/
> ---
> 
> (Updated May 4, 2017, 4:06 p.m.)
> 
> 
> Review request for mesos, Gilbert Song, Jie Yu, and Vinod Kone.
> 
> 
> Bugs: MESOS-7418
> https://issues.apache.org/jira/browse/MESOS-7418
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Introduced SecretFetcher module interface.
> 
> 
> Diffs
> -
> 
>   include/mesos/module/secretfetcher.hpp PRE-CREATION 
>   include/mesos/secret/fetcher.hpp PRE-CREATION 
>   src/Makefile.am 29da17bee13226e18757e2ad3a7a091427fd35f4 
>   src/module/manager.cpp 7d875fcb7fcec0d57274e644b0a3b67b333ac193 
> 
> 
> Diff: https://reviews.apache.org/r/58759/diff/3/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Kapil Arya
> 
>

Re: Review Request 58999: Added --secret_fetcher flag for agent.

[Kapil Arya]

> On May 4, 2017, 6:29 p.m., Jie Yu wrote:
> > src/slave/containerizer/containerizer.hpp
> > Lines 66 (patched)
> > 
> >
> > Why optional?
> 
> Kapil Arya wrote:
> That way we don't have to update the existing unit tests involving 
> containerizers.
> 
> Jie Yu wrote:
> Can you try using default parameter?
> 
> Jie Yu wrote:
> Even default parameter is hacky. We should at least have a TODO 
> somewhere. I saw the isolator will simply call secretFetcher.get() assuming 
> it's Some(), while you're passing None() to containerizer in the test.
> 
> I don't like the way we inject `Fetcher` in tests also. That's the reason 
> why RAW pointer is evil. If you have a managed pointer, you probably don't 
> have this issue. Maybe we should use a managed pointer in the interface? The 
> fact that the module create returns a raw pointer is a bad decision in 
> retrospect. It should have been unique_ptr or Owned pointer.
> 
> Jie Yu wrote:
> Any reason this cannot be a Shared pointer? `Shared`?
> 
> just make the method in SecretFetcher const.

Are you suggesting `SecretFecther::fetch(...) const;` ?


- Kapil


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


On May 4, 2017, 4:07 p.m., Kapil Arya wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58999/
> ---
> 
> (Updated May 4, 2017, 4:07 p.m.)
> 
> 
> Review request for mesos, Gilbert Song, Jie Yu, and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated Containerizer to accept SecretFetcher.
> 
> 
> Diffs
> -
> 
>   src/local/local.cpp e47980929db2da1f31cf899a0e1fc452070e11f3 
>   src/slave/containerizer/containerizer.hpp 
> 4c31a1f5c853c1dc66480c7b4c867a87a1bb5c41 
>   src/slave/containerizer/containerizer.cpp 
> 9024371b6c4228f0903cfeef3bbec736e1a425f8 
>   src/slave/containerizer/mesos/containerizer.hpp 
> 29a99f33e646593127b9dc126f398f7bca88e21d 
>   src/slave/containerizer/mesos/containerizer.cpp 
> b58baed64480e22f640a4852537f85922ed382ae 
>   src/slave/flags.hpp c7a4604ed994e15c1db6accfaded2e882f1aec94 
>   src/slave/flags.cpp c50e43c0e0ba633f6b905b0d78668c0a0eebb173 
>   src/slave/main.cpp 72b141cb66f9df5bcc7b3f8cfcc2b06fcbd17e52 
>   src/tests/cluster.hpp 65634121b7fe076a7cd9a1c5aa6101a96b1c009d 
>   src/tests/cluster.cpp a4f57e0e9c4f221c42bdbb36d8925f688b3d742f 
>   src/tests/containerizer/docker_volume_isolator_tests.cpp 
> b47a6b5081a63ac474ac4634701b1a572eb58137 
>   src/tests/containerizer/mesos_containerizer_tests.cpp 
> 13e0f7e603a3ffdd0965b253d7abfe6a069cd2b4 
> 
> 
> Diff: https://reviews.apache.org/r/58999/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Kapil Arya
> 
>

Re: Review Request 58999: Added --secret_fetcher flag for agent.

[Jie Yu]

> On May 4, 2017, 10:29 p.m., Jie Yu wrote:
> > src/slave/containerizer/containerizer.hpp
> > Lines 66 (patched)
> > 
> >
> > Why optional?
> 
> Kapil Arya wrote:
> That way we don't have to update the existing unit tests involving 
> containerizers.
> 
> Jie Yu wrote:
> Can you try using default parameter?
> 
> Jie Yu wrote:
> Even default parameter is hacky. We should at least have a TODO 
> somewhere. I saw the isolator will simply call secretFetcher.get() assuming 
> it's Some(), while you're passing None() to containerizer in the test.
> 
> I don't like the way we inject `Fetcher` in tests also. That's the reason 
> why RAW pointer is evil. If you have a managed pointer, you probably don't 
> have this issue. Maybe we should use a managed pointer in the interface? The 
> fact that the module create returns a raw pointer is a bad decision in 
> retrospect. It should have been unique_ptr or Owned pointer.

Any reason this cannot be a Shared pointer? `Shared`?

just make the method in SecretFetcher const.


- Jie


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


On May 4, 2017, 8:07 p.m., Kapil Arya wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58999/
> ---
> 
> (Updated May 4, 2017, 8:07 p.m.)
> 
> 
> Review request for mesos, Gilbert Song, Jie Yu, and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated Containerizer to accept SecretFetcher.
> 
> 
> Diffs
> -
> 
>   src/local/local.cpp e47980929db2da1f31cf899a0e1fc452070e11f3 
>   src/slave/containerizer/containerizer.hpp 
> 4c31a1f5c853c1dc66480c7b4c867a87a1bb5c41 
>   src/slave/containerizer/containerizer.cpp 
> 9024371b6c4228f0903cfeef3bbec736e1a425f8 
>   src/slave/containerizer/mesos/containerizer.hpp 
> 29a99f33e646593127b9dc126f398f7bca88e21d 
>   src/slave/containerizer/mesos/containerizer.cpp 
> b58baed64480e22f640a4852537f85922ed382ae 
>   src/slave/flags.hpp c7a4604ed994e15c1db6accfaded2e882f1aec94 
>   src/slave/flags.cpp c50e43c0e0ba633f6b905b0d78668c0a0eebb173 
>   src/slave/main.cpp 72b141cb66f9df5bcc7b3f8cfcc2b06fcbd17e52 
>   src/tests/cluster.hpp 65634121b7fe076a7cd9a1c5aa6101a96b1c009d 
>   src/tests/cluster.cpp a4f57e0e9c4f221c42bdbb36d8925f688b3d742f 
>   src/tests/containerizer/docker_volume_isolator_tests.cpp 
> b47a6b5081a63ac474ac4634701b1a572eb58137 
>   src/tests/containerizer/mesos_containerizer_tests.cpp 
> 13e0f7e603a3ffdd0965b253d7abfe6a069cd2b4 
> 
> 
> Diff: https://reviews.apache.org/r/58999/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Kapil Arya
> 
>

Re: Review Request 58759: Introduced SecretFetcher module interface.

[Jie Yu]

> On May 4, 2017, 10:25 p.m., Jie Yu wrote:
> > include/mesos/secret/fetcher.hpp
> > Lines 17 (patched)
> > 
> >
> > I'd put the file to include/mesos/fetcher/secret_fetcher.hpp
> 
> Kapil Arya wrote:
> I am not sure so sure about it because there is nothing in common between 
> `Fetcher` and `SecretFetcher`. One can think of `SecretFetcher` as more like 
> secret-resolver.
> 
> Perhaps, next to authentication/secret_generator.hpp would be slightly 
> better, but then it doesn't have much to do with authentication either.

OK, i am fine with secret/fetcher.hpp. Make sure it's consistent with the cpp 
file.


- Jie


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


On May 4, 2017, 8:06 p.m., Kapil Arya wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58759/
> ---
> 
> (Updated May 4, 2017, 8:06 p.m.)
> 
> 
> Review request for mesos, Gilbert Song, Jie Yu, and Vinod Kone.
> 
> 
> Bugs: MESOS-7418
> https://issues.apache.org/jira/browse/MESOS-7418
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Introduced SecretFetcher module interface.
> 
> 
> Diffs
> -
> 
>   include/mesos/module/secretfetcher.hpp PRE-CREATION 
>   include/mesos/secret/fetcher.hpp PRE-CREATION 
>   src/Makefile.am 29da17bee13226e18757e2ad3a7a091427fd35f4 
>   src/module/manager.cpp 7d875fcb7fcec0d57274e644b0a3b67b333ac193 
> 
> 
> Diff: https://reviews.apache.org/r/58759/diff/3/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Kapil Arya
> 
>

Re: Review Request 58999: Added --secret_fetcher flag for agent.

[Jie Yu]

> On May 4, 2017, 10:29 p.m., Jie Yu wrote:
> > src/slave/containerizer/containerizer.hpp
> > Lines 66 (patched)
> > 
> >
> > Why optional?
> 
> Kapil Arya wrote:
> That way we don't have to update the existing unit tests involving 
> containerizers.
> 
> Jie Yu wrote:
> Can you try using default parameter?

Even default parameter is hacky. We should at least have a TODO somewhere. I 
saw the isolator will simply call secretFetcher.get() assuming it's Some(), 
while you're passing None() to containerizer in the test.

I don't like the way we inject `Fetcher` in tests also. That's the reason why 
RAW pointer is evil. If you have a managed pointer, you probably don't have 
this issue. Maybe we should use a managed pointer in the interface? The fact 
that the module create returns a raw pointer is a bad decision in retrospect. 
It should have been unique_ptr or Owned pointer.


- Jie


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


On May 4, 2017, 8:07 p.m., Kapil Arya wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58999/
> ---
> 
> (Updated May 4, 2017, 8:07 p.m.)
> 
> 
> Review request for mesos, Gilbert Song, Jie Yu, and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated Containerizer to accept SecretFetcher.
> 
> 
> Diffs
> -
> 
>   src/local/local.cpp e47980929db2da1f31cf899a0e1fc452070e11f3 
>   src/slave/containerizer/containerizer.hpp 
> 4c31a1f5c853c1dc66480c7b4c867a87a1bb5c41 
>   src/slave/containerizer/containerizer.cpp 
> 9024371b6c4228f0903cfeef3bbec736e1a425f8 
>   src/slave/containerizer/mesos/containerizer.hpp 
> 29a99f33e646593127b9dc126f398f7bca88e21d 
>   src/slave/containerizer/mesos/containerizer.cpp 
> b58baed64480e22f640a4852537f85922ed382ae 
>   src/slave/flags.hpp c7a4604ed994e15c1db6accfaded2e882f1aec94 
>   src/slave/flags.cpp c50e43c0e0ba633f6b905b0d78668c0a0eebb173 
>   src/slave/main.cpp 72b141cb66f9df5bcc7b3f8cfcc2b06fcbd17e52 
>   src/tests/cluster.hpp 65634121b7fe076a7cd9a1c5aa6101a96b1c009d 
>   src/tests/cluster.cpp a4f57e0e9c4f221c42bdbb36d8925f688b3d742f 
>   src/tests/containerizer/docker_volume_isolator_tests.cpp 
> b47a6b5081a63ac474ac4634701b1a572eb58137 
>   src/tests/containerizer/mesos_containerizer_tests.cpp 
> 13e0f7e603a3ffdd0965b253d7abfe6a069cd2b4 
> 
> 
> Diff: https://reviews.apache.org/r/58999/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Kapil Arya
> 
>

Re: Review Request 58759: Introduced SecretFetcher module interface.

[Kapil Arya]

> On May 4, 2017, 6:25 p.m., Jie Yu wrote:
> > include/mesos/secret/fetcher.hpp
> > Lines 17 (patched)
> > 
> >
> > I'd put the file to include/mesos/fetcher/secret_fetcher.hpp

I am not sure so sure about it because there is nothing in common between 
`Fetcher` and `SecretFetcher`. One can think of `SecretFetcher` as more like 
secret-resolver.

Perhaps, next to authentication/secret_generator.hpp would be slightly better, 
but then it doesn't have much to do with authentication either.


- Kapil


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


On May 4, 2017, 4:06 p.m., Kapil Arya wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58759/
> ---
> 
> (Updated May 4, 2017, 4:06 p.m.)
> 
> 
> Review request for mesos, Gilbert Song, Jie Yu, and Vinod Kone.
> 
> 
> Bugs: MESOS-7418
> https://issues.apache.org/jira/browse/MESOS-7418
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Introduced SecretFetcher module interface.
> 
> 
> Diffs
> -
> 
>   include/mesos/module/secretfetcher.hpp PRE-CREATION 
>   include/mesos/secret/fetcher.hpp PRE-CREATION 
>   src/Makefile.am 29da17bee13226e18757e2ad3a7a091427fd35f4 
>   src/module/manager.cpp 7d875fcb7fcec0d57274e644b0a3b67b333ac193 
> 
> 
> Diff: https://reviews.apache.org/r/58759/diff/3/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Kapil Arya
> 
>

Re: Review Request 58880: Added Secret to Image::Docker in v1/mesos.proto.

[Gilbert Song]

> On May 2, 2017, 5:11 p.m., Vinod Kone wrote:
> > Make sure to update unversioned mesos.proto as well.

Follow up by https://reviews.apache.org/r/59010/


- Gilbert


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


On April 30, 2017, 4:23 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58880/
> ---
> 
> (Updated April 30, 2017, 4:23 p.m.)
> 
> 
> Review request for mesos, Chun-Hung Hsiao, Jie Yu, Kapil Arya, and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This is a followup patch for https://reviews.apache.org/r/58775/.
> 
> 
> Diffs
> -
> 
>   include/mesos/v1/mesos.proto f7c05a82f8265aedc0bd8fd20dd30e21af46e775 
> 
> 
> Diff: https://reviews.apache.org/r/58880/diff/1/
> 
> 
> Testing
> ---
> 
> make
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Review Request 59019: Updated the documentation for '--docker_registry' option.

[Chun-Hung Hsiao]

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

Review request for mesos, Gilbert Song and Jie Yu.


Repository: mesos


Description
---

Updated the documentation for '--docker_registry' option.


Diffs
-

  docs/configuration.md 79cada3c9403881bf257d653f721d32e55607a7f 


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


Testing
---


Thanks,

Chun-Hung Hsiao

Review Request 59013: Implemented passing Image::Secret Puller::pull().

[Gilbert Song]

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

Review request for mesos, Adam B, Chun-Hung Hsiao, Jie Yu, Kapil Arya, Till 
Toenshoff, and Vinod Kone.


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


Repository: mesos


Description
---

Implemented passing Image::Secret Puller::pull().


Diffs
-

  src/slave/containerizer/mesos/provisioner/docker/local_puller.hpp 
344baab5c0365c3fc2dc814887bb2b48082b050f 
  src/slave/containerizer/mesos/provisioner/docker/local_puller.cpp 
5bc68d2047a7b3e3fa30315d61073e342ba6affe 
  src/slave/containerizer/mesos/provisioner/docker/puller.hpp 
6dacdb1403433a37dd31a93d4ff2e37d4685eb87 
  src/slave/containerizer/mesos/provisioner/docker/registry_puller.hpp 
62ddb7a332030f3116477408d8b16c19e434c159 
  src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp 
6db788dd0c582deadf3e91c4d21bb9c20cf94e6b 
  src/slave/containerizer/mesos/provisioner/docker/store.cpp 
68ce265a5d0a61f8d9ed55dd14c630dcf893a7d2 
  src/tests/containerizer/provisioner_docker_tests.cpp 
4341621767a9fa5be2c66e77ef60f0c65dae58ca 


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


Testing
---

make check


Thanks,

Gilbert Song

Review Request 59018: Supported Image::Secret in docker URI fetcher plugin.

[Gilbert Song]

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

Review request for mesos, Adam B, Chun-Hung Hsiao, Jie Yu, Kapil Arya, Till 
Toenshoff, and Vinod Kone.


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


Repository: mesos


Description
---

Supported Image::Secret in docker URI fetcher plugin.


Diffs
-

  src/uri/fetchers/docker.cpp 44169bf5f22f0ffd9fad7bb3b8f7d2a4989c6415 


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


Testing
---

make check

Manually tested using mesos-execute with task group. Verified that pulling 
separate private image from different registries using different image secrets 
works correctly.


Thanks,

Gilbert Song

Review Request 59014: Implemented resolving an image secret in registry puller.

[Gilbert Song]

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

Review request for mesos, Adam B, Chun-Hung Hsiao, Jie Yu, Kapil Arya, Till 
Toenshoff, and Vinod Kone.


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


Repository: mesos


Description
---

Implemented resolving an image secret in registry puller.


Diffs
-

  src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp 
6db788dd0c582deadf3e91c4d21bb9c20cf94e6b 


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


Testing
---

make check


Thanks,

Gilbert Song

Review Request 59016: Fixed the comment style issue in docker/spec.hpp.

[Gilbert Song]

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

Review request for mesos, Adam B, Chun-Hung Hsiao, Jie Yu, Kapil Arya, Till 
Toenshoff, and Vinod Kone.


Repository: mesos


Description
---

Fixed the comment style issue in docker/spec.hpp.


Diffs
-

  include/mesos/docker/spec.hpp b90f731ef07c22259715543526eaf25cb94eab03 


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


Testing
---

N/A


Thanks,

Gilbert Song

Review Request 59017: Added support for docker spec helper 'parseAuthConfig()'.

[Gilbert Song]

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

Review request for mesos, Adam B, Chun-Hung Hsiao, Jie Yu, Kapil Arya, Till 
Toenshoff, and Vinod Kone.


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


Repository: mesos


Description
---

Added support for docker spec helper 'parseAuthConfig()'.


Diffs
-

  include/mesos/docker/spec.hpp b90f731ef07c22259715543526eaf25cb94eab03 
  src/docker/spec.cpp 6b5588e534215451593cc20011847c3abbab9b17 


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


Testing
---

make check


Thanks,

Gilbert Song

Review Request 59011: Fixed docker/appc store 'using' format.

[Gilbert Song]

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

Review request for mesos, Adam B, Chun-Hung Hsiao, Jie Yu, Kapil Arya, Till 
Toenshoff, and Vinod Kone.


Repository: mesos


Description
---

Fixed docker/appc store 'using' format.


Diffs
-

  include/mesos/appc/spec.hpp e9430df03b49f49d367e19b18991ead77fe5b830 
  src/slave/containerizer/mesos/provisioner/appc/store.cpp 
09a40a5835454bb7519d11bae4a851337a89b935 
  src/slave/containerizer/mesos/provisioner/docker/store.cpp 
68ce265a5d0a61f8d9ed55dd14c630dcf893a7d2 
  src/slave/containerizer/mesos/provisioner/store.cpp 
7141d63fcf2dbc3fbf00508c7f92945aab014fb2 


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


Testing
---

make check


Thanks,

Gilbert Song

Review Request 59015: Implemented passing docker config to URIs.

[Gilbert Song]

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

Review request for mesos, Adam B, Chun-Hung Hsiao, Jie Yu, Kapil Arya, Till 
Toenshoff, and Vinod Kone.


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


Repository: mesos


Description
---

Implemented passing docker config to URIs.


Diffs
-

  src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp 
6db788dd0c582deadf3e91c4d21bb9c20cf94e6b 
  src/uri/schemes/docker.hpp 527c7e4ba6f1b505543655cca870e39b93a5266e 


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


Testing
---

make check


Thanks,

Gilbert Song

Review Request 59012: Implemented passing the secret fetcher to registry puller.

[Gilbert Song]

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

Review request for mesos, Adam B, Chun-Hung Hsiao, Jie Yu, Kapil Arya, Till 
Toenshoff, and Vinod Kone.


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


Repository: mesos


Description
---

Implemented passing the secret fetcher to registry puller.


Diffs
-

  src/slave/containerizer/mesos/containerizer.cpp 
b58baed64480e22f640a4852537f85922ed382ae 
  src/slave/containerizer/mesos/provisioner/appc/store.hpp 
15c79e9401a821e445fbd32b34503e4fb0014d42 
  src/slave/containerizer/mesos/provisioner/appc/store.cpp 
09a40a5835454bb7519d11bae4a851337a89b935 
  src/slave/containerizer/mesos/provisioner/docker/puller.hpp 
6dacdb1403433a37dd31a93d4ff2e37d4685eb87 
  src/slave/containerizer/mesos/provisioner/docker/puller.cpp 
ac9dae8ecbb897b8ff942d11ac70281a63e06831 
  src/slave/containerizer/mesos/provisioner/docker/registry_puller.hpp 
62ddb7a332030f3116477408d8b16c19e434c159 
  src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp 
6db788dd0c582deadf3e91c4d21bb9c20cf94e6b 
  src/slave/containerizer/mesos/provisioner/docker/store.hpp 
e1abff19c8877ad07900bb2f44deca1cdda41f58 
  src/slave/containerizer/mesos/provisioner/docker/store.cpp 
68ce265a5d0a61f8d9ed55dd14c630dcf893a7d2 
  src/slave/containerizer/mesos/provisioner/provisioner.hpp 
7d6c1b93a2c0e265b9344a0fc27f1cf4ed5325f2 
  src/slave/containerizer/mesos/provisioner/provisioner.cpp 
be45fc59027f176b43b767e9441fd8089ceec7b4 
  src/slave/containerizer/mesos/provisioner/store.hpp 
82a9be64264ae829773c1e2e8a4360f78641cbf6 
  src/slave/containerizer/mesos/provisioner/store.cpp 
7141d63fcf2dbc3fbf00508c7f92945aab014fb2 


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


Testing
---

make check


Thanks,

Gilbert Song

Review Request 59010: Updated protobuf comments for Image::Secret.

[Gilbert Song]

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

Review request for mesos, Adam B, Chun-Hung Hsiao, Jie Yu, Kapil Arya, Till 
Toenshoff, and Vinod Kone.


Repository: mesos


Description
---

Updated protobuf comments for Image::Secret.


Diffs
-

  include/mesos/mesos.proto 1935f47a52840f6b395ecb2d2829551fa691 
  include/mesos/v1/mesos.proto c7f0bec5c96f2f41344d4261d0696f9fe0421db7 


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


Testing
---

N/A.


Thanks,

Gilbert Song

Re: Review Request 58999: Added --secret_fetcher flag for agent.

[Jie Yu]

> On May 4, 2017, 10:29 p.m., Jie Yu wrote:
> > src/slave/containerizer/containerizer.hpp
> > Lines 66 (patched)
> > 
> >
> > Why optional?
> 
> Kapil Arya wrote:
> That way we don't have to update the existing unit tests involving 
> containerizers.

Can you try using default parameter?


- Jie


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


On May 4, 2017, 8:07 p.m., Kapil Arya wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58999/
> ---
> 
> (Updated May 4, 2017, 8:07 p.m.)
> 
> 
> Review request for mesos, Gilbert Song, Jie Yu, and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated Containerizer to accept SecretFetcher.
> 
> 
> Diffs
> -
> 
>   src/local/local.cpp e47980929db2da1f31cf899a0e1fc452070e11f3 
>   src/slave/containerizer/containerizer.hpp 
> 4c31a1f5c853c1dc66480c7b4c867a87a1bb5c41 
>   src/slave/containerizer/containerizer.cpp 
> 9024371b6c4228f0903cfeef3bbec736e1a425f8 
>   src/slave/containerizer/mesos/containerizer.hpp 
> 29a99f33e646593127b9dc126f398f7bca88e21d 
>   src/slave/containerizer/mesos/containerizer.cpp 
> b58baed64480e22f640a4852537f85922ed382ae 
>   src/slave/flags.hpp c7a4604ed994e15c1db6accfaded2e882f1aec94 
>   src/slave/flags.cpp c50e43c0e0ba633f6b905b0d78668c0a0eebb173 
>   src/slave/main.cpp 72b141cb66f9df5bcc7b3f8cfcc2b06fcbd17e52 
>   src/tests/cluster.hpp 65634121b7fe076a7cd9a1c5aa6101a96b1c009d 
>   src/tests/cluster.cpp a4f57e0e9c4f221c42bdbb36d8925f688b3d742f 
>   src/tests/containerizer/docker_volume_isolator_tests.cpp 
> b47a6b5081a63ac474ac4634701b1a572eb58137 
>   src/tests/containerizer/mesos_containerizer_tests.cpp 
> 13e0f7e603a3ffdd0965b253d7abfe6a069cd2b4 
> 
> 
> Diff: https://reviews.apache.org/r/58999/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Kapil Arya
> 
>

Re: Review Request 58999: Added --secret_fetcher flag for agent.

[Kapil Arya]

> On May 4, 2017, 6:29 p.m., Jie Yu wrote:
> > src/slave/containerizer/containerizer.hpp
> > Lines 66 (patched)
> > 
> >
> > Why optional?

That way we don't have to update the existing unit tests involving 
containerizers.


- Kapil


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


On May 4, 2017, 4:07 p.m., Kapil Arya wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58999/
> ---
> 
> (Updated May 4, 2017, 4:07 p.m.)
> 
> 
> Review request for mesos, Gilbert Song, Jie Yu, and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated Containerizer to accept SecretFetcher.
> 
> 
> Diffs
> -
> 
>   src/local/local.cpp e47980929db2da1f31cf899a0e1fc452070e11f3 
>   src/slave/containerizer/containerizer.hpp 
> 4c31a1f5c853c1dc66480c7b4c867a87a1bb5c41 
>   src/slave/containerizer/containerizer.cpp 
> 9024371b6c4228f0903cfeef3bbec736e1a425f8 
>   src/slave/containerizer/mesos/containerizer.hpp 
> 29a99f33e646593127b9dc126f398f7bca88e21d 
>   src/slave/containerizer/mesos/containerizer.cpp 
> b58baed64480e22f640a4852537f85922ed382ae 
>   src/slave/flags.hpp c7a4604ed994e15c1db6accfaded2e882f1aec94 
>   src/slave/flags.cpp c50e43c0e0ba633f6b905b0d78668c0a0eebb173 
>   src/slave/main.cpp 72b141cb66f9df5bcc7b3f8cfcc2b06fcbd17e52 
>   src/tests/cluster.hpp 65634121b7fe076a7cd9a1c5aa6101a96b1c009d 
>   src/tests/cluster.cpp a4f57e0e9c4f221c42bdbb36d8925f688b3d742f 
>   src/tests/containerizer/docker_volume_isolator_tests.cpp 
> b47a6b5081a63ac474ac4634701b1a572eb58137 
>   src/tests/containerizer/mesos_containerizer_tests.cpp 
> 13e0f7e603a3ffdd0965b253d7abfe6a069cd2b4 
> 
> 
> Diff: https://reviews.apache.org/r/58999/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Kapil Arya
> 
>

Re: Review Request 58967: Set the working directory to parent task's for DEBUG containers.

[Jie Yu]

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


Ship it!




Ship It!

- Jie Yu


On May 3, 2017, 4:46 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58967/
> ---
> 
> (Updated May 3, 2017, 4:46 p.m.)
> 
> 
> Review request for mesos, Gastón Kleiman, Jie Yu, and Vinod Kone.
> 
> 
> Bugs: MESOS-7433
> https://issues.apache.org/jira/browse/MESOS-7433
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> A DEBUG container's working directory should be set to the parent
> task's working directory. For the command executor case, even if
> the task itself has a root filesystem, the executor container still
> uses the host filesystem, hence
> `ContainerLaunchInfo.working_directory`, pointing to the executor's
> working directory in the host filesystem, may be different from the
> task's working directory in the root filesystem.
> 
> 
> Diffs
> -
> 
>   include/mesos/slave/containerizer.proto 
> c30b1fc659ee9b3cd343899638ced6408d8b51a2 
>   src/slave/containerizer/mesos/containerizer.cpp 
> b58baed64480e22f640a4852537f85922ed382ae 
> 
> 
> Diff: https://reviews.apache.org/r/58967/diff/1/
> 
> 
> Testing
> ---
> 
> make check on Mac OS and Fedora 24
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 59000: Added environment secret isolator.

[Jie Yu]

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




src/slave/containerizer/mesos/isolators/environment_secret.cpp
Lines 37-44 (patched)


Why you need this? Can you make sure to avoid such small nits while copying 
pasting code? Please remove all unncessary headers, and do a sweep for other 
reviews as well!



src/slave/containerizer/mesos/isolators/environment_secret.cpp
Lines 57-58 (patched)


Do you use those in this file? Ditto my other comments.



src/slave/containerizer/mesos/isolators/environment_secret.cpp
Lines 100 (patched)


Please remove std:: prefix here. Please do a sweep to fix all such issues 
in other reviews!


- Jie Yu


On May 4, 2017, 8:08 p.m., Kapil Arya wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59000/
> ---
> 
> (Updated May 4, 2017, 8:08 p.m.)
> 
> 
> Review request for mesos, Gilbert Song, Greg Mann, Jie Yu, and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This isolator resolves Secret-type environment variables using a provided 
> SecretFetcher module.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 29da17bee13226e18757e2ad3a7a091427fd35f4 
>   src/launcher/executor.cpp b05f73e539c22d4d40f07df76168a06373b818d4 
>   src/slave/containerizer/mesos/containerizer.cpp 
> b58baed64480e22f640a4852537f85922ed382ae 
>   src/slave/containerizer/mesos/isolators/environment_secret.hpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/environment_secret.cpp PRE-CREATION 
>   src/tests/containerizer/environment_secret_isolator_tests.cpp PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/59000/diff/1/
> 
> 
> Testing
> ---
> 
> Added a new test and ran `make check`.
> 
> 
> Thanks,
> 
> Kapil Arya
> 
>

Re: Review Request 58999: Added --secret_fetcher flag for agent.

[Jie Yu]

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




src/slave/containerizer/containerizer.hpp
Lines 66 (patched)


Why optional?


- Jie Yu


On May 4, 2017, 8:07 p.m., Kapil Arya wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58999/
> ---
> 
> (Updated May 4, 2017, 8:07 p.m.)
> 
> 
> Review request for mesos, Gilbert Song, Jie Yu, and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated Containerizer to accept SecretFetcher.
> 
> 
> Diffs
> -
> 
>   src/local/local.cpp e47980929db2da1f31cf899a0e1fc452070e11f3 
>   src/slave/containerizer/containerizer.hpp 
> 4c31a1f5c853c1dc66480c7b4c867a87a1bb5c41 
>   src/slave/containerizer/containerizer.cpp 
> 9024371b6c4228f0903cfeef3bbec736e1a425f8 
>   src/slave/containerizer/mesos/containerizer.hpp 
> 29a99f33e646593127b9dc126f398f7bca88e21d 
>   src/slave/containerizer/mesos/containerizer.cpp 
> b58baed64480e22f640a4852537f85922ed382ae 
>   src/slave/flags.hpp c7a4604ed994e15c1db6accfaded2e882f1aec94 
>   src/slave/flags.cpp c50e43c0e0ba633f6b905b0d78668c0a0eebb173 
>   src/slave/main.cpp 72b141cb66f9df5bcc7b3f8cfcc2b06fcbd17e52 
>   src/tests/cluster.hpp 65634121b7fe076a7cd9a1c5aa6101a96b1c009d 
>   src/tests/cluster.cpp a4f57e0e9c4f221c42bdbb36d8925f688b3d742f 
>   src/tests/containerizer/docker_volume_isolator_tests.cpp 
> b47a6b5081a63ac474ac4634701b1a572eb58137 
>   src/tests/containerizer/mesos_containerizer_tests.cpp 
> 13e0f7e603a3ffdd0965b253d7abfe6a069cd2b4 
> 
> 
> Diff: https://reviews.apache.org/r/58999/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Kapil Arya
> 
>

Re: Review Request 58760: Added default secret fetcher module.

[Jie Yu]

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




src/Makefile.am
Lines 934 (patched)


Ditto. put under fetcher/secret_fetcher.cpp


- Jie Yu


On May 4, 2017, 8:06 p.m., Kapil Arya wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58760/
> ---
> 
> (Updated May 4, 2017, 8:06 p.m.)
> 
> 
> Review request for mesos, Gilbert Song, Jie Yu, and Vinod Kone.
> 
> 
> Bugs: MESOS-7418
> https://issues.apache.org/jira/browse/MESOS-7418
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added default secret fetcher module.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 29da17bee13226e18757e2ad3a7a091427fd35f4 
>   src/secret/fetcher.hpp PRE-CREATION 
>   src/secret/fetcher.cpp PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/58760/diff/3/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Kapil Arya
> 
>

Re: Review Request 58759: Introduced SecretFetcher module interface.

[Jie Yu]

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




include/mesos/module/secretfetcher.hpp
Lines 17 (patched)


secret_fetcher.hpp

(we have secret_generator.hpp). Let's make sure it's consistent.



include/mesos/secret/fetcher.hpp
Lines 17 (patched)


I'd put the file to include/mesos/fetcher/secret_fetcher.hpp


- Jie Yu


On May 4, 2017, 8:06 p.m., Kapil Arya wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58759/
> ---
> 
> (Updated May 4, 2017, 8:06 p.m.)
> 
> 
> Review request for mesos, Gilbert Song, Jie Yu, and Vinod Kone.
> 
> 
> Bugs: MESOS-7418
> https://issues.apache.org/jira/browse/MESOS-7418
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Introduced SecretFetcher module interface.
> 
> 
> Diffs
> -
> 
>   include/mesos/module/secretfetcher.hpp PRE-CREATION 
>   include/mesos/secret/fetcher.hpp PRE-CREATION 
>   src/Makefile.am 29da17bee13226e18757e2ad3a7a091427fd35f4 
>   src/module/manager.cpp 7d875fcb7fcec0d57274e644b0a3b67b333ac193 
> 
> 
> Diff: https://reviews.apache.org/r/58759/diff/3/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Kapil Arya
> 
>

Re: Review Request 58137: CLI: Added 'mesos config show' command to display the config file.

[Armand Grillet]

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

(Updated May 4, 2017, 9:39 p.m.)


Review request for mesos, Joseph Wu and Kevin Klues.


Changes
---

Fixed issues raised in comments.


Repository: mesos


Description
---

This command displays the contents of the user-defined config.toml file.


Diffs (updated)
-

  src/cli_new/lib/cli/plugins/config/main.py 
d95a36f4a66c66b4477c6816b7fa5a721f9212f7 


Diff: https://reviews.apache.org/r/58137/diff/7/

Changes: https://reviews.apache.org/r/58137/diff/6-7/


Testing
---

Tested manually, PEP8 and Pylint used to make sure that the code style is 
correct.


Thanks,

Armand Grillet

Re: Review Request 52064: Support for multiple versions of docs.

[Tim Anderegg]

> On Dec. 23, 2016, 5:38 p.m., haosdent huang wrote:
> >

Sorry for the long delay on this, I've finally carved up some time to try and 
close this out.  I've left some comments below, and am working on addressing 
the issues, I will update the review tomorrow with the final result, hopefully 
addressing all issues raised.  Thansk!


- Tim


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


On May 4, 2017, 9:26 p.m., Tim Anderegg wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52064/
> ---
> 
> (Updated May 4, 2017, 9:26 p.m.)
> 
> 
> Review request for mesos, haosdent huang and Vinod Kone.
> 
> 
> Bugs: MESOS-3011
> https://issues.apache.org/jira/browse/MESOS-3011
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Support for multiple versions of docs.
> 
> 
> Diffs
> -
> 
>   site/Dockerfile 8ba0be0c28e924f7a2b89e6e5a3237deb3751a41 
>   site/Gemfile 4dcd0a5ad452085e6e3bb152a6547575636cd807 
>   site/Gemfile.lock 1393246f06631155bb052669a86e431778e891bf 
>   site/Rakefile 01356891c29f9e69fa0f7813cf87e7662eda400b 
>   site/build.sh 11f15e15621c4d3db1472e88911787b9b3100f97 
>   site/config.rb 9e6738fccc48365e16221188269c1ed40772eca7 
>   site/data/releases.yml 755300aa83a1017362129bad256381d10c815609 
>   site/source/assets/css/main.css 83596ddbd833e36b60bdbbd487ebd464b3874119 
>   site/source/assets/js/versions.js PRE-CREATION 
>   site/source/layouts/basic.erb 7464e40b619e883daad93c72c3fbdbfbdda8f152 
>   site/source/layouts/documentation.erb 
> a91f916a5fb7348b2702c272e7a2059bdf628c66 
>   site/source/layouts/gettingstarted.erb PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/52064/diff/2/
> 
> 
> Testing
> ---
> 
> Testing was done manually to verify that the documentation was built for each 
> version of Mesos that is supported (some older versions do not have 
> compatible documentation).
> 
> 
> Thanks,
> 
> Tim Anderegg
> 
>

Re: Review Request 52064: Support for multiple versions of docs.

[Tim Anderegg]

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

(Updated May 4, 2017, 9:26 p.m.)


Review request for mesos, haosdent huang and Vinod Kone.


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


Repository: mesos


Description
---

Support for multiple versions of docs.


Diffs
-

  site/Dockerfile 8ba0be0c28e924f7a2b89e6e5a3237deb3751a41 
  site/Gemfile 4dcd0a5ad452085e6e3bb152a6547575636cd807 
  site/Gemfile.lock 1393246f06631155bb052669a86e431778e891bf 
  site/Rakefile 01356891c29f9e69fa0f7813cf87e7662eda400b 
  site/build.sh 11f15e15621c4d3db1472e88911787b9b3100f97 
  site/config.rb 9e6738fccc48365e16221188269c1ed40772eca7 
  site/data/releases.yml 755300aa83a1017362129bad256381d10c815609 
  site/source/assets/css/main.css 83596ddbd833e36b60bdbbd487ebd464b3874119 
  site/source/assets/js/versions.js PRE-CREATION 
  site/source/layouts/basic.erb 7464e40b619e883daad93c72c3fbdbfbdda8f152 
  site/source/layouts/documentation.erb 
a91f916a5fb7348b2702c272e7a2059bdf628c66 
  site/source/layouts/gettingstarted.erb PRE-CREATION 


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


Testing
---

Testing was done manually to verify that the documentation was built for each 
version of Mesos that is supported (some older versions do not have compatible 
documentation).


Thanks,

Tim Anderegg

Review Request 59001: Added volume secret isolator.

[Kapil Arya]

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

Review request for mesos, Gilbert Song, Jie Yu, and Vinod Kone.


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


Repository: mesos


Description
---

Here is the design doc: 
https://docs.google.com/document/d/18raiiUfxTh-JBvjd6RyHe_TOScY87G_bMi5zBzMZmpc


Diffs
-

  src/Makefile.am 29da17bee13226e18757e2ad3a7a091427fd35f4 
  src/slave/containerizer/mesos/containerizer.cpp 
b58baed64480e22f640a4852537f85922ed382ae 
  src/slave/containerizer/mesos/isolators/volume/secret.hpp PRE-CREATION 
  src/slave/containerizer/mesos/isolators/volume/secret.cpp PRE-CREATION 
  src/tests/containerizer/rootfs.cpp fdfecc65a3fcd19d6a4dfa574320f4d1f2755322 
  src/tests/containerizer/volume_secret_isolator_tests.cpp PRE-CREATION 


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


Testing
---


Thanks,

Kapil Arya

Review Request 59000: Added environment secret isolator.

[Kapil Arya]

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

Review request for mesos, Gilbert Song, Greg Mann, Jie Yu, and Vinod Kone.


Repository: mesos


Description
---

This isolator resolves Secret-type environment variables using a provided 
SecretFetcher module.


Diffs
-

  src/Makefile.am 29da17bee13226e18757e2ad3a7a091427fd35f4 
  src/launcher/executor.cpp b05f73e539c22d4d40f07df76168a06373b818d4 
  src/slave/containerizer/mesos/containerizer.cpp 
b58baed64480e22f640a4852537f85922ed382ae 
  src/slave/containerizer/mesos/isolators/environment_secret.hpp PRE-CREATION 
  src/slave/containerizer/mesos/isolators/environment_secret.cpp PRE-CREATION 
  src/tests/containerizer/environment_secret_isolator_tests.cpp PRE-CREATION 


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


Testing
---

Added a new test and ran `make check`.


Thanks,

Kapil Arya

Review Request 58999: Added --secret_fetcher flag for agent.

[Kapil Arya]

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

Review request for mesos, Gilbert Song, Jie Yu, and Vinod Kone.


Repository: mesos


Description
---

Updated Containerizer to accept SecretFetcher.


Diffs
-

  src/local/local.cpp e47980929db2da1f31cf899a0e1fc452070e11f3 
  src/slave/containerizer/containerizer.hpp 
4c31a1f5c853c1dc66480c7b4c867a87a1bb5c41 
  src/slave/containerizer/containerizer.cpp 
9024371b6c4228f0903cfeef3bbec736e1a425f8 
  src/slave/containerizer/mesos/containerizer.hpp 
29a99f33e646593127b9dc126f398f7bca88e21d 
  src/slave/containerizer/mesos/containerizer.cpp 
b58baed64480e22f640a4852537f85922ed382ae 
  src/slave/flags.hpp c7a4604ed994e15c1db6accfaded2e882f1aec94 
  src/slave/flags.cpp c50e43c0e0ba633f6b905b0d78668c0a0eebb173 
  src/slave/main.cpp 72b141cb66f9df5bcc7b3f8cfcc2b06fcbd17e52 
  src/tests/cluster.hpp 65634121b7fe076a7cd9a1c5aa6101a96b1c009d 
  src/tests/cluster.cpp a4f57e0e9c4f221c42bdbb36d8925f688b3d742f 
  src/tests/containerizer/docker_volume_isolator_tests.cpp 
b47a6b5081a63ac474ac4634701b1a572eb58137 
  src/tests/containerizer/mesos_containerizer_tests.cpp 
13e0f7e603a3ffdd0965b253d7abfe6a069cd2b4 


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


Testing
---


Thanks,

Kapil Arya

Re: Review Request 58759: Introduced SecretFetcher module interface.

[Kapil Arya]

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

(Updated May 4, 2017, 4:06 p.m.)


Review request for mesos, Gilbert Song, Jie Yu, and Vinod Kone.


Changes
---

Addressed Vinod's comments.


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


Repository: mesos


Description
---

Introduced SecretFetcher module interface.


Diffs (updated)
-

  include/mesos/module/secretfetcher.hpp PRE-CREATION 
  include/mesos/secret/fetcher.hpp PRE-CREATION 
  src/Makefile.am 29da17bee13226e18757e2ad3a7a091427fd35f4 
  src/module/manager.cpp 7d875fcb7fcec0d57274e644b0a3b67b333ac193 


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

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


Testing
---


Thanks,

Kapil Arya

Re: Review Request 58760: Added default secret fetcher module.

[Kapil Arya]

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

(Updated May 4, 2017, 4:06 p.m.)


Review request for mesos, Gilbert Song, Jie Yu, and Vinod Kone.


Changes
---

Rebased


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


Repository: mesos


Description (updated)
---

Added default secret fetcher module.


Diffs (updated)
-

  src/Makefile.am 29da17bee13226e18757e2ad3a7a091427fd35f4 
  src/secret/fetcher.hpp PRE-CREATION 
  src/secret/fetcher.cpp PRE-CREATION 


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

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


Testing
---


Thanks,

Kapil Arya

Re: Review Request 58720: CLI: Extended the unit test infrastructure.

[Mesos Reviewbot]

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



Patch looks great!

Reviews applied: [58719, 58720]

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

- Mesos Reviewbot


On May 4, 2017, 1:59 p.m., Armand Grillet wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58720/
> ---
> 
> (Updated May 4, 2017, 1:59 p.m.)
> 
> 
> Review request for mesos and Kevin Klues.
> 
> 
> Bugs: MESOS-7283
> https://issues.apache.org/jira/browse/MESOS-7283
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This infrastructure includes the ability to bring up a test cluster to
> run the CLI against. Future unit tests will use this infrastructure to
> test their commands against a running mesos cluster. The tests require
> some binaries created when building Mesos.
> 
> 
> Diffs
> -
> 
>   src/cli_new/lib/cli/tests/__init__.py PRE-CREATION 
>   src/cli_new/lib/cli/tests/base.py PRE-CREATION 
>   src/cli_new/lib/cli/tests/constants.py PRE-CREATION 
>   src/cli_new/lib/cli/tests/tests.py PRE-CREATION 
>   src/cli_new/pip-requirements.txt 28613e56a5c6d5c7606a7e58d6125b0c34748e83 
>   src/cli_new/tests/main.py dff5d48b0ddae87960a78f9d05e4ae597912f1f6 
> 
> 
> Diff: https://reviews.apache.org/r/58720/diff/3/
> 
> 
> Testing
> ---
> 
> PEP8 and Pylint used to make sure that the code style is correct. Manuel test:
> 
> $ cd src/cli_new
> $ ./bootstrap
> $ source activate
> 
> (mesos-cli) $ mesos-cli-tests
> 
> 
> Thanks,
> 
> Armand Grillet
> 
>

Review Request 58998: Fixed extra newline in webui for re-registered frameworks.

[Neil Conway]

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

Review request for mesos and haosdent huang.


Repository: mesos


Description
---

Fixed extra newline in webui for re-registered frameworks.


Diffs
-

  src/webui/master/static/framework.html 
79dd1a7fa8c41c1e09141c273cc5d2e261e578b3 


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


Testing
---

Visual inspection.

Screenshot before: http://www.neilconway.org/tmp/screenshot_before.png

Screenshot after: http://www.neilconway.org/tmp/screenshot_after.png


Thanks,

Neil Conway

Re: Review Request 58987: Add failure print when socket recv data.

[Mesos Reviewbot]

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



Patch looks great!

Reviews applied: [58987]

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

- Mesos Reviewbot


On May 4, 2017, 10:02 a.m., Andy Pang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58987/
> ---
> 
> (Updated May 4, 2017, 10:02 a.m.)
> 
> 
> Review request for mesos, Jie Yu and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add failure print when socket recv data.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/process.cpp 96ce7db 
> 
> 
> Diff: https://reviews.apache.org/r/58987/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Andy Pang
> 
>

Re: Review Request 58942: CHANGELOG for 1.3.0 release.

[Mesos Reviewbot]

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



Patch looks great!

Reviews applied: [58942]

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

- Mesos Reviewbot


On May 2, 2017, 11:48 p.m., Michael Park wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58942/
> ---
> 
> (Updated May 2, 2017, 11:48 p.m.)
> 
> 
> Review request for mesos and Neil Conway.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> CHANGELOG for 1.3.0 release.
> 
> 
> Diffs
> -
> 
>   CHANGELOG 998f30d576d87e72b1d52e8bd1e43ce6ba67a54b 
> 
> 
> Diff: https://reviews.apache.org/r/58942/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Michael Park
> 
>

Re: Review Request 58224: Optionally verify the source IP address for libprocess messages.

[James Peach]

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

(Updated May 4, 2017, 3:52 p.m.)


Review request for mesos and Benjamin Mahler.


Changes
---

Added documentation.


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


Repository: mesos


Description
---

In general, libprocess is unable to validate that a peer
is a legitimate owner of the UPID it claims in a libprocess
message. This change adds a check that the IP address in the
UPID matches the peer address. This makes spoofing the UPID
harder (eg. to send authenticated messages), but also breaks
some legitimate configurations, particularly on multihomed
hosts.


Diffs (updated)
-

  3rdparty/libprocess/src/process.cpp f5b666f894215cb1861c244c94b382e0739bc5c9 
  docs/configuration.md 79cada3c9403881bf257d653f721d32e55607a7f 


Diff: https://reviews.apache.org/r/58224/diff/7/

Changes: https://reviews.apache.org/r/58224/diff/6-7/


Testing
---

make check (Fedora 25). Light manual testing.

With LIBPROCESS_require_peer_address_ip_match=true, all Mesos tests pass except 
``ExamplesTest.DiskFullFramework``, however enabling this will definitely break 
some libprocess APIs (though not in the way that Mesos uses them) and 
legitimate multi-homed configurations. Note that setting 
LIBPROCESS_ip=127.0.0.1 makes you multihomed for this purpose, which is why 
``ExamplesTest.DiskFullFramework`` breaks.


Thanks,

James Peach

Re: Review Request 58941: Exposed full unreserved resources in /state endpoint on agent.

[Jie Yu]

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


Ship it!




Ship It!

- Jie Yu


On May 2, 2017, 11:33 p.m., Vinod Kone wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58941/
> ---
> 
> (Updated May 2, 2017, 11:33 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Neil Conway.
> 
> 
> Bugs: MESOS-7451
> https://issues.apache.org/jira/browse/MESOS-7451
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The JSON key for this information is "unreserved_resources_full".
> 
> 
> Diffs
> -
> 
>   src/slave/http.cpp 2ce345d9958a867ece835e47287215cefd561abf 
>   src/tests/reservation_endpoints_tests.cpp 
> cc8499a5ec05cf7b2283c075e47298918f50bd24 
> 
> 
> Diff: https://reviews.apache.org/r/58941/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Vinod Kone
> 
>

Re: Review Request 58940: Exposed full unreserved resources in /slaves endpoint on master.

[Jie Yu]

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


Ship it!




Ship It!

- Jie Yu


On May 2, 2017, 11:32 p.m., Vinod Kone wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58940/
> ---
> 
> (Updated May 2, 2017, 11:32 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Neil Conway.
> 
> 
> Bugs: MESOS-7451
> https://issues.apache.org/jira/browse/MESOS-7451
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The JSON key for this information is "unreserved_resources_full".
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp 5aae52870451d883ef1ea1fda5a27764d7f318e8 
>   src/tests/persistent_volume_endpoints_tests.cpp 
> 1237d081d781948975f66a8240ae9bdb5e819a3b 
> 
> 
> Diff: https://reviews.apache.org/r/58940/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Vinod Kone
> 
>

Re: Review Request 58224: Optionally verify the source IP address for libprocess messages.

[Mesos Reviewbot]

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



Patch looks great!

Reviews applied: [58977, 58928, 58224]

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

- Mesos Reviewbot


On May 4, 2017, 12:31 a.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58224/
> ---
> 
> (Updated May 4, 2017, 12:31 a.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Bugs: MESOS-7401
> https://issues.apache.org/jira/browse/MESOS-7401
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> In general, libprocess is unable to validate that a peer
> is a legitimate owner of the UPID it claims in a libprocess
> message. This change adds a check that the IP address in the
> UPID matches the peer address. This makes spoofing the UPID
> harder (eg. to send authenticated messages), but also breaks
> some legitimate configurations, particularly on multihomed
> hosts.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/process.cpp 
> f5b666f894215cb1861c244c94b382e0739bc5c9 
> 
> 
> Diff: https://reviews.apache.org/r/58224/diff/6/
> 
> 
> Testing
> ---
> 
> make check (Fedora 25). Light manual testing.
> 
> With LIBPROCESS_require_peer_address_ip_match=true, all Mesos tests pass 
> except ``ExamplesTest.DiskFullFramework``, however enabling this will 
> definitely break some libprocess APIs (though not in the way that Mesos uses 
> them) and legitimate multi-homed configurations. Note that setting 
> LIBPROCESS_ip=127.0.0.1 makes you multihomed for this purpose, which is why 
> ``ExamplesTest.DiskFullFramework`` breaks.
> 
> 
> Thanks,
> 
> James Peach
> 
>

Re: Review Request 58720: CLI: Extended the unit test infrastructure.

[Armand Grillet]

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

(Updated May 4, 2017, 1:59 p.m.)


Review request for mesos and Kevin Klues.


Changes
---

Fixed raised issues.


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


Repository: mesos


Description
---

This infrastructure includes the ability to bring up a test cluster to
run the CLI against. Future unit tests will use this infrastructure to
test their commands against a running mesos cluster. The tests require
some binaries created when building Mesos.


Diffs (updated)
-

  src/cli_new/lib/cli/tests/__init__.py PRE-CREATION 
  src/cli_new/lib/cli/tests/base.py PRE-CREATION 
  src/cli_new/lib/cli/tests/constants.py PRE-CREATION 
  src/cli_new/lib/cli/tests/tests.py PRE-CREATION 
  src/cli_new/pip-requirements.txt 28613e56a5c6d5c7606a7e58d6125b0c34748e83 
  src/cli_new/tests/main.py dff5d48b0ddae87960a78f9d05e4ae597912f1f6 


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

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


Testing
---

PEP8 and Pylint used to make sure that the code style is correct. Manuel test:

$ cd src/cli_new
$ ./bootstrap
$ source activate

(mesos-cli) $ mesos-cli-tests


Thanks,

Armand Grillet

Re: Review Request 58980: Passed '--default_container_info' to the command executor.

[Mesos Reviewbot]

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



Patch looks great!

Reviews applied: [58980]

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

- Mesos Reviewbot


On May 4, 2017, 12:12 a.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58980/
> ---
> 
> (Updated May 4, 2017, 12:12 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Gilbert Song, and Jie Yu.
> 
> 
> Bugs: mesos-7007
> https://issues.apache.org/jira/browse/mesos-7007
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> When there is no container info in the task info, copy that from the executor 
> info so the command executor can set up the container correctly.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/containerizer.cpp 
> b58baed64480e22f640a4852537f85922ed382ae 
> 
> 
> Diff: https://reviews.apache.org/r/58980/diff/1/
> 
> 
> Testing
> ---
> 
> sudo make check
> Manually tested on the scenario provided in mesos-7007.
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

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

[Mesos Reviewbot]

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



Patch looks great!

Reviews applied: [58939]

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

- Mesos Reviewbot


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

Review Request 58987: Add failure print when socket recv data.

[Andy Pang]

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

Review request for mesos, Jie Yu and Vinod Kone.


Repository: mesos


Description
---

Add failure print when socket recv data.


Diffs
-

  3rdparty/libprocess/src/process.cpp 96ce7db 


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


Testing
---

make check


Thanks,

Andy Pang

Re: Review Request 58974: Fixed flakiness in HierarchicalAllocatorTest.NestedRoleQuota.

[Mesos Reviewbot]

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



Patch looks great!

Reviews applied: [58974]

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

- Mesos Reviewbot


On May 3, 2017, 8:46 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58974/
> ---
> 
> (Updated May 3, 2017, 8:46 p.m.)
> 
> 
> Review request for mesos and Alexander Rukletsov.
> 
> 
> Bugs: MESOS-7457
> https://issues.apache.org/jira/browse/MESOS-7457
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed flakiness in HierarchicalAllocatorTest.NestedRoleQuota.
> 
> 
> Diffs
> -
> 
>   src/tests/hierarchical_allocator_tests.cpp 
> ebc4868a6b7e2a04cc202a74a4633969ade40aca 
> 
> 
> Diff: https://reviews.apache.org/r/58974/diff/1/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> I wasn't able to repro the flakiness on my local box, but the previous test 
> coding does seem wrong, now that `setQuota` doesn't trigger a batch 
> allocation.
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 58967: Set the working directory to parent task's for DEBUG containers.

[Mesos Reviewbot]

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



Patch looks great!

Reviews applied: [58847, 58262, 58718, 58817, 58818, 58819, 58820, 58263, 
58821, 58967]

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

- Mesos Reviewbot


On May 3, 2017, 4:46 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58967/
> ---
> 
> (Updated May 3, 2017, 4:46 p.m.)
> 
> 
> Review request for mesos, Gastón Kleiman, Jie Yu, and Vinod Kone.
> 
> 
> Bugs: MESOS-7433
> https://issues.apache.org/jira/browse/MESOS-7433
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> A DEBUG container's working directory should be set to the parent
> task's working directory. For the command executor case, even if
> the task itself has a root filesystem, the executor container still
> uses the host filesystem, hence
> `ContainerLaunchInfo.working_directory`, pointing to the executor's
> working directory in the host filesystem, may be different from the
> task's working directory in the root filesystem.
> 
> 
> Diffs
> -
> 
>   include/mesos/slave/containerizer.proto 
> c30b1fc659ee9b3cd343899638ced6408d8b51a2 
>   src/slave/containerizer/mesos/containerizer.cpp 
> b58baed64480e22f640a4852537f85922ed382ae 
> 
> 
> Diff: https://reviews.apache.org/r/58967/diff/1/
> 
> 
> Testing
> ---
> 
> make check on Mac OS and Fedora 24
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>