You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Joseph Wu <jo...@mesosphere.io> on 2017/10/16 23:41:02 UTC
Re: Review Request 60890: Defined API for launching standalone
containers.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60890/
-----------------------------------------------------------
(Updated Oct. 16, 2017, 4:41 p.m.)
Review request for mesos, Gilbert Song and Jie Yu.
Changes
-------
Rebased and addressed top-level/standalone wording nit.
Summary (updated)
-----------------
Defined API for launching standalone containers.
Bugs: MESOS-7305
https://issues.apache.org/jira/browse/MESOS-7305
Repository: mesos
Description
-------
Launching a standalone container is very similar to launching a
nested container, except that the caller must specify some Resources.
As such, this patch deprecates some nested container APIs
and replaces them with more generically named APIs.
This applies to the Launch, Wait, Kill, and Remove
(nested) container APIs.
Diffs (updated)
-----
include/mesos/agent/agent.proto 4df3dcef4bd1d42cb8b4955e290bd38038355e46
include/mesos/v1/agent/agent.proto e99d23d55a0951f2ed728360e103d83ea5a1ad7f
Diff: https://reviews.apache.org/r/60890/diff/4/
Changes: https://reviews.apache.org/r/60890/diff/3-4/
Testing
-------
make
See later patches in chain.
Thanks,
Joseph Wu
Re: Review Request 60890: Defined API for launching standalone
containers.
Posted by Jie Yu <yu...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60890/#review188449
-----------------------------------------------------------
include/mesos/agent/agent.proto
Lines 230 (patched)
<https://reviews.apache.org/r/60890/#comment265443>
Can you also docuemnt the HTTP code. For instance, that's the HTTP code if container does not exists?
include/mesos/agent/agent.proto
Lines 235 (patched)
<https://reviews.apache.org/r/60890/#comment265444>
Ditto on document HTTP code.
include/mesos/agent/agent.proto
Lines 244 (patched)
<https://reviews.apache.org/r/60890/#comment265445>
Ditto on document HTTP code.
- Jie Yu
On Oct. 16, 2017, 11:41 p.m., Joseph Wu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60890/
> -----------------------------------------------------------
>
> (Updated Oct. 16, 2017, 11:41 p.m.)
>
>
> Review request for mesos, Gilbert Song and Jie Yu.
>
>
> Bugs: MESOS-7305
> https://issues.apache.org/jira/browse/MESOS-7305
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Launching a standalone container is very similar to launching a
> nested container, except that the caller must specify some Resources.
> As such, this patch deprecates some nested container APIs
> and replaces them with more generically named APIs.
>
> This applies to the Launch, Wait, Kill, and Remove
> (nested) container APIs.
>
>
> Diffs
> -----
>
> include/mesos/agent/agent.proto 4df3dcef4bd1d42cb8b4955e290bd38038355e46
> include/mesos/v1/agent/agent.proto e99d23d55a0951f2ed728360e103d83ea5a1ad7f
>
>
> Diff: https://reviews.apache.org/r/60890/diff/4/
>
>
> Testing
> -------
>
> make
>
> See later patches in chain.
>
>
> Thanks,
>
> Joseph Wu
>
>
Re: Review Request 60890: Defined API for launching standalone
containers.
Posted by Jie Yu <yu...@gmail.com>.
> On Nov. 2, 2017, 3:56 p.m., Joseph Wu wrote:
> > include/mesos/agent/agent.proto
> > Lines 253-255 (patched)
> > <https://reviews.apache.org/r/60890/diff/5/?file=1879160#file1879160line253>
> >
> > Leaving this up for discussion:
> >
> > The existing call (`RemoveNestedContainer`) either returns 200 or 500, so for backwards compatibility, `RemoveContainer` does the same.
> >
> > On the other hand, returning a 404 may be somewhat difficult as `Containerizer::remove` returns a `Future<Nothing>`. Other methods like `::launch`, `::wait`, or `::kill` all a magic value if the container doesn't exist.
Can you add a TODO. I think we should add a magic value for `remove` interface as well!
- Jie
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60890/#review189922
-----------------------------------------------------------
On Nov. 2, 2017, 3:50 p.m., Joseph Wu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60890/
> -----------------------------------------------------------
>
> (Updated Nov. 2, 2017, 3:50 p.m.)
>
>
> Review request for mesos, Gilbert Song and Jie Yu.
>
>
> Bugs: MESOS-7305
> https://issues.apache.org/jira/browse/MESOS-7305
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Launching a standalone container is very similar to launching a
> nested container, except that the caller must specify some Resources.
> As such, this patch deprecates some nested container APIs
> and replaces them with more generically named APIs.
>
> This applies to the Launch, Wait, Kill, and Remove
> (nested) container APIs.
>
>
> Diffs
> -----
>
> include/mesos/agent/agent.proto 4df3dcef4bd1d42cb8b4955e290bd38038355e46
> include/mesos/v1/agent/agent.proto e99d23d55a0951f2ed728360e103d83ea5a1ad7f
>
>
> Diff: https://reviews.apache.org/r/60890/diff/5/
>
>
> Testing
> -------
>
> make
>
> See later patches in chain.
>
>
> Thanks,
>
> Joseph Wu
>
>
Re: Review Request 60890: Defined API for launching standalone
containers.
Posted by Joseph Wu <jo...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60890/#review189922
-----------------------------------------------------------
include/mesos/agent/agent.proto
Lines 253-255 (patched)
<https://reviews.apache.org/r/60890/#comment267147>
Leaving this up for discussion:
The existing call (`RemoveNestedContainer`) either returns 200 or 500, so for backwards compatibility, `RemoveContainer` does the same.
On the other hand, returning a 404 may be somewhat difficult as `Containerizer::remove` returns a `Future<Nothing>`. Other methods like `::launch`, `::wait`, or `::kill` all a magic value if the container doesn't exist.
- Joseph Wu
On Nov. 2, 2017, 8:50 a.m., Joseph Wu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60890/
> -----------------------------------------------------------
>
> (Updated Nov. 2, 2017, 8:50 a.m.)
>
>
> Review request for mesos, Gilbert Song and Jie Yu.
>
>
> Bugs: MESOS-7305
> https://issues.apache.org/jira/browse/MESOS-7305
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Launching a standalone container is very similar to launching a
> nested container, except that the caller must specify some Resources.
> As such, this patch deprecates some nested container APIs
> and replaces them with more generically named APIs.
>
> This applies to the Launch, Wait, Kill, and Remove
> (nested) container APIs.
>
>
> Diffs
> -----
>
> include/mesos/agent/agent.proto 4df3dcef4bd1d42cb8b4955e290bd38038355e46
> include/mesos/v1/agent/agent.proto e99d23d55a0951f2ed728360e103d83ea5a1ad7f
>
>
> Diff: https://reviews.apache.org/r/60890/diff/5/
>
>
> Testing
> -------
>
> make
>
> See later patches in chain.
>
>
> Thanks,
>
> Joseph Wu
>
>
Re: Review Request 60890: Defined API for launching standalone
containers.
Posted by Joseph Wu <jo...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60890/
-----------------------------------------------------------
(Updated Nov. 13, 2017, 5:22 p.m.)
Review request for mesos, Gilbert Song and Jie Yu.
Changes
-------
Comment changes. Added a beefy section for `RemoveContainer` and deleted some comments in the deprecated calls to remove some duplication.
Bugs: MESOS-7305
https://issues.apache.org/jira/browse/MESOS-7305
Repository: mesos
Description
-------
Launching a standalone container is very similar to launching a
nested container, except that the caller must specify some Resources.
As such, this patch deprecates some nested container APIs
and replaces them with more generically named APIs.
This applies to the Launch, Wait, Kill, and Remove
(nested) container APIs.
Diffs (updated)
-----
include/mesos/agent/agent.proto 4df3dcef4bd1d42cb8b4955e290bd38038355e46
include/mesos/v1/agent/agent.proto e99d23d55a0951f2ed728360e103d83ea5a1ad7f
Diff: https://reviews.apache.org/r/60890/diff/6/
Changes: https://reviews.apache.org/r/60890/diff/5-6/
Testing
-------
make
See later patches in chain.
Thanks,
Joseph Wu
Re: Review Request 60890: Defined API for launching standalone
containers.
Posted by Joseph Wu <jo...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60890/
-----------------------------------------------------------
(Updated Nov. 2, 2017, 8:50 a.m.)
Review request for mesos, Gilbert Song and Jie Yu.
Changes
-------
Added HTTP response codes to the comments. Also copy-pasted a missing comment from the unversioned proto of WaitNestedContainer to the versioned proto.
Bugs: MESOS-7305
https://issues.apache.org/jira/browse/MESOS-7305
Repository: mesos
Description
-------
Launching a standalone container is very similar to launching a
nested container, except that the caller must specify some Resources.
As such, this patch deprecates some nested container APIs
and replaces them with more generically named APIs.
This applies to the Launch, Wait, Kill, and Remove
(nested) container APIs.
Diffs (updated)
-----
include/mesos/agent/agent.proto 4df3dcef4bd1d42cb8b4955e290bd38038355e46
include/mesos/v1/agent/agent.proto e99d23d55a0951f2ed728360e103d83ea5a1ad7f
Diff: https://reviews.apache.org/r/60890/diff/5/
Changes: https://reviews.apache.org/r/60890/diff/4-5/
Testing
-------
make
See later patches in chain.
Thanks,
Joseph Wu
Re: Review Request 60890: Defined API for launching standalone
containers.
Posted by Jie Yu <yu...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60890/#review188445
-----------------------------------------------------------
include/mesos/agent/agent.proto
Lines 437-440 (patched)
<https://reviews.apache.org/r/60890/#comment265441>
You need a rebase. jpeach recently added some new fields for `WaitNestedContainer`. If you want to deprecate `WAIT_NESTED_CONTAINER`, you'll have to make sure it has feature parity.
- Jie Yu
On Oct. 16, 2017, 11:41 p.m., Joseph Wu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60890/
> -----------------------------------------------------------
>
> (Updated Oct. 16, 2017, 11:41 p.m.)
>
>
> Review request for mesos, Gilbert Song and Jie Yu.
>
>
> Bugs: MESOS-7305
> https://issues.apache.org/jira/browse/MESOS-7305
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Launching a standalone container is very similar to launching a
> nested container, except that the caller must specify some Resources.
> As such, this patch deprecates some nested container APIs
> and replaces them with more generically named APIs.
>
> This applies to the Launch, Wait, Kill, and Remove
> (nested) container APIs.
>
>
> Diffs
> -----
>
> include/mesos/agent/agent.proto 4df3dcef4bd1d42cb8b4955e290bd38038355e46
> include/mesos/v1/agent/agent.proto e99d23d55a0951f2ed728360e103d83ea5a1ad7f
>
>
> Diff: https://reviews.apache.org/r/60890/diff/4/
>
>
> Testing
> -------
>
> make
>
> See later patches in chain.
>
>
> Thanks,
>
> Joseph Wu
>
>