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

Use dnsutils image for DNS debugging #18001

Merged
merged 1 commit into from
Jan 15, 2020

Conversation

aojea
Copy link
Member

@aojea aojea commented Dec 8, 2019

DNS debugging documentation is using a busybox image as example.
This image has different known issues with the way it resolves
the DNS names in k8s that create confusion with users following the
docs.

The e2e tests use a custom small image ~4.3MB for DNS testing
with all the tools necessary. Also, the fact that this image is
being used for the k8s e2e testing guarantees it's compatibility.

@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Dec 8, 2019
@aojea
Copy link
Member Author

aojea commented Dec 8, 2019

/assign @BenTheElder @bowei

@k8s-ci-robot k8s-ci-robot requested review from bowei and tengqm December 8, 2019 22:32
@k8s-ci-robot k8s-ci-robot added language/en Issues or PRs related to English language sig/docs Categorizes an issue or PR as relevant to SIG Docs. labels Dec 8, 2019
@aojea
Copy link
Member Author

aojea commented Dec 8, 2019

/assign @zparnold

@k8s-ci-robot k8s-ci-robot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Dec 8, 2019
@netlify
Copy link

netlify bot commented Dec 8, 2019

Deploy preview for kubernetes-io-master-staging ready!

Built with commit 9831193

https://deploy-preview-18001--kubernetes-io-master-staging.netlify.com

@netlify
Copy link

netlify bot commented Dec 8, 2019

Deploy preview for kubernetes-io-master-staging ready!

Built with commit 6debada

https://deploy-preview-18001--kubernetes-io-master-staging.netlify.com

@tengqm
Copy link
Contributor

tengqm commented Dec 9, 2019

Not sure it is a good thing to test a feature using an image customized for it. If the DNS resolving feature works well, why we cannot use busybox or other images?

@aojea
Copy link
Member Author

aojea commented Dec 9, 2019

@tengqm this is the context of this issue https://kubernetes.slack.com/archives/C09QYUH5W/p1575667447086200

As you can see in current documentation the busybox version has to be pinned to 1.28 because later versions don't work with kubernetes docker-library/busybox#48

Also, you can see that there are already some known issues https://kubernetes.io/docs/tasks/administer-cluster/dns-debugging-resolution/#known-issues , unfortunately, DNS is a complex topic with many moving parts.

Having an image that is being tested daily guarantees that the docs will be up to date, using a generic image it may break in the future without anybody noticing.

@BenTheElder
Copy link
Member

Not sure it is a good thing to test a feature using an image customized for it. If the DNS resolving feature works well, why we cannot use busybox or other images?

the set of tools is the only thing customized, not any details about how it behaves. most common images lack DNS utilities.

the busybox tools are not a great choice, they are known to behave badly on kubernetes. we are observing the search paths not being respected.

this could certainly happen with other real tools, but they should respect this.

@BenTheElder
Copy link
Member

/lgtm
thanks!

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Dec 10, 2019
@tengqm
Copy link
Contributor

tengqm commented Dec 10, 2019

we are observing the search paths not being respected.

Okay. That is a convincing.

/lgtm

Copy link
Contributor

@sftim sftim left a comment

Choose a reason for hiding this comment

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

@aojea Can I suggest leaving the existing example in place? It might be linked to from elsewhere.

After a while it might be OK to tidy up. If you agree about leaving it in place, how about logging an instantly-frozen issue to track that future work?

@aojea
Copy link
Member Author

aojea commented Dec 17, 2019

/hold

  • keep old example

  • log new issue to track future clean up

@k8s-ci-robot k8s-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Dec 17, 2019
@bowei
Copy link
Member

bowei commented Dec 17, 2019

/lgtm
/approve

DNS debugging documentation is using a busybox image as example.
This image has different known issues with the way it resolves
the DNS names in k8s, creating confusion with users that try to
follow the docs.

The e2e tests use a custom small image ~4.3MB for DNS testing
with all the necessary tools. Also, the fact that this image is
being used for the k8s e2e testing guarantees it's compatibility.
@k8s-ci-robot k8s-ci-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed lgtm "Looks good to me", indicates that a PR is ready to be merged. size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Dec 19, 2019
@aojea
Copy link
Member Author

aojea commented Dec 19, 2019

/hold cancel
Please take a look

@k8s-ci-robot k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Dec 19, 2019
@aojea
Copy link
Member Author

aojea commented Dec 20, 2019

/assign @zparnold

@BenTheElder
Copy link
Member

gentle poke 🙃
we're seeing users encounter this semi-frequently.

@sftim
Copy link
Contributor

sftim commented Jan 15, 2020

/lgtm
/approve
Thanks @aojea

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jan 15, 2020
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: bowei, sftim

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jan 15, 2020
@k8s-ci-robot k8s-ci-robot merged commit d2dc2c6 into kubernetes:master Jan 15, 2020
wawa0210 pushed a commit to wawa0210/website that referenced this pull request Mar 2, 2020
DNS debugging documentation is using a busybox image as example.
This image has different known issues with the way it resolves
the DNS names in k8s, creating confusion with users that try to
follow the docs.

The e2e tests use a custom small image ~4.3MB for DNS testing
with all the necessary tools. Also, the fact that this image is
being used for the k8s e2e testing guarantees it's compatibility.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. language/en Issues or PRs related to English language lgtm "Looks good to me", indicates that a PR is ready to be merged. sig/docs Categorizes an issue or PR as relevant to SIG Docs. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants