kubeadm: Don't match DNS versions to K8s versions #64761

rosti · 2018-06-05T11:45:38Z

What this PR does / why we need it:

Some code in kubeadm was designed with the intent, that in the future CoreDNS
and kube-dns versions will match to specific K8s versions. This code is not
functional, since it does not perform any version matching. As of this moment,
no version matching is planned and a lot of boilerplate code is left useless.
The solution is simple - remove the unneeded parts to simplify the flow.

Signed-off-by: Rostislav M. Georgiev rostislavg@vmware.com

Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged):
Fixes kubernetes/kubeadm#870

Special notes for your reviewer:
/cc @kubernetes/sig-cluster-lifecycle-pr-reviews
/area kubeadm
/assign @luxas
/assign @timothysc
/kind cleanup

Release note:

NONE

k8s-ci-robot · 2018-06-05T11:45:47Z

@rosti: Reiterating the mentions to trigger a notification:
@kubernetes/sig-cluster-lifecycle-pr-reviews

In response to this:

What this PR does / why we need it:

Some code in kubeadm was designed with the intent, that in the future CoreDNS
and kube-dns versions will match to specific K8s versions. This code is not
functional, since it does not perform any version matching. As of this moment,
no version matching is planned and a lot of boilerplate code is left useless.
The solution is simple - remove the unneeded parts to simplify the flow.

Signed-off-by: Rostislav M. Georgiev rostislavg@vmware.com

Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged):
Fixes kubernetes/kubeadm#870

Special notes for your reviewer:
/cc @kubernetes/sig-cluster-lifecycle-pr-reviews
/area kubeadm
/assign @luxas
/assign @timothysc
/kind cleanup

Release note:
NONE

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.

wgliang · 2018-06-05T12:04:17Z

/ok-to-test

fturib · 2018-06-05T12:41:58Z

@rajansandeep : can you review here and double check there will be no impact for CoreDNS deployment/upgrade using kubeadm ? Thanks.

chuckha

I'm very happy about this change, it simplifies this workflow greatly. 👏

I want to check my understanding to make sure this is what I think it is: kubedns and coredns both do not depend on the k8s version. In this case 1.14.10 works for 1.9, 1.10 and 1.11 and when the version of coredns gets bumped to say, 1.15, it will work with all of the supported versions of k8s? Is this documented somewhere or are you in the core/kubeDNS community?

I want to double check this because we've had a lot of upgrade issues in the past cycle and want to make sure we don't run into more.

Thank you for this change!

chuckha · 2018-06-05T13:19:14Z

cmd/kubeadm/app/phases/addons/dns/dns.go

-	kubeDNSDeploymentBytes := GetKubeDNSManifest(k8sVersion)
-	dnsDeploymentBytes, err := kubeadmutil.ParseTemplate(kubeDNSDeploymentBytes,
+	// v1.8.0+ has only one known YAML manifest spec for KubeDNS
+	dnsDeploymentBytes, err := kubeadmutil.ParseTemplate(KubeDNSDeployment,
 		struct{ ImageRepository, Arch, Version, DNSBindAddr, DNSProbeAddr, DNSDomain, MasterTaintKey string }{
 			ImageRepository: cfg.ImageRepository,
 			Arch:            runtime.GOARCH,
 			// Get the kube-dns version conditionally based on the k8s version


This comment should be removed

rosti · 2018-06-05T13:52:45Z

This PR does not change the behavior of kubeadm in any way. It is a simple cleanup. The CoreDNS/kube-dns versions are known to work with the supported by kubeadm Kubernetes versions.
Considering this and kubernetes/kubeadm#870, in a short discussion on Slack with @luxas I decided to do this PR.
On the question of documentation, I am not sure if this behavior is documented somewhere currently. Yet, it is the current behavior and if it isn't documented, then we should do so.

chuckha

thanks for the update! lgtm

chuckha · 2018-06-05T13:54:49Z

/lgtm

rosti · 2018-06-05T13:56:32Z

Removed the old 1.8 comment

chuckha · 2018-06-05T13:58:53Z

/lgtm

rosti · 2018-06-05T14:04:47Z

Fixed a couple of other comments, that were left over.

dixudx · 2018-06-05T14:32:39Z

@rosti Very good cleanup.

/lgtm

timothysc · 2018-06-05T19:21:23Z

@luxas what do you think here? in or wait on 1.11?

luxas

@timothysc I think this is valuable to get into v1.11.
We had this for a reason, the manifest changed frequently for different k8s versions. Now things have settled and we're using coredns as the default which my impression is kinda stable, so now we can remove the unnecessary conditionals here IMO.

/lgtm
/approve

luxas · 2018-06-05T22:23:32Z

/milestone v1.11
/status approved-for-milestone
/priority critical-urgent

luxas · 2018-06-05T22:23:46Z

@rosti needs rebase now though

k8s-github-robot · 2018-06-05T22:24:23Z

[MILESTONENOTIFIER] Milestone Pull Request: Up-to-date for process

@chuckha @dixudx @luxas @rosti @timothysc

Pull Request Labels

sig/cluster-lifecycle: Pull Request will be escalated to these SIGs if needed.
priority/critical-urgent: Never automatically move pull request out of a release milestone; continually escalate to contributor and SIG through all available channels.
kind/cleanup: Adding tests, refactoring, fixing old bugs.

Help

fejta-bot · 2018-06-06T00:38:21Z

/retest
This bot automatically retries jobs that failed/flaked on approved PRs (send feedback to fejta).

Review the full test history for this PR.

Silence the bot with an /lgtm cancel comment for consistent failures.

fejta-bot · 2018-06-06T02:44:20Z

/retest
This bot automatically retries jobs that failed/flaked on approved PRs (send feedback to fejta).

Review the full test history for this PR.

Silence the bot with an /lgtm cancel comment for consistent failures.

fejta-bot · 2018-06-06T05:32:20Z

/retest
This bot automatically retries jobs that failed/flaked on approved PRs (send feedback to fejta).

Review the full test history for this PR.

Silence the bot with an /lgtm cancel comment for consistent failures.

fejta-bot · 2018-06-06T07:39:01Z

/retest
This bot automatically retries jobs that failed/flaked on approved PRs (send feedback to fejta).

Review the full test history for this PR.

Silence the bot with an /lgtm cancel comment for consistent failures.

Some code in kubeadm was designed with the intent, that in the future CoreDNS and kube-dns versions will match to specific K8s versions. This code is not functional, since it does not perform any version matching. As of this moment, no version matching is planned and a lot of boilerplate code is left useless. The solution is simple - remove the unneeded parts to simplify the flow. Signed-off-by: Rostislav M. Georgiev <rostislavg@vmware.com>

dixudx · 2018-06-06T13:03:55Z

/lgtm

k8s-ci-robot · 2018-06-06T13:04:03Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: chuckha, dixudx, luxas, rosti

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:

~~cmd/kubeadm/OWNERS~~ [luxas]

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

k8s-github-robot · 2018-06-06T13:58:50Z

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

k8s-github-robot · 2018-06-06T15:27:24Z

Automatic merge from submit-queue. If you want to cherry-pick this change to another branch, please follow the instructions here.

k8s-ci-robot assigned luxas and timothysc Jun 5, 2018

k8s-ci-robot requested review from chuckha and dixudx June 5, 2018 11:46

k8s-ci-robot removed the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label Jun 5, 2018

chuckha reviewed Jun 5, 2018

View reviewed changes

chuckha approved these changes Jun 5, 2018

View reviewed changes

k8s-ci-robot assigned chuckha Jun 5, 2018

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jun 5, 2018

rosti force-pushed the kubeadm-dnsverfix branch from 19db48e to aee9101 Compare June 5, 2018 13:56

k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jun 5, 2018

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jun 5, 2018

rosti force-pushed the kubeadm-dnsverfix branch from aee9101 to 1d0cb62 Compare June 5, 2018 14:04

k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jun 5, 2018

k8s-ci-robot assigned dixudx Jun 5, 2018

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jun 5, 2018

luxas approved these changes Jun 5, 2018

View reviewed changes

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jun 5, 2018

k8s-ci-robot added the status/approved-for-milestone label Jun 5, 2018

k8s-ci-robot added this to the v1.11 milestone Jun 5, 2018

k8s-ci-robot added the priority/critical-urgent Highest priority. Must be actively worked on as someone's top priority right now. label Jun 5, 2018

rosti force-pushed the kubeadm-dnsverfix branch from 1d0cb62 to 19d2bbb Compare June 6, 2018 09:16

k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jun 6, 2018

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jun 6, 2018

k8s-github-robot merged commit bceb90c into kubernetes:master Jun 6, 2018

rosti deleted the kubeadm-dnsverfix branch November 22, 2018 15:29

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

kubeadm: Don't match DNS versions to K8s versions #64761

kubeadm: Don't match DNS versions to K8s versions #64761

rosti commented Jun 5, 2018

k8s-ci-robot commented Jun 5, 2018

wgliang commented Jun 5, 2018

fturib commented Jun 5, 2018

chuckha left a comment

chuckha Jun 5, 2018

rosti commented Jun 5, 2018

chuckha left a comment

chuckha commented Jun 5, 2018

rosti commented Jun 5, 2018 •

edited

chuckha commented Jun 5, 2018

rosti commented Jun 5, 2018

dixudx commented Jun 5, 2018

timothysc commented Jun 5, 2018 •

edited

luxas left a comment

luxas commented Jun 5, 2018

luxas commented Jun 5, 2018

k8s-github-robot commented Jun 5, 2018

fejta-bot commented Jun 6, 2018

fejta-bot commented Jun 6, 2018

fejta-bot commented Jun 6, 2018

fejta-bot commented Jun 6, 2018

dixudx commented Jun 6, 2018

k8s-ci-robot commented Jun 6, 2018

k8s-github-robot commented Jun 6, 2018

k8s-github-robot commented Jun 6, 2018

kubeadm: Don't match DNS versions to K8s versions #64761

kubeadm: Don't match DNS versions to K8s versions #64761

Conversation

rosti commented Jun 5, 2018

k8s-ci-robot commented Jun 5, 2018

wgliang commented Jun 5, 2018

fturib commented Jun 5, 2018

chuckha left a comment

Choose a reason for hiding this comment

chuckha Jun 5, 2018

Choose a reason for hiding this comment

rosti commented Jun 5, 2018

chuckha left a comment

Choose a reason for hiding this comment

chuckha commented Jun 5, 2018

rosti commented Jun 5, 2018 • edited

chuckha commented Jun 5, 2018

rosti commented Jun 5, 2018

dixudx commented Jun 5, 2018

timothysc commented Jun 5, 2018 • edited

luxas left a comment

Choose a reason for hiding this comment

luxas commented Jun 5, 2018

luxas commented Jun 5, 2018

k8s-github-robot commented Jun 5, 2018

fejta-bot commented Jun 6, 2018

fejta-bot commented Jun 6, 2018

fejta-bot commented Jun 6, 2018

fejta-bot commented Jun 6, 2018

dixudx commented Jun 6, 2018

k8s-ci-robot commented Jun 6, 2018

k8s-github-robot commented Jun 6, 2018

k8s-github-robot commented Jun 6, 2018

rosti commented Jun 5, 2018 •

edited

timothysc commented Jun 5, 2018 •

edited