You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Zhitao Li <zh...@gmail.com> on 2017/10/09 23:18:45 UTC

Re: Review Request 56722: Added a new operator API for `PRUNE_IMAGES`.

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

(Updated Oct. 9, 2017, 11:18 p.m.)


Review request for mesos, Gilbert Song, Jason Lai, and Jie Yu.


Changes
-------

Remove `excluded_images` for now since it's too experimental.


Summary (updated)
-----------------

Added a new operator API for `PRUNE_IMAGES`.


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


Repository: mesos


Description (updated)
-------

Added a new operator API for `PRUNE_IMAGES`.


Diffs (updated)
-----

  include/mesos/agent/agent.proto 7c8c8a7d8298e91e4e002327b3b27d4c74b5cbae 
  include/mesos/v1/agent/agent.proto 3e199124b23fa027232790d99370fe2f33660096 
  src/slave/http.hpp 44a95dec4c9b8bb65d712c5538bbd7afffe2cf7b 
  src/slave/http.cpp 3ea7829df8c1c35d2fa3a44f19a60b7e261042ce 
  src/slave/validation.cpp a575d88dffd2714447d7780a7433506a6e8c085f 


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

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


Testing
-------

1. New unit test to trigger the new call.
2. Manually tested that images previous pulled but not running can be purged through new operator API call while active images (running or being pulled) are not affected.


Thanks,

Zhitao Li


Re: Review Request 56722: Added a new operator API for `PRUNE_IMAGES`.

Posted by Gilbert Song <so...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/56722/#review191540
-----------------------------------------------------------


Fix it, then Ship it!





include/mesos/v1/agent/agent.proto
Lines 276-279 (patched)
<https://reviews.apache.org/r/56722/#comment269384>

    could we remove it for now? (add it back when we introduce the excluded list.


- Gilbert Song


On Nov. 17, 2017, 8:48 a.m., Zhitao Li wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56722/
> -----------------------------------------------------------
> 
> (Updated Nov. 17, 2017, 8:48 a.m.)
> 
> 
> Review request for mesos, Gilbert Song, Jason Lai, and Jie Yu.
> 
> 
> Bugs: MESOS-4945
>     https://issues.apache.org/jira/browse/MESOS-4945
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Added a new operator API for `PRUNE_IMAGES`.
> 
> 
> Diffs
> -----
> 
>   include/mesos/agent/agent.proto 0f92f73ba0f7729f0ba7cd89a692ab3685125e8b 
>   include/mesos/v1/agent/agent.proto 012ffef5f0dd7720fa95ae484c99479aaf256d7b 
>   src/slave/http.hpp a51831cdcebc1998ce6d4c3c19285e598a4ec9a3 
>   src/slave/http.cpp 394e91013dc11e0a79e2e00534864281cc74ad2f 
>   src/slave/validation.cpp 32781fd8f124f71e61744804aec3fe4da59a5df2 
> 
> 
> Diff: https://reviews.apache.org/r/56722/diff/7/
> 
> 
> Testing
> -------
> 
> 1. New unit test to trigger the new call.
> 2. Manually tested that images previous pulled but not running can be purged through new operator API call while active images (running or being pulled) are not affected.
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>


Re: Review Request 56722: Added a new operator API for `PRUNE_IMAGES`.

Posted by Gilbert Song <so...@gmail.com>.

> On Dec. 22, 2017, 3:55 p.m., Jie Yu wrote:
> > @gilbert, @zhitao, is this a 1.5.0 blocker? Do we want to land this?

yes, but we still have one small issue to address. I will cherrypick it to the 1.5.x branch. thanks!


- Gilbert


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


On Dec. 22, 2017, 12:10 p.m., Zhitao Li wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56722/
> -----------------------------------------------------------
> 
> (Updated Dec. 22, 2017, 12:10 p.m.)
> 
> 
> Review request for mesos, Gilbert Song, Jason Lai, and Jie Yu.
> 
> 
> Bugs: MESOS-4945
>     https://issues.apache.org/jira/browse/MESOS-4945
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Added a new operator API for `PRUNE_IMAGES`.
> 
> 
> Diffs
> -----
> 
>   include/mesos/agent/agent.proto 25af6dc64ca35126fd70dd10388bd2cea6dcbd36 
>   include/mesos/v1/agent/agent.proto 4ae9013e6fb27bdce72b4aa9fc9cbd99f79bc548 
>   src/slave/http.hpp 3cdbf988db923801e2b3d93cf60b33ad14653742 
>   src/slave/http.cpp 3e20c90fb94f0128a757745052534ef03612d849 
>   src/slave/validation.cpp b7243fc2316a69efa23285bf3394104e0d58759e 
> 
> 
> Diff: https://reviews.apache.org/r/56722/diff/9/
> 
> 
> Testing
> -------
> 
> 1. New unit test to trigger the new call.
> 2. Manually tested that images previous pulled but not running can be purged through new operator API call while active images (running or being pulled) are not affected.
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>


Re: Review Request 56722: Added a new operator API for `PRUNE_IMAGES`.

Posted by Jie Yu <yu...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/56722/#review194466
-----------------------------------------------------------



@gilbert, @zhitao, is this a 1.5.0 blocker? Do we want to land this?

- Jie Yu


On Dec. 22, 2017, 8:10 p.m., Zhitao Li wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56722/
> -----------------------------------------------------------
> 
> (Updated Dec. 22, 2017, 8:10 p.m.)
> 
> 
> Review request for mesos, Gilbert Song, Jason Lai, and Jie Yu.
> 
> 
> Bugs: MESOS-4945
>     https://issues.apache.org/jira/browse/MESOS-4945
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Added a new operator API for `PRUNE_IMAGES`.
> 
> 
> Diffs
> -----
> 
>   include/mesos/agent/agent.proto 25af6dc64ca35126fd70dd10388bd2cea6dcbd36 
>   include/mesos/v1/agent/agent.proto 4ae9013e6fb27bdce72b4aa9fc9cbd99f79bc548 
>   src/slave/http.hpp 3cdbf988db923801e2b3d93cf60b33ad14653742 
>   src/slave/http.cpp 3e20c90fb94f0128a757745052534ef03612d849 
>   src/slave/validation.cpp b7243fc2316a69efa23285bf3394104e0d58759e 
> 
> 
> Diff: https://reviews.apache.org/r/56722/diff/9/
> 
> 
> Testing
> -------
> 
> 1. New unit test to trigger the new call.
> 2. Manually tested that images previous pulled but not running can be purged through new operator API call while active images (running or being pulled) are not affected.
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>


Re: Review Request 56722: Added a new operator API for `PRUNE_IMAGES`.

Posted by Gilbert Song <so...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/56722/#review194533
-----------------------------------------------------------


Fix it, then Ship it!





include/mesos/agent/agent.proto
Lines 97 (patched)
<https://reviews.apache.org/r/56722/#comment273352>

    s/docker/container/g



include/mesos/v1/agent/agent.proto
Lines 97 (patched)
<https://reviews.apache.org/r/56722/#comment273353>

    ditto.


- Gilbert Song


On Dec. 24, 2017, 11:31 a.m., Zhitao Li wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56722/
> -----------------------------------------------------------
> 
> (Updated Dec. 24, 2017, 11:31 a.m.)
> 
> 
> Review request for mesos, Gilbert Song, Jason Lai, and Jie Yu.
> 
> 
> Bugs: MESOS-4945
>     https://issues.apache.org/jira/browse/MESOS-4945
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Added a new operator API for `PRUNE_IMAGES`.
> 
> 
> Diffs
> -----
> 
>   include/mesos/agent/agent.proto 25af6dc64ca35126fd70dd10388bd2cea6dcbd36 
>   include/mesos/v1/agent/agent.proto 4ae9013e6fb27bdce72b4aa9fc9cbd99f79bc548 
>   src/slave/http.hpp 3cdbf988db923801e2b3d93cf60b33ad14653742 
>   src/slave/http.cpp 3e20c90fb94f0128a757745052534ef03612d849 
>   src/slave/validation.cpp b7243fc2316a69efa23285bf3394104e0d58759e 
> 
> 
> Diff: https://reviews.apache.org/r/56722/diff/9/
> 
> 
> Testing
> -------
> 
> 1. New unit test to trigger the new call.
> 2. Manually tested that images previous pulled but not running can be purged through new operator API call while active images (running or being pulled) are not affected.
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>


Re: Review Request 56722: Added a new operator API for `PRUNE_IMAGES`.

Posted by Zhitao Li <zh...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/56722/#review194516
-----------------------------------------------------------




src/slave/http.cpp
Lines 2452 (patched)
<https://reviews.apache.org/r/56722/#comment273307>

    In your scenario, if the operator wants to persist the excluded_images selection while auto gc is enabled, requiring it to be configured in the agent flag seems reasonable to me.
    
    I think this API is useful for opertors who cannot use auto gc for whatever reason (i.e, disk space calculation is not applicable), so I don't see it is a real problem.
    
    Keeping this field out seems a strange discrepancy between the operator API and agent flag.


- Zhitao Li


On Dec. 24, 2017, 7:31 p.m., Zhitao Li wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56722/
> -----------------------------------------------------------
> 
> (Updated Dec. 24, 2017, 7:31 p.m.)
> 
> 
> Review request for mesos, Gilbert Song, Jason Lai, and Jie Yu.
> 
> 
> Bugs: MESOS-4945
>     https://issues.apache.org/jira/browse/MESOS-4945
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Added a new operator API for `PRUNE_IMAGES`.
> 
> 
> Diffs
> -----
> 
>   include/mesos/agent/agent.proto 25af6dc64ca35126fd70dd10388bd2cea6dcbd36 
>   include/mesos/v1/agent/agent.proto 4ae9013e6fb27bdce72b4aa9fc9cbd99f79bc548 
>   src/slave/http.hpp 3cdbf988db923801e2b3d93cf60b33ad14653742 
>   src/slave/http.cpp 3e20c90fb94f0128a757745052534ef03612d849 
>   src/slave/validation.cpp b7243fc2316a69efa23285bf3394104e0d58759e 
> 
> 
> Diff: https://reviews.apache.org/r/56722/diff/9/
> 
> 
> Testing
> -------
> 
> 1. New unit test to trigger the new call.
> 2. Manually tested that images previous pulled but not running can be purged through new operator API call while active images (running or being pulled) are not affected.
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>


Re: Review Request 56722: Added a new operator API for `PRUNE_IMAGES`.

Posted by Zhitao Li <zh...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/56722/
-----------------------------------------------------------

(Updated Dec. 24, 2017, 7:31 p.m.)


Review request for mesos, Gilbert Song, Jason Lai, and Jie Yu.


Changes
-------

Remove unnecessary patch dependancy.


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


Repository: mesos


Description
-------

Added a new operator API for `PRUNE_IMAGES`.


Diffs
-----

  include/mesos/agent/agent.proto 25af6dc64ca35126fd70dd10388bd2cea6dcbd36 
  include/mesos/v1/agent/agent.proto 4ae9013e6fb27bdce72b4aa9fc9cbd99f79bc548 
  src/slave/http.hpp 3cdbf988db923801e2b3d93cf60b33ad14653742 
  src/slave/http.cpp 3e20c90fb94f0128a757745052534ef03612d849 
  src/slave/validation.cpp b7243fc2316a69efa23285bf3394104e0d58759e 


Diff: https://reviews.apache.org/r/56722/diff/9/


Testing
-------

1. New unit test to trigger the new call.
2. Manually tested that images previous pulled but not running can be purged through new operator API call while active images (running or being pulled) are not affected.


Thanks,

Zhitao Li


Re: Review Request 56722: Added a new operator API for `PRUNE_IMAGES`.

Posted by Gilbert Song <so...@gmail.com>.

> On Dec. 22, 2017, 7:34 p.m., Gilbert Song wrote:
> > src/slave/http.cpp
> > Lines 2452 (patched)
> > <https://reviews.apache.org/r/56722/diff/9/?file=1926989#file1926989line2452>
> >
> >     As we discussed offline, we should follow the semantic that if there are excluded images from the agent flag, they should have higher priority.
> 
> Zhitao Li wrote:
>     Fixed in r/64812

After reviewing r/64812, is `excluded_images` in the API very necessary? I am asking due to this scenario:

let say if an operator enables auto gc (with default excluded images in image_gc_config), and he wants to trigger GC by the operator API with another set of excluded images, which means that it is possible that some of the `excluded_images` may not persist (cleaned up by another auto-triggered GC).

I feel a little strange about that. Could we just have the `PRUNE_IMAGES` call for now?


- Gilbert


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


On Dec. 24, 2017, 11:31 a.m., Zhitao Li wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56722/
> -----------------------------------------------------------
> 
> (Updated Dec. 24, 2017, 11:31 a.m.)
> 
> 
> Review request for mesos, Gilbert Song, Jason Lai, and Jie Yu.
> 
> 
> Bugs: MESOS-4945
>     https://issues.apache.org/jira/browse/MESOS-4945
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Added a new operator API for `PRUNE_IMAGES`.
> 
> 
> Diffs
> -----
> 
>   include/mesos/agent/agent.proto 25af6dc64ca35126fd70dd10388bd2cea6dcbd36 
>   include/mesos/v1/agent/agent.proto 4ae9013e6fb27bdce72b4aa9fc9cbd99f79bc548 
>   src/slave/http.hpp 3cdbf988db923801e2b3d93cf60b33ad14653742 
>   src/slave/http.cpp 3e20c90fb94f0128a757745052534ef03612d849 
>   src/slave/validation.cpp b7243fc2316a69efa23285bf3394104e0d58759e 
> 
> 
> Diff: https://reviews.apache.org/r/56722/diff/9/
> 
> 
> Testing
> -------
> 
> 1. New unit test to trigger the new call.
> 2. Manually tested that images previous pulled but not running can be purged through new operator API call while active images (running or being pulled) are not affected.
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>


Re: Review Request 56722: Added a new operator API for `PRUNE_IMAGES`.

Posted by Zhitao Li <zh...@gmail.com>.

> On Dec. 23, 2017, 3:34 a.m., Gilbert Song wrote:
> > src/slave/http.cpp
> > Lines 2452 (patched)
> > <https://reviews.apache.org/r/56722/diff/9/?file=1926989#file1926989line2452>
> >
> >     As we discussed offline, we should follow the semantic that if there are excluded images from the agent flag, they should have higher priority.

Fixed in r/64812


- Zhitao


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


On Dec. 24, 2017, 7:31 p.m., Zhitao Li wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56722/
> -----------------------------------------------------------
> 
> (Updated Dec. 24, 2017, 7:31 p.m.)
> 
> 
> Review request for mesos, Gilbert Song, Jason Lai, and Jie Yu.
> 
> 
> Bugs: MESOS-4945
>     https://issues.apache.org/jira/browse/MESOS-4945
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Added a new operator API for `PRUNE_IMAGES`.
> 
> 
> Diffs
> -----
> 
>   include/mesos/agent/agent.proto 25af6dc64ca35126fd70dd10388bd2cea6dcbd36 
>   include/mesos/v1/agent/agent.proto 4ae9013e6fb27bdce72b4aa9fc9cbd99f79bc548 
>   src/slave/http.hpp 3cdbf988db923801e2b3d93cf60b33ad14653742 
>   src/slave/http.cpp 3e20c90fb94f0128a757745052534ef03612d849 
>   src/slave/validation.cpp b7243fc2316a69efa23285bf3394104e0d58759e 
> 
> 
> Diff: https://reviews.apache.org/r/56722/diff/9/
> 
> 
> Testing
> -------
> 
> 1. New unit test to trigger the new call.
> 2. Manually tested that images previous pulled but not running can be purged through new operator API call while active images (running or being pulled) are not affected.
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>


Re: Review Request 56722: Added a new operator API for `PRUNE_IMAGES`.

Posted by Gilbert Song <so...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/56722/#review194469
-----------------------------------------------------------




src/slave/http.cpp
Lines 2452 (patched)
<https://reviews.apache.org/r/56722/#comment273237>

    As we discussed offline, we should follow the semantic that if there are excluded images from the agent flag, they should have higher priority.


- Gilbert Song


On Dec. 22, 2017, 12:10 p.m., Zhitao Li wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56722/
> -----------------------------------------------------------
> 
> (Updated Dec. 22, 2017, 12:10 p.m.)
> 
> 
> Review request for mesos, Gilbert Song, Jason Lai, and Jie Yu.
> 
> 
> Bugs: MESOS-4945
>     https://issues.apache.org/jira/browse/MESOS-4945
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Added a new operator API for `PRUNE_IMAGES`.
> 
> 
> Diffs
> -----
> 
>   include/mesos/agent/agent.proto 25af6dc64ca35126fd70dd10388bd2cea6dcbd36 
>   include/mesos/v1/agent/agent.proto 4ae9013e6fb27bdce72b4aa9fc9cbd99f79bc548 
>   src/slave/http.hpp 3cdbf988db923801e2b3d93cf60b33ad14653742 
>   src/slave/http.cpp 3e20c90fb94f0128a757745052534ef03612d849 
>   src/slave/validation.cpp b7243fc2316a69efa23285bf3394104e0d58759e 
> 
> 
> Diff: https://reviews.apache.org/r/56722/diff/9/
> 
> 
> Testing
> -------
> 
> 1. New unit test to trigger the new call.
> 2. Manually tested that images previous pulled but not running can be purged through new operator API call while active images (running or being pulled) are not affected.
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>


Re: Review Request 56722: Added a new operator API for `PRUNE_IMAGES`.

Posted by Zhitao Li <zh...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/56722/
-----------------------------------------------------------

(Updated Dec. 22, 2017, 8:10 p.m.)


Review request for mesos, Gilbert Song, Jason Lai, and Jie Yu.


Changes
-------

Rebase.


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


Repository: mesos


Description
-------

Added a new operator API for `PRUNE_IMAGES`.


Diffs (updated)
-----

  include/mesos/agent/agent.proto 25af6dc64ca35126fd70dd10388bd2cea6dcbd36 
  include/mesos/v1/agent/agent.proto 4ae9013e6fb27bdce72b4aa9fc9cbd99f79bc548 
  src/slave/http.hpp 3cdbf988db923801e2b3d93cf60b33ad14653742 
  src/slave/http.cpp 3e20c90fb94f0128a757745052534ef03612d849 
  src/slave/validation.cpp b7243fc2316a69efa23285bf3394104e0d58759e 


Diff: https://reviews.apache.org/r/56722/diff/9/

Changes: https://reviews.apache.org/r/56722/diff/8-9/


Testing
-------

1. New unit test to trigger the new call.
2. Manually tested that images previous pulled but not running can be purged through new operator API call while active images (running or being pulled) are not affected.


Thanks,

Zhitao Li


Re: Review Request 56722: Added a new operator API for `PRUNE_IMAGES`.

Posted by Zhitao Li <zh...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/56722/
-----------------------------------------------------------

(Updated Nov. 22, 2017, 10:50 p.m.)


Review request for mesos, Gilbert Song, Jason Lai, and Jie Yu.


Changes
-------

Remove unused message.


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


Repository: mesos


Description
-------

Added a new operator API for `PRUNE_IMAGES`.


Diffs (updated)
-----

  include/mesos/agent/agent.proto 0f92f73ba0f7729f0ba7cd89a692ab3685125e8b 
  include/mesos/v1/agent/agent.proto 012ffef5f0dd7720fa95ae484c99479aaf256d7b 
  src/slave/http.hpp a51831cdcebc1998ce6d4c3c19285e598a4ec9a3 
  src/slave/http.cpp 394e91013dc11e0a79e2e00534864281cc74ad2f 
  src/slave/validation.cpp 32781fd8f124f71e61744804aec3fe4da59a5df2 


Diff: https://reviews.apache.org/r/56722/diff/8/

Changes: https://reviews.apache.org/r/56722/diff/7-8/


Testing
-------

1. New unit test to trigger the new call.
2. Manually tested that images previous pulled but not running can be purged through new operator API call while active images (running or being pulled) are not affected.


Thanks,

Zhitao Li


Re: Review Request 56722: Added a new operator API for `PRUNE_IMAGES`.

Posted by Zhitao Li <zh...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/56722/
-----------------------------------------------------------

(Updated Nov. 17, 2017, 4:48 p.m.)


Review request for mesos, Gilbert Song, Jason Lai, and Jie Yu.


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


Repository: mesos


Description
-------

Added a new operator API for `PRUNE_IMAGES`.


Diffs (updated)
-----

  include/mesos/agent/agent.proto 0f92f73ba0f7729f0ba7cd89a692ab3685125e8b 
  include/mesos/v1/agent/agent.proto 012ffef5f0dd7720fa95ae484c99479aaf256d7b 
  src/slave/http.hpp a51831cdcebc1998ce6d4c3c19285e598a4ec9a3 
  src/slave/http.cpp 394e91013dc11e0a79e2e00534864281cc74ad2f 
  src/slave/validation.cpp 32781fd8f124f71e61744804aec3fe4da59a5df2 


Diff: https://reviews.apache.org/r/56722/diff/7/

Changes: https://reviews.apache.org/r/56722/diff/6-7/


Testing
-------

1. New unit test to trigger the new call.
2. Manually tested that images previous pulled but not running can be purged through new operator API call while active images (running or being pulled) are not affected.


Thanks,

Zhitao Li


Re: Review Request 56722: Added a new operator API for `PRUNE_IMAGES`.

Posted by Zhitao Li <zh...@gmail.com>.

> On Nov. 17, 2017, 1:24 a.m., Gilbert Song wrote:
> > src/slave/http.cpp
> > Lines 2302-2304 (patched)
> > <https://reviews.apache.org/r/56722/diff/6/?file=1894077#file1894077line2302>
> >
> >     one thing to notice is that the current interface returns `Future<Nothing>`, so we dont have a way to distringuish a `failure` or `user need to drain all containers`. maybe add a TODO?

I think the error message returned in response body as well as logged here should be sufficient to distinguish.


- Zhitao


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


On Nov. 17, 2017, 4:48 p.m., Zhitao Li wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56722/
> -----------------------------------------------------------
> 
> (Updated Nov. 17, 2017, 4:48 p.m.)
> 
> 
> Review request for mesos, Gilbert Song, Jason Lai, and Jie Yu.
> 
> 
> Bugs: MESOS-4945
>     https://issues.apache.org/jira/browse/MESOS-4945
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Added a new operator API for `PRUNE_IMAGES`.
> 
> 
> Diffs
> -----
> 
>   include/mesos/agent/agent.proto 0f92f73ba0f7729f0ba7cd89a692ab3685125e8b 
>   include/mesos/v1/agent/agent.proto 012ffef5f0dd7720fa95ae484c99479aaf256d7b 
>   src/slave/http.hpp a51831cdcebc1998ce6d4c3c19285e598a4ec9a3 
>   src/slave/http.cpp 394e91013dc11e0a79e2e00534864281cc74ad2f 
>   src/slave/validation.cpp 32781fd8f124f71e61744804aec3fe4da59a5df2 
> 
> 
> Diff: https://reviews.apache.org/r/56722/diff/7/
> 
> 
> Testing
> -------
> 
> 1. New unit test to trigger the new call.
> 2. Manually tested that images previous pulled but not running can be purged through new operator API call while active images (running or being pulled) are not affected.
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>


Re: Review Request 56722: Added a new operator API for `PRUNE_IMAGES`.

Posted by Zhitao Li <zh...@gmail.com>.

> On Nov. 17, 2017, 1:24 a.m., Gilbert Song wrote:
> > src/slave/http.cpp
> > Lines 2299 (patched)
> > <https://reviews.apache.org/r/56722/diff/6/?file=1894077#file1894077line2299>
> >
> >     instead of passing in the `acceptType`, we might want to pass the containerId and construct the error msg with the containerID.

Sorry I don't get it: there is no containerID here?


- Zhitao


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


On Nov. 17, 2017, 4:48 p.m., Zhitao Li wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56722/
> -----------------------------------------------------------
> 
> (Updated Nov. 17, 2017, 4:48 p.m.)
> 
> 
> Review request for mesos, Gilbert Song, Jason Lai, and Jie Yu.
> 
> 
> Bugs: MESOS-4945
>     https://issues.apache.org/jira/browse/MESOS-4945
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Added a new operator API for `PRUNE_IMAGES`.
> 
> 
> Diffs
> -----
> 
>   include/mesos/agent/agent.proto 0f92f73ba0f7729f0ba7cd89a692ab3685125e8b 
>   include/mesos/v1/agent/agent.proto 012ffef5f0dd7720fa95ae484c99479aaf256d7b 
>   src/slave/http.hpp a51831cdcebc1998ce6d4c3c19285e598a4ec9a3 
>   src/slave/http.cpp 394e91013dc11e0a79e2e00534864281cc74ad2f 
>   src/slave/validation.cpp 32781fd8f124f71e61744804aec3fe4da59a5df2 
> 
> 
> Diff: https://reviews.apache.org/r/56722/diff/7/
> 
> 
> Testing
> -------
> 
> 1. New unit test to trigger the new call.
> 2. Manually tested that images previous pulled but not running can be purged through new operator API call while active images (running or being pulled) are not affected.
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>


Re: Review Request 56722: Added a new operator API for `PRUNE_IMAGES`.

Posted by Gilbert Song <so...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/56722/#review191277
-----------------------------------------------------------




src/slave/http.hpp
Lines 306 (patched)
<https://reviews.apache.org/r/56722/#comment269014>

    why isn't it a `ContentType acceptType,`?



src/slave/http.cpp
Lines 2296 (patched)
<https://reviews.apache.org/r/56722/#comment269019>

    why do you need it?



src/slave/http.cpp
Lines 2299 (patched)
<https://reviews.apache.org/r/56722/#comment269020>

    instead of passing in the `acceptType`, we might want to pass the containerId and construct the error msg with the containerID.



src/slave/http.cpp
Lines 2301 (patched)
<https://reviews.apache.org/r/56722/#comment269035>

    could we add more log? (like `Http::_killContainer()`)



src/slave/http.cpp
Lines 2302-2304 (patched)
<https://reviews.apache.org/r/56722/#comment269038>

    one thing to notice is that the current interface returns `Future<Nothing>`, so we dont have a way to distringuish a `failure` or `user need to drain all containers`. maybe add a TODO?


- Gilbert Song


On Nov. 15, 2017, 8:03 p.m., Zhitao Li wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56722/
> -----------------------------------------------------------
> 
> (Updated Nov. 15, 2017, 8:03 p.m.)
> 
> 
> Review request for mesos, Gilbert Song, Jason Lai, and Jie Yu.
> 
> 
> Bugs: MESOS-4945
>     https://issues.apache.org/jira/browse/MESOS-4945
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Added a new operator API for `PRUNE_IMAGES`.
> 
> 
> Diffs
> -----
> 
>   include/mesos/agent/agent.proto 0f92f73ba0f7729f0ba7cd89a692ab3685125e8b 
>   include/mesos/v1/agent/agent.proto 012ffef5f0dd7720fa95ae484c99479aaf256d7b 
>   src/slave/http.hpp a51831cdcebc1998ce6d4c3c19285e598a4ec9a3 
>   src/slave/http.cpp 394e91013dc11e0a79e2e00534864281cc74ad2f 
>   src/slave/validation.cpp 32781fd8f124f71e61744804aec3fe4da59a5df2 
> 
> 
> Diff: https://reviews.apache.org/r/56722/diff/6/
> 
> 
> Testing
> -------
> 
> 1. New unit test to trigger the new call.
> 2. Manually tested that images previous pulled but not running can be purged through new operator API call while active images (running or being pulled) are not affected.
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>


Re: Review Request 56722: Added a new operator API for `PRUNE_IMAGES`.

Posted by Zhitao Li <zh...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/56722/
-----------------------------------------------------------

(Updated Nov. 16, 2017, 4:03 a.m.)


Review request for mesos, Gilbert Song, Jason Lai, and Jie Yu.


Changes
-------

Rebase after standalone container support.


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


Repository: mesos


Description
-------

Added a new operator API for `PRUNE_IMAGES`.


Diffs (updated)
-----

  include/mesos/agent/agent.proto 0f92f73ba0f7729f0ba7cd89a692ab3685125e8b 
  include/mesos/v1/agent/agent.proto 012ffef5f0dd7720fa95ae484c99479aaf256d7b 
  src/slave/http.hpp a51831cdcebc1998ce6d4c3c19285e598a4ec9a3 
  src/slave/http.cpp 394e91013dc11e0a79e2e00534864281cc74ad2f 
  src/slave/validation.cpp 32781fd8f124f71e61744804aec3fe4da59a5df2 


Diff: https://reviews.apache.org/r/56722/diff/6/

Changes: https://reviews.apache.org/r/56722/diff/5-6/


Testing
-------

1. New unit test to trigger the new call.
2. Manually tested that images previous pulled but not running can be purged through new operator API call while active images (running or being pulled) are not affected.


Thanks,

Zhitao Li