Skip to content

Conversation

pratikmore0290
Copy link
Contributor

Description

Fixes #353964666

Note: If you are not associated with Google, open an issue for discussion before submitting a pull request.

Checklist

Readiness

  • Yes, merge this PR after it is approved
  • No, don't merge this PR after it is approved

Style

Testing

Intended location

API enablement

  • If the sample needs an API enabled to pass testing, I have added the service to the Test setup file

Review

  • If this sample adds a new directory, I have added codeowners to the CODEOWNERS file

@pratikmore0290 pratikmore0290 requested review from a team as code owners August 29, 2024 22:53
@snippet-bot
Copy link

snippet-bot bot commented Aug 29, 2024

Here is the summary of changes.

You are about to add 1 region tag.

This comment is generated by snippet-bot.
If you find problems with this result, please file an issue at:
https://github.com/googleapis/repo-automation-bots/issues.
To update this comment, add snippet-bot:force-run label or use the checkbox below:

  • Refresh this comment

@pratikmore0290
Copy link
Contributor Author

/gcbrun

@glasnt
Copy link
Contributor

glasnt commented Aug 30, 2024

/gcbrun

@glasnt
Copy link
Contributor

glasnt commented Aug 30, 2024

/gcbrun

@glasnt glasnt enabled auto-merge (squash) August 30, 2024 04:34
@glasnt
Copy link
Contributor

glasnt commented Sep 2, 2024

Terraform destroy is failing due to dependency issues:

Error: Error waiting for Deleting TagValue: Error code 9, message: Cannot delete tag value, 
tagValues/[value], because it is still attached to resources in 'us' region. To delete this tag value, 
remove all holds and then remove it from all resources. At least one binding was found to 
an active or deleted resource in the 'us' region.

I need to investigate more, as this part concerns me:

At least one binding was found to an active or deleted resource in the 'us' region.

This error did not occur in #733

@glasnt glasnt disabled auto-merge September 3, 2024 23:55
@glasnt
Copy link
Contributor

glasnt commented Sep 3, 2024

/gcbrun

@glasnt glasnt self-assigned this Sep 4, 2024
@glasnt glasnt added the do not merge Indicates a pull request not ready for merge, due to either quality or timing. label Sep 4, 2024
@glasnt
Copy link
Contributor

glasnt commented Sep 4, 2024

Logged upstream issue

@priyanshsaxena
Copy link

The resource is adding tags at creation. Since Terraform can deduce the implicit dependency of the table-resource on tag-value-resource because of the resource_tags field, the creation-order should be value-then-table. Terraform documentation mentions that both implicit and explicit dependencies should affect the order of creation and destruction.

One way to explicitly mark the dependency is to use the depends_on meta-field. Please try to run the graph command before and after adding the explicit dependency to see if changes anything.

terraform graph -type=plan | dot -Tpng >graph.png

@glasnt
Copy link
Contributor

glasnt commented Sep 10, 2024

The resource is adding tags at creation. Since Terraform can deduce the implicit dependency of the table-resource on tag-value-resource because of the resource_tags field, the creation-order should be value-then-table. Terraform documentation mentions that both implicit and explicit dependencies should affect the order of creation and destruction.

One way to explicitly mark the dependency is to use the depends_on meta-field. Please try to run the graph command before and after adding the explicit dependency to see if changes anything.

terraform graph -type=plan | dot -Tpng >graph.png

I've tried adding depends_on in multiple forms in local testing, but it didn't work, hence the upstream report.

@glasnt
Copy link
Contributor

glasnt commented Sep 10, 2024

To unblock this for now, we can add test.yaml to remove testing this sample.

@glasnt
Copy link
Contributor

glasnt commented Sep 10, 2024

/gcbrun

@pratikmore0290
Copy link
Contributor Author

/gcbrun

@glasnt glasnt removed the do not merge Indicates a pull request not ready for merge, due to either quality or timing. label Sep 10, 2024
@glasnt
Copy link
Contributor

glasnt commented Sep 10, 2024

/gcbrun

@glasnt glasnt enabled auto-merge (squash) September 10, 2024 21:50
@glasnt glasnt merged commit 7c9be60 into terraform-google-modules:main Sep 10, 2024
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants