JENKINS-63607 Do not disconnect agent when remoting doesn't match

[jonesbusy]

I'm putting this PR as draft because I have dependency with #189 which change the descriptor to a field and it include some similar changes. Will need to be rebased when merged to main branch

Partially fix JENKINS-63607 to not put the agent offline if there is any mismatch between remoting version

This PR doesn't add any comparison mechanism but only a similar option to not disconnect the agent and keep it online

It also miss some automated test (At least unit for the new field, but I want to check if it's possible to test using JenkinsRule and DumbSlave)

Testing done

• Ensure retro compatibility to disconnect by default (view and when plugin is upgraded)
• Tested with same and different remoting version to ensure correct behavior

The new field

The default behavior to disconnect the agent

Node is not disconnected with option is selected

Submitter checklist

Beta Give feedback

1. Make sure you are opening from a topic/feature/bugfix branch (right side) and not your main branch!
2. Ensure that the pull request title represents the desired changelog entry
3. Please describe what you did
4. Link to relevant issues in GitHub or Jira
5. Link to relevant pull requests, esp. upstream and downstream changes
6. Ensure you have provided tests - that demonstrates feature works or fixes the issue

[mawinter69]

This while PR is not necessary. When a monitor is ignored (it is not checked in the node monitoring config), then the markOffline method in core will not take an agent offline (https://github.com/jenkinsci/jenkins/blob/551043065bc672102fb8b07e8008f959ebc566fa/core/src/main/java/hudson/node_monitors/AbstractNodeMonitorDescriptor.java#L229) . The fact that the java version monitor has an explicit disconnect, is because it has additional configuration. When you put a monitor to ignored it has no configuration and default values will be used. Here you just add configuration to allow to prevent disconnection.
The node monitor overview will always show all monitors (at least for admins), regardless if they are enabled or not.
Also the current change means a regression. You put the logic when to take an agent offline from the monitor method of the descriptor to the data method of the monitor. The problem is that the data method is only called in the UI, i.e. when someone opens the node overview page .../computer. So if someone never does this, the agent will never be taken offline.
This wrong behaviour can be seen for the diskspace monitors as well which I try to fix with jenkinsci/jenkins#8593

[mawinter69]

BTW, the Java version monitor is doing this also wrong.

[jonesbusy]

This while PR is not necessary. When a monitor is ignored (it is not checked in the node monitoring config), then the markOffline method in core will not take an agent offline (https://github.com/jenkinsci/jenkins/blob/551043065bc672102fb8b07e8008f959ebc566fa/core/src/main/java/hudson/node_monitors/AbstractNodeMonitorDescriptor.java#L229) . The fact that the java version monitor has an explicit disconnect, is because it has additional configuration. When you put a monitor to ignored it has no configuration and default values will be used. Here you just add configuration to allow to prevent disconnection. The node monitor overview will always show all monitors (at least for admins), regardless if they are enabled or not. Also the current change means a regression. You put the logic when to take an agent offline from the monitor method of the descriptor to the data method of the monitor. The problem is that the data method is only called in the UI, i.e. when someone opens the node overview page .../computer. So if someone never does this, the agent will never be taken offline. This wrong behaviour can be seen for the diskspace monitors as well which I try to fix with jenkinsci/jenkins#8593

Thanks for this detailed report. I will close the PR then