-
Notifications
You must be signed in to change notification settings - Fork 1.5k
KEP-4762: Allows setting any FQDN as the pod's hostname #4768
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
KEP-4762: Allows setting any FQDN as the pod's hostname #4768
Conversation
HirazawaUi
commented
Jul 18, 2024
- One-line PR description: Allows users to set any FQDN as the hostname of a pod
- Issue link: Allows setting any FQDN as the pod's hostname #4762
- Other comments:
keps/sig-network/4762-allow-arbitrary-fqdn-as-pod-hostname/README.md
Outdated
Show resolved
Hide resolved
615fb45
to
e219291
Compare
keps/sig-network/4762-allow-arbitrary-fqdn-as-pod-hostname/README.md
Outdated
Show resolved
Hide resolved
8728dcf
to
d935541
Compare
/cc @thockin |
keps/sig-network/4762-allow-arbitrary-fqdn-as-pod-hostname/README.md
Outdated
Show resolved
Hide resolved
9daccf9
to
18d6ffd
Compare
12dc314
to
f2c3e30
Compare
The Kubernetes project currently lacks enough contributors to adequately respond to all PRs. This bot triages PRs according to the following rules:
You can:
Please send feedback to sig-contributor-experience at kubernetes/community. /lifecycle stale |
/remove-lifecycle stale |
ping @thockin |
The Kubernetes project currently lacks enough contributors to adequately respond to all PRs. This bot triages PRs according to the following rules:
You can:
Please send feedback to sig-contributor-experience at kubernetes/community. /lifecycle stale |
2d4d3e4
to
e8e81d6
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM from API
/approve
| Behavior | result for `hostname` | result for `hostname -f` | DNS Record | | ||
| ------------------------------------------------------------ | ---------------------------------------------------- | ---------------------------------------------------- | ---------------------------------------------------- | | ||
| hostname: busybox1 | busybox1 | busybox1 | None | | ||
| hostname: busybox1<br />subdomain: busybox-subdomain | busybox1 | busybox1.busybox-subdomain.default.svc.cluster.local | busybox1.busybox-subdomain.default.svc.cluster.local | | ||
| hostname: busybox1<br />subdomain: busybox-subdomain<br />setHostnameAsFQDN: true | busybox1.busybox-subdomain.default.svc.cluster.local | busybox1.busybox-subdomain.default.svc.cluster.local | busybox1.busybox-subdomain.default.svc.cluster.local | | ||
| hostnameOverride: www.example.com | www.example.com | www.example.com | None | | ||
| hostname: busybox1<br />hostnameOverride: www.example.com | www.example.com | www.example.com | None | | ||
| hostname: busybox1<br />hostnameOverride: www.example.com<br />subdomain: busybox-subdomain | www.example.com | www.example.com | busybox1.busybox-subdomain.default.svc.cluster.local | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| Behavior | result for `hostname` | result for `hostname -f` | DNS Record | | |
| ------------------------------------------------------------ | ---------------------------------------------------- | ---------------------------------------------------- | ---------------------------------------------------- | | |
| hostname: busybox1 | busybox1 | busybox1 | None | | |
| hostname: busybox1<br />subdomain: busybox-subdomain | busybox1 | busybox1.busybox-subdomain.default.svc.cluster.local | busybox1.busybox-subdomain.default.svc.cluster.local | | |
| hostname: busybox1<br />subdomain: busybox-subdomain<br />setHostnameAsFQDN: true | busybox1.busybox-subdomain.default.svc.cluster.local | busybox1.busybox-subdomain.default.svc.cluster.local | busybox1.busybox-subdomain.default.svc.cluster.local | | |
| hostnameOverride: www.example.com | www.example.com | www.example.com | None | | |
| hostname: busybox1<br />hostnameOverride: www.example.com | www.example.com | www.example.com | None | | |
| hostname: busybox1<br />hostnameOverride: www.example.com<br />subdomain: busybox-subdomain | www.example.com | www.example.com | busybox1.busybox-subdomain.default.svc.cluster.local | | |
| # | `.hostname` | `.subdomain` | `.setHostnameAsFQDN` | `.hostnameOverride` | `$(hostname)` | `$(hostname -f)` | DNS (assuming service exists) | | |
| -- | ----------- | ------------ | -------------------- | ------------------- | ------------------------------ | ------------------------------ | ------------------------------ | | |
| 0 | | | | | `<podname>` | `<podname>` | | | |
| 1 | `aa` | | | | `aa` | `aa` | | | |
| 2 | | `bb` | | | `<podname>` | `<podname>.bb.<ns>.svc.<zone>` | `<podname>.bb.<ns>.svc.<zone>` | | |
| 3 | `aa` | `bb` | | | `aa` | `aa.bb.<ns>.svc.<zone>` | `aa.bb.<ns>.svc.<zone>` | | |
| 4 | | | true | | `<podname>` | `<podname>` | | | |
| 5 | `aa` | | true | | `aa` | `aa` | | | |
| 6 | | `bb` | true | | `<podname>.bb.<ns>.svc.<zone>` | `<podname>.bb.<ns>.svc.<zone>` | `<podname>.bb.<ns>.svc.<zone>` | | |
| 7 | `aa` | `bb` | true | | `aa.bb.<ns>.svc.<zone>` | `aa.bb.<ns>.svc.<zone>` | `aa.bb.<ns>.svc.<zone>` | | |
| 8 | | | | `xx.yy.zz` | `xx.yy.zz` | `xx.yy.zz` | | | |
| 9 | `aa` | | | `xx.yy.zz` | `xx.yy.zz` | `xx.yy.zz` | | | |
| 10 | | `bb` | | `xx.yy.zz` | `xx.yy.zz` | `xx.yy.zz` | `<podname>.bb.<ns>.svc.<zone>` | | |
| 11 | `aa` | `bb` | | `xx.yy.zz` | `xx.yy.zz` | `xx.yy.zz` | `aa.bb.<ns>.svc.<zone>` | | |
| 12 | | | true | `xx.yy.zz` | INVALID | INVALID | INVALID | | |
| 13 | `aa` | | true | `xx.yy.zz` | INVALID | INVALID | INVALID | | |
| 14 | | `bb` | true | `xx.yy.zz` | INVALID | INVALID | INVALID | | |
| 15 | `aa` | `bb` | true | `xx.yy.zz` | INVALID | INVALID | INVALID | | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you very much for providing this table. I had promised to improve it over the weekend, but got caught up with other matters. Anyway, thank you very much.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I didn't want to lose track of it, and I think it should probably go into the k8s docs in some nicer form :)
d2e0df4
to
a06df4f
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm, except I suggest this be tested with the Conformance tests. Maybe add it also to graduation criteria
Co-authored-by: Tim Hockin <thockin@google.com>
a06df4f
to
28de557
Compare
Thanks! /lgtm |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Left several comments.
keps/sig-network/4762-allow-arbitrary-fqdn-as-pod-hostname/README.md
Outdated
Show resolved
Hide resolved
|
||
###### Were upgrade and rollback tested? Was the upgrade->downgrade->upgrade path tested? | ||
|
||
N/A |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is fine for alpha, but you'll need to perform manual tests for beta promotion.
keps/sig-network/4762-allow-arbitrary-fqdn-as-pod-hostname/README.md
Outdated
Show resolved
Hide resolved
keps/sig-network/4762-allow-arbitrary-fqdn-as-pod-hostname/README.md
Outdated
Show resolved
Hide resolved
keps/sig-network/4762-allow-arbitrary-fqdn-as-pod-hostname/kep.yaml
Outdated
Show resolved
Hide resolved
dcda75c
to
28e9e15
Compare
@soltysh Thank you so much for taking the time with this KEP! I've addressed all the PRR comments, could you please take another look? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/approve
the PRR
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: HirazawaUi, soltysh, thockin 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 |
Since this was already tagged by Sergey and Tim, I'll go ahead and re-apply it /lgtm |
Great to see this PR merged! Truly appreciate everyone's patient reviews and assistance. |