Added integration tests#481
Conversation
|
This is copying a significant amount of code from https://github.com/gophercloud/gophercloud/tree/main/internal/acceptance. I am going to create a ticket there about potentially trying to expose this code out instead of copying it. |
6a71182 to
1b6d7ae
Compare
There was a problem hiding this comment.
Pull request overview
This PR introduces comprehensive integration tests for the OpenStack exporter, enabling automated testing against live OpenStack services (Ironic, Nova, Neutron, Glance, Keystone, Placement) via DevStack in CI. The tests validate that the exporter correctly generates Prometheus metrics from real OpenStack resources.
Key Changes
- Added integration test infrastructure with helper packages for creating test resources and clients
- Created service-specific integration test suites (baremetal, compute, networking, images, identity, placement)
- Set up GitHub Actions workflows to run integration tests on pull requests using DevStack
Reviewed changes
Copilot reviewed 28 out of 28 changed files in this pull request and generated 18 comments.
Show a summary per file
| File | Description |
|---|---|
| script/stackenv | Bash script to prepare DevStack testing environment with required resources and environment variables |
| script/integration-suites/* | Individual test runner scripts for each OpenStack service integration test suite |
| script/collectlogs | Utility script to collect diagnostic logs after integration test failures |
| integration/utils.go | Core integration test utilities for starting the OpenStack exporter |
| integration/tools/tools.go | Helper functions for waiting, random generation, and resource printing in tests |
| integration/tools/pkg.go | Build tags and package documentation for test tools |
| integration/funcs/compute.go | Helper functions for creating and managing compute resources in tests |
| integration/funcs/baremetal.go | Helper functions for creating and managing baremetal nodes in tests |
| integration/funcs/pkg.go | Build tags and package documentation for test resource functions |
| integration/clients/clients.go | OpenStack service client creation functions for various services |
| integration/clients/conditions.go | Test skip conditions for different OpenStack configurations and features |
| integration/clients/http.go | HTTP round tripper with request/response logging and header redaction |
| integration/*_test.go | Integration test files validating exporter metrics for each OpenStack service |
| .github/workflows/integration-*.yaml | GitHub Actions workflows for running integration tests in CI |
| .github/workflows/ci.yaml | Updated to exclude integration tests from unit test runs |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
CI Fixes Fixing more lint errors Tried to make tests more specific Fixed issues Fixes Fixes More fixes Trying more Test Fix Testing Testing Changes Removing placement Trying things out More changes Testing Trying things Fixes Fixes chore(deps): bump actions/checkout from 4 to 6 Bumps [actions/checkout](https://github.com/actions/checkout) from 4 to 6. - [Release notes](https://github.com/actions/checkout/releases) - [Commits](actions/checkout@v4...v6) --- updated-dependencies: - dependency-name: actions/checkout dependency-version: '6' dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] <support@github.com> Update integration/funcs/compute.go Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Update script/stackenv Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> chore(deps): bump reviewdog/action-actionlint from 1.66.1 to 1.69.0 Bumps [reviewdog/action-actionlint](https://github.com/reviewdog/action-actionlint) from 1.66.1 to 1.69.0. - [Release notes](https://github.com/reviewdog/action-actionlint/releases) - [Commits](reviewdog/action-actionlint@e37e2ca...437bbe9) --- updated-dependencies: - dependency-name: reviewdog/action-actionlint dependency-version: 1.69.0 dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <support@github.com>
7a9117f to
35eba72
Compare
|
|
I think the main thing to not is BASIS_OPS for ironic is false. For some reason, this makes the tests pass, where they didn't before |
|
@vooon - I'd appreciate your feedback on this in any capacity |
|
@vooon appreciate this. Thank you! |
ClifHouck
left a comment
There was a problem hiding this comment.
I mostly skimmed this. It looks fine on the surface, you can always disable it later or correct things if there's flakiness.
|
@vooon @ClifHouck I'll merge after a final approval from either of you, I think this in its present state is better than nothing |
vooon
left a comment
There was a problem hiding this comment.
Maybe better to parse prom text and then validate lables/metrics that way instead of simple "contains" in body?
But overall looks good.
| run: | | ||
| go test -v -covermode=atomic -coverprofile=coverage.out ./... | ||
| mapfile -t packages < <(go list ./... | grep -v '/integration') | ||
| go test -v -covermode=atomic -coverprofile=coverage.out "${packages[@]}" |
There was a problem hiding this comment.
Why not to use something like that in integration tests?
//go:build acceptance
| sudo journalctl -o short-precise --no-pager &> "$LOG_DIR/journal.log" | ||
| # shellcheck disable=SC2024 | ||
| sudo systemctl status "devstack@*" &> "$LOG_DIR/devstack-services.txt" | ||
| for service in $(systemctl list-units --output json devstack@* | jq -r '.[].unit | capture("devstack@(?<svc>[a-z\\-]+).service") | '.svc'') |
There was a problem hiding this comment.
Single quotes around .svc ambiguous and maybe unneeded.
| EOL | ||
|
|
||
| if _=$(openstack service list | grep container-infra); then | ||
| _MAGNUM_IMAGE_ID=$(openstack image list --format value -c Name -c ID | grep coreos | cut -d ' ' -f 1) |
There was a problem hiding this comment.
You can use filter by property or tag:
openstack image list --property os_distro=fedora-coreos
|
@mnaser @niedbalski I intend to merge on or after 14th April. If I don't hear from either of you, I will self merge this |
|
Just a bump if this will get merged as the month is ending. |
|
@WizardBit pushed |
This should help make trusting new changes easier.