Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

SkyDNS is the only NS for Pods with DNSPolicy=ClusterFirst #18089

Merged
merged 1 commit into from
Dec 7, 2015
Merged

SkyDNS is the only NS for Pods with DNSPolicy=ClusterFirst #18089

merged 1 commit into from
Dec 7, 2015
+28 −13

Conversation

ArtfulCoder
Copy link
Contributor

#15645 was rolled back.

Addresses issue : #15592
Created this PR to take it forward again, with one additional change:
For pods with ClusterFirst DNSPolicy and kubelet without DNSCluster, the pod will be created with DNSDefault policy.

@ArtfulCoder ArtfulCoder added sig/network Categorizes an issue or PR as relevant to SIG Network. team/cluster labels Dec 2, 2015
@ArtfulCoder
Copy link
Contributor Author

Kubernetes pods have default policy of ClusterFirst, but the cluster DNS server is only an addon.
So, we silently fallback to DNSDefault policy for pods that want ClusterFirst DNSPolicy, but the kubelet does not have DNS configured.
Eventually, cluster DNS will not be an addon and the fallback will not be required.

@k8s-github-robot
Copy link

Labelling this PR as size/M

@k8s-github-robot k8s-github-robot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Dec 2, 2015
@k8s-bot
Copy link

k8s-bot commented Dec 2, 2015

GCE e2e test build/test passed for commit 6c3a88c.

@ArtfulCoder
Copy link
Contributor Author

This PR is in need of LGTM blessings

@thockin thockin added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Dec 2, 2015
@k8s-github-robot
Copy link

@k8s-bot test this

Tests are more than 48 hours old. Re-running tests.

@k8s-bot
Copy link

k8s-bot commented Dec 4, 2015

GCE e2e build/test failed for commit 6c3a88c.

@ArtfulCoder
Copy link
Contributor Author

@k8s-bot test this

@k8s-bot
Copy link

k8s-bot commented Dec 5, 2015

GCE e2e test build/test passed for commit 6c3a88c.

@k8s-github-robot
Copy link

@k8s-bot test this

Tests are more than 48 hours old. Re-running tests.

@k8s-bot
Copy link

k8s-bot commented Dec 7, 2015

GCE e2e build/test failed for commit 6c3a88c.

@ArtfulCoder
Copy link
Contributor Author

@k8s-bot test this

Sent from my iPhone

On Dec 6, 2015, at 5:56 PM, Kubernetes Bot notifications@github.com wrote:

GCE e2e build/test failed for commit 6c3a88c.

Build Log
Test Artifacts
Internal Jenkins Results
—
Reply to this email directly or view it on GitHub.

@k8s-bot
Copy link

k8s-bot commented Dec 7, 2015

GCE e2e test build/test passed for commit 6c3a88c.

@ArtfulCoder
Copy link
Contributor Author

This PR has not been merged for over 5 days. Self-merging.

ArtfulCoder added a commit that referenced this pull request Dec 7, 2015
@ArtfulCoder
Merge pull request #18089 from ArtfulCoder/oneNSAgain
bb3f49e 
SkyDNS is the only NS for Pods with DNSPolicy=ClusterFirst
@ArtfulCoder ArtfulCoder merged commit bb3f49e into kubernetes:master Dec 7, 2015
@ArtfulCoder ArtfulCoder mentioned this pull request Dec 8, 2015
Closed
soltysh added a commit to soltysh/origin that referenced this pull request Jan 25, 2016
@soltysh
UPSTREAM: TODO: allow hostDNS to be included along with ClusterDNS se…
520f003 
…tting

This was changed in kubernetes/kubernetes#18089
@soltysh soltysh mentioned this pull request Jan 25, 2016
Merged
46 tasks
soltysh added a commit to soltysh/origin that referenced this pull request Jan 25, 2016
@soltysh
UPSTREAM: TODO: allow hostDNS to be included along with ClusterDNS se…
7af7d02 
…tting

This was changed in kubernetes/kubernetes#18089
@liggitt
Copy link
Member

liggitt commented Jan 25, 2016

I consider this PR a compatibility breakage. It assumes clusterDNS does recursive lookups, which is not a valid assumption. Will open an issue to make "clusterDNS-only" behavior optional

@liggitt liggitt mentioned this pull request Jan 25, 2016
Closed
liggitt pushed a commit to soltysh/origin that referenced this pull request Jan 25, 2016
@soltysh @liggitt
UPSTREAM: <carry>: allow hostDNS to be included along with ClusterDNS…
dda408c 
… setting

Reverts behavior change made in k8s PR 18089

Carry until k8s issue 20090 is resolved and provides a way to opt out of clusterDNS-only behavior

This was changed in kubernetes/kubernetes#18089
liggitt pushed a commit to soltysh/origin that referenced this pull request Jan 25, 2016
@soltysh @liggitt
UPSTREAM: <carry>: allow hostDNS to be included along with ClusterDNS…
cd7db0f 
… setting

Reverts behavior change made in k8s PR 18089

Carry until k8s issue 20090 is resolved and provides a way to opt out of clusterDNS-only behavior

This was changed in kubernetes/kubernetes#18089
liggitt pushed a commit to soltysh/origin that referenced this pull request Jan 25, 2016
@soltysh @liggitt
UPSTREAM: <carry>: allow hostDNS to be included along with ClusterDNS…
9487f2c 
… setting

Reverts behavior change made in k8s PR 18089

Carry until k8s issue 20090 is resolved and provides a way to opt out of clusterDNS-only behavior

This was changed in kubernetes/kubernetes#18089
@thockin
Copy link
Member

thockin commented Jan 25, 2016

If cluster DNS is not recursive you will run into resolvers that are
broken. There is no specification for what recolv.conf really means, and
therefore implementations vary. Some resolvers (e.g. musl libc) will try
all nameservers in parallel, taking the first response - even if that is a
NAK. Busybox does something else (forget the details). Glibc does
something else.

We made this change after a large amount of deliberation and testing of
various resolvers.

On Mon, Jan 25, 2016 at 7:06 AM, Jordan Liggitt notifications@github.com
wrote:

I this PR a compatibility breakage. It assumes clusterDNS does recursive
lookups, which is not a valid assumption. Will open an issue to make
"clusterDNS-only" behavior optional

—
Reply to this email directly or view it on GitHub
#18089 (comment)
.

liggitt pushed a commit to soltysh/origin that referenced this pull request Jan 25, 2016
@soltysh @liggitt
UPSTREAM: <carry>: allow hostDNS to be included along with ClusterDNS…
ffab4b4 
… setting

Reverts behavior change made in k8s PR 18089

Carry until k8s issue 20090 is resolved and provides a way to opt out of clusterDNS-only behavior

This was changed in kubernetes/kubernetes#18089
@sdodson sdodson mentioned this pull request Feb 1, 2016
Closed
@mingfang
Copy link

Please revert this. After upgrade to 1.2 nothing in my cluster works anymore. I don't see any benefit for this at all.

@thockin
Copy link
Member

thockin commented Mar 17, 2016

I am sorry, can you be a bit more specific about what problems you are
having? This is all tested for months, so it is unlikely that "nothing
works". The kube-dns server is designed to be used this way.

What exactly are you experiencing?
On Mar 16, 2016 7:06 PM, "Ming Fang" notifications@github.com wrote:

Please revert this. After upgrade to 1.2 nothing in my cluster works
anymore. I don't see any benefit for this at all.

—
You are receiving this because you were assigned.
Reply to this email directly or view it on GitHub
#18089 (comment)

@mingfang
Copy link

I currently run kubelet with cluster-dns set to dnsmasq with no-resolv.
Prior to 1.2, every pod's /etc/resolv.conf had the value of my cluster-dns, followed by my regular dns.
That setup worked very well.

With 1.2, the pods now only have the cluster-dns in /etc/resolv.conf.
That broke my system.

There must be a reason why the DNSPolicy=ClusterFirst and not ClusterOnly.
Anyway, I commented out no-resolv in my dnsmasq and my system is working again.

@thockin
Copy link
Member

thockin commented Mar 17, 2016

Please read this issue regarding why this change is needed. Doing it the
old way is just not correct enough to rely on.

We will be investing significant energy into DNS in the next development
cycle.
On Mar 16, 2016 8:22 PM, "Ming Fang" notifications@github.com wrote:

I currently run kubelet with cluster-dns set to dnsmasq with no-resolv.
Prior to 1.2, every pod's /etc/resolv.conf had the value of my
cluster-dns, followed by my regular dns.
That setup worked very well.

With 1.2, the pods now only have the cluster-dns in /etc/resolv.conf.
That broke my system.

There must be a reason why the DNSPolicy=ClusterFirst and not ClusterOnly.
Anyway, I commented out no-resolv in my dnsmasq and my system is working
again.

—
You are receiving this because you were assigned.
Reply to this email directly or view it on GitHub
#18089 (comment)

@thockin thockin mentioned this pull request Mar 24, 2016
Closed
tnqn added a commit to tnqn/website that referenced this pull request Jun 24, 2018
@tnqn
Fix out-of-date dns doc
8ee6e0b 
Pods with "ClusterFirst" dnsPolicy didn't prepend the cluster DNS server
to the node's own DNS settings anymore since
kubernetes/kubernetes#18089. Only pods with
"default" dnsPolicy inherit DNS from nodes.

Besides, fixed a dead link in the doc.
tnqn added a commit to tnqn/website that referenced this pull request Jun 24, 2018
@tnqn
Fix out-of-date dns doc
90a91b1 
Pods with "ClusterFirst" dnsPolicy didn't prepend the cluster DNS server
to the node's own DNS settings anymore since kubernetes/kubernetes#18089.
Only pods with "default" dnsPolicy inherit DNS from nodes.

Besides, fixed a dead link in the doc.
@tnqn tnqn mentioned this pull request Jun 24, 2018
Merged
tnqn added a commit to tnqn/website that referenced this pull request Jun 24, 2018
@tnqn
Fix out-of-date dns doc
45e27d4 
Pods with "ClusterFirst" dnsPolicy didn't prepend the cluster DNS server
to the node's own DNS settings anymore since kubernetes/kubernetes#18089.
Only pods with "default" dnsPolicy inherit DNS from nodes.

Besides, fixed a dead link in the doc.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@thockin thockin

Labels
lgtm "Looks good to me", indicates that a PR is ready to be merged. sig/network Categorizes an issue or PR as relevant to SIG Network. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@ArtfulCoder @k8s-github-robot @k8s-bot @liggitt @thockin @mingfang @googlebot