Fix using miekg dns #3136
Fix using miekg dns #3136
Conversation
christoph-zededa
force-pushed
the
fix_using_miekg_dns
branch
2 times, most recently
from
323661a to
2a3a81e
Compare
…pkg/pillar"" This reverts commit 34a3926. Signed-off-by: Christoph Ostarek <christoph@zededa.com>
This reverts commit 6a7b769. Signed-off-by: Christoph Ostarek <christoph@zededa.com>
This reverts commit 4569202. Signed-off-by: Christoph Ostarek <christoph@zededa.com>
Signed-off-by: Christoph Ostarek <christoph@zededa.com>
if the host cannot be resolved and empty array would be returned. In the following line the first element would be dereferenced, but that does not exist and so the process would panic Signed-off-by: Christoph Ostarek <christoph@zededa.com>
christoph-zededa
force-pushed
the
fix_using_miekg_dns
branch
from
2a3a81e to
6b23df2
Compare
christoph-zededa
requested review from
milan-zededa,
eriknordmark and
rvs
as code owners
|
I am not sure if we should keep c6cbde5 |
pkg/pillar/cmd/nim/controllerdns.go
Outdated
|
|
||
| if len(dnsResponses) == 0 { | ||
| newhosts = append(newhosts, etchosts...) | ||
| } else { | ||
| ttlSec = int(dnsResponses[0].TTL) | ||
| lookupIPaddr = dnsResponses[0].IP.String() |
There was a problem hiding this comment.
Is the old/current code only writing one IP address to the hosts file? Or does it write all?
You could make this loop over dnsResponses and Sprintf ine line for each if that is the behavior we want.
There was a problem hiding this comment.
The current master only writes one IP address to the hosts file; this branch writes all IP addresses to the hosts file.
I think this is the behavior we want; I saw that f.e. ping would choose the ip address that is reachable from the hosts file if there is more than one.
pkg/pillar/cmd/nim/controllerdns.go
Outdated
| lastDNSResponse := dnsResponses[len(dnsResponses)-1] | ||
| ipaddrCached = lastDNSResponse.IP.String() | ||
| ttlSec = getTTL(time.Duration(lastDNSResponse.TTL)) |
There was a problem hiding this comment.
AFAICT old/current code picks the first. Any particular motivation for picking the last here?
There was a problem hiding this comment.
I understand the code like this:
https://github.com/lf-edge/eve/blob/master/pkg/pillar/cmd/nim/controllerdns.go#L124 is iterating over all the responses and everytime assigns/overwrites the lookupIPaddr in https://github.com/lf-edge/eve/blob/master/pkg/pillar/cmd/nim/controllerdns.go#L130. So after the loop lookupIPaddr holds the value from the last iteration.
Same with newhosts as it does not append the previous value of newshosts but instead overwrites it here https://github.com/lf-edge/eve/blob/master/pkg/pillar/cmd/nim/controllerdns.go#L137 .
Nevertheless I think now it is better to choose the first one; this way pillar respects the case that an administrator wants to decrease the load on one IP address.
into writeHostsFile method Signed-off-by: Christoph Ostarek <christoph@zededa.com>
if as host has two ip addresses, both will be written to /etc/hosts; use `getent ahostsv4 <host>` to query them Signed-off-by: Christoph Ostarek <christoph@zededa.com>
christoph-zededa
force-pushed
the
fix_using_miekg_dns
branch
from
6b23df2 to
08f27e1
Compare
(test with cloudflare.com as they do DNS round robin:
)
see also #3096 and #3125