benchmark-wrapper icon indicating copy to clipboard operation
benchmark-wrapper copied to clipboard

Inconsistent Benchmark and Quay Image Names

Open learnitall opened this issue 3 years ago • 10 comments

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?

learnitall avatar Sep 10 '21 15:09 learnitall

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

jtaleric avatar Sep 10 '21 15:09 jtaleric

+1 to consistent naming :-)

dry923 avatar Sep 10 '21 15:09 dry923

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 avatar Sep 10 '21 15:09 learnitall

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.

learnitall avatar Sep 17 '21 12:09 learnitall

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 avatar Nov 29 '21 10:11 chentex

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

  • https://github.com/cloud-bulldozer/benchmark-operator/pull/715
  • #398

chentex avatar Nov 29 '21 12:11 chentex

Oh fantastic! I'm taking a look now!

learnitall avatar Nov 30 '21 04:11 learnitall

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.

learnitall avatar Nov 30 '21 04:11 learnitall

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.

chentex avatar Nov 30 '21 09:11 chentex

Ah ok, sounds good!

learnitall avatar Nov 30 '21 17:11 learnitall