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 2022/09/18 14:59:22 UTC

[GitHub] [apisix] monkeyDluffy6017 opened a new pull request, #7935: feat: support configuring the order of the upstream dns service disco…

monkeyDluffy6017 opened a new pull request, #7935:
URL: https://github.com/apache/apisix/pull/7935

   …very resolve
   
   ### Description
   
   Some users are having problems with the upstream DNS service discovery feature, because the current DNS resolution order is fixed:  1. "last" 2. "SRV" 3. "A" 4. "AAAA"  5. "CNAME".
   "last" means the last successful resolving method; that is, if the last resolving is successful, the last resolving method will be used first.
   This causes the problem that if for some reason the A record is resolved successfully, the higher priority SRV record will not be resolved in the future, and the user expects to get the result of resolving the SRV record.
   
   As a user, I want to configure how and in what order the upstream DNS service discovery function resolves, and APISIX is able to resolve in the way and in the order I configure.
   
   ### Checklist
   
   - [x] I have explained the need for this PR and the problem it solves
   - [x] I have explained the changes or the new features added to this PR
   - [x] I have added tests corresponding to this change
   - [ ] I have updated the documentation to reflect this change
   - [x] I have verified that this change is backward compatible (If not, please discuss on the [APISIX 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.

To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [apisix] monkeyDluffy6017 commented on a diff in pull request #7935: feat: support configuring the order of the upstream dns service disco…

Posted by GitBox <gi...@apache.org>.
monkeyDluffy6017 commented on code in PR #7935:
URL: https://github.com/apache/apisix/pull/7935#discussion_r974177975


##########
t/discovery/dns/sanity.t:
##########
@@ -318,3 +318,24 @@ qr/upstream nodes: \{[^}]+\}/
 qr/upstream nodes: \{("127.0.0.1:1980":60,"127.0.0.2:1980":20|"127.0.0.2:1980":20,"127.0.0.1:1980":60)\}/
 --- response_body
 hello world
+
+
+
+=== TEST 16: prefer A than SRV when A is ahead of SRV in config.yaml
+--- apisix_yaml
+upstreams:
+    - service_name: "srv-a.test.local"
+      discovery_type: dns
+      type: roundrobin
+      id: 1
+discovery:
+    dns:
+        servers:
+            - "127.0.0.1:1053"
+        order:
+            - A

Review Comment:
   Done



-- 
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.

To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [apisix] monkeyDluffy6017 commented on pull request #7935: feat: support configuring the order of the upstream dns service disco…

Posted by GitBox <gi...@apache.org>.
monkeyDluffy6017 commented on PR #7935:
URL: https://github.com/apache/apisix/pull/7935#issuecomment-1257513095

   > 
   
   Done


-- 
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.

To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [apisix] spacewander commented on a diff in pull request #7935: feat: support configuring the order of the upstream dns service disco…

Posted by GitBox <gi...@apache.org>.
spacewander commented on code in PR #7935:
URL: https://github.com/apache/apisix/pull/7935#discussion_r973882047


##########
apisix/discovery/dns/schema.lua:
##########
@@ -24,6 +24,18 @@ return {
                 type = "string",
             },
         },
+        order = {
+            type = "array",
+            minItems = 1,
+            maxItems = 5,
+            uniqueItems = true,
+            items = {
+                enum = {"last", "SRV", "A", "AAAA", "CNAME"},
+     --           ["not"] = {

Review Comment:
   Just for debug?



##########
t/discovery/dns/sanity.t:
##########
@@ -318,3 +318,24 @@ qr/upstream nodes: \{[^}]+\}/
 qr/upstream nodes: \{("127.0.0.1:1980":60,"127.0.0.2:1980":20|"127.0.0.2:1980":20,"127.0.0.1:1980":60)\}/
 --- response_body
 hello world
+
+
+
+=== TEST 16: prefer A than SRV when A is ahead of SRV in config.yaml
+--- apisix_yaml
+upstreams:
+    - service_name: "srv-a.test.local"
+      discovery_type: dns
+      type: roundrobin
+      id: 1
+discovery:
+    dns:
+        servers:
+            - "127.0.0.1:1053"
+        order:
+            - A

Review Comment:
   Is it possible to add some test about invalid input, like `a` or repeated record type?



-- 
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.

To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [apisix] tzssangglass commented on a diff in pull request #7935: feat: support configuring the order of the upstream dns service disco…

Posted by GitBox <gi...@apache.org>.
tzssangglass commented on code in PR #7935:
URL: https://github.com/apache/apisix/pull/7935#discussion_r974819534


##########
t/discovery/dns/sanity.t:
##########
@@ -318,3 +326,115 @@ qr/upstream nodes: \{[^}]+\}/
 qr/upstream nodes: \{("127.0.0.1:1980":60,"127.0.0.2:1980":20|"127.0.0.2:1980":20,"127.0.0.1:1980":60)\}/
 --- response_body
 hello world
+
+
+
+=== TEST 16: prefer A than SRV when A is ahead of SRV in config.yaml
+--- yaml_config
+apisix:
+    node_listen: 1984
+    enable_admin: false
+deployment:
+    role: data_plane
+    role_data_plane:
+        config_provider: yaml
+discovery:
+    dns:
+        servers:
+            - "127.0.0.1:1053"
+        order:
+            - A
+            - SRV
+--- apisix_yaml
+upstreams:
+    - service_name: "srv-a.test.local"
+      discovery_type: dns
+      type: roundrobin
+      id: 1
+--- error_code: 502
+--- error_log
+proxy request to 127.0.0.1:80
+
+
+
+=== TEST 17: Invalid order type in config.yaml
+--- yaml_config
+apisix:
+    node_listen: 1984
+    enable_admin: false
+deployment:
+    role: data_plane
+    role_data_plane:
+        config_provider: yaml
+discovery:
+    dns:
+        servers:
+            - "127.0.0.1:1053"
+        order:
+            - B
+            - SRV
+--- apisix_yaml
+upstreams:
+    - service_name: "srv-a.test.local"
+      discovery_type: dns
+      type: roundrobin
+      id: 1
+--- must_die
+--- error_log
+matches none of the enum values
+
+
+
+=== TEST 18: Multiple order type in config.yaml
+--- yaml_config
+apisix:
+    node_listen: 1984
+    enable_admin: false
+deployment:
+    role: data_plane
+    role_data_plane:
+        config_provider: yaml
+discovery:
+    dns:
+        servers:
+            - "127.0.0.1:1053"
+        order:
+            - SRV
+            - SRV
+--- apisix_yaml
+upstreams:
+    - service_name: "srv-a.test.local"
+      discovery_type: dns
+      type: roundrobin
+      id: 1
+--- must_die
+--- error_log
+expected unique items but items 1 and 2 are equal
+
+
+
+=== TEST 19: invalid order type in config.yaml
+--- yaml_config
+apisix:
+    node_listen: 1984
+    enable_admin: false
+deployment:
+    role: data_plane
+    role_data_plane:
+        config_provider: yaml
+discovery:
+    dns:
+        servers:
+            - "127.0.0.1:1053"
+        order:
+            - a
+            - SRV
+--- apisix_yaml
+upstreams:
+    - service_name: "srv-a.test.local"
+      discovery_type: dns
+      type: roundrobin
+      id: 1
+--- must_die
+--- error_log
+matches none of the enum values

Review Comment:
   Can we add a test case: DNS Server returns SRV records first for the same domain name, and the next query returns A records?



-- 
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.

To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [apisix] spacewander merged pull request #7935: feat: support configuring the order of the upstream dns service discovery resolve

Posted by GitBox <gi...@apache.org>.
spacewander merged PR #7935:
URL: https://github.com/apache/apisix/pull/7935


-- 
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.

To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [apisix] monkeyDluffy6017 commented on a diff in pull request #7935: feat: support configuring the order of the upstream dns service disco…

Posted by GitBox <gi...@apache.org>.
monkeyDluffy6017 commented on code in PR #7935:
URL: https://github.com/apache/apisix/pull/7935#discussion_r974175516


##########
apisix/discovery/dns/schema.lua:
##########
@@ -24,6 +24,18 @@ return {
                 type = "string",
             },
         },
+        order = {
+            type = "array",
+            minItems = 1,
+            maxItems = 5,
+            uniqueItems = true,
+            items = {
+                enum = {"last", "SRV", "A", "AAAA", "CNAME"},
+     --           ["not"] = {

Review Comment:
   Done



-- 
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.

To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org