mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Andrei Sekretenko <asekrete...@apache.org>
Subject Re: Review Request 72956: Added validation that offer constraints are set only for existing roles.
Date Wed, 14 Oct 2020 18:42:23 GMT


> On Oct. 14, 2020, 12:54 a.m., Benjamin Mahler wrote:
> > src/master/master.cpp
> > Line 2635 (original), 2635-2642 (patched)
> > <https://reviews.apache.org/r/72956/diff/1/?file=2240634#file2240634line2635>
> >
> >     Ditto here from the last review, shouldn't this go into the validation.hpp existing
stateless validation?

Tried to move this into a separate function in `validation.cpp`; not sure if this makes things
better or worse.

The thing with the validation of roles in constraints is that I want to be very clear that
this is orthogonal to the internal validity of `OfferConstraints`.

Hence, a stateless validation function like `validate(const FrameworkInfo&, const set<string>&
suppressedRoles, const OfferConstraints&)` would look very misleading (or, it will have
to do the job of the filter regexp construction just the sake of constraints validation; this
will also add the regex construction options to the signature).

One more alternative would be to take the whole `createAllocatorFrameworkOptions()` out of
`master.cpp`, which will make it unit-testable.


- Andrei


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


On Oct. 14, 2020, 8:18 p.m., Andrei Sekretenko wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/72956/
> -----------------------------------------------------------
> 
> (Updated Oct. 14, 2020, 8:18 p.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Bugs: MESOS-10176
>     https://issues.apache.org/jira/browse/MESOS-10176
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This patch makes SUBSCRIBE/UPDATE_FRAMEWORK calls validate that
> the framework does not specify offer constraints for roles to which
> it is not going to be subscribed.
> 
> 
> Diffs
> -----
> 
>   src/master/master.cpp d6d3ea7e27539a65b601783eca2ac4e3b688f188 
>   src/master/validation.hpp 7fe8f087b322f8e77218f4dd9f9f75707afa3208 
>   src/master/validation.cpp 5b1bcb5207aac0553bf9c1a0d89525c835393ccb 
>   src/tests/master/update_framework_tests.cpp 3f86573e8dfeea63fe99064f2137c61d901f4fc7

> 
> 
> Diff: https://reviews.apache.org/r/72956/diff/2/
> 
> 
> Testing
> -------
> 
> `make check`
> `src/mesos-tests --gtest_filter='*UpdateFrameworkTest.OfferConstraintsForUnsubscribedRole*'
--gtest_break_on_failure --gtest_repeat=1000`
> 
> 
> Thanks,
> 
> Andrei Sekretenko
> 
>


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