Remove a statement to skip over adding the kube-dns addon

[rbierman]

This should fix issue: #2808

However it seems to create a new issue as kubeadm creates a DNS deployment by itself. Based on the documentation found here: https://kubernetes.io/docs/tasks/administer-cluster/coredns/ it seems like you're only able to switch between kube-dns and core-dns and not disable the deployment. @r2d4 Do you have any thoughts how we could fix this?

NVM @dlorenc I'm curious if by merging #3073 we have 2 core-dns deployments on kubernetes version: v1.11.0 or higher?

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is NOT APPROVED

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

If they are not already assigned, you can assign the PR to them by writing /assign @jimmidyson 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

[k8s-ci-robot]

Thanks for your pull request. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please follow instructions at https://git.k8s.io/community/CLA.md#the-contributor-license-agreement to sign the CLA.

It may take a couple minutes for the CLA signature to be fully registered; after that, please reply here with a new comment and we'll verify. Thanks.

---

• If you've already signed a CLA, it's possible we don't have your GitHub username or you're using a different email address. Check your existing CLA data and verify that your email is set on your git commits.
• If you signed the CLA as a corporation, please sign in with your organization's credentials at https://identity.linuxfoundation.org/projects/cncf to be authorized.
• If you have done the above and are still having issues with the CLA being reported as unsigned, please email the CNCF helpdesk: helpdesk@rt.linuxfoundation.org

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

[minikube-bot]

Can one of the admins verify this patch?

[dlorenc]

@dlorenc I'm curious if by merging #3073 we have 2 core-dns deployments on kubernetes version: v1.11.0 or higher?

I don't think so - I think that file is basically just ignored now because of the range statement you're skipping over here.

What's the end goal? The ability to disable core-dns?

[rbierman]

@dlorenc Sorry I mixed up two PR's. The PR related to possibly multiple core-dns servers: #3072 could potentially introduce that problem.

This PR should fix the problem of not being able to disable the kube-dns addon. Right now minikube enables the kube-dns plugin as it's installed by default. Hhowever because of this check it doesn't actually move any files into the cluster. When you try to disable to plugin it fails as it's not able to delete any of the assets that are associated with this addon. (See: #2808 (comment))

The end goal of this PR would be to be able to enable end disable the kube-dns addon as expected.

[dlorenc]

Do you think we should just delete the kube-dns addon? It won't work with kubeadm for the reasons you pointed out above, and would still allow it to be "disabled".

[rbierman]

Hmm deleting the kube-dns addon 'fixes' this problem but it leaves us with the same problem for core-dns. Also it doesn't allow our users to use kube-dns for older versions of Kubernetes.

I was trying to figure out if kubeadm has the option to not install any DNS service at all but it seems like that's not the case. So we either try to get that feature in the kubeadm tool or we can consider a solution along the lines of:

1. kubeadm version detection to set the feature-gates=CoreDNS flag in case it's required.
2. Remove the default dns installation that's done by kubeadm
3. Install the core-dns addon.

@rajansandeep Do you have any thoughts around this (Especially point 1)?

[rajansandeep]

@rbierman
The current soln doesn't work when CoreDNS is now the default addon.
However, once minikube uses the version of kubeadm where CoreDNS is the default, everything should be working with this fix, right?

[rbierman]

Yeah it should work for that version but what about all previous versions?

If someone executes: ./minikube start --kubernetes-version v1.10.0 it will have both core-dns and kube-dns installed won't it?

As this is the output from: kubectl get all --all-namespaces

NAMESPACE     NAME                                    READY     STATUS    RESTARTS   AGE
kube-system   coredns-64c447755d-q658j                1/1       Running   0          2m
kube-system   etcd-minikube                           1/1       Running   0          2m
kube-system   kube-addon-manager-minikube             1/1       Running   0          2m
kube-system   kube-apiserver-minikube                 1/1       Running   0          2m
kube-system   kube-controller-manager-minikube        1/1       Running   0          2m
kube-system   kube-dns-86f4d74b45-ww6kg               3/3       Running   0          2m
kube-system   kube-proxy-c6mqc                        1/1       Running   0          2m
kube-system   kube-scheduler-minikube                 1/1       Running   0          2m
kube-system   kubernetes-dashboard-5498ccf677-t4jbs   1/1       Running   0          2m
kube-system   storage-provisioner                     1/1       Running   0          2m

NAMESPACE     NAME                   TYPE        CLUSTER-IP       EXTERNAL-IP   PORT(S)         AGE
default       kubernetes             ClusterIP   10.96.0.1        <none>        443/TCP         2m
kube-system   kube-dns               ClusterIP   10.96.0.10       <none>        53/UDP,53/TCP   2m
kube-system   kubernetes-dashboard   NodePort    10.108.116.139   <none>        80:30000/TCP    2m

NAMESPACE     NAME         DESIRED   CURRENT   READY     UP-TO-DATE   AVAILABLE   NODE SELECTOR   AGE
kube-system   kube-proxy   1         1         1         1            1           <none>          2m

NAMESPACE     NAME                   DESIRED   CURRENT   UP-TO-DATE   AVAILABLE   AGE
kube-system   coredns                1         1         1            1           2m
kube-system   kube-dns               1         1         1            1           2m
kube-system   kubernetes-dashboard   1         1         1            1           2m

NAMESPACE     NAME                              DESIRED   CURRENT   READY     AGE
kube-system   coredns-64c447755d                1         1         1         2m
kube-system   kube-dns-86f4d74b45               1         1         1         2m
kube-system   kubernetes-dashboard-5498ccf677   1         1         1         2m

[rajansandeep]

@rbierman Aah yes!
I think the best solution is to remove kube-dns and CoreDNS from the addon manager and let kubeadm install DNS from its side. If users want to choose between the DNS server, they can do so by toggling the feature-gate flag.
@dlorenc wdyt?

[rbierman]

@rajansandeep I think the best solution would be to see if we can make a change in kubeadm to not install any dns service at all.

As a second best we could go with disabling both plugins but leave them in there. So users can enable them if they would like to for older versions. (We might want to add a message to warn the user for having two dns services running if they don't disable the default one).

@dlorenc Thoughs?

[rbierman]

/assign @dlorenc

[dlorenc]

I think the best solution is to remove kube-dns and CoreDNS from the addon manager and let kubeadm install DNS from its side. If users want to choose between the DNS server, they can do so by toggling the feature-gate flag.
@dlorenc wdyt?

I think this is probably the best solution. Deleting both addons and letting kubeadm pick makes sense to me.

[tstromberg]

Closing this PR as obsolete. Please re-open if I am incorrectly doing so.