-
Notifications
You must be signed in to change notification settings - Fork 569
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
Add recreate benchmark script #8473
Conversation
No need to spend time on running the checks. We also don't run the tests. That's what we have CI for.
734352a
to
4a9076a
Compare
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. Some minor comments.
recreateBenchmark.sh
Outdated
docker build --build-arg DISTBALL=dist/target/camunda-cloud-zeebe-*.tar.gz --build-arg APP_ENV=dev -t "gcr.io/zeebe-io/zeebe:$benchmark" . | ||
docker push "gcr.io/zeebe-io/zeebe:$benchmark" | ||
|
||
cd "benchmarks/setup/$benchmark" |
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.
This folder may not exists, right? I don't think we commit release benchmark setup anywhere.
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.
Good point. We should commit those somewhere, and this reminds me that it should also rebuild the benchmark project. So we should check that the folder exists
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.
Rebuilding the benchmark project also doesn't require this folder specifically. So no need for the check.
I've update the script to rebuild the benchmark project.
d1340a5
to
f633e3a
Compare
This script allows you to easily recreate an existing benchmark that was created with createBenchmark.sh. It simply builds the docker images and updates the deployment. An additional check that the k8s namespace is available is added to this, as a quick check that you're using the correct namespace and are on the correct context. Or perhaps the user was actually intending to run ./createBenchmark.sh instead. Sadly, the make update does not restart the pods. We should consider solutions for that. However, as soon as the pods restart, the pods are updated because the pullPolicy is set to always. Therefore this applies a hard delete of all the broker and gateway pods to force the update. This is not a rolling update.
f633e3a
to
3529bf2
Compare
Thanks for the review @deepthidevaki. Can you have another look? |
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.
Thanks 🚀
cd "$pwd/benchmarks/project" | ||
sed_inplace "s/:SNAPSHOT/:$benchmark/" docker-compose.yml | ||
# Use --no-cache to force rebuild the image for the benchmark application. Without this changes to zeebe-client were not picked up. This can take longer to build. | ||
docker-compose build --no-cache | ||
docker-compose push | ||
git restore -- docker-compose.yml | ||
|
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.
In most cases there won't be any need to re-build the starters and workers. But it make sense to do it always incase there are some 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.
I agree. If it bothers us we could spend some time adding a flag for this, so it can be turned on and off at will. But I didn't want to spend too much time on it now. Perhaps another time.
bors merge |
8473: Add recreate benchmark script r=korthout a=korthout ## Description <!-- Please explain the changes you made here. --> One of the steps in the daily release process is to check whether there are new commits on the release branch. If there are any, the benchmark must be restarted. There did not exist any script for this yet, and the manual steps are not straight forward (e.g. `newBenchmark` will fail because the namespace is already in use). To avoid these problems, a new `recreateBenchmark.sh` script is introduced that: - checks whether the namespace already exists and if not suggests solutions - rebuilds the project and the docker images - redeploys the benchmark This uses a special check for the benchmark name to be a valid DNS label. See #8455 for more details ## Related issues <!-- Which issues are closed by this PR or are related --> NA Co-authored-by: Nico Korthout <nico.korthout@camunda.com>
Build failed: |
Flaky test bors merge |
Opened #8475 for the flaky test |
Successfully created backport PR #8476 for |
Successfully created backport PR #8477 for |
Successfully created backport PR #8478 for |
Description
One of the steps in the daily release process is to check whether there are new commits on the release branch. If there are any, the benchmark must be restarted. There did not exist any script for this yet, and the manual steps are not straight forward (e.g.
newBenchmark
will fail because the namespace is already in use).To avoid these problems, a new
recreateBenchmark.sh
script is introduced that:This uses a special check for the benchmark name to be a valid DNS label. See #8455 for more details
Related issues
NA
Definition of Done
Not all items need to be done depending on the issue and the pull request.
Code changes:
backport stable/0.25
) to the PR, in case that fails you need to create backports manually.Testing:
Documentation: