search

containerd / nri

Node Resource Interface
Apache License 2.0
257 stars 65 forks source link

set stop status when exiting. #73

Closed yylt closed 8 months ago

yylt commented 8 months ago

update stub.started when stub stop

Signed-off-by: yylt yang8518296@163.com

codecov-commenter commented 8 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 64.44%. Comparing base (53d3371) to head (c1655e6).

:exclamation: Current head c1655e6 differs from pull request most recent head b778f84. Consider uploading reports for the commit b778f84 to get more accurate results

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #73 +/- ## ========================================== - Coverage 64.55% 64.44% -0.11% ========================================== Files 10 10 Lines 1845 1845 ========================================== - Hits 1191 1189 -2 - Misses 503 505 +2 Partials 151 151 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

klihub commented 8 months ago

@yylt I suspect you try to make stub.Stub re-Start()able, right ?

I am not sure if that is a good idea, mostly because I'm not sure at the moment if this is enough to safely be able to do that. If you need to restart the your plugin after a disconnect from the runtime, I think it would be a safer alternative to create a new stub for the plugin and start that one instead.

yylt commented 8 months ago

@yylt I suspect you try to make stub.Stub re-Start()able, right ?

I am not sure if that is a good idea, mostly because I'm not sure at the moment if this is enough to safely be able to do that. If you need to restart the your plugin after a disconnect from the runtime, I think it would be a safer alternative to create a new stub for the plugin and start that one instead.

haha... indeed, the intention was to restart, but later it was found that there were still other issues about channel with the start , so it was changed to using the method of creating a new struct.

If it is still not possible to start after stopping, there is really no need to set it up.

I'm not sure how things will evolve afterwards, but perhaps this change can be added.

klihub commented 8 months ago

@yylt I suspect you try to make stub.Stub re-Start()able, right ? I am not sure if that is a good idea, mostly because I'm not sure at the moment if this is enough to safely be able to do that. If you need to restart the your plugin after a disconnect from the runtime, I think it would be a safer alternative to create a new stub for the plugin and start that one instead.

haha... indeed, the intention was to restart, but later it was found that there were still other issues about channel with the start , so it was changed to using the method of creating a new struct.

If it is still not possible to start after stopping, there is really no need to set it up.

I'm not sure how things will evolve afterwards, but perhaps this change can be added.

@yylt Is it then okay for you if I close this with a comment summarizing the above conclusion ? If we later decide that we want to make the stub reStart()able, we can reopen this and stack any other necessary commits on top to achieve that goal.