Re: Review Request 43293: Ignored invalid env vars.

[Joris Van Remoortere]

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



Closing this review due to inactivity. Please see our 
[guidelines](https://github.com/apache/mesos/blob/master/docs/reopening-reviews.md)
 for reopening reviews.

- Joris Van Remoortere


On March 4, 2016, 3:34 p.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43293/
> ---
> 
> (Updated March 4, 2016, 3:34 p.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 
> 7ef52ade0d3389f9e24e3c5c7dda4f8809b9d83f 
> 
> 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.

[Mesos ReviewBot]

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



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 March 4, 2016, 3:34 p.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43293/
> ---
> 
> (Updated March 4, 2016, 3:34 p.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 
> 7ef52ade0d3389f9e24e3c5c7dda4f8809b9d83f 
> 
> 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.

[Guangya Liu]

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

(Updated March 4, 2016, 3:34 p.m.)


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


Changes
---

Rebase


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

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.

[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 43293: Ignored invalid env vars.

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

[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 43293: Ignored invalid env vars.

[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 43293: Ignored invalid env vars.

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

[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 43293: Ignored invalid env vars.

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

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"


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

[Timothy Chen]

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




src/tests/containerizer/docker_tests.cpp (lines 506 - 507)


Is this even a valid docker inspect output?
In what situations will we get this?


- 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 43293: Ignored invalid env vars.

[Gilbert Song]

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


Ship it!




Ship It!

- Gilbert Song


On Feb. 7, 2016, 1: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, 1: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.

[Mesos ReviewBot]

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



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. 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 43293: Ignored invalid env vars.

[Guangya Liu]

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


Changes
---

Addressed haosdent's comments.


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

[Guangya Liu]

> On 二月 7, 2016, 9:13 a.m., haosdent huang wrote:
> > src/docker/docker.cpp, line 398
> > 
> >
> > Seems add quote back would be better.

I was adding '' instead of quota, hope it is OK.


> On 二月 7, 2016, 9:13 a.m., haosdent huang wrote:
> > src/docker/docker.cpp, line 382
> > 
> >
> > Shoud we use LOG(WARNING) here?

This error will not impact the mesos behavior, so I'm just using VLOG(1) 
instead.


- Guangya


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


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.

[haosdent huang]

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


Fix it, then Ship it!




Ship It!


src/docker/docker.cpp (line 382)


Shoud we use LOG(WARNING) here?



src/docker/docker.cpp (line 391)


Suggest add `'` for value here as well.



src/docker/docker.cpp (line 398)


Seems add quote back would be better.


- haosdent huang


On Feb. 7, 2016, 2:35 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, 2:35 a.m.)
> 
> 
> Review request for mesos, 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+8u66for '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.

[Mesos ReviewBot]

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



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. 7, 2016, 2:35 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, 2:35 a.m.)
> 
> 
> Review request for mesos, 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+8u66for '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
> 
>

Review Request 43293: Ignored invalid env vars.

[Guangya Liu]

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

Review request for mesos, 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+8u66for '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