-
Notifications
You must be signed in to change notification settings - Fork 69
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 GitHub workflows to create and delete a shared ROSA cluster #345
Conversation
working-directory: provision/aws | ||
env: | ||
VERSION: ${{ env.OPENSHIFT_VERSION }} | ||
CLUSTER_NAME: shared |
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.
Makes sense to me. Then we can add the cluster name as the parameter for the benchmark run.
In the referenced run : https://github.com/stianst/keycloak-benchmark/actions/runs/5088576833/jobs/9145155404
I see that it failed to authenticate a bunch of times before it succeeded., which is unlike the behavior we saw on local runs., is this expected ? |
also we are generating some debug logs for each custer create and delete using rosa scripts, do we want to retrieve them somehow for debugging the errors on aws or otherwise ? if you think, that should be done in a future PR, thats OK with me too. @stianst |
I changed it to not wait for 1m, but instead just keep trying every 10 seconds, so that'll be the reason for a few more failures there. I think it is better to have it fail a few times, then waiting for too long? |
I'd say let's add it later. Probably just grab the logs and store as artifacts if it fails to deploy. Don't think we need any logs if it is successful? Related to this is we could probably also make sure everything is cleaned-up if something fails. |
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.
Agreed on log management to be taken care later, fine for it fail fast, than have to wait. This PR looks good to be merged, if there are no other comments
I've tested this on my fork: