Extend cilium-cli connectivity perf to allow testing egress gateway performance #37748
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
5c3997d to
4ef7d9b
Compare
|
/test |
75ff51c to
84c62ca
Compare
|
/test |
tklauser
approved these changes
Feb 26, 2025
marseel
approved these changes
Feb 26, 2025
Artyop
approved these changes
Feb 26, 2025
Currently, the ConcurrentLogger implementation can panic when stopped
due to the messageCh channel being closed twice. Let's slightly rework
the implementation to ensure that the channel can only be closed once.
Example output of the crash:
^CCancellation request (context canceled) received, cancelling tests...
panic: close of closed channel
goroutine 1 [running]:
github.com/cilium/cilium/cilium-cli/connectivity/check.(*ConcurrentLogger).Stop(0xc000a74320)
.../cilium/cilium-cli/connectivity/check/logger.go:56 +0x1e
github.com/cilium/cilium/cilium-cli/cli.newCmdConnectivityPerf.RunE.func2(0xc0008fb808, {0x453e450?, 0x4?, 0x453e310?})
.../cilium/cilium-cli/cli/connectivity.go:106 +0x7f0
github.com/spf13/cobra.(*Command).execute(0xc0008fb808, {0xc000599e40, 0x1, 0x1})
.../cilium/vendor/github.com/spf13/cobra/command.go:985 +0xaaa
github.com/spf13/cobra.(*Command).ExecuteC(0xc0002aac08)
.../cilium/vendor/github.com/spf13/cobra/command.go:1117 +0x3ff
github.com/spf13/cobra.(*Command).Execute(0x624ecd0?)
.../cilium/vendor/github.com/spf13/cobra/command.go:1041 +0x13
main.main()
.../cilium/cilium-cli/cmd/cilium/main.go:22 +0xc5
Signed-off-by: Marco Iorio <marco.iorio@isovalent.com>
Hubble is not leverage by performance tests, and the corresponding flag is not even registered. Hence, ensure that it is disabled, so that the CLI does not attempt to contact hubble relay, emitting warnings in case it fails. Signed-off-by: Marco Iorio <marco.iorio@isovalent.com>
Currently, the perf tests panic when executed against a cluster with at least one of the nodes not running Cilium. The reason is that the common logic expects host-netns pods to be running on these nodes, which are instead not deployed when running in perf mode. Let's fix this by disabling the NodeWithoutCilium feature in this mode to turn off the extra logic, given that it is not relevant in this context. Signed-off-by: Marco Iorio <marco.iorio@isovalent.com>
The client does not expose any ports, hence it does not make sense to configure it in the container definition. Signed-off-by: Marco Iorio <marco.iorio@isovalent.com>
Configure the server to directly run in foreground, rather than sleeping forever. Additionally, change the container port to actually reflect the one used by netserver. Signed-off-by: Marco Iorio <marco.iorio@isovalent.com>
Currently, performance tests are always run both with client and server hosted on the same node, and with the client hosted on a different node. Let's add two new flags to allow selectively enabling only one of the two modes if desired. The default values preserve the current behavior. Signed-off-by: Marco Iorio <marco.iorio@isovalent.com>
Currently, performance tests can be run for the pod-to-pod, host-to-host and possibly mixed scenarios. Let's extend this to allow running mixed configurations only (e.g., pod-to-host), without necessarily having to run all the others. Signed-off-by: Marco Iorio <marco.iorio@isovalent.com>
Extend the granularity of the node selectors configurable for the performance test pods, allowing to separately specify the selector associated with the client and server pods. This enables extra flexibility for the cases in which placement really matters. Signed-off-by: Marco Iorio <marco.iorio@isovalent.com>
Extend the performance tests to allow configuring the list of tolerations assigned to the test pods, so that they can be scheduled on tainted nodes as well (e.g., to ensure that no other pod gets scheduled there). Signed-off-by: Marco Iorio <marco.iorio@isovalent.com>
Add the possibility of specifying a configurable delay before starting the performance measurements, in case it is needed for state propagation. For instance, this can be useful in the egress gateway context, to give enough time for the datapath to be configured accordingly. Signed-off-by: Marco Iorio <marco.iorio@isovalent.com>
84c62ca to
5e4ebc2
Compare
|
Rebased onto main to fix a conflict. |
ghost
approved these changes
Feb 26, 2025
|
/test |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Please review commit by commit, and refer to the individual commit messages for additional details.