Skip to content

apigee: Added in-place update for consumer_accept_list field in google_apigee_instance #11479

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

entertvl
Copy link
Contributor

Fixes hashicorp/terraform-provider-google#14237.

Release Note Template for Downstream PRs (will be copied)

apigee: Added in-place update for `consumer_accept_list` field in `google_apigee_instance`.

Copy link

google-cla bot commented Aug 17, 2024

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

@github-actions github-actions bot requested a review from hao-nan-li August 17, 2024 11:24
Copy link

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

@hao-nan-li, 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 awaiting-approval Pull requests that need reviewer's approval to run presubmit tests service/apigee and removed awaiting-approval Pull requests that need reviewer's approval to run presubmit tests labels Aug 17, 2024
@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 ( 2 files changed, 83 insertions(+), 1 deletion(-))
google-beta provider: Diff ( 2 files changed, 83 insertions(+), 1 deletion(-))

@modular-magician
Copy link
Collaborator

Tests analytics

Total tests: 41
Passed tests: 14
Skipped tests: 27
Affected tests: 0

Click here to see the affected service packages
  • apigee

$\textcolor{green}{\textsf{All tests passed!}}$

View the build log

Copy link
Contributor

@hao-nan-li hao-nan-li left a comment

Choose a reason for hiding this comment

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

Could you add an update test just to make sure it's working as intended?

@github-actions github-actions bot requested a review from hao-nan-li August 20, 2024 13:50
@modular-magician modular-magician added the awaiting-approval Pull requests that need reviewer's approval to run presubmit tests label Aug 20, 2024
@entertvl
Copy link
Contributor Author

@hao-nan-li
I've added the update test.
Would you mind reviewing this again?

@modular-magician modular-magician removed the awaiting-approval Pull requests that need reviewer's approval to run presubmit tests label Aug 20, 2024
@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, 86 insertions(+), 1 deletion(-))
google-beta provider: Diff ( 3 files changed, 394 insertions(+), 1 deletion(-))

@modular-magician
Copy link
Collaborator

Tests analytics

Total tests: 42
Passed tests: 14
Skipped tests: 28
Affected tests: 0

Click here to see the affected service packages
  • apigee
#### Non-exercised tests

Tests were added that are skipped in VCR:

  • TestAccApigeeInstance_updateConsumerAcceptList
    $\textcolor{green}{\textsf{All tests passed!}}$

View the build log

@github-actions github-actions bot requested a review from hao-nan-li August 21, 2024 04:16
@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 Aug 21, 2024
@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, 278 insertions(+), 1 deletion(-))
google-beta provider: Diff ( 3 files changed, 278 insertions(+), 1 deletion(-))

@modular-magician
Copy link
Collaborator

Tests analytics

Total tests: 42
Passed tests: 14
Skipped tests: 28
Affected tests: 0

Click here to see the affected service packages
  • apigee
#### Non-exercised tests

Tests were added that are skipped in VCR:

  • TestAccApigeeInstance_updateConsumerAcceptList
    $\textcolor{green}{\textsf{All tests passed!}}$

View the build log

@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 Aug 22, 2024
@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, 277 insertions(+), 1 deletion(-))
google-beta provider: Diff ( 3 files changed, 277 insertions(+), 1 deletion(-))

@modular-magician
Copy link
Collaborator

Tests analytics

Total tests: 42
Passed tests: 14
Skipped tests: 27
Affected tests: 1

Click here to see the affected service packages
  • apigee

Action taken

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

Get to know how VCR tests work

@modular-magician
Copy link
Collaborator

$\textcolor{red}{\textsf{Tests failed during RECORDING mode:}}$
TestAccApigeeInstance_updateConsumerAcceptList[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

@entertvl
Copy link
Contributor Author

entertvl commented Sep 5, 2024

@hao-nan-li
Thank you for letting me know.

I'm trying to build and run the tests again, but errors are occurring in areas outside my focus.
Wait for a while and try again later.

$ cd ~/git/github.com/entertvl/magic-modules
$ git checkout main && git clean -f . && git checkout -- . && git pull
$ cd $GOPATH/src/github.com/hashicorp/terraform-provider-google; git checkout main && git clean -f . && git checkout -- . && git pull
$ cd $GOPATH/src/github.com/hashicorp/terraform-provider-google-beta; git checkout main && git clean -f . && git checkout -- . && git pull
Proceeding with the operation...
Already on 'main'
Your branch is up to date with 'origin/main'.
Already up to date.
Already on 'main'
Your branch is up to date with 'origin/main'.
Already up to date.
Already on 'main'
Your branch is up to date with 'origin/main'.
Already up to date.
Returned to the original directory: ~/git/github.com/entertvl/magic-modules

$ make provider VERSION=ga OUTPUT_PATH="$GOPATH/src/github.com/hashicorp/terraform-provider-google"

$ make provider VERSION=beta OUTPUT_PATH="$GOPATH/src/github.com/hashicorp/terraform-provider-google-beta"

$ cd $GOPATH/src/github.com/hashicorp/terraform-provider-google; make lint
sh -c "'/path/to/go/1.21.3/src/github.com/hashicorp/terraform-provider-google/scripts/gofmtcheck.sh'"
==> Checking that code complies with gofmt requirements...
go vet
# github.com/hashicorp/terraform-provider-google/google/fwprovider
google/fwprovider/data_source_provider_config_plugin_framework.go:208:38: d.providerConfig.Credentials undefined (type *fwtransport.FrameworkProviderConfig has no field or method Credentials)
make: *** [vet] Error 1

@hao-nan-li
Copy link
Contributor

@hao-nan-li Thank you for letting me know.

I'm trying to build and run the tests again, but errors are occurring in areas outside my focus. Wait for a while and try again later.

$ cd ~/git/github.com/entertvl/magic-modules
$ git checkout main && git clean -f . && git checkout -- . && git pull
$ cd $GOPATH/src/github.com/hashicorp/terraform-provider-google; git checkout main && git clean -f . && git checkout -- . && git pull
$ cd $GOPATH/src/github.com/hashicorp/terraform-provider-google-beta; git checkout main && git clean -f . && git checkout -- . && git pull
Proceeding with the operation...
Already on 'main'
Your branch is up to date with 'origin/main'.
Already up to date.
Already on 'main'
Your branch is up to date with 'origin/main'.
Already up to date.
Already on 'main'
Your branch is up to date with 'origin/main'.
Already up to date.
Returned to the original directory: ~/git/github.com/entertvl/magic-modules

$ make provider VERSION=ga OUTPUT_PATH="$GOPATH/src/github.com/hashicorp/terraform-provider-google"

$ make provider VERSION=beta OUTPUT_PATH="$GOPATH/src/github.com/hashicorp/terraform-provider-google-beta"

$ cd $GOPATH/src/github.com/hashicorp/terraform-provider-google; make lint
sh -c "'/path/to/go/1.21.3/src/github.com/hashicorp/terraform-provider-google/scripts/gofmtcheck.sh'"
==> Checking that code complies with gofmt requirements...
go vet
# github.com/hashicorp/terraform-provider-google/google/fwprovider
google/fwprovider/data_source_provider_config_plugin_framework.go:208:38: d.providerConfig.Credentials undefined (type *fwtransport.FrameworkProviderConfig has no field or method Credentials)
make: *** [vet] Error 1

Have you set GOOGLE_CREDENTIALS? https://googlecloudplatform.github.io/magic-modules/get-started/generate-providers/#test-changes

@modular-magician modular-magician removed the awaiting-approval Pull requests that need reviewer's approval to run presubmit tests label Sep 5, 2024
@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, 278 insertions(+), 1 deletion(-))
google-beta provider: Diff ( 3 files changed, 278 insertions(+), 1 deletion(-))

@modular-magician
Copy link
Collaborator

Tests analytics

Total tests: 42
Passed tests: 14
Skipped tests: 28
Affected tests: 0

Click here to see the affected service packages
  • apigee
#### Non-exercised tests

Tests were added that are skipped in VCR:

  • TestAccApigeeInstance_updateConsumerAcceptList
    $\textcolor{green}{\textsf{All tests passed!}}$

View the build log

@entertvl
Copy link
Contributor Author

entertvl commented Sep 6, 2024

Have you set GOOGLE_CREDENTIALS?

Yes, I was able to build successfully with ADC before, and they are still set.

$ printenv | grep GOOGLE
GOOGLE_BILLING_ACCOUNT=
GOOGLE_ORG=
GOOGLE_FOLDER=
GOOGLE_PROJECT=
GOOGLE_REGION=
GOOGLE_ZONE=
GOOGLE_USE_DEFAULT_CREDENTIALS=true

@hao-nan-li
Copy link
Contributor

Have you set GOOGLE_CREDENTIALS?

Yes, I was able to build successfully with ADC before, and they are still set.

$ printenv | grep GOOGLE
GOOGLE_BILLING_ACCOUNT=
GOOGLE_ORG=
GOOGLE_FOLDER=
GOOGLE_PROJECT=
GOOGLE_REGION=
GOOGLE_ZONE=
GOOGLE_USE_DEFAULT_CREDENTIALS=true

Could you rebase and clean all existing change from TPG? Also make sure to pull the latest change from TPG

@entertvl entertvl force-pushed the feat-apigee-instance-update branch from 90a61b8 to 24b52bc Compare September 9, 2024 08:56
@modular-magician modular-magician added the awaiting-approval Pull requests that need reviewer's approval to run presubmit tests label Sep 9, 2024
@entertvl
Copy link
Contributor Author

entertvl commented Sep 9, 2024

Could you rebase and clean all existing change from TPG? Also make sure to pull the latest change from TPG

Thank you, I was able to re-run the testacc by rebasing.

I do see that the added functionality works fine, but need to check the entire log including the deletion part.

I found a solution from new features of project deletion_policy, it was explicitly specifying a value is required for project deletion.

And re-run tests result here from locally:

TF_LOG=DEBUG make testacc TEST=./google/services/apigee TESTARGS='-run=TestAccApigeeInstance_' > test.log

https://gist.github.com/entertvl/b20734230df59d28f29674fe0831a028

@hao-nan-li
Copy link
Contributor

Could you rebase and clean all existing change from TPG? Also make sure to pull the latest change from TPG

Thank you, I was able to re-run the testacc by rebasing.

I do see that the added functionality works fine, but need to check the entire log including the deletion part.

I found a solution from new features of project deletion_policy, it was explicitly specifying a value is required for project deletion.

And re-run tests result here from locally:

TF_LOG=DEBUG make testacc TEST=./google/services/apigee TESTARGS='-run=TestAccApigeeInstance_' > test.log

https://gist.github.com/entertvl/b20734230df59d28f29674fe0831a028

Looks like there's a URL malformed error in the log, any chance of fixing it locally?

@entertvl
Copy link
Contributor Author

Will check if it's possible to fix the sections where the two test cases are failing in the log.

--- FAIL: TestAccApigeeInstance_apigeeInstanceIpRangeTestExample (148.20s)
--- FAIL: TestAccApigeeInstance_apigeeInstanceServiceAttachmentBasicTestExample (2450.98s)

@entertvl
Copy link
Contributor Author

--- FAIL: TestAccApigeeInstance_apigeeInstanceIpRangeTestExample (148.20s)

  • Passed when run single test (test log)
  • Likely failed due to parallel execution with other tests
  • No code changes

--- FAIL: TestAccApigeeInstance_apigeeInstanceServiceAttachmentBasicTestExample (2450.98s)

  • It seems that pass a literal IP address caused an error
  • Replaced it with the resource ID like partial URL or by name as per the doc
  • Passed when single test (test log)

@modular-magician modular-magician removed the awaiting-approval Pull requests that need reviewer's approval to run presubmit tests label Sep 10, 2024
@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 ( 4 files changed, 282 insertions(+), 2 deletions(-))
google-beta provider: Diff ( 4 files changed, 282 insertions(+), 2 deletions(-))

@modular-magician
Copy link
Collaborator

Tests analytics

Total tests: 42
Passed tests: 14
Skipped tests: 28
Affected tests: 0

Click here to see the affected service packages
  • apigee
#### Non-exercised tests

Tests were added that are skipped in VCR:

  • TestAccApigeeInstance_updateConsumerAcceptList
    $\textcolor{green}{\textsf{All tests passed!}}$

View the build log

@hao-nan-li
Copy link
Contributor

Could you confirm TestAccApigeeInstance_updateConsumerAcceptList passed locally? If so we're good to go.

@entertvl
Copy link
Contributor Author

Could you confirm TestAccApigeeInstance_updateConsumerAcceptList passed locally?

Results of local testing here:

--- PASS: TestAccApigeeInstance_updateConsumerAcceptList (2600.02s)
PASS
ok  	github.com/hashicorp/terraform-provider-google/google/services/apigee	2601.368s

https://gist.github.com/entertvl/2f29a4fde72df615436a960dac1ba3e6

@hao-nan-li hao-nan-li merged commit ba4c9d3 into GoogleCloudPlatform:main Sep 11, 2024
12 checks passed
@entertvl entertvl deleted the feat-apigee-instance-update branch September 11, 2024 23:46
iyabchen pushed a commit to iyabchen/magic-modules that referenced this pull request Sep 14, 2024
abd-goog pushed a commit to abd-goog/magic-modules that referenced this pull request Sep 23, 2024
niharika-98 pushed a commit to niharika-98/magic-modules that referenced this pull request Oct 7, 2024
Philip-Jonany pushed a commit to Philip-Jonany/magic-modules that referenced this pull request Nov 4, 2024
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.

Resource google_apigee_instance update on "consumer_accept_list" attribute change
3 participants