admin: support /drain_listeners?skip_exit#30003
Conversation
c885a1c to
cece6bf
Compare
soulxu
left a comment
There was a problem hiding this comment.
LGTM except main merge needed, thanks
bcba142 to
31adab8
Compare
changelogs/current.yaml
Outdated
There was a problem hiding this comment.
Bad merge? Can you remove this line?
kyessenov
left a comment
There was a problem hiding this comment.
LGTM, please update the change notes.
31adab8 to
b80434f
Compare
|
@howardjohn you need to fix the format. Looks like some white space issue by your editor. Can you run |
Head branch was pushed to by a user without write access
Ok I don;t really understand why me adding 1 line means the entire file should reformat, but done. Hope that works |
source/server/admin/admin.cc
Outdated
There was a problem hiding this comment.
Looks like a true compilation problem here?
There was a problem hiding this comment.
I think I finally got it... thanks for the patience 🙂
ae4a409 to
e4836d0
Compare
Signed-off-by: John Howard <howardjohn@google.com>
e4836d0 to
d1866ef
Compare
|
Can you resolve the conflict? Also, please don't force push. |
Signed-off-by: John Howard <howardjohn@google.com>
Commit Message:
admin: support /drain_listeners?skip_exitAdditional Description:
Due to changes in Kubernetes (https://kubernetes.io/blog/2023/08/25/native-sidecar-containers/) around sidecars, there is a more optimal way to manage the lifecycle of Envoy as a sidecar in Kubernetes. However, it is not currently possible with the current knobs.
Our goal:
The time between (1) and (3) can be quite large (but not unbounded; Kubernetes has a max time before it will SIGKILL everything).
How we want to implement this:
/drain_listeners?graceful&skip_exitThe reason we need a
skip_exitis due to the timing between the two. If we just do this withoutskip_exitif the application container takes >drain-time-swe will block new connections unexpectedly. While we could just raise thedrain-time, this also impacts other draining (notably, LDS update), which may be unexpected.Risk Level: Low
Testing: Added new integration test based on existing ones around draining
Docs Changes: Add docs to admin.rst