Re: Review Request 47695: Updated the website generation and development workflows with docker.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [47695]

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 24, 2016, 2:38 a.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47695/
> ---
> 
> (Updated May 24, 2016, 2:38 a.m.)
> 
> 
> Review request for mesos, Tomasz Janiszewski, Kevin Klues, Neil Conway, and 
> Vinod Kone.
> 
> 
> Bugs: MESOS-5431
> https://issues.apache.org/jira/browse/MESOS-5431
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> In this changes, we include below items to make it more convenience to
> develop and generate the website.
> 
> * added `doxygen` and `javadoc` to `rake` default target.
> * added `build.sh` as the wrapper for the website generation to make
>   sure we clean up generated documents when exit.
> * switched the `Dockerfile` to based on `centos:7` because the default
>   `doxygen` version in `ubuntu:14.04` doesn't include `jquery.js`
>   correctly when generate `doxygen` documents.
> * adjusted the `Dockerfile` and `README.md` locations to avoid we
>   distract at finding the website generation document.
> * updated `release-guide.md` with the new website generation and
>   development workflows.
> 
> 
> Diffs
> -
> 
>   docs/release-guide.md 36e79796657a37b0b4649602281556f4ae471c34 
>   site/.gitignore 987a5d3cc161a67de1fb21c2002ed9698bbb0689 
>   site/Gemfile 14c50340d66fd7a703949a7932ab93a6b60f4e07 
>   site/Gemfile.lock 065107ec25f0eff28e2a09051b61152d49f77f9a 
>   site/README.md 468021ef4f8ed942c4a16784bc82349af9ae0d5e 
>   site/Rakefile 845ec3fd8b9e47ebe49991db30718912d4f9c41e 
>   site/build.sh PRE-CREATION 
>   support/site-docker/Dockerfile 8ea3d5c5fe531ec76950ed0bf110d7d477339158 
>   support/site-docker/README.md 957117e1211f06b40c5fb621039dd90aff562a87 
> 
> Diff: https://reviews.apache.org/r/47695/diff/
> 
> 
> Testing
> ---
> 
> Tested in OS X and Ubuntu 14.04
> 
> For document changes, could verified from 
> https://github.com/haosdent/mesos/tree/MESOS-5431/site
> 
> To verify website works fine after this changes, could check it via this gif:
> 
> ![website.gif](https://issues.apache.org/jira/secure/attachment/12805684/website.gif)
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

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

[Joerg Schad]

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




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


UpdateWeight



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


ViewFramework



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


ViewTask



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


Add message VIewExecutor



include/mesos/authorizer/authorizer.proto (lines 69 - 70)


Support these actions in Request based model.



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


Add comments about set fields.



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


VIEW_EXECUTOR



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


Add implementation for different actions.


- Joerg Schad


On May 21, 2016, 9:29 p.m., Joerg Schad wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46613/
> ---
> 
> (Updated May 21, 2016, 9:29 p.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rojas, and Michael Park.
> 
> 
> Bugs: MESOS-5169
> https://issues.apache.org/jira/browse/MESOS-5169
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> In order to allow for framework and task level filtering we introduce
> the following authorizer actions:
> * VIEW_FRAMEWORK_WITH_INFO
> * VIEW_TASK_WITH_TASK
> 
> Note that we need different actions for authorizing a tasks
> based on the object being authorized.
> 
> We also introduce the following acls for the local authorizer:
> * ViewFrameworks  (giving access to frameworks running under
>   a specific OS user)
> * ViewTasks view_tasks (giving access to Tasks run under a
> specific OS user)
> 
> 
> Diffs
> -
> 
>   include/mesos/authorizer/acls.proto 
> b178f53a299a2941afc073af963f6aff26af1ca8 
>   include/mesos/authorizer/authorizer.proto 
> 911a2271211249a41c4467f6754e9996f640bf38 
>   src/authorizer/local/authorizer.cpp 
> dc53bc4374aea98b5ed41ade5617374d2447229b 
> 
> Diff: https://reviews.apache.org/r/46613/diff/
> 
> 
> Testing
> ---
> 
> make check (OSX)
> 
> 
> Thanks,
> 
> Joerg Schad
> 
>

Re: Review Request 47707: Refactored resource enumeration for containerizers and isolators.

[Kevin Klues]

> On May 23, 2016, 7:56 p.m., Benjamin Mahler wrote:
> > src/slave/containerizer/containerizer.hpp, lines 64-68
> > 
> >
> > Hm.. it seems more like the default set of resources would not include 
> > the flags.
> > 
> > The current function is a little more than just the default resources, 
> > it is the flag provided resources along with defaults injected in for any 
> > that were omitted.
> > 
> > Could we instead provide just the defaults (per the suggested signature 
> > above) and have containerizer implementations inject defaults with the help 
> > of this function?
> > 
> > As an example:
> > 
> > ```
> > // Provides a means for containerizer implementation to use a 
> > // consistent set of default resources when resources are not
> > // specified via the '--resources' flag. The containerizer may
> > // choose its own default value by probing the system, however
> > // this helper provides a standard set of defaults for:
> > //
> > // ["cpus", "mem", "disk", "ports"]
> > //
> > // A containerizer is free to choose alternative defaults for these
> > // resources, but this increases the likelihood that containerizers
> > // disagree about the amount of resources to manage when used within
> > // the composing containerizer.
> > static Try defaultResources();
> > ```
> > 
> > This means we could augment the Containerizer::resources and 
> > Isolator::resources to do two things:
> > 
> > (1) Get informed about the resources provided via flags, and
> > (2) Return additional resources that the containerizer or isolator 
> > wishes to manage.
> > 
> > Something like:
> > 
> > ```
> > // Informs the containerizer / isolator about the resources
> > // that are explicitly specified. The containerizer / isolator
> > // returns a subset of the resources that it will manage. The
> > // containerizer / isolator may also return additional resources
> > // that it wishes to manage if they were not specified.
> > // For example, a GPU isolator, seeing that "gpus" is unspecified
> > // may probe the system and decide to manage some GPUs (i.e. it
> > // will return these GPUs from this function).
> > Future manage(const Resources& provided);
> > ```
> 
> Kevin Klues wrote:
> We could do this (though it feels a little less intuitive and less 
> flexible than allowing arbitrary flags to be passed in).  We are also 
> unnecessarily enumerating all of the `cpu, mem, disk, ports` available on the 
> system even if the flags would indicate to override them. Is the assumption 
> that the actual class implementing `manage()` would need to find a way to get 
> at the flags itself if it relies on them?  How would the default 
> implementation of `manage()` work?  The whole goal of it was to avoid having 
> to push new logic down into each existing containerizer (which your proposal 
> will require in order to do the flags parsing somehwere to override the 
> defaults).

Also, how do you envision this working for `defaultResources()` not taking 
flags, when it relies on `flags.work_dir` (for disk) and `flags.default_role` 
(for all default resources when setting their role).


- Kevin


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


On May 23, 2016, 12:45 a.m., Kevin Klues wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47707/
> ---
> 
> (Updated May 23, 2016, 12:45 a.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Jie Yu.
> 
> 
> Bugs: MESOS-5256
> https://issues.apache.org/jira/browse/MESOS-5256
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Previously, the `Containerizer` class contained a static `resources()`
> function for enumerating all of the resources available on an agent.
> This made it hard to add new resources to this enumeration that were,
> for example, containerizer-specific or tied to a new isolator module
> loaded at runtime.
> 
> This commit refactors the resource enumeration logic to allow each
> containerizer to enumerate resources itself. It does this by adding a
> new virtual `resources()` function, whose default implementation
> includes the same logic as the old static version of `resources()`.
> Individual containerizers simply overwrite this function to do their
> own enumeration if they want to.
> 
> Similar logic exists to push resource enumeration down into isolators
> as well. As such, the logic for the Nvidia GPU resource enumeration has
> been moved down into the 

Re: Review Request 47528: Updated validation tests with creator principal.

[Greg Mann]

> On May 23, 2016, 2:06 p.m., Bernd Mathiske wrote:
> > src/tests/master_validation_tests.cpp, line 454
> > 
> >
> > Where are we setting the principal for DiskInfo.Persistence?
> > 
> > How is this block of code any different from the one below?

Ah, whoops!! Thanks Bernd :-)


- Greg


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


On May 24, 2016, 4:44 a.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47528/
> ---
> 
> (Updated May 24, 2016, 4:44 a.m.)
> 
> 
> Review request for mesos, Bernd Mathiske, Joerg Schad, and Neil Conway.
> 
> 
> Bugs: MESOS-5005
> https://issues.apache.org/jira/browse/MESOS-5005
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The master validation tests are updated to include the
> new `validate()` signature, and a new test is added,
> `CreateOperationValidationTest.NonMatchingPrincipal`,
> to ensure that a non-matching creator principal will
> be invalidated.
> 
> 
> Diffs
> -
> 
>   src/tests/master_validation_tests.cpp 
> ca4442aa1ef0087a7d058d1b3aa430a1dbc16960 
> 
> Diff: https://reviews.apache.org/r/47528/diff/
> 
> 
> Testing
> ---
> 
> `make check` was used to test on OSX at the end of this review chain.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 47528: Updated validation tests with creator principal.

[Greg Mann]

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

(Updated May 24, 2016, 4:44 a.m.)


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


Changes
---

Addressed comments.


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


Repository: mesos


Description
---

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


Diffs (updated)
-

  src/tests/master_validation_tests.cpp 
ca4442aa1ef0087a7d058d1b3aa430a1dbc16960 

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


Testing
---

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


Thanks,

Greg Mann

Re: Review Request 47162: Libprocess: Made some of the tests work on Windows.

[Joris Van Remoortere]

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


Fix it, then Ship it!





3rdparty/libprocess/src/tests/subprocess_tests.cpp (line 65)


we can substitute some of these `.get().` with `->`



3rdparty/libprocess/src/tests/subprocess_tests.cpp (lines 85 - 86)


We can use this pattern:
```
Try testdir = os::mkdtemp();
ASSERT_SOME(testdir);
```



3rdparty/libprocess/src/tests/subprocess_tests.cpp (lines 115 - 121)


indentation.



3rdparty/libprocess/src/tests/subprocess_tests.cpp (lines 206 - 227)


indentation.



3rdparty/libprocess/src/tests/subprocess_tests.cpp (line 213)


`snake_case`


- Joris Van Remoortere


On May 13, 2016, 6:18 p.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47162/
> ---
> 
> (Updated May 13, 2016, 6:18 p.m.)
> 
> 
> Review request for mesos, Alex Naparu, Daniel Pravat, Artem Harutyunyan, 
> Joris Van Remoortere, Michael Park, M Lawindi, and Yi Sun.
> 
> 
> Bugs: MESOS-3371
> https://issues.apache.org/jira/browse/MESOS-3371
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Libprocess: Made some of the tests work on Windows.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/tests/main.cpp 
> c2ce9bc7830cd7bb0aa473f985106ec745207bf8 
>   3rdparty/libprocess/src/tests/subprocess_tests.cpp 
> 727e940f12643974de4ff2734fba431b285b5de3 
> 
> Diff: https://reviews.apache.org/r/47162/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>

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

[Guangya Liu]

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

(Updated May 24, 2016, 4:11 a.m.)


Review request for mesos, Gilbert Song and Jie Yu.


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


Repository: mesos


Description
---

Added documentation for `docker/volume` isolator.


Diffs (updated)
-

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

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


Testing
---

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


Thanks,

Guangya Liu

Re: Review Request 47374: Experimental: Separated mesos test helpers into a separate library.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [47374]

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 24, 2016, 2:02 a.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47374/
> ---
> 
> (Updated May 24, 2016, 2:02 a.m.)
> 
> 
> Review request for mesos, Adam B, Artem Harutyunyan, Jie Yu, Kapil Arya, Jan 
> Schlicht, and Till Toenshoff.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This gives external projects easier access to the test helpers used in
> mesos tests.  
> 
> For example, a module writer may want to write a test like 
> `src/tests/oversubscription_tests.cpp`.  To build and link against 
> this library, the module writer would mimic the build flags for tests:
> ```
> my_module_tests_CPPFLAGS = \
>   -I$(ZOOKEEPER)/include   \
>   -I$(ZOOKEEPER)/generated \
>   -DSOURCE_DIR=\"$(abs_top_srcdir)\"   \
>   -DBUILD_DIR=\"$(abs_top_builddir)\"  \
>   ...
>   
> my_module_tests_LDADD = libmesos_tests.la
> ```
> 
> 
> Diffs
> -
> 
>   src/Makefile.am ce5245883f3d2661812272702c0d2060513b6d88 
> 
> Diff: https://reviews.apache.org/r/47374/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Re: Review Request 47216: Wired up the new docker environment hook.

[Kapil Arya]

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




src/slave/containerizer/docker.cpp (line 1042)


Can we bring the RHS on the next line and adjust the rest to make it 
slightly more readable?


- Kapil Arya


On May 11, 2016, 3:30 p.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47216/
> ---
> 
> (Updated May 11, 2016, 3:30 p.m.)
> 
> 
> Review request for mesos, Adam B, Artem Harutyunyan, Jie Yu, and Kapil Arya.
> 
> 
> Bugs: MESOS-5350
> https://issues.apache.org/jira/browse/MESOS-5350
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Modifies the code path for docker command executors. (Custom 
> executors are not supported because they may break if exposed to an
> unfamiliar flag.)
> 
> Docker command executors are now launched with an additional flag
> that is filled in by a hook.  The --task_environment flag tells the
> command executor to pass some specified mapping of environment
> variables to the task.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/docker.hpp fac26a239e9981e5b1c2f2f90b52785819492c2a 
>   src/slave/containerizer/docker.cpp 7af14f4472283ceefd73c06dd8df60af4cf6f7e8 
> 
> Diff: https://reviews.apache.org/r/47216/diff/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Re: Review Request 47150: Implemented new asynchronous docker pre-launch hook.

[Kapil Arya]

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




include/mesos/hook.hpp (line 112)


Can we replace `name` with something more explicit/precise?



include/mesos/hook.hpp (line 116)


I wonder if we should force the module to use `Environment` and internally 
use `map` if needed?



src/examples/test_hook_module.cpp (line 188)


s/an/a/



src/examples/test_hook_module.cpp (line 202)


It could be my screen, but could you double-check the alignment here?


- Kapil Arya


On May 10, 2016, 10:13 p.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47150/
> ---
> 
> (Updated May 10, 2016, 10:13 p.m.)
> 
> 
> Review request for mesos, Adam B, Artem Harutyunyan, Jie Yu, and Kapil Arya.
> 
> 
> Bugs: MESOS-5350
> https://issues.apache.org/jira/browse/MESOS-5350
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Introduces, but does not fully wire up a new hook.
> 
> The new hook, "slavePreLaunchDockerEnvironmentDecoratorAndValidator",
> has divergent semantics compared with existing hooks:
> 
> * The hook is asynchronous,
> * can prevent a task from launching if it errors,
> * can overwrite environment variables.
> 
> 
> Diffs
> -
> 
>   include/mesos/hook.hpp 210ffba09f5acae34ca49b888a781f683777f9ca 
>   src/examples/test_hook_module.cpp 4b97f84204934d0e678786fd6cde38b89a6f8f48 
>   src/hook/manager.hpp 528674e36639fe78137ba0a4bb004c99730e7a22 
>   src/hook/manager.cpp 381807d582998043d73e9b8c9d3c1fddbcf73cf1 
>   src/tests/hook_tests.cpp 60d52c5849ba555f6f3070883d87aadf105f05b0 
> 
> Diff: https://reviews.apache.org/r/47150/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Re: Review Request 47215: Changed the dockerized docker command executor CommandInfo usage.

[Kapil Arya]

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


Ship it!




Ship It!

- Kapil Arya


On May 10, 2016, 10:13 p.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47215/
> ---
> 
> (Updated May 10, 2016, 10:13 p.m.)
> 
> 
> Review request for mesos, Adam B, Artem Harutyunyan, Jie Yu, and Kapil Arya.
> 
> 
> Bugs: MESOS-5350
> https://issues.apache.org/jira/browse/MESOS-5350
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This changes how we override the `CommandInfo` when launching a 
> dockerized executor; from `shell == true` to `shell = false`.  
> This means that flags are now passed directly rather than as 
> a long string.
> 
> i.e. 
> From: 'mesos-docker-executor --foo="bar" --some="thing"'
> To: [ 'mesos-docker-executor', '--foo=bar', '--some=thing' ]
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/docker.cpp 7af14f4472283ceefd73c06dd8df60af4cf6f7e8 
> 
> Diff: https://reviews.apache.org/r/47215/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Re: Review Request 46438: Added the test "CniIsolatorTest.ROOT_SlaveRecovery".

[Qian Zhang]

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

(Updated May 24, 2016, 11:20 a.m.)


Review request for mesos, Avinash sridharan and Jie Yu.


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


Repository: mesos


Description
---

Added the test "CniIsolatorTest.ROOT_SlaveRecovery".


Diffs (updated)
-

  src/tests/containerizer/cni_isolator_tests.cpp 
6ef15cc9b089319c4e62f8cd20565f00686f2089 

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


Testing (updated)
---

[ RUN  ] CniIsolatorTest.ROOT_SlaveRecovery
I0524 11:06:18.697321 20808 exec.cpp:150] Version: 0.29.0
I0524 11:06:18.706725 20807 exec.cpp:225] Executor registered on agent 
7399d7c2-a7b4-47ed-9b95-8a0a87312816-S0
Received SUBSCRIBED event
Subscribed executor on mesos
Received LAUNCH event
Starting task d4e6feb2-dce2-4d90-b7c4-35ce27b8757d
Forked command at 20814
sh -c 'sleep 1000'
I0524 11:06:18.895862 20806 exec.cpp:271] Received reconnect request from agent 
7399d7c2-a7b4-47ed-9b95-8a0a87312816-S0
I0524 11:06:18.899616 20807 exec.cpp:248] Executor re-registered on agent 
7399d7c2-a7b4-47ed-9b95-8a0a87312816-S0
Received SUBSCRIBED event
Subscribed executor on mesos
Received KILL event
Received kill for task d4e6feb2-dce2-4d90-b7c4-35ce27b8757d with grace period 
of 3secs
Sending SIGTERM to process tree at pid 20814
Sent SIGTERM to the following process trees:
[ 
-+- 20814 sh -c sleep 1000 
 --- 20815 sleep 1000 
]
Scheduling escalation to SIGKILL in 3secs from now
Command terminated with signal Terminated (pid: 20814)
I0524 11:06:21.020820 20808 exec.cpp:399] Executor asked to shutdown
[   OK ] CniIsolatorTest.ROOT_SlaveRecovery (3575 ms)


Thanks,

Qian Zhang

Re: Review Request 47214: Replaced subprocess flag stringification with flags.toVector().

[Kapil Arya]

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


Ship it!




Ship It!

- Kapil Arya


On May 16, 2016, 2:01 p.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47214/
> ---
> 
> (Updated May 16, 2016, 2:01 p.m.)
> 
> 
> Review request for mesos, Adam B, Artem Harutyunyan, Jie Yu, and Kapil Arya.
> 
> 
> Bugs: MESOS-5350
> https://issues.apache.org/jira/browse/MESOS-5350
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/subprocess.cpp 
> 44073146118b6c6e9e730b8c901852594080a3eb 
> 
> Diff: https://reviews.apache.org/r/47214/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Re: Review Request 47213: Added FlagsBase::toVector method as an alternative to stringify(flags).

[Kapil Arya]

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


Ship it!





3rdparty/stout/include/stout/flags/flags.hpp (line 1027)


Just curious, will it do the right thing for boolean flags such as 
`--quiet`, `--no-quiet`, `--version` and `--no-version`?


- Kapil Arya


On May 16, 2016, 2 p.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47213/
> ---
> 
> (Updated May 16, 2016, 2 p.m.)
> 
> 
> Review request for mesos, Adam B, Artem Harutyunyan, Jie Yu, and Kapil Arya.
> 
> 
> Bugs: MESOS-5350
> https://issues.apache.org/jira/browse/MESOS-5350
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> `stringify(flags)` will return a version of the flags that can be passed
> through `/bin/sh` for execution.  This method returns the same flags,
> but in a more `exec`-friendly form.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/flags/flags.hpp 
> 91542f56431579be8a3da5ec6e3e58f68e7dfcbe 
> 
> Diff: https://reviews.apache.org/r/47213/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Re: Review Request 47212: Removed duplicate call to containerizer::executorEnvironment.

[Kapil Arya]

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




src/slave/containerizer/docker.cpp (line 1195)


Would there be any difference in the computed environment here vs in 
container constructor?


- Kapil Arya


On May 10, 2016, 10:13 p.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47212/
> ---
> 
> (Updated May 10, 2016, 10:13 p.m.)
> 
> 
> Review request for mesos, Adam B, Artem Harutyunyan, Jie Yu, and Kapil Arya.
> 
> 
> Bugs: MESOS-5350
> https://issues.apache.org/jira/browse/MESOS-5350
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> In this code path, where the task uses the default command executor, 
> and the agent is not dockerized
> (i.e. `taskInfo.isSome() && flags.docker_mesos_image.isNone()`), 
> the `executorEnvironment` function is called twice.
> 
> The first call is inside the `Container*` constructor called by 
> `Container::create`.  Since `Container::create` gives passes `None` 
> for the `environment` field, the constructor will call 
> `executorEnvironment` to populate the `environment` field.  
> The populated field is then accessible by `launchExecutorProcess`.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/docker.cpp 7af14f4472283ceefd73c06dd8df60af4cf6f7e8 
> 
> Diff: https://reviews.apache.org/r/47212/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Re: Review Request 47205: Added optional environment variable argument to mesos-docker-executor.

[Kapil Arya]

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




src/docker/executor.hpp (lines 80 - 107)


Can we keep just a simple `add` here and move this logic to the cpp file?


- Kapil Arya


On May 11, 2016, 4:03 p.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47205/
> ---
> 
> (Updated May 11, 2016, 4:03 p.m.)
> 
> 
> Review request for mesos, Adam B, Artem Harutyunyan, Jie Yu, and Kapil Arya.
> 
> 
> Bugs: MESOS-5350
> https://issues.apache.org/jira/browse/MESOS-5350
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This flag opens up a way for hooks to specify environment variables for
> docker tasks.  Existing hooks can only affect the environment variables
> of docker executors.
> 
> 
> Diffs
> -
> 
>   src/docker/executor.hpp 798ca3d4e261854a3b911d59929f2ca2afeb9ac8 
>   src/docker/executor.cpp d60addcbe4a1869945ce42f4bb4b1e80e3f29f19 
> 
> Diff: https://reviews.apache.org/r/47205/diff/
> 
> 
> Testing
> ---
> 
> See later reviews in chain.
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Re: Review Request 47149: Split DockerContainerizerProcess::launch into two functions.

[Kapil Arya]

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


Ship it!




- Kapil Arya


On May 10, 2016, 10:13 p.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47149/
> ---
> 
> (Updated May 10, 2016, 10:13 p.m.)
> 
> 
> Review request for mesos, Adam B, Artem Harutyunyan, Jie Yu, and Kapil Arya.
> 
> 
> Bugs: MESOS-5350
> https://issues.apache.org/jira/browse/MESOS-5350
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This prepares the `::launch` method for an asynchronous hook.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/docker.hpp fac26a239e9981e5b1c2f2f90b52785819492c2a 
>   src/slave/containerizer/docker.cpp 7af14f4472283ceefd73c06dd8df60af4cf6f7e8 
> 
> Diff: https://reviews.apache.org/r/47149/diff/
> 
> 
> Testing
> ---
> 
> sudo make check (CentOS 7)
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Review Request 47756: Stout: `make tests` now automatically builds dependencies.

[Kapil Arya]

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

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


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


Repository: mesos


Description
---

Earlier, doing `make tests` within stout failed if bundled
dependencies were not already built.


Diffs
-

  3rdparty/stout/Makefile.am eeaca480872b97d60891376b8c21fda7b65a6f00 

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


Testing
---

`make tests` works without first doing a `make` in 3rdparty.


Thanks,

Kapil Arya

Review Request 47753: Enabled building libprocess without building 3rdparty first.

[Kapil Arya]

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

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


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


Repository: mesos


Description
---

Doing a `make` inside libprocess would now automatically build bundled
dependencies.


Diffs
-

  3rdparty/libprocess/Makefile.am 3c3249fe9799ba919ac7bd13e2ddb07a306737f0 

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


Testing
---

`make clean` in 3rdparty/ followed by `make` in libprocess succeeds with this 
patch.


Thanks,

Kapil Arya

Re: Review Request 47752: Added build-stamp files for gmock/glog/http-parse/libev.

[Kapil Arya]

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

(Updated May 23, 2016, 10:50 p.m.)


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


Summary (updated)
-

Added build-stamp files for gmock/glog/http-parse/libev.


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


Repository: mesos


Description
---

This would make it easier for libprocess to declare/build these
dependencies.


Diffs
-

  3rdparty/Makefile.am fb4a37d50e751703b4ccddb0e004b58560707067 

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


Testing
---


Thanks,

Kapil Arya

Review Request 47755: Libprocess: `make tests` now automatically builds dependencies.

[Kapil Arya]

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

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


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


Repository: mesos


Description
---

Earlier, doing `make tests` within libprocess failed if bundled
dependencies were not already built.


Diffs
-

  3rdparty/libprocess/Makefile.am 3c3249fe9799ba919ac7bd13e2ddb07a306737f0 

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


Testing
---

`make tests` works without first doing a `make` in 3rdparty.


Thanks,

Kapil Arya

Review Request 47754: Enabled building stout tests without building 3rdparty first.

[Kapil Arya]

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

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


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


Repository: mesos


Description
---

Doing a `make` inside stout now automatically build bundled
dependencies.


Diffs
-

  3rdparty/stout/Makefile.am eeaca480872b97d60891376b8c21fda7b65a6f00 

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


Testing
---

`make clean` in 3rdparty/ followed by `make` in stout succeeds with this patch.


Thanks,

Kapil Arya

Re: Review Request 47695: Updated the website generation and development workflows with docker.

[haosdent huang]

> On May 23, 2016, 6:31 p.m., Tomasz Janiszewski wrote:
> >

Hi, @janisz Really appreciate your favour to help test this! Just updated, may 
you help to review this again? Thank you in advance.


- haosdent


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


On May 24, 2016, 2:38 a.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47695/
> ---
> 
> (Updated May 24, 2016, 2:38 a.m.)
> 
> 
> Review request for mesos, Tomasz Janiszewski, Kevin Klues, Neil Conway, and 
> Vinod Kone.
> 
> 
> Bugs: MESOS-5431
> https://issues.apache.org/jira/browse/MESOS-5431
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> In this changes, we include below items to make it more convenience to
> develop and generate the website.
> 
> * added `doxygen` and `javadoc` to `rake` default target.
> * added `build.sh` as the wrapper for the website generation to make
>   sure we clean up generated documents when exit.
> * switched the `Dockerfile` to based on `centos:7` because the default
>   `doxygen` version in `ubuntu:14.04` doesn't include `jquery.js`
>   correctly when generate `doxygen` documents.
> * adjusted the `Dockerfile` and `README.md` locations to avoid we
>   distract at finding the website generation document.
> * updated `release-guide.md` with the new website generation and
>   development workflows.
> 
> 
> Diffs
> -
> 
>   docs/release-guide.md 36e79796657a37b0b4649602281556f4ae471c34 
>   site/.gitignore 987a5d3cc161a67de1fb21c2002ed9698bbb0689 
>   site/Gemfile 14c50340d66fd7a703949a7932ab93a6b60f4e07 
>   site/Gemfile.lock 065107ec25f0eff28e2a09051b61152d49f77f9a 
>   site/README.md 468021ef4f8ed942c4a16784bc82349af9ae0d5e 
>   site/Rakefile 845ec3fd8b9e47ebe49991db30718912d4f9c41e 
>   site/build.sh PRE-CREATION 
>   support/site-docker/Dockerfile 8ea3d5c5fe531ec76950ed0bf110d7d477339158 
>   support/site-docker/README.md 957117e1211f06b40c5fb621039dd90aff562a87 
> 
> Diff: https://reviews.apache.org/r/47695/diff/
> 
> 
> Testing
> ---
> 
> Tested in OS X and Ubuntu 14.04
> 
> For document changes, could verified from 
> https://github.com/haosdent/mesos/tree/MESOS-5431/site
> 
> To verify website works fine after this changes, could check it via this gif:
> 
> ![website.gif](https://issues.apache.org/jira/secure/attachment/12805684/website.gif)
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 47695: Updated the website generation and development workflows with docker.

[haosdent huang]

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

(Updated May 24, 2016, 2:38 a.m.)


Review request for mesos, Tomasz Janiszewski, Kevin Klues, Neil Conway, and 
Vinod Kone.


Changes
---

Address @janisz's comments.


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


Repository: mesos


Description
---

In this changes, we include below items to make it more convenience to
develop and generate the website.

* added `doxygen` and `javadoc` to `rake` default target.
* added `build.sh` as the wrapper for the website generation to make
  sure we clean up generated documents when exit.
* switched the `Dockerfile` to based on `centos:7` because the default
  `doxygen` version in `ubuntu:14.04` doesn't include `jquery.js`
  correctly when generate `doxygen` documents.
* adjusted the `Dockerfile` and `README.md` locations to avoid we
  distract at finding the website generation document.
* updated `release-guide.md` with the new website generation and
  development workflows.


Diffs (updated)
-

  docs/release-guide.md 36e79796657a37b0b4649602281556f4ae471c34 
  site/.gitignore 987a5d3cc161a67de1fb21c2002ed9698bbb0689 
  site/Gemfile 14c50340d66fd7a703949a7932ab93a6b60f4e07 
  site/Gemfile.lock 065107ec25f0eff28e2a09051b61152d49f77f9a 
  site/README.md 468021ef4f8ed942c4a16784bc82349af9ae0d5e 
  site/Rakefile 845ec3fd8b9e47ebe49991db30718912d4f9c41e 
  site/build.sh PRE-CREATION 
  support/site-docker/Dockerfile 8ea3d5c5fe531ec76950ed0bf110d7d477339158 
  support/site-docker/README.md 957117e1211f06b40c5fb621039dd90aff562a87 

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


Testing
---

Tested in OS X and Ubuntu 14.04

For document changes, could verified from 
https://github.com/haosdent/mesos/tree/MESOS-5431/site

To verify website works fine after this changes, could check it via this gif:

![website.gif](https://issues.apache.org/jira/secure/attachment/12805684/website.gif)


Thanks,

haosdent huang

Re: Review Request 47695: Updated the website generation and development workflows with docker.

[haosdent huang]

> On May 23, 2016, 6:31 p.m., Tomasz Janiszewski wrote:
> > site/README.md, line 12
> > 
> >
> > Running this command will end up with error: `unable to prepare 
> > context: unable to evaluate symlinks in context path: lstat no such file or 
> > directory`. Later this document assumed user is in `site` dir. To be 
> > consistend replace `site` with `.`

Oh, I former document, we suppose user under `mesos` root directory

```
docker build -t mesos/website support/site-docker
```

Let me describe this in the document.


> On May 23, 2016, 6:31 p.m., Tomasz Janiszewski wrote:
> > support/site-docker/Dockerfile, line 6
> > 
> >
> > Do we really need whole `Development Tools` to build website.

Good catch! We only need `gcc-c++ make` to build the native library when 
`bundle install`.


> On May 23, 2016, 6:31 p.m., Tomasz Janiszewski wrote:
> > site/README.md, line 47
> > 
> >
> > I think this section needs example that will work for 99% users.

Good suggestion! Let me add.


- haosdent


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


On May 23, 2016, 5:49 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47695/
> ---
> 
> (Updated May 23, 2016, 5:49 p.m.)
> 
> 
> Review request for mesos, Tomasz Janiszewski, Kevin Klues, Neil Conway, and 
> Vinod Kone.
> 
> 
> Bugs: MESOS-5431
> https://issues.apache.org/jira/browse/MESOS-5431
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> In this changes, we include below items to make it more convenience to
> develop and generate the website.
> 
> * added `doxygen` and `javadoc` to `rake` default target.
> * added `build.sh` as the wrapper for the website generation to make
>   sure we clean up generated documents when exit.
> * switched the `Dockerfile` to based on `centos:7` because the default
>   `doxygen` version in `ubuntu:14.04` doesn't include `jquery.js`
>   correctly when generate `doxygen` documents.
> * adjusted the `Dockerfile` and `README.md` locations to avoid we
>   distract at finding the website generation document.
> * updated `release-guide.md` with the new website generation and
>   development workflows.
> 
> 
> Diffs
> -
> 
>   docs/release-guide.md 36e79796657a37b0b4649602281556f4ae471c34 
>   site/.gitignore 987a5d3cc161a67de1fb21c2002ed9698bbb0689 
>   site/Gemfile 14c50340d66fd7a703949a7932ab93a6b60f4e07 
>   site/Gemfile.lock 065107ec25f0eff28e2a09051b61152d49f77f9a 
>   site/README.md 468021ef4f8ed942c4a16784bc82349af9ae0d5e 
>   site/Rakefile 845ec3fd8b9e47ebe49991db30718912d4f9c41e 
>   site/build.sh PRE-CREATION 
>   support/site-docker/Dockerfile 8ea3d5c5fe531ec76950ed0bf110d7d477339158 
>   support/site-docker/README.md 957117e1211f06b40c5fb621039dd90aff562a87 
> 
> Diff: https://reviews.apache.org/r/47695/diff/
> 
> 
> Testing
> ---
> 
> Tested in OS X and Ubuntu 14.04
> 
> For document changes, could verified from 
> https://github.com/haosdent/mesos/tree/MESOS-5431/site
> 
> To verify website works fine after this changes, could check it via this gif:
> 
> ![website.gif](https://issues.apache.org/jira/secure/attachment/12805684/website.gif)
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 47374: Experimental: Separated mesos test helpers into a separate library.

[Joseph Wu]

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

(Updated May 23, 2016, 7:02 p.m.)


Review request for mesos, Adam B, Artem Harutyunyan, Jie Yu, Kapil Arya, Jan 
Schlicht, and Till Toenshoff.


Repository: mesos


Description
---

This gives external projects easier access to the test helpers used in
mesos tests.  

For example, a module writer may want to write a test like 
`src/tests/oversubscription_tests.cpp`.  To build and link against 
this library, the module writer would mimic the build flags for tests:
```
my_module_tests_CPPFLAGS = \
  -I$(ZOOKEEPER)/include   \
  -I$(ZOOKEEPER)/generated \
  -DSOURCE_DIR=\"$(abs_top_srcdir)\"   \
  -DBUILD_DIR=\"$(abs_top_builddir)\"  \
  ...
  
my_module_tests_LDADD = libmesos_tests.la
```


Diffs
-

  src/Makefile.am ce5245883f3d2661812272702c0d2060513b6d88 

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


Testing
---

make check


Thanks,

Joseph Wu

Re: Review Request 44839: Enabled mesos containerizer do not cache image for appc.

[Guangya Liu]

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

(Updated 五月 24, 2016, 2:01 a.m.)


Review request for mesos, Gilbert Song, haosdent huang, and Jie Yu.


Summary (updated)
-

Enabled mesos containerizer do not cache image for appc.


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


Repository: mesos


Description
---

Enabled mesos containerizer force_pull_image for appc.


Diffs (updated)
-

  src/slave/containerizer/mesos/provisioner/appc/store.cpp 
33e97fc645a9b7fbc9ae47f67c1b5dacf999fce0 

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


Testing
---

make
make check


Thanks,

Guangya Liu

Re: Review Request 44839: Enabled mesos containerizer do not cache image for appc.

[Guangya Liu]

> On 五月 23, 2016, 9:15 p.m., Jie Yu wrote:
> >
> 
> Jie Yu wrote:
> Please make sure to a manual test. Why adding unit test is hard?

Yes, test manually. If adding a new unit test, I did not found a good way to 
check if the image was pulled from repo or just copied from local cache, any 
suggestions for this? Thanks.


- Guangya


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


On 五月 24, 2016, 2:01 a.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44839/
> ---
> 
> (Updated 五月 24, 2016, 2:01 a.m.)
> 
> 
> Review request for mesos, Gilbert Song, haosdent huang, and Jie Yu.
> 
> 
> Bugs: MESOS-4886
> https://issues.apache.org/jira/browse/MESOS-4886
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Enabled mesos containerizer force_pull_image for appc.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/provisioner/appc/store.cpp 
> 33e97fc645a9b7fbc9ae47f67c1b5dacf999fce0 
> 
> Diff: https://reviews.apache.org/r/44839/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 47374: Experimental: Separated mesos test helpers into a separate library.

[Joseph Wu]

> On May 23, 2016, 5:09 p.m., Adam B wrote:
> > src/Makefile.am, lines 1937-1938
> > 
> >
> > Why does libmesos_tests_la_SOURCES need to include qos_controllers code?

I don't remember the exact reason, but one of my previous iterations (not 
published) broke on this module.  It's unnecessary for the current 
implementation.


> On May 23, 2016, 5:09 p.m., Adam B wrote:
> > src/Makefile.am, lines 1991-1994
> > 
> >
> > I guess all these `tests/*.cpp` aren't needed anymore because they're 
> > covered in `libmesos_tests_la_SOURCES` which is also included in 
> > `mesos_tests_DEPENDENCIES` and `mesos_tests_LDADD`?

Yup.  These are all replaced with an `LDADD` later on.


- Joseph


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


On May 23, 2016, 7 p.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47374/
> ---
> 
> (Updated May 23, 2016, 7 p.m.)
> 
> 
> Review request for mesos, Adam B, Artem Harutyunyan, Jie Yu, Kapil Arya, Jan 
> Schlicht, and Till Toenshoff.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This gives external projects easier access to the test helpers used in
> mesos tests.  
> 
> For example, a module writer may want to write a test like 
> `src/tests/oversubscription_tests.cpp`.  To build and link against 
> this library, the module writer would mimic the build flags for tests:
> ```
> my_module_tests_CPPFLAGS = \
>   -I$(ZOOKEEPER)/include   \
>   -I$(ZOOKEEPER)/generated \
>   -DSOURCE_DIR=\"$(abs_top_srcdir)\"   \
>   -DBUILD_DIR=\"$(abs_top_builddir)\"  \
>   ...
>   
> my_module_tests_LDADD = libmesos_tests.la
> ```
> 
> 
> Diffs
> -
> 
>   src/Makefile.am ce5245883f3d2661812272702c0d2060513b6d88 
> 
> Diff: https://reviews.apache.org/r/47374/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Re: Review Request 47702: Added framework role & principal to the Web UI.

[haosdent huang]

> On May 23, 2016, 1:03 a.m., haosdent huang wrote:
> > Do you forget to update
> > * `framework.html` 
> > https://github.com/apache/mesos/blob/master/src/webui/master/static/framework.html#L24
> > * `agent.html` 
> > https://github.com/apache/mesos/blob/master/src/webui/master/static/agent.html
> >  
> > * `agent_framework.html` 
> > https://github.com/apache/mesos/blob/master/src/webui/master/static/agent_framework.html
> 
> Deshna Jain wrote:
> I thought that the change is required only on frameworks page. Thanks for 
> bringing it up :-)
> 
> I have made changes in framework.html.
> 
> I have a few concerns/queries here:
> - The information displayed on Agents page is of agents only. So, I 
> couldn't get you. What did you mean?
> - @Vinod/Hoasdent: The agent's state endpoint does not have 
> framework-principal information. Shall I add only 'Role' field to agent.html 
> and agent_framework.html or just leave it to be tackled later?

Hi, @deshna Thanks a lot for your update.

>The information displayed on Agents page is of agents only. So, I couldn't get 
>you. What did you mean?

Oh, I mean should we update these data table related to frameworks.
https://github.com/apache/mesos/blob/master/src/webui/master/static/agent_framework.html#L20
https://github.com/apache/mesos/blob/master/src/webui/master/static/agent.html#L124

>The agent's state endpoint does not have framework-principal information.

It looks a bug here. I file this ticket 
[MESOS-5444](https://issues.apache.org/jira/browse/MESOS-5444) to track it.

>Shall I add only 'Role' field to agent.html and agent_framework.html

It is OK for me :-)


- haosdent


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


On May 23, 2016, 9:08 p.m., Deshna Jain wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47702/
> ---
> 
> (Updated May 23, 2016, 9:08 p.m.)
> 
> 
> Review request for mesos, haosdent huang and Vinod Kone.
> 
> 
> Bugs: MESOS-4788
> https://issues.apache.org/jira/browse/MESOS-4788
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/webui/master/static/framework.html 
> f108e92f5b8d9ed4de718e55aa7302728a84003e 
>   src/webui/master/static/frameworks.html 
> f172e022e18df5b6aa3d232e610c3c732e20aa09 
> 
> Diff: https://reviews.apache.org/r/47702/diff/
> 
> 
> Testing
> ---
> 
> make check (see attached screenshot for ui after the change)
> 
> 
> File Attachments
> 
> 
> image after making the change
>   
> https://reviews.apache.org/media/uploaded/files/2016/05/23/a2b9738e-b33a-4fce-98b0-9457d42a31f7__Screen_Shot_2016-05-23_at_12.38.01_PM.png
> 
> 
> Thanks,
> 
> Deshna Jain
> 
>

Re: Review Request 47374: Experimental: Separated mesos test helpers into a separate library.

[Joseph Wu]

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

(Updated May 23, 2016, 7 p.m.)


Review request for mesos, Adam B, Artem Harutyunyan, Jie Yu, Kapil Arya, Jan 
Schlicht, and Till Toenshoff.


Changes
---

Remove QoS controller code from common test library.


Repository: mesos


Description (updated)
---

This gives external projects easier access to the test helpers used in
mesos tests.  

For example, a module writer may want to write a test like 
`src/tests/oversubscription_tests.cpp`.  To build and link against 
this library, the module writer would mimic the build flags for tests:
```
my_module_tests_CPPFLAGS = \
  -I$(ZOOKEEPER)/include   \
  -I$(ZOOKEEPER)/generated \
  -DSOURCE_DIR=\"$(abs_top_srcdir)\"   \
  -DBUILD_DIR=\"$(abs_top_builddir)\"  \
  ...
  
my_module_tests_LDADD = libmesos_tests.la
```


Diffs (updated)
-

  src/Makefile.am ce5245883f3d2661812272702c0d2060513b6d88 

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


Testing
---

make check


Thanks,

Joseph Wu

Re: Review Request 45668: Enable CMake build.

[Vinod Kone]

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



can you rebase this off the latest master branch and push an updated diff. I 
updated the ReviewBot to test with BUILDTOOL. So want to see if the bot is 
happy before I merge it upstream.

- Vinod Kone


On May 22, 2016, 8:15 p.m., Juan Larriba wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45668/
> ---
> 
> (Updated May 22, 2016, 8:15 p.m.)
> 
> 
> Review request for mesos, Alex Clemmer, Joerg Schad, and Vinod Kone.
> 
> 
> Bugs: MESOS-5101
> https://issues.apache.org/jira/browse/MESOS-5101
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Enable CMake build with autotools configuration.
> 
> 
> Diffs
> -
> 
>   support/docker_build.sh 28ef4dce3f473adab9919d4c2170075a0900af41 
> 
> Diff: https://reviews.apache.org/r/45668/diff/
> 
> 
> Testing
> ---
> 
> Built using docker_build.sh on both centos:7 and ubuntu:14.04 using both 
> cmake and autotools. In ubuntu:14.04 was built using gcc and clang, in 
> centos:7 only gcc.
> 
> 
> Thanks,
> 
> Juan Larriba
> 
>

Re: Review Request 45668: Enable CMake build.

[Vinod Kone]

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


Ship it!




Ship It!

- Vinod Kone


On May 22, 2016, 8:15 p.m., Juan Larriba wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45668/
> ---
> 
> (Updated May 22, 2016, 8:15 p.m.)
> 
> 
> Review request for mesos, Alex Clemmer, Joerg Schad, and Vinod Kone.
> 
> 
> Bugs: MESOS-5101
> https://issues.apache.org/jira/browse/MESOS-5101
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Enable CMake build with autotools configuration.
> 
> 
> Diffs
> -
> 
>   support/docker_build.sh 28ef4dce3f473adab9919d4c2170075a0900af41 
> 
> Diff: https://reviews.apache.org/r/45668/diff/
> 
> 
> Testing
> ---
> 
> Built using docker_build.sh on both centos:7 and ubuntu:14.04 using both 
> cmake and autotools. In ubuntu:14.04 was built using gcc and clang, in 
> centos:7 only gcc.
> 
> 
> Thanks,
> 
> Juan Larriba
> 
>

Re: Review Request 47374: Experimental: Separated mesos test helpers into a separate library.

[Adam B]

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



Looks ok to me, but I'd love for @jieyu or @karya or @tillt to give it the 
final ShipIt, since they have more experience with tests in the Makefile.


src/Makefile.am (lines 1937 - 1938)


Why does libmesos_tests_la_SOURCES need to include qos_controllers code?



src/Makefile.am (lines 1991 - 1992)


I guess all these `tests/*.cpp` aren't needed anymore because they're 
covered in `libmesos_tests_la_SOURCES` which is also included in 
`mesos_tests_DEPENDENCIES` and `mesos_tests_LDADD`?


- Adam B


On May 23, 2016, 5:08 p.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47374/
> ---
> 
> (Updated May 23, 2016, 5:08 p.m.)
> 
> 
> Review request for mesos, Adam B, Artem Harutyunyan, Jie Yu, Kapil Arya, Jan 
> Schlicht, and Till Toenshoff.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This gives external projects easier access to the test helpers used in
> mesos tests.  
> 
> For example, a module writer may want to write a test like 
> `src/tests/oversubscription_tests.cpp`.  To build and link against 
> this library, the module writer would mimic the build flags for tests:
> ```
> my_module_CPPFLAGS =   \
>   -I$(ZOOKEEPER)/include   \
>   -I$(ZOOKEEPER)/generated \
>   -DSOURCE_DIR=\"$(abs_top_srcdir)\"   \
>   -DBUILD_DIR=\"$(abs_top_builddir)\"  \
>   ...
> ```
> 
> 
> Diffs
> -
> 
>   src/Makefile.am ce5245883f3d2661812272702c0d2060513b6d88 
> 
> Diff: https://reviews.apache.org/r/47374/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Re: Review Request 47403: Stout: Set `_fmode` to binary in `protobuf.hpp`.

[Joris Van Remoortere]

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




3rdparty/stout/include/stout/protobuf.hpp (lines 122 - 124)


indentation.



3rdparty/stout/include/stout/protobuf.hpp (lines 153 - 155)


indentation.



3rdparty/stout/include/stout/protobuf.hpp (lines 337 - 339)


indentation


- Joris Van Remoortere


On May 23, 2016, 12:39 p.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47403/
> ---
> 
> (Updated May 23, 2016, 12:39 p.m.)
> 
> 
> Review request for mesos, Daniel Pravat, Artem Harutyunyan, Joris Van 
> Remoortere, and Michael Park.
> 
> 
> Bugs: MESOS-3443 and MESOS-3658
> https://issues.apache.org/jira/browse/MESOS-3443
> https://issues.apache.org/jira/browse/MESOS-3658
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> In Windows, the I/O subsystem will automatically convert linefeed
> endings if the I/O is done on "text" files. For binary files, this
> effect is almost never desirable.
> 
> This commit will turn of these conversions for protobuf I/O operations.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/protobuf.hpp 
> eb4ac8cd738859a119f4f64bf52ae49a5bbef899 
> 
> Diff: https://reviews.apache.org/r/47403/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>

Re: Review Request 47411: Agent: Changed the names of symbols that are ambiguous on MSVC.

[Joris Van Remoortere]

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



I'll update this to remove the bad merge.


src/slave/slave.cpp (line 960)


This looks like a bad Merge.


- Joris Van Remoortere


On May 23, 2016, 12:39 p.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47411/
> ---
> 
> (Updated May 23, 2016, 12:39 p.m.)
> 
> 
> Review request for mesos, Daniel Pravat, Artem Harutyunyan, Joris Van 
> Remoortere, and Michael Park.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Agent: Changed the names of symbols that are ambiguous on MSVC.
> 
> 
> Diffs
> -
> 
>   src/docker/docker.hpp da6e9a25b99079940958001128ee949cdb9b931b 
>   src/slave/slave.cpp feb9f291b4188aac1a63ab9cfae8349a048663b1 
> 
> Diff: https://reviews.apache.org/r/47411/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>

Re: Review Request 47389: Stout: Added support for correct path delimiters in Windows.

[Joris Van Remoortere]

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




3rdparty/stout/include/stout/os/mkdir.hpp (line 51)


this can just be `path = os::PATH_SEPARATOR;`



3rdparty/stout/include/stout/os/mkdir.hpp (line 60)


This can just be `path += os::PATH_SEPARATOR;`



3rdparty/stout/include/stout/path.hpp (lines 20 - 21)


alphabetize.



3rdparty/stout/include/stout/path.hpp (lines 32 - 34)


`char_separator` -> `separator_`
`string_separator` -> `separator`



3rdparty/stout/include/stout/path.hpp (line 34)


this can be `stringify`.



3rdparty/stout/include/stout/path.hpp (line 81)


Can remove the `For obvious reasons, this was not spelled out every time.`


- Joris Van Remoortere


On May 23, 2016, 5:03 p.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47389/
> ---
> 
> (Updated May 23, 2016, 5:03 p.m.)
> 
> 
> Review request for mesos, Daniel Pravat, Artem Harutyunyan, Joris Van 
> Remoortere, and Michael Park.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Stout: Added support for correct path delimiters in Windows.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/os/constants.hpp 
> 438840aaef189ef4fb457b856790efb3b6333a7d 
>   3rdparty/stout/include/stout/os/mkdir.hpp 
> e86dbfd2416fc2835ef7c6c55a20f00429f7b4c6 
>   3rdparty/stout/include/stout/path.hpp 
> ef538045a8b7a1e3d8962c869317d86a85e0259f 
> 
> Diff: https://reviews.apache.org/r/47389/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>

Re: Review Request 44838: Enabled mesos containerizer do not cache image for docker.

[Guangya Liu]

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

(Updated 五月 23, 2016, 11:30 p.m.)


Review request for mesos, Gilbert Song, haosdent huang, and Jie Yu.


Summary (updated)
-

Enabled mesos containerizer do not cache image for docker.


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


Repository: mesos


Description (updated)
---

Enabled mesos containerizer do not cache image for docker.


Diffs (updated)
-

  src/slave/containerizer/mesos/provisioner/docker/metadata_manager.hpp 
137af502a66e6a65773c00eaacbe392576376284 
  src/slave/containerizer/mesos/provisioner/docker/metadata_manager.cpp 
cf630cc0b67a325529fa04ad2b1708e013b9596a 
  src/slave/containerizer/mesos/provisioner/docker/store.cpp 
eeec94326a4fd67675df10e0b6a32267e555fa96 

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


Testing
---

make
make check

It is difficult to test this with unit test, but just test with mesos execute 
by setting `force_pull_image` as true and found that the iamge was always 
pulled when `force_pull_image` is true.


Thanks,

Guangya Liu

Re: Review Request 44838: Enabled mesos containerizer do not cache image for docker.

[Guangya Liu]

> On 五月 23, 2016, 7:19 p.m., Jie Yu wrote:
> > src/slave/containerizer/mesos/provisioner/docker/metadata_manager.hpp, line 
> > 93
> > 
> >
> > WE don't use 'const' for primitive types. Please use `bool cached` here.

I saw that there are many places using `const bool`, such as 
https://github.com/apache/mesos/blob/master/src/slave/containerizer/mesos/isolators/cgroups/mem.cpp#L75
 , filed a JIRA https://issues.apache.org/jira/browse/MESOS-5443 for this.


- Guangya


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


On 五月 23, 2016, 11:30 p.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44838/
> ---
> 
> (Updated 五月 23, 2016, 11:30 p.m.)
> 
> 
> Review request for mesos, Gilbert Song, haosdent huang, and Jie Yu.
> 
> 
> Bugs: MESOS-4886
> https://issues.apache.org/jira/browse/MESOS-4886
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Enabled mesos containerizer do not cache image for docker.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/provisioner/docker/metadata_manager.hpp 
> 137af502a66e6a65773c00eaacbe392576376284 
>   src/slave/containerizer/mesos/provisioner/docker/metadata_manager.cpp 
> cf630cc0b67a325529fa04ad2b1708e013b9596a 
>   src/slave/containerizer/mesos/provisioner/docker/store.cpp 
> eeec94326a4fd67675df10e0b6a32267e555fa96 
> 
> Diff: https://reviews.apache.org/r/44838/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> It is difficult to test this with unit test, but just test with mesos execute 
> by setting `force_pull_image` as true and found that the iamge was always 
> pulled when `force_pull_image` is true.
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 44837: Added cached field to Image protobuf.

[Guangya Liu]

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

(Updated 五月 23, 2016, 11:30 p.m.)


Review request for mesos, Gilbert Song, haosdent huang, and Jie Yu.


Summary (updated)
-

Added cached field to Image protobuf.


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


Repository: mesos


Description (updated)
---

Added cached field to Image protobuf.


Diffs (updated)
-

  include/mesos/mesos.proto 887ffe9c5f04e25539a4c6b3d52ce5299c65e8d3 
  include/mesos/v1/mesos.proto 9e59aed20965d50ee10989ff6b75db742cf2b83b 

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


Testing
---

make
make check


Thanks,

Guangya Liu

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

[Gilbert Song]

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




docs/docker-volume-isolator.md (line 7)


Could we add a section #Motivation?



docs/docker-volume-isolator.md (line 8)


The docker volume isolator is introduced...



docs/docker-volume-isolator.md (lines 17 - 20)


newline for each.



docs/docker-volume-isolator.md (line 24)


Remove "The rest of this document"



docs/docker-volume-isolator.md (line 30)


s/docker volume driver isolator/ docker volume isolator/g



docs/docker-volume-isolator.md (line 33)


When a new task with docker volumes specified is launched



docs/docker-volume-isolator.md (lines 33 - 40)


Merge together and rephase.



docs/docker-volume-isolator.md (line 42)


Move this below when introducing `container_path`:

The docker volume isolator supports tasks with/without rootfs:
with rootfs v.s. without rootfs
absolute path v.s. relative path



docs/docker-volume-isolator.md (lines 45 - 52)


kill this.



docs/docker-volume-isolator.md (lines 54 - 75)


kill this.

Generally, we may not need to explain the implementation in detail. Let's 
just explain the diagram and mention the recoverablity in one sentence.



docs/docker-volume-isolator.md (lines 79 - 80)


could we rephase?



docs/docker-volume-isolator.md (line 84)


let's numify each step:

1. pre-condition
2. configuration
...



docs/docker-volume-isolator.md (lines 91 - 92)


strongly recommend to create volumes explicitly, otherwise, volumes will be 
created by dvdcli.



docs/docker-volume-isolator.md (lines 106 - 107)


dependency on linux filesystem isolator.



docs/docker-volume-isolator.md (lines 120 - 187)


let's just pick out related message fields and filter out all comments.



docs/docker-volume-isolator.md (lines 198 - 209)


could we draw a table?



docs/docker-volume-isolator.md (line 200)


if it is a command task V.S. a mesos container with a specified image...



docs/docker-volume-isolator.md (line 213)


please fix the grammar.



docs/docker-volume-isolator.md (lines 216 - 288)


we dont need this if we explain ContainerInfo.Volumes above.



docs/docker-volume-isolator.md (line 290)


could rephase and fix the grammar.



docs/docker-volume-isolator.md (line 302)


do we need this section for now? since users may not be able to test it out 
depending on the code base.



docs/docker-volume-isolator.md (lines 329 - 339)


kill this.



docs/docker-volume-isolator.md (line 341)


let's add it later.


- Gilbert Song


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

Re: Review Request 47199: Implemented parsing docker labels in v1 spec.

[Jojy Varghese]

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




src/docker/spec.cpp (lines 207 - 230)


Looks like there is scope for de-duplicating code between this block and 
the block above(`config` and `container_config`)?


- Jojy Varghese


On May 13, 2016, 8:49 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47199/
> ---
> 
> (Updated May 13, 2016, 8:49 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Artem Harutyunyan, Jie Yu, and 
> Kevin Klues.
> 
> 
> Bugs: MESOS-5272
> https://issues.apache.org/jira/browse/MESOS-5272
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Implemented parsing docker labels in v1 spec.
> 
> 
> Diffs
> -
> 
>   src/docker/spec.cpp ac28331a17edb8c2ff81d5a2f79a794f869a3e5d 
> 
> Diff: https://reviews.apache.org/r/47199/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 47671: Windows: Allowed `dynamic_cast` on an object being constructed.

[Michael Park]

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


Ship it!




Ship It!

- Michael Park


On May 22, 2016, 6:59 a.m., Daniel Pravat wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47671/
> ---
> 
> (Updated May 22, 2016, 6:59 a.m.)
> 
> 
> Review request for mesos, Alex Naparu, Artem Harutyunyan, Alex Clemmer, Joris 
> Van Remoortere, and Michael Park.
> 
> 
> Bugs: MESOS-5428
> https://issues.apache.org/jira/browse/MESOS-5428
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Windows: Allowed `dynamic_cast` on an object being constructed.
> 
> 
> Diffs
> -
> 
>   cmake/CompilationConfigure.cmake 5c7833ceaed556cc4ffb650996e918c1a542c5f0 
> 
> Diff: https://reviews.apache.org/r/47671/diff/
> 
> 
> Testing
> ---
> 
> Windows: build/run
> 
> 
> Thanks,
> 
> Daniel Pravat
> 
>

Re: Review Request 47702: Added framework role & principal to the Web UI.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [47702]

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

- Mesos ReviewBot


On May 23, 2016, 9:08 p.m., Deshna Jain wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47702/
> ---
> 
> (Updated May 23, 2016, 9:08 p.m.)
> 
> 
> Review request for mesos, haosdent huang and Vinod Kone.
> 
> 
> Bugs: MESOS-4788
> https://issues.apache.org/jira/browse/MESOS-4788
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/webui/master/static/framework.html 
> f108e92f5b8d9ed4de718e55aa7302728a84003e 
>   src/webui/master/static/frameworks.html 
> f172e022e18df5b6aa3d232e610c3c732e20aa09 
> 
> Diff: https://reviews.apache.org/r/47702/diff/
> 
> 
> Testing
> ---
> 
> make check (see attached screenshot for ui after the change)
> 
> 
> File Attachments
> 
> 
> image after making the change
>   
> https://reviews.apache.org/media/uploaded/files/2016/05/23/a2b9738e-b33a-4fce-98b0-9457d42a31f7__Screen_Shot_2016-05-23_at_12.38.01_PM.png
> 
> 
> Thanks,
> 
> Deshna Jain
> 
>

Re: Review Request 45952: Implemented support for passing agent default docker config.

[Gilbert Song]

> On April 11, 2016, 12:37 a.m., Guangya Liu wrote:
> > src/uri/fetchers/docker.cpp, lines 323-325
> > 
> >
> > Does there are any document change for this?

Unfortunately, we dont have document for docker fetcher yet.


> On April 11, 2016, 12:37 a.m., Guangya Liu wrote:
> > src/uri/fetchers/docker.cpp, line 636
> > 
> >
> > Add a log here to identify that the fetcher is now using auth to fetch 
> > the image?
> 
> Guangya Liu wrote:
> Adding the log in `getCredential` may be better, post some comments in 
> https://reviews.apache.org/r/45949/ , please check. Thanks.

Thanks for the comments, Guangya! Seems like we dont have any log in docker 
fetcher.


- Gilbert


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


On May 23, 2016, 2:52 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45952/
> ---
> 
> (Updated May 23, 2016, 2:52 p.m.)
> 
> 
> Review request for mesos, Guangya Liu, Artem Harutyunyan, Jie Yu, and Timothy 
> Chen.
> 
> 
> Bugs: MESOS-4938
> https://issues.apache.org/jira/browse/MESOS-4938
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Implemented support for passing agent default docker config.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/provisioner/docker/store.cpp 
> eeec94326a4fd67675df10e0b6a32267e555fa96 
>   src/uri/fetchers/docker.hpp c855a2b55a07bb398f7547b44a85b8ba2d2b2ec3 
>   src/uri/fetchers/docker.cpp ab8f5e05758b7de2573605c81ac80e656bb1db24 
> 
> Diff: https://reviews.apache.org/r/45952/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> Tested with mesos-execute, using a private repo from docker hub. Both cases 
> are tested:
> 1. --docker_registry=private_registry
> 2. private_registry/repo
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 47200: Modified docker spec test for docker label support.

[Jie Yu]

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


Ship it!




Ship It!

- Jie Yu


On May 12, 2016, 3:56 a.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47200/
> ---
> 
> (Updated May 12, 2016, 3:56 a.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Artem Harutyunyan, Jie Yu, and 
> Kevin Klues.
> 
> 
> Bugs: MESOS-5272
> https://issues.apache.org/jira/browse/MESOS-5272
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Modified docker spec test for docker label support.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/docker_spec_tests.cpp 
> 796b020f58f8451362bc1357ab6d7ceb4e946b3c 
> 
> Diff: https://reviews.apache.org/r/47200/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 47199: Implemented parsing docker labels in v1 spec.

[Jie Yu]

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


Ship it!




Ship It!

- Jie Yu


On May 13, 2016, 8:49 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47199/
> ---
> 
> (Updated May 13, 2016, 8:49 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Artem Harutyunyan, Jie Yu, and 
> Kevin Klues.
> 
> 
> Bugs: MESOS-5272
> https://issues.apache.org/jira/browse/MESOS-5272
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Implemented parsing docker labels in v1 spec.
> 
> 
> Diffs
> -
> 
>   src/docker/spec.cpp ac28331a17edb8c2ff81d5a2f79a794f869a3e5d 
> 
> Diff: https://reviews.apache.org/r/47199/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 45950: Added test for docker spec get credential helper.

[Gilbert Song]

> On April 10, 2016, 10:20 p.m., Guangya Liu wrote:
> >

Good catch! Thanks!!


- Gilbert


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


On May 23, 2016, 2:51 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45950/
> ---
> 
> (Updated May 23, 2016, 2:51 p.m.)
> 
> 
> Review request for mesos, Guangya Liu, Artem Harutyunyan, Jie Yu, and Timothy 
> Chen.
> 
> 
> Bugs: MESOS-4938
> https://issues.apache.org/jira/browse/MESOS-4938
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added test for docker spec get credential helper.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/docker_spec_tests.cpp 
> 796b020f58f8451362bc1357ab6d7ceb4e946b3c 
> 
> Diff: https://reviews.apache.org/r/45950/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 47198: Added labels to docker v1 spec config.

[Jie Yu]

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




include/mesos/docker/v1.proto (lines 53 - 56)


Mind pulling this to the top level?
```
spec::v1::Label
```



include/mesos/docker/v1.proto (lines 58 - 64)


I would mention that we cannot use `Labels` here because otherwise, 
json->protobuf parsing will fail. `labels` is manually set during parsing.


- Jie Yu


On May 13, 2016, 8:48 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47198/
> ---
> 
> (Updated May 13, 2016, 8:48 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Artem Harutyunyan, Jie Yu, and 
> Kevin Klues.
> 
> 
> Bugs: MESOS-5272
> https://issues.apache.org/jira/browse/MESOS-5272
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added labels to docker v1 spec config.
> 
> 
> Diffs
> -
> 
>   include/mesos/docker/v1.proto ff18f8cadfce3315467f194d50d3469fa839f686 
> 
> Diff: https://reviews.apache.org/r/47198/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 45949: Implemented docker config get credential helper.

[Gilbert Song]

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

(Updated May 23, 2016, 2:51 p.m.)


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


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


Repository: mesos


Description
---

Implemented docker config get credential helper.


Diffs (updated)
-

  include/mesos/docker/spec.hpp 2ebacc70d92a593c8dd006b34519c3a2a5225481 
  src/docker/spec.cpp ac28331a17edb8c2ff81d5a2f79a794f869a3e5d 

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


Testing
---

make check


Thanks,

Gilbert Song

Re: Review Request 45952: Implemented support for passing agent default docker config.

[Gilbert Song]

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

(Updated May 23, 2016, 2:52 p.m.)


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


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


Repository: mesos


Description
---

Implemented support for passing agent default docker config.


Diffs (updated)
-

  src/slave/containerizer/mesos/provisioner/docker/store.cpp 
eeec94326a4fd67675df10e0b6a32267e555fa96 
  src/uri/fetchers/docker.hpp c855a2b55a07bb398f7547b44a85b8ba2d2b2ec3 
  src/uri/fetchers/docker.cpp ab8f5e05758b7de2573605c81ac80e656bb1db24 

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


Testing
---

make check

Tested with mesos-execute, using a private repo from docker hub. Both cases are 
tested:
1. --docker_registry=private_registry
2. private_registry/repo


Thanks,

Gilbert Song

Re: Review Request 45950: Added test for docker spec get credential helper.

[Gilbert Song]

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

(Updated May 23, 2016, 2:51 p.m.)


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


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


Repository: mesos


Description
---

Added test for docker spec get credential helper.


Diffs (updated)
-

  src/tests/containerizer/docker_spec_tests.cpp 
796b020f58f8451362bc1357ab6d7ceb4e946b3c 

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


Testing
---

make check


Thanks,

Gilbert Song

Re: Review Request 45951: Implemented http basic auth to get docker auth token.

[Gilbert Song]

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

(Updated May 23, 2016, 2:51 p.m.)


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


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


Repository: mesos


Description
---

Implemented http basic auth to get docker auth token.


Diffs (updated)
-

  src/uri/fetchers/docker.cpp ab8f5e05758b7de2573605c81ac80e656bb1db24 

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


Testing
---

make check

Tested with mesos-execute, using a private repo from docker hub.


Thanks,

Gilbert Song

Re: Review Request 46438: Added the test "CniIsolatorTest.ROOT_SlaveRecovery".

[Jie Yu]

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




src/tests/containerizer/cni_isolator_tests.cpp (line 457)


Instead of that, let's use sleep 1000 and issue a kill after slave recovers.


- Jie Yu


On May 12, 2016, 8:14 a.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46438/
> ---
> 
> (Updated May 12, 2016, 8:14 a.m.)
> 
> 
> Review request for mesos, Avinash sridharan and Jie Yu.
> 
> 
> Bugs: MESOS-5167
> https://issues.apache.org/jira/browse/MESOS-5167
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added the test "CniIsolatorTest.ROOT_SlaveRecovery".
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/cni_isolator_tests.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/46438/diff/
> 
> 
> Testing
> ---
> 
> [ RUN  ] CniIsolatorTest.ROOT_SlaveRecovery
> I0420 22:36:44.632558 10440 exec.cpp:150] Version: 0.29.0
> I0420 22:36:44.655817 10456 exec.cpp:225] Executor registered on agent 
> 444d7f44-a320-42e4-874e-efa06378dc0f-S0
> Registered executor on mesos
> Starting task 190bb5f3-c917-4848-820b-bf76d9251cf6
> sh -c 'sleep 5'
> Forked command at 10463
> I0420 22:36:44.856591 10455 exec.cpp:271] Received reconnect request from 
> agent 444d7f44-a320-42e4-874e-efa06378dc0f-S0
> I0420 22:36:44.859297 10455 exec.cpp:248] Executor re-registered on agent 
> 444d7f44-a320-42e4-874e-efa06378dc0f-S0
> Re-registered executor on mesos
> Command exited with status 0 (pid: 10463)
> I0420 22:36:49.796159 10456 exec.cpp:399] Executor asked to shutdown
> [   OK ] CniIsolatorTest.ROOT_SlaveRecovery (6436 ms)
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Re: Review Request 46436: Added the test "CniIsolatorTest.ROOT_FailedPlugin".

[Jie Yu]

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


Ship it!




Ship It!

- Jie Yu


On May 12, 2016, 8:13 a.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46436/
> ---
> 
> (Updated May 12, 2016, 8:13 a.m.)
> 
> 
> Review request for mesos, Avinash sridharan and Jie Yu.
> 
> 
> Bugs: MESOS-5167
> https://issues.apache.org/jira/browse/MESOS-5167
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added the test "CniIsolatorTest.ROOT_FailedPlugin".
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/cni_isolator_tests.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/46436/diff/
> 
> 
> Testing
> ---
> 
> [ RUN  ] CniIsolatorTest.ROOT_FailedPlugin
> [   OK ] CniIsolatorTest.ROOT_FailedPlugin (917 ms)
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Re: Review Request 46435: Added the test "CniIsolatorTest.ROOT_VerifyCheckpointedInfo".

[Jie Yu]

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


Ship it!




Ship It!

- Jie Yu


On May 12, 2016, 8:12 a.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46435/
> ---
> 
> (Updated May 12, 2016, 8:12 a.m.)
> 
> 
> Review request for mesos, Avinash sridharan and Jie Yu.
> 
> 
> Bugs: MESOS-5167
> https://issues.apache.org/jira/browse/MESOS-5167
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added the test "CniIsolatorTest.ROOT_VerifyCheckpointedInfo".
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/cni_isolator_tests.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/46435/diff/
> 
> 
> Testing
> ---
> 
> [ RUN  ] CniIsolatorTest.ROOT_VerifyCheckpointedInfo
> I0420 22:34:55.747752 10309 exec.cpp:150] Version: 0.29.0
> I0420 22:34:55.768620 10328 exec.cpp:225] Executor registered on agent 
> e5403695-ee6e-435c-a273-b9523adf7051-S0
> Registered executor on mesos
> Starting task 51bf9caa-9305-4533-8ba5-1d7982d807b7
> sh -c 'sleep 1000'
> Forked command at 10332
> Received killTask for task 51bf9caa-9305-4533-8ba5-1d7982d807b7
> Sending SIGTERM to process tree at pid 10332
> Sent SIGTERM to the following process trees:
> [ 
> -+- 10332 sh -c sleep 1000 
>  --- 10333 sleep 1000 
> ]
> Command terminated with signal Terminated (pid: 10332)
> [   OK ] CniIsolatorTest.ROOT_VerifyCheckpointedInfo (3090 ms)
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Re: Review Request 46097: Added the test "CniIsolatorTest.ROOT_LaunchCommandTask".

[Jie Yu]

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


Ship it!




Ship It!

- Jie Yu


On May 12, 2016, 8:11 a.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46097/
> ---
> 
> (Updated May 12, 2016, 8:11 a.m.)
> 
> 
> Review request for mesos, Avinash sridharan and Jie Yu.
> 
> 
> Bugs: MESOS-5167
> https://issues.apache.org/jira/browse/MESOS-5167
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added the test "CniIsolatorTest.ROOT_LaunchCommandTask".
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 710e1644e2f0a8e9b87cc997b2211291f4e055fd 
>   src/tests/containerizer/cni_isolator_tests.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/46097/diff/
> 
> 
> Testing
> ---
> 
> [ RUN  ] CniIsolatorTest.ROOT_LaunchCommandTask
> + /home/stack/workspace/mesos/build/src/mesos-containerizer mount 
> --help=false --operation=make-rslave --path=/
> + grep+  -E /tmp/CniIsolatorTest_ROOT_LaunchCommandTask_HRK4Dz/.+ 
> /proc/self/mountinfo
> + grepcut -v -d  -f5
>  d06b117d-518b-41e2-b8e0-62a12083773c
> + xargs --no-run-if-empty umount -l
> + mount -n --rbind 
> /tmp/CniIsolatorTest_ROOT_LaunchCommandTask_HRK4Dz/provisioner/containers/d06b117d-518b-41e2-b8e0-62a12083773c/backends/copy/rootfses/7ea27011-cd3a-43b0-8301-b0b94d9f9b47
>  
> /tmp/CniIsolatorTest_ROOT_LaunchCommandTask_HRK4Dz/slaves/18dea042-5bb5-4336-8bc8-358ed1fbf6dd-S0/frameworks/18dea042-5bb5-4336-8bc8-358ed1fbf6dd-/executors/60e6d35d-6d33-47ae-9c23-d2e5c913c892/runs/d06b117d-518b-41e2-b8e0-62a12083773c/.rootfs
> I0420 22:26:00.924844  9305 exec.cpp:150] Version: 0.29.0
> I0420 22:26:00.942319  9375 exec.cpp:225] Executor registered on agent 
> 18dea042-5bb5-4336-8bc8-358ed1fbf6dd-S0
> Registered executor on mesos
> Starting task 60e6d35d-6d33-47ae-9c23-d2e5c913c892
> Forked command at 9382
> sh -c 'ls /'
> bin  dev  etc  home lib  linuxrc  mediamnt  proc  
>root run  sbin sys  tmp  usr  var
> Command exited with status 0 (pid: 9382)
> I0420 22:26:01.098331  9380 exec.cpp:399] Executor asked to shutdown
> [   OK ] CniIsolatorTest.ROOT_LaunchCommandTask (42603 ms)
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Re: Review Request 47489: Windows: Symplified `os::exists`.

[Daniel Pravat]

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

(Updated May 23, 2016, 9:36 p.m.)


Review request for mesos, Alex Naparu, Artem Harutyunyan, Alex Clemmer, Joris 
Van Remoortere, and Michael Park.


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


Repository: mesos


Description (updated)
---

os::exists determines if the process identified by the `pid` parameter
is present in the system. `OpenProcess` succeeds when the process object
exists and the caller has `PROCESS_QUERY_LIMITED_INFORMATION' right.


Diffs (updated)
-

  3rdparty/stout/include/stout/os/windows/exists.hpp 
423e4a81b4d34460f7f9f073577e11dfa2d2a520 

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


Testing
---

Windows build/run


Thanks,

Daniel Pravat

Re: Review Request 44839: Enabled mesos containerizer force_pull_image for appc.

[Jie Yu]

> On May 23, 2016, 9:15 p.m., Jie Yu wrote:
> >

Please make sure to a manual test. Why adding unit test is hard?


- Jie


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


On May 21, 2016, 1:32 a.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44839/
> ---
> 
> (Updated May 21, 2016, 1:32 a.m.)
> 
> 
> Review request for mesos, Gilbert Song, haosdent huang, and Jie Yu.
> 
> 
> Bugs: MESOS-4886
> https://issues.apache.org/jira/browse/MESOS-4886
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Enabled mesos containerizer force_pull_image for appc.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/provisioner/appc/store.cpp 
> 33e97fc645a9b7fbc9ae47f67c1b5dacf999fce0 
> 
> Diff: https://reviews.apache.org/r/44839/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 44839: Enabled mesos containerizer force_pull_image for appc.

[Jie Yu]

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




src/slave/containerizer/mesos/provisioner/appc/store.cpp (line 69)


per my comments in the previous patch, please `s/forcePullImage/cached/`



src/slave/containerizer/mesos/provisioner/appc/store.cpp (lines 220 - 229)


```
Option imageId = appc.has_id() ? appc.id() : cache->find(appc);
if (cached && imageId.isSome()) {
  if (os::exists(...)) {
...
  }
}

return _fetchImage(appc)
  .then(defer(self(), ...));
```


- Jie Yu


On May 21, 2016, 1:32 a.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44839/
> ---
> 
> (Updated May 21, 2016, 1:32 a.m.)
> 
> 
> Review request for mesos, Gilbert Song, haosdent huang, and Jie Yu.
> 
> 
> Bugs: MESOS-4886
> https://issues.apache.org/jira/browse/MESOS-4886
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Enabled mesos containerizer force_pull_image for appc.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/provisioner/appc/store.cpp 
> 33e97fc645a9b7fbc9ae47f67c1b5dacf999fce0 
> 
> Diff: https://reviews.apache.org/r/44839/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 47702: Added framework role & principal to the Web UI.

[Deshna Jain]

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

(Updated May 23, 2016, 9:08 p.m.)


Review request for mesos, haosdent huang and Vinod Kone.


Changes
---

See summary.


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


Repository: mesos


Description
---

See summary.


Diffs (updated)
-

  src/webui/master/static/framework.html 
f108e92f5b8d9ed4de718e55aa7302728a84003e 
  src/webui/master/static/frameworks.html 
f172e022e18df5b6aa3d232e610c3c732e20aa09 

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


Testing
---

make check (see attached screenshot for ui after the change)


File Attachments (updated)


image after making the change
  
https://reviews.apache.org/media/uploaded/files/2016/05/23/a2b9738e-b33a-4fce-98b0-9457d42a31f7__Screen_Shot_2016-05-23_at_12.38.01_PM.png


Thanks,

Deshna Jain

Re: Review Request 47702: Added framework role & principal to the Web UI.

[Deshna Jain]

> On May 23, 2016, 1:03 a.m., haosdent huang wrote:
> > Do you forget to update
> > * `framework.html` 
> > https://github.com/apache/mesos/blob/master/src/webui/master/static/framework.html#L24
> > * `agent.html` 
> > https://github.com/apache/mesos/blob/master/src/webui/master/static/agent.html
> >  
> > * `agent_framework.html` 
> > https://github.com/apache/mesos/blob/master/src/webui/master/static/agent_framework.html

I thought that the change is required only on frameworks page. Thanks for 
bringing it up :-)

I have made changes in framework.html.

I have a few concerns/queries here:
- The information displayed on Agents page is of agents only. So, I couldn't 
get you. What did you mean?
- @Vinod/Hoasdent: The agent's state endpoint does not have framework-principal 
information. Shall I add only 'Role' field to agent.html and 
agent_framework.html or just leave it to be tackled later?


- Deshna


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


On May 23, 2016, 9:08 p.m., Deshna Jain wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47702/
> ---
> 
> (Updated May 23, 2016, 9:08 p.m.)
> 
> 
> Review request for mesos, haosdent huang and Vinod Kone.
> 
> 
> Bugs: MESOS-4788
> https://issues.apache.org/jira/browse/MESOS-4788
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/webui/master/static/framework.html 
> f108e92f5b8d9ed4de718e55aa7302728a84003e 
>   src/webui/master/static/frameworks.html 
> f172e022e18df5b6aa3d232e610c3c732e20aa09 
> 
> Diff: https://reviews.apache.org/r/47702/diff/
> 
> 
> Testing
> ---
> 
> make check (see attached screenshot for ui after the change)
> 
> 
> File Attachments
> 
> 
> image after making the change
>   
> https://reviews.apache.org/media/uploaded/files/2016/05/23/a2b9738e-b33a-4fce-98b0-9457d42a31f7__Screen_Shot_2016-05-23_at_12.38.01_PM.png
> 
> 
> Thanks,
> 
> Deshna Jain
> 
>

Re: Review Request 47707: Refactored resource enumeration for containerizers and isolators.

[Kevin Klues]

> On May 23, 2016, 7:56 p.m., Benjamin Mahler wrote:
> > src/slave/containerizer/composing.cpp, lines 276-280
> > 
> >
> > How about "managed" and "manage" instead of "enumerated" and 
> > "enumerate" here? It seems like the important thing to understand in this 
> > comment is that each containerizer is trying to manage resources?

Sounds good to me.


> On May 23, 2016, 7:56 p.m., Benjamin Mahler wrote:
> > src/slave/containerizer/composing.cpp, lines 304-306
> > 
> >
> > Hm.. how would this read?
> > 
> > Failed to merge 'cpus' managed by the composed containerizers: one 
> > containerizer is managing 2cpus and another is managing 3cpus
> > 
> > You could print this by stringifying the filtered resources.

Sure, this sounds fine as well.


> On May 23, 2016, 7:56 p.m., Benjamin Mahler wrote:
> > src/slave/containerizer/containerizer.hpp, lines 64-68
> > 
> >
> > Hm.. it seems more like the default set of resources would not include 
> > the flags.
> > 
> > The current function is a little more than just the default resources, 
> > it is the flag provided resources along with defaults injected in for any 
> > that were omitted.
> > 
> > Could we instead provide just the defaults (per the suggested signature 
> > above) and have containerizer implementations inject defaults with the help 
> > of this function?
> > 
> > As an example:
> > 
> > ```
> > // Provides a means for containerizer implementation to use a 
> > // consistent set of default resources when resources are not
> > // specified via the '--resources' flag. The containerizer may
> > // choose its own default value by probing the system, however
> > // this helper provides a standard set of defaults for:
> > //
> > // ["cpus", "mem", "disk", "ports"]
> > //
> > // A containerizer is free to choose alternative defaults for these
> > // resources, but this increases the likelihood that containerizers
> > // disagree about the amount of resources to manage when used within
> > // the composing containerizer.
> > static Try defaultResources();
> > ```
> > 
> > This means we could augment the Containerizer::resources and 
> > Isolator::resources to do two things:
> > 
> > (1) Get informed about the resources provided via flags, and
> > (2) Return additional resources that the containerizer or isolator 
> > wishes to manage.
> > 
> > Something like:
> > 
> > ```
> > // Informs the containerizer / isolator about the resources
> > // that are explicitly specified. The containerizer / isolator
> > // returns a subset of the resources that it will manage. The
> > // containerizer / isolator may also return additional resources
> > // that it wishes to manage if they were not specified.
> > // For example, a GPU isolator, seeing that "gpus" is unspecified
> > // may probe the system and decide to manage some GPUs (i.e. it
> > // will return these GPUs from this function).
> > Future manage(const Resources& provided);
> > ```

We could do this (though it feels a little less intuitive and less flexible 
than allowing arbitrary flags to be passed in).  We are also unnecessarily 
enumerating all of the `cpu, mem, disk, ports` available on the system even if 
the flags would indicate to override them. Is the assumption that the actual 
class implementing `manage()` would need to find a way to get at the flags 
itself if it relies on them?  How would the default implementation of 
`manage()` work?  The whole goal of it was to avoid having to push new logic 
down into each existing containerizer (which your proposal will require in 
order to do the flags parsing somehwere to override the defaults).


> On May 23, 2016, 7:56 p.m., Benjamin Mahler wrote:
> > src/slave/containerizer/containerizer.cpp, lines 57-61
> > 
> >
> > Why this comment in the .cpp in addition to the one in the .hpp?

I can remove it.


> On May 23, 2016, 7:56 p.m., Benjamin Mahler wrote:
> > src/slave/containerizer/mesos/containerizer.cpp, lines 479-480
> > 
> >
> > Ditto on "manage" vs "enumerate" here.

OK.


> On May 23, 2016, 7:56 p.m., Benjamin Mahler wrote:
> > src/slave/containerizer/mesos/isolators/cgroups/devices/gpus/nvidia.cpp, 
> > line 242
> > 
> >
> > It's a bit implicit that this condition ensures that 
> > `resources.gpus().isSome()`, could we store the return value of 
> > `resources.gpus()` and use it in both the 

Re: Review Request 47695: Updated the website generation and development workflows with docker.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [47695]

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

- Mesos ReviewBot


On May 23, 2016, 5:49 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47695/
> ---
> 
> (Updated May 23, 2016, 5:49 p.m.)
> 
> 
> Review request for mesos, Tomasz Janiszewski, Kevin Klues, Neil Conway, and 
> Vinod Kone.
> 
> 
> Bugs: MESOS-5431
> https://issues.apache.org/jira/browse/MESOS-5431
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> In this changes, we include below items to make it more convenience to
> develop and generate the website.
> 
> * added `doxygen` and `javadoc` to `rake` default target.
> * added `build.sh` as the wrapper for the website generation to make
>   sure we clean up generated documents when exit.
> * switched the `Dockerfile` to based on `centos:7` because the default
>   `doxygen` version in `ubuntu:14.04` doesn't include `jquery.js`
>   correctly when generate `doxygen` documents.
> * adjusted the `Dockerfile` and `README.md` locations to avoid we
>   distract at finding the website generation document.
> * updated `release-guide.md` with the new website generation and
>   development workflows.
> 
> 
> Diffs
> -
> 
>   docs/release-guide.md 36e79796657a37b0b4649602281556f4ae471c34 
>   site/.gitignore 987a5d3cc161a67de1fb21c2002ed9698bbb0689 
>   site/Gemfile 14c50340d66fd7a703949a7932ab93a6b60f4e07 
>   site/Gemfile.lock 065107ec25f0eff28e2a09051b61152d49f77f9a 
>   site/README.md 468021ef4f8ed942c4a16784bc82349af9ae0d5e 
>   site/Rakefile 845ec3fd8b9e47ebe49991db30718912d4f9c41e 
>   site/build.sh PRE-CREATION 
>   support/site-docker/Dockerfile 8ea3d5c5fe531ec76950ed0bf110d7d477339158 
>   support/site-docker/README.md 957117e1211f06b40c5fb621039dd90aff562a87 
> 
> Diff: https://reviews.apache.org/r/47695/diff/
> 
> 
> Testing
> ---
> 
> Tested in OS X and Ubuntu 14.04
> 
> For document changes, could verified from 
> https://github.com/haosdent/mesos/tree/MESOS-5431/site
> 
> To verify website works fine after this changes, could check it via this gif:
> 
> ![website.gif](https://issues.apache.org/jira/secure/attachment/12805684/website.gif)
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 47707: Refactored resource enumeration for containerizers and isolators.

[Benjamin Mahler]

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




src/slave/containerizer/composing.cpp (lines 276 - 280)


How about "managed" and "manage" instead of "enumerated" and "enumerate" 
here? It seems like the important thing to understand in this comment is that 
each containerizer is trying to manage resources?



src/slave/containerizer/composing.cpp (lines 304 - 306)


Hm.. how would this read?

Failed to merge 'cpus' managed by the composed containerizers: one 
containerizer is managing 2cpus and another is managing 3cpus

You could print this by stringifying the filtered resources.



src/slave/containerizer/containerizer.hpp (lines 64 - 68)


Hm.. it seems more like the default set of resources would not include the 
flags.

The current function is a little more than just the default resources, it 
is the flag provided resources along with defaults injected in for any that 
were omitted.

Could we instead provide just the defaults (per the suggested signature 
above) and have containerizer implementations inject defaults with the help of 
this function?

As an example:

```
// Provides a means for containerizer implementation to use a 
// consistent set of default resources when resources are not
// specified via the '--resources' flag. The containerizer may
// choose its own default value by probing the system, however
// this helper provides a standard set of defaults for:
//
// ["cpus", "mem", "disk", "ports"]
//
// A containerizer is free to choose alternative defaults for these
// resources, but this increases the likelihood that containerizers
// disagree about the amount of resources to manage when used within
// the composing containerizer.
static Try defaultResources();
```

This means we could augment the Containerizer::resources and 
Isolator::resources to do two things:

(1) Get informed about the resources provided via flags, and
(2) Return additional resources that the containerizer or isolator wishes 
to manage.

Something like:

```
// Informs the containerizer / isolator about the resources
// that are explicitly specified. The containerizer / isolator
// returns a subset of the resources that it will manage. The
// containerizer / isolator may also return additional resources
// that it wishes to manage if they were not specified.
// For example, a GPU isolator, seeing that "gpus" is unspecified
// may probe the system and decide to manage some GPUs (i.e. it
// will return these GPUs from this function).
Future manage(const Resources& provided);
```



src/slave/containerizer/containerizer.cpp (lines 57 - 61)


Why this comment in the .cpp in addition to the one in the .hpp?



src/slave/containerizer/mesos/containerizer.cpp (lines 479 - 480)


Ditto on "manage" vs "enumerate" here.



src/slave/containerizer/mesos/isolators/cgroups/devices/gpus/nvidia.cpp (line 
242)


It's a bit implicit that this condition ensures that 
`resources.gpus().isSome()`, could we store the return value of 
`resources.gpus()` and use it in both the condition and the validation?

```
Option gpus = resources.gpus();

if (gpus.isSome()) {
  ...
  long long millis = static_cast(gpus.get() * 1000);
  ...
}
```

This seems clearer?



src/slave/slave.cpp (lines 476 - 478)


Would you mind adding a TODO to avoid waiting forever? I'm just thinking of 
the debugability when there's a inifitely pending future coming out of an 
isolator module.



src/slave/slave.cpp (lines 481 - 484)


Could you handle the discarded case? We don't expect a discarded result 
since we do not request a discard on this future, but it would be nice to avoid 
the call to .get() without validating this:

```
CHECK_READY(resources);
```

Or modify the error handling:

```
  if (!resources.isReady()) {
EXIT(EXIT_FAILURE)
  << "Failed to determine agent resources: " << resources.isFailed() ? 
resources.failure() : "discarded";
  }
```


- Benjamin Mahler


On May 23, 2016, 12:45 a.m., Kevin Klues wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> 

Re: Review Request 44838: Enabled mesos containerizer force_pull_image for docker.

[Jie Yu]

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


Fix it, then Ship it!





src/slave/containerizer/mesos/provisioner/docker/metadata_manager.hpp (line 93)


WE don't use 'const' for primitive types. Please use `bool cached` here.



src/slave/containerizer/mesos/provisioner/docker/metadata_manager.cpp (lines 
174 - 175)


```
VLOG(1) << "Ignored cached image '" << imageReference << "'";
```


- Jie Yu


On May 14, 2016, 12:58 a.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44838/
> ---
> 
> (Updated May 14, 2016, 12:58 a.m.)
> 
> 
> Review request for mesos, Gilbert Song, haosdent huang, and Jie Yu.
> 
> 
> Bugs: MESOS-4886
> https://issues.apache.org/jira/browse/MESOS-4886
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Enabled mesos containerizer force_pull_image for docker.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/provisioner/docker/metadata_manager.hpp 
> 137af502a66e6a65773c00eaacbe392576376284 
>   src/slave/containerizer/mesos/provisioner/docker/metadata_manager.cpp 
> cf630cc0b67a325529fa04ad2b1708e013b9596a 
>   src/slave/containerizer/mesos/provisioner/docker/store.cpp 
> eeec94326a4fd67675df10e0b6a32267e555fa96 
> 
> Diff: https://reviews.apache.org/r/44838/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> It is difficult to test this with unit test, but just test with mesos execute 
> by setting `force_pull_image` as true and found that the iamge was always 
> pulled when `force_pull_image` is true.
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 47732: Fixed a race in long lived executor.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [47732]

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

- Mesos ReviewBot


On May 23, 2016, 5:29 p.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47732/
> ---
> 
> (Updated May 23, 2016, 5:29 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> There is a possible race condition when the `TASK_FINISHED`
> update calls into `update` when the executor process might
> itself be already executing. Also after this fix, even if the 
> executor is disconnected from the agent by the time the `update`
> function is called, the updates would be sent again as part of 
> unacknowledged updates on re-registration.
> 
> 
> Diffs
> -
> 
>   src/examples/long_lived_executor.cpp 
> 94379b01c3ac8dbf513559789046677bceea688c 
> 
> Diff: https://reviews.apache.org/r/47732/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> This is a bit hard to reproduce due to the random delay. e.g., this happened 
> once in ~2-3 days on our internal cluster.
> 
> ```
> Backtrace:
> ./long-lived-executor(_ZNSt4listIN2id4UUIDESaIS1_EE5eraseESt14_List_iteratorIS1_E+0x14)[0x41ffec]
> ./long-lived-executor[0x41cf81]
> ./long-lived-executor[0x41ad3a]
> ./long-lived-executor[0x41f245]
> ./long-lived-executor[0x424622]
> ./long-lived-executor(_ZN7process11ProcessBase5serveERKNS_5EventE+0x2e)[0x419010]
> /opt/mesosphere/lib/libmesos-0.29.0.so(_ZN7process14ProcessManager6resumeEPNS_11ProcessBaseE+0x2d1)[0x7fa3eba5cc21]
> /opt/mesosphere/lib/libmesos-0.29.0.so(+0x13b1f27)[0x7fa3eba5cf27]
> /lib64/libstdc++.so.6(+0xb5220)[0x7fa3e9f43220]
> /lib64/libpthread.so.0(+0x7dc5)[0x7fa3ea19ddc5]
> /lib64/libc.so.6(clone+0x6d)[0x7fa3e99ad28d]
> ```
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Re: Review Request 47732: Fixed a race in long lived executor.

[Vinod Kone]

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


Ship it!




Ship It!

- Vinod Kone


On May 23, 2016, 5:29 p.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47732/
> ---
> 
> (Updated May 23, 2016, 5:29 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> There is a possible race condition when the `TASK_FINISHED`
> update calls into `update` when the executor process might
> itself be already executing. Also after this fix, even if the 
> executor is disconnected from the agent by the time the `update`
> function is called, the updates would be sent again as part of 
> unacknowledged updates on re-registration.
> 
> 
> Diffs
> -
> 
>   src/examples/long_lived_executor.cpp 
> 94379b01c3ac8dbf513559789046677bceea688c 
> 
> Diff: https://reviews.apache.org/r/47732/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> This is a bit hard to reproduce due to the random delay. e.g., this happened 
> once in ~2-3 days on our internal cluster.
> 
> ```
> Backtrace:
> ./long-lived-executor(_ZNSt4listIN2id4UUIDESaIS1_EE5eraseESt14_List_iteratorIS1_E+0x14)[0x41ffec]
> ./long-lived-executor[0x41cf81]
> ./long-lived-executor[0x41ad3a]
> ./long-lived-executor[0x41f245]
> ./long-lived-executor[0x424622]
> ./long-lived-executor(_ZN7process11ProcessBase5serveERKNS_5EventE+0x2e)[0x419010]
> /opt/mesosphere/lib/libmesos-0.29.0.so(_ZN7process14ProcessManager6resumeEPNS_11ProcessBaseE+0x2d1)[0x7fa3eba5cc21]
> /opt/mesosphere/lib/libmesos-0.29.0.so(+0x13b1f27)[0x7fa3eba5cf27]
> /lib64/libstdc++.so.6(+0xb5220)[0x7fa3e9f43220]
> /lib64/libpthread.so.0(+0x7dc5)[0x7fa3ea19ddc5]
> /lib64/libc.so.6(clone+0x6d)[0x7fa3e99ad28d]
> ```
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Re: Review Request 44837: Added force_pull_image to Image protobuf.

[Jie Yu]

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




include/mesos/mesos.proto (line 1580)


I would instead calling it `cached`. The `Image` here should be declarative 
and describe the property of this image, rather than _how_ to get the image.


- Jie Yu


On May 13, 2016, 3:26 a.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44837/
> ---
> 
> (Updated May 13, 2016, 3:26 a.m.)
> 
> 
> Review request for mesos, Gilbert Song, haosdent huang, and Jie Yu.
> 
> 
> Bugs: MESOS-4886
> https://issues.apache.org/jira/browse/MESOS-4886
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added force_pull_image to Image protobuf.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 887ffe9c5f04e25539a4c6b3d52ce5299c65e8d3 
>   include/mesos/v1/mesos.proto 9e59aed20965d50ee10989ff6b75db742cf2b83b 
> 
> Diff: https://reviews.apache.org/r/44837/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 47695: Updated the website generation and development workflows with docker.

[Tomasz Janiszewski]

> On May 23, 2016, 7:50 a.m., Tomasz Janiszewski wrote:
> >
> 
> haosdent huang wrote:
> Hi, @janisz. Thank you very much for your detail reviews, may you help to 
> review this patch again? Thank you in advance.

Tested. LGTM but I left some minor comments.


- Tomasz


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


On May 23, 2016, 5:49 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47695/
> ---
> 
> (Updated May 23, 2016, 5:49 p.m.)
> 
> 
> Review request for mesos, Tomasz Janiszewski, Kevin Klues, Neil Conway, and 
> Vinod Kone.
> 
> 
> Bugs: MESOS-5431
> https://issues.apache.org/jira/browse/MESOS-5431
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> In this changes, we include below items to make it more convenience to
> develop and generate the website.
> 
> * added `doxygen` and `javadoc` to `rake` default target.
> * added `build.sh` as the wrapper for the website generation to make
>   sure we clean up generated documents when exit.
> * switched the `Dockerfile` to based on `centos:7` because the default
>   `doxygen` version in `ubuntu:14.04` doesn't include `jquery.js`
>   correctly when generate `doxygen` documents.
> * adjusted the `Dockerfile` and `README.md` locations to avoid we
>   distract at finding the website generation document.
> * updated `release-guide.md` with the new website generation and
>   development workflows.
> 
> 
> Diffs
> -
> 
>   docs/release-guide.md 36e79796657a37b0b4649602281556f4ae471c34 
>   site/.gitignore 987a5d3cc161a67de1fb21c2002ed9698bbb0689 
>   site/Gemfile 14c50340d66fd7a703949a7932ab93a6b60f4e07 
>   site/Gemfile.lock 065107ec25f0eff28e2a09051b61152d49f77f9a 
>   site/README.md 468021ef4f8ed942c4a16784bc82349af9ae0d5e 
>   site/Rakefile 845ec3fd8b9e47ebe49991db30718912d4f9c41e 
>   site/build.sh PRE-CREATION 
>   support/site-docker/Dockerfile 8ea3d5c5fe531ec76950ed0bf110d7d477339158 
>   support/site-docker/README.md 957117e1211f06b40c5fb621039dd90aff562a87 
> 
> Diff: https://reviews.apache.org/r/47695/diff/
> 
> 
> Testing
> ---
> 
> Tested in OS X and Ubuntu 14.04
> 
> For document changes, could verified from 
> https://github.com/haosdent/mesos/tree/MESOS-5431/site
> 
> To verify website works fine after this changes, could check it via this gif:
> 
> ![website.gif](https://issues.apache.org/jira/secure/attachment/12805684/website.gif)
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 47695: Updated the website generation and development workflows with docker.

[Tomasz Janiszewski]

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




site/README.md (line 9)


Running this command will end up with error: `unable to prepare context: 
unable to evaluate symlinks in context path: lstat no such file or directory`. 
Later this document assumed user is in `site` dir. To be consistend replace 
`site` with `.`



site/README.md (line 32)


I think this section needs example that will work for 99% users.



support/site-docker/Dockerfile (line 6)


Do we really need whole `Development Tools` to build website.


- Tomasz Janiszewski


On May 23, 2016, 5:49 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47695/
> ---
> 
> (Updated May 23, 2016, 5:49 p.m.)
> 
> 
> Review request for mesos, Tomasz Janiszewski, Kevin Klues, Neil Conway, and 
> Vinod Kone.
> 
> 
> Bugs: MESOS-5431
> https://issues.apache.org/jira/browse/MESOS-5431
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> In this changes, we include below items to make it more convenience to
> develop and generate the website.
> 
> * added `doxygen` and `javadoc` to `rake` default target.
> * added `build.sh` as the wrapper for the website generation to make
>   sure we clean up generated documents when exit.
> * switched the `Dockerfile` to based on `centos:7` because the default
>   `doxygen` version in `ubuntu:14.04` doesn't include `jquery.js`
>   correctly when generate `doxygen` documents.
> * adjusted the `Dockerfile` and `README.md` locations to avoid we
>   distract at finding the website generation document.
> * updated `release-guide.md` with the new website generation and
>   development workflows.
> 
> 
> Diffs
> -
> 
>   docs/release-guide.md 36e79796657a37b0b4649602281556f4ae471c34 
>   site/.gitignore 987a5d3cc161a67de1fb21c2002ed9698bbb0689 
>   site/Gemfile 14c50340d66fd7a703949a7932ab93a6b60f4e07 
>   site/Gemfile.lock 065107ec25f0eff28e2a09051b61152d49f77f9a 
>   site/README.md 468021ef4f8ed942c4a16784bc82349af9ae0d5e 
>   site/Rakefile 845ec3fd8b9e47ebe49991db30718912d4f9c41e 
>   site/build.sh PRE-CREATION 
>   support/site-docker/Dockerfile 8ea3d5c5fe531ec76950ed0bf110d7d477339158 
>   support/site-docker/README.md 957117e1211f06b40c5fb621039dd90aff562a87 
> 
> Diff: https://reviews.apache.org/r/47695/diff/
> 
> 
> Testing
> ---
> 
> Tested in OS X and Ubuntu 14.04
> 
> For document changes, could verified from 
> https://github.com/haosdent/mesos/tree/MESOS-5431/site
> 
> To verify website works fine after this changes, could check it via this gif:
> 
> ![website.gif](https://issues.apache.org/jira/secure/attachment/12805684/website.gif)
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 47706: Provided defaults for all virtual functions in `mesos::slave::Isolator'.

[Benjamin Mahler]

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


Ship it!




Ship It!

- Benjamin Mahler


On May 23, 2016, 7:06 a.m., Kevin Klues wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47706/
> ---
> 
> (Updated May 23, 2016, 7:06 a.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Jie Yu.
> 
> 
> Bugs: MESOS-5435
> https://issues.apache.org/jira/browse/MESOS-5435
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Previously all of the virtual functions in `mesos::slave::Isolator` were
> pure virtual (expect status()). For many isolators, however, it
> doesn't make sense to implement all of these virtual functions.
> Currently, each isolator has to provide its own default implementation
> of these functions even if they aren't really relying on them. This
> adds unnecessary extra code to many isolators that don't need them.
> 
> In addition to the changes to `mesos::slave::Isolator`, this commit
> also provides default implementations for all of the corresponding
> virtual functions in `MesosIsolatorProcess`. This way, classes that
> inherit from `MesosIsolatorProcess` get the same benefit of code reuse
> as those that inherit form `mesos::slave::Isolator`.
> 
> 
> Diffs
> -
> 
>   include/mesos/slave/isolator.hpp 4be8c2bb409052e2e07138483408209384f41e23 
>   src/slave/containerizer/mesos/isolator.hpp 
> bacd86af42d16cb7c9b6622dfb298dcaa7007b75 
>   src/slave/containerizer/mesos/isolators/cgroups/devices/gpus/nvidia.hpp 
> 502204650192d5ea44aa631eac8eb37e051843f0 
>   src/slave/containerizer/mesos/isolators/cgroups/devices/gpus/nvidia.cpp 
> 8f81cb79c10261670efc9eaa8614751854f53806 
>   src/slave/containerizer/mesos/isolators/cgroups/net_cls.hpp 
> d7592cf49f4e4c5f3fc6a3244d9b922d4eb70a9f 
>   src/slave/containerizer/mesos/isolators/cgroups/net_cls.cpp 
> 54ded996c879c41163cbf7e9c1bb7ae6807c8801 
>   src/slave/containerizer/mesos/isolators/cgroups/perf_event.hpp 
> 65e731886b9e5cac07ae3ad6398faf8f50de5650 
>   src/slave/containerizer/mesos/isolators/cgroups/perf_event.cpp 
> 336ae0a4c6e2ad519f7913cd819f2ddea82c5cec 
>   src/slave/containerizer/mesos/isolators/docker/runtime.hpp 
> 90179119ef297855091dad3fe969aa79810bf209 
>   src/slave/containerizer/mesos/isolators/docker/runtime.cpp 
> a12220a2693271fc192ab9165b176b1f3d18b9ce 
>   src/slave/containerizer/mesos/isolators/docker/volume/isolator.hpp 
> 01777d4a656657e591593631fda49787f7d9fe55 
>   src/slave/containerizer/mesos/isolators/docker/volume/isolator.cpp 
> 92fce1cbdcd0d2cbd68b02ab02c5c224c42d67a4 
>   src/slave/containerizer/mesos/isolators/filesystem/linux.hpp 
> c99f33a77e4db5407cc26361a2f253b00e91f5b5 
>   src/slave/containerizer/mesos/isolators/filesystem/linux.cpp 
> 5edbce95cc9eb4d6d22f9ab1528902cc745780af 
>   src/slave/containerizer/mesos/isolators/filesystem/posix.hpp 
> c6cea98e16f2bdea2da0220c235468080bbcd17b 
>   src/slave/containerizer/mesos/isolators/filesystem/posix.cpp 
> 01c0ad6dbb6d509e62e769365586b3d23dcb240d 
>   src/slave/containerizer/mesos/isolators/filesystem/shared.hpp 
> dd0481cd7b5aaa8e198160d9d604090041033605 
>   src/slave/containerizer/mesos/isolators/filesystem/shared.cpp 
> 9ae4d937c27286f18026c47b61b2f8fcff6e74a6 
>   src/slave/containerizer/mesos/isolators/namespaces/pid.hpp 
> c8f389d36138681795641088d3ef686def0a4e64 
>   src/slave/containerizer/mesos/isolators/namespaces/pid.cpp 
> 5970d22e7198236d22c55de6153f465ed5f5fd7a 
>   src/slave/containerizer/mesos/isolators/network/cni/cni.hpp 
> a06bb89a7a79a62949a48274df806f9f95da09e7 
>   src/slave/containerizer/mesos/isolators/network/cni/cni.cpp 
> dae369aadb940150aa806b28d9269e3d88cf57ed 
>   src/slave/containerizer/mesos/isolators/xfs/disk.hpp 
> 822de65d23487e1ae36bec9c1f706a06166a187a 
>   src/slave/containerizer/mesos/isolators/xfs/disk.cpp 
> 2f65f0a072911d3384684609828ec6cb6a27d19a 
> 
> Diff: https://reviews.apache.org/r/47706/diff/
> 
> 
> Testing
> ---
> 
> make -j check
> 
> 
> Thanks,
> 
> Kevin Klues
> 
>

Re: Review Request 47719: Removed invalid gpu statistics columns in framework data tables.

[haosdent huang]

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

(Updated May 23, 2016, 6:11 p.m.)


Review request for mesos, Benjamin Mahler, Kevin Klues, and Vinod Kone.


Changes
---

Update bugs link


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


Repository: mesos


Description
---

Removed invalid gpu statistics columns in framework data tables.


Diffs
-

  src/webui/master/static/agent.html 2f40034cf89e5f92ace3043578881b7b7157e25f 
  src/webui/master/static/agent_framework.html 
4d61c144ffbc51b9855e4b9f9cd99c0bbb1d35a5 

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


Testing
---

# For apgent framework page:
## Before:
![](https://issues.apache.org/jira/secure/attachment/12805672/incorrect_agent_framework_page.png)
## After:
![](https://issues.apache.org/jira/secure/attachment/12805670/after_agent_framework_page.png)

# For agent page:
## Before:
![](https://issues.apache.org/jira/secure/attachment/12805673/incorrect_agent_page.png)
## After:
![](https://issues.apache.org/jira/secure/attachment/12805671/after_agent_page.png)


Thanks,

haosdent huang

Re: Review Request 47719: Removed invalid gpu statistics columns in framework data tables.

[Kevin Klues]

Can you also link the issue back to the review in JIRA:

Workflow -> Post Review

https://issues.apache.org/jira/browse/MESOS-5436

On Mon, May 23, 2016 at 11:02 AM, haosdent huang  wrote:

> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47719/
>
> On May 23rd, 2016, 5:29 p.m. UTC, *Kevin Klues* wrote:
>
> Looks good.  Just curous -- how did you upload the pictures to the testing 
> section?  Is there someway to make them smaller next time?
>
> LoL I upload it via jira and use ![](image_link) to show it in reviewboard. 
> Seems reviewboard don't allow specify the image width and height, I would 
> resize them before upload next time. ;-)
>
>
> - haosdent
>
> On May 23rd, 2016, 4:56 p.m. UTC, haosdent huang wrote:
> Review request for mesos, Benjamin Mahler, Kevin Klues, and Vinod Kone.
> By haosdent huang.
>
> *Updated May 23, 2016, 4:56 p.m.*
> *Bugs: * MESOS-5157 
> *Repository: * mesos
> Description
>
> Removed invalid gpu statistics columns in framework data tables.
>
> Testing
>
> For apgent framework page:Before:
>
> After:
>
> For agent page:Before:
>
> After:
>
> Diffs
>
>- src/webui/master/static/agent.html
>(2f40034cf89e5f92ace3043578881b7b7157e25f)
>- src/webui/master/static/agent_framework.html
>(4d61c144ffbc51b9855e4b9f9cd99c0bbb1d35a5)
>
> View Diff 
>



-- 
~Kevin

Re: Review Request 47719: Removed invalid gpu statistics columns in framework data tables.

[haosdent huang]

> On May 23, 2016, 5:29 p.m., Kevin Klues wrote:
> > Looks good.  Just curous -- how did you upload the pictures to the testing 
> > section?  Is there someway to make them smaller next time?

LoL I upload it via jira and use `![](image_link)` to show it in reviewboard. 
Seems reviewboard don't allow specify the image width and height, I would 
resize them before upload next time. ;-)


- haosdent


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


On May 23, 2016, 4:56 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47719/
> ---
> 
> (Updated May 23, 2016, 4:56 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Kevin Klues, and Vinod Kone.
> 
> 
> Bugs: MESOS-5157
> https://issues.apache.org/jira/browse/MESOS-5157
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Removed invalid gpu statistics columns in framework data tables.
> 
> 
> Diffs
> -
> 
>   src/webui/master/static/agent.html 2f40034cf89e5f92ace3043578881b7b7157e25f 
>   src/webui/master/static/agent_framework.html 
> 4d61c144ffbc51b9855e4b9f9cd99c0bbb1d35a5 
> 
> Diff: https://reviews.apache.org/r/47719/diff/
> 
> 
> Testing
> ---
> 
> # For apgent framework page:
> ## Before:
> ![](https://issues.apache.org/jira/secure/attachment/12805672/incorrect_agent_framework_page.png)
> ## After:
> ![](https://issues.apache.org/jira/secure/attachment/12805670/after_agent_framework_page.png)
> 
> # For agent page:
> ## Before:
> ![](https://issues.apache.org/jira/secure/attachment/12805673/incorrect_agent_page.png)
> ## After:
> ![](https://issues.apache.org/jira/secure/attachment/12805671/after_agent_page.png)
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 47719: Removed invalid gpu statistics columns in framework data tables.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [47719]

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

- Mesos ReviewBot


On May 23, 2016, 4:56 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47719/
> ---
> 
> (Updated May 23, 2016, 4:56 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Kevin Klues, and Vinod Kone.
> 
> 
> Bugs: MESOS-5157
> https://issues.apache.org/jira/browse/MESOS-5157
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Removed invalid gpu statistics columns in framework data tables.
> 
> 
> Diffs
> -
> 
>   src/webui/master/static/agent.html 2f40034cf89e5f92ace3043578881b7b7157e25f 
>   src/webui/master/static/agent_framework.html 
> 4d61c144ffbc51b9855e4b9f9cd99c0bbb1d35a5 
> 
> Diff: https://reviews.apache.org/r/47719/diff/
> 
> 
> Testing
> ---
> 
> # For apgent framework page:
> ## Before:
> ![](https://issues.apache.org/jira/secure/attachment/12805672/incorrect_agent_framework_page.png)
> ## After:
> ![](https://issues.apache.org/jira/secure/attachment/12805670/after_agent_framework_page.png)
> 
> # For agent page:
> ## Before:
> ![](https://issues.apache.org/jira/secure/attachment/12805673/incorrect_agent_page.png)
> ## After:
> ![](https://issues.apache.org/jira/secure/attachment/12805671/after_agent_page.png)
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 47695: Updated the website generation and development workflows with docker.

[haosdent huang]

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




support/site-docker/Dockerfile (line 10)


We have to use `en_US.UTF-8` otherwise `rake` would failed when I test this.


- haosdent huang


On May 23, 2016, 5:49 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47695/
> ---
> 
> (Updated May 23, 2016, 5:49 p.m.)
> 
> 
> Review request for mesos, Tomasz Janiszewski, Kevin Klues, Neil Conway, and 
> Vinod Kone.
> 
> 
> Bugs: MESOS-5431
> https://issues.apache.org/jira/browse/MESOS-5431
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> In this changes, we include below items to make it more convenience to
> develop and generate the website.
> 
> * added `doxygen` and `javadoc` to `rake` default target.
> * added `build.sh` as the wrapper for the website generation to make
>   sure we clean up generated documents when exit.
> * switched the `Dockerfile` to based on `centos:7` because the default
>   `doxygen` version in `ubuntu:14.04` doesn't include `jquery.js`
>   correctly when generate `doxygen` documents.
> * adjusted the `Dockerfile` and `README.md` locations to avoid we
>   distract at finding the website generation document.
> * updated `release-guide.md` with the new website generation and
>   development workflows.
> 
> 
> Diffs
> -
> 
>   docs/release-guide.md 36e79796657a37b0b4649602281556f4ae471c34 
>   site/.gitignore 987a5d3cc161a67de1fb21c2002ed9698bbb0689 
>   site/Gemfile 14c50340d66fd7a703949a7932ab93a6b60f4e07 
>   site/Gemfile.lock 065107ec25f0eff28e2a09051b61152d49f77f9a 
>   site/README.md 468021ef4f8ed942c4a16784bc82349af9ae0d5e 
>   site/Rakefile 845ec3fd8b9e47ebe49991db30718912d4f9c41e 
>   site/build.sh PRE-CREATION 
>   support/site-docker/Dockerfile 8ea3d5c5fe531ec76950ed0bf110d7d477339158 
>   support/site-docker/README.md 957117e1211f06b40c5fb621039dd90aff562a87 
> 
> Diff: https://reviews.apache.org/r/47695/diff/
> 
> 
> Testing
> ---
> 
> Tested in OS X and Ubuntu 14.04
> 
> For document changes, could verified from 
> https://github.com/haosdent/mesos/tree/MESOS-5431/site
> 
> To verify website works fine after this changes, could check it via this gif:
> 
> ![website.gif](https://issues.apache.org/jira/secure/attachment/12805684/website.gif)
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 47695: Updated the website generation and development workflows with docker.

[haosdent huang]

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




site/Rakefile (line 75)


My bad that didn't update it correctly in 
https://reviews.apache.org/r/47698/


- haosdent huang


On May 23, 2016, 5:49 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47695/
> ---
> 
> (Updated May 23, 2016, 5:49 p.m.)
> 
> 
> Review request for mesos, Tomasz Janiszewski, Kevin Klues, Neil Conway, and 
> Vinod Kone.
> 
> 
> Bugs: MESOS-5431
> https://issues.apache.org/jira/browse/MESOS-5431
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> In this changes, we include below items to make it more convenience to
> develop and generate the website.
> 
> * added `doxygen` and `javadoc` to `rake` default target.
> * added `build.sh` as the wrapper for the website generation to make
>   sure we clean up generated documents when exit.
> * switched the `Dockerfile` to based on `centos:7` because the default
>   `doxygen` version in `ubuntu:14.04` doesn't include `jquery.js`
>   correctly when generate `doxygen` documents.
> * adjusted the `Dockerfile` and `README.md` locations to avoid we
>   distract at finding the website generation document.
> * updated `release-guide.md` with the new website generation and
>   development workflows.
> 
> 
> Diffs
> -
> 
>   docs/release-guide.md 36e79796657a37b0b4649602281556f4ae471c34 
>   site/.gitignore 987a5d3cc161a67de1fb21c2002ed9698bbb0689 
>   site/Gemfile 14c50340d66fd7a703949a7932ab93a6b60f4e07 
>   site/Gemfile.lock 065107ec25f0eff28e2a09051b61152d49f77f9a 
>   site/README.md 468021ef4f8ed942c4a16784bc82349af9ae0d5e 
>   site/Rakefile 845ec3fd8b9e47ebe49991db30718912d4f9c41e 
>   site/build.sh PRE-CREATION 
>   support/site-docker/Dockerfile 8ea3d5c5fe531ec76950ed0bf110d7d477339158 
>   support/site-docker/README.md 957117e1211f06b40c5fb621039dd90aff562a87 
> 
> Diff: https://reviews.apache.org/r/47695/diff/
> 
> 
> Testing
> ---
> 
> Tested in OS X and Ubuntu 14.04
> 
> For document changes, could verified from 
> https://github.com/haosdent/mesos/tree/MESOS-5431/site
> 
> To verify website works fine after this changes, could check it via this gif:
> 
> ![website.gif](https://issues.apache.org/jira/secure/attachment/12805684/website.gif)
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 45377: Made "driver" as optional for DockerVolume.

[Jie Yu]

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




include/mesos/mesos.proto (line 1624)


Please do not change the tag number.


- Jie Yu


On May 10, 2016, 3:41 a.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45377/
> ---
> 
> (Updated May 10, 2016, 3:41 a.m.)
> 
> 
> Review request for mesos, David vonThenen, Gilbert Song, haosdent huang, and 
> Jie Yu.
> 
> 
> Bugs: MESOS-5354
> https://issues.apache.org/jira/browse/MESOS-5354
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Made "driver" as optional for DockerVolume.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 73d4bc0ec3f34033eddf04aa41893c1fc66933b3 
>   include/mesos/v1/mesos.proto 44b4f8a059f9dfdcbf02f0c30c1b859898c2e617 
>   src/slave/containerizer/mesos/isolators/docker/volume/isolator.cpp 
> 76d0fa183c9099584da6a4ee1e5d474b871310c3 
> 
> Diff: https://reviews.apache.org/r/45377/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 47695: Updated the website generation and development workflows with docker.

[haosdent huang]

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




site/Gemfile.lock (line 100)


`Gemfile.lock` is generated by `bundle install`. Because we add `gem 
"therubyracer"` to Gemfile, `bundle install` update this file.


- haosdent huang


On May 23, 2016, 5:49 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47695/
> ---
> 
> (Updated May 23, 2016, 5:49 p.m.)
> 
> 
> Review request for mesos, Tomasz Janiszewski, Kevin Klues, Neil Conway, and 
> Vinod Kone.
> 
> 
> Bugs: MESOS-5431
> https://issues.apache.org/jira/browse/MESOS-5431
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> In this changes, we include below items to make it more convenience to
> develop and generate the website.
> 
> * added `doxygen` and `javadoc` to `rake` default target.
> * added `build.sh` as the wrapper for the website generation to make
>   sure we clean up generated documents when exit.
> * switched the `Dockerfile` to based on `centos:7` because the default
>   `doxygen` version in `ubuntu:14.04` doesn't include `jquery.js`
>   correctly when generate `doxygen` documents.
> * adjusted the `Dockerfile` and `README.md` locations to avoid we
>   distract at finding the website generation document.
> * updated `release-guide.md` with the new website generation and
>   development workflows.
> 
> 
> Diffs
> -
> 
>   docs/release-guide.md 36e79796657a37b0b4649602281556f4ae471c34 
>   site/.gitignore 987a5d3cc161a67de1fb21c2002ed9698bbb0689 
>   site/Gemfile 14c50340d66fd7a703949a7932ab93a6b60f4e07 
>   site/Gemfile.lock 065107ec25f0eff28e2a09051b61152d49f77f9a 
>   site/README.md 468021ef4f8ed942c4a16784bc82349af9ae0d5e 
>   site/Rakefile 845ec3fd8b9e47ebe49991db30718912d4f9c41e 
>   site/build.sh PRE-CREATION 
>   support/site-docker/Dockerfile 8ea3d5c5fe531ec76950ed0bf110d7d477339158 
>   support/site-docker/README.md 957117e1211f06b40c5fb621039dd90aff562a87 
> 
> Diff: https://reviews.apache.org/r/47695/diff/
> 
> 
> Testing
> ---
> 
> Tested in OS X and Ubuntu 14.04
> 
> For document changes, could verified from 
> https://github.com/haosdent/mesos/tree/MESOS-5431/site
> 
> To verify website works fine after this changes, could check it via this gif:
> 
> ![website.gif](https://issues.apache.org/jira/secure/attachment/12805684/website.gif)
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 47695: Updated the website generation and development workflows with docker.

[haosdent huang]

> On May 23, 2016, 7:50 a.m., Tomasz Janiszewski wrote:
> >

Hi, @janisz. Thank you very much for your detail reviews, may you help to 
review this patch again? Thank you in advance.


- haosdent


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


On May 23, 2016, 5:49 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47695/
> ---
> 
> (Updated May 23, 2016, 5:49 p.m.)
> 
> 
> Review request for mesos, Tomasz Janiszewski, Kevin Klues, Neil Conway, and 
> Vinod Kone.
> 
> 
> Bugs: MESOS-5431
> https://issues.apache.org/jira/browse/MESOS-5431
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> In this changes, we include below items to make it more convenience to
> develop and generate the website.
> 
> * added `doxygen` and `javadoc` to `rake` default target.
> * added `build.sh` as the wrapper for the website generation to make
>   sure we clean up generated documents when exit.
> * switched the `Dockerfile` to based on `centos:7` because the default
>   `doxygen` version in `ubuntu:14.04` doesn't include `jquery.js`
>   correctly when generate `doxygen` documents.
> * adjusted the `Dockerfile` and `README.md` locations to avoid we
>   distract at finding the website generation document.
> * updated `release-guide.md` with the new website generation and
>   development workflows.
> 
> 
> Diffs
> -
> 
>   docs/release-guide.md 36e79796657a37b0b4649602281556f4ae471c34 
>   site/.gitignore 987a5d3cc161a67de1fb21c2002ed9698bbb0689 
>   site/Gemfile 14c50340d66fd7a703949a7932ab93a6b60f4e07 
>   site/Gemfile.lock 065107ec25f0eff28e2a09051b61152d49f77f9a 
>   site/README.md 468021ef4f8ed942c4a16784bc82349af9ae0d5e 
>   site/Rakefile 845ec3fd8b9e47ebe49991db30718912d4f9c41e 
>   site/build.sh PRE-CREATION 
>   support/site-docker/Dockerfile 8ea3d5c5fe531ec76950ed0bf110d7d477339158 
>   support/site-docker/README.md 957117e1211f06b40c5fb621039dd90aff562a87 
> 
> Diff: https://reviews.apache.org/r/47695/diff/
> 
> 
> Testing
> ---
> 
> Tested in OS X and Ubuntu 14.04
> 
> For document changes, could verified from 
> https://github.com/haosdent/mesos/tree/MESOS-5431/site
> 
> To verify website works fine after this changes, could check it via this gif:
> 
> ![website.gif](https://issues.apache.org/jira/secure/attachment/12805684/website.gif)
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 47695: Updated the website generation and development workflows with docker.

[haosdent huang]

> On May 22, 2016, 5:46 p.m., Vinod Kone wrote:
> > site/Rakefile, line 5
> > 
> >
> > shouldn't the "build" task happen after "doxygen" and "javadoc"?

According my test, there are not differences not matter we put it before 
`:build` or after `:build`. I put it after `:build` because I saw

```
The doxygen and javadoc pages must be generated after running rake.
```

in https://github.com/apache/mesos/tree/master/site

But let me move it before `:build` because not differences.


> On May 22, 2016, 5:46 p.m., Vinod Kone wrote:
> > site/Rakefile, lines 75-82
> > 
> >
> > why the change from "pubish" to "source" here?

haha, there is a long story here. At first, I use `publish` folder when 
generate `javadoc` and `doxygen`.
However, `middleman server` only serve files under `sources`(`middlman server` 
would generate a sitemap structure internal and route requests by it). If we 
want to make it serve files for our custom folder under `publish`, we need to 
upgrade `middleman` from 3 to 4 and use `import_path` config item.
After I upgrade `middleman` to 4, I find the site broken and try to fix them 
and make it work under `middleman` 4 . Eventually, I neary change almost files 
under `site` folder... At that time, I think the better way is just put then 
under `source` folder when generate `javadoc` and `doxygen`.

We still could keep it use `publish` here, and it should be shown after we 
public it to the apache website. However, we could not preview `javadoc` and 
`doxygen` locally when using `middleman server`. So I perfer to use `source` 
folder here.


- haosdent


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


On May 23, 2016, 5:49 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47695/
> ---
> 
> (Updated May 23, 2016, 5:49 p.m.)
> 
> 
> Review request for mesos, Tomasz Janiszewski, Kevin Klues, Neil Conway, and 
> Vinod Kone.
> 
> 
> Bugs: MESOS-5431
> https://issues.apache.org/jira/browse/MESOS-5431
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> In this changes, we include below items to make it more convenience to
> develop and generate the website.
> 
> * added `doxygen` and `javadoc` to `rake` default target.
> * added `build.sh` as the wrapper for the website generation to make
>   sure we clean up generated documents when exit.
> * switched the `Dockerfile` to based on `centos:7` because the default
>   `doxygen` version in `ubuntu:14.04` doesn't include `jquery.js`
>   correctly when generate `doxygen` documents.
> * adjusted the `Dockerfile` and `README.md` locations to avoid we
>   distract at finding the website generation document.
> * updated `release-guide.md` with the new website generation and
>   development workflows.
> 
> 
> Diffs
> -
> 
>   docs/release-guide.md 36e79796657a37b0b4649602281556f4ae471c34 
>   site/.gitignore 987a5d3cc161a67de1fb21c2002ed9698bbb0689 
>   site/Gemfile 14c50340d66fd7a703949a7932ab93a6b60f4e07 
>   site/Gemfile.lock 065107ec25f0eff28e2a09051b61152d49f77f9a 
>   site/README.md 468021ef4f8ed942c4a16784bc82349af9ae0d5e 
>   site/Rakefile 845ec3fd8b9e47ebe49991db30718912d4f9c41e 
>   site/build.sh PRE-CREATION 
>   support/site-docker/Dockerfile 8ea3d5c5fe531ec76950ed0bf110d7d477339158 
>   support/site-docker/README.md 957117e1211f06b40c5fb621039dd90aff562a87 
> 
> Diff: https://reviews.apache.org/r/47695/diff/
> 
> 
> Testing
> ---
> 
> Tested in OS X and Ubuntu 14.04
> 
> For document changes, could verified from 
> https://github.com/haosdent/mesos/tree/MESOS-5431/site
> 
> To verify website works fine after this changes, could check it via this gif:
> 
> ![website.gif](https://issues.apache.org/jira/secure/attachment/12805684/website.gif)
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 47695: Updated the website generation and development workflows with docker.

[haosdent huang]

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

(Updated May 23, 2016, 5:49 p.m.)


Review request for mesos, Tomasz Janiszewski, Kevin Klues, Neil Conway, and 
Vinod Kone.


Changes
---

Address @vinodkone, @janisz's comments.


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


Repository: mesos


Description (updated)
---

In this changes, we include below items to make it more convenience to
develop and generate the website.

* added `doxygen` and `javadoc` to `rake` default target.
* added `build.sh` as the wrapper for the website generation to make
  sure we clean up generated documents when exit.
* switched the `Dockerfile` to based on `centos:7` because the default
  `doxygen` version in `ubuntu:14.04` doesn't include `jquery.js`
  correctly when generate `doxygen` documents.
* adjusted the `Dockerfile` and `README.md` locations to avoid we
  distract at finding the website generation document.
* updated `release-guide.md` with the new website generation and
  development workflows.


Diffs (updated)
-

  docs/release-guide.md 36e79796657a37b0b4649602281556f4ae471c34 
  site/.gitignore 987a5d3cc161a67de1fb21c2002ed9698bbb0689 
  site/Gemfile 14c50340d66fd7a703949a7932ab93a6b60f4e07 
  site/Gemfile.lock 065107ec25f0eff28e2a09051b61152d49f77f9a 
  site/README.md 468021ef4f8ed942c4a16784bc82349af9ae0d5e 
  site/Rakefile 845ec3fd8b9e47ebe49991db30718912d4f9c41e 
  site/build.sh PRE-CREATION 
  support/site-docker/Dockerfile 8ea3d5c5fe531ec76950ed0bf110d7d477339158 
  support/site-docker/README.md 957117e1211f06b40c5fb621039dd90aff562a87 

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


Testing (updated)
---

Tested in OS X and Ubuntu 14.04

For document changes, could verified from 
https://github.com/haosdent/mesos/tree/MESOS-5431/site

To verify website works fine after this changes, could check it via this gif:

![website.gif](https://issues.apache.org/jira/secure/attachment/12805684/website.gif)


Thanks,

haosdent huang

Review Request 47732: Fixed a race in long lived executor.

[Anand Mazumdar]

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

Review request for mesos and Vinod Kone.


Repository: mesos


Description
---

There is a possible race condition when the `TASK_FINISHED`
update calls into `update` when the executor process might
itself be already executing. Also after this fix, even if the 
executor is disconnected from the agent by the time the `update`
function is called, the updates would be sent again as part of 
unacknowledged updates on re-registration.


Diffs
-

  src/examples/long_lived_executor.cpp 94379b01c3ac8dbf513559789046677bceea688c 

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


Testing
---

make check

This is a bit hard to reproduce due to the random delay. e.g., this happened 
once in ~2-3 days on our internal cluster.

```
Backtrace:
./long-lived-executor(_ZNSt4listIN2id4UUIDESaIS1_EE5eraseESt14_List_iteratorIS1_E+0x14)[0x41ffec]
./long-lived-executor[0x41cf81]
./long-lived-executor[0x41ad3a]
./long-lived-executor[0x41f245]
./long-lived-executor[0x424622]
./long-lived-executor(_ZN7process11ProcessBase5serveERKNS_5EventE+0x2e)[0x419010]
/opt/mesosphere/lib/libmesos-0.29.0.so(_ZN7process14ProcessManager6resumeEPNS_11ProcessBaseE+0x2d1)[0x7fa3eba5cc21]
/opt/mesosphere/lib/libmesos-0.29.0.so(+0x13b1f27)[0x7fa3eba5cf27]
/lib64/libstdc++.so.6(+0xb5220)[0x7fa3e9f43220]
/lib64/libpthread.so.0(+0x7dc5)[0x7fa3ea19ddc5]
/lib64/libc.so.6(clone+0x6d)[0x7fa3e99ad28d]
```


Thanks,

Anand Mazumdar

Re: Review Request 47608: Add Labels from TaskInfo into TaskStatus message.

[Jojy Varghese]

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




src/tests/slave_tests.cpp (line 3265)


If this is a WIP, I would recommend to limit the reviewers list. Its 
currently `mesos` group.

If this review is intended for `mesos` group, I would recommend taking out 
debug statements like these. 

I would also recommend to use meaningful variable names (unlike L3318). 
(L3318 in particular, does not need the extra variable. See other similar code.)


- Jojy Varghese


On May 20, 2016, 6:38 p.m., Srinivas Brahmaroutu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47608/
> ---
> 
> (Updated May 20, 2016, 6:38 p.m.)
> 
> 
> Review request for mesos, Gilbert Song and Jojy Varghese.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> WIP: Add Labels from TaskInfo into TaskStatus message.
> 
> 
> Diffs
> -
> 
>   src/slave/slave.cpp ce0e7b1f1d17c3b82d835b0a6296ed7b1e9eeac1 
>   src/tests/hook_tests.cpp c6b4e8a50534e455cf44f427b09f74eef71177b2 
>   src/tests/slave_tests.cpp e1f5bfe074c357f46403887365b3f9ae554000b4 
> 
> Diff: https://reviews.apache.org/r/47608/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Srinivas Brahmaroutu
> 
>

Re: Review Request 47409: Libprocess: Implemented `HANDLE` versions of file descriptor functions.

[Alex Clemmer]

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

(Updated May 23, 2016, 5:04 p.m.)


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


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


Repository: mesos


Description
---

Libprocess: Implemented `HANDLE` versions of file descriptor functions.


Diffs (updated)
-

  3rdparty/libprocess/include/process/io.hpp 
2f1e626c0eec7c965ddb7acbc9cfe890a621afd3 
  3rdparty/libprocess/src/io.cpp 44dee23ec9fa958f8e047cf93b87a4031638ab5e 

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


Testing
---


Thanks,

Alex Clemmer

Re: Review Request 47389: Stout: Added support for correct path delimiters in Windows.

[Alex Clemmer]

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

(Updated May 23, 2016, 5:03 p.m.)


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


Repository: mesos


Description
---

Stout: Added support for correct path delimiters in Windows.


Diffs (updated)
-

  3rdparty/stout/include/stout/os/constants.hpp 
438840aaef189ef4fb457b856790efb3b6333a7d 
  3rdparty/stout/include/stout/os/mkdir.hpp 
e86dbfd2416fc2835ef7c6c55a20f00429f7b4c6 
  3rdparty/stout/include/stout/path.hpp 
ef538045a8b7a1e3d8962c869317d86a85e0259f 

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


Testing
---


Thanks,

Alex Clemmer

Re: Review Request 47719: Removed invalid gpu statistics columns in framework data tables.

[haosdent huang]

> On May 23, 2016, 4:16 p.m., Kevin Klues wrote:
> > I don't think we want to remove the headers.  Instead, we want to add 
> > column bodies with 0 values or "N/A" ass appropriate.
> 
> haosdent huang wrote:
> Got it, already updated. Let me upload the screenshots as well.

Just update the screenshots, @klueska, may you help review this again? Thank 
you in advance.


- haosdent


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


On May 23, 2016, 4:56 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47719/
> ---
> 
> (Updated May 23, 2016, 4:56 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Kevin Klues, and Vinod Kone.
> 
> 
> Bugs: MESOS-5157
> https://issues.apache.org/jira/browse/MESOS-5157
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Removed invalid gpu statistics columns in framework data tables.
> 
> 
> Diffs
> -
> 
>   src/webui/master/static/agent.html 2f40034cf89e5f92ace3043578881b7b7157e25f 
>   src/webui/master/static/agent_framework.html 
> 4d61c144ffbc51b9855e4b9f9cd99c0bbb1d35a5 
> 
> Diff: https://reviews.apache.org/r/47719/diff/
> 
> 
> Testing
> ---
> 
> # For apgent framework page:
> ## Before:
> ![](https://issues.apache.org/jira/secure/attachment/12805672/incorrect_agent_framework_page.png)
> ## After:
> ![](https://issues.apache.org/jira/secure/attachment/12805670/after_agent_framework_page.png)
> 
> # For agent page:
> ## Before:
> ![](https://issues.apache.org/jira/secure/attachment/12805673/incorrect_agent_page.png)
> ## After:
> ![](https://issues.apache.org/jira/secure/attachment/12805671/after_agent_page.png)
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 47594: Skipped the bind mount of CNI net info root dir if possible.

[Jie Yu]

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


Fix it, then Ship it!





src/slave/containerizer/mesos/isolators/network/cni/cni.cpp (lines 251 - 252)


Please align the error messages:
```
return Error(
"Cannot ..."
" ...");
```


- Jie Yu


On May 19, 2016, 2:31 p.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47594/
> ---
> 
> (Updated May 19, 2016, 2:31 p.m.)
> 
> 
> Review request for mesos, Avinash sridharan and Jie Yu.
> 
> 
> Bugs: MESOS-5413
> https://issues.apache.org/jira/browse/MESOS-5413
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Skipped the bind mount of CNI net info root dir if possible.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/network/cni/cni.cpp 
> dae369aadb940150aa806b28d9269e3d88cf57ed 
> 
> Diff: https://reviews.apache.org/r/47594/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Re: Review Request 47719: Removed invalid gpu statistics columns in framework data tables.

[haosdent huang]

> On May 23, 2016, 4:16 p.m., Kevin Klues wrote:
> > I don't think we want to remove the headers.  Instead, we want to add 
> > column bodies with 0 values or "N/A" ass appropriate.

Got it, already updated. Let me upload the screenshots as well.


- haosdent


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


On May 23, 2016, 4:40 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47719/
> ---
> 
> (Updated May 23, 2016, 4:40 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Kevin Klues, and Vinod Kone.
> 
> 
> Bugs: MESOS-5157
> https://issues.apache.org/jira/browse/MESOS-5157
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Removed invalid gpu statistics columns in framework data tables.
> 
> 
> Diffs
> -
> 
>   src/webui/master/static/agent.html 2f40034cf89e5f92ace3043578881b7b7157e25f 
>   src/webui/master/static/agent_framework.html 
> 4d61c144ffbc51b9855e4b9f9cd99c0bbb1d35a5 
> 
> Diff: https://reviews.apache.org/r/47719/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 47707: Refactored resource enumeration for containerizers and isolators.

[Kevin Klues]

> On May 23, 2016, 10:03 a.m., Guangya Liu wrote:
> > Did not go to detail for now, but post one early comment: It is suggested 
> > that every patch should be atomic, which means that you should merge your 
> > patch  https://reviews.apache.org/r/47708/ to this one to make sure the 
> > test will not be failed once this patch merged.

I always post them separately and most shepherds appreciate it so that there's 
not too much crammed into a single patch.


- Kevin


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


On May 23, 2016, 12:45 a.m., Kevin Klues wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47707/
> ---
> 
> (Updated May 23, 2016, 12:45 a.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Jie Yu.
> 
> 
> Bugs: MESOS-5256
> https://issues.apache.org/jira/browse/MESOS-5256
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Previously, the `Containerizer` class contained a static `resources()`
> function for enumerating all of the resources available on an agent.
> This made it hard to add new resources to this enumeration that were,
> for example, containerizer-specific or tied to a new isolator module
> loaded at runtime.
> 
> This commit refactors the resource enumeration logic to allow each
> containerizer to enumerate resources itself. It does this by adding a
> new virtual `resources()` function, whose default implementation
> includes the same logic as the old static version of `resources()`.
> Individual containerizers simply overwrite this function to do their
> own enumeration if they want to.
> 
> Similar logic exists to push resource enumeration down into isolators
> as well. As such, the logic for the Nvidia GPU resource enumeration has
> been moved down into the Nvidia GPU isolator instead of being
> hard-coded into the mesos containerizer itself.
> 
> 
> Diffs
> -
> 
>   include/mesos/slave/isolator.hpp 4be8c2bb409052e2e07138483408209384f41e23 
>   src/slave/containerizer/composing.hpp 
> f3eebd19bc9e6b3b8a969a2ad967b3e2909e0ee4 
>   src/slave/containerizer/composing.cpp 
> 15d059f0bbda4e8cb93c65c09327dde1e34d3e7b 
>   src/slave/containerizer/containerizer.hpp 
> ff78b4d0fd4a3b862f6019fc757c16b7367cd3cf 
>   src/slave/containerizer/containerizer.cpp 
> faa0c789dda8a6f36fdb6217b0bae270b6b8f2e2 
>   src/slave/containerizer/mesos/containerizer.hpp 
> a1a00020668f6da8d611f26e5637afffc87d09ba 
>   src/slave/containerizer/mesos/containerizer.cpp 
> 75e5a32a3e70ec60a6800e21a621673184ea0956 
>   src/slave/containerizer/mesos/isolator.hpp 
> bacd86af42d16cb7c9b6622dfb298dcaa7007b75 
>   src/slave/containerizer/mesos/isolator.cpp 
> 253ff3cea8aff3e7a3051fb5a763cc081f455f18 
>   src/slave/containerizer/mesos/isolators/cgroups/devices/gpus/nvidia.hpp 
> 502204650192d5ea44aa631eac8eb37e051843f0 
>   src/slave/containerizer/mesos/isolators/cgroups/devices/gpus/nvidia.cpp 
> 8f81cb79c10261670efc9eaa8614751854f53806 
>   src/slave/slave.cpp ce0e7b1f1d17c3b82d835b0a6296ed7b1e9eeac1 
> 
> Diff: https://reviews.apache.org/r/47707/diff/
> 
> 
> Testing
> ---
> 
> make -j check ( which fails on one test -- see 
> https://reviews.apache.org/r/47708/ )
> 
> 
> Thanks,
> 
> Kevin Klues
> 
>

Re: Review Request 44154: Added appc_simple_discovery_uri_prefix to configuration.md.

[Jie Yu]

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


Ship it!




Ship It!

- Jie Yu


On May 23, 2016, 9:41 a.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44154/
> ---
> 
> (Updated May 23, 2016, 9:41 a.m.)
> 
> 
> Review request for mesos, Gilbert Song, Jie Yu, Jojy Varghese, and Vinod Kone.
> 
> 
> Bugs: MESOS-5437
> https://issues.apache.org/jira/browse/MESOS-5437
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added appc_simple_discovery_uri_prefix to configuration.md.
> 
> 
> Diffs
> -
> 
>   docs/configuration.md dd66d538ab3df6dd75d47bd4240e65f03a8fc763 
>   src/slave/flags.cpp 2d59329997197966f7d30d6f1375a988edbceb9c 
> 
> Diff: https://reviews.apache.org/r/44154/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

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

[Bernd Mathiske]

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


Ship it!




Ship It!

- Bernd Mathiske


On May 18, 2016, 1:06 a.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47522/
> ---
> 
> (Updated May 18, 2016, 1:06 a.m.)
> 
> 
> Review request for mesos, Bernd Mathiske, Joerg Schad, and Neil Conway.
> 
> 
> Bugs: MESOS-5005
> https://issues.apache.org/jira/browse/MESOS-5005
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch adds a creator principal to the
> persistent volumes used in the PersistentVolumeTests.
> 
> 
> Diffs
> -
> 
>   src/tests/persistent_volume_tests.cpp 
> d246f35046fff469b847c908de2b305ae629212f 
> 
> Diff: https://reviews.apache.org/r/47522/diff/
> 
> 
> Testing
> ---
> 
> `make check` was used to test on OSX at the end of this review chain.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 47520: Updated test helpers with creator principal.

[Bernd Mathiske]

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


Ship it!




Ship It!

- Bernd Mathiske


On May 18, 2016, 1:04 a.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47520/
> ---
> 
> (Updated May 18, 2016, 1:04 a.m.)
> 
> 
> Review request for mesos, Bernd Mathiske, Joerg Schad, and Neil Conway.
> 
> 
> Bugs: MESOS-5005
> https://issues.apache.org/jira/browse/MESOS-5005
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch updates the `createPersistentVolume` and
> `createDiskInfo` test helper functions to accept an
> argument specifying the creator principal to be included
> in `DiskInfo.Persistence`.
> 
> 
> Diffs
> -
> 
>   src/tests/mesos.hpp 79bf1ff16412ce2a510a9b75ab1ac91c1c182653 
> 
> Diff: https://reviews.apache.org/r/47520/diff/
> 
> 
> Testing
> ---
> 
> `make check` was done on OSX at the end of this review chain.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

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

[Bernd Mathiske]

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


Ship it!




Ship It!

- Bernd Mathiske


On May 18, 2016, 1:04 a.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47519/
> ---
> 
> (Updated May 18, 2016, 1:04 a.m.)
> 
> 
> Review request for mesos, Bernd Mathiske, Joerg Schad, and Neil Conway.
> 
> 
> Bugs: MESOS-5005
> https://issues.apache.org/jira/browse/MESOS-5005
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch adds the creator principal into the
> persistent volumes created by the persistent volume
> example framework.
> 
> 
> Diffs
> -
> 
>   src/examples/persistent_volume_framework.cpp 
> 53a6f6f82101ebb75abdc8f586fb23ab13298979 
> 
> Diff: https://reviews.apache.org/r/47519/diff/
> 
> 
> Testing
> ---
> 
> Tested manually with the following commands:
> `bin/mesos-master.sh --ip=127.0.0.1 --work_dir="$HOME/var/mesos1" 
> --authenticate=false`
> `bin/mesos-slave.sh --master=127.0.0.1:5050 --work_dir="$HOME/var/mesos2" 
> --"resources=disk(test):2048;mem(test):2048;cpu(test):4"`
> `src/persistent-volume-framework --master=127.0.0.1:5050`
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 47528: Updated validation tests with creator principal.

[Bernd Mathiske]

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




src/tests/master_validation_tests.cpp (line 451)


Please add a test description for every new test.



src/tests/master_validation_tests.cpp (line 454)


Where are we setting the principal for DiskInfo.Persistence?

How is this block of code any different from the one below?


- Bernd Mathiske


On May 18, 2016, 1:02 a.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47528/
> ---
> 
> (Updated May 18, 2016, 1:02 a.m.)
> 
> 
> Review request for mesos, Bernd Mathiske, Joerg Schad, and Neil Conway.
> 
> 
> Bugs: MESOS-5005
> https://issues.apache.org/jira/browse/MESOS-5005
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The master validation tests are updated to include the
> new `validate()` signature, and a new test is added,
> `CreateOperationValidationTest.NonMatchingPrincipal`,
> to ensure that a non-matching creator principal will
> be invalidated.
> 
> 
> Diffs
> -
> 
>   src/tests/master_validation_tests.cpp 
> ca4442aa1ef0087a7d058d1b3aa430a1dbc16960 
> 
> Diff: https://reviews.apache.org/r/47528/diff/
> 
> 
> Testing
> ---
> 
> `make check` was used to test on OSX at the end of this review chain.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

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

[Bernd Mathiske]

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


Ship it!




Ship It!

- Bernd Mathiske


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

Re: Review Request 47123: Added --modules_dir flag to read module manifests from a directory.

[Till Toenshoff]

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



Looks good Kapil!

I would like to have tests and documentation updates as well before feeling 
comfortable to commit this - at least as [WIP] added here in a chain.


src/master/flags.cpp (line 407)


Could you please add a similar comment as you did for the `modules` flag 
above, hinting that we need to keep stuff in sync?

Same for all other references of that flag / help messages?


- Till Toenshoff


On May 20, 2016, 2:40 a.m., Kapil Arya wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47123/
> ---
> 
> (Updated May 20, 2016, 2:40 a.m.)
> 
> 
> Review request for mesos, Cody Maloney and Till Toenshoff.
> 
> 
> Bugs: MESOS-5173
> https://issues.apache.org/jira/browse/MESOS-5173
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This allows the operator to use separate manifest JSON files for each
> module.  Previously, one had to merge all module manifest files into a
> single JSON file before passing on to the master/agent.
> 
> 
> Diffs
> -
> 
>   src/master/flags.hpp d08509667c919e9cfd076762b622c18732edf910 
>   src/master/flags.cpp ceb4bd4f863d3857eede8287041a53d66f1c9e4c 
>   src/master/main.cpp d00bbef2c4fb74544056ab81aeb4fcd6625b89fa 
>   src/module/manager.hpp 9944af0daf6c9cb5a8ff338099401b1db88ee237 
>   src/module/manager.cpp 9f88ec3addab59e4a40b0b40612518178d535aa5 
>   src/sched/flags.hpp b4ca12b667283cee1f96a4b421fcf3b06bbe59d7 
>   src/sched/sched.cpp 9e55885704d5c4a8bc0e25e324b9c65e7bc34798 
>   src/slave/flags.hpp 3363412099ca2841e175bd6b6ed3d5f13605e6f0 
>   src/slave/flags.cpp 2d59329997197966f7d30d6f1375a988edbceb9c 
>   src/slave/main.cpp 13ddfb9ae5a1ca240b738e67391ce5c5fc9ac0b6 
>   src/tests/flags.hpp ae232b1a087edfaf678bd1c67bc509efd6c740d8 
>   src/tests/main.cpp efec92286c78f7783e1de0781eb9e61c202a4fb8 
> 
> Diff: https://reviews.apache.org/r/47123/diff/
> 
> 
> Testing
> ---
> 
> Manual testing with:
> 1. --module_dir
> 2. --modules
> 3. --module_dir in conjunction with --modules
> 
> The first two succeeded while the third failed as expected.
> 
> 
> Thanks,
> 
> Kapil Arya
> 
>

Re: Review Request 44154: Added appc_simple_discovery_uri_prefix to configuration.md.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [44154]

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

- Mesos ReviewBot


On May 23, 2016, 9:41 a.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44154/
> ---
> 
> (Updated May 23, 2016, 9:41 a.m.)
> 
> 
> Review request for mesos, Gilbert Song, Jie Yu, Jojy Varghese, and Vinod Kone.
> 
> 
> Bugs: MESOS-5437
> https://issues.apache.org/jira/browse/MESOS-5437
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added appc_simple_discovery_uri_prefix to configuration.md.
> 
> 
> Diffs
> -
> 
>   docs/configuration.md dd66d538ab3df6dd75d47bd4240e65f03a8fc763 
>   src/slave/flags.cpp 2d59329997197966f7d30d6f1375a988edbceb9c 
> 
> Diff: https://reviews.apache.org/r/44154/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 47411: Agent: Changed the names of symbols that are ambiguous on MSVC.

[Alex Clemmer]

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

(Updated May 23, 2016, 12:39 p.m.)


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


Repository: mesos


Description
---

Agent: Changed the names of symbols that are ambiguous on MSVC.


Diffs (updated)
-

  src/docker/docker.hpp da6e9a25b99079940958001128ee949cdb9b931b 
  src/slave/slave.cpp feb9f291b4188aac1a63ab9cfae8349a048663b1 

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


Testing
---


Thanks,

Alex Clemmer