-
Notifications
You must be signed in to change notification settings - Fork 319
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
Broke with Jackson 2 API v2.12.0 #821
Comments
@felipecrs With what version of Jenkins are you getting this? I think that there was no problems with the latest weekly version (2.268). EDIT: it is broken with the weekly release (2.268) of Jenkins too. |
I'd suggest you take a look in the Jenkins logs - there's probably an exception trace in there somewhere. Also, take screenshots and post them here too - show folks what you're seeing - it'll help us understand what's going on. |
@pjdarton sorry, I know how frustrating it is to get these "this is broken" messages with no specific info of the error.
|
Thanks for that - it's said that, in journalism, "a picture is worth 1000 words" ... but in software, it's "an exception trace is worth 1000 words" 😉 ...so, looking at that exception trace, I don't see any part of the docker-plugin get a mention there (no mention of package FYI the docker-plugin (i.e. this repo's code) depends on the docker-java-api plugin, and that provides the docker-java library to this plugin (and to other docker-related plugins in Jenkins). So, either the docker-java-api plugin needs to tie down jackson to a compatible (earlier!) version, or it needs to use a docker-java library that's happy with the later Jackson ... which may require changes in the docker-java library. i.e. the code that needs to change to fix this isn't in this plugin. |
PS. Make sure that you're using the latest docker-plugin release (with the latest docker-java-api plugin too), as it's more likely that the older versions will be less compatible with newer versions of jackson. |
Latest Jenkins LTS I keep all my plugins up-to-date, so the issue started with the last version of Jackson 2 API as stated in the issue description. There are no other upgrades available in my Jenkins other than Jackson 2 API plugin since I reverted. |
Same error. I created an issue on Jenkins JIRA. |
So... my thinking is that this is one of the following possibilities: One would normally hope for there to be no breaking changes without a change in major version number, so while having a bug in Jackson would (I think) normally be unlikely, this is evidence supporting that idea. I think that, to make further headway, what we're going to need is a full log of the JSON response that Jackson was being asked to parse. So that's the next step, I think: Someone who's seeing this issue needs to put some form of logging proxy between Jenkins and the docker daemon and find out exactly what traffic is sent and returned when this failure happens. Post the result here and that'll then help folks figure out which situation we're facing and hence where we should direct our combined investigatory powers next... |
Experiencing the same here. I rolled back Jackson 2 to 2.11.3 but that brought up a new error for me.
Not sure why it's trying port 32791 when I have it configured on port 2375 |
@thefueley Are you 100% sure that the "new error" is this issue? That's the sort of thing you might see if you're trying to use an SSH connection method to talk to a container that wasn't running sshd at the time. |
@pjdarton You're right. I that's the issue I'm facing now. |
reported to docker-java in docker-java/docker-java#1509 |
Same problem here. Reverting the Jackson 2 API plugin to v2.11.3 solved the problem. |
Most likely cause of the problem I suspect is https://github.com/FasterXML/jackson/wiki/Jackson-Release-2.12#annotation-less-constructor-auto-detection. |
Unfortunately reverting doesn't work in all cases, as there are several other plugins (such as the Kubernetes one) which rely on the latest version of Jackson 2 API. |
And unfortunately the docker-api-plugin (which depends on jackson 2 api) is up for adoption. So a quick fix is not on the way... |
In this case it is needed to downgrade the kubernetes plugin as well. |
The downgrade for kubernetes is not enough, also the new junit plugin needs the new jackson-api. So we can not update misc plugins any more... |
Likely needs a fix in either docker-java or jackson (most likely docker-java). |
This requires a fix in the docker-java. I have similar problems with Jackson 2.12.0. A lot of automatic object mapping from either JSON and XML to Java broke for our usages. Often just applying the correct annotations solved the problems. |
Actually, the right place to be fixed would be in Jackson 2 API, since they pushed a breaking changed without bumping the major version. |
@cowtowncoder FYI I'm not sure you're aware, I suspect the new creator discovery algorithm isn't backwards compatible for all cases. |
jackson uses a versionion scheme where any point release should be considered to have API breaking changes like 2.9.x is not 100% compatible with 2.10.x is is not 100% compatible with 2.11.x and is now is not 100% compatible with 2.12.x |
Makes sense, I thought it was semver 2. Is there any way to pin a dependency at a given version for a plugin? For example, docker-java-api could set jackson 2 2.11 as dependency. |
@jvz fyi |
Any plugin that wishes to bundle Jackson itself can do so using the Breaking up the Jackson API plugin version to indicate the breaking change isn't so great, either, since Jackson 3.0 is still in development as it is (though I'd imagine that might end up as a separate API plugin depending on how it gets packaged) which would make a version mismatch get confusing. |
Will have a look tomorrow into the code. Can't be that hard to fix JSON parsing with the Objectmapper. |
@lpandzic No intentional changes to logic, obviously. All existing tests, regression tests passed between 2.11 and 2.12. All problems reported during 2 release candidates were resolved before 2.12.0 as well; too bad issue here was not encountered by anyone during pre-release testing. But as to issues reported so far regarding 2.12.0, this one about Jackson module auto-registration: FasterXML/jackson-databind#2983 could explain problems with single-constructor-argument implicit Creator method auto-detection. Without reproduction of the problem I cannot comment much more but I wish people resisted jumping to conclusions of there being big intentional breaking change that Jackson project chose to make (compatibility changes that are intended and know are included in release notes like https://github.com/FasterXML/jackson/wiki/Jackson-Release-2.12). It would make sense to first isolate the specific case(s) and see what the root cause is. There is a good chance this could be resolved for 2.12.1 patch. |
w.r.t. "specific case(s)", I can tell you that the "how we got here" trail is as follows...
...but that's the limit of my knowledge at present. |
@pjdarton Ah! That is helpful -- I think the underlying issue then is this one: FasterXML/jackson-databind#2962 which was reported after 2.12.0, has been fixed and will be included in 2.12.1 when it gets released (late December or early January depending how things go). One suggestion for a work-around wrt 2.12.0 (and arguably improves readability) would be to add public Volume(String path) { } which simply makes it explicit that this constructor should also be used, in addition to explicitly annotated factory method. |
My personal preference is for DTO code annotations to provide only one entry point so it's clear to anyone which route in is the one being used. That class already has an annotated factory method, so adding another "Hey, Jackson, you can use this" annotation to provide a second route in just creates ambiguity (especially to those, like myself, who aren't sufficiently familiar with Jackson to determine which is used and which is unnecessary). ...and I can't be certain if that'd be the only bit of code that'd be affected and need tweaking; all we know right now is that's the first place where such an exception is triggered - if we fix it there, we might hit a similar issue in another class, and another etc. ...and even if we did that, a more awkward issues is that we're not using the latest docker-java code, it would be non-trivial to upgrade to using the latest docker-java code, and it'd be unreasonable to ask the docker-java maintainer to create a 3.1.x release with such a change when their focus is on 3.2.x (no maintainer likes doing patches against old code, and I'm not geographically co-located so I can't provide beer as an incentive!) However, what might be a more viable option would be to make a newer Jenkins Jackson plugin containing the as-yet-unreleased Jackson code, so that folks who are suffering from this issue (who can't use 2.11.3 because of other dependencies demanding 2.12) can then use that as a temporary workaround until the official release happens. |
would be good to verify if 2.12.1-SNAPSHOT fixes this for people, would need someone to build this with a patched jackson: https://github.com/jenkinsci/jackson2-api-plugin |
@pjdarton Perhaps I did not explain this clearly... annotation is not adding anything that did not exist prior: both entry points were discovered before 2.12.0: one that takes JSON String (non-annotated constructor), and another that takes JSON Object (annotated factory method, where The unintended change in 2.12.0 was to assume that if ANY factory method were annotated then ALL constructors to be considered must also be annotated (actual existing logic was that if one factory method annotated, all factory method must be; similarly for constructors). So adding annotation would be formally documenting the fact that the constructor is to be used; not to add another entry point but to explicitly specify both, instead of relying on auto-detection for one of them. I agree that this might not be the only case that would require addition. |
How can I build the plugin using the jackson-databind:pom:2.12.1 ? |
@MauricioPenteado You don't build this plugin; what has been suggested is to build the Jenkins jackson2-api-plugin with Jackson 2.12.1 ... but to do that you'll need to grab a 2.12.1-SNAPSHOT pre-release version (because 2.12.1 isn't released yet). I'm not familiar with the jackson2-api-plugin so I don't know if there's any special measures necessary for doing that; I'd guess that if the pre-releases aren't published alongside the main releases, you'd have to manually download a pre-release & put that into your local maven cache, but then you update your jackson2-api-plugin pom to use the pre-release, build it, then install the .hpi in your Jenkins and hopefully that ought to fix things. ...and if you do do that, please do report back here with your findings; there's probably more folks watching this issue than there are actively involved... |
By following comments of @cowtowncoder and @pjdarton, took a plugin build from using both jackson and databind 2.12.1-SNAPSHOT and it seems to fix the issue. All cloud builds started running again. Cheers for the explanations. |
@yavuzd Excellent news! Thanks for testing this; it's much appreciated. |
ist it safe to add jackson2-api.hpi in manage-plugins-advanced tab manually? today i did some updates and jackson was again installed as dependency plugin. There are now a lot dependencies and it is pretty difficult to rollback all the plugins. |
2.12.1 is out FasterXML/jackson-databind#2995 (comment) |
Not recommended to use a zip file uploaded by someone, it's been released and a new release candidate of jackson2-api plugin has been automatically generated. I would suggest using that in the meantime: https://repo.jenkins-ci.org/incrementals/org/jenkins-ci/plugins/jackson2-api/2.12.1-rc174.8fd0dd0459f3/jackson2-api-2.12.1-rc174.8fd0dd0459f3.hpi |
That's technically correct from a security point of view ... but... the official docker-plugin hpi files are (usually) built and uploaded by me and I'm "someone" too. ...and I trusted github, java, maven & release scripts, my PC's CPU's microcode etc, we all trust the Jenkins update site not to let anyone tinker with what was uploaded and the list goes on... I agree that an incrementals build is more "official" than "a zip file uploaded by someone" but saying "not recommended" is a tad harsh IMO. |
merge is done waiting 4 release :) |
release is done. tested it. Docker Slave is working again. Thanks alot :) |
https://github.com/jenkinsci/jackson2-api-plugin/releases/tag/jackson2-api-2.12.1 FTR. |
Tested and the Docker plugin is working properly. |
Works for us as well. Thanks! |
please reopen still producing error with Volumes:
|
@mlechner Original: Your Error: I got the same Plugin Versions but the original error does still not occur. |
After updating Jackson 2 API to v2.12.0, this plugin stopped working. It did not start any container anymore, with an ERROR statement in the agent template section but when I click on it nothing happens.
Reverting the Jackson 2 API plugin to v2.11.3 solved the problem.
The text was updated successfully, but these errors were encountered: