Skip to content
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

Add DSF on resource_labels in node_config #12877

Merged
merged 4 commits into from
Jan 28, 2025

Conversation

slevenick
Copy link
Contributor

Fixes: hashicorp/terraform-provider-google#15848

Well, only adds a DSF on resource_labels, doesn't fix the inability to remove all resource_labels
Release Note Template for Downstream PRs (will be copied)

See Write release notes for guidance.

container: fixed a diff caused by server-side set values for `node_config.resource_labels`

@modular-magician
Copy link
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, 32 insertions(+))
google-beta provider: Diff ( 1 file changed, 32 insertions(+))

Errors

google provider:

  • The diff processor failed to build. This is usually due to the downstream provider failing to compile.

@modular-magician
Copy link
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, 33 insertions(+))
google-beta provider: Diff ( 1 file changed, 32 insertions(+))

@modular-magician
Copy link
Collaborator

Tests analytics

Total tests: 223
Passed tests: 212
Skipped tests: 11
Affected tests: 0

Click here to see the affected service packages
  • container

🟢 All tests passed!

View the build log

@modular-magician
Copy link
Collaborator

Tests analytics

Total tests: 223
Passed tests: 212
Skipped tests: 11
Affected tests: 0

Click here to see the affected service packages
  • container

🟢 All tests passed!

View the build log

@modular-magician
Copy link
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, 29 insertions(+), 4 deletions(-))
google-beta provider: Diff ( 1 file changed, 28 insertions(+), 4 deletions(-))

Missing test report

Your PR includes resource fields which are not covered by any test.

Resource: google_container_cluster (429 total tests)
Please add an acceptance test which includes these fields. The test should include the following:

resource "google_container_cluster" "primary" {
  node_config {
    resource_labels = # value needed
  }
  node_pool {
    node_config {
      resource_labels = # value needed
    }
  }
}

@modular-magician
Copy link
Collaborator

Tests analytics

Total tests: 223
Passed tests: 212
Skipped tests: 11
Affected tests: 0

Click here to see the affected service packages
  • container

🟢 All tests passed!

View the build log

@slevenick slevenick requested a review from rileykarson January 28, 2025 00:15
@modular-magician
Copy link
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, 21 insertions(+), 4 deletions(-))
google-beta provider: Diff ( 1 file changed, 20 insertions(+), 4 deletions(-))

Missing test report

Your PR includes resource fields which are not covered by any test.

Resource: google_container_cluster (429 total tests)
Please add an acceptance test which includes these fields. The test should include the following:

resource "google_container_cluster" "primary" {
  node_config {
    resource_labels = # value needed
  }
  node_pool {
    node_config {
      resource_labels = # value needed
    }
  }
}

@modular-magician
Copy link
Collaborator

Tests analytics

Total tests: 223
Passed tests: 212
Skipped tests: 11
Affected tests: 0

Click here to see the affected service packages
  • container

🟢 All tests passed!

View the build log

Copy link
Member

@rileykarson rileykarson left a comment

Choose a reason for hiding this comment

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

Have you tested this in all 3 locations node_config.resource_labels appears?

@slevenick
Copy link
Contributor Author

Have you tested this in all 3 locations node_config.resource_labels appears?

Yes, manually tested by creating clusters/node pools and manually adding goog-gke resource_labels via the UI and seeing no diff. Triggered update and removal of resource_labels via TF and everything seems to work correctly

@slevenick slevenick merged commit 8e47ba0 into GoogleCloudPlatform:main Jan 28, 2025
13 checks passed
anoopkverma-google pushed a commit to anoopkverma-google/magic-modules that referenced this pull request Jan 31, 2025
PerlMonker303 pushed a commit to PerlMonker303/magic-modules that referenced this pull request Feb 3, 2025
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.

Removing All Labels on GKE Node Pool Fails
3 participants