You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Lily Chen <li...@mesosphere.io> on 2015/07/02 02:40:04 UTC
Re: Review Request 34142: AppC provisioner.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34142/#review90173
-----------------------------------------------------------
src/slave/containerizer/provisioners/appc.cpp (lines 143 - 151)
<https://reviews.apache.org/r/34142/#comment143167>
What if the candidate is over-specified (has more labels)? Should this still be a match?
src/slave/containerizer/provisioners/appc.cpp (line 203)
<https://reviews.apache.org/r/34142/#comment143168>
Maybe generalize the Failure message to something like "Provisioner and Image type mismatch"
src/slave/containerizer/provisioners/appc.cpp (lines 484 - 487)
<https://reviews.apache.org/r/34142/#comment143174>
Do we want to remove everything on a failure? This could make finding the failure point difficult.
- Lily Chen
On June 22, 2015, 4:57 p.m., Ian Downes wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34142/
> -----------------------------------------------------------
>
> (Updated June 22, 2015, 4:57 p.m.)
>
>
> Review request for mesos, Chi Zhang, Paul Brett, Timothy Chen, and Vinod Kone.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Discovers image(s), fetches to the image store, then provisions using
> a backend.
>
>
> Diffs
> -----
>
> include/mesos/mesos.proto 8df1211165169c9595e0e6e85b5ddc404345ff70
> src/Makefile.am e7de0f3d1a5efeaef47d5074defe3b40db94f573
> src/slave/containerizer/mesos/containerizer.cpp 8c102fb7d1f79ee768cb06de3a976ea12f958712
> src/slave/containerizer/provisioners/appc.hpp PRE-CREATION
> src/slave/containerizer/provisioners/appc.cpp PRE-CREATION
> src/slave/flags.hpp 7634e368c72e83932dcd992d78eaca146326606b
> src/slave/flags.cpp cbf431eb0627bdaf07241cc0fc4630df06fb20e2
>
> Diff: https://reviews.apache.org/r/34142/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Ian Downes
>
>
Re: Review Request 34142: AppC provisioner.
Posted by Jiang Yan Xu <ya...@jxu.me>.
> On July 1, 2015, 5:40 p.m., Lily Chen wrote:
> > src/slave/containerizer/provisioners/appc.cpp, lines 143-151
> > <https://reviews.apache.org/r/34142/diff/2/?file=989787#file989787line143>
> >
> > What if the candidate is over-specified (has more labels)? Should this still be a match?
According to the spec it should be a match.
- Jiang Yan
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34142/#review90173
-----------------------------------------------------------
On July 7, 2015, 12:43 p.m., Ian Downes wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34142/
> -----------------------------------------------------------
>
> (Updated July 7, 2015, 12:43 p.m.)
>
>
> Review request for mesos, Chi Zhang, Paul Brett, Timothy Chen, and Vinod Kone.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Discovers image(s), fetches to the image store, then provisions using
> a backend.
>
>
> Diffs
> -----
>
> include/mesos/mesos.proto 8df1211165169c9595e0e6e85b5ddc404345ff70
> src/Makefile.am e7de0f3d1a5efeaef47d5074defe3b40db94f573
> src/slave/containerizer/mesos/containerizer.cpp 8c102fb7d1f79ee768cb06de3a976ea12f958712
> src/slave/containerizer/provisioners/appc.hpp PRE-CREATION
> src/slave/containerizer/provisioners/appc.cpp PRE-CREATION
> src/slave/flags.hpp 7634e368c72e83932dcd992d78eaca146326606b
> src/slave/flags.cpp cbf431eb0627bdaf07241cc0fc4630df06fb20e2
>
> Diff: https://reviews.apache.org/r/34142/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Ian Downes
>
>
Re: Review Request 34142: AppC provisioner.
Posted by Jiang Yan Xu <ya...@jxu.me>.
> On July 1, 2015, 5:40 p.m., Lily Chen wrote:
> > src/slave/containerizer/provisioners/appc.cpp, line 203
> > <https://reviews.apache.org/r/34142/diff/2/?file=989787#file989787line203>
> >
> > Maybe generalize the Failure message to something like "Provisioner and Image type mismatch"
Changed to `return Failure("Unsupported container image type: " + stringify(image.type()));`
> On July 1, 2015, 5:40 p.m., Lily Chen wrote:
> > src/slave/containerizer/provisioners/appc.cpp, lines 484-487
> > <https://reviews.apache.org/r/34142/diff/2/?file=989787#file989787line484>
> >
> > Do we want to remove everything on a failure? This could make finding the failure point difficult.
We can probably leave the rootfs dir behind to make it easier to debug.
But another issue is that we cannot actually directly remove the rootfs here because it could be in a state that additionally procedures are necessary to reverse the effect of a partially provisioned rootfs. (e.g. If multiple mounts are required for some backends such as overlayfs and the failure occurred between them).
The important thing is that the provisioner is not supposed to know all these things, the backends are.
Added a TODO to sort this out.
- Jiang Yan
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34142/#review90173
-----------------------------------------------------------
On July 7, 2015, 12:43 p.m., Ian Downes wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34142/
> -----------------------------------------------------------
>
> (Updated July 7, 2015, 12:43 p.m.)
>
>
> Review request for mesos, Chi Zhang, Paul Brett, Timothy Chen, and Vinod Kone.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Discovers image(s), fetches to the image store, then provisions using
> a backend.
>
>
> Diffs
> -----
>
> include/mesos/mesos.proto 8df1211165169c9595e0e6e85b5ddc404345ff70
> src/Makefile.am e7de0f3d1a5efeaef47d5074defe3b40db94f573
> src/slave/containerizer/mesos/containerizer.cpp 8c102fb7d1f79ee768cb06de3a976ea12f958712
> src/slave/containerizer/provisioners/appc.hpp PRE-CREATION
> src/slave/containerizer/provisioners/appc.cpp PRE-CREATION
> src/slave/flags.hpp 7634e368c72e83932dcd992d78eaca146326606b
> src/slave/flags.cpp cbf431eb0627bdaf07241cc0fc4630df06fb20e2
>
> Diff: https://reviews.apache.org/r/34142/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Ian Downes
>
>