Re: Review Request 56681: Use glog to log EXIT() messages.

[Jiang Yan Xu]

> On March 20, 2017, 1:32 p.m., Benjamin Mahler wrote:
> > Have you seen the feedback 
> > [here](https://issues.apache.org/jira/browse/MESOS-3465?focusedCommentId=14944383=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-14944383)?
> >  It would be nice to make it clear in the log that the program is exiting 
> > IMHO.

I think you meant https://issues.apache.org/jira/browse/MESOS-3465


- Jiang Yan


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


On May 8, 2017, 4:30 p.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56681/
> ---
> 
> (Updated May 8, 2017, 4:30 p.m.)
> 
> 
> Review request for mesos, haosdent huang and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-7115
> https://issues.apache.org/jira/browse/MESOS-7115
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Use glog to log EXIT() messages.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/exit.hpp 
> 762864afd8f86f7e1f439ef6ea7e3965a5f147d5 
> 
> 
> Diff: https://reviews.apache.org/r/56681/diff/2/
> 
> 
> Testing
> ---
> 
> make check (Fedora 25)
> 
> **Example output after this change:**
> ```
> [jpeach@jpeach build]$ sudo ./src/mesos-agent --work_dir=/nothing/here 
> --master=phoney:5050
> WARNING: Logging before InitGoogleLogging() is written to STDERR
> I0320 11:01:10.362782   921 main.cpp:368] Build: 2017-03-20 10:43:40 by jpeach
> I0320 11:01:10.362900   921 main.cpp:369] Version: 1.3.0
> I0320 11:01:10.362912   921 main.cpp:376] Git SHA: 
> b0d82241a886e9c6239ce82c5135c4c72c44aca7
> I0320 11:01:10.381340   921 systemd.cpp:238] systemd version `231` detected
> I0320 11:01:10.381404   921 main.cpp:478] Inializing systemd state
> I0320 11:01:10.390058   921 systemd.cpp:326] Started systemd slice 
> `mesos_executors.slice`
> I0320 11:01:10.392071   921 containerizer.cpp:221] Using isolation: 
> posix/cpu,posix/mem,filesystem/posix,network/cni
> I0320 11:01:10.401108   921 linux_launcher.cpp:150] Using 
> /sys/fs/cgroup/freezer as the freezer hierarchy for the Linux launcher
> I0320 11:01:10.402873   921 provisioner.cpp:249] Using default backend 
> 'overlay'
> E0320 11:01:10.426219   921 main.cpp:507] Failed to create a master detector: 
> Failed to parse 'phoney:5050'
> ```
> 
> **Example output before this change:**
> ```
> [jpeach@jpeach build]$ sudo /opt/mesos/sbin//mesos-agent 
> --work_dir=/nothing/here --master=phoney:5050
> WARNING: Logging before InitGoogleLogging() is written to STDERR
> I0320 11:03:43.868015  1188 main.cpp:243] Build: 2017-03-10 10:49:55 by jpeach
> I0320 11:03:43.868135  1188 main.cpp:244] Version: 1.3.0
> I0320 11:03:43.868151  1188 main.cpp:251] Git SHA: 
> 487c667260ec89127e18c77b9e8c8c243cf6ec57
> I0320 11:03:43.886561  1188 systemd.cpp:238] systemd version `231` detected
> I0320 11:03:43.886631  1188 main.cpp:342] Inializing systemd state
> I0320 11:03:43.896606  1188 systemd.cpp:326] Started systemd slice 
> `mesos_executors.slice`
> I0320 11:03:43.899202  1188 containerizer.cpp:221] Using isolation: 
> posix/cpu,posix/mem,filesystem/posix,network/cni
> I0320 11:03:43.911183  1188 linux_launcher.cpp:150] Using 
> /sys/fs/cgroup/freezer as the freezer hierarchy for the Linux launcher
> I0320 11:03:43.913216  1188 provisioner.cpp:249] Using default backend 
> 'overlay'
> Failed to create a master detector: Failed to parse 'phoney:5050'
> ```
> 
> 
> Thanks,
> 
> James Peach
> 
>

Re: Review Request 59061: Fixed provisioner recover blockage by non-existing rootfses dir.

[Chun-Hung Hsiao]

> On May 8, 2017, 8:49 p.m., Jie Yu wrote:
> > src/slave/containerizer/mesos/provisioner/paths.cpp
> > Lines 210 (patched)
> > 
> >
> > Why `isdir`? not os::exists?
> 
> Gilbert Song wrote:
> Following the patern in this file. We are only using `stat::isdir` in 
> this helper file. I guess you want me to fix all?

I perfor the more specific `isdir` check here if the `rootfs` file is always a 
directory in all scenarios we want to ignore.


- Chun-Hung


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


On May 8, 2017, 7:51 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59061/
> ---
> 
> (Updated May 8, 2017, 7:51 p.m.)
> 
> 
> Review request for mesos, Chun-Hung Hsiao, Jie Yu, Michael Park, Neil Conway, 
> and Vinod Kone.
> 
> 
> Bugs: MESOS-7471
> https://issues.apache.org/jira/browse/MESOS-7471
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> In provisioner recover, when listing the container rootfses, it is
> possible that the 'rootfses' dir does not exist. Because a possible
> race between the provisioner destroy and the agent restart. For
> instance, while the provisioner is destroying the container dir the
> agent restarts. Due to os::rmdir() is recursive by traversing the
> FTS tree, it is possible that 'rootfses' dir is removed but the
> others (e.g., scratch dir) are not.
> 
> Currently, we are returning an error if the 'rootfses' dir does not
> exist, which blocks the agent from recovery. We should skip it if
> 'rootfses' does not exist.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/provisioner/paths.cpp 
> d2de98b66b7cf8331e82c31d0602dfe7e77d2130 
> 
> 
> Diff: https://reviews.apache.org/r/59061/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 59061: Fixed provisioner recover blockage by non-existing rootfses dir.

[Mesos Reviewbot]

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



Patch looks great!

Reviews applied: [59061]

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 8, 2017, 12:51 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59061/
> ---
> 
> (Updated May 8, 2017, 12:51 p.m.)
> 
> 
> Review request for mesos, Chun-Hung Hsiao, Jie Yu, Michael Park, Neil Conway, 
> and Vinod Kone.
> 
> 
> Bugs: MESOS-7471
> https://issues.apache.org/jira/browse/MESOS-7471
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> In provisioner recover, when listing the container rootfses, it is
> possible that the 'rootfses' dir does not exist. Because a possible
> race between the provisioner destroy and the agent restart. For
> instance, while the provisioner is destroying the container dir the
> agent restarts. Due to os::rmdir() is recursive by traversing the
> FTS tree, it is possible that 'rootfses' dir is removed but the
> others (e.g., scratch dir) are not.
> 
> Currently, we are returning an error if the 'rootfses' dir does not
> exist, which blocks the agent from recovery. We should skip it if
> 'rootfses' does not exist.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/provisioner/paths.cpp 
> d2de98b66b7cf8331e82c31d0602dfe7e77d2130 
> 
> 
> Diff: https://reviews.apache.org/r/59061/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 59015: Implemented passing docker config to URIs.

[Vinod Kone]

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




src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp
Lines 270-271 (patched)


am i reading it correctly that `None` represents principal here? i thought 
both username and password are inside the docker config, so i'm a bit confused.


- Vinod Kone


On May 4, 2017, 11:22 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59015/
> ---
> 
> (Updated May 4, 2017, 11:22 p.m.)
> 
> 
> 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
> 
>

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

[Vinod Kone]

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


Ship it!




Ship It!

- Vinod Kone


On May 5, 2017, 8:30 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59014/
> ---
> 
> (Updated May 5, 2017, 8:30 p.m.)
> 
> 
> 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/2/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

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

[Vinod Kone]

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


Ship it!





src/slave/containerizer/mesos/provisioner/docker/store.cpp
Line 235 (original), 243-244 (patched)


I'm assuming you need `config` here explicitly and cannot get it from 
`image` below?


- Vinod Kone


On May 4, 2017, 11:22 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59013/
> ---
> 
> (Updated May 4, 2017, 11:22 p.m.)
> 
> 
> 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
> 
>

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

[Vinod Kone]

> On May 5, 2017, 11:55 p.m., Chun-Hung Hsiao wrote:
> > Should we use a shared pointer for `SecretResolver` instead of passing a 
> > `SecretResolver*` around?

+1. Since provisioner and env isolator are both sharing the pointer, it's 
probably worth having the containerizer take in a shared pointer instead of raw 
pointer. cc @karya.


- Vinod


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


On May 5, 2017, 11:16 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59012/
> ---
> 
> (Updated May 5, 2017, 11:16 p.m.)
> 
> 
> 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/3/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

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

[Vinod Kone]

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



Updated summary/description to "SecretResolver"


src/slave/containerizer/mesos/provisioner/docker/puller.hpp
Lines 48 (patched)


no need for a default?



src/slave/containerizer/mesos/provisioner/provisioner.hpp
Lines 77 (patched)


is the default needed here to avoid updating tests?



src/slave/containerizer/mesos/provisioner/store.hpp
Lines 64 (patched)


ditto.


- Vinod Kone


On May 5, 2017, 11:16 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59012/
> ---
> 
> (Updated May 5, 2017, 11:16 p.m.)
> 
> 
> 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/3/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

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

[Vinod Kone]

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


Ship it!




Ship It!

- Vinod Kone


On May 4, 2017, 11:22 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59011/
> ---
> 
> (Updated May 4, 2017, 11:22 p.m.)
> 
> 
> 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
> 
>

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

[Vinod Kone]

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


Ship it!




Ship It!

- Vinod Kone


On May 5, 2017, 8:30 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59010/
> ---
> 
> (Updated May 5, 2017, 8:30 p.m.)
> 
> 
> 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/2/
> 
> 
> Testing
> ---
> 
> N/A.
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 59000: Added environment secret isolator.

[Vinod Kone]

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




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


should this be "environment/secret" instead? i'm assuming there might be 
other env based isolators in the future.



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


s/have been/should have been/



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


s/environment/Enviornment/

log the variable name?



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


log variable name.



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


can you use `collect` instead?



src/tests/containerizer/environment_secret_isolator_tests.cpp
Lines 52 (patched)


kill the comment.



src/tests/containerizer/environment_secret_isolator_tests.cpp
Lines 76 (patched)


no need for `Times(1)`


- Vinod Kone


On May 8, 2017, 10:19 p.m., Kapil Arya wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59000/
> ---
> 
> (Updated May 8, 2017, 10:19 p.m.)
> 
> 
> Review request for mesos, Gilbert Song, Greg Mann, Jie Yu, and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added environment secret isolator.
> 
> 
> Diffs
> -
> 
>   src/CMakeLists.txt 40d921ee7026f5ac47efbf0243e1cafab57825f9 
>   src/Makefile.am 6bb81fd49b4564a0afa993b2cef6baa9d370ee7a 
>   src/launcher/executor.cpp b05f73e539c22d4d40f07df76168a06373b818d4 
>   src/slave/containerizer/mesos/containerizer.cpp 
> 58ab74571fb14c6dbb1907151dc421f93e324bb5 
>   src/slave/containerizer/mesos/isolators/environment_secret.hpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/environment_secret.cpp PRE-CREATION 
>   src/tests/CMakeLists.txt 9f2af9cdd1cf50485f4cd84ce67bcceba64b9328 
>   src/tests/containerizer/environment_secret_isolator_tests.cpp PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/59000/diff/4/
> 
> 
> Testing
> ---
> 
> Added a new test and ran `make check`.
> 
> 
> Thanks,
> 
> Kapil Arya
> 
>

Re: Review Request 59061: Fixed provisioner recover blockage by non-existing rootfses dir.

[Gilbert Song]

> On May 8, 2017, 1:49 p.m., Jie Yu wrote:
> > src/slave/containerizer/mesos/provisioner/paths.cpp
> > Lines 210 (patched)
> > 
> >
> > Why `isdir`? not os::exists?

Following the patern in this file. We are only using `stat::isdir` in this 
helper file. I guess you want me to fix all?


- Gilbert


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


On May 8, 2017, 12:51 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59061/
> ---
> 
> (Updated May 8, 2017, 12:51 p.m.)
> 
> 
> Review request for mesos, Chun-Hung Hsiao, Jie Yu, Michael Park, Neil Conway, 
> and Vinod Kone.
> 
> 
> Bugs: MESOS-7471
> https://issues.apache.org/jira/browse/MESOS-7471
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> In provisioner recover, when listing the container rootfses, it is
> possible that the 'rootfses' dir does not exist. Because a possible
> race between the provisioner destroy and the agent restart. For
> instance, while the provisioner is destroying the container dir the
> agent restarts. Due to os::rmdir() is recursive by traversing the
> FTS tree, it is possible that 'rootfses' dir is removed but the
> others (e.g., scratch dir) are not.
> 
> Currently, we are returning an error if the 'rootfses' dir does not
> exist, which blocks the agent from recovery. We should skip it if
> 'rootfses' does not exist.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/provisioner/paths.cpp 
> d2de98b66b7cf8331e82c31d0602dfe7e77d2130 
> 
> 
> Diff: https://reviews.apache.org/r/59061/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 58999: Added --secret_resolver flag to agent.

[Vinod Kone]

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




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


why this include?



src/slave/containerizer/containerizer.cpp
Lines 25 (patched)


ditto?



src/slave/containerizer/mesos/containerizer.cpp
Lines 335-338 (patched)


why is this introduced in this review?



src/slave/flags.cpp
Lines 963 (patched)


The name of the secret resolver to use for resolving enivornment and file 
based secrets.


- Vinod Kone


On May 8, 2017, 10:19 p.m., Kapil Arya wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58999/
> ---
> 
> (Updated May 8, 2017, 10:19 p.m.)
> 
> 
> Review request for mesos, Gilbert Song, Jie Yu, and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated Containerizer to accept SecretResolver.
> 
> 
> 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 
> 58ab74571fb14c6dbb1907151dc421f93e324bb5 
>   src/slave/flags.hpp e5784ef81ad0720c7ec061ee0b28b8fadae77afd 
>   src/slave/flags.cpp ed99fadbf1aa91f7f0a57c9fb351c0247a40c6f4 
>   src/slave/main.cpp 507d59996a90f51c7cd1e0f836a34c2e9c484791 
>   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/4/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Kapil Arya
> 
>

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

[Vinod Kone]

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


Fix it, then Ship it!





src/secret/resolver.cpp
Lines 40 (patched)


The comment about default resolver behavior in the previous review should 
be here instead.



src/secret/resolver.cpp
Lines 72 (patched)


new line above.


- Vinod Kone


On May 8, 2017, 10:19 p.m., Kapil Arya wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58760/
> ---
> 
> (Updated May 8, 2017, 10:19 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 resolver module.
> 
> 
> Diffs
> -
> 
>   src/CMakeLists.txt 40d921ee7026f5ac47efbf0243e1cafab57825f9 
>   src/Makefile.am 6bb81fd49b4564a0afa993b2cef6baa9d370ee7a 
>   src/secret/resolver.cpp PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/58760/diff/6/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Kapil Arya
> 
>

Re: Review Request 58759: Introduced SecretResolver module interface.

[Vinod Kone]

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


Fix it, then Ship it!





include/mesos/module.hpp
Lines 113 (patched)


s/to during/during/ ?



include/mesos/secret/resolver.hpp
Lines 30 (patched)


s/::value/::Value/



include/mesos/secret/resolver.hpp
Lines 31 (patched)


s/cannot/secret cannot/



include/mesos/secret/resolver.hpp
Lines 41 (patched)


s/instanciated/instantiated/



include/mesos/secret/resolver.hpp
Lines 42-47 (patched)


Do we typically talk about default implementations here?



include/mesos/secret/resolver.hpp
Lines 54 (patched)


s/to a/a/


- Vinod Kone


On May 8, 2017, 10:19 p.m., Kapil Arya wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58759/
> ---
> 
> (Updated May 8, 2017, 10:19 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 SecretResolver module interface.
> 
> 
> Diffs
> -
> 
>   include/mesos/module.hpp c28d01df54906b2b8ebfe7ac9e719a0d1be45614 
>   include/mesos/module/secret_resolver.hpp PRE-CREATION 
>   include/mesos/secret/resolver.hpp PRE-CREATION 
>   src/Makefile.am 6bb81fd49b4564a0afa993b2cef6baa9d370ee7a 
>   src/module/manager.cpp 7d875fcb7fcec0d57274e644b0a3b67b333ac193 
> 
> 
> Diff: https://reviews.apache.org/r/58759/diff/6/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Kapil Arya
> 
>

Review Request 59074: Updated all tests that use Containerizer::launch(...).

[Joseph Wu]

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

Review request for mesos and Jie Yu.


Bugs: MESOS-7304 and MESOS-7449
https://issues.apache.org/jira/browse/MESOS-7304
https://issues.apache.org/jira/browse/MESOS-7449


Repository: mesos


Description
---

This sweeps through the unit tests and translates all existing
tests that use the two variants of Containerizer::launch(...)
(i.e. nested and non-nested) and translates the arguments to the
new interface.  The fetcher interface changes are also addressed
in this sweep.


Diffs
-

  src/tests/api_tests.cpp faf3242f9c86d866c4bb5e457fcfe47c1063cc09 
  src/tests/check_tests.cpp ec0d5ee78a94991c68f38174d09b41e8f2e4be35 
  src/tests/command_executor_tests.cpp da4b653303d436309f6cb190ad6b600b8d934678 
  src/tests/container_logger_tests.cpp 28436b6b67c1251d877064751e695c6696725a23 
  src/tests/containerizer/cgroups_isolator_tests.cpp 
4e1d027bde7f0ab51e778bfd3bac8797fc7f7f1b 
  src/tests/containerizer/cni_isolator_tests.cpp 
565e58ae75e918453e4386f5e35a5a844a8b15f8 
  src/tests/containerizer/composing_containerizer_tests.cpp 
d7fd6216080f2f437503b898bff6b1046317933e 
  src/tests/containerizer/cpu_isolator_tests.cpp 
3c6f7481de2d6d1f6abf9850bd33cea44931480a 
  src/tests/containerizer/docker_volume_isolator_tests.cpp 
b47a6b5081a63ac474ac4634701b1a572eb58137 
  src/tests/containerizer/isolator_tests.cpp 
355e15ff69ca6bdd94821f6566fd09a280d03b47 
  src/tests/containerizer/linux_filesystem_isolator_tests.cpp 
70a0dce0f0166fcb3c15a03a89151152954abbac 
  src/tests/containerizer/memory_isolator_tests.cpp 
0df4aa892b531eb305eb3012862ad25527344ab6 
  src/tests/containerizer/memory_pressure_tests.cpp 
c4ad779906ee31d07fdd8a6fc0e51e1edd1cbe85 
  src/tests/containerizer/port_mapping_tests.cpp 
a528382e8b4831b9c7e8dcc877a5e242909f0cd5 
  src/tests/containerizer/volume_image_isolator_tests.cpp 
ad1884459e87ece767e1019adcf702a2f2ff8f1c 
  src/tests/containerizer/xfs_quota_tests.cpp 
7beb60b059910a0d4451b1ace895a35dc974a043 
  src/tests/disk_quota_tests.cpp f758bee07c448afe09dc380e99c4a81e2dd9f650 
  src/tests/fetcher_cache_tests.cpp 3bd63ed0a66493829a82c542ad05ebe0f7828d1a 
  src/tests/fetcher_tests.cpp 27ea7244c713934ef5a721ec133804a006dcb26e 
  src/tests/health_check_tests.cpp 6c1b9a0ead6edb34c20cf4973fffcff913c5d7ad 
  src/tests/hook_tests.cpp 02d8f800c3eb9b1e617a14c78c2ef1e45d1c72bb 
  src/tests/master_tests.cpp ceee2f4a5d38e0f4200f444769e058d2173de821 
  src/tests/slave_recovery_tests.cpp 52e78b6b6280a159233b402ce2849448204d4f11 
  src/tests/slave_tests.cpp 8c97dc6d088708d301dc3ccf90d413fd785b782f 


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


Testing
---

make check

Currently fails a few of the fetcher tests, which might need tweaking (these 
tests assume the fetcher caches things in the current directory):
```
[  FAILED  ] FetcherTest.RelativeFilePath
[  FAILED  ] FetcherCacheTest.SimpleEviction
[  FAILED  ] FetcherCacheTest.FallbackFromEviction
[  FAILED  ] FetcherCacheTest.RemoveLRUCacheEntries
```


Thanks,

Joseph Wu

Review Request 59073: Updated Docker containerizer tests to use the old naming scheme.

[Joseph Wu]

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

Review request for mesos and Jie Yu.


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


Repository: mesos


Description
---

This changes test expectations of Docker container names back to the
0.22 scheme and simultaneously updates the tests to the new
containerizer interface.


Diffs
-

  src/tests/containerizer/docker_containerizer_tests.cpp 
4eef399b05f6e35d75c7c23992f0ccda04576277 


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


Testing
---

See last patch in chain.


Thanks,

Joseph Wu

Re: Review Request 58910: Addressed a TODO about checkpointing in tests.

[Joseph Wu]

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

(Updated May 8, 2017, 4:22 p.m.)


Review request for mesos and Jie Yu.


Changes
---

Split part of this patch into two other patches.  Two patches contain 
refactoring plus slight tweaks in functionality.  The last patch contains only 
refactoring.


Summary (updated)
-

Addressed a TODO about checkpointing in tests.


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


Repository: mesos


Description (updated)
---

Checkpointing only plays a role in tests when the containerizer
is destroyed and subsequently recovered.  This commit updates a
couple of test files to the new containerizer interface and
also turns checkpointing _off_ in tests which do not perform any
containerizer recovery.


Diffs (updated)
-

  src/tests/containerizer/io_switchboard_tests.cpp 
b19961ee91af3c174d4377c2514465d6c99cf331 
  src/tests/containerizer/mesos_containerizer_tests.cpp 
13e0f7e603a3ffdd0965b253d7abfe6a069cd2b4 
  src/tests/containerizer/nested_mesos_containerizer_tests.cpp 
c163b882fb2fc463537d6906c5a47b24a9a756c4 
  src/tests/containerizer/volume_sandbox_path_isolator_tests.cpp 
5f4e382baf80439b9a61f22c1912cadeba1109b7 


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

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


Testing (updated)
---

See last patch in chain.


Thanks,

Joseph Wu

Re: Review Request 59068: Updated release/versioning docs for "-dev" version scheme.

[Vinod Kone]

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




docs/release-guide.md
Lines 150 (patched)


Shouldn't the release branch also have "-dev" suffix to indicate 
development of the next patch version to be consistent? 

Maybe after a release has been voted/tagged, we need to update the version 
in the release branch to next patch version with "-dev"? Not sure if this 
breaks our release scripts though.


- Vinod Kone


On May 8, 2017, 10:01 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59068/
> ---
> 
> (Updated May 8, 2017, 10:01 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-7473
> https://issues.apache.org/jira/browse/MESOS-7473
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Mesos now uses the "-dev" prerelease label to identify code in the
> master branch in the period before the next release branch has been
> cut.
> 
> For example, after creating the 1.3.x release branch, we previously
> updated the version number in the master branch to be "1.4.0". The new
> policy is to instead use the version number "1.4.0-dev" in the master
> branch. When the release branch for 1.4.x is created, the version number
> will then be updated to remove the "-dev" prerelease label.
> 
> This commit also makes a few minor procedural changes to the release
> process -- for example, we now create the release branch first and then
> tag a commit in that branch, rather than first tagging a release and
> then creating the branch from the tag.
> 
> 
> Diffs
> -
> 
>   docs/release-guide.md fa607287456dff5e078fe97690ef3728a6419e8a 
>   docs/versioning.md 178026ccca3569ee295291381f4b43ed17fcd4fe 
> 
> 
> Diff: https://reviews.apache.org/r/59068/diff/1/
> 
> 
> Testing
> ---
> 
> Visual inspection, previewed with github gist.
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 58908: Updated test mocks per interface changes.

[Joseph Wu]

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

(Updated May 8, 2017, 3:45 p.m.)


Review request for mesos and Jie Yu.


Changes
---

Fixed how the test containerizer's two launch paths were combined.


Bugs: MESOS-7304 and MESOS-7449
https://issues.apache.org/jira/browse/MESOS-7304
https://issues.apache.org/jira/browse/MESOS-7449


Repository: mesos


Description
---

This updates the mocks for the containerizers and fetcher after
the interface changes.  For the containerizers, the two launch methods
have been combined into a single method.

The fetcher now has an argument in its constructor and no longer
takes some arguments (SlaveID and Flags) in its methods.

Launching nested containers via the test containerizer was
originally a no-op, so similar behavior is implemented in the
combined method.


Diffs (updated)
-

  src/tests/cluster.cpp a4f57e0e9c4f221c42bdbb36d8925f688b3d742f 
  src/tests/containerizer.hpp 63fe2366d5fe79c71cbe62813a9bb0b830e70057 
  src/tests/containerizer.cpp 548da3a8757c9ad3ab115c6b91f8a1f2f5e37144 
  src/tests/containerizer/mock_containerizer.hpp 
ca0ae053b1eed01d6c04f581cd08485beec0c5fb 
  src/tests/mesos.hpp 3c57f25ff0bec55582b8a7decef53be61a9ec6f4 
  src/tests/mesos.cpp a79ec62d6b8354a97a50533dc66e04c1afc4bef6 
  src/tests/mock_docker.hpp f58211d93e3231bfef7afa9e495930a70aae9f2b 
  src/tests/mock_docker.cpp 81a14ca346f0fbe6458e03b06932e1ad00a4a238 


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

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


Testing
---

See last patch in chain.


Thanks,

Joseph Wu

Re: Review Request 58977: Add local and peer address accessors to http::Connection.

[Benjamin Mahler]

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




3rdparty/libprocess/include/process/http.hpp
Lines 960-968 (patched)


Hm.. is it possible to make these synchronous? Since these are synchronous 
accesses to the state, it seems unfortuante to be returning futures here, I can 
imagine this will be burdensome on the callers.

Is it possible at construction time of `Connection` to get this information 
out of the socket that is passed in and store it within `Data`? That way, we 
could just read them out synchronously here.


- Benjamin Mahler


On May 3, 2017, 10:35 p.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58977/
> ---
> 
> (Updated May 3, 2017, 10:35 p.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Bugs: MESOS-7401
> https://issues.apache.org/jira/browse/MESOS-7401
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add local and peer address accessors to http::Connection.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/http.hpp 
> 650b9d8aaba5636e1a445abf9e27e018ff82e610 
>   3rdparty/libprocess/src/http.cpp 9789607933745f1fc4e37f47ce1be6aecb33a6e6 
> 
> 
> Diff: https://reviews.apache.org/r/58977/diff/1/
> 
> 
> Testing
> ---
> 
> make check (Fedora 25)
> 
> 
> Thanks,
> 
> James Peach
> 
>

Re: Review Request 58900: Changed the default fetcher cache directory.

[Joseph Wu]

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



Request for comment...


src/slave/containerizer/fetcher.cpp
Lines 261 (patched)


This patch is still missing cleanup for this directory.

Before this patch, we would "leak" fetcher cache directories whenever the 
SlaveID changed.  This change will make the fetcher leak the cache every time 
the agent is started.

I'm considering the following options:
1) Change the `--fetcher_cache_dir` to be the cache directory, rather than 
the _parent_ of the cache directory.  This has some implications when running 
multiple agents one a single machine (as the default value of the flag will no 
longer be acceptable).  But with a single cache directory, managing leaks is as 
easy as clearing the directory on startup.
2) Delete the fetcher cache directory in the fetcher's destructor.  This 
will only really do anything when the agent is gracefully shut down.  But 
graceful shutdowns can be considered somewhat rare.
3) Checkpoint the PID of the current process inside the fetcher cache 
directory.  Whenever the agent starts, it could scan for these PIDs and check 
which processes are still running.  We can then delete the caches of dead 
processes.


- Joseph Wu


On May 8, 2017, 3:20 p.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58900/
> ---
> 
> (Updated May 8, 2017, 3:20 p.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-7304
> https://issues.apache.org/jira/browse/MESOS-7304
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The fetcher cache directory was historically located (by default)
> in `/tmp/mesos/fetch`.  The agent flag `--fetcher_cache_dir` could
> be used to change this value.
> 
> The fetcher would create a subdirectory underneath `/tmp/mesos/fetch`
> for each `SlaveID`.  This was done because multiple agents can run on
> the same node.  If all the agents use the same default fetcher cache
> directory, they will collide and cause unpredictable results.
> As a result, the `SlaveID` needed to be passed into the fetcher
> after the agent recovers and/or registers with the master, because
> that is when the `SlaveID` is determined.
> 
> This changes the default fetcher cache directory to
> `/tmp/mesos/fetch/XX`, where the 6 X's are replaced by `::mkdtemp`.
> The `SlaveID` subdirectory has also been removed.
> 
> This change, while techically a breaking change, is safe because of
> how the fetcher uses this directory.  Upon starting up, the fetcher
> "recovers" by clearing this directory.  By using a temporary directory,
> we similarly get an empty fetcher cache upon startup.
> 
> This change will only cause breakages if multiple agents are run
> with the same `--fetcher_cache_dir`.  In this case, each agent
> will delete the fetcher caches of all the other agents.
> 
> ---
> 
> With the removal of the `SlaveID` field in the fetcher's methods,
> it is no longer necessary to pass in the `SlaveID` or agent Flags
> at agent recovery time.  Instead, the flags can be passed in during
> the fetcher's construction.
> 
> Similarly, the fetcher's "recovery" (clearing the fetcher cache)
> can be done immediately upon construction, which simplifies the
> code slightly.
> 
> 
> Diffs
> -
> 
>   src/local/local.cpp e47980929db2da1f31cf899a0e1fc452070e11f3 
>   src/slave/containerizer/fetcher.hpp 
> 9e3018dc087ed55c61b2824d0105bc5339b83043 
>   src/slave/containerizer/fetcher.cpp 
> a910fea5a5556afb376524c5bb2ff98d7d84e611 
>   src/slave/flags.hpp e5784ef81ad0720c7ec061ee0b28b8fadae77afd 
>   src/slave/flags.cpp ed99fadbf1aa91f7f0a57c9fb351c0247a40c6f4 
>   src/slave/main.cpp 507d59996a90f51c7cd1e0f836a34c2e9c484791 
>   src/slave/slave.cpp a37c6c888e7573209aadb07576cfb727fa1ec4ff 
> 
> 
> Diff: https://reviews.apache.org/r/58900/diff/2/
> 
> 
> Testing
> ---
> 
> See last patch in chain.
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Re: 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/
---

(Updated May 8, 2017, 6:20 p.m.)


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


Changes
---

Use ramfs instead of tmpfs; Added nested containerizer tests.


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


Repository: mesos


Description
---

Added volume secret isolator.


Diffs (updated)
-

  src/CMakeLists.txt 40d921ee7026f5ac47efbf0243e1cafab57825f9 
  src/Makefile.am 6bb81fd49b4564a0afa993b2cef6baa9d370ee7a 
  src/slave/containerizer/mesos/containerizer.cpp 
58ab74571fb14c6dbb1907151dc421f93e324bb5 
  src/slave/containerizer/mesos/isolators/volume/secret.hpp PRE-CREATION 
  src/slave/containerizer/mesos/isolators/volume/secret.cpp PRE-CREATION 
  src/tests/CMakeLists.txt 9f2af9cdd1cf50485f4cd84ce67bcceba64b9328 
  src/tests/containerizer/rootfs.cpp fdfecc65a3fcd19d6a4dfa574320f4d1f2755322 
  src/tests/containerizer/volume_secret_isolator_tests.cpp PRE-CREATION 


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

Changes: https://reviews.apache.org/r/59001/diff/4-5/


Testing
---

Added new tests an ran `make check`.


Thanks,

Kapil Arya

Re: Review Request 58900: Changed the default fetcher cache directory.

[Joseph Wu]

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

(Updated May 8, 2017, 3:20 p.m.)


Review request for mesos and Jie Yu.


Changes
---

Changed when the fetcher cache directory is generated (in the default case).  
Instead of being generated by the flags, it is instead generated by the fetcher 
process.


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


Repository: mesos


Description (updated)
---

The fetcher cache directory was historically located (by default)
in `/tmp/mesos/fetch`.  The agent flag `--fetcher_cache_dir` could
be used to change this value.

The fetcher would create a subdirectory underneath `/tmp/mesos/fetch`
for each `SlaveID`.  This was done because multiple agents can run on
the same node.  If all the agents use the same default fetcher cache
directory, they will collide and cause unpredictable results.
As a result, the `SlaveID` needed to be passed into the fetcher
after the agent recovers and/or registers with the master, because
that is when the `SlaveID` is determined.

This changes the default fetcher cache directory to
`/tmp/mesos/fetch/XX`, where the 6 X's are replaced by `::mkdtemp`.
The `SlaveID` subdirectory has also been removed.

This change, while techically a breaking change, is safe because of
how the fetcher uses this directory.  Upon starting up, the fetcher
"recovers" by clearing this directory.  By using a temporary directory,
we similarly get an empty fetcher cache upon startup.

This change will only cause breakages if multiple agents are run
with the same `--fetcher_cache_dir`.  In this case, each agent
will delete the fetcher caches of all the other agents.

---

With the removal of the `SlaveID` field in the fetcher's methods,
it is no longer necessary to pass in the `SlaveID` or agent Flags
at agent recovery time.  Instead, the flags can be passed in during
the fetcher's construction.

Similarly, the fetcher's "recovery" (clearing the fetcher cache)
can be done immediately upon construction, which simplifies the
code slightly.


Diffs (updated)
-

  src/local/local.cpp e47980929db2da1f31cf899a0e1fc452070e11f3 
  src/slave/containerizer/fetcher.hpp 9e3018dc087ed55c61b2824d0105bc5339b83043 
  src/slave/containerizer/fetcher.cpp a910fea5a5556afb376524c5bb2ff98d7d84e611 
  src/slave/flags.hpp e5784ef81ad0720c7ec061ee0b28b8fadae77afd 
  src/slave/flags.cpp ed99fadbf1aa91f7f0a57c9fb351c0247a40c6f4 
  src/slave/main.cpp 507d59996a90f51c7cd1e0f836a34c2e9c484791 
  src/slave/slave.cpp a37c6c888e7573209aadb07576cfb727fa1ec4ff 


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

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


Testing
---

See last patch in chain.


Thanks,

Joseph Wu

Re: 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/
---

(Updated May 8, 2017, 6:19 p.m.)


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


Changes
---

Rebased


Repository: mesos


Description
---

Added environment secret isolator.


Diffs (updated)
-

  src/CMakeLists.txt 40d921ee7026f5ac47efbf0243e1cafab57825f9 
  src/Makefile.am 6bb81fd49b4564a0afa993b2cef6baa9d370ee7a 
  src/launcher/executor.cpp b05f73e539c22d4d40f07df76168a06373b818d4 
  src/slave/containerizer/mesos/containerizer.cpp 
58ab74571fb14c6dbb1907151dc421f93e324bb5 
  src/slave/containerizer/mesos/isolators/environment_secret.hpp PRE-CREATION 
  src/slave/containerizer/mesos/isolators/environment_secret.cpp PRE-CREATION 
  src/tests/CMakeLists.txt 9f2af9cdd1cf50485f4cd84ce67bcceba64b9328 
  src/tests/containerizer/environment_secret_isolator_tests.cpp PRE-CREATION 


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

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


Testing
---

Added a new test and ran `make check`.


Thanks,

Kapil Arya

Re: Review Request 58999: Added --secret_resolver flag to agent.

[Kapil Arya]

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

(Updated May 8, 2017, 6:19 p.m.)


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


Changes
---

Rebased


Repository: mesos


Description
---

Updated Containerizer to accept SecretResolver.


Diffs (updated)
-

  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 
58ab74571fb14c6dbb1907151dc421f93e324bb5 
  src/slave/flags.hpp e5784ef81ad0720c7ec061ee0b28b8fadae77afd 
  src/slave/flags.cpp ed99fadbf1aa91f7f0a57c9fb351c0247a40c6f4 
  src/slave/main.cpp 507d59996a90f51c7cd1e0f836a34c2e9c484791 
  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/4/

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


Testing
---


Thanks,

Kapil Arya

Re: Review Request 58759: Introduced SecretResolver module interface.

[Kapil Arya]

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

(Updated May 8, 2017, 6:19 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
---

Introduced SecretResolver module interface.


Diffs (updated)
-

  include/mesos/module.hpp c28d01df54906b2b8ebfe7ac9e719a0d1be45614 
  include/mesos/module/secret_resolver.hpp PRE-CREATION 
  include/mesos/secret/resolver.hpp PRE-CREATION 
  src/Makefile.am 6bb81fd49b4564a0afa993b2cef6baa9d370ee7a 
  src/module/manager.cpp 7d875fcb7fcec0d57274e644b0a3b67b333ac193 


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

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


Testing
---


Thanks,

Kapil Arya

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

[Kapil Arya]

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

(Updated May 8, 2017, 6:19 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
---

Added default secret resolver module.


Diffs (updated)
-

  src/CMakeLists.txt 40d921ee7026f5ac47efbf0243e1cafab57825f9 
  src/Makefile.am 6bb81fd49b4564a0afa993b2cef6baa9d370ee7a 
  src/secret/resolver.cpp PRE-CREATION 


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

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


Testing
---


Thanks,

Kapil Arya

Re: Review Request 58847: Checkpointed and recovered ContainerLaunchInfo for non-orphans.

[Gastón Kleiman]

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


Ship it!




Ship It!

- Gastón Kleiman


On May 3, 2017, 4:45 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58847/
> ---
> 
> (Updated May 3, 2017, 4:45 p.m.)
> 
> 
> Review request for mesos, Gastón Kleiman, Jie Yu, and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> In mesos containerizer, cache each container's launch info and
> recover it for non-orphan containers. This is a prerequisite for
> persisting some container characteristics across agent failover.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/containerizer.hpp 
> 29a99f33e646593127b9dc126f398f7bca88e21d 
>   src/slave/containerizer/mesos/containerizer.cpp 
> b58baed64480e22f640a4852537f85922ed382ae 
>   src/slave/containerizer/mesos/paths.hpp 
> d85fd34660faacd9c73de2ba7b87b3bbd4b6007b 
>   src/slave/containerizer/mesos/paths.cpp 
> ed4bbd2491e71ad1e4a41e0575b514377d02da9b 
> 
> 
> Diff: https://reviews.apache.org/r/58847/diff/2/
> 
> 
> Testing
> ---
> 
> See https://reviews.apache.org/r/58718/
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 59068: Updated release/versioning docs for "-dev" version scheme.

[Neil Conway]

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

(Updated May 8, 2017, 10:01 p.m.)


Review request for mesos and Vinod Kone.


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


Repository: mesos


Description
---

Mesos now uses the "-dev" prerelease label to identify code in the
master branch in the period before the next release branch has been
cut.

For example, after creating the 1.3.x release branch, we previously
updated the version number in the master branch to be "1.4.0". The new
policy is to instead use the version number "1.4.0-dev" in the master
branch. When the release branch for 1.4.x is created, the version number
will then be updated to remove the "-dev" prerelease label.

This commit also makes a few minor procedural changes to the release
process -- for example, we now create the release branch first and then
tag a commit in that branch, rather than first tagging a release and
then creating the branch from the tag.


Diffs
-

  docs/release-guide.md fa607287456dff5e078fe97690ef3728a6419e8a 
  docs/versioning.md 178026ccca3569ee295291381f4b43ed17fcd4fe 


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


Testing (updated)
---

Visual inspection, previewed with github gist.


Thanks,

Neil Conway

Review Request 59069: Changed version number to "1.4.0-dev".

[Neil Conway]

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

Review request for mesos and Vinod Kone.


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


Repository: mesos


Description
---

Per recent change to project versioning/release policy.


Diffs
-

  CMakeLists.txt 4ba6dd04ad594b0087784d12b785894d9ae257ac 
  cmake/MesosConfigure.cmake e44f662b6da71caa493cc568a5e5bcb0a608a8b1 
  configure.ac fe5b20b53ea007dd8f2e3349e9904f6c4f43a14b 
  include/mesos/version.hpp.in 63b6d95f9c29435ab8743e3369d85386a0e46b69 


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


Testing
---

`make check` with autotools and cmake on OSX.


Thanks,

Neil Conway

Review Request 59068: Updated release/versioning docs for "-dev" version scheme.

[Neil Conway]

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

Review request for mesos.


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


Repository: mesos


Description
---

Mesos now uses the "-dev" prerelease label to identify code in the
master branch in the period before the next release branch has been
cut.

For example, after creating the 1.3.x release branch, we previously
updated the version number in the master branch to be "1.4.0". The new
policy is to instead use the version number "1.4.0-dev" in the master
branch. When the release branch for 1.4.x is created, the version number
will then be updated to remove the "-dev" prerelease label.

This commit also makes a few minor procedural changes to the release
process -- for example, we now create the release branch first and then
tag a commit in that branch, rather than first tagging a release and
then creating the branch from the tag.


Diffs
-

  docs/release-guide.md fa607287456dff5e078fe97690ef3728a6419e8a 
  docs/versioning.md 178026ccca3569ee295291381f4b43ed17fcd4fe 


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


Testing
---


Thanks,

Neil Conway

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

[Chun-Hung Hsiao]

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

(Updated May 8, 2017, 9:43 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 (updated)
-

  src/slave/containerizer/mesos/containerizer.cpp 
58ab74571fb14c6dbb1907151dc421f93e324bb5 
  src/slave/containerizer/mesos/isolators/docker/runtime.cpp 
2a6e0b179394e0485d2495ceb4bbbcb184af08fe 
  src/tests/containerizer/docker_volume_isolator_tests.cpp 
b47a6b5081a63ac474ac4634701b1a572eb58137 
  src/tests/containerizer/mesos_containerizer_tests.cpp 
13e0f7e603a3ffdd0965b253d7abfe6a069cd2b4 


Diff: https://reviews.apache.org/r/58939/diff/5/

Changes: https://reviews.apache.org/r/58939/diff/4-5/


Testing
---

sudo make check
Manually tested on a simplified case of mesos-7374.


Thanks,

Chun-Hung Hsiao

Review Request 59067: Changed `#elif ...` to `#elif defined(...)` in Stout.

[Chun-Hung Hsiao]

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

Review request for mesos, Anand Mazumdar, Gilbert Song, and Jie Yu.


Repository: mesos


Description
---

Changed `#elif ...` to `#elif defined(...)` to match `#ifdef`.


Diffs
-

  3rdparty/stout/include/stout/os/posix/xattr.hpp 
6913c1529007cc1431a370a9cc97b8af5807d463 
  3rdparty/stout/include/stout/stopwatch.hpp 
093660d83572e873532769370921cc8ff25de226 
  3rdparty/stout/tests/os/process_tests.cpp 
d1760ef3cc36c1c13f1ab00ea4ab17fda4a46d8b 


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


Testing
---

make check


Thanks,

Chun-Hung Hsiao

Re: Review Request 59061: Fixed provisioner recover blockage by non-existing rootfses dir.

[Jie Yu]

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




src/slave/containerizer/mesos/provisioner/paths.cpp
Lines 210 (patched)


Why `isdir`? not os::exists?


- Jie Yu


On May 8, 2017, 7:51 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59061/
> ---
> 
> (Updated May 8, 2017, 7:51 p.m.)
> 
> 
> Review request for mesos, Chun-Hung Hsiao, Jie Yu, Michael Park, Neil Conway, 
> and Vinod Kone.
> 
> 
> Bugs: MESOS-7471
> https://issues.apache.org/jira/browse/MESOS-7471
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> In provisioner recover, when listing the container rootfses, it is
> possible that the 'rootfses' dir does not exist. Because a possible
> race between the provisioner destroy and the agent restart. For
> instance, while the provisioner is destroying the container dir the
> agent restarts. Due to os::rmdir() is recursive by traversing the
> FTS tree, it is possible that 'rootfses' dir is removed but the
> others (e.g., scratch dir) are not.
> 
> Currently, we are returning an error if the 'rootfses' dir does not
> exist, which blocks the agent from recovery. We should skip it if
> 'rootfses' does not exist.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/provisioner/paths.cpp 
> d2de98b66b7cf8331e82c31d0602dfe7e77d2130 
> 
> 
> Diff: https://reviews.apache.org/r/59061/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Review Request 59063: Changed #elif to #elif defined() in Mesos.

[Chun-Hung Hsiao]

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

Review request for mesos, Anand Mazumdar, Gilbert Song, and Jie Yu.


Repository: mesos


Description
---

Fixed mistakes that uses #elif  instead of #elif defined() 
after #ifdef .


Diffs
-

  src/linux/ns.hpp da4b716e06f4041f44659e1e73f0b7ea73ab4a05 
  src/slave/containerizer/mesos/containerizer.cpp 
58ab74571fb14c6dbb1907151dc421f93e324bb5 
  src/slave/flags.cpp ed99fadbf1aa91f7f0a57c9fb351c0247a40c6f4 


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


Testing
---

make check


Thanks,

Chun-Hung Hsiao

Review Request 59061: Fixed provisioner recover blockage by non-existing rootfses dir.

[Gilbert Song]

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

Review request for mesos, Chun-Hung Hsiao, Jie Yu, Michael Park, Neil Conway, 
and Vinod Kone.


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


Repository: mesos


Description
---

In provisioner recover, when listing the container rootfses, it is
possible that the 'rootfses' dir does not exist. Because a possible
race between the provisioner destroy and the agent restart. For
instance, while the provisioner is destroying the container dir the
agent restarts. Due to os::rmdir() is recursive by traversing the
FTS tree, it is possible that 'rootfses' dir is removed but the
others (e.g., scratch dir) are not.

Currently, we are returning an error if the 'rootfses' dir does not
exist, which blocks the agent from recovery. We should skip it if
'rootfses' does not exist.


Diffs
-

  src/slave/containerizer/mesos/provisioner/paths.cpp 
d2de98b66b7cf8331e82c31d0602dfe7e77d2130 


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


Testing
---

make check


Thanks,

Gilbert Song

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

[Chun-Hung Hsiao]

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

(Updated May 8, 2017, 7:03 p.m.)


Review request for mesos, Anand Mazumdar, Gilbert Song, and Jie Yu.


Changes
---

Addressed Jie's comment.


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 (updated)
-

  src/slave/containerizer/mesos/containerizer.cpp 
58ab74571fb14c6dbb1907151dc421f93e324bb5 
  src/slave/containerizer/mesos/isolators/docker/runtime.cpp 
2a6e0b179394e0485d2495ceb4bbbcb184af08fe 
  src/tests/containerizer/docker_volume_isolator_tests.cpp 
b47a6b5081a63ac474ac4634701b1a572eb58137 
  src/tests/containerizer/mesos_containerizer_tests.cpp 
13e0f7e603a3ffdd0965b253d7abfe6a069cd2b4 


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

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


Testing
---

sudo make check
Manually tested on a simplified case of mesos-7374.


Thanks,

Chun-Hung Hsiao

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

[Armand Grillet]

> On May 7, 2017, 8:13 p.m., Kevin Klues wrote:
> > src/cli_new/lib/cli/constants.py
> > Lines 18-25 (patched)
> > 
> >
> > Aso is, I think we should bundle this under cli/tests/constants.py.
> > 
> > However, if you want to keep this file as cli/constants.py, that's 
> > fine, but the default ports should be 5050 and 5051.
> > 
> > I chose 9090 and 9091 for the tests so they wouldn't conflict with and 
> > mesos instances running on the (normal) default ports.
> > 
> > If you want to keep these constants all in one place, we should add 
> > separate default ports for the tests. Something like:
> > ```
> > class namespace:
> > def __init__(self, **entries): self.__dict__.update(entries)
> > 
> > DEFAULT_MASTER_IP = "127.0.0.1"
> > DEFAULT_MASTER_PORT = "9090"
> > 
> > DEFAULT_AGENT_IP = "127.0.0.1"
> > DEFAULT_AGENT_PORT = "9091"
> > 
> > test = namespace(
> >   DEFAULT_MASTER_IP = DEFAULT_MASTER_IP,
> >   DEFAULT_MASTER_PORT = "9090",
> > 
> >   DEFAULT_AGENT_IP = DEFAULT_AGENT_IP
> >   DEFAULT_AGENT_PORT = "9091"
> > )
> > ```
> > 
> > Though I'd probably put the namespace function into `cli/util.py`

Having constants with different values depending on the namespace seems 
cumbersome. I will put the constants in this review request back under 
cli/tests/constants.py and create cli/constants.py in a future review request.


> On May 7, 2017, 8:13 p.m., Kevin Klues wrote:
> > src/cli_new/lib/cli/tests/base.py
> > Lines 139 (patched)
> > 
> >
> > The parameters to this function should not mix having 2 parameters on 
> > one line and just one on the next.
> > 
> > Either choose:
> > ```
> > self.executable = os.path.join(mesos_build_path(),
> >"bin",
> >"mesos-{name}.sh".format(name=self.name))
> >  
> > ```
> > 
> > or
> > ```
> > self.executable = os.path.join(
> > mesos_build_path(),
> > "bin",
> > "mesos-{name}.sh".format(name=self.name))
> > ```
> > 
> > I typically prefer the second.

I have used the first solution as it is everywhere like this right now.


> On May 7, 2017, 8:13 p.m., Kevin Klues wrote:
> > src/cli_new/lib/cli/tests/base.py
> > Lines 384-401 (patched)
> > 
> >
> > This is fine for now, but I'm wondering if we can't find a more generic 
> > way of discovering the build directorty.  This current method assumes we 
> > always create the build directory in a specific location.
> > 
> > It also assumes we have a build directory at all. Which maybe we should 
> > safeguard againts.

True, we could add an option like for our configure script with 
`--with-mesos-build-dir`. Our script mesos-cli-tests is currently small and 
supporting long command line options is gonna require a lot of code (it does 
not work out of the box on macOS). The easiest solution would be to have a 
short command line option to specify your build path (e.g. `mesos-cli-tests 
-b=path/to/mesos/build/dir`).


- Armand


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


On May 8, 2017, 5:27 p.m., Armand Grillet wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58720/
> ---
> 
> (Updated May 8, 2017, 5:27 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/__init__.py f4fc3f18af5641a4a87143adaba81e62334ccffb 
>   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/5/
> 
> 
> Testing
> ---
> 
> PEP8 and Pylint used to make sure that the code style is correct. Manuel 

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 8, 2017, 5:27 p.m.)


Review request for mesos and Kevin Klues.


Changes
---

Changed code following the issues raised.


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/__init__.py f4fc3f18af5641a4a87143adaba81e62334ccffb 
  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/5/

Changes: https://reviews.apache.org/r/58720/diff/4-5/


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 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/
---

(Updated May 8, 2017, 5:04 p.m.)


Review request for mesos and haosdent huang.


Changes
---

Address review comment.


Repository: mesos


Description
---

Fixed extra newline in webui for re-registered frameworks.


Diffs (updated)
-

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


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

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


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 58874: Stripped spaces between directory elements in WebUI.

[Mesos Reviewbot]

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



Patch looks great!

Reviews applied: [58874]

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 8, 2017, 8:01 a.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58874/
> ---
> 
> (Updated May 8, 2017, 8:01 a.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Tomasz Janiszewski.
> 
> 
> Bugs: MESOS-7468
> https://issues.apache.org/jira/browse/MESOS-7468
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Stripped spaces between directory elements in WebUI.
> 
> 
> Diffs
> -
> 
>   src/webui/master/static/browse.html 
> b9849197227b06df348789a49348e2b5d4cfd2ae 
>   src/webui/master/static/css/mesos.css 
> 9f3de5427071fc61d3791c4bc2a660368c2cd3c2 
> 
> 
> Diff: https://reviews.apache.org/r/58874/diff/3/
> 
> 
> Testing
> ---
> 
> 
> File Attachments
> 
> 
> strip_space.gif
>   
> https://reviews.apache.org/media/uploaded/files/2017/05/03/79afd0b1-5fb1-437c-a91c-732009af8fe3__strip_space.gif
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 59038: Fixed flakiness in HierarchicalAllocatorTest.NestedRoleDRF.

[Neil Conway]

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

(Updated May 8, 2017, 4:40 p.m.)


Review request for mesos, Anindya Sinha and Michael Park.


Changes
---

Address review comments.


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


Repository: mesos


Description
---

Fixed flakiness in HierarchicalAllocatorTest.NestedRoleDRF.


Diffs (updated)
-

  src/tests/hierarchical_allocator_tests.cpp 
33d5c0ea0182e09b3f3f30d20a33d46c23d81697 


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

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


Testing
---

`./bin/mesos-tests.sh --gtest_filter="HierarchicalAllocatorTest.*" --verbose 
--gtest_repeat=1000 --gtest_break_on_failure .`


Thanks,

Neil Conway

Re: Review Request 58924: Updated containerizer for isolator task_environment merge.

[Till Toenshoff]

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

(Updated May 8, 2017, 3:41 p.m.)


Review request for mesos, Adam B, Gilbert Song, and Jie Yu.


Changes
---

Addressed Jie's comment.


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


Repository: mesos


Description
---

See summary.


Diffs (updated)
-

  src/slave/containerizer/mesos/containerizer.cpp 
b58baed64480e22f640a4852537f85922ed382ae 


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

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


Testing
---

make check and functional testing on chain end.


Thanks,

Till Toenshoff

Re: Review Request 58949: Introduced ResourceProviderInfo proto.

[Benjamin Bannier]

> On May 8, 2017, 4:59 p.m., Jie Yu wrote:
> > src/common/type_utils.cpp
> > Lines 372-377 (patched)
> > 
> >
> > Let's don't introduce this yet until it's actually being used. For that 
> > reason, we don't have that for AgentInfo in v1.

I believe we should add this particular operator just for symmetry. 
`operator==` will immediately be useful to the first users.


> On May 8, 2017, 4:59 p.m., Jie Yu wrote:
> > src/v1/mesos.cpp
> > Lines 374-379 (patched)
> > 
> >
> > Ditto. Remove

Ditto. See above.


- Benjamin


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


On May 8, 2017, 4:49 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58949/
> ---
> 
> (Updated May 8, 2017, 4:49 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Jan Schlicht.
> 
> 
> Bugs: MESOS-7388
> https://issues.apache.org/jira/browse/MESOS-7388
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Introduced ResourceProviderInfo proto.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 1935f47a52840f6b395ecb2d2829551fa691 
>   include/mesos/type_utils.hpp 4f0a59cf7bc4172eb98db01cf73bf285c931b4d0 
>   include/mesos/v1/mesos.hpp e665ce7046eb6e9c4f0e896e4f8cf9e976c40454 
>   include/mesos/v1/mesos.proto c7f0bec5c96f2f41344d4261d0696f9fe0421db7 
>   src/common/type_utils.cpp 6bfb558d9f7dcdf921acdf737515a25c3a0bdaa0 
>   src/v1/mesos.cpp babe39e0e29ae7ff35360d9dfdabf771fdc940ea 
> 
> 
> Diff: https://reviews.apache.org/r/58949/diff/2/
> 
> 
> Testing
> ---
> 
> `make check` (Fedora25, clang-5)
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 58924: Updated containerizer for isolator task_environment merge.

[Jie Yu]

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


Fix it, then Ship it!





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


Let's also log the container id?
```
LOG(WARNING) << "Overwriting environment variable '" << name << "' "
 << "for container " << containerId;
```


- Jie Yu


On May 2, 2017, 5:28 p.m., Till Toenshoff wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58924/
> ---
> 
> (Updated May 2, 2017, 5:28 p.m.)
> 
> 
> Review request for mesos, Adam B, Gilbert Song, and Jie Yu.
> 
> 
> Bugs: MESOS-7429
> https://issues.apache.org/jira/browse/MESOS-7429
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/containerizer.cpp 
> b58baed64480e22f640a4852537f85922ed382ae 
> 
> 
> Diff: https://reviews.apache.org/r/58924/diff/3/
> 
> 
> Testing
> ---
> 
> make check and functional testing on chain end.
> 
> 
> Thanks,
> 
> Till Toenshoff
> 
>

Re: Review Request 58949: Introduced ResourceProviderInfo proto.

[Jie Yu]

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


Fix it, then Ship it!





include/mesos/mesos.proto
Line 107 (original), 107-116 (patched)


I'll move this right below `SlaveInfo` for consistency. Looks like for the 
proto file, we use logical grouping. We put ID and basic type definitions 
first, and then the rest.



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


Ditto on ordering.



src/common/type_utils.cpp
Lines 372-377 (patched)


Let's don't introduce this yet until it's actually being used. For that 
reason, we don't have that for AgentInfo in v1.



src/v1/mesos.cpp
Lines 346-353 (original), 346-354 (patched)


Can you swap the ordering between these two? Make sure the order in the cpp 
file is consistent with that in the header.



src/v1/mesos.cpp
Lines 374-379 (patched)


Ditto. Remove


- Jie Yu


On May 8, 2017, 2:49 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58949/
> ---
> 
> (Updated May 8, 2017, 2:49 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Jan Schlicht.
> 
> 
> Bugs: MESOS-7388
> https://issues.apache.org/jira/browse/MESOS-7388
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Introduced ResourceProviderInfo proto.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 1935f47a52840f6b395ecb2d2829551fa691 
>   include/mesos/type_utils.hpp 4f0a59cf7bc4172eb98db01cf73bf285c931b4d0 
>   include/mesos/v1/mesos.hpp e665ce7046eb6e9c4f0e896e4f8cf9e976c40454 
>   include/mesos/v1/mesos.proto c7f0bec5c96f2f41344d4261d0696f9fe0421db7 
>   src/common/type_utils.cpp 6bfb558d9f7dcdf921acdf737515a25c3a0bdaa0 
>   src/v1/mesos.cpp babe39e0e29ae7ff35360d9dfdabf771fdc940ea 
> 
> 
> Diff: https://reviews.apache.org/r/58949/diff/2/
> 
> 
> Testing
> ---
> 
> `make check` (Fedora25, clang-5)
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 58949: Introduced ResourceProviderInfo proto.

[Benjamin Bannier]

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

(Updated May 8, 2017, 4:49 p.m.)


Review request for mesos, Jie Yu and Jan Schlicht.


Changes
---

Track agent_id separate from ResourceProviderInfo.


Summary (updated)
-

Introduced ResourceProviderInfo proto.


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


Repository: mesos


Description
---

Introduced ResourceProviderInfo proto.


Diffs (updated)
-

  include/mesos/mesos.proto 1935f47a52840f6b395ecb2d2829551fa691 
  include/mesos/type_utils.hpp 4f0a59cf7bc4172eb98db01cf73bf285c931b4d0 
  include/mesos/v1/mesos.hpp e665ce7046eb6e9c4f0e896e4f8cf9e976c40454 
  include/mesos/v1/mesos.proto c7f0bec5c96f2f41344d4261d0696f9fe0421db7 
  src/common/type_utils.cpp 6bfb558d9f7dcdf921acdf737515a25c3a0bdaa0 
  src/v1/mesos.cpp babe39e0e29ae7ff35360d9dfdabf771fdc940ea 


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

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


Testing (updated)
---

`make check` (Fedora25, clang-5)


Thanks,

Benjamin Bannier

Re: Review Request 58099: Added authorization for frameworks in `GetRoles` v1 API.

[Mesos Reviewbot]

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



Patch looks great!

Reviews applied: [58095, 58096, 58097, 58099]

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 8, 2017, 7:56 a.m., Jay Guo wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58099/
> ---
> 
> (Updated May 8, 2017, 7:56 a.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rojas, and Benjamin Mahler.
> 
> 
> Bugs: MESOS-7260
> https://issues.apache.org/jira/browse/MESOS-7260
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added authorization for frameworks in `GetRoles` v1 API.
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp e2590a17044ac019b24a24629428d4ec8adc0c31 
> 
> 
> Diff: https://reviews.apache.org/r/58099/diff/5/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Jay Guo
> 
>

Re: Review Request 59047: Added internal wrapper messages for local resource provider Event/Call.

[Benjamin Bannier]

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


Fix it, then Ship it!





src/messages/messages.proto
Lines 605-607 (patched)


How about:

> A wrapper message that is sent for a local resource provider Call by the 
agent to the master. The agent serves as a proxy between the local resource 
provider and the master.



src/messages/messages.proto
Lines 621-623 (patched)


See above.


- Benjamin Bannier


On May 8, 2017, 2:27 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59047/
> ---
> 
> (Updated May 8, 2017, 2:27 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and Jan Schlicht.
> 
> 
> Bugs: MESOS-7469
> https://issues.apache.org/jira/browse/MESOS-7469
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> For local resource providers, the agent will serve as a proxy between
> local resource provider and the master. It'll forward resource
> provider Events and Calls.
> 
> 
> Diffs
> -
> 
>   src/messages/messages.proto eae9ce5a055a2cafef968c714abe4cea7dde627a 
> 
> 
> Diff: https://reviews.apache.org/r/59047/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 59046: Added basic resource provider Event/Call types.

[Benjamin Bannier]

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


Ship it!




Ship It!

- Benjamin Bannier


On May 8, 2017, 2:20 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59046/
> ---
> 
> (Updated May 8, 2017, 2:20 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and Jan Schlicht.
> 
> 
> Bugs: MESOS-7336
> https://issues.apache.org/jira/browse/MESOS-7336
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch added the basic Event/Call for resource providers to
> subscribe and apply offer operations.
> 
> 
> Diffs
> -
> 
>   include/mesos/resource_provider/resource_provider.proto 
> 73cfd31a68fb34da91c5d8df7aee8d8860aff3bd 
>   include/mesos/v1/resource_provider/resource_provider.proto 
> f83586d3ae3e1e48b3384b7961018682db942efb 
> 
> 
> Diff: https://reviews.apache.org/r/59046/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Review Request 59047: Added internal wrapper messages for local resource provider Event/Call.

[Jie Yu]

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

Review request for mesos, Benjamin Bannier and Jan Schlicht.


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


Repository: mesos


Description
---

For local resource providers, the agent will serve as a proxy between
local resource provider and the master. It'll forward resource
provider Events and Calls.


Diffs
-

  src/messages/messages.proto eae9ce5a055a2cafef968c714abe4cea7dde627a 


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


Testing
---

make check


Thanks,

Jie Yu

Review Request 59046: Added basic resource provider Event/Call types.

[Jie Yu]

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

Review request for mesos, Benjamin Bannier and Jan Schlicht.


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


Repository: mesos


Description
---

This patch added the basic Event/Call for resource providers to
subscribe and apply offer operations.


Diffs
-

  include/mesos/resource_provider/resource_provider.proto 
73cfd31a68fb34da91c5d8df7aee8d8860aff3bd 
  include/mesos/v1/resource_provider/resource_provider.proto 
f83586d3ae3e1e48b3384b7961018682db942efb 


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


Testing
---

make check


Thanks,

Jie Yu

Re: Review Request 58874: Stripped spaces between directory elements in WebUI.

[haosdent huang]

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

(Updated May 8, 2017, 8:01 a.m.)


Review request for mesos, Benjamin Mahler and Tomasz Janiszewski.


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


Repository: mesos


Description
---

Stripped spaces between directory elements in WebUI.


Diffs
-

  src/webui/master/static/browse.html b9849197227b06df348789a49348e2b5d4cfd2ae 
  src/webui/master/static/css/mesos.css 
9f3de5427071fc61d3791c4bc2a660368c2cd3c2 


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


Testing
---


File Attachments


strip_space.gif
  
https://reviews.apache.org/media/uploaded/files/2017/05/03/79afd0b1-5fb1-437c-a91c-732009af8fe3__strip_space.gif


Thanks,

haosdent huang

Re: Review Request 58874: Stripped spaces between directory elements in WebUI.

[haosdent huang]

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

(Updated May 8, 2017, 8 a.m.)


Review request for mesos, Benjamin Mahler and Tomasz Janiszewski.


Changes
---

Fixed @bmahler's comments.


Repository: mesos


Description
---

Stripped spaces between directory elements in WebUI.


Diffs (updated)
-

  src/webui/master/static/browse.html b9849197227b06df348789a49348e2b5d4cfd2ae 
  src/webui/master/static/css/mesos.css 
9f3de5427071fc61d3791c4bc2a660368c2cd3c2 


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

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


Testing
---


File Attachments


strip_space.gif
  
https://reviews.apache.org/media/uploaded/files/2017/05/03/79afd0b1-5fb1-437c-a91c-732009af8fe3__strip_space.gif


Thanks,

haosdent huang

Re: Review Request 58097: Added a test to check framework filtering in /roles endpoint.

[Jay Guo]

> On April 27, 2017, 11:05 p.m., Alexander Rojas wrote:
> > src/tests/master_authorization_tests.cpp
> > Line 2063 (original), 2062 (patched)
> > 
> >
> > Not yours, but no test verifies that the serialzation of quotas/weights 
> > is done correctly. Since you are changing how it is performed, coul you 
> > test that too?

I feel `RoleTest` is sufficient, no?


- Jay


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


On May 8, 2017, 3:56 p.m., Jay Guo wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58097/
> ---
> 
> (Updated May 8, 2017, 3:56 p.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rojas, and Benjamin Mahler.
> 
> 
> Bugs: MESOS-7260
> https://issues.apache.org/jira/browse/MESOS-7260
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added a test to check framework filtering in /roles endpoint.
> 
> 
> Diffs
> -
> 
>   src/tests/master_authorization_tests.cpp 
> e4233c19b1d9e3e2734259503d0daec4ce243667 
> 
> 
> Diff: https://reviews.apache.org/r/58097/diff/5/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jay Guo
> 
>

Re: Review Request 58874: Stripped spaces between directory elements in WebUI.

[haosdent huang]

> On May 3, 2017, 7:45 p.m., Benjamin Mahler wrote:
> > src/webui/master/static/browse.html
> > Lines 15-16 (original), 15-16 (patched)
> > 
> >
> > Looking at the gif, it seems the slashes weren't copied before? Do you 
> > want to clarify that here? Specifically, it seems this also ensures the 
> > slashes are being copied?

> the slashes weren't copied before? Do you want to clarify that here?

Thx @bmahler, I create a ticket at 
https://issues.apache.org/jira/browse/MESOS-7468 to clarify the problem.

> Specifically, it seems this also ensures the slashes are being copied?

The strange line break here only ensure there are no spaces, so I didn't 
mention `/` at here.


- haosdent


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


On May 3, 2017, 4:29 a.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58874/
> ---
> 
> (Updated May 3, 2017, 4:29 a.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Tomasz Janiszewski.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Stripped spaces between directory elements in WebUI.
> 
> 
> Diffs
> -
> 
>   src/webui/master/static/browse.html 
> b9849197227b06df348789a49348e2b5d4cfd2ae 
>   src/webui/master/static/css/mesos.css 
> 9f3de5427071fc61d3791c4bc2a660368c2cd3c2 
> 
> 
> Diff: https://reviews.apache.org/r/58874/diff/2/
> 
> 
> Testing
> ---
> 
> 
> File Attachments
> 
> 
> strip_space.gif
>   
> https://reviews.apache.org/media/uploaded/files/2017/05/03/79afd0b1-5fb1-437c-a91c-732009af8fe3__strip_space.gif
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 58099: Added authorization for frameworks in `GetRoles` v1 API.

[Jay Guo]

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

(Updated May 8, 2017, 3:56 p.m.)


Review request for mesos, Adam B, Alexander Rojas, and Benjamin Mahler.


Changes
---

rebase


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


Repository: mesos


Description
---

Added authorization for frameworks in `GetRoles` v1 API.


Diffs (updated)
-

  src/master/http.cpp e2590a17044ac019b24a24629428d4ec8adc0c31 


Diff: https://reviews.apache.org/r/58099/diff/5/

Changes: https://reviews.apache.org/r/58099/diff/4-5/


Testing
---


Thanks,

Jay Guo

Re: Review Request 58097: Added a test to check framework filtering in /roles endpoint.

[Jay Guo]

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

(Updated May 8, 2017, 3:56 p.m.)


Review request for mesos, Adam B, Alexander Rojas, and Benjamin Mahler.


Changes
---

rebase


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


Repository: mesos


Description
---

Added a test to check framework filtering in /roles endpoint.


Diffs (updated)
-

  src/tests/master_authorization_tests.cpp 
e4233c19b1d9e3e2734259503d0daec4ce243667 


Diff: https://reviews.apache.org/r/58097/diff/5/

Changes: https://reviews.apache.org/r/58097/diff/4-5/


Testing
---

make check


Thanks,

Jay Guo

Re: Review Request 58096: Added authorization for frameworks in /roles endpoint.

[Jay Guo]

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

(Updated May 8, 2017, 3:56 p.m.)


Review request for mesos, Adam B, Alexander Rojas, and Benjamin Mahler.


Changes
---

rebase


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


Repository: mesos


Description
---

While /roles displays a list of frameworksIds that register with
a role, it did NOT filter them based on VIEW_FRAMEWORK ACL, which
impose a security risk. This patch fixed this issue by taking a
frameworksApprover in `Master::Http::roles()` which is used to
filter framework IDs.


Diffs (updated)
-

  src/master/http.cpp e2590a17044ac019b24a24629428d4ec8adc0c31 


Diff: https://reviews.apache.org/r/58096/diff/5/

Changes: https://reviews.apache.org/r/58096/diff/4-5/


Testing
---

see next patch in the chain.


Thanks,

Jay Guo

Re: Review Request 58095: Refactored `Master::Http::roles` to use `jsonify`.

[Jay Guo]

> On April 27, 2017, 10:58 p.m., Alexander Rojas wrote:
> > src/master/http.cpp
> > Line 3401 (original), 3403 (patched)
> > 
> >
> > your changes here make this function non safe thread. Notice that the 
> > original would run in the context of the `MasterProcess` thread, while this 
> > will do so in the caller's thread, causing a possible race condition.

Reverted to keep using `_roles()`.


> On April 27, 2017, 10:58 p.m., Alexander Rojas wrote:
> > src/master/http.cpp
> > Lines 3481-3482 (original), 3456-3457 (patched)
> > 
> >
> > Not yours, but I feel the formatting of this lambda is really off. The 
> > body of the lambda starts a couple of columns earlier than declaration.

Other indents in this file are off too, e.g. `state()`, `frameworks()`


> On April 27, 2017, 10:58 p.m., Alexander Rojas wrote:
> > src/master/http.cpp
> > Line 3504 (original), 3505 (patched)
> > 
> >
> > Is ist really necesary to add empty fields?

Without this, `RoleTest.EndpointNoFrameworks` would be broken. I suppose we 
need empty fields for backwards compatibility?


> On April 27, 2017, 10:58 p.m., Alexander Rojas wrote:
> > src/master/master.hpp
> > Lines 1399-1401 (original), 1399-1405 (patched)
> > 
> >
> > How about rewording this to:
> > 
> > ```c++
> > // List of active roles, i.e. roles which are
> > // explicitly white listed, plus roles with one or
> > // more registered frameworks, plus all roles with a
> > // non default weight or quota.
> > ```
> > 
> > This gives you a clear understanding of what the method returns without 
> > describing how you do it (which is non interesting for API documentation). 
> > Also, the first paragraph doesn't really says anything useful.

Left original method untouched.


- Jay


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


On May 8, 2017, 3:41 p.m., Jay Guo wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58095/
> ---
> 
> (Updated May 8, 2017, 3:41 p.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rojas, Benjamin Mahler, and 
> Michael Park.
> 
> 
> Bugs: MESOS-4732 and MESOS-7260
> https://issues.apache.org/jira/browse/MESOS-4732
> https://issues.apache.org/jira/browse/MESOS-7260
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Refactored `Master::Http::roles` to use `jsonify`.
> 
> 
> Diffs
> -
> 
>   src/common/http.hpp 93d6088e97c2384f9f6d26e010a501abf2deb43e 
>   src/common/http.cpp 167dce2b9a2d3b68a1df5b4079f701482d34db28 
>   src/master/http.cpp e2590a17044ac019b24a24629428d4ec8adc0c31 
> 
> 
> Diff: https://reviews.apache.org/r/58095/diff/5/
> 
> 
> Testing
> ---
> 
> no functional changes
> make check
> 
> 
> Thanks,
> 
> Jay Guo
> 
>

Re: Review Request 58095: Refactored `Master::Http::roles` to use `jsonify`.

[Jay Guo]

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

(Updated May 8, 2017, 3:41 p.m.)


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


Changes
---

address comments.


Summary (updated)
-

Refactored `Master::Http::roles` to use `jsonify`.


Bugs: MESOS-4732 and MESOS-7260
https://issues.apache.org/jira/browse/MESOS-4732
https://issues.apache.org/jira/browse/MESOS-7260


Repository: mesos


Description (updated)
---

Refactored `Master::Http::roles` to use `jsonify`.


Diffs (updated)
-

  src/common/http.hpp 93d6088e97c2384f9f6d26e010a501abf2deb43e 
  src/common/http.cpp 167dce2b9a2d3b68a1df5b4079f701482d34db28 
  src/master/http.cpp e2590a17044ac019b24a24629428d4ec8adc0c31 


Diff: https://reviews.apache.org/r/58095/diff/5/

Changes: https://reviews.apache.org/r/58095/diff/4-5/


Testing
---

no functional changes
make check


Thanks,

Jay Guo