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

Reload daemons for Logstash restart #347

Merged
merged 2 commits into from
Oct 23, 2024
Merged

Conversation

widhalmt
Copy link
Member

Reload unit files when restart Logstash via handler. We're running the handler for upgrades of the package, not only when changing configuration.

This might make reloads a bit slower but it seems to be the only chance to deal with changes in the Logstash unit file. We're not managing this file with the collection, so changing it manually is fine.

If someone does, we need to reload the unit file.

We could introduce a separate handler. One for upgrades of the package that does the daemon reload and another without reload for regular configuration changes. @frankhetterich @afeefghannam89 @martialblog what do you think? I went for one handler which does it all but that's just a suggestion so far.

Thank you, @frankhetterich for the bug report and the suggested fix.

fixes #342

This might make reloads a bit slower but it seems to be the only chance to deal with changes in the Logstash unit file. We're not managing this file witht the collection, so changing it manually is fine.

If someone does, we need to reload the unit file.

fixes #342
@widhalmt widhalmt added the bug Something isn't working label Sep 25, 2024
@widhalmt widhalmt added this to the 0.1.0 milestone Sep 25, 2024
@widhalmt widhalmt self-assigned this Sep 25, 2024
Copy link
Member

@afeefghannam89 afeefghannam89 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think to have the daemon reload in the same reload task as you made is better.

@frankhetterich
Copy link

We could introduce a separate handler. One for upgrades of the package that does the daemon reload and another without reload for regular configuration changes. @frankhetterich @afeefghannam89 @martialblog what do you think? I went for one handler which does it all but that's just a suggestion so far.

I would also vote for one handler which covers all

@widhalmt widhalmt added this pull request to the merge queue Oct 23, 2024
Merged via the queue into main with commit 65dfa4f Oct 23, 2024
8 checks passed
@widhalmt widhalmt deleted the fix/logstash-daemon-reload-342 branch October 23, 2024 15:12
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 this pull request may close these issues.

[Bug]: Daemon reload command is missing after Logstash install
3 participants