Skip to content
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

Add field to specify autoresize limit for Cloud SQL instance #4647

Closed
wants to merge 2 commits into from
Closed

Add field to specify autoresize limit for Cloud SQL instance #4647

wants to merge 2 commits into from

Conversation

EvyBongers
Copy link
Contributor

@EvyBongers EvyBongers commented Apr 1, 2021

This PR adds a field to specify settings.storageAutoResizeLimit for SQL database instance

If this PR is for Terraform, I acknowledge that I have:

  • Searched through the issue tracker for an open issue that this either resolves or contributes to, commented on it to claim it, and written "fixes {url}" or "part of {url}" in this PR description. If there were no relevant open issues, I opened one and commented that I would like to work on it (not necessary for very small changes).
  • Generated Terraform, and ran make test and make lint to ensure it passes unit and linter tests.
  • Ensured that all new fields I added that can be set by a user appear in at least one example (for generated resources) or third_party test (for handwritten resources or update tests).
  • Ran relevant acceptance tests (If the acceptance tests do not yet pass or you are unable to run them, please let your reviewer know).
  • Read the Release Notes Guide before writing my release note below.

Release Note Template for Downstream PRs (will be copied)

sql: added field `disk_autoresize_limit` to `sql_database_instance`

@google-cla google-cla bot added the cla: yes label Apr 1, 2021
@modular-magician
Copy link
Collaborator

Hello! I am a robot who works on Magic Modules PRs.

I have detected that you are a community contributor, so your PR will be assigned to someone with a commit-bit on this repo for initial review.

Thanks for your contribution! A human will be with you soon.

@ScottSuarez, please review this PR or find an appropriate assignee.

@modular-magician
Copy link
Collaborator

Hi! I'm the modular magician. Your PR generated some diffs in downstreams - here they are.

Diff report:

Terraform GA: Diff ( 2 files changed, 11 insertions(+))
Terraform Beta: Diff ( 4 files changed, 13 insertions(+), 3 deletions(-))
TF Conversion: Diff ( 1 file changed, 1 insertion(+))

@modular-magician
Copy link
Collaborator

I have triggered VCR tests based on this PR's diffs. See the results here: "https://ci-oss.hashicorp.engineering/viewQueued.html?itemId=180145"

@EvyBongers
Copy link
Contributor Author

@ScottSuarez can you give an indication when you can follow up on this? If there's more work needed for the PR to be merged, please let me know.

@ScottSuarez
Copy link
Contributor

ScottSuarez commented Apr 5, 2021

@modular-magician
Copy link
Collaborator

Hi! I'm the modular magician. Your PR generated some diffs in downstreams - here they are.

Diff report:

Terraform GA: Diff ( 3 files changed, 16 insertions(+), 4 deletions(-))
Terraform Beta: Diff ( 5 files changed, 18 insertions(+), 7 deletions(-))
TF Conversion: Diff ( 1 file changed, 1 insertion(+))

@modular-magician
Copy link
Collaborator

I have triggered VCR tests based on this PR's diffs. See the results here: "https://ci-oss.hashicorp.engineering/viewQueued.html?itemId=181420"

@EvyBongers
Copy link
Contributor Author

@ScottSuarez By the way, I don't have permissions to view the Google cloud build in generate-diffs CI. If this is something for me to follow up, could I be granted access?

@ScottSuarez
Copy link
Contributor

/gcbrun

no worries, I'll look and let you know if anything needs to be addressed

@modular-magician
Copy link
Collaborator

I have triggered VCR tests based on this PR's diffs. See the results here: "https://ci-oss.hashicorp.engineering/viewQueued.html?itemId=181435"

@modular-magician
Copy link
Collaborator

Hi! I'm the modular magician. Your PR generated some diffs in downstreams - here they are.

Diff report:

Terraform GA: Diff ( 3 files changed, 16 insertions(+), 4 deletions(-))
Terraform Beta: Diff ( 4 files changed, 17 insertions(+), 5 deletions(-))
TF Conversion: Diff ( 1 file changed, 1 insertion(+))

@modular-magician
Copy link
Collaborator

I have triggered VCR tests based on this PR's diffs. See the results here: "https://ci-oss.hashicorp.engineering/viewQueued.html?itemId=181435"

@EvyBongers
Copy link
Contributor Author

@ScottSuarez can you give an update on this?

@ScottSuarez
Copy link
Contributor

/gcbrun

Hi Evy,
I'll try to prioritize trying to get this in for you today. Thanks for your patience

@modular-magician
Copy link
Collaborator

I have triggered VCR tests based on this PR's diffs. See the results here: "https://ci-oss.hashicorp.engineering/viewQueued.html?itemId=182085"

@modular-magician
Copy link
Collaborator

Hi! I'm the modular magician. Your PR generated some diffs in downstreams - here they are.

Diff report:

Terraform GA: Diff ( 3 files changed, 16 insertions(+), 4 deletions(-))
Terraform Beta: Diff ( 3 files changed, 16 insertions(+), 4 deletions(-))
TF Conversion: Diff ( 1 file changed, 1 insertion(+))

@modular-magician
Copy link
Collaborator

I have triggered VCR tests based on this PR's diffs. See the results here: "https://ci-oss.hashicorp.engineering/viewQueued.html?itemId=182086"

@ScottSuarez
Copy link
Contributor

ScottSuarez commented Apr 14, 2021

@EvyBongers could you pull master? There seems to be transient issue that has been resolved in a future pr. This is blocking the test pipeline

@EvyBongers
Copy link
Contributor Author

@EvyBongers could you pull master? There seems to be transient issue that has been resolved in a future pr. This is blocking the test pipeline

Done and rebased.

@modular-magician
Copy link
Collaborator

Hi! I'm the modular magician. Your PR generated some diffs in downstreams - here they are.

Diff report:

Terraform GA: Diff ( 3 files changed, 16 insertions(+), 4 deletions(-))
Terraform Beta: Diff ( 4 files changed, 17 insertions(+), 5 deletions(-))
TF Conversion: Diff ( 1 file changed, 1 insertion(+))

@modular-magician
Copy link
Collaborator

I have triggered VCR tests based on this PR's diffs. See the results here: "https://ci-oss.hashicorp.engineering/viewQueued.html?itemId=182298"

@EvyBongers
Copy link
Contributor Author

@ScottSuarez any update?

@ScottSuarez
Copy link
Contributor

/gcbrun

gah ! Let me try now !

@modular-magician
Copy link
Collaborator

I have triggered VCR tests based on this PR's diffs. See the results here: "https://ci-oss.hashicorp.engineering/viewQueued.html?itemId=182734"

@modular-magician
Copy link
Collaborator

Hi! I'm the modular magician. Your PR generated some diffs in downstreams - here they are.

Diff report:

Terraform GA: Diff ( 3 files changed, 16 insertions(+), 4 deletions(-))
Terraform Beta: Diff ( 3 files changed, 16 insertions(+), 4 deletions(-))
TF Conversion: Diff ( 1 file changed, 1 insertion(+))

@modular-magician
Copy link
Collaborator

I have triggered VCR tests based on this PR's diffs. See the results here: "https://ci-oss.hashicorp.engineering/viewQueued.html?itemId=182735"

@EvyBongers
Copy link
Contributor Author

Still nothing has changed with the CI?

@ScottSuarez
Copy link
Contributor

ScottSuarez commented Apr 16, 2021

Getting the following build error

google/resource_sql_database_instance.go:975:11: settings.StorageAutoResizeLimitGb undefined (type *sqladmin.Settings has no field or method StorageAutoResizeLimitGb)

Do you not get this when you build?

Its possible we need to upgrade the version in the gomod?

@EvyBongers
Copy link
Contributor Author

Getting the following build error

google/resource_sql_database_instance.go:975:11: settings.StorageAutoResizeLimitGb undefined (type *sqladmin.Settings has no field or method StorageAutoResizeLimitGb)

Do you not get this when you build?

Its possible we need to upgrade the version in the gomod?

I'm not sure which CI job should trigger this? I generated terraform, but when I try to run make test in the repositories with generated code, I get a number of errors that seem to be related by missing code injection.

$ make test
==> Checking source code against gofmt...
==> Checking that code complies with gofmt requirements...
go generate  ./...
go test  -timeout=30s $(go list ./...)
# github.com/hashicorp/terraform-provider-google/google
google/resource_composer_environment.go:613:61: undefined: composer
google/resource_composer_environment.go:633:79: undefined: composer
google/resource_composer_environment.go:705:47: undefined: composer
google/resource_composer_environment.go:721:71: undefined: composer
google/resource_composer_environment.go:735:58: undefined: composer
google/resource_composer_environment.go:752:77: undefined: composer
google/resource_composer_environment.go:780:66: undefined: composer
google/resource_composer_environment.go:793:95: undefined: composer
google/resource_composer_environment.go:837:119: undefined: composer
google/resource_composer_environment.go:871:105: undefined: composer
google/resource_composer_environment.go:871:105: too many errors
# github.com/hashicorp/terraform-provider-google/google [github.com/hashicorp/terraform-provider-google/google.test]
google/resource_composer_environment.go:613:61: undefined: composer
google/resource_composer_environment.go:633:79: undefined: composer
google/resource_composer_environment.go:705:47: undefined: composer
google/resource_composer_environment.go:721:71: undefined: composer
google/resource_composer_environment.go:735:58: undefined: composer
google/resource_composer_environment.go:752:77: undefined: composer
google/resource_composer_environment.go:780:66: undefined: composer
google/resource_composer_environment.go:793:95: undefined: composer
google/resource_composer_environment.go:837:119: undefined: composer
google/resource_composer_environment.go:871:105: undefined: composer
google/resource_composer_environment.go:871:105: too many errors
FAIL	github.com/hashicorp/terraform-provider-google/google [build failed]
make: *** [GNUmakefile:11: test] Error 2

Therefore I don't know if/how I should proceed running tests locally.

@ScottSuarez
Copy link
Contributor

ScottSuarez commented Apr 19, 2021

it seems like goimports didn't run as there is a dependency issue? I unfortunately am unable to push to your pr. Otherwise I could pull it to diagnose/resolve the issue?

Also make sure you update with master on both tpg and mm before generating the provider. Let me know what command you are using to run the generation.

@EvyBongers
Copy link
Contributor Author

Hi Scott, these are the commands I used (output left out). Both rbenv and goenv set up according to the relevant docs.

rbenv version
2.6.0 (set by /path/to/magic-modules/mmv1/.ruby-version)
goenv version
1.16.2 (set by $GOPATH/src/github.com/hashicorp/terraform-provider-google/.go-version)
sudo rm -rf $GOPATH/src/github.com/hashicorp
cd /path/to/magic-modules
git fetch --all
git rebase upstream/master
cd mmv1; tools/bootstrap; cd -

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

cd "$GOPATH/src/github.com/hashicorp/terraform-provider-google"
# For some reason, `google/resource_composer_environment.go` has changes
# Other files with changes:
# - google/resource_sql_database_instance.go
# - google/resource_sql_database_instance_test.go
# - website/docs/d/sql_database_instance.html.markdown
make test
# Produces the mentioned error

cd "$GOPATH/src/github.com/hashicorp/terraform-provider-google-beta"
# In addition to the files in ga, this repo also has changes to:
# - google-beta/resource_dataproc_cluster.go
# - google-beta/resource_dataproc_cluster_test.go
make test
# Also produces the mentioned error

@ScottSuarez
Copy link
Contributor

Yeah this is weird... I can try and fork this pull request and try to make a new one where I try and resolve this. Unfortunately I am limited to what I can do with this PR to my discussions with you.

@EvyBongers
Copy link
Contributor Author

Yeah this is weird... I can try and fork this pull request and try to make a new one where I try and resolve this. Unfortunately I am limited to what I can do with this PR to my discussions with you.

If you can, please do, because this back and forth is slowing it down too much.

@ScottSuarez
Copy link
Contributor

absolutely, I'm a bit throttled with work but I'll try to get something set aside for you by the next release cut (end of next Tuesday). Sorry this process has been so painful for you!

@ScottSuarez
Copy link
Contributor

Looks like this a dependency issue, upgrading the dependency will take some time.

@modular-magician
Copy link
Collaborator

Hi! I'm the modular magician. Your PR generated some diffs in downstreams - here they are.

Diff report:

Terraform GA: Diff ( 3 files changed, 16 insertions(+), 4 deletions(-))
Terraform Beta: Diff ( 3 files changed, 16 insertions(+), 4 deletions(-))
TF Conversion: Diff ( 1 file changed, 1 insertion(+))

@modular-magician
Copy link
Collaborator

I have triggered VCR tests based on this PR's diffs. See the results here: "https://ci-oss.hashicorp.engineering/viewQueued.html?itemId=185662"

@ScottSuarez
Copy link
Contributor

checked in on #4743

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants