Re: Review Request 70546: WIP: Relaxed protobuf union validation strictness.

2019-04-26 Thread Benno Evers

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/70546/#review214920
---


Ship it!




Ship It!

- Benno Evers


On April 25, 2019, 8:43 p.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70546/
> ---
> 
> (Updated April 25, 2019, 8:43 p.m.)
> 
> 
> Review request for mesos, Andrei Sekretenko, Benno Evers, and Greg Mann.
> 
> 
> Bugs: MESOS-9740
> https://issues.apache.org/jira/browse/MESOS-9740
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> As part of MESOS-6874, the master validates protobuf unions passed as
> part of an ExecutorInfo::ContainerInfo.  This prevents a task from
> specifying, for example, a ContainerInfo::MESOS, but filling
> out the docker field (which is then ignored by the agent).
> 
> This validation change is actually an API change, because previously
> runnable ExecutorInfo's and TaskInfo's will now fail validation.
> This has two visible effects on clusters:
>   * Agents running containers with invalid protobuf unions will not
> be able to reregister with the master.
>   * Existing frameworks will not be able to re-launch the same tasks
> that were working before a Mesos master upgrade.
> 
> This changes the validation to print a warning instead.  Where possible,
> the warning will provide some information to indicate which task
> or executor is sending the invalid protobuf.
> 
> 
> Diffs
> -
> 
>   src/common/validation.cpp 458f2258cc5fb76e65e2988dd3ab8bb827b0ac2d 
>   src/master/validation.cpp d7f210fc1ed228113c7f97bce9a43916840b2252 
>   src/tests/master_validation_tests.cpp 
> c98f7517a1c29eea36f9a3c3da2cda1441967b77 
> 
> 
> Diff: https://reviews.apache.org/r/70546/diff/2/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>



Re: Review Request 70546: WIP: Relaxed protobuf union validation strictness.

2019-04-25 Thread Mesos Reviewbot

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/70546/#review214900
---



Patch looks great!

Reviews applied: [70546]

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 April 25, 2019, 8:43 p.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70546/
> ---
> 
> (Updated April 25, 2019, 8:43 p.m.)
> 
> 
> Review request for mesos, Andrei Sekretenko, Benno Evers, and Greg Mann.
> 
> 
> Bugs: MESOS-9740
> https://issues.apache.org/jira/browse/MESOS-9740
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> As part of MESOS-6874, the master validates protobuf unions passed as
> part of an ExecutorInfo::ContainerInfo.  This prevents a task from
> specifying, for example, a ContainerInfo::MESOS, but filling
> out the docker field (which is then ignored by the agent).
> 
> This validation change is actually an API change, because previously
> runnable ExecutorInfo's and TaskInfo's will now fail validation.
> This has two visible effects on clusters:
>   * Agents running containers with invalid protobuf unions will not
> be able to reregister with the master.
>   * Existing frameworks will not be able to re-launch the same tasks
> that were working before a Mesos master upgrade.
> 
> This changes the validation to print a warning instead.  Where possible,
> the warning will provide some information to indicate which task
> or executor is sending the invalid protobuf.
> 
> 
> Diffs
> -
> 
>   src/common/validation.cpp 458f2258cc5fb76e65e2988dd3ab8bb827b0ac2d 
>   src/master/validation.cpp d7f210fc1ed228113c7f97bce9a43916840b2252 
>   src/tests/master_validation_tests.cpp 
> c98f7517a1c29eea36f9a3c3da2cda1441967b77 
> 
> 
> Diff: https://reviews.apache.org/r/70546/diff/2/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>



Re: Review Request 70546: WIP: Relaxed protobuf union validation strictness.

2019-04-25 Thread Joseph Wu

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/70546/
---

(Updated April 25, 2019, 1:43 p.m.)


Review request for mesos, Andrei Sekretenko, Benno Evers, and Greg Mann.


Changes
---

Disabled the test that was added to test the protobuf union validation change.


Bugs: MESOS-9740
https://issues.apache.org/jira/browse/MESOS-9740


Repository: mesos


Description
---

As part of MESOS-6874, the master validates protobuf unions passed as
part of an ExecutorInfo::ContainerInfo.  This prevents a task from
specifying, for example, a ContainerInfo::MESOS, but filling
out the docker field (which is then ignored by the agent).

This validation change is actually an API change, because previously
runnable ExecutorInfo's and TaskInfo's will now fail validation.
This has two visible effects on clusters:
  * Agents running containers with invalid protobuf unions will not
be able to reregister with the master.
  * Existing frameworks will not be able to re-launch the same tasks
that were working before a Mesos master upgrade.

This changes the validation to print a warning instead.  Where possible,
the warning will provide some information to indicate which task
or executor is sending the invalid protobuf.


Diffs (updated)
-

  src/common/validation.cpp 458f2258cc5fb76e65e2988dd3ab8bb827b0ac2d 
  src/master/validation.cpp d7f210fc1ed228113c7f97bce9a43916840b2252 
  src/tests/master_validation_tests.cpp 
c98f7517a1c29eea36f9a3c3da2cda1441967b77 


Diff: https://reviews.apache.org/r/70546/diff/2/

Changes: https://reviews.apache.org/r/70546/diff/1-2/


Testing (updated)
---

make check


Thanks,

Joseph Wu



Re: Review Request 70546: WIP: Relaxed protobuf union validation strictness.

2019-04-25 Thread Vinod Kone

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/70546/#review214896
---


Ship it!




Ship It!

- Vinod Kone


On April 25, 2019, 8:07 p.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70546/
> ---
> 
> (Updated April 25, 2019, 8:07 p.m.)
> 
> 
> Review request for mesos, Andrei Sekretenko, Benno Evers, and Greg Mann.
> 
> 
> Bugs: MESOS-9740
> https://issues.apache.org/jira/browse/MESOS-9740
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> As part of MESOS-6874, the master validates protobuf unions passed as
> part of an ExecutorInfo::ContainerInfo.  This prevents a task from
> specifying, for example, a ContainerInfo::MESOS, but filling
> out the docker field (which is then ignored by the agent).
> 
> This validation change is actually an API change, because previously
> runnable ExecutorInfo's and TaskInfo's will now fail validation.
> This has two visible effects on clusters:
>   * Agents running containers with invalid protobuf unions will not
> be able to reregister with the master.
>   * Existing frameworks will not be able to re-launch the same tasks
> that were working before a Mesos master upgrade.
> 
> This changes the validation to print a warning instead.  Where possible,
> the warning will provide some information to indicate which task
> or executor is sending the invalid protobuf.
> 
> 
> Diffs
> -
> 
>   src/common/validation.cpp 458f2258cc5fb76e65e2988dd3ab8bb827b0ac2d 
>   src/master/validation.cpp d7f210fc1ed228113c7f97bce9a43916840b2252 
>   src/tests/master_validation_tests.cpp 
> c98f7517a1c29eea36f9a3c3da2cda1441967b77 
> 
> 
> Diff: https://reviews.apache.org/r/70546/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>



Review Request 70546: WIP: Relaxed protobuf union validation strictness.

2019-04-25 Thread Joseph Wu

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/70546/
---

Review request for mesos, Andrei Sekretenko, Benno Evers, and Greg Mann.


Bugs: MESOS-9740
https://issues.apache.org/jira/browse/MESOS-9740


Repository: mesos


Description
---

As part of MESOS-6874, the master validates protobuf unions passed as
part of an ExecutorInfo::ContainerInfo.  This prevents a task from
specifying, for example, a ContainerInfo::MESOS, but filling
out the docker field (which is then ignored by the agent).

This validation change is actually an API change, because previously
runnable ExecutorInfo's and TaskInfo's will now fail validation.
This has two visible effects on clusters:
  * Agents running containers with invalid protobuf unions will not
be able to reregister with the master.
  * Existing frameworks will not be able to re-launch the same tasks
that were working before a Mesos master upgrade.

This changes the validation to print a warning instead.  Where possible,
the warning will provide some information to indicate which task
or executor is sending the invalid protobuf.


Diffs
-

  src/common/validation.cpp 458f2258cc5fb76e65e2988dd3ab8bb827b0ac2d 
  src/master/validation.cpp d7f210fc1ed228113c7f97bce9a43916840b2252 
  src/tests/master_validation_tests.cpp 
c98f7517a1c29eea36f9a3c3da2cda1441967b77 


Diff: https://reviews.apache.org/r/70546/diff/1/


Testing
---


Thanks,

Joseph Wu