use correct order of operations in termination_drain_duration description #2047

alam0rt · 2021-07-07T07:11:48Z

Description of problem

While we were testing the terminationDrainDuration config (by way of pod annotation), we realised the observed behaviour was different than what is described in the documentation.

For example we set the pod's termination grace period to 30s and also the terminationDrainDuration to 30s. We observe that the proxy allows curl to continue establishing connections up until SIGKILL is sent.

➜ kubectl run curl --image=radial/busyboxplus:curl -i --tty --rm
If you don't see a command prompt, try pressing enter.
[ root@curl:/ ]$ while true; do curl google.com; sleep 1; done
<HTML><HEAD><meta http-equiv="content-type" content="text/html;charset=utf-8">
<TITLE>301 Moved</TITLE></HEAD><BODY>
<H1>301 Moved</H1>
The document has moved
<A HREF="http://www.google.com/">here</A>.
</BODY></HTML>
<HTML><HEAD><meta http-equiv="content-type" content="text/html;charset=utf-8">
<TITLE>301 Moved</TITLE></HEAD><BODY>
<H1>301 Moved</H1>
The document has moved
<A HREF="http://www.google.com/">here</A>.
</BODY></HTML>
<HTML><HEAD><meta http-equiv="content-type" content="text/html;charset=utf-8">
<TITLE>301 Moved</TITLE></HEAD><BODY>
<H1>301 Moved</H1>
The document has moved
<A HREF="http://www.google.com/">here</A>.
</BODY></HTML>
<HTML><HEAD><meta http-equiv="content-type" content="text/html;charset=utf-8">
<TITLE>301 Moved</TITLE></HEAD><BODY>
<H1>301 Moved</H1>
The document has moved
<A HREF="http://www.google.com/">here</A>.
</BODY></HTML>
<HTML><HEAD><meta http-equiv="content-type" content="text/html;charset=utf-8">
<TITLE>301 Moved</TITLE></HEAD><BODY>
<H1>301 Moved</H1>
The document has moved
<A HREF="http://www.google.com/">here</A>.
</BODY></HTML>
<HTML><HEAD><meta http-equiv="content-type" content="text/html;charset=utf-8">
<TITLE>301 Moved</TITLE></HEAD><BODY>
<H1>301 Moved</H1>
The document has moved
<A HREF="http://www.google.com/">here</A>.
</BODY></HTML>
<HTML><HEAD><meta http-equiv="content-type" content="text/html;charset=utf-8">
<TITLE>301 Moved</TITLE></HEAD><BODY>
<H1>301 Moved</H1>
The document has moved
<A HREF="http://www.google.com/">here</A>.
</BODY></HTML>
<HTML><HEAD><meta http-equiv="content-type" content="text/html;charset=utf-8">
<TITLE>301 Moved</TITLE></HEAD><BODY>
<H1>301 Moved</H1>
The document has moved
<A HREF="http://www.google.com/">here</A>.
</BODY></HTML>
<HTML><HEAD><meta http-equiv="content-type" content="text/html;charset=utf-8">
<TITLE>301 Moved</TITLE></HEAD><BODY>
<H1>301 Moved</H1>
The document has moved
<A HREF="http://www.google.com/">here</A>.
</BODY></HTML>
<HTML><HEAD><meta http-equiv="content-type" content="text/html;charset=utf-8">
<TITLE>301 Moved</TITLE></HEAD><BODY>
<H1>301 Moved</H1>
The document has moved
<A HREF="http://www.google.com/">here</A>.
</BODY></HTML>
<HTML><HEAD><meta http-equiv="content-type" content="text/html;charset=utf-8">
<TITLE>301 Moved</TITLE></HEAD><BODY>
<H1>301 Moved</H1>
The document has moved
<A HREF="http://www.google.com/">here</A>.
</BODY></HTML>
<HTML><HEAD><meta http-equiv="content-type" content="text/html;charset=utf-8">
<TITLE>301 Moved</TITLE></HEAD><BODY>
<H1>301 Moved</H1>
The document has moved
<A HREF="http://www.google.com/">here</A>.
</BODY></HTML>
<HTML><HEAD><meta http-equiv="content-type" content="text/html;charset=utf-8">
<TITLE>301 Moved</TITLE></HEAD><BODY>
<H1>301 Moved</H1>
The document has moved
<A HREF="http://www.google.com/">here</A>.
</BODY></HTML>
<HTML><HEAD><meta http-equiv="content-type" content="text/html;charset=utf-8">
<TITLE>301 Moved</TITLE></HEAD><BODY>
<H1>301 Moved</H1>
The document has moved
<A HREF="http://www.google.com/">here</A>.
</BODY></HTML>
<HTML><HEAD><meta http-equiv="content-type" content="text/html;charset=utf-8">
<TITLE>301 Moved</TITLE></HEAD><BODY>
<H1>301 Moved</H1>
The document has moved
<A HREF="http://www.google.com/">here</A>.
</BODY></HTML>
<HTML><HEAD><meta http-equiv="content-type" content="text/html;charset=utf-8">
<TITLE>301 Moved</TITLE></HEAD><BODY>
<H1>301 Moved</H1>
The document has moved
<A HREF="http://www.google.com/">here</A>.
</BODY></HTML>
<HTML><HEAD><meta http-equiv="content-type" content="text/html;charset=utf-8">
<TITLE>301 Moved</TITLE></HEAD><BODY>
<H1>301 Moved</H1>
The document has moved
<A HREF="http://www.google.com/">here</A>.
</BODY></HTML>
<HTML><HEAD><meta http-equiv="content-type" content="text/html;charset=utf-8">
<TITLE>301 Moved</TITLE></HEAD><BODY>
<H1>301 Moved</H1>
The document has moved
<A HREF="http://www.google.com/">here</A>.
</BODY></HTML>
<HTML><HEAD><meta http-equiv="content-type" content="text/html;charset=utf-8">
<TITLE>301 Moved</TITLE></HEAD><BODY>
<H1>301 Moved</H1>
The document has moved
<A HREF="http://www.google.com/">here</A>.
</BODY></HTML>
<HTML><HEAD><meta http-equiv="content-type" content="text/html;charset=utf-8">
<TITLE>301 Moved</TITLE></HEAD><BODY>
<H1>301 Moved</H1>
The document has moved
<A HREF="http://www.google.com/">here</A>.
</BODY></HTML>
<HTML><HEAD><meta http-equiv="content-type" content="text/html;charset=utf-8">
<TITLE>301 Moved</TITLE></HEAD><BODY>
<H1>301 Moved</H1>
The document has moved
<A HREF="http://www.google.com/">here</A>.
</BODY></HTML>
<HTML><HEAD><meta http-equiv="content-type" content="text/html;charset=utf-8">
<TITLE>301 Moved</TITLE></HEAD><BODY>
<H1>301 Moved</H1>
The document has moved
<A HREF="http://www.google.com/">here</A>.
</BODY></HTML>
<HTML><HEAD><meta http-equiv="content-type" content="text/html;charset=utf-8">
<TITLE>301 Moved</TITLE></HEAD><BODY>
<H1>301 Moved</H1>
The document has moved
<A HREF="http://www.google.com/">here</A>.
</BODY></HTML>
<HTML><HEAD><meta http-equiv="content-type" content="text/html;charset=utf-8">
<TITLE>301 Moved</TITLE></HEAD><BODY>
<H1>301 Moved</H1>
The document has moved
<A HREF="http://www.google.com/">here</A>.
</BODY></HTML>
<HTML><HEAD><meta http-equiv="content-type" content="text/html;charset=utf-8">
<TITLE>301 Moved</TITLE></HEAD><BODY>
<H1>301 Moved</H1>
The document has moved
<A HREF="http://www.google.com/">here</A>.
</BODY></HTML>
<HTML><HEAD><meta http-equiv="content-type" content="text/html;charset=utf-8">
<TITLE>301 Moved</TITLE></HEAD><BODY>
<H1>301 Moved</H1>
The document has moved
<A HREF="http://www.google.com/">here</A>.
</BODY></HTML>
<HTML><HEAD><meta http-equiv="content-type" content="text/html;charset=utf-8">
<TITLE>301 Moved</TITLE></HEAD><BODY>
<H1>301 Moved</H1>
The document has moved
<A HREF="http://www.google.com/">here</A>.
</BODY></HTML>
<HTML><HEAD><meta http-equiv="content-type" content="text/html;charset=utf-8">
<TITLE>301 Moved</TITLE></HEAD><BODY>
<H1>301 Moved</H1>
The document has moved
<A HREF="http://www.google.com/">here</A>.
</BODY></HTML>
<HTML><HEAD><meta http-equiv="content-type" content="text/html;charset=utf-8">
<TITLE>301 Moved</TITLE></HEAD><BODY>
<H1>301 Moved</H1>
The document has moved
<A HREF="http://www.google.com/">here</A>.
</BODY></HTML>
<HTML><HEAD><meta http-equiv="content-type" content="text/html;charset=utf-8">
<TITLE>301 Moved</TITLE></HEAD><BODY>
<H1>301 Moved</H1>
The document has moved
<A HREF="http://www.google.com/">here</A>.
</BODY></HTML>
<HTML><HEAD><meta http-equiv="content-type" content="text/html;charset=utf-8">
<TITLE>301 Moved</TITLE></HEAD><BODY>
<H1>301 Moved</H1>
The document has moved
<A HREF="http://www.google.com/">here</A>.
</BODY></HTML>
<HTML><HEAD><meta http-equiv="content-type" content="text/html;charset=utf-8">
<TITLE>301 Moved</TITLE></HEAD><BODY>
<H1>301 Moved</H1>
The document has moved
<A HREF="http://www.google.com/">here</A>.
</BODY></HTML>
Session ended, resume using 'kubectl attach curl -c curl -i -t' command when the pod is running
pod "curl" deleted

Here we have set the terminationDrainDuration to 15s, while keeping the pod's at 30s:

# while true; do curl google.com; sleep 1; done
<HTML><HEAD><meta http-equiv="content-type" content="text/html;charset=utf-8">
<TITLE>301 Moved</TITLE></HEAD><BODY>
<H1>301 Moved</H1>
The document has moved
<A HREF="http://www.google.com/">here</A>.
</BODY></HTML>
<HTML><HEAD><meta http-equiv="content-type" content="text/html;charset=utf-8">
<TITLE>301 Moved</TITLE></HEAD><BODY>
<H1>301 Moved</H1>
The document has moved
<A HREF="http://www.google.com/">here</A>.
</BODY></HTML>
<HTML><HEAD><meta http-equiv="content-type" content="text/html;charset=utf-8">
<TITLE>301 Moved</TITLE></HEAD><BODY>
<H1>301 Moved</H1>
The document has moved
<A HREF="http://www.google.com/">here</A>.
</BODY></HTML>
<HTML><HEAD><meta http-equiv="content-type" content="text/html;charset=utf-8">
<TITLE>301 Moved</TITLE></HEAD><BODY>
<H1>301 Moved</H1>
The document has moved
<A HREF="http://www.google.com/">here</A>.
</BODY></HTML>
<HTML><HEAD><meta http-equiv="content-type" content="text/html;charset=utf-8">
<TITLE>301 Moved</TITLE></HEAD><BODY>
<H1>301 Moved</H1>
The document has moved
<A HREF="http://www.google.com/">here</A>.
</BODY></HTML>
<HTML><HEAD><meta http-equiv="content-type" content="text/html;charset=utf-8">
<TITLE>301 Moved</TITLE></HEAD><BODY>
<H1>301 Moved</H1>
The document has moved
<A HREF="http://www.google.com/">here</A>.
</BODY></HTML>
<HTML><HEAD><meta http-equiv="content-type" content="text/html;charset=utf-8">
<TITLE>301 Moved</TITLE></HEAD><BODY>
<H1>301 Moved</H1>
The document has moved
<A HREF="http://www.google.com/">here</A>.
</BODY></HTML>
<HTML><HEAD><meta http-equiv="content-type" content="text/html;charset=utf-8">
<TITLE>301 Moved</TITLE></HEAD><BODY>
<H1>301 Moved</H1>
The document has moved
<A HREF="http://www.google.com/">here</A>.
</BODY></HTML>
<HTML><HEAD><meta http-equiv="content-type" content="text/html;charset=utf-8">
<TITLE>301 Moved</TITLE></HEAD><BODY>
<H1>301 Moved</H1>
The document has moved
<A HREF="http://www.google.com/">here</A>.
</BODY></HTML>
<HTML><HEAD><meta http-equiv="content-type" content="text/html;charset=utf-8">
<TITLE>301 Moved</TITLE></HEAD><BODY>
<H1>301 Moved</H1>
The document has moved
<A HREF="http://www.google.com/">here</A>.
</BODY></HTML>
<HTML><HEAD><meta http-equiv="content-type" content="text/html;charset=utf-8">
<TITLE>301 Moved</TITLE></HEAD><BODY>
<H1>301 Moved</H1>
The document has moved
<A HREF="http://www.google.com/">here</A>.
</BODY></HTML>
<HTML><HEAD><meta http-equiv="content-type" content="text/html;charset=utf-8">
<TITLE>301 Moved</TITLE></HEAD><BODY>
<H1>301 Moved</H1>
The document has moved
<A HREF="http://www.google.com/">here</A>.
</BODY></HTML>
<HTML><HEAD><meta http-equiv="content-type" content="text/html;charset=utf-8">
<TITLE>301 Moved</TITLE></HEAD><BODY>
<H1>301 Moved</H1>
The document has moved
<A HREF="http://www.google.com/">here</A>.
</BODY></HTML>
<HTML><HEAD><meta http-equiv="content-type" content="text/html;charset=utf-8">
<TITLE>301 Moved</TITLE></HEAD><BODY>
<H1>301 Moved</H1>
The document has moved
<A HREF="http://www.google.com/">here</A>.
</BODY></HTML>
<HTML><HEAD><meta http-equiv="content-type" content="text/html;charset=utf-8">
<TITLE>301 Moved</TITLE></HEAD><BODY>
<H1>301 Moved</H1>
The document has moved
<A HREF="http://www.google.com/">here</A>.
</BODY></HTML>
<HTML><HEAD><meta http-equiv="content-type" content="text/html;charset=utf-8">
<TITLE>301 Moved</TITLE></HEAD><BODY>
<H1>301 Moved</H1>
The document has moved
<A HREF="http://www.google.com/">here</A>.
</BODY></HTML>
<HTML><HEAD><meta http-equiv="content-type" content="text/html;charset=utf-8">
<TITLE>301 Moved</TITLE></HEAD><BODY>
<H1>301 Moved</H1>
The document has moved
<A HREF="http://www.google.com/">here</A>.
</BODY></HTML>
<HTML><HEAD><meta http-equiv="content-type" content="text/html;charset=utf-8">
<TITLE>301 Moved</TITLE></HEAD><BODY>
<H1>301 Moved</H1>
The document has moved
<A HREF="http://www.google.com/">here</A>.
</BODY></HTML>
<HTML><HEAD><meta http-equiv="content-type" content="text/html;charset=utf-8">
<TITLE>301 Moved</TITLE></HEAD><BODY>
<H1>301 Moved</H1>
The document has moved
<A HREF="http://www.google.com/">here</A>.
</BODY></HTML>
<HTML><HEAD><meta http-equiv="content-type" content="text/html;charset=utf-8">
<TITLE>301 Moved</TITLE></HEAD><BODY>
<H1>301 Moved</H1>
The document has moved
<A HREF="http://www.google.com/">here</A>.
</BODY></HTML>
curl: (7) Failed to connect to google.com port 80: Connection refused
curl: (7) Failed to connect to google.com port 80: Connection refused
curl: (7) Failed to connect to google.com port 80: Connection refused
curl: (7) Failed to connect to google.com port 80: Connection refused
curl: (7) Failed to connect to google.com port 80: Connection refused
curl: (7) Failed to connect to google.com port 80: Connection refused
curl: (7) Failed to connect to google.com port 80: Connection refused
curl: (7) Failed to connect to google.com port 80: Connection refused
curl: (7) Failed to connect to google.com port 80: Connection refused
curl: (7) Failed to connect to google.com port 80: Connection refused
curl: (7) Failed to connect to google.com port 80: Connection refused
curl: (7) Failed to connect to google.com port 80: Connection refused
curl: (7) Failed to connect to google.com port 80: Connection refused
curl: (7) Failed to connect to google.com port 80: Connection refused
curl: (7) Failed to connect to google.com port 80: Connection refused
command terminated with exit code 137

With a curl running every 1s, we can observe that for 15s we experience connection refused. Based off of this, I think it is safe to assume that the terminationDrainDuration increases the amount of time before envoy blocks the application from making requests.

Thanks to @senior88oqz for doing most of the debugging!

istio-testing · 2021-07-07T07:12:01Z

Hi @alam0rt. Thanks for your PR.

I'm waiting for a istio member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

craigbox · 2021-07-07T09:24:51Z

Thanks for the comprehensive debugging!

/ok-to-test

howardjohn

Draining does not mean rejecting all calls. The drain is graceful; it will initially just send Connection:close header. See https://www.envoyproxy.io/docs/envoy/latest/intro/arch_overview/operations/draining

Code: https://github.com/istio/istio/blob/a1c6ca2060860e97deb1f6eb12ca6770c8026672/pkg/envoy/agent.go#L102

howardjohn · 2021-07-07T14:27:19Z

By the way, in the future, the html file are auto generated from the corresponding .proto file. You should modify that file then run make gen. Thanks for the PR!

alam0rt · 2021-07-08T00:09:26Z

/retest

alam0rt · 2021-07-08T00:13:18Z

Thanks for getting back to me and clarifiying @howardjohn and @craigbox.

I've tweaked the wording a bit to include envoy's language.

As an aside, I had trouble running make gen and had to manually download buf and hack ./common/scripts/setup_env.sh by adding CONDITIONAL_HOST_MOUNTS+="--mount type=bind,source=${HOME}/buf,destination=/usr/bin/buf,readonly " as without:

./clean.sh
./gen.sh
Syncing ./networking/v1beta1/destination_rule.proto from networking/v1alpha3/destination_rule.proto
Syncing ./networking/v1beta1/gateway.proto from networking/v1alpha3/gateway.proto
Syncing ./networking/v1beta1/service_entry.proto from networking/v1alpha3/service_entry.proto
Syncing ./networking/v1beta1/sidecar.proto from networking/v1alpha3/sidecar.proto
Syncing ./networking/v1beta1/virtual_service.proto from networking/v1alpha3/virtual_service.proto
Syncing ./networking/v1beta1/workload_entry.proto from networking/v1alpha3/workload_entry.proto
./gen.sh: line 23: buf: command not found
make[1]: *** [Makefile.core.mk:37: gen-proto] Error 127
make: *** [gen] Error 2

thanks :)

howardjohn · 2021-07-08T00:19:36Z

/retest

istio-testing · 2022-12-16T22:03:04Z

@alam0rt: PR needs rebase.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

linux-foundation-easycla · 2024-05-15T23:58:15Z

❌ The email address for the commit (3a4511a, 21e90f4, ab43ab1, 4d1d470) is not linked to the GitHub account, preventing the EasyCLA check. Consult this Help Article and GitHub Help to resolve. (To view the commit's email address, add .patch at the end of this PR page's URL.) For further assistance with EasyCLA, please submit a support request ticket.

istio-testing · 2024-05-15T23:58:32Z

@alam0rt: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
gencheck_api	`4d1d470`	link	true	`/test gencheck`
build_api	`4d1d470`	link	true	`/test build`
release-notes_api	`4d1d470`	link	false	`/test release-notes`

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.

craigbox · 2024-05-15T23:58:56Z

/close

use correct order of operations

ab43ab1

alam0rt requested review from brian-avery, howardjohn, linsun, louiscryan, nrjpoddar and smawson as code owners July 7, 2021 07:11

istio-testing added size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. needs-ok-to-test labels Jul 7, 2021

google-cla bot added the cla: yes Set by the Google CLA bot to indicate the author of a PR has signed the Google CLA. label Jul 7, 2021

alam0rt mentioned this pull request Jul 7, 2021

fix up order of operations in description istio/istio.io#10037

Closed

alam0rt changed the title ~~use correct order of operations~~ use correct order of operations in termination_drain_duration description Jul 7, 2021

istio-testing added ok-to-test Set this label allow normal testing to take place for a PR not submitted by an Istio org member. and removed needs-ok-to-test labels Jul 7, 2021

howardjohn requested changes Jul 7, 2021

View reviewed changes

Samuel Lockart added 2 commits July 8, 2021 09:26

edit the schema instead

21e90f4

edit proto and revert the gen json

3a4511a

alam0rt force-pushed the fix-the-termination-drain-duration-description branch from 51cb50d to 3a4511a Compare July 7, 2021 23:50

ran make gen

4d1d470

howardjohn approved these changes Jul 8, 2021

View reviewed changes

howardjohn added the release-notes-none Indicates a PR that does not require release notes. label Jul 8, 2021

istio-testing added the needs-rebase Indicates a PR needs to be rebased before being merged label Dec 16, 2022

howardjohn removed the cla: yes Set by the Google CLA bot to indicate the author of a PR has signed the Google CLA. label May 15, 2024

istio-policy-bot closed this May 15, 2024

istio-policy-bot added the lifecycle/automatically-closed Indicates a PR or issue that has been closed automatically. label May 15, 2024

craigbox reopened this May 15, 2024

craigbox requested a review from a team as a code owner May 15, 2024 23:58

istio-policy-bot closed this May 15, 2024

craigbox mentioned this pull request May 16, 2024

Update docs on draining (from #2047) #3202

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

use correct order of operations in termination_drain_duration description #2047

use correct order of operations in termination_drain_duration description #2047

Uh oh!

alam0rt commented Jul 7, 2021

Uh oh!

istio-testing commented Jul 7, 2021

Uh oh!

craigbox commented Jul 7, 2021

Uh oh!

howardjohn left a comment

Uh oh!

howardjohn commented Jul 7, 2021

Uh oh!

alam0rt commented Jul 8, 2021

Uh oh!

alam0rt commented Jul 8, 2021

Uh oh!

howardjohn commented Jul 8, 2021

Uh oh!

istio-testing commented Dec 16, 2022

Uh oh!

linux-foundation-easycla bot commented May 15, 2024

Uh oh!

istio-testing commented May 15, 2024

Uh oh!

craigbox commented May 15, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

use correct order of operations in termination_drain_duration description #2047

use correct order of operations in termination_drain_duration description #2047

Uh oh!

Conversation

alam0rt commented Jul 7, 2021

Description of problem

Uh oh!

istio-testing commented Jul 7, 2021

Uh oh!

craigbox commented Jul 7, 2021

Uh oh!

howardjohn left a comment

Choose a reason for hiding this comment

Uh oh!

howardjohn commented Jul 7, 2021

Uh oh!

alam0rt commented Jul 8, 2021

Uh oh!

alam0rt commented Jul 8, 2021

Uh oh!

howardjohn commented Jul 8, 2021

Uh oh!

istio-testing commented Dec 16, 2022

Uh oh!

linux-foundation-easycla bot commented May 15, 2024

Uh oh!

istio-testing commented May 15, 2024

Uh oh!

craigbox commented May 15, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants