jenkins icon indicating copy to clipboard operation
jenkins copied to clipboard

Execute groovy script on node-monitor state change

Open mPokornyETM opened this issue 2 years ago • 8 comments

Execute user (admin) defined groovy script when node-monitor change the node state due threshold change.

It helps to automate tasks when Node Monitor change the node state. Ex:

  • Start clean-up job for node, when no more space left,
  • send mail notification to admin-group
  • restart NTP service on node
  • ...

Following callbacks are possible:

  • On-Online change. This callback will be executed, when the node monitor marks the node online.
  • On-Offline change This callback will be executed, when the node monitor marks the node offline.

It will indirect fixed following issues: See JENKINS-24947 See JENKINS-39286 User can read which node is marked offline and the 'offline-cause'. So it will be possible to change status back online.

See JENKINS-2107 Allow to send e-mail on monitor status change

Proposed changelog entries

  • Execute user (admin) defined groovy script when node-monitor change the node state due threshold change.

Proposed upgrade guidelines

N/A

Submitter checklist

  • [ ] (If applicable) Jira issue is well described
  • [ ] Changelog entries and upgrade guidelines are appropriate for the audience affected by the change (users or developer, depending on the change) and are in the imperative mood. Examples
    • Fill-in the Proposed changelog entries section only if there are breaking changes or other changes which may require extra steps from users during the upgrade
  • [ ] Appropriate autotests or explanation to why this change has no tests
  • [ ] New public classes, fields, and methods are annotated with @Restricted or have @since TODO Javadoc, as appropriate.
  • [ ] For dependency updates: links to external changelogs and, if possible, full diffs

Desired reviewers

@mention

Maintainer checklist

Before the changes are marked as ready-for-merge:

  • [ ] There are at least 2 approvals for the pull request and no outstanding requests for change
  • [ ] Conversations in the pull request are over OR it is explicit that a reviewer does not block the change
  • [ ] Changelog entries in the PR title and/or Proposed changelog entries are accurate, human-readable, and in the imperative mood
  • [ ] Proper changelog labels are set so that the changelog can be generated automatically
  • [ ] If the change needs additional upgrade steps from users, upgrade-guide-needed label is set and there is a Proposed upgrade guidelines section in the PR title. (example)
  • [ ] If it would make sense to backport the change to LTS, a Jira issue must exist, be a Bug or Improvement, and be labeled as lts-candidate to be considered (see query).

mPokornyETM avatar Jun 14 '22 20:06 mPokornyETM

This is more or less proof of concept. I know there are missing things like, inline-documentation, documentation, tests, grammar (I known my English is crappy, but my native language is not better .-)) ... But to get the overview it shall be enough. I'm looking forward to any suggestions and if you (the community) are fit with the idea. In my POV i will add also callbacks, when the node goes on/offline, temp-on/offline so you can inform the administrator ASSAP or just kill all executors when nodes is offline or what ever. Also it will be possible to check the node if is ready-to-use (on-online callback). In your case, are the tools installed? git, java version .... But I will spare my time, when somebody find this solution wrong.

mPokornyETM avatar Jun 14 '22 20:06 mPokornyETM

Interesting suggestion. Node monitors are quite terrible and are probably overdue for an improvement.

yes, you are right :-( I mentioned it also here: 26466 It is very old issue but it still makes sense. I think it will be fine, when we create new JEP and describe there, how monitors works (or not) now and how shall works. It will clean clean many of buggy code in core. And I (as jenkins administrator in my company) does not need any of node monitors per default. Means from my POV it can be removed from jenkins core (and pleas all the deprecated code too)

mPokornyETM avatar Jun 14 '22 21:06 mPokornyETM

Still, this particular change would be better off as a plugin. IMO we should expose Groovy less, not more, to users long-term.

Is there any specific reason? We use groovy (in pipelines) as well and are very happy with them, because we can do (almost) any things automatically. we has more the 300 nodes (several OS) and jenkins+groovy allow use fully administration. Therefor I (as jenkins administrator in my company) will be very happy when we has some more hooks possibilities. When the node get online, we need to wait for some actions like (autologin user has been fully logged, or node has display ...) So it will be fine to get some programmatic way for administrators to do some special things. And I does not see any other way (at the moment) as start groovy script. Otherwise you need to create jenkins plugin for every things, which helps only you. (the count of not really maintained plugins is also terrible but, that other point) Please, take this comment as positive-critic. I love jenkins. 🥇

mPokornyETM avatar Jun 14 '22 21:06 mPokornyETM

@mPokornyETM

It's an extremely steep learning curve for beginners, and even experienced users are more likely than not to break something or build something unsafe.

Please note that I'm talking about the out of the box experience specifically. Having this functionality in a plugin (that wouldn't be installed by default) is fine, and enhancing core to support that as needed, in a generic way, would be a welcome addition.

daniel-beck avatar Jun 15 '22 07:06 daniel-beck

@daniel-beck you are right, Maybe with some small changes on core (extension point to call onMonitorChange or something like this) it will be to extract all of them in plugin. Let me call it extend-node-monitoring-plugin. In this case I does not need to change the node-monitoring in drastic way, and I can use other helpful plugins (ex. safe-groovy scripts ...). So it is time to try create my first plugin. This PR will be converted to draft, to make the "possible" feature visible for others and do not disturb your core guy's .

thx for your time

mPokornyETM avatar Jun 15 '22 08:06 mPokornyETM

@mPokornyETM There's no "disturbing" here, we're very happy to see contributions, even if they don't all work out. I look forward to seeing your plugin.

FWIW I'm also not the judge who decides what gets merged into core and what doesn't -- you may want to give it a few more days to see what others think about this PR.

daniel-beck avatar Jun 15 '22 09:06 daniel-beck

From my POV I agree with Daniel, I support an extension point to allow this sort of thing in a plugin but not in core itself

timja avatar Jun 18 '22 15:06 timja

Please take a moment and address the merge conflicts of your pull request. Thanks!

github-actions[bot] avatar Sep 09 '23 03:09 github-actions[bot]