Re: Review Request 42362: Added persistent volume endpoint test without authentication.

[Greg Mann]

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

(Updated Jan. 28, 2016, 4:43 p.m.)


Review request for mesos, Alexander Rukletsov, Kapil Arya, and Neil Conway.


Changes
---

Addressed comments.


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


Repository: mesos


Description
---

Added persistent volume endpoint tests with HTTP authentication disabled.

The persistent volume endpoint tests allow volume creation and destruction when 
HTTP authentication is disabled; this patch introduces a test for this 
scenario: `PersistentVolumeEndpointsTest.NoAuthentication`.


Diffs (updated)
-

  src/tests/persistent_volume_endpoints_tests.cpp 
22e18758ee91a649486725473d9e50fae9d43b01 

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


Testing
---

A new test, `PersistentVolumeEndpointsTest.NoAuthentication`, was added to the 
persistent volume endpoint tests.

`make check` was used to test, and the new test was run with 
`--gtest_repeat=1000 -gtest_break_on_failure=1`.


Thanks,

Greg Mann

Re: Review Request 42914: Listed supported file extensions in fetcher documentation.

[haosdent huang]

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




docs/fetcher.md (line 122)


do you also support `.gz`?


- haosdent huang


On Jan. 28, 2016, 4:29 p.m., Bernd Mathiske wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42914/
> ---
> 
> (Updated Jan. 28, 2016, 4:29 p.m.)
> 
> 
> Review request for mesos, Jie Yu, Jan Schlicht, and Till Toenshoff.
> 
> 
> Bugs: MESOS-4336
> https://issues.apache.org/jira/browse/MESOS-4336
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Edited fetcher.md, listing the supported file extensions when extracting an 
> archive.
> 
> 
> Diffs
> -
> 
>   docs/fetcher.md cb4f3c33fb67f7ac65dbe7ebe1347d2599c43f37 
> 
> Diff: https://reviews.apache.org/r/42914/diff/
> 
> 
> Testing
> ---
> 
> Opened file with MD renderer.
> 
> 
> Thanks,
> 
> Bernd Mathiske
> 
>

Re: Review Request 42773: Added utility function to compute SHA512 digest.

[Jie Yu]

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


Fix it, then Ship it!





src/common/command_utils.hpp (line 65)


can you s/sha512/shasum/ and add an algorithm parameter (type string).

we can probably create sha512 method still, but will invoke the underlying 
shasum method.



src/common/command_utils.hpp (lines 65 - 66)


s/filePath/file/

I mentioned this in a couple of reviews. Please don't forget next time:)



src/common/command_utils.cpp (line 20)


Why do you need this?



src/common/command_utils.cpp (lines 30 - 31)


Is it used?



src/common/command_utils.cpp (line 173)


please make sure the parameter name is consistent with that in the header!



src/common/command_utils.cpp (line 177)


First letter capitalized



src/common/command_utils.cpp (line 178)


First letter capitalized.



src/common/command_utils.cpp (line 187)


s/output//

from shasum command



src/common/command_utils.cpp (lines 193 - 194)


Ditto.



src/tests/common/command_utils_tests.cpp (line 226)


s/DigestTest/ShasumTest/



src/tests/common/command_utils_tests.cpp (line 231)


do you need this temp var?



src/tests/common/command_utils_tests.cpp (line 237)


s/sha512Future/sha512/



src/tests/common/command_utils_tests.cpp (lines 242 - 243)


Please don't split this string. You can use
```
// NOLINT(whitespace/line_length)
```

in the end to silence style checker.


- Jie Yu


On Jan. 27, 2016, 10:29 p.m., Jojy Varghese wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42773/
> ---
> 
> (Updated Jan. 27, 2016, 10:29 p.m.)
> 
> 
> Review request for mesos.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added utility function to compute SHA512 digest.
> 
> 
> Diffs
> -
> 
>   src/common/command_utils.hpp PRE-CREATION 
>   src/common/command_utils.cpp PRE-CREATION 
>   src/tests/common/command_utils_tests.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/42773/diff/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Jojy Varghese
> 
>

Review Request 42914: Listed supported file extensions in fetcher documentation.

[Bernd Mathiske]

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

Review request for mesos, Jan Schlicht and Till Toenshoff.


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


Repository: mesos


Description
---

Edited fetcher.md, listing the supported file extensions when extracting an 
archive.


Diffs
-

  docs/fetcher.md cb4f3c33fb67f7ac65dbe7ebe1347d2599c43f37 

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


Testing
---

Opened file with MD renderer.


Thanks,

Bernd Mathiske

Re: Review Request 42911: Removed extra blank line.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [42908, 42910, 42633, 42636, 42657, 42658, 42672, 41950, 42911]

Passed command: export OS=ubuntu:14.04;export CONFIGURATION="--verbose";export 
COMPILER=gcc; ./support/docker_build.sh

- Mesos ReviewBot


On Jan. 28, 2016, 1:13 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42911/
> ---
> 
> (Updated Jan. 28, 2016, 1:13 p.m.)
> 
> 
> Review request for mesos and Joris Van Remoortere.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Removed extra blank line.
> 
> 
> Diffs
> -
> 
>   src/master/quota.cpp e7ce0f1fc1c83b2ec30185db005d2671a93baed6 
> 
> Diff: https://reviews.apache.org/r/42911/diff/
> 
> 
> Testing
> ---
> 
> None
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 42362: Added persistent volume endpoint test without authentication.

[Greg Mann]

> On Jan. 28, 2016, 11:05 a.m., Alexander Rukletsov wrote:
> >

Thanks Alex! :-)


> On Jan. 28, 2016, 11:05 a.m., Alexander Rukletsov wrote:
> > src/tests/persistent_volume_endpoints_tests.cpp, line 1087
> > 
> >
> > Does it make sense to add `masterFlags.credentials = None();`?

I don't think it makes sense in this case. Setting the `authenticate_http` flag 
to false will disable authentication regardless of whether or not we explicitly 
set the credentials to `None()`.


- Greg


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


On Jan. 28, 2016, 4:43 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42362/
> ---
> 
> (Updated Jan. 28, 2016, 4:43 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Kapil Arya, and Neil Conway.
> 
> 
> Bugs: MESOS-4395
> https://issues.apache.org/jira/browse/MESOS-4395
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added persistent volume endpoint tests with HTTP authentication disabled.
> 
> The persistent volume endpoint tests allow volume creation and destruction 
> when HTTP authentication is disabled; this patch introduces a test for this 
> scenario: `PersistentVolumeEndpointsTest.NoAuthentication`.
> 
> 
> Diffs
> -
> 
>   src/tests/persistent_volume_endpoints_tests.cpp 
> 22e18758ee91a649486725473d9e50fae9d43b01 
> 
> Diff: https://reviews.apache.org/r/42362/diff/
> 
> 
> Testing
> ---
> 
> A new test, `PersistentVolumeEndpointsTest.NoAuthentication`, was added to 
> the persistent volume endpoint tests.
> 
> `make check` was used to test, and the new test was run with 
> `--gtest_repeat=1000 -gtest_break_on_failure=1`.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 42914: Listed supported file extensions in fetcher documentation.

[haosdent huang]

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




docs/fetcher.md (line 122)


seems miss `gz`, `zip`.


- haosdent huang


On Jan. 28, 2016, 4:29 p.m., Bernd Mathiske wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42914/
> ---
> 
> (Updated Jan. 28, 2016, 4:29 p.m.)
> 
> 
> Review request for mesos, Jie Yu, Jan Schlicht, and Till Toenshoff.
> 
> 
> Bugs: MESOS-4336
> https://issues.apache.org/jira/browse/MESOS-4336
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Edited fetcher.md, listing the supported file extensions when extracting an 
> archive.
> 
> 
> Diffs
> -
> 
>   docs/fetcher.md cb4f3c33fb67f7ac65dbe7ebe1347d2599c43f37 
> 
> Diff: https://reviews.apache.org/r/42914/diff/
> 
> 
> Testing
> ---
> 
> Opened file with MD renderer.
> 
> 
> Thanks,
> 
> Bernd Mathiske
> 
>

Re: Review Request 42810: Added the CgroupInfo protobuf. The agent can use this message to reflect any cgroup configuration that might have been applied to a container.

[Avinash sridharan]

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

(Updated Jan. 28, 2016, 5:31 p.m.)


Review request for mesos and Jie Yu.


Summary (updated)
-

Added the CgroupInfo protobuf. The agent can use this message to reflect any 
cgroup configuration that might have been applied to a container.


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


Repository: mesos


Description
---

Added the CgroupInfo protobuf. The agent can use this message to reflect any 
cgroup configuration that might have been applied to a container.


Diffs (updated)
-

  include/mesos/mesos.proto 96b911fb370223933df52f9370897871827d2247 
  include/mesos/v1/mesos.proto 0501dfa27ed610666226953591a902eac4c295f8 

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


Testing
---

make


Thanks,

Avinash sridharan

Re: Review Request 42936: Edited flag help strings for style.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [42936]

Passed command: export OS=ubuntu:14.04;export CONFIGURATION="--verbose";export 
COMPILER=gcc; ./support/docker_build.sh

- Mesos ReviewBot


On Jan. 29, 2016, 5:34 a.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42936/
> ---
> 
> (Updated Jan. 29, 2016, 5:34 a.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-4298
> https://issues.apache.org/jira/browse/MESOS-4298
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Edited flag help strings for style.
> 
> Backticks were inserted where appropriate. This not only matches the 
> convention in the comments within our code, helps to enable the automatic 
> generation of markdown-formatted documentation from these help strings.
> 
> 
> Diffs
> -
> 
>   src/logging/flags.cpp 978d735c8c8e9f3c46669cc633773f1ec1e1725d 
>   src/master/flags.cpp 6e7e17650341bc17c3af6f92fe83f974d4ce1efd 
>   src/master/main.cpp 0d5ac49121491152d8426cd764a1f1a0f37483ae 
>   src/slave/flags.cpp 75d7429a4e3e1d6259296257c0ace1ade365ac2b 
>   src/slave/main.cpp 9d48a0823189ea6505073a2803f02d90dc382ab4 
> 
> Diff: https://reviews.apache.org/r/42936/diff/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 41892: DockerContinerizer infers hostPath for persistent volumes.

[Guangya Liu]

> On Jan. 15, 2016, 12:03 a.m., Zhitao Li wrote:
> > src/docker/docker.cpp, lines 410-420
> > 
> >
> > (Sorry I just got time to come back to this).
> > 
> > I don't exactly understand your suggestion about "add additional 
> > containerInfo.volumes() in 
> > DockerContainerizerProcess::Container::create()", because the latter 
> > function actually does not pass a `ContainerInfo`.
> > 
> > Because the volume is actually included in 
> > `TaskInfo::ContainerInfo::volumes`, we would need to mutate the `TaskInfo` 
> > before passing it to the `DockerExecutorProcess::launchTask()` function, 
> > which I don't really know how to do it, and I even doubt whether it's a 
> > good idea for logging/clarity purpose.
> > 
> > Please let me know if I didn't understand your suggestion, or if you 
> > think we should explore the other alternative (passing `hostPath` earlier 
> > in resource offer).
> 
> Jie Yu wrote:
> Sorry for being late on this.
> 
> My sugguestion is that we add some logics in 
> DockerContainerProcess::Container::create(), the 'create' function has both 
> TaskINfo and ExecutorInfo. So, you should be able to get the ContainerInfo 
> from them. When you generate DockerContainerizerProcess::Container, you 
> should be able to generate a new ContainerInfo that has persistent volumes in 
> it. Let me know if you still don't understand.
> 
> FYI, docker just merged its mount propagation support for volumes. That 
> means we can have full persistent volume support. We need to mount the 
> sandbox as a shared mount into the docker container. ANy mounts under the 
> sandbox mount will be automatically propergated to the docker container.
> 
> Zhitao Li wrote:
> @jieyu, I tried your recommendation and managed to generate a different 
> `ContainerInfo` object inside `DockerContainerProcess::Container::create()`. 
> However, I don't think it could work either because the actual docker 
> container is created by the call sequence `DockerExecutor::launchTask()` -> 
> `Docker::run()` in the forked executor subprocess, and it seems impossible to 
> properly pass the modified `ContainerInfo` to that.
> 
> My proposal is:
> - 1) keep the `persistent_volumes_root` flag to the executor;
> - 2) implement this logic inside `DockerExecutor::launchTask()` (see my 
> other inline comment for exact location) so we don't need to change `Docker` 
> class;
> - 3) for testing, create a test case in docker_containerizer_tests.cpp 
> which creates persistent volume and launch an executor.
> 
> I'll rebase and update this diff with the above proposal.
> 
> Guangya Liu wrote:
> The `DockerContainerProcess::Container::create()` only include `TaskInfo` 
> when using a `command executor`, otherwise, the 
> `DockerContainerProcess::Container::create()` will not include task info. For 
> this case it is not `command executor`, so I think that @Zhitao's solution 
> should be the right choice. Please correct me if there's sth wrong, thanks.
> 
> Zhitao Li wrote:
> Hi Guangya,
> 
> I had some offline chat with Jie about this issue and agreed that this 
> should instead be fixed with a more involved solution on a different 
> direction (mounting persistent volume inside sandbox as a "shared volume"). 
> The bug has been reassigned to Timothy Chen who's implementing on that.
> 
> I'll abandon this review for now.

@Zhitao, can you please append some of your dicussion here? Want to get some 
detail for this, thanks.


- Guangya


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


On Jan. 4, 2016, 9:15 p.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41892/
> ---
> 
> (Updated Jan. 4, 2016, 9:15 p.m.)
> 
> 
> Review request for mesos, haosdent huang and Jie Yu.
> 
> 
> Bugs: MESOS-3413
> https://issues.apache.org/jira/browse/MESOS-3413
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This diff makes DockerContainerizer infers hostPath for persistent volumes 
> from resources passed-in.
> 
> This allows current DockerContainerizer to use persistent volumes w/o hard 
> code slave's work_dir. I also checked that the inferred directory actually 
> exists to avoid framework messing up role or persistent id.
> 
> Note that some validation on the resource part should be done before we start 
> the container, which seems to belong to slave.
> 
> 
> Diffs
> -
> 
>   src/docker/docker.hpp dde2b29deda7e40929e0169935a5bafdd43136b1 
>   src/docker/docker.cpp 

Re: Review Request 42946: Replace tabs with spaces in configure.ac.

[haosdent huang]

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


Ship it!




Ship It!

- haosdent huang


On Jan. 29, 2016, 2:55 a.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42946/
> ---
> 
> (Updated Jan. 29, 2016, 2:55 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and Joris Van Remoortere.
> 
> 
> Bugs: MESOS-4519
> https://issues.apache.org/jira/browse/MESOS-4519
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Replace tabs with spaces in configure.ac.
> 
> 
> Diffs
> -
> 
>   configure.ac cb39c7f 
> 
> Diff: https://reviews.apache.org/r/42946/diff/
> 
> 
> Testing
> ---
> 
> ../configure
> make
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>

Re: Review Request 42914: Listed supported file extensions in fetcher documentation.

[Bernd Mathiske]

> On Jan. 28, 2016, 11:40 p.m., Timothy Chen wrote:
> > docs/fetcher.md, line 119
> > 
> >
> > then files with a recognized extension
> 
> Bernd Mathiske wrote:
> Soryy, I do not understand what you mean.
> 
> Timothy Chen wrote:
> Sorry, I meant:
> 
> s/extensions that hint/a recognized extension that hint/g

Got it, will fix. Thanks!


- Bernd


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


On Jan. 28, 2016, 10:17 a.m., Bernd Mathiske wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42914/
> ---
> 
> (Updated Jan. 28, 2016, 10:17 a.m.)
> 
> 
> Review request for mesos, Adam B, Benjamin Hindman, Till Toenshoff, and 
> Timothy Chen.
> 
> 
> Bugs: MESOS-4336
> https://issues.apache.org/jira/browse/MESOS-4336
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Edited fetcher.md, listing the supported file extensions when extracting an 
> archive.
> 
> 
> Diffs
> -
> 
>   docs/fetcher.md cb4f3c33fb67f7ac65dbe7ebe1347d2599c43f37 
> 
> Diff: https://reviews.apache.org/r/42914/diff/
> 
> 
> Testing
> ---
> 
> Opened file with MD renderer.
> 
> 
> Thanks,
> 
> Bernd Mathiske
> 
>

Re: Review Request 42927: Deleted Dockerfile in favor of `support/docker_build.sh`.

[Guangya Liu]

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


Ship it!




Ship It!

- Guangya Liu


On Jan. 29, 2016, 12:05 a.m., Vinod Kone wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42927/
> ---
> 
> (Updated Jan. 29, 2016, 12:05 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Greg Mann, and Kevin Klues.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> No one seems to be using it and this is not maintained anymore.
> 
> 
> Diffs
> -
> 
>   Dockerfile 35abf25aa77ef3f8a68b803eb8ff18ad90a0ff7b 
> 
> Diff: https://reviews.apache.org/r/42927/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Vinod Kone
> 
>

Re: Review Request 42946: Replace tabs with spaces in configure.ac.

[Benjamin Bannier]

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


Ship it!




Commit message should probably be `s/Replace/Replaced/`.

In addition to building fine we now have

$ egrep '   ' configure.ac | wc -l
0

- Benjamin Bannier


On Jan. 29, 2016, 3:55 a.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42946/
> ---
> 
> (Updated Jan. 29, 2016, 3:55 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and Joris Van Remoortere.
> 
> 
> Bugs: MESOS-4519
> https://issues.apache.org/jira/browse/MESOS-4519
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Replace tabs with spaces in configure.ac.
> 
> 
> Diffs
> -
> 
>   configure.ac cb39c7f 
> 
> Diff: https://reviews.apache.org/r/42946/diff/
> 
> 
> Testing
> ---
> 
> ../configure
> make
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>

Re: Review Request 42517: Added discussion about allowing multiple frameworks in a role.

[Joris Van Remoortere]

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


Ship it!




Ship It!

- Joris Van Remoortere


On Jan. 20, 2016, 9:57 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42517/
> ---
> 
> (Updated Jan. 20, 2016, 9:57 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Joris Van Remoortere.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added discussion about allowing multiple frameworks in a role.
> 
> 
> Diffs
> -
> 
>   docs/persistent-volume.md 4af7d6e8dc648fb630f56db5fbad1b7b438ebcfb 
>   docs/roles.md 609a63cbff2d9c652af45ba16152ce3caf48 
> 
> Diff: https://reviews.apache.org/r/42517/diff/
> 
> 
> Testing
> ---
> 
> Previewed on github.
> 
> Note that the link to `roles.md` doesn't work at the moment, but I believe it 
> should work once Joerg's fix for the `Rakefile` is merged.
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 42914: Listed supported file extensions in fetcher documentation.

[Timothy Chen]

> On Jan. 29, 2016, 7:40 a.m., Timothy Chen wrote:
> > docs/fetcher.md, line 119
> > 
> >
> > then files with a recognized extension
> 
> Bernd Mathiske wrote:
> Soryy, I do not understand what you mean.

Sorry, I meant:

s/extensions that hint/a recognized extension that hint/g


- Timothy


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


On Jan. 28, 2016, 6:17 p.m., Bernd Mathiske wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42914/
> ---
> 
> (Updated Jan. 28, 2016, 6:17 p.m.)
> 
> 
> Review request for mesos, Adam B, Benjamin Hindman, Till Toenshoff, and 
> Timothy Chen.
> 
> 
> Bugs: MESOS-4336
> https://issues.apache.org/jira/browse/MESOS-4336
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Edited fetcher.md, listing the supported file extensions when extracting an 
> archive.
> 
> 
> Diffs
> -
> 
>   docs/fetcher.md cb4f3c33fb67f7ac65dbe7ebe1347d2599c43f37 
> 
> Diff: https://reviews.apache.org/r/42914/diff/
> 
> 
> Testing
> ---
> 
> Opened file with MD renderer.
> 
> 
> Thanks,
> 
> Bernd Mathiske
> 
>

Re: Review Request 42914: Listed supported file extensions in fetcher documentation.

[Timothy Chen]

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


Fix it, then Ship it!




Ship It!


docs/fetcher.md (line 119)


then files with a recognized extension


- Timothy Chen


On Jan. 28, 2016, 6:17 p.m., Bernd Mathiske wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42914/
> ---
> 
> (Updated Jan. 28, 2016, 6:17 p.m.)
> 
> 
> Review request for mesos, Adam B, Benjamin Hindman, Till Toenshoff, and 
> Timothy Chen.
> 
> 
> Bugs: MESOS-4336
> https://issues.apache.org/jira/browse/MESOS-4336
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Edited fetcher.md, listing the supported file extensions when extracting an 
> archive.
> 
> 
> Diffs
> -
> 
>   docs/fetcher.md cb4f3c33fb67f7ac65dbe7ebe1347d2599c43f37 
> 
> Diff: https://reviews.apache.org/r/42914/diff/
> 
> 
> Testing
> ---
> 
> Opened file with MD renderer.
> 
> 
> Thanks,
> 
> Bernd Mathiske
> 
>

Re: Review Request 42914: Listed supported file extensions in fetcher documentation.

[Guangya Liu]

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


Ship it!




Ship It!

- Guangya Liu


On Jan. 28, 2016, 6:17 p.m., Bernd Mathiske wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42914/
> ---
> 
> (Updated Jan. 28, 2016, 6:17 p.m.)
> 
> 
> Review request for mesos, Adam B, Benjamin Hindman, Till Toenshoff, and 
> Timothy Chen.
> 
> 
> Bugs: MESOS-4336
> https://issues.apache.org/jira/browse/MESOS-4336
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Edited fetcher.md, listing the supported file extensions when extracting an 
> archive.
> 
> 
> Diffs
> -
> 
>   docs/fetcher.md cb4f3c33fb67f7ac65dbe7ebe1347d2599c43f37 
> 
> Diff: https://reviews.apache.org/r/42914/diff/
> 
> 
> Testing
> ---
> 
> Opened file with MD renderer.
> 
> 
> Thanks,
> 
> Bernd Mathiske
> 
>

Re: Review Request 42914: Listed supported file extensions in fetcher documentation.

[Bernd Mathiske]

> On Jan. 28, 2016, 11:40 p.m., Timothy Chen wrote:
> > docs/fetcher.md, line 119
> > 
> >
> > then files with a recognized extension

Soryy, I do not understand what you mean.


- Bernd


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


On Jan. 28, 2016, 10:17 a.m., Bernd Mathiske wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42914/
> ---
> 
> (Updated Jan. 28, 2016, 10:17 a.m.)
> 
> 
> Review request for mesos, Adam B, Benjamin Hindman, Till Toenshoff, and 
> Timothy Chen.
> 
> 
> Bugs: MESOS-4336
> https://issues.apache.org/jira/browse/MESOS-4336
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Edited fetcher.md, listing the supported file extensions when extracting an 
> archive.
> 
> 
> Diffs
> -
> 
>   docs/fetcher.md cb4f3c33fb67f7ac65dbe7ebe1347d2599c43f37 
> 
> Diff: https://reviews.apache.org/r/42914/diff/
> 
> 
> Testing
> ---
> 
> Opened file with MD renderer.
> 
> 
> Thanks,
> 
> Bernd Mathiske
> 
>

Re: Review Request 42362: Added persistent volume endpoint test without authentication.

[Greg Mann]

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

(Updated Jan. 28, 2016, 5:59 p.m.)


Review request for mesos, Alexander Rukletsov, Kapil Arya, and Neil Conway.


Changes
---

Added new comments.


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


Repository: mesos


Description
---

Added persistent volume endpoint tests with HTTP authentication disabled.

The persistent volume endpoint tests allow volume creation and destruction when 
HTTP authentication is disabled; this patch introduces a test for this 
scenario: `PersistentVolumeEndpointsTest.NoAuthentication`.


Diffs (updated)
-

  src/tests/persistent_volume_endpoints_tests.cpp 
22e18758ee91a649486725473d9e50fae9d43b01 

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


Testing
---

A new test, `PersistentVolumeEndpointsTest.NoAuthentication`, was added to the 
persistent volume endpoint tests.

`make check` was used to test, and the new test was run with 
`--gtest_repeat=1000 -gtest_break_on_failure=1`.


Thanks,

Greg Mann

Re: Review Request 42832: Added a status method to the Isolator interface.

[Jie Yu]

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


Ship it!




Ship It!

- Jie Yu


On Jan. 28, 2016, 5:57 p.m., Avinash sridharan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42832/
> ---
> 
> (Updated Jan. 28, 2016, 5:57 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Kapil Arya.
> 
> 
> Bugs: MESOS-4520
> https://issues.apache.org/jira/browse/MESOS-4520
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added a status method to the Isolator interface. This method can be used to 
> query the run-time state of isolator specific properties associated with a 
> container.
> 
> 
> Diffs
> -
> 
>   include/mesos/slave/isolator.hpp 2ea1fb77b4bbe811466fac18fdf7cd48d482bc75 
>   src/slave/containerizer/mesos/isolator.hpp 
> b3babd09c9dc70d6ca3dac1964eda321cb0f8be9 
>   src/slave/containerizer/mesos/isolator.cpp 
> 6d6863814fde5601bd311f6dc0816f7f1e446537 
> 
> Diff: https://reviews.apache.org/r/42832/diff/
> 
> 
> Testing
> ---
> 
> make
> 
> 
> Thanks,
> 
> Avinash sridharan
> 
>

Re: Review Request 42810: Added the CgroupInfo protobuf.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [42810]

Passed command: export OS=ubuntu:14.04;export CONFIGURATION="--verbose";export 
COMPILER=gcc; ./support/docker_build.sh

- Mesos ReviewBot


On Jan. 28, 2016, 5:32 p.m., Avinash sridharan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42810/
> ---
> 
> (Updated Jan. 28, 2016, 5:32 p.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-4488
> https://issues.apache.org/jira/browse/MESOS-4488
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added the CgroupInfo protobuf. The agent can use this message to reflect any 
> cgroup configuration that might have been applied to a container.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 96b911fb370223933df52f9370897871827d2247 
>   include/mesos/v1/mesos.proto 0501dfa27ed610666226953591a902eac4c295f8 
> 
> Diff: https://reviews.apache.org/r/42810/diff/
> 
> 
> Testing
> ---
> 
> make
> 
> 
> Thanks,
> 
> Avinash sridharan
> 
>

Re: Review Request 42841: WIP: Introducing appc image fetcher.

[Jojy Varghese]

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

(Updated Jan. 28, 2016, 7:10 p.m.)


Review request for Jie Yu.


Changes
---

rebased.


Repository: mesos


Description
---

WIP: Introducing appc image fetcher.


Diffs (updated)
-

  src/CMakeLists.txt 93caf000f39bbb71a17d9876689afb50cf637e44 
  src/Makefile.am fac17f4bac3b2ddda0384dec8b0ed6f960bd24d1 
  src/slave/containerizer/mesos/provisioner/appc/image_fetcher.hpp PRE-CREATION 
  src/slave/containerizer/mesos/provisioner/appc/image_fetcher.cpp PRE-CREATION 
  src/slave/flags.hpp 23ec158f4a95fa76d657d9eb6a4a6fe30d57e5b6 
  src/slave/flags.cpp 75d7429a4e3e1d6259296257c0ace1ade365ac2b 

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


Testing
---

make check; local image server


Thanks,

Jojy Varghese

Re: Review Request 42915: Check SlaveId when recover Docker containers.

[Mesos ReviewBot]

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



Bad review!

Reviews applied: []

Error:
 No reviewers specified. Please find a reviewer by asking on JIRA or the 
mailing list.

- Mesos ReviewBot


On Jan. 28, 2016, 5:37 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42915/
> ---
> 
> (Updated Jan. 28, 2016, 5:37 p.m.)
> 
> 
> Review request for mesos.
> 
> 
> Bugs: MESOS-3573
> https://issues.apache.org/jira/browse/MESOS-3573
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Check SlaveId when recover Docker containers.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/docker.hpp 77a50d80179672cf3c270cbdd7fa003c7d9ad6f3 
>   src/slave/containerizer/docker.cpp 2887cb4a01febbbf276026e584ffc466289e10c9 
> 
> Diff: https://reviews.apache.org/r/42915/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 42554: Added 'dependencies' message to AppcImageManifest.

[Jie Yu]

> On Jan. 28, 2016, 10:08 p.m., Anand Mazumdar wrote:
> > include/mesos/mesos.proto, line 1637
> > 
> >
> > hmmm .. we have generally just used `name` as the field-name at other 
> > places for specifying the image name in this file. What is different about 
> > this occurence?
> > 
> > In a similar vein, `imageID` should just be `id`?

Anand, the reason is that we need to match the same in the following spec:
https://github.com/appc/spec/blob/master/spec/aci.md

So that our json->protobuf parsing will work properly.


- Jie


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


On Jan. 26, 2016, 2:19 a.m., Jojy Varghese wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42554/
> ---
> 
> (Updated Jan. 26, 2016, 2:19 a.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added 'dependencies' message to AppcImageManifest.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 0be4bed336e86a5c377e87ac6212c70ac3b4c66b 
> 
> Diff: https://reviews.apache.org/r/42554/diff/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Jojy Varghese
> 
>

Review Request 42921: Fixed flaky MaxCompletedTasksPerFrameworkFlag test.

[Kevin Klues]

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

Review request for mesos, Anand Mazumdar and Ben Mahler.


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


Repository: mesos


Description
---

Previously, this test was calling EXPECT_CALL() on
SchedulerDriver.resourceOffers() after the scheduler driver itself had
already been started. This resulted in flaky test results when offers
came in between schedDriver.start() and the EXPECT_CALL().

This commit fixes this test by moving the EXPECT_CALL() above the
schedDriver.start() call. However, because of the nature of this test,
this introduced a few complexities related to processing the incoming offers
in a loop later on. To allow this, we had to introduce a custom gmock
ACTION to enqueue offers in a process::Queue for later processing. In
the future we shoudl consider generalizing this custom ACTION or at
least moving it into src/tests/mesos.hpp for more general use.


Diffs
-

  src/tests/master_tests.cpp ce6ce25a03cdb0883612fe40b20996ec2e50de40 

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


Testing
---

Tested locally on my mac, as well as with running support/docker_build.sh on a 
linux machine.


Thanks,

Kevin Klues

Re: Review Request 42921: Fixed flaky MaxCompletedTasksPerFrameworkFlag test.

[Kevin Klues]

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

(Updated Jan. 28, 2016, 10:56 p.m.)


Review request for mesos, Anand Mazumdar and Ben Mahler.


Changes
---

Updated based on comments, including a move of EnqueueOffers() into 
src/tests/mesos.hpp


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


Repository: mesos


Description (updated)
---

Previously, this test was calling EXPECT_CALL() on
SchedulerDriver.resourceOffers() after the scheduler driver itself had
already been started. This resulted in flaky test results when offers
came in between schedDriver.start() and the EXPECT_CALL().

This commit fixes this test by moving the EXPECT_CALL() above the
schedDriver.start() call. However, because of the nature of this test,
this introduced a few complexities related to processing the incoming offers
in a loop later on. To allow this, we had to introduce a custom gmock
ACTION to enqueue offers in a process::Queue for later processing. This
ACTION is generally useful and has been placed in src/tests/mesos.hpp.


Diffs (updated)
-

  src/tests/master_tests.cpp ce6ce25a03cdb0883612fe40b20996ec2e50de40 
  src/tests/mesos.hpp 845b637c765b5b97568e9b3797b53814d2ab7782 

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


Testing
---

Tested locally on my mac, as well as with running support/docker_build.sh on a 
linux machine.


Thanks,

Kevin Klues

Review Request 42915: Check SlaveId when recover Docker containers.

[haosdent huang]

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

Review request for mesos.


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


Repository: mesos


Description
---

Check SlaveId when recover Docker containers.


Diffs
-

  src/slave/containerizer/docker.hpp 77a50d80179672cf3c270cbdd7fa003c7d9ad6f3 
  src/slave/containerizer/docker.cpp 2887cb4a01febbbf276026e584ffc466289e10c9 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 42915: Check SlaveId when recover Docker containers.

[haosdent huang]

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

(Updated Jan. 28, 2016, 5:37 p.m.)


Review request for mesos.


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


Repository: mesos


Description
---

Check SlaveId when recover Docker containers.


Diffs
-

  src/slave/containerizer/docker.hpp 77a50d80179672cf3c270cbdd7fa003c7d9ad6f3 
  src/slave/containerizer/docker.cpp 2887cb4a01febbbf276026e584ffc466289e10c9 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 42362: Added persistent volume endpoint test without authentication.

[Greg Mann]

> On Jan. 28, 2016, 5:49 p.m., Alexander Rukletsov wrote:
> > src/tests/persistent_volume_endpoints_tests.cpp, lines 1118-1119
> > 
> >
> > I advocate the practice when each block wrapped in curly braces is 
> > prepended with a comment. Effectively, each such block is a test case 
> > inside a test, so since we strive to provide a comment for each test we 
> > should do the same for each block. I think you had great comments that you 
> > could reused here. The most important bit is to drag people's attention to 
> > the absence of credentials. Does it make sense?

Yep, it makes sense. Anand advocated removing the bit about not including 
authentication headers because it was self-explanatory, but I do think that 
it's better to call it out explicitly, since if you don't know the function 
signature of `createBasicAuthHeaders`, it's not immediately obvious. Thanks, 
will update.


- Greg


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


On Jan. 28, 2016, 4:43 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42362/
> ---
> 
> (Updated Jan. 28, 2016, 4:43 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Kapil Arya, and Neil Conway.
> 
> 
> Bugs: MESOS-4395
> https://issues.apache.org/jira/browse/MESOS-4395
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added persistent volume endpoint tests with HTTP authentication disabled.
> 
> The persistent volume endpoint tests allow volume creation and destruction 
> when HTTP authentication is disabled; this patch introduces a test for this 
> scenario: `PersistentVolumeEndpointsTest.NoAuthentication`.
> 
> 
> Diffs
> -
> 
>   src/tests/persistent_volume_endpoints_tests.cpp 
> 22e18758ee91a649486725473d9e50fae9d43b01 
> 
> Diff: https://reviews.apache.org/r/42362/diff/
> 
> 
> Testing
> ---
> 
> A new test, `PersistentVolumeEndpointsTest.NoAuthentication`, was added to 
> the persistent volume endpoint tests.
> 
> `make check` was used to test, and the new test was run with 
> `--gtest_repeat=1000 -gtest_break_on_failure=1`.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 42921: Fixed flaky MaxCompletedTasksPerFrameworkFlag test.

[Ben Mahler]

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



Looks great, just some minor comments below.


src/tests/master_tests.cpp (line 3993)


In general we'll add a newline between the end of your comment and the 
start of the TODO:

```
// Action to enqueue all received offers from 
SchedulerDriver.resourceOffers()
// into a libprocess queue. Currently, this is only used in the
// MaxCompletedTasksPerFrameworkFlag test below.
//
// TODO(klueska): Move this into src/tests/mesos.hpp if we decide it is more
// generally useful.
```

Just to make it a bit easier to read when there are multiple TODOs, NOTEs, 
etc. Or it becomes sometimes hard to tell where a TODO ends and a comment 
starts again (e.g. if the TODO line ends with a period, is the next line part 
of the TODO or the comment?)

But mind just moving this to mesos.hpp? Seems generally useful (at some 
point we should have a gmock.hpp or gmock/actions.hpp header).



src/tests/master_tests.cpp (line 3998)


Sometimes we'll use single letter names, but it's more typical that we 
don't abbreviate here:

s/o/offer/



src/tests/master_tests.cpp (line 4038)


Usually the type is omitted from the name, e.g. schedRegistered instead of 
schedRegisteredFuture.

So the following would be more consistent with our naming style:

```
s/offersQueue/offers/
```


- Ben Mahler


On Jan. 28, 2016, 10:16 p.m., Kevin Klues wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42921/
> ---
> 
> (Updated Jan. 28, 2016, 10:16 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar and Ben Mahler.
> 
> 
> Bugs: MESOS-4518
> https://issues.apache.org/jira/browse/MESOS-4518
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Previously, this test was calling EXPECT_CALL() on
> SchedulerDriver.resourceOffers() after the scheduler driver itself had
> already been started. This resulted in flaky test results when offers
> came in between schedDriver.start() and the EXPECT_CALL().
> 
> This commit fixes this test by moving the EXPECT_CALL() above the
> schedDriver.start() call. However, because of the nature of this test,
> this introduced a few complexities related to processing the incoming offers
> in a loop later on. To allow this, we had to introduce a custom gmock
> ACTION to enqueue offers in a process::Queue for later processing. In
> the future we shoudl consider generalizing this custom ACTION or at
> least moving it into src/tests/mesos.hpp for more general use.
> 
> 
> Diffs
> -
> 
>   src/tests/master_tests.cpp ce6ce25a03cdb0883612fe40b20996ec2e50de40 
> 
> Diff: https://reviews.apache.org/r/42921/diff/
> 
> 
> Testing
> ---
> 
> Tested locally on my mac, as well as with running support/docker_build.sh on 
> a linux machine.
> 
> 
> Thanks,
> 
> Kevin Klues
> 
>

Re: Review Request 42921: Fixed flaky MaxCompletedTasksPerFrameworkFlag test.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [42921]

Passed command: export OS=ubuntu:14.04;export CONFIGURATION="--verbose";export 
COMPILER=gcc; ./support/docker_build.sh

- Mesos ReviewBot


On Jan. 28, 2016, 10:56 p.m., Kevin Klues wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42921/
> ---
> 
> (Updated Jan. 28, 2016, 10:56 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar and Ben Mahler.
> 
> 
> Bugs: MESOS-4518
> https://issues.apache.org/jira/browse/MESOS-4518
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Previously, this test was calling EXPECT_CALL() on
> SchedulerDriver.resourceOffers() after the scheduler driver itself had
> already been started. This resulted in flaky test results when offers
> came in between schedDriver.start() and the EXPECT_CALL().
> 
> This commit fixes this test by moving the EXPECT_CALL() above the
> schedDriver.start() call. However, because of the nature of this test,
> this introduced a few complexities related to processing the incoming offers
> in a loop later on. To allow this, we had to introduce a custom gmock
> ACTION to enqueue offers in a process::Queue for later processing. This
> ACTION is generally useful and has been placed in src/tests/mesos.hpp.
> 
> 
> Diffs
> -
> 
>   src/tests/master_tests.cpp ce6ce25a03cdb0883612fe40b20996ec2e50de40 
>   src/tests/mesos.hpp 845b637c765b5b97568e9b3797b53814d2ab7782 
> 
> Diff: https://reviews.apache.org/r/42921/diff/
> 
> 
> Testing
> ---
> 
> Tested locally on my mac, as well as with running support/docker_build.sh on 
> a linux machine.
> 
> 
> Thanks,
> 
> Kevin Klues
> 
>

Re: Review Request 42554: Added 'dependencies' message to AppcImageManifest.

[Jie Yu]

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


Fix it, then Ship it!





include/mesos/mesos.proto (line 1640)


Use uint64 here?


- Jie Yu


On Jan. 26, 2016, 2:19 a.m., Jojy Varghese wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42554/
> ---
> 
> (Updated Jan. 26, 2016, 2:19 a.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added 'dependencies' message to AppcImageManifest.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 0be4bed336e86a5c377e87ac6212c70ac3b4c66b 
> 
> Diff: https://reviews.apache.org/r/42554/diff/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Jojy Varghese
> 
>

Re: Review Request 42554: Added 'dependencies' message to AppcImageManifest.

[Anand Mazumdar]

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




include/mesos/mesos.proto (line 1637)


hmmm .. we have generally just used `name` as the field-name at other 
places for specifying the image name in this file. What is different about this 
occurence?

In a similar vein, `imageID` should just be `id`?


- Anand Mazumdar


On Jan. 26, 2016, 2:19 a.m., Jojy Varghese wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42554/
> ---
> 
> (Updated Jan. 26, 2016, 2:19 a.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added 'dependencies' message to AppcImageManifest.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 0be4bed336e86a5c377e87ac6212c70ac3b4c66b 
> 
> Diff: https://reviews.apache.org/r/42554/diff/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Jojy Varghese
> 
>

Re: Review Request 42554: Added 'dependencies' message to AppcImageManifest.

[Jie Yu]

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



it's strange that this protobuf is exposed in Mesos API. Similar to Docker, can 
we move this to include/mesos/appc/spec.proto?

- Jie Yu


On Jan. 26, 2016, 2:19 a.m., Jojy Varghese wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42554/
> ---
> 
> (Updated Jan. 26, 2016, 2:19 a.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added 'dependencies' message to AppcImageManifest.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 0be4bed336e86a5c377e87ac6212c70ac3b4c66b 
> 
> Diff: https://reviews.apache.org/r/42554/diff/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Jojy Varghese
> 
>

Re: Review Request 40731: Added a fixture to test the floating point precision for CPU resource allocation.

[Mesos ReviewBot]

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



Bad patch!

Reviews applied: [40731]

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

Error:
 ..
2016-01-28 20:13:02 URL:https://reviews.apache.org/r/40731/diff/raw/ 
[4029/4029] -> "40731.patch" [1]
Total errors found: 0
Checking 1 files
Error: Commit message summary (the first line) must not exceed 72 characters.

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

- Mesos ReviewBot


On Jan. 28, 2016, 6 p.m., Avinash sridharan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40731/
> ---
> 
> (Updated Jan. 28, 2016, 6 p.m.)
> 
> 
> Review request for mesos, Bernd Mathiske, Klaus Ma, Mandeep Chadha, and Neil 
> Conway.
> 
> 
> Bugs: MESOS-3552
> https://issues.apache.org/jira/browse/MESOS-3552
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added a fixture to test the floating point precision for CPU resource 
> allocation. This patch is based on the commit submitted by : Mandeep 
> (@mchadha) https://reviews.apache.org/r/39056/
> 
> 
> Diffs
> -
> 
>   src/tests/reservation_tests.cpp d0c88560a5618ebec39fc4e0539ddb2d9ca554a2 
> 
> Diff: https://reviews.apache.org/r/40731/diff/
> 
> 
> Testing
> ---
> 
> Ran make check after adding Mandeep's test case to the GTEST framework.
> 
> 
> Thanks,
> 
> Avinash sridharan
> 
>

Re: Review Request 42901: Fixed a few typos in the HA framework guide.

[Vinod Kone]

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


Ship it!




Thank you!

- Vinod Kone


On Jan. 28, 2016, 7:26 a.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42901/
> ---
> 
> (Updated Jan. 28, 2016, 7:26 a.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed a few typos in the HA framework guide.
> 
> 
> Diffs
> -
> 
>   docs/high-availability-framework-guide.md 
> 3d429d85edce695f608320cab05cdafdafa4a42e 
> 
> Diff: https://reviews.apache.org/r/42901/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 42914: Listed supported file extensions in fetcher documentation.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [42914]

Passed command: export OS=ubuntu:14.04;export CONFIGURATION="--verbose";export 
COMPILER=gcc; ./support/docker_build.sh

- Mesos ReviewBot


On Jan. 28, 2016, 6:17 p.m., Bernd Mathiske wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42914/
> ---
> 
> (Updated Jan. 28, 2016, 6:17 p.m.)
> 
> 
> Review request for mesos, Adam B, Benjamin Hindman, Till Toenshoff, and 
> Timothy Chen.
> 
> 
> Bugs: MESOS-4336
> https://issues.apache.org/jira/browse/MESOS-4336
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Edited fetcher.md, listing the supported file extensions when extracting an 
> archive.
> 
> 
> Diffs
> -
> 
>   docs/fetcher.md cb4f3c33fb67f7ac65dbe7ebe1347d2599c43f37 
> 
> Diff: https://reviews.apache.org/r/42914/diff/
> 
> 
> Testing
> ---
> 
> Opened file with MD renderer.
> 
> 
> Thanks,
> 
> Bernd Mathiske
> 
>

Re: Review Request 42773: Added utility function to compute SHA512 digest.

[Jie Yu]

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




src/common/command_utils.hpp (line 68)


s/alg/algorithm/

(e.g., 512)



src/common/command_utils.hpp (line 71)


s/alg/algorithm/



src/common/command_utils.cpp (line 169)


hum, can you move the body of this method to shasum and call shasum in this 
function?

```
Future sha512(const Path& input)
{
  return shasum("512", input);
}
```


- Jie Yu


On Jan. 28, 2016, 6:44 p.m., Jojy Varghese wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42773/
> ---
> 
> (Updated Jan. 28, 2016, 6:44 p.m.)
> 
> 
> Review request for mesos.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added utility function to compute SHA512 digest.
> 
> 
> Diffs
> -
> 
>   src/common/command_utils.hpp bfab34571dd37980d9f99585e506eb015b499e28 
>   src/common/command_utils.cpp e9d782127f948e165f55d03a2b1bf47946cc7f4e 
>   src/tests/common/command_utils_tests.cpp 
> 938f3995aacf74bac28ae11040de292aa328f1e5 
> 
> Diff: https://reviews.apache.org/r/42773/diff/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Jojy Varghese
> 
>

Re: Review Request 41892: DockerContinerizer infers hostPath for persistent volumes.

[Zhitao Li]

> On Jan. 15, 2016, 12:03 a.m., Zhitao Li wrote:
> > src/docker/docker.cpp, lines 410-420
> > 
> >
> > (Sorry I just got time to come back to this).
> > 
> > I don't exactly understand your suggestion about "add additional 
> > containerInfo.volumes() in 
> > DockerContainerizerProcess::Container::create()", because the latter 
> > function actually does not pass a `ContainerInfo`.
> > 
> > Because the volume is actually included in 
> > `TaskInfo::ContainerInfo::volumes`, we would need to mutate the `TaskInfo` 
> > before passing it to the `DockerExecutorProcess::launchTask()` function, 
> > which I don't really know how to do it, and I even doubt whether it's a 
> > good idea for logging/clarity purpose.
> > 
> > Please let me know if I didn't understand your suggestion, or if you 
> > think we should explore the other alternative (passing `hostPath` earlier 
> > in resource offer).
> 
> Jie Yu wrote:
> Sorry for being late on this.
> 
> My sugguestion is that we add some logics in 
> DockerContainerProcess::Container::create(), the 'create' function has both 
> TaskINfo and ExecutorInfo. So, you should be able to get the ContainerInfo 
> from them. When you generate DockerContainerizerProcess::Container, you 
> should be able to generate a new ContainerInfo that has persistent volumes in 
> it. Let me know if you still don't understand.
> 
> FYI, docker just merged its mount propagation support for volumes. That 
> means we can have full persistent volume support. We need to mount the 
> sandbox as a shared mount into the docker container. ANy mounts under the 
> sandbox mount will be automatically propergated to the docker container.
> 
> Zhitao Li wrote:
> @jieyu, I tried your recommendation and managed to generate a different 
> `ContainerInfo` object inside `DockerContainerProcess::Container::create()`. 
> However, I don't think it could work either because the actual docker 
> container is created by the call sequence `DockerExecutor::launchTask()` -> 
> `Docker::run()` in the forked executor subprocess, and it seems impossible to 
> properly pass the modified `ContainerInfo` to that.
> 
> My proposal is:
> - 1) keep the `persistent_volumes_root` flag to the executor;
> - 2) implement this logic inside `DockerExecutor::launchTask()` (see my 
> other inline comment for exact location) so we don't need to change `Docker` 
> class;
> - 3) for testing, create a test case in docker_containerizer_tests.cpp 
> which creates persistent volume and launch an executor.
> 
> I'll rebase and update this diff with the above proposal.
> 
> Guangya Liu wrote:
> The `DockerContainerProcess::Container::create()` only include `TaskInfo` 
> when using a `command executor`, otherwise, the 
> `DockerContainerProcess::Container::create()` will not include task info. For 
> this case it is not `command executor`, so I think that @Zhitao's solution 
> should be the right choice. Please correct me if there's sth wrong, thanks.

Hi Guangya,

I had some offline chat with Jie about this issue and agreed that this should 
instead be fixed with a more involved solution on a different direction 
(mounting persistent volume inside sandbox as a "shared volume"). The bug has 
been reassigned to Timothy Chen who's implementing on that.

I'll abandon this review for now.


- Zhitao


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


On Jan. 4, 2016, 9:15 p.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41892/
> ---
> 
> (Updated Jan. 4, 2016, 9:15 p.m.)
> 
> 
> Review request for mesos, haosdent huang and Jie Yu.
> 
> 
> Bugs: MESOS-3413
> https://issues.apache.org/jira/browse/MESOS-3413
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This diff makes DockerContainerizer infers hostPath for persistent volumes 
> from resources passed-in.
> 
> This allows current DockerContainerizer to use persistent volumes w/o hard 
> code slave's work_dir. I also checked that the inferred directory actually 
> exists to avoid framework messing up role or persistent id.
> 
> Note that some validation on the resource part should be done before we start 
> the container, which seems to belong to slave.
> 
> 
> Diffs
> -
> 
>   src/docker/docker.hpp dde2b29deda7e40929e0169935a5bafdd43136b1 
>   src/docker/docker.cpp e93280735f1c5f66c765fcbabfd3e50b46c024e8 
>   src/docker/executor.hpp abbc419533ab40312e917931a2fc2ce78b38da41 
>   src/docker/executor.cpp 7512d07de6c8324340e6b5f3e5162ef00efc47fc 
>   

Re: Review Request 42539: Support image specified Entrypoint and Cmd.

[Gilbert Song]

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

(Updated Jan. 28, 2016, 12:28 p.m.)


Review request for mesos, Jie Yu and Timothy Chen.


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


Repository: mesos


Description
---

Support image specified Entrypoint and Cmd.


Diffs (updated)
-

  src/launcher/executor.cpp 356d311fdf97b2c4663c60e13ede7cdb71a264c7 
  src/slave/containerizer/mesos/containerizer.hpp 
811ab7937279c4a55da450c136f9fcb1303ea0d5 
  src/slave/containerizer/mesos/containerizer.cpp 
4b504dbb58823ce7675f1d2048dcc7a27c05663d 

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


Testing
---

make check(ubuntu14.04 + clang-3.6)


Thanks,

Gilbert Song

Review Request 42928: Updated docker_build.sh to generate xml output for all OSes.

[Vinod Kone]

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

Review request for mesos, Anand Mazumdar, Greg Mann, and Kevin Klues.


Repository: mesos


Description
---

xml output was originally only enabled for ubuntu. This enables it for all OSes.


Diffs
-

  support/docker_build.sh da94be87fae98825e60331634259eab0e7a4ebd1 

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


Testing
---


Thanks,

Vinod Kone

Re: Review Request 42927: Deleted Dockerfile in favor of `support/docker_build.sh`.

[Anand Mazumdar]

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


Ship it!




Ship It!

- Anand Mazumdar


On Jan. 29, 2016, 12:05 a.m., Vinod Kone wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42927/
> ---
> 
> (Updated Jan. 29, 2016, 12:05 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Greg Mann, and Kevin Klues.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> No one seems to be using it and this is not maintained anymore.
> 
> 
> Diffs
> -
> 
>   Dockerfile 35abf25aa77ef3f8a68b803eb8ff18ad90a0ff7b 
> 
> Diff: https://reviews.apache.org/r/42927/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Vinod Kone
> 
>

Re: Review Request 42841: WIP: Introducing appc image fetcher.

[Jojy Varghese]

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

(Updated Jan. 29, 2016, 12:58 a.m.)


Review request for Jie Yu.


Repository: mesos


Description
---

WIP: Introducing appc image fetcher.


Diffs (updated)
-

  src/CMakeLists.txt 93caf000f39bbb71a17d9876689afb50cf637e44 
  src/Makefile.am fac17f4bac3b2ddda0384dec8b0ed6f960bd24d1 
  src/slave/containerizer/mesos/provisioner/appc/image_fetcher.hpp PRE-CREATION 
  src/slave/containerizer/mesos/provisioner/appc/image_fetcher.cpp PRE-CREATION 
  src/slave/flags.hpp 23ec158f4a95fa76d657d9eb6a4a6fe30d57e5b6 
  src/slave/flags.cpp 75d7429a4e3e1d6259296257c0ace1ade365ac2b 

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


Testing
---

make check; local image server


Thanks,

Jojy Varghese

Re: Review Request 42866: Disabled the test RegistryClientTest.BadTokenServerAddress.

[Timothy Chen]

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


Ship it!




Ship It!

- Timothy Chen


On Jan. 27, 2016, 8:38 p.m., Jojy Varghese wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42866/
> ---
> 
> (Updated Jan. 27, 2016, 8:38 p.m.)
> 
> 
> Review request for mesos and Timothy Chen.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> It has shown flaky behavior and needs further investigation. Disabling now as
> its not a very important test for functionality.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/provisioner_docker_tests.cpp 
> 33df60065903228749833bbad20449ba8784594a 
> 
> Diff: https://reviews.apache.org/r/42866/diff/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Jojy Varghese
> 
>

Re: Review Request 42828: Updated ReviewBot to tee build output to a file.

[Vinod Kone]

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

(Updated Jan. 29, 2016, 1:15 a.m.)


Review request for mesos, Anand Mazumdar, Greg Mann, Kevin Klues, and Michael 
Park.


Changes
---

addressed comments.


Bugs: MESOS-1469 and MESOS-4478
https://issues.apache.org/jira/browse/MESOS-1469
https://issues.apache.org/jira/browse/MESOS-4478


Repository: mesos


Description (updated)
---

Updated ReviewBot to tee build output to a file.


Diffs (updated)
-

  support/verify_reviews.py 251d57ec050b087f13b3f5e0d8f421fe8eb76787 

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


Testing
---

?  mesos git:(vinod/review_bot_tee_output) 
BUILD_URL=https://builds.apache.org/job/mesos-reviewbot/10980/ 
./support/verify_reviews.py vinod kone 1
git rev-parse HEAD
Checking if review: 41092 needs verification
Latest diff timestamp: 2016-01-26 13:53:35
Verifying review 41092
Dependent review: https://reviews.apache.org/api/review-requests/41090/ 
Dependent review: https://reviews.apache.org/api/review-requests/40951/ 
Applying review 41092
./support/apply-review.sh -n -r 41092
+ : ubuntu:14.04
+ : gcc
+ : --verbose
+++ dirname ./support/docker_build.sh
++ cd ./support/..
++ pwd
+ MESOS_DIRECTORY=/Users/vinodkone/workspace/mesos
+ cd /Users/vinodkone/workspace/mesos
+ DOCKERFILE=Dockerfile
+ rm -f Dockerfile
+ case $OS in
+ append_dockerfile 'FROM ubuntu:14.04'
+ echo FROM ubuntu:14.04
+ append_dockerfile 'RUN rm -rf /var/lib/apt/lists/*'
+ echo RUN rm -rf '/var/lib/apt/lists/*'
+ append_dockerfile 'RUN apt-get update'
+ echo RUN apt-get update
+ append_dockerfile 'RUN apt-get -y install build-essential clang git maven 
autoconf libtool'
+ echo RUN apt-get -y install build-essential clang git maven autoconf libtool
+ append_dockerfile 'RUN apt-get -y install openjdk-7-jdk python-dev 
python-boto libcurl4-nss-dev libsasl2-dev libapr1-dev libsvn-dev libevent-dev 
libev-dev'
+ echo RUN apt-get -y install openjdk-7-jdk python-dev python-boto 
libcurl4-nss-dev libsasl2-dev libapr1-dev libsvn-dev libevent-dev libev-dev
+ append_dockerfile 'RUN adduser --disabled-password --gecos '\'''\'' mesos'
+ echo RUN adduser --disabled-password --gecos ''\'''\''' mesos
+ append_dockerfile 'ENV GTEST_FILTER -FsTest.FileSystemTableRead'
+ echo ENV GTEST_FILTER -FsTest.FileSystemTableRead
+ append_dockerfile 'ENV GTEST_OUTPUT xml:report.xml'
+ echo ENV GTEST_OUTPUT xml:report.xml
+ case $COMPILER in
+ append_dockerfile 'ENV CC gcc'
+ echo ENV CC gcc
+ append_dockerfile 'ENV CXX g++'
+ echo ENV CXX g++
+ append_dockerfile 'WORKDIR mesos'
+ echo WORKDIR mesos
+ append_dockerfile 'COPY . /mesos/'
+ echo COPY . /mesos/
+ append_dockerfile 'RUN chown -R mesos /mesos'
+ echo RUN chown -R mesos /mesos
+ append_dockerfile 'USER mesos'
+ echo USER mesos
+ append_dockerfile 'CMD ./bootstrap && ./configure --verbose && 
DISTCHECK_CONFIGURE_FLAGS="--verbose" GLOG_v=1 MESOS_VERBOSE=1 make -j8 
distcheck'
+ echo CMD ./bootstrap '&&' ./configure --verbose '&&' 
'DISTCHECK_CONFIGURE_FLAGS="--verbose"' GLOG_v=1 MESOS_VERBOSE=1 make -j8 
distcheck
++ date +%s
+ TAG=mesos-1453845969-21946
+ docker build --no-cache=true -t mesos-1453845969-21946 .
./support/docker_build.sh: line 117: docker: command not found
Posting review: Bad patch!

Reviews applied: [40951, 41090, 41092]

Failed command: ['bash', '-c', 'set -o pipefail; export OS=ubuntu:14.04;export 
CONFIGURATION="--verbose";export COMPILER=gcc; ./support/docker_build.sh 2>&1 | 
tee build_41092']

Error:
 ..
+ : ubuntu:14.04
+ : gcc
+ : --verbose
+++ dirname ./support/docker_build.sh
++ cd ./support/..
++ pwd
+ MESOS_DIRECTORY=/Users/vinodkone/workspace/mesos
+ cd /Users/vinodkone/workspace/mesos
+ DOCKERFILE=Dockerfile
+ rm -f Dockerfile
+ case $OS in
+ append_dockerfile 'FROM ubuntu:14.04'
+ echo FROM ubuntu:14.04
+ append_dockerfile 'RUN rm -rf /var/lib/apt/lists/*'
+ echo RUN rm -rf '/var/lib/apt/lists/*'
+ append_dockerfile 'RUN apt-get update'
+ echo RUN apt-get update
+ append_dockerfile 'RUN apt-get -y install build-essential clang git maven 
autoconf libtool'
+ echo RUN apt-get -y install build-essential clang git maven autoconf libtool
+ append_dockerfile 'RUN apt-get -y install openjdk-7-jdk python-dev 
python-boto libcurl4-nss-dev libsasl2-dev libapr1-dev libsvn-dev libevent-dev 
libev-dev'
+ echo RUN apt-get -y install openjdk-7-jdk python-dev python-boto 
libcurl4-nss-dev libsasl2-dev libapr1-dev libsvn-dev libevent-dev libev-dev
+ append_dockerfile 'RUN adduser --disabled-password --gecos '\'''\'' mesos'
+ echo RUN adduser --disabled-password --gecos ''\'''\''' mesos
+ append_dockerfile 'ENV GTEST_FILTER -FsTest.FileSystemTableRead'
+ echo ENV GTEST_FILTER -FsTest.FileSystemTableRead
+ append_dockerfile 'ENV GTEST_OUTPUT xml:report.xml'
+ echo ENV GTEST_OUTPUT xml:report.xml
+ case $COMPILER in
+ 

Re: Review Request 42828: Updated ReviewBot to tee build output to a file.

[Vinod Kone]

> On Jan. 29, 2016, 1:03 a.m., Anand Mazumdar wrote:
> > support/verify_reviews.py, line 151
> > 
> >
> > hmm .. what happens to the build_output file thereafter? Should we be 
> > cleaning it up too?

This gets cleaned up as part of "git clean" that we do in cleanup().


> On Jan. 29, 2016, 1:03 a.m., Anand Mazumdar wrote:
> > support/verify_reviews.py, line 122
> > 
> >
> > Minor Nit: More pythonic version:
> > 
> > ```
> > "build_{}".format(review_request["id"])
> > ```

we don't follow that style in this file. so i will keep it consistent for now.


> On Jan. 29, 2016, 1:03 a.m., Anand Mazumdar wrote:
> > support/verify_reviews.py, line 138
> > 
> >
> > Nit: Can we wrap this at 80 chars? Not sure if we follow the style 
> > guidelines from C++ here too.

done.


> On Jan. 29, 2016, 1:03 a.m., Anand Mazumdar wrote:
> > support/verify_reviews.py, line 149
> > 
> >
> > Ditto as above.

done.


- Vinod


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


On Jan. 29, 2016, 12:03 a.m., Vinod Kone wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42828/
> ---
> 
> (Updated Jan. 29, 2016, 12:03 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Greg Mann, Kevin Klues, and Michael 
> Park.
> 
> 
> Bugs: MESOS-1469 and MESOS-4478
> https://issues.apache.org/jira/browse/MESOS-1469
> https://issues.apache.org/jira/browse/MESOS-4478
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This fixes 2 issues
> 
> 1) Stream output to the console when build is in progress. 
> https://issues.apache.org/jira/browse/MESOS-1469
> 
> 2) Make sure the full build output (and not just truncated build output) is 
> output to the console. This was a bug introduced while fixing 
> https://issues.apache.org/jira/browse/MESOS-4478
> 
> 
> Diffs
> -
> 
>   support/verify_reviews.py 251d57ec050b087f13b3f5e0d8f421fe8eb76787 
> 
> Diff: https://reviews.apache.org/r/42828/diff/
> 
> 
> Testing
> ---
> 
> ?  mesos git:(vinod/review_bot_tee_output) 
> BUILD_URL=https://builds.apache.org/job/mesos-reviewbot/10980/ 
> ./support/verify_reviews.py vinod kone 1
> git rev-parse HEAD
> Checking if review: 41092 needs verification
> Latest diff timestamp: 2016-01-26 13:53:35
> Verifying review 41092
> Dependent review: https://reviews.apache.org/api/review-requests/41090/ 
> Dependent review: https://reviews.apache.org/api/review-requests/40951/ 
> Applying review 41092
> ./support/apply-review.sh -n -r 41092
> + : ubuntu:14.04
> + : gcc
> + : --verbose
> +++ dirname ./support/docker_build.sh
> ++ cd ./support/..
> ++ pwd
> + MESOS_DIRECTORY=/Users/vinodkone/workspace/mesos
> + cd /Users/vinodkone/workspace/mesos
> + DOCKERFILE=Dockerfile
> + rm -f Dockerfile
> + case $OS in
> + append_dockerfile 'FROM ubuntu:14.04'
> + echo FROM ubuntu:14.04
> + append_dockerfile 'RUN rm -rf /var/lib/apt/lists/*'
> + echo RUN rm -rf '/var/lib/apt/lists/*'
> + append_dockerfile 'RUN apt-get update'
> + echo RUN apt-get update
> + append_dockerfile 'RUN apt-get -y install build-essential clang git maven 
> autoconf libtool'
> + echo RUN apt-get -y install build-essential clang git maven autoconf libtool
> + append_dockerfile 'RUN apt-get -y install openjdk-7-jdk python-dev 
> python-boto libcurl4-nss-dev libsasl2-dev libapr1-dev libsvn-dev libevent-dev 
> libev-dev'
> + echo RUN apt-get -y install openjdk-7-jdk python-dev python-boto 
> libcurl4-nss-dev libsasl2-dev libapr1-dev libsvn-dev libevent-dev libev-dev
> + append_dockerfile 'RUN adduser --disabled-password --gecos '\'''\'' mesos'
> + echo RUN adduser --disabled-password --gecos ''\'''\''' mesos
> + append_dockerfile 'ENV GTEST_FILTER -FsTest.FileSystemTableRead'
> + echo ENV GTEST_FILTER -FsTest.FileSystemTableRead
> + append_dockerfile 'ENV GTEST_OUTPUT xml:report.xml'
> + echo ENV GTEST_OUTPUT xml:report.xml
> + case $COMPILER in
> + append_dockerfile 'ENV CC gcc'
> + echo ENV CC gcc
> + append_dockerfile 'ENV CXX g++'
> + echo ENV CXX g++
> + append_dockerfile 'WORKDIR mesos'
> + echo WORKDIR mesos
> + append_dockerfile 'COPY . /mesos/'
> + echo COPY . /mesos/
> + append_dockerfile 'RUN chown -R mesos /mesos'
> + echo RUN chown -R mesos /mesos
> + append_dockerfile 'USER mesos'
> + echo USER mesos
> + append_dockerfile 'CMD ./bootstrap && ./configure --verbose && 
> 

Re: Review Request 42828: Updated ReviewBot to tee build output to a file.

[Vinod Kone]

> On Jan. 29, 2016, 1:10 a.m., Kevin Klues wrote:
> > support/verify_reviews.py, line 140
> > 
> >
> > To avoid using tee, you could just do:
> > 
> > "%s > %s 2>&1" % (command, build_output)

The above just outputs it to a file right? We want it to also output the 
contents to the console (stdout) in real time.


- Vinod


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


On Jan. 29, 2016, 1:15 a.m., Vinod Kone wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42828/
> ---
> 
> (Updated Jan. 29, 2016, 1:15 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Greg Mann, Kevin Klues, and Michael 
> Park.
> 
> 
> Bugs: MESOS-1469 and MESOS-4478
> https://issues.apache.org/jira/browse/MESOS-1469
> https://issues.apache.org/jira/browse/MESOS-4478
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated ReviewBot to tee build output to a file.
> 
> 
> Diffs
> -
> 
>   support/verify_reviews.py 251d57ec050b087f13b3f5e0d8f421fe8eb76787 
> 
> Diff: https://reviews.apache.org/r/42828/diff/
> 
> 
> Testing
> ---
> 
> ?  mesos git:(vinod/review_bot_tee_output) 
> BUILD_URL=https://builds.apache.org/job/mesos-reviewbot/10980/ 
> ./support/verify_reviews.py vinod kone 1
> git rev-parse HEAD
> Checking if review: 41092 needs verification
> Latest diff timestamp: 2016-01-26 13:53:35
> Verifying review 41092
> Dependent review: https://reviews.apache.org/api/review-requests/41090/ 
> Dependent review: https://reviews.apache.org/api/review-requests/40951/ 
> Applying review 41092
> ./support/apply-review.sh -n -r 41092
> + : ubuntu:14.04
> + : gcc
> + : --verbose
> +++ dirname ./support/docker_build.sh
> ++ cd ./support/..
> ++ pwd
> + MESOS_DIRECTORY=/Users/vinodkone/workspace/mesos
> + cd /Users/vinodkone/workspace/mesos
> + DOCKERFILE=Dockerfile
> + rm -f Dockerfile
> + case $OS in
> + append_dockerfile 'FROM ubuntu:14.04'
> + echo FROM ubuntu:14.04
> + append_dockerfile 'RUN rm -rf /var/lib/apt/lists/*'
> + echo RUN rm -rf '/var/lib/apt/lists/*'
> + append_dockerfile 'RUN apt-get update'
> + echo RUN apt-get update
> + append_dockerfile 'RUN apt-get -y install build-essential clang git maven 
> autoconf libtool'
> + echo RUN apt-get -y install build-essential clang git maven autoconf libtool
> + append_dockerfile 'RUN apt-get -y install openjdk-7-jdk python-dev 
> python-boto libcurl4-nss-dev libsasl2-dev libapr1-dev libsvn-dev libevent-dev 
> libev-dev'
> + echo RUN apt-get -y install openjdk-7-jdk python-dev python-boto 
> libcurl4-nss-dev libsasl2-dev libapr1-dev libsvn-dev libevent-dev libev-dev
> + append_dockerfile 'RUN adduser --disabled-password --gecos '\'''\'' mesos'
> + echo RUN adduser --disabled-password --gecos ''\'''\''' mesos
> + append_dockerfile 'ENV GTEST_FILTER -FsTest.FileSystemTableRead'
> + echo ENV GTEST_FILTER -FsTest.FileSystemTableRead
> + append_dockerfile 'ENV GTEST_OUTPUT xml:report.xml'
> + echo ENV GTEST_OUTPUT xml:report.xml
> + case $COMPILER in
> + append_dockerfile 'ENV CC gcc'
> + echo ENV CC gcc
> + append_dockerfile 'ENV CXX g++'
> + echo ENV CXX g++
> + append_dockerfile 'WORKDIR mesos'
> + echo WORKDIR mesos
> + append_dockerfile 'COPY . /mesos/'
> + echo COPY . /mesos/
> + append_dockerfile 'RUN chown -R mesos /mesos'
> + echo RUN chown -R mesos /mesos
> + append_dockerfile 'USER mesos'
> + echo USER mesos
> + append_dockerfile 'CMD ./bootstrap && ./configure --verbose && 
> DISTCHECK_CONFIGURE_FLAGS="--verbose" GLOG_v=1 MESOS_VERBOSE=1 make -j8 
> distcheck'
> + echo CMD ./bootstrap '&&' ./configure --verbose '&&' 
> 'DISTCHECK_CONFIGURE_FLAGS="--verbose"' GLOG_v=1 MESOS_VERBOSE=1 make -j8 
> distcheck
> ++ date +%s
> + TAG=mesos-1453845969-21946
> + docker build --no-cache=true -t mesos-1453845969-21946 .
> ./support/docker_build.sh: line 117: docker: command not found
> Posting review: Bad patch!
> 
> Reviews applied: [40951, 41090, 41092]
> 
> Failed command: ['bash', '-c', 'set -o pipefail; export 
> OS=ubuntu:14.04;export CONFIGURATION="--verbose";export COMPILER=gcc; 
> ./support/docker_build.sh 2>&1 | tee build_41092']
> 
> Error:
>  ..
> + : ubuntu:14.04
> + : gcc
> + : --verbose
> +++ dirname ./support/docker_build.sh
> ++ cd ./support/..
> ++ pwd
> + MESOS_DIRECTORY=/Users/vinodkone/workspace/mesos
> + cd /Users/vinodkone/workspace/mesos
> + DOCKERFILE=Dockerfile
> + rm -f Dockerfile
> + case $OS in
> + append_dockerfile 'FROM ubuntu:14.04'
> + echo FROM ubuntu:14.04
> + append_dockerfile 'RUN rm -rf /var/lib/apt/lists/*'
> + echo RUN rm -rf '/var/lib/apt/lists/*'
> + append_dockerfile 'RUN apt-get 

Re: Review Request 42828: Updated ReviewBot to tee build output to a file.

[Anand Mazumdar]

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


Ship it!




You would need to update the Summary/Description again while committing. 
Probably got erased by not having `--guess_fields=auto`. :-(

- Anand Mazumdar


On Jan. 29, 2016, 1:15 a.m., Vinod Kone wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42828/
> ---
> 
> (Updated Jan. 29, 2016, 1:15 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Greg Mann, Kevin Klues, and Michael 
> Park.
> 
> 
> Bugs: MESOS-1469 and MESOS-4478
> https://issues.apache.org/jira/browse/MESOS-1469
> https://issues.apache.org/jira/browse/MESOS-4478
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated ReviewBot to tee build output to a file.
> 
> 
> Diffs
> -
> 
>   support/verify_reviews.py 251d57ec050b087f13b3f5e0d8f421fe8eb76787 
> 
> Diff: https://reviews.apache.org/r/42828/diff/
> 
> 
> Testing
> ---
> 
> ?  mesos git:(vinod/review_bot_tee_output) 
> BUILD_URL=https://builds.apache.org/job/mesos-reviewbot/10980/ 
> ./support/verify_reviews.py vinod kone 1
> git rev-parse HEAD
> Checking if review: 41092 needs verification
> Latest diff timestamp: 2016-01-26 13:53:35
> Verifying review 41092
> Dependent review: https://reviews.apache.org/api/review-requests/41090/ 
> Dependent review: https://reviews.apache.org/api/review-requests/40951/ 
> Applying review 41092
> ./support/apply-review.sh -n -r 41092
> + : ubuntu:14.04
> + : gcc
> + : --verbose
> +++ dirname ./support/docker_build.sh
> ++ cd ./support/..
> ++ pwd
> + MESOS_DIRECTORY=/Users/vinodkone/workspace/mesos
> + cd /Users/vinodkone/workspace/mesos
> + DOCKERFILE=Dockerfile
> + rm -f Dockerfile
> + case $OS in
> + append_dockerfile 'FROM ubuntu:14.04'
> + echo FROM ubuntu:14.04
> + append_dockerfile 'RUN rm -rf /var/lib/apt/lists/*'
> + echo RUN rm -rf '/var/lib/apt/lists/*'
> + append_dockerfile 'RUN apt-get update'
> + echo RUN apt-get update
> + append_dockerfile 'RUN apt-get -y install build-essential clang git maven 
> autoconf libtool'
> + echo RUN apt-get -y install build-essential clang git maven autoconf libtool
> + append_dockerfile 'RUN apt-get -y install openjdk-7-jdk python-dev 
> python-boto libcurl4-nss-dev libsasl2-dev libapr1-dev libsvn-dev libevent-dev 
> libev-dev'
> + echo RUN apt-get -y install openjdk-7-jdk python-dev python-boto 
> libcurl4-nss-dev libsasl2-dev libapr1-dev libsvn-dev libevent-dev libev-dev
> + append_dockerfile 'RUN adduser --disabled-password --gecos '\'''\'' mesos'
> + echo RUN adduser --disabled-password --gecos ''\'''\''' mesos
> + append_dockerfile 'ENV GTEST_FILTER -FsTest.FileSystemTableRead'
> + echo ENV GTEST_FILTER -FsTest.FileSystemTableRead
> + append_dockerfile 'ENV GTEST_OUTPUT xml:report.xml'
> + echo ENV GTEST_OUTPUT xml:report.xml
> + case $COMPILER in
> + append_dockerfile 'ENV CC gcc'
> + echo ENV CC gcc
> + append_dockerfile 'ENV CXX g++'
> + echo ENV CXX g++
> + append_dockerfile 'WORKDIR mesos'
> + echo WORKDIR mesos
> + append_dockerfile 'COPY . /mesos/'
> + echo COPY . /mesos/
> + append_dockerfile 'RUN chown -R mesos /mesos'
> + echo RUN chown -R mesos /mesos
> + append_dockerfile 'USER mesos'
> + echo USER mesos
> + append_dockerfile 'CMD ./bootstrap && ./configure --verbose && 
> DISTCHECK_CONFIGURE_FLAGS="--verbose" GLOG_v=1 MESOS_VERBOSE=1 make -j8 
> distcheck'
> + echo CMD ./bootstrap '&&' ./configure --verbose '&&' 
> 'DISTCHECK_CONFIGURE_FLAGS="--verbose"' GLOG_v=1 MESOS_VERBOSE=1 make -j8 
> distcheck
> ++ date +%s
> + TAG=mesos-1453845969-21946
> + docker build --no-cache=true -t mesos-1453845969-21946 .
> ./support/docker_build.sh: line 117: docker: command not found
> Posting review: Bad patch!
> 
> Reviews applied: [40951, 41090, 41092]
> 
> Failed command: ['bash', '-c', 'set -o pipefail; export 
> OS=ubuntu:14.04;export CONFIGURATION="--verbose";export COMPILER=gcc; 
> ./support/docker_build.sh 2>&1 | tee build_41092']
> 
> Error:
>  ..
> + : ubuntu:14.04
> + : gcc
> + : --verbose
> +++ dirname ./support/docker_build.sh
> ++ cd ./support/..
> ++ pwd
> + MESOS_DIRECTORY=/Users/vinodkone/workspace/mesos
> + cd /Users/vinodkone/workspace/mesos
> + DOCKERFILE=Dockerfile
> + rm -f Dockerfile
> + case $OS in
> + append_dockerfile 'FROM ubuntu:14.04'
> + echo FROM ubuntu:14.04
> + append_dockerfile 'RUN rm -rf /var/lib/apt/lists/*'
> + echo RUN rm -rf '/var/lib/apt/lists/*'
> + append_dockerfile 'RUN apt-get update'
> + echo RUN apt-get update
> + append_dockerfile 'RUN apt-get -y install build-essential clang git maven 
> autoconf libtool'
> + echo RUN apt-get -y install build-essential clang git maven autoconf libtool
> + append_dockerfile 'RUN 

Re: Review Request 42773: Added utility function to compute SHA512 digest.

[Jie Yu]

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


Fix it, then Ship it!




I'll fix the remaining issue for you.


src/common/command_utils.cpp (lines 178 - 179)


I would put these two in one line.



src/common/command_utils.cpp (lines 191 - 197)


I would remove this check for now (will keep the TODO here).


- Jie Yu


On Jan. 29, 2016, 12:54 a.m., Jojy Varghese wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42773/
> ---
> 
> (Updated Jan. 29, 2016, 12:54 a.m.)
> 
> 
> Review request for mesos.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added utility function to compute SHA512 digest.
> 
> 
> Diffs
> -
> 
>   src/common/command_utils.hpp bfab34571dd37980d9f99585e506eb015b499e28 
>   src/common/command_utils.cpp e9d782127f948e165f55d03a2b1bf47946cc7f4e 
>   src/tests/common/command_utils_tests.cpp 
> 938f3995aacf74bac28ae11040de292aa328f1e5 
> 
> Diff: https://reviews.apache.org/r/42773/diff/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Jojy Varghese
> 
>

Review Request 42927: Deleted Dockerfile in favor of `support/docker_build.sh`.

[Vinod Kone]

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

Review request for mesos, Anand Mazumdar, Greg Mann, and Kevin Klues.


Repository: mesos


Description
---

No one seems to be using it and this is not maintained anymore.


Diffs
-

  Dockerfile 35abf25aa77ef3f8a68b803eb8ff18ad90a0ff7b 

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


Testing
---


Thanks,

Vinod Kone

Re: Review Request 40936: Windows: Unified POSIX and Windows implementation of `shell.hpp`.

[Daniel Pravat]

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

(Updated Jan. 29, 2016, 12:04 a.m.)


Review request for mesos, Alex Naparu, Alex Clemmer, M Lawindi, and Yi Sun.


Summary (updated)
-

Windows: Unified POSIX and Windows implementation of `shell.hpp`.


Repository: mesos


Description (updated)
---

Windows: Unified POSIX and Windows implementation of `shell.hpp`.


Diffs (updated)
-

  3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/shell.hpp 
b18cae1118302e18d2cfb7ce4089ab5079a01d1a 
  3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/shell.hpp 
bf737c87b8a337cc46e6c16d6fec2eef61e6ea05 
  3rdparty/libprocess/3rdparty/stout/include/stout/posix/os.hpp 
74af0077a39ef4cfa636b0b9e0c6b93eabc04bc8 
  3rdparty/libprocess/3rdparty/stout/include/stout/windows.hpp 
061bb4a9b9efe4ddd364f589175446b07152c315 

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


Testing
---

OSX: make check
Windows: Build


Thanks,

Daniel Pravat

Re: Review Request 42755: Added documentation for labeled reserved resources.

[Qian Zhang]

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


Ship it!




Ship It!

- Qian Zhang


On Jan. 26, 2016, 7:02 a.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42755/
> ---
> 
> (Updated Jan. 26, 2016, 7:02 a.m.)
> 
> 
> Review request for mesos and Michael Park.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added documentation for labeled reserved resources.
> 
> 
> Diffs
> -
> 
>   docs/reservation.md 8d2d33a6518c73542cbfb3a5ee36da1c00c6ff1a 
> 
> Diff: https://reviews.apache.org/r/42755/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 42554: Added 'dependencies' message to AppcImageManifest.

[Jojy Varghese]

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

(Updated Jan. 29, 2016, 12:56 a.m.)


Review request for mesos and Jie Yu.


Changes
---

review addressed


Repository: mesos


Description
---

Added 'dependencies' message to AppcImageManifest.


Diffs (updated)
-

  include/mesos/mesos.proto 96b911fb370223933df52f9370897871827d2247 

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


Testing
---

make check.


Thanks,

Jojy Varghese

Re: Review Request 42554: Added 'dependencies' message to AppcImageManifest.

[Jojy Varghese]

> On Jan. 28, 2016, 10:12 p.m., Jie Yu wrote:
> > it's strange that this protobuf is exposed in Mesos API. Similar to Docker, 
> > can we move this to include/mesos/appc/spec.proto?

Will be working on a separate patch for moving this message to appc/spec.proto.


- Jojy


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


On Jan. 26, 2016, 2:19 a.m., Jojy Varghese wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42554/
> ---
> 
> (Updated Jan. 26, 2016, 2:19 a.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added 'dependencies' message to AppcImageManifest.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 0be4bed336e86a5c377e87ac6212c70ac3b4c66b 
> 
> Diff: https://reviews.apache.org/r/42554/diff/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Jojy Varghese
> 
>

Re: Review Request 42828: Updated ReviewBot to tee build output to a file.

[Kevin Klues]

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




support/verify_reviews.py (line 139)


To avoid using tee, you could just do:

"%s > %s 2>&1" % (command, build_output)


- Kevin Klues


On Jan. 29, 2016, 12:03 a.m., Vinod Kone wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42828/
> ---
> 
> (Updated Jan. 29, 2016, 12:03 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Greg Mann, Kevin Klues, and Michael 
> Park.
> 
> 
> Bugs: MESOS-1469 and MESOS-4478
> https://issues.apache.org/jira/browse/MESOS-1469
> https://issues.apache.org/jira/browse/MESOS-4478
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This fixes 2 issues
> 
> 1) Stream output to the console when build is in progress. 
> https://issues.apache.org/jira/browse/MESOS-1469
> 
> 2) Make sure the full build output (and not just truncated build output) is 
> output to the console. This was a bug introduced while fixing 
> https://issues.apache.org/jira/browse/MESOS-4478
> 
> 
> Diffs
> -
> 
>   support/verify_reviews.py 251d57ec050b087f13b3f5e0d8f421fe8eb76787 
> 
> Diff: https://reviews.apache.org/r/42828/diff/
> 
> 
> Testing
> ---
> 
> ?  mesos git:(vinod/review_bot_tee_output) 
> BUILD_URL=https://builds.apache.org/job/mesos-reviewbot/10980/ 
> ./support/verify_reviews.py vinod kone 1
> git rev-parse HEAD
> Checking if review: 41092 needs verification
> Latest diff timestamp: 2016-01-26 13:53:35
> Verifying review 41092
> Dependent review: https://reviews.apache.org/api/review-requests/41090/ 
> Dependent review: https://reviews.apache.org/api/review-requests/40951/ 
> Applying review 41092
> ./support/apply-review.sh -n -r 41092
> + : ubuntu:14.04
> + : gcc
> + : --verbose
> +++ dirname ./support/docker_build.sh
> ++ cd ./support/..
> ++ pwd
> + MESOS_DIRECTORY=/Users/vinodkone/workspace/mesos
> + cd /Users/vinodkone/workspace/mesos
> + DOCKERFILE=Dockerfile
> + rm -f Dockerfile
> + case $OS in
> + append_dockerfile 'FROM ubuntu:14.04'
> + echo FROM ubuntu:14.04
> + append_dockerfile 'RUN rm -rf /var/lib/apt/lists/*'
> + echo RUN rm -rf '/var/lib/apt/lists/*'
> + append_dockerfile 'RUN apt-get update'
> + echo RUN apt-get update
> + append_dockerfile 'RUN apt-get -y install build-essential clang git maven 
> autoconf libtool'
> + echo RUN apt-get -y install build-essential clang git maven autoconf libtool
> + append_dockerfile 'RUN apt-get -y install openjdk-7-jdk python-dev 
> python-boto libcurl4-nss-dev libsasl2-dev libapr1-dev libsvn-dev libevent-dev 
> libev-dev'
> + echo RUN apt-get -y install openjdk-7-jdk python-dev python-boto 
> libcurl4-nss-dev libsasl2-dev libapr1-dev libsvn-dev libevent-dev libev-dev
> + append_dockerfile 'RUN adduser --disabled-password --gecos '\'''\'' mesos'
> + echo RUN adduser --disabled-password --gecos ''\'''\''' mesos
> + append_dockerfile 'ENV GTEST_FILTER -FsTest.FileSystemTableRead'
> + echo ENV GTEST_FILTER -FsTest.FileSystemTableRead
> + append_dockerfile 'ENV GTEST_OUTPUT xml:report.xml'
> + echo ENV GTEST_OUTPUT xml:report.xml
> + case $COMPILER in
> + append_dockerfile 'ENV CC gcc'
> + echo ENV CC gcc
> + append_dockerfile 'ENV CXX g++'
> + echo ENV CXX g++
> + append_dockerfile 'WORKDIR mesos'
> + echo WORKDIR mesos
> + append_dockerfile 'COPY . /mesos/'
> + echo COPY . /mesos/
> + append_dockerfile 'RUN chown -R mesos /mesos'
> + echo RUN chown -R mesos /mesos
> + append_dockerfile 'USER mesos'
> + echo USER mesos
> + append_dockerfile 'CMD ./bootstrap && ./configure --verbose && 
> DISTCHECK_CONFIGURE_FLAGS="--verbose" GLOG_v=1 MESOS_VERBOSE=1 make -j8 
> distcheck'
> + echo CMD ./bootstrap '&&' ./configure --verbose '&&' 
> 'DISTCHECK_CONFIGURE_FLAGS="--verbose"' GLOG_v=1 MESOS_VERBOSE=1 make -j8 
> distcheck
> ++ date +%s
> + TAG=mesos-1453845969-21946
> + docker build --no-cache=true -t mesos-1453845969-21946 .
> ./support/docker_build.sh: line 117: docker: command not found
> Posting review: Bad patch!
> 
> Reviews applied: [40951, 41090, 41092]
> 
> Failed command: ['bash', '-c', 'set -o pipefail; export 
> OS=ubuntu:14.04;export CONFIGURATION="--verbose";export COMPILER=gcc; 
> ./support/docker_build.sh 2>&1 | tee build_41092']
> 
> Error:
>  ..
> + : ubuntu:14.04
> + : gcc
> + : --verbose
> +++ dirname ./support/docker_build.sh
> ++ cd ./support/..
> ++ pwd
> + MESOS_DIRECTORY=/Users/vinodkone/workspace/mesos
> + cd /Users/vinodkone/workspace/mesos
> + DOCKERFILE=Dockerfile
> + rm -f Dockerfile
> + case $OS in
> + append_dockerfile 'FROM ubuntu:14.04'
> + echo FROM ubuntu:14.04
> + append_dockerfile 'RUN rm -rf /var/lib/apt/lists/*'
> 

Review Request 42929: Updated docker_build.sh to make build environment configurable.

[Vinod Kone]

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

Review request for mesos, Anand Mazumdar, Greg Mann, and Kevin Klues.


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


Repository: mesos


Description
---

This sets up the path for creating a new CI job (ENVIRONMENT="MESOS_BENCHMARK=1 
GTEST_FILTER='*BENCHMARK*'" that runs benchmark tests.


Diffs
-

  support/docker_build.sh da94be87fae98825e60331634259eab0e7a4ebd1 

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


Testing
---

Tested this via a custom CI job on ASF CI.

Note that the benchmark tests need to be reconfigured to not take a long time. 
Currently they get aborted because they ran for > 5 hours.


Thanks,

Vinod Kone

Re: Review Request 42929: Updated docker_build.sh to make build environment configurable.

[Mesos ReviewBot]

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



Bad patch!

Reviews applied: [42927, 42928, 42929]

Failed command: export OS=ubuntu:14.04;export CONFIGURATION="--verbose";export 
COMPILER=gcc; ./support/docker_build.sh

Error:
 ..
+ : ubuntu:14.04
+ : gcc
+ : --verbose
./support/docker_build.sh: line 12: ENVIRONMENT: Environment variable 
'ENVIRONMENT' must be set (e.g., ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1')

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

- Mesos ReviewBot


On Jan. 29, 2016, 12:10 a.m., Vinod Kone wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42929/
> ---
> 
> (Updated Jan. 29, 2016, 12:10 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Greg Mann, and Kevin Klues.
> 
> 
> Bugs: MESOS-4523
> https://issues.apache.org/jira/browse/MESOS-4523
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This sets up the path for creating a new CI job 
> (ENVIRONMENT="MESOS_BENCHMARK=1 GTEST_FILTER='*BENCHMARK*'" that runs 
> benchmark tests.
> 
> 
> Diffs
> -
> 
>   support/docker_build.sh da94be87fae98825e60331634259eab0e7a4ebd1 
> 
> Diff: https://reviews.apache.org/r/42929/diff/
> 
> 
> Testing
> ---
> 
> Tested this via a custom CI job on ASF CI.
> 
> Note that the benchmark tests need to be reconfigured to not take a long 
> time. Currently they get aborted because they ran for > 5 hours.
> 
> 
> Thanks,
> 
> Vinod Kone
> 
>

Re: Review Request 40731: Added a test case for floating point precision of resource allocation.

[Avinash sridharan]

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

(Updated Jan. 29, 2016, 12:42 a.m.)


Review request for mesos, Bernd Mathiske, Klaus Ma, Mandeep Chadha, and Neil 
Conway.


Summary (updated)
-

Added a test case for floating point precision of resource allocation.


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


Repository: mesos


Description (updated)
---

Added a test case for floating point precision of resource allocation.  This 
patch is based on the commit submitted by : Mandeep (@mchadha) 
https://reviews.apache.org/r/39056/


Diffs (updated)
-

  CHANGELOG 82c1be60deca0eba0bcaff0331662df1b26690ef 
  bootstrap d0dd491e99b1e60dfe5c1ada3388e78673112b96 
  bootstrap.bat 584b5c3ca228ff485b22473cd167d3f454d6dad4 
  docs/high-availability-framework-guide.md 
3d429d85edce695f608320cab05cdafdafa4a42e 
  docs/release-guide.md 3a6b7d984222cc20c2d3cd547f74c802b3a24ce9 
  include/mesos/resources.hpp 6bfac2e7e8799e74d87c7570fc5eef320ba76eb1 
  include/mesos/slave/isolator.hpp 4be8c2bb409052e2e07138483408209384f41e23 
  include/mesos/v1/resources.hpp 5a88c0756db2ea8db0f5df7ea3019b511ea135af 
  src/CMakeLists.txt 93caf000f39bbb71a17d9876689afb50cf637e44 
  src/Makefile.am fac17f4bac3b2ddda0384dec8b0ed6f960bd24d1 
  src/common/command_utils.hpp bfab34571dd37980d9f99585e506eb015b499e28 
  src/common/command_utils.cpp e9d782127f948e165f55d03a2b1bf47946cc7f4e 
  src/common/type_utils.cpp 42e3061818c49c6b62d4fee2479db078dfe0fc89 
  src/java/MESOS-MAVEN-README 2812fb14718246371abdb0ff56d0761654c6ebe2 
  src/master/allocator/mesos/hierarchical.cpp 
1a07d69016407e5aad2209586da37fecbcddb765 
  src/master/allocator/sorter/drf/sorter.cpp 
db47d640e36c0302d7c6254a9c58caa878feac01 
  src/master/http.cpp 54264f6e4ad5802ce92eaec7dc33164afe779da5 
  src/master/master.cpp 98441a543611d4083b2495ee103f0ab5e2187e83 
  src/slave/container_loggers/lib_logrotate.cpp 
8d2f89503cd8ace1971933006a46ed4d3936ff8e 
  src/slave/containerizer/mesos/isolator.hpp 
bacd86af42d16cb7c9b6622dfb298dcaa7007b75 
  src/slave/containerizer/mesos/isolator.cpp 
253ff3cea8aff3e7a3051fb5a763cc081f455f18 
  src/slave/containerizer/mesos/isolators/cgroups/net_cls.cpp 
ddc1bf0939e5e8995e6f34fe7b8509b51704f63e 
  src/slave/containerizer/mesos/isolators/posix/disk.hpp 
3998251248ecef2d174f0ea68e0493d09b012952 
  src/slave/containerizer/mesos/isolators/posix/disk.cpp 
fd9b3abfe2c502c69ba7b2f0b2015fe90888d5bc 
  src/slave/resource_estimators/fixed.cpp 
c858a48bc137185d1e1e24a20f6b75b0dd7912ff 
  src/tests/common/command_utils_tests.cpp 
938f3995aacf74bac28ae11040de292aa328f1e5 
  src/tests/container_logger_tests.cpp e161fd671972d365a25a5f2e238e11815e574164 
  src/tests/containerizer/isolator_tests.cpp 
8d101df957fd36adac388310eddba2db1f98c029 
  src/tests/disk_quota_tests.cpp 413e562026a4fc9779f616e921ae2fa2ca51e012 
  src/tests/master_tests.cpp ce6ce25a03cdb0883612fe40b20996ec2e50de40 
  src/tests/reservation_tests.cpp d0c88560a5618ebec39fc4e0539ddb2d9ca554a2 
  src/tests/resources_tests.cpp 4b25e82c13e4f46c73803f773db90f269c09c48a 
  src/tests/status_update_manager_tests.cpp 
7bedd499a241a61938069381e0d4fafa4b8f96db 
  src/v1/mesos.cpp 21b14cf7050270facab1ba2b844c3fde664665a4 
  src/v1/resources.cpp be4a5d153e9313cb71a6e85d1ed25a358537f2b7 
  support/gitignore  

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


Testing
---

Ran make check after adding Mandeep's test case to the GTEST framework.


Thanks,

Avinash sridharan

Re: Review Request 42928: Updated docker_build.sh to generate xml output for all OSes.

[Kevin Klues]

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


Ship it!




Ship It!

- Kevin Klues


On Jan. 29, 2016, 12:06 a.m., Vinod Kone wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42928/
> ---
> 
> (Updated Jan. 29, 2016, 12:06 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Greg Mann, and Kevin Klues.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> xml output was originally only enabled for ubuntu. This enables it for all 
> OSes.
> 
> 
> Diffs
> -
> 
>   support/docker_build.sh da94be87fae98825e60331634259eab0e7a4ebd1 
> 
> Diff: https://reviews.apache.org/r/42928/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Vinod Kone
> 
>

Re: Review Request 42828: Updated ReviewBot to tee build output to a file.

[Anand Mazumdar]

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



LGTM. Just one minor query about what happens to the piped file and if we 
should delete it too.


support/verify_reviews.py (line 122)


Minor Nit: More pythonic version:

```
"build_{}".format(review_request["id"])
```



support/verify_reviews.py (line 137)


Nit: Can we wrap this at 80 chars? Not sure if we follow the style 
guidelines from C++ here too.



support/verify_reviews.py (line 148)


Ditto as above.



support/verify_reviews.py (line 150)


hmm .. what happens to the build_output file thereafter? Should we be 
cleaning it up too?


- Anand Mazumdar


On Jan. 29, 2016, 12:03 a.m., Vinod Kone wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42828/
> ---
> 
> (Updated Jan. 29, 2016, 12:03 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Greg Mann, Kevin Klues, and Michael 
> Park.
> 
> 
> Bugs: MESOS-1469 and MESOS-4478
> https://issues.apache.org/jira/browse/MESOS-1469
> https://issues.apache.org/jira/browse/MESOS-4478
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This fixes 2 issues
> 
> 1) Stream output to the console when build is in progress. 
> https://issues.apache.org/jira/browse/MESOS-1469
> 
> 2) Make sure the full build output (and not just truncated build output) is 
> output to the console. This was a bug introduced while fixing 
> https://issues.apache.org/jira/browse/MESOS-4478
> 
> 
> Diffs
> -
> 
>   support/verify_reviews.py 251d57ec050b087f13b3f5e0d8f421fe8eb76787 
> 
> Diff: https://reviews.apache.org/r/42828/diff/
> 
> 
> Testing
> ---
> 
> ?  mesos git:(vinod/review_bot_tee_output) 
> BUILD_URL=https://builds.apache.org/job/mesos-reviewbot/10980/ 
> ./support/verify_reviews.py vinod kone 1
> git rev-parse HEAD
> Checking if review: 41092 needs verification
> Latest diff timestamp: 2016-01-26 13:53:35
> Verifying review 41092
> Dependent review: https://reviews.apache.org/api/review-requests/41090/ 
> Dependent review: https://reviews.apache.org/api/review-requests/40951/ 
> Applying review 41092
> ./support/apply-review.sh -n -r 41092
> + : ubuntu:14.04
> + : gcc
> + : --verbose
> +++ dirname ./support/docker_build.sh
> ++ cd ./support/..
> ++ pwd
> + MESOS_DIRECTORY=/Users/vinodkone/workspace/mesos
> + cd /Users/vinodkone/workspace/mesos
> + DOCKERFILE=Dockerfile
> + rm -f Dockerfile
> + case $OS in
> + append_dockerfile 'FROM ubuntu:14.04'
> + echo FROM ubuntu:14.04
> + append_dockerfile 'RUN rm -rf /var/lib/apt/lists/*'
> + echo RUN rm -rf '/var/lib/apt/lists/*'
> + append_dockerfile 'RUN apt-get update'
> + echo RUN apt-get update
> + append_dockerfile 'RUN apt-get -y install build-essential clang git maven 
> autoconf libtool'
> + echo RUN apt-get -y install build-essential clang git maven autoconf libtool
> + append_dockerfile 'RUN apt-get -y install openjdk-7-jdk python-dev 
> python-boto libcurl4-nss-dev libsasl2-dev libapr1-dev libsvn-dev libevent-dev 
> libev-dev'
> + echo RUN apt-get -y install openjdk-7-jdk python-dev python-boto 
> libcurl4-nss-dev libsasl2-dev libapr1-dev libsvn-dev libevent-dev libev-dev
> + append_dockerfile 'RUN adduser --disabled-password --gecos '\'''\'' mesos'
> + echo RUN adduser --disabled-password --gecos ''\'''\''' mesos
> + append_dockerfile 'ENV GTEST_FILTER -FsTest.FileSystemTableRead'
> + echo ENV GTEST_FILTER -FsTest.FileSystemTableRead
> + append_dockerfile 'ENV GTEST_OUTPUT xml:report.xml'
> + echo ENV GTEST_OUTPUT xml:report.xml
> + case $COMPILER in
> + append_dockerfile 'ENV CC gcc'
> + echo ENV CC gcc
> + append_dockerfile 'ENV CXX g++'
> + echo ENV CXX g++
> + append_dockerfile 'WORKDIR mesos'
> + echo WORKDIR mesos
> + append_dockerfile 'COPY . /mesos/'
> + echo COPY . /mesos/
> + append_dockerfile 'RUN chown -R mesos /mesos'
> + echo RUN chown -R mesos /mesos
> + append_dockerfile 'USER mesos'
> + echo USER mesos
> + append_dockerfile 'CMD ./bootstrap && ./configure --verbose && 
> DISTCHECK_CONFIGURE_FLAGS="--verbose" GLOG_v=1 MESOS_VERBOSE=1 make -j8 
> distcheck'
> + echo CMD ./bootstrap '&&' ./configure --verbose '&&' 
> 'DISTCHECK_CONFIGURE_FLAGS="--verbose"' GLOG_v=1 MESOS_VERBOSE=1 make -j8 
> distcheck
> ++ date +%s
> + TAG=mesos-1453845969-21946
> + docker build --no-cache=true -t mesos-1453845969-21946 .
> ./support/docker_build.sh: line 117: docker: command not found
> Posting review: Bad patch!
> 
> Reviews applied: [40951, 41090, 41092]
> 

Re: Review Request 42928: Updated docker_build.sh to generate xml output for all OSes.

[Anand Mazumdar]

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


Ship it!




Can we also set up the Jenkins xUnit Plugin to find the XML files we are now 
generating? AFAICT, we don't seem to be doing that currently.

- Anand Mazumdar


On Jan. 29, 2016, 12:06 a.m., Vinod Kone wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42928/
> ---
> 
> (Updated Jan. 29, 2016, 12:06 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Greg Mann, and Kevin Klues.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> xml output was originally only enabled for ubuntu. This enables it for all 
> OSes.
> 
> 
> Diffs
> -
> 
>   support/docker_build.sh da94be87fae98825e60331634259eab0e7a4ebd1 
> 
> Diff: https://reviews.apache.org/r/42928/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Vinod Kone
> 
>

Re: Review Request 42929: Updated docker_build.sh to make build environment configurable.

[Anand Mazumdar]

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


Ship it!




LGTM. Looks like the review bot vaidated that the `Environment` stuff works 
already ;-)

- Anand Mazumdar


On Jan. 29, 2016, 12:10 a.m., Vinod Kone wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42929/
> ---
> 
> (Updated Jan. 29, 2016, 12:10 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Greg Mann, and Kevin Klues.
> 
> 
> Bugs: MESOS-4523
> https://issues.apache.org/jira/browse/MESOS-4523
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This sets up the path for creating a new CI job 
> (ENVIRONMENT="MESOS_BENCHMARK=1 GTEST_FILTER='*BENCHMARK*'" that runs 
> benchmark tests.
> 
> 
> Diffs
> -
> 
>   support/docker_build.sh da94be87fae98825e60331634259eab0e7a4ebd1 
> 
> Diff: https://reviews.apache.org/r/42929/diff/
> 
> 
> Testing
> ---
> 
> Tested this via a custom CI job on ASF CI.
> 
> Note that the benchmark tests need to be reconfigured to not take a long 
> time. Currently they get aborted because they ran for > 5 hours.
> 
> 
> Thanks,
> 
> Vinod Kone
> 
>

Re: Review Request 40731: Added a test case for floating point precision of resource allocation.

[Avinash sridharan]

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

(Updated Jan. 29, 2016, 12:45 a.m.)


Review request for mesos, Bernd Mathiske, Klaus Ma, Mandeep Chadha, and Neil 
Conway.


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


Repository: mesos


Description
---

Added a test case for floating point precision of resource allocation.  This 
patch is based on the commit submitted by : Mandeep (@mchadha) 
https://reviews.apache.org/r/39056/


Diffs (updated)
-

  src/tests/reservation_tests.cpp d0c88560a5618ebec39fc4e0539ddb2d9ca554a2 

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


Testing
---

Ran make check after adding Mandeep's test case to the GTEST framework.


Thanks,

Avinash sridharan

Re: Review Request 42773: Added utility function to compute SHA512 digest.

[Jojy Varghese]

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

(Updated Jan. 29, 2016, 12:54 a.m.)


Review request for mesos.


Changes
---

review addressed.


Repository: mesos


Description
---

Added utility function to compute SHA512 digest.


Diffs (updated)
-

  src/common/command_utils.hpp bfab34571dd37980d9f99585e506eb015b499e28 
  src/common/command_utils.cpp e9d782127f948e165f55d03a2b1bf47946cc7f4e 
  src/tests/common/command_utils_tests.cpp 
938f3995aacf74bac28ae11040de292aa328f1e5 

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


Testing
---

make check.


Thanks,

Jojy Varghese

Re: Review Request 42929: Updated docker_build.sh to make build environment configurable.

[Kevin Klues]

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


Ship it!




Ship It!

- Kevin Klues


On Jan. 29, 2016, 12:10 a.m., Vinod Kone wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42929/
> ---
> 
> (Updated Jan. 29, 2016, 12:10 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Greg Mann, and Kevin Klues.
> 
> 
> Bugs: MESOS-4523
> https://issues.apache.org/jira/browse/MESOS-4523
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This sets up the path for creating a new CI job 
> (ENVIRONMENT="MESOS_BENCHMARK=1 GTEST_FILTER='*BENCHMARK*'" that runs 
> benchmark tests.
> 
> 
> Diffs
> -
> 
>   support/docker_build.sh da94be87fae98825e60331634259eab0e7a4ebd1 
> 
> Diff: https://reviews.apache.org/r/42929/diff/
> 
> 
> Testing
> ---
> 
> Tested this via a custom CI job on ASF CI.
> 
> Note that the benchmark tests need to be reconfigured to not take a long 
> time. Currently they get aborted because they ran for > 5 hours.
> 
> 
> Thanks,
> 
> Vinod Kone
> 
>

Re: Review Request 42927: Deleted Dockerfile in favor of `support/docker_build.sh`.

[Kevin Klues]

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


Ship it!




Ship It!

- Kevin Klues


On Jan. 29, 2016, 12:05 a.m., Vinod Kone wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42927/
> ---
> 
> (Updated Jan. 29, 2016, 12:05 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Greg Mann, and Kevin Klues.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> No one seems to be using it and this is not maintained anymore.
> 
> 
> Diffs
> -
> 
>   Dockerfile 35abf25aa77ef3f8a68b803eb8ff18ad90a0ff7b 
> 
> Diff: https://reviews.apache.org/r/42927/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Vinod Kone
> 
>

Re: Review Request 42755: Added documentation for labeled reserved resources.

[Qian Zhang]

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




docs/reservation.md (lines 80 - 81)


I understand the two reservations with different labels will not be 
combined. But what about the two reservation with same label? Do we need to 
describe its behavior here too?

I think the latter is a bit complex, e.g., one reservation made by 
framework with a label and another reservation made by operator with the same 
label. Will we combine them into one reservation? In this case, the principle 
of the two reservations could be different, right? So we will end up with two 
separate reservations with same label?


- Qian Zhang


On Jan. 26, 2016, 7:02 a.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42755/
> ---
> 
> (Updated Jan. 26, 2016, 7:02 a.m.)
> 
> 
> Review request for mesos and Michael Park.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added documentation for labeled reserved resources.
> 
> 
> Diffs
> -
> 
>   docs/reservation.md 8d2d33a6518c73542cbfb3a5ee36da1c00c6ff1a 
> 
> Diff: https://reviews.apache.org/r/42755/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 42554: Added 'dependencies' message to AppcImageManifest.

[Jojy Varghese]

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

(Updated Jan. 29, 2016, 1:27 a.m.)


Review request for mesos and Jie Yu.


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


Repository: mesos


Description
---

Added 'dependencies' message to AppcImageManifest.


Diffs
-

  include/mesos/mesos.proto 96b911fb370223933df52f9370897871827d2247 

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


Testing
---

make check.


Thanks,

Jojy Varghese

Re: Review Request 42929: Updated docker_build.sh to make build environment configurable.

[Vinod Kone]

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

(Updated Jan. 29, 2016, 1:32 a.m.)


Review request for mesos, Anand Mazumdar, Greg Mann, and Kevin Klues.


Changes
---

Updated verify_reviews.py too. Thanks ReviewBot!


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


Repository: mesos


Description (updated)
---

Updated docker_build.sh to make build environment configurable.


Diffs (updated)
-

  support/docker_build.sh da94be87fae98825e60331634259eab0e7a4ebd1 
  support/verify_reviews.py 251d57ec050b087f13b3f5e0d8f421fe8eb76787 

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


Testing
---

Tested this via a custom CI job on ASF CI.

Note that the benchmark tests need to be reconfigured to not take a long time. 
Currently they get aborted because they ran for > 5 hours.


Thanks,

Vinod Kone

Re: Review Request 42929: Updated docker_build.sh to make build environment configurable.

[Anand Mazumdar]

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


Ship it!




Ship It!

- Anand Mazumdar


On Jan. 29, 2016, 1:32 a.m., Vinod Kone wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42929/
> ---
> 
> (Updated Jan. 29, 2016, 1:32 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Greg Mann, and Kevin Klues.
> 
> 
> Bugs: MESOS-4523
> https://issues.apache.org/jira/browse/MESOS-4523
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated docker_build.sh to make build environment configurable.
> 
> 
> Diffs
> -
> 
>   support/docker_build.sh da94be87fae98825e60331634259eab0e7a4ebd1 
>   support/verify_reviews.py 251d57ec050b087f13b3f5e0d8f421fe8eb76787 
> 
> Diff: https://reviews.apache.org/r/42929/diff/
> 
> 
> Testing
> ---
> 
> Tested this via a custom CI job on ASF CI.
> 
> Note that the benchmark tests need to be reconfigured to not take a long 
> time. Currently they get aborted because they ran for > 5 hours.
> 
> 
> Thanks,
> 
> Vinod Kone
> 
>

Re: Review Request 42362: Added persistent volume endpoint test without authentication.

[haosdent huang]

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


Ship it!




Ship It!

- haosdent huang


On Jan. 28, 2016, 5:59 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42362/
> ---
> 
> (Updated Jan. 28, 2016, 5:59 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Kapil Arya, and Neil Conway.
> 
> 
> Bugs: MESOS-4395
> https://issues.apache.org/jira/browse/MESOS-4395
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added persistent volume endpoint tests with HTTP authentication disabled.
> 
> The persistent volume endpoint tests allow volume creation and destruction 
> when HTTP authentication is disabled; this patch introduces a test for this 
> scenario: `PersistentVolumeEndpointsTest.NoAuthentication`.
> 
> 
> Diffs
> -
> 
>   src/tests/persistent_volume_endpoints_tests.cpp 
> 22e18758ee91a649486725473d9e50fae9d43b01 
> 
> Diff: https://reviews.apache.org/r/42362/diff/
> 
> 
> Testing
> ---
> 
> A new test, `PersistentVolumeEndpointsTest.NoAuthentication`, was added to 
> the persistent volume endpoint tests.
> 
> `make check` was used to test, and the new test was run with 
> `--gtest_repeat=1000 -gtest_break_on_failure=1`.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 42673: Silenced two more GMock warnings about shutdown expectations.

[Timothy Chen]

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


Ship it!




Ship It!

- Timothy Chen


On Jan. 22, 2016, 11:38 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42673/
> ---
> 
> (Updated Jan. 22, 2016, 11:38 p.m.)
> 
> 
> Review request for mesos and Timothy Chen.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> In these tests:
> * MasterTest.OrphanTasks
> * StatusUpdateManagerTest.LatestTaskState
> 
> 
> Diffs
> -
> 
>   src/tests/master_tests.cpp cd426fda633c2d665b9533271610863062b1ed4e 
>   src/tests/status_update_manager_tests.cpp 
> 24302d8627e635f3825a31d5383de2448ad9d557 
> 
> Diff: https://reviews.apache.org/r/42673/diff/
> 
> 
> Testing
> ---
> 
> Verified that GMock warnings are observed w/o this review; after applying the 
> review and running `--gtest_repeat=100`, no warnings were observed.
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 40731: Added a fixture to test the floating point precision during CPU resource allocation.

[Mesos ReviewBot]

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



Bad patch!

Reviews applied: [40731]

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

Error:
 ..
2016-01-28 08:01:29 URL:https://reviews.apache.org/r/40731/diff/raw/ 
[4029/4029] -> "40731.patch" [1]
Total errors found: 0
Checking 1 files
Error: Commit message summary (the first line) must not exceed 72 characters.

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

- Mesos ReviewBot


On Jan. 28, 2016, 2:20 a.m., Avinash sridharan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40731/
> ---
> 
> (Updated Jan. 28, 2016, 2:20 a.m.)
> 
> 
> Review request for mesos, Bernd Mathiske, Klaus Ma, Mandeep Chadha, and Neil 
> Conway.
> 
> 
> Bugs: MESOS-3552
> https://issues.apache.org/jira/browse/MESOS-3552
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added a fixture to test the floating point precision during CPU resource 
> allocation. This patch is based on the commit submitted by : Mandeep 
> (@mchadha) https://reviews.apache.org/r/39056/
> 
> 
> Diffs
> -
> 
>   src/tests/reservation_tests.cpp d0c88560a5618ebec39fc4e0539ddb2d9ca554a2 
> 
> Diff: https://reviews.apache.org/r/40731/diff/
> 
> 
> Testing
> ---
> 
> Ran make check after adding Mandeep's test case to the GTEST framework.
> 
> 
> Thanks,
> 
> Avinash sridharan
> 
>

Re: Review Request 41892: DockerContinerizer infers hostPath for persistent volumes.

[Guangya Liu]

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




src/docker/docker.cpp (lines 20 - 29)


Not yours but I think that we should also adjust the order here:

#include 
#include 
#include 

#include 
#include 
#include 
#include 

#include 



src/docker/docker.cpp (line 31)


Should put this to line 20



src/docker/executor.hpp (lines 66 - 70)


The configuration.md should also be updated.


- Guangya Liu


On Jan. 4, 2016, 9:15 p.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41892/
> ---
> 
> (Updated Jan. 4, 2016, 9:15 p.m.)
> 
> 
> Review request for mesos, haosdent huang and Jie Yu.
> 
> 
> Bugs: MESOS-3413
> https://issues.apache.org/jira/browse/MESOS-3413
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This diff makes DockerContainerizer infers hostPath for persistent volumes 
> from resources passed-in.
> 
> This allows current DockerContainerizer to use persistent volumes w/o hard 
> code slave's work_dir. I also checked that the inferred directory actually 
> exists to avoid framework messing up role or persistent id.
> 
> Note that some validation on the resource part should be done before we start 
> the container, which seems to belong to slave.
> 
> 
> Diffs
> -
> 
>   src/docker/docker.hpp dde2b29deda7e40929e0169935a5bafdd43136b1 
>   src/docker/docker.cpp e93280735f1c5f66c765fcbabfd3e50b46c024e8 
>   src/docker/executor.hpp abbc419533ab40312e917931a2fc2ce78b38da41 
>   src/docker/executor.cpp 7512d07de6c8324340e6b5f3e5162ef00efc47fc 
>   src/slave/containerizer/docker.cpp aacf90f2cb6c08f94340936d29b2df513ac9825a 
>   src/tests/containerizer/docker_containerizer_tests.cpp 
> cb58b7183c36d96b9ac4803c63980c278a50c97b 
>   src/tests/containerizer/docker_tests.cpp 
> 83eceacaddc38d0ccfc42e65e700a09406d8df36 
>   src/tests/mesos.hpp a4811b0d8dee33ff2ca4968f532ce64b7ea95249 
>   src/tests/mesos.cpp f4b0f82449c4b6a2b4b7b7f14518714485d5a13a 
> 
> Diff: https://reviews.apache.org/r/41892/diff/
> 
> 
> Testing
> ---
> 
> New unit test.
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>

Re: Review Request 41892: DockerContinerizer infers hostPath for persistent volumes.

[Guangya Liu]

> On Jan. 15, 2016, 12:03 a.m., Zhitao Li wrote:
> > src/docker/docker.cpp, lines 410-420
> > 
> >
> > (Sorry I just got time to come back to this).
> > 
> > I don't exactly understand your suggestion about "add additional 
> > containerInfo.volumes() in 
> > DockerContainerizerProcess::Container::create()", because the latter 
> > function actually does not pass a `ContainerInfo`.
> > 
> > Because the volume is actually included in 
> > `TaskInfo::ContainerInfo::volumes`, we would need to mutate the `TaskInfo` 
> > before passing it to the `DockerExecutorProcess::launchTask()` function, 
> > which I don't really know how to do it, and I even doubt whether it's a 
> > good idea for logging/clarity purpose.
> > 
> > Please let me know if I didn't understand your suggestion, or if you 
> > think we should explore the other alternative (passing `hostPath` earlier 
> > in resource offer).
> 
> Jie Yu wrote:
> Sorry for being late on this.
> 
> My sugguestion is that we add some logics in 
> DockerContainerProcess::Container::create(), the 'create' function has both 
> TaskINfo and ExecutorInfo. So, you should be able to get the ContainerInfo 
> from them. When you generate DockerContainerizerProcess::Container, you 
> should be able to generate a new ContainerInfo that has persistent volumes in 
> it. Let me know if you still don't understand.
> 
> FYI, docker just merged its mount propagation support for volumes. That 
> means we can have full persistent volume support. We need to mount the 
> sandbox as a shared mount into the docker container. ANy mounts under the 
> sandbox mount will be automatically propergated to the docker container.
> 
> Zhitao Li wrote:
> @jieyu, I tried your recommendation and managed to generate a different 
> `ContainerInfo` object inside `DockerContainerProcess::Container::create()`. 
> However, I don't think it could work either because the actual docker 
> container is created by the call sequence `DockerExecutor::launchTask()` -> 
> `Docker::run()` in the forked executor subprocess, and it seems impossible to 
> properly pass the modified `ContainerInfo` to that.
> 
> My proposal is:
> - 1) keep the `persistent_volumes_root` flag to the executor;
> - 2) implement this logic inside `DockerExecutor::launchTask()` (see my 
> other inline comment for exact location) so we don't need to change `Docker` 
> class;
> - 3) for testing, create a test case in docker_containerizer_tests.cpp 
> which creates persistent volume and launch an executor.
> 
> I'll rebase and update this diff with the above proposal.

The `DockerContainerProcess::Container::create()` only include `TaskInfo` when 
using a `command executor`, otherwise, the 
`DockerContainerProcess::Container::create()` will not include task info. For 
this case it is not `command executor`, so I think that @Zhitao's solution 
should be the right choice. Please correct me if there's sth wrong, thanks.


- Guangya


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


On Jan. 4, 2016, 9:15 p.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41892/
> ---
> 
> (Updated Jan. 4, 2016, 9:15 p.m.)
> 
> 
> Review request for mesos, haosdent huang and Jie Yu.
> 
> 
> Bugs: MESOS-3413
> https://issues.apache.org/jira/browse/MESOS-3413
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This diff makes DockerContainerizer infers hostPath for persistent volumes 
> from resources passed-in.
> 
> This allows current DockerContainerizer to use persistent volumes w/o hard 
> code slave's work_dir. I also checked that the inferred directory actually 
> exists to avoid framework messing up role or persistent id.
> 
> Note that some validation on the resource part should be done before we start 
> the container, which seems to belong to slave.
> 
> 
> Diffs
> -
> 
>   src/docker/docker.hpp dde2b29deda7e40929e0169935a5bafdd43136b1 
>   src/docker/docker.cpp e93280735f1c5f66c765fcbabfd3e50b46c024e8 
>   src/docker/executor.hpp abbc419533ab40312e917931a2fc2ce78b38da41 
>   src/docker/executor.cpp 7512d07de6c8324340e6b5f3e5162ef00efc47fc 
>   src/slave/containerizer/docker.cpp aacf90f2cb6c08f94340936d29b2df513ac9825a 
>   src/tests/containerizer/docker_containerizer_tests.cpp 
> cb58b7183c36d96b9ac4803c63980c278a50c97b 
>   src/tests/containerizer/docker_tests.cpp 
> 83eceacaddc38d0ccfc42e65e700a09406d8df36 
>   src/tests/mesos.hpp a4811b0d8dee33ff2ca4968f532ce64b7ea95249 
>   src/tests/mesos.cpp 

Re: Review Request 41950: Cleaned up hierarchical allocator tests.

[Alexander Rukletsov]

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

(Updated Jan. 28, 2016, 1:12 p.m.)


Review request for mesos, Bernd Mathiske, Ben Mahler, Joerg Schad, and Joris 
Van Remoortere.


Changes
---

Rebased.


Repository: mesos


Description
---

Changes made:
- empty resource map promoted to a const class field;
- removed variable numeric suffixes where appropriate;
- added const where appropriate.


Diffs (updated)
-

  src/tests/hierarchical_allocator_tests.cpp 
f18e6eb10572b0f5b8bbff338384d9406f6ad62b 

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


Testing
---

On Mac OS 10.10.4:

`make check` 

`GTEST_FILTER="HierarchicalAllocatorTest.*" ./bin/mesos-tests.sh 
--gtest_repeat=100 --gtest_break_on_failure --gtest_shuffle`


Thanks,

Alexander Rukletsov

Re: Review Request 42672: Explicitly checked for the absence of allocation.

[Alexander Rukletsov]

> On Jan. 23, 2016, 6:31 a.m., Guangya Liu wrote:
> > src/tests/hierarchical_allocator_tests.cpp, line 1494
> > 
> >
> > remove the blank line?

Which line do you mean?


- Alexander


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


On Jan. 22, 2016, 11:21 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42672/
> ---
> 
> (Updated Jan. 22, 2016, 11:21 p.m.)
> 
> 
> Review request for mesos and Ben Mahler.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Ensure the future for allocation is pending when there should be no
> allocations. Reduce the number of allocations triggered in the test.
> 
> 
> Diffs
> -
> 
>   src/tests/hierarchical_allocator_tests.cpp 
> b1cb955b7eb1213c7ba4a9c5181545bb49154f06 
> 
> Diff: https://reviews.apache.org/r/42672/diff/
> 
> 
> Testing
> ---
> 
> On Mac OS 10.10.4:
> 
> `make check`
> 
> `GTEST_FILTER="HierarchicalAllocatorTest.*" ./bin/mesos-tests.sh 
> --gtest_repeat=100 --gtest_break_on_failure --gtest_shuffle`
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Review Request 42908: Fixed a flaky test in quota tests.

[Alexander Rukletsov]

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

Review request for mesos, Michael Park and Qian Zhang.


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


Repository: mesos


Description
---

The `AvailableResourcesAfterRescinding` test became flaky after we
stopped offering unreserved resources beyond quota in
https://reviews.apache.org/r/42835. Hence the allocator offers
rescinded resources to `framework1` if an allocation happens before
the test finishes, which violates the expectation that `framework1`
receives resources only once. Since we do not really care about
allocations in this test but rather about rescinded resources, the
fix is just to ignore subsequent offers to `framework1`.


Diffs
-

  src/tests/master_quota_tests.cpp 04efcf3362d3594e0ad8077793fa1f32536dd658 

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


Testing
---

On Mac OS 10.10.4:
`GTEST_FILTER="MasterQuotaTest.AvailableResourcesAfterRescinding" 
./bin/mesos-tests.sh --gtest_shuffle --gtest_break_on_failure 
--gtest_repeat=5000`


Thanks,

Alexander Rukletsov

Review Request 42910: Added a note about revocable resources beyond quota in the user doc.

[Alexander Rukletsov]

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

Review request for mesos, Joris Van Remoortere and Michael Park.


Repository: mesos


Description
---

See summary.


Diffs
-

  docs/quota.md 2762557ad1cec0655a27c4dda4016e07c7f63cd9 

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


Testing
---

None.


Thanks,

Alexander Rukletsov

Re: Review Request 42633: Corrected a comment in the allocator.

[Alexander Rukletsov]

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

(Updated Jan. 28, 2016, 1:07 p.m.)


Review request for mesos, Ben Mahler and Qian Zhang.


Changes
---

Rebased.


Repository: mesos


Description
---

Revocable resources may come from different sources: usage slack (aka
oversubscribed resources), allocation slack (aka optimistic offers).
We should not use "oversubscribed" and "revocable" interchangeably.


Diffs (updated)
-

  src/master/allocator/mesos/hierarchical.hpp 
1f0c440565ee0f9926d168734df30fd740a53f88 

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


Testing
---

None: not a functional change.


Thanks,

Alexander Rukletsov

Re: Review Request 42658: Restructured comments in allocator tests for clarity.

[Alexander Rukletsov]

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

(Updated Jan. 28, 2016, 1:11 p.m.)


Review request for mesos and Ben Mahler.


Changes
---

Rebased.


Repository: mesos


Description
---

Do not describe cluster state in advance to avoid confusion,
rather right after the state changes.


Diffs (updated)
-

  src/tests/hierarchical_allocator_tests.cpp 
f18e6eb10572b0f5b8bbff338384d9406f6ad62b 

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


Testing
---

None: not a functional change.


Thanks,

Alexander Rukletsov

Re: Review Request 42672: Explicitly checked for the absence of allocation.

[Alexander Rukletsov]

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

(Updated Jan. 28, 2016, 1:22 p.m.)


Review request for mesos and Ben Mahler.


Changes
---

Reabsed.


Repository: mesos


Description
---

Ensure the future for allocation is pending when there should be no
allocations. Reduce the number of allocations triggered in the test.


Diffs (updated)
-

  src/tests/hierarchical_allocator_tests.cpp 
f18e6eb10572b0f5b8bbff338384d9406f6ad62b 

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


Testing
---

On Mac OS 10.10.4:

`make check`

`GTEST_FILTER="HierarchicalAllocatorTest.*" ./bin/mesos-tests.sh 
--gtest_repeat=100 --gtest_break_on_failure --gtest_shuffle`


Thanks,

Alexander Rukletsov

Re: Review Request 42657: Corrected a typo in allocator tests.

[Alexander Rukletsov]

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

(Updated Jan. 28, 2016, 1:08 p.m.)


Review request for mesos and Ben Mahler.


Changes
---

Reabsed.


Repository: mesos


Description
---

See summary.


Diffs (updated)
-

  src/tests/hierarchical_allocator_tests.cpp 
f18e6eb10572b0f5b8bbff338384d9406f6ad62b 

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


Testing
---

None: not a functional change.


Thanks,

Alexander Rukletsov

Re: Review Request 42636: Replaced term 'periodic allocation' for consistency.

[Alexander Rukletsov]

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

(Updated Jan. 28, 2016, 1:08 p.m.)


Review request for mesos and Ben Mahler.


Changes
---

Rebased.


Repository: mesos


Description
---

Though "periodic" may be a better name, we refer to such allocation
cycles as "batch" in the hierarchical allocator and some other tests.
For consistency, rename all occurrences in the code.


Diffs (updated)
-

  src/tests/hierarchical_allocator_tests.cpp 
f18e6eb10572b0f5b8bbff338384d9406f6ad62b 

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


Testing
---

None: not a functional change.


Thanks,

Alexander Rukletsov

Review Request 42911: Removed extra blank line.

[Alexander Rukletsov]

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

Review request for mesos and Joris Van Remoortere.


Repository: mesos


Description
---

Removed extra blank line.


Diffs
-

  src/master/quota.cpp e7ce0f1fc1c83b2ec30185db005d2671a93baed6 

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


Testing
---

None


Thanks,

Alexander Rukletsov

Re: Review Request 42910: Added a note about revocable resources beyond quota in the user doc.

[Guangya Liu]

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




docs/quota.md (line 279)


I think that the current logic is "A quota'ed role is only allocated the 
reserved resources beyond its quota"? Because I see that the current logic is 
both non reserved and non revocable which should count in non reserved 
resources are not allocated. 


https://github.com/apache/mesos/blob/master/src/master/allocator/mesos/hierarchical.cpp#L1346-L1349


- Guangya Liu


On 一月 28, 2016, 1:07 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42910/
> ---
> 
> (Updated 一月 28, 2016, 1:07 p.m.)
> 
> 
> Review request for mesos, Joris Van Remoortere and Michael Park.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   docs/quota.md 2762557ad1cec0655a27c4dda4016e07c7f63cd9 
> 
> Diff: https://reviews.apache.org/r/42910/diff/
> 
> 
> Testing
> ---
> 
> None.
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 42901: Fixed a few typos in the HA framework guide.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [42901]

Passed command: export OS=ubuntu:14.04;export CONFIGURATION="--verbose";export 
COMPILER=gcc; ./support/docker_build.sh

- Mesos ReviewBot


On Jan. 28, 2016, 7:26 a.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42901/
> ---
> 
> (Updated Jan. 28, 2016, 7:26 a.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed a few typos in the HA framework guide.
> 
> 
> Diffs
> -
> 
>   docs/high-availability-framework-guide.md 
> 3d429d85edce695f608320cab05cdafdafa4a42e 
> 
> Diff: https://reviews.apache.org/r/42901/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 42362: Added persistent volume endpoint test without authentication.

[Alexander Rukletsov]

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


Fix it, then Ship it!





src/tests/persistent_volume_endpoints_tests.cpp (line 1085)


I think this comment should say something about disabling authn rather then 
creating a master. Moreover, though it's not the way it's done in legacy tests, 
I would omit a self explanatory comment about creating a master here.



src/tests/persistent_volume_endpoints_tests.cpp (line 1087)


Does it make sense to add `masterFlags.credentials = None();`?



src/tests/persistent_volume_endpoints_tests.cpp (line 1094)


s/a slave/an agent



src/tests/persistent_volume_endpoints_tests.cpp (line 1099)


s/slaveId/agentId

and everywhere if you decide to abolish slavery in this test : ).



src/tests/persistent_volume_endpoints_tests.cpp (line 1108)


Let's extract "role1" into a constant in the beginning of the test.



src/tests/persistent_volume_endpoints_tests.cpp (line 1120)


To be 100% correct, you should `ASSERT_READY(slaveId);` before.


- Alexander Rukletsov


On Jan. 27, 2016, 11:04 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42362/
> ---
> 
> (Updated Jan. 27, 2016, 11:04 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Kapil Arya, and Neil Conway.
> 
> 
> Bugs: MESOS-4395
> https://issues.apache.org/jira/browse/MESOS-4395
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added persistent volume endpoint tests with HTTP authentication disabled.
> 
> The persistent volume endpoint tests allow volume creation and destruction 
> when HTTP authentication is disabled; this patch introduces a test for this 
> scenario: `PersistentVolumeEndpointsTest.NoAuthentication`.
> 
> 
> Diffs
> -
> 
>   src/tests/persistent_volume_endpoints_tests.cpp 
> 22e18758ee91a649486725473d9e50fae9d43b01 
> 
> Diff: https://reviews.apache.org/r/42362/diff/
> 
> 
> Testing
> ---
> 
> A new test, `PersistentVolumeEndpointsTest.NoAuthentication`, was added to 
> the persistent volume endpoint tests.
> 
> `make check` was used to test, and the new test was run with 
> `--gtest_repeat=1000 -gtest_break_on_failure=1`.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 42900: Fixed some typos.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [42900]

Passed command: export OS=ubuntu:14.04;export CONFIGURATION="--verbose";export 
COMPILER=gcc; ./support/docker_build.sh

- Mesos ReviewBot


On Jan. 28, 2016, 7:26 a.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42900/
> ---
> 
> (Updated Jan. 28, 2016, 7:26 a.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed some typos.
> 
> 
> Diffs
> -
> 
>   docs/reservation.md 8d2d33a6518c73542cbfb3a5ee36da1c00c6ff1a 
>   src/slave/containerizer/mesos/isolators/posix/disk.cpp 
> fd9b3abfe2c502c69ba7b2f0b2015fe90888d5bc 
> 
> Diff: https://reviews.apache.org/r/42900/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 42362: Added persistent volume endpoint test without authentication.

[Alexander Rukletsov]

> On Jan. 27, 2016, 11:20 p.m., Anand Mazumdar wrote:
> > src/tests/persistent_volume_endpoints_tests.cpp, line 1112
> > 
> >
> > Nit: Can we omit the part about not setting the authentication headers 
> > since it's self-explanatory. 
> > 
> > How about just:
> > 
> > ```
> > // Try a request to create a volume without authentication headers. 
> > Additionally, authorization is not enabled, so any principal, including 
> > `None()`, can create and destroy volumes.
> > ```
> > 
> > What do you think?

I think this comment should prepend the test in some form. It's an important 
bit that we can't do authz if authn is disabled (i.e. no "honor system"). Here 
we just need to drag reader's attention that there are no authn headers 
created. How about "Send a create volume request with absent credentials."?


- Alexander


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


On Jan. 27, 2016, 11:04 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42362/
> ---
> 
> (Updated Jan. 27, 2016, 11:04 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Kapil Arya, and Neil Conway.
> 
> 
> Bugs: MESOS-4395
> https://issues.apache.org/jira/browse/MESOS-4395
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added persistent volume endpoint tests with HTTP authentication disabled.
> 
> The persistent volume endpoint tests allow volume creation and destruction 
> when HTTP authentication is disabled; this patch introduces a test for this 
> scenario: `PersistentVolumeEndpointsTest.NoAuthentication`.
> 
> 
> Diffs
> -
> 
>   src/tests/persistent_volume_endpoints_tests.cpp 
> 22e18758ee91a649486725473d9e50fae9d43b01 
> 
> Diff: https://reviews.apache.org/r/42362/diff/
> 
> 
> Testing
> ---
> 
> A new test, `PersistentVolumeEndpointsTest.NoAuthentication`, was added to 
> the persistent volume endpoint tests.
> 
> `make check` was used to test, and the new test was run with 
> `--gtest_repeat=1000 -gtest_break_on_failure=1`.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 42362: Added persistent volume endpoint test without authentication.

[Alexander Rukletsov]

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




src/tests/persistent_volume_endpoints_tests.cpp (lines 1118 - 1119)


I advocate the practice when each block wrapped in curly braces is 
prepended with a comment. Effectively, each such block is a test case inside a 
test, so since we strive to provide a comment for each test we should do the 
same for each block. I think you had great comments that you could reused here. 
The most important bit is to drag people's attention to the absence of 
credentials. Does it make sense?


- Alexander Rukletsov


On Jan. 28, 2016, 4:43 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42362/
> ---
> 
> (Updated Jan. 28, 2016, 4:43 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Kapil Arya, and Neil Conway.
> 
> 
> Bugs: MESOS-4395
> https://issues.apache.org/jira/browse/MESOS-4395
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added persistent volume endpoint tests with HTTP authentication disabled.
> 
> The persistent volume endpoint tests allow volume creation and destruction 
> when HTTP authentication is disabled; this patch introduces a test for this 
> scenario: `PersistentVolumeEndpointsTest.NoAuthentication`.
> 
> 
> Diffs
> -
> 
>   src/tests/persistent_volume_endpoints_tests.cpp 
> 22e18758ee91a649486725473d9e50fae9d43b01 
> 
> Diff: https://reviews.apache.org/r/42362/diff/
> 
> 
> Testing
> ---
> 
> A new test, `PersistentVolumeEndpointsTest.NoAuthentication`, was added to 
> the persistent volume endpoint tests.
> 
> `make check` was used to test, and the new test was run with 
> `--gtest_repeat=1000 -gtest_break_on_failure=1`.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 42810: Added the CgroupInfo protobuf.

[Avinash sridharan]

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

(Updated Jan. 28, 2016, 5:32 p.m.)


Review request for mesos and Jie Yu.


Summary (updated)
-

Added the CgroupInfo protobuf.


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


Repository: mesos


Description
---

Added the CgroupInfo protobuf. The agent can use this message to reflect any 
cgroup configuration that might have been applied to a container.


Diffs
-

  include/mesos/mesos.proto 96b911fb370223933df52f9370897871827d2247 
  include/mesos/v1/mesos.proto 0501dfa27ed610666226953591a902eac4c295f8 

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


Testing
---

make


Thanks,

Avinash sridharan

Re: Review Request 42914: Listed supported file extensions in fetcher documentation.

[Joerg Schad]

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




docs/fetcher.md (line 122)


According to the code it also supports .gz and .zip.


- Joerg Schad


On Jan. 28, 2016, 4:29 p.m., Bernd Mathiske wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42914/
> ---
> 
> (Updated Jan. 28, 2016, 4:29 p.m.)
> 
> 
> Review request for mesos, Jie Yu, Jan Schlicht, and Till Toenshoff.
> 
> 
> Bugs: MESOS-4336
> https://issues.apache.org/jira/browse/MESOS-4336
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Edited fetcher.md, listing the supported file extensions when extracting an 
> archive.
> 
> 
> Diffs
> -
> 
>   docs/fetcher.md cb4f3c33fb67f7ac65dbe7ebe1347d2599c43f37 
> 
> Diff: https://reviews.apache.org/r/42914/diff/
> 
> 
> Testing
> ---
> 
> Opened file with MD renderer.
> 
> 
> Thanks,
> 
> Bernd Mathiske
> 
>

Re: Review Request 42362: Added persistent volume endpoint test without authentication.

[Greg Mann]

> On Jan. 28, 2016, 5:49 p.m., Alexander Rukletsov wrote:
> > src/tests/persistent_volume_endpoints_tests.cpp, lines 1118-1119
> > 
> >
> > I advocate the practice when each block wrapped in curly braces is 
> > prepended with a comment. Effectively, each such block is a test case 
> > inside a test, so since we strive to provide a comment for each test we 
> > should do the same for each block. I think you had great comments that you 
> > could reused here. The most important bit is to drag people's attention to 
> > the absence of credentials. Does it make sense?
> 
> Greg Mann wrote:
> Yep, it makes sense. Anand advocated removing the bit about not including 
> authentication headers because it was self-explanatory, but I do think that 
> it's better to call it out explicitly, since if you don't know the function 
> signature of `createBasicAuthHeaders`, it's not immediately obvious. Thanks, 
> will update.

Ah sorry, my mistake. Looking at Anand's comment above, he was suggesting 
something similar :-)


- Greg


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


On Jan. 28, 2016, 4:43 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42362/
> ---
> 
> (Updated Jan. 28, 2016, 4:43 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Kapil Arya, and Neil Conway.
> 
> 
> Bugs: MESOS-4395
> https://issues.apache.org/jira/browse/MESOS-4395
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added persistent volume endpoint tests with HTTP authentication disabled.
> 
> The persistent volume endpoint tests allow volume creation and destruction 
> when HTTP authentication is disabled; this patch introduces a test for this 
> scenario: `PersistentVolumeEndpointsTest.NoAuthentication`.
> 
> 
> Diffs
> -
> 
>   src/tests/persistent_volume_endpoints_tests.cpp 
> 22e18758ee91a649486725473d9e50fae9d43b01 
> 
> Diff: https://reviews.apache.org/r/42362/diff/
> 
> 
> Testing
> ---
> 
> A new test, `PersistentVolumeEndpointsTest.NoAuthentication`, was added to 
> the persistent volume endpoint tests.
> 
> `make check` was used to test, and the new test was run with 
> `--gtest_repeat=1000 -gtest_break_on_failure=1`.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 42832: Added a status method to the Isolator interface.

[Avinash sridharan]

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

(Updated Jan. 28, 2016, 5:57 p.m.)


Review request for mesos, Jie Yu and Kapil Arya.


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


Repository: mesos


Description
---

Added a status method to the Isolator interface. This method can be used to 
query the run-time state of isolator specific properties associated with a 
container.


Diffs (updated)
-

  include/mesos/slave/isolator.hpp 2ea1fb77b4bbe811466fac18fdf7cd48d482bc75 
  src/slave/containerizer/mesos/isolator.hpp 
b3babd09c9dc70d6ca3dac1964eda321cb0f8be9 
  src/slave/containerizer/mesos/isolator.cpp 
6d6863814fde5601bd311f6dc0816f7f1e446537 

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


Testing
---

make


Thanks,

Avinash sridharan

Re: Review Request 42362: Added persistent volume endpoint test without authentication.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [42530, 42362]

Passed command: export OS=ubuntu:14.04;export CONFIGURATION="--verbose";export 
COMPILER=gcc; ./support/docker_build.sh

- Mesos ReviewBot


On Jan. 28, 2016, 5:59 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42362/
> ---
> 
> (Updated Jan. 28, 2016, 5:59 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Kapil Arya, and Neil Conway.
> 
> 
> Bugs: MESOS-4395
> https://issues.apache.org/jira/browse/MESOS-4395
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added persistent volume endpoint tests with HTTP authentication disabled.
> 
> The persistent volume endpoint tests allow volume creation and destruction 
> when HTTP authentication is disabled; this patch introduces a test for this 
> scenario: `PersistentVolumeEndpointsTest.NoAuthentication`.
> 
> 
> Diffs
> -
> 
>   src/tests/persistent_volume_endpoints_tests.cpp 
> 22e18758ee91a649486725473d9e50fae9d43b01 
> 
> Diff: https://reviews.apache.org/r/42362/diff/
> 
> 
> Testing
> ---
> 
> A new test, `PersistentVolumeEndpointsTest.NoAuthentication`, was added to 
> the persistent volume endpoint tests.
> 
> `make check` was used to test, and the new test was run with 
> `--gtest_repeat=1000 -gtest_break_on_failure=1`.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 40731: Added a fixture to test the floating point precision for CPU resource allocation.

[Avinash sridharan]

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

(Updated Jan. 28, 2016, 6 p.m.)


Review request for mesos, Bernd Mathiske, Klaus Ma, Mandeep Chadha, and Neil 
Conway.


Summary (updated)
-

Added a fixture to test the floating point precision for CPU resource 
allocation.


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


Repository: mesos


Description (updated)
---

Added a fixture to test the floating point precision for CPU resource 
allocation. This patch is based on the commit submitted by : Mandeep (@mchadha) 
https://reviews.apache.org/r/39056/


Diffs (updated)
-

  src/tests/reservation_tests.cpp d0c88560a5618ebec39fc4e0539ddb2d9ca554a2 

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


Testing
---

Ran make check after adding Mandeep's test case to the GTEST framework.


Thanks,

Avinash sridharan

Re: Review Request 42773: Added utility function to compute SHA512 digest.

[Jojy Varghese]

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

(Updated Jan. 28, 2016, 6:44 p.m.)


Review request for mesos.


Changes
---

addressed review comments


Repository: mesos


Description
---

Added utility function to compute SHA512 digest.


Diffs (updated)
-

  src/common/command_utils.hpp bfab34571dd37980d9f99585e506eb015b499e28 
  src/common/command_utils.cpp e9d782127f948e165f55d03a2b1bf47946cc7f4e 
  src/tests/common/command_utils_tests.cpp 
938f3995aacf74bac28ae11040de292aa328f1e5 

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


Testing
---

make check.


Thanks,

Jojy Varghese

Re: Review Request 42828: Updated ReviewBot to tee build output to a file.

[Kevin Klues]

> On Jan. 29, 2016, 1:10 a.m., Kevin Klues wrote:
> > support/verify_reviews.py, line 140
> > 
> >
> > To avoid using tee, you could just do:
> > 
> > "%s > %s 2>&1" % (command, build_output)
> 
> Vinod Kone wrote:
> The above just outputs it to a file right? We want it to also output the 
> contents to the console (stdout) in real time.

Yes, that just outputs to the file.  If you want it to also go to the console, 
you do need tee.


- Kevin


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


On Jan. 29, 2016, 1:15 a.m., Vinod Kone wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42828/
> ---
> 
> (Updated Jan. 29, 2016, 1:15 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Greg Mann, Kevin Klues, and Michael 
> Park.
> 
> 
> Bugs: MESOS-1469 and MESOS-4478
> https://issues.apache.org/jira/browse/MESOS-1469
> https://issues.apache.org/jira/browse/MESOS-4478
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated ReviewBot to tee build output to a file.
> 
> 
> Diffs
> -
> 
>   support/verify_reviews.py 251d57ec050b087f13b3f5e0d8f421fe8eb76787 
> 
> Diff: https://reviews.apache.org/r/42828/diff/
> 
> 
> Testing
> ---
> 
> ?  mesos git:(vinod/review_bot_tee_output) 
> BUILD_URL=https://builds.apache.org/job/mesos-reviewbot/10980/ 
> ./support/verify_reviews.py vinod kone 1
> git rev-parse HEAD
> Checking if review: 41092 needs verification
> Latest diff timestamp: 2016-01-26 13:53:35
> Verifying review 41092
> Dependent review: https://reviews.apache.org/api/review-requests/41090/ 
> Dependent review: https://reviews.apache.org/api/review-requests/40951/ 
> Applying review 41092
> ./support/apply-review.sh -n -r 41092
> + : ubuntu:14.04
> + : gcc
> + : --verbose
> +++ dirname ./support/docker_build.sh
> ++ cd ./support/..
> ++ pwd
> + MESOS_DIRECTORY=/Users/vinodkone/workspace/mesos
> + cd /Users/vinodkone/workspace/mesos
> + DOCKERFILE=Dockerfile
> + rm -f Dockerfile
> + case $OS in
> + append_dockerfile 'FROM ubuntu:14.04'
> + echo FROM ubuntu:14.04
> + append_dockerfile 'RUN rm -rf /var/lib/apt/lists/*'
> + echo RUN rm -rf '/var/lib/apt/lists/*'
> + append_dockerfile 'RUN apt-get update'
> + echo RUN apt-get update
> + append_dockerfile 'RUN apt-get -y install build-essential clang git maven 
> autoconf libtool'
> + echo RUN apt-get -y install build-essential clang git maven autoconf libtool
> + append_dockerfile 'RUN apt-get -y install openjdk-7-jdk python-dev 
> python-boto libcurl4-nss-dev libsasl2-dev libapr1-dev libsvn-dev libevent-dev 
> libev-dev'
> + echo RUN apt-get -y install openjdk-7-jdk python-dev python-boto 
> libcurl4-nss-dev libsasl2-dev libapr1-dev libsvn-dev libevent-dev libev-dev
> + append_dockerfile 'RUN adduser --disabled-password --gecos '\'''\'' mesos'
> + echo RUN adduser --disabled-password --gecos ''\'''\''' mesos
> + append_dockerfile 'ENV GTEST_FILTER -FsTest.FileSystemTableRead'
> + echo ENV GTEST_FILTER -FsTest.FileSystemTableRead
> + append_dockerfile 'ENV GTEST_OUTPUT xml:report.xml'
> + echo ENV GTEST_OUTPUT xml:report.xml
> + case $COMPILER in
> + append_dockerfile 'ENV CC gcc'
> + echo ENV CC gcc
> + append_dockerfile 'ENV CXX g++'
> + echo ENV CXX g++
> + append_dockerfile 'WORKDIR mesos'
> + echo WORKDIR mesos
> + append_dockerfile 'COPY . /mesos/'
> + echo COPY . /mesos/
> + append_dockerfile 'RUN chown -R mesos /mesos'
> + echo RUN chown -R mesos /mesos
> + append_dockerfile 'USER mesos'
> + echo USER mesos
> + append_dockerfile 'CMD ./bootstrap && ./configure --verbose && 
> DISTCHECK_CONFIGURE_FLAGS="--verbose" GLOG_v=1 MESOS_VERBOSE=1 make -j8 
> distcheck'
> + echo CMD ./bootstrap '&&' ./configure --verbose '&&' 
> 'DISTCHECK_CONFIGURE_FLAGS="--verbose"' GLOG_v=1 MESOS_VERBOSE=1 make -j8 
> distcheck
> ++ date +%s
> + TAG=mesos-1453845969-21946
> + docker build --no-cache=true -t mesos-1453845969-21946 .
> ./support/docker_build.sh: line 117: docker: command not found
> Posting review: Bad patch!
> 
> Reviews applied: [40951, 41090, 41092]
> 
> Failed command: ['bash', '-c', 'set -o pipefail; export 
> OS=ubuntu:14.04;export CONFIGURATION="--verbose";export COMPILER=gcc; 
> ./support/docker_build.sh 2>&1 | tee build_41092']
> 
> Error:
>  ..
> + : ubuntu:14.04
> + : gcc
> + : --verbose
> +++ dirname ./support/docker_build.sh
> ++ cd ./support/..
> ++ pwd
> + MESOS_DIRECTORY=/Users/vinodkone/workspace/mesos
> + cd /Users/vinodkone/workspace/mesos
> + DOCKERFILE=Dockerfile
> + rm -f Dockerfile
> + case $OS in
> + append_dockerfile 'FROM ubuntu:14.04'
> + echo FROM ubuntu:14.04
> +