-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
Adding version overrides ENVs for e2e and individual tools #46763
Adding version overrides ENVs for e2e and individual tools #46763
Conversation
eefef5a
to
1ee2018
Compare
git clone https://github.com/cloud-bulldozer/e2e-benchmarking --depth=1 | ||
REPO_URL="https://github.com/cloud-bulldozer/e2e-benchmarking"; | ||
LATEST_TAG=$(curl -s "https://api.github.com/repos/cloud-bulldozer/e2e-benchmarking/releases/latest" | jq -r '.tag_name'); | ||
TAG_OPTION="--branch $(if [ "$E2E_VERSION" = "default" ]; then echo "$LATEST_TAG"; else echo "$E2E_VERSION"; fi)"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TAG_OPTION="--branch $(if [ "$E2E_VERSION" = "default" ]; then echo "$LATEST_TAG"; else echo "$E2E_VERSION"; fi)"; | |
TAG_OPTION="--branch $(if [ "$E2E_VERSION" == "default" ]; then echo "$LATEST_TAG"; else echo "$E2E_VERSION"; fi)"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changed as suggested for better readability.
1ee2018
to
9ac399e
Compare
/pj-rehearse periodic-ci-openshift-qe-ocp-qe-perfscale-ci-main-gcp-4.13-nightly-x86-data-path-6nodes |
/assign @jtaleric |
/pj-rehearse ack |
@@ -15,7 +15,10 @@ ES_PASSWORD=$(cat "/secret/password") | |||
ES_USERNAME=$(cat "/secret/username") | |||
|
|||
# Clone the e2e repo | |||
git clone https://github.com/cloud-bulldozer/e2e-benchmarking | |||
REPO_URL="https://github.com/cloud-bulldozer/e2e-benchmarking"; | |||
LATEST_TAG=$(curl -s "https://api.github.com/repos/cloud-bulldozer/e2e-benchmarking/releases/latest" | jq -r '.tag_name'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we had issues on jenins being able to consistently connect and get the latest tag. I wonder if we will hit that here as well. Might need another option
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1 Also, do we even want a latest tag vs latest i.e. git clone of main branch?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That shouldn't ideally happen here as it is just hitting the endpoint and getting the tag_name. Where as previous command had some processing like below
$(curl -s https://api.github.com/repos/cloud-bulldozer/kube-burner/releases/latest | jq -r '.assets | map(select(.name | test("linux-x86_64"))) | .[0].browser_download_url')
And it used to run tar
on top of it to extract binaries too.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So if someone wants a specific tag, they can override E2E_VERSION
in their test or else latest
would be picked by default.
@@ -14,8 +14,10 @@ source ./venv_qe/bin/activate | |||
ES_PASSWORD=$(cat "/secret/password") | |||
ES_USERNAME=$(cat "/secret/username") | |||
|
|||
|
|||
git clone https://github.com/cloud-bulldozer/e2e-benchmarking --depth=1 | |||
REPO_URL="https://github.com/cloud-bulldozer/e2e-benchmarking"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should we add this repo url as an env variable as well so that in the future we can specify it in the config files if we ever want to test a pr/someone else repo/branch
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- Testing PRs is out of scope for this change. It needs to be taken through this JIRA: https://issues.redhat.com/browse/PERFSCALE-2731
- And the idea is to use only tags for testing but not branches going forward as they are error prone. (because as soon as a PR gets merged branch gets updated with those changes whereas release tag does not)
@@ -5,8 +5,12 @@ ref: | |||
name: ocp-qe-perfscale-ci | |||
tag: latest | |||
env: | |||
- name: E2E_VERSION | |||
default: "default" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe this is the version we want to hard-code to lock-in the specific e2e tag? "default" is the same as previous "latest", which leaves us open to unexpected changes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes it picks latest
tag by default. But before we even cut a release with latest
tag, it is recommended to test and verify them using pre-release
version and once everything works fine we can get it tagged as latest
.
Example: https://github.com/vishnuchalla/e2e-benchmarking/releases/tag/v0.0.0
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think using latest solves the problem. Please hard-code the exact release version we've tested.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My idea was to use latest tag by default, and pass on a rule to the team as "whenever some is cutting a release go with the pre-release first, verify it in PROW, and if it works well mark it as latest". If we think this plan doesn't work, we can hard code the tag number here. But one thing to note with hard-coding is, for every new release we will need to raise a PR here updating the value of E2E_VERSION
env in multiple files, whereas it is not required in the above approach that I am suggesting.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok. I think that's a process we can refine as needed.
But one thing to note with hard-coding is, for every new release we will need to raise a PR here updating the value of E2E_VERSION env in multiple files,
This is indeed very much the intent for now: To freeze the e2e version and require a PR and human eyes/hands on the upgrade step so nothing catches us by surprise.
Thanks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
9ac399e
to
38e15b2
Compare
/pj-rehearse periodic-ci-openshift-qe-ocp-qe-perfscale-ci-main-gcp-4.13-nightly-x86-data-path-6nodes |
/retest |
[REHEARSALNOTIFIER]
A total of 185 jobs have been affected by this change. The above listing is non-exhaustive and limited to 25 jobs. A full list of affected jobs can be found here Interacting with pj-rehearseComment: Once you are satisfied with the results of the rehearsals, comment: |
/pj-rehearse periodic-ci-openshift-qe-ocp-qe-perfscale-ci-main-aws-4.15-nightly-x86-payload-control-plane-6nodes periodic-ci-openshift-qe-ocp-qe-perfscale-ci-main-aws-4.15-nightly-x86-payload-node-density-cni-6nodes periodic-ci-openshift-qe-ocp-qe-perfscale-ci-main-aws-4.15-nightly-x86-payload-node-density-heavy-6nodes |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/lgtm
/lgtm |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: afcollins, paigerube14, vishnuchalla The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
/pj-rehearse ack |
@vishnuchalla: all tests passed! Full PR test history. Your PR dashboard. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here. |
Pre-requisite PR: cloud-bulldozer/e2e-benchmarking#661 to run rehearsal tests and verify the results.
This PR helps us to override E2E and individual tool versions to experiment/test with them. If any of the tools versions are not specified, default versions will be used from e2e-benchmarking repo.