-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
Upgrade dcl #7012
Upgrade dcl #7012
Conversation
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. Terraform GA: Diff ( 5 files changed, 65 insertions(+), 29 deletions(-)) |
Tests analyticsTotal tests: Action takenTriggering VCR tests in RECORDING mode for the tests that failed during VCR. Click here to see the failed testsTestAccContainerCluster_withInvalidGatewayApiConfigChannel|TestAccFirebaserulesRelease_BasicRelease|TestAccLoggingBucketConfigProject_cmekSettings|TestAccOsConfigOsPolicyAssignment_FixedOsPolicyAssignment|TestAccOsConfigOsPolicyAssignment_PercentOsPolicyAssignment |
Tests passed during RECORDING mode: Tests failed during RECORDING mode: Please fix these to complete your PR |
/gcbrun cleared out ci |
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. Terraform GA: Diff ( 5 files changed, 65 insertions(+), 29 deletions(-)) |
Tests analyticsTotal tests: Action takenTriggering VCR tests in RECORDING mode for the tests that failed during VCR. Click here to see the failed testsTestAccContainerCluster_withInvalidGatewayApiConfigChannel|TestAccLoggingBucketConfigProject_cmekSettings|TestAccOsConfigOsPolicyAssignment_FixedOsPolicyAssignment|TestAccOsConfigOsPolicyAssignment_PercentOsPolicyAssignment |
Tests passed during RECORDING mode: Tests failed during RECORDING mode: Please fix these 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. Terraform GA: Diff ( 5 files changed, 201 insertions(+), 1700 deletions(-)) |
Tests analyticsTotal tests: Action takenTriggering VCR tests in RECORDING mode for the tests that failed during VCR. Click here to see the failed testsTestAccLoggingBucketConfigProject_cmekSettings|TestAccOsConfigOsPolicyAssignment_PercentOsPolicyAssignment|TestAccOsConfigOsPolicyAssignment_FixedOsPolicyAssignment|TestAccContainerCluster_withInvalidGatewayApiConfigChannel |
Tests passed during RECORDING mode: Tests failed during RECORDING mode: Please fix these 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. Terraform GA: Diff ( 5 files changed, 205 insertions(+), 1704 deletions(-)) |
Tests analyticsTotal tests: Action takenTriggering VCR tests in RECORDING mode for the tests that failed during VCR. Click here to see the failed testsTestAccContainerCluster_withInvalidGatewayApiConfigChannel|TestAccComputeForwardingRule_update|TestAccOsConfigOsPolicyAssignment_PercentOsPolicyAssignment|TestAccOsConfigOsPolicyAssignment_FixedOsPolicyAssignment|TestAccLoggingBucketConfigProject_cmekSettings |
Tests passed during RECORDING mode: Tests failed during RECORDING mode: Please fix these to complete your PR |
16beadb
to
ec20de4
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. Terraform GA: Diff ( 5 files changed, 80 insertions(+), 1936 deletions(-)) |
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. Terraform GA: Diff ( 6 files changed, 205 insertions(+), 2850 deletions(-)) |
Tests analyticsTotal tests: Action takenTriggering VCR tests in RECORDING mode for the tests that failed during VCR. Click here to see the failed testsTestAccContainerCluster_withInvalidGatewayApiConfigChannel|TestAccLoggingBucketConfigProject_cmekSettings |
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.
In general this LGTM. Will this fix hashicorp/terraform-provider-google#11070? It could be nice to make the test more like a full test by adding more fields, but since it's the same as a test that previously existed I don't think that's a blocker.
The one issue is that this seems to cause the entire documentation page for the resource to get deleted - possibly tpgtools requires at least one sample?
name = "tf-test-assignment%{random_suffix}" | ||
|
||
os_policies { | ||
id = "policy" |
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.
possibly add allow_no_resource_group_match and description in here to get full coverage?
id = "policy" | ||
mode = "VALIDATION" | ||
|
||
resource_groups { |
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.
possibly add inventory_filters to get full coverage?
|
||
resource_groups { | ||
resources { | ||
id = "apt-to-yum" |
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.
possibly add exec, file, and pkg fields & their subfields here?
I'll look into the doc generation. The problem is that these tests fail due to long (1h+) rollout times of the resource because they have to run checks against every instance in the project. They pass locally, but will never pass in CI it seems The current tests have full coverage but getting that coverage causes many updates (extending test length) and frankly unwieldy configs to show up in the examples. I tried to simplify them and target unused zones to get the rollout to succeed, but even that seems to have failed |
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.
re: making the docs unwieldy, doc_hide
in the meta.yaml
file should help address that?
mmv1/third_party/terraform/tests/resource_os_config_os_policy_assignment_test.go.erb
Outdated
Show resolved
Hide resolved
mmv1/third_party/terraform/tests/resource_os_config_os_policy_assignment_test.go.erb
Show resolved
Hide resolved
Any chance that we could isolate these tests in their own project(s) to reduce the rollout time? |
Good call! Works locally, testing in CI now |
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.
No blockers from me, approving so there aren't too many cooks in the kitchen. Primary review's @melinath.
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. Terraform GA: Diff ( 6 files changed, 228 insertions(+), 2041 deletions(-)) |
The provider crashed while running the VCR tests in REPLAYING mode |
It looks like this crash is due to a change merged earlier; I'll make a revert PR. |
d7d4146
to
11211d9
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. Terraform GA: Diff ( 6 files changed, 260 insertions(+), 2041 deletions(-)) |
Tests analyticsTotal tests: Action takenTriggering VCR tests in RECORDING mode for the tests that failed during VCR. Click here to see the failed testsTestAccFirebaserulesRelease_BasicRelease|TestAccContainerCluster_withInvalidGatewayApiConfigChannel|TestAccBigtableAppProfile_bigtableAppProfileMulticlusterExample|TestAccOsConfigOsPolicyAssignment_basicOsPolicyAssignment |
Tests passed during RECORDING mode: All tests passed |
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
* Upgrade dcl * Add new basic test * Simplify tests * Fix yaml * Use other zone * Update tests * Simplify tests * Remove sample tests, use handwritten * Rename test * Test in separate project, add back docs-only sample * Add test changes too * Add a couple fields
* Upgrade dcl * Add new basic test * Simplify tests * Fix yaml * Use other zone * Update tests * Simplify tests * Remove sample tests, use handwritten * Rename test * Test in separate project, add back docs-only sample * Add test changes too * Add a couple fields
* Upgrade dcl * Add new basic test * Simplify tests * Fix yaml * Use other zone * Update tests * Simplify tests * Remove sample tests, use handwritten * Rename test * Test in separate project, add back docs-only sample * Add test changes too * Add a couple fields
Upgrade DCL to 1.30.0. Adds support for OSPolicyConfig skip_await_rollout
Disables tests in VCR as they haven't passed successfully in months due to long rollout times. skip_await_rollout won't work in tests due to deletion needing to happen as the last test step, but these resources cannot be deleted until they finish rollout
If this PR is for Terraform, I acknowledge that I have:
make test
andmake lint
to ensure it passes unit and linter tests.Release Note Template for Downstream PRs (will be copied)