> On March 2, 2016, 4:07 p.m., Avinash sridharan wrote:
> > docs/endpoints/slave/state.json.md, line 91
> > <https://reviews.apache.org/r/44200/diff/1/?file=1275003#file1275003line91>
> >
> >     Do we need to change some install scripts to create these paths? I 
> > think we should leave the paths as empty. Giving a default path implies 
> > that the path should exist during mesos installation. Leaving the path 
> > empty would force the operator to specify it while launching `network/cni` 
> > isolator.
> 
> Qian Zhang wrote:
>     I think operator will be forced to specify it if the default path 
> `/tmp/mesos/cni/plugins` does not exist because there is a check in 
> `NetworkCniIsolatorProcess::create()`, please see 
> https://reviews.apache.org/r/44269/ for details, if the default path does not 
> exist, the agent will eventually exit with a meaningful error message to 
> operator.

Firstly having an install path for plugins in /tmp doesn't make sense. /tmp is 
usually mounted as tmpfs (ramfs) so its not going to be persistent across 
reboots.  Also, if we are not creating /tmp/mesos/cni/plugins then by default 
this path is not going to exist, so having a default path here does not make 
sense. Lets keep the default path as empty, and as you mentioned, let the 
isolator bail out if the path is not specified.


> On March 2, 2016, 4:07 p.m., Avinash sridharan wrote:
> > src/slave/flags.cpp, line 694
> > <https://reviews.apache.org/r/44200/diff/1/?file=1275006#file1275006line694>
> >
> >     s/directory/location
> >     
> >     remove this line:
> >     This flag is used for\n"
> >           "the `network/cni` isolator.\n",
> >           "/tmp/mesos/cni/plugins"
> 
> Qian Zhang wrote:
>     Did you mean removing the line: `This flag is used for the 'network/cni' 
> isolator.`? I think this is the existing convention, please see the help 
> message of `Flags::eth0_name`, `Flags::network_enable_snmp_statistics`, 
> `Flags::container_disk_watch_interval`, etc. They all have the line like: 
> `This flag is used for the 'xxx' isolator.`.

Not sure what you mean by convention in help strings? The sentence is redundant 
(doesn't given any more information than the previous sentences). Lets use the 
correct form here.  Help strings should be concise.


- Avinash


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


On March 1, 2016, 7:04 a.m., Qian Zhang wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44200/
> -----------------------------------------------------------
> 
> (Updated March 1, 2016, 7:04 a.m.)
> 
> 
> Review request for mesos, Avinash sridharan, Gilbert Song, and Jie Yu.
> 
> 
> Bugs: MESOS-4761
>     https://issues.apache.org/jira/browse/MESOS-4761
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Add agent flags for specifying CNI plugin and config directories.
> 
> 
> Diffs
> -----
> 
>   docs/configuration.md c4d094aac4acb95fd6648071413a3d5160dbf381 
>   docs/endpoints/slave/state.json.md a6403eea3426bdec81d170d0829ffe7174862e55 
>   docs/endpoints/slave/state.md bdd555e0231935c668dea1ba63582348db2b3629 
>   src/slave/flags.hpp c07932157ffe36ea36f1def4c7d92b79d2219c48 
>   src/slave/flags.cpp 7f139e99e8392fe95bb30070896ed9d95dfe5701 
>   src/slave/http.cpp 4eb1fafdfa72094511b0b2684a3c2705bd8c7c5e 
> 
> Diff: https://reviews.apache.org/r/44200/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Reply via email to