You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Jacob Janco <jj...@gmail.com> on 2019/05/14 18:51:17 UTC

Re: Review Request 70581: Add flag to decouple docker runtime.

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

(Updated May 14, 2019, 6:51 p.m.)


Review request for mesos, Gilbert Song and James Peach.


Summary (updated)
-----------------

Add flag to decouple docker runtime.


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


Repository: mesos


Description (updated)
-------

Docker runtime isolator propagates manifest configuration metadata.
This is not always desirable, e.g. a customer may want to ignore the
defined WORKDIR/ENV defined in the manifest. This patch decouples
isolator work from the selection of Docker as an image provider.

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


Diffs (updated)
-----

  docs/configuration/agent.md 04d7114e92c189afd04a8889b3ab853ade9deec6 
  docs/upgrades.md ece8462b9950626166c832d3b554ad51f68f42e1 
  src/slave/containerizer/mesos/containerizer.cpp c4a6827c5574586475152b1dc31572d6e1821a82 
  src/slave/containerizer/mesos/provisioner/store.cpp 11fce0eb47e9e6dfce6289afe04a1d58a0c4461a 
  src/slave/flags.hpp 09921cb6172202b5c1d2f8d03f9ccaeb3d0e8c94 
  src/slave/flags.cpp 49a350f9e13409493fa9612c53d9d58b62122371 
  src/tests/containerizer/provisioner_docker_tests.cpp 1c5d72091d570001ad543e342ab0b9128ffc01ae 


Diff: https://reviews.apache.org/r/70581/diff/4/

Changes: https://reviews.apache.org/r/70581/diff/3-4/


Testing
-------


Thanks,

Jacob Janco


Re: Review Request 70581: Add flag to decouple docker runtime.

Posted by James Peach <jp...@apache.org>.

> On May 15, 2019, 6:16 a.m., Qian Zhang wrote:
> > src/slave/containerizer/mesos/containerizer.cpp
> > Lines 230-235 (patched)
> > <https://reviews.apache.org/r/70581/diff/4/?file=2145030#file2145030line230>
> >
> >     s/ignore_docker_runtime_isolator/docker_ignore_runtime/
> >     
> >     And erasing `docker/runtime` from `isolations` seems a bit weird to me, should we instead add a flag (like `--ignore-docker-image-manifest`) and let `docker/runtime` isolator handle it (i.e., if it is true the isolator will just do nothing)? Or maybe this should be a configuration per container? Like add a field in `ContainerInfo`.

> s/ignore_docker_runtime_isolator/docker_ignore_runtime/

Agreed.

> And erasing docker/runtime from isolations seems a bit weird to me, should we instead add a flag (like --ignore-docker-image-manifest) and
> let docker/runtime isolator handle it (i.e., if it is true the isolator will just do nothing)? Or maybe this should be a configuration per
> container? Like add a field in ContainerInfo.

We did go back and forth on this a bit. The documentation is pretty clear in a number of places that `docker/runtime` is required and it is the isolator that deals with the Docker runtime config. So in the end I thought that it was clearer to link the flag name to the isolator name quite explicitly. Once you do that, then the implementation follows naturally. Whether we drop the isolator or whether the isolator remains but takes no action is an implementation detail that is invisible to an operator.

I would have prefered that we just allow the `docker/runtime` isolator the be absent, but we felt constrained by backwards compatibility a bit here.

It did not occur to me to make this a per-image flag. In the future, this could still be added. We could change the `docker/runtime` isolator to check the `ContainerInfo` flag as you suggest. However, our reason for needing this is that we have to retain compatibility with an existing PaaS implementation, so I think that the need for this as a container configuration is limited.


- James


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


On May 14, 2019, 6:51 p.m., Jacob Janco wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70581/
> -----------------------------------------------------------
> 
> (Updated May 14, 2019, 6:51 p.m.)
> 
> 
> Review request for mesos, Gilbert Song and James Peach.
> 
> 
> Bugs: MESOS-9760
>     https://issues.apache.org/jira/browse/MESOS-9760
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Docker runtime isolator propagates manifest configuration metadata.
> This is not always desirable, e.g. a customer may want to ignore the
> defined WORKDIR/ENV defined in the manifest. This patch decouples
> isolator work from the selection of Docker as an image provider.
> 
> https://issues.apache.org/jira/browse/MESOS-9760
> 
> 
> Diffs
> -----
> 
>   docs/configuration/agent.md 04d7114e92c189afd04a8889b3ab853ade9deec6 
>   docs/upgrades.md ece8462b9950626166c832d3b554ad51f68f42e1 
>   src/slave/containerizer/mesos/containerizer.cpp c4a6827c5574586475152b1dc31572d6e1821a82 
>   src/slave/containerizer/mesos/provisioner/store.cpp 11fce0eb47e9e6dfce6289afe04a1d58a0c4461a 
>   src/slave/flags.hpp 09921cb6172202b5c1d2f8d03f9ccaeb3d0e8c94 
>   src/slave/flags.cpp 49a350f9e13409493fa9612c53d9d58b62122371 
>   src/tests/containerizer/provisioner_docker_tests.cpp 1c5d72091d570001ad543e342ab0b9128ffc01ae 
> 
> 
> Diff: https://reviews.apache.org/r/70581/diff/4/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Jacob Janco
> 
>


Re: Review Request 70581: Add flag to decouple docker runtime.

Posted by Jacob Janco <jj...@gmail.com>.

> On May 15, 2019, 6:16 a.m., Qian Zhang wrote:
> > src/slave/containerizer/mesos/containerizer.cpp
> > Lines 230-235 (patched)
> > <https://reviews.apache.org/r/70581/diff/4/?file=2145030#file2145030line230>
> >
> >     s/ignore_docker_runtime_isolator/docker_ignore_runtime/
> >     
> >     And erasing `docker/runtime` from `isolations` seems a bit weird to me, should we instead add a flag (like `--ignore-docker-image-manifest`) and let `docker/runtime` isolator handle it (i.e., if it is true the isolator will just do nothing)? Or maybe this should be a configuration per container? Like add a field in `ContainerInfo`.
> 
> James Peach wrote:
>     > s/ignore_docker_runtime_isolator/docker_ignore_runtime/
>     
>     Agreed.
>     
>     > And erasing docker/runtime from isolations seems a bit weird to me, should we instead add a flag (like --ignore-docker-image-manifest) and
>     > let docker/runtime isolator handle it (i.e., if it is true the isolator will just do nothing)? Or maybe this should be a configuration per
>     > container? Like add a field in ContainerInfo.
>     
>     We did go back and forth on this a bit. The documentation is pretty clear in a number of places that `docker/runtime` is required and it is the isolator that deals with the Docker runtime config. So in the end I thought that it was clearer to link the flag name to the isolator name quite explicitly. Once you do that, then the implementation follows naturally. Whether we drop the isolator or whether the isolator remains but takes no action is an implementation detail that is invisible to an operator.
>     
>     I would have prefered that we just allow the `docker/runtime` isolator the be absent, but we felt constrained by backwards compatibility a bit here.
>     
>     It did not occur to me to make this a per-image flag. In the future, this could still be added. We could change the `docker/runtime` isolator to check the `ContainerInfo` flag as you suggest. However, our reason for needing this is that we have to retain compatibility with an existing PaaS implementation, so I think that the need for this as a container configuration is limited.
> 
> Qian Zhang wrote:
>     I think what we usually do in Mesos code is an agent flag for agent-wide configuration and a protobuf message field for per container configuration. I agree that we can implement the per container configuration later, but for the agent flag I would suggest to let the `docker/runtime` isolatior handle it rather than using the flag to drop the isolator because in future when we add the per containner configuration, the implemenation would be nature and easy to read since all the codes are in a single place (the isolator) rather than in multiple places (like the containerizer and store).

This seems reasonable - I'll move this into the isolator.


- Jacob


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


On May 14, 2019, 6:51 p.m., Jacob Janco wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70581/
> -----------------------------------------------------------
> 
> (Updated May 14, 2019, 6:51 p.m.)
> 
> 
> Review request for mesos, Gilbert Song and James Peach.
> 
> 
> Bugs: MESOS-9760
>     https://issues.apache.org/jira/browse/MESOS-9760
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Docker runtime isolator propagates manifest configuration metadata.
> This is not always desirable, e.g. a customer may want to ignore the
> defined WORKDIR/ENV defined in the manifest. This patch decouples
> isolator work from the selection of Docker as an image provider.
> 
> https://issues.apache.org/jira/browse/MESOS-9760
> 
> 
> Diffs
> -----
> 
>   docs/configuration/agent.md 04d7114e92c189afd04a8889b3ab853ade9deec6 
>   docs/upgrades.md ece8462b9950626166c832d3b554ad51f68f42e1 
>   src/slave/containerizer/mesos/containerizer.cpp c4a6827c5574586475152b1dc31572d6e1821a82 
>   src/slave/containerizer/mesos/provisioner/store.cpp 11fce0eb47e9e6dfce6289afe04a1d58a0c4461a 
>   src/slave/flags.hpp 09921cb6172202b5c1d2f8d03f9ccaeb3d0e8c94 
>   src/slave/flags.cpp 49a350f9e13409493fa9612c53d9d58b62122371 
>   src/tests/containerizer/provisioner_docker_tests.cpp 1c5d72091d570001ad543e342ab0b9128ffc01ae 
> 
> 
> Diff: https://reviews.apache.org/r/70581/diff/4/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Jacob Janco
> 
>


Re: Review Request 70581: Add flag to decouple docker runtime.

Posted by Qian Zhang <zh...@gmail.com>.

> On May 15, 2019, 2:16 p.m., Qian Zhang wrote:
> > src/slave/containerizer/mesos/containerizer.cpp
> > Lines 230-235 (patched)
> > <https://reviews.apache.org/r/70581/diff/4/?file=2145030#file2145030line230>
> >
> >     s/ignore_docker_runtime_isolator/docker_ignore_runtime/
> >     
> >     And erasing `docker/runtime` from `isolations` seems a bit weird to me, should we instead add a flag (like `--ignore-docker-image-manifest`) and let `docker/runtime` isolator handle it (i.e., if it is true the isolator will just do nothing)? Or maybe this should be a configuration per container? Like add a field in `ContainerInfo`.
> 
> James Peach wrote:
>     > s/ignore_docker_runtime_isolator/docker_ignore_runtime/
>     
>     Agreed.
>     
>     > And erasing docker/runtime from isolations seems a bit weird to me, should we instead add a flag (like --ignore-docker-image-manifest) and
>     > let docker/runtime isolator handle it (i.e., if it is true the isolator will just do nothing)? Or maybe this should be a configuration per
>     > container? Like add a field in ContainerInfo.
>     
>     We did go back and forth on this a bit. The documentation is pretty clear in a number of places that `docker/runtime` is required and it is the isolator that deals with the Docker runtime config. So in the end I thought that it was clearer to link the flag name to the isolator name quite explicitly. Once you do that, then the implementation follows naturally. Whether we drop the isolator or whether the isolator remains but takes no action is an implementation detail that is invisible to an operator.
>     
>     I would have prefered that we just allow the `docker/runtime` isolator the be absent, but we felt constrained by backwards compatibility a bit here.
>     
>     It did not occur to me to make this a per-image flag. In the future, this could still be added. We could change the `docker/runtime` isolator to check the `ContainerInfo` flag as you suggest. However, our reason for needing this is that we have to retain compatibility with an existing PaaS implementation, so I think that the need for this as a container configuration is limited.

I think what we usually do in Mesos code is an agent flag for agent-wide configuration and a protobuf message field for per container configuration. I agree that we can implement the per container configuration later, but for the agent flag I would suggest to let the `docker/runtime` isolatior handle it rather than using the flag to drop the isolator because in future when we add the per containner configuration, the implemenation would be nature and easy to read since all the codes are in a single place (the isolator) rather than in multiple places (like the containerizer and store).


- Qian


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


On May 15, 2019, 2:51 a.m., Jacob Janco wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70581/
> -----------------------------------------------------------
> 
> (Updated May 15, 2019, 2:51 a.m.)
> 
> 
> Review request for mesos, Gilbert Song and James Peach.
> 
> 
> Bugs: MESOS-9760
>     https://issues.apache.org/jira/browse/MESOS-9760
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Docker runtime isolator propagates manifest configuration metadata.
> This is not always desirable, e.g. a customer may want to ignore the
> defined WORKDIR/ENV defined in the manifest. This patch decouples
> isolator work from the selection of Docker as an image provider.
> 
> https://issues.apache.org/jira/browse/MESOS-9760
> 
> 
> Diffs
> -----
> 
>   docs/configuration/agent.md 04d7114e92c189afd04a8889b3ab853ade9deec6 
>   docs/upgrades.md ece8462b9950626166c832d3b554ad51f68f42e1 
>   src/slave/containerizer/mesos/containerizer.cpp c4a6827c5574586475152b1dc31572d6e1821a82 
>   src/slave/containerizer/mesos/provisioner/store.cpp 11fce0eb47e9e6dfce6289afe04a1d58a0c4461a 
>   src/slave/flags.hpp 09921cb6172202b5c1d2f8d03f9ccaeb3d0e8c94 
>   src/slave/flags.cpp 49a350f9e13409493fa9612c53d9d58b62122371 
>   src/tests/containerizer/provisioner_docker_tests.cpp 1c5d72091d570001ad543e342ab0b9128ffc01ae 
> 
> 
> Diff: https://reviews.apache.org/r/70581/diff/4/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Jacob Janco
> 
>


Re: Review Request 70581: Add flag to decouple docker runtime.

Posted by Qian Zhang <zh...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/70581/#review215279
-----------------------------------------------------------




src/slave/containerizer/mesos/containerizer.cpp
Lines 230-235 (patched)
<https://reviews.apache.org/r/70581/#comment301933>

    s/ignore_docker_runtime_isolator/docker_ignore_runtime/
    
    And erasing `docker/runtime` from `isolations` seems a bit weird to me, should we instead add a flag (like `--ignore-docker-image-manifest`) and let `docker/runtime` isolator handle it (i.e., if it is true the isolator will just do nothing)? Or maybe this should be a configuration per container? Like add a field in `ContainerInfo`.


- Qian Zhang


On May 15, 2019, 2:51 a.m., Jacob Janco wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70581/
> -----------------------------------------------------------
> 
> (Updated May 15, 2019, 2:51 a.m.)
> 
> 
> Review request for mesos, Gilbert Song and James Peach.
> 
> 
> Bugs: MESOS-9760
>     https://issues.apache.org/jira/browse/MESOS-9760
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Docker runtime isolator propagates manifest configuration metadata.
> This is not always desirable, e.g. a customer may want to ignore the
> defined WORKDIR/ENV defined in the manifest. This patch decouples
> isolator work from the selection of Docker as an image provider.
> 
> https://issues.apache.org/jira/browse/MESOS-9760
> 
> 
> Diffs
> -----
> 
>   docs/configuration/agent.md 04d7114e92c189afd04a8889b3ab853ade9deec6 
>   docs/upgrades.md ece8462b9950626166c832d3b554ad51f68f42e1 
>   src/slave/containerizer/mesos/containerizer.cpp c4a6827c5574586475152b1dc31572d6e1821a82 
>   src/slave/containerizer/mesos/provisioner/store.cpp 11fce0eb47e9e6dfce6289afe04a1d58a0c4461a 
>   src/slave/flags.hpp 09921cb6172202b5c1d2f8d03f9ccaeb3d0e8c94 
>   src/slave/flags.cpp 49a350f9e13409493fa9612c53d9d58b62122371 
>   src/tests/containerizer/provisioner_docker_tests.cpp 1c5d72091d570001ad543e342ab0b9128ffc01ae 
> 
> 
> Diff: https://reviews.apache.org/r/70581/diff/4/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Jacob Janco
> 
>


Re: Review Request 70581: Add flag to decouple docker runtime.

Posted by James Peach <jp...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/70581/#review215281
-----------------------------------------------------------




docs/upgrades.md
Lines 508 (patched)
<https://reviews.apache.org/r/70581/#comment301942>

    You can link the flag to it's official doc anchor:
    ```
    [`--docker_ignore_runtime`](configuration/agent.md#docker_ignore_runtime)
    ```
    
    Phrase the paragraph like this:
    ```
    A new [--flag]() has been added. This causes the agent to ignore any runtime configuration present in Docker images.
    ```



src/slave/containerizer/mesos/containerizer.cpp
Lines 231 (patched)
<https://reviews.apache.org/r/70581/#comment301935>

    Should be `--docker_ignore_runtime`. Make it `if the xxx flag is present`.



src/slave/containerizer/mesos/containerizer.cpp
Lines 233 (patched)
<https://reviews.apache.org/r/70581/#comment301936>

    This fits nicely on one line.



src/slave/containerizer/mesos/provisioner/store.cpp
Lines 82 (patched)
<https://reviews.apache.org/r/70581/#comment301937>

    Sorry, you were right about this originally. As @qianzhang points out, if we keep this check, we preserve our ability to change the implementation in the future.



src/tests/containerizer/provisioner_docker_tests.cpp
Lines 1389 (patched)
<https://reviews.apache.org/r/70581/#comment301938>

    I think that the middle sentence here isn't necessary. Could you please capitalize `Docker` consistently?



src/tests/containerizer/provisioner_docker_tests.cpp
Lines 1394 (patched)
<https://reviews.apache.org/r/70581/#comment301941>

    You can drop the `CURL` token from here since you don't have a curl dependency.



src/tests/containerizer/provisioner_docker_tests.cpp
Lines 1414 (patched)
<https://reviews.apache.org/r/70581/#comment301939>

    I guess you will have to add `docker/runtime` back to the isolation.



src/tests/containerizer/provisioner_docker_tests.cpp
Lines 1470 (patched)
<https://reviews.apache.org/r/70581/#comment301940>

    Do you really need 60sec here? Unless there's a good reason, just use `AWAIT_READY()`.


- James Peach


On May 14, 2019, 6:51 p.m., Jacob Janco wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70581/
> -----------------------------------------------------------
> 
> (Updated May 14, 2019, 6:51 p.m.)
> 
> 
> Review request for mesos, Gilbert Song and James Peach.
> 
> 
> Bugs: MESOS-9760
>     https://issues.apache.org/jira/browse/MESOS-9760
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Docker runtime isolator propagates manifest configuration metadata.
> This is not always desirable, e.g. a customer may want to ignore the
> defined WORKDIR/ENV defined in the manifest. This patch decouples
> isolator work from the selection of Docker as an image provider.
> 
> https://issues.apache.org/jira/browse/MESOS-9760
> 
> 
> Diffs
> -----
> 
>   docs/configuration/agent.md 04d7114e92c189afd04a8889b3ab853ade9deec6 
>   docs/upgrades.md ece8462b9950626166c832d3b554ad51f68f42e1 
>   src/slave/containerizer/mesos/containerizer.cpp c4a6827c5574586475152b1dc31572d6e1821a82 
>   src/slave/containerizer/mesos/provisioner/store.cpp 11fce0eb47e9e6dfce6289afe04a1d58a0c4461a 
>   src/slave/flags.hpp 09921cb6172202b5c1d2f8d03f9ccaeb3d0e8c94 
>   src/slave/flags.cpp 49a350f9e13409493fa9612c53d9d58b62122371 
>   src/tests/containerizer/provisioner_docker_tests.cpp 1c5d72091d570001ad543e342ab0b9128ffc01ae 
> 
> 
> Diff: https://reviews.apache.org/r/70581/diff/4/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Jacob Janco
> 
>


Re: Review Request 70581: Add flag to decouple docker runtime.

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



Patch looks great!

Reviews applied: [70581]

Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' CONFIGURATION='--verbose --disable-libtool-wrappers --disable-parallel-test-execution' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker-build.sh

- Mesos Reviewbot


On May 15, 2019, 11:21 p.m., Jacob Janco wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70581/
> -----------------------------------------------------------
> 
> (Updated May 15, 2019, 11:21 p.m.)
> 
> 
> Review request for mesos, Gilbert Song and James Peach.
> 
> 
> Bugs: MESOS-9760
>     https://issues.apache.org/jira/browse/MESOS-9760
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Docker runtime isolator propagates manifest configuration metadata.
> This is not always desirable, e.g. a customer may want to ignore the
> defined WORKDIR/ENV defined in the manifest. This patch decouples
> isolator work from the selection of Docker as an image provider.
> 
> https://issues.apache.org/jira/browse/MESOS-9760
> 
> 
> Diffs
> -----
> 
>   docs/configuration/agent.md 04d7114e92c189afd04a8889b3ab853ade9deec6 
>   docs/upgrades.md ece8462b9950626166c832d3b554ad51f68f42e1 
>   src/slave/containerizer/mesos/isolators/docker/runtime.cpp 53d00082dae4980426294f0f4be210e53c7c8b48 
>   src/slave/flags.hpp 09921cb6172202b5c1d2f8d03f9ccaeb3d0e8c94 
>   src/slave/flags.cpp 49a350f9e13409493fa9612c53d9d58b62122371 
>   src/tests/containerizer/provisioner_docker_tests.cpp 1c5d72091d570001ad543e342ab0b9128ffc01ae 
> 
> 
> Diff: https://reviews.apache.org/r/70581/diff/6/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Jacob Janco
> 
>


Re: Review Request 70581: Add flag to decouple docker runtime.

Posted by James Peach <jp...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/70581/#review215290
-----------------------------------------------------------


Ship it!




Looks good to me.

I'm make some cosmetic adjustments to the commit message when I land this.

- James Peach


On May 15, 2019, 11:21 p.m., Jacob Janco wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70581/
> -----------------------------------------------------------
> 
> (Updated May 15, 2019, 11:21 p.m.)
> 
> 
> Review request for mesos, Gilbert Song and James Peach.
> 
> 
> Bugs: MESOS-9760
>     https://issues.apache.org/jira/browse/MESOS-9760
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Docker runtime isolator propagates manifest configuration metadata.
> This is not always desirable, e.g. a customer may want to ignore the
> defined WORKDIR/ENV defined in the manifest. This patch decouples
> isolator work from the selection of Docker as an image provider.
> 
> https://issues.apache.org/jira/browse/MESOS-9760
> 
> 
> Diffs
> -----
> 
>   docs/configuration/agent.md 04d7114e92c189afd04a8889b3ab853ade9deec6 
>   docs/upgrades.md ece8462b9950626166c832d3b554ad51f68f42e1 
>   src/slave/containerizer/mesos/isolators/docker/runtime.cpp 53d00082dae4980426294f0f4be210e53c7c8b48 
>   src/slave/flags.hpp 09921cb6172202b5c1d2f8d03f9ccaeb3d0e8c94 
>   src/slave/flags.cpp 49a350f9e13409493fa9612c53d9d58b62122371 
>   src/tests/containerizer/provisioner_docker_tests.cpp 1c5d72091d570001ad543e342ab0b9128ffc01ae 
> 
> 
> Diff: https://reviews.apache.org/r/70581/diff/6/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Jacob Janco
> 
>


Re: Review Request 70581: Add flag to decouple docker runtime.

Posted by Jacob Janco <jj...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/70581/
-----------------------------------------------------------

(Updated May 15, 2019, 11:21 p.m.)


Review request for mesos, Gilbert Song and James Peach.


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


Repository: mesos


Description
-------

Docker runtime isolator propagates manifest configuration metadata.
This is not always desirable, e.g. a customer may want to ignore the
defined WORKDIR/ENV defined in the manifest. This patch decouples
isolator work from the selection of Docker as an image provider.

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


Diffs (updated)
-----

  docs/configuration/agent.md 04d7114e92c189afd04a8889b3ab853ade9deec6 
  docs/upgrades.md ece8462b9950626166c832d3b554ad51f68f42e1 
  src/slave/containerizer/mesos/isolators/docker/runtime.cpp 53d00082dae4980426294f0f4be210e53c7c8b48 
  src/slave/flags.hpp 09921cb6172202b5c1d2f8d03f9ccaeb3d0e8c94 
  src/slave/flags.cpp 49a350f9e13409493fa9612c53d9d58b62122371 
  src/tests/containerizer/provisioner_docker_tests.cpp 1c5d72091d570001ad543e342ab0b9128ffc01ae 


Diff: https://reviews.apache.org/r/70581/diff/6/

Changes: https://reviews.apache.org/r/70581/diff/5-6/


Testing
-------


Thanks,

Jacob Janco


Re: Review Request 70581: Add flag to decouple docker runtime.

Posted by Jacob Janco <jj...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/70581/
-----------------------------------------------------------

(Updated May 15, 2019, 11:17 p.m.)


Review request for mesos, Gilbert Song and James Peach.


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


Repository: mesos


Description
-------

Docker runtime isolator propagates manifest configuration metadata.
This is not always desirable, e.g. a customer may want to ignore the
defined WORKDIR/ENV defined in the manifest. This patch decouples
isolator work from the selection of Docker as an image provider.

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


Diffs (updated)
-----

  docs/configuration/agent.md 04d7114e92c189afd04a8889b3ab853ade9deec6 
  docs/upgrades.md ece8462b9950626166c832d3b554ad51f68f42e1 
  src/slave/containerizer/mesos/isolators/docker/runtime.cpp 53d00082dae4980426294f0f4be210e53c7c8b48 
  src/slave/flags.hpp 09921cb6172202b5c1d2f8d03f9ccaeb3d0e8c94 
  src/slave/flags.cpp 49a350f9e13409493fa9612c53d9d58b62122371 
  src/tests/containerizer/provisioner_docker_tests.cpp 1c5d72091d570001ad543e342ab0b9128ffc01ae 


Diff: https://reviews.apache.org/r/70581/diff/5/

Changes: https://reviews.apache.org/r/70581/diff/4-5/


Testing
-------


Thanks,

Jacob Janco


Re: Review Request 70581: Add flag to decouple docker runtime.

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



Patch looks great!

Reviews applied: [70581]

Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' CONFIGURATION='--verbose --disable-libtool-wrappers --disable-parallel-test-execution' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker-build.sh

- Mesos Reviewbot


On May 14, 2019, 6:51 p.m., Jacob Janco wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70581/
> -----------------------------------------------------------
> 
> (Updated May 14, 2019, 6:51 p.m.)
> 
> 
> Review request for mesos, Gilbert Song and James Peach.
> 
> 
> Bugs: MESOS-9760
>     https://issues.apache.org/jira/browse/MESOS-9760
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Docker runtime isolator propagates manifest configuration metadata.
> This is not always desirable, e.g. a customer may want to ignore the
> defined WORKDIR/ENV defined in the manifest. This patch decouples
> isolator work from the selection of Docker as an image provider.
> 
> https://issues.apache.org/jira/browse/MESOS-9760
> 
> 
> Diffs
> -----
> 
>   docs/configuration/agent.md 04d7114e92c189afd04a8889b3ab853ade9deec6 
>   docs/upgrades.md ece8462b9950626166c832d3b554ad51f68f42e1 
>   src/slave/containerizer/mesos/containerizer.cpp c4a6827c5574586475152b1dc31572d6e1821a82 
>   src/slave/containerizer/mesos/provisioner/store.cpp 11fce0eb47e9e6dfce6289afe04a1d58a0c4461a 
>   src/slave/flags.hpp 09921cb6172202b5c1d2f8d03f9ccaeb3d0e8c94 
>   src/slave/flags.cpp 49a350f9e13409493fa9612c53d9d58b62122371 
>   src/tests/containerizer/provisioner_docker_tests.cpp 1c5d72091d570001ad543e342ab0b9128ffc01ae 
> 
> 
> Diff: https://reviews.apache.org/r/70581/diff/4/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Jacob Janco
> 
>