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

Add dnsmasq as a DNS cache in kube-dns pod #26114

Merged
merged 2 commits into from
May 26, 2016
Merged

Add dnsmasq as a DNS cache in kube-dns pod #26114

merged 2 commits into from
May 26, 2016

Conversation

ArtfulCoder
Copy link
Contributor

@ArtfulCoder ArtfulCoder commented May 23, 2016 •
edited
Loading

  1. added dns-port flag
  2. updated imports to not use the v1 version of the api objects.

@k8s-github-robot k8s-github-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. release-note-label-needed labels May 23, 2016
@caesarxuchao caesarxuchao added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label May 23, 2016
@caesarxuchao
Copy link
Member

LGTM. Thanks.

@ArtfulCoder
Copy link
Contributor Author

@thockin

@ArtfulCoder ArtfulCoder added the release-note Denotes a PR that will be considered when it comes time to generate release notes. label May 23, 2016
@ArtfulCoder ArtfulCoder assigned thockin and unassigned caesarxuchao May 23, 2016
@ArtfulCoder
Copy link
Contributor Author

Chao reviewed the import changes.
Waiting for Tim to review the DNS specific changes.

@k8s-github-robot k8s-github-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label May 23, 2016
thockin
thockin reviewed May 24, 2016
View reviewed changes
cluster/saltbase/salt/kube-dns/kubedns-rc.yaml.in
@@ -54,13 +54,21 @@ spec:
args:
# command = "/kube-dns"
- --domain={{ pillar['dns_domain'] }}.
- --dns-port=10053
ports:
- containerPort: 53
Copy link
Member

Choose a reason for hiding this comment

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

you should update the ports info to :10053 here and duplicate this into dnsmasq on :53

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done.

@thockin thockin changed the title added dns-port flag to have a custom DNS port for skydns to serve DNS… Add dnsmasq as a DNS cache in kube-dns pod May 24, 2016
@ArtfulCoder ArtfulCoder changed the title Add dnsmasq as a DNS cache in kube-dns pod kube-dns pod has configurable dns port May 24, 2016
@ArtfulCoder ArtfulCoder changed the title kube-dns pod has configurable dns port kube-dns pod has configurable dns port (NEEDS IMAGE PUSH) May 24, 2016
@ArtfulCoder ArtfulCoder force-pushed the dnsport branch 4 times, most recently from f243dbf to b638913 Compare May 24, 2016 23:13
@thockin thockin added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label May 25, 2016
@thockin thockin changed the title kube-dns pod has configurable dns port (NEEDS IMAGE PUSH) Add dnsmasq as a DNS cache in kube-dns pod May 25, 2016
@thockin
Copy link
Member

thockin commented May 25, 2016

LGTM - push your image and kick e2e

@ArtfulCoder ArtfulCoder added the priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. label May 25, 2016
@k8s-github-robot k8s-github-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label May 25, 2016
@k8s-github-robot
Copy link

@ArtfulCoder
You must link to the test flake issue which caused you to request this manual re-test.
Re-test requests should be in the form of: k8s-bot test this issue: #<number>
Here is the list of open test flakes.

@ArtfulCoder
Copy link
Contributor Author

gce e2e never ran any tests and failed..
kicking it again

@k8s-bot test this issue: #IGNORE

@ArtfulCoder
Copy link
Contributor Author

@k8s-bot test this issue: #IGNORE

@thockin thockin added this to the v1.3 milestone May 25, 2016
@ArtfulCoder
Copy link
Contributor Author

@k8s-bot test this issue: #IGNORE

@ArtfulCoder ArtfulCoder added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label May 26, 2016
@k8s-github-robot
Copy link

@k8s-bot test this [submit-queue is verifying that this PR is safe to merge]

@k8s-bot
Copy link

k8s-bot commented May 26, 2016

GCE e2e build/test passed for commit 10f9789.

@ArtfulCoder ArtfulCoder merged commit 965e8dc into kubernetes:master May 26, 2016
@luxas
Copy link
Member

luxas commented May 26, 2016

@ArtfulCoder Is dnsmasq a required container to run in order to get it working with 1.1?

@kjvalencik
Copy link

kjvalencik commented Jul 6, 2016 •
edited
Loading

Would it be better to run dnsmasq in a separate RC / pod in production? AFAIK, the skydns pod still has a race condition when it starts that it is reported healthy before having a chance to add all of the DNS entries. Putting dnsmasq in a separate pod could help mitigate this.

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. priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. release-note Denotes a PR that will be considered when it comes time to generate release notes. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Milestone
v1.3
Development

Successfully merging this pull request may close these issues.
8 participants
@ArtfulCoder @caesarxuchao @thockin @k8s-github-robot @k8s-bot @luxas @kjvalencik @googlebot