Change ExpectedError in TestAccComputeInstanceNetworkIntefaceWithSecurityPolicy

[karolgorc]

References issue #17838

The tests expects a wrong error in config. The error isn't thrown by the Security Policy code but rather by the AccessConfig code

Didn't have an environment to test this so testing it in PR

Release Note Template for Downstream PRs (will be copied)

[github-actions]

Hello! I am a robot. Tests will require approval from a repository maintainer to run.

@shuyama1, 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.

[modular-magician]

Hi there, I'm the Modular magician. I've detected the following information about your changes:

Diff report

Your PR generated some diffs in downstreams - here they are.

google-beta provider: Diff ( 1 file changed, 2 insertions(+), 4 deletions(-))

[modular-magician]

Tests analytics

Total tests: 976
Passed tests: 902
Skipped tests: 73
Affected tests: 1

Click here to see the affected service packages

• compute

Action taken

Found 1 affected test(s) by replaying old test recordings. Starting RECORDING based on the most recent commit. Click here to see the affected tests

• TestAccComputeInstanceNetworkIntefaceWithSecurityPolicy

Get to know how VCR tests work

[modular-magician]

$\textcolor{red}{\textsf{Tests failed during RECORDING mode:}}$
TestAccComputeInstanceNetworkIntefaceWithSecurityPolicy[Error message] [Debug log]

$\textcolor{red}{\textsf{Errors occurred during RECORDING mode. Please fix them to complete your PR.}}$

View the build log or the debug log for each test

[shuyama1]

Tests failed due to errors:

=== RUN   TestAccComputeInstanceNetworkIntefaceWithSecurityPolicy/access_config_update_access_config
    vcr_utils.go:152: Step 3/5 error: Error running apply: exit status 1
        
        Error: Error deleting old access_config: googleapi: Error 400: Invalid value for field 'accessConfig': 'external-nat'. Cannot delete an access config with a security policy set. Please remove the security policy first., invalid
        
          with google_compute_instance.foobar,
          on terraform_plugin_test.tf line 72, in resource "google_compute_instance" "foobar":
          72: resource "google_compute_instance" "foobar" {

=== RUN   TestAccComputeInstanceNetworkIntefaceWithSecurityPolicy/wit_no_access_config
    vcr_utils.go:152: Step 1/1, expected an error with pattern, no match on: Error running apply: exit status 1
        
        Error: Error setting security policy to the instance since at least one access config must exist
        
          with google_compute_instance.foobar,
          on terraform_plugin_test.tf line 72, in resource "google_compute_instance" "foobar":
          72: resource "google_compute_instance" "foobar" {

=== RUN   TestAccComputeInstanceNetworkIntefaceWithSecurityPolicy/two_nics_two_access_configs_update_remove_access_config
    vcr_utils.go:152: Step 3/4 error: Error running apply: exit status 1
        
        Error: Error deleting old access_config: googleapi: Error 400: Invalid value for field 'accessConfig': 'external-nat'. Cannot delete an access config with a security policy set. Please remove the security policy first., invalid
        
          with google_compute_instance.foobar,
          on terraform_plugin_test.tf line 119, in resource "google_compute_instance" "foobar":
         119: resource "google_compute_instance" "foobar" {

--- FAIL: TestAccComputeInstanceNetworkIntefaceWithSecurityPolicy (1627.42s)
    --- PASS: TestAccComputeInstanceNetworkIntefaceWithSecurityPolicy/two_nics_two_access_configs_update_two_policies (284.63s)
    --- FAIL: TestAccComputeInstanceNetworkIntefaceWithSecurityPolicy/access_config_update_access_config (234.75s)
    --- FAIL: TestAccComputeInstanceNetworkIntefaceWithSecurityPolicy/wit_no_access_config (130.68s)
    --- PASS: TestAccComputeInstanceNetworkIntefaceWithSecurityPolicy/two_access_config (209.55s)
    --- PASS: TestAccComputeInstanceNetworkIntefaceWithSecurityPolicy/two_nics_access_config_with_empty_nil_security_policy (136.89s)
    --- PASS: TestAccComputeInstanceNetworkIntefaceWithSecurityPolicy/two_nics_two_access_configs_update_one_policy (228.45s)
    --- PASS: TestAccComputeInstanceNetworkIntefaceWithSecurityPolicy/two_access_config_update_policy_with_stopped_machine (189.48s)
    --- FAIL: TestAccComputeInstanceNetworkIntefaceWithSecurityPolicy/two_nics_two_access_configs_update_remove_access_config (213.00s)
FAIL
FAIL	github.com/hashicorp/terraform-provider-google-beta/google-beta/services/compute	1627.478s
FAIL

Please let me know if you need help debugging the issue.

[karolgorc]

Ok it's actually the test cases that do not expect an error that are failing the tests because of access config errors. Don't know how will the access_config_update_access_config behave with 2 Expect error steps.

Could run into some apply issues that will require rewriting the terraform config.

[karolgorc]

Also not sure if the regular expressions .* will work as intended but we should expect the same error all the time so will just update if it fails

[modular-magician]

Hi there, I'm the Modular magician. I've detected the following information about your changes:

Diff report

Your PR generated some diffs in downstreams - here they are.

google-beta provider: Diff ( 1 file changed, 4 insertions(+), 14 deletions(-))

[modular-magician]

Tests analytics

Total tests: 979
Passed tests: 907
Skipped tests: 71
Affected tests: 1

Click here to see the affected service packages

• compute

Action taken

Found 1 affected test(s) by replaying old test recordings. Starting RECORDING based on the most recent commit. Click here to see the affected tests

• TestAccComputeInstanceNetworkIntefaceWithSecurityPolicy

Get to know how VCR tests work

[modular-magician]

$\textcolor{red}{\textsf{Tests failed during RECORDING mode:}}$
TestAccComputeInstanceNetworkIntefaceWithSecurityPolicy[Error message] [Debug log]

$\textcolor{red}{\textsf{Errors occurred during RECORDING mode. Please fix them to complete your PR.}}$

View the build log or the debug log for each test

[github-actions]

@shuyama1 This PR has been waiting for review for 3 weekdays. Please take a look! Use the label disable-review-reminders to disable these notifications.

[github-actions]

@GoogleCloudPlatform/terraform-team @shuyama1 This PR has been waiting for review for 1 week. Please take a look! Use the label disable-review-reminders to disable these notifications.

[shuyama1]

Test failed with error

=== RUN   TestAccComputeInstanceNetworkIntefaceWithSecurityPolicy/two_nics_two_access_configs_update_remove_access_config
...
    vcr_utils.go:152: Step 3/4, expected an error with pattern, no match on: Error running apply: exit status 1
        
        Error: Error deleting old access_config: googleapi: Error 400: Invalid value for field 'accessConfig': 'external-nat'. Cannot delete an access config with a security policy set. Please remove the security policy first., invalid
        
          with google_compute_instance.foobar,
          on terraform_plugin_test.tf line 119, in resource "google_compute_instance" "foobar":
         119: resource "google_compute_instance" "foobar" {

=== RUN   TestAccComputeInstanceNetworkIntefaceWithSecurityPolicy/access_config_update_access_config
    vcr_utils.go:152: Step 3/5, expected an error with pattern, no match on: Error running apply: exit status 1
        
        Error: Error deleting old access_config: googleapi: Error 400: Invalid value for field 'accessConfig': 'external-nat'. Cannot delete an access config with a security policy set. Please remove the security policy first., invalid
        
          with google_compute_instance.foobar,
          on terraform_plugin_test.tf line 72, in resource "google_compute_instance" "foobar":
          72: resource "google_compute_instance" "foobar" {

Sorry for the delay on review. Let me know if there's anything I can help with. Thanks

[karolgorc]

This will go through steps 3/4 and 3/5 as expected because we can't delete access config with a sec_policy on it. It may fail on next ones due to this error happening

[modular-magician]

Hi there, I'm the Modular magician. I've detected the following information about your changes:

Diff report

Your PR generated some diffs in downstreams - here they are.

google-beta provider: Diff ( 1 file changed, 6 insertions(+), 14 deletions(-))

[modular-magician]

Tests analytics

Total tests: 982
Passed tests: 908
Skipped tests: 71
Affected tests: 3

Click here to see the affected service packages

• compute

Action taken

Found 3 affected test(s) by replaying old test recordings. Starting RECORDING based on the most recent commit. Click here to see the affected tests

• TestAccComputeDisk_storagePoolSpecified
• TestAccComputeInstanceNetworkIntefaceWithSecurityPolicy
• TestAccComputeInstance_bootDisk_storagePoolSpecified

Get to know how VCR tests work

[modular-magician]

$\textcolor{green}{\textsf{Tests passed during RECORDING mode:}}$
TestAccComputeInstanceNetworkIntefaceWithSecurityPolicy[Debug log]

$\textcolor{green}{\textsf{No issues found for passed tests after REPLAYING rerun.}}$

---

$\textcolor{red}{\textsf{Tests failed during RECORDING mode:}}$
TestAccComputeDisk_storagePoolSpecified[Error message] [Debug log]
TestAccComputeInstance_bootDisk_storagePoolSpecified[Error message] [Debug log]

$\textcolor{red}{\textsf{Errors occurred during RECORDING mode. Please fix them to complete your PR.}}$

View the build log or the debug log for each test

[karolgorc]

i don't think these errors are related because they pass when i run them on my env that's a few commits behind
--- PASS: TestAccComputeDisk_storagePoolSpecified (27.76s)
--- PASS: TestAccComputeInstance_bootDisk_storagePoolSpecified (198.70s)

[github-actions]

@shuyama1 This PR has been waiting for review for 3 weekdays. Please take a look! Use the label disable-review-reminders to disable these notifications.

[shuyama1]

i don't think these errors are related because they pass when i run them on my env that's a few commits behind --- PASS: TestAccComputeDisk_storagePoolSpecified (27.76s) --- PASS: TestAccComputeInstance_bootDisk_storagePoolSpecified (198.70s)

Thank you for running the test locally! yes, the test failures are not related to this change and they should be fixed at head now. Kicking off a rerun.

/gcbrun

[modular-magician]

Hi there, I'm the Modular magician. I've detected the following information about your changes:

Diff report

Your PR generated some diffs in downstreams - here they are.

google-beta provider: Diff ( 1 file changed, 6 insertions(+), 14 deletions(-))

[modular-magician]

Tests analytics

Total tests: 985
Passed tests: 908
Skipped tests: 76
Affected tests: 1

Click here to see the affected service packages

• compute

Action taken

Found 1 affected test(s) by replaying old test recordings. Starting RECORDING based on the most recent commit. Click here to see the affected tests

• TestAccComputeInstanceNetworkIntefaceWithSecurityPolicy

Get to know how VCR tests work

[modular-magician]

$\textcolor{green}{\textsf{Tests passed during RECORDING mode:}}$
TestAccComputeInstanceNetworkIntefaceWithSecurityPolicy[Debug log]

$\textcolor{green}{\textsf{No issues found for passed tests after REPLAYING rerun.}}$

---

$\textcolor{green}{\textsf{All tests passed!}}$

View the build log or the debug log for each test

[karolgorc]

Is this unit test fail related? Got a similar one in another PR but it was on bigquery package. This one is datastream

[github-actions]

@GoogleCloudPlatform/terraform-team @shuyama1 This PR has been waiting for review for 1 week. Please take a look! Use the label disable-review-reminders to disable these notifications.

[shuyama1]

Is this unit test fail related? Got a similar one in another PR but it was on bigquery package. This one is datastream

The failing unit test is not related. Please ignore it. Sorry for the noise. I'll take a look at the PR today.

[shuyama1]

Sorry for the delay on review. Left a question regarding the change.

[github-actions]

@karolgorc, this PR is waiting for action from you. Please address any comments or change requests, or re-request review from a core reviewer if no action is required.

If no action is taken, this PR will be closed in 28 days.

This notification can be disabled with the disable-automatic-closure label.

[karolgorc]

The test is passing now, confirmed that the change is needed because google API throws an error. Previous gcburn failed due to unrelated unit-tests.

Is there anything else you need me to do?

[shuyama1]

/gcbrun

[shuyama1]

The test is passing now, confirmed that the change is needed because google API throws an error. Previous gcburn failed due to unrelated unit-tests.

Is there anything else you need me to do?

Apologies for missing your last comment—I didn’t realize the PR was waiting on me. I’m triggering a new CI run now, and everything looks good to me overall. No action needed from your end at this time. Thanks!

[modular-magician]

Hi there, I'm the Modular magician. I've detected the following information about your changes:

Diff report

Your PR generated some diffs in downstreams - here they are.

google-beta provider: Diff ( 1 file changed, 6 insertions(+), 14 deletions(-))

[modular-magician]

Tests analytics

Total tests: 1004
Passed tests: 930
Skipped tests: 74
Affected tests: 0

Click here to see the affected service packages

• compute

$\textcolor{green}{\textsf{All tests passed!}}$

View the build log