Skip to content
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

metrics: cleans k8s iperf deployment when the test finishes. #8542

Merged

Conversation

dborquez
Copy link
Contributor

@dborquez dborquez commented Dec 1, 2023

This PR fixes small issues like:

  1. Cleaning up the k8s environment by removing the iperf test implementation even when the test fails.
  2. Checks if the workload returned a result before generating an empty results json file as it was bein done.
  3. Removes the redundancy of calls to functions that process subtests and should compose the results json file only when all results are ready and not before.
  4. The tcp service manifest was added to the server deployment which targets TCP port 5201.

Fixes: #8534

@katacontainersbot katacontainersbot added the size/large Task of significant size label Dec 1, 2023
@dborquez dborquez added no-backport-needed area/metrics Issues impacting metrics ok-to-test and removed size/large Task of significant size labels Dec 1, 2023
@dborquez dborquez force-pushed the metrics_fix_deployment_cleaning branch from 8b5c8f0 to db573d4 Compare December 1, 2023 03:20
@katacontainersbot katacontainersbot added the size/large Task of significant size label Dec 1, 2023
@dborquez dborquez force-pushed the metrics_fix_deployment_cleaning branch from db573d4 to c3f633c Compare December 1, 2023 03:51
@dborquez dborquez requested a review from GabyCT December 1, 2023 03:54
@dborquez
Copy link
Contributor Author

dborquez commented Dec 1, 2023

/test


trap remove_tmp_file EXIT
if [ -z "${bandwidth_result}" ] || [ -z "${jitter_result}" ] || [ -z "${cpu_result}" ] || [ -z "${parallel_result}" ]; then
die "iperf has not results to save"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggested rewording: iperf couldn't find any results to save

metrics_json_end_array "Results"
fi
metrics_json_add_array_element "${json}"
metrics_json_end_array "Results"
}

function iperf3_cpu() {
# Start server
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this comment still relevant in this part of the function? iiuc, the iperf server is started in the iperf3_start_deployment function. I see this same comment in other functions.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we can't get rid of this comment, thanks @chavafg

@dborquez dborquez force-pushed the metrics_fix_deployment_cleaning branch from c3f633c to 8fd0eb7 Compare December 7, 2023 23:13
@dborquez
Copy link
Contributor Author

dborquez commented Dec 7, 2023

/test

@dborquez dborquez requested a review from chavafg December 7, 2023 23:15
This PR fixes small issues like:
1. Cleaning up the k8s environment by removing the iperf test
implementation even when the test fails.
2. Checks if the workload returned a result before generating
an empty results json file as it was bein done.
3. Removes the redundancy of calls to functions that process
subtests and should compose the results json file only when
all results are ready and not before.
4. The tcp service manifest was added to the server deployment
which targets TCP port 5201.

Fixes: kata-containers#8534

Signed-off-by: David Esparza <david.esparza.borquez@intel.com>
A prerequisite for measuring kata network bandwidth is
run Iperf3 tool at a the transport layer provided by a
k8s service for exposing a network where the clients
inside the cluster can use to contact Pods in the service.

Signed-off-by: David Esparza <david.esparza.borquez@intel.com>
@dborquez dborquez force-pushed the metrics_fix_deployment_cleaning branch from 8fd0eb7 to b257700 Compare December 8, 2023 00:33
@dborquez
Copy link
Contributor Author

dborquez commented Dec 8, 2023

/test

@dborquez dborquez merged commit 584a26d into kata-containers:main Dec 11, 2023
168 of 173 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/metrics Issues impacting metrics ok-to-test size/large Task of significant size
Projects
None yet
Development

Successfully merging this pull request may close these issues.

metrics: prevent iperf from creating an empty results file.
5 participants