8000 cli: bugfix connectivity perf command by marseel · Pull Request #35063 · cilium/cilium · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

cli: bugfix connectivity perf command #35063

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
merged 1 commit into from
Sep 28, 2024
Merged

Conversation

marseel
Copy link
Contributor
@marseel marseel commented Sep 27, 2024

Currently, regular connectivity tests and perf tests are still a bit coupled, but they can't be run concurrently.
In the case of LRP enabled, we were deploying only perf pods, but the validation of deployments was waiting for lrp pod too:

✨ Deploying perf-client deployment...
✨ Deploying perf-client-other-node deployment...
✨ Deploying perf-server deployment...
⌛ Waiting for deployment cilium-test-1/perf-client to become ready...
⌛ Waiting for deployment cilium-test-1/perf-client-other-node to become ready...
⌛ Waiting for deployment cilium-test-1/perf-server to become ready...
⌛ Waiting for deployment cilium-test-1/lrp-client to become ready...
timeout reached waiting for deployment cilium-test-1/lrp-client to become ready

As we have to separate functions for deploying deployments and validating deployments, we need to keep them in sync. Now in validation of deployments, return early in case of perf command, just like in a deploy function.

cli: fix a case when connectivity perf command was hanging if LRP was enabled in the cluster

Currently, regular connectivity tests and perf tests are still a bit
coupled, but they can't be run concurrently.
In case of LRP enabled, we were deploying only perf pods, but validation
of deployments was waiting for lrp pod too.

As we have to separate functions for deploying deployments and
validating deployments, we need to keep them in sync.
Now in validation of deployments, return early in case of perf command,
just like in a deploy function.

Signed-off-by: Marcel Zieba <marcel.zieba@isovalent.com>
@marseel marseel added kind/bug This is a bug in the Cilium logic. kind/performance There is a performance impact of this. cilium-cli This PR contains changes related with cilium-cli cilium-cli-exclusive This PR only impacts cilium-cli binary labels Sep 27, 2024
@maintainer-s-little-helper maintainer-s-little-helper bot added dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. labels Sep 27, 2024
@marseel marseel added the release-note/bug This PR fixes an issue in a previous release of Cilium. label Sep 27, 2024
@maintainer-s-little-helper 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 Sep 27, 2024
@marseel
Copy link
Contributor Author
marseel commented Sep 27, 2024

/test

@marseel marseel marked this pull request as ready for review September 27, 2024 11:56
@marseel marseel requested a review from a team as a code owner September 27, 2024 11:56
@marseel marseel requested a review from christarazi September 27, 2024 11:56
@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 Sep 28, 2024
@borkmann borkmann added this pull request to the merge queue Sep 28, 2024
Merged via the queue into main with commit b06e62e Sep 28, 2024
209 checks passed
@borkmann borkmann deleted the pr/marseel/fix_perf_command branch September 28, 2024 19:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cilium-cli This PR contains changes related with cilium-cli cilium-cli-exclusive This PR only impacts cilium-cli binary kind/bug This is a bug in the Cilium logic. kind/performance There is a performance impact of this. ready-to-merge This PR has passed all tests and received consensus from code owners to merge. release-note/bug This PR fixes an issue in a previous release of Cilium.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants
0