Skip to content
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

[JENKINS-71950] List plugins in deterministic order #8453

Merged
merged 1 commit into from
Sep 2, 2023

Conversation

basil
Copy link
Member

@basil basil commented Aug 31, 2023

I am aware of a case where the same set of plugins would lead to one controller starting successfully and one controller failing to start, depending on the order in which the plugins were listed. We currently use file system iteration order, which is undefined. This PR sorts the list by filename before passing the result to the topological sorting function, so every controller with the same set of plugins will now load them in the same order. This should make it easier to debug such problems, because they will either always occur, or they will never occur — but at least the behavior will be deterministic.

Testing done

I added a log statement to print the list of active plugins as consumed by the UberClassLoader and verified that at each level of the topological sort the entries were in lexicographical order.

Proposed changelog entries

List plugins in deterministic order to improve diagnosability of plugin linkage errors.

Proposed upgrade guidelines

N/A

Submitter checklist

Desired reviewers

@mention

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

Maintainer checklist

@basil basil added the rfe For changelog: Minor enhancement. use `major-rfe` for changes to be highlighted label Aug 31, 2023
@MarkEWaite
Copy link
Contributor

This PR is now ready for merge. We will merge it after approximately 24 hours if there is no negative feedback.

@MarkEWaite MarkEWaite added the ready-for-merge The PR is ready to go, and it will be merged soon if there is no negative feedback label Sep 1, 2023
@Dohbedoh
Copy link
Contributor

Dohbedoh commented Sep 1, 2023

@MarkEWaite @basil I thought it be good to create a bug for this, that also has an example of the issue that lead to this https://issues.jenkins.io/browse/JENKINS-71950

@MarkEWaite MarkEWaite changed the title List plugins in deterministic order [JENKINS-71950] List plugins in deterministic order Sep 1, 2023
@MarkEWaite MarkEWaite self-assigned this Sep 1, 2023
@MarkEWaite MarkEWaite merged commit 33dc1fb into jenkinsci:master Sep 2, 2023
15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready-for-merge The PR is ready to go, and it will be merged soon if there is no negative feedback rfe For changelog: Minor enhancement. use `major-rfe` for changes to be highlighted
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants