Skip to content
This repository has been archived by the owner on Jan 9, 2023. It is now read-only.

Use core-dns over kube-dns for clusters >= 1.10 #715

Merged
merged 5 commits into from
Feb 7, 2019

Conversation

JoshVanL
Copy link
Contributor

What this PR does / why we need it:
Use core-dns instead of kube-dns by default for clusters >= 1.10

Ensures that kube-dns is replaced by core-dns when upgrading >= 1.10 and visa-versa.

fixes #422

Default to core-dns over kube-dns for clusters >= 1.10

/assign

@jetstack-bot jetstack-bot added release-note Denotes a PR that will be considered when it comes time to generate release notes. dco-signoff: yes Indicates that all commits in the pull request have the valid DCO sign-off message. approved Indicates a PR has been approved by an approver from all required OWNERS files. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Jan 31, 2019
@jetstack-bot jetstack-bot added area/puppet Indicates a PR is affecting Puppet manifests kind/documentation Categorizes issue or PR as related to documentation. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Jan 31, 2019
@jetstack-bot jetstack-bot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Feb 4, 2019
@jetstack-bot
Copy link
Collaborator

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: JoshVanL

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:

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

@JoshVanL JoshVanL force-pushed the 422-core-dns branch 2 times, most recently from f755d29 to 01fa4be Compare February 5, 2019 11:03
@JoshVanL
Copy link
Contributor Author

JoshVanL commented Feb 5, 2019

/test puppet-tarmak-acceptance-centos v1.11

@JoshVanL JoshVanL force-pushed the 422-core-dns branch 4 times, most recently from 20fd014 to 285174b Compare February 5, 2019 14:25
@JoshVanL
Copy link
Contributor Author

JoshVanL commented Feb 6, 2019

/unassign
/assign @simonswine

@jetstack-bot jetstack-bot assigned simonswine and unassigned JoshVanL Feb 6, 2019
Copy link
Contributor

@simonswine simonswine left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A couple of bits and pieces to change.

The delete is quite ugly, I would prefer a file{ensure => 'absent'}

/unassign
/assign @JoshVanL

metadata:
labels:
kubernetes.io/bootstrapping: rbac-defaults
addonmanager.kubernetes.io/mode: EnsureExists
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reconcile

metadata:
annotations:
rbac.authorization.kubernetes.io/autoupdate: "true"
addonmanager.kubernetes.io/mode: EnsureExists
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reconcile

upstream
fallthrough in-addr.arpa ip6.arpa
}
prometheus :9153
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we setup the annotation to scrape it?

$protocol = 'http'
}

$command = "/bin/bash -c \"while true; do if [[ \$(curl -k -w '%{http_code}' -s -o /dev/null ${protocol}://localhost:${server_port}/healthz) == 200 ]]; then break; else sleep 2; fi; done; kubectl delete -f '${apply_file}'; rm -f '${apply_file}'; exit 0\""
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we really need to run kubectl exec. I am bit concered, that we get some more race conditions back.

Deleting through kube-addon-manager would be way more beneficial. As it has some leader election builtin.

$post_1_10 = versioncmp($::kubernetes::version, '1.10.0') >= 0

if $post_1_10 {
$service = 'core-dns'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we keep the service as kube-dns? That's at least what kubeadm is doing and a change is quite risky and affecting all applications during runtime

@jetstack-bot jetstack-bot assigned JoshVanL and unassigned simonswine Feb 6, 2019
Signed-off-by: JoshVanL <vleeuwenjoshua@gmail.com>
Signed-off-by: JoshVanL <vleeuwenjoshua@gmail.com>
Signed-off-by: JoshVanL <vleeuwenjoshua@gmail.com>
Signed-off-by: JoshVanL <vleeuwenjoshua@gmail.com>
Signed-off-by: JoshVanL <vleeuwenjoshua@gmail.com>
@JoshVanL
Copy link
Contributor Author

JoshVanL commented Feb 6, 2019

Tested Prometheus and was scraping core-dns
/unassign
/assign @simonswine

@jetstack-bot jetstack-bot assigned simonswine and unassigned JoshVanL Feb 6, 2019
@simonswine
Copy link
Contributor

Upgrade works well for me, thanks this is way cleaner @JoshVanL

/lgtm

@jetstack-bot jetstack-bot added the lgtm Indicates that a PR is ready to be merged. label Feb 7, 2019
@jetstack-bot jetstack-bot merged commit 7c4f36e into jetstack:master Feb 7, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. area/puppet Indicates a PR is affecting Puppet manifests dco-signoff: yes Indicates that all commits in the pull request have the valid DCO sign-off message. kind/documentation Categorizes issue or PR as related to documentation. lgtm Indicates that a PR is ready to be merged. release-note Denotes a PR that will be considered when it comes time to generate release notes. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Migration kube-dns => core-dns
3 participants