You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Gilbert Song <so...@gmail.com> on 2017/12/02 01:10:13 UTC

Review Request 64266: Added an optional agent flag '--image_gc_config'.

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

Review request for mesos, Jie Yu, Qian Zhang, Vinod Kone, and Zhitao Li.


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


Repository: mesos


Description
-------

This is an optional agent flag. If it is not set, it means
the automatic container image gc is not enabled. Users have
to trigger image gc manually via the operator API. If it is
set, the image auto gc is enabled.


Diffs
-----

  docs/configuration/agent.md f1e0681be34d194d66e2b2ecd3f76653a54068e2 
  src/messages/flags.hpp 00e26cd2699a3da9722170324b5c0190850405da 
  src/slave/flags.hpp 0c02b49c054ef2597298aefe0997beacfc9efeb8 
  src/slave/flags.cpp 0eeecdc6023443a99c8e8fe29c2f7bf791c0a36e 


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


Testing
-------

make check


Thanks,

Gilbert Song


Re: Review Request 64266: Added an optional agent flag '--image_gc_config'.

Posted by Qian Zhang <zh...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/64266/#review192618
-----------------------------------------------------------


Fix it, then Ship it!





docs/configuration/agent.md
Lines 882 (patched)
<https://reviews.apache.org/r/64266/#comment270880>

    Since it can be a file or a string, I would suggest to change it from `JSON-formatted config file` to `JSON-formatted configuration`.
    
    And do we want to mention it can only work with Mesos containerizer?



docs/configuration/agent.md
Lines 886 (patched)
<https://reviews.apache.org/r/64266/#comment270881>

    s/image auto gc/auto image gc/



docs/configuration/agent.md
Lines 889 (patched)
<https://reviews.apache.org/r/64266/#comment270882>

    s/details/the expected format/



src/slave/flags.cpp
Lines 159-166 (patched)
<https://reviews.apache.org/r/64266/#comment270884>

    Ditto for my comments in agent.md.


- Qian Zhang


On Dec. 2, 2017, 9:10 a.m., Gilbert Song wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64266/
> -----------------------------------------------------------
> 
> (Updated Dec. 2, 2017, 9:10 a.m.)
> 
> 
> Review request for mesos, Jie Yu, Qian Zhang, Vinod Kone, and Zhitao Li.
> 
> 
> Bugs: MESOS-8294
>     https://issues.apache.org/jira/browse/MESOS-8294
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This is an optional agent flag. If it is not set, it means
> the automatic container image gc is not enabled. Users have
> to trigger image gc manually via the operator API. If it is
> set, the image auto gc is enabled.
> 
> 
> Diffs
> -----
> 
>   docs/configuration/agent.md f1e0681be34d194d66e2b2ecd3f76653a54068e2 
>   src/messages/flags.hpp 00e26cd2699a3da9722170324b5c0190850405da 
>   src/slave/flags.hpp 0c02b49c054ef2597298aefe0997beacfc9efeb8 
>   src/slave/flags.cpp 0eeecdc6023443a99c8e8fe29c2f7bf791c0a36e 
> 
> 
> Diff: https://reviews.apache.org/r/64266/diff/1/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>


Re: Review Request 64266: Added an optional agent flag '--image_gc_config'.

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

> On Dec. 3, 2017, 2:16 p.m., Zhitao Li wrote:
> > docs/configuration/agent.md
> > Lines 886-887 (patched)
> > <https://reviews.apache.org/r/64266/diff/1/?file=1906400#file1906400line886>
> >
> >     I would suggest only support files with support of refreshing its content without agent reboot.
> >     
> >     From operator perspective, the frequency in which we want to update excluded_images is often higher than permitted agent restart, and they often have different cycles.

As we chatted offline. This relates to the feature of supporting agent/master configuration update dynamically. I added a `TODO` in the agent code.


- Gilbert


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


On Dec. 1, 2017, 5:10 p.m., Gilbert Song wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64266/
> -----------------------------------------------------------
> 
> (Updated Dec. 1, 2017, 5:10 p.m.)
> 
> 
> Review request for mesos, Jie Yu, Qian Zhang, Vinod Kone, and Zhitao Li.
> 
> 
> Bugs: MESOS-8294
>     https://issues.apache.org/jira/browse/MESOS-8294
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This is an optional agent flag. If it is not set, it means
> the automatic container image gc is not enabled. Users have
> to trigger image gc manually via the operator API. If it is
> set, the image auto gc is enabled.
> 
> 
> Diffs
> -----
> 
>   docs/configuration/agent.md f1e0681be34d194d66e2b2ecd3f76653a54068e2 
>   src/messages/flags.hpp 00e26cd2699a3da9722170324b5c0190850405da 
>   src/slave/flags.hpp 0c02b49c054ef2597298aefe0997beacfc9efeb8 
>   src/slave/flags.cpp 0eeecdc6023443a99c8e8fe29c2f7bf791c0a36e 
> 
> 
> Diff: https://reviews.apache.org/r/64266/diff/1/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>


Re: Review Request 64266: Added an optional agent flag '--image_gc_config'.

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




docs/configuration/agent.md
Lines 886-887 (patched)
<https://reviews.apache.org/r/64266/#comment270910>

    I would suggest only support files with support of refreshing its content without agent reboot.
    
    From operator perspective, the frequency in which we want to update excluded_images is often higher than permitted agent restart, and they often have different cycles.


- Zhitao Li


On Dec. 2, 2017, 1:10 a.m., Gilbert Song wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64266/
> -----------------------------------------------------------
> 
> (Updated Dec. 2, 2017, 1:10 a.m.)
> 
> 
> Review request for mesos, Jie Yu, Qian Zhang, Vinod Kone, and Zhitao Li.
> 
> 
> Bugs: MESOS-8294
>     https://issues.apache.org/jira/browse/MESOS-8294
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This is an optional agent flag. If it is not set, it means
> the automatic container image gc is not enabled. Users have
> to trigger image gc manually via the operator API. If it is
> set, the image auto gc is enabled.
> 
> 
> Diffs
> -----
> 
>   docs/configuration/agent.md f1e0681be34d194d66e2b2ecd3f76653a54068e2 
>   src/messages/flags.hpp 00e26cd2699a3da9722170324b5c0190850405da 
>   src/slave/flags.hpp 0c02b49c054ef2597298aefe0997beacfc9efeb8 
>   src/slave/flags.cpp 0eeecdc6023443a99c8e8fe29c2f7bf791c0a36e 
> 
> 
> Diff: https://reviews.apache.org/r/64266/diff/1/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>


Re: Review Request 64266: Added an optional agent flag '--image_gc_config'.

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


Ship it!




Ship It!

- Zhitao Li


On Dec. 6, 2017, 7:09 p.m., Gilbert Song wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64266/
> -----------------------------------------------------------
> 
> (Updated Dec. 6, 2017, 7:09 p.m.)
> 
> 
> Review request for mesos, Jie Yu, Qian Zhang, Vinod Kone, and Zhitao Li.
> 
> 
> Bugs: MESOS-8294
>     https://issues.apache.org/jira/browse/MESOS-8294
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This is an optional agent flag. If it is not set, it means
> the automatic container image gc is not enabled. Users have
> to trigger image gc manually via the operator API. If it is
> set, the image auto gc is enabled.
> 
> 
> Diffs
> -----
> 
>   docs/configuration/agent.md 83b08235a9e8022f03649d57a5d28f229afac5e3 
>   src/messages/flags.hpp 00e26cd2699a3da9722170324b5c0190850405da 
>   src/slave/flags.hpp f84ba5a3ff0f9de879ac340cb77540c1a480a43b 
>   src/slave/flags.cpp d8764745e6aca81283d8b96388df1320c3465952 
> 
> 
> Diff: https://reviews.apache.org/r/64266/diff/2/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>


Re: Review Request 64266: Added an optional agent flag '--image_gc_config'.

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

(Updated Dec. 6, 2017, 11:09 a.m.)


Review request for mesos, Jie Yu, Qian Zhang, Vinod Kone, and Zhitao Li.


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


Repository: mesos


Description
-------

This is an optional agent flag. If it is not set, it means
the automatic container image gc is not enabled. Users have
to trigger image gc manually via the operator API. If it is
set, the image auto gc is enabled.


Diffs (updated)
-----

  docs/configuration/agent.md 83b08235a9e8022f03649d57a5d28f229afac5e3 
  src/messages/flags.hpp 00e26cd2699a3da9722170324b5c0190850405da 
  src/slave/flags.hpp f84ba5a3ff0f9de879ac340cb77540c1a480a43b 
  src/slave/flags.cpp d8764745e6aca81283d8b96388df1320c3465952 


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

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


Testing
-------

make check


Thanks,

Gilbert Song