-
Notifications
You must be signed in to change notification settings - Fork 393
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
Test with Jenkins 2.190.1 #467
Test with Jenkins 2.190.1 #467
Conversation
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 provided the build passes :)
<!-- Jenkins 2.190.1 requires this newer version --> | ||
<groupId>org.slf4j</groupId> | ||
<artifactId>log4j-over-slf4j</artifactId> | ||
<version>1.7.26</version> |
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.
FYI there's actually a property slf4j.version
to set this for all slf4j*
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.
I couldn't find the property slf4j.version
in bom 2.138.x. That didn't surprise me too much, since it was not needed (at least in this case) until 2.190.1.
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.
It is in plugin-pom
not bom
. And it should not be necessary to override but we are waiting on jenkinsci/plugin-pom#229.
Build passes on my CI configuration. ci.jenkins.io is offline. I'd like at least one more review before I merge, or will let it sit for a day before I merge it. I've also done a local merge of this change into the master branch and deployed it into my test environment. It ran hundreds of jobs in the last 8 hours with many different configurations. I will double check the results of those jobs today just to assure that this change did not introduce surprise regressions. |
<dependency> | ||
<groupId>org.jenkins-ci.plugins</groupId> | ||
<artifactId>trilead-api</artifactId> | ||
<version>1.0.1</version> |
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.
I think this cannot work so long as your baseline is 2.138.x. This version of trilead-api
requires 2.150.x.
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.
Maybe you meant to use 1.0.0? But I am not even sure if things will work naturally when you are running on any line prior to 2.190.x, as Jenkins core will be bundling Trilead so this dependency would at best be ignored (at worst might mix some different versions of classes in the same package).
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.
Maybe I'm misreading the pom for trilead-api, but I think it declares jenkins.version as 2.89.4.
I'll configure a quick test with Jenkins 2.138 LTS to see if I can confirm that it passes some interactive smoke tests.
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.
Hmm, jenkinsci/trilead-api-plugin@a71565d and jenkinsci/trilead-api-plugin@0fa9517 do not agree. There was a bump to 2.150.1 which occurred prior to one. Did @kuisathaverat botch some releases? Anyway I checked in http://repo.jenkins-ci.org/public/org/jenkins-ci/plugins/trilead-api/1.0.1/ and indeed it seems to be on 2.89.x. Still,
Jenkins core will be bundling Trilead so this dependency would at best be ignored (at worst might mix some different versions of classes in the same package).
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.
@jglick a prototype of a merge of this change to the master branch has been running various authentication tests successfully on multiple agents connected to a Jenkins 2.138.4 master with latest plugin versions supported on that release.
That may only indicate that the tested configurations are just lucky or that I didn't test enough to detect the root problem. Any other recommendations of things to check?
Unfortunately, if this technique is not workable, I'm not sure what path to take to allow plugin compatibility tester to run with git client plugin on Jenkins 2.190.1 while still supporting older versions. Any recommendations?
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.
The trilead-api plugin 1.0.0 is used by ssh-slaves 1.29.1. It was released Nov 2018. That is the version of ssh-slaves in my test installation that is running Jenkins 2.138.4.
Version 1.0.1 of the trilead-api plugin is used by ssh-slaves 1.29.0, also released in Nov 2018.
If someone is using an ssh-slaves plugin from a year ago or newer, doesn't that assure us that the trilead-api plugin is already installed in their environment?
If this is not the preferred compromise to allow plugin compatibility tester to run on Jenkins 2.190.1 with the git client plugin, what alternatives should I be investigating?
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.
ssh-slaves 1.29.1 was another epic fail three days later I've had to release ssh-slaves-1.29.2 there the trilead-api is used only on test, it is still used only on test, so ssh-slaves relay on the trilead-ssh2 lib provided by the core right now. When 2.190 would release I could release an ssh-slaves plugin that depends on trilead-api 1.0.5
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.
Thanks @kuisathaverat for the insights on specific versions of ssh-slaves. I've been the very happy recipient of your improvements to the ssh-slaves plugin. They have made my life easier, especially since I can now manage my Windows agents using ssh.
Your comment seems to support the concern from @jglick that depending on trilead-api 1.0.1 or trilead-api 1.0.0 may not be the best solution to allow us to run a single version of git client plugin in plugin compatibility tester for Jenkins 2.176.4 and for Jenkins 2.190.1.
Any suggestions of alternate approaches?
We could release a git client plugin with minimum Jenkins version 2.190.1, but I think that may surprise or confuse users of earlier Jenkins versions.
We could list the failures as an exception in the Jenkins 2.190.1 plugin compatibility tests, but that seems like it hides the issue rather than resolving it.
We could proceed with the approach in this pull request and use interactive tests to check more thoroughly for issues.
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.
I have the same issue with ssh-slaves, I think the only solution is to bump the minimum core to 1.190.1, I do not want to mess with the Jenkins class loader, I never have luck with it.
Extract a module from the Core it is always a PITA and it is understandable that we have to move forward.
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.
We could list the failures as an exception in the Jenkins 2.190.1 plugin compatibility tests
That is what I may end up doing in jenkinsci/bom#110, unfortunately. Still need to work out how exactly to move that forward.
it hides the issue rather than resolving it
Well…yes, though the “issue” may merely be that PCT is complaining about nothing. The whole system of JenkinsRule
+ PCT has fundamental limitations. See JENKINS-41827 for discussion. PCT does attempt to honor plugin splits and perhaps I just need to fix the BOM build.
Personally my inclination is generally to just keep it simple and bump the Jenkins baseline. Critical bug fixes are easy enough to backport to an older line. If some user of an older line complains about being unable to get the latest minor fixes or features in some plugin, you can point out that they are running a version of Jenkins with acknowledged security vulnerabilities.
<!-- Jenkins 2.190.1 requires this newer version --> | ||
<groupId>org.slf4j</groupId> | ||
<artifactId>log4j-over-slf4j</artifactId> | ||
<version>1.7.26</version> |
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.
It is in plugin-pom
not bom
. And it should not be necessary to override but we are waiting on jenkinsci/plugin-pom#229.
We have the first issue due to the split of trilead JENKINS-59676 |
I'm not clear what causes JENKINS-59676. Can you explain it further for my benefit? If a git client plugin release were created that required Jenkins 2.190.1, it would need to depend on trilead-api. At that point, it could even depend on trilead-api 1.0.5. Would that resolve the bug? If a version of git client plugin were released that depended on trilead-api 1.0.0 or 1.0.1, would that resolve the bug? I see in JENKINS-59676 that the environment report shows that the trilead-api dependency is declared as optional in all cases. Will it increase the chances this technique will work on older releases if I make it optional in git client plugin as well? |
I've tested JENKINS-59676 on a local test environment (https://github.com/kuisathaverat/jenkins-issues/tree/master/JENKINS-59676), and I cannot replicate the issue so we need more details about how he install/update Jenkins |
Based on the description in the comment , it seems the solution in that case was to install the trilead-api plugin into Jenkins 2.190.1. That seems to me that it further supports making the git client plugin explicitly depend on the trilead-api. However, it doesn't yet persuade me that there is any solution other than what @jglick and @kuisathaverat have described, release a new git client plugin version that requires Jenkins 2.190.1 as its base. I'm tempted to make that new version be git client plugin 3.0.0. However, I won't be able to release it for at least 2 weeks. I'm leaving on a week-long vacation in a few days and won't have access to provide quick turn-around fixes when the inevitable surprises happen in git client plugin 3.0.0. |
Enjoy your vacation. I think this can wait. |
Bumps [equalsverifier](https://github.com/jqno/equalsverifier) from 3.1.9 to 3.1.10. - [Changelog](https://github.com/jqno/equalsverifier/blob/master/CHANGELOG.md) - [Commits](jqno/equalsverifier@equalsverifier-3.1.9...equalsverifier-3.1.10) - [Release notes](https://github.com/jqno/equalsverifier/releases)
Do not bundle transitive dependency in jar. The dependency on trilead-api should not bundle transitive dependencies from the trilead-api in this jar. Rely on the trilead-api plugin to provide those dependencies. Make trilead-api dependency optional. Other trilead-api consumers mentioned in JENKINS-59676 seem to all declare it as an optional dependency.
1de050d
to
19c1020
Compare
I'm closing this without merging since @jglick and @kuisathaverat both recommend against the technique. JENKINS-60203 reports this type of class loading problem with JDK 1.8.0_101 with git client plugin 3.0.0 on Windows Server 2016. I can't duplicate the problem and am unwilling to install a JDK version that is almost 4 years old in an attempt to duplicate it. |
Test with Jenkins 2.190.1
Jenkins 2.190.1 is the most recent long term support release. Test with that release instead of 2.176.x.
Checklist
Types of Changes
Additional Comments
Intentionally depends on an older version of trilead-api so that it will not break compatibility with Jenkins 2.138.x. The trilead-api 1.0.2 and 1.0.3 releases require Jenkins 2.150.1. The trilead-api plugin 1.0.4 and 1.0.5 releases require Jenkins 2.184.