feat: improve network performance test to make it reusable and modifiable
What type of PR is this?
/kind feature
What this PR does / why we need it:
This pull request includes significant updates to the network/benchmarks/netperf directory, focusing on improving the build process, enhancing test capabilities, and updating dependencies. The most important changes include modifying the Makefile to streamline Docker builds and test execution, updating the go.mod file with new dependencies, and adding new utility libraries for Kubernetes interactions.
Build and Test Improvements:
network/benchmarks/netperf/Makefile: Updated the Docker build process to use dynamic repository and image tags, added atesttarget for running Go tests, and introduced alocaltesttarget for local test execution. [1] [2] [3]
Dependency Updates:
network/benchmarks/netperf/go.mod: Updated module path and dependencies to more recent versions, includingk8s.io/api,k8s.io/apimachinery, andk8s.io/client-go.
New Utility Libraries:
network/benchmarks/netperf/lib/outputlib.go: Added functions for retrieving logs and data from Kubernetes pods and processing raw data into output files.network/benchmarks/netperf/lib/testlib.go: Introduced a new library for handling test parameters, performing tests, and managing Kubernetes resources.network/benchmarks/netperf/lib/utilslib.go: Added utility functions for setting up Kubernetes clients, creating services, and managing replication controllers.
Which issue(s) this PR fixes:
Fixes #2876
Hi @ritwikranjan. Thanks for your PR.
I'm waiting for a kubernetes member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.
Once the patch is verified, the new status will be reflected by the ok-to-test label.
I understand the commands that are listed here.
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.
Hey @aojea, Can you take a look and let me know if it can be improved in any way?
@wojtek-t do you know if this code is being used somewhere? I honestly can't remember and I will not have time until December to check on this, sorry
Hi @aojea, Would you be able to take a look at these changes now?
We need
- identify where are these tests running
- split the PR in independent commits, so it can be easy to review, the modification of the makefile should be independent of the commit that refactors the test code, ...
- explain how we probe these works (maybe we just need to know .1) and we merge and iterate, that is fine with me too
Okay, sounds good to me, I'll go ahead and split into commits.
[APPROVALNOTIFIER] This PR is NOT APPROVED
This pull-request has been approved by: ritwikranjan Once this PR has been reviewed and has the lgtm label, please assign aojea for approval. For more information see the Code Review Process.
The full list of commands accepted by this bot can be found here.
Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment
Hi @aojea, is there anything else I can do to better this?
We need
- identify where are these tests running
- split the PR in independent commits, so it can be easy to review, the modification of the makefile should be independent of the commit that refactors the test code, ...
- explain how we probe these works (maybe we just need to know .1) and we merge and iterate, that is fine with me too
missing step .1 and 3. , how can we sure this does not break anything?
@aojea For 1 & 3, I have not found any explicit place where these tests are used directly however I have made sure keep backward compatibility, all the targets in Makefile still give the same output and the older images of nptest also work in the same way. Whatever changes are made are either using extra flag (such as -json) or by keeping the api definitions intact. Now the proof that the changes work, we are currently using a fork Azure/perf-tests for testing an open source project Retina. Let me know if you want details of the implementations for the same. If you are aware of any other usage of this tool do let me know and i'll do test out those as well.
/ok-to-test
do you think you can add a manual job in https://github.com/kubernetes/test-infra so we can at least now if it works?
/retest
@aojea I have addressed the failing linter tests.
I will add a couple of scripts in kubernetes/test-infra in experiment directory in order to verify both the cli as well the api. Thanks for taking the time to look into this.
/retest
Hi @aojea, anything else we want to consider here? Would like me to add the tests to test-infra first before merging or can I do it after?
Would like me to add the tests to test-infra first before merging or can I do it after?
if we add after we don't know if it works, I really think it will be useful to have signal on this before merge if we can
Hi @aojea , I have added a script: kubernetes/test-infra#34222
The Kubernetes project currently lacks enough contributors to adequately respond to all PRs.
This bot triages PRs according to the following rules:
- After 90d of inactivity,
lifecycle/staleis applied - After 30d of inactivity since
lifecycle/stalewas applied,lifecycle/rottenis applied - After 30d of inactivity since
lifecycle/rottenwas applied, the PR is closed
You can:
- Mark this PR as fresh with
/remove-lifecycle stale - Close this PR with
/close - Offer to help out with Issue Triage
Please send feedback to sig-contributor-experience at kubernetes/community.
/lifecycle stale
The Kubernetes project currently lacks enough active contributors to adequately respond to all PRs.
This bot triages PRs according to the following rules:
- After 90d of inactivity,
lifecycle/staleis applied - After 30d of inactivity since
lifecycle/stalewas applied,lifecycle/rottenis applied - After 30d of inactivity since
lifecycle/rottenwas applied, the PR is closed
You can:
- Mark this PR as fresh with
/remove-lifecycle rotten - Close this PR with
/close - Offer to help out with Issue Triage
Please send feedback to sig-contributor-experience at kubernetes/community.
/lifecycle rotten
The Kubernetes project currently lacks enough active contributors to adequately respond to all issues and PRs.
This bot triages PRs according to the following rules:
- After 90d of inactivity,
lifecycle/staleis applied - After 30d of inactivity since
lifecycle/stalewas applied,lifecycle/rottenis applied - After 30d of inactivity since
lifecycle/rottenwas applied, the PR is closed
You can:
- Reopen this PR with
/reopen - Mark this PR as fresh with
/remove-lifecycle rotten - Offer to help out with Issue Triage
Please send feedback to sig-contributor-experience at kubernetes/community.
/close
@k8s-triage-robot: Closed this PR.
In response to this:
The Kubernetes project currently lacks enough active contributors to adequately respond to all issues and PRs.
This bot triages PRs according to the following rules:
- After 90d of inactivity,
lifecycle/staleis applied- After 30d of inactivity since
lifecycle/stalewas applied,lifecycle/rottenis applied- After 30d of inactivity since
lifecycle/rottenwas applied, the PR is closedYou can:
- Reopen this PR with
/reopen- Mark this PR as fresh with
/remove-lifecycle rotten- Offer to help out with Issue Triage
Please send feedback to sig-contributor-experience at kubernetes/community.
/close
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.