Delete failed Plugin/VMs via fail callbacks. #181
Conversation
Signed-off-by: Takeshi Yoneda <[email protected]>
There was a problem hiding this comment.
Would it be possible to add some basic tests for this behavior to verify that:
- Failed VMs and Plugins are not reused.
- VMs and Plugins can be restarted using the same keys.
I think this works fine right now, so those tests are primarly about protecting us against future regressions.
mathetake
force-pushed
the
add-failabackcall
branch
2 times, most recently
from
9cded75 to
d04a12b
Compare
|
OK added test! |
Signed-off-by: Takeshi Yoneda <[email protected]>
mathetake
force-pushed
the
add-failabackcall
branch
from
459799f to
dce6347
Compare
mathetake
changed the title
Signed-off-by: Takeshi Yoneda <[email protected]>
Signed-off-by: Takeshi Yoneda <[email protected]>
Signed-off-by: Takeshi Yoneda <[email protected]>
Signed-off-by: Takeshi Yoneda <[email protected]>
There was a problem hiding this comment.
LGTM, but did you see envoyproxy/envoy#17386 (comment)? Would it be possible to add PluginHandleManager here? It could be in a separate PR.
|
sorry, I am thinking I will do that in a separate PR since this PR itself is a little bit large, plus this requires no code change in Envoy. Thanks! |
|
You might consider waiting with merging this until the next PR is ready to minimize the window when we cannot update Envoy. |
|
well I think this PR requires no change in Envoy so this won't block anyone even after I merge this right now. |
Towards envoyproxy/envoy#16726
Signed-off-by: Takeshi Yoneda [email protected]