cloud-bulldozer / benchmark-operator

The Chuck Norris of cloud benchmarks
Apache License 2.0
282 stars 128 forks source link

SCTP support for pod2service scenarios in uperf workloads #734

Closed josecastillolema closed 2 years ago

josecastillolema commented 2 years ago

Description

Besides tcp and udp, we have added sctp support for uperf workloads service scenarios.

Fixes

josecastillolema commented 2 years ago

We did not create a test for it because SCTP module needs to be preloaded in the kernel.

codecov-commenter commented 2 years ago

Codecov Report

Merging #734 (5e8f401) into master (68671d1) will decrease coverage by 0.30%. The diff coverage is n/a.

@@            Coverage Diff             @@
##           master     #734      +/-   ##
==========================================
- Coverage   53.47%   53.17%   -0.31%     
==========================================
  Files           8        8              
  Lines         331      331              
==========================================
- Hits          177      176       -1     
- Misses        154      155       +1     
Flag Coverage Δ
gha 53.17% <ø> (-0.31%) :arrow_down:
python-3.9 53.17% <ø> (-0.31%) :arrow_down:
unit 53.17% <ø> (-0.31%) :arrow_down:

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
cli/ripsaw/clients/k8s.py 90.00% <0.00%> (-1.12%) :arrow_down:

Help us with your feedback. Take ten seconds to tell us how you rate us.

stale[bot] commented 2 years ago

Is this still relevant? If so, what is blocking it? Is there anything you can do to help move it forward?

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

rsevilla87 commented 2 years ago

@josecastillolema seems like with the latest changes this one needs some rebase

josecastillolema commented 2 years ago

I understand bats is being used to run the k8s e2e tests. Would it be possible (or even worth it) to enable SCTP on the e2e cluster to include SCTP tests? We have to take into consideration that the enablement of SCTP requires a reboot of the host.

rsevilla87 commented 2 years ago

I understand bats is being used to run the k8s e2e tests. Would it be possible (or even worth it) to enable SCTP on the e2e cluster to include SCTP tests? We have to take into consideration that the enablement of SCTP requires a reboot of the host.

Yeah, it would be nice to have an e2e test for this scenario, uperf tests are specified at https://github.com/cloud-bulldozer/benchmark-operator/blob/master/e2e/013-uperf.bats, it will be required to add your testcase there.

In the meantime, Im gonna enable sctp in the CI cluster, I'll let you know when ready.

rsevilla87 commented 2 years ago

Hey @josecastillolema, just finished enabling SCTP in the CI cluster, feel free to add the test case now.

josecastillolema commented 2 years ago

Thanks @rsevilla87 ! About the tests, I was thinking in including them here: https://github.com/cloud-bulldozer/benchmark-operator/blob/master/tests/test_crs/valid_uperf_serviceip.yaml#L34, is that ok?

rsevilla87 commented 2 years ago

Thanks @rsevilla87 ! About the tests, I was thinking in including them here: https://github.com/cloud-bulldozer/benchmark-operator/blob/master/tests/test_crs/valid_uperf_serviceip.yaml#L34, is that ok?

The tests directory is deprecated and just used by benchmark-wrapper. If you want to add a new test case you have to update https://github.com/cloud-bulldozer/benchmark-operator/blob/master/e2e/013-uperf.bats and add your new CR file to https://github.com/cloud-bulldozer/benchmark-operator/tree/master/e2e/uperf

josecastillolema commented 2 years ago

Seems like the e2e test passed w/o issues, LGTM!

BTW, I guess that this PR is also adding sctp support to the pod2pod scenario, doesn't it?

We are about to find out. @rsevilla87 can u please approve the workflow to see if pod2pod tests pass as well?