Skip to content

Apply new annotations model to more resources #8931

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

Merged

Conversation

zli82016
Copy link
Member

@zli82016 zli82016 commented Sep 12, 2023

Release Note Template for Downstream PRs (will be copied)

provider: made the `annotations` field be non-authoritative and only manage the annotations defined by the users on the resource through Terraform in all of the resources with standard `annotations` field.
provider: added the output-only `effective_annotations` to all of the resources with standard `annotations` field. This field lists all of annotations present on the resource in GCP, including the annotations configured through Terraform, the system, and other clients.

@zli82016 zli82016 force-pushed the annotation-more-resources branch from 8c0feef to 1cb43d5 Compare September 12, 2023 22:00
@modular-magician
Copy link
Collaborator

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

Breaking Change(s) Detected

The following breaking change(s) were detected within your pull request.

  • Field annotations transitioned from optional+computed to optional google_gkeonprem_bare_metal_cluster - reference
  • Field annotations transitioned from optional+computed to optional google_gkeonprem_bare_metal_node_pool - reference
  • Field annotations transitioned from optional+computed to optional google_gkeonprem_vmware_cluster - reference
  • Field annotations transitioned from optional+computed to optional google_gkeonprem_vmware_node_pool - reference

If you believe this detection to be incorrect please raise the concern with your reviewer. If you intend to make this change you will need to wait for a major release window. An override-breaking-change label can be added to allow merging.

Diff report

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

Terraform GA: Diff ( 16 files changed, 150 insertions(+), 77 deletions(-))
Terraform Beta: Diff ( 32 files changed, 576 insertions(+), 307 deletions(-))
TF Conversion: Diff ( 10 files changed, 156 insertions(+), 156 deletions(-))

@modular-magician
Copy link
Collaborator

Tests analytics

Total tests: 3008
Passed tests 2698
Skipped tests: 302
Affected tests: 8

Action taken

Found 8 affected test(s) by replaying old test recordings. Starting RECORDING based on the most recent commit. Click here to see the affected tests
TestAccBigtableAppProfile_bigtableAppProfileMulticlusterExample|TestAccBigtableAppProfile_bigtableAppProfileSingleclusterExample|TestAccBigtableAppProfile_bigtableAppProfileAnyclusterExample|TestAccCloudRunV2Job_cloudrunv2JobFullUpdate|TestAccSecretManagerSecret_annotationsUpdate|TestAccWorkstationsWorkstationConfig_update|TestAccWorkstationsWorkstationCluster_update|TestAccWorkstationsWorkstationCluster_Private_update

Get to know how VCR tests work

@modular-magician
Copy link
Collaborator

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

Rerun these tests in REPLAYING mode to catch issues

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


$\textcolor{red}{\textsf{Tests failed during RECORDING mode:}}$
TestAccCloudRunV2Job_cloudrunv2JobFullUpdate[Error message] [Debug log]
TestAccSecretManagerSecret_annotationsUpdate[Error message] [Debug log]
TestAccWorkstationsWorkstationConfig_update[Error message] [Debug log]
TestAccWorkstationsWorkstationCluster_update[Error message] [Debug log]
TestAccWorkstationsWorkstationCluster_Private_update[Error message] [Debug log]

$\textcolor{red}{\textsf{Please fix these to complete your PR.}}$
View the build log or the debug log for each test

@zli82016 zli82016 force-pushed the annotation-more-resources branch from 1cb43d5 to 4a979d8 Compare September 13, 2023 20:46
@modular-magician
Copy link
Collaborator

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

Breaking Change(s) Detected

The following breaking change(s) were detected within your pull request.

  • Field annotations transitioned from optional+computed to optional google_gkeonprem_bare_metal_cluster - reference
  • Field annotations transitioned from optional+computed to optional google_gkeonprem_bare_metal_node_pool - reference
  • Field annotations transitioned from optional+computed to optional google_gkeonprem_vmware_cluster - reference
  • Field annotations transitioned from optional+computed to optional google_gkeonprem_vmware_node_pool - reference

If you believe this detection to be incorrect please raise the concern with your reviewer. If you intend to make this change you will need to wait for a major release window. An override-breaking-change label can be added to allow merging.

Diff report

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

Terraform GA: Diff ( 18 files changed, 155 insertions(+), 82 deletions(-))
Terraform Beta: Diff ( 37 files changed, 594 insertions(+), 319 deletions(-))
TF Conversion: Diff ( 10 files changed, 156 insertions(+), 156 deletions(-))

@zli82016 zli82016 force-pushed the annotation-more-resources branch from 4a979d8 to fc9d361 Compare September 13, 2023 21:15
@modular-magician
Copy link
Collaborator

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

Breaking Change(s) Detected

The following breaking change(s) were detected within your pull request.

  • Field annotations transitioned from optional+computed to optional google_gkeonprem_bare_metal_cluster - reference
  • Field annotations transitioned from optional+computed to optional google_gkeonprem_bare_metal_node_pool - reference
  • Field annotations transitioned from optional+computed to optional google_gkeonprem_vmware_cluster - reference
  • Field annotations transitioned from optional+computed to optional google_gkeonprem_vmware_node_pool - reference

If you believe this detection to be incorrect please raise the concern with your reviewer. If you intend to make this change you will need to wait for a major release window. An override-breaking-change label can be added to allow merging.

Diff report

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

Terraform GA: Diff ( 18 files changed, 162 insertions(+), 83 deletions(-))
Terraform Beta: Diff ( 41 files changed, 678 insertions(+), 351 deletions(-))
TF Conversion: Diff ( 10 files changed, 156 insertions(+), 156 deletions(-))
TF OiCS: Diff ( 1 file changed, 4 insertions(+))

@zli82016 zli82016 force-pushed the annotation-more-resources branch from fc9d361 to 24b5b69 Compare September 13, 2023 21:38
@modular-magician
Copy link
Collaborator

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

Breaking Change(s) Detected

The following breaking change(s) were detected within your pull request.

  • Field annotations transitioned from optional+computed to optional google_gkeonprem_bare_metal_cluster - reference
  • Field annotations transitioned from optional+computed to optional google_gkeonprem_bare_metal_node_pool - reference
  • Field annotations transitioned from optional+computed to optional google_gkeonprem_vmware_cluster - reference
  • Field annotations transitioned from optional+computed to optional google_gkeonprem_vmware_node_pool - reference

If you believe this detection to be incorrect please raise the concern with your reviewer. If you intend to make this change you will need to wait for a major release window. An override-breaking-change label can be added to allow merging.

Diff report

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

Terraform GA: Diff ( 18 files changed, 162 insertions(+), 83 deletions(-))
Terraform Beta: Diff ( 41 files changed, 678 insertions(+), 351 deletions(-))
TF Conversion: Diff ( 10 files changed, 156 insertions(+), 156 deletions(-))
TF OiCS: Diff ( 1 file changed, 4 insertions(+))

@zli82016 zli82016 force-pushed the annotation-more-resources branch from 24b5b69 to 0d026ca Compare September 13, 2023 22:01
@modular-magician
Copy link
Collaborator

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

Breaking Change(s) Detected

The following breaking change(s) were detected within your pull request.

  • Field annotations transitioned from optional+computed to optional google_gkeonprem_bare_metal_cluster - reference
  • Field annotations transitioned from optional+computed to optional google_gkeonprem_bare_metal_node_pool - reference
  • Field annotations transitioned from optional+computed to optional google_gkeonprem_vmware_cluster - reference
  • Field annotations transitioned from optional+computed to optional google_gkeonprem_vmware_node_pool - reference

If you believe this detection to be incorrect please raise the concern with your reviewer. If you intend to make this change you will need to wait for a major release window. An override-breaking-change label can be added to allow merging.

Diff report

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

Terraform GA: Diff ( 18 files changed, 162 insertions(+), 83 deletions(-))
Terraform Beta: Diff ( 41 files changed, 678 insertions(+), 351 deletions(-))
TF Conversion: Diff ( 10 files changed, 156 insertions(+), 156 deletions(-))
TF OiCS: Diff ( 1 file changed, 4 insertions(+))

@modular-magician
Copy link
Collaborator

Tests analytics

Total tests: 3009
Passed tests 2696
Skipped tests: 303
Affected tests: 10

Action taken

Found 10 affected test(s) by replaying old test recordings. Starting RECORDING based on the most recent commit. Click here to see the affected tests
TestAccGkeonpremBareMetalCluster_bareMetalClusterUpdateBasic|TestAccGkeonpremBareMetalAdminCluster_gkeonpremBareMetalAdminClusterFullExample|TestAccGkeonpremVmwareCluster_vmwareClusterUpdateBasic|TestAccGkeonpremBareMetalNodePool_bareMetalNodePoolUpdate|TestAccGkeonpremVmwareNodePool_vmwareNodePoolUpdate|TestAccWorkstationsWorkstationConfigIamBindingGenerated|TestAccWorkstationsWorkstationConfig_workstationConfigBasicExample|TestAccWorkstationsWorkstationConfigIamMemberGenerated|TestAccWorkstationsWorkstationConfig_update|TestAccWorkstationsWorkstationConfigIamPolicyGenerated

Get to know how VCR tests work

@modular-magician
Copy link
Collaborator

$\textcolor{green}{\textsf{Tests passed during RECORDING mode:}}$
TestAccGkeonpremBareMetalCluster_bareMetalClusterUpdateBasic[Debug log]
TestAccGkeonpremBareMetalAdminCluster_gkeonpremBareMetalAdminClusterFullExample[Debug log]
TestAccGkeonpremVmwareCluster_vmwareClusterUpdateBasic[Debug log]
TestAccGkeonpremBareMetalNodePool_bareMetalNodePoolUpdate[Debug log]
TestAccGkeonpremVmwareNodePool_vmwareNodePoolUpdate[Debug log]
TestAccWorkstationsWorkstationConfigIamBindingGenerated[Debug log]
TestAccWorkstationsWorkstationConfig_workstationConfigBasicExample[Debug log]
TestAccWorkstationsWorkstationConfigIamMemberGenerated[Debug log]
TestAccWorkstationsWorkstationConfig_update[Debug log]
TestAccWorkstationsWorkstationConfigIamPolicyGenerated[Debug log]

Rerun these tests in REPLAYING mode to catch issues

$\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

@zli82016 zli82016 requested a review from c2thorn September 14, 2023 16:32
@@ -100,7 +100,6 @@ properties:
Prefix must be a DNS subdomain.
Name must be 63 characters or less, begin and end with alphanumerics,
with dashes (-), underscores (_), dots (.), and alphanumerics between.
default_from_api: true
Copy link
Member

Choose a reason for hiding this comment

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

double checking / need a reminder - there is another field that is computed that will replace the functionality of this field being default_from_api right? And this field is moving to write-only?

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.

3 participants