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

Manage services peered-dns-domains via terraform #5158

Conversation

DeviaVir
Copy link
Contributor

@DeviaVir DeviaVir commented Aug 31, 2021

https://cloud.google.com/sdk/gcloud/reference/services/peered-dns-domains/list hit GA a little while back, this is what allows you to configure private DNS access to a shared network (i.e. a private cloud build worker pool could use private DNS on a different GCP project). We're currently configured it using the gcloud binary, but would like to move this into terraform.
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)

`google_service_networking_peered_dns_domain`
`google_service_networking_peered_dns_domain`

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

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

@DeviaVir DeviaVir force-pushed the terraform-provider-google/peered-dns-domain branch from 5be652a to e92a63a Compare August 31, 2021 12:14
@DeviaVir
Copy link
Contributor Author

Not too sure how to proceed with acceptance tests, if it's easy for you to run- please go ahead.

@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 ( 6 files changed, 399 insertions(+))
Terraform Beta: Diff ( 7 files changed, 400 insertions(+), 1 deletion(-))

@melinath
Copy link
Member

/gcbrun to start the full acceptance tests.

If there are any issues, you can find the instructions on running the tests yourself here: https://github.com/hashicorp/terraform-provider-google/blob/master/.github/CONTRIBUTING.md#tests

As a heads up, we are no longer generating inspec from Magic Modules - we just haven't cleaned up that code yet. If you want this resource supported in inspec and you're not sure where to submit it, let me know.

@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 ( 6 files changed, 399 insertions(+))
Terraform Beta: Diff ( 8 files changed, 402 insertions(+), 2 deletions(-))

@DeviaVir DeviaVir force-pushed the terraform-provider-google/peered-dns-domain branch from e92a63a to 02fc3e4 Compare August 31, 2021 15:54
@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 ( 6 files changed, 395 insertions(+))
Terraform Beta: Diff ( 6 files changed, 395 insertions(+))

@modular-magician
Copy link
Collaborator

Tests were added that did not run in TeamCity:

  • TestAccProjectServicePeeredDNSDomain_basic

@modular-magician

This comment has been minimized.

@DeviaVir DeviaVir force-pushed the terraform-provider-google/peered-dns-domain branch 3 times, most recently from 25c13fe to 5a204cd Compare August 31, 2021 16:54
@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 ( 6 files changed, 399 insertions(+))
Terraform Beta: Diff ( 6 files changed, 399 insertions(+))

@modular-magician
Copy link
Collaborator

Tests were added that did not run in TeamCity:

  • TestAccProjectServicePeeredDNSDomain_basic

@modular-magician

This comment has been minimized.

@modular-magician

This comment has been minimized.

@DeviaVir DeviaVir force-pushed the terraform-provider-google/peered-dns-domain branch from 5a204cd to 98a5616 Compare September 2, 2021 07:17
@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 ( 6 files changed, 365 insertions(+))
Terraform Beta: Diff ( 7 files changed, 366 insertions(+), 1 deletion(-))

@modular-magician

This comment has been minimized.

@modular-magician

This comment has been minimized.

@DeviaVir
Copy link
Contributor Author

DeviaVir commented Sep 2, 2021

Tests failed during RECORDING mode: TestAccDataSourceGoogleCloudRunService_optionalProject|TestAccDataSourceComputeLbIpRanges_basic|TestAccDataSourceDNSKeys_noDnsSec|TestAccDataSourceCloudIdentityGroupMemberships_basic|TestAccDataSourceComputeNetworkEndpointGroup|TestAccDataSourceCloudIdentityGroups_basic|TestAccDataSourceGoogleCloudRunLocations_basic|TestAccDataSourceDNSKeys_basic|TestAccDataSourceDnsManagedZone_basic|TestAccComputeHealthCheckDatasource_basic|TestAccDataSourceGameServicesGameServerDeploymentRollout_basic|TestAccDataSourceGoogleCloudRunService_basic|TestAccDataSourceGoogleActiveFolder_default|TestAccDataSourceGoogleBillingAccount_byShortName|TestAccDataSourceGoogleAppEngineDefaultServiceAccount_basic|TestAccDataSourceGoogleBillingAccount_byFullNameClosed|TestAccDataSourceGoogleActiveFolder_dash|TestAccDataSourceGoogleBillingAccount_byDisplayName|TestAccDataSourceGoogleBillingAccount_byFullName|TestAccDataSourceGoogleActiveFolder_space|TestAccDataSourceGoogleClientConfig_basic|TestAccDataSourceGoogleClientOpenIDUserinfo_basic|TestAccDataSourceGoogleBigqueryDefaultServiceAccount_basic|TestAccDataSourceGoogleCloudFunctionsFunction_basic|TestAccDataSourceComputeGlobalAddress|TestAccDataSourceComputeBackendService_basic|TestAccDataSourceComposerImageVersions_basic|TestAccDataSourceComputeHaVpnGateway|TestAccDataSourceComputeImage|TestAccDataSourceComputeAddress|TestAccDataSourceGoogleForwardingRule|TestAccDataSourceComputeBackendBucket_basic|TestAccDataSourceGoogleComputeDefaultServiceAccount_basic|TestAccDataSourceComposerEnvironment_basic|TestAccDataSourceGoogleComputeInstanceGroup_basic|TestAccDataSourceComputeImageFilter|TestAccDataSourceComputeInstanceSerialPort_basic|TestAccDataSourceGoogleComputeInstanceGroup_fromIGM|TestAccDataSourceGoogleComputeInstanceGroup_withNamedPort|TestAccInstanceTemplateDatasource_filter_mostRecent|TestAccInstanceTemplateDatasource_filter|TestAccComputeRegions_basic|TestAccDataSourceComputeInstance_basic|TestAccInstanceTemplateDatasource_name|TestAccDataSourceGoogleNetwork|TestAccDataSourceComputeRegionSslCertificate|TestAccDataSourceComputeResourcePolicy|TestAccDataSourceComputeNodeTypes_basic|TestAccDataSourceComputeRouter|TestAccContainerClusterDatasource_zonal|TestAccDataSourceGoogleSslPolicy|TestAccDataSourceGoogleSubnetwork|TestAccDataSourceGoogleVpnGateway|TestAccDataSourceComputeSslCertificate|TestAccComputeZones_filter|TestAccComputeZones_basic|TestAccContainerEngineVersions_basic|TestAccContainerClusterDatasource_regional|TestAccDataSourceGoogleFolderOrganizationPolicy_basic|TestAccContainerEngineVersions_filtered|TestAccDataSourceGoogleFolder_byFullName|TestAccDataSourceGoogleFolder_lookupOrganization|TestAccDataSourceGoogleFolder_byShortName|TestAccDataSourceGoogleFolder_byFullNameNotFound|TestAccDataSourceGoogleGlobalForwardingRule|TestAccDataSourceIAMRole|TestAccDataSourceGoogleIamTestablePermissions_basic|TestAccDataSourceGoogleMonitoringUptimeCheckIps_basic|TestAccDataSourceGoogleKmsCryptoKey_basic|TestAccDataSourceGoogleKmsCryptoKeyVersion_basic|TestAccDataKmsSecretCiphertext_basic|TestAccDataSourceGoogleKmsKeyRing_basic|TestAccDataSourceGoogleNetblockIpRanges_basic|TestAccDataSourceGoogleOrganization_byFullName|TestAccDataSourceGoogleOrganization_byShortName|TestAccDataSourceGoogleServiceAccountAccessToken_basic|TestAccDataSourceGoogleProject_basic|TestAccDataSourceGoogleOrganization_byDomain|TestAccDataSourceGoogleProjectOrganizationPolicy_basic|TestAccDataSourceGoogleProjects_basic|TestAccDataSourceGoogleServiceAccountIdToken_basic|TestAccDatasourceGoogleServiceAccount_basic|TestAccDataSourceGoogleServiceAccountIdToken_impersonation|TestAccDatasourceGoogleServiceAccountKey_basic|TestAccDataSourceGoogleSQLCaCerts_basic|TestAccDataSourceGoogleStorageProjectServiceAccount_basic|TestAccDataSourceIAMBetaWorkloadIdentityPool_basic|TestAccDataSourceGoogleMonitoringNotificationChannel_byDisplayName|TestAccDataSourceGoogleMonitoringNotificationChannel_byTypeAndLabel|TestAccDataSourceGoogleStorageTransferProjectServiceAccount_basic|TestAccDataSourceIAMBetaWorkloadIdentityPoolProvider_basic|TestAccIapClient_Datasource_basic|TestAccDataSourceGoogleMonitoringNotificationChannel_UserLabel|TestAccDataSourceGoogleMonitoringNotificationChannel_ErrorNoDisplayNameOrType|TestAccDataSourceGoogleMonitoringNotificationChannel_byDisplayNameAndType|TestAccDataSourceGoogleMonitoringNotificationChannel_ErrorNotUnique|TestAccDataSourceGoogleMonitoringNotificationChannel_ErrorNotFound|TestAccDataSourceMonitoringService_AppEngine|TestAccDataSourceGooglePubsubTopic_optionalProject|TestAccDataSourceGooglePubsubTopic_basic Please fix these to complete your PR

My test is not in this list, does that mean it was successful? Can I do something about these failed tests? @melinath

@melinath
Copy link
Member

melinath commented Sep 2, 2021

Hm, I'm seeing that test failing; there might be something wrong with how we're collecting test failures (or maybe it just doesn't handle the volume well.)

Here's the error message I'm seeing:

Error: InternalValidate
Internal validation of the provider failed! This is always a bug
with the provider itself, and not a user issue. Please report
this bug:
1 error occurred:
  * resource google_project_service_peered_dns_domain: All fields are ForceNew
or Computed w/out Optional, Update is superfluous

I'm seeing that for all the failed tests, so this is probably not related to you. Maybe try rebasing? This may be an issue that was resolved on master.

@DeviaVir DeviaVir force-pushed the terraform-provider-google/peered-dns-domain branch from 98a5616 to 82915a4 Compare September 2, 2021 16:13
@DeviaVir
Copy link
Contributor Author

DeviaVir commented Sep 2, 2021

Huh, I removed the Update since it didn't do anything (had it pointed at an empty function first).

@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 ( 6 files changed, 359 insertions(+))
Terraform Beta: Diff ( 6 files changed, 359 insertions(+))

@modular-magician

This comment has been minimized.

@modular-magician
Copy link
Collaborator

Tests failed during RECORDING mode: TestAccServiceNetworkingPeeredDNSDomain_basic|TestAccDatasourceGoogleServiceNetworkingPeeredDnsDomain_basic Please fix these to complete your PR

@DeviaVir DeviaVir force-pushed the terraform-provider-google/peered-dns-domain branch 9 times, most recently from 260e987 to a53a34a Compare September 30, 2021 16:24
@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 ( 9 files changed, 597 insertions(+))
Terraform Beta: Diff ( 10 files changed, 598 insertions(+), 1 deletion(-))

@DeviaVir DeviaVir force-pushed the terraform-provider-google/peered-dns-domain branch from a53a34a to 6fd6480 Compare September 30, 2021 18:09
@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 ( 9 files changed, 597 insertions(+))
Terraform Beta: Diff ( 10 files changed, 598 insertions(+), 1 deletion(-))

@DeviaVir DeviaVir force-pushed the terraform-provider-google/peered-dns-domain branch from 6fd6480 to fcf9156 Compare September 30, 2021 18:21
@DeviaVir
Copy link
Contributor Author

DeviaVir commented Sep 30, 2021

make testacc TEST=./google TESTARGS='-run=TestAccServiceNetworkingPeeredDNSDomain_basic'
==> Checking source code against gofmt...
==> Checking that code complies with gofmt requirements...
go generate  ./...
TF_ACC=1 TF_SCHEMA_PANIC_ON_ERROR=1 go test ./google -v -run=TestAccServiceNetworkingPeeredDNSDomain_basic -timeout 240m -ldflags="-X=github.com/hashicorp/terraform-provider-google/version.ProviderVersion=acc"
=== RUN   TestAccServiceNetworkingPeeredDNSDomain_basic
=== PAUSE TestAccServiceNetworkingPeeredDNSDomain_basic
=== CONT  TestAccServiceNetworkingPeeredDNSDomain_basic
--- PASS: TestAccServiceNetworkingPeeredDNSDomain_basic (377.77s)
PASS
ok      github.com/hashicorp/terraform-provider-google/google   377.794s
make testacc TEST=./google TESTARGS='-run=TestAccDatasourceGoogleServiceNetworkingPeeredDnsDomain_basic'
==> Checking source code against gofmt...
==> Checking that code complies with gofmt requirements...
go generate  ./...
TF_ACC=1 TF_SCHEMA_PANIC_ON_ERROR=1 go test ./google -v -run=TestAccDatasourceGoogleServiceNetworkingPeeredDnsDomain_basic -timeout 240m -ldflags="-X=github.com/hashicorp/terraform-provider-google/version.ProviderVersion=acc"
=== RUN   TestAccDatasourceGoogleServiceNetworkingPeeredDnsDomain_basic
=== PAUSE TestAccDatasourceGoogleServiceNetworkingPeeredDnsDomain_basic
=== CONT  TestAccDatasourceGoogleServiceNetworkingPeeredDnsDomain_basic
--- PASS: TestAccDatasourceGoogleServiceNetworkingPeeredDnsDomain_basic (328.71s)
PASS
ok      github.com/hashicorp/terraform-provider-google/google   328.730s

🥳

@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 ( 9 files changed, 600 insertions(+))
Terraform Beta: Diff ( 10 files changed, 601 insertions(+), 1 deletion(-))

@modular-magician
Copy link
Collaborator

I have triggered VCR tests in RECORDING mode for the following tests that failed during VCR: TestAccDatasourceGoogleServiceNetworkingPeeredDnsDomain_basic|TestAccComputeForwardingRule_update|TestAccServiceNetworkingPeeredDNSDomain_basic|TestAccTags You can view the result here: https://ci-oss.hashicorp.engineering/viewQueued.html?itemId=208501

@DeviaVir DeviaVir force-pushed the terraform-provider-google/peered-dns-domain branch from fcf9156 to 09758e1 Compare October 1, 2021 08:41
@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 ( 9 files changed, 600 insertions(+))
Terraform Beta: Diff ( 10 files changed, 601 insertions(+), 1 deletion(-))

@DeviaVir
Copy link
Contributor Author

DeviaVir commented Oct 1, 2021

$ make testacc TEST=./google TESTARGS='-run=TestAccDatasourceGoogleServiceNetworkingPeeredDnsDomain_basic'
==> Checking source code against gofmt...
==> Checking that code complies with gofmt requirements...
go generate  ./...
TF_ACC=1 TF_SCHEMA_PANIC_ON_ERROR=1 go test ./google -v -run=TestAccDatasourceGoogleServiceNetworkingPeeredDnsDomain_basic -timeout 240m -ldflags="-X=github.com/hashicorp/terraform-provider-google/version.ProviderVersion=acc"
=== RUN   TestAccDatasourceGoogleServiceNetworkingPeeredDnsDomain_basic
=== PAUSE TestAccDatasourceGoogleServiceNetworkingPeeredDnsDomain_basic
=== CONT  TestAccDatasourceGoogleServiceNetworkingPeeredDnsDomain_basic
--- PASS: TestAccDatasourceGoogleServiceNetworkingPeeredDnsDomain_basic (422.86s)
PASS
ok      github.com/hashicorp/terraform-provider-google/google   422.872s

$ make testacc TEST=./google TESTARGS='-run=TestAccServiceNetworkingPeeredDNSDomain_basic'
==> Checking source code against gofmt...
==> Checking that code complies with gofmt requirements...
go generate  ./...
TF_ACC=1 TF_SCHEMA_PANIC_ON_ERROR=1 go test ./google -v -run=TestAccServiceNetworkingPeeredDNSDomain_basic -timeout 240m -ldflags="-X=github.com/hashicorp/terraform-provider-google/version.ProviderVersion=acc"
=== RUN   TestAccServiceNetworkingPeeredDNSDomain_basic
=== PAUSE TestAccServiceNetworkingPeeredDNSDomain_basic
=== CONT  TestAccServiceNetworkingPeeredDNSDomain_basic
--- PASS: TestAccServiceNetworkingPeeredDNSDomain_basic (386.43s)
PASS
ok      github.com/hashicorp/terraform-provider-google/google   386.446s

Seems fine for me now, not sure why it's still breaking?

@modular-magician
Copy link
Collaborator

I have triggered VCR tests in RECORDING mode for the following tests that failed during VCR: TestAccServiceNetworkingPeeredDNSDomain_basic You can view the result here: https://ci-oss.hashicorp.engineering/viewQueued.html?itemId=208678

@melinath
Copy link
Member

melinath commented Oct 1, 2021

@DeviaVir TLDR TerraformVCRCommunity is marked as "failed" if it doesn't need to run, but you're fine because beta-provider-vcr-test has succeeded for you already. I'll take a look at the PR later today.

@melinath melinath merged commit 8d1e485 into GoogleCloudPlatform:master Oct 1, 2021
@DeviaVir DeviaVir deleted the terraform-provider-google/peered-dns-domain branch October 6, 2021 07:55
khajduczenia pushed a commit to khajduczenia/magic-modules that referenced this pull request Oct 12, 2021
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