just to double check.

what happens if the user passes kube-dns as the type after this change:
https://github.com/kubernetes/kubernetes/blob/700047303c5414b5fe9b0d1240602cb680274ca1/cmd/kubeadm/app/apis/kubeadm/v1beta2/types.go#L93

is it a NOOP now?

if so, i think we must reflect that in the release note with an additional sentence.

The "kube-dns" type is no longer recognized if passed to ClusterConfiguration.dns.type.

should we error out if anything other than "CoreDNS" is used?

@rajansandeep

should we error out if anything other than "CoreDNS" is used?

i cannot find validation for ClusterConfiguration.DNS.Type.
in EnsureDNSAddon() we just check if Type is coredns first and if not it uses kube-dns.
https://github.com/kubernetes/kubernetes/blob/master/cmd/kubeadm/app/phases/addons/dns/dns.go#L103-L114

this validation should have been originally in:
https://github.com/kubernetes/kubernetes/blob/master/cmd/kubeadm/app/apis/kubeadm/validation/validation.go#L59

so...we can instead do this in the release note:

kubeadm: the deprecated kube-dns is no longer supported as an option. Given kubeadm now only supports CoreDNS as the DNS addon, the value of the field "ClusterConfiguration.dns.type" is ignored.

instead of adding the validation in this PR.

Got it. I updated the release notes

the value of the field "ClusterConfiguration.dns.type" is ignored

That's surprising to me... was that field validated in 1.20? If so, we should error rather than silently accept a config that explicitly requests kube-dns and previously worked and not honor it.

apparently the field was never validated after the API was converted to internal type...
and during runtime the check was if type == "CoreDNS" { // install coredns } else { // install kube-dns }.

having the validation seems better to me overall, but i didn't want to introduce it now.

if we are going it introduce it, it would look like this:

add this:

// ValidateDNS validates the DNS object and collects all encountered errors
func ValidateDNS(dns *kubeadm.DNS, fldPath *field.Path) field.ErrorList {
	allErrs := field.ErrorList{}
	if dns.Type != kubeadm.CoreDNS {
		allErrs = append(allErrs, field.Invalid(fldPath, dns.Type, fmt.Sprintf("only DNS type %q is supported", kubeadm.CoreDNS)))
	}
	return allErrs
}

here https://github.com/kubernetes/kubernetes/blob/master/cmd/kubeadm/app/apis/kubeadm/validation/validation.go#L493

it should be called like so:

allErrs = append(allErrs, ValidateDNS(&c.DNS, field.NewPath("dns"))...)

here:
https://github.com/kubernetes/kubernetes/blob/master/cmd/kubeadm/app/apis/kubeadm/validation/validation.go#L60

this means that we need to keep the kube-dns type constant and related issues open with a TODO.

Seems like you could inline it as "kube-dns" with a comment that this value is no longer supported. Not sure why you'd need to remove or adjust it in the future.

ok, it can be inlined.

since i logged this ticket to remove the "type" field in v1beta3.
kubernetes/kubeadm#2398
i guess we will have to remove this validation too...EDIT: actually it should be removed with v1beta2 i guess.

@rajansandeep please let me know if you have any questions for this update.

// ValidateDNS validates the DNS object and collects all encountered errors
func ValidateDNS(dns *kubeadm.DNS, fldPath *field.Path) field.ErrorList {
	allErrs := field.ErrorList{}
	const kubeDNSType = "kube-dns"
	if dns.Type == kubeDNSType {
		allErrs = append(allErrs, field.Invalid(fldPath, dns.Type, fmt.Sprintf("DNS type %q is no longer supported", kubeDNSType)))
	}
	return allErrs
}

@rajansandeep
we also need to change the release note to:

kubeadm: the deprecated kube-dns is no longer supported as an option. If "ClusterConfiguration.dns.type" is set to "kube-dns" kubeadm will now throw an error.

I've added the validation and changed the release notes.
Please let me know if I should squash the commits or leave it as is...

thanks.