Re: Review Request 59105: Allowed leading zeros in input to stout's Version parser.

[Neil Conway]

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

(Updated May 10, 2017, 4:08 a.m.)


Review request for mesos and Benjamin Mahler.


Changes
---

Split comment changes to a separate review.


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


Repository: mesos


Description
---

The original behavior was to allow leading zeros. When Version was
enhanced with more complete support for SemVer [1], it was also changed
to reject leading zeros in numeric components (version numbers and
prerelease identifiers). Although this change was consistent with the
SemVer spec (which mandates that such version strings "MUST" be
considered invalid), it breaks compatibility with the versions used by
some common software packages (e.g., Docker).

This commit reverts the change in behavior, so that leading zeros are
once again allowed. In the future, we might consider adding a "strict"
mode to the Version parser, and/or supporting an "options" scheme that
would allow the user to customize the version parser's behavior.

[1] 287556284d76e03c11cff3fc076224fe966096e0


Diffs (updated)
-

  3rdparty/stout/include/stout/version.hpp 
4be722208329f838a3a3ebaa3b44affb2a6905f4 
  3rdparty/stout/tests/version_tests.cpp 
bce185ec493054ec81d0764088d04f3e4147303d 


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

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


Testing
---

`make check`


Thanks,

Neil Conway

Review Request 59122: Cleaned up comments in stout's Version.

[Neil Conway]

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

Review request for mesos and Benjamin Mahler.


Repository: mesos


Description
---

Cleaned up comments in stout's Version.


Diffs
-

  3rdparty/stout/include/stout/version.hpp 
4be722208329f838a3a3ebaa3b44affb2a6905f4 


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


Testing
---

`make check`


Thanks,

Neil Conway

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

[Mesos Reviewbot]

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



Patch looks great!

Reviews applied: [59067]

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, 9:34 p.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59067/
> ---
> 
> (Updated May 8, 2017, 9:34 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Gilbert Song, and Jie Yu.
> 
> 
> Bugs: mesos-7482
> https://issues.apache.org/jira/browse/mesos-7482
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> When doing conditional compilation for different platforms, we mostly use 
> `#ifdef X` ... `#elif defined(Y)` ... `#endif`. But there are some places in 
> the codebase that uses `#elif Y`. Although in the current GCC checking either 
> the existence or the value of a platform macro works, making the checks more 
> consistent is more preferable.
> 
> 
> 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 59092: Added local resource provider driver.

[Qian Zhang]

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




include/mesos/v1/resource_provider.hpp
Lines 39 (patched)


s/alraedy/already/



src/Makefile.am
Lines 770 (patched)


This line should be put before the above line.


- Qian Zhang


On May 9, 2017, 9:48 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59092/
> ---
> 
> (Updated May 9, 2017, 9:48 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
> ---
> 
> The local resource provider driver will be used by local resource
> providers. It relies on the agent's connection to the master. The
> agent will be responsible for forwarding Events and Calls by invoking
> corresponding functions from this driver.
> 
> 
> Diffs
> -
> 
>   include/mesos/v1/resource_provider.hpp PRE-CREATION 
>   include/mesos/v1/resource_provider/resource_provider.hpp 
> 2b8c8afab852621fb49b132813d512d0c96bc68c 
>   src/CMakeLists.txt 40d921ee7026f5ac47efbf0243e1cafab57825f9 
>   src/Makefile.am 6bb81fd49b4564a0afa993b2cef6baa9d370ee7a 
>   src/resource_provider/local_driver.cpp PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/59092/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 59101: Enabled authorization for v1 API call GET_MAINTENANCE_STATUS.

[Mesos Reviewbot]

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



Patch looks great!

Reviews applied: [58955, 58964, 59099, 59100, 59101]

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 9, 2017, 3:58 p.m., Alexander Rojas wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59101/
> ---
> 
> (Updated May 9, 2017, 3:58 p.m.)
> 
> 
> Review request for mesos, Adam B and Greg Mann.
> 
> 
> Bugs: MESOS-7415
> https://issues.apache.org/jira/browse/MESOS-7415
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Enables the use of authorization for the `GET_MAINTENANCE_STATUS`
> v1 API call, using the ACL `GetMaintenanceStatus` and the action
> of the same name as the API call.
> 
> It also updates the `ApiTests` to take into account the authorization
> case.
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp e2590a17044ac019b24a24629428d4ec8adc0c31 
>   src/tests/api_tests.cpp faf3242f9c86d866c4bb5e457fcfe47c1063cc09 
> 
> 
> Diff: https://reviews.apache.org/r/59101/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Alexander Rojas
> 
>

Re: Review Request 59105: Allowed leading zeros in input to stout's Version parser.

[Neil Conway]

> On May 10, 2017, 12:34 a.m., Benjamin Mahler wrote:
> > 3rdparty/stout/include/stout/version.hpp
> > Lines 53-57 (patched)
> > 
> >
> > Why move this comment down in this patch? Seems unrelated

Well, I was adjusting the header comment to clarify the current behavioral 
differences with SemVer, so it increasingly seemed like the comment was out of 
place. I can split it into a separate commit though.


- Neil


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


On May 9, 2017, 6:29 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59105/
> ---
> 
> (Updated May 9, 2017, 6:29 p.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Bugs: MESOS-7464
> https://issues.apache.org/jira/browse/MESOS-7464
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The original behavior was to allow leading zeros. When Version was
> enhanced with more complete support for SemVer [1], it was also changed
> to reject leading zeros in numeric components (version numbers and
> prerelease identifiers). Although this change was consistent with the
> SemVer spec (which mandates that such version strings "MUST" be
> considered invalid), it breaks compatibility with the versions used by
> some common software packages (e.g., Docker).
> 
> This commit reverts the change in behavior, so that leading zeros are
> once again allowed. In the future, we might consider adding a "strict"
> mode to the Version parser, and/or supporting an "options" scheme that
> would allow the user to customize the version parser's behavior.
> 
> [1] 287556284d76e03c11cff3fc076224fe966096e0
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/version.hpp 
> 9120e42eedcfb4fc5ee41f08354441bc10914baf 
>   3rdparty/stout/tests/version_tests.cpp 
> bce185ec493054ec81d0764088d04f3e4147303d 
> 
> 
> Diff: https://reviews.apache.org/r/59105/diff/1/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 59105: Allowed leading zeros in input to stout's Version parser.

[Benjamin Mahler]

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


Fix it, then Ship it!




Sorry, didn't see this until Adam pinged me, I'm a bit behind on email. :)


3rdparty/stout/include/stout/version.hpp
Lines 53-57 (patched)


Why move this comment down in this patch? Seems unrelated


- Benjamin Mahler


On May 9, 2017, 6:29 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59105/
> ---
> 
> (Updated May 9, 2017, 6:29 p.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Bugs: MESOS-7464
> https://issues.apache.org/jira/browse/MESOS-7464
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The original behavior was to allow leading zeros. When Version was
> enhanced with more complete support for SemVer [1], it was also changed
> to reject leading zeros in numeric components (version numbers and
> prerelease identifiers). Although this change was consistent with the
> SemVer spec (which mandates that such version strings "MUST" be
> considered invalid), it breaks compatibility with the versions used by
> some common software packages (e.g., Docker).
> 
> This commit reverts the change in behavior, so that leading zeros are
> once again allowed. In the future, we might consider adding a "strict"
> mode to the Version parser, and/or supporting an "options" scheme that
> would allow the user to customize the version parser's behavior.
> 
> [1] 287556284d76e03c11cff3fc076224fe966096e0
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/version.hpp 
> 9120e42eedcfb4fc5ee41f08354441bc10914baf 
>   3rdparty/stout/tests/version_tests.cpp 
> bce185ec493054ec81d0764088d04f3e4147303d 
> 
> 
> Diff: https://reviews.apache.org/r/59105/diff/1/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 55895: Extract a BasicBlocks class for disk block arithmetic.

[James Peach]

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

(Updated May 9, 2017, 11:59 p.m.)


Review request for mesos, Jie Yu and Jiang Yan Xu.


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


Repository: mesos


Description
---

Extract a BasicBlocks class to handle disk blocks to clarify block-based
arithmetic in the XFS disk isolator.


Diffs (updated)
-

  src/slave/containerizer/mesos/isolators/xfs/utils.hpp 
eddd4c37fb42339ca21ecb392dea47acf6b277bb 
  src/slave/containerizer/mesos/isolators/xfs/utils.cpp 
8018ad348d26bd962357543a5fb9f6cb43ff13b1 
  src/tests/containerizer/xfs_quota_tests.cpp 
7beb60b059910a0d4451b1ace895a35dc974a043 


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

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


Testing
---

sudo make check (Fedora 25)


Thanks,

James Peach

Re: Review Request 55895: Extract a BasicBlocks class for disk block arithmetic.

[James Peach]

> On May 9, 2017, 11:55 p.m., Jiang Yan Xu wrote:
> > src/slave/containerizer/mesos/isolators/xfs/utils.cpp
> > Line 62 (original), 62 (patched)
> > 
> >
> > This is unnecessary?

This comment is now on the `class BasicBlocks`.


- James


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


On April 29, 2017, 5:41 p.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55895/
> ---
> 
> (Updated April 29, 2017, 5:41 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-5116
> https://issues.apache.org/jira/browse/MESOS-5116
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Extract a BasicBlocks class to handle disk blocks to clarify block-based
> arithmetic in the XFS disk isolator.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/xfs/utils.hpp 
> eddd4c37fb42339ca21ecb392dea47acf6b277bb 
>   src/slave/containerizer/mesos/isolators/xfs/utils.cpp 
> 8018ad348d26bd962357543a5fb9f6cb43ff13b1 
>   src/tests/containerizer/xfs_quota_tests.cpp 
> 7beb60b059910a0d4451b1ace895a35dc974a043 
> 
> 
> Diff: https://reviews.apache.org/r/55895/diff/5/
> 
> 
> Testing
> ---
> 
> sudo make check (Fedora 25)
> 
> 
> Thanks,
> 
> James Peach
> 
>

Re: Review Request 55895: Extract a BasicBlocks class for disk block arithmetic.

[Jiang Yan Xu]

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


Fix it, then Ship it!




I can fix up the following the suggestions.


src/slave/containerizer/mesos/isolators/xfs/utils.cpp
Line 62 (original), 62 (patched)


This is unnecessary?



src/tests/containerizer/xfs_quota_tests.cpp
Lines 950-951 (patched)


We can group the two statements and remove the redundant "// A partial 
block should round up."?


- Jiang Yan Xu


On April 29, 2017, 10:41 a.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55895/
> ---
> 
> (Updated April 29, 2017, 10:41 a.m.)
> 
> 
> Review request for mesos, Jie Yu and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-5116
> https://issues.apache.org/jira/browse/MESOS-5116
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Extract a BasicBlocks class to handle disk blocks to clarify block-based
> arithmetic in the XFS disk isolator.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/xfs/utils.hpp 
> eddd4c37fb42339ca21ecb392dea47acf6b277bb 
>   src/slave/containerizer/mesos/isolators/xfs/utils.cpp 
> 8018ad348d26bd962357543a5fb9f6cb43ff13b1 
>   src/tests/containerizer/xfs_quota_tests.cpp 
> 7beb60b059910a0d4451b1ace895a35dc974a043 
> 
> 
> Diff: https://reviews.apache.org/r/55895/diff/5/
> 
> 
> Testing
> ---
> 
> sudo make check (Fedora 25)
> 
> 
> Thanks,
> 
> James Peach
> 
>

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

[Vinod Kone]

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




include/mesos/docker/spec.hpp
Line 76 (original), 76 (patched)


s/as a/from a/



include/mesos/docker/spec.hpp
Lines 82 (patched)


s/as a/from a/



include/mesos/docker/spec.hpp
Lines 83 (patched)


no unit test for this?


- 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/59017/
> ---
> 
> (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
> ---
> 
> Added support for docker spec helper 'parseAuthConfig()'.
> 
> 
> Diffs
> -
> 
>   include/mesos/docker/spec.hpp b90f731ef07c22259715543526eaf25cb94eab03 
>   src/docker/spec.cpp 6b5588e534215451593cc20011847c3abbab9b17 
> 
> 
> Diff: https://reviews.apache.org/r/59017/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

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

[Vinod Kone]

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


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/59016/
> ---
> 
> (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 the comment style issue in docker/spec.hpp.
> 
> 
> Diffs
> -
> 
>   include/mesos/docker/spec.hpp b90f731ef07c22259715543526eaf25cb94eab03 
> 
> 
> Diff: https://reviews.apache.org/r/59016/diff/1/
> 
> 
> Testing
> ---
> 
> N/A
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

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

[Vinod Kone]

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




include/mesos/slave/containerizer.proto
Lines 194 (patched)


s/in the root filesystem/(e.g., when task defines an image)



src/slave/containerizer/mesos/containerizer.cpp
Lines 1501-1505 (original), 1505-1536 (patched)


Instead of overwriting the working directory in some special cases in a 
nested way, I personally find it easier to follow the logic if it is done in a 
nested "if else loop". Also, please add comments because this logic is not 
straightforward.

```
if (debug container) {
   if (parent has launch info) {
  if (parent is a command task) {
 if (parent task has a rootfs) {
// Set working directory to the parent task's work directory.
 } else {
   // Set working directory to the parent task's executor's work 
directory.
 }
  
  } else { // Parent is a non-command task
 // Set working directory to the parent task's work directory.
  }
   } else { // Paren't work directory is unknown.
  // No working directory is set.
   }
}

```



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


For clarify, I would move this after the foreach loop and do

```
// If "--working_directory" argument is not found, default to the sandbox 
directory.
if (!launchInfo.has_working_directory()) {
  launchInfo.set_working_directory(flags.sandbox_directory);
}
```


- Vinod Kone


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

Re: Review Request 58587: Clarified comments about resource operations through operator API.

[Jiang Yan Xu]

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

(Updated May 9, 2017, 3:29 p.m.)


Review request for mesos, Anindya Sinha and Michael Park.


Changes
---

Comments.


Repository: mesos


Description
---

Seems like the comments about "virtually always win the race against
'allocate'" could be made more precise in helping people understand
how it works.


Diffs (updated)
-

  src/master/http.cpp e2590a17044ac019b24a24629428d4ec8adc0c31 


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

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


Testing
---

N/A


Thanks,

Jiang Yan Xu

Re: Review Request 59114: Improved error reporting in status update manager.

[Vinod Kone]

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


Ship it!




Ship It!

- Vinod Kone


On May 9, 2017, 10:11 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59114/
> ---
> 
> (Updated May 9, 2017, 10:11 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> When an I/O operation fails, we should include the error message
> returned by the I/O operation in the error we return to the caller.
> 
> 
> Diffs
> -
> 
>   src/slave/status_update_manager.cpp 
> df63a708c70a2f22f3ca7692adfaa847c972c302 
> 
> 
> Diff: https://reviews.apache.org/r/59114/diff/1/
> 
> 
> Testing
> ---
> 
> Visual inspection, `make check`.
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 58587: Clarified comments about resource operations through operator API.

[Jiang Yan Xu]

> On May 2, 2017, 12:13 a.m., Michael Park wrote:
> > src/master/http.cpp
> > Lines 4884-4889 (patched)
> > 
> >
> > I'm not quite following this example. Specifically, I don't get what "a 
> > persistent volume can be destroyed if it is not allocated" means..
> 
> Jiang Yan Xu wrote:
> Ok I guess I shoud say "a persistent volume can be destroyed only if it 
> is not used". In this case, it's not going to be pending offers so you would 
> unncessarily rescind all offers.
> 
> Michael Park wrote:
> Hm... it seems like this should be mitigated by
> ```cpp
> // If rescinding the offer would not contribute to satisfying
> // the required resources, skip it.
> Resources recovered = offer->resources();
> recovered.unallocate();
> 
> if (required == required - recovered) {
>   continue;
> }
> ```
> Is this not true?

Hm you are right. I'll drop this comment and leave just the one above. I'll 
send another rr for caveats about shared persistent volumes, which is different.


- Jiang Yan


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


On May 1, 2017, 2:29 p.m., Jiang Yan Xu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58587/
> ---
> 
> (Updated May 1, 2017, 2:29 p.m.)
> 
> 
> Review request for mesos, Anindya Sinha and Michael Park.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Seems like the comments about "virtually always win the race against
> 'allocate'" could be made more precise in helping people understand
> how it works.
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp 5aae52870451d883ef1ea1fda5a27764d7f318e8 
> 
> 
> Diff: https://reviews.apache.org/r/58587/diff/1/
> 
> 
> Testing
> ---
> 
> N/A
> 
> 
> Thanks,
> 
> Jiang Yan Xu
> 
>

Review Request 59114: Improved error reporting in status update manager.

[Neil Conway]

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

Review request for mesos and Vinod Kone.


Repository: mesos


Description
---

When an I/O operation fails, we should include the error message
returned by the I/O operation in the error we return to the caller.


Diffs
-

  src/slave/status_update_manager.cpp df63a708c70a2f22f3ca7692adfaa847c972c302 


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


Testing
---

Visual inspection, `make check`.


Thanks,

Neil Conway

Re: Review Request 59110: Fixed `Version::validateIdentifier()` for Unicode edge case.

[Neil Conway]

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


Ship it!




Ship It!

- Neil Conway


On May 9, 2017, 8:32 p.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59110/
> ---
> 
> (Updated May 9, 2017, 8:32 p.m.)
> 
> 
> Review request for mesos, Li Li and Neil Conway.
> 
> 
> Bugs: MESOS-7484
> https://issues.apache.org/jira/browse/MESOS-7484
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The test case `u8"1.0.0-b\u00e9ta"` in `VersionTest.ParseInvalid` caused
> an abort in `isctype.cpp` with VS 2017. Changing the `char` to an
> `unsigned char` prevents this.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/version.hpp 
> 9120e42eedcfb4fc5ee41f08354441bc10914baf 
> 
> 
> Diff: https://reviews.apache.org/r/59110/diff/1/
> 
> 
> Testing
> ---
> 
> Build and ran `stout-tests` target on Windows. `make check` in 
> `build/3rdparty/stout` on Linux.
> 
> ```
> [ RUN  ] VersionTest.ParseValid
> [   OK ] VersionTest.ParseValid (0 ms)
> ```
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>

Re: Review Request 58587: Clarified comments about resource operations through operator API.

[Michael Park]

> On May 2, 2017, 12:13 a.m., Michael Park wrote:
> > src/master/http.cpp
> > Lines 4884-4889 (patched)
> > 
> >
> > I'm not quite following this example. Specifically, I don't get what "a 
> > persistent volume can be destroyed if it is not allocated" means..
> 
> Jiang Yan Xu wrote:
> Ok I guess I shoud say "a persistent volume can be destroyed only if it 
> is not used". In this case, it's not going to be pending offers so you would 
> unncessarily rescind all offers.

Hm... it seems like this should be mitigated by
```cpp
// If rescinding the offer would not contribute to satisfying
// the required resources, skip it.
Resources recovered = offer->resources();
recovered.unallocate();

if (required == required - recovered) {
  continue;
}
```
Is this not true?


- Michael


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


On May 1, 2017, 2:29 p.m., Jiang Yan Xu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58587/
> ---
> 
> (Updated May 1, 2017, 2:29 p.m.)
> 
> 
> Review request for mesos, Anindya Sinha and Michael Park.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Seems like the comments about "virtually always win the race against
> 'allocate'" could be made more precise in helping people understand
> how it works.
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp 5aae52870451d883ef1ea1fda5a27764d7f318e8 
> 
> 
> Diff: https://reviews.apache.org/r/58587/diff/1/
> 
> 
> Testing
> ---
> 
> N/A
> 
> 
> Thanks,
> 
> Jiang Yan Xu
> 
>

Re: Review Request 58821: Added a test that verifies a task and its check share the work dir.

[Vinod Kone]

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




src/tests/check_tests.cpp
Lines 706-730 (patched)


I wonder if it might be better to improve the check command instead of 
doing this (is there a guarantee that the file exists by the second check 
update?) For example the check command could wait until the file exists in a 
while loop and we can rely on the check timeout to break the loop incase of a 
failure.


- Vinod Kone


On May 9, 2017, 2:32 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58821/
> ---
> 
> (Updated May 9, 2017, 2:32 p.m.)
> 
> 
> Review request for mesos, Gastón Kleiman, Jie Yu, and Vinod Kone.
> 
> 
> Bugs: MESOS-7433
> https://issues.apache.org/jira/browse/MESOS-7433
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/tests/check_tests.cpp ec0d5ee78a94991c68f38174d09b41e8f2e4be35 
> 
> 
> Diff: https://reviews.apache.org/r/58821/diff/4/
> 
> 
> Testing
> ---
> 
> `make check` on Mac OS and Fedora 24
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 58263: Inherited working dir from parent when launching a DEBUG container.

[Vinod Kone]

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


Ship it!




- Vinod Kone


On May 9, 2017, 2:32 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58263/
> ---
> 
> (Updated May 9, 2017, 2:32 p.m.)
> 
> 
> Review request for mesos, Gastón Kleiman, Jie Yu, Kevin Klues, and Vinod Kone.
> 
> 
> Bugs: MESOS-7433
> https://issues.apache.org/jira/browse/MESOS-7433
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   include/mesos/slave/containerizer.proto 
> 41f2905df690bfe88ed762f1cd1246689fa4d3ea 
>   src/slave/containerizer/mesos/containerizer.cpp 
> 3d724945812c0359ed175ce232f70886dc4401c8 
>   src/tests/containerizer/nested_mesos_containerizer_tests.cpp 
> c163b882fb2fc463537d6906c5a47b24a9a756c4 
> 
> 
> Diff: https://reviews.apache.org/r/58263/diff/6/
> 
> 
> Testing
> ---
> 
> See https://reviews.apache.org/r/58821/
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 59078: Fixed the MULTI_ROLE upgrade path for an old master and new agent.

[Benjamin Mahler]

> On May 9, 2017, 8:49 p.m., Michael Park wrote:
> > src/slave/slave.cpp
> > Line 4997 (original), 4997-4998 (patched)
> > 
> >
> > Maybe mention here that if the task does not have an allocation role, 
> > both `ExecutorInfo` and `TaskInfo` will have the allocation role injected 
> > later?

Sounds good.


- Benjamin


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


On May 9, 2017, 7:16 p.m., Benjamin Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59078/
> ---
> 
> (Updated May 9, 2017, 7:16 p.m.)
> 
> 
> Review request for mesos and Michael Park.
> 
> 
> Bugs: MESOS-7478
> https://issues.apache.org/jira/browse/MESOS-7478
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The agent was accidentally assuming that the task sent by the master
> had the `allocation_info` set. The fix is for `getExecutorInfo` to
> inject only in the case where it is already set.
> 
> 
> Diffs
> -
> 
>   src/slave/slave.cpp a37c6c888e7573209aadb07576cfb727fa1ec4ff 
> 
> 
> Diff: https://reviews.apache.org/r/59078/diff/1/
> 
> 
> Testing
> ---
> 
> Manually performed upgrade tests:
> 
> 1.2.0 agent with 1.1.0 master, crashes as MESOS-7478
> HEAD+fix with 1.1.0 master, works
> 1.2.x+fix with 1.1.0 master, works
> 1.1.0 master with 1.1.0 agent, then upgrade agent to 1.2.x+fix works
> 
> 
> Thanks,
> 
> Benjamin Mahler
> 
>

Re: Review Request 58872: Ensured sandbox URI request reroute after fetched `$scope.state`.

[Benjamin Mahler]

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


Fix it, then Ship it!




Before you commit this, it would be great if you could test this (I can't tell 
how you tested this from the testing done section), e.g.:

* Clicking on a link from the top level task page (state should already be 
loaded)
* Navigating directly to a valid URL (state should not be loaded yet)
* Navigating directly to an invalid URL (e.g. bad agent ID) (state should not 
be loaded)

To make sure that these all work as you expect.


src/webui/master/static/js/controllers.js
Lines 1016-1017 (patched)


I'm a bit confused as to why we would want to set up the listener if we 
already called reroute(). Does this belong in the else case?

```
if ($scope.state) {
  reroute();
} else {
  var removeListener = $scope.$on('state_updated', reroute);
  $scope.$on('$routeChangeStart', removeListener);
}
```

Also, why is it safe to call reroute on every state update? It seems there 
are a few cases to consider:

Case 1: reroute actually routes away from the page, then we're ok because 
the listener is removed

Case 2: reroute calls goBack, we're ok because the listener is removed 
(there is a separate question we should consider later, which is whether going 
back is the best thing to do)

Case 3: reroute navigates "home" (/) if we get an error response, this is 
ok since the listener is removed

How about adding a comment here that reroute is expected to always route 
away from the page and so the listener is removed after the first state update?


- Benjamin Mahler


On May 3, 2017, 4:15 a.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58872/
> ---
> 
> (Updated May 3, 2017, 4:15 a.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Tomasz Janiszewski.
> 
> 
> Bugs: MESOS-4992
> https://issues.apache.org/jira/browse/MESOS-4992
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Open sandbox link in a new tab would fail since it depends on agents
> information loading finish. This patch fixes it by ensuring the agents
> information loaded first and then rerouting the sandbox request.
> 
> 
> Diffs
> -
> 
>   src/webui/master/static/js/controllers.js 
> a021962573d452de1581e6a7717016eac7d0cd85 
> 
> 
> Diff: https://reviews.apache.org/r/58872/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 59078: Fixed the MULTI_ROLE upgrade path for an old master and new agent.

[Michael Park]

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


Ship it!





src/slave/slave.cpp
Line 4997 (original), 4997-4998 (patched)


Maybe mention here that if the task does not have an allocation role, both 
`ExecutorInfo` and `TaskInfo` will have the allocation role injected later?


- Michael Park


On May 9, 2017, 12:16 p.m., Benjamin Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59078/
> ---
> 
> (Updated May 9, 2017, 12:16 p.m.)
> 
> 
> Review request for mesos and Michael Park.
> 
> 
> Bugs: MESOS-7478
> https://issues.apache.org/jira/browse/MESOS-7478
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The agent was accidentally assuming that the task sent by the master
> had the `allocation_info` set. The fix is for `getExecutorInfo` to
> inject only in the case where it is already set.
> 
> 
> Diffs
> -
> 
>   src/slave/slave.cpp a37c6c888e7573209aadb07576cfb727fa1ec4ff 
> 
> 
> Diff: https://reviews.apache.org/r/59078/diff/1/
> 
> 
> Testing
> ---
> 
> Manually performed upgrade tests:
> 
> 1.2.0 agent with 1.1.0 master, crashes as MESOS-7478
> HEAD+fix with 1.1.0 master, works
> 1.2.x+fix with 1.1.0 master, works
> 1.1.0 master with 1.1.0 agent, then upgrade agent to 1.2.x+fix works
> 
> 
> Thanks,
> 
> Benjamin Mahler
> 
>

Review Request 59110: Fixed `Version::validateIdentifier()` for Unicode edge case.

[Andrew Schwartzmeyer]

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

Review request for mesos, Li Li and Neil Conway.


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


Repository: mesos


Description
---

The test case `u8"1.0.0-b\u00e9ta"` in `VersionTest.ParseInvalid` caused
an abort in `isctype.cpp` with VS 2017. Changing the `char` to an
`unsigned char` prevents this.


Diffs
-

  3rdparty/stout/include/stout/version.hpp 
9120e42eedcfb4fc5ee41f08354441bc10914baf 


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


Testing
---

Build and ran `stout-tests` target on Windows. `make check` in 
`build/3rdparty/stout` on Linux.

```
[ RUN  ] VersionTest.ParseValid
[   OK ] VersionTest.ParseValid (0 ms)
```


Thanks,

Andrew Schwartzmeyer

Review Request 59107: [WIP] Added extra debugging statements.

[Gastón Kleiman]

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

Review request for mesos.


Repository: mesos


Description
---

Added extra debugging statements.


Diffs
-

  src/slave/containerizer/mesos/containerizer.cpp 
3d724945812c0359ed175ce232f70886dc4401c8 
  src/slave/containerizer/mesos/io/switchboard.cpp 
64180944f8680828781168faa67417489abf1bc8 
  src/slave/http.cpp 5beaf91de1ba948658fa7fa299a364432afcf50b 


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


Testing
---

Publishing this RR only to make the patch go through the Apache CI.


Thanks,

Gastón Kleiman

Review Request 59078: Fixed the MULTI_ROLE upgrade path for an old master and new agent.

[Benjamin Mahler]

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

Review request for mesos and Michael Park.


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


Repository: mesos


Description
---

The agent was accidentally assuming that the task sent by the master
had the `allocation_info` set. The fix is for `getExecutorInfo` to
inject only in the case where it is already set.


Diffs
-

  src/slave/slave.cpp a37c6c888e7573209aadb07576cfb727fa1ec4ff 


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


Testing
---

Manually performed upgrade tests:

1.2.0 agent with 1.1.0 master, crashes as MESOS-7478
HEAD+fix with 1.1.0 master, works
1.2.x+fix with 1.1.0 master, works
1.1.0 master with 1.1.0 agent, then upgrade agent to 1.2.x+fix works


Thanks,

Benjamin Mahler

Review Request 59105: Allowed leading zeros in input to stout's Version parser.

[Neil Conway]

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

Review request for mesos and Benjamin Mahler.


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


Repository: mesos


Description
---

The original behavior was to allow leading zeros. When Version was
enhanced with more complete support for SemVer [1], it was also changed
to reject leading zeros in numeric components (version numbers and
prerelease identifiers). Although this change was consistent with the
SemVer spec (which mandates that such version strings "MUST" be
considered invalid), it breaks compatibility with the versions used by
some common software packages (e.g., Docker).

This commit reverts the change in behavior, so that leading zeros are
once again allowed. In the future, we might consider adding a "strict"
mode to the Version parser, and/or supporting an "options" scheme that
would allow the user to customize the version parser's behavior.

[1] 287556284d76e03c11cff3fc076224fe966096e0


Diffs
-

  3rdparty/stout/include/stout/version.hpp 
9120e42eedcfb4fc5ee41f08354441bc10914baf 
  3rdparty/stout/tests/version_tests.cpp 
bce185ec493054ec81d0764088d04f3e4147303d 


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


Testing
---

`make check`


Thanks,

Neil Conway

Re: Review Request 59097: Fixed flakiness in agent registration validation tests.

[James Peach]

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


Ship it!




Ship It!

- James Peach


On May 9, 2017, 6 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59097/
> ---
> 
> (Updated May 9, 2017, 6 p.m.)
> 
> 
> Review request for mesos, Jie Yu and James Peach.
> 
> 
> Bugs: MESOS-7441
> https://issues.apache.org/jira/browse/MESOS-7441
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> RegisterSlaveValidationTest.DropInvalidReregistration and
> RegisterSlaveValidationTest.DropInvalidRegistration.
> 
> 
> Diffs
> -
> 
>   src/tests/master_validation_tests.cpp 
> 3308803332bfae0d4b410eacd6d9a2a4a1265eff 
> 
> 
> Diff: https://reviews.apache.org/r/59097/diff/2/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> `./src/mesos-tests 
> --gtest_filter="RegisterSlaveValidationTest.DropInvalidReregistration" 
> --gtest_repeat=2000 --gtest_break_on_failure .`
> `./src/mesos-tests 
> --gtest_filter="RegisterSlaveValidationTest.DropInvalidRegistration" 
> --gtest_repeat=2000 --gtest_break_on_failure .`
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 59000: Added environment secret isolator.

[Kapil Arya]

> On May 8, 2017, 8:03 p.m., Vinod Kone wrote:
> > 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.

Jie suggested that we keep it at the top-level. This is also an special case 
where this isolator is not visible outside and is turned on by default.


- Kapil


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


On May 9, 2017, 2:11 p.m., Kapil Arya wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59000/
> ---
> 
> (Updated May 9, 2017, 2:11 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/5/
> 
> 
> Testing
> ---
> 
> Added a new test and ran `make check`.
> 
> 
> Thanks,
> 
> Kapil Arya
> 
>

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

[Kapil Arya]

> On May 8, 2017, 8:05 p.m., Vinod Kone wrote:
> > src/slave/main.cpp
> > Line 450 (original), 463 (patched)
> > 
> >
> > If the pointer to resolver is only passed to the containerizer, how 
> > will the agent get access to it to resolve executor auth token?

We'll be modifying the `executorEnvironment` signature to return 
`Try` instead of `map` so that all env vars can be 
resolved in the containerizer itself (via env isolator).


- Kapil


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


On May 9, 2017, 2:11 p.m., Kapil Arya wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58999/
> ---
> 
> (Updated May 9, 2017, 2:11 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/5/
> 
> 
> Testing
> ---
> 
> 
> 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 9, 2017, 2:11 p.m.)


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


Changes
---

Addressed Vinod's comments!


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

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


Testing
---


Thanks,

Kapil Arya

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 9, 2017, 2:11 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 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/6/

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


Testing
---

Added new tests an ran `make check`.


Thanks,

Kapil Arya

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 9, 2017, 2:11 p.m.)


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


Changes
---

Addressed Vinod's comments!


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

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


Testing
---

Added a new test and ran `make check`.


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 9, 2017, 2:11 p.m.)


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


Changes
---

Addressed Vinod's comments!


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


Repository: mesos


Description
---

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

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


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 9, 2017, 2:10 p.m.)


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


Changes
---

Addressed Vinod's comments!


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


Repository: mesos


Description
---

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

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


Testing
---


Thanks,

Kapil Arya

Re: Review Request 58263: Inherited working dir from parent when launching a DEBUG container.

[Gastón Kleiman]

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


Ship it!





src/tests/containerizer/nested_mesos_containerizer_tests.cpp
Line 579 (original), 595-598 (patched)


Why do you prefer not to use the helper method?


- Gastón Kleiman


On May 9, 2017, 2:32 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58263/
> ---
> 
> (Updated May 9, 2017, 2:32 p.m.)
> 
> 
> Review request for mesos, Gastón Kleiman, Jie Yu, Kevin Klues, and Vinod Kone.
> 
> 
> Bugs: MESOS-7433
> https://issues.apache.org/jira/browse/MESOS-7433
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   include/mesos/slave/containerizer.proto 
> 41f2905df690bfe88ed762f1cd1246689fa4d3ea 
>   src/slave/containerizer/mesos/containerizer.cpp 
> 3d724945812c0359ed175ce232f70886dc4401c8 
>   src/tests/containerizer/nested_mesos_containerizer_tests.cpp 
> c163b882fb2fc463537d6906c5a47b24a9a756c4 
> 
> 
> Diff: https://reviews.apache.org/r/58263/diff/6/
> 
> 
> Testing
> ---
> 
> See https://reviews.apache.org/r/58821/
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

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 9, 2017, 6:08 p.m.)


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


Changes
---

Changed the unit tests requiring the linux launcher to root tests.


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

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


Testing
---

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


Thanks,

Chun-Hung Hsiao

Re: Review Request 59097: Fixed flakiness in agent registration validation tests.

[Neil Conway]

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

(Updated May 9, 2017, 6 p.m.)


Review request for mesos, Jie Yu and James Peach.


Changes
---

Fixed additional flakiness in re-registration test, per jpeach report.


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


Repository: mesos


Description
---

RegisterSlaveValidationTest.DropInvalidReregistration and
RegisterSlaveValidationTest.DropInvalidRegistration.


Diffs (updated)
-

  src/tests/master_validation_tests.cpp 
3308803332bfae0d4b410eacd6d9a2a4a1265eff 


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

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


Testing
---

`make check`

`./src/mesos-tests 
--gtest_filter="RegisterSlaveValidationTest.DropInvalidReregistration" 
--gtest_repeat=2000 --gtest_break_on_failure .`
`./src/mesos-tests 
--gtest_filter="RegisterSlaveValidationTest.DropInvalidRegistration" 
--gtest_repeat=2000 --gtest_break_on_failure .`


Thanks,

Neil Conway

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

[Tim Anderegg]

> On May 9, 2017, 4:30 p.m., haosdent huang wrote:
> > Hi, @tim Thanks a lot for your update. I am still reading you patch and 
> > have not finished. Could return my comments if it works at my side. Thanks 
> > a lot for your contributions.

Thanks @haosdent, please let me know if you have any questions or need 
clarification on anything!


- Tim


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


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

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

[James Peach]

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




3rdparty/libprocess/src/process.cpp
Lines 955 (patched)


This is a bug. If we hit this, we will not re-arm the accept event and 
won't accept any new connections.


- James Peach


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

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

[haosdent huang]

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



Hi, @tim Thanks a lot for your update. I am still reading you patch and have 
not finished. Could return my comments if it works at my side. Thanks a lot for 
your contributions.

- haosdent huang


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

Review Request 59101: Enabled authorization for v1 API call GET_MAINTENANCE_STATUS.

[Alexander Rojas]

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

Review request for mesos, Adam B and Greg Mann.


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


Repository: mesos


Description
---

Enables the use of authorization for the `GET_MAINTENANCE_STATUS`
v1 API call, using the ACL `GetMaintenanceStatus` and the action
of the same name as the API call.

It also updates the `ApiTests` to take into account the authorization
case.


Diffs
-

  src/master/http.cpp e2590a17044ac019b24a24629428d4ec8adc0c31 
  src/tests/api_tests.cpp faf3242f9c86d866c4bb5e457fcfe47c1063cc09 


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


Testing
---

make check


Thanks,

Alexander Rojas

Review Request 59100: Enabled authorization for v1 API call GET_MAINTENANCE_SCHEDULE.

[Alexander Rojas]

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

Review request for mesos, Adam B and Greg Mann.


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


Repository: mesos


Description
---

Enables the use of authorization for the `GET_MAINTENANCE_SCHEDULE`
v1 API call, using the ACL `GetMaintenanceSchedule` and the action
of the same name as the API call.

It also updates the ApiTests to take into account the authorization
case.


Diffs
-

  src/master/http.cpp e2590a17044ac019b24a24629428d4ec8adc0c31 
  src/tests/api_tests.cpp faf3242f9c86d866c4bb5e457fcfe47c1063cc09 


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


Testing
---

make check


Thanks,

Alexander Rojas

Re: Review Request 59099: Enabled authorization for v1 API call UPDATE_MAINTENANCE_SCHEDULE.

[Alexander Rojas]

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

(Updated May 9, 2017, 5:53 p.m.)


Review request for mesos, Adam B and Greg Mann.


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


Repository: mesos


Description (updated)
---

Enables the use of authorization for the `UPDATE_MAINTENANCE_SCHEDULEd`
v1 API call, using the ACL `UpdateMaintenanceSchedule` and the action
of the same name as the API call.

It also updates the `ApiTests` to take into account the authorization
case.


Diffs
-

  src/master/http.cpp e2590a17044ac019b24a24629428d4ec8adc0c31 
  src/master/master.hpp 89d0790fd5fea59e74276f462581fe0073594732 
  src/tests/api_tests.cpp faf3242f9c86d866c4bb5e457fcfe47c1063cc09 


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


Testing
---

make check


Thanks,

Alexander Rojas

Re: Review Request 59099: Enabled authorization for v1 API call UPDATE_MAINTENANCE_SCHEDULE.

[Alexander Rojas]

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

(Updated May 9, 2017, 5:53 p.m.)


Review request for mesos, Adam B and Greg Mann.


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


Repository: mesos


Description (updated)
---

Enables the use of authorization for the `UPDATE_MAINTENANCE_SCHEDULE`
v1 API call, using the ACL `UpdateMaintenanceSchedule` and the action
of the same name as the API call.

It also updates the `ApiTests` to take into account the authorization
case.


Diffs
-

  src/master/http.cpp e2590a17044ac019b24a24629428d4ec8adc0c31 
  src/master/master.hpp 89d0790fd5fea59e74276f462581fe0073594732 
  src/tests/api_tests.cpp faf3242f9c86d866c4bb5e457fcfe47c1063cc09 


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


Testing
---

make check


Thanks,

Alexander Rojas

Review Request 59099: Enabled authorization for v1 API call UPDATE_MAINTENANCE_SCHEDULE.

[Alexander Rojas]

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

Review request for mesos, Adam B and Greg Mann.


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


Repository: mesos


Description
---

Enables the use of authorization for the `UPDATE_MAINTENANCE_SCHEDULEd`
v1 API call, using the ACL `UpdateMaintenanceSchedule` and the action
of the same name as the API call.

It also updates the `ApiTests` to take into account the authorization
case.


Diffs
-

  src/master/http.cpp e2590a17044ac019b24a24629428d4ec8adc0c31 
  src/master/master.hpp 89d0790fd5fea59e74276f462581fe0073594732 
  src/tests/api_tests.cpp faf3242f9c86d866c4bb5e457fcfe47c1063cc09 


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


Testing
---

make check


Thanks,

Alexander Rojas

Re: Review Request 59097: Fixed flakiness in agent registration validation tests.

[James Peach]

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



This improves the reliability of this test on my normal dev machine, but on my 
super slow vagrant VM this fails with the following error:

```
[ RUN  ] RegisterSlaveValidationTest.DropInvalidReregistration
../../src/tests/master_validation_tests.cpp:3762: Failure
Failed to wait 15secs for reregisterSlaveMessage
*** Aborted at 1494344715 (unix time) try "date -d @1494344715" if you are 
using GNU date ***
PC: @ 0x55c82ca8fd10 testing::UnitTest::AddTestPartResult()
*** SIGSEGV (@0x0) received by PID 25238 (TID 0x7fadc2313860) from PID 0; stack 
trace: ***
@ 0x7fadbaecb7e0 (unknown)
@ 0x55c82ca8fd10 testing::UnitTest::AddTestPartResult()
@ 0x55c82ca84775 testing::internal::AssertHelper::operator=()
@ 0x55c82c07beab 
mesos::internal::tests::RegisterSlaveValidationTest_DropInvalidReregistration_Test::TestBody()
@ 0x55c82caad476 
testing::internal::HandleSehExceptionsInMethodIfSupported<>()
@ 0x55c82caa86aa 
testing::internal::HandleExceptionsInMethodIfSupported<>()
@ 0x55c82ca89aff testing::Test::Run()
@ 0x55c82ca8a28d testing::TestInfo::Run()
@ 0x55c82ca8a8b7 testing::TestCase::Run()
@ 0x55c82ca911f7 testing::internal::UnitTestImpl::RunAllTests()
@ 0x55c82caae105 
testing::internal::HandleSehExceptionsInMethodIfSupported<>()
@ 0x55c82caa91ca 
testing::internal::HandleExceptionsInMethodIfSupported<>()
@ 0x55c82ca8ff2d testing::UnitTest::Run()
@ 0x55c82bdd917b RUN_ALL_TESTS()
@ 0x55c82bdd8c58 main
@ 0x7fadba3b4d1d (unknown)
@ 0x55c82b572179 (unknown)
```

- James Peach


On May 9, 2017, 3:01 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59097/
> ---
> 
> (Updated May 9, 2017, 3:01 p.m.)
> 
> 
> Review request for mesos, Jie Yu and James Peach.
> 
> 
> Bugs: MESOS-7441
> https://issues.apache.org/jira/browse/MESOS-7441
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> RegisterSlaveValidationTest.DropInvalidReregistration and
> RegisterSlaveValidationTest.DropInvalidRegistration.
> 
> 
> Diffs
> -
> 
>   src/tests/master_validation_tests.cpp 
> 3308803332bfae0d4b410eacd6d9a2a4a1265eff 
> 
> 
> Diff: https://reviews.apache.org/r/59097/diff/1/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> `./src/mesos-tests 
> --gtest_filter="RegisterSlaveValidationTest.DropInvalidReregistration" 
> --gtest_repeat=2000 --gtest_break_on_failure .`
> `./src/mesos-tests 
> --gtest_filter="RegisterSlaveValidationTest.DropInvalidRegistration" 
> --gtest_repeat=2000 --gtest_break_on_failure .`
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 58964: Added authorization support for operator endpoints.

[Alexander Rojas]

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

(Updated May 9, 2017, 5:48 p.m.)


Review request for mesos, Adam B and Greg Mann.


Changes
---

Change to implicit object so that operator can work on all nodes or none.


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


Repository: mesos


Description
---

Adds the actions `UPDATE_MAINTENANCE_SCHEDULE`,
`GET_MAINTENANCE_SCHEDULE`, `START_MAINTENANCE`, `STOP_MAINTENANCE`
and `GET_MAINTENANCE_STATUS` to the authorizer API as well as the
necesary code to handle these new actions.


Diffs (updated)
-

  include/mesos/authorizer/acls.proto ae0b1ea2e6417d186b1606542d75f3a20e0811db 
  include/mesos/authorizer/authorizer.proto 
c9184d151befa4cea9bdebb36a315c760e6424b2 
  src/authorizer/local/authorizer.cpp 89aaf4b712d337d519445c922606789c334e5101 


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

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


Testing
---

make check

specific tests not yet written.


Thanks,

Alexander Rojas

Re: Review Request 58955: Adds authorization for the master's v1 API call SET_LOGGING_LEVEL.

[Alexander Rojas]

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

(Updated May 9, 2017, 5:43 p.m.)


Review request for mesos, Adam B and Greg Mann.


Changes
---

rebase.


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


Repository: mesos


Description
---

Enables authorization in the v1 API call `SET_LOGGING_LEVEL` following
the existing implementation for the agent call with the same name.
Likewise it reuses the authorizer action `SET_LOG_LEVEL`.


Diffs (updated)
-

  src/master/http.cpp e2590a17044ac019b24a24629428d4ec8adc0c31 
  src/tests/api_tests.cpp faf3242f9c86d866c4bb5e457fcfe47c1063cc09 


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

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


Testing
---

make check


Thanks,

Alexander Rojas

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 9, 2017, 3:40 p.m.)


Review request for mesos and Vinod Kone.


Changes
---

Use `-dev` suffix for release branches.


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

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


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

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


Testing
---

Visual inspection, previewed with github gist.


Thanks,

Neil Conway

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

[Neil Conway]

> On May 8, 2017, 11:19 p.m., Vinod Kone wrote:
> > 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.

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

Sure, using "-dev" for release branches as well would be more consistent, I'll 
make that change.


- Neil


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


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 58900: Changed the default fetcher cache directory.

[Jie Yu]

> On May 8, 2017, 10:35 p.m., Joseph Wu wrote:
> > 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.

First of all, i think making `--fetcher_cache_dir` be the cache directory makes 
sense to me. I won't be too worried about the case of running multiple agents. 
If we want to run multiple agents, there are many other flags that we need to 
make unique as well (e.g., runtime_dir, cgroups_root, etc.)

For 3), can we avoid checkpointing. For instance, can we introduce a `trash` 
directory under `fetcher_cache_dir`, and rely on `rename` to atomically move 
the cached artifacts to the trash directory for removal.


- Jie


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


On May 8, 2017, 10: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, 10: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
> 
>

Review Request 59097: Fixed flakiness in agent registration validation tests.

[Neil Conway]

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

Review request for mesos, Jie Yu and James Peach.


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


Repository: mesos


Description
---

RegisterSlaveValidationTest.DropInvalidReregistration and
RegisterSlaveValidationTest.DropInvalidRegistration.


Diffs
-

  src/tests/master_validation_tests.cpp 
3308803332bfae0d4b410eacd6d9a2a4a1265eff 


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


Testing
---

`make check`

`./src/mesos-tests 
--gtest_filter="RegisterSlaveValidationTest.DropInvalidReregistration" 
--gtest_repeat=2000 --gtest_break_on_failure .`
`./src/mesos-tests 
--gtest_filter="RegisterSlaveValidationTest.DropInvalidRegistration" 
--gtest_repeat=2000 --gtest_break_on_failure .`


Thanks,

Neil Conway

Re: Review Request 58953: Added transitional allocator overloads.

[Jan Schlicht]

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


Ship it!




Ship It!

- Jan Schlicht


On May 9, 2017, 4:46 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58953/
> ---
> 
> (Updated May 9, 2017, 4:46 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Jie Yu, and Jan Schlicht.
> 
> 
> Bugs: MESOS-7388
> https://issues.apache.org/jira/browse/MESOS-7388
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This commits adds allocator overloads working with resource providers
> instead of agents which currently are assumed to be called with agent
> resource providers. This is so that users of the allocator can
> transition to the overloads taking resource providers.
> 
> In a subsequent commit the allocator overloads directly taking agent
> variables will be removed in favor of the ones taking resource
> providers; at the same time all methods in the allocator interface
> will be made virtual again in order to call specific derived
> implementation methods.
> 
> 
> Diffs
> -
> 
>   include/mesos/allocator/allocator.hpp 
> dc34a1b6f0c2bdf0d653bd53394364cfa9873ca7 
> 
> 
> Diff: https://reviews.apache.org/r/58953/diff/2/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 59081: Updated master to use resource provider IDs in allocator calls.

[Mesos Reviewbot]

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



Bad patch!

Reviews applied: [59081, 58953, 58952, 58951, 58950, 58949]

Failed command: python support/apply-reviews.py -n -r 58950

Error:
2017-05-09 14:51:16 URL:https://reviews.apache.org/r/58950/diff/raw/ 
[47997/47997] -> "58950.patch" [1]
error: patch failed: src/tests/persistent_volume_endpoints_tests.cpp:1964
error: src/tests/persistent_volume_endpoints_tests.cpp: patch does not apply

Full log: https://builds.apache.org/job/Mesos-Reviewbot/18004/console

- Mesos Reviewbot


On May 9, 2017, 8:14 a.m., Jan Schlicht wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59081/
> ---
> 
> (Updated May 9, 2017, 8:14 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The allocator interface expects resource provider IDs instead of slave
> IDs. The internal slave instance in the master will still use their
> slave IDs and create a resource provider ID from them to provide
> backwards compatibility.
> A later patch will change the callbacks of the allocator, until then
> the resource provider ID is copied from the slave ID of the callback.
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp e2590a17044ac019b24a24629428d4ec8adc0c31 
>   src/master/master.hpp 89d0790fd5fea59e74276f462581fe0073594732 
>   src/master/master.cpp 4e7a161f431624bd78d3e9032eb8587687149cad 
>   src/master/quota_handler.cpp 7fe55808d4561ae5a8850ad904d09a7c65e014ce 
>   src/master/weights_handler.cpp a4d2fed758878f3e2b9557a61965816aa9e0399c 
> 
> 
> Diff: https://reviews.apache.org/r/59081/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jan Schlicht
> 
>

Re: Review Request 59092: Added local resource provider driver.

[Benjamin Bannier]

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




include/mesos/v1/resource_provider.hpp
Lines 37 (patched)


Add a `virtual` dtr here.



include/mesos/v1/resource_provider.hpp
Lines 59-60 (patched)


Recently some people have used `override` to mark overridden functions 
instead of the purely documenting `virtual`. Up to you.



include/mesos/v1/resource_provider.hpp
Lines 69 (patched)


Any reason this needs to be potentially shareable? We typically seem to 
prefer owned semantics, how about a `unique_ptr`?



src/resource_provider/local_driver.cpp
Lines 17 (patched)


unused?



src/resource_provider/local_driver.cpp
Lines 23 (patched)


unused.



src/resource_provider/local_driver.cpp
Lines 63 (patched)


This overload hides `ProcessBase::send`. We should either rename this 
function, or unhide the base class method by pulling it into the class 
explicitly with a using decl.


- Benjamin Bannier


On May 9, 2017, 3:48 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59092/
> ---
> 
> (Updated May 9, 2017, 3:48 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
> ---
> 
> The local resource provider driver will be used by local resource
> providers. It relies on the agent's connection to the master. The
> agent will be responsible for forwarding Events and Calls by invoking
> corresponding functions from this driver.
> 
> 
> Diffs
> -
> 
>   include/mesos/v1/resource_provider.hpp PRE-CREATION 
>   include/mesos/v1/resource_provider/resource_provider.hpp 
> 2b8c8afab852621fb49b132813d512d0c96bc68c 
>   src/CMakeLists.txt 40d921ee7026f5ac47efbf0243e1cafab57825f9 
>   src/Makefile.am 6bb81fd49b4564a0afa993b2cef6baa9d370ee7a 
>   src/resource_provider/local_driver.cpp PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/59092/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 58953: Added transitional allocator overloads.

[Benjamin Bannier]

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

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


Review request for mesos, Benjamin Mahler, Jie Yu, and Jan Schlicht.


Changes
---

Rebased.


Summary (updated)
-

Added transitional allocator overloads.


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


Repository: mesos


Description
---

This commits adds allocator overloads working with resource providers
instead of agents which currently are assumed to be called with agent
resource providers. This is so that users of the allocator can
transition to the overloads taking resource providers.

In a subsequent commit the allocator overloads directly taking agent
variables will be removed in favor of the ones taking resource
providers; at the same time all methods in the allocator interface
will be made virtual again in order to call specific derived
implementation methods.


Diffs (updated)
-

  include/mesos/allocator/allocator.hpp 
dc34a1b6f0c2bdf0d653bd53394364cfa9873ca7 


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

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


Testing
---

`make check`


Thanks,

Benjamin Bannier

Re: Review Request 58952: Added test helper to disambiguate allocator method call.

[Benjamin Bannier]

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

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


Review request for mesos, Benjamin Mahler, Jie Yu, and Jan Schlicht.


Changes
---

Rebased.


Summary (updated)
-

Added test helper to disambiguate allocator method call.


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


Repository: mesos


Description
---

In a subsequent commit we will transitionally introduce a second
'addFramework' allocator method overload where the 'used' set refers
to 'ResourceProviderID's instead of 'SlaveID's. Introduce a test
helper here to remove a future ambiguity when an empty set '{}' is
passed. We introduce this in a separate patch so that this change can
more easily be removed one the old allocator method is removed and the
ambiguity vanishes.


Diffs (updated)
-

  src/tests/hierarchical_allocator_tests.cpp 
08180b9975869de328f0c095dd3cddf0c84fbecf 


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

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


Testing
---

`make check`


Thanks,

Benjamin Bannier

Re: Review Request 58821: Added a test that verifies a task and its check share the work dir.

[Alexander Rukletsov]

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

(Updated May 9, 2017, 2:32 p.m.)


Review request for mesos, Gastón Kleiman, Jie Yu, and Vinod Kone.


Changes
---

Avoided a potential race in the test.


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


Repository: mesos


Description
---

See summary.


Diffs (updated)
-

  src/tests/check_tests.cpp ec0d5ee78a94991c68f38174d09b41e8f2e4be35 


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

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


Testing
---

`make check` on Mac OS and Fedora 24


Thanks,

Alexander Rukletsov

Re: Review Request 58818: Ensured DEBUG container shares MESOS_SANDBOX with its parent.

[Alexander Rukletsov]

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

(Updated May 9, 2017, 2:31 p.m.)


Review request for mesos, Gastón Kleiman, Jie Yu, Kevin Klues, and Vinod Kone.


Changes
---

Rebased. NNTR.


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


Repository: mesos


Description
---

See summary.


Diffs (updated)
-

  src/slave/containerizer/mesos/containerizer.cpp 
3d724945812c0359ed175ce232f70886dc4401c8 
  src/tests/containerizer/nested_mesos_containerizer_tests.cpp 
c163b882fb2fc463537d6906c5a47b24a9a756c4 


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

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


Testing
---

`make check` on Mac OS and Fedora 24


Thanks,

Alexander Rukletsov

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

[Alexander Rukletsov]

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

(Updated May 9, 2017, 2:31 p.m.)


Review request for mesos, Gastón Kleiman, Jie Yu, and Vinod Kone.


Changes
---

Rebased. NNTR.


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

  src/slave/containerizer/mesos/containerizer.hpp 
29a99f33e646593127b9dc126f398f7bca88e21d 
  src/slave/containerizer/mesos/containerizer.cpp 
3d724945812c0359ed175ce232f70886dc4401c8 
  src/slave/containerizer/mesos/paths.hpp 
d85fd34660faacd9c73de2ba7b87b3bbd4b6007b 
  src/slave/containerizer/mesos/paths.cpp 
ed4bbd2491e71ad1e4a41e0575b514377d02da9b 


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

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


Testing
---

See https://reviews.apache.org/r/58718/


Thanks,

Alexander Rukletsov

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

[Alexander Rukletsov]

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

(Updated May 9, 2017, 2:32 p.m.)


Review request for mesos, Gastón Kleiman, Jie Yu, and Vinod Kone.


Changes
---

Fall back to sandbox directory in case of command executor and task with rootfs.


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


Repository: mesos


Description
---

A DEBUG container's working directory should be set to the parent
task's working directory. For the command executor case, even if
the task itself has a root filesystem, the executor container still
uses the host filesystem, hence
`ContainerLaunchInfo.working_directory`, pointing to the executor's
working directory in the host filesystem, may be different from the
task's working directory in the root filesystem.


Diffs (updated)
-

  include/mesos/slave/containerizer.proto 
41f2905df690bfe88ed762f1cd1246689fa4d3ea 
  src/slave/containerizer/mesos/containerizer.cpp 
3d724945812c0359ed175ce232f70886dc4401c8 


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

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


Testing
---

make check on Mac OS and Fedora 24


Thanks,

Alexander Rukletsov

Re: Review Request 58718: Added a test that verifies a task's env var is seen by its check.

[Alexander Rukletsov]

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

(Updated May 9, 2017, 2:31 p.m.)


Review request for mesos, Gastón Kleiman, Jie Yu, and Vinod Kone.


Changes
---

Rebased. NNTR.


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


Repository: mesos


Description
---

See summary.


Diffs (updated)
-

  src/tests/check_tests.cpp ec0d5ee78a94991c68f38174d09b41e8f2e4be35 


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

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


Testing
---

`make check` on various Linux distributions.


Thanks,

Alexander Rukletsov

Re: Review Request 58263: Inherited working dir from parent when launching a DEBUG container.

[Alexander Rukletsov]

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

(Updated May 9, 2017, 2:32 p.m.)


Review request for mesos, Gastón Kleiman, Jie Yu, Kevin Klues, and Vinod Kone.


Changes
---

Avoided a potential race in the test.


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


Repository: mesos


Description
---

See summary.


Diffs (updated)
-

  include/mesos/slave/containerizer.proto 
41f2905df690bfe88ed762f1cd1246689fa4d3ea 
  src/slave/containerizer/mesos/containerizer.cpp 
3d724945812c0359ed175ce232f70886dc4401c8 
  src/tests/containerizer/nested_mesos_containerizer_tests.cpp 
c163b882fb2fc463537d6906c5a47b24a9a756c4 


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

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


Testing
---

See https://reviews.apache.org/r/58821/


Thanks,

Alexander Rukletsov

Re: Review Request 58820: Explained how container working directory differs from its sandbox.

[Alexander Rukletsov]

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

(Updated May 9, 2017, 2:31 p.m.)


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


Changes
---

Rebased. NNTR.


Repository: mesos


Description
---

See summary.


Diffs (updated)
-

  include/mesos/slave/containerizer.proto 
41f2905df690bfe88ed762f1cd1246689fa4d3ea 
  src/slave/containerizer/mesos/containerizer.cpp 
3d724945812c0359ed175ce232f70886dc4401c8 


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

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


Testing
---

None: not a functional change.


Thanks,

Alexander Rukletsov

Re: Review Request 58819: Clarified the comment about Container.directory.

[Alexander Rukletsov]

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

(Updated May 9, 2017, 2:31 p.m.)


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


Changes
---

Rebased. NNTR.


Repository: mesos


Description
---

See summary.


Diffs (updated)
-

  src/slave/containerizer/mesos/containerizer.hpp 
29a99f33e646593127b9dc126f398f7bca88e21d 


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

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


Testing
---

None: not a functional change.


Thanks,

Alexander Rukletsov

Re: Review Request 58262: Inherited environment from parent when launching a DEBUG container.

[Alexander Rukletsov]

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

(Updated May 9, 2017, 2:31 p.m.)


Review request for mesos, Gastón Kleiman, Jie Yu, Kevin Klues, and Vinod Kone.


Changes
---

Rebased. NNTR.


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


Repository: mesos


Description
---

See summary.


Diffs (updated)
-

  src/slave/containerizer/mesos/containerizer.cpp 
3d724945812c0359ed175ce232f70886dc4401c8 
  src/tests/containerizer/nested_mesos_containerizer_tests.cpp 
c163b882fb2fc463537d6906c5a47b24a9a756c4 


Diff: https://reviews.apache.org/r/58262/diff/8/

Changes: https://reviews.apache.org/r/58262/diff/7-8/


Testing
---

See https://reviews.apache.org/r/58718/


Thanks,

Alexander Rukletsov

Re: Review Request 58817: Captured AgentID from the offer by reference in tests.

[Alexander Rukletsov]

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

(Updated May 9, 2017, 2:31 p.m.)


Review request for mesos, Gastón Kleiman and Vinod Kone.


Changes
---

Rebased. NNTR.


Repository: mesos


Description
---

See summary.


Diffs (updated)
-

  src/tests/check_tests.cpp ec0d5ee78a94991c68f38174d09b41e8f2e4be35 
  src/tests/slave_tests.cpp 8c97dc6d088708d301dc3ccf90d413fd785b782f 


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

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


Testing
---

make check


Thanks,

Alexander Rukletsov

Re: Review Request 59094: Fixed an ordering style issue in automake file.

[Benjamin Bannier]

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


Ship it!




- Benjamin Bannier


On May 9, 2017, 4:01 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59094/
> ---
> 
> (Updated May 9, 2017, 4:01 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and Jan Schlicht.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed an ordering style issue in automake file.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 6bb81fd49b4564a0afa993b2cef6baa9d370ee7a 
> 
> 
> Diff: https://reviews.apache.org/r/59094/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Review Request 59094: Fixed an ordering style issue in automake file.

[Jie Yu]

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

Review request for mesos, Benjamin Bannier and Jan Schlicht.


Repository: mesos


Description
---

Fixed an ordering style issue in automake file.


Diffs
-

  src/Makefile.am 6bb81fd49b4564a0afa993b2cef6baa9d370ee7a 


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


Testing
---

make check


Thanks,

Jie Yu

Review Request 59093: Added a general resource provider implementation.

[Jie Yu]

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

Review request for mesos, Benjamin Bannier and Jan Schlicht.


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


Repository: mesos


Description
---

The newly introduced `ResourceProvider` class will serve as the base
class for all resource provider implementations in Mesos. It
implements some of the basic functionalities for a resource provider
and allow subclasses to do specialization by implementing some virtual
functions.


Diffs
-

  src/CMakeLists.txt 40d921ee7026f5ac47efbf0243e1cafab57825f9 
  src/Makefile.am 6bb81fd49b4564a0afa993b2cef6baa9d370ee7a 
  src/resource_provider/resource_provider.hpp PRE-CREATION 
  src/resource_provider/resource_provider.cpp PRE-CREATION 


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


Testing
---

make check


Thanks,

Jie Yu

Review Request 59092: Added local resource provider driver.

[Jie Yu]

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

Review request for mesos, Benjamin Bannier and Jan Schlicht.


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


Repository: mesos


Description
---

The local resource provider driver will be used by local resource
providers. It relies on the agent's connection to the master. The
agent will be responsible for forwarding Events and Calls by invoking
corresponding functions from this driver.


Diffs
-

  include/mesos/v1/resource_provider.hpp PRE-CREATION 
  include/mesos/v1/resource_provider/resource_provider.hpp 
2b8c8afab852621fb49b132813d512d0c96bc68c 
  src/CMakeLists.txt 40d921ee7026f5ac47efbf0243e1cafab57825f9 
  src/Makefile.am 6bb81fd49b4564a0afa993b2cef6baa9d370ee7a 
  src/resource_provider/local_driver.cpp PRE-CREATION 


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


Testing
---

make check


Thanks,

Jie Yu

Review Request 59091: Added resource provider protobufs to Java and Python bindings.

[Jie Yu]

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

Review request for mesos, Benjamin Bannier and Jan Schlicht.


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


Repository: mesos


Description
---

Added resource provider protobufs to Java and Python bindings.


Diffs
-

  src/Makefile.am 6bb81fd49b4564a0afa993b2cef6baa9d370ee7a 


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


Testing
---

make check


Thanks,

Jie Yu

Re: Review Request 59083: Added resource provider actions.

[Jan Schlicht]

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

(Updated May 9, 2017, 1:06 p.m.)


Review request for mesos, Benjamin Bannier and Jie Yu.


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


Repository: mesos


Description
---

Added resource provider actions.


Diffs
-

  include/mesos/resource_provider/resource_provider.proto 
0efef33010d53c3f0098e70ad5c626b9b69bcc1a 
  include/mesos/v1/resource_provider/resource_provider.proto 
82e6741d96cf1005b1398fb6453cf2a899583c9f 


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


Testing
---

make check


Thanks,

Jan Schlicht

Re: Review Request 59083: Added resource provider actions.

[Jie Yu]

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


Ship it!




Ship It!

- Jie Yu


On May 9, 2017, 9:40 a.m., Jan Schlicht wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59083/
> ---
> 
> (Updated May 9, 2017, 9:40 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added resource provider actions.
> 
> 
> Diffs
> -
> 
>   include/mesos/resource_provider/resource_provider.proto 
> 0efef33010d53c3f0098e70ad5c626b9b69bcc1a 
>   include/mesos/v1/resource_provider/resource_provider.proto 
> 82e6741d96cf1005b1398fb6453cf2a899583c9f 
> 
> 
> Diff: https://reviews.apache.org/r/59083/diff/3/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jan Schlicht
> 
>

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

[Mesos Reviewbot]

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



Patch looks great!

Reviews applied: [58719, 58720]

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

- Mesos Reviewbot


On May 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/6/
> 
> 
> Testing
> ---
> 
> PEP8 and Pylint used to make sure that the code style is correct. Manuel test:
> 
> $ cd src/cli_new
> $ ./bootstrap
> $ source activate
> 
> (mesos-cli) $ mesos-cli-tests
> 
> 
> Thanks,
> 
> Armand Grillet
> 
>

Review Request 59090: Renabled some tests, fixed some errors.

[Jay Guo]

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

Review request for mesos, Benjamin Mahler, Michael Park, and Neil Conway.


Repository: mesos


Description
---

Renabled some tests, fixed some errors.


Diffs
-

  src/tests/hierarchical_allocator_tests.cpp 
08180b9975869de328f0c095dd3cddf0c84fbecf 


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


Testing
---

make check


Thanks,

Jay Guo

Review Request 59089: Changed hierarchical allocator to use QuotaTree.

[Jay Guo]

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

Review request for mesos, Benjamin Mahler, Michael Park, and Neil Conway.


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


Repository: mesos


Description
---

Changed hierarchical allocator to use QuotaTree to keep track of
quotas instead of using hashmap. This allows us to correctly
account hierarchical quota, see MESOS-7402.


Diffs
-

  src/master/allocator/mesos/hierarchical.hpp 
123f97cf495bff0f822838e09df0d88818f04da6 
  src/master/allocator/mesos/hierarchical.cpp 
b75ed9a20a9a42f958cebbacd91e5e15b0d21394 


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


Testing
---


Thanks,

Jay Guo

Review Request 59088: Implemented QuotaTree structure to be used by allocator.

[Jay Guo]

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

Review request for mesos, Benjamin Mahler, Michael Park, and Neil Conway.


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


Repository: mesos


Description
---

Implemented QuotaTree structure to be used by allocator.


Diffs
-

  src/Makefile.am 6bb81fd49b4564a0afa993b2cef6baa9d370ee7a 
  src/master/quota.hpp 479112d1a94ede34e2d46a27ff03999fc1634b8c 
  src/master/quota.cpp 3e0d280efff69904b74e5d4c71985ab44ffa34a7 
  src/tests/quota_tests.cpp PRE-CREATION 


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


Testing
---

see end of chain.


Thanks,

Jay Guo

Re: Review Request 59083: Added resource provider actions.

[Jan Schlicht]

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

(Updated May 9, 2017, 11:40 a.m.)


Review request for mesos, Benjamin Bannier and Jie Yu.


Changes
---

Added missing import.


Repository: mesos


Description
---

Added resource provider actions.


Diffs (updated)
-

  include/mesos/resource_provider/resource_provider.proto 
0efef33010d53c3f0098e70ad5c626b9b69bcc1a 
  include/mesos/v1/resource_provider/resource_provider.proto 
82e6741d96cf1005b1398fb6453cf2a899583c9f 


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

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


Testing
---

make check


Thanks,

Jan Schlicht

Re: Review Request 59083: Added resource provider actions.

[Jan Schlicht]

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

(Updated May 9, 2017, 11:37 a.m.)


Review request for mesos, Benjamin Bannier and Jie Yu.


Changes
---

Added changes to v1 proto.


Repository: mesos


Description
---

Added resource provider actions.


Diffs (updated)
-

  include/mesos/resource_provider/resource_provider.proto 
0efef33010d53c3f0098e70ad5c626b9b69bcc1a 
  include/mesos/v1/resource_provider/resource_provider.proto 
82e6741d96cf1005b1398fb6453cf2a899583c9f 


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

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


Testing
---

make check


Thanks,

Jan Schlicht

Re: Review Request 59083: Added resource provider actions.

[Jie Yu]

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



Can you update v1 API as well?

- Jie Yu


On May 9, 2017, 8:51 a.m., Jan Schlicht wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59083/
> ---
> 
> (Updated May 9, 2017, 8:51 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added resource provider actions.
> 
> 
> Diffs
> -
> 
>   include/mesos/resource_provider/resource_provider.proto 
> 0efef33010d53c3f0098e70ad5c626b9b69bcc1a 
> 
> 
> Diff: https://reviews.apache.org/r/59083/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jan Schlicht
> 
>

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

[Jie Yu]

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




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


+ @kapil

It's sad to see that raw pointer has been consistently used here, where it 
should really be a managed pointer.

Let's make sure we do a cleanup ASAP.


- Jie Yu


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

Review Request 59083: Added resource provider actions.

[Jan Schlicht]

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

Review request for mesos, Benjamin Bannier and Jie Yu.


Repository: mesos


Description
---

Added resource provider actions.


Diffs
-

  include/mesos/resource_provider/resource_provider.proto 
0efef33010d53c3f0098e70ad5c626b9b69bcc1a 


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


Testing
---

make check


Thanks,

Jan Schlicht

Review Request 59082: Added provider ID to offers.

[Jan Schlicht]

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

Review request for mesos, Benjamin Bannier and Jie Yu.


Repository: mesos


Description
---

Added provider ID to offers.


Diffs
-

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


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


Testing
---

make check


Thanks,

Jan Schlicht

Re: Review Request 58136: Cleaned up pattern matching in Resource operator==.

[Jie Yu]

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


Ship it!




Ship It!

- Jie Yu


On April 3, 2017, 7:57 a.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58136/
> ---
> 
> (Updated April 3, 2017, 7:57 a.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The usual pattern for checking optional fields here is to check
> whether the field is is either set for both sides or unset for both.
> Subsequentially we check if possible values are equal, returning
> 'false' if they are not.
> 
> The code changed here did not follow this pattern, but instead assumed
> it was the last check to run. This made it possible that a careless
> addition after these checks would never be called.
> 
> 
> Diffs
> -
> 
>   src/common/resources.cpp c26e0f995006dc6b2e70a491cea58fa90347e42a 
>   src/v1/resources.cpp a53deafbea399a1bcf729d1c151bc46e9da04e11 
> 
> 
> Diff: https://reviews.apache.org/r/58136/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Review Request 59081: Updated master to use resource provider IDs in allocator calls.

[Jan Schlicht]

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

Review request for mesos, Benjamin Bannier and Jie Yu.


Repository: mesos


Description
---

The allocator interface expects resource provider IDs instead of slave
IDs. The internal slave instance in the master will still use their
slave IDs and create a resource provider ID from them to provide
backwards compatibility.
A later patch will change the callbacks of the allocator, until then
the resource provider ID is copied from the slave ID of the callback.


Diffs
-

  src/master/http.cpp e2590a17044ac019b24a24629428d4ec8adc0c31 
  src/master/master.hpp 89d0790fd5fea59e74276f462581fe0073594732 
  src/master/master.cpp 4e7a161f431624bd78d3e9032eb8587687149cad 
  src/master/quota_handler.cpp 7fe55808d4561ae5a8850ad904d09a7c65e014ce 
  src/master/weights_handler.cpp a4d2fed758878f3e2b9557a61965816aa9e0399c 


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


Testing
---

make check


Thanks,

Jan Schlicht

Re: Review Request 58949: Introduced ResourceProviderInfo proto.

[Jan Schlicht]

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


Ship it!




Ship It!

- Jan Schlicht


On May 9, 2017, 9:58 a.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58949/
> ---
> 
> (Updated May 9, 2017, 9:58 a.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/3/
> 
> 
> 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 9, 2017, 9:58 a.m.)


Review request for mesos, Jie Yu and Jan Schlicht.


Changes
---

Addressed Jie's comments.


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

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


Testing
---

`make check` (Fedora25, clang-5)


Thanks,

Benjamin Bannier

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

[Jie Yu]

> 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?
> 
> Chun-Hung Hsiao wrote:
> I perfor the more specific `isdir` check here if the `rootfs` file is 
> always a directory in all scenarios we want to ignore.

OK. i don't realized that all existence check in this file uses 'isdir'


- Jie


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

[Jie Yu]

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


Ship it!




Ship It!

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

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

[Jie Yu]

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


Ship it!




Ship It!

- Jie Yu


On May 8, 2017, 8:48 p.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59063/
> ---
> 
> (Updated May 8, 2017, 8:48 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Gilbert Song, and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Changed `#elif ...` to `#elif defined(...)` to match `#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
> 
>

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

[Jie Yu]

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



As we discussed offline, let's not mess the URI struct. Instead, passing the 
secret to the fetcher using an function parameter in the `fetch` method. This 
will be useful in the future also because fetcher other URIs might need secret 
as well.

- Jie Yu


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