-
Notifications
You must be signed in to change notification settings - Fork 24k
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
fix monit version check #31212
fix monit version check #31212
Conversation
it was erroneously checking > 3.18 instead of > 5.18 Fixes ansible#30614
Also affects 2.4; does not affect 2.3 or below. |
I don't have a way to test that at the moment but that makes sense to me. Although there might be some confusion on the OS package versions vs the actual software version so if it works for you it could've been my mistake ! |
Looking at monit it seems like 5.18 is the correct version so it looks alright to me although I am not a maintainer so I don't know if I can shipit! |
@mangas, thanks for the review. My certainty comes both from testing it and seeing it fixed my problem (which of course is only half the story) but also (and mostly) from the changes documentation for monit 5.18 (https://mmonit.com/monit/changes/#5.18.0) which puts the introduction of colors and For some context, what makes this bug a stopper for me (and I assume others) is that apt-get's main repository (on Ubuntu 14.04 at least) doesn't let you install anything above monit 5.6:
meaning that there's no easily available path to working around this bug by upgrading monit. |
maybe @calfonso could help! |
Just saw your last message. Thanks, @mangas ! @brian-brazil @dstoflet Just confirmed with the author of the original PR (#21891) that this bugfix is correct. Mind taking a look? |
SUMMARY
Fixes #30614. There is logic to distinguish
monit
versions that do and don't support/require the use of-B
to get colorless output; this logic was erroneously checking > 3.18 instead of > 5.18.Current version as the time of this writing is 5.24.0 (https://mmonit.com/monit/) and color support (and consequently support for/requirement of
-B
for colorless output) was introduced in 5.18.0 (https://mmonit.com/monit/changes/). However, currently the major version is being compared with 3 rather than 5. I believe this to be a simple typo.In addition to change 3 to 5, I also changed the expression behind it to be more obvious and the function name to be more explicit.
ISSUE TYPE
COMPONENT NAME
lib/ansible/modules/monitoring/monit.py
ANSIBLE VERSION
ADDITIONAL INFORMATION
Before
After