Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

feat: add dns discovery #3629

Merged
merged 1 commit into from
Feb 25, 2021
Merged

Conversation

spacewander
Copy link
Member

Fix #3517
Signed-off-by: spacewander spacewanderlzx@gmail.com

What this PR does / why we need it:

Pre-submission checklist:

  • Did you explain what problem does this PR solve? Or what new features have been added?
  • Have you added corresponding test cases?
  • Have you modified the corresponding document?
  • Is this PR backward compatible? If it is not backward compatible, please discuss on the mailing list first

@spacewander spacewander force-pushed the dns_discovery branch 5 times, most recently from 1b6f00d to b3c6ecf Compare February 22, 2021 11:32
Fix apache#3517
Signed-off-by: spacewander <spacewanderlzx@gmail.com>
@spacewander spacewander marked this pull request as ready for review February 22, 2021 12:51
Copy link
Member

@membphis membphis left a comment

Choose a reason for hiding this comment

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

LGTM, except minor style things


local new_nodes, err = dis.nodes(up_conf.service_name)
if not new_nodes then
return http_code_upstream_unavailable, "no valid upstream node: " .. (err or "nil")
Copy link
Member

Choose a reason for hiding this comment

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

http_code_upstream_unavailable:

For static variables, use capital letters

Copy link
Member Author

Choose a reason for hiding this comment

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

@membphis
I have submitted another PR to use the number literal directly: #3672, which is more consistent in the style.

Copy link
Member Author

Choose a reason for hiding this comment

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

I changed my idea and followed your suggestion.

@spacewander spacewander merged commit 2a6e2b4 into apache:master Feb 25, 2021
@spacewander spacewander deleted the dns_discovery branch February 25, 2021 01:42
spacewander added a commit to spacewander/incubator-apisix that referenced this pull request Feb 25, 2021
See apache#3629 (comment)

Signed-off-by: spacewander <spacewanderlzx@gmail.com>
@spacewander spacewander mentioned this pull request Feb 25, 2021
4 tasks
spacewander added a commit to spacewander/incubator-apisix that referenced this pull request Feb 25, 2021
See apache#3629 (comment)

Signed-off-by: spacewander <spacewanderlzx@gmail.com>
membphis pushed a commit that referenced this pull request Feb 27, 2021
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[DISCUSS] add per node DNS selector
3 participants