Trivial doc tidying.#2686
Conversation
I pulled the command line options out of the source and into this doc to save other people the trouble of digging into the source. Hope nobody minds!
|
Hi @rk295. Thanks for your PR. I'm waiting for a kubernetes member to verify that this patch is reasonable to test. If it is, they should reply with I understand the commands that are listed here. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here. |
|
Thanks for documenting those flags! I verified that no content is lost in the other parts, and they're all sensible changes. I would like to ask you to keep the line width <= 80 or <=72 in |
|
Thanks for the review @WillemMali, I've pushed some changes that tidies them up. I've kept |
|
Looks good to me! I again verified no content was misplaced. One Git-related suggestion for the future: you can force-push over the commit in your fork, this is correctly picked up by the CI system. If you end up getting conflicts you can rebase onto master, fix the conflicts there and then force-commit again. This way you always end up with a single commit for the PR. (Also, I love rebase workflows, I'm so glad I learned how to do that here.) It's really no problem at all, thanks for the PR! @chrislovecnm could you give this one the green light? |
|
@WillemMali wicked, thanks. I'll go read about force-pushing over the existing commit! |
|
@k8s-bot ok to test |
|
Need @justinsb or @sethpollack to review. They are far more familiar with the dns controller ;) |
| but for now we just expose it via DNS. | ||
|
|
||
| `dns.alpha.kubernetes.io/external` will set up records for accessing the resource externally | ||
| The dns-controller recognizes annotations on nodes. |
There was a problem hiding this comment.
I don't think it supports annotations on nodes. It just sets up internal ALIAS records.
There was a problem hiding this comment.
I'm unsure either tbh, I only slightly improved the formatting in this file, I can't vouch for its accuracy.
|
|
||
| `dns.alpha.kubernetes.io/external` creates a Route53 A record with `public` IPs of all the nodes | ||
| `dns.alpha.kubernetes.io/internal` creates a Route53 A record with `private` IPs of all the nodes | ||
| * `dns.alpha.kubernetes.io/external` creates a Route53 A record with |
There was a problem hiding this comment.
It creates CNAME's for hostnames and A records for IP addresses
There was a problem hiding this comment.
Happy to change the wording to reflect that.
| `public` IPs of all the nodes | ||
| * `dns.alpha.kubernetes.io/internal` creates a Route53 A record with | ||
| `private` IPs of all the nodes | ||
|
|
There was a problem hiding this comment.
You can also add the annotations to pods. It will create an A record for pods that are HostNetwork=true
There was a problem hiding this comment.
Ok, I'll re-word to explain that.
|
Generally LGTM - if you clean up some of the things @sethpollack has observed we can merge IMO! Thanks @rk295 |
|
What is the status on this? |
|
Going to merge & we can iterate, unless I hear an objection from @sethpollack |
|
Go for it. |
|
Thanks @rk295 and @sethpollack |
I pulled the command line options out of the source and into this doc to
save other people the trouble of digging into the source. Hope nobody
minds!
This change is