ForwardToKubeDNS when using NodeLocal DNSCache is not working as expected

[ederst]

/kind bug

1. What kops version are you running? The command kops version, will display
this information.

1.20.1

2. What Kubernetes version are you running? kubectl version will print the
version if a cluster is running or provide the Kubernetes version specified as
a kops flag.

1.20.5

3. What cloud provider are you using?

OpenStack

4. What commands did you run? What is the simplest way to reproduce this issue?

When executing kops update cluster the NodeLocal DNSCache Corefile config will not be updated when the forwardToKubeDNS value - introduced with #10111 - has changed.

The easiest way to reproduce the issue is, to enable the NodeLocal DNSCache and set forwardToKubeDNS to either true or false or not set it at all, which should default to false. Irrespective of the set value, the resulting Corefile config for the NodeLocal DNSCache will always use the true clause:

kops/upup/models/cloudup/resources/addons/nodelocaldns.addons.k8s.io/k8s-1.12.yaml.template

Lines 59 to 103 in 8e938ba

	{{- if KubeDNS.NodeLocalDNS.ForwardToKubeDNS }}
	.:53 {
	errors
	cache 30
	reload
	loop
	bind {{ KubeDNS.NodeLocalDNS.LocalIP }}
	forward . {{ NodeLocalDNSClusterIP }} {
	force_tcp
	}
	prometheus :9253
	}
	{{- else }}
	in-addr.arpa:53 {
	errors
	cache 30
	reload
	loop
	bind {{ KubeDNS.NodeLocalDNS.LocalIP }}
	forward . {{ NodeLocalDNSClusterIP }} {
	force_tcp
	}
	prometheus :9253
	}
	ip6.arpa:53 {
	errors
	cache 30
	reload
	loop
	bind {{ KubeDNS.NodeLocalDNS.LocalIP }}
	forward . {{ NodeLocalDNSClusterIP }} {
	force_tcp
	}
	prometheus :9253
	}
	.:53 {
	errors
	cache 30
	reload
	loop
	bind {{ KubeDNS.NodeLocalDNS.LocalIP }}
	forward . __PILLAR__UPSTREAM__SERVERS__
	prometheus :9253
	}
	{{- end }}

5. What happened after the commands executed?

The Corefile of the NodeLocal DNSCache was configured wrong when using forwardToKubeDNS: false.

6. What did you expect to happen?

I expected the Corefile of the NodeLocal DNSCache to contain the the config of the else clause (when forwardToKubeDNS is set to false or not set at all, which defaults to false as well) of the template (see link to template above).

7. Please provide your cluster manifest. Execute
kops get --name my.example.com -o yaml to display your cluster manifest.
You may want to remove your cluster name and other sensitive information.

Relevant part:

spec:
  kubeDNS:
    provider: CoreDNS
    nodeLocalDNS:
      enabled: true
      forwardToKubeDNS: false  # or do not set it at all

8. Please run the commands with most verbose logging by adding the -v 10 flag.
Paste the logs into this report, or in a gist and provide the gist link here.

n/a

9. Anything else do we need to know?

The issue seems to be related to the fact that ForwardToKubeDNS is a bool pointer and does not get dereferenced in the template:

kops/pkg/apis/kops/v1alpha2/cluster.go

Line 495 in d906f83

ForwardToKubeDNS *bool `json:"forwardToKubeDNS,omitempty"`

kops/upup/models/cloudup/resources/addons/nodelocaldns.addons.k8s.io/k8s-1.12.yaml.template

Line 59 in 8e938ba

{{- if KubeDNS.NodeLocalDNS.ForwardToKubeDNS }}

Thus the address is used, which always results in a "true". The proper way of doing this would be to either change ForwardToKubeDNS to a bool or to dereference it in the template, with for example WithDefaultBool:

kops/upup/pkg/fi/cloudup/template_functions.go

Lines 91 to 96 in 8a6ec14

	dest["WithDefaultBool"] = func(v *bool, defaultValue bool) bool {
	if v != nil {
	return *v
	}
	return defaultValue
	}

I will provide a PR for the latter.