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

Adding CoreDNS as an addon #1995

Merged
merged 3 commits into from
Oct 9, 2017
Merged

Adding CoreDNS as an addon #1995

merged 3 commits into from
Oct 9, 2017

Conversation

rajansandeep
Copy link
Contributor

Adding CoreDNS as one of the default add-on in minikube. CoreDNS will be disabled by default.

@k8s-ci-robot k8s-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Sep 21, 2017
@minikube-bot
Copy link
Collaborator

Can one of the admins verify this patch?

@rajansandeep
Copy link
Contributor Author

@johnbelamaric @chrisohaver

@johnbelamaric
Copy link
Member

See kubernetes/enhancements#427

johnbelamaric
johnbelamaric suggested changes Sep 21, 2017
View reviewed changes
deploy/addons/coredns/coreDNS-controller.yaml Outdated
serviceAccountName: coredns
containers:
- name: coredns
image: coredns/coredns:latest
Copy link
Member

Choose a reason for hiding this comment

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

Use coredns/coredns:011; we will updated it when we release 1.0.0.

deploy/addons/coredns/coreDNS-configmap.yaml Outdated
errors
log stdout
health
kubernetes cluster.local
Copy link
Member

Choose a reason for hiding this comment

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

We should have the service CIDR here too for reverse lookups.

deploy/addons/coredns/coreDNS-configmap.yaml Outdated
Corefile: |
.:53 {
errors
log stdout
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 just put log not log stdout.

deploy/addons/coredns/coreDNS-controller.yaml
name: coredns
items:
- key: Corefile
path: Corefile
Copy link
Member

Choose a reason for hiding this comment

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

Fix missing newline.

deploy/addons/coredns/coreDNS-clusterrole.yaml
- namespaces
verbs:
- list
- watch
Copy link
Member

Choose a reason for hiding this comment

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

Fix missing newline.

deploy/addons/coredns/coreDNS-crbinding.yaml
subjects:
- kind: ServiceAccount
name: coredns
namespace: kube-system
Copy link
Member

Choose a reason for hiding this comment

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

Fix missing newline.

deploy/addons/coredns/coreDNS-sa.yaml
name: coredns
namespace: kube-system
labels:
addonmanager.kubernetes.io/mode: Reconcile
Copy link
Member

Choose a reason for hiding this comment

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

Fix missing newline.

deploy/addons/coredns/coreDNS-svc.yaml
protocol: TCP
- name: metrics
port: 9153
protocol: TCP
Copy link
Member

Choose a reason for hiding this comment

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

newline

@codecov-io
Copy link

codecov-io commented Sep 21, 2017 •
edited
Loading

Codecov Report

Merging #1995 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #1995   +/-   ##
=======================================
  Coverage   29.87%   29.87%           
=======================================
  Files          77       77           
  Lines        4760     4760           
=======================================
  Hits         1422     1422           
  Misses       3158     3158           
  Partials      180      180
Impacted Files Coverage Δ
cmd/minikube/cmd/config/config.go 39.28% <ø> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 685f570...98b7ec6. Read the comment docs.

@r2d4
Copy link
Contributor

r2d4 commented Sep 21, 2017

@minikube-bot ok to test

r2d4
r2d4 approved these changes Oct 9, 2017
View reviewed changes
Copy link
Contributor

@r2d4 r2d4 left a comment

Choose a reason for hiding this comment

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

Thanks! Sorry for the delay in reviewing.

@r2d4 r2d4 merged commit b4de04f into kubernetes:master Oct 9, 2017
@johnbelamaric johnbelamaric mentioned this pull request Oct 27, 2017
Closed
@luxas
Copy link
Member

luxas commented Nov 9, 2017

@rajansandeep Can you now send a new PR to align these manifests with what got merged in kubeadm?
There weren't many changes, but many enough that it's worthwhile to be consistent

@rajansandeep
Copy link
Contributor Author

@luxas Made the changes here. #2179

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@johnbelamaric johnbelamaric johnbelamaric requested changes

@r2d4 r2d4 r2d4 approved these changes

Assignees
No one assigned
Labels
cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@rajansandeep @minikube-bot @johnbelamaric @codecov-io @r2d4 @luxas @k8s-ci-robot