You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Meng Zhu <mz...@mesosphere.io> on 2018/06/28 23:00:59 UTC

Review Request 67777: Added a helper to match agent-framework capabilities in the allocator.

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

Review request for mesos and Benjamin Mahler.


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


Repository: mesos


Description
-------

`isFrameworkCapableReceivingAgent` checks if the
framework is capable of receiving resources on the given
agent based on the agent and framework capabilities.


Diffs
-----

  src/master/allocator/mesos/hierarchical.hpp 0f6c0e96a105c64465d3f5db4ff663d8fdfe7e26 
  src/master/allocator/mesos/hierarchical.cpp cbdfb2ba9c25755ac631557e0e7dbd721f861a4d 


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


Testing
-------

make check


Thanks,

Meng Zhu


Re: Review Request 67777: Added a helper to match agent-framework capabilities in the allocator.

Posted by Meng Zhu <mz...@mesosphere.io>.

> On July 2, 2018, 3:42 p.m., Benjamin Mahler wrote:
> > src/master/allocator/mesos/hierarchical.cpp
> > Line 1756 (original), 1755 (patched)
> > <https://reviews.apache.org/r/67777/diff/2/?file=2047194#file2047194line1756>
> >
> >     How about a member function of the framework struct?
> >     
> >     ```
> >     if (!framework.isCapableOnAgent(slave)) {
> >       ...
> >     }
> >     ```
> 
> Meng Zhu wrote:
>     That would need to move nested Slave class forward or outside. Currently, all the helper functions for slave/framework are member functions of the outer class. I was trying to be consistent. If you think it is necessary, I can go ahead and overhaul the structure.

I could forward declare Slave in the outer class. Yet it would still depend on `filterGpuResources` of the outer class. Need to either cache a reference or pass in the flag. Feel cleaner this way.


- Meng


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


On June 28, 2018, 4 p.m., Meng Zhu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67777/
> -----------------------------------------------------------
> 
> (Updated June 28, 2018, 4 p.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Bugs: MESOS-8916
>     https://issues.apache.org/jira/browse/MESOS-8916
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> `isFrameworkCapableReceivingAgent` checks if a framework
> is capable of receiving resources on the agent based on
> the framework capability.
> 
> 
> Diffs
> -----
> 
>   src/master/allocator/mesos/hierarchical.hpp 0f6c0e96a105c64465d3f5db4ff663d8fdfe7e26 
>   src/master/allocator/mesos/hierarchical.cpp cbdfb2ba9c25755ac631557e0e7dbd721f861a4d 
> 
> 
> Diff: https://reviews.apache.org/r/67777/diff/2/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Meng Zhu
> 
>


Re: Review Request 67777: Added a helper to match agent-framework capabilities in the allocator.

Posted by Meng Zhu <mz...@mesosphere.io>.

> On July 2, 2018, 3:42 p.m., Benjamin Mahler wrote:
> > src/master/allocator/mesos/hierarchical.cpp
> > Line 1756 (original), 1755 (patched)
> > <https://reviews.apache.org/r/67777/diff/2/?file=2047194#file2047194line1756>
> >
> >     How about a member function of the framework struct?
> >     
> >     ```
> >     if (!framework.isCapableOnAgent(slave)) {
> >       ...
> >     }
> >     ```

That would need to move nested Slave class forward or outside. Currently, all the helper functions for slave/framework are member functions of the outer class. I was trying to be consistent. If you think it is necessary, I can go ahead and overhaul the structure.


- Meng


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


On June 28, 2018, 4 p.m., Meng Zhu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67777/
> -----------------------------------------------------------
> 
> (Updated June 28, 2018, 4 p.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Bugs: MESOS-8916
>     https://issues.apache.org/jira/browse/MESOS-8916
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> `isFrameworkCapableReceivingAgent` checks if a framework
> is capable of receiving resources on the agent based on
> the framework capability.
> 
> 
> Diffs
> -----
> 
>   src/master/allocator/mesos/hierarchical.hpp 0f6c0e96a105c64465d3f5db4ff663d8fdfe7e26 
>   src/master/allocator/mesos/hierarchical.cpp cbdfb2ba9c25755ac631557e0e7dbd721f861a4d 
> 
> 
> Diff: https://reviews.apache.org/r/67777/diff/2/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Meng Zhu
> 
>


Re: Review Request 67777: Added a helper to match agent-framework capabilities in the allocator.

Posted by Benjamin Mahler <bm...@apache.org>.

> On July 2, 2018, 10:42 p.m., Benjamin Mahler wrote:
> > src/master/allocator/mesos/hierarchical.cpp
> > Line 1756 (original), 1755 (patched)
> > <https://reviews.apache.org/r/67777/diff/2/?file=2047194#file2047194line1756>
> >
> >     How about a member function of the framework struct?
> >     
> >     ```
> >     if (!framework.isCapableOnAgent(slave)) {
> >       ...
> >     }
> >     ```
> 
> Meng Zhu wrote:
>     That would need to move nested Slave class forward or outside. Currently, all the helper functions for slave/framework are member functions of the outer class. I was trying to be consistent. If you think it is necessary, I can go ahead and overhaul the structure.
> 
> Meng Zhu wrote:
>     I could forward declare Slave in the outer class. Yet it would still depend on `filterGpuResources` of the outer class. Need to either cache a reference or pass in the flag. Feel cleaner this way.

Which functions are you referring to? I only see isRemoteSlave and isWhitelisted that look like they should be members of Slave?

Yeah I think the ideal would be to pull these structs out of being nested so that the logic can be more clearly divided between them and the allocator, like we did in master.hpp for Slave and Framework structs. We can just put the TODOs in this patch and follow up separately for that, can you mark as fixed by adding TODOs?


- Benjamin


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


On July 4, 2018, 12:19 a.m., Meng Zhu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67777/
> -----------------------------------------------------------
> 
> (Updated July 4, 2018, 12:19 a.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Bugs: MESOS-8916
>     https://issues.apache.org/jira/browse/MESOS-8916
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> `isFrameworkCapableReceivingAgent` checks if a framework
> is capable of receiving resources on the agent based on
> the framework capability.
> 
> 
> Diffs
> -----
> 
>   src/master/allocator/mesos/hierarchical.hpp 0f6c0e96a105c64465d3f5db4ff663d8fdfe7e26 
>   src/master/allocator/mesos/hierarchical.cpp 5a6cd3d2fc5bdbaaee2d05b9be9e83d4107c749b 
> 
> 
> Diff: https://reviews.apache.org/r/67777/diff/3/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Meng Zhu
> 
>


Re: Review Request 67777: Added a helper to match agent-framework capabilities in the allocator.

Posted by Benjamin Mahler <bm...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/67777/#review205647
-----------------------------------------------------------



Nice :)


src/master/allocator/mesos/hierarchical.cpp
Line 1756 (original), 1755 (patched)
<https://reviews.apache.org/r/67777/#comment288535>

    How about a member function of the framework struct?
    
    ```
    if (!framework.isCapableOnAgent(slave)) {
      ...
    }
    ```


- Benjamin Mahler


On June 28, 2018, 11 p.m., Meng Zhu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67777/
> -----------------------------------------------------------
> 
> (Updated June 28, 2018, 11 p.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Bugs: MESOS-8916
>     https://issues.apache.org/jira/browse/MESOS-8916
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> `isFrameworkCapableReceivingAgent` checks if a framework
> is capable of receiving resources on the agent based on
> the framework capability.
> 
> 
> Diffs
> -----
> 
>   src/master/allocator/mesos/hierarchical.hpp 0f6c0e96a105c64465d3f5db4ff663d8fdfe7e26 
>   src/master/allocator/mesos/hierarchical.cpp cbdfb2ba9c25755ac631557e0e7dbd721f861a4d 
> 
> 
> Diff: https://reviews.apache.org/r/67777/diff/2/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Meng Zhu
> 
>


Re: Review Request 67777: Added a helper to match agent-framework capabilities in the allocator.

Posted by Mesos Reviewbot Windows <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/67777/#review205707
-----------------------------------------------------------



FAIL: Some of the unit tests failed. Please check the relevant logs.

Reviews applied: `['67444', '67777']`

Failed command: `Start-MesosCITesting`

All the build artifacts available at: http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/67777

Relevant logs:

- [mesos-tests-stdout.log](http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/67777/logs/mesos-tests-stdout.log):

```
[ RUN      ] DockerContainerizerTest.ROOT_DOCKER_DestroyWhilePulling
[       OK ] DockerContainerizerTest.ROOT_DOCKER_DestroyWhilePulling (802 ms)
[ RUN      ] DockerContainerizerTest.ROOT_DOCKER_DestroyUnknownContainer
[       OK ] DockerContainerizerTest.ROOT_DOCKER_DestroyUnknownContainer (603 ms)
[ RUN      ] DockerContainerizerTest.ROOT_DOCKER_ExecutorCleanupWhenLaunchFailed
[       OK ] DockerContainerizerTest.ROOT_DOCKER_ExecutorCleanupWhenLaunchFailed (1225 ms)
[ RUN      ] DockerContainerizerTest.ROOT_DOCKER_FetchFailure
[       OK ] DockerContainerizerTest.ROOT_DOCKER_FetchFailure (800 ms)
[ RUN      ] DockerContainerizerTest.ROOT_DOCKER_DockerPullFailure
[       OK ] DockerContainerizerTest.ROOT_DOCKER_DockerPullFailure (800 ms)
[ RUN      ] DockerContainerizerTest.ROOT_DOCKER_DockerInspectDiscard
[       OK ] DockerContainerizerTest.ROOT_DOCKER_DockerInspectDiscard (1005 ms)
[ RUN      ] DockerContainerizerTest.ROOT_DOCKER_WaitUnknownContainer
[       OK ] DockerContainerizerTest.ROOT_DOCKER_WaitUnknownContainer (598 ms)
[ RUN      ] DockerContainerizerTest.ROOT_DOCKER_NoTransitionFromKillingToRunning
[       OK ] DockerContainerizerTest.ROOT_DOCKER_NoTransitionFromKillingToRunning (5157 ms)
[ RUN      ] DockerContainerizerTest.ROOT_DOCKER_DefaultDNS
[       OK ] DockerContainerizerTest.ROOT_DOCKER_DefaultDNS (4958 ms)
[----------] 24 tests from DockerContainerizerTest (91902 ms total)

[----------] 1 test from HungDockerTest
[ RUN      ] HungDockerTest.ROOT_DOCKER_InspectHungDuringPull

d:\dcos\mesos\mesos\src\tests\mock_docker.hpp(155): ERROR: this mock object (used in test HungDockerTest.ROOT_DOCKER_InspectHungDuringPull) should be deleted but never is. Its address is @000000B2D18FBDF0.
d:\dcos\mesos\mesos\src\tests\containerizer\docker_containerizer_tests.cpp(5187): ERROR: this mock object (used in test HungDockerTest.ROOT_DOCKER_InspectHungDuringPull) should be deleted but never is. Its address is @000000B2D18FC050.
d:\dcos\mesos\mesos\src\tests\mock_docker.cpp(48): ERROR: this mock object (used in test HungDockerTest.ROOT_DOCKER_InspectHungDuringPull) should be deleted but never is. Its address is @0000022C4A108500.
d:\dcos\mesos\mesos\src\tests\mock_registrar.cpp(54): ERROR: this mock object (used in test HungDockerTest.ROOT_DOCKER_InspectHungDuringPull) should be deleted but never is. Its address is @0000022C4A22F4A0.
d:\dcos\mesos\mesos\3rdparty\libprocess\include\process\gmock.hpp(235): ERROR: this mock object (used in test HungDockerTest.ROOT_DOCKER_InspectHungDuringPull) should be deleted but never is. Its address is @0000022C4AF5A398.
ERROR: 5 leaked mock objects found at program exit.
```

- [mesos-tests-stderr.log](http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/67777/logs/mesos-tests-stderr.log):

```
I0704 01:50:36.285436  9864 authenticatee.cpp:299] Authentication success
I0704 01:50:36.285436  8188 master.cpp:9802] Successfully authenticated principal 'test-principal' at scheduler-46f75a01-a36a-4d74-b2e0-e99a91dfeb17@192.10.1.6:59619
I0704 01:50:36.286437  7284 sched.cpp:501] Successfully authenticated with master master@192.10.1.6:59619
I0704 01:50:36.287441  8188 master.cpp:2927] Received SUBSCRIBE call for framework 'default' at scheduler-46f75a01-a36a-4d74-b2e0-e99a91dfeb17@192.10.1.6:59619
I0704 01:50:36.287441  8188 master.cpp:2234] Authorizing framework principal 'test-principal' to receive offers for roles '{ * }'
I0704 01:50:36.289436  9752 master.cpp:3008] Subscribing framework default with checkpointing disabled and capabilities [ MULTI_ROLE, RESERVATION_REFINEMENT ]
I0704 01:50:36.289436  9752 master.cpp:9993] Adding framework ebdb3629-3c86-4c70-8655-c4a251fefbfe-0000 (default) at scheduler-46f75a01-a36a-4d74-b2e0-e99a91dfeb17@192.10.1.6:59619 with roles {  } suppressed
I0704 01:50:36.290439  5892 sched.cpp:749] Framework registered with ebdb3629-3c86-4c70-8655-c4a251fefbfe-0000
I0704 01:50:36.291432  7248 hierarchical.cpp:298] Added framework ebdb3629-3c86-4c70-8655-c4a251fefbfe-0000
E0704 01:50:36.388473  9752 slave.cpp:7289] EXIT with status 1: Failed to perform recovery: Collect failed: Failed to run 'C:\Program Files (x86)\Microsoft Visual Studio\2017\Community\VC\Auxiliary\Build\test-docker.bat -H npipe:////./pipe/docker_engine ps -a': exited with status 1; stderr=''C:\Program' is not recognized as an internal or external command,

operable program or batch file.

'
If recovery failed due to a change in configuration and you want to
keep the current agent id, you might want to change the
`--reconfiguration_policy` flag to a more permissive value.

To restart this agent with a new agent id instead, do as follows:
rm -f C:\Users\jenkins\AppData\Local\Temp\Xka2wI\meta\slaves\latest
This ensures that the agent does not recover old live executors.

If you use the Docker containerizer and think that the Docker
daemon state is broken, you can try to clear it. But be careful:
these commands will erase all containers and images from this host,
not just those started by Mesos!
docker kill $(docker ps -q)
docker rm $(docker ps -a -q)
docker rmi $(docker images -q)

Finally, restart the agent.
```

- Mesos Reviewbot Windows


On July 4, 2018, 12:19 a.m., Meng Zhu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67777/
> -----------------------------------------------------------
> 
> (Updated July 4, 2018, 12:19 a.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Bugs: MESOS-8916
>     https://issues.apache.org/jira/browse/MESOS-8916
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> `isFrameworkCapableReceivingAgent` checks if a framework
> is capable of receiving resources on the agent based on
> the framework capability.
> 
> 
> Diffs
> -----
> 
>   src/master/allocator/mesos/hierarchical.hpp 0f6c0e96a105c64465d3f5db4ff663d8fdfe7e26 
>   src/master/allocator/mesos/hierarchical.cpp 5a6cd3d2fc5bdbaaee2d05b9be9e83d4107c749b 
> 
> 
> Diff: https://reviews.apache.org/r/67777/diff/3/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Meng Zhu
> 
>


Re: Review Request 67777: Added a helper to match agent-framework capabilities in the allocator.

Posted by Benjamin Mahler <bm...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/67777/#review206360
-----------------------------------------------------------


Ship it!




Looks good! Will get this committed shortly.


src/master/allocator/mesos/hierarchical.cpp
Lines 2805 (patched)
<https://reviews.apache.org/r/67777/#comment289297>

    Rogue newline?


- Benjamin Mahler


On July 20, 2018, 11:28 p.m., Meng Zhu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67777/
> -----------------------------------------------------------
> 
> (Updated July 20, 2018, 11:28 p.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Bugs: MESOS-9104
>     https://issues.apache.org/jira/browse/MESOS-9104
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> `isCapableOfReceivingAgent*(` checks if a framework
> is capable of receiving resources on the agent based on
> the framework capability.
> 
> 
> Diffs
> -----
> 
>   src/master/allocator/mesos/hierarchical.hpp 702e7c0aa84b4b672d82c759c25a28a77c78ad50 
>   src/master/allocator/mesos/hierarchical.cpp 707dd6bd0f255a64d759ce87cbf75be57d86b392 
> 
> 
> Diff: https://reviews.apache.org/r/67777/diff/5/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Meng Zhu
> 
>


Re: Review Request 67777: Added a helper to match agent-framework capabilities in the allocator.

Posted by Meng Zhu <mz...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/67777/
-----------------------------------------------------------

(Updated July 20, 2018, 4:28 p.m.)


Review request for mesos and Benjamin Mahler.


Changes
-------

Rebased.


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


Repository: mesos


Description
-------

`isCapableOfReceivingAgent*(` checks if a framework
is capable of receiving resources on the agent based on
the framework capability.


Diffs (updated)
-----

  src/master/allocator/mesos/hierarchical.hpp 702e7c0aa84b4b672d82c759c25a28a77c78ad50 
  src/master/allocator/mesos/hierarchical.cpp 707dd6bd0f255a64d759ce87cbf75be57d86b392 


Diff: https://reviews.apache.org/r/67777/diff/5/

Changes: https://reviews.apache.org/r/67777/diff/4-5/


Testing
-------

make check


Thanks,

Meng Zhu


Re: Review Request 67777: Added a helper to match agent-framework capabilities in the allocator.

Posted by Meng Zhu <mz...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/67777/
-----------------------------------------------------------

(Updated July 17, 2018, 1:35 p.m.)


Review request for mesos and Benjamin Mahler.


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


Repository: mesos


Description (updated)
-------

`isCapableOfReceivingAgent*(` checks if a framework
is capable of receiving resources on the agent based on
the framework capability.


Diffs (updated)
-----

  src/master/allocator/mesos/hierarchical.hpp 0f6c0e96a105c64465d3f5db4ff663d8fdfe7e26 
  src/master/allocator/mesos/hierarchical.cpp 1f1a73fc2308301cfed6ea1131e09fbf3393dfc7 


Diff: https://reviews.apache.org/r/67777/diff/4/

Changes: https://reviews.apache.org/r/67777/diff/3-4/


Testing
-------

make check


Thanks,

Meng Zhu


Re: Review Request 67777: Added a helper to match agent-framework capabilities in the allocator.

Posted by Meng Zhu <mz...@mesosphere.io>.

> On July 5, 2018, 2:33 p.m., Benjamin Mahler wrote:
> > src/master/allocator/mesos/hierarchical.hpp
> > Lines 666 (patched)
> > <https://reviews.apache.org/r/67777/diff/3/?file=2049465#file2049465line666>
> >
> >     Can you just take the Framework struct here? Feels a little odd that we take framework capabilities but not agent capabilities (I realize we can't take agent capabilities only, but seems like the caller doesn't need to know which bits of framework and slave structs we need?)

I want to make it clear that this filter function only considers the framework capability, not other things like suppressed roles, offer filters and etc. What do you think?


- Meng


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


On July 17, 2018, 1:35 p.m., Meng Zhu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67777/
> -----------------------------------------------------------
> 
> (Updated July 17, 2018, 1:35 p.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Bugs: MESOS-8916
>     https://issues.apache.org/jira/browse/MESOS-8916
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> `isCapableOfReceivingAgent*(` checks if a framework
> is capable of receiving resources on the agent based on
> the framework capability.
> 
> 
> Diffs
> -----
> 
>   src/master/allocator/mesos/hierarchical.hpp 0f6c0e96a105c64465d3f5db4ff663d8fdfe7e26 
>   src/master/allocator/mesos/hierarchical.cpp 1f1a73fc2308301cfed6ea1131e09fbf3393dfc7 
> 
> 
> Diff: https://reviews.apache.org/r/67777/diff/4/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Meng Zhu
> 
>


Re: Review Request 67777: Added a helper to match agent-framework capabilities in the allocator.

Posted by Benjamin Mahler <bm...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/67777/#review205766
-----------------------------------------------------------


Fix it, then Ship it!





src/master/allocator/mesos/hierarchical.hpp
Lines 663-664 (patched)
<https://reviews.apache.org/r/67777/#comment288656>

    Can you add a TODO to consider pulling this up into the framework struct and what's needed to make that happen?



src/master/allocator/mesos/hierarchical.hpp
Lines 665 (patched)
<https://reviews.apache.org/r/67777/#comment288657>

    How about `isCapableForAgent` or `isCapableOnAgent` or `isCapableOfReceivingAgent`?



src/master/allocator/mesos/hierarchical.hpp
Lines 666 (patched)
<https://reviews.apache.org/r/67777/#comment288654>

    Can you just take the Framework struct here? Feels a little odd that we take framework capabilities but not agent capabilities (I realize we can't take agent capabilities only, but seems like the caller doesn't need to know which bits of framework and slave structs we need?)


- Benjamin Mahler


On July 4, 2018, 12:19 a.m., Meng Zhu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67777/
> -----------------------------------------------------------
> 
> (Updated July 4, 2018, 12:19 a.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Bugs: MESOS-8916
>     https://issues.apache.org/jira/browse/MESOS-8916
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> `isFrameworkCapableReceivingAgent` checks if a framework
> is capable of receiving resources on the agent based on
> the framework capability.
> 
> 
> Diffs
> -----
> 
>   src/master/allocator/mesos/hierarchical.hpp 0f6c0e96a105c64465d3f5db4ff663d8fdfe7e26 
>   src/master/allocator/mesos/hierarchical.cpp 5a6cd3d2fc5bdbaaee2d05b9be9e83d4107c749b 
> 
> 
> Diff: https://reviews.apache.org/r/67777/diff/3/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Meng Zhu
> 
>


Re: Review Request 67777: Added a helper to match agent-framework capabilities in the allocator.

Posted by Meng Zhu <mz...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/67777/
-----------------------------------------------------------

(Updated July 3, 2018, 5:19 p.m.)


Review request for mesos and Benjamin Mahler.


Changes
-------

Added const.


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


Repository: mesos


Description
-------

`isFrameworkCapableReceivingAgent` checks if a framework
is capable of receiving resources on the agent based on
the framework capability.


Diffs (updated)
-----

  src/master/allocator/mesos/hierarchical.hpp 0f6c0e96a105c64465d3f5db4ff663d8fdfe7e26 
  src/master/allocator/mesos/hierarchical.cpp 5a6cd3d2fc5bdbaaee2d05b9be9e83d4107c749b 


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

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


Testing
-------

make check


Thanks,

Meng Zhu


Re: Review Request 67777: Added a helper to match agent-framework capabilities in the allocator.

Posted by Mesos Reviewbot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/67777/#review205604
-----------------------------------------------------------



Patch looks great!

Reviews applied: [67773, 67444, 67777]

Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' CONFIGURATION='--verbose --disable-libtool-wrappers' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker-build.sh

- Mesos Reviewbot


On June 28, 2018, 4 p.m., Meng Zhu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67777/
> -----------------------------------------------------------
> 
> (Updated June 28, 2018, 4 p.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Bugs: MESOS-8916
>     https://issues.apache.org/jira/browse/MESOS-8916
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> `isFrameworkCapableReceivingAgent` checks if a framework
> is capable of receiving resources on the agent based on
> the framework capability.
> 
> 
> Diffs
> -----
> 
>   src/master/allocator/mesos/hierarchical.hpp 0f6c0e96a105c64465d3f5db4ff663d8fdfe7e26 
>   src/master/allocator/mesos/hierarchical.cpp cbdfb2ba9c25755ac631557e0e7dbd721f861a4d 
> 
> 
> Diff: https://reviews.apache.org/r/67777/diff/2/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Meng Zhu
> 
>


Re: Review Request 67777: Added a helper to match agent-framework capabilities in the allocator.

Posted by Mesos Reviewbot Windows <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/67777/#review205603
-----------------------------------------------------------



PASS: Mesos patch 67777 was successfully built and tested.

Reviews applied: `['67773', '67444', '67777']`

All the build artifacts available at: http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/67777

- Mesos Reviewbot Windows


On June 28, 2018, 11 p.m., Meng Zhu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67777/
> -----------------------------------------------------------
> 
> (Updated June 28, 2018, 11 p.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Bugs: MESOS-8916
>     https://issues.apache.org/jira/browse/MESOS-8916
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> `isFrameworkCapableReceivingAgent` checks if a framework
> is capable of receiving resources on the agent based on
> the framework capability.
> 
> 
> Diffs
> -----
> 
>   src/master/allocator/mesos/hierarchical.hpp 0f6c0e96a105c64465d3f5db4ff663d8fdfe7e26 
>   src/master/allocator/mesos/hierarchical.cpp cbdfb2ba9c25755ac631557e0e7dbd721f861a4d 
> 
> 
> Diff: https://reviews.apache.org/r/67777/diff/2/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Meng Zhu
> 
>


Re: Review Request 67777: Added a helper to match agent-framework capabilities in the allocator.

Posted by Mesos Reviewbot Windows <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/67777/#review205542
-----------------------------------------------------------



PASS: Mesos patch 67777 was successfully built and tested.

Reviews applied: `['67773', '67444', '67777']`

All the build artifacts available at: http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/67777

- Mesos Reviewbot Windows


On June 28, 2018, 4 p.m., Meng Zhu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67777/
> -----------------------------------------------------------
> 
> (Updated June 28, 2018, 4 p.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Bugs: MESOS-8916
>     https://issues.apache.org/jira/browse/MESOS-8916
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> `isFrameworkCapableReceivingAgent` checks if the
> framework is capable of receiving resources on the given
> agent based on the agent and framework capabilities.
> 
> 
> Diffs
> -----
> 
>   src/master/allocator/mesos/hierarchical.hpp 0f6c0e96a105c64465d3f5db4ff663d8fdfe7e26 
>   src/master/allocator/mesos/hierarchical.cpp cbdfb2ba9c25755ac631557e0e7dbd721f861a4d 
> 
> 
> Diff: https://reviews.apache.org/r/67777/diff/1/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Meng Zhu
> 
>


Re: Review Request 67777: Added a helper to match agent-framework capabilities in the allocator.

Posted by Mesos Reviewbot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/67777/#review205558
-----------------------------------------------------------



Patch looks great!

Reviews applied: [67773, 67444, 67777]

Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' CONFIGURATION='--verbose --disable-libtool-wrappers' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker-build.sh

- Mesos Reviewbot


On June 28, 2018, 11 p.m., Meng Zhu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67777/
> -----------------------------------------------------------
> 
> (Updated June 28, 2018, 11 p.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Bugs: MESOS-8916
>     https://issues.apache.org/jira/browse/MESOS-8916
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> `isFrameworkCapableReceivingAgent` checks if the
> framework is capable of receiving resources on the given
> agent based on the agent and framework capabilities.
> 
> 
> Diffs
> -----
> 
>   src/master/allocator/mesos/hierarchical.hpp 0f6c0e96a105c64465d3f5db4ff663d8fdfe7e26 
>   src/master/allocator/mesos/hierarchical.cpp cbdfb2ba9c25755ac631557e0e7dbd721f861a4d 
> 
> 
> Diff: https://reviews.apache.org/r/67777/diff/1/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Meng Zhu
> 
>