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: set CLOUD_DNS as provider for gke autopilot cluster #1700

Conversation

pziggo
Copy link

@pziggo pziggo commented Aug 3, 2023

Starting in August 2023, the default DNS provider for your new GKE Autopilot
clusters using version 1.25.9-gke.400 or later and 1.26.4-gke.500 or later
becomes Cloud DNS, at no extra charge. This change will be gradual and
expected to be completed by Aug 12th.

Without this change, the default setting PROVIDER_UNSPECIFIED for
dns_config.cluster_dns is used with the google_container_cluster
ressource.

Thus running terraform apply to update parts of an deployment will
always recreate the cluster:

- dns_config { # forces replacement
  - cluster_dns        = "CLOUD_DNS" -> null
  - cluster_dns_domain = "cluster.local" -> null
  - cluster_dns_scope  = "CLUSTER_SCOPE" -> null
}

@pziggo pziggo requested review from Jberlinsky, ericyz and a team as code owners August 3, 2023 08:18
@pziggo pziggo force-pushed the pziggo_autopilot_dns_config branch from 86325d6 to 8e2d9fe Compare August 3, 2023 08:19
@jmesterh
Copy link

jmesterh commented Aug 5, 2023

This is hitting us in production since yesterday, Terraform always wants to replace the autopilot cluster:

      - dns_config { # forces replacement
          - cluster_dns        = "CLOUD_DNS" -> null
          - cluster_dns_domain = "cluster.local" -> null
          - cluster_dns_scope  = "CLUSTER_SCOPE" -> null
        }

@okguru1
Copy link

okguru1 commented Aug 9, 2023

have been experiencing the same issue, perhaps exposing this (dns_config) to be configurable (rather than hardcoded), with CloudDNS as default would be better

@pziggo
Copy link
Author

pziggo commented Aug 9, 2023

perhaps exposing this (dns_config) to be configurable (rather than hardcoded), with CloudDNS as default would be better

I agree, especially after realising that existing clusters will be migrated over time. So, it might make sense to overwrite the variable while kube_dns is still used in the cluster and change it once it has been migrated.

I will update the PR accordingly.

> Starting in August 2023, the default DNS provider for your new GKE Autopilot
> clusters using version 1.25.9-gke.400 or later and 1.26.4-gke.500 or later
> becomes Cloud DNS, at no extra charge. This change will be gradual and
> expected to be completed by Aug 12th.

Without this change, the default setting `PROVIDER_UNSPECIFIED` for
`dns_config.cluster_dns` is used with the `google_container_cluster`
ressource.

Thus running terraform apply to update parts of an deployment will
always recreate the cluster:

```
- dns_config { # forces replacement
  - cluster_dns        = "CLOUD_DNS" -> null
  - cluster_dns_domain = "cluster.local" -> null
  - cluster_dns_scope  = "CLUSTER_SCOPE" -> null
}
```
@pziggo pziggo force-pushed the pziggo_autopilot_dns_config branch from 8e2d9fe to f88afab Compare August 9, 2023 09:20
@apeabody
Copy link
Contributor

/gcbrun

1 similar comment
@ericyz
Copy link
Collaborator

ericyz commented Aug 15, 2023

/gcbrun

@apeabody
Copy link
Contributor

@ericyz - Likely needs test updates from #1708

@GLStephen
Copy link

Just to confirm: I've used this on multiple clusters to fix the recreation issue.

@dpasiukevich
Copy link

Hi, may I suggest a better fix for this PR if that sg and if it's possible to do in the codebase?

The GKE Autopilot API does not allow to modify these fields (dns_config/...), there's no point to parameterise those as any unexpected value for dns_config/... will be rejected by the API.

It's better to ignore the field values in this module. So that the module would just ignore the changes and won't recreate the resource.

  1. The proposed change should make codebase easier.
  2. No need to unnecessarily parameterise those vars.
  3. With this PR, the users of the lib would have to manually update variables. But if the dns_config is not tracked by TF, users won't have to do anything and it will be fixed for them once they upgrade.

@apeabody
Copy link
Contributor

/gcbrun

Copy link

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 Nov 12, 2023
@github-actions github-actions bot closed this Nov 19, 2023
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.

7 participants