-
Notifications
You must be signed in to change notification settings - Fork 3.4k
Proxy persist proxy ports 1.15 #35684
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
Merged
jrajahalme
merged 6 commits into
cilium:v1.15
from
jrajahalme:proxy-persist-proxy-ports-1.15
Nov 6, 2024
Merged
Proxy persist proxy ports 1.15 #35684
jrajahalme
merged 6 commits into
cilium:v1.15
from
jrajahalme:proxy-persist-proxy-ports-1.15
Nov 6, 2024
Conversation
This file contains hidden or 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
[ upstream commit 00b7717 ] Clear proxy port on failure so that we'll try another random port next time. We have two failure cases for creating new redirects: - syncronous, for DNS proxy. Simply zero the proxy port in the retry loop. - asynchronous, for Envoy proxy. Clear proxy port state on revert callback. Noticed the need for this change when accidentally trying to use port 1 for Envoy when developing another feature. In this case Envoy can't bind the port, and all further tries were also trying the same port, as the proxy port was not cleared on revert. Signed-off-by: Jarno Rajahalme <jarno@isovalent.com>
[ upstream commit bfbd3d6 ] "rulesPort" is non-zero when we have a datapath (iptables) rules with a specific port. Try re-creating a redirect with that port if no other port is configured. This reduces churn on the datapath, as the existing rule can be reused without a need for an update. Signed-off-by: Jarno Rajahalme <jarno@isovalent.com>
[ upstream commit ee1cf1c ] Rename AllocateProxyPort as AllocateCRDProxyPort to make it clear that it is only to be used for CEC/CCEC CRD listeners. Also enforce the current practice where CRD proxy ports are only accessed by name, and 'ingress' member is always false for CRD ports. Fix the unit test to verify that the proxy port was not reallocated for the same listener name. Signed-off-by: Jarno Rajahalme <jarno@isovalent.com>
[ upstream commit 8149af7 ] We now only ever have 'localOnly = true' so that we always bind the proxy listeners to localhost address. Remove support for all-hosts addresses. Signed-off-by: Jarno Rajahalme <jarno@isovalent.com>
[ upstream commit d11e4d2 ] Previously we only reused the DNS proxy port from the datapath. Change to reuse the old proxy ports for all proxy redirects from datapath, if possible. This reduces Listener churn on daemonset Envoy on agent restart. With this change the listener update in Envoy logs after agent restart looks like this: begin add/update listener: name=cilium-http-egress:13563 hash=11560876577076369351 duplicate/locked listener 'cilium-http-egress:13563'. no add/update lds: add/update listener 'cilium-http-egress:13563' skipped Signed-off-by: Jarno Rajahalme <jarno@isovalent.com>
[ upstream commit 9f210b5 ] Write proxy ports into /var/cilium/state/proxy_ports_state.json and restore from there. If the file is not available, only then restore from iptables rules. Restoration from iptables is still needed for upgrades. This fixes the issue with bpf TPROXY, as it does not rely on iptables rules, so the proxy ports can not be recovered from them. Note: File IO is patterned after similar methods for cache.CachingIdentityAllocator. Signed-off-by: Jarno Rajahalme <jarno@isovalent.com>
e81ea2d
to
054fda9
Compare
/test-backport-1.15 |
youngnick
approved these changes
Nov 4, 2024
@jrajahalme I think you also need to backport #33230 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
backport/1.15
This PR represents a backport for Cilium 1.15.x of a PR that was merged to main.
kind/backports
This PR provides functionality previously merged into master.
ready-to-merge
This PR has passed all tests and received consensus from code owners to merge.
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.
Once this PR is merged, a GitHub action will update the labels of these PRs: