-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
feat: SRV record on headless service #4055
base: master
Are you sure you want to change the base?
feat: SRV record on headless service #4055
Conversation
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Welcome @theloneexplorerquest! |
Hi @theloneexplorerquest. Thanks for your PR. I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. Instructions 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. |
happy to update PR for test/doc/conflict once the form of SRV record in headless service has been confirmed. |
@theloneexplorerquest For SRV record, there is a PR aiming to change it, see #4001. It's also using port name for the SRV record. Correct me if I'm wrong, but it seems this kind of SRV record is already created by Kubernetes. The SRV record format you suggest is interesting mainly inside a k8s cluster. External-DNS is about creating DNS records outside the k8s cluster. Would you please detail your use case ? |
I could be wrong here, disclaim I am not using this feature. Just wanna to contribute the project to learning propose, will take a look! |
I opened the initial request for this in #3993. The request is different to #4001 - that PR changes the behaviour of The first service I'd like to use this for is
I think this could also be useful for |
@mloiseleur do you think above use case is justified? Happy to progress further on this PR 😃 |
Looking again at kubernetes doc on headless service, it does not says anything about SRV records. At the end, TBH, I do not have a strong opinion on this matter. As long as it can be useful for some use case and it's not breaking or overcomplexify source code, why not ? |
My use case is far less technical. I just want to create SRV records for minecraft servers I'm hosting on my k8s cluster.
|
My use case is to use a Rook Ceph cluster to look up the monitor IP addresses via SRV DNS entries. |
/ok-to-test |
@theloneexplorerquest Do you think you can rebase this PR ? |
@theloneexplorerquest test and documentation are needed on this PR |
/retitle feat: SRV record on headless service |
PR needs rebase. Instructions 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. |
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 |
@@ -1154,6 +1154,8 @@ github.com/vinyldns/go-vinyldns v0.9.16 h1:GZJStDkcCk1F1AcRc64LuuMh+ENL8pHA0CVd4 | |||
github.com/vinyldns/go-vinyldns v0.9.16/go.mod h1:5qIJOdmzAnatKjurI+Tl4uTus7GJKJxb+zitufjHs3Q= | |||
github.com/vultr/govultr/v2 v2.17.2 h1:gej/rwr91Puc/tgh+j33p/BLR16UrIPnSr+AIwYWZQs= | |||
github.com/vultr/govultr/v2 v2.17.2/go.mod h1:ZFOKGWmgjytfyjeyAdhQlSWwTjh2ig+X49cAp50dzXI= | |||
github.com/wadey/gocovmerge v0.0.0-20160331181800-b5bfa59ec0ad h1:W0LEBv82YCGEtcmPA3uNZBI33/qF//HAAs3MawDjRa0= | |||
github.com/wadey/gocovmerge v0.0.0-20160331181800-b5bfa59ec0ad/go.mod h1:Hy8o65+MXnS6EwGElrSRjUzQDLXreJlzYLlWiHtt8hM= |
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 should be not added
@@ -295,12 +294,35 @@ func (sc *serviceSource) extractHeadlessEndpoints(svc *v1.Service, hostname stri | |||
log.Errorf("Pod %s not found for address %v", address.TargetRef.Name, address) | |||
continue | |||
} | |||
for _, container := range pod.Spec.Containers { |
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.
we would need to guard to enable if you need it. We can not create more than 50k records in a bigger cluster.
I don't mind it but:
|
Hi @szuecs thanks for review. I will update this PR after when I am free |
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 |
Description
Support creating SRV records in headless services (ClusterIP).
SRV record form:
_container-port-name._port-protocol.my-svc.my-namespace.svc.cluster-domain.example
target form:
pod-hostname.my-svc.my-namespace.svc.cluster-domain.example
SRV record:
_container-port-name._port-protocol.my-svc.my-namespace.svc.cluster-domain.example 0 IN SRV 0 50 container-port pod-hostname.my-svc.my-namespace.svc.cluster-domain.example
This is referenced from https://kubernetes.io/docs/concepts/services-networking/dns-pod-service/#srv-records. However, I have notice it does not quite align with the convention of external-dns. Can anyone tell me the properly form of SRV record in the context of external-dns?
I have identified that the test needs to be updated, happy to make update test after someone take a quick look at my PR.
An example:
namespace: default
service name: cassandra
hostname (annotation): example.org
pod1: cassandra-0
container port/name/protocol:
7000, intra-node, TCP
,7001,tls-intra-node, TCP
,7199, jmx, TCP
,9042, cql, TCP
pod2: cassandra-1
container port/name/protocol:
7000, intra-node, TCP
,7001,tls-intra-node, TCP
,7199, jmx, TCP
,9042, cql, TCP
SRV records created:
Fixes #3993
Checklist