Change dns healthcheck to look at external domain

[carlpett]

What this PR does / why we need it: The healthcheck for kube-dns should ensure it is possible with recursive queries to detect network issues outside the cluster. Related to #2971.

Special notes for your reviewer: Implements the suggested solution from this comment

[acs-bot]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: carlpett
To fully approve this pull request, please assign additional approvers.
We suggest the following additional approver: cecilerobertmichon

Assign the PR to them by writing /assign @cecilerobertmichon in a comment when ready.

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

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

[carlpett]

/assign @CecileRobertMichon

[jackfrancis]

Thanks @carlpett We should probably make this an additive change, instead of replacing the previous checks. @khenidak thoughts?

[khenidak]

I think this is based on the temp workaround for intermitted DNS issues we had. I believe, We should do both. Internal and external lookups.

[jackfrancis]

@carlpett could you kindly update the manifest accordingly? Thanks so much!

[khenidak]

@carlpett and thanks a lot for this !

[carlpett]

Yes, I thought a while about that too, but I don't think there's an (obvious) way to do both? Each command/domain lookup is tied to one url, which is then used as a livenessProbe. And we can't have multiple probes per container.
I suppose we could do nslookup <internal> && nslookup <external>, or wrap in in sh -c '...' if it isn't executed in a shell already, what do you think?

[khenidak]

Yes - Exactly. The health prop is on the command exit code, it does not matter how many we execute.

[codecov]

Codecov Report

Merging #3282 into master will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master    #3282   +/-   ##
=======================================
  Coverage   53.31%   53.31%           
=======================================
  Files         104      104           
  Lines       15574    15574           
=======================================
  Hits         8304     8304           
  Misses       6537     6537           
  Partials      733      733

[carlpett]

I found the readability a bit poor when chaining it with &&, so I made it loop over a list of domains instead. Do you find this more or less readable?

[carlpett]

@jackfrancis Anything else needed here?

[jackfrancis]

@carlpett I'm kicking E2E to see how it likes these changes. Thanks for you patience—

[carlpett]

@jackfrancis If I read the test output correctly, this is the cause:

2018/07/02 18:21:47 Error while connecting to Windows dashboard:exit status 7
2018/07/02 18:21:47 Warning: Permanently added 'kubernetes-southeastasia-63593.southeastasia.cloudapp.azure.com,13.67.76.16' (ECDSA) to the list of known hosts.

  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed

  0     0    0     0    0     0      0      0 --:--:-- --:--:-- --:--:--     0curl: (7) Failed to connect to 10.240.0.34 port 31687: Connection refused

I'm not sure how I could have caused connection refused on Windows nodes... Is it flaky?

[jackfrancis]

lgtm