Skip to content

Conversation

dkoshkin
Copy link
Contributor

@dkoshkin dkoshkin commented Sep 9, 2025

What problem does this PR solve?:
Set Cilium's configuration to preserve source IPs from external connections. See https://docs.cilium.io/en/stable/network/kubernetes/kubeproxy-free/#client-source-ip-preservation

Which issue(s) this PR fixes:
Fixes #

How Has This Been Tested?:

Special notes for your reviewer:

@jimmidyson
Copy link
Member

I wonder if we can add a test that checks that source IP is actually being preserved with this configuration?

@dkoshkin
Copy link
Contributor Author

Here is the diff of the ConfigMap used by Cilium after applying kube-proxy replacement

diff cm-pre.yaml cm-after.yaml 
11a12
>   bpf-lb-dsr-dispatch: geneve
13a15
>   bpf-lb-mode: dsr
63d64
<   enable-node-port: "false"
99c100
<   kube-proxy-replacement: "false"
---
>   kube-proxy-replacement: "true"
141c142
<   tunnel-protocol: vxlan
---
>   tunnel-protocol: geneve
159c160
<   resourceVersion: "570"
---
>   resourceVersion: "2378"

Tested this with:

kubectl run nginx --image=docker.io/library/nginx:latest --labels app=nginx
kubectl create service loadbalancer nginx --tcp=80:80 
curl $(kubectl get svc nginx -ojsonpath='{.status.loadBalancer.ingress[0].ip}')

Notice how the client IP changes after enabling:

$ kubectl logs nginx
...
192.168.1.141 - - [10/Sep/2025:15:58:54 +0000] "GET / HTTP/1.1" 200 615 "-" "curl/8.7.1" "-"
10.22.24.127 - - [10/Sep/2025:16:00:20 +0000] "GET / HTTP/1.1" 200 615 "-" "curl/8.7.1" "-"

@yanhua121
Copy link
Contributor

I wonder if we can add a test that checks that source IP is actually being preserved with this configuration?

+1

@dkoshkin
Copy link
Contributor Author

I wonder if we can add a test that checks that source IP is actually being preserved with this configuration?

+1

@jimmidyson @yanhua121 please see #1304, I will rebase and extend the test in this PR after the other changes are merged.

@dkoshkin dkoshkin force-pushed the dkoshkin/feat-enable-ip-preservation branch 7 times, most recently from 33ab351 to a9998c3 Compare September 17, 2025 15:17
@dkoshkin dkoshkin force-pushed the dkoshkin/feat-enable-ip-preservation branch 2 times, most recently from 3d343fe to f5f4bb9 Compare September 24, 2025 21:54
dkoshkin added a commit that referenced this pull request Sep 25, 2025
**What problem does this PR solve?**:
Make the migration process from kube-proxy to Cilium's kube-proxy
replacement more resilient.
Just setting `kubeProxyReplacement: true` is not enough for the Cilium
operator to restart the DaemonSet Pods and pickup the new configuration.
Instead of relying on `k8sServiceHost` to cause a rollout, this change
forces a rollout during the migration process.
This also fixes a potential race where the Cilium DaemonSet wait
returned early and delete kube-proxy before all the Pods were restarted.

Another fix here is that this whole migration process is now safer and
only done once when kube-proxy is installed.

Pulled out from
#1295

**Which issue(s) this PR fixes**:
Fixes #

**How Has This Been Tested?**:
<!--
Please describe the tests that you ran to verify your changes.
Provide output from the tests and any manual steps needed to replicate
the tests.
-->

**Special notes for your reviewer**:
<!--
Use this to provide any additional information to the reviewers.
This may include:
- Best way to review the PR.
- Where the author wants the most review attention on.
- etc.
-->

---------

Co-authored-by: Jimmi Dyson <[email protected]>
@dkoshkin dkoshkin force-pushed the dkoshkin/feat-enable-ip-preservation branch from f5f4bb9 to 0edbe03 Compare September 25, 2025 13:46
@dkoshkin dkoshkin force-pushed the dkoshkin/feat-enable-ip-preservation branch from 0edbe03 to cd63f74 Compare September 26, 2025 03:34
@dkoshkin dkoshkin merged commit 49ba749 into main Oct 1, 2025
22 checks passed
@dkoshkin dkoshkin deleted the dkoshkin/feat-enable-ip-preservation branch October 1, 2025 00:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants