Fix temporary offline state of computer is lost on config submit#26154
Fix temporary offline state of computer is lost on config submit#26154timja merged 1 commit intojenkinsci:masterfrom
Conversation
69d0799 to
b6e0ea3
Compare
b6e0ea3 to
18abd6a
Compare
|
I've updated the label on the bug report to add |
There was a problem hiding this comment.
Pull request overview
This PR fixes a bug where a computer's temporary offline state was lost when its configuration was submitted. The root cause was that configuration is based on a Computer object while persistence is based on a Node object - when saving configuration, a new Node object is created from form fields, but the temporary offline cause is not part of the form and was therefore lost.
Changes:
- Preserve temporary offline cause during node reconfiguration by saving it before reconfiguration and restoring it to the new node
- Add unit test to verify temporary offline cause persists through configuration roundtrip
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| core/src/main/java/hudson/model/Computer.java | Added code to preserve and restore the temporary offline cause when reconfiguring a node |
| test/src/test/java/hudson/model/ComputerTest.java | Added test case to verify temporary offline cause is preserved through configuration roundtrip |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
MarkEWaite
left a comment
There was a problem hiding this comment.
This PR is now ready for merge. We will merge it after approximately 24 hours if there is no negative feedback.
/label ready-for-merge
|
I assume it also fixes jenkinsci/configuration-as-code-plugin#2682 ? |
|
likely |
…kinsci#26154) (cherry picked from commit 620b902)
…kinsci#26154) (cherry picked from commit 620b902)
…kinsci#26154) (cherry picked from commit 620b902)
Fixes #26146
I suspect that #9855 is the cause for the issue.
I could reproduce the issue in version 2.492.3 so it exists since a while.
Root cause is that the config is based on the a Computer Object while the persistence is based on a Node object. Upon saving the configuration of a computer a new Node object is created based from the form fields but as the cause is not part of the form it is not persisted. Before #9855 the temporaryOfflineCcause was kept in the computer object directly, so a change in config didn't lead to a loss as the Computer object continues to live (might have caused a loss at restart though).
The fix is to set the cause after the new Node object is created before re-establishing the connection between computer and new Node object.
Testing done
Added a unit test that fails without the fix and succeeds with the fix
Screenshots (UI changes only)
Before
After
Proposed changelog entries
Proposed changelog category
/label bug,regression-fix
Proposed upgrade guidelines
N/A
Submitter checklist
@Restrictedor have@since TODOJavadocs, as appropriate.@Deprecated(since = "TODO")or@Deprecated(forRemoval = true, since = "TODO"), if applicable.evalto ease future introduction of Content Security Policy (CSP) directives (see documentation).Desired reviewers
@mention
Before the changes are marked as
ready-for-merge:Maintainer checklist
upgrade-guide-neededlabel is set and there is a Proposed upgrade guidelines section in the pull request title (see example).lts-candidateto be considered.