Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

bug: The logic for method fetch_full_registry in nacos.lua will break the loop if the previous get_url request failed #4949

Closed
rztao opened this issue Sep 1, 2021 · 1 comment · Fixed by #5112
Assignees
Labels
bug Something isn't working

Comments

@rztao
Copy link

rztao commented Sep 1, 2021

Issue description

data, err = get_url(base_uri, instance_list_path .. service_info.service_name .. token_param .. namespace_param .. group_name_param) if err then log.error('get_url:', instance_list_path, ' err:', err) if not applications then applications = up_apps end return end

The above logic for method fetch_full_registry in nacos.lua will break the loop if the previous get_url request failed. This issue can be reproduced with a wrong route with service discovery configuration.

`

local data, err
    for _, service_info in ipairs(infos) do
        local namespace_param = get_namespace_param(service_info.namespace_id)
        local group_name_param = get_group_name_param(service_info.group_name)
        data, err = get_url(base_uri, instance_list_path .. service_info.service_name
                            .. token_param .. namespace_param .. group_name_param)
        if err then
            log.error('get_url:', instance_list_path, ' err:', err)
            if not applications then
                applications = up_apps
            end
            return
        end

        for _, host in ipairs(data.hosts) do
            local nodes = up_apps[service_info.service_name]
            if not nodes then
                nodes = {}
                up_apps[service_info.service_name] = nodes
            end
            core.table.insert(nodes, {
                host = host.ip,
                port = host.port,
                weight = host.weight or default_weight,
            })
        end
    end

Environment

  • apisix version (cmd: apisix version): 2.9
  • OS (cmd: uname -a): MacOs (same with Linux, not tested)
  • OpenResty / Nginx version (cmd: nginx -V or openresty -V): nginx version: openresty/1.19.3.2

Steps to reproduce

  1. Configure a route with a wrong service_name not registered in Nacos.
  2. The fetch data request for service_name configured in 1) failed or timeout is reached, it will break the loop,
    and the next requests to fetch data will be skipped but they are correctly configured.

Actual result

The next requests to fetch data will be skipped but they are correctly configured if the previous request not correctly configured or timeout reached.

Error log

log.error('get_url:', instance_list_path, ' err:', err)

Expected result

The next requests to fetch data will be not skipped when the previous request not correctly configured or timeout reached.

@Zheaoli
Copy link
Member

Zheaoli commented Sep 1, 2021

Yes, You are right. Maybe its a bad idea to return direct

@spacewander spacewander added the bug Something isn't working label Sep 2, 2021
@spacewander spacewander self-assigned this Sep 22, 2021
spacewander added a commit to spacewander/incubator-apisix that referenced this issue Sep 22, 2021
Fix apache#4949

Signed-off-by: spacewander <spacewanderlzx@gmail.com>
spacewander added a commit to spacewander/incubator-apisix that referenced this issue Sep 22, 2021
Fix apache#4949

Signed-off-by: spacewander <spacewanderlzx@gmail.com>
spacewander added a commit to spacewander/incubator-apisix that referenced this issue Sep 22, 2021
Fix apache#4949

Signed-off-by: spacewander <spacewanderlzx@gmail.com>
spacewander added a commit that referenced this issue Sep 22, 2021
…5112)

Fix #4949

Signed-off-by: spacewander <spacewanderlzx@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants