feat: Add support for preemption_notice_duration in compute instance scheduling#16514
Conversation
|
Googlers: For automatic test runs see go/terraform-auto-test-runs. @melinath, 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. |
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
1787db2 to
283e8bc
Compare
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
|
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.
Missing test reportYour PR includes resource fields which are not covered by any test. Resource: resource "google_compute_instance" "primary" {
scheduling {
maintenance_interval = # value needed
}
}
Resource: resource "google_compute_instance_from_machine_image" "primary" {
scheduling {
availability_domain = # value needed
maintenance_interval = # value needed
min_node_cpus = # value needed
preemption_notice_duration {
nanos = # value needed
}
}
}
Resource: resource "google_compute_instance_from_template" "primary" {
scheduling {
availability_domain = # value needed
maintenance_interval = # value needed
min_node_cpus = # value needed
on_host_maintenance = # value needed
}
}
Missing doc report (experimental)The following resources have fields missing in documents.
|
Tests analyticsTotal tests: 6086 Click here to see the affected service packages
Action takenFound 5 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. |
melinath
left a comment
There was a problem hiding this comment.
Overall this looks good. The failing tests are unrelated to your code changes (but could be related to the dependency changes). The missing tests look mostly unrelated (with one exception noted below.) The missing docs notices seem like misfires due to the way these docs pages work; I see the fields added.
Please create a separate PR containing just the dependency upgrades, as described in https://googlecloudplatform.github.io/magic-modules/develop/update-dependencies/ Once that's merged & this PR is rebased, I think we should be good to go.
c98a9d8 to
5444c30
Compare
|
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.
Missing test reportYour PR includes resource fields which are not covered by any test. Resource: resource "google_compute_instance" "primary" {
scheduling {
maintenance_interval = # value needed
}
}
Resource: resource "google_compute_instance_from_machine_image" "primary" {
scheduling {
availability_domain = # value needed
maintenance_interval = # value needed
min_node_cpus = # value needed
}
}
Resource: resource "google_compute_instance_from_template" "primary" {
scheduling {
availability_domain = # value needed
maintenance_interval = # value needed
min_node_cpus = # value needed
on_host_maintenance = # value needed
}
}
Missing doc report (experimental)The following resources have fields missing in documents.
|
Tests analyticsTotal tests: 1372 Click here to see the affected service packages
🟢 All tests passed! View the build log |
melinath
left a comment
There was a problem hiding this comment.
I see the go.mod changes aren't here any more but I don't see a recent upgrade to the compute package or dependency upgrade from you... but also, the tests here aren't failing. So this is probably all good, but I just want to double-check that everything's fine? Why isn't the go.mod change required any more?
I performed a git pull to sync with the latest main branch and re-ran the test suite. Everything is passing consistently now. It’s possible a recent parallel update to our dependencies addressed the versioning requirement, or the specific upgrade is no longer a blocker for these tests. |
Preemption notice duration feature support in terraform
See Scheduling PreemptionNoticeDuration section in https://pkg.go.dev/google.golang.org/api/compute/v0.beta.
Release Note Template for Downstream PRs (will be copied)
See Write release notes for guidance.