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

Fix dns service SRV lookup when service address is a fqdn #6792

Merged
merged 1 commit into from
Dec 3, 2019

Conversation

tionebsalocin
Copy link
Contributor

Refactor dns to have same behavior between A and SRV.
Current implementation returns the node name instead of the service
address.
With this fix when querying for SRV record service address is return in
the SRV record.
And when performing a simple dns lookup it returns a CNAME to the
service address.

(For context history: #6252 and #6283)

@hashicorp-cla
Copy link

hashicorp-cla commented Nov 14, 2019

CLA assistant check
All committers have signed the CLA.

Refactor dns to have same behavior between A and SRV.
Current implementation returns the node name instead of the service
address.
With this fix when querying for SRV record service address is return in
the SRV record.
And when performing a simple dns lookup it returns a CNAME to the
service address.
@tionebsalocin
Copy link
Contributor Author

Note: Contributor License has already been signed. I don't know why it is not validated here

@crhino
Copy link
Contributor

crhino commented Nov 22, 2019

@tionebsalocin Is the email you used to author this commit (n.benoit@criteo.com) associated to your Github account? It looks like Github is unable to make that link right now.

@tionebsalocin
Copy link
Contributor Author

@tionebsalocin Is the email you used to author this commit (n.benoit@criteo.com) associated to your Github account? It looks like Github is unable to make that link right now.

Hello @crhino, I've added this email to my account but still enable to submit the form (all field are pre-filled and not editable). Any suggestion on what I should do from here?

@banks
Copy link
Member

banks commented Dec 3, 2019

@tionebsalocin I kicked the CLA check again and it's happy with you this time! I think @mkeeler was going to look over this once more, thanks for your effort here!

@banks banks added this to the 1.7.0 milestone Dec 3, 2019
@mkeeler
Copy link
Member

mkeeler commented Dec 3, 2019

Yep, looking now.

Copy link
Member

@mkeeler mkeeler left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks for all your work on this.

@mkeeler mkeeler merged commit be8fd29 into hashicorp:master Dec 3, 2019
@ghost
Copy link

ghost commented Jan 25, 2020

Hey there,

This issue has been automatically locked because it is closed and there hasn't been any activity for at least 30 days.

If you are still experiencing problems, or still have questions, feel free to open a new one 👍.

@ghost ghost locked and limited conversation to collaborators Jan 25, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants