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/09/09 09:14:30 UTC

[GitHub] [apisix] sunhao-java opened a new issue #5026: bug: upstream admin api doc or upstream.lua's bug?

sunhao-java opened a new issue #5026:
URL: https://github.com/apache/apisix/issues/5026


   ### Issue description
   
   1. Update upstream info via Apisix Admin api.
   ```
   curl --location --request PATCH 'http://localhost:9080/apisix/admin/upstreams/804436770317012992' \
   --header 'x-api-key: 0a86d8c17dcb4608b20d9b6d5473dd01' \
   --header 'Content-Type: application/json' \
   --data '{
       "nodes": {
           "ip:port": 100
       },
       "timeout": {
           "connect": 60,
           "send": 60,
           "read": 60
       },
       "type": "roundrobin",
       "pass_host": "pass",
       "scheme": "http",
       "id": "804436770317012992"
   }'
   ```
   
   2. This http request's method is `PATCH`
   3. Docs about upstream api: [Upstream API](https://apisix.apache.org/docs/apisix/admin-api#upstream).
       
   ![image](https://user-images.githubusercontent.com/2178487/132655935-eb1ab7c6-c8d3-490d-bd63-6977c1623f59.png)
   
   4. But, when I request the request (In step 1) for more times, the `nodes` field in this upstream will be more and more.
   5. The description of patch in docs says, when the value of the attribute is an array, the attribute will be updated in full.
   
   ### Environment
   
   - apisix version (cmd: `apisix version`): the latest.
   - OS (cmd: `uname -a`): docker on windows
   - OpenResty / Nginx version (cmd: `nginx -V` or `openresty -V`): openresty/1.19.3.1
   - etcd version, if have (cmd: run `curl http://127.0.0.1:9090/v1/server_info` to get the info from server-info API): 3.4.0
   - apisix-dashboard version, if have: no
   - luarocks version, if the issue is about installation (cmd: `luarocks --version`): no
   
   
   ### Steps to reproduce
   
   1. run this command
   ```
   curl --location --request PUT 'http://localhost:9080/apisix/admin/upstreams/804436770317012992' \
   --header 'x-api-key: 0a86d8c17dcb4608b20d9b6d5473dd01' \
   --header 'Content-Type: application/json' \
   --data '{
       "nodes": {
           "ip:port": 100
       },
       "timeout": {
           "connect": 60,
           "send": 60,
           "read": 60
       },
       "type": "roundrobin",
       "pass_host": "pass",
       "scheme": "http",
       "id": "804436770317012992"
   }'
   ```
   2. run this command
   ```
   curl --location --request PATCH 'http://localhost:9080/apisix/admin/upstreams/804436770317012992' \
   --header 'x-api-key: 0a86d8c17dcb4608b20d9b6d5473dd01' \
   --header 'Content-Type: application/json' \
   --data '{
       "nodes": {
           "ip1:port1": 100
       },
       "timeout": {
           "connect": 60,
           "send": 60,
           "read": 60
       },
       "type": "roundrobin",
       "pass_host": "pass",
       "scheme": "http",
       "id": "804436770317012992"
   }'
   ```
   3. First request use mthod `PUT`, Second request use method `PATCH`.
   
   ### Actual result
   
   ```
   {
       "update_time": 1629795074,
       "create_time": 1629795074,
       "type": "roundrobin",
       "scheme": "http",
       "hash_on": "vars",
       "id": "804436770317012992",
       "pass_host": "node",
       "nodes": {
           "ip:port": 100,
           "ip1:port1": 100
       }
   }
   ```
   
   ### Error log
   
   ```
   2021/09/09 09:07:59 [info] 49#49: *111589 [lua] upstreams.lua:264: key: /upstreams/804436770317012992 old value: {"body_reader":"function: 0x7f8b2d7e8798","reason":"OK","has_body":true,"body":{"action":"get","count":"1","node":{"key":"\/apisix\/upstreams\/804436770317012992","value":{"id":"804436770317012992","hash_on":"vars","timeout":{"send":60,"read":60,"connect":60},"nodes":{"ip:port":100},"type":"roundrobin","create_time":1629795074,"update_time":1631178467,"scheme":"http","pass_host":"pass"},"createdIndex":15403,"modifiedIndex":16741},"header":{"cluster_id":"14841639068965178418","member_id":"10276657743932975437","revision":"16741","raft_term":"22"}},"status":200,"read_body":"function: 0x7f8b3048cb48","headers":{"Access-Control-Allow-Origin":"*","X-Etcd-Index":"16741","Access-Control-Allow-Headers":"accept, content-type, authorization","Grpc-Metadata-Content-Type":"application\/grpc","Content-Length":"565","Content-Type":"application\/json","Access-Control-Allow-Methods":"P
 OST, GET, OPTIONS, PUT, DELETE","Date":"Thu, 09 Sep 2021 09:07:59 GMT"},"read_trailers":"function: 0x7f8b3048cc00"}, client: 172.21.0.1, server: _, request: "PATCH /apisix/admin/upstreams/804436770317012992 HTTP/1.1", host: "localhost:9080"
   2021/09/09 09:07:59 [info] 49#49: *111589 [lua] upstreams.lua:293: new value {"id":"804436770317012992","hash_on":"vars","timeout":{"send":60,"read":60,"connect":60},"nodes":{"ip1:port1":100,"ip:port":100},"type":"roundrobin","create_time":1629795074,"update_time":1631178479,"scheme":"http","pass_host":"pass"}, client: 172.21.0.1, server: _, request: "PATCH /apisix/admin/upstreams/804436770317012992 HTTP/1.1", host: "localhost:9080"
   2021/09/09 09:07:59 [info] 49#49: *111589 [lua] upstreams.lua:123: check_conf(): schema: {"oneOf":[{"required":["type","nodes"]},{"required":["type","service_name","discovery_type"]}],"properties":{"id":{"anyOf":[{"maxLength":64,"pattern":"^[a-zA-Z0-9-_.]+$","type":"string","minLength":1},{"minimum":1,"type":"integer"}]},"hash_on":{"enum":["vars","header","cookie","consumer","vars_combinations"],"default":"vars","type":"string"},"timeout":{"properties":{"send":{"exclusiveMinimum":0,"type":"number"},"read":{"exclusiveMinimum":0,"type":"number"},"connect":{"exclusiveMinimum":0,"type":"number"}},"required":["connect","send","read"],"type":"object"},"labels":{"maxProperties":16,"description":"key\/value pairs to specify attributes","patternProperties":{".*":{"maxLength":64,"description":"value of label","pattern":"^\\S+$","minLength":1,"type":"string"}},"type":"object"},"discovery_type":{"type":"string","description":"discovery type"},"desc":{"maxLength":256,"type":"string"},"pass_hos
 t":{"enum":["pass","node","rewrite"],"description":"mod of host passing","default":"pass","type":"string"},"upstream_host":{"pattern":"^\\*?[0-9a-zA-Z-._]+$","type":"string"},"nodes":{"anyOf":[{"patternProperties":{".*":{"description":"weight of node","minimum":0,"type":"integer"}},"type":"object"},{"items":{"properties":{"host":{"pattern":"^\\*?[0-9a-zA-Z-._]+$","type":"string"},"weight":{"description":"weight of node","minimum":0,"type":"integer"},"priority":{"description":"priority of node","default":0,"type":"integer"},"port":{"description":"port of node","minimum":1,"type":"integer"},"metadata":{"type":"object","description":"metadata of node"}},"required":["host","port","weight"],"type":"object"},"type":"array"}]},"type":{"enum":["chash","roundrobin","ewma","least_conn"],"description":"algorithms of load balancing","type":"string"},"key":{"type":"string","description":"the key of chash for dynamic load balancing"},"service_name":{"maxLength":256,"minLength":1,"type":"string"},
 "checks":{"anyOf":[{"required":["active"]},{"required":["active","passive"]}],"properties":{"active":{"properties":{"healthy":{"properties":{"interval":{"default":1,"minimum":1,"type":"integer"},"successes":{"maximum":254,"default":2,"minimum":1,"type":"integer"},"http_statuses":{"default":[200,302],"type":"array","minItems":1,"items":{"maximum":599,"minimum":200,"type":"integer"},"uniqueItems":true}},"type":"object"},"req_headers":{"minItems":1,"items":{"uniqueItems":true,"type":"string"},"type":"array"},"timeout":{"default":1,"type":"number"},"unhealthy":{"properties":{"http_failures":{"maximum":254,"default":5,"minimum":1,"type":"integer"},"tcp_failures":{"maximum":254,"default":2,"minimum":1,"type":"integer"},"timeouts":{"maximum":254,"default":3,"minimum":1,"type":"integer"},"interval":{"default":1,"minimum":1,"type":"integer"},"http_statuses":{"default":[429,404,500,501,502,503,504,505],"type":"array","minItems":1,"items":{"maximum":599,"minimum":200,"type":"integer"},"uniqueI
 tems":true}},"type":"object"},"type":{"enum":["http","https","tcp"],"default":"http","type":"string"},"host":{"pattern":"^\\*?[0-9a-zA-Z-._]+$","type":"string"},"port":{"maximum":65535,"minimum":1,"type":"integer"},"concurrency":{"default":10,"type":"integer"},"http_path":{"default":"\/","type":"string"},"https_verify_certificate":{"default":true,"type":"boolean"}},"type":"object"},"passive":{"properties":{"healthy":{"properties":{"successes":{"maximum":254,"default":5,"minimum":1,"type":"integer"},"http_statuses":{"default":[200,201,202,203,204,205,206,207,208,226,300,301,302,303,304,305,306,307,308],"type":"array","minItems":1,"items":{"maximum":599,"minimum":200,"type":"integer"},"uniqueItems":true}},"type":"object"},"unhealthy":{"properties":{"http_failures":{"maximum":254,"default":5,"minimum":1,"type":"integer"},"tcp_failures":{"maximum":254,"default":2,"minimum":1,"type":"integer"},"timeouts":{"maximum":254,"default":7,"minimum":1,"type":"integer"},"http_statuses":{"default":
 [429,500,503],"type":"array","minItems":1,"items":{"maximum":599,"minimum":200,"type":"integer"},"uniqueItems
   2021/09/09 09:07:59 [info] 49#49: *111589 [lua] upstreams.lua:124: check_conf(): conf  : {"id":"804436770317012992","hash_on":"vars","timeout":{"send":60,"connect":60,"read":60},"nodes":{"ip1:port1":100,"ip:port":100},"type":"roundrobin","create_time":1629795074,"pass_host":"pass","scheme":"http","update_time":1631178479}, client: 172.21.0.1, server: _, request: "PATCH /apisix/admin/upstreams/804436770317012992 HTTP/1.1", host: "localhost:9080"
   ```
   
   ### Expected result
   
   ```
   {
       "id": "804436770317012992",
       "hash_on": "vars",
       "timeout": {
           "send": 60,
           "connect": 60,
           "read": 60
       },
       "nodes": {
           "ip1:port1": 100
       },
       "type": "roundrobin",
       "create_time": 1629795074,
       "pass_host": "pass",
       "scheme": "http",
       "update_time": 1631178479
   }
   ```
   
   I read the code of apisix, `/apisix/admin/upstream.lua`. The function _M.patch:
   ```
   -- I think this new_value named wrong. It maybe named old_value??
   local new_value = res_old.body.node.value
   local modified_index = res_old.body.node.modifiedIndex
   
   if sub_path and sub_path ~= "" then
       local code, err, node_val = core.table.patch(new_value, sub_path, conf)
       new_value = node_val
       if code then
           return code, err
       end
       utils.inject_timestamp(new_value, nil, true)
   else
       -- method `table.merge` in lua maybe merge array too?
       new_value = core.table.merge(new_value, conf);
       utils.inject_timestamp(new_value, nil, conf)
   end
   ```
   


-- 
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 issue #5026: bug: upstream admin api doc or upstream.lua's bug?

Posted by GitBox <gi...@apache.org>.
spacewander commented on issue #5026:
URL: https://github.com/apache/apisix/issues/5026#issuecomment-927292832


   I find a way to replace a node in hash format:
   ```
   $ curl http://127.0.0.1:9080/apisix/admin/upstreams/100 -H'X-API-KEY: edd1c9f034335f136f87ad84b625c8f1' -X PATCH -i -d '
   {
       "nodes": {
           "39.97.63.215:80": null,
   		"39.97.63.215:81": 10
       }
   }'
   ```
   
   It bases on an example in https://github.com/apache/apisix/blob/master/docs/en/latest/admin-api.md#upstream


-- 
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 issue #5026: bug: upstream admin api doc or upstream.lua's bug?

Posted by GitBox <gi...@apache.org>.
tzssangglass commented on issue #5026:
URL: https://github.com/apache/apisix/issues/5026#issuecomment-916059750


   nodes support both hash and array to appear ambiguous, I prefer to make nodes only support the form of arrays.


-- 
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 issue #5026: bug: upstream admin api doc or upstream.lua's bug?

Posted by GitBox <gi...@apache.org>.
spacewander commented on issue #5026:
URL: https://github.com/apache/apisix/issues/5026#issuecomment-915970808


   PATCH request should merge new fields to hash. Otherwise, the other fields in upstream will be removed, as upstream is a hash. Since the nodes field is hash too, it looks like there is no way to override it.
   
   However, you can switch the nodes to an array format:
   ```
   "nodes": [
               {"host": "127.0.0.1", "port": 1980, "weight": 2000},
               {"host": "127.0.0.2", "port": 1980, "weight": 1, "priority": -1}
   ]
   ```
   
   What about your guys' opinion?
   @membphis @tzssangglass 


-- 
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 closed issue #5026: bug: upstream admin api doc or upstream.lua's bug?

Posted by GitBox <gi...@apache.org>.
spacewander closed issue #5026:
URL: https://github.com/apache/apisix/issues/5026


   


-- 
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 issue #5026: bug: upstream admin api doc or upstream.lua's bug?

Posted by GitBox <gi...@apache.org>.
spacewander commented on issue #5026:
URL: https://github.com/apache/apisix/issues/5026#issuecomment-916071248


   I do some research and find an RFC for JSON Merge Patch:
   https://datatracker.ietf.org/doc/html/rfc7396
   
   According to the example (https://datatracker.ietf.org/doc/html/rfc7396#section-3), look like our current way is OK.


-- 
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 issue #5026: bug: upstream admin api doc or upstream.lua's bug?

Posted by GitBox <gi...@apache.org>.
tzssangglass commented on issue #5026:
URL: https://github.com/apache/apisix/issues/5026#issuecomment-916107710


   OK, I think we can point out this difference in the documentation.


-- 
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] tokers commented on issue #5026: bug: upstream admin api doc or upstream.lua's bug?

Posted by GitBox <gi...@apache.org>.
tokers commented on issue #5026:
URL: https://github.com/apache/apisix/issues/5026#issuecomment-916066812


   > nodes support both hash and array to appear ambiguous, I prefer to make nodes only support the form of arrays.
   
   Could we provide an utility to convert the node in hash to array? So that people can change their configurations easily.


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