-
-
Notifications
You must be signed in to change notification settings - Fork 9k
[JENKINS-68155] Fix state of previous labels when updating a node label #10501
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
Conversation
Prior to jenkinsci#5882, trimLabels was global, so any node operation would cause all labels to be rebuilt. After jenkinsci#5882, the labels to rebuild were taken from the current state of nodes. When updating a node label however, both the old label and the new label needs to be rebuilt. When a node labels are updated, the previous values are now saved aside in order to compute the proper labels to trim.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM.
j.jenkins.updateNode(slave); | ||
label = Label.get("label"); | ||
assertNotNull(label); | ||
assertThat(label.getNodes(), empty()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Expected: an empty collection
but: <[hudson.slaves.DumbSlave[node]]>
without the patch
j.jenkins.updateNode(slave); | ||
label = Label.get("label"); | ||
assertNotNull(label); | ||
assertThat(label.getNodes(), empty()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fails here without the patch
Expected: an empty collection
but: <[hudson.slaves.DumbSlave[node]]>
/label ready-for-merge This PR is now ready for merge, after ~24 hours, we will merge it if there's no negative feedback. |
Prior to #5882,
trimLabels
was global, so any node operation would cause all labels to be rebuilt.After #5882, the labels to rebuild were taken from the current state of nodes. When updating a node label however, both the old label and the new label need to be rebuilt.
When a node labels are updated, the previous values are now saved aside in order to compute the proper labels to trim.
See JENKINS-68155.
Testing done
Proposed changelog entries
Proposed changelog category
/label bug
Proposed upgrade guidelines
N/A
Submitter checklist
@Restricted
or have@since TODO
Javadocs, as appropriate.@Deprecated(since = "TODO")
or@Deprecated(forRemoval = true, since = "TODO")
, if applicable.eval
to 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-needed
label is set and there is a Proposed upgrade guidelines section in the pull request title (see example).lts-candidate
to be considered (see query).