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

fix: apply all valid configurations for cluster_dns_provider #1805

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

54nd20
Copy link

@54nd20 54nd20 commented Nov 23, 2023

This pull request fixes issue #1783 where cloud_dns_provider only gets applied when it is set to CLOUD_DNS.

@54nd20 54nd20 requested review from ericyz and a team as code owners November 23, 2023 01:01
Copy link

google-cla bot commented Nov 23, 2023

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.

@54nd20 54nd20 force-pushed the fix/cluster-dns-provider branch from b518a18 to c873b75 Compare December 21, 2023 16:35
@54nd20
Copy link
Author

54nd20 commented Jan 3, 2024

Fixes #1833

Copy link

github-actions bot commented Mar 3, 2024

This PR is stale because it has been open 60 days with no activity. Remove stale label or comment or this will be closed in 7 days

@github-actions github-actions bot added the Stale label Mar 3, 2024
@psanzm
Copy link

psanzm commented Mar 4, 2024

Tested in our lab environment and works like a charm

@apeabody apeabody removed the Stale label Mar 4, 2024
Copy link

github-actions bot commented May 3, 2024

This PR is stale because it has been open 60 days with no activity. Remove stale label or comment or this will be closed in 7 days

@github-actions github-actions bot added the Stale label May 3, 2024
@github-actions github-actions bot closed this May 11, 2024
@AlvaroGG0
Copy link

Any plan to implement this fix? It's an annoying bug, we have been forking versions of the module implementing this during months and works correctly! @ericyz

@apeabody apeabody reopened this Dec 5, 2024
@apeabody apeabody requested a review from gtsorbo as a code owner December 5, 2024 17:50
@apeabody
Copy link
Contributor

apeabody commented Dec 5, 2024

Hi @54nd20 - Can you please ensure all the commits are covered by CLA so we can review? Thanks!

@54nd20
Copy link
Author

54nd20 commented Dec 5, 2024

@apeabody Thanks for the hint. All commits should be covered by the CLA now.

@apeabody
Copy link
Contributor

apeabody commented Dec 5, 2024

/gcbrun

@apeabody
Copy link
Contributor

apeabody commented Dec 5, 2024

Hi @54nd20 - Appears we are seeing some drift during verify due to the change?

Step #50 - "verify beta-cluster": TestBetaCluster 2024-12-05T21:50:37Z command.go:185: Terraform used the selected providers to generate the following execution
Step #50 - "verify beta-cluster": TestBetaCluster 2024-12-05T21:50:37Z command.go:185: plan. Resource actions are indicated with the following symbols:
Step #50 - "verify beta-cluster": TestBetaCluster 2024-12-05T21:50:37Z command.go:185:   ~ update in-place
Step #50 - "verify beta-cluster": TestBetaCluster 2024-12-05T21:50:37Z command.go:185: 
Step #50 - "verify beta-cluster": TestBetaCluster 2024-12-05T21:50:37Z command.go:185: Terraform will perform the following actions:
Step #50 - "verify beta-cluster": TestBetaCluster 2024-12-05T21:50:37Z command.go:185: 
Step #50 - "verify beta-cluster": TestBetaCluster 2024-12-05T21:50:37Z command.go:185:   # module.this.module.gke.google_container_cluster.primary will be updated in-place
Step #50 - "verify beta-cluster": TestBetaCluster 2024-12-05T21:50:37Z command.go:185:   ~ resource "google_container_cluster" "primary" {
Step #50 - "verify beta-cluster": TestBetaCluster 2024-12-05T21:50:37Z command.go:185:         id                                       = "projects/ci-gke-f66b6530-7593/locations/us-central1/clusters/simple-regional-beta-cluster-fi2k"
Step #50 - "verify beta-cluster": TestBetaCluster 2024-12-05T21:50:37Z command.go:185:         name                                     = "simple-regional-beta-cluster-fi2k"
Step #50 - "verify beta-cluster": TestBetaCluster 2024-12-05T21:50:37Z command.go:185:         # (36 unchanged attributes hidden)
Step #50 - "verify beta-cluster": TestBetaCluster 2024-12-05T21:50:37Z command.go:185: 
Step #50 - "verify beta-cluster": TestBetaCluster 2024-12-05T21:50:37Z command.go:185:       ~ dns_config {
Step #50 - "verify beta-cluster": TestBetaCluster 2024-12-05T21:50:37Z command.go:185:           + cluster_dns                   = "PROVIDER_UNSPECIFIED"
Step #50 - "verify beta-cluster": TestBetaCluster 2024-12-05T21:50:37Z command.go:185:           + cluster_dns_scope             = "DNS_SCOPE_UNSPECIFIED"
Step #50 - "verify beta-cluster": TestBetaCluster 2024-12-05T21:50:37Z command.go:185:             # (2 unchanged attributes hidden)
Step #50 - "verify beta-cluster": TestBetaCluster 2024-12-05T21:50:37Z command.go:185:         }
Step #50 - "verify beta-cluster": TestBetaCluster 2024-12-05T21:50:37Z command.go:185: 
Step #50 - "verify beta-cluster": TestBetaCluster 2024-12-05T21:50:37Z command.go:185:         # (28 unchanged blocks hidden)
Step #50 - "verify beta-cluster": TestBetaCluster 2024-12-05T21:50:37Z command.go:185:     }
Step #50 - "verify beta-cluster": TestBetaCluster 2024-12-05T21:50:37Z command.go:185: 
Step #50 - "verify beta-cluster": TestBetaCluster 2024-12-05T21:50:37Z command.go:185: Plan: 0 to add, 1 to change, 0 to destroy.
Step #50 - "verify beta-cluster": TestBetaCluster 2024-12-05T21:50:37Z command.go:185: 
Step #50 - "verify beta-cluster": TestBetaCluster 2024-12-05T21:50:37Z command.go:185: ─────────────────────────────────────────────────────────────────────────────
Step #50 - "verify beta-cluster": TestBetaCluster 2024-12-05T21:50:37Z command.go:185: 
Step #50 - "verify beta-cluster": TestBetaCluster 2024-12-05T21:50:37Z command.go:185: Note: You didn't use the -out option to save this plan, so Terraform can't
Step #50 - "verify beta-cluster": TestBetaCluster 2024-12-05T21:50:37Z command.go:185: guarantee to take exactly these actions if you run "terraform apply" now.
Step #50 - "verify beta-cluster":     terraform.go:537: 
Step #50 - "verify beta-cluster":         	Error Trace:	/builder/home/go/pkg/mod/github.com/!google!cloud!platform/cloud-foundation-toolkit/infra/[email protected]/pkg/tft/terraform.go:537
Step #50 - "verify beta-cluster":         	            				/workspace/test/integration/beta_cluster/beta_cluster_test.go:36
Step #50 - "verify beta-cluster":         	            				/builder/home/go/pkg/mod/github.com/!google!cloud!platform/cloud-foundation-toolkit/infra/[email protected]/pkg/tft/terraform.go:638
Step #50 - "verify beta-cluster":         	            				/builder/home/go/pkg/mod/github.com/!google!cloud!platform/cloud-foundation-toolkit/infra/[email protected]/pkg/tft/terraform.go:670
Step #50 - "verify beta-cluster":         	            				/builder/home/go/pkg/mod/github.com/!google!cloud!platform/cloud-foundation-toolkit/infra/[email protected]/pkg/utils/stages.go:31
Step #50 - "verify beta-cluster":         	            				/builder/home/go/pkg/mod/github.com/!google!cloud!platform/cloud-foundation-toolkit/infra/[email protected]/pkg/tft/terraform.go:670
Step #50 - "verify beta-cluster":         	Error:      	Should not be: 2
Step #50 - "verify beta-cluster":         	Test:       	TestBetaCluster
Step #50 - "verify beta-cluster":         	Messages:   	plan after apply should have no diff

@github-actions github-actions bot removed the Stale label Dec 5, 2024
@54nd20
Copy link
Author

54nd20 commented Dec 10, 2024

Hi @apeabody

That change is expected. This is just the default configuration now being added to the state. This is necessary because before you couldn't go back to the default if the configuration was changed outside of terraform.

@apeabody
Copy link
Contributor

/gcbrun

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.

4 participants