You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Benjamin Bannier <be...@mesosphere.io> on 2018/12/21 14:03:34 UTC
Review Request 69606: Made starting and stopping on CSI plugin
containers more verbose.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69606/
-----------------------------------------------------------
Review request for mesos and Chun-Hung Hsiao.
Bugs: MESOS-9223
https://issues.apache.org/jira/browse/MESOS-9223
Repository: mesos
Description
-------
This patch adds some additional logging so it becomes easier to follow
the lifecycle of CSI plugins. The container daemon already logged some
related information, but didn't directly call out that it was working
with CSI plugin containers.
Diffs
-----
src/resource_provider/storage/provider.cpp d6e20a549ede189c757ae3ae922ab7cb86d2be2c
Diff: https://reviews.apache.org/r/69606/diff/1/
Testing
-------
`sudo make check`
Thanks,
Benjamin Bannier
Re: Review Request 69606: Made starting and stopping on CSI plugin
containers more verbose.
Posted by Benjamin Bannier <be...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69606/
-----------------------------------------------------------
(Updated Jan. 15, 2019, 9:43 a.m.)
Review request for mesos and Chun-Hung Hsiao.
Bugs: MESOS-9223
https://issues.apache.org/jira/browse/MESOS-9223
Repository: mesos
Description
-------
This patch adds some additional logging so it becomes easier to follow
the lifecycle of CSI plugins. The container daemon already logged some
related information, but didn't directly call out that it was working
with CSI plugin containers.
Diffs (updated)
-----
src/resource_provider/storage/provider.cpp a0641665710e08f6faaaa3b4b3ece7bcc5c6d4ef
Diff: https://reviews.apache.org/r/69606/diff/3/
Changes: https://reviews.apache.org/r/69606/diff/2-3/
Testing
-------
`sudo make check`
Thanks,
Benjamin Bannier
Re: Review Request 69606: Made starting and stopping on CSI plugin
containers more verbose.
Posted by Chun-Hung Hsiao <ch...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69606/#review211967
-----------------------------------------------------------
Fix it, then Ship it!
Not sure how much we gain from this patch, since the container ID, which is printed out by the container daemon before invoking the post-start/stop hooks, already has the type and name of the CSI plugin.
src/resource_provider/storage/provider.cpp
Lines 2021-2024 (patched)
<https://reviews.apache.org/r/69606/#comment297545>
Since it is possible for a CSI plugin to run two containers, one for the controller service and one for the node service, how about the following?
```
LOG(INFO)
<< "CSI plugin container '" << containerId << "' started for plugin type '"
<< info.storage().plugin().type() << "' and name '"
<< info.storage().plugin().name() << "'";
```
It's also more consistent with how type-name pairs are logged elsewhere.
Ditto below.
- Chun-Hung Hsiao
On Dec. 21, 2018, 2:03 p.m., Benjamin Bannier wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69606/
> -----------------------------------------------------------
>
> (Updated Dec. 21, 2018, 2:03 p.m.)
>
>
> Review request for mesos and Chun-Hung Hsiao.
>
>
> Bugs: MESOS-9223
> https://issues.apache.org/jira/browse/MESOS-9223
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This patch adds some additional logging so it becomes easier to follow
> the lifecycle of CSI plugins. The container daemon already logged some
> related information, but didn't directly call out that it was working
> with CSI plugin containers.
>
>
> Diffs
> -----
>
> src/resource_provider/storage/provider.cpp a0641665710e08f6faaaa3b4b3ece7bcc5c6d4ef
>
>
> Diff: https://reviews.apache.org/r/69606/diff/2/
>
>
> Testing
> -------
>
> `sudo make check`
>
>
> Thanks,
>
> Benjamin Bannier
>
>