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/05/26 03:49:12 UTC

[GitHub] [apisix] Demogorgon314 opened a new pull request #4308: feat: nacos discovery support namespace and group and support multiple host

Demogorgon314 opened a new pull request #4308:
URL: https://github.com/apache/apisix/pull/4308


   ### What this PR does / why we need it:
   #4306
   support multiple host
   ### Pre-submission checklist:
   
   * [x] Did you explain what problem does this PR solve? Or what new features have been added?
   * [ ] Have you added corresponding test cases?
   * [x] Have you modified the corresponding document?
   * [x] 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 pull request #4308: feat: nacos discovery support namespace and group and support multiple host

Posted by GitBox <gi...@apache.org>.
spacewander commented on pull request #4308:
URL: https://github.com/apache/apisix/pull/4308#issuecomment-848438383


   Test must be added for the new feature.


-- 
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] Demogorgon314 closed pull request #4308: feat: nacos discovery support namespace and group and support multiple host

Posted by GitBox <gi...@apache.org>.
Demogorgon314 closed pull request #4308:
URL: https://github.com/apache/apisix/pull/4308


   


-- 
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 pull request #4308: feat: nacos discovery support namespace and group and support multiple host

Posted by GitBox <gi...@apache.org>.
spacewander commented on pull request #4308:
URL: https://github.com/apache/apisix/pull/4308#issuecomment-848439277


   My suggestion is that we need to split the PR into steps:
   1. add namespace
   2. add group
   3. add multiple host
   
   So that the PR can be merged quickly.


-- 
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] Demogorgon314 commented on pull request #4308: feat: nacos discovery support namespace and group and support multiple host

Posted by GitBox <gi...@apache.org>.
Demogorgon314 commented on pull request #4308:
URL: https://github.com/apache/apisix/pull/4308#issuecomment-848476830


   > My suggestion is that we need to split the PR into steps:
   > 
   > 1. add namespace
   > 2. add group
   > 3. add multiple host
   > 
   > So that the PR can be merged quickly.
   
   Thx I will close this pr and open another


-- 
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 #4308: feat: nacos discovery support namespace and group and support multiple host

Posted by GitBox <gi...@apache.org>.
spacewander commented on a change in pull request #4308:
URL: https://github.com/apache/apisix/pull/4308#discussion_r639385391



##########
File path: apisix/schema_def.lua
##########
@@ -389,6 +389,24 @@ local upstream_schema = {
             description = "discovery type",
             type = "string",
         },
+        discovery_host = {

Review comment:
       Better to use `nacos_address` instead.
   All these 3 fields should go under a separate field called `discovery_args`, as it is nacos specific.
   And we need to check them only when discovery_type `nacos` is used.

##########
File path: apisix/discovery/nacos.lua
##########
@@ -208,7 +222,12 @@ local function iter_and_add_service(services, values)
         end
 
         if up.discovery_type == 'nacos' then
-            core.table.insert(services, up.service_name)
+            core.table.insert(services, {
+                service_name=up.service_name,

Review comment:
       Need space around the `=`

##########
File path: apisix/discovery/nacos.lua
##########
@@ -157,9 +157,23 @@ local function get_token_param(base_uri, username, password)
     return '&accessToken=' .. data.accessToken
 end
 
+local function get_group_and_namespace_param(service_info)

Review comment:
       Need to encode the args.

##########
File path: docs/en/latest/discovery/nacos.md
##########
@@ -99,3 +99,97 @@ The format response as below:
   "action": "set"
 }
 ```
+
+example of routing a request with a URL of "/nacosWithNamespaceIdAndGroupId/*" to a service which name, namespaceId, groupName "http://192.168.33.1:8848/nacos/v1/ns/instance/list?serviceName=APISIX-NACOS&groupName=test&namespaceId=test" and use nacos discovery client in the registry :
+
+```shell
+$ curl http://127.0.0.1:9080/apisix/admin/routes/2 -H 'X-API-KEY: edd1c9f034335f136f87ad84b625c8f1' -X PUT -i -d '
+{
+    "uri": "/nacosWithNamespaceIdAndGroupId/*",
+    "upstream": {
+        "service_name": "APISIX-NACOS",
+        "type": "roundrobin",
+        "discovery_type": "nacos",
+        "namespace_id": "test",
+        "group_name":"test"
+    }
+}'
+```
+
+The format response as below:
+
+```json
+{
+  "node": {
+    "key": "\/apisix\/routes\/2",
+    "value": {
+      "id": "1",
+      "create_time": 1615796097,
+      "status": 1,
+      "update_time": 1615799165,
+      "upstream": {
+        "hash_on": "vars",
+        "pass_host": "pass",
+        "scheme": "http",
+        "service_name": "APISIX-NACOS",
+        "type": "roundrobin",
+        "discovery_type": "nacos",
+        "namespace_id": "test",
+        "group_name": "test"
+      },
+      "priority": 0,
+      "uri": "\/nacosWithNamespaceIdAndGroupId\/*"
+    }
+  },
+  "action": "set"
+}
+```
+
+example to use other nacos host

Review comment:
       In what situation we need to switch Nacos address in the per upstream level?

##########
File path: docs/en/latest/discovery/nacos.md
##########
@@ -99,3 +99,97 @@ The format response as below:
   "action": "set"
 }
 ```
+
+example of routing a request with a URL of "/nacosWithNamespaceIdAndGroupId/*" to a service which name, namespaceId, groupName "http://192.168.33.1:8848/nacos/v1/ns/instance/list?serviceName=APISIX-NACOS&groupName=test&namespaceId=test" and use nacos discovery client in the registry :

Review comment:
       This first letter should be uppercase.




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