Skip to content

Add merged *nix agent#28

Closed
rawiriblundell wants to merge 29 commits into
Checkmk:masterfrom
rawiriblundell:merged_nix_agent
Closed

Add merged *nix agent#28
rawiriblundell wants to merge 29 commits into
Checkmk:masterfrom
rawiriblundell:merged_nix_agent

Conversation

@rawiriblundell
Copy link
Copy Markdown
Contributor

@rawiriblundell rawiriblundell commented Jul 25, 2019

In PR #21, Sven mentioned the desire to reduce agent counts down to two and to eliminate bashisms.

This PR provides a merged agent script with all but a small handful of non-POSIX-isms eliminated.

It features:

  • Added vim standards line at top of script
  • Updated homepage URL in copyright header
  • Portability fixes like ensuring that commonly called ENV vars are available, and one or two uncommon ones
  • Auto-builds PATH, primarily to ensure that Solaris uses XPG4 where possible
  • A function to handle the main portability issues of 'echo'
  • Addition of MK_VERSION variable.
  • Addition of MK_OSSTR variable. This one is pretty important.
  • A fix for timeout/waitmax to map it to the busybox variant of timeout (if found)
  • Because 'local' is not POSIX, most functions now have a leading underscore for the variables (e.g. "${_example_var}") to denote local scope, and these are explicitly unset. Implementing this script-wide is incomplete and ongoing.
  • I've made an attempt to provide the desired arg handling in the run_cached() function via getopts
  • The openbsd network interface audit code was a portability disaster, so I've completely overhauled it, it tests fine on a couple of openbsd vm's.
  • Other large blocks of code were portability disasters requiring similar overhauls^[1]
  • I've added the basis of a section_iostat() function to provide those metrics if /proc/diskstats doesn't exist. This needs more work. openbsd does not appear to use the same output format as others, so it's ringfenced.
  • Almost everything is a function now
  • Additional comments on some non-obvious code
  • Countless other things

Things that are known-broken and needing attention:

  • Exporting functions is not POSIX, these might need to be written to temp files
  • Encryption, as process substitution is not defined in POSIX
  • RTC_SECRET variable is modified within a subshell

To-do:

  • So many things. Some are even noted in the code (search for "TO-DO')

/Edit: Finally, I expect that if this accepted and tested sufficiently, that at some point the '.merged' will be dropped and it will simply be 'check_mk_agent'. This 'extension' is simply intended as a differentiator until that point.

^[1]I expect that this will break a few things, so I recommend testing. Lots of testing. It would be insanity to think that 11 years of shell script development could be perfectly untangled at the first attempt, but it's a start.

@rawiriblundell
Copy link
Copy Markdown
Contributor Author

One quick comment about the shebang: /bin/sh is going to fail miserably on Solaris 10 or older, as it's a pre-POSIX shell on those systems. For everything else, /bin/sh should be some variant of ksh, ash, bash or dash, which are all POSIX compliant or near enough. And even then, on older versions of supported systems (and we're talking 1990-1994 era... WHY would you have that in production???), they are all going to come packaged with a ksh88 variant.

In short: if you're using this on Solaris, it'll blow up. Change the shebang to /usr/xpg4/bin/sh or /usr/bin/ksh (or /path/to/bash if you have it installed) and try again. On everything else -
bugs/errors/regressions aside - it should just work (famous last words?)

@jonaskluger
Copy link
Copy Markdown
Contributor

Hello Rawiri,

thank you for this agent file. 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.

Internal Ref: CMK-2913

@rawiriblundell
Copy link
Copy Markdown
Contributor Author

Hi, the force push a few days ago appears to have broken every existing PR's conflict checks. Are you guys able to fix this mess, or would it be easier if the PR's were recreated?

@LarsMichelsen
Copy link
Copy Markdown
Member

Sorry, this mess was made necessary by a required force-push. We had to make some history adjustments to our Github repository (yes, you don't actually...).

To fix this, you would have to rebase your local clones to the upstream branch and skip all commits except your local commits.

rawiriblundell referenced this pull request Dec 6, 2019
There was a escaping problem while constructing the command to be executed
by mrpe when specifing a non-root user.

Change-Id: I1be70e1339bb28e893976db6362b5e9b7d1280e3
@rawiriblundell
Copy link
Copy Markdown
Contributor Author

Thanks Mike,
That confirms that the one outstanding portability issue actually is an issue:

# Make run_cached available for subshells (plugins, local checks, etc.)
# TO-DO: Check if this export is still required
export -f run_cached
+ export -f run_cached
/usr/bin/check_mk_agent.merged[651]: export: bad option(s)

i.e. export -f is not portable.

I'll have a think about a solution for this.

@mike1098
Copy link
Copy Markdown
Contributor

@rawiriblundell As mentioned in the other pull request from you. In AIX 7 /bin/sh and /usr/bin/sh is linked to the korn shell.

@rawiriblundell
Copy link
Copy Markdown
Contributor Author

rawiriblundell commented Jan 21, 2020

Hi @mike1098 , Thanks for that confirmation :)

@rawiriblundell
Copy link
Copy Markdown
Contributor Author

Closing due to lack of engagement from tribe29.

@mo-ki
Copy link
Copy Markdown
Member

mo-ki commented Sep 14, 2021

I am sorry. And I know that doesn't help the cause, but I still want to say that the lack of engagement results from a lack of time, not a lack of interest (speaking for myself, there).
I've come here frequently for reference, but I think the only way we can even come close to this (not even to mention your fork) is by incremental changes. You may have noticed my rather blunt attempt to at least stop five of the agents from diverging further, and be able to incrementally align them.
Closed or not, your first comment contains a valid to-do list.

@rawiriblundell rawiriblundell deleted the merged_nix_agent branch September 15, 2021 12:53
@rawiriblundell
Copy link
Copy Markdown
Contributor Author

@mo-ki I have also attempted the incremental approach and have had mixed, but mostly disappointing results :(

And, I mean, I was in the middle of creating a PR for an incremental change when tribe29 literally told me to not bother:

https://forum.checkmk.com/t/clarity-required-on-prs-and-bugfixes/23988

Otherwise, keep up the good work (speaking to you directly, there :) )

LarsMichelsen pushed a commit that referenced this pull request May 5, 2022
 - cmk.gui.watolib.sites

CMK-10313

Change-Id: I2035c98d28aa065ba5b41b025c26a7dca96d1519
CheckmkCI pushed a commit that referenced this pull request Jul 8, 2024
Change-Id: If132a193e67ff3cf2fd6a70d841d63cb24242f4b
CheckmkCI pushed a commit that referenced this pull request Jul 10, 2024
Change-Id: Icf8b90454d31aaec29c6f9d8904e34ea67a770c3
CheckmkCI pushed a commit that referenced this pull request Aug 21, 2025
Change-Id: I3df9b77a0445bbf3a01f703075b4e2796e713960
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants