Skip to content

Conversation

jrajahalme
Copy link
Member

Revert #36032 as it did not resolve the issue with Envoy initial fetch timeout warnings. Demote those warnings in the inlined logs from the embedded cilium-envoy instead.

Start waiting for Envoy acknowledgements as soon as endpoints have regenerated, rather than only when the first request on the xDS stream has been received.

Note that any proxied DNS requests will not wait for acknowledgements if issued before endpoints have regenerated, and even if they did, the maximum delay for passing the DNS response to the source pod is shorter than the time span allowed for endpoint regeneration.

We could start serving xDS resources sooner if we computed policies for the regenerated endpoints first, after which we could start serving xDS resources to envoy. Then the endpoints could take the time to recompute their bpf programs etc.

Fixes: #36032
Fixes: #35984

Envoy "initial fetch timeout" warnings are now demoted to info level, as they are expected to happen
during Cilium Agent restart.

@jrajahalme jrajahalme added the area/proxy Impacts proxy components, including DNS, Kafka, Envoy and/or XDS servers. label Nov 20, 2024
@jrajahalme jrajahalme requested a review from a team as a code owner November 20, 2024 07:28
@jrajahalme jrajahalme requested a review from sayboras November 20, 2024 07:28
@maintainer-s-little-helper maintainer-s-little-helper bot added the dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. label Nov 20, 2024
@maintainer-s-little-helper
Copy link

Commit 428e7e7 does not match "(?m)^Signed-off-by:".

Please follow instructions provided in https://docs.cilium.io/en/stable/contributing/development/contributing_guide/#developer-s-certificate-of-origin

@maintainer-s-little-helper maintainer-s-little-helper bot added the dont-merge/needs-sign-off The author needs to add signoff to their commits before merge. label Nov 20, 2024
@jrajahalme jrajahalme added the release-note/bug This PR fixes an issue in a previous release of Cilium. label Nov 20, 2024
@maintainer-s-little-helper maintainer-s-little-helper bot removed the dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. label Nov 20, 2024
@jrajahalme jrajahalme force-pushed the envoy-demote-initial-fetch-timeout-warning branch from 9825e27 to 4d4a363 Compare November 20, 2024 07:29
@maintainer-s-little-helper
Copy link

Commit 428e7e7 does not match "(?m)^Signed-off-by:".

Please follow instructions provided in https://docs.cilium.io/en/stable/contributing/development/contributing_guide/#developer-s-certificate-of-origin

@jrajahalme jrajahalme removed the dont-merge/needs-sign-off The author needs to add signoff to their commits before merge. label Nov 20, 2024
@jrajahalme jrajahalme force-pushed the envoy-demote-initial-fetch-timeout-warning branch from 4d4a363 to 6d9b8e0 Compare November 20, 2024 07:31
This reverts commit 40ac5c3.

Signed-off-by: Jarno Rajahalme <jarno@isovalent.com>
Demote the expected initial fetch timeout warning due to Cilium Agent
delaying serving resources until endpoints have regenerated.

Signed-off-by: Jarno Rajahalme <jarno@isovalent.com>
We can start waiting for acknowledgements as soon as all endpoints have
regenerated, there is no need to wait for the first xDS request to be
received.

Signed-off-by: Jarno Rajahalme <jarno@isovalent.com>
@jrajahalme jrajahalme force-pushed the envoy-demote-initial-fetch-timeout-warning branch from 6d9b8e0 to 84e77ee Compare November 20, 2024 07:32
@jrajahalme
Copy link
Member Author

/test

@maintainer-s-little-helper maintainer-s-little-helper bot added the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label Nov 22, 2024
@aanm aanm added this pull request to the merge queue Nov 22, 2024
Merged via the queue into cilium:main with commit de53198 Nov 22, 2024
64 checks passed
@jrajahalme jrajahalme added the needs-backport/1.16 This PR / issue needs backporting to the v1.16 branch label Nov 28, 2024
@jrajahalme jrajahalme mentioned this pull request Dec 1, 2024
2 tasks
@jrajahalme jrajahalme added backport-pending/1.16 The backport for Cilium 1.16.x for this PR is in progress. and removed needs-backport/1.16 This PR / issue needs backporting to the v1.16 branch labels Dec 1, 2024
@github-actions github-actions bot added backport-done/1.16 The backport for Cilium 1.16.x for this PR is done. and removed backport-pending/1.16 The backport for Cilium 1.16.x for this PR is in progress. labels Dec 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/proxy Impacts proxy components, including DNS, Kafka, Envoy and/or XDS servers. backport-done/1.16 The backport for Cilium 1.16.x for this PR is done. ready-to-merge This PR has passed all tests and received consensus from code owners to merge. release-note/bug This PR fixes an issue in a previous release of Cilium.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants