Skip to content

Remove SchemaConfigModeAttr #11506

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

Conversation

c2thorn
Copy link
Member

@c2thorn c2thorn commented Aug 20, 2024

fixes hashicorp/terraform-provider-google#12824

Compute subnetwork preparation change: #11410
Guest Accelerator preparation change: #11425

Composer's ip_allocation_policy did not actually send an empty list when specifying an empty block, and it is create-only. Therefore removing configModeAttr doesn't make a difference

google_compute_instance_from_template and google_compute_instance_from_machine_image functionality for empty blocks is being removed to prevent accidental breaking changes for new subfields from the parent google_compute_instance

Release Note Template for Downstream PRs (will be copied)

composer: `ip_allocation_policy = []` in `google_composer_environment` is no longer valid configuration. Removing the field from configuration should not produce a diff.
compute: `secondary_ip_ranges = []` in `google_compute_subnetwork` is no longer valid configuration. To set an explicitly empty list, use `send_secondary_ip_range_if_empty` and completely remove `secondary_ip_range` from config.
compute: `guest_accelerator = []` is no longer valid configuration in `google_compute_instance`. To explicitly set an empty list of objects, set guest_accelerator.count = 0.
compute: `google_compute_instance_from_template` and `google_compute_instance_from_machine_image` `network_interface.alias_ip_range, network_interface.access_config, attached_disk, guest_accelerator, service_account, scratch_disk` can no longer be set to an empty block `[]`. Removing the fields from configuration should not produce a diff.
container: `guest_accelerator = []` is no longer valid configuration in `google_container_cluster` and `google_container_node_pool`. To explicitly set an empty list of objects, set guest_accelerator.count = 0.
container: `guest_accelerator.gpu_driver_installation_config = []` and `guest_accelerator.gpu_sharing_config = []` are no longer valid configuration in `google_container_cluster` and `google_container_node_pool`. Removing the fields from configuration should not produce a diff.

@@ -26,9 +26,6 @@
<% if property.default_from_api -%>
Computed: true,
Optional: true,
<% if property.schema_config_mode_attr -%>
Copy link
Member Author

Choose a reason for hiding this comment

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

Quick way to stop accidental generation going forward. Will need to fully remove probably after/during Go Rewrite

@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 ( 9 files changed, 19 insertions(+), 245 deletions(-))
google-beta provider: Diff ( 10 files changed, 19 insertions(+), 256 deletions(-))

@c2thorn
Copy link
Member Author

c2thorn commented Aug 20, 2024

terraform-google-conversion-build-and-unit-tests is already broken and will be fixed after the full 6.0 merge

rileykarson
rileykarson previously approved these changes Aug 20, 2024
@modular-magician
Copy link
Collaborator

Tests analytics

Total tests: 1235
Passed tests: 1138
Skipped tests: 87
Affected tests: 10

Click here to see the affected service packages
  • composer
  • compute
  • container

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
  • TestAccComposerEnvironment_update
  • TestAccComposerEnvironment_withWebServerConfig
  • TestAccComputeDisk_storagePoolSpecified
  • TestAccComputeInstance_bootDisk_storagePoolSpecified
  • TestAccComputeInstance_confidentialHyperDiskBootDisk
  • TestAccComputeInstance_guestAcceleratorSkip
  • TestAccComputeNetworkFirewallPolicyRule_multipleRules
  • TestAccComputeSubnetwork_secondaryIpRanges
  • TestAccComputeSubnetwork_secondaryIpRanges_sendEmpty
  • TestAccContainerClusterDatasource_regional

Get to know how VCR tests work

@modular-magician
Copy link
Collaborator

$\textcolor{red}{\textsf{Tests failed during RECORDING mode:}}$
TestAccComposerEnvironment_update[Error message] [Debug log]
TestAccComposerEnvironment_withWebServerConfig[Error message] [Debug log]
TestAccComputeDisk_storagePoolSpecified[Error message] [Debug log]
TestAccComputeInstance_bootDisk_storagePoolSpecified[Error message] [Debug log]
TestAccComputeInstance_confidentialHyperDiskBootDisk[Error message] [Debug log]
TestAccComputeInstance_guestAcceleratorSkip[Error message] [Debug log]
TestAccComputeNetworkFirewallPolicyRule_multipleRules[Error message] [Debug log]
TestAccComputeSubnetwork_secondaryIpRanges[Error message] [Debug log]
TestAccComputeSubnetwork_secondaryIpRanges_sendEmpty[Error message] [Debug log]
TestAccContainerClusterDatasource_regional[Error message] [Debug log]

$\textcolor{red}{\textsf{Errors occurred during RECORDING mode. Please fix them to complete your PR.}}$

View the build log or the debug log for each test

@rileykarson rileykarson dismissed their stale review August 21, 2024 15:57

TestAccComputeSubnetwork_*, TestAccComputeInstance_guestAcceleratorSkip failures

@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 ( 11 files changed, 19 insertions(+), 348 deletions(-))
google-beta provider: Diff ( 12 files changed, 19 insertions(+), 359 deletions(-))

@modular-magician
Copy link
Collaborator

Tests analytics

Total tests: 1235
Passed tests: 1139
Skipped tests: 87
Affected tests: 9

Click here to see the affected service packages
  • composer
  • compute
  • container

Action taken

Found 9 affected test(s) by replaying old test recordings. Starting RECORDING based on the most recent commit. Click here to see the affected tests
  • TestAccComposerEnvironment_update
  • TestAccComposerEnvironment_withWebServerConfig
  • TestAccComputeDisk_storagePoolSpecified
  • TestAccComputeInstance_bootDisk_storagePoolSpecified
  • TestAccComputeInstance_confidentialHyperDiskBootDisk
  • TestAccComputeNetworkFirewallPolicyRule_multipleRules
  • TestAccComputeSubnetwork_secondaryIpRanges
  • TestAccComputeSubnetwork_secondaryIpRanges_sendEmpty
  • TestAccContainerClusterDatasource_regional

Get to know how VCR tests work

@modular-magician
Copy link
Collaborator

$\textcolor{green}{\textsf{Tests passed during RECORDING mode:}}$
TestAccComputeSubnetwork_secondaryIpRanges[Debug log]
TestAccComputeSubnetwork_secondaryIpRanges_sendEmpty[Debug log]

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


$\textcolor{red}{\textsf{Tests failed during RECORDING mode:}}$
TestAccComposerEnvironment_update[Error message] [Debug log]
TestAccComposerEnvironment_withWebServerConfig[Error message] [Debug log]
TestAccComputeDisk_storagePoolSpecified[Error message] [Debug log]
TestAccComputeInstance_bootDisk_storagePoolSpecified[Error message] [Debug log]
TestAccComputeInstance_confidentialHyperDiskBootDisk[Error message] [Debug log]
TestAccComputeNetworkFirewallPolicyRule_multipleRules[Error message] [Debug log]
TestAccContainerClusterDatasource_regional[Error message] [Debug log]

$\textcolor{red}{\textsf{Errors occurred during RECORDING mode. Please fix them to complete your PR.}}$

View the build log or the debug log for each test

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.

I think TestAccComputeInstance_guestAcceleratorSkip doesn't get re-recorded because the old cassette allows it to replay successfully

@c2thorn
Copy link
Member Author

c2thorn commented Aug 21, 2024

I think TestAccComputeInstance_guestAcceleratorSkip doesn't get re-recorded because the old cassette allows it to replay successfully

I also ran it manually before pushing

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.

3 participants