Worker process doesn't let parallel API package stage updates to complete when terminated #9267

yhabteab · 2022-02-28T16:34:39Z

When the second request isn't finished processing till the new worker process is started and its configs are validated, the old worker is terminated immediately after the new worker becomes ready. When the worker process is terminated, all the worker threads of the Process class that are responsible for processing such requests are also destroyed. Thus, the second API request remains incomplete and produces a package stage without startup logs and status file.

Changes:

Concurrent package updates are strictly prohibited.

To Reproduce

First create two different config packages
- curl -k -s -S -i -u root:icinga -H 'Accept: application/json' -X POST 'https://localhost:5665/v1/config/packages/cmdb?pretty=1'
- curl -k -s -S -i -u root:icinga -H 'Accept: application/json' -X POST 'https://localhost:5665/v1/config/packages/test?pretty=1'
Next just update these packages asynchronously. You need also to specify different configuration each time so that the reload signal can be triggered internally. You also need to post the requests in a bit order (not strictly simultaneous) so that you can reproduce the issue.

curl -k -s -S -i -u root:icinga -H 'Accept: application/json' -X POST 'https://localhost:5665/v1/config/stages/cmdb'  -d '{ "files": { "conf.d/test.conf": "object Host \"cmdb-host\" { check_command = \"dummy\" }" }, "pretty": true }'
curl -k -s -S -i -u root:icinga -H 'Accept: application/json' -X POST 'https://localhost:5665/v1/config/stages/test'  -d '{ "files": { "conf.d/test.conf": "object Host \"test-host\" { check_command = \"dummy\" }" }, "pretty": true }'

fixes #7898

lib/cli/daemoncommand.cpp

julianbrost

Shouldn't it be possible to solve this within ApiListener? Adding this to Process feels somewhat random for me. Possible options:

Wait for pending actions in ApiListener::Stop(). You'd have to be careful that you actually reload with a new configuration that's consistent with all the requests answered.
Fail concurrent requests for modification altogether, i.e. just return some kind of "conflicting operation already in progress, please try again later" error. Clients then would have to handle this, but they have to handle failures already anyways since the API is gone during the reload, but the error would look a bit different.

yhabteab · 2022-03-02T16:16:38Z

Fail concurrent requests for modification altogether

You want me to forbid concurrent stage updates that are completely independent from each other?

Al2Klimov · 2022-03-02T16:20:18Z

Tom and I also do. (Icinga/icingaweb2-module-director#2412)

yhabteab · 2022-03-02T19:13:24Z

Before:
See in the referenced issue.

After:

Package cmdb:

curl -k -s -S -i -u root:icinga -H 'Accept: application/json' -X POST 'https://localhost:5665/v1/config/stages/cmdb'  -d '{ "files": { "conf.d/test.conf": "object Host \"cmdb-host\" { check_command = \"dummy\" }" }, "pretty": true }'

HTTP/1.1 200 OK
Server: Icinga/v2.13.0-209-g62e0c349b
Content-Type: application/json
Content-Length: 221

{
    "results": [
        {
            "code": 200,
            "package": "cmdb",
            "stage": "9a218b57-8b04-4f66-80ff-3c9c8763fe24",
            "status": "Created stage. Reload triggered."
        }
    ]
}

Package icinga:

curl -k -s -S -i -u root:icinga -H 'Accept: application/json' -X POST 'https://localhost:5665/v1/config/stage
s/icinga'  -d '{ "files": { "conf.d/test.conf": "object Host \"test-host\" { check_command = \"dummy\" }" }, "pretty": true }'
HTTP/1.1 409 Conflict
Server: Icinga/v2.13.0-209-g62e0c349b
Content-Type: application/json
Content-Length: 142

{
    "error": 409,
    "status": "Conflicting request, there is already an ongoing package update in progress. Please try it again later."
}

lib/remote/configstageshandler.hpp