Skip to content

apigee: fix TestAccApigeeOrganization_apigeeOrganizationCloudFullDisableVpcPeeringTestExample#16940

Open
xuchenma wants to merge 2 commits intoGoogleCloudPlatform:mainfrom
xuchenma:441164722
Open

apigee: fix TestAccApigeeOrganization_apigeeOrganizationCloudFullDisableVpcPeeringTestExample#16940
xuchenma wants to merge 2 commits intoGoogleCloudPlatform:mainfrom
xuchenma:441164722

Conversation

@xuchenma
Copy link
Copy Markdown
Contributor

@xuchenma xuchenma commented Apr 4, 2026

Summary

Fixed two root causes that caused TestAccApigeeOrganization_apigeeOrganizationCloudFullDisableVpcPeeringTestExample to fail:

  1. Properties ordering mismatch: The Apigee API returns properties.property entries in alphabetical order, while Terraform compares lists order-sensitively. Added a custom_flatten that uses SortMapsByConfigOrder to sort the API response in config-specified order before storing in state. This fix was already present in the GA provider via custom_flatten/apigee_organization_property.go.tmpl, but the test_check_destroy template was needed to also fix CheckDestroy.

  2. Async org deletion race: Apigee Organization deletion is asynchronous. CheckDestroy ran immediately after the delete API call returned, finding the org still in DELETING state. Added a test_check_destroy custom template that polls up to 10 minutes, treating both HTTP 404 (fully deleted) and state == DELETING (deletion in progress) as success.

The apigee_organization_cloud_full_disable_vpc_peering_test.tf.tmpl also had template issues (unsupported deletion_policy field in beta, and using .member attribute which is only in GA); these are fixed in this PR.

Test evidence

--- PASS: TestAccApigeeOrganization_apigeeOrganizationCloudFullDisableVpcPeeringTestExample (1160.98s)

Fixes: hashicorp/terraform-provider-google#24143

apigee: fixed `google_apigee_organization` perpetual plan diff caused by API returning properties in non-deterministic order and flaky CheckDestroy caused by async organization deletion

…bleVpcPeeringTestExample

- Add custom_check_destroy template that polls for async org deletion
- Fix apigee_organization_cloud_full_disable_vpc_peering_test.tf.tmpl:
  remove deletion_policy (unsupported in beta) and use explicit member format
- The full_test.tf.tmpl gets the same member format fix

Apigee Organization deletion is async. CheckDestroy was racing with the
background deletion, causing false 'still exists' failures. The fix polls
until the org returns 404 or reaches DELETING state.

The properties ordering issue (plan not empty after apply) was already fixed
via the custom_flatten/apigee_organization_property.go.tmpl template.
@modular-magician modular-magician added the awaiting-approval Pull requests that need reviewer's approval to run presubmit tests label Apr 4, 2026
@github-actions github-actions bot requested a review from BBBmau April 4, 2026 00:42
@github-actions
Copy link
Copy Markdown

github-actions bot commented Apr 4, 2026

Googlers: For automatic test runs see go/terraform-auto-test-runs.

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

@xuchenma
Copy link
Copy Markdown
Contributor Author

xuchenma commented Apr 4, 2026

@modular-magician modular-magician added service/apigee and removed awaiting-approval Pull requests that need reviewer's approval to run presubmit tests labels Apr 6, 2026
@modular-magician
Copy link
Copy Markdown
Collaborator

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 provider: Diff ( 1 file changed, 26 insertions(+), 9 deletions(-))
google-beta provider: Diff ( 1 file changed, 28 insertions(+), 13 deletions(-))

@modular-magician
Copy link
Copy Markdown
Collaborator

Tests analytics

Total tests: 72
Passed tests: 33
Skipped tests: 37
Affected tests: 2

Click here to see the affected service packages
  • apigee

Action taken

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

Get to know how VCR tests work

@modular-magician
Copy link
Copy Markdown
Collaborator

🔴 Tests failed during RECORDING mode:
TestAccApigeeOrganization_apigeeOrganizationCloudFullDisableVpcPeeringTestExample [Error message] [Debug log]
TestAccApigeeOrganization_apigeeOrganizationCloudFullTestExample [Error message] [Debug log]

🔴 Errors occurred during RECORDING mode. Please fix them to complete your PR.

View the build log or the debug log for each test

name = "tf-test%{random_suffix}"
org_id = "{{index $.TestEnvVars "org_id"}}"
billing_account = "{{index $.TestEnvVars "billing_account"}}"
deletion_policy = "DELETE"
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what's the reason for removing this? This resulted in the following error to occur as by default we prevent deleitons from occurring with deletion_policy having the default value of "PREVENT"

=== CONT  TestAccApigeeOrganization_apigeeOrganizationCloudFullDisableVpcPeeringTestExample
    resource_apigee_organization_generated_test.go:501: Error running post-test destroy, there may be dangling resources: exit status 1
        
        Error: Cannot destroy project as deletion_policy is set to PREVENT.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed — removing deletion_policy = "DELETE" was unintentional. The actual fix in this PR is adding a test_check_destroy template that properly polls for the async Apigee org deletion; the deletion_policy lines were accidentally dropped during that edit.

I've restored deletion_policy = "DELETE" in both template files:

  • apigee_organization_cloud_full_disable_vpc_peering_test.tf.tmpl
  • apigee_organization_cloud_full_test.tf.tmpl

@modular-magician modular-magician added the awaiting-approval Pull requests that need reviewer's approval to run presubmit tests label Apr 7, 2026
@github-actions github-actions bot requested a review from BBBmau April 7, 2026 06:56
@modular-magician modular-magician removed the awaiting-approval Pull requests that need reviewer's approval to run presubmit tests label Apr 7, 2026
@modular-magician
Copy link
Copy Markdown
Collaborator

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 provider: Diff ( 1 file changed, 26 insertions(+), 9 deletions(-))
google-beta provider: Diff ( 1 file changed, 28 insertions(+), 11 deletions(-))

@modular-magician
Copy link
Copy Markdown
Collaborator

Tests analytics

Total tests: 72
Passed tests: 32
Skipped tests: 37
Affected tests: 3

Click here to see the affected service packages
  • apigee

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
  • TestAccApigeeEnvironmentAddonsConfig_apigeeEnvAddonsAnalyticsTestExample
  • TestAccApigeeOrganization_apigeeOrganizationCloudBasicDisableVpcPeeringTestExample
  • TestAccApigeeOrganization_apigeeOrganizationCloudFullDisableVpcPeeringTestExample

Get to know how VCR tests work

@modular-magician
Copy link
Copy Markdown
Collaborator

🟢 Tests passed during RECORDING mode:
TestAccApigeeEnvironmentAddonsConfig_apigeeEnvAddonsAnalyticsTestExample [Debug log]
TestAccApigeeOrganization_apigeeOrganizationCloudBasicDisableVpcPeeringTestExample [Debug log]
TestAccApigeeOrganization_apigeeOrganizationCloudFullDisableVpcPeeringTestExample [Debug log]

🟢 No issues found for passed tests after REPLAYING rerun.


🟢 All tests passed!

View the build log or the debug log for each test

@github-actions
Copy link
Copy Markdown

github-actions bot commented Apr 9, 2026

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

UserAgent: config.UserAgent,
})
if err != nil {
// 404 (or any error) means the org is gone.
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How confident are we to accept any error from the API meaning that the org is gone? I'm not sure why we can't explicitly check for when the status response is a 404 have 100% confirmation of the apigee org not existing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Failing test(s): TestAccApigeeOrganization_apigeeOrganizationCloudFullDisableVpcPeeringTestExample

3 participants