benchmark-wrapper
benchmark-wrapper copied to clipboard
Inconsistent Benchmark and Quay Image Names
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?
Yes, if this can be scripted to ensure names stay consistent, lets do it!
+1 to consistent naming :-)
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?
Talked internally a bit. Will perform the following changes to maintain consistent naming:
- Rename iperf3 image to iperf
- Rename ycsb-server image to ycsb
- Rename fs-drift image to fs_drift
- Rename fs-drift tool to fs_drift
- Create dns_perf image on quay
- Create upgrade_openshift image on quay
- Rename upgrade tool to upgrade_openshift
- Rename scale tool to scale_openshift
Might take a couple of steps to do this, as these are pretty breaking, but should be simple enough to get done.
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
Hey @learnitall I created the following PRs to address this issues:
- https://github.com/cloud-bulldozer/benchmark-operator/pull/715
- #398
Oh fantastic! I'm taking a look now!
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.
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.
Ah ok, sounds good!