You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Benjamin Bannier <bb...@apache.org> on 2019/08/21 13:22:49 UTC
Review Request 71341: Validated provider ID use in some resource
provider calls.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/71341/
-----------------------------------------------------------
Review request for mesos, Chun-Hung Hsiao and Jan Schlicht.
Bugs: MESOS-9482
https://issues.apache.org/jira/browse/MESOS-9482
Repository: mesos
Description
-------
For some calls we expect resource providers to set provider IDs with the
calls. While the resource provider manager has always asserted that the
calls were correct we never validated this.
With this patch we perform additional validation for calls taking a
`ResourceProviderInfo` into account. We add both unit tests for the
validation code and an integration test confirming that the validation
is actually triggered.
Diffs
-----
src/resource_provider/manager.cpp ceed1225b37d23998f523afcc2184dfaaad60636
src/resource_provider/validation.cpp df55b5efe3543c1dfd8441997302ab76fdd4bcc1
src/tests/resource_provider_manager_tests.cpp bcf6a03aa5d4931feff0299c811faa216efd95b6
src/tests/resource_provider_validation_tests.cpp a9989412ae30bd8244be808fc88fbe70f47d6ad9
Diff: https://reviews.apache.org/r/71341/diff/1/
Testing
-------
`ninja check`
Thanks,
Benjamin Bannier
Re: Review Request 71341: Validated provider ID use in some resource
provider calls.
Posted by Mesos Reviewbot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/71341/#review217364
-----------------------------------------------------------
Patch looks great!
Reviews applied: [71339, 71340, 71341]
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 Aug. 21, 2019, 6:22 a.m., Benjamin Bannier wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71341/
> -----------------------------------------------------------
>
> (Updated Aug. 21, 2019, 6:22 a.m.)
>
>
> Review request for mesos, Chun-Hung Hsiao and Jan Schlicht.
>
>
> Bugs: MESOS-9482
> https://issues.apache.org/jira/browse/MESOS-9482
>
>
> Repository: mesos
>
>
> Description
> -------
>
> For some calls we expect resource providers to set provider IDs with the
> calls. While the resource provider manager has always asserted that the
> calls were correct we never validated this.
>
> With this patch we perform additional validation for calls taking a
> `ResourceProviderInfo` into account. We add both unit tests for the
> validation code and an integration test confirming that the validation
> is actually triggered.
>
>
> Diffs
> -----
>
> src/resource_provider/manager.cpp ceed1225b37d23998f523afcc2184dfaaad60636
> src/resource_provider/validation.cpp df55b5efe3543c1dfd8441997302ab76fdd4bcc1
> src/tests/resource_provider_manager_tests.cpp bcf6a03aa5d4931feff0299c811faa216efd95b6
> src/tests/resource_provider_validation_tests.cpp a9989412ae30bd8244be808fc88fbe70f47d6ad9
>
>
> Diff: https://reviews.apache.org/r/71341/diff/1/
>
>
> Testing
> -------
>
> `ninja check`
>
>
> Thanks,
>
> Benjamin Bannier
>
>
Re: Review Request 71341: Validated provider ID use in some resource
provider calls.
Posted by Benjamin Bannier <bb...@apache.org>.
> On Aug. 22, 2019, 2:38 p.m., Jan Schlicht wrote:
> > src/resource_provider/validation.cpp
> > Lines 17 (patched)
> > <https://reviews.apache.org/r/71341/diff/1/?file=2162387#file2162387line17>
> >
> > Include this after `resource_provider/validation.hpp`.
Ups, parsed this as another not public include :D
> On Aug. 22, 2019, 2:38 p.m., Jan Schlicht wrote:
> > src/resource_provider/validation.cpp
> > Lines 98 (patched)
> > <https://reviews.apache.org/r/71341/diff/1/?file=2162387#file2162387line98>
> >
> > s/provider/resource provider/
Note that the field is called `provider_id`, but I guess using _resource_ provider here is just more consistent.
- Benjamin
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/71341/#review217376
-----------------------------------------------------------
On Aug. 22, 2019, 3:04 p.m., Benjamin Bannier wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71341/
> -----------------------------------------------------------
>
> (Updated Aug. 22, 2019, 3:04 p.m.)
>
>
> Review request for mesos, Chun-Hung Hsiao and Jan Schlicht.
>
>
> Bugs: MESOS-9482
> https://issues.apache.org/jira/browse/MESOS-9482
>
>
> Repository: mesos
>
>
> Description
> -------
>
> For some calls we expect resource providers to set provider IDs with the
> calls. While the resource provider manager has always asserted that the
> calls were correct we never validated this.
>
> With this patch we perform additional validation for calls taking a
> `ResourceProviderInfo` into account. We add both unit tests for the
> validation code and an integration test confirming that the validation
> is actually triggered.
>
>
> Diffs
> -----
>
> src/resource_provider/manager.cpp ceed1225b37d23998f523afcc2184dfaaad60636
> src/resource_provider/validation.cpp df55b5efe3543c1dfd8441997302ab76fdd4bcc1
> src/tests/resource_provider_manager_tests.cpp bcf6a03aa5d4931feff0299c811faa216efd95b6
> src/tests/resource_provider_validation_tests.cpp a9989412ae30bd8244be808fc88fbe70f47d6ad9
>
>
> Diff: https://reviews.apache.org/r/71341/diff/2/
>
>
> Testing
> -------
>
> `ninja check`
>
>
> Thanks,
>
> Benjamin Bannier
>
>
Re: Review Request 71341: Validated provider ID use in some resource
provider calls.
Posted by Jan Schlicht <ja...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/71341/#review217376
-----------------------------------------------------------
Looks great, only have a few nits.
src/resource_provider/manager.cpp
Lines 415 (patched)
<https://reviews.apache.org/r/71341/#comment304665>
s/Inconstent/Inconsistent/
src/resource_provider/validation.cpp
Lines 17 (patched)
<https://reviews.apache.org/r/71341/#comment304666>
Include this after `resource_provider/validation.hpp`.
src/resource_provider/validation.cpp
Lines 98 (patched)
<https://reviews.apache.org/r/71341/#comment304667>
s/provider/resource provider/
- Jan Schlicht
On Aug. 21, 2019, 3:22 p.m., Benjamin Bannier wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71341/
> -----------------------------------------------------------
>
> (Updated Aug. 21, 2019, 3:22 p.m.)
>
>
> Review request for mesos, Chun-Hung Hsiao and Jan Schlicht.
>
>
> Bugs: MESOS-9482
> https://issues.apache.org/jira/browse/MESOS-9482
>
>
> Repository: mesos
>
>
> Description
> -------
>
> For some calls we expect resource providers to set provider IDs with the
> calls. While the resource provider manager has always asserted that the
> calls were correct we never validated this.
>
> With this patch we perform additional validation for calls taking a
> `ResourceProviderInfo` into account. We add both unit tests for the
> validation code and an integration test confirming that the validation
> is actually triggered.
>
>
> Diffs
> -----
>
> src/resource_provider/manager.cpp ceed1225b37d23998f523afcc2184dfaaad60636
> src/resource_provider/validation.cpp df55b5efe3543c1dfd8441997302ab76fdd4bcc1
> src/tests/resource_provider_manager_tests.cpp bcf6a03aa5d4931feff0299c811faa216efd95b6
> src/tests/resource_provider_validation_tests.cpp a9989412ae30bd8244be808fc88fbe70f47d6ad9
>
>
> Diff: https://reviews.apache.org/r/71341/diff/1/
>
>
> Testing
> -------
>
> `ninja check`
>
>
> Thanks,
>
> Benjamin Bannier
>
>
Re: Review Request 71341: Validated provider ID use in some resource
provider calls.
Posted by Mesos Reviewbot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/71341/#review217382
-----------------------------------------------------------
Patch looks great!
Reviews applied: [71339, 71340, 71341]
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 Aug. 22, 2019, 6:04 a.m., Benjamin Bannier wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71341/
> -----------------------------------------------------------
>
> (Updated Aug. 22, 2019, 6:04 a.m.)
>
>
> Review request for mesos, Chun-Hung Hsiao and Jan Schlicht.
>
>
> Bugs: MESOS-9482
> https://issues.apache.org/jira/browse/MESOS-9482
>
>
> Repository: mesos
>
>
> Description
> -------
>
> For some calls we expect resource providers to set provider IDs with the
> calls. While the resource provider manager has always asserted that the
> calls were correct we never validated this.
>
> With this patch we perform additional validation for calls taking a
> `ResourceProviderInfo` into account. We add both unit tests for the
> validation code and an integration test confirming that the validation
> is actually triggered.
>
>
> Diffs
> -----
>
> src/resource_provider/manager.cpp ceed1225b37d23998f523afcc2184dfaaad60636
> src/resource_provider/validation.cpp df55b5efe3543c1dfd8441997302ab76fdd4bcc1
> src/tests/resource_provider_manager_tests.cpp bcf6a03aa5d4931feff0299c811faa216efd95b6
> src/tests/resource_provider_validation_tests.cpp a9989412ae30bd8244be808fc88fbe70f47d6ad9
>
>
> Diff: https://reviews.apache.org/r/71341/diff/3/
>
>
> Testing
> -------
>
> `ninja check`
>
>
> Thanks,
>
> Benjamin Bannier
>
>
Re: Review Request 71341: Validated provider ID use in some resource
provider calls.
Posted by Jan Schlicht <ja...@d2iq.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/71341/#review217380
-----------------------------------------------------------
Ship it!
Ship It!
- Jan Schlicht
On Aug. 22, 2019, 3:04 p.m., Benjamin Bannier wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71341/
> -----------------------------------------------------------
>
> (Updated Aug. 22, 2019, 3:04 p.m.)
>
>
> Review request for mesos, Chun-Hung Hsiao and Jan Schlicht.
>
>
> Bugs: MESOS-9482
> https://issues.apache.org/jira/browse/MESOS-9482
>
>
> Repository: mesos
>
>
> Description
> -------
>
> For some calls we expect resource providers to set provider IDs with the
> calls. While the resource provider manager has always asserted that the
> calls were correct we never validated this.
>
> With this patch we perform additional validation for calls taking a
> `ResourceProviderInfo` into account. We add both unit tests for the
> validation code and an integration test confirming that the validation
> is actually triggered.
>
>
> Diffs
> -----
>
> src/resource_provider/manager.cpp ceed1225b37d23998f523afcc2184dfaaad60636
> src/resource_provider/validation.cpp df55b5efe3543c1dfd8441997302ab76fdd4bcc1
> src/tests/resource_provider_manager_tests.cpp bcf6a03aa5d4931feff0299c811faa216efd95b6
> src/tests/resource_provider_validation_tests.cpp a9989412ae30bd8244be808fc88fbe70f47d6ad9
>
>
> Diff: https://reviews.apache.org/r/71341/diff/4/
>
>
> Testing
> -------
>
> `ninja check`
>
>
> Thanks,
>
> Benjamin Bannier
>
>
Re: Review Request 71341: Validated provider ID use in some resource
provider calls.
Posted by Mesos Reviewbot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/71341/#review217420
-----------------------------------------------------------
Patch looks great!
Reviews applied: [71339, 71340, 71341]
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 Aug. 22, 2019, 6:04 a.m., Benjamin Bannier wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71341/
> -----------------------------------------------------------
>
> (Updated Aug. 22, 2019, 6:04 a.m.)
>
>
> Review request for mesos, Chun-Hung Hsiao and Jan Schlicht.
>
>
> Bugs: MESOS-9482
> https://issues.apache.org/jira/browse/MESOS-9482
>
>
> Repository: mesos
>
>
> Description
> -------
>
> For some calls we expect resource providers to set provider IDs with the
> calls. While the resource provider manager has always asserted that the
> calls were correct we never validated this.
>
> With this patch we perform additional validation for calls taking a
> `ResourceProviderInfo` into account. We add both unit tests for the
> validation code and an integration test confirming that the validation
> is actually triggered.
>
>
> Diffs
> -----
>
> src/resource_provider/manager.cpp ceed1225b37d23998f523afcc2184dfaaad60636
> src/resource_provider/validation.cpp df55b5efe3543c1dfd8441997302ab76fdd4bcc1
> src/tests/resource_provider_manager_tests.cpp bcf6a03aa5d4931feff0299c811faa216efd95b6
> src/tests/resource_provider_validation_tests.cpp a9989412ae30bd8244be808fc88fbe70f47d6ad9
>
>
> Diff: https://reviews.apache.org/r/71341/diff/4/
>
>
> Testing
> -------
>
> `ninja check`
>
>
> Thanks,
>
> Benjamin Bannier
>
>
Re: Review Request 71341: Validated provider ID use in some resource
provider calls.
Posted by Benjamin Bannier <bb...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/71341/
-----------------------------------------------------------
(Updated Aug. 22, 2019, 3:04 p.m.)
Review request for mesos, Chun-Hung Hsiao and Jan Schlicht.
Bugs: MESOS-9482
https://issues.apache.org/jira/browse/MESOS-9482
Repository: mesos
Description
-------
For some calls we expect resource providers to set provider IDs with the
calls. While the resource provider manager has always asserted that the
calls were correct we never validated this.
With this patch we perform additional validation for calls taking a
`ResourceProviderInfo` into account. We add both unit tests for the
validation code and an integration test confirming that the validation
is actually triggered.
Diffs (updated)
-----
src/resource_provider/manager.cpp ceed1225b37d23998f523afcc2184dfaaad60636
src/resource_provider/validation.cpp df55b5efe3543c1dfd8441997302ab76fdd4bcc1
src/tests/resource_provider_manager_tests.cpp bcf6a03aa5d4931feff0299c811faa216efd95b6
src/tests/resource_provider_validation_tests.cpp a9989412ae30bd8244be808fc88fbe70f47d6ad9
Diff: https://reviews.apache.org/r/71341/diff/2/
Changes: https://reviews.apache.org/r/71341/diff/1-2/
Testing
-------
`ninja check`
Thanks,
Benjamin Bannier