Adds DNSControllerSpec and WatchIngress flag

[geojaz]

This PR is in reference to #2496, #2468 and the issues referenced in there relating to use of the watch-ingress flag.

This PR attempts to rectify this situation and gives users who want it, the option to turn on watch-ingress without forcing it on them. Also spits out a warning to the logs about potential side effects.

Includes notes in docs/cluster_spec.md to explain.

---

This change is 

[geojaz]

@justinsb Minor API changes for a good cause here :) Would you pleas take a look?

[justinsb]

I like this, but can we just revert the change so we can get 1.6.0 out?

[justinsb]

I'm also wondering if we adopt external-dns for ingress

[geojaz]

Yes please, let's get 1.6 out

[DerekV]

Any plans to include this in the next release?

[chrislovecnm]

@justinsb we good to get this in?

[chrislovecnm]

@geojaz need a rebase

[chrislovecnm]

@geojaz status on this?

[geojaz]

@chrislovecnm I'll try to get this done tonight

[geojaz]

@chrislovecnm :)

[justinsb]

FYI, I tend to do this:

dnsController := tf.cluster.Spec.DNSController
if dnsController == nil {
  dnsController = &DNSControllerSpec{}
}
...

With one option it's a wash, with a bunch it makes things simpler :-)

[justinsb]

This LGTM. I'm not sure if we should call it dnsController or externalDns though... I'll leave that up to you @geojaz

My thoughts:

• eventually kops will use external-dns
• external-dns makes it clearer that this is not kube-internal DNS

But:

• it doesn't really matter anyway; we can change in future and both are reasonably clear :-)

[justinsb]

/lgtm

[geojaz]

I think we should call it externalDns with some language on the docs/commenting in the code to explain that this is also the config for dnsController. I'll clean this up and let you take one more look at it before merge. Thanks for the ideas :)

[kenden]

"Set this option at risk of interrupting Service updates in some cases.".
It would be nice to add more info about when this would/could happen.

[geojaz]

Will squash commits as make sense. I moved the config from DNSControllerSpec to ExternalDNSConfig and updated the docs and such. I think next step is to refactor to combine the DNSControllerArgv and ExternalDNSArgv in template_functions... but maybe not crucial for this submission.

[chrislovecnm]

Need a rebase and e2e failed.

[k8s-github-robot]

@geojaz PR needs rebase

[chrislovecnm]

/lgtm cancel

[chrislovecnm]

/lgtm

[DerekV]

Is there some way to move this forward if it just needs a rebase?

[chrislovecnm]

Paging @geojaz :)

[k8s-github-robot]

/lgtm cancel //PR changed after LGTM, removing LGTM. @chrislovecnm @geojaz @justinsb

[chrislovecnm]

Thanks

[chrislovecnm]

/lgtm

@DerekV will be in our next release, let us know if you have any questions about using it before the release

[k8s-github-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: chrislovecnm, geojaz, justinsb

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

Needs approval from an approver in each of these OWNERS Files:

• OWNERS [chrislovecnm,justinsb]

You can indicate your approval by writing /approve in a comment
You can cancel your approval by writing /approve cancel in a comment

[k8s-github-robot]

/test all [submit-queue is verifying that this PR is safe to merge]

[k8s-github-robot]

Automatic merge from submit-queue