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.

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

Split dns healthcheck into two different urls #32406

Merged
merged 1 commit into from Sep 25, 2016

Conversation

MrHohn
Copy link
Member

@MrHohn MrHohn commented Sep 9, 2016

Attempt to fix #30633.

This new kube-dns pod template creates two exechealthz processes listen on two different ports for kubedns and dnsmasq correspondingly.

@thockin @girishkalele

Split dns healthcheck into two different urls

This change is Reviewable

@k8s-github-robot k8s-github-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. release-note-label-needed labels Sep 9, 2016
@k8s-bot
Copy link

k8s-bot commented Sep 9, 2016

GCE e2e build/test passed for commit 952bcf6.

@k8s-github-robot k8s-github-robot added needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. and removed needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Sep 11, 2016
- -port=8080
- -quiet
command: ["/bin/sh","-c"]
args: ["/exechealthz -cmd='nslookup kubernetes.default.svc.__PILLAR__DNS__DOMAIN__ 127.0.0.1 >/dev/null' -port=8082 -quiet & /exechealthz -cmd='nslookup kubernetes.default.svc.__PILLAR__DNS__DOMAIN__ 127.0.0.1:10053 >/dev/null' -port=8080 -quiet"]
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 not do this - if one of them dies it will not get restarted

We should either run two whole containers or make one container able to handle both (as you suggested)

@thockin
Copy link
Member

thockin commented Sep 22, 2016

Now that the exec-healthz is in, ping me when this is ready

@MrHohn
Copy link
Member Author

MrHohn commented Sep 22, 2016

Thanks for get that in. Sure, no problem.

@k8s-github-robot k8s-github-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Sep 22, 2016
@MrHohn MrHohn changed the title Split dns healthcheck into two different ports Split dns healthcheck into two different urls Sep 23, 2016
@k8s-github-robot k8s-github-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Sep 23, 2016
@MrHohn
Copy link
Member Author

MrHohn commented Sep 23, 2016

This new commit now using the latest version exec-healthz to split the health check into two separate urls.

Also all related dns yaml files are kept in sync.

kubernetes.io/cluster-service: "true" label is removed for non-top level addon-managed resources according to this potential usage.

@MrHohn
Copy link
Member Author

MrHohn commented Sep 23, 2016

@thockin

@thockin thockin added lgtm "Looks good to me", indicates that a PR is ready to be merged. release-note-none Denotes a PR that doesn't merit a release note. and removed release-note-label-needed labels Sep 23, 2016
@thockin
Copy link
Member

thockin commented Sep 23, 2016

LGTM

@k8s-bot
Copy link

k8s-bot commented Sep 23, 2016

Build finished. 691 tests run, 200 skipped, 0 failed.

@foxish
Copy link
Contributor

foxish commented Sep 23, 2016

@k8s-bot test this please, issue #IGNORE

@foxish foxish added this to the v1.5 milestone Sep 23, 2016
@k8s-ci-robot
Copy link
Contributor

Jenkins verification failed for commit d17cd1a. Full PR test history.

The magic incantation to run this job again is @k8s-bot verify test this. Please help us cut down flakes by linking to an open flake issue when you hit one in your PR.

@MrHohn
Copy link
Member Author

MrHohn commented Sep 25, 2016

@k8s-bot test this please, issue #33357

@k8s-github-robot
Copy link

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

@k8s-ci-robot
Copy link
Contributor

Jenkins GKE smoke e2e failed for commit d17cd1a. Full PR test history.

The magic incantation to run this job again is @k8s-bot gke e2e test this. Please help us cut down flakes by linking to an open flake issue when you hit one in your PR.

@k8s-github-robot
Copy link

Automatic merge from submit-queue

@k8s-github-robot k8s-github-robot merged commit b79c99d into kubernetes:master Sep 25, 2016
euank added a commit to euank/kubernetes that referenced this pull request Sep 29, 2016
This bug was inadvertently introduced in kubernetes#32406.

The longer term plan (shouldn't be too much longer) is to remove this
file entirely and rely on the `gci-trusty` version of it, but to stop
some bleeding and allow our jenkins using kube-up + coreos to work, we
should merge this fix until we have the more complete solution.
k8s-github-robot pushed a commit that referenced this pull request Sep 30, 2016
Automatic merge from submit-queue

gce/coreos: Fix dnsmasq image name

This bug was inadvertently introduced in #32406.

The longer term plan (shouldn't be too much longer) is to remove this
file entirely and rely on the `gci-trusty` version of it, but to stop
some bleeding and allow our jenkins using kube-up + coreos to work, we
should merge this fix until we have the more complete solution.

cc @MrHohn @yifan-gu @thockin
MrHohn pushed a commit to MrHohn/kubernetes that referenced this pull request Oct 7, 2016
This bug was inadvertently introduced in kubernetes#32406.

The longer term plan (shouldn't be too much longer) is to remove this
file entirely and rely on the `gci-trusty` version of it, but to stop
some bleeding and allow our jenkins using kube-up + coreos to work, we
should merge this fix until we have the more complete solution.
k8s-github-robot pushed a commit that referenced this pull request Oct 7, 2016
…32422-#32406-#33146-#33774-upstream-release-1.4

Automatic merge from submit-queue

Automated cherry pick of #31894 #32422 #32406 #33146 #33774

Cherry pick of #31894 #32422 #32406 #33146 #33774 on release-1.4.

#31894: Support graceful termination in kube-dns
#32422: Added --log-facility flag to enhance dnsmasq logging
#32406: Split dns healthcheck into two different urls
#33146: Tune down initialDelaySeconds for readinessProbe
#33774: Bump up addon kube-dns to v20 for graceful termination
@jessfraz
Copy link
Contributor

jessfraz commented Oct 7, 2016

cherry-picked to 1.4 in #34290 removing cherry-pick candidate

@jessfraz jessfraz added release-note Denotes a PR that will be considered when it comes time to generate release notes. and removed cherrypick-candidate release-note-none Denotes a PR that doesn't merit a release note. labels Oct 7, 2016
shyamjvs pushed a commit to shyamjvs/kubernetes that referenced this pull request Dec 1, 2016
…-of-#31894-kubernetes#32422-kubernetes#32406-kubernetes#33146-kubernetes#33774-upstream-release-1.4

Automatic merge from submit-queue

Automated cherry pick of kubernetes#31894 kubernetes#32422 kubernetes#32406 kubernetes#33146 kubernetes#33774

Cherry pick of kubernetes#31894 kubernetes#32422 kubernetes#32406 kubernetes#33146 kubernetes#33774 on release-1.4.

kubernetes#31894: Support graceful termination in kube-dns
kubernetes#32422: Added --log-facility flag to enhance dnsmasq logging
kubernetes#32406: Split dns healthcheck into two different urls
kubernetes#33146: Tune down initialDelaySeconds for readinessProbe
kubernetes#33774: Bump up addon kube-dns to v20 for graceful termination
@MrHohn MrHohn deleted the kubedns-healthz branch May 16, 2017 23:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lgtm "Looks good to me", 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/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

DNS healthcheck kills kube-dns if dnsmasq is hung
9 participants