You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Gilbert Song <so...@gmail.com> on 2017/08/12 01:04:48 UTC

Review Request 61602: Fixed mesos containerizer to support docker image WORKDIR missing.

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

Review request for mesos, Jie Yu, Kevin Klues, and Qian Zhang.


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


Repository: mesos


Description
-------

Some docker image may have 'WORKDIR' set in its manifest but that
'WORKDIR' does not exist in the image rootfs (e.g., the workdir
is removed in the following dockerfile).

From the reference of dockerfile, "If the WORKDIR doesn’t exist,
it will be created even if it’s not used in any subsequent
Dockerfile instruction". So we should create the working directory
if it does not exist in the image's rootfs.


Diffs
-----

  src/slave/containerizer/mesos/launch.cpp 8e662931697a2f20e0ac1e80a2911b96f646b5af 


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


Testing
-------

make

Manually tested using 'quay.io/spinnaker/front50:master' docker image.


Thanks,

Gilbert Song


Re: Review Request 61602: Fixed mesos containerizer to support docker image WORKDIR missing.

Posted by Mesos Reviewbot Windows <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/61602/#review182776
-----------------------------------------------------------



Patch looks great!

Reviews applied: [61602]

Logs available here: http://104.210.40.105/logs/master/61602

- Mesos Reviewbot Windows


On Aug. 12, 2017, 1:04 a.m., Gilbert Song wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61602/
> -----------------------------------------------------------
> 
> (Updated Aug. 12, 2017, 1:04 a.m.)
> 
> 
> Review request for mesos, Jie Yu, Kevin Klues, and Qian Zhang.
> 
> 
> Bugs: MESOS-7652
>     https://issues.apache.org/jira/browse/MESOS-7652
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Some docker image may have 'WORKDIR' set in its manifest but that
> 'WORKDIR' does not exist in the image rootfs (e.g., the workdir
> is removed in the following dockerfile).
> 
> From the reference of dockerfile, "If the WORKDIR doesn’t exist,
> it will be created even if it’s not used in any subsequent
> Dockerfile instruction". So we should create the working directory
> if it does not exist in the image's rootfs.
> 
> 
> Diffs
> -----
> 
>   src/slave/containerizer/mesos/launch.cpp 8e662931697a2f20e0ac1e80a2911b96f646b5af 
> 
> 
> Diff: https://reviews.apache.org/r/61602/diff/1/
> 
> 
> Testing
> -------
> 
> make
> 
> Manually tested using 'quay.io/spinnaker/front50:master' docker image.
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>


Re: Review Request 61602: Fixed mesos containerizer to support docker image WORKDIR missing.

Posted by Jie Yu <yu...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/61602/#review183097
-----------------------------------------------------------


Ship it!




Ship It!

- Jie Yu


On Aug. 17, 2017, 3:33 a.m., Gilbert Song wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61602/
> -----------------------------------------------------------
> 
> (Updated Aug. 17, 2017, 3:33 a.m.)
> 
> 
> Review request for mesos, Jie Yu, Kevin Klues, and Qian Zhang.
> 
> 
> Bugs: MESOS-7652
>     https://issues.apache.org/jira/browse/MESOS-7652
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Some docker image may have 'WORKDIR' set in its manifest but that
> 'WORKDIR' does not exist in the image rootfs (e.g., the workdir
> is removed in the following dockerfile).
> 
> From the reference of dockerfile, "If the WORKDIR doesn’t exist,
> it will be created even if it’s not used in any subsequent
> Dockerfile instruction". So we should create the working directory
> if it does not exist in the image's rootfs.
> 
> 
> Diffs
> -----
> 
>   src/slave/containerizer/mesos/launch.cpp 8e662931697a2f20e0ac1e80a2911b96f646b5af 
> 
> 
> Diff: https://reviews.apache.org/r/61602/diff/2/
> 
> 
> Testing
> -------
> 
> make
> 
> Manually tested using 'quay.io/spinnaker/front50:master' docker image.
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>


Re: Review Request 61602: Fixed mesos containerizer to support docker image WORKDIR missing.

Posted by Mesos Reviewbot Windows <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/61602/#review183096
-----------------------------------------------------------



Bad patch!

Reviews applied: [61602]

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

Error:
Traceback (most recent call last):
  File "support/apply-reviews.py", line 417, in <module>
    main()
  File "support/apply-reviews.py", line 412, in main
    reviewboard(options)
  File "support/apply-reviews.py", line 402, in reviewboard
    apply_review(options)
  File "support/apply-reviews.py", line 160, in apply_review
    commit_patch(options)
  File "support/apply-reviews.py", line 261, in commit_patch
    message.write(data['message'])
UnicodeEncodeError: 'ascii' codec can't encode character u'\u2019' in position 295: ordinal not in range(128)

Full log: http://mesos-winbot.westus.cloudapp.azure.com/logs/240/console

- Mesos Reviewbot Windows


On Aug. 16, 2017, 8:33 p.m., Gilbert Song wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61602/
> -----------------------------------------------------------
> 
> (Updated Aug. 16, 2017, 8:33 p.m.)
> 
> 
> Review request for mesos, Jie Yu, Kevin Klues, and Qian Zhang.
> 
> 
> Bugs: MESOS-7652
>     https://issues.apache.org/jira/browse/MESOS-7652
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Some docker image may have 'WORKDIR' set in its manifest but that
> 'WORKDIR' does not exist in the image rootfs (e.g., the workdir
> is removed in the following dockerfile).
> 
> From the reference of dockerfile, "If the WORKDIR doesn’t exist,
> it will be created even if it’s not used in any subsequent
> Dockerfile instruction". So we should create the working directory
> if it does not exist in the image's rootfs.
> 
> 
> Diffs
> -----
> 
>   src/slave/containerizer/mesos/launch.cpp 8e662931697a2f20e0ac1e80a2911b96f646b5af 
> 
> 
> Diff: https://reviews.apache.org/r/61602/diff/2/
> 
> 
> Testing
> -------
> 
> make
> 
> Manually tested using 'quay.io/spinnaker/front50:master' docker image.
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>


Re: Review Request 61602: Fixed mesos containerizer to support docker image WORKDIR missing.

Posted by Gilbert Song <so...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/61602/
-----------------------------------------------------------

(Updated Aug. 16, 2017, 8:33 p.m.)


Review request for mesos, Jie Yu, Kevin Klues, and Qian Zhang.


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


Repository: mesos


Description
-------

Some docker image may have 'WORKDIR' set in its manifest but that
'WORKDIR' does not exist in the image rootfs (e.g., the workdir
is removed in the following dockerfile).

From the reference of dockerfile, "If the WORKDIR doesn’t exist,
it will be created even if it’s not used in any subsequent
Dockerfile instruction". So we should create the working directory
if it does not exist in the image's rootfs.


Diffs (updated)
-----

  src/slave/containerizer/mesos/launch.cpp 8e662931697a2f20e0ac1e80a2911b96f646b5af 


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

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


Testing
-------

make

Manually tested using 'quay.io/spinnaker/front50:master' docker image.


Thanks,

Gilbert Song