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-41932, JENKINS-42176] - Update libzfs4j from 0.5 to 0.8 #2776

Merged
merged 3 commits into from Apr 13, 2017

Conversation

@oleg-nenashev
Copy link
Member

oleg-nenashev commented Mar 5, 2017

So this PR updates libzfs from a 8-year-old release (0.5) to the newest one, which has been updated by @kohsuke and @jimklimov in order to get better support of new forks of Solaris.

Full diff: kohsuke/libzfs4j@libzfs-0.5...libzfs-0.8. Reviews are welcome

Changelog entry:

  • JENKINS-41949, JENKINS-41932 - Update LibZFS from 0.5 to 0.8 to fix compatibility issues with ZFS filesystem and illumos distributions.

CC @jimklimov

@jimklimov
Copy link

jimklimov commented Mar 5, 2017

I believe the source URL may have to be changed somewhere. The original version was on java.net, the new one is on github.

@oleg-nenashev
Copy link
Member Author

oleg-nenashev commented Mar 6, 2017

And.... there is a binary compatibility issue in the lib

@oleg-nenashev
Copy link
Member Author

oleg-nenashev commented Mar 6, 2017

@kohsuke @jimklimov so allow() and unallow() methods have been removed by @kohsuke in kohsuke/libzfs4j@4b1b412 . I cannot elaborate the impac, so it is up to you to provide further step guidelines

@jimklimov
Copy link

jimklimov commented Mar 13, 2017

I'd vouch for returning the methods, if only to satisfy Jenkins core's desires (and possibly even active usage), perhaps as togglables in the recent fashion (I think we can even auto-detect presence or lack of them in the run-time host ZFS ABI).

I'm not sure what should be done if they are absent (which seems likely on new platforms) - log and/or noop, or some sort of exception to better notice where in Jenkins or other consumers the rotten codepath is called (and if someone does use that - look for modern alternatives to these calls)? In aim for stability, I'd go for log+noop...

@oleg-nenashev
Copy link
Member Author

oleg-nenashev commented Mar 13, 2017

log+noop is fine for me

@kohsuke
Copy link
Member

kohsuke commented Mar 14, 2017

I don't recall exactly what test failure I saw that led me to remove it. I need to work on this.

I agree the binary compatibility should be restored somehow.

@jimklimov
Copy link

jimklimov commented Mar 14, 2017

I think when I made the original fixes a month ago, I looked and could not find the allow routines in current OpenZFS code, so they must have been gone (or perhaps renamed?) a long time ago :) so I did not look closer at "restoring" them back then.

UPDATE: Routines by this name (zfs_perm_set and zfs_perm_remove) are last present in binaries of Sun Solaris 10u6, and already not present in still-Sun Solaris 10u8 (last Sun release, which is sort of popularly deployed as it still had rather permissive licensing). So newer Solaris and illumos and OpenZFS releases use some other mechanism (maybe same new, maybe different new) to manage the delegation of permissions to manage datasets - one thing for certain, the user-facing feature is still there and it does work (e.g. via CLI tools), so it is a matter of hunting down the needed function names if we need to wrap this in the JAR. Maybe this info is available in 200x history of illumos-gate, as OpenSolaris co-evolved with official Solaris for quite a while (there's probably a commit somewhere there to implement and comment the renaming or refactoring).

@jimklimov
Copy link

jimklimov commented Mar 14, 2017

Posted kohsuke/libzfs4j#6 with my take on this repair.

Looking at code changes, I see that also since libzfs-0.5 the zfs_send and zfs_receive got commented away in kohsuke/libzfs4j@494c719

They might become next nits of the CI tests, but probably not as they are only mentioned in libzfs.java (JNA mappings)...

@jimklimov
Copy link

jimklimov commented Mar 14, 2017

Also, zpool_vdev_name got a new Bool verbose argument in kohsuke/libzfs4j@e9a6178

We have a notion of that difference in libzfs.java (JNA mappings), but the routine does not seem called from the Java classes, so it should not be a big issue.

@oleg-nenashev
Copy link
Member Author

oleg-nenashev commented Mar 15, 2017

@jimklimov Yes, I think we just need to fix the compatibility in the core and then ensure that plugins like https://github.com/jenkinsci/zfs-plugin do not blow up as well. BTW it has been never released from what I see, CC @ndeloof

@ndeloof
Copy link
Member

ndeloof commented Mar 15, 2017

@oleg-nenashev I'm not a maintainer on this one, just had wrote a script to update all plugins as jenkins-ci changed maven infra in 2012

@oleg-nenashev
Copy link
Member Author

oleg-nenashev commented Mar 15, 2017

@ndeloof Yeah, sorry. Will ask @kohsuke what is that and how to proceed with that plugin

@jimklimov
Copy link

jimklimov commented Mar 15, 2017

So, for now we are waiting for kohsuke/libzfs4j#6 to get reviewed to perfection and ultimately merged, then probably this PR will raise the bar to require libzfs-0.8.jar, and rebuild to see if they fit together now :) Right?

@oleg-nenashev
Copy link
Member Author

oleg-nenashev commented Mar 15, 2017

Yep. Once you get a new release, just ping me.
By the way, you can run the Jenkins build locally with a snapshot versions of the library to test if everything else is fine

@oleg-nenashev oleg-nenashev self-assigned this Apr 1, 2017
@oleg-nenashev oleg-nenashev changed the title Update libzfs4j from 0.5 to 0.7 Update libzfs4j from 0.5 to 0.8 Apr 9, 2017
@oleg-nenashev
Copy link
Member Author

oleg-nenashev commented Apr 10, 2017

It is good to go, one PR build has passed.
@jenkinsci/code-reviewers any concerns?

Copy link
Contributor

MarkEWaite left a comment

No objection from me.

Copy link
Member Author

oleg-nenashev left a comment

The latests state is fine for me as well

@oleg-nenashev
Copy link
Member Author

oleg-nenashev commented Apr 11, 2017

@jimklimov I would appreciate if you could help with a proper changelog entry (or entries), which would address ZFS users. If there are existing issues reporting to Jenkins, it would be great to have links as well

@jimklimov
Copy link

jimklimov commented Apr 11, 2017

Hi, I am not sure about links... I did post some issues in Jenkins JIRA, some can be pursued later separately from this story (building on it), but can try some log entry to summarize what we ended up with in the end :)

Subj: Improve portability of Jenkins CI to illumos distros - SunOS descendants based on modern OpenZFS
Kudos: Kohsuke Kawaguchi, Jim Klimov, Oleg Nenashev, Adam Stevko
Details: Since the ZFS was introduced in Sun Solaris over a decade ago, the main interaction API was to use command-line tools. Also an OS-internal native library libzfs.so was available, interfacing those tools to the OS kernel. In absence of other ABIs, a few projects still risked to link against this "uncommitted" library - including the libzfs.jar wrapper employed by Jenkins CI on matching platforms. As time went and Solaris evolved, some function signatures were changed in the library, causing a moderate bit of headache for consumers like this wrapper - but it could be handwaved by requiring an upgrade to the newest release of the single OS. As more time had passed, OpenSolaris and later illumos and numerous open-source distributions had splintered off from Solaris (which went its own proprietary path under Oracle stewardship). Also ZFS was adopted into multiple other operating system kernels (including BSD, Linux and MacOS efforts), cross-pollinating under the foundations of OpenZFS. The libzfs.so library is still an "uncommitted implementation detail" of respective distributions, with random ABI changes causing now a much bigger headache due to fragmentation - because there is no single ZFS-wielding OS that you can require an upgrade to, and because there are dozens of combinations of possible function signatures that can be present in a particular OS deployment. The effect for libzfs.jar and Jenkins CI was that as the CI server booted up on an OS with incompatible function signatures (and began interacting with ZFS datasets), its Java process just dumped core and died - and with evolution of OpenZFS consuming and contributing operating systems, it became more likely than not to expect a wrong single fixed ABI. The accepted solution for libzfs.jar wrapper was to identify which of the wrapped routines have different native ABI signatures "in the wild" and introduce a way to pick and call the correct signature during run-time. While the JAR tries its best to guess the correct set for a given host OS, it includes a way for the end-user (sysadmin) to enforce particular implementation for each such function, using environment variables or java properties - and since such settings can be passed from environment outside the Jenkins web-app, they would survive eventual upgrades of jenkins.war). Also this technique is expandable, to uniformly handle more such cases as the future comes down upon us and the libzfs.jar sources have to be updated again :)
Note that there is more work possible in this area, such as expanding Jenkins ZFS support to operating systems that do not identify as a SunOS, but such improvements are out of the scope for this update.

@jimklimov
Copy link

jimklimov commented Apr 11, 2017

Googled up some links for "Jenkins" and "SunOS" and "ZFS" :)

@oleg-nenashev
Copy link
Member Author

oleg-nenashev commented Apr 11, 2017

@jimklimov Changelog entry is commonly about several sentences. Regarding this text, maybe it worth putting it in a blogpost or to README.md of the library (the latter thing would be definitely useful). If there is a CHANGELOG.md file there, it would be also great.

I propose the following Changelog entry:

WDYT?

@oleg-nenashev
Copy link
Member Author

oleg-nenashev commented Apr 11, 2017

Oh, missed the last comment. Will check how to reference/merge these issues

@oleg-nenashev oleg-nenashev changed the title Update libzfs4j from 0.5 to 0.8 [JENKINS-41949, JENKINS-41932, JENKINS-42176] - Update libzfs4j from 0.5 to 0.8 Apr 11, 2017
@jimklimov
Copy link

jimklimov commented Apr 11, 2017

Well, in particular JENKINS-41949 is NOT covered by this update (bringing ZFS to other platforms - it is even outside the libzfs.jar codebase). Issues 41932 and 42176 are related to this PR (the JAR improvements and making Jenkins use them).

Otherwise, your short entry seems good... as a nit, I think the "filesystem" part is called still ZFS, and OpenZFS is rather an umbrella name for the less tangible entities like the multiple-OS collaboration project and planned legal foundation. Also, these wrapper improvements should help stay on top of evolution with original (Sun/Oracle) Solaris ZFS as well.

@jimklimov jimklimov mentioned this pull request Apr 11, 2017
@oleg-nenashev oleg-nenashev changed the title [JENKINS-41949, JENKINS-41932, JENKINS-42176] - Update libzfs4j from 0.5 to 0.8 [JENKINS-41932, JENKINS-42176] - Update libzfs4j from 0.5 to 0.8 Apr 13, 2017
@oleg-nenashev
Copy link
Member Author

oleg-nenashev commented Apr 13, 2017

@jimklimov Thanks, updated the changelog accordingly. Merging

@oleg-nenashev oleg-nenashev merged commit e9f0995 into jenkinsci:master Apr 13, 2017
1 of 2 checks passed
1 of 2 checks passed
continuous-integration/jenkins/pr-head This commit has test failures
Details
Jenkins This pull request looks good
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

5 participants
You can’t perform that action at this time.