Set cluster DNS correctly in case of nodelocal dns cache

[nysthee]

Set the cluster_dns appropriately in case nodelocaldns is enabled.
Follow up of #3861

[woopstar]

There is one more bug:

https://github.com/kubernetes-sigs/kubespray/pull/3861/files#diff-f4a83b1729e5db14f3ab49f94d10e406R12

clusterIP should be accodingly to the dns mode:

{% if dns_mode in ['kubedns', 'coredns', 'coredns_dual'] %}
clusterIP: "{{ skydns_server }}"
{% elif dns_mode == 'dnsmasq_kubedns' %}
clusterIP: "{{ dnsmasq_dns_server }}"
{% elif dns_mode == 'manual' %}
clusterIP: "{{ manual_dns_server }}"
{% else %}

[woopstar]

ci check this

[woopstar]

Sorry, it should not be enabled by default yet. Can you switch back

[woopstar]

Syntax error ;)

[woopstar]

9e6ace6 got removed in your latest commit

[nysthee]

Ok I am obviously doing something wrong with the if check in the vars. Any examples.
Regarding default. Weird it shows likes this but in git commit's its now showing this change is reverted.

[nysthee]

ci check this

[nysthee]

@woopstar can you check again please?

[woopstar]

TASK [kubernetes/node : Write kubelet config file (kubeadm)] *******************
task path: /kargo-ci/kubernetes-sigs-kubespray/roles/kubernetes/node/tasks/main.yml:32
Tuesday 11 December 2018  18:32:51 +0000 (0:00:00.130)       0:23:59.514 ****** 
fatal: [k8s-39770198-133069148-3]: FAILED! => {"changed": false, "msg": "AnsibleUndefinedVariable: 'nodelocaldns_ip' is undefined"}
fatal: [k8s-39770198-133069148-1]: FAILED! => {"changed": false, "msg": "AnsibleUndefinedVariable: 'nodelocaldns_ip' is undefined"}
fatal: [k8s-39770198-133069148-2]: FAILED! => {"changed": false, "msg": "AnsibleUndefinedVariable: 'nodelocaldns_ip' is undefined"}

[woopstar]

failed: [k8s-39836923-133487652-1] (item=nodelocaldns-config.yml) => {"attempts": 4, "changed": false, "item": {"changed": true, "checksum": "d8902c7843760e4e2c7c972dc72098df46933c47", "dest": "/etc/kubernetes/nodelocaldns-config.yml", "diff": [], "failed": false, "gid": 0, "group": "root", "invocation": {"module_args": {"_original_basename": "nodelocaldns-config.yml.j2", "attributes": null, "backup": false, "checksum": "d8902c7843760e4e2c7c972dc72098df46933c47", "content": null, "delimiter": null, "dest": "/etc/kubernetes/nodelocaldns-config.yml", "directory_mode": null, "follow": false, "force": true, "group": null, "local_follow": null, "mode": null, "owner": null, "regexp": null, "remote_src": null, "selevel": null, "serole": null, "setype": null, "seuser": null, "src": "/home/travis/.ansible/tmp/ansible-tmp-1544622076.96-21926197592337/source", "unsafe_writes": null, "validate": null}}, "item": {"file": "nodelocaldns-config.yml", "name": "nodelocaldns", "type": "configmap"}, "md5sum": "57898d53b3518ee30dbad1b06b7a116e", "mode": "0644", "owner": "root", "secontext": "system_u:object_r:etc_t:s0", "size": 1078, "src": "/home/travis/.ansible/tmp/ansible-tmp-1544622076.96-21926197592337/source", "state": "file", "uid": 0}, "msg": "error running kubectl (/usr/local/bin/kubectl --namespace=kube-system apply --force --filename=/etc/kubernetes/nodelocaldns-config.yml) command (rc=1), out='', err='error: error parsing /etc/kubernetes/nodelocaldns-config.yml: error converting YAML to JSON: yaml: line 17: did not find expected key\n'"}

[woopstar]

ping @nysthee

[woopstar]

any news on this?

[nysthee]

Yes, I had several go’s at it but I am lacking some time atm to continue on it. - - - Thomas From: Andreas Krüger <notifications@github.com>(mailto:notifications@github.com) Reply: kubernetes-sigs/kubespray <reply@reply.github.com>(mailto:reply@reply.github.com) Date: 21 January 2019 at 12:46:16 To: kubernetes-sigs/kubespray <kubespray@noreply.github.com>(mailto:kubespray@noreply.github.com) CC: Thomas Nys <hello@thomasnys.com>(mailto:hello@thomasnys.com), Mention <mention@noreply.github.com>(mailto:mention@noreply.github.com)

…

Subject: Re: [kubernetes-sigs/kubespray] Set cluster DNS correctly in case of nodelocal dns cache (#3879) any news on this? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub(#3879 (comment)), or mute the thread(https://github.com/notifications/unsubscribe-auth/ALCIkAF10DHHxgFiYN1XKj1ShD4q7Vnaks5vFaiIgaJpZM4ZNAU5).

[dannyk81]

FYI, the following PR kubernetes/dns#286 is required to ensure the nodelocaldns network interface is created in a deterministic way.

This seems to affect only CoreOS based deployments at this point, but still better to to use this once it's released.

[dannyk81]

@nysthee I've taken this PR for a spin and I think I can help with the config interpolation issues.

The following diff fixed the issue for me and allowed the proper rendering of the configuration file:

diff --git a/roles/kubernetes-apps/ansible/tasks/nodelocaldns.yml b/roles/kubernetes-apps/ansible/tasks/nodelocaldns.yml
index 1d50fa21..62801e71 100644
--- a/roles/kubernetes-apps/ansible/tasks/nodelocaldns.yml
+++ b/roles/kubernetes-apps/ansible/tasks/nodelocaldns.yml
@@ -2,13 +2,13 @@
 - name: Kubernetes Apps | set up necessary nodelocaldns parameters
   set_fact:
     clusterIP: >-
-      {% if dns_mode in ['kubedns', 'coredns', 'coredns_dual'] %}
-          "{{ skydns_server }}"
-      {% elif dns_mode == 'dnsmasq_kubedns' %}
-          "{{ dnsmasq_dns_server }}"
-      {% elif dns_mode == 'manual' %}
-          "{{ manual_dns_server }}"
-      {% endif %}
+      {%- if dns_mode in ['kubedns', 'coredns', 'coredns_dual'] -%}
+      {{ skydns_server }}
+      {%- elif dns_mode == 'dnsmasq_kubedns' -%}
+      {{ dnsmasq_dns_server }}
+      {%- elif dns_mode == 'manual' -%}
+      {{ manual_dns_server }}
+      {%- endif -%}
     secondaryclusterIP: "{{ skydns_server_secondary }}"


@@ -23,17 +23,17 @@
   register: nodelocaldns_manifests
   vars:
     forwardTarget: >-
-      {%- if secondaryclusterIP is defined and dns_mode == 'coredns_dual' %}
-          {{ clusterIP }} {{ secondaryclusterIP }}
+      {%- if secondaryclusterIP is defined and dns_mode == 'coredns_dual' -%}
+      {{ clusterIP }} {{ secondaryclusterIP }}
       {%- else -%}
-          {{ clusterIP }}
-      {%- endif %}
+      {{ clusterIP }}
+      {%- endif -%}
     upstreamForwardTarget: >-
-      {%- if resolvconf_mode == 'host_resolvconf' and upstream_dns_servers is defined and upstream_dns_servers|length > 0 %}
-        {{ upstream_dns_servers|join(' ') }}
-      {%- else %}
-        /etc/resolv.conf
-      {%- endif %}
+      {%- if resolvconf_mode == 'host_resolvconf' and upstream_dns_servers is defined and upstream_dns_servers|length > 0 -%}
+      {{ upstream_dns_servers|join(' ') }}
+      {%- else -%}
+      /etc/resolv.conf
+      {%- endif -%}
   when:
     - enable_nodelocaldns == True
     - inventory_hostname == groups['kube-master'] | first

I hope this can help move this PR a long, would ❤️ to see this merged as we desperately need this functionality.

[woopstar]

@nysthee Can we move along ? :)

[nysthee]

Thanks a lot @dannyk81 !!! I integrated your fix and will continue. 🤞

[dannyk81]

Thanks a lot @dannyk81 !!! I integrated your fix and will continue. 🤞

my pleasure! I've been doing some testing with this on a new cluster and so far things look really good 😄

[nysthee]

@nysthee Can we move along ? :)

Yes please review the pr.

[woopstar]

/lgtm
/approve

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: nysthee, woopstar

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• OWNERS [woopstar]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment