Adds a generic CoreDNS chart #785
Conversation
k8s-ci-robot
added
the
cncf-cla: yes
|
lgtm from a CoreDNS viewpoint |
|
Thanks @miekg |
|
@shashidharatd, very happy to proceed with this PR. I'm caught up with a few other projects so haven't been able to spend much time on the chart. |
shashidharatd
force-pushed
the
coredns
branch
2 times, most recently
from
45e2b4d
to
efd68ac
Compare
|
cc: @michelleN |
| annotations: | ||
| {{- if .Values.isClusterService }} | ||
| scheduler.alpha.kubernetes.io/critical-pod: '' | ||
| scheduler.alpha.kubernetes.io/tolerations: '[{"key":"CriticalAddonsOnly", "operator":"Exists"}]' |
There was a problem hiding this comment.
@prydonius @lachie83 should use of alpha schedulers warrant this chart belonging in incubator/?
There was a problem hiding this comment.
@seanknox, thanks for the review. This alpha scheduler API is used by almost all the addons in kubernetes including the kube-dns. I do not think the alpha tagging, affect the stability of that API and moreover its used in annotation. All said i am OK to move this chart to incubator if warrants.
There was a problem hiding this comment.
This only applies to alpha APIs, not alpha annotations
|
@shashidharatd I have one outstanding question around the use of the alpha scheduler APIs, but otherwise LGTM, thanks! |
|
Hi @prydonius @lachie83, can you PTAL |
stable/coredns/templates/NOTES.txt
Outdated
| echo $SERVICE_IP | ||
| {{- else if contains "ClusterIP" .Values.serviceType }} | ||
| echo "{{ template "fullname" . }}.{{ .Release.Namespace }}.svc.cluster.local" | ||
| echo "from within the cluster" |
There was a problem hiding this comment.
For the ClusterIP case this just prints information instead of commands to run, so remove the prepended echo ?
There was a problem hiding this comment.
To clarify, this is what I see with ClusterIP in the notes output:
CoreDNS is now running in the cluster.
It can be accessed using the below endpoint
echo "ns-coredns.default.svc.cluster.local"
echo "from within the cluster"
There was a problem hiding this comment.
Thanks @bzub for the review. I have handled your comments now. PTAL.
|
Thanks @lachie83 |
* CoreDNS Adds a basic CoreDNS chart based off the project deployment documentation * Made CoreDNS chart generic to deploy in multiple scenarios * Address review comments for CoreDNS chart
This pr builds upon the initial pr for CoreDNS chart #758 done by @hunter,
I have included the commit in that pr and extended the chart to be deployable for multiple scenarios.
The original pr was handling only the scenario of replacing kube-dns by CoreDNS
I have extended now to deploy CoreDNS as a normal k8s app offering DNS service, which can be used from within cluster as well as outside the cluster like external dns service.
Request all members cc'ed below for a review.
cc @hunter @johnbelamaric @miekg
@hunter, if you agree we can push this pr instead of the original one as this one already includes your commit. we can iterate over this pr and push for merge asap.