Re: Review Request 72267: Added docs for UPDATE_FRAMEWORK call.

[Andrei Sekretenko]

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

(Updated May 6, 2020, 3:10 p.m.)


Review request for mesos, Benjamin Mahler and Greg Mann.


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


Repository: mesos


Description
---

Added docs for UPDATE_FRAMEWORK call.


Diffs (updated)
-

  docs/scheduler-http-api.md 9831d527cc1f832a6fb0d0d330ebdc2a0b0f3774 


Diff: https://reviews.apache.org/r/72267/diff/3/

Changes: https://reviews.apache.org/r/72267/diff/2-3/


Testing
---

Checked rendering in Github: 
https://github.com/asekretenko/mesos/blob/update_framework_doc/docs/scheduler-http-api.md


Thanks,

Andrei Sekretenko

Re: Review Request 72267: Added docs for UPDATE_FRAMEWORK call.

[Mesos Reviewbot]

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



Patch looks great!

Reviews applied: [72267]

Passed command: export OS='ubuntu:16.04' BUILDTOOL='autotools' COMPILER='gcc' 
CONFIGURATION='--verbose --disable-libtool-wrappers 
--disable-parallel-test-execution' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; 
./support/jenkins/buildbot.sh

- Mesos Reviewbot


On May 5, 2020, 7:06 p.m., Andrei Sekretenko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/72267/
> ---
> 
> (Updated May 5, 2020, 7:06 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Greg Mann.
> 
> 
> Bugs: MESOS-9979
> https://issues.apache.org/jira/browse/MESOS-9979
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added docs for UPDATE_FRAMEWORK call.
> 
> 
> Diffs
> -
> 
>   docs/scheduler-http-api.md 9831d527cc1f832a6fb0d0d330ebdc2a0b0f3774 
> 
> 
> Diff: https://reviews.apache.org/r/72267/diff/2/
> 
> 
> Testing
> ---
> 
> Checked rendering in Github: 
> https://github.com/asekretenko/mesos/blob/update_framework_doc/docs/scheduler-http-api.md
> 
> 
> Thanks,
> 
> Andrei Sekretenko
> 
>

Re: Review Request 72267: Added docs for UPDATE_FRAMEWORK call.

[Benjamin Mahler]

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


Fix it, then Ship it!





docs/scheduler-http-api.md
Lines 544-546 (patched)


Hm.. this note applies to all of the field updates, but since it's only 
specified for capabilities, the reader will assume there is some semantic 
importance of the agent processing a framework capability update. This is 
probably something they have to understand specific to each particular 
capability.

Perhaps instead just a general note up above about the possible response 
codes and when we send them? (i.e. it's just when the master processes it)



docs/scheduler-http-api.md
Lines 547 (patched)


Instead of "not specified", maybe we explain here that we don't disallow 
capability removal **BUT** there are currently no safeguards in place to 
prevent capability removal when the capability semantically can no longer be 
removed (e.g. if the framework started using shared resources, then 
SHARED_RESOURCES became a requirement, removing it should be disallowed but we 
don't currently protect against it). So: don't remove a capability, and if you 
*really really* need to, contact the dev@ or user@ list first.

A link to the minimum capability stuff would be good too.


- Benjamin Mahler


On May 5, 2020, 7:06 p.m., Andrei Sekretenko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/72267/
> ---
> 
> (Updated May 5, 2020, 7:06 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Greg Mann.
> 
> 
> Bugs: MESOS-9979
> https://issues.apache.org/jira/browse/MESOS-9979
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added docs for UPDATE_FRAMEWORK call.
> 
> 
> Diffs
> -
> 
>   docs/scheduler-http-api.md 9831d527cc1f832a6fb0d0d330ebdc2a0b0f3774 
> 
> 
> Diff: https://reviews.apache.org/r/72267/diff/2/
> 
> 
> Testing
> ---
> 
> Checked rendering in Github: 
> https://github.com/asekretenko/mesos/blob/update_framework_doc/docs/scheduler-http-api.md
> 
> 
> Thanks,
> 
> Andrei Sekretenko
> 
>

Re: Review Request 72267: Added docs for UPDATE_FRAMEWORK call.

[Andrei Sekretenko]

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

(Updated May 5, 2020, 7:06 p.m.)


Review request for mesos, Benjamin Mahler and Greg Mann.


Changes
---

Addressed issues and split UPDATE_FRAMEWORK into subsections.


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


Repository: mesos


Description
---

Added docs for UPDATE_FRAMEWORK call.


Diffs (updated)
-

  docs/scheduler-http-api.md 9831d527cc1f832a6fb0d0d330ebdc2a0b0f3774 


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

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


Testing
---

Checked rendering in Github: 
https://github.com/asekretenko/mesos/blob/update_framework_doc/docs/scheduler-http-api.md


Thanks,

Andrei Sekretenko

Re: Review Request 72267: Added docs for UPDATE_FRAMEWORK call.

[Benjamin Mahler]

> On March 25, 2020, 7:20 p.m., Benjamin Mahler wrote:
> > docs/scheduler-http-api.md
> > Lines 534-535 (patched)
> > 
> >
> > Huh? Which fields are being referred to here?
> 
> Andrei Sekretenko wrote:
> Everything not named above: all the fileds except for `principal`, 
> `user`, `checkpoint`, `roles`/`role` and, obviously, except of `id`.
> 
> 
> There, actually, is a good question: how well tested is update of other 
> fields and what we can document here?
> 
> I tried to dig into the current state a bit. 
> 
> First, there are basic tests that resubscription/UPDATE_FRAMEWORK can 
> change all other fields in `FrameworkInfo` as reported by Master.
> 
> Then, I would say, there are three categories of fields:
> 1) `name`, `hostname`, `webui_url` and `labels`: these are not really 
> used by Mesos, basic tests seem enough.
> 
> 2) `failover_timeout` and `offer_filters`: don't see where we test the 
> effects of updating them, but those effects look more or less trivial.
> 
> 3) `capabilities` field: changing some of them has non-trivial effects. 
>For example, removing `REVOCABLE_RESOURCES` results in rescinging 
> offered revocable resources; this particular scenario is tested. 
>At this point I'm not really sure that every non-trivial capability 
> change is well-designed and covered by tests.
> 
> Do you have any suggestions how to better document all this stuff? 
> Can we safely say that Mesos supports update of (1)? How about (2)?
> What to state about `capabilities` other than `REVOCABLE_RESOURCES`?...

I would say that if we don't explicitly deny it and we've told the user 200 OK, 
then it should be changed in the master and the updates should be propagated to 
the agents. 

1) should be supported, with the caveat that anything that is looking at these 
needs to be aware that they may change

2) should definitely be supported

3) Capabilities get tricky since we never implemented minimum capabilities for 
schedulers yet (so we can't validate that the removal of capabilities is safe). 
Like [MESOS-8878](https://issues.apache.org/jira/browse/MESOS-8878) but for 
schedulers.

I would suggest spelling these subtleties out, so that users know there are 
some pitfalls here, and there are no safety guarantees for 3).


- Benjamin


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


On March 25, 2020, 2:08 p.m., Andrei Sekretenko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/72267/
> ---
> 
> (Updated March 25, 2020, 2:08 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Greg Mann.
> 
> 
> Bugs: MESOS-9979
> https://issues.apache.org/jira/browse/MESOS-9979
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added docs for UPDATE_FRAMEWORK call.
> 
> 
> Diffs
> -
> 
>   docs/scheduler-http-api.md 9831d527cc1f832a6fb0d0d330ebdc2a0b0f3774 
> 
> 
> Diff: https://reviews.apache.org/r/72267/diff/1/
> 
> 
> Testing
> ---
> 
> Checked rendering in Github: 
> https://github.com/asekretenko/mesos/blob/update_framework_doc/docs/scheduler-http-api.md
> 
> 
> Thanks,
> 
> Andrei Sekretenko
> 
>

Re: Review Request 72267: Added docs for UPDATE_FRAMEWORK call.

[Andrei Sekretenko]

> On March 25, 2020, 7:20 p.m., Benjamin Mahler wrote:
> > docs/scheduler-http-api.md
> > Lines 529-530 (patched)
> > 
> >
> > Is this true?
> > 
> > 
> > https://github.com/apache/mesos/blob/1.9.0/src/master/allocator/mesos/hierarchical.cpp#L765-L766
> > 
> > ```
> >   suppressRoles(framework, suppressedRoles - oldSuppressedRoles);
> >   reviveRoles(framework, (oldSuppressedRoles - suppressedRoles) & 
> > newRoles);
> > ```
> > 
> > 1. suppress roles that transitioned into supressed
> > 2. revive roles that were transitioned out of supressed
> > 
> > So, more like "For roles that were transitioned out of suppressed, 
> > offer filters (set by `ACCEPT`/`DECLINE`) will be cleared.
> > 
> > Right?

Yes, you are right, existing roles that have filters but are not suppressed 
will not have the filters cleared. 
Will amend this line.


> On March 25, 2020, 7:20 p.m., Benjamin Mahler wrote:
> > docs/scheduler-http-api.md
> > Lines 534-535 (patched)
> > 
> >
> > Huh? Which fields are being referred to here?

Everything not named above: all the fileds except for `principal`, `user`, 
`checkpoint`, `roles`/`role` and, obviously, except of `id`.


There, actually, is a good question: how well tested is update of other fields 
and what we can document here?

I tried to dig into the current state a bit. 

First, there are basic tests that resubscription/UPDATE_FRAMEWORK can change 
all other fields in `FrameworkInfo` as reported by Master.

Then, I would say, there are three categories of fields:
1) `name`, `hostname`, `webui_url` and `labels`: these are not really used by 
Mesos, basic tests seem enough.

2) `failover_timeout` and `offer_filters`: don't see where we test the effects 
of updating them, but those effects look more or less trivial.

3) `capabilities` field: changing some of them has non-trivial effects. 
   For example, removing `REVOCABLE_RESOURCES` results in rescinging offered 
revocable resources; this particular scenario is tested. 
   At this point I'm not really sure that every non-trivial capability change 
is well-designed and covered by tests.

Do you have any suggestions how to better document all this stuff? 
Can we safely say that Mesos supports update of (1)? How about (2)?
What to state about `capabilities` other than `REVOCABLE_RESOURCES`?...


- Andrei


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


On March 25, 2020, 2:08 p.m., Andrei Sekretenko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/72267/
> ---
> 
> (Updated March 25, 2020, 2:08 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Greg Mann.
> 
> 
> Bugs: MESOS-9979
> https://issues.apache.org/jira/browse/MESOS-9979
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added docs for UPDATE_FRAMEWORK call.
> 
> 
> Diffs
> -
> 
>   docs/scheduler-http-api.md 9831d527cc1f832a6fb0d0d330ebdc2a0b0f3774 
> 
> 
> Diff: https://reviews.apache.org/r/72267/diff/1/
> 
> 
> Testing
> ---
> 
> Checked rendering in Github: 
> https://github.com/asekretenko/mesos/blob/update_framework_doc/docs/scheduler-http-api.md
> 
> 
> Thanks,
> 
> Andrei Sekretenko
> 
>

Re: Review Request 72267: Added docs for UPDATE_FRAMEWORK call.

[Benjamin Mahler]

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



Oh yeah forgot about this, thanks for adding it!


docs/scheduler-http-api.md
Lines 528 (patched)


I assume this is the same as when a role is suppressed via SUPPRESS call?



docs/scheduler-http-api.md
Lines 529-530 (patched)


Is this true?


https://github.com/apache/mesos/blob/1.9.0/src/master/allocator/mesos/hierarchical.cpp#L765-L766

```
  suppressRoles(framework, suppressedRoles - oldSuppressedRoles);
  reviveRoles(framework, (oldSuppressedRoles - suppressedRoles) & newRoles);
```

1. suppress roles that transitioned into supressed
2. revive roles that were transitioned out of supressed

So, more like "For roles that were transitioned out of suppressed, offer 
filters (set by `ACCEPT`/`DECLINE`) will be cleared.

Right?



docs/scheduler-http-api.md
Lines 534-535 (patched)


Huh? Which fields are being referred to here?



docs/scheduler-http-api.md
Lines 575-579 (patched)


Can you link to the ticket here?


- Benjamin Mahler


On March 25, 2020, 2:08 p.m., Andrei Sekretenko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/72267/
> ---
> 
> (Updated March 25, 2020, 2:08 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Greg Mann.
> 
> 
> Bugs: MESOS-9979
> https://issues.apache.org/jira/browse/MESOS-9979
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added docs for UPDATE_FRAMEWORK call.
> 
> 
> Diffs
> -
> 
>   docs/scheduler-http-api.md 9831d527cc1f832a6fb0d0d330ebdc2a0b0f3774 
> 
> 
> Diff: https://reviews.apache.org/r/72267/diff/1/
> 
> 
> Testing
> ---
> 
> Checked rendering in Github: 
> https://github.com/asekretenko/mesos/blob/update_framework_doc/docs/scheduler-http-api.md
> 
> 
> Thanks,
> 
> Andrei Sekretenko
> 
>

Re: Review Request 72267: Added docs for UPDATE_FRAMEWORK call.

[Mesos Reviewbot]

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



Patch looks great!

Reviews applied: [72267]

Passed command: export OS='ubuntu:16.04' BUILDTOOL='autotools' COMPILER='gcc' 
CONFIGURATION='--verbose --disable-libtool-wrappers 
--disable-parallel-test-execution' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; 
./support/jenkins/buildbot.sh

- Mesos Reviewbot


On March 25, 2020, 2:08 p.m., Andrei Sekretenko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/72267/
> ---
> 
> (Updated March 25, 2020, 2:08 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Greg Mann.
> 
> 
> Bugs: MESOS-9979
> https://issues.apache.org/jira/browse/MESOS-9979
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added docs for UPDATE_FRAMEWORK call.
> 
> 
> Diffs
> -
> 
>   docs/scheduler-http-api.md 9831d527cc1f832a6fb0d0d330ebdc2a0b0f3774 
> 
> 
> Diff: https://reviews.apache.org/r/72267/diff/1/
> 
> 
> Testing
> ---
> 
> Checked rendering in Github: 
> https://github.com/asekretenko/mesos/blob/update_framework_doc/docs/scheduler-http-api.md
> 
> 
> Thanks,
> 
> Andrei Sekretenko
> 
>