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
proxy: Ignore visibility annotation if proxy is disabled #27597
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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
Aug 20, 2023
sayboras
force-pushed
the
tam/visibility-proxy-false
branch
from
August 20, 2023 13:03
0d1177c
to
852119a
Compare
sayboras
added
release-note/bug
This PR fixes an issue in a previous release of Cilium.
needs-backport/1.12
needs-backport/1.13
This PR / issue needs backporting to the v1.13 branch
needs-backport/1.14
This PR / issue needs backporting to the v1.14 branch
labels
Aug 20, 2023
maintainer-s-little-helper
bot
removed
dont-merge/needs-release-note-label
The author needs to describe the release impact of these changes.
labels
Aug 20, 2023
/test |
sayboras
force-pushed
the
tam/visibility-proxy-false
branch
from
August 21, 2023 06:04
852119a
to
09c59e5
Compare
/test |
sayboras
force-pushed
the
tam/visibility-proxy-false
branch
from
August 21, 2023 10:42
09c59e5
to
d1a51fc
Compare
/test |
kaworu
approved these changes
Aug 21, 2023
youngnick
approved these changes
Aug 21, 2023
zacharysarah
requested changes
Aug 21, 2023
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@sayboras Small changes for clarity and consistency, otherwise LGTM for docs.
This is to avoid the below error if proxy is disabled as part of installation, but the visibility annotation is added to pods later. ``` 2023-08-11T12:42:41.390575371Z goroutine 1522 [running]: 2023-08-11T12:42:41.390581994Z github.com/cilium/cilium/pkg/proxy.(*Proxy).CreateOrUpdateRedirect(0x0, {0x3ae74b8, 0xc000650280}, {0x3aeb440?, 0xc0016e4560?}, {0xc0010ba1b0, 0x12}, {0x3afd260, 0xc000982000}, 0xc001c9c980) 2023-08-11T12:42:41.390589198Z github.com/cilium/cilium/pkg/proxy/proxy.go:459 +0xb7 2023-08-11T12:42:41.390596081Z github.com/cilium/cilium/pkg/endpoint.(*Endpoint).addVisibilityRedirects(0xc000982000, 0x1, 0xc001874ba0?, 0xc001c9c980?) 2023-08-11T12:42:41.390602613Z github.com/cilium/cilium/pkg/endpoint/bpf.go:343 +0x443 2023-08-11T12:42:41.390614345Z github.com/cilium/cilium/pkg/endpoint.(*Endpoint).addNewRedirects(0xc000982000, 0xc001874870?) 2023-08-11T12:42:41.390651325Z github.com/cilium/cilium/pkg/endpoint/bpf.go:424 +0x3c5 2023-08-11T12:42:41.390658068Z github.com/cilium/cilium/pkg/endpoint.(*Endpoint).runPreCompilationSteps(0xc000982000, 0xc000e31800, 0x0) 2023-08-11T12:42:41.390663999Z github.com/cilium/cilium/pkg/endpoint/bpf.go:802 +0x4fe 2023-08-11T12:42:41.390669690Z github.com/cilium/cilium/pkg/endpoint.(*Endpoint).regenerateBPF(0xc000982000, 0xc000e31800) 2023-08-11T12:42:41.390675451Z github.com/cilium/cilium/pkg/endpoint/bpf.go:542 +0x1a5 2023-08-11T12:42:41.390681112Z github.com/cilium/cilium/pkg/endpoint.(*Endpoint).regenerate(0xc000982000, 0xc000e31800) 2023-08-11T12:42:41.390686893Z github.com/cilium/cilium/pkg/endpoint/policy.go:467 +0x9c6 2023-08-11T12:42:41.390692564Z github.com/cilium/cilium/pkg/endpoint.(*EndpointRegenerationEvent).Handle(0xc000131b50, 0x0?) 2023-08-11T12:42:41.390698385Z github.com/cilium/cilium/pkg/endpoint/events.go:53 +0x325 2023-08-11T12:42:41.390704045Z github.com/cilium/cilium/pkg/eventqueue.(*EventQueue).run.func1() 2023-08-11T12:42:41.390709716Z github.com/cilium/cilium/pkg/eventqueue/eventqueue.go:245 +0x142 ``` Fixes: cilium#27594 Signed-off-by: Tam Mach <tam.mach@cilium.io>
This is to ensure that proxy must be enabled if Envoy L7 Load balancer feature is enabled. Signed-off-by: Tam Mach <tam.mach@cilium.io>
sayboras
force-pushed
the
tam/visibility-proxy-false
branch
from
August 22, 2023 04:35
d1a51fc
to
2b7caae
Compare
/test |
zacharysarah
approved these changes
Aug 22, 2023
jrajahalme
approved these changes
Aug 23, 2023
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
Aug 23, 2023
2 tasks
tklauser
added
backport-pending/1.14
The backport for Cilium 1.14.x for this PR is in progress.
backport-done/1.12
The backport for Cilium 1.12.x for this PR is done.
and removed
needs-backport/1.14
This PR / issue needs backporting to the v1.14 branch
backport-pending/1.12
labels
Aug 24, 2023
joestringer
added
backport-done/1.14
The backport for Cilium 1.14.x for this PR is done.
and removed
backport-pending/1.14
The backport for Cilium 1.14.x for this PR is in progress.
labels
Aug 25, 2023
pippolo84
added
backport-pending/1.13
The backport for Cilium 1.13.x for this PR is in progress.
and removed
needs-backport/1.13
This PR / issue needs backporting to the v1.13 branch
labels
Aug 28, 2023
michi-covalent
added
backport-done/1.13
The backport for Cilium 1.13.x for this PR is done.
and removed
backport-pending/1.13
The backport for Cilium 1.13.x for this PR is in progress.
labels
Sep 9, 2023
This was referenced Sep 9, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
backport-done/1.12
The backport for Cilium 1.12.x for this PR is done.
backport-done/1.13
The backport for Cilium 1.13.x for this PR is done.
backport-done/1.14
The backport for Cilium 1.14.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.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
This is to avoid the below error if proxy is disabled as part of
installation, but the visibility annotation is added to pods later.
Fixes: #27594
Testing
Testing was done locally with l7Proxy=false, and then annotate a pod with visibility annotation