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/12/22 20:10:10 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 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 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
>
>