-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
Remove references to deleted compute images in multiple tests #12062
Remove references to deleted compute images in multiple tests #12062
Conversation
Hello! I am a robot. Tests will require approval from a repository maintainer to run. @NickElliot, a repository maintainer, has been assigned to review your changes. If you have not received review feedback within 2 business days, please leave a comment on this PR asking them to take a look. You can help make sure that review is quick by doing a self-review and by running impacted tests locally. |
Hi there, I'm the Modular magician. I've detected the following information about your changes: Diff reportYour PR generated some diffs in downstreams - here they are.
|
Tests analyticsTotal tests: 1036 Click here to see the affected service packages
Action takenFound 15 affected test(s) by replaying old test recordings. Starting RECORDING based on the most recent commit. Click here to see the affected tests
|
🟢 Tests passed during RECORDING mode: 🟢 No issues found for passed tests after REPLAYING rerun. 🔴 Tests failed during RECORDING mode: 🔴 Errors occurred during RECORDING mode. Please fix them to complete your PR. |
Hi there, I'm the Modular magician. I've detected the following information about your changes: Diff reportYour PR generated some diffs in downstreams - here they are.
|
Tests analyticsTotal tests: 1036 Click here to see the affected service packages
Action takenFound 9 affected test(s) by replaying old test recordings. Starting RECORDING based on the most recent commit. Click here to see the affected tests
|
🟢 Tests passed during RECORDING mode: 🟢 No issues found for passed tests after REPLAYING rerun. 🟢 All tests passed! |
@NickElliot This PR has been waiting for review for 3 weekdays. Please take a look! Use the label |
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.
changes LGTM! how long is the actual slowdown in the generated tests with deploying new disks?
Looking at the logs, it's about 10 seconds at the start of the test to create the disk, and another 10 seconds at the end to delete it. |
It seems like the fix in GoogleCloudPlatform#12062 may not have fixed the images relying on Ubuntu images in the `tdx-guest-images` project. Part of hashicorp/terraform-provider-google#19885
There are multiple tests that refer to preconfigured images in the
bosh-gce-raw-stemcells
storage bucket. That bucket was deleted recently, so the tests began failing. That bucket was at least five years old and we never controlled it (bosh
is a Cloud Foundry tool), so it was bound to happen.Replacing the missing bucket with another preconfigured bucket will just kick the can down the road, so the tests are updated to no longer require one. There's three different fixes applied in this PR:
google_compute_image
resource was removed. These are tests that are not testinggoogle_compute_image
at all (and were likely copied and pasted from other tests that do).raw-disk-image.tar.gz
file, which is a valid but small disk image (it's mostly zeros so it compresses down to about 3KB). The tests using this method are replicating their earlier behavior, but without the preconfigured bucket.google_compute_image
references that disk. This is a bit slower than the second method but doesn't rely on a test fixture so the tests can continue to be generated.Fixes hashicorp/terraform-provider-google#19885
Fixes hashicorp/terraform-provider-google#19729
Release Note Template for Downstream PRs (will be copied)