You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by James Peach <jp...@apache.org> on 2017/01/13 17:22:28 UTC
Review Request 55509: Validate the StatusUpdate UUID in
Master::statusUpdate.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55509/
-----------------------------------------------------------
Review request for mesos and Jiang Yan Xu.
Bugs: MESOS-6920
https://issues.apache.org/jira/browse/MESOS-6920
Repository: mesos
Description
-------
To be consistent with Master::statusUpdateAcknowledgement(), validate
that the UUID in the StatusUpdate message is acceptable to the
UUID::fromBytes() parser.
Diffs
-----
3rdparty/stout/include/stout/os/linux.hpp 68b4090aa5e6f23609c487128d91301755ae0e46
docs/contributors.yaml d2b686c9c15782eefeed3376dc84edba22968745
src/linux/ns.hpp da43266ba67f76c9ca856584a540794062097691
src/master/master.cpp 1746a88953dbdc148d98881bcf7027b62ad6b040
src/slave/containerizer/mesos/containerizer.cpp 4d2b4344e9698aed6ef0d49bb56d9292f2c3703b
src/slave/containerizer/mesos/isolators/network/port_mapping.cpp f6f2bfe1d5d3c113036ad44a480f97bbb462a269
support/apply-reviews.py c77b4c2b2d7a3d5e74e225403e71a84e23a9a1e7
Diff: https://reviews.apache.org/r/55509/diff/
Testing
-------
Make check (Fedora 25). Manual testing with a handmade StatusUpdate message.
Thanks,
James Peach
Re: Review Request 55509: Validate the StatusUpdate UUID in
Master::statusUpdate.
Posted by James Peach <jp...@apache.org>.
> On Jan. 13, 2017, 6:39 p.m., Neil Conway wrote:
> > Seems like you need to rebase?
Oops, thanks :)
- James
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55509/#review161555
-----------------------------------------------------------
On Jan. 13, 2017, 6:41 p.m., James Peach wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55509/
> -----------------------------------------------------------
>
> (Updated Jan. 13, 2017, 6:41 p.m.)
>
>
> Review request for mesos and Jiang Yan Xu.
>
>
> Bugs: MESOS-6920
> https://issues.apache.org/jira/browse/MESOS-6920
>
>
> Repository: mesos
>
>
> Description
> -------
>
> To be consistent with Master::statusUpdateAcknowledgement(), validate
> that the UUID in the StatusUpdate message is acceptable to the
> UUID::fromBytes() parser.
>
>
> Diffs
> -----
>
> src/master/master.cpp 1746a88953dbdc148d98881bcf7027b62ad6b040
>
> Diff: https://reviews.apache.org/r/55509/diff/
>
>
> Testing
> -------
>
> Make check (Fedora 25). Manual testing with a handmade StatusUpdate message.
>
>
> Thanks,
>
> James Peach
>
>
Re: Review Request 55509: Validate the StatusUpdate UUID in
Master::statusUpdate.
Posted by Neil Conway <ne...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55509/#review161555
-----------------------------------------------------------
Seems like you need to rebase?
- Neil Conway
On Jan. 13, 2017, 5:22 p.m., James Peach wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55509/
> -----------------------------------------------------------
>
> (Updated Jan. 13, 2017, 5:22 p.m.)
>
>
> Review request for mesos and Jiang Yan Xu.
>
>
> Bugs: MESOS-6920
> https://issues.apache.org/jira/browse/MESOS-6920
>
>
> Repository: mesos
>
>
> Description
> -------
>
> To be consistent with Master::statusUpdateAcknowledgement(), validate
> that the UUID in the StatusUpdate message is acceptable to the
> UUID::fromBytes() parser.
>
>
> Diffs
> -----
>
> 3rdparty/stout/include/stout/os/linux.hpp 68b4090aa5e6f23609c487128d91301755ae0e46
> docs/contributors.yaml d2b686c9c15782eefeed3376dc84edba22968745
> src/linux/ns.hpp da43266ba67f76c9ca856584a540794062097691
> src/master/master.cpp 1746a88953dbdc148d98881bcf7027b62ad6b040
> src/slave/containerizer/mesos/containerizer.cpp 4d2b4344e9698aed6ef0d49bb56d9292f2c3703b
> src/slave/containerizer/mesos/isolators/network/port_mapping.cpp f6f2bfe1d5d3c113036ad44a480f97bbb462a269
> support/apply-reviews.py c77b4c2b2d7a3d5e74e225403e71a84e23a9a1e7
>
> Diff: https://reviews.apache.org/r/55509/diff/
>
>
> Testing
> -------
>
> Make check (Fedora 25). Manual testing with a handmade StatusUpdate message.
>
>
> Thanks,
>
> James Peach
>
>
Re: Review Request 55509: Validate the StatusUpdate UUID in
Master::statusUpdate.
Posted by Jiang Yan Xu <ya...@jxu.me>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55509/#review161617
-----------------------------------------------------------
Fix it, then Ship it!
Sorry missed a few minor things.
src/master/master.cpp (line 5848)
<https://reviews.apache.org/r/55509/#comment232876>
Sorry forgot to mention that
s/uuid_/uuid/ (no need for it, the other instance in this file has a method argument `uuid` already)
src/master/master.cpp (line 5852)
<https://reviews.apache.org/r/55509/#comment232877>
s/":"/": "/
- Jiang Yan Xu
On Jan. 13, 2017, 3:29 p.m., James Peach wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55509/
> -----------------------------------------------------------
>
> (Updated Jan. 13, 2017, 3:29 p.m.)
>
>
> Review request for mesos and Jiang Yan Xu.
>
>
> Bugs: MESOS-6920
> https://issues.apache.org/jira/browse/MESOS-6920
>
>
> Repository: mesos
>
>
> Description
> -------
>
> To be consistent with Master::statusUpdateAcknowledgement(), validate
> that the UUID in the StatusUpdate message is acceptable to the
> UUID::fromBytes() parser.
>
>
> Diffs
> -----
>
> src/master/master.cpp 1746a88953dbdc148d98881bcf7027b62ad6b040
>
> Diff: https://reviews.apache.org/r/55509/diff/
>
>
> Testing
> -------
>
> Make check (Fedora 25). Manual testing with a handmade StatusUpdate message.
>
>
> Thanks,
>
> James Peach
>
>
Re: Review Request 55509: Validate the StatusUpdate UUID in
Master::statusUpdate.
Posted by Mesos ReviewBot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55509/#review161642
-----------------------------------------------------------
Bad patch!
Reviews applied: [55509]
Failed command: ['bash', '-c', "set -o pipefail; export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' CONFIGURATION='--verbose' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker_build.sh 2>&1 | tee build_55509"]
Error:
bash: ./support/docker_build.sh: No such file or directory
Full log: https://builds.apache.org/job/Mesos-Reviewbot/16721/console
- Mesos ReviewBot
On Jan. 13, 2017, 11:38 p.m., James Peach wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55509/
> -----------------------------------------------------------
>
> (Updated Jan. 13, 2017, 11:38 p.m.)
>
>
> Review request for mesos and Jiang Yan Xu.
>
>
> Bugs: MESOS-6920
> https://issues.apache.org/jira/browse/MESOS-6920
>
>
> Repository: mesos
>
>
> Description
> -------
>
> To be consistent with Master::statusUpdateAcknowledgement(), validate
> that the UUID in the StatusUpdate message is acceptable to the
> UUID::fromBytes() parser.
>
>
> Diffs
> -----
>
> src/master/master.cpp 1746a88953dbdc148d98881bcf7027b62ad6b040
>
> Diff: https://reviews.apache.org/r/55509/diff/
>
>
> Testing
> -------
>
> Make check (Fedora 25). Manual testing with a handmade StatusUpdate message.
>
>
> Thanks,
>
> James Peach
>
>
Re: Review Request 55509: Validate the StatusUpdate UUID in
Master::statusUpdate.
Posted by James Peach <jp...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55509/
-----------------------------------------------------------
(Updated Jan. 13, 2017, 11:38 p.m.)
Review request for mesos and Jiang Yan Xu.
Bugs: MESOS-6920
https://issues.apache.org/jira/browse/MESOS-6920
Repository: mesos
Description
-------
To be consistent with Master::statusUpdateAcknowledgement(), validate
that the UUID in the StatusUpdate message is acceptable to the
UUID::fromBytes() parser.
Diffs (updated)
-----
src/master/master.cpp 1746a88953dbdc148d98881bcf7027b62ad6b040
Diff: https://reviews.apache.org/r/55509/diff/
Testing
-------
Make check (Fedora 25). Manual testing with a handmade StatusUpdate message.
Thanks,
James Peach
Re: Review Request 55509: Validate the StatusUpdate UUID in
Master::statusUpdate.
Posted by James Peach <jp...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55509/
-----------------------------------------------------------
(Updated Jan. 13, 2017, 11:29 p.m.)
Review request for mesos and Jiang Yan Xu.
Changes
-------
Addressed review feedback.
Bugs: MESOS-6920
https://issues.apache.org/jira/browse/MESOS-6920
Repository: mesos
Description
-------
To be consistent with Master::statusUpdateAcknowledgement(), validate
that the UUID in the StatusUpdate message is acceptable to the
UUID::fromBytes() parser.
Diffs (updated)
-----
src/master/master.cpp 1746a88953dbdc148d98881bcf7027b62ad6b040
Diff: https://reviews.apache.org/r/55509/diff/
Testing
-------
Make check (Fedora 25). Manual testing with a handmade StatusUpdate message.
Thanks,
James Peach
Re: Review Request 55509: Validate the StatusUpdate UUID in
Master::statusUpdate.
Posted by Jiang Yan Xu <ya...@jxu.me>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55509/#review161611
-----------------------------------------------------------
Fix it, then Ship it!
Looks like there are a few more cases of status update acknowledgements that have this UUID issue.
src/master/master.cpp (line 5824)
<https://reviews.apache.org/r/55509/#comment232870>
Actually preincrement is the more common/style-guide-conforming style in Mesos and in the rest of the file, it's the other instances in this method that are inconsistent.
https://google.github.io/styleguide/cppguide.html#Preincrement_and_Predecrement
src/master/master.cpp (lines 5829 - 5830)
<https://reviews.apache.org/r/55509/#comment232872>
About
> ... from agent pid with id ...
If we move this check down to be after
```
Slave* slave = slaves.registered.get(update.slave_id());
if (slave == nullptr) {
...
}
```
Then you can just use the output overload to do
```
"agent " << *slave << ...;
```
which prints the agent info more uniformly.
- Jiang Yan Xu
On Jan. 13, 2017, 10:41 a.m., James Peach wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55509/
> -----------------------------------------------------------
>
> (Updated Jan. 13, 2017, 10:41 a.m.)
>
>
> Review request for mesos and Jiang Yan Xu.
>
>
> Bugs: MESOS-6920
> https://issues.apache.org/jira/browse/MESOS-6920
>
>
> Repository: mesos
>
>
> Description
> -------
>
> To be consistent with Master::statusUpdateAcknowledgement(), validate
> that the UUID in the StatusUpdate message is acceptable to the
> UUID::fromBytes() parser.
>
>
> Diffs
> -----
>
> src/master/master.cpp 1746a88953dbdc148d98881bcf7027b62ad6b040
>
> Diff: https://reviews.apache.org/r/55509/diff/
>
>
> Testing
> -------
>
> Make check (Fedora 25). Manual testing with a handmade StatusUpdate message.
>
>
> Thanks,
>
> James Peach
>
>
Re: Review Request 55509: Validate the StatusUpdate UUID in
Master::statusUpdate.
Posted by Mesos ReviewBot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55509/#review161596
-----------------------------------------------------------
Patch looks great!
Reviews applied: [55509]
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, 6:41 p.m., James Peach wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55509/
> -----------------------------------------------------------
>
> (Updated Jan. 13, 2017, 6:41 p.m.)
>
>
> Review request for mesos and Jiang Yan Xu.
>
>
> Bugs: MESOS-6920
> https://issues.apache.org/jira/browse/MESOS-6920
>
>
> Repository: mesos
>
>
> Description
> -------
>
> To be consistent with Master::statusUpdateAcknowledgement(), validate
> that the UUID in the StatusUpdate message is acceptable to the
> UUID::fromBytes() parser.
>
>
> Diffs
> -----
>
> src/master/master.cpp 1746a88953dbdc148d98881bcf7027b62ad6b040
>
> Diff: https://reviews.apache.org/r/55509/diff/
>
>
> Testing
> -------
>
> Make check (Fedora 25). Manual testing with a handmade StatusUpdate message.
>
>
> Thanks,
>
> James Peach
>
>
Re: Review Request 55509: Validate the StatusUpdate UUID in
Master::statusUpdate.
Posted by James Peach <jp...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55509/
-----------------------------------------------------------
(Updated Jan. 13, 2017, 6:41 p.m.)
Review request for mesos and Jiang Yan Xu.
Changes
-------
Rebased onto master.
Bugs: MESOS-6920
https://issues.apache.org/jira/browse/MESOS-6920
Repository: mesos
Description
-------
To be consistent with Master::statusUpdateAcknowledgement(), validate
that the UUID in the StatusUpdate message is acceptable to the
UUID::fromBytes() parser.
Diffs (updated)
-----
src/master/master.cpp 1746a88953dbdc148d98881bcf7027b62ad6b040
Diff: https://reviews.apache.org/r/55509/diff/
Testing
-------
Make check (Fedora 25). Manual testing with a handmade StatusUpdate message.
Thanks,
James Peach