Re: Review Request 43293: Ignored invalid env vars.

2016-02-15 Thread Mesos ReviewBot

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



Patch looks great!

Reviews applied: [43293]

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

- Mesos ReviewBot


On Feb. 16, 2016, 4:17 a.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43293/
> ---
> 
> (Updated Feb. 16, 2016, 4:17 a.m.)
> 
> 
> Review request for mesos, haosdent huang, Jie Yu, and Timothy Chen.
> 
> 
> Bugs: MESOS-4607
> https://issues.apache.org/jira/browse/MESOS-4607
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Ignored invalid env vars when creating docker image.
> 
> 
> Diffs
> -
> 
>   src/docker/docker.cpp 52728707d985517e57525af7e470ccb468039373 
>   src/tests/containerizer/docker_tests.cpp 
> 620819330847a10d9dcd817968df9d2b180a9a29 
> 
> Diff: https://reviews.apache.org/r/43293/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> $ GLOG_v=2 ./bin/mesos-tests.sh  
> --gtest_filter="DockerImageTest.ParseInspectonImage" --verbose
> [==] Running 1 test from 1 test case.
> [--] Global test environment set-up.
> [--] 1 test from DockerImageTest
> [ RUN  ] DockerImageTest.ParseInspectonImage
> I0216 12:13:00.501356 1918300928 process.cpp:2489] Spawned process 
> files@192.168.1.102:54061
> I0216 12:13:00.501369 216694784 process.cpp:2499] Resuming 
> files@192.168.1.102:54061 at 2016-02-16 04:13:00.501399040+00:00
> I0216 12:13:00.501513 217231360 process.cpp:2499] Resuming 
> help@192.168.1.102:54061 at 2016-02-16 04:13:00.501527040+00:00
> I0216 12:13:00.505641 1918300928 docker.cpp:398] Overriding the environment 
> variable 'JAVA_VERSION' from '8u66' to '8u77'
> W0216 12:13:00.505677 1918300928 docker.cpp:391] Skipping invalid environment 
> variable 'JAVA_VERSION+8u66' for 'ContainerConfig.Env'
> I0216 12:13:00.506271 214548480 process.cpp:2499] Resuming 
> AuthenticationRouter(1)@192.168.1.102:54061 at 2016-02-16 
> 04:13:00.506306048+00:00
> I0216 12:13:00.506393 216158208 process.cpp:2499] Resuming 
> files@192.168.1.102:54061 at 2016-02-16 04:13:00.506411008+00:00
> I0216 12:13:00.506433 216158208 process.cpp:2604] Cleaning up 
> files@192.168.1.102:54061
> I0216 12:13:00.506475 215621632 process.cpp:2499] Resuming 
> help@192.168.1.102:54061 at 2016-02-16 04:13:00.506503168+00:00
> [   OK ] DockerImageTest.ParseInspectonImage (6 ms)
> [--] 1 test from DockerImageTest (6 ms total)
> 
> [--] Global test environment tear-down
> [==] 1 test from 1 test case ran. (18 ms total)
> [  PASSED  ] 1 test.
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>



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

2016-02-15 Thread Klaus Ma

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


Ship it!




Ship It!

- Klaus Ma


On Jan. 28, 2016, 9:12 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41950/
> ---
> 
> (Updated Jan. 28, 2016, 9:12 p.m.)
> 
> 
> Review request for mesos, Bernd Mathiske, Ben Mahler, Joerg Schad, and Joris 
> Van Remoortere.
> 
> 
> 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
> -
> 
>   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 43588: Added allocator recovery tests in presence of quota.

2016-02-15 Thread Klaus Ma


> On Feb. 16, 2016, 11:28 a.m., Klaus Ma wrote:
> > src/tests/hierarchical_allocator_tests.cpp, line 2400
> > 
> >
> > Move to HierarchicalAllocatorTest.
> 
> Joerg Schad wrote:
> This will be done by/after https://reviews.apache.org/r/41950.

Just checked r41950, that's OK to me :).


- Klaus


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


On Feb. 16, 2016, 5:23 a.m., Joerg Schad wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43588/
> ---
> 
> (Updated Feb. 16, 2016, 5:23 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Klaus Ma.
> 
> 
> Bugs: MESOS-3986
> https://issues.apache.org/jira/browse/MESOS-3986
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added allocator recovery tests in presence of quota.
> 
> 
> Diffs
> -
> 
>   src/tests/hierarchical_allocator_tests.cpp 
> 0acfc098750ff8ff9505207b983a34c1ccf3ad06 
> 
> Diff: https://reviews.apache.org/r/43588/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Joerg Schad
> 
>



Re: Review Request 43293: Ignored invalid env vars.

2016-02-15 Thread Guangya Liu


> On 二月 16, 2016, 3:46 a.m., Timothy Chen wrote:
> > src/docker/docker.cpp, line 398
> > 
> >
> > Actually shouldn't we take the latest value? What does Docker do when 
> > it have duplicated env var?

Yes, we should take the latest value.

root@mesos002:/home/gyliu# docker run -it -e env1=a -e env1=b ubuntu:14.04 
/bin/bash
root@fac8798b4572:/# env
HOSTNAME=fac8798b4572
TERM=xterm
LS_COLORS=rs=0:di=01;34:ln=01;36:mh=00:pi=40;33:so=01;35:do=01;35:bd=40;33;01:cd=40;33;01:or=40;31;01:su=37;41:sg=30;43:ca=30;41:tw=30;42:ow=34;42:st=37;44:ex=01;32:*.tar=01;31:*.tgz=01;31:*.arj=01;31:*.taz=01;31:*.lzh=01;31:*.lzma=01;31:*.tlz=01;31:*.txz=01;31:*.zip=01;31:*.z=01;31:*.Z=01;31:*.dz=01;31:*.gz=01;31:*.lz=01;31:*.xz=01;31:*.bz2=01;31:*.bz=01;31:*.tbz=01;31:*.tbz2=01;31:*.tz=01;31:*.deb=01;31:*.rpm=01;31:*.jar=01;31:*.war=01;31:*.ear=01;31:*.sar=01;31:*.rar=01;31:*.ace=01;31:*.zoo=01;31:*.cpio=01;31:*.7z=01;31:*.rz=01;31:*.jpg=01;35:*.jpeg=01;35:*.gif=01;35:*.bmp=01;35:*.pbm=01;35:*.pgm=01;35:*.ppm=01;35:*.tga=01;35:*.xbm=01;35:*.xpm=01;35:*.tif=01;35:*.tiff=01;35:*.png=01;35:*.svg=01;35:*.svgz=01;35:*.mng=01;35:*.pcx=01;35:*.mov=01;35:*.mpg=01;35:*.mpeg=01;35:*.m2v=01;35:*.mkv=01;35:*.webm=01;35:*.ogm=01;35:*.mp4=01;35:*.m4v=01;35:*.mp4v=01;35:*.vob=01;35:*.qt=01;35:*.nuv=01;35:*.wmv=01;35:*.asf=01;35:*.rm=01;35:*.rmvb=01;35:*.flc=01;35:*.avi=01;35:*.fli=01;35:*.flv=01;
 
35:*.gl=01;35:*.dl=01;35:*.xcf=01;35:*.xwd=01;35:*.yuv=01;35:*.cgm=01;35:*.emf=01;35:*.axv=01;35:*.anx=01;35:*.ogv=01;35:*.ogx=01;35:*.aac=00;36:*.au=00;36:*.flac=00;36:*.mid=00;36:*.midi=00;36:*.mka=00;36:*.mp3=00;36:*.mpc=00;36:*.ogg=00;36:*.ra=00;36:*.wav=00;36:*.axa=00;36:*.oga=00;36:*.spx=00;36:*.xspf=00;36:
PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin
env1=b  
PWD=/
SHLVL=1
HOME=/root
LESSOPEN=| /usr/bin/lesspipe %s
LESSCLOSE=/usr/bin/lesspipe %s %s
_=/usr/bin/env


- Guangya


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


On 二月 16, 2016, 4:17 a.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43293/
> ---
> 
> (Updated 二月 16, 2016, 4:17 a.m.)
> 
> 
> Review request for mesos, haosdent huang, Jie Yu, and Timothy Chen.
> 
> 
> Bugs: MESOS-4607
> https://issues.apache.org/jira/browse/MESOS-4607
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Ignored invalid env vars when creating docker image.
> 
> 
> Diffs
> -
> 
>   src/docker/docker.cpp 52728707d985517e57525af7e470ccb468039373 
>   src/tests/containerizer/docker_tests.cpp 
> 620819330847a10d9dcd817968df9d2b180a9a29 
> 
> Diff: https://reviews.apache.org/r/43293/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> $ GLOG_v=2 ./bin/mesos-tests.sh  
> --gtest_filter="DockerImageTest.ParseInspectonImage" --verbose
> [==] Running 1 test from 1 test case.
> [--] Global test environment set-up.
> [--] 1 test from DockerImageTest
> [ RUN  ] DockerImageTest.ParseInspectonImage
> I0216 12:13:00.501356 1918300928 process.cpp:2489] Spawned process 
> files@192.168.1.102:54061
> I0216 12:13:00.501369 216694784 process.cpp:2499] Resuming 
> files@192.168.1.102:54061 at 2016-02-16 04:13:00.501399040+00:00
> I0216 12:13:00.501513 217231360 process.cpp:2499] Resuming 
> help@192.168.1.102:54061 at 2016-02-16 04:13:00.501527040+00:00
> I0216 12:13:00.505641 1918300928 docker.cpp:398] Overriding the environment 
> variable 'JAVA_VERSION' from '8u66' to '8u77'
> W0216 12:13:00.505677 1918300928 docker.cpp:391] Skipping invalid environment 
> variable 'JAVA_VERSION+8u66' for 'ContainerConfig.Env'
> I0216 12:13:00.506271 214548480 process.cpp:2499] Resuming 
> AuthenticationRouter(1)@192.168.1.102:54061 at 2016-02-16 
> 04:13:00.506306048+00:00
> I0216 12:13:00.506393 216158208 process.cpp:2499] Resuming 
> files@192.168.1.102:54061 at 2016-02-16 04:13:00.506411008+00:00
> I0216 12:13:00.506433 216158208 process.cpp:2604] Cleaning up 
> files@192.168.1.102:54061
> I0216 12:13:00.506475 215621632 process.cpp:2499] Resuming 
> help@192.168.1.102:54061 at 2016-02-16 04:13:00.506503168+00:00
> [   OK ] DockerImageTest.ParseInspectonImage (6 ms)
> [--] 1 test from DockerImageTest (6 ms total)
> 
> [--] Global test environment tear-down
> [==] 1 test from 1 test case ran. (18 ms total)
> [  PASSED  ] 1 test.
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>



Re: Review Request 43293: Ignored invalid env vars.

2016-02-15 Thread Guangya Liu

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

(Updated 二月 16, 2016, 4:17 a.m.)


Review request for mesos, haosdent huang, Jie Yu, and Timothy Chen.


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


Repository: mesos


Description
---

Ignored invalid env vars when creating docker image.


Diffs (updated)
-

  src/docker/docker.cpp 52728707d985517e57525af7e470ccb468039373 
  src/tests/containerizer/docker_tests.cpp 
620819330847a10d9dcd817968df9d2b180a9a29 

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


Testing (updated)
---

make
make check

$ GLOG_v=2 ./bin/mesos-tests.sh  
--gtest_filter="DockerImageTest.ParseInspectonImage" --verbose
[==] Running 1 test from 1 test case.
[--] Global test environment set-up.
[--] 1 test from DockerImageTest
[ RUN  ] DockerImageTest.ParseInspectonImage
I0216 12:13:00.501356 1918300928 process.cpp:2489] Spawned process 
files@192.168.1.102:54061
I0216 12:13:00.501369 216694784 process.cpp:2499] Resuming 
files@192.168.1.102:54061 at 2016-02-16 04:13:00.501399040+00:00
I0216 12:13:00.501513 217231360 process.cpp:2499] Resuming 
help@192.168.1.102:54061 at 2016-02-16 04:13:00.501527040+00:00
I0216 12:13:00.505641 1918300928 docker.cpp:398] Overriding the environment 
variable 'JAVA_VERSION' from '8u66' to '8u77'
W0216 12:13:00.505677 1918300928 docker.cpp:391] Skipping invalid environment 
variable 'JAVA_VERSION+8u66' for 'ContainerConfig.Env'
I0216 12:13:00.506271 214548480 process.cpp:2499] Resuming 
AuthenticationRouter(1)@192.168.1.102:54061 at 2016-02-16 
04:13:00.506306048+00:00
I0216 12:13:00.506393 216158208 process.cpp:2499] Resuming 
files@192.168.1.102:54061 at 2016-02-16 04:13:00.506411008+00:00
I0216 12:13:00.506433 216158208 process.cpp:2604] Cleaning up 
files@192.168.1.102:54061
I0216 12:13:00.506475 215621632 process.cpp:2499] Resuming 
help@192.168.1.102:54061 at 2016-02-16 04:13:00.506503168+00:00
[   OK ] DockerImageTest.ParseInspectonImage (6 ms)
[--] 1 test from DockerImageTest (6 ms total)

[--] Global test environment tear-down
[==] 1 test from 1 test case ran. (18 ms total)
[  PASSED  ] 1 test.


Thanks,

Guangya Liu



Re: Review Request 42516: Add support for user-defined networks.

2016-02-15 Thread Timothy Chen

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



Thinking about it a bit more I think it's fine without a unit test as it 
requires docker network create. Did you test this manually and made sure it 
worked?
Once you update the comments I can merge it.

- Timothy Chen


On Feb. 11, 2016, 1:51 p.m., Ezra Silvera wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42516/
> ---
> 
> (Updated Feb. 11, 2016, 1:51 p.m.)
> 
> 
> Review request for mesos and Timothy Chen.
> 
> 
> Bugs: MESOS-4369
> https://issues.apache.org/jira/browse/MESOS-4369
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Signed-off-by: Ezra Silvera 
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 5d1a20b4ff647f43701a353472b56a4b74b6bbc3 
>   include/mesos/v1/mesos.proto 415a20d9f3385e9627635dd89d78c58d03e3c116 
>   src/docker/docker.cpp 52728707d985517e57525af7e470ccb468039373 
> 
> Diff: https://reviews.apache.org/r/42516/diff/
> 
> 
> Testing
> ---
> 
> Using Swarm running on Mesos create a network with "docker  network create 
> --driver=bridge myNetwork"   and then create a container on that network:  
> "docker run --net=myNetwork"
> 
> 
> Thanks,
> 
> Ezra Silvera
> 
>



Re: Review Request 42516: Add support for user-defined networks.

2016-02-15 Thread Timothy Chen

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




include/mesos/v1/mesos.proto (line 1534)


Remove extra space.



src/docker/docker.cpp (line 521)


Capitalize the first letter of your comment ( // User defined...)



src/docker/docker.cpp (line 528)


Space between next if



src/docker/docker.cpp (line 530)


Fix the formatting, need to move one more char to the right.


- Timothy Chen


On Feb. 11, 2016, 1:51 p.m., Ezra Silvera wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42516/
> ---
> 
> (Updated Feb. 11, 2016, 1:51 p.m.)
> 
> 
> Review request for mesos and Timothy Chen.
> 
> 
> Bugs: MESOS-4369
> https://issues.apache.org/jira/browse/MESOS-4369
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Signed-off-by: Ezra Silvera 
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 5d1a20b4ff647f43701a353472b56a4b74b6bbc3 
>   include/mesos/v1/mesos.proto 415a20d9f3385e9627635dd89d78c58d03e3c116 
>   src/docker/docker.cpp 52728707d985517e57525af7e470ccb468039373 
> 
> Diff: https://reviews.apache.org/r/42516/diff/
> 
> 
> Testing
> ---
> 
> Using Swarm running on Mesos create a network with "docker  network create 
> --driver=bridge myNetwork"   and then create a container on that network:  
> "docker run --net=myNetwork"
> 
> 
> Thanks,
> 
> Ezra Silvera
> 
>



Re: Review Request 43093: MESOS-4370 NetworkSettings.IPAddress field is deprectaed in Docker

2016-02-15 Thread Timothy Chen

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




src/docker/docker.cpp (line 310)


This can fit in 80 char width line right?



src/docker/docker.cpp (line 314)


Also fix the formatting for the strings concat. Move 
networkModeValue.get().value aligned with the "NetworkSettings"


- Timothy Chen


On Feb. 4, 2016, 9:27 p.m., Travis Hegner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43093/
> ---
> 
> (Updated Feb. 4, 2016, 9:27 p.m.)
> 
> 
> Review request for mesos, haosdent huang and Kapil Arya.
> 
> 
> Bugs: MESOS-4370
> https://issues.apache.org/jira/browse/MESOS-4370
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixes [MESOS-4370]
> 
> 
> Diffs
> -
> 
>   src/docker/docker.cpp b4b8d3e 
> 
> Diff: https://reviews.apache.org/r/43093/diff/
> 
> 
> Testing
> ---
> 
> This patch will first query the docker API for the HostConfig.NetworkMode, 
> which is populated with the network name. (Essentially what was passed in 
> --net  to the docker run command). This name is then used as a key in 
> NetworkSettings.Networks..IPAddress to get the IP address that is 
> currently in use by the container.
> 
> It appears that even though the docker API has been set up to allow for 
> multiple networks, our testing has indicated that it's still only applying 
> one network to the container (the last one via the --net argument on the run 
> line). I can only speculate that the docker API will change again in the near 
> future, but I can't speculate how, so at least this fixes the problem as it 
> stands right now.
> 
> Tested and working with Docker 1.9.1, Mesos 0.27.0, on Ubuntu 14.04.
> 
> 
> Thanks,
> 
> Travis Hegner
> 
>



Re: Review Request 43293: Ignored invalid env vars.

2016-02-15 Thread Timothy Chen

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




src/docker/docker.cpp (line 382)


I would make this a LOG(WARNING) since we don't really expect this to 
happen at all with Mesos.



src/docker/docker.cpp (line 391)


This also isn't an expected condition as well right? LOG(WARNING) will be 
more appropriate as well. Can you also log the image name? Same as above one.



src/docker/docker.cpp (line 398)


Actually shouldn't we take the latest value? What does Docker do when it 
have duplicated env var?


- Timothy Chen


On Feb. 7, 2016, 9:40 a.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43293/
> ---
> 
> (Updated Feb. 7, 2016, 9:40 a.m.)
> 
> 
> Review request for mesos, haosdent huang, Jie Yu, and Timothy Chen.
> 
> 
> Bugs: MESOS-4607
> https://issues.apache.org/jira/browse/MESOS-4607
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Ignored invalid env vars when creating docker image.
> 
> 
> Diffs
> -
> 
>   src/docker/docker.cpp 52728707d985517e57525af7e470ccb468039373 
>   src/tests/containerizer/docker_tests.cpp 
> 620819330847a10d9dcd817968df9d2b180a9a29 
> 
> Diff: https://reviews.apache.org/r/43293/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> $ GLOG_v=2 ./bin/mesos-tests.sh  
> --gtest_filter="DockerImageTest.ParseInspectonImage" --verbose
> [==] Running 1 test from 1 test case.
> [--] Global test environment set-up.
> [--] 1 test from DockerImageTest
> [ RUN  ] DockerImageTest.ParseInspectonImage
> I0207 10:30:01.894125 2034615040 process.cpp:2489] Spawned process 
> files@192.168.0.100:49551
> I0207 10:30:01.894136 211529728 process.cpp:2499] Resuming 
> files@192.168.0.100:49551 at 2016-02-07 02:30:01.894163968+00:00
> I0207 10:30:01.894317 210456576 process.cpp:2499] Resuming 
> help@192.168.0.100:49551 at 2016-02-07 02:30:01.894332928+00:00
> I0207 10:30:01.898862 2034615040 docker.cpp:397] Skipping duplicate 
> environment variable 'JAVA_VERSION'
> I0207 10:30:01.898892 2034615040 docker.cpp:390] Skipping invalid environment 
> variable 'JAVA_VERSION+8u66' for 'ContainerConfig.Env'
> I0207 10:30:01.899473 20992 process.cpp:2499] Resuming 
> AuthenticationRouter(1)@192.168.0.100:49551 at 2016-02-07 
> 02:30:01.899522048+00:00
> I0207 10:30:01.899983 213139456 process.cpp:2499] Resuming 
> files@192.168.0.100:49551 at 2016-02-07 02:30:01.9+00:00
> I0207 10:30:01.900032 213139456 process.cpp:2604] Cleaning up 
> files@192.168.0.100:49551
> I0207 10:30:01.900244 211529728 process.cpp:2499] Resuming 
> help@192.168.0.100:49551 at 2016-02-07 02:30:01.900256000+00:00
> [   OK ] DockerImageTest.ParseInspectonImage (8 ms)
> [--] 1 test from DockerImageTest (8 ms total)
> 
> [--] Global test environment tear-down
> [==] 1 test from 1 test case ran. (18 ms total)
> [  PASSED  ] 1 test.
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>



Re: Review Request 43588: Added allocator recovery tests in presence of quota.

2016-02-15 Thread Joerg Schad


> On Feb. 16, 2016, 3:28 a.m., Klaus Ma wrote:
> > src/tests/hierarchical_allocator_tests.cpp, line 2400
> > 
> >
> > Move to HierarchicalAllocatorTest.

This will be done by/after https://reviews.apache.org/r/41950.


- Joerg


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


On Feb. 15, 2016, 9:23 p.m., Joerg Schad wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43588/
> ---
> 
> (Updated Feb. 15, 2016, 9:23 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Klaus Ma.
> 
> 
> Bugs: MESOS-3986
> https://issues.apache.org/jira/browse/MESOS-3986
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added allocator recovery tests in presence of quota.
> 
> 
> Diffs
> -
> 
>   src/tests/hierarchical_allocator_tests.cpp 
> 0acfc098750ff8ff9505207b983a34c1ccf3ad06 
> 
> Diff: https://reviews.apache.org/r/43588/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Joerg Schad
> 
>



Re: Review Request 43592: Updated libnl3 download links

2016-02-15 Thread Timothy Chen

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


Ship it!




Ship It!

- Timothy Chen


On Feb. 16, 2016, 3:31 a.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43592/
> ---
> 
> (Updated Feb. 16, 2016, 3:31 a.m.)
> 
> 
> Review request for mesos, Guangya Liu and Jie Yu.
> 
> 
> Bugs: MESOS-4681
> https://issues.apache.org/jira/browse/MESOS-4681
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Update libnl3 download links.
> 
> 
> Diffs
> -
> 
>   configure.ac b9a99060fe02f95ee1dca4a28deec200301e320b 
>   docs/network-monitoring.md 08fcaa8ebc7313a4cf9a719ff1a9b11fcd926c7f 
> 
> Diff: https://reviews.apache.org/r/43592/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>



Re: Review Request 43592: pdated libnl3 download links

2016-02-15 Thread haosdent huang

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

(Updated Feb. 16, 2016, 3:30 a.m.)


Review request for mesos, Guangya Liu and Jie Yu.


Summary (updated)
-

pdated libnl3 download links


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


Repository: mesos


Description
---

Update libnl3 download links.


Diffs
-

  configure.ac b9a99060fe02f95ee1dca4a28deec200301e320b 
  docs/network-monitoring.md 08fcaa8ebc7313a4cf9a719ff1a9b11fcd926c7f 

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


Testing
---


Thanks,

haosdent huang



Re: Review Request 43592: Updated libnl3 download links

2016-02-15 Thread haosdent huang

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

(Updated Feb. 16, 2016, 3:31 a.m.)


Review request for mesos, Guangya Liu and Jie Yu.


Summary (updated)
-

Updated libnl3 download links


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


Repository: mesos


Description
---

Update libnl3 download links.


Diffs
-

  configure.ac b9a99060fe02f95ee1dca4a28deec200301e320b 
  docs/network-monitoring.md 08fcaa8ebc7313a4cf9a719ff1a9b11fcd926c7f 

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


Testing
---


Thanks,

haosdent huang



Re: Review Request 43588: Added allocator recovery tests in presence of quota.

2016-02-15 Thread Klaus Ma

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




src/tests/hierarchical_allocator_tests.cpp (line 2400)


Move to HierarchicalAllocatorTest.



src/tests/hierarchical_allocator_tests.cpp (line 2417)


No allocation because there is no slaves. We have trigger allocation by 
`Clock::advance(flags.allocation_interval);`.



src/tests/hierarchical_allocator_tests.cpp (line 2423)


Add var on half of agents; we can used it when add 80% resources.



src/tests/hierarchical_allocator_tests.cpp (line 2438)


delete emepty line.



src/tests/hierarchical_allocator_tests.cpp (line 2442)


Replace `4u` to var `10 * AGENT_RECOVERY_FACTOR - half_agents_num + 1`. So 
when we update the value of `AGENT_RECOVERY_FACTOR` or export it as flags, we 
did not need to re-calculate the number.



src/tests/hierarchical_allocator_tests.cpp (line 2459)


Add check on whether all resources are got.

@Alex/@Joerg, I'd like to confirm the behaviour of following two cases:

In `::addSlave`, the slaves after 80% will offer to framework firstly; the 
others (< 80%) will offer to framework until next allocation interval. I think 
we'd better to offer ALL recovered resources when 80% slaves recovered.

Another case that the counter did not update in `::removeSlave`. What's our 
expected behaviour on the following case?

* 100 slaves
* 70 slaves are added ino allocator
* 60 of 70 added slaves are down
* another 20 slaves are added into alloator
 
Current behaviour, allocator will continue to offer resources based on 30 
slaves because "90%" are added. We'd better to wait for timeout for this case.



src/tests/hierarchical_allocator_tests.cpp (line 2507)


ditto



src/tests/hierarchical_allocator_tests.cpp (line 2513)


ditto


- Klaus Ma


On Feb. 16, 2016, 5:23 a.m., Joerg Schad wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43588/
> ---
> 
> (Updated Feb. 16, 2016, 5:23 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Klaus Ma.
> 
> 
> Bugs: MESOS-3986
> https://issues.apache.org/jira/browse/MESOS-3986
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added allocator recovery tests in presence of quota.
> 
> 
> Diffs
> -
> 
>   src/tests/hierarchical_allocator_tests.cpp 
> 0acfc098750ff8ff9505207b983a34c1ccf3ad06 
> 
> Diff: https://reviews.apache.org/r/43588/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Joerg Schad
> 
>



Re: Review Request 43592: Update libnl3 download links.

2016-02-15 Thread Guangya Liu

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


Ship it!




It is better to update the summary as "Updated libnl3 download links." with 
past tone.

- Guangya Liu


On 二月 16, 2016, 3 a.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43592/
> ---
> 
> (Updated 二月 16, 2016, 3 a.m.)
> 
> 
> Review request for mesos, Guangya Liu and Jie Yu.
> 
> 
> Bugs: MESOS-4681
> https://issues.apache.org/jira/browse/MESOS-4681
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Update libnl3 download links.
> 
> 
> Diffs
> -
> 
>   configure.ac b9a99060fe02f95ee1dca4a28deec200301e320b 
>   docs/network-monitoring.md 08fcaa8ebc7313a4cf9a719ff1a9b11fcd926c7f 
> 
> Diff: https://reviews.apache.org/r/43592/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>



Re: Review Request 43015: Fixed persistent volumes with docker tasks.

2016-02-15 Thread Timothy Chen

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

(Updated Feb. 16, 2016, 3:01 a.m.)


Review request for mesos and Jie Yu.


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


Repository: mesos


Description
---

Fixed persistent volumes with docker tasks.


Diffs (updated)
-

  src/slave/containerizer/docker.hpp 77a50d80179672cf3c270cbdd7fa003c7d9ad6f3 
  src/slave/containerizer/docker.cpp ed1c9a551f03a37d572470e4c495f5df834198cc 
  src/slave/containerizer/mesos/isolators/filesystem/linux.hpp 
cb181265482c884b02bdfc576f906aa0dd9f00f2 
  src/slave/containerizer/mesos/isolators/filesystem/linux.cpp 
7fdf518deeb388218438245623719f41613d031b 
  src/tests/containerizer/docker_containerizer_tests.cpp 
645bdcf095145097d8b8c65d592c787417883145 

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


Testing
---

make check


Thanks,

Timothy Chen



Review Request 43592: Update libnl3 download links.

2016-02-15 Thread haosdent huang

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

Review request for mesos, Jie Yu and Timothy Chen.


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


Repository: mesos


Description
---

Update libnl3 download links.


Diffs
-

  configure.ac b9a99060fe02f95ee1dca4a28deec200301e320b 
  docs/network-monitoring.md 08fcaa8ebc7313a4cf9a719ff1a9b11fcd926c7f 

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


Testing
---


Thanks,

haosdent huang



Re: Review Request 43015: Fixed persistent volumes with docker tasks.

2016-02-15 Thread Timothy Chen


> On Feb. 16, 2016, 12:29 a.m., Jie Yu wrote:
> > src/slave/containerizer/docker.cpp, lines 1707-1719
> > 
> >
> > Do you need this? Looks like 'unmountPersistentVolumes' will take care 
> > of the umount.

We do still need this just in case the container is destroyed right before it's 
launched but the persistent volumes are already mounted. I think it's better to 
use unmountVolumes here since we only LOG(ERROR) with custom executors case.


- Timothy


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


On Feb. 15, 2016, 3:39 p.m., Timothy Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43015/
> ---
> 
> (Updated Feb. 15, 2016, 3:39 p.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-3413
> https://issues.apache.org/jira/browse/MESOS-3413
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed persistent volumes with docker tasks.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/docker.hpp 77a50d80179672cf3c270cbdd7fa003c7d9ad6f3 
>   src/slave/containerizer/docker.cpp ed1c9a551f03a37d572470e4c495f5df834198cc 
>   src/tests/containerizer/docker_containerizer_tests.cpp 
> 645bdcf095145097d8b8c65d592c787417883145 
> 
> Diff: https://reviews.apache.org/r/43015/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Timothy Chen
> 
>



Re: Review Request 43588: Added allocator recovery tests in presence of quota.

2016-02-15 Thread Guangya Liu

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




src/tests/hierarchical_allocator_tests.cpp (line 2390)


s/we/the allocator will



src/tests/hierarchical_allocator_tests.cpp (line 2391)


s/RecoverPercentage/RecoverPercentageWithQuota?

How about move this above `DeactivateAndReactivateFramework` to make sure 
group all `Quota` related tests?



src/tests/hierarchical_allocator_tests.cpp (line 2434)


How about s/Test that the allocator still pauses./Wait for all `addSlave` 
messages to be dispatched and processed completely?



src/tests/hierarchical_allocator_tests.cpp (line 2457)


s/A we/The framework



src/tests/hierarchical_allocator_tests.cpp (line 2474)


s/we/the framework



src/tests/hierarchical_allocator_tests.cpp (line 2482)


s/the the/the



src/tests/hierarchical_allocator_tests.cpp (line 2483)


s/RecoverTimeout/RecoverTimeoutWithQuota?

How about move this above `DeactivateAndReactivateFramework` to make sure 
group all `Quota` related tests?



src/tests/hierarchical_allocator_tests.cpp (line 2524)


How about s/Test that the allocator still pauses./Wait for all `addSlave` 
messages to be dispatched and processed completely?


- Guangya Liu


On 二月 15, 2016, 9:23 p.m., Joerg Schad wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43588/
> ---
> 
> (Updated 二月 15, 2016, 9:23 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Klaus Ma.
> 
> 
> Bugs: MESOS-3986
> https://issues.apache.org/jira/browse/MESOS-3986
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added allocator recovery tests in presence of quota.
> 
> 
> Diffs
> -
> 
>   src/tests/hierarchical_allocator_tests.cpp 
> 0acfc098750ff8ff9505207b983a34c1ccf3ad06 
> 
> Diff: https://reviews.apache.org/r/43588/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Joerg Schad
> 
>



Re: Review Request 43587: Replaced use of *_MESSAGE macros with *_PROTOBUF equivalents.

2016-02-15 Thread Klaus Ma

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


Ship it!




Ship It!

- Klaus Ma


On Feb. 16, 2016, 5:19 a.m., Michael Browning wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43587/
> ---
> 
> (Updated Feb. 16, 2016, 5:19 a.m.)
> 
> 
> Review request for mesos, Joris Van Remoortere and Neil Conway.
> 
> 
> Bugs: MESOS-3486
> https://issues.apache.org/jira/browse/MESOS-3486
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Replaced use of *_MESSAGE macros with *_PROTOBUF equivalents.
> 
> Some usages of the DROP_MESSAGE, DROP_MESSAGES, and FUTURE_MESSAGE macros 
> from the libprocess tests perform logic already implemented in the 
> DROP_PROTOBUF, DROP_PROTOBUFS, and FUTURE_PROTOBUF macros defined in 
> tests/mesos.hpp. This diff moves those tests to use the latter versions where 
> appropriate.
> 
> 
> Diffs
> -
> 
>   src/tests/hook_tests.cpp 59a00ea722a17bbc82b14c69bda826f68cbac6e9 
>   src/tests/log_tests.cpp 923d71f48e743a77f2a3ba9e982aef4417c6c7fe 
>   src/tests/partition_tests.cpp c5badbe90e302793bfbf3f16373efe241decb7d5 
>   src/tests/slave_recovery_tests.cpp e2a78a0f55b7657057ee351a747caff51024fd67 
>   src/tests/slave_tests.cpp c7f5a701eff2c2f9aa3df5722583a131bf2c072a 
> 
> Diff: https://reviews.apache.org/r/43587/diff/
> 
> 
> Testing
> ---
> 
> This change involves changing existing macro invocations in testing code, so 
> no additional testing was performed.
> 
> 
> Thanks,
> 
> Michael Browning
> 
>



Re: Review Request 43015: Fixed persistent volumes with docker tasks.

2016-02-15 Thread Jie Yu

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




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


Looks like we use 'directory' consistently in this file for container 
working directory. So please

s/sandboxDirectory/directory/



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


s/currentResources/current/



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


s/newResources/updated/



src/slave/containerizer/docker.cpp (lines 420 - 424)


You may want to make sure `slave.work_dir` is a shared mount in its own 
peer group (see code in `LinuxFilesystemIsolatorProcess::create` and also 
ticket https://issues.apache.org/jira/browse/MESOS-3483).

The reason we need to do that is because when we fork an executor process 
(in Mesos containerizer) with a new mount namespace, if the mount is a private 
mount, the mount in the new mount namespace will be a private mount as well, 
holding an extra reference to the mount. That mount can be for other 
containers. When the slave remove the mount followed by an rmdir (removing the 
mount point), the rmdir could fail due to the extra references to the mount.

Although we did some best effort umount in the new mount namespace (trying 
to umount any irrelevant mounts), but race still exists. For instance, while 
we're umounting, the slave tries to umount+rmdir for some mounts.



src/slave/containerizer/docker.cpp (lines 512 - 513)


Returning a failure doesn't match the existing semantics. Could you instead 
use a LOG(ERROR) here?



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


Hum, can you make 'updatePersistentVolumes' a member function so that you 
have access to 'flags' and do not need to pass in this parameter.



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


Insert a blank line above.



src/slave/containerizer/docker.cpp (lines 1281 - 1289)


This is useless right now, isn't it? Can you remove it and instead, drop a 
TODO here. To fully support updating volumes, we need to have docker's mount 
propagation support.



src/slave/containerizer/docker.cpp (lines 1707 - 1719)


Do you need this? Looks like 'unmountPersistentVolumes' will take care of 
the umount.


- Jie Yu


On Feb. 15, 2016, 3:39 p.m., Timothy Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43015/
> ---
> 
> (Updated Feb. 15, 2016, 3:39 p.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-3413
> https://issues.apache.org/jira/browse/MESOS-3413
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed persistent volumes with docker tasks.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/docker.hpp 77a50d80179672cf3c270cbdd7fa003c7d9ad6f3 
>   src/slave/containerizer/docker.cpp ed1c9a551f03a37d572470e4c495f5df834198cc 
>   src/tests/containerizer/docker_containerizer_tests.cpp 
> 645bdcf095145097d8b8c65d592c787417883145 
> 
> Diff: https://reviews.apache.org/r/43015/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Timothy Chen
> 
>



Re: Review Request 43587: Replaced use of *_MESSAGE macros with *_PROTOBUF equivalents.

2016-02-15 Thread Neil Conway

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


Ship it!




Thanks for the contribution, Michael!

"make check" passed for me.

- Neil Conway


On Feb. 15, 2016, 9:19 p.m., Michael Browning wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43587/
> ---
> 
> (Updated Feb. 15, 2016, 9:19 p.m.)
> 
> 
> Review request for mesos, Joris Van Remoortere and Neil Conway.
> 
> 
> Bugs: MESOS-3486
> https://issues.apache.org/jira/browse/MESOS-3486
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Replaced use of *_MESSAGE macros with *_PROTOBUF equivalents.
> 
> Some usages of the DROP_MESSAGE, DROP_MESSAGES, and FUTURE_MESSAGE macros 
> from the libprocess tests perform logic already implemented in the 
> DROP_PROTOBUF, DROP_PROTOBUFS, and FUTURE_PROTOBUF macros defined in 
> tests/mesos.hpp. This diff moves those tests to use the latter versions where 
> appropriate.
> 
> 
> Diffs
> -
> 
>   src/tests/hook_tests.cpp 59a00ea722a17bbc82b14c69bda826f68cbac6e9 
>   src/tests/log_tests.cpp 923d71f48e743a77f2a3ba9e982aef4417c6c7fe 
>   src/tests/partition_tests.cpp c5badbe90e302793bfbf3f16373efe241decb7d5 
>   src/tests/slave_recovery_tests.cpp e2a78a0f55b7657057ee351a747caff51024fd67 
>   src/tests/slave_tests.cpp c7f5a701eff2c2f9aa3df5722583a131bf2c072a 
> 
> Diff: https://reviews.apache.org/r/43587/diff/
> 
> 
> Testing
> ---
> 
> This change involves changing existing macro invocations in testing code, so 
> no additional testing was performed.
> 
> 
> Thanks,
> 
> Michael Browning
> 
>



Re: Review Request 43588: Added allocator recovery tests in presence of quota.

2016-02-15 Thread Mesos ReviewBot

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



Patch looks great!

Reviews applied: [43588]

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

- Mesos ReviewBot


On Feb. 15, 2016, 9:23 p.m., Joerg Schad wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43588/
> ---
> 
> (Updated Feb. 15, 2016, 9:23 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Klaus Ma.
> 
> 
> Bugs: MESOS-3986
> https://issues.apache.org/jira/browse/MESOS-3986
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added allocator recovery tests in presence of quota.
> 
> 
> Diffs
> -
> 
>   src/tests/hierarchical_allocator_tests.cpp 
> 0acfc098750ff8ff9505207b983a34c1ccf3ad06 
> 
> Diff: https://reviews.apache.org/r/43588/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Joerg Schad
> 
>



Re: Review Request 42688: Fixed equality semantics for Labels.

2016-02-15 Thread Neil Conway

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



Per comments in MESOS-4445, the current plan is to not change equality behavior 
right now, and just to document that duplicates in `Labels` are not supported.

- Neil Conway


On Jan. 23, 2016, 8:04 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42688/
> ---
> 
> (Updated Jan. 23, 2016, 8:04 p.m.)
> 
> 
> Review request for mesos and Michael Park.
> 
> 
> Bugs: MESOS-4445
> https://issues.apache.org/jira/browse/MESOS-4445
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The previous implementation was buggy in two scenarios: (1) the label sets
> contained different #s of duplicate elements (2) the "left" label set 
> contained
> duplicates and the "right" label set contained a value that didn't appear in 
> the
> "left" set.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 19bf3a7c2e43ca04ed6e6d506e052de5537f7c2f 
>   src/common/type_utils.cpp 76f48f6a1f5467db032ded8acd296d03353b4172 
>   src/tests/labels_tests.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/42688/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> REVIEW NOTES:
> 
> * This is a WIP.
> * Rather than implement a correct equality algorithm myself, I decided to 
> just use `std::unordered_multiset`. Computing equality correctly is a bit 
> involved.
> * This requires a hash implementation for `Label`. I supplied this in new 
> function object in the `mesos` namespace, but we could also supply a 
> specialization of `std::hash`. Not sure which is better.
> * The same bug occurs in other equality operators for repeated fields 
> (`CommandInfo.uris`, `Environment`, `DockerInfo`, etc.). I suppose we should 
> probably fix these as well? Note that the bugs in the old approach are a lot 
> less likely to occur when the struct has a lot of fields.
> 
> 
> Thanks,
> 
> Neil Conway
> 
>



Re: Review Request 43583: Added documentation for multiple-disk support.

2016-02-15 Thread Neil Conway


> On Feb. 15, 2016, 8:39 p.m., Neil Conway wrote:
> > docs/multiple-disk.md, line 12
> > 
> >
> > "three forms"
> > 
> > Is there a reason to say "currently"? I'd be inclined to remove it as 
> > redundant.
> 
> Joris Van Remoortere wrote:
> The reason I used currently is because we will likely introduce a 4th: 
> block devices. Is there a better way to express this? Maybe it's not 
> important to clarify that just yet?

I'd opt for just documenting the current state without saying "currently"; we 
can then update the docs when a fourth variant is added.


- Neil


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


On Feb. 15, 2016, 7:42 p.m., Joris Van Remoortere wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43583/
> ---
> 
> (Updated Feb. 15, 2016, 7:42 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Neil Conway.
> 
> 
> Bugs: MESOS-4531
> https://issues.apache.org/jira/browse/MESOS-4531
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   docs/multiple-disk.md PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/43583/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Joris Van Remoortere
> 
>



Re: Review Request 43587: Replaced use of *_MESSAGE macros with *_PROTOBUF equivalents.

2016-02-15 Thread Mesos ReviewBot

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



Patch looks great!

Reviews applied: [43587]

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

- Mesos ReviewBot


On Feb. 15, 2016, 9:19 p.m., Michael Browning wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43587/
> ---
> 
> (Updated Feb. 15, 2016, 9:19 p.m.)
> 
> 
> Review request for mesos, Joris Van Remoortere and Neil Conway.
> 
> 
> Bugs: MESOS-3486
> https://issues.apache.org/jira/browse/MESOS-3486
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Replaced use of *_MESSAGE macros with *_PROTOBUF equivalents.
> 
> Some usages of the DROP_MESSAGE, DROP_MESSAGES, and FUTURE_MESSAGE macros 
> from the libprocess tests perform logic already implemented in the 
> DROP_PROTOBUF, DROP_PROTOBUFS, and FUTURE_PROTOBUF macros defined in 
> tests/mesos.hpp. This diff moves those tests to use the latter versions where 
> appropriate.
> 
> 
> Diffs
> -
> 
>   src/tests/hook_tests.cpp 59a00ea722a17bbc82b14c69bda826f68cbac6e9 
>   src/tests/log_tests.cpp 923d71f48e743a77f2a3ba9e982aef4417c6c7fe 
>   src/tests/partition_tests.cpp c5badbe90e302793bfbf3f16373efe241decb7d5 
>   src/tests/slave_recovery_tests.cpp e2a78a0f55b7657057ee351a747caff51024fd67 
>   src/tests/slave_tests.cpp c7f5a701eff2c2f9aa3df5722583a131bf2c072a 
> 
> Diff: https://reviews.apache.org/r/43587/diff/
> 
> 
> Testing
> ---
> 
> This change involves changing existing macro invocations in testing code, so 
> no additional testing was performed.
> 
> 
> Thanks,
> 
> Michael Browning
> 
>



Review Request 43588: Added allocator recovery tests in presence of quota.

2016-02-15 Thread Joerg Schad

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

Review request for mesos, Alexander Rukletsov and Klaus Ma.


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


Repository: mesos


Description
---

Added allocator recovery tests in presence of quota.


Diffs
-

  src/tests/hierarchical_allocator_tests.cpp 
0acfc098750ff8ff9505207b983a34c1ccf3ad06 

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


Testing
---

make check


Thanks,

Joerg Schad



Review Request 43587: Replaced use of *_MESSAGE macros with *_PROTOBUF equivalents.

2016-02-15 Thread Michael Browning

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

Review request for mesos, Joris Van Remoortere and Neil Conway.


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


Repository: mesos


Description
---

Replaced use of *_MESSAGE macros with *_PROTOBUF equivalents.


Diffs
-

  src/tests/hook_tests.cpp 59a00ea722a17bbc82b14c69bda826f68cbac6e9 
  src/tests/log_tests.cpp 923d71f48e743a77f2a3ba9e982aef4417c6c7fe 
  src/tests/partition_tests.cpp c5badbe90e302793bfbf3f16373efe241decb7d5 
  src/tests/slave_recovery_tests.cpp e2a78a0f55b7657057ee351a747caff51024fd67 
  src/tests/slave_tests.cpp c7f5a701eff2c2f9aa3df5722583a131bf2c072a 

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


Testing
---

This change involves changing existing macro invocations in testing code, so no 
additional testing was performed.


Thanks,

Michael Browning



Re: Review Request 43583: Added documentation for multiple-disk support.

2016-02-15 Thread Joris Van Remoortere


> On Feb. 15, 2016, 8:39 p.m., Neil Conway wrote:
> > docs/multiple-disk.md, line 12
> > 
> >
> > "three forms"
> > 
> > Is there a reason to say "currently"? I'd be inclined to remove it as 
> > redundant.

The reason I used currently is because we will likely introduce a 4th: block 
devices. Is there a better way to express this? Maybe it's not important to 
clarify that just yet?


- Joris


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


On Feb. 15, 2016, 7:42 p.m., Joris Van Remoortere wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43583/
> ---
> 
> (Updated Feb. 15, 2016, 7:42 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Neil Conway.
> 
> 
> Bugs: MESOS-4531
> https://issues.apache.org/jira/browse/MESOS-4531
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   docs/multiple-disk.md PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/43583/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Joris Van Remoortere
> 
>



Re: Review Request 43583: Added documentation for multiple-disk support.

2016-02-15 Thread Mesos ReviewBot

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



Patch looks great!

Reviews applied: [43583]

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

- Mesos ReviewBot


On Feb. 15, 2016, 7:42 p.m., Joris Van Remoortere wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43583/
> ---
> 
> (Updated Feb. 15, 2016, 7:42 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Neil Conway.
> 
> 
> Bugs: MESOS-4531
> https://issues.apache.org/jira/browse/MESOS-4531
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   docs/multiple-disk.md PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/43583/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Joris Van Remoortere
> 
>



Re: Review Request 43583: Added documentation for multiple-disk support.

2016-02-15 Thread Neil Conway

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



Can we link to this page from `home.md`? Also, we should update 
`persistent-volume.md` to discuss the concepts here briefly, and add a link to 
this page.


docs/multiple-disk.md (line 9)


"understanding" doesn't seem like the best choice of phrase. "by 
examining", perhaps?

maybe "`source` field", not "qualifier"?



docs/multiple-disk.md (line 12)


"three forms"

Is there a reason to say "currently"? I'd be inclined to remove it as 
redundant.



docs/multiple-disk.md (line 22)


"JSON-formatted"



docs/multiple-disk.md (line 79)


"can __not__" => "__cannot__"



docs/multiple-disk.md (line 80)


"in to" => "into"


- Neil Conway


On Feb. 15, 2016, 7:42 p.m., Joris Van Remoortere wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43583/
> ---
> 
> (Updated Feb. 15, 2016, 7:42 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Neil Conway.
> 
> 
> Bugs: MESOS-4531
> https://issues.apache.org/jira/browse/MESOS-4531
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   docs/multiple-disk.md PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/43583/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Joris Van Remoortere
> 
>



Re: Review Request 43565: Implemented `status` method in `ComposingContainerizer`.

2016-02-15 Thread Avinash sridharan


> On Feb. 15, 2016, 1:17 a.m., Guangya Liu wrote:
> > src/slave/containerizer/composing.cpp, line 230
> > 
> >
> > It is already 81 chars, it is better to split this to several lines.
> 
> Avinash sridharan wrote:
> On my VIM editor this shows up as exactly 80 characters?

Also, the git commit-hook would not allow a commit for any line > 80 characters.


- Avinash


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


On Feb. 14, 2016, 4:31 p.m., Avinash sridharan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43565/
> ---
> 
> (Updated Feb. 14, 2016, 4:31 p.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-4670
> https://issues.apache.org/jira/browse/MESOS-4670
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This method will be used by the agent to retrieve `ContainerStatus`
> from the `Containerizer`, that was responsible for launching the
> container.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/composing.cpp 
> f0a7bba4a56702872c9b73a12128b5292708d0e7 
> 
> Diff: https://reviews.apache.org/r/43565/diff/
> 
> 
> Testing
> ---
> 
> make and make check.
> 
> 
> Thanks,
> 
> Avinash sridharan
> 
>



Re: Review Request 43565: Implemented `status` method in `ComposingContainerizer`.

2016-02-15 Thread Avinash sridharan


> On Feb. 15, 2016, 1:17 a.m., Guangya Liu wrote:
> > src/slave/containerizer/composing.cpp, line 230
> > 
> >
> > It is already 81 chars, it is better to split this to several lines.

On my VIM editor this shows up as exactly 80 characters?


- Avinash


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


On Feb. 14, 2016, 4:31 p.m., Avinash sridharan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43565/
> ---
> 
> (Updated Feb. 14, 2016, 4:31 p.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-4670
> https://issues.apache.org/jira/browse/MESOS-4670
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This method will be used by the agent to retrieve `ContainerStatus`
> from the `Containerizer`, that was responsible for launching the
> container.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/composing.cpp 
> f0a7bba4a56702872c9b73a12128b5292708d0e7 
> 
> Diff: https://reviews.apache.org/r/43565/diff/
> 
> 
> Testing
> ---
> 
> make and make check.
> 
> 
> Thanks,
> 
> Avinash sridharan
> 
>



Review Request 43583: Added documentation for multiple-disk support.

2016-02-15 Thread Joris Van Remoortere

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

Review request for mesos, Jie Yu and Neil Conway.


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


Repository: mesos


Description
---

See summary.


Diffs
-

  docs/multiple-disk.md PRE-CREATION 

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


Testing
---


Thanks,

Joris Van Remoortere



Re: Review Request 43582: Added flag to disable systemd support.

2016-02-15 Thread Mesos ReviewBot

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



Patch looks great!

Reviews applied: [43582]

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

- Mesos ReviewBot


On Feb. 15, 2016, 6:55 p.m., Joris Van Remoortere wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43582/
> ---
> 
> (Updated Feb. 15, 2016, 6:55 p.m.)
> 
> 
> Review request for mesos and Benjamin Hindman.
> 
> 
> Bugs: MESOS-4675
> https://issues.apache.org/jira/browse/MESOS-4675
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added flag to disable systemd support.
> 
> 
> Diffs
> -
> 
>   docs/configuration.md eea985cc251d6edd8476ade174b10a58aebefab1 
>   src/linux/systemd.hpp d868fda3d77eb8f42a5ec9877af2e6a79bbec228 
>   src/linux/systemd.cpp 13b5c8212c97912cae44fd0d7c92c93a714f695d 
>   src/slave/flags.hpp 20232d645175d0d574c6d896188435277619010d 
>   src/slave/flags.cpp 14ad4dcc0dfb1d7745e58e11e8f66386288395d7 
>   src/slave/main.cpp 222198ca89f672332cb80773a3f36fe1f0438f64 
> 
> Diff: https://reviews.apache.org/r/43582/diff/
> 
> 
> Testing
> ---
> 
> run on systemd system with flag disabled.
> 
> 
> Thanks,
> 
> Joris Van Remoortere
> 
>



Re: Review Request 43582: Added flag to disable systemd support.

2016-02-15 Thread Joris Van Remoortere

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

(Updated Feb. 15, 2016, 6:55 p.m.)


Review request for mesos and Benjamin Hindman.


Changes
---

re-ordering flags.


Summary (updated)
-

Added flag to disable systemd support.


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


Repository: mesos


Description (updated)
---

Added flag to disable systemd support.


Diffs (updated)
-

  docs/configuration.md eea985cc251d6edd8476ade174b10a58aebefab1 
  src/linux/systemd.hpp d868fda3d77eb8f42a5ec9877af2e6a79bbec228 
  src/linux/systemd.cpp 13b5c8212c97912cae44fd0d7c92c93a714f695d 
  src/slave/flags.hpp 20232d645175d0d574c6d896188435277619010d 
  src/slave/flags.cpp 14ad4dcc0dfb1d7745e58e11e8f66386288395d7 
  src/slave/main.cpp 222198ca89f672332cb80773a3f36fe1f0438f64 

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


Testing
---

run on systemd system with flag disabled.


Thanks,

Joris Van Remoortere



Re: Review Request 43582: Added flag to disable system support.

2016-02-15 Thread Joerg Schad

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




src/slave/main.cpp (line 234)


Regarding the summary: 
s/`to disable system support.`/`to disable systemd support`?


- Joerg Schad


On Feb. 15, 2016, 5:59 p.m., Joris Van Remoortere wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43582/
> ---
> 
> (Updated Feb. 15, 2016, 5:59 p.m.)
> 
> 
> Review request for mesos and Benjamin Hindman.
> 
> 
> Bugs: MESOS-4675
> https://issues.apache.org/jira/browse/MESOS-4675
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added flag to disable system support.
> 
> 
> Diffs
> -
> 
>   src/linux/systemd.hpp d868fda3d77eb8f42a5ec9877af2e6a79bbec228 
>   src/linux/systemd.cpp 13b5c8212c97912cae44fd0d7c92c93a714f695d 
>   src/slave/flags.hpp 20232d645175d0d574c6d896188435277619010d 
>   src/slave/flags.cpp 14ad4dcc0dfb1d7745e58e11e8f66386288395d7 
>   src/slave/main.cpp 222198ca89f672332cb80773a3f36fe1f0438f64 
> 
> Diff: https://reviews.apache.org/r/43582/diff/
> 
> 
> Testing
> ---
> 
> run on systemd system with flag disabled.
> 
> 
> Thanks,
> 
> Joris Van Remoortere
> 
>



Re: Review Request 43582: Added flag to disable system support.

2016-02-15 Thread Joerg Schad


> On Feb. 15, 2016, 6:35 p.m., Joerg Schad wrote:
> > src/slave/flags.hpp, line 97
> > 
> >
> > Should we document this in configure.md?

Or are we generating this automatically after 
https://reviews.apache.org/r/42939/? In that case could you point me to the 
script for the generation?


- Joerg


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


On Feb. 15, 2016, 5:59 p.m., Joris Van Remoortere wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43582/
> ---
> 
> (Updated Feb. 15, 2016, 5:59 p.m.)
> 
> 
> Review request for mesos and Benjamin Hindman.
> 
> 
> Bugs: MESOS-4675
> https://issues.apache.org/jira/browse/MESOS-4675
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added flag to disable system support.
> 
> 
> Diffs
> -
> 
>   src/linux/systemd.hpp d868fda3d77eb8f42a5ec9877af2e6a79bbec228 
>   src/linux/systemd.cpp 13b5c8212c97912cae44fd0d7c92c93a714f695d 
>   src/slave/flags.hpp 20232d645175d0d574c6d896188435277619010d 
>   src/slave/flags.cpp 14ad4dcc0dfb1d7745e58e11e8f66386288395d7 
>   src/slave/main.cpp 222198ca89f672332cb80773a3f36fe1f0438f64 
> 
> Diff: https://reviews.apache.org/r/43582/diff/
> 
> 
> Testing
> ---
> 
> run on systemd system with flag disabled.
> 
> 
> Thanks,
> 
> Joris Van Remoortere
> 
>



Re: Review Request 43582: Added flag to disable system support.

2016-02-15 Thread Joerg Schad

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




src/linux/systemd.cpp (line 52)


does it make sense to have the same order here as above in the hpp?



src/slave/flags.hpp (line 97)


Should we document this in configure.md?



src/slave/flags.cpp (line 399)


What precisly do you mean by `Top level control systemd support`?



src/slave/main.cpp (line 243)


Isn't flags.systemd_enable_support always true at this point? Feel free to 
drop, but I personally feel `true` would be easier to read.


- Joerg Schad


On Feb. 15, 2016, 5:59 p.m., Joris Van Remoortere wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43582/
> ---
> 
> (Updated Feb. 15, 2016, 5:59 p.m.)
> 
> 
> Review request for mesos and Benjamin Hindman.
> 
> 
> Bugs: MESOS-4675
> https://issues.apache.org/jira/browse/MESOS-4675
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added flag to disable system support.
> 
> 
> Diffs
> -
> 
>   src/linux/systemd.hpp d868fda3d77eb8f42a5ec9877af2e6a79bbec228 
>   src/linux/systemd.cpp 13b5c8212c97912cae44fd0d7c92c93a714f695d 
>   src/slave/flags.hpp 20232d645175d0d574c6d896188435277619010d 
>   src/slave/flags.cpp 14ad4dcc0dfb1d7745e58e11e8f66386288395d7 
>   src/slave/main.cpp 222198ca89f672332cb80773a3f36fe1f0438f64 
> 
> Diff: https://reviews.apache.org/r/43582/diff/
> 
> 
> Testing
> ---
> 
> run on systemd system with flag disabled.
> 
> 
> Thanks,
> 
> Joris Van Remoortere
> 
>



Re: Review Request 43269: MasterContender/MasterDetector loadable as modules.

2016-02-15 Thread Benjamin Hindman

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




include/mesos/master/contender.hpp (lines 17 - 18)


Let's use the `MESOS` prefix like in include/master/allocator.hpp please. 
Please also do this in detector.hpp, thanks!



include/mesos/scheduler.hpp (line 46)


We don't indent here, my guess is your editor did this automagically but 
you can just copy this up without the indentation please.



src/examples/test_contender_module.cpp (line 50)


We try and embed single use functions as lambdas here, you can just replace 
`createTestContender` with:

```
[](const Parameters& parameters) {
  return new StandaloneMasterContender();
}
```

Same for the test detector module too please.



src/master/contender.cpp (line 80)


Why the removal of the `Option`? Conveying the information via an empty 
string is far less explicit and not our preferred approach in the code base. 
Can you elaborate?



src/tests/module.cpp (line 303)


We try and wrap one per line for these (even if you've seen some older code 
in the code base that doesn't do that):

```
addModule(
library,
TestContender,
"org_apache_mesos_TestContender");
```

Below as well, thanks!


- Benjamin Hindman


On Feb. 15, 2016, 6:06 p.m., Mark Cavage wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43269/
> ---
> 
> (Updated Feb. 15, 2016, 6:06 p.m.)
> 
> 
> Review request for mesos and Benjamin Hindman.
> 
> 
> Bugs: MESOS-4610
> https://issues.apache.org/jira/browse/MESOS-4610
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> MasterContender/MasterDetector loadable as modules.
> 
> There is a large set of changes here, mostly due to a namespace rename. The 
> previous version had the MasterContender/Detector APIs under an internal 
> namespace, which didn't seem appropriate for an external contract. So the 
> actual code changes that were impactful are relatively surgical, with then a 
> wide ranging downstream cleanup to make the existing code base reference the 
> new location.
> 
> 
> Diffs
> -
> 
>   include/mesos/master/contender.hpp PRE-CREATION 
>   include/mesos/master/detector.hpp PRE-CREATION 
>   include/mesos/module/contender.hpp PRE-CREATION 
>   include/mesos/module/detector.hpp PRE-CREATION 
>   include/mesos/scheduler.hpp 14c7ff964aa7b94f439d16e605380661d2279d54 
>   src/Makefile.am 22f51319a1c06da02cac5822d42315e3027cf500 
>   src/cli/resolve.cpp 257e29034abf32491511f9a4e476b6859714829d 
>   src/examples/test_contender_module.cpp PRE-CREATION 
>   src/examples/test_detector_module.cpp PRE-CREATION 
>   src/local/local.cpp 359fc54d7c4081f536a8de8b1dfcde413d75c9a9 
>   src/master/contender.hpp 3fd20f8e94daab349b76d8f5ecc87398a187a847 
>   src/master/contender.cpp 9ad49ce10439fb41d78d52eaa4c1e6b9c5c7f735 
>   src/master/detector.hpp eb5d2a90b60c629150ddf04acf00f0edca1ca723 
>   src/master/detector.cpp 9274435802d6292b183be48f42b43999476e016e 
>   src/master/main.cpp 4263110c9b889984ef74eb94fed629958f2abd79 
>   src/master/master.hpp 3a7e18232323a1c051bcc97915484b1195fffe58 
>   src/master/master.cpp e1ca81dab85a7ab1391eca0d6bd995548bf79c22 
>   src/module/manager.cpp 6ae99504005581b22a44768949b1d305cec517d9 
>   src/sched/sched.cpp 525255eec808c3fe5c0e38b3d1a2086bbd4eb171 
>   src/scheduler/scheduler.cpp 99a7d0dfff7b0c61decc9ff6d9e6d46ef13a7e75 
>   src/slave/slave.hpp a3830ff460a6f6c5661fb8a0172fae303b245889 
>   src/tests/authentication_tests.cpp 85f14c3d453ca5aeffa1c915f38fe3031c2cf712 
>   src/tests/cluster.hpp 99a785ab0d4ee1a1e745202d2551de58a7631a85 
>   src/tests/cluster.cpp 084fb1ce37a315c561c4587c4761c870f54c8625 
>   src/tests/fault_tolerance_tests.cpp 
> 982468f851cd9d95eb6cde7c57f2d737d46a827c 
>   src/tests/master_allocator_tests.cpp 
> cba7c36471f93b678d94e1da0251a28a893696b1 
>   src/tests/master_authorization_tests.cpp 
> 29c89fb11da792c3e71eb880a19657ea225b3cc8 
>   src/tests/master_contender_detector_tests.cpp 
> 6375586c31b1fd406529bf299dad6e321b945de8 
>   src/tests/master_slave_reconciliation_tests.cpp 
> d41178eb41df519073fc0890c5716bbc9fed6ad2 
>   src/tests/master_tests.cpp 0357b1c259472213181a65e5adbe6d5caa1698ad 
>   src/tests/module.hpp 4b32f29f2ce76100433621a5cb6b8cc87c9b38f8 
>   src/tests/module.cpp 246f3a402d4fe3b273c459f6e02c009f3de65f3e 
>   src/tests/oversubscription_tests.cpp 
> 

Re: Review Request 43582: Added flag to disable system support.

2016-02-15 Thread Benjamin Hindman

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


Fix it, then Ship it!





src/linux/systemd.cpp (line 55)


"... are enabled unless there is an explicit flag to disable these (see 
other flags)."



src/slave/flags.cpp (line 400)


See comment above.


- Benjamin Hindman


On Feb. 15, 2016, 5:59 p.m., Joris Van Remoortere wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43582/
> ---
> 
> (Updated Feb. 15, 2016, 5:59 p.m.)
> 
> 
> Review request for mesos and Benjamin Hindman.
> 
> 
> Bugs: MESOS-4675
> https://issues.apache.org/jira/browse/MESOS-4675
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added flag to disable system support.
> 
> 
> Diffs
> -
> 
>   src/linux/systemd.hpp d868fda3d77eb8f42a5ec9877af2e6a79bbec228 
>   src/linux/systemd.cpp 13b5c8212c97912cae44fd0d7c92c93a714f695d 
>   src/slave/flags.hpp 20232d645175d0d574c6d896188435277619010d 
>   src/slave/flags.cpp 14ad4dcc0dfb1d7745e58e11e8f66386288395d7 
>   src/slave/main.cpp 222198ca89f672332cb80773a3f36fe1f0438f64 
> 
> Diff: https://reviews.apache.org/r/43582/diff/
> 
> 
> Testing
> ---
> 
> run on systemd system with flag disabled.
> 
> 
> Thanks,
> 
> Joris Van Remoortere
> 
>



Re: Review Request 43582: Added flag to disable system support.

2016-02-15 Thread Joris Van Remoortere

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

(Updated Feb. 15, 2016, 5:59 p.m.)


Review request for mesos and Benjamin Hindman.


Changes
---

More generic help strings.


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


Repository: mesos


Description (updated)
---

Added flag to disable system support.


Diffs (updated)
-

  src/linux/systemd.hpp d868fda3d77eb8f42a5ec9877af2e6a79bbec228 
  src/linux/systemd.cpp 13b5c8212c97912cae44fd0d7c92c93a714f695d 
  src/slave/flags.hpp 20232d645175d0d574c6d896188435277619010d 
  src/slave/flags.cpp 14ad4dcc0dfb1d7745e58e11e8f66386288395d7 
  src/slave/main.cpp 222198ca89f672332cb80773a3f36fe1f0438f64 

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


Testing
---

run on systemd system with flag disabled.


Thanks,

Joris Van Remoortere



Re: Review Request 43582: Added flag to disable system support.

2016-02-15 Thread Benjamin Hindman

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




src/slave/flags.cpp (lines 397 - 403)


My $0.02: this flag makes it sound like we are "enabling systemd" not 
"enabling moving executors and associated processes into a systemd slice". IMHO 
this could mean two different things in the future, i.e., we want to enable 
systemd for one thing but not for moving executors? I'd call out the moving 
executors support explicitly in this flag, perhaps: 
`systemd_enable_executors_slice` or a better name if you decided previously to 
not call it an executor slice any more because it holds more than just 
executors.


- Benjamin Hindman


On Feb. 15, 2016, 5:37 p.m., Joris Van Remoortere wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43582/
> ---
> 
> (Updated Feb. 15, 2016, 5:37 p.m.)
> 
> 
> Review request for mesos and Benjamin Hindman.
> 
> 
> Bugs: MESOS-4675
> https://issues.apache.org/jira/browse/MESOS-4675
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/linux/systemd.hpp d868fda3d77eb8f42a5ec9877af2e6a79bbec228 
>   src/linux/systemd.cpp 13b5c8212c97912cae44fd0d7c92c93a714f695d 
>   src/slave/flags.hpp 20232d645175d0d574c6d896188435277619010d 
>   src/slave/flags.cpp 14ad4dcc0dfb1d7745e58e11e8f66386288395d7 
>   src/slave/main.cpp 222198ca89f672332cb80773a3f36fe1f0438f64 
> 
> Diff: https://reviews.apache.org/r/43582/diff/
> 
> 
> Testing
> ---
> 
> run on systemd system with flag disabled.
> 
> 
> Thanks,
> 
> Joris Van Remoortere
> 
>



Review Request 43582: Added flag to disable system support.

2016-02-15 Thread Joris Van Remoortere

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

Review request for mesos and Benjamin Hindman.


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


Repository: mesos


Description
---

See summary.


Diffs
-

  src/linux/systemd.hpp d868fda3d77eb8f42a5ec9877af2e6a79bbec228 
  src/linux/systemd.cpp 13b5c8212c97912cae44fd0d7c92c93a714f695d 
  src/slave/flags.hpp 20232d645175d0d574c6d896188435277619010d 
  src/slave/flags.cpp 14ad4dcc0dfb1d7745e58e11e8f66386288395d7 
  src/slave/main.cpp 222198ca89f672332cb80773a3f36fe1f0438f64 

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


Testing
---

run on systemd system with flag disabled.


Thanks,

Joris Van Remoortere



Re: Review Request 43569: Updated log message if container not found.

2016-02-15 Thread Mesos ReviewBot

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



Patch looks great!

Reviews applied: [43015, 43569]

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

- Mesos ReviewBot


On Feb. 15, 2016, 4:08 p.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43569/
> ---
> 
> (Updated Feb. 15, 2016, 4:08 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Timothy Chen.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated log message if container not found.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/docker.cpp ed1c9a551f03a37d572470e4c495f5df834198cc 
>   src/slave/containerizer/mesos/containerizer.cpp 
> 3de214d6c71773f5deb5db9fe3f527dd5064737f 
> 
> Diff: https://reviews.apache.org/r/43569/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>



Review Request 43580: Add indent to comments in protobuf_tests.cpp.

2016-02-15 Thread Klaus Ma

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

Review request for mesos and Michael Park.


Repository: mesos


Description
---

Add indent to comments in protobuf_tests.cpp.


Diffs
-

  3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.cpp 
8dd9cfd3e7d1e3ab4ace87066a43a3094b776d82 

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


Testing
---


Thanks,

Klaus Ma



Re: Review Request 43569: Updated log message if container not found.

2016-02-15 Thread Guangya Liu

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

(Updated 二月 15, 2016, 4:08 p.m.)


Review request for mesos, Jie Yu and Timothy Chen.


Repository: mesos


Description
---

Updated log message if container not found.


Diffs (updated)
-

  src/slave/containerizer/docker.cpp ed1c9a551f03a37d572470e4c495f5df834198cc 
  src/slave/containerizer/mesos/containerizer.cpp 
3de214d6c71773f5deb5db9fe3f527dd5064737f 

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


Testing
---

make
make check


Thanks,

Guangya Liu



Re: Review Request 43015: Fixed persistent volumes with docker tasks.

2016-02-15 Thread Timothy Chen

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

(Updated Feb. 15, 2016, 3:39 p.m.)


Review request for mesos and Jie Yu.


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


Repository: mesos


Description
---

Fixed persistent volumes with docker tasks.


Diffs (updated)
-

  src/slave/containerizer/docker.hpp 77a50d80179672cf3c270cbdd7fa003c7d9ad6f3 
  src/slave/containerizer/docker.cpp ed1c9a551f03a37d572470e4c495f5df834198cc 
  src/tests/containerizer/docker_containerizer_tests.cpp 
645bdcf095145097d8b8c65d592c787417883145 

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


Testing
---

make check


Thanks,

Timothy Chen



Re: Review Request 43015: Fixed persistent volumes with docker tasks.

2016-02-15 Thread Timothy Chen


> On Feb. 15, 2016, 3:47 a.m., Guangya Liu wrote:
> > src/slave/containerizer/docker.cpp, line 505
> > 
> >
> > What about add `containerId` in the log message?

It's not added in all the other places. I see you have a commit adding it to 
the failure message, let's rebase it once this is merged soon.


- Timothy


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


On Feb. 14, 2016, 8:38 a.m., Timothy Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43015/
> ---
> 
> (Updated Feb. 14, 2016, 8:38 a.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-3413
> https://issues.apache.org/jira/browse/MESOS-3413
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed persistent volumes with docker tasks.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/docker.hpp 77a50d80179672cf3c270cbdd7fa003c7d9ad6f3 
>   src/slave/containerizer/docker.cpp ed1c9a551f03a37d572470e4c495f5df834198cc 
>   src/tests/containerizer/docker_containerizer_tests.cpp 
> 645bdcf095145097d8b8c65d592c787417883145 
> 
> Diff: https://reviews.apache.org/r/43015/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Timothy Chen
> 
>



Re: Review Request 43015: Fixed persistent volumes with docker tasks.

2016-02-15 Thread Timothy Chen

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

(Updated Feb. 15, 2016, 3:38 p.m.)


Review request for mesos and Jie Yu.


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


Repository: mesos


Description
---

Fixed persistent volumes with docker tasks.


Diffs (updated)
-

  src/slave/containerizer/docker.hpp 77a50d80179672cf3c270cbdd7fa003c7d9ad6f3 
  src/slave/containerizer/docker.cpp ed1c9a551f03a37d572470e4c495f5df834198cc 
  src/tests/containerizer/docker_containerizer_tests.cpp 
645bdcf095145097d8b8c65d592c787417883145 

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


Testing
---

make check


Thanks,

Timothy Chen



Re: Review Request 43569: Updated log message if container not found.

2016-02-15 Thread Timothy Chen

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




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


Let's stick with the same formatting with single quotes for container id



src/slave/containerizer/mesos/containerizer.cpp (line 1611)


This won't happen since the previous if statement already checks for this.



src/slave/containerizer/mesos/containerizer.cpp (line 1616)


This as well


- Timothy Chen


On Feb. 15, 2016, 4:17 a.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43569/
> ---
> 
> (Updated Feb. 15, 2016, 4:17 a.m.)
> 
> 
> Review request for mesos, Jie Yu and Timothy Chen.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated log message if container not found.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/docker.cpp ed1c9a551f03a37d572470e4c495f5df834198cc 
>   src/slave/containerizer/mesos/containerizer.cpp 
> 3de214d6c71773f5deb5db9fe3f527dd5064737f 
> 
> Diff: https://reviews.apache.org/r/43569/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>



Re: Review Request 43293: Ignored invalid env vars.

2016-02-15 Thread Guangya Liu


> On 二月 14, 2016, 8:42 a.m., Timothy Chen wrote:
> > src/tests/containerizer/docker_tests.cpp, lines 506-507
> > 
> >
> > Is this even a valid docker inspect output?
> > In what situations will we get this?
> 
> Guangya Liu wrote:
> Yes, there might be such output if the end user input some invalid env 
> vars.
> 
> root@mesos002:/home/gyliu# docker run -it -e env1=xxx -e env1=xxx -e 
> env1+xx ubuntu:14.04 /bin/bash
> root@84330a72f8b4:/# 
> 
> root@mesos002:/home/gyliu# docker inspect 84330a72f8b4 | grep env1
> "env1=xxx",
> "env1=xxx",
> "env1+xx"
> 
> Timothy Chen wrote:
> I see, so if user manually create a container with bad -e docker doesn't 
> filter them. But how does a user get invalid input with Mesos? We don't allow 
> arbitrary docker runs, so can a docker image has invalid env vars?

Yes, with mesos integration, docker image cannot have invalid vars, but there 
can be indeed duplicate env vars. 

Does it make sense to make the code more robust by adding some checking here?


- Guangya


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


On 二月 7, 2016, 9:40 a.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43293/
> ---
> 
> (Updated 二月 7, 2016, 9:40 a.m.)
> 
> 
> Review request for mesos, haosdent huang, Jie Yu, and Timothy Chen.
> 
> 
> Bugs: MESOS-4607
> https://issues.apache.org/jira/browse/MESOS-4607
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Ignored invalid env vars when creating docker image.
> 
> 
> Diffs
> -
> 
>   src/docker/docker.cpp 52728707d985517e57525af7e470ccb468039373 
>   src/tests/containerizer/docker_tests.cpp 
> 620819330847a10d9dcd817968df9d2b180a9a29 
> 
> Diff: https://reviews.apache.org/r/43293/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> $ GLOG_v=2 ./bin/mesos-tests.sh  
> --gtest_filter="DockerImageTest.ParseInspectonImage" --verbose
> [==] Running 1 test from 1 test case.
> [--] Global test environment set-up.
> [--] 1 test from DockerImageTest
> [ RUN  ] DockerImageTest.ParseInspectonImage
> I0207 10:30:01.894125 2034615040 process.cpp:2489] Spawned process 
> files@192.168.0.100:49551
> I0207 10:30:01.894136 211529728 process.cpp:2499] Resuming 
> files@192.168.0.100:49551 at 2016-02-07 02:30:01.894163968+00:00
> I0207 10:30:01.894317 210456576 process.cpp:2499] Resuming 
> help@192.168.0.100:49551 at 2016-02-07 02:30:01.894332928+00:00
> I0207 10:30:01.898862 2034615040 docker.cpp:397] Skipping duplicate 
> environment variable 'JAVA_VERSION'
> I0207 10:30:01.898892 2034615040 docker.cpp:390] Skipping invalid environment 
> variable 'JAVA_VERSION+8u66' for 'ContainerConfig.Env'
> I0207 10:30:01.899473 20992 process.cpp:2499] Resuming 
> AuthenticationRouter(1)@192.168.0.100:49551 at 2016-02-07 
> 02:30:01.899522048+00:00
> I0207 10:30:01.899983 213139456 process.cpp:2499] Resuming 
> files@192.168.0.100:49551 at 2016-02-07 02:30:01.9+00:00
> I0207 10:30:01.900032 213139456 process.cpp:2604] Cleaning up 
> files@192.168.0.100:49551
> I0207 10:30:01.900244 211529728 process.cpp:2499] Resuming 
> help@192.168.0.100:49551 at 2016-02-07 02:30:01.900256000+00:00
> [   OK ] DockerImageTest.ParseInspectonImage (8 ms)
> [--] 1 test from DockerImageTest (8 ms total)
> 
> [--] Global test environment tear-down
> [==] 1 test from 1 test case ran. (18 ms total)
> [  PASSED  ] 1 test.
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>



Re: Review Request 43293: Ignored invalid env vars.

2016-02-15 Thread Timothy Chen


> On Feb. 14, 2016, 8:42 a.m., Timothy Chen wrote:
> > src/tests/containerizer/docker_tests.cpp, lines 506-507
> > 
> >
> > Is this even a valid docker inspect output?
> > In what situations will we get this?
> 
> Guangya Liu wrote:
> Yes, there might be such output if the end user input some invalid env 
> vars.
> 
> root@mesos002:/home/gyliu# docker run -it -e env1=xxx -e env1=xxx -e 
> env1+xx ubuntu:14.04 /bin/bash
> root@84330a72f8b4:/# 
> 
> root@mesos002:/home/gyliu# docker inspect 84330a72f8b4 | grep env1
> "env1=xxx",
> "env1=xxx",
> "env1+xx"

I see, so if user manually create a container with bad -e docker doesn't filter 
them. But how does a user get invalid input with Mesos? We don't allow 
arbitrary docker runs, so can a docker image has invalid env vars?


- Timothy


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


On Feb. 7, 2016, 9:40 a.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43293/
> ---
> 
> (Updated Feb. 7, 2016, 9:40 a.m.)
> 
> 
> Review request for mesos, haosdent huang, Jie Yu, and Timothy Chen.
> 
> 
> Bugs: MESOS-4607
> https://issues.apache.org/jira/browse/MESOS-4607
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Ignored invalid env vars when creating docker image.
> 
> 
> Diffs
> -
> 
>   src/docker/docker.cpp 52728707d985517e57525af7e470ccb468039373 
>   src/tests/containerizer/docker_tests.cpp 
> 620819330847a10d9dcd817968df9d2b180a9a29 
> 
> Diff: https://reviews.apache.org/r/43293/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> $ GLOG_v=2 ./bin/mesos-tests.sh  
> --gtest_filter="DockerImageTest.ParseInspectonImage" --verbose
> [==] Running 1 test from 1 test case.
> [--] Global test environment set-up.
> [--] 1 test from DockerImageTest
> [ RUN  ] DockerImageTest.ParseInspectonImage
> I0207 10:30:01.894125 2034615040 process.cpp:2489] Spawned process 
> files@192.168.0.100:49551
> I0207 10:30:01.894136 211529728 process.cpp:2499] Resuming 
> files@192.168.0.100:49551 at 2016-02-07 02:30:01.894163968+00:00
> I0207 10:30:01.894317 210456576 process.cpp:2499] Resuming 
> help@192.168.0.100:49551 at 2016-02-07 02:30:01.894332928+00:00
> I0207 10:30:01.898862 2034615040 docker.cpp:397] Skipping duplicate 
> environment variable 'JAVA_VERSION'
> I0207 10:30:01.898892 2034615040 docker.cpp:390] Skipping invalid environment 
> variable 'JAVA_VERSION+8u66' for 'ContainerConfig.Env'
> I0207 10:30:01.899473 20992 process.cpp:2499] Resuming 
> AuthenticationRouter(1)@192.168.0.100:49551 at 2016-02-07 
> 02:30:01.899522048+00:00
> I0207 10:30:01.899983 213139456 process.cpp:2499] Resuming 
> files@192.168.0.100:49551 at 2016-02-07 02:30:01.9+00:00
> I0207 10:30:01.900032 213139456 process.cpp:2604] Cleaning up 
> files@192.168.0.100:49551
> I0207 10:30:01.900244 211529728 process.cpp:2499] Resuming 
> help@192.168.0.100:49551 at 2016-02-07 02:30:01.900256000+00:00
> [   OK ] DockerImageTest.ParseInspectonImage (8 ms)
> [--] 1 test from DockerImageTest (8 ms total)
> 
> [--] Global test environment tear-down
> [==] 1 test from 1 test case ran. (18 ms total)
> [  PASSED  ] 1 test.
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>



Re: Review Request 43491: Added note about implicit default filter to javadoc.

2016-02-15 Thread Bernd Mathiske

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


Ship it!




Ship It!

- Bernd Mathiske


On Feb. 13, 2016, 6:38 p.m., Joerg Schad wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43491/
> ---
> 
> (Updated Feb. 13, 2016, 6:38 p.m.)
> 
> 
> Review request for mesos, Alexander Rojas, Bernd Mathiske, and Joerg Schad.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Make framework developers aware that the MesosSchedulerDriver will add an 
> implicit filter declining unused resources.
> 
> 
> Diffs
> -
> 
>   src/java/src/org/apache/mesos/SchedulerDriver.java 
> efe71b03eb45b295df0e5c250b753a766ed7688c 
> 
> Diff: https://reviews.apache.org/r/43491/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Joerg Schad
> 
>