Skip to content

Conversation

pib
Copy link
Contributor

@pib pib commented Sep 16, 2025

Fixes #2330

@pib pib requested review from a team, apeabody and ericyz as code owners September 16, 2025 17:37
@pib pib force-pushed the auto-config-resource-manager-tags branch from 6ddcc03 to 2c2393a Compare September 16, 2025 17:52
@apeabody
Copy link
Collaborator

/gcbrun

@pib
Copy link
Contributor Author

pib commented Sep 24, 2025

@apeabody would you like me to add a new test or two for this change? Or I could add the new field to one or more existing tests if you'd like.

I got the test projects set up and ran the tests in a sandbox org. All the tests in build/int.cloudbuild.yaml passed (I put them in a shell script with set -e and ran them all that way).

@apeabody
Copy link
Collaborator

@apeabody would you like me to add a new test or two for this change? Or I could add the new field to one or more existing tests if you'd like.

I got the test projects set up and ran the tests in a sandbox org. All the tests in build/int.cloudbuild.yaml passed (I put them in a shell script with set -e and ran them all that way).

Thanks @pib! - The current test failure does not appear to be related to your change and can be re-run. That said we should add test coverage to an existing example, likely examples/node_pool.

@pib
Copy link
Contributor Author

pib commented Sep 25, 2025

@apeabody I have added it to the node_pool test. Rather than putting the new field in the golden file, I had to check for it explicitly in the test because testutils.TGKEAssertGolden doesn't handle having values that need to be sanitized in map keys.

@apeabody
Copy link
Collaborator

/gcbrun

Copy link
Collaborator

@apeabody apeabody left a comment

Choose a reason for hiding this comment

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

Thanks for the contribution @pib!

@apeabody apeabody self-assigned this Sep 26, 2025
@apeabody apeabody merged commit 3a1945f into terraform-google-modules:main Sep 26, 2025
4 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.

dynamic block "node_pool_auto_config" in cluster.tf missing "resource_manager_tags" field

2 participants