Skip to content

Conversation

jrajahalme
Copy link
Member

Once this PR is merged, a GitHub action will update the labels of these PRs:

 32973

[ 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>
@jrajahalme jrajahalme added kind/backports This PR provides functionality previously merged into master. backport/1.15 This PR represents a backport for Cilium 1.15.x of a PR that was merged to main. labels Oct 31, 2024
@jrajahalme jrajahalme requested a review from a team as a code owner October 31, 2024 15:58
[ 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>
@jrajahalme jrajahalme force-pushed the proxy-persist-proxy-ports-1.15 branch from e81ea2d to 054fda9 Compare November 2, 2024 13:09
@jrajahalme
Copy link
Member Author

/test-backport-1.15

@jrajahalme jrajahalme enabled auto-merge (rebase) November 6, 2024 09:57
@jrajahalme jrajahalme merged commit 78684dc into cilium:v1.15 Nov 6, 2024
59 checks passed
@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 6, 2024
@gandro
Copy link
Member

gandro commented Nov 6, 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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants