cilium-cli: extend no-interrupted-connections to test NodePort from outside #37294

ysksuzuki · 2025-01-28T05:47:51Z

This PR extends the conn-disrupt-test to ensure that external connections through a NodePort are not disrupted.

The test deploys a conn-disrupt-test server with a single replica in the cluster and multiple clients on an external node.
Clients establish long-lived TCP connections with the server using a NodePort. Each client selects a node where the backend is running, and one where the backend is not running. The clients send requests to both nodes using IPv4 and IPv6 addresses.

After upgrade, the test checks the restart counters, and compares them with the counters stored before the upgrade. A mismatch indicates that a connection was interrupted.

Example output from kubectl -n cilium-test-1 get po -o wide:

NAME                                                              READY   STATUS    RESTARTS   AGE     IP             NODE
test-conn-disrupt-client-backend-node-ipv4-internalip-7fc97pbp9   1/1     Running   0          34s     172.18.0.2     kind-worker3
test-conn-disrupt-client-backend-node-ipv6-internalip-66bdtmg2z   1/1     Running   0          34s     172.18.0.2     kind-worker3
test-conn-disrupt-client-non-backend-node-ipv4-internalip-d6bns   1/1     Running   0          34s     172.18.0.2     kind-worker3
test-conn-disrupt-client-non-backend-node-ipv6-internalip-4x77p   1/1     Running   0          34s     172.18.0.2     kind-worker3
test-conn-disrupt-server-ns-traffic-5cc46556fb-bm9lz              1/1     Running   0          37s     10.244.2.173   kind-worker2

The conn-disrupt-test for NS traffic runs in the following workflows

ci-e2e-upgrade
ci-ipsec-upgrade
ci-ipsec-e2

It will be skipped in the following because the feature node-without-cilium is disabled (The one for EW traffic will run)

conformance-eks
ci-clustermesh upgrade
ci-ces-migrate

Note that ci-e2e-upgrade and ci-ipsec-upgrade become unstable if we enable no-interrupted-connections for NS traffic test. Will skip these conditions (lb-acceleration and ipsec + KPR + ipv6) for now, and investigate the cause as follow-up.

ci-e2e-upgrade (lb-acceleration enabled)
https://github.com/cilium/cilium/actions/runs/12984833400/job/36208601353

  ❌ Found unexpected packet drops:
{
  "labels": {
    "direction": "EGRESS",
    "reason": "FIB lookup failed"
  },
  "name": "cilium_drop_count_total",
  "value": 1
}

ci-ipsec-upgrade (ipsec + KPR + ipv6)
https://github.com/cilium/cilium/actions/runs/12982469981/job/36203776562

  [-] Scenario [no-interrupted-connections/no-interrupted-connections]
  🟥 Pod test-conn-disrupt-client-non-backend-node-ipv6-internalip-9rp69 flow was interrupted (restart count does not match 0 != 1)

Fixes: #13530

ysksuzuki · 2025-01-28T06:13:33Z

/test

ysksuzuki · 2025-01-28T07:57:24Z

/test

joestringer · 2025-01-29T00:55:38Z

Fly-by comment, but I see this is making changes to test "from outside" which can often be a source of unreliable test results in CI runs (eg, passes sometimes but fails sometimes). I think it would be worthwhile to heavily stress these changes by running CI many times in order to demonstrate that the test changes have high reliability.

ysksuzuki · 2025-01-29T04:29:15Z

/test

ysksuzuki · 2025-01-29T09:19:06Z

Looks like ci-e2e-upgrade failed due to a flake. opened #37325

YutaroHayakawa

LGTM. Some nits.

cilium-cli/connectivity/check/deployment.go

…utside This commit extends the conn-disrupt-test to ensure that external connections through a NodePort are not disrupted. The test deploys a conn-disrupt-test server with a single replica in the cluster and multiple clients on an external node. Clients establish long-lived TCP connections with the server using a NodePort. Each client selects a node where the backend is running, and one where the backend is not running. The clients send requests to both nodes using IPv4 and IPv6 addresses. After upgrade, the test checks the restart counters, and compares them with the counters stored before the upgrade. A mismatch indicates that a connection was interrupted. Example output from `kubectl -n cilium-test-1 get po -o wide`: ``` NAME READY STATUS RESTARTS AGE IP NODE test-conn-disrupt-client-backend-node-ipv4-internalip-7fc97pbp9 1/1 Running 0 34s 172.18.0.2 kind-worker3 test-conn-disrupt-client-backend-node-ipv6-internalip-66bdtmg2z 1/1 Running 0 34s 172.18.0.2 kind-worker3 test-conn-disrupt-client-non-backend-node-ipv4-internalip-d6bns 1/1 Running 0 34s 172.18.0.2 kind-worker3 test-conn-disrupt-client-non-backend-node-ipv6-internalip-4x77p 1/1 Running 0 34s 172.18.0.2 kind-worker3 test-conn-disrupt-server-ns-traffic-5cc46556fb-bm9lz 1/1 Running 0 37s 10.244.2.173 kind-worker2 ``` Signed-off-by: Yusuke Suzuki <yusuke.suzuki@isovalent.com>

include-conn-disrupt-test-ns-traffic is false by default, and we need to explicitly set it to true to run the conn-disrupt-test for NS traffic. This is to prevent CI from disrupting when updating cilium-cli on the stable branch. Also, this is intended to provide a way to opt out if this test causes instability in CI. Signed-off-by: Yusuke Suzuki <yusuke.suzuki@isovalent.com>

lb-acceleration is enabled or ipsec and KPR is enabled. ci-e2e-upgrade and ci-ipsec-upgrade become unstable with those settings. Skip it for now. Signed-off-by: Yusuke Suzuki <yusuke.suzuki@isovalent.com>

This commit enables conn-disrupt-test for NS traffic in ci-e2e-upgrade, ci-ipsec-upgrade and ci-ipsec-e2e. It will be skipped in conformance-eks, ci-clustermesh upgrade and ci-ces-migrate because the feature node-without-cilium is disabled. (The one for EW traffic will run) Signed-off-by: Yusuke Suzuki <yusuke.suzuki@isovalent.com>

ysksuzuki · 2025-01-30T07:33:21Z

/test

julianwiedmann · 2025-01-30T08:54:13Z

The conn-disrupt-test for NS traffic runs in the following workflows
* ci-e2e-upgrade

* ci-ipsec-upgrade

* ci-ipsec-e2

Just to emphasize - the IPsec workflows would then currently run this test in any configuration where KPR is switched off, correct?

julianwiedmann · 2025-01-30T09:04:32Z

Fly-by comment, but I see this is making changes to test "from outside" which can often be a source of unreliable test results in CI runs (eg, passes sometimes but fails sometimes). I think it would be worthwhile to heavily stress these changes by running CI many times in order to demonstrate that the test changes have high reliability.

@joestringer At least for the kind-based CI parts this should be not a big concern - the "outside" entity is really just another kind node that's not part of the cluster. We use the same approach in the EgressGW tests for instance, and I'm not aware that it's adding much flakiness to the results.

But ack on the overall sentiment - I like that the PR is enabling this new test very selectively, and for instance we're not exposing older Cilium releases to it when bumping the CLI version in CI.

ysksuzuki · 2025-01-30T09:12:36Z

the IPsec workflows would then currently run this test in any configuration where KPR is switched off, correct?

Yes, that is correct.

cilium-cli/connectivity/check/context.go

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 Jan 28, 2025

github-actions bot added the cilium-cli This PR contains changes related with cilium-cli label Jan 28, 2025

ysksuzuki added the release-note/ci This PR makes changes to the CI. label Jan 28, 2025

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 Jan 28, 2025

ysksuzuki mentioned this pull request Jan 28, 2025

cilium-cli: extend no-interrupted-connections to test NodePort from outside #37239

Closed

ysksuzuki force-pushed the pr/ysksuzuki/test-conn-disrupt-ns-traffic branch from b9785bf to 341684d Compare January 28, 2025 07:29

ysksuzuki force-pushed the pr/ysksuzuki/test-conn-disrupt-ns-traffic branch 2 times, most recently from 0c0ea09 to 620378f Compare January 29, 2025 04:09

ysksuzuki marked this pull request as ready for review January 29, 2025 11:18

ysksuzuki requested review from a team as code owners January 29, 2025 11:18

ysksuzuki requested review from youngnick, YutaroHayakawa, brlbil and asauber January 29, 2025 11:18

YutaroHayakawa approved these changes Jan 30, 2025

View reviewed changes

cilium-cli/connectivity/check/deployment.go Outdated Show resolved Hide resolved

cilium-cli/connectivity/check/deployment.go Outdated Show resolved Hide resolved

ysksuzuki added 3 commits January 30, 2025 16:12

cilium-cli: skip conn-disrupt-test for NS traffic when

eed221e

lb-acceleration is enabled or ipsec and KPR is enabled. ci-e2e-upgrade and ci-ipsec-upgrade become unstable with those settings. Skip it for now. Signed-off-by: Yusuke Suzuki <yusuke.suzuki@isovalent.com>

ysksuzuki force-pushed the pr/ysksuzuki/test-conn-disrupt-ns-traffic branch from 620378f to 24ffc76 Compare January 30, 2025 07:13

ysksuzuki force-pushed the pr/ysksuzuki/test-conn-disrupt-ns-traffic branch from 24ffc76 to 22fdd9a Compare January 30, 2025 07:21

brlbil approved these changes Jan 30, 2025

View reviewed changes

julianwiedmann added area/loadbalancing Impacts load-balancing and Kubernetes service implementations upgrade-impact This PR has potential upgrade or downgrade impact. labels Jan 30, 2025

youngnick approved these changes Feb 4, 2025

View reviewed changes

asauber reviewed Feb 4, 2025

View reviewed changes

cilium-cli/connectivity/check/context.go Show resolved Hide resolved

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 Feb 4, 2025

joestringer enabled auto-merge February 4, 2025 15:44

asauber approved these changes Feb 5, 2025

View reviewed changes

joestringer added this pull request to the merge queue Feb 5, 2025

Merged via the queue into main with commit 820a62d Feb 5, 2025
290 checks passed

joestringer deleted the pr/ysksuzuki/test-conn-disrupt-ns-traffic branch February 5, 2025 09:21

ysksuzuki mentioned this pull request Feb 11, 2025

CI: ci-ipsec-upgrade - Pod test-conn-disrupt-client-non-backend-node-ipv6-internalip-9rp69 flow was interrupted #37540

Closed

julianwiedmann mentioned this pull request Feb 12, 2025

Enable N/S no-interrupted-connections on stable branches #37542

Closed

joestringer mentioned this pull request Feb 20, 2025

chore(deps): update dependency cilium/cilium-cli to v0.17.0 #37782

Closed

github-actions bot mentioned this pull request Feb 24, 2025

Bump ciliumcli from v0.16.0 to v0.18.3 kokyhm/kubespray_dependency_upgrade_test#153

Open

julianwiedmann mentioned this pull request Mar 24, 2025

[v1.17] cilium-cli: extend no-interrupted-connections to test NodePort from outside #37797

Merged

1 task

github-actions bot added the backport-done/1.17 The backport for Cilium 1.17.x for this PR is done. label Mar 25, 2025

julianwiedmann added backport/author The backport will be carried out by the author of the PR. needs-backport/1.16 This PR / issue needs backporting to the v1.16 branch and removed needs-backport/1.16 This PR / issue needs backporting to the v1.16 branch labels Apr 3, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

cilium-cli: extend no-interrupted-connections to test NodePort from outside #37294

cilium-cli: extend no-interrupted-connections to test NodePort from outside #37294

Uh oh!

ysksuzuki commented Jan 28, 2025 •

edited

Loading

Uh oh!

ysksuzuki commented Jan 28, 2025

Uh oh!

ysksuzuki commented Jan 28, 2025

Uh oh!

joestringer commented Jan 29, 2025

Uh oh!

ysksuzuki commented Jan 29, 2025

Uh oh!

ysksuzuki commented Jan 29, 2025

Uh oh!

YutaroHayakawa left a comment •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

ysksuzuki commented Jan 30, 2025

Uh oh!

julianwiedmann commented Jan 30, 2025

Uh oh!

julianwiedmann commented Jan 30, 2025

Uh oh!

ysksuzuki commented Jan 30, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

cilium-cli: extend no-interrupted-connections to test NodePort from outside #37294

cilium-cli: extend no-interrupted-connections to test NodePort from outside #37294

Uh oh!

Conversation

ysksuzuki commented Jan 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ysksuzuki commented Jan 28, 2025

Uh oh!

ysksuzuki commented Jan 28, 2025

Uh oh!

joestringer commented Jan 29, 2025

Uh oh!

ysksuzuki commented Jan 29, 2025

Uh oh!

ysksuzuki commented Jan 29, 2025

Uh oh!

YutaroHayakawa left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

ysksuzuki commented Jan 30, 2025

Uh oh!

julianwiedmann commented Jan 30, 2025

Uh oh!

julianwiedmann commented Jan 30, 2025

Uh oh!

ysksuzuki commented Jan 30, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ysksuzuki commented Jan 28, 2025 •

edited

Loading

YutaroHayakawa left a comment •

edited

Loading