consul-k8s icon indicating copy to clipboard operation
consul-k8s copied to clipboard

Published 20 hours ago • verified publisher icon hashicorp

Add configurable preStop delay to Envoy sidecar

Open hamishforbes opened this issue 3 years ago • 6 comments

Changes proposed in this PR:

  • Add a new annotation consul.hashicorp.com/sidecar-proxy-prestop-delay
  • Pods with this annotation have a Lifecycle hook added to delay the Envoy proxy sidecar shutdown by the specified number of seconds

Should probably be some validation on the value of the annotation, atm this would allow random command injection to the sidecar container on shutdown

How I've tested this PR:

How I expect reviewers to test this PR:

Checklist:

  • [ ] Tests added
  • [ ] CHANGELOG entry added

    HashiCorp engineers only, community PRs should not add a changelog entry. Entries should use present tense (e.g. Add support for...)

Related: #650

hamishforbes avatar Dec 08 '21 03:12 hamishforbes

Thanks for the PR. We'd like to support a more graceful way of shutting down Envoy so that if the is still running Envoy shuts down after the app is shut down first. We will take this into consideration for future development of this feature.

I'm curious is this workaround working well in your environment?

david-yu avatar Mar 07 '22 19:03 david-yu

Yeah for sure, it's not a particularly elegant solution!

I've been running a fork with this patch and a couple of other tweaks in production since opening the PR though, works perfectly for me.

hamishforbes avatar Mar 07 '22 21:03 hamishforbes

CLA assistant check
All committers have signed the CLA.

hashicorp-cla avatar Mar 12 '22 17:03 hashicorp-cla

I'd also note that I'm running a fork with the changes in this pull request. This feature really needs to get prioritized. There is no way to do rolling deployments without 5xx errors due to the envoy sidecar stopping before the appliation container completes.

dschaaff avatar Jun 20 '22 17:06 dschaaff

We are also testing this changes as we have some very high traffic apps and any issues gets quickly noticed and reported. A solution to this feature would be really helpful.

narendrapatel avatar Aug 11 '22 10:08 narendrapatel

Can't understand, how Consul Connect/Mesh can be used in production without this?

alt-dima avatar Aug 13 '22 11:08 alt-dima

We also have the same issue, we created this fork since our version (0.45.0) is behind main. We validated that adding the preStop hook fixed the problem.

natitomattis avatar Oct 24 '22 17:10 natitomattis

Closing as this is now resolved by https://github.com/hashicorp/consul-k8s/pull/2233 and will be released in upcoming patch releases for 1.0.x, 1.1.x and 1.2.x.

david-yu avatar Jun 27 '23 18:06 david-yu