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.

Sign up for GitHub

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

Jump to bottom

Envoy silence expected internal listener warning #29786

Merged
merged 2 commits into from
Dec 12, 2023
Merged

Envoy silence expected internal listener warning #29786

merged 2 commits into from
Dec 12, 2023

Conversation

jrajahalme
Copy link
Member

Silence the expected warning about internal listener being
work-in-progress. This special case can be removed when at Envoy 1.28,
as internal listener is a stable feature in Envoy 1.28.

This warning is not silenced if debug-verbose=envoy (which enables Envoy
trace logs). Due to this the warning still needs to be allow-listed for
tests, as some tests run with debug-verbose=envoy.

Fixes: #29682

@jrajahalme
envoy: Remove log filter that is no longer needed
8e7bdc0 
While envoyproxy/envoy#13504 is still open to
cover some cases, the reason for Envoy logs filtering seems to have been
fixed, so we can remove the logs filter for this.

Signed-off-by: Jarno Rajahalme <jarno@isovalent.com>
@jrajahalme jrajahalme added kind/bug This is a bug in the Cilium logic. release-blocker/1.15 This issue will prevent the release of the next version of Cilium. labels Dec 11, 2023
@jrajahalme jrajahalme requested a review from a team as a code owner December 11, 2023 13:09
@maintainer-s-little-helper maintainer-s-little-helper bot added dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. labels Dec 11, 2023
@jrajahalme
Copy link
Member Author

/test

@pchaigno
Copy link
Member

pchaigno commented Dec 11, 2023 •
edited
Loading

Note you can get this tested in CI by removing the related message from the allowlist:

cilium/test/helpers/cons.go

Line 252 in 5a82225

wipEnvoyFeature = "envoy/extensions/bootstrap/internal_listener/v3/internal_listener.proto" // cf. https://github.com/cilium/cilium/issues/29682

@mhofstetter mhofstetter added area/proxy Impacts proxy components, including DNS, Kafka, Envoy and/or XDS servers. release-note/misc This PR makes changes that have no direct user impact. labels Dec 11, 2023
@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 Dec 11, 2023
@jrajahalme jrajahalme requested a review from a team as a code owner December 11, 2023 13:20
mhofstetter
mhofstetter approved these changes Dec 11, 2023
View reviewed changes
Copy link
Member

@mhofstetter mhofstetter left a comment

Choose a reason for hiding this comment

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

Thanks!

@jrajahalme
Copy link
Member Author

@pchaigno Added commit to remove the allow-listing for this warning in tests. At first I thought we have tests running with debug-verbose=envoy so I did not remove the allow listing, but then I realized it was my local kind config that enabled Envoy tracing (for me).

@jrajahalme jrajahalme force-pushed the envoy-silence-expected-internal-listener-warning branch from baf2a81 to 74218c2 Compare December 11, 2023 13:22
@jrajahalme
Copy link
Member Author

/test

sayboras
sayboras approved these changes Dec 11, 2023
View reviewed changes
Copy link
Member

@sayboras sayboras left a comment

Choose a reason for hiding this comment

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

LGTM ✅

@jrajahalme
Copy link
Member Author

ginkgo tests are using debug-verbose=envoy, so they get this warning:

2023-12-11T13:30:12.607028019Z level=info msg="  --debug-verbose='envoy'" subsys=daemon
...
2023-12-11T13:31:05.659740136Z level=warning msg="[message 'envoy.extensions.bootstrap.internal_listener.v3.InternalListener' is contained in proto file 'envoy/extensions/bootstrap/internal_listener/v3/internal_listener.proto' marked as work-in-progress. API features marked as work-in-progress are not considered stable, are not covered by the threat model, are not supported by the security team, and are subject to breaking changes. Do not use this feature without understanding each of the previous points." subsys=envoy-misc threadID=451

I guess I should try to make the allow-listing conditional on the envoy trace setting?

@jrajahalme
envoy: Silence expected warning about internal listener feature
871e4dd 
Silence the expected warning about internal listener being
work-in-progress. This special case can be removed when at Envoy 1.28,
as internal listener is a stable feature in Envoy 1.28.

CI Ginkgo tests use Envoy trace mode (debug-verbose=envoy) so we still
need to whitelist this warning.

Signed-off-by: Jarno Rajahalme <jarno@isovalent.com>
@jrajahalme jrajahalme force-pushed the envoy-silence-expected-internal-listener-warning branch from 74218c2 to 871e4dd Compare December 11, 2023 15:45
@jrajahalme
Copy link
Member Author

Had to keep whitelisting the warning due to ci-ginkgo tests using debug-verbose=envoy. We had a CI run where only ci-ginkgo tests failed while the allowlisting was removed, though.

@jrajahalme
Copy link
Member Author

/test

@jrajahalme jrajahalme added this pull request to the merge queue Dec 12, 2023
Merged via the queue into cilium:main with commit 6017b77 Dec 12, 2023
61 checks passed
@jrajahalme jrajahalme deleted the envoy-silence-expected-internal-listener-warning branch December 12, 2023 09:05
@joestringer joestringer mentioned this pull request Dec 14, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mhofstetter mhofstetter mhofstetter approved these changes

@sayboras sayboras sayboras approved these changes

@christarazi christarazi Awaiting requested review from christarazi christarazi was automatically assigned from cilium/ci-structure

Assignees
No one assigned
Labels
area/proxy Impacts proxy components, including DNS, Kafka, Envoy and/or XDS servers. kind/bug This is a bug in the Cilium logic. release-blocker/1.15 This issue will prevent the release of the next version of Cilium. release-note/misc This PR makes changes that have no direct user impact.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

CI: Envoy warning on work-in-progress feature being used
4 participants
@jrajahalme @pchaigno @mhofstetter @sayboras