-
Notifications
You must be signed in to change notification settings - Fork 6
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
Rewrite integration CI #117
Conversation
Sometimes the CI fails with ceilometer timing out when trying to send metrics. I hope that these logs make it clearer what is happening.
I think it might be a good idea to somehow make it, that the jobs don't get executed twice on PR. Because it's getting a little cluttered now with 16 jobs running instead of just 8. I thought it might be solved by changing the "on" parameter in the workflow file, but for example here: openstack-k8s-operators/telemetry-operator#172 The "on" parameter is also |
8489ab5
to
a8c18c2
Compare
I tried to remove the mysql workaround from the workflow to see, if it's still required. I'll delete the commit if this fails. |
886f2ea
to
a8c18c2
Compare
I deleted the commit. Both of the workarounds (mysql, postgres) seem to be required to deploy devstack. |
I'm done trying things, this is ready for review |
$TEST_IMAGE bash $PROJECT_ROOT/ci/integration/metrics/run_bridge.sh | ||
- name: Install collectd | ||
run: | | ||
sudo apt-get install collectd |
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.
What version of collectd does this install?
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 see now you've just moved what was already happening up here. Still curious, but this seems fine :)
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 have ubuntu on hand right now, but I found this: https://www.ubuntuupdates.org/package/core/jammy/universe/updates/collectd So I guess it could be 5.12?
--volume ${{ github.workspace }}:$PROJECT_ROOT:z --workdir $PROJECT_ROOT \ | ||
$TEST_IMAGE bash $PROJECT_ROOT/ci/integration/metrics/collectd/run_validation.sh | ||
#------------------------------------------------------------------------------- | ||
ceilometer-metrics-bridge: |
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.
Just noting that further refactoring could happen here using composite actions (https://docs.github.com/en/actions/creating-actions/creating-a-composite-action) in order to remove some of this duplication.
I am not advocating doing it here though, just noting in case a future refactoring was warranted.
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 I agree. For the metric jobs for example, the first ~5 steps are the same. I was looking into the composite actions but I decided to not implement them. As I understand the documentation, we would either need to create a new repository for the actions or we could have them in this repo, but we would probably need to tag the repository. So that we can use the uses: action@tag
syntax. I haven't tried this, but after reading the docs and understanding it like this, I decided to not use the composite actions to not create some confusion in the repository tags just for CI deduplication.
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.
Went through the code and some existing scripts etc to see how this looks, and see some of the interactions. Couple minor questions I was able to answer. Looks good. Thanks!
Co-authored-by: Leif Madsen <[email protected]>
Just FYI. The logging CI job just failed. I looked at the logs and it failed to install the golang package, so not our fault right now. I'll restart the job. |
Looks good to me! Thanks for doing this! |
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
As we discussed with Leif previously, I rewrote the CI to be closer to how we actually use sg-core.