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

Exporter not working when keepalived container init doesn't handle keepalived signals #72

Closed
sbienkow opened this issue Apr 13, 2021 · 2 comments
Labels
wontfix This will not be worked on

Comments

@sbienkow
Copy link

Currently, if docker mode is enabled, this exporter first checks signal numbers, by executing command in container: docker exec "${container-name}" keepalived --signum [STATS|DATA]. Related lines: https://github.com/cafebazaar/keepalived-exporter/blob/master/internal/types/container/keepalived_container_collector_host.go#L76-L104

It then uses those signal numbers to send them through docker to process with PID1 inside the container `docker kill : https://github.com/cafebazaar/keepalived-exporter/blob/master/internal/types/container/keepalived_container_collector_host.go#L106-L117

This unfortunately fails with osixia keepalived docker image, because it's init script doesn't relay these signals to keepalived instances it spawns osixia/docker-keepalived#50. While I believe this should be fixed in the keepalived image itself, maybe it could be considered to be handled in the exporter. I've chosen to use that docker image, because it seems most up to date.

The fix for this seems relatively simple - instead of running docker kill --signal 12 keepalived, one could run docker exec keepalived sh -c 'kill -12 "$( cat /var/run/keepalived.pid )"'. Of course this assumes that sh exists, so would probably require it to be a separate command line argument to switch between these 2 behaviors.

@mehdy
Copy link
Owner

mehdy commented Jan 27, 2023

Sorry for the very long delay. We'll investigate soon.

@clwluvw Could you please investigate this issue?

@clwluvw
Copy link
Collaborator

clwluvw commented Feb 6, 2023

We discussed that internally and agreed that this is the used keepalived image issue that is not running keepalived daemon as a pid 1 (which is the correct behavior in container runtime). So trying to support that bad practices would put us in a hole of dirty if/else which makes the project unmaintainable as an open source project (as it's going to accept source code changes from others).

As the mentioned image repo is deprecated (hasn't been updated for two years) you probably can build your own image (by replaying what the repo does or just simply installing keepalived from an alpine base which is not very outdated - e.g. https://github.com/mehdy/keepalived-exporter/blob/master/deployments/dev/keepalived.Dockerfile).
Also forking the repo and adding the ability on your own is also an option.

@clwluvw clwluvw closed this as not planned Won't fix, can't repro, duplicate, stale Feb 6, 2023
@clwluvw clwluvw added the wontfix This will not be worked on label Feb 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
wontfix This will not be worked on
Projects
None yet
Development

No branches or pull requests

3 participants