Skip to content

Enhance subnetwork.yaml to allow purpose field to be patch-able #12922

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
merged 53 commits into from
Mar 6, 2025

Conversation

gvijbha
Copy link
Contributor

@gvijbha gvijbha commented Jan 30, 2025

Added necessary code to make 'purpose' field updatable.

This pull request takes care of issue: hashicorp/terraform-provider-google#19937

Release Note Template for Downstream PRs (will be copied)

See Write release notes for guidance.

compute: added necessary code to make subnetwork's 'purpose' field updatable.

Added necessary code to make 'purpose' field updatable.
Copy link

Hello! I am a robot. Tests will require approval from a repository maintainer to run.

@melinath, a repository maintainer, has been assigned to review your changes. If you have not received review feedback within 2 business days, please leave a comment on this PR asking them to take a look.

You can help make sure that review is quick by doing a self-review and by running impacted tests locally.

@github-actions github-actions bot requested a review from melinath January 30, 2025 16:41
@modular-magician modular-magician added awaiting-approval Pull requests that need reviewer's approval to run presubmit tests and removed awaiting-approval Pull requests that need reviewer's approval to run presubmit tests labels Jan 30, 2025
Copy link
Member

@melinath melinath left a comment

Choose a reason for hiding this comment

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

In addition to the change requested below, please add this field to an update test: https://googlecloudplatform.github.io/magic-modules/test/test/#add-an-update-test

Co-authored-by: Stephen Lewis (Burrows) <[email protected]>
@github-actions github-actions bot requested a review from melinath January 31, 2025 04:41
@modular-magician modular-magician added the awaiting-approval Pull requests that need reviewer's approval to run presubmit tests label Jan 31, 2025
Copy link
Member

@melinath melinath left a comment

Choose a reason for hiding this comment

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

As previously requested, please add this field to an update test: https://googlecloudplatform.github.io/magic-modules/test/test/#add-an-update-test

Copy link

@gvijbha, this PR is waiting for action from you. If no action is taken, this PR will be closed in 28 days.

Please address any comments or change requests, or re-request review from a core reviewer if no action is required.

Image showing the re-request review button

This notification can be disabled with the disable-automatic-closure label.

@github-actions github-actions bot requested a review from melinath February 15, 2025 03:58
@modular-magician modular-magician added service/compute-vpc and removed awaiting-approval Pull requests that need reviewer's approval to run presubmit tests labels Feb 15, 2025
@modular-magician

This comment was marked as outdated.

@modular-magician

This comment was marked as outdated.

@modular-magician

This comment was marked as outdated.

@modular-magician

This comment was marked as outdated.

@modular-magician

This comment was marked as outdated.

@modular-magician

This comment was marked as outdated.

@modular-magician

This comment was marked as outdated.

@modular-magician

This comment was marked as outdated.

@modular-magician

This comment was marked as outdated.

@modular-magician
Copy link
Collaborator

Tests analytics

Total tests: 1125
Passed tests: 1044
Skipped tests: 79
Affected tests: 2

Click here to see the affected service packages
  • compute

Action taken

Found 2 affected test(s) by replaying old test recordings. Starting RECORDING based on the most recent commit. Click here to see the affected tests
  • TestAccComputeSubnetwork_purposeUpdate
  • TestAccComputeSubnetwork_update

Get to know how VCR tests work

1 similar comment
@modular-magician
Copy link
Collaborator

Tests analytics

Total tests: 1125
Passed tests: 1044
Skipped tests: 79
Affected tests: 2

Click here to see the affected service packages
  • compute

Action taken

Found 2 affected test(s) by replaying old test recordings. Starting RECORDING based on the most recent commit. Click here to see the affected tests
  • TestAccComputeSubnetwork_purposeUpdate
  • TestAccComputeSubnetwork_update

Get to know how VCR tests work

@modular-magician
Copy link
Collaborator

🔴 Tests failed during RECORDING mode:
TestAccComputeSubnetwork_purposeUpdate [Error message] [Debug log]
TestAccComputeSubnetwork_update [Error message] [Debug log]

🔴 Errors occurred during RECORDING mode. Please fix them to complete your PR.

View the build log or the debug log for each test

@modular-magician
Copy link
Collaborator

🔴 Tests failed during RECORDING mode:
TestAccComputeSubnetwork_purposeUpdate [Error message] [Debug log]
TestAccComputeSubnetwork_update [Error message] [Debug log]

🔴 Errors occurred during RECORDING mode. Please fix them to complete your PR.

View the build log or the debug log for each test

Comment on lines +176 to +184
{
// update the purpose from PEER_MIGRATION to PRIVATE
Config: testAccComputeSubnetwork_purposeUpdate1(cnName, subnetworkName),
ConfigPlanChecks: resource.ConfigPlanChecks{
PreApply: []plancheck.PlanCheck{
plancheck.ExpectResourceAction("google_compute_subnetwork.network-with-migration-purpose", plancheck.ResourceActionUpdate),
},
},
},
Copy link
Member

Choose a reason for hiding this comment

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

This test is failing with the following error message:

    resource_compute_subnetwork_test.go:162: Step 2/3 error: Pre-apply plan check(s) failed:
        'google_compute_subnetwork.network-with-migration-purpose' - expected Update, got action(s): [delete create]

The error message means that google_compute_subnetwork.network-with-migration-purpose is getting recreated in step 2 (this step) instead of updated. That means an immutable field was changed in the configuration, preventing us from testing the update logic - this is exactly the kind of test issue that the ConfigPlanChecks are there to catch 🎉

Looking at your configs, I don't see any obvious changes to immutable fields. Digging a bit more, it appears that the overall Subnetwork resource is considered immutable:

This means that only fields that specify a custom update_url can be updated in-place. However, it looks like a number of fields use the normal resource update URL as their "custom URL" - the Subnetwork resource is very old, so this seems like it's probably an oversight / left over from before the resource could be updated in place.

I'd recommend removing the resource-level immutability and adding immutable: true to all immutable fields. You'll also need to set the resource-level update_url and update_verb properties. That should fix the issue.

@github-actions github-actions bot requested a review from melinath March 6, 2025 05:11
@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 ( 3 files changed, 154 insertions(+), 11 deletions(-))
google-beta provider: Diff ( 3 files changed, 154 insertions(+), 11 deletions(-))

2 similar comments
@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 ( 3 files changed, 154 insertions(+), 11 deletions(-))
google-beta provider: Diff ( 3 files changed, 154 insertions(+), 11 deletions(-))

@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 ( 3 files changed, 154 insertions(+), 11 deletions(-))
google-beta provider: Diff ( 3 files changed, 154 insertions(+), 11 deletions(-))

@modular-magician
Copy link
Collaborator

Tests analytics

Total tests: 1133
Passed tests: 1048
Skipped tests: 79
Affected tests: 6

Click here to see the affected service packages
  • compute

Action taken

Found 6 affected test(s) by replaying old test recordings. Starting RECORDING based on the most recent commit. Click here to see the affected tests
  • TestAccComputeInstanceFromMachineImage_terminationTime
  • TestAccComputeInstanceFromTemplate_TerminationTime
  • TestAccComputeInstanceTemplate_instanceTerminationAction_terminationTime
  • TestAccComputeInstance_schedulingTerminationTime
  • TestAccComputeRegionInstanceTemplate_instanceTerminationAction_terminationTime
  • TestAccComputeSubnetwork_purposeUpdate

Get to know how VCR tests work

2 similar comments
@modular-magician
Copy link
Collaborator

Tests analytics

Total tests: 1133
Passed tests: 1048
Skipped tests: 79
Affected tests: 6

Click here to see the affected service packages
  • compute

Action taken

Found 6 affected test(s) by replaying old test recordings. Starting RECORDING based on the most recent commit. Click here to see the affected tests
  • TestAccComputeInstanceFromMachineImage_terminationTime
  • TestAccComputeInstanceFromTemplate_TerminationTime
  • TestAccComputeInstanceTemplate_instanceTerminationAction_terminationTime
  • TestAccComputeInstance_schedulingTerminationTime
  • TestAccComputeRegionInstanceTemplate_instanceTerminationAction_terminationTime
  • TestAccComputeSubnetwork_purposeUpdate

Get to know how VCR tests work

@modular-magician
Copy link
Collaborator

Tests analytics

Total tests: 1133
Passed tests: 1048
Skipped tests: 79
Affected tests: 6

Click here to see the affected service packages
  • compute

Action taken

Found 6 affected test(s) by replaying old test recordings. Starting RECORDING based on the most recent commit. Click here to see the affected tests
  • TestAccComputeInstanceFromMachineImage_terminationTime
  • TestAccComputeInstanceFromTemplate_TerminationTime
  • TestAccComputeInstanceTemplate_instanceTerminationAction_terminationTime
  • TestAccComputeInstance_schedulingTerminationTime
  • TestAccComputeRegionInstanceTemplate_instanceTerminationAction_terminationTime
  • TestAccComputeSubnetwork_purposeUpdate

Get to know how VCR tests work

@modular-magician
Copy link
Collaborator

🟢 Tests passed during RECORDING mode:
TestAccComputeInstanceFromMachineImage_terminationTime [Debug log]
TestAccComputeInstanceFromTemplate_TerminationTime [Debug log]
TestAccComputeInstanceTemplate_instanceTerminationAction_terminationTime [Debug log]
TestAccComputeInstance_schedulingTerminationTime [Debug log]
TestAccComputeRegionInstanceTemplate_instanceTerminationAction_terminationTime [Debug log]
TestAccComputeSubnetwork_purposeUpdate [Debug log]

🟢 No issues found for passed tests after REPLAYING rerun.


🟢 All tests passed!

View the build log or the debug log for each test

@modular-magician
Copy link
Collaborator

🟢 Tests passed during RECORDING mode:
TestAccComputeInstanceFromMachineImage_terminationTime [Debug log]
TestAccComputeInstanceFromTemplate_TerminationTime [Debug log]
TestAccComputeInstanceTemplate_instanceTerminationAction_terminationTime [Debug log]
TestAccComputeInstance_schedulingTerminationTime [Debug log]
TestAccComputeRegionInstanceTemplate_instanceTerminationAction_terminationTime [Debug log]
TestAccComputeSubnetwork_purposeUpdate [Debug log]

🟢 No issues found for passed tests after REPLAYING rerun.


🟢 All tests passed!

View the build log or the debug log for each test

@modular-magician
Copy link
Collaborator

🟢 Tests passed during RECORDING mode:
TestAccComputeInstanceFromMachineImage_terminationTime [Debug log]
TestAccComputeInstanceFromTemplate_TerminationTime [Debug log]
TestAccComputeInstanceTemplate_instanceTerminationAction_terminationTime [Debug log]
TestAccComputeInstance_schedulingTerminationTime [Debug log]
TestAccComputeRegionInstanceTemplate_instanceTerminationAction_terminationTime [Debug log]
TestAccComputeSubnetwork_purposeUpdate [Debug log]

🟢 No issues found for passed tests after REPLAYING rerun.


🟢 All tests passed!

View the build log or the debug log for each test

Copy link
Member

@melinath melinath left a comment

Choose a reason for hiding this comment

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

LGTM!

@melinath melinath added this pull request to the merge queue Mar 6, 2025
Merged via the queue into GoogleCloudPlatform:main with commit 37d08f7 Mar 6, 2025
24 checks passed
NA2047 pushed a commit to NA2047/magic-modules that referenced this pull request Mar 13, 2025
yerniyazN pushed a commit to yerniyazN/magic-modules that referenced this pull request Mar 17, 2025
JaylonmcShan03 pushed a commit to JaylonmcShan03/magic-modules-jmcshan that referenced this pull request Mar 25, 2025
JaylonmcShan03 pushed a commit to JaylonmcShan03/magic-modules-jmcshan that referenced this pull request Mar 30, 2025
Dawid212 pushed a commit to Dawid212/magic-modules that referenced this pull request Apr 9, 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.

3 participants