You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Jay Guo <gu...@gmail.com> on 2017/02/10 09:32:41 UTC
Review Request 56537: Added a test to ensure framework can upgrade to
support MULTI_ROLE.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/56537/
-----------------------------------------------------------
Review request for mesos, Benjamin Bannier, Benjamin Mahler, Guangya Liu, and Michael Park.
Bugs: MESOS-7035
https://issues.apache.org/jira/browse/MESOS-7035
Repository: mesos
Description
-------
A framework can be upgraded to be MULTI_ROLE capable even with task
running, as long as it does not change role while upgrading.
Diffs
-----
src/tests/master_tests.cpp 3b4123b49ee32c902a5d2a01fcc7026da21fdd18
Diff: https://reviews.apache.org/r/56537/diff/
Testing
-------
WIP
Thanks,
Jay Guo
Re: Review Request 56537: Added a test to ensure framework can
upgrade to support MULTI_ROLE.
Posted by Mesos Reviewbot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/56537/#review165160
-----------------------------------------------------------
Patch looks great!
Reviews applied: [56537]
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 Feb. 10, 2017, 9:32 a.m., Jay Guo wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56537/
> -----------------------------------------------------------
>
> (Updated Feb. 10, 2017, 9:32 a.m.)
>
>
> Review request for mesos, Benjamin Bannier, Benjamin Mahler, Guangya Liu, and Michael Park.
>
>
> Bugs: MESOS-7035
> https://issues.apache.org/jira/browse/MESOS-7035
>
>
> Repository: mesos
>
>
> Description
> -------
>
> A framework can be upgraded to be MULTI_ROLE capable even with task
> running, as long as it does not change role while upgrading.
>
>
> Diffs
> -----
>
> src/tests/master_tests.cpp 3b4123b49ee32c902a5d2a01fcc7026da21fdd18
>
> Diff: https://reviews.apache.org/r/56537/diff/
>
>
> Testing
> -------
>
> WIP
>
>
> Thanks,
>
> Jay Guo
>
>
Re: Review Request 56537: Added a test to ensure framework can
upgrade to support MULTI_ROLE.
Posted by Benjamin Bannier <be...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/56537/#review165920
-----------------------------------------------------------
Fix it, then Ship it!
src/tests/master_tests.cpp (line 6406)
<https://reviews.apache.org/r/56537/#comment237764>
`s/slave/agent/`
src/tests/master_tests.cpp (line 6416)
<https://reviews.apache.org/r/56537/#comment237773>
Should we make explicit that this future will be ready, e.g., by awaiting it right before we await `offers`?
src/tests/master_tests.cpp (line 6428)
<https://reviews.apache.org/r/56537/#comment237767>
I believe this needs to be an assertion, e.g.,
ASSERT_FALSE(offers->empty());
src/tests/master_tests.cpp (lines 6433 - 6435)
<https://reviews.apache.org/r/56537/#comment237772>
Since we are working on the default-constructed fields, `MergeFrom` is identical to `CopyFrom` here, right?
src/tests/master_tests.cpp (line 6438)
<https://reviews.apache.org/r/56537/#comment237770>
You can drop `Times(1)` here as it is the default.
src/tests/master_tests.cpp (lines 6450 - 6452)
<https://reviews.apache.org/r/56537/#comment237765>
`s/status.get()./status->/g'
src/tests/master_tests.cpp (lines 6455 - 6456)
<https://reviews.apache.org/r/56537/#comment237774>
We can make explicit that we keep the same role and avoid a hardcoded value by inverting setting `roles` and clearing `role`, e.g.,
frameworkInfo.add_roles(frameworkInfo.role());
frameworkInfo.clear_role();
src/tests/master_tests.cpp (lines 6482 - 6489)
<https://reviews.apache.org/r/56537/#comment237768>
Seems like just using
driver2.reconcileTasks({status.get()});
would be identical, but hardcode less information.
src/tests/master_tests.cpp (line 6492)
<https://reviews.apache.org/r/56537/#comment237766>
`explicitReconciliation->state()`
src/tests/master_tests.cpp (lines 6494 - 6495)
<https://reviews.apache.org/r/56537/#comment237769>
`driver1` would be inactive as soon as `registered2` above is ready. We could move this block just after that to create a bracket around the second registration,
EXPECT_CALL(exec, shutdown(_));
driver2.start();
AWAIT_READY(registered2);
driver1.stop();
driver1.join();
I believe stopping `driver1` here makes sure that `exec` will be around long enough so it actually can see its `shutdown` call allowing us to expect exactly one call (`.Times(1)` is the default for `EXPECT_CALL`).
- Benjamin Bannier
On Feb. 14, 2017, 2:49 a.m., Jay Guo wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56537/
> -----------------------------------------------------------
>
> (Updated Feb. 14, 2017, 2:49 a.m.)
>
>
> Review request for mesos, Benjamin Bannier, Benjamin Mahler, Guangya Liu, and Michael Park.
>
>
> Bugs: MESOS-7035
> https://issues.apache.org/jira/browse/MESOS-7035
>
>
> Repository: mesos
>
>
> Description
> -------
>
> A framework can be upgraded to be MULTI_ROLE capable even with task
> running, as long as it does not change role while upgrading.
>
>
> Diffs
> -----
>
> src/tests/master_tests.cpp 9a7a953adbbbed0b37082bb0166d03b1388c80ae
>
> Diff: https://reviews.apache.org/r/56537/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Jay Guo
>
>
Re: Review Request 56537: Added a test to ensure framework can
upgrade to support MULTI_ROLE.
Posted by Benjamin Mahler <bm...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/56537/#review166265
-----------------------------------------------------------
Ship it!
src/tests/master_tests.cpp (lines 6489 - 6497)
<https://reviews.apache.org/r/56537/#comment238181>
How about s/explicitReconciliation/status2/?
- Benjamin Mahler
On Feb. 20, 2017, 5:56 a.m., Jay Guo wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56537/
> -----------------------------------------------------------
>
> (Updated Feb. 20, 2017, 5:56 a.m.)
>
>
> Review request for mesos, Benjamin Bannier, Benjamin Mahler, Guangya Liu, and Michael Park.
>
>
> Bugs: MESOS-7035
> https://issues.apache.org/jira/browse/MESOS-7035
>
>
> Repository: mesos
>
>
> Description
> -------
>
> A framework can be upgraded to be MULTI_ROLE capable even with task
> running, as long as it does not change role while upgrading.
>
>
> Diffs
> -----
>
> src/tests/master_tests.cpp 64443dc6e2ca4ab8f37269a0dced49908526649b
>
> Diff: https://reviews.apache.org/r/56537/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Jay Guo
>
>
Re: Review Request 56537: Added a test to ensure framework can
upgrade to support MULTI_ROLE.
Posted by Mesos Reviewbot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/56537/#review166081
-----------------------------------------------------------
Patch looks great!
Reviews applied: [56537]
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 Feb. 20, 2017, 5:56 a.m., Jay Guo wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56537/
> -----------------------------------------------------------
>
> (Updated Feb. 20, 2017, 5:56 a.m.)
>
>
> Review request for mesos, Benjamin Bannier, Benjamin Mahler, Guangya Liu, and Michael Park.
>
>
> Bugs: MESOS-7035
> https://issues.apache.org/jira/browse/MESOS-7035
>
>
> Repository: mesos
>
>
> Description
> -------
>
> A framework can be upgraded to be MULTI_ROLE capable even with task
> running, as long as it does not change role while upgrading.
>
>
> Diffs
> -----
>
> src/tests/master_tests.cpp 64443dc6e2ca4ab8f37269a0dced49908526649b
>
> Diff: https://reviews.apache.org/r/56537/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Jay Guo
>
>
Re: Review Request 56537: Added a test to ensure framework can
upgrade to support MULTI_ROLE.
Posted by Jay Guo <gu...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/56537/
-----------------------------------------------------------
(Updated Feb. 20, 2017, 1:56 p.m.)
Review request for mesos, Benjamin Bannier, Benjamin Mahler, Guangya Liu, and Michael Park.
Changes
-------
rebase and address bbannier's comments.
Bugs: MESOS-7035
https://issues.apache.org/jira/browse/MESOS-7035
Repository: mesos
Description
-------
A framework can be upgraded to be MULTI_ROLE capable even with task
running, as long as it does not change role while upgrading.
Diffs (updated)
-----
src/tests/master_tests.cpp 64443dc6e2ca4ab8f37269a0dced49908526649b
Diff: https://reviews.apache.org/r/56537/diff/
Testing
-------
make check
Thanks,
Jay Guo
Re: Review Request 56537: Added a test to ensure framework can
upgrade to support MULTI_ROLE.
Posted by Jay Guo <gu...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/56537/
-----------------------------------------------------------
(Updated Feb. 14, 2017, 9:49 a.m.)
Review request for mesos, Benjamin Bannier, Benjamin Mahler, Guangya Liu, and Michael Park.
Bugs: MESOS-7035
https://issues.apache.org/jira/browse/MESOS-7035
Repository: mesos
Description
-------
A framework can be upgraded to be MULTI_ROLE capable even with task
running, as long as it does not change role while upgrading.
Diffs
-----
src/tests/master_tests.cpp 9a7a953adbbbed0b37082bb0166d03b1388c80ae
Diff: https://reviews.apache.org/r/56537/diff/
Testing (updated)
-------
make check
Thanks,
Jay Guo
Re: Review Request 56537: Added a test to ensure framework can
upgrade to support MULTI_ROLE.
Posted by Mesos Reviewbot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/56537/#review165320
-----------------------------------------------------------
Patch looks great!
Reviews applied: [56537]
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 Feb. 13, 2017, 10:33 a.m., Jay Guo wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56537/
> -----------------------------------------------------------
>
> (Updated Feb. 13, 2017, 10:33 a.m.)
>
>
> Review request for mesos, Benjamin Bannier, Benjamin Mahler, Guangya Liu, and Michael Park.
>
>
> Bugs: MESOS-7035
> https://issues.apache.org/jira/browse/MESOS-7035
>
>
> Repository: mesos
>
>
> Description
> -------
>
> A framework can be upgraded to be MULTI_ROLE capable even with task
> running, as long as it does not change role while upgrading.
>
>
> Diffs
> -----
>
> src/tests/master_tests.cpp 9a7a953adbbbed0b37082bb0166d03b1388c80ae
>
> Diff: https://reviews.apache.org/r/56537/diff/
>
>
> Testing
> -------
>
> WIP
>
>
> Thanks,
>
> Jay Guo
>
>
Re: Review Request 56537: Added a test to ensure framework can
upgrade to support MULTI_ROLE.
Posted by Jay Guo <gu...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/56537/
-----------------------------------------------------------
(Updated Feb. 13, 2017, 6:33 p.m.)
Review request for mesos, Benjamin Bannier, Benjamin Mahler, Guangya Liu, and Michael Park.
Bugs: MESOS-7035
https://issues.apache.org/jira/browse/MESOS-7035
Repository: mesos
Description
-------
A framework can be upgraded to be MULTI_ROLE capable even with task
running, as long as it does not change role while upgrading.
Diffs (updated)
-----
src/tests/master_tests.cpp 9a7a953adbbbed0b37082bb0166d03b1388c80ae
Diff: https://reviews.apache.org/r/56537/diff/
Testing
-------
WIP
Thanks,
Jay Guo
Re: Review Request 56537: Added a test to ensure framework can
upgrade to support MULTI_ROLE.
Posted by Benjamin Mahler <bm...@apache.org>.
> On Feb. 10, 2017, 9:33 a.m., Jay Guo wrote:
> > src/tests/master_tests.cpp, lines 6409-6411
> > <https://reviews.apache.org/r/56537/diff/1/?file=1629346#file1629346line6409>
> >
> > How to check the task is still running here?
If no status update is received we know it's still running. To check it more explicitly we can perform reconciliation.
- Benjamin
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/56537/#review165087
-----------------------------------------------------------
On Feb. 10, 2017, 9:32 a.m., Jay Guo wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56537/
> -----------------------------------------------------------
>
> (Updated Feb. 10, 2017, 9:32 a.m.)
>
>
> Review request for mesos, Benjamin Bannier, Benjamin Mahler, Guangya Liu, and Michael Park.
>
>
> Bugs: MESOS-7035
> https://issues.apache.org/jira/browse/MESOS-7035
>
>
> Repository: mesos
>
>
> Description
> -------
>
> A framework can be upgraded to be MULTI_ROLE capable even with task
> running, as long as it does not change role while upgrading.
>
>
> Diffs
> -----
>
> src/tests/master_tests.cpp 3b4123b49ee32c902a5d2a01fcc7026da21fdd18
>
> Diff: https://reviews.apache.org/r/56537/diff/
>
>
> Testing
> -------
>
> WIP
>
>
> Thanks,
>
> Jay Guo
>
>
Re: Review Request 56537: Added a test to ensure framework can
upgrade to support MULTI_ROLE.
Posted by Jay Guo <gu...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/56537/#review165087
-----------------------------------------------------------
src/tests/master_tests.cpp (lines 6409 - 6411)
<https://reviews.apache.org/r/56537/#comment236913>
How to check the task is still running here?
- Jay Guo
On Feb. 10, 2017, 5:32 p.m., Jay Guo wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56537/
> -----------------------------------------------------------
>
> (Updated Feb. 10, 2017, 5:32 p.m.)
>
>
> Review request for mesos, Benjamin Bannier, Benjamin Mahler, Guangya Liu, and Michael Park.
>
>
> Bugs: MESOS-7035
> https://issues.apache.org/jira/browse/MESOS-7035
>
>
> Repository: mesos
>
>
> Description
> -------
>
> A framework can be upgraded to be MULTI_ROLE capable even with task
> running, as long as it does not change role while upgrading.
>
>
> Diffs
> -----
>
> src/tests/master_tests.cpp 3b4123b49ee32c902a5d2a01fcc7026da21fdd18
>
> Diff: https://reviews.apache.org/r/56537/diff/
>
>
> Testing
> -------
>
> WIP
>
>
> Thanks,
>
> Jay Guo
>
>