mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Benno Evers <bev...@mesosphere.com>
Subject Re: Review Request 70538: WIP: Fixed upgrade path for tasks with invalid protobuf unions.
Date Wed, 24 Apr 2019 08:38:07 GMT

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



Probably clearing the incorrect protobufs should be done in the master, otherwise we get the
same problem when running 1.7 agents with an 1.8 master.

So maybe an alternative approach would be to remove the check from the general `ContainerInfo`
validation and only do it for incoming `LaunchTasks` messages. 

However, I think the bigger problem is the following: If we have reasonable suspicion that
this validation will trigger often, we're going to break a lot of users' frameworks that generated
these incorrect protobuf unions in the first place. So we should think about either removing
the validation completely, with a comment explaining why it can't be done, or at least provide
a master flag to turn off this behaviour for operators who are not in a position to easily
fix their frameworks.

- Benno Evers


On April 24, 2019, 3:28 a.m., Joseph Wu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70538/
> -----------------------------------------------------------
> 
> (Updated April 24, 2019, 3:28 a.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 now 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).
> 
> However, if a task was already launched with an invalid protobuf
> union, the same validation will happen when the agent tries to
> reregister with the master.  In this case, if the master is upgraded
> to validate protobuf unions, the agent reregistration will be rejected.
> 
> This adds a hack to wipe invalid fields from the agent's reregistration
> message before sending it to the master.
> 
> 
> Diffs
> -----
> 
>   src/slave/slave.cpp 95f05a18c7905d5032de1cd35726ac3a17f0b682 
> 
> 
> Diff: https://reviews.apache.org/r/70538/diff/1/
> 
> 
> Testing
> -------
> 
> WIP!!!
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message