search

cloud-bulldozer / benchmark-wrapper

Python Library to run benchmarks
https://benchmark-wrapper.readthedocs.io
Apache License 2.0
19 stars 56 forks source link

Inconsistent Benchmark and Quay Image Names #347

Open learnitall opened 3 years ago

learnitall commented 3 years ago

Looking at any of the build results after #344 (i.e. https://github.com/cloud-bulldozer/benchmark-wrapper/actions/runs/1221646689), we have some inconsistent names between the wrappers in our code and the wrapper images up on quay. Additionally, some wrappers are missing image repositories altogether. Here are the inconsistencies that I found:

snafu wrapper module name snafu wrapper tool name (--tool) quay image name
iperf n/a iperf3
ycsb ycsb ycsb-server
fs_drift fs-drift fs-drift
dns_perf dns_perf No repo on quay
upgrade_openshift upgrade No repo on quay

I can add logic into the build_matrix.py script in order to take care of this, or we can do some refactoring of our image names. What do we think?

jtaleric commented 3 years ago

Yes, if this can be scripted to ensure names stay consistent, lets do it!

dry923 commented 3 years ago

+1 to consistent naming :-)

learnitall commented 3 years ago

Wait, just to clarify because I'm not sure what y'all mean: are we modifying build_matrix so that the quay image names stay the same? Or are we modifying the quay image names to be consistent with benchmark-wrapper benchmark names?

learnitall commented 2 years ago

Talked internally a bit. Will perform the following changes to maintain consistent naming:

Might take a couple of steps to do this, as these are pretty breaking, but should be simple enough to get done.

chentex commented 2 years ago

Talked internally a bit. Will perform the following changes to maintain consistent naming:

  • [x] Rename iperf3 image to iperf
  • [x] Rename ycsb-server image to ycsb
  • [x] Rename fs-drift image to fs_drift
  • [x] Rename fs-drift tool to fs_drift
  • [x] Create dns_perf image on quay
  • [x] Create upgrade_openshift image on quay
  • [x] Rename upgrade tool to upgrade_openshift
  • [x] Rename scale tool to scale_openshift
chentex commented 2 years ago

Hey @learnitall I created the following PRs to address this issues:

learnitall commented 2 years ago

Oh fantastic! I'm taking a look now!

learnitall commented 2 years ago

Thanks for putting this together! This is a tough change. Left comments on the PRs- looks like there were some places here and there that were missing an updated name. I think we might want to take a look at e2e as well? I'm pretty sure the the PR for snafu would impact some scripts in there.

Only other change that I can think of is giving our GHA quay bot permissions to the updated image repositories. Looks like we're getting permission denied errors: https://github.com/cloud-bulldozer/benchmark-wrapper/runs/4287869509?check_suite_focus=true

Tomorrow I'll try to do a more thorough check and look through more CI logs to see if any other changes need to be made.

chentex commented 2 years ago

Thanks, we will let those PR as draft for the moment, there is some issues regardign renaming some tools, since this affects also the cli, we want to have input from Joe to see how we proceed.

learnitall commented 2 years ago

Ah ok, sounds good!