You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@apisix.apache.org by GitBox <gi...@apache.org> on 2021/02/26 08:30:00 UTC
[GitHub] [apisix] spacewander opened a new pull request #3686: feat: support SRV record
spacewander opened a new pull request #3686:
URL: https://github.com/apache/apisix/pull/3686
Signed-off-by: spacewander <sp...@gmail.com>
### What this PR does / why we need it:
<!--- Why is this change required? What problem does it solve? -->
<!--- If it fixes an open issue, please link to the issue here. -->
### Pre-submission checklist:
* [x] Did you explain what problem does this PR solve? Or what new features have been added?
* [x] Have you added corresponding test cases?
* [x] Have you modified the corresponding document?
* [ ] Is this PR backward compatible? **If it is not backward compatible, please discuss on the [mailing list](https://github.com/apache/apisix/tree/master#community) first**
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] spacewander commented on a change in pull request #3686: feat: support SRV record
Posted by GitBox <gi...@apache.org>.
spacewander commented on a change in pull request #3686:
URL: https://github.com/apache/apisix/pull/3686#discussion_r584029064
##########
File path: apisix/core/dns/client.lua
##########
@@ -29,6 +31,67 @@ local _M = {
}
+local function gcd(a, b)
+ if b == 0 then
+ return a
+ end
+
+ return gcd(b, a % b)
+end
+
+
+local function resolve_srv(client, answers)
+ if #answers == 0 then
+ return nil, "empty SRV record"
+ end
+
+ local resolved_answers = {}
+ local answer_to_count = {}
+ for _, answer in ipairs(answers) do
+ if answer.type ~= client.TYPE_SRV then
+ return nil, "mess SRV with other record"
+ end
+
+ local resolved, err = client.resolve(answer.target)
+ if not resolved then
+ local msg = "failed to resolve SRV record " .. answer.target .. ": " .. err
+ return nil, msg
+ end
+
+ log.info("dns resolve SRV ", answer.target, ", result: ",
+ json.delay_encode(resolved))
+
+ local weight = answer.weight
Review comment:
Added: https://github.com/apache/apisix/pull/3686/files#diff-eeebb3321eff56ec1f7b883a6a08b29bbc6359cdd06407eca329772b9a619676R122
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] spacewander commented on a change in pull request #3686: feat: support SRV record
Posted by GitBox <gi...@apache.org>.
spacewander commented on a change in pull request #3686:
URL: https://github.com/apache/apisix/pull/3686#discussion_r584029441
##########
File path: apisix/core/dns/client.lua
##########
@@ -29,6 +31,67 @@ local _M = {
}
+local function gcd(a, b)
+ if b == 0 then
+ return a
+ end
+
+ return gcd(b, a % b)
+end
+
+
+local function resolve_srv(client, answers)
+ if #answers == 0 then
+ return nil, "empty SRV record"
+ end
+
+ local resolved_answers = {}
+ local answer_to_count = {}
+ for _, answer in ipairs(answers) do
+ if answer.type ~= client.TYPE_SRV then
+ return nil, "mess SRV with other record"
+ end
+
+ local resolved, err = client.resolve(answer.target)
+ if not resolved then
+ local msg = "failed to resolve SRV record " .. answer.target .. ": " .. err
+ return nil, msg
+ end
+
+ log.info("dns resolve SRV ", answer.target, ", result: ",
+ json.delay_encode(resolved))
+
+ local weight = answer.weight
+ if weight == 0 then
+ weight = 1
+ end
+
+ local count = #resolved
+ answer_to_count[answer] = count
+ -- one target may have multiple resolved results
+ for _, res in ipairs(resolved) do
+ local copy = table.deepcopy(res)
+ copy.weight = weight / count
+ copy.port = answer.port
+ insert_tab(resolved_answers, copy)
+ end
+ end
+
+ -- find the least common multiple of the counts
+ local lcm = answer_to_count[answers[1]]
+ for i = 1, #answers do
Review comment:
Solved
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] spacewander commented on a change in pull request #3686: feat: support SRV record
Posted by GitBox <gi...@apache.org>.
spacewander commented on a change in pull request #3686:
URL: https://github.com/apache/apisix/pull/3686#discussion_r584123760
##########
File path: docs/en/latest/dns.md
##########
@@ -74,3 +74,60 @@ If you want to specify the port for the upstream server, you can add it to the `
"type": "roundrobin"
}
```
+
+Another way to do it is via the SRV record, see below.
+
+### SRV record
+
+By using SRV record you can specify the port and the weight of a service.
+
+Assumed you have the SRV record like this:
+
+```
+; under the section of blah.service
+A 300 IN A 1.1.1.1
+B 300 IN A 1.1.1.2
+B 300 IN A 1.1.1.3
+srv 86400 IN SRV 10 60 1980 A
+srv 86400 IN SRV 10 20 1981 B
+```
+
+Upstream configuration like:
+
+```json
+{
+ "id": 1,
+ "discovery_type": "dns",
+ "service_name": "srv.blah.service",
+ "type": "roundrobin"
+}
+```
+
+is the same as:
+
+```json
+{
+ "id": 1,
+ "type": "roundrobin",
+ "nodes": {
Review comment:
@membphis
Solved.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] spacewander merged pull request #3686: feat: support SRV record
Posted by GitBox <gi...@apache.org>.
spacewander merged pull request #3686:
URL: https://github.com/apache/apisix/pull/3686
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] tokers commented on a change in pull request #3686: feat: support SRV record
Posted by GitBox <gi...@apache.org>.
tokers commented on a change in pull request #3686:
URL: https://github.com/apache/apisix/pull/3686#discussion_r584012410
##########
File path: apisix/core/dns/client.lua
##########
@@ -29,6 +31,67 @@ local _M = {
}
+local function gcd(a, b)
+ if b == 0 then
+ return a
+ end
+
+ return gcd(b, a % b)
+end
+
+
+local function resolve_srv(client, answers)
+ if #answers == 0 then
+ return nil, "empty SRV record"
+ end
+
+ local resolved_answers = {}
+ local answer_to_count = {}
+ for _, answer in ipairs(answers) do
+ if answer.type ~= client.TYPE_SRV then
+ return nil, "mess SRV with other record"
+ end
+
+ local resolved, err = client.resolve(answer.target)
+ if not resolved then
+ local msg = "failed to resolve SRV record " .. answer.target .. ": " .. err
+ return nil, msg
+ end
+
+ log.info("dns resolve SRV ", answer.target, ", result: ",
+ json.delay_encode(resolved))
+
+ local weight = answer.weight
+ if weight == 0 then
+ weight = 1
+ end
+
+ local count = #resolved
+ answer_to_count[answer] = count
+ -- one target may have multiple resolved results
+ for _, res in ipairs(resolved) do
+ local copy = table.deepcopy(res)
+ copy.weight = weight / count
+ copy.port = answer.port
+ insert_tab(resolved_answers, copy)
+ end
+ end
+
+ -- find the least common multiple of the counts
+ local lcm = answer_to_count[answers[1]]
+ for i = 1, #answers do
Review comment:
We can iterate from `i = 2`.
##########
File path: apisix/core/dns/client.lua
##########
@@ -29,6 +31,67 @@ local _M = {
}
+local function gcd(a, b)
+ if b == 0 then
+ return a
+ end
+
+ return gcd(b, a % b)
+end
+
+
+local function resolve_srv(client, answers)
+ if #answers == 0 then
+ return nil, "empty SRV record"
+ end
+
+ local resolved_answers = {}
+ local answer_to_count = {}
+ for _, answer in ipairs(answers) do
+ if answer.type ~= client.TYPE_SRV then
+ return nil, "mess SRV with other record"
+ end
+
+ local resolved, err = client.resolve(answer.target)
+ if not resolved then
+ local msg = "failed to resolve SRV record " .. answer.target .. ": " .. err
+ return nil, msg
+ end
+
+ log.info("dns resolve SRV ", answer.target, ", result: ",
+ json.delay_encode(resolved))
+
+ local weight = answer.weight
Review comment:
I think the explantation to treat zero weight record to `1` https://github.com/apache/apisix/blob/3af2175629f727d0374b7f77dbdc3a510cf44d81/docs/en/latest/dns.md#srv-record, can also add here.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] membphis commented on a change in pull request #3686: feat: support SRV record
Posted by GitBox <gi...@apache.org>.
membphis commented on a change in pull request #3686:
URL: https://github.com/apache/apisix/pull/3686#discussion_r584113078
##########
File path: docs/en/latest/dns.md
##########
@@ -74,3 +74,60 @@ If you want to specify the port for the upstream server, you can add it to the `
"type": "roundrobin"
}
```
+
+Another way to do it is via the SRV record, see below.
+
+### SRV record
+
+By using SRV record you can specify the port and the weight of a service.
+
+Assumed you have the SRV record like this:
+
+```
+; under the section of blah.service
+A 300 IN A 1.1.1.1
+B 300 IN A 1.1.1.2
+B 300 IN A 1.1.1.3
+srv 86400 IN SRV 10 60 1980 A
+srv 86400 IN SRV 10 20 1981 B
+```
+
+Upstream configuration like:
+
+```json
+{
+ "id": 1,
+ "discovery_type": "dns",
+ "service_name": "srv.blah.service",
+ "type": "roundrobin"
+}
+```
+
+is the same as:
+
+```json
+{
+ "id": 1,
+ "type": "roundrobin",
+ "nodes": {
Review comment:
should use `[` here, it an array object here.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org