You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Gastón Kleiman <ga...@mesosphere.com> on 2017/01/12 15:30:25 UTC
Review Request 55464: Made the Agent API able to handle containers
nested at arbitrary levels.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55464/
-----------------------------------------------------------
Review request for mesos, Adam B, Alexander Rukletsov, Anand Mazumdar, Alexander Rojas, and haosdent huang.
Bugs: MESOS-6864
https://issues.apache.org/jira/browse/MESOS-6864
Repository: mesos
Description
-------
Made the Agent API able to handle containers nested at arbitrary levels.
Diffs
-----
src/slave/http.cpp 24fc23b229c624835a24cdda9587c99c6ac9c3bb
src/slave/slave.cpp 11e8833fd5998abb71a7bb08e5dec451d894aba9
src/tests/api_tests.cpp ea62226fbc5dd0bcc5ea60d1bbcd3748f9367467
Diff: https://reviews.apache.org/r/55464/diff/
Testing
-------
`make check` in Linux.
I have a proof of concept that makes the `DefaultExecutor` leverage this change to perform CMD health checks.
Thanks,
Gast�n Kleiman
Re: Review Request 55464: Made the Agent API able to handle containers
nested at arbitrary levels.
Posted by Mesos ReviewBot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55464/#review161515
-----------------------------------------------------------
Patch looks great!
Reviews applied: [55463, 55464]
Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' CONFIGURATION='--verbose' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker_build.sh
- Mesos ReviewBot
On Jan. 13, 2017, 10:08 a.m., Gast�n Kleiman wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55464/
> -----------------------------------------------------------
>
> (Updated Jan. 13, 2017, 10:08 a.m.)
>
>
> Review request for mesos, Adam B, Alexander Rukletsov, Anand Mazumdar, Alexander Rojas, and haosdent huang.
>
>
> Bugs: MESOS-6864
> https://issues.apache.org/jira/browse/MESOS-6864
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Made the Agent API able to handle containers nested at arbitrary levels.
>
>
> Diffs
> -----
>
> src/slave/http.cpp 24fc23b229c624835a24cdda9587c99c6ac9c3bb
> src/slave/slave.cpp 11e8833fd5998abb71a7bb08e5dec451d894aba9
> src/tests/api_tests.cpp ea62226fbc5dd0bcc5ea60d1bbcd3748f9367467
>
> Diff: https://reviews.apache.org/r/55464/diff/
>
>
> Testing
> -------
>
> `GTEST_FILTER="" make -j 8 check` in macOS.
> `make check` in Linux.
>
> I also did some manual testing using a proof of concept that makes the `DefaultExecutor` leverage this change to perform CMD health checks.
>
>
> Thanks,
>
> Gast�n Kleiman
>
>
Re: Review Request 55464: Made the Agent API able to handle containers
nested at arbitrary levels.
Posted by Gastón Kleiman <ga...@mesosphere.com>.
> On Jan. 16, 2017, 7:33 p.m., Anand Mazumdar wrote:
> > src/slave/http.cpp, line 2184
> > <https://reviews.apache.org/r/55464/diff/3/?file=1604722#file1604722line2184>
> >
> > Not yours, but we should rename this to `getExecutor(...)` to be consistent with other similar functions in the same file.
> >
> > Worth doing in an earlier review with this review as dependent on it.
https://reviews.apache.org/r/55678/
> On Jan. 16, 2017, 7:33 p.m., Anand Mazumdar wrote:
> > src/slave/http.cpp, line 2199
> > <https://reviews.apache.org/r/55464/diff/3/?file=1604722#file1604722line2199>
> >
> > hmm, it looks error prone to look up frameworks only when an executor could be found and then do error checking on L2192. It was fine to do so earlier since the information was being populated while looping at one place.
> >
> > How about:
> > ```cpp
> > Executor* executor = slave->getExecutor(containerId);
> > if (executor == nullptr) {
> > return NotFound("Unable to locate executor for container"
> > " " + stringify(containerId));
> > }
> >
> > Framework* framework = slave->getFramework(executor->frameworkId);
> > CHECK_NOTNULL(framework);
> > ```
> >
> > Note that the following invariant is always true that is an executor is *found*, it would be associated with a framework.
https://reviews.apache.org/r/55679/
> On Jan. 16, 2017, 7:33 p.m., Anand Mazumdar wrote:
> > src/slave/http.cpp, lines 2209-2214
> > <https://reviews.apache.org/r/55464/diff/3/?file=1604722#file1604722line2209>
> >
> > hmm, why return a `BadRequest` here? This looks inconsistent with other instances where we return `NotFound`. It does not seem that there was anything malformed with the client request. This can also happen when the nested container exited right before the request was received on the agent.
> >
> > Also, can we make the other error messages consistent with this?
> > (See the snippet I posted in the previous comment)
Also in https://reviews.apache.org/r/55679/
> On Jan. 16, 2017, 7:33 p.m., Anand Mazumdar wrote:
> > src/slave/slave.cpp, lines 6227-6236
> > <https://reviews.apache.org/r/55464/diff/3/?file=1604723#file1604723line6227>
> >
> > hmm, can we somehow reuse the helper `getRootContainerId()` in `slave/containerizer/mesos/utils.hpp`?
> >
> > You might have to pull it out though first and then re-use it in this review? Strictly speaking, this method is pretty generic and should not be a part of the Mesos containerizer code.
https://reviews.apache.org/r/55676/
- Gast�n
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55464/#review161753
-----------------------------------------------------------
On Jan. 18, 2017, 2:48 p.m., Gast�n Kleiman wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55464/
> -----------------------------------------------------------
>
> (Updated Jan. 18, 2017, 2:48 p.m.)
>
>
> Review request for mesos, Adam B, Alexander Rukletsov, Anand Mazumdar, Alexander Rojas, haosdent huang, and Vinod Kone.
>
>
> Bugs: MESOS-6864
> https://issues.apache.org/jira/browse/MESOS-6864
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Made the Agent API able to handle containers nested at arbitrary levels.
>
>
> Diffs
> -----
>
> src/slave/http.cpp ee7119179a6ddd935c3f43a618ef645619c305ee
> src/slave/slave.cpp ae183e6b8b40094b4764525a6d63164f210ef9d8
> src/tests/api_tests.cpp ea62226fbc5dd0bcc5ea60d1bbcd3748f9367467
>
> Diff: https://reviews.apache.org/r/55464/diff/
>
>
> Testing
> -------
>
> `GTEST_FILTER="" make -j 8 check` in macOS.
> `make check` in Linux.
>
> I also did some manual testing using a proof of concept that makes the `DefaultExecutor` leverage this change to perform CMD health checks.
>
>
> Thanks,
>
> Gast�n Kleiman
>
>
Re: Review Request 55464: Made the Agent API able to handle containers
nested at arbitrary levels.
Posted by Anand Mazumdar <an...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55464/#review161753
-----------------------------------------------------------
Looks good for the most part! Mostly minor cleanup comments.
src/slave/http.cpp (line 2184)
<https://reviews.apache.org/r/55464/#comment233051>
Not yours, but we should rename this to `getExecutor(...)` to be consistent with other similar functions in the same file.
Worth doing in an earlier review with this review as dependent on it.
src/slave/http.cpp (line 2186)
<https://reviews.apache.org/r/55464/#comment233057>
hmm, it looks error prone to look up frameworks only when an executor could be found and then do error checking on L2192. It was fine to do so earlier since the information was being populated while looping at one place.
How about:
```cpp
Executor* executor = slave->getExecutor(containerId);
if (executor == nullptr) {
return NotFound("Unable to locate executor for container"
" " + stringify(containerId));
}
Framework* framework = slave->getFramework(executor->frameworkId);
CHECK_NOTNULL(framework);
```
Note that the following invariant is always true that is an executor is *found*, it would be associated with a framework.
src/slave/http.cpp (lines 2190 - 2195)
<https://reviews.apache.org/r/55464/#comment233056>
hmm, why return a `BadRequest` here? This looks inconsistent with other instances where we return `NotFound`. It does not seem that there was anything malformed with the client request. This can also happen when the nested container exited right before the request was received on the agent.
Also, can we make the other error messages consistent with this?
(See the snippet I posted in the previous comment)
src/slave/slave.cpp (lines 6227 - 6236)
<https://reviews.apache.org/r/55464/#comment233065>
hmm, can we somehow reuse the helper `getRootContainerId()` in `slave/containerizer/mesos/utils.hpp`?
You might have to pull it out though first and then re-use it in this review? Strictly speaking, this method is pretty generic and should not be a part of the Mesos containerizer code.
src/tests/api_tests.cpp (line 3619)
<https://reviews.apache.org/r/55464/#comment233070>
Why did you change the existing test `NestedContainerLaunch` ? It's not immediately clear from the diff. Can you instead create a review that just deletes the `NestedContainerLaunchGrandchildNotSupported` test and then this review adds this new test. This might help RB not get confused with the diff.
src/tests/api_tests.cpp (line 3740)
<https://reviews.apache.org/r/55464/#comment233072>
How about `TwoLevelNestedContainerLaunch` instead?
src/tests/api_tests.cpp (line 3791)
<https://reviews.apache.org/r/55464/#comment233075>
// Launch a two level nested parent/child container and then wait for them to finish.
src/tests/api_tests.cpp (line 3818)
<https://reviews.apache.org/r/55464/#comment233076>
s/child child/child
src/tests/api_tests.cpp (line 3836)
<https://reviews.apache.org/r/55464/#comment233078>
s/parent/parent container.
src/tests/api_tests.cpp (line 3838)
<https://reviews.apache.org/r/55464/#comment233077>
newline before.
src/tests/api_tests.cpp (line 3852)
<https://reviews.apache.org/r/55464/#comment233079>
s/child/child container.
src/tests/api_tests.cpp (line 3854)
<https://reviews.apache.org/r/55464/#comment233080>
newline before
- Anand Mazumdar
On Jan. 13, 2017, 1:52 p.m., Gast�n Kleiman wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55464/
> -----------------------------------------------------------
>
> (Updated Jan. 13, 2017, 1:52 p.m.)
>
>
> Review request for mesos, Adam B, Alexander Rukletsov, Anand Mazumdar, Alexander Rojas, and haosdent huang.
>
>
> Bugs: MESOS-6864
> https://issues.apache.org/jira/browse/MESOS-6864
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Made the Agent API able to handle containers nested at arbitrary levels.
>
>
> Diffs
> -----
>
> src/slave/http.cpp 24fc23b229c624835a24cdda9587c99c6ac9c3bb
> src/slave/slave.cpp 11e8833fd5998abb71a7bb08e5dec451d894aba9
> src/tests/api_tests.cpp ea62226fbc5dd0bcc5ea60d1bbcd3748f9367467
>
> Diff: https://reviews.apache.org/r/55464/diff/
>
>
> Testing
> -------
>
> `GTEST_FILTER="" make -j 8 check` in macOS.
> `make check` in Linux.
>
> I also did some manual testing using a proof of concept that makes the `DefaultExecutor` leverage this change to perform CMD health checks.
>
>
> Thanks,
>
> Gast�n Kleiman
>
>
Re: Review Request 55464: Made the Agent API able to handle containers
nested at arbitrary levels.
Posted by Mesos Reviewbot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55464/#review162361
-----------------------------------------------------------
Patch looks great!
Reviews applied: [55676, 55722, 55677, 55678, 55679, 55464]
Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' CONFIGURATION='--verbose' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker-build.sh
- Mesos Reviewbot
On Jan. 19, 2017, 4:37 p.m., Gast�n Kleiman wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55464/
> -----------------------------------------------------------
>
> (Updated Jan. 19, 2017, 4:37 p.m.)
>
>
> Review request for mesos, Adam B, Alexander Rukletsov, Anand Mazumdar, Alexander Rojas, haosdent huang, and Vinod Kone.
>
>
> Bugs: MESOS-6864
> https://issues.apache.org/jira/browse/MESOS-6864
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Made the Agent API able to handle containers nested at arbitrary levels.
>
>
> Diffs
> -----
>
> src/slave/http.cpp ee7119179a6ddd935c3f43a618ef645619c305ee
> src/slave/slave.cpp 205138add7a63289ff8ed81138f8b603828c748e
> src/tests/api_tests.cpp ea62226fbc5dd0bcc5ea60d1bbcd3748f9367467
>
> Diff: https://reviews.apache.org/r/55464/diff/
>
>
> Testing
> -------
>
> `GTEST_FILTER="" make -j 8 check` in macOS.
> `make check` in Linux.
>
> I also did some manual testing using a proof of concept that makes the `DefaultExecutor` leverage this change to perform CMD health checks.
>
>
> Thanks,
>
> Gast�n Kleiman
>
>
Re: Review Request 55464: Made the Agent API able to handle containers
nested at arbitrary levels.
Posted by Gastón Kleiman <ga...@mesosphere.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55464/
-----------------------------------------------------------
(Updated Jan. 19, 2017, 4:37 p.m.)
Review request for mesos, Adam B, Alexander Rukletsov, Anand Mazumdar, Alexander Rojas, haosdent huang, and Vinod Kone.
Changes
-------
Addressed feedback.
Bugs: MESOS-6864
https://issues.apache.org/jira/browse/MESOS-6864
Repository: mesos
Description
-------
Made the Agent API able to handle containers nested at arbitrary levels.
Diffs (updated)
-----
src/slave/http.cpp ee7119179a6ddd935c3f43a618ef645619c305ee
src/slave/slave.cpp 205138add7a63289ff8ed81138f8b603828c748e
src/tests/api_tests.cpp ea62226fbc5dd0bcc5ea60d1bbcd3748f9367467
Diff: https://reviews.apache.org/r/55464/diff/
Testing
-------
`GTEST_FILTER="" make -j 8 check` in macOS.
`make check` in Linux.
I also did some manual testing using a proof of concept that makes the `DefaultExecutor` leverage this change to perform CMD health checks.
Thanks,
Gast�n Kleiman
Re: Review Request 55464: Made the Agent API able to handle containers
nested at arbitrary levels.
Posted by Mesos Reviewbot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55464/#review162210
-----------------------------------------------------------
Patch looks great!
Reviews applied: [55676, 55677, 55678, 55679, 55464]
Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' CONFIGURATION='--verbose' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker-build.sh
- Mesos Reviewbot
On Jan. 18, 2017, 2:48 p.m., Gast�n Kleiman wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55464/
> -----------------------------------------------------------
>
> (Updated Jan. 18, 2017, 2:48 p.m.)
>
>
> Review request for mesos, Adam B, Alexander Rukletsov, Anand Mazumdar, Alexander Rojas, haosdent huang, and Vinod Kone.
>
>
> Bugs: MESOS-6864
> https://issues.apache.org/jira/browse/MESOS-6864
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Made the Agent API able to handle containers nested at arbitrary levels.
>
>
> Diffs
> -----
>
> src/slave/http.cpp ee7119179a6ddd935c3f43a618ef645619c305ee
> src/slave/slave.cpp ae183e6b8b40094b4764525a6d63164f210ef9d8
> src/tests/api_tests.cpp ea62226fbc5dd0bcc5ea60d1bbcd3748f9367467
>
> Diff: https://reviews.apache.org/r/55464/diff/
>
>
> Testing
> -------
>
> `GTEST_FILTER="" make -j 8 check` in macOS.
> `make check` in Linux.
>
> I also did some manual testing using a proof of concept that makes the `DefaultExecutor` leverage this change to perform CMD health checks.
>
>
> Thanks,
>
> Gast�n Kleiman
>
>
Re: Review Request 55464: Made the Agent API able to handle containers
nested at arbitrary levels.
Posted by Anand Mazumdar <an...@apache.org>.
> On Jan. 19, 2017, 3:20 p.m., Vinod Kone wrote:
> > src/tests/api_tests.cpp, line 3896
> > <https://reviews.apache.org/r/55464/diff/4/?file=1607711#file1607711line3896>
> >
> > Maybe also verify parent container is still running.
> >
> > ```
> > EXPECT_TRUE(waitParent.isPending());
> >
> > ```
The subsequent call to kill the parent container would fail if the parent container is not running/already destroyed (!= 200 OK response) on L3910.
- Anand
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55464/#review162292
-----------------------------------------------------------
On Jan. 18, 2017, 2:48 p.m., Gast�n Kleiman wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55464/
> -----------------------------------------------------------
>
> (Updated Jan. 18, 2017, 2:48 p.m.)
>
>
> Review request for mesos, Adam B, Alexander Rukletsov, Anand Mazumdar, Alexander Rojas, haosdent huang, and Vinod Kone.
>
>
> Bugs: MESOS-6864
> https://issues.apache.org/jira/browse/MESOS-6864
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Made the Agent API able to handle containers nested at arbitrary levels.
>
>
> Diffs
> -----
>
> src/slave/http.cpp ee7119179a6ddd935c3f43a618ef645619c305ee
> src/slave/slave.cpp ae183e6b8b40094b4764525a6d63164f210ef9d8
> src/tests/api_tests.cpp ea62226fbc5dd0bcc5ea60d1bbcd3748f9367467
>
> Diff: https://reviews.apache.org/r/55464/diff/
>
>
> Testing
> -------
>
> `GTEST_FILTER="" make -j 8 check` in macOS.
> `make check` in Linux.
>
> I also did some manual testing using a proof of concept that makes the `DefaultExecutor` leverage this change to perform CMD health checks.
>
>
> Thanks,
>
> Gast�n Kleiman
>
>
Re: Review Request 55464: Made the Agent API able to handle containers
nested at arbitrary levels.
Posted by Vinod Kone <vi...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55464/#review162292
-----------------------------------------------------------
Fix it, then Ship it!
src/tests/api_tests.cpp (line 3644)
<https://reviews.apache.org/r/55464/#comment233625>
kill this.
src/tests/api_tests.cpp (line 3656)
<https://reviews.apache.org/r/55464/#comment233626>
kill this.
src/tests/api_tests.cpp (lines 3797 - 3799)
<https://reviews.apache.org/r/55464/#comment233629>
move this to #3819
src/tests/api_tests.cpp (line 3894)
<https://reviews.apache.org/r/55464/#comment233630>
Maybe also verify parent container is still running.
```
EXPECT_TRUE(waitParent.isPending());
```
- Vinod Kone
On Jan. 18, 2017, 2:48 p.m., Gast�n Kleiman wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55464/
> -----------------------------------------------------------
>
> (Updated Jan. 18, 2017, 2:48 p.m.)
>
>
> Review request for mesos, Adam B, Alexander Rukletsov, Anand Mazumdar, Alexander Rojas, haosdent huang, and Vinod Kone.
>
>
> Bugs: MESOS-6864
> https://issues.apache.org/jira/browse/MESOS-6864
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Made the Agent API able to handle containers nested at arbitrary levels.
>
>
> Diffs
> -----
>
> src/slave/http.cpp ee7119179a6ddd935c3f43a618ef645619c305ee
> src/slave/slave.cpp ae183e6b8b40094b4764525a6d63164f210ef9d8
> src/tests/api_tests.cpp ea62226fbc5dd0bcc5ea60d1bbcd3748f9367467
>
> Diff: https://reviews.apache.org/r/55464/diff/
>
>
> Testing
> -------
>
> `GTEST_FILTER="" make -j 8 check` in macOS.
> `make check` in Linux.
>
> I also did some manual testing using a proof of concept that makes the `DefaultExecutor` leverage this change to perform CMD health checks.
>
>
> Thanks,
>
> Gast�n Kleiman
>
>
Re: Review Request 55464: Made the Agent API able to handle containers
nested at arbitrary levels.
Posted by Anand Mazumdar <an...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55464/#review162297
-----------------------------------------------------------
Fix it, then Ship it!
Modulo other comments from Vinod
src/slave/slave.cpp (line 6476)
<https://reviews.apache.org/r/55464/#comment233631>
s/ContainerID&/ContainerID
- Anand Mazumdar
On Jan. 18, 2017, 2:48 p.m., Gast�n Kleiman wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55464/
> -----------------------------------------------------------
>
> (Updated Jan. 18, 2017, 2:48 p.m.)
>
>
> Review request for mesos, Adam B, Alexander Rukletsov, Anand Mazumdar, Alexander Rojas, haosdent huang, and Vinod Kone.
>
>
> Bugs: MESOS-6864
> https://issues.apache.org/jira/browse/MESOS-6864
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Made the Agent API able to handle containers nested at arbitrary levels.
>
>
> Diffs
> -----
>
> src/slave/http.cpp ee7119179a6ddd935c3f43a618ef645619c305ee
> src/slave/slave.cpp ae183e6b8b40094b4764525a6d63164f210ef9d8
> src/tests/api_tests.cpp ea62226fbc5dd0bcc5ea60d1bbcd3748f9367467
>
> Diff: https://reviews.apache.org/r/55464/diff/
>
>
> Testing
> -------
>
> `GTEST_FILTER="" make -j 8 check` in macOS.
> `make check` in Linux.
>
> I also did some manual testing using a proof of concept that makes the `DefaultExecutor` leverage this change to perform CMD health checks.
>
>
> Thanks,
>
> Gast�n Kleiman
>
>
Re: Review Request 55464: Made the Agent API able to handle containers
nested at arbitrary levels.
Posted by Gastón Kleiman <ga...@mesosphere.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55464/
-----------------------------------------------------------
(Updated Jan. 18, 2017, 2:48 p.m.)
Review request for mesos, Adam B, Alexander Rukletsov, Anand Mazumdar, Alexander Rojas, haosdent huang, and Vinod Kone.
Changes
-------
Addressed Anand's feedback.
Bugs: MESOS-6864
https://issues.apache.org/jira/browse/MESOS-6864
Repository: mesos
Description
-------
Made the Agent API able to handle containers nested at arbitrary levels.
Diffs (updated)
-----
src/slave/http.cpp ee7119179a6ddd935c3f43a618ef645619c305ee
src/slave/slave.cpp ae183e6b8b40094b4764525a6d63164f210ef9d8
src/tests/api_tests.cpp ea62226fbc5dd0bcc5ea60d1bbcd3748f9367467
Diff: https://reviews.apache.org/r/55464/diff/
Testing
-------
`GTEST_FILTER="" make -j 8 check` in macOS.
`make check` in Linux.
I also did some manual testing using a proof of concept that makes the `DefaultExecutor` leverage this change to perform CMD health checks.
Thanks,
Gast�n Kleiman
Re: Review Request 55464: Made the Agent API able to handle containers
nested at arbitrary levels.
Posted by Gastón Kleiman <ga...@mesosphere.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55464/
-----------------------------------------------------------
(Updated Jan. 13, 2017, 1:52 p.m.)
Review request for mesos, Adam B, Alexander Rukletsov, Anand Mazumdar, Alexander Rojas, and haosdent huang.
Changes
-------
Rebased.
Bugs: MESOS-6864
https://issues.apache.org/jira/browse/MESOS-6864
Repository: mesos
Description
-------
Made the Agent API able to handle containers nested at arbitrary levels.
Diffs (updated)
-----
src/slave/http.cpp 24fc23b229c624835a24cdda9587c99c6ac9c3bb
src/slave/slave.cpp 11e8833fd5998abb71a7bb08e5dec451d894aba9
src/tests/api_tests.cpp ea62226fbc5dd0bcc5ea60d1bbcd3748f9367467
Diff: https://reviews.apache.org/r/55464/diff/
Testing
-------
`GTEST_FILTER="" make -j 8 check` in macOS.
`make check` in Linux.
I also did some manual testing using a proof of concept that makes the `DefaultExecutor` leverage this change to perform CMD health checks.
Thanks,
Gast�n Kleiman
Re: Review Request 55464: Made the Agent API able to handle containers
nested at arbitrary levels.
Posted by Gastón Kleiman <ga...@mesosphere.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55464/
-----------------------------------------------------------
(Updated Jan. 13, 2017, 10:08 a.m.)
Review request for mesos, Adam B, Alexander Rukletsov, Anand Mazumdar, Alexander Rojas, and haosdent huang.
Changes
-------
Added a test.
Bugs: MESOS-6864
https://issues.apache.org/jira/browse/MESOS-6864
Repository: mesos
Description
-------
Made the Agent API able to handle containers nested at arbitrary levels.
Diffs (updated)
-----
src/slave/http.cpp 24fc23b229c624835a24cdda9587c99c6ac9c3bb
src/slave/slave.cpp 11e8833fd5998abb71a7bb08e5dec451d894aba9
src/tests/api_tests.cpp ea62226fbc5dd0bcc5ea60d1bbcd3748f9367467
Diff: https://reviews.apache.org/r/55464/diff/
Testing (updated)
-------
`GTEST_FILTER="" make -j 8 check` in macOS.
`make check` in Linux.
I also did some manual testing using a proof of concept that makes the `DefaultExecutor` leverage this change to perform CMD health checks.
Thanks,
Gast�n Kleiman
Re: Review Request 55464: Made the Agent API able to handle containers
nested at arbitrary levels.
Posted by Mesos ReviewBot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55464/#review161401
-----------------------------------------------------------
Bad patch!
Reviews applied: [55464, 55463]
Failed command: python support/apply-reviews.py -n -r 55464
Error:
Traceback (most recent call last):
File "support/apply-reviews.py", line 349, in <module>
reviewboard()
File "support/apply-reviews.py", line 328, in reviewboard
apply_review()
File "support/apply-reviews.py", line 121, in apply_review
fetch_patch()
File "support/apply-reviews.py", line 150, in fetch_patch
r = urllib2.urlopen(patch_url(), context=ssl_create_default_context())
File "support/apply-reviews.py", line 131, in ssl_create_default_context
context = ssl.SSLContext(ssl.PROTOCOL_SSLv23)
AttributeError: 'module' object has no attribute 'SSLContext'
Error in atexit._run_exitfuncs:
Traceback (most recent call last):
File "/usr/lib/python2.7/atexit.py", line 24, in _run_exitfuncs
func(*targs, **kargs)
File "support/apply-reviews.py", line 119, in <lambda>
atexit.register(lambda: os.remove('%s.patch' % patch_id()))
OSError: [Errno 2] No such file or directory: '55464.patch'
Error in sys.exitfunc:
Traceback (most recent call last):
File "/usr/lib/python2.7/atexit.py", line 24, in _run_exitfuncs
func(*targs, **kargs)
File "support/apply-reviews.py", line 119, in <lambda>
atexit.register(lambda: os.remove('%s.patch' % patch_id()))
OSError: [Errno 2] No such file or directory: '55464.patch'
Full log: https://builds.apache.org/job/Mesos-Reviewbot/16700/console
- Mesos ReviewBot
On Jan. 12, 2017, 3:30 p.m., Gast�n Kleiman wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55464/
> -----------------------------------------------------------
>
> (Updated Jan. 12, 2017, 3:30 p.m.)
>
>
> Review request for mesos, Adam B, Alexander Rukletsov, Anand Mazumdar, Alexander Rojas, and haosdent huang.
>
>
> Bugs: MESOS-6864
> https://issues.apache.org/jira/browse/MESOS-6864
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Made the Agent API able to handle containers nested at arbitrary levels.
>
>
> Diffs
> -----
>
> src/slave/http.cpp 24fc23b229c624835a24cdda9587c99c6ac9c3bb
> src/slave/slave.cpp 11e8833fd5998abb71a7bb08e5dec451d894aba9
> src/tests/api_tests.cpp ea62226fbc5dd0bcc5ea60d1bbcd3748f9367467
>
> Diff: https://reviews.apache.org/r/55464/diff/
>
>
> Testing
> -------
>
> `make check` in Linux.
>
> I have a proof of concept that makes the `DefaultExecutor` leverage this change to perform CMD health checks.
>
>
> Thanks,
>
> Gast�n Kleiman
>
>
Re: Review Request 55464: Made the Agent API able to handle containers
nested at arbitrary levels.
Posted by Gastón Kleiman <ga...@mesosphere.com>.
> On Jan. 12, 2017, 3:50 p.m., Alexander Rojas wrote:
> > src/tests/api_tests.cpp, lines 3619-3659
> > <https://reviews.apache.org/r/55464/diff/1/?file=1603868#file1603868line3619>
> >
> > Why not replacing this test with one that shows the opposite?
I added a test that launches parent and child containers nested under an executor, kills them and waits for them to be completed.
- Gast�n
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55464/#review161391
-----------------------------------------------------------
On Jan. 13, 2017, 10:08 a.m., Gast�n Kleiman wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55464/
> -----------------------------------------------------------
>
> (Updated Jan. 13, 2017, 10:08 a.m.)
>
>
> Review request for mesos, Adam B, Alexander Rukletsov, Anand Mazumdar, Alexander Rojas, and haosdent huang.
>
>
> Bugs: MESOS-6864
> https://issues.apache.org/jira/browse/MESOS-6864
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Made the Agent API able to handle containers nested at arbitrary levels.
>
>
> Diffs
> -----
>
> src/slave/http.cpp 24fc23b229c624835a24cdda9587c99c6ac9c3bb
> src/slave/slave.cpp 11e8833fd5998abb71a7bb08e5dec451d894aba9
> src/tests/api_tests.cpp ea62226fbc5dd0bcc5ea60d1bbcd3748f9367467
>
> Diff: https://reviews.apache.org/r/55464/diff/
>
>
> Testing
> -------
>
> `GTEST_FILTER="" make -j 8 check` in macOS.
> `make check` in Linux.
>
> I also did some manual testing using a proof of concept that makes the `DefaultExecutor` leverage this change to perform CMD health checks.
>
>
> Thanks,
>
> Gast�n Kleiman
>
>
Re: Review Request 55464: Made the Agent API able to handle containers
nested at arbitrary levels.
Posted by Alexander Rojas <al...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55464/#review161391
-----------------------------------------------------------
src/tests/api_tests.cpp
<https://reviews.apache.org/r/55464/#comment232633>
Why not replacing this test with one that shows the opposite?
- Alexander Rojas
On Jan. 12, 2017, 4:30 p.m., Gast�n Kleiman wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55464/
> -----------------------------------------------------------
>
> (Updated Jan. 12, 2017, 4:30 p.m.)
>
>
> Review request for mesos, Adam B, Alexander Rukletsov, Anand Mazumdar, Alexander Rojas, and haosdent huang.
>
>
> Bugs: MESOS-6864
> https://issues.apache.org/jira/browse/MESOS-6864
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Made the Agent API able to handle containers nested at arbitrary levels.
>
>
> Diffs
> -----
>
> src/slave/http.cpp 24fc23b229c624835a24cdda9587c99c6ac9c3bb
> src/slave/slave.cpp 11e8833fd5998abb71a7bb08e5dec451d894aba9
> src/tests/api_tests.cpp ea62226fbc5dd0bcc5ea60d1bbcd3748f9367467
>
> Diff: https://reviews.apache.org/r/55464/diff/
>
>
> Testing
> -------
>
> `make check` in Linux.
>
> I have a proof of concept that makes the `DefaultExecutor` leverage this change to perform CMD health checks.
>
>
> Thanks,
>
> Gast�n Kleiman
>
>