Re: Review Request 55509: Validate the StatusUpdate UUID in Master::statusUpdate.

2017-01-14 Thread Mesos ReviewBot 

---
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.

2017-01-13 Thread James Peach 

---
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.

2017-01-13 Thread Jiang Yan Xu 

---
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)


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)


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.

2017-01-13 Thread James Peach 

---
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.

2017-01-13 Thread Jiang Yan Xu 

---
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)


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)


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.

2017-01-13 Thread Mesos ReviewBot 

---
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.

2017-01-13 Thread James Peach 


> 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.

2017-01-13 Thread James Peach 

---
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

Re: Review Request 55509: Validate the StatusUpdate UUID in Master::statusUpdate.

2017-01-13 Thread Neil Conway 

---
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
> 
>

Review Request 55509: Validate the StatusUpdate UUID in Master::statusUpdate.

2017-01-13 Thread James Peach 

---
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