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

Nodelocal dnscache #477

Merged
merged 4 commits into from
Apr 9, 2020
Merged

Nodelocal dnscache #477

merged 4 commits into from
Apr 9, 2020

Conversation

jmymy
Copy link
Contributor

@jmymy jmymy commented Apr 8, 2020

Added support for NodeLocal DNSCache addon that came in provider version 3.14.0

#428

https://github.com/terraform-providers/terraform-provider-google/releases/tag/v3.14.0

@jmymy
Copy link
Contributor Author

jmymy commented Apr 8, 2020

the error im getting locally is
Error: Error running command 'PATH=/workspace/test/fixtures/simple_zonal/.terraform/modules/example.acm.acm_operator/terraform-google-gcloud-0.5.1/cache/42adaccd/google-cloud-sdk/bin:$PATH ../../../modules/acm/scripts/kubectl_wrapper.sh https://<crud> = kubectl apply -f /workspace/acm.yaml ': exit status 1. Output: -d, --decode decode data error: the path "/workspace/acm.yaml" does not exist

gsutil cp gs://config-management-release/released/1.3.0/config-management-operator.yaml config-management-operator.yaml
AccessDeniedException: 403 jonathan@ does not have storage.objects.list access to config-management-release.

@bharathkkb
Copy link
Member

bharathkkb commented Apr 8, 2020

Thanks for the contribution @jmymy
You can run individual test suites by following this, that particular test that is failing requires access to a GCS bucket and hence it is failing.

In addition, may I propose enabling this in one of the existing tests and making sure that it passes?
We can enable it in a beta fixture like this one and add a small test here
something like "dnsCacheConfig" => {foo:bar}

Signed-off-by: Jonathan Meyers <jonathan@cybrary.it>
@jmymy jmymy requested review from bharathkkb and a team as code owners April 9, 2020 02:57
@bharathkkb
Copy link
Member

LGTM

@morgante morgante merged commit de8e1d5 into terraform-google-modules:master Apr 9, 2020
@jmymy jmymy deleted the nodelocal-dnscache branch April 10, 2020 07:09
@jmymy
Copy link
Contributor Author

jmymy commented Apr 15, 2020

The addition of
dns_cache_config { enabled = var.dns_cache }

even if set to disable (when the existing cluster has it disabled), is appearing as a destructive action now when bumping the module version on existing clusters. I'm wondering if we should wrap logic around that and only include the block only when set to true?

need to do some more digging

Screen Shot 2020-04-15 at 5 35 01 PM

FYI @bharathkkb @morgante

@morgante
Copy link
Contributor

@jmymy Please check the discussion/solution here - #428 (comment)

@jmymy
Copy link
Contributor Author

jmymy commented Apr 15, 2020

@jmymy Please check the discussion/solution here - #428 (comment)

doh.. i literally pulled up that issue before commenting here. Sorry for the noise.

CPL-markus pushed a commit to WALTER-GROUP/terraform-google-kubernetes-engine that referenced this pull request Jul 15, 2024
…erraform-google-modules#477)

* added support for node local dns cache

* config block

* updates to testing

Signed-off-by: Jonathan Meyers <jonathan@cybrary.it>

* generate docs again for example change
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.

None yet

3 participants