Linux agent: Fixes for ntpq handling, for non-Ubuntu and non-systemd#116
Closed
rawiriblundell wants to merge 6 commits into
Closed
Linux agent: Fixes for ntpq handling, for non-Ubuntu and non-systemd#116rawiriblundell wants to merge 6 commits into
rawiriblundell wants to merge 6 commits into
Conversation
Contributor
|
Hello Rawiri, thank you for your help this fix! We will have a closer look at your patch and discuss internally on how and if we can implement it into the official code base. Best Internal Ref: CMK-3605 |
anthonyh209
reviewed
Jan 20, 2020
Contributor
|
Hi Rawiri, I wanted to make some things clear before I include your changes (see above). Thank you for your suggestions again. I had your comments from October back in mind, but they disappeared unfortunately. Best, |
anthonyh209
reviewed
Jan 22, 2020
…es in merged agent script
Contributor
Author
|
This appears to have been resolved with ae6103c |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Hi,
cc8af1a does not resolve the issues I raised in October (see e.g. d17e58#r35653501 ). I have also noticed another issue - a service named 'ntp' is assumed, whereas on some distributions this might be 'ntpd'. According to your contribution guidelines, you're building to Ubuntu, but that's somewhat short sighted. In the commercial/enterprise world (i.e. potential checkmk customers/users), the likes of RHEL and Suse cannot be ignored.
In this PR, I propose some code that will hopefully resolve all of the issues raised. This should theoretically work across varied Linux distributions, whether they are newer and systemd based, newer and not-systemd based, or legacy systems running another init system.
As an example of the systemd lock-in, I spun up a CentOS 5.11 VM (there is, very sadly, a LOT of RHEL 5.11 in govt and enterprise):
That's the output of
psand the script code. With the proposed code in place:For reference, the current code run on a CentOS 7 VM:
At present there is no intelligence for the case of a non-systemd host i.e.
ntpqis blindly run ifntpqexists. For one possible approach for this, seesection_ntp()here: https://github.com/rawiriblundell/checkMK/blob/merged_nix_agent/agents/check_mk_agent.merged#L1861