Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

envoy: Keep track of proxy listeners separately #16834

Merged
merged 1 commit into from Jul 12, 2021

Conversation

jrajahalme
Copy link
Member

@jrajahalme jrajahalme commented Jul 8, 2021

Since the addition of Envoy prometheus listener it has been possible
to have non-proxy listeners configured with Envoy. Waiting for Envoy
N/ACKs must be disabled when no proxy listeners are configured, even
if a prometheus listener may still be configured.

Without this fix adding endpoints may fail due to not receiving N/ACKs
from Envoy after Envoy has been started due to an L7 network policy,
and this policy is removed, if the Cilium option
'--proxy-prometheus-port' is also configured.

Fixes: #12949
Fixes: #16375
Fixes: #16667

Envoy configuration with `--proxy-prometheus-port` is fixed.

@jrajahalme jrajahalme added kind/bug This is a bug in the Cilium logic. release-note/bug This PR fixes an issue in a previous release of Cilium. backport/1.9 labels Jul 8, 2021
@jrajahalme jrajahalme requested a review from a team as a code owner July 8, 2021 19:00
@jrajahalme jrajahalme requested a review from aditighag July 8, 2021 19:00
@maintainer-s-little-helper maintainer-s-little-helper bot added this to Needs backport from master in 1.10.3 Jul 8, 2021
@jrajahalme
Copy link
Member Author

test-me-please

@jrajahalme jrajahalme assigned jrajahalme and unassigned aditighag Jul 8, 2021
@jrajahalme jrajahalme requested review from gandro and kkourt July 8, 2021 19:05
@maintainer-s-little-helper maintainer-s-little-helper bot added this to Needs backport from master in 1.9.9 Jul 8, 2021
Copy link
Contributor

@kkourt kkourt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

LGTM, only some suggestions for your consideration.

pkg/envoy/server.go Outdated Show resolved Hide resolved
pkg/envoy/server.go Show resolved Hide resolved
Since the addition of Envoy prometheus listener it has been possible
to have non-proxy listeners configured with Envoy. Waiting for Envoy
N/ACKs must be disabled when no proxy listeners are configured, even
if a prometheus listener may still be configured.

Without this fix adding endpoints may fail due to not receiving N/ACKs
from Envoy after Envoy has been started due to an L7 network policy,
and this policy is removed, if the Cilium option
'--proxy-prometheus-port' is also configured.

Fixes: #12949
Signed-off-by: Jarno Rajahalme <jarno@isovalent.com>
@jrajahalme jrajahalme force-pushed the pr/jrajahalme/completion-debugging branch from cb3e484 to f1c8e2c Compare July 9, 2021 17:18
@jrajahalme
Copy link
Member Author

test-me-please

@jrajahalme jrajahalme added the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label Jul 12, 2021
@aditighag aditighag merged commit 099c34d into master Jul 12, 2021
@aditighag aditighag deleted the pr/jrajahalme/completion-debugging branch July 12, 2021 17:19
@maintainer-s-little-helper maintainer-s-little-helper bot moved this from Needs backport from master to Backport pending to v1.10 in 1.10.3 Jul 14, 2021
@maintainer-s-little-helper maintainer-s-little-helper bot moved this from Backport pending to v1.10 to Needs backport from master in 1.10.3 Jul 14, 2021
@maintainer-s-little-helper maintainer-s-little-helper bot moved this from Needs backport from master to Backport pending to v1.10 in 1.10.3 Jul 14, 2021
@maintainer-s-little-helper maintainer-s-little-helper bot moved this from Needs backport from master to Backport pending to v1.10 in 1.10.3 Jul 14, 2021
@maintainer-s-little-helper maintainer-s-little-helper bot moved this from Backport pending to v1.10 to Backport done to v1.10 in 1.10.3 Jul 15, 2021
@maintainer-s-little-helper maintainer-s-little-helper bot moved this from Backport pending to v1.10 to Backport done to v1.10 in 1.10.3 Jul 15, 2021
@maintainer-s-little-helper maintainer-s-little-helper bot moved this from Needs backport from master to Backport pending to v1.9 in 1.9.9 Jul 15, 2021
@maintainer-s-little-helper maintainer-s-little-helper bot moved this from Needs backport from master to Backport pending to v1.9 in 1.9.9 Jul 15, 2021
@maintainer-s-little-helper maintainer-s-little-helper bot moved this from Backport pending to v1.9 to Backport done to v1.9 in 1.9.9 Jul 16, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug This is a bug in the Cilium logic. 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
No open projects
1.10.3
Backport done to v1.10
1.9.9
Backport done to v1.9
6 participants