Skip to content

Default from API for ServiceAttachment.reconcileConnections#9003

Closed
bobyu-google wants to merge 55 commits into
GoogleCloudPlatform:FEATURE-BRANCH-major-release-5.0.0from
bobyu-google:bobyu-google-patch-1
Closed

Default from API for ServiceAttachment.reconcileConnections#9003
bobyu-google wants to merge 55 commits into
GoogleCloudPlatform:FEATURE-BRANCH-major-release-5.0.0from
bobyu-google:bobyu-google-patch-1

Conversation

@bobyu-google

@bobyu-google bobyu-google commented Sep 19, 2023

Copy link
Copy Markdown
Contributor

Release Note Template for Downstream PRs (will be copied)

Set the default value of reconcileConnections from Service Attachment API

trodge and others added 30 commits September 13, 2023 13:40
* Add new TPUv2 product and VM resource in beta

* Add TPUv2 vm full example

* Add async operation polling and update API version

* Update examples to pass tests

* Change example zone to avoid capacity issues during tests on those examples

* Hardcode version and accelerator type in to examples since github check values vary from local

* Change API version from v2alpha1 to v2, will override to v2alpha1 in the terraform-eap process

* Add update test for description
* Artifact Registry: implement VPC SC Config

* Altering behavior from reset resource to simply drop resource

* removing skip_test

* re-adding skip_test since organization-level resources are out of scope for testing
… usable (GoogleCloudPlatform#8939)

* Fix slo_id pattern in google_monitoring_slo

* Fix servie_id pattern in google_monitoring_custom_service
* Merge base branch before running rake tests

* Fetch depth 2
…like the SDK (GoogleCloudPlatform#8798)

* Make PF config code change `project = ""` to null value before checking ENVs

* Make plugin framework config code change all empty strings in config to null values

* Fix defect in unit test for `project`, uncomment

* Uncomment empty string unit tests

* Fix defect in test; expeceted value should be null list

* Add handling of empty lists in `HandleZeroValues`

* Fix typo in comment

* Add test case asserting `impersonate_service_account` empty strings are overridden by an ENV

* Update SDK `batching` tests: rename test cases,  make it clear values are arbitary

* Update `HandleZeroValues` to handle `batching` argument

* Uncomment empty string test

* Change test inputs from 123s to 45s

This is because 123s is transformed to 2m3s, but 45s remains 45s

* Protect against Batching being null/unknown in `HandleZeroValues`

* Add non-VCR acceptance test that shows provider behaviour when `credentials=""` and `GOOGLE_CREDENTIALS` interact
* Add test for unavailable zones logic

* Add test for unavailable zones logic

* Add test for unavailable zones logic

* Add test for unavailable zones logic

* Add test for unavailable zones logic
* adding RestorePlan to Backup for GKE

* fixed spacing issue

* fixed more spacing issue

* adding newline end of file

* fixed trailing spaces

* fixed test file names

* added test gaps

* adding description adn labels

* fixed tests

* fixed protected application name length

* fixed protected app test

* fix restore all namespaces test names

* removed bp prefix from resource names

* removed last bp ref
SarahFrench and others added 3 commits September 19, 2023 20:11
…oogleCloudPlatform#8943)

* Uncomment test cases for unknown values

* Update test case names

* Update unknown value test for `project`, make it pass

* Update unknown value test for `access_token`, make `access_token` and `credentials` tests pass

* Update unknown value tests for `region` and `zone`, make those tests pass

* Update unknown value tests for `user_project_override`, make that test pass

* Update unknown value test for `impersonate_service_account`, make that test pass

* Update unknown value tests for `request_reason` and `request_timeout`, make those tests pass

* Make unknown batching.send_after and batching.enable_batching values be set to same defaults as if they were null, update test

* Update code to handle when the whole batching block is Unknown

* Update the test function for `batching` unit tests to navigate how `GetBatchingConfig` is used by the code

* Update code to handle null/unknown Scopes and ImpersonateServiceAccountDelegates values

* Improve `impersonate_service_account_delegates` tests for unknown values

* Add missing null test case for `batching` field

* Add non-VCR acceptance test to assert handling of unknown values in provider config

Only testing `credentials` currently.
GoogleCloudPlatform#8971)

* Replace InstanceInfo call with Instances call for regional reliability

* Handle unavailable error when no overlap

* Handle unavailable error when no overlap

* Add tests

* Add tests

* Add tests

* Add tests
@modular-magician

Copy link
Copy Markdown
Collaborator

Hello! I am a robot. It looks like you are a: Community Contributor Googler Core Contributor. Tests will require approval to run.

@shuyama1, 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.

@modular-magician modular-magician added the awaiting-approval Pull requests that need reviewer's approval to run presubmit tests label Sep 19, 2023
@shuyama1

Copy link
Copy Markdown
Member

@bobyu-google would you mind taking a look at the internal contribution guide to get your GH account registered so the tests will automatically run for your PR for faster test feedback.

Plus, regarding this change, is the default value changed for this field?

@bobyu-google

Copy link
Copy Markdown
Contributor Author

@bobyu-google would you mind taking a look at the internal contribution guide to get your GH account registered so the tests will automatically run for your PR for faster test feedback.

Plus, regarding this change, is the default value changed for this field?

Updated my email to public. Could you let me know if I'm still missing something?

@shuyama1

Copy link
Copy Markdown
Member

let 's see if the tests got triggered automatically /gcbrun

@shuyama1

Copy link
Copy Markdown
Member

/gcbrun

@modular-magician modular-magician removed the awaiting-approval Pull requests that need reviewer's approval to run presubmit tests label Sep 19, 2023
@modular-magician

Copy link
Copy Markdown
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 reconcile_connections default value changed from true to on google_compute_service_attachment - 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 ( 1 file changed, 1 insertion(+), 1 deletion(-))
Terraform Beta: Diff ( 1 file changed, 1 insertion(+), 1 deletion(-))

burov and others added 3 commits September 19, 2023 14:23
)

* Added support for project-level custom modules

Related to b/296259216

* gofmt

* Added client-side validation of display_name

* Fixed expressions in update test

* Added mutex
Co-authored-by: Hamza Hassan <hamzahassan@google.com>
@modular-magician

Copy link
Copy Markdown
Collaborator

Tests analytics

Total tests: 3070
Passed tests 2763
Skipped tests: 300
Affected tests: 7

Action taken

Found 7 affected test(s) by replaying old test recordings. Starting RECORDING based on the most recent commit. Click here to see the affected tests
TestAccComputeServiceAttachment_serviceAttachmentBasicExample|TestAccComputeServiceAttachment_serviceAttachmentBasicExampleUpdate|TestAccComputeServiceAttachment_serviceAttachmentExplicitProjectsExample|TestAccComputeRegionNetworkEndpointGroup_regionNetworkEndpointGroupPscServiceAttachmentExample|TestAccComputeForwardingRule_forwardingRuleVpcPscExampleUpdate|TestAccComputeForwardingRule_forwardingRuleVpcPscNoAutomateDnsExample|TestAccComputeForwardingRule_forwardingRuleVpcPscExample

Get to know how VCR tests work

@modular-magician

Copy link
Copy Markdown
Collaborator

$\textcolor{green}{\textsf{Tests passed during RECORDING mode:}}$
TestAccComputeServiceAttachment_serviceAttachmentBasicExample[Debug log]
TestAccComputeServiceAttachment_serviceAttachmentBasicExampleUpdate[Debug log]
TestAccComputeServiceAttachment_serviceAttachmentExplicitProjectsExample[Debug log]
TestAccComputeRegionNetworkEndpointGroup_regionNetworkEndpointGroupPscServiceAttachmentExample[Debug log]
TestAccComputeForwardingRule_forwardingRuleVpcPscExampleUpdate[Debug log]
TestAccComputeForwardingRule_forwardingRuleVpcPscNoAutomateDnsExample[Debug log]
TestAccComputeForwardingRule_forwardingRuleVpcPscExample[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

- !ruby/object:Api::Type::Boolean
name: reconcileConnections
default_value: true
default_from_api: true

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Why is this change needed? Is the default value changed for this field?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

We want to default from the API instead of setting the default explicitly at terraform

@modular-magician

Copy link
Copy Markdown
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 reconcile_connections default value changed from true to on google_compute_service_attachment - 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 ( 2 files changed, 2 insertions(+), 5 deletions(-))
Terraform Beta: Diff ( 2 files changed, 2 insertions(+), 5 deletions(-))

@bobyu-google bobyu-google changed the base branch from main to feature-branch-release-5.0.0 September 20, 2023 00:23
@modular-magician

Copy link
Copy Markdown
Collaborator

Tests analytics

Total tests: 3075
Passed tests 2776
Skipped tests: 299
Affected tests: 0

$\textcolor{green}{\textsf{All tests passed in REPLAYING mode.}}$
View the build log

@bobyu-google bobyu-google changed the base branch from feature-branch-release-5.0.0 to FEATURE-BRANCH-major-release-5.0.0 September 20, 2023 18:04
@bobyu-google bobyu-google force-pushed the bobyu-google-patch-1 branch 2 times, most recently from 8e93d5a to 3fae246 Compare September 20, 2023 18:07
@shuyama1

Copy link
Copy Markdown
Member

updated the change in #9026

@shuyama1 shuyama1 closed this Sep 20, 2023
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.