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

Removed Python 2 and Python 3.5- support #12925

Merged
merged 1 commit into from
Jan 13, 2022
Merged

Removed Python 2 and Python 3.5- support #12925

merged 1 commit into from
Jan 13, 2022

Conversation

szubersk
Copy link
Contributor

@szubersk szubersk commented Dec 31, 2021

Description

Deprecation of Python versions below 3.6 gives opportunity to unify the
build and install requirements for OpenZFS packages. The minimal
supported Python version is 3.6 as this is the most recent Python
package CentOS/RHEL 7 users can get.

How Has This Been Tested?

$ make pkg-kmod pkg-utils

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Performance enhancement (non-breaking change which improves efficiency)
  • Code cleanup (non-breaking change which makes code smaller or more readable)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Library ABI change (libzfs, libzfs_core, libnvpair, libuutil and libzfsbootenv)
  • Documentation (a change to man pages or other documentation)

Checklist:

@szubersk
Copy link
Contributor Author

@behlendorf @ahrens
Is it worth to push through with this change?
Python 2.7 is long-time gone, so is Python 3.5 and below. CentOS 7 offers Python 3, CentOS 6 is gone anyways. Some of the testcases require Python 3.5 anyways. Feels like unification to Python 3.6+ simplifies things, I'm not sure if there are requirements to support Python 2 still that I am not aware of.

rpm/generic/zfs.spec.in Outdated Show resolved Hide resolved
@Evernow
Copy link

Evernow commented Dec 31, 2021

Note that Python 3.6 is end of life and no longer receiving any security updates upstream. May be worth pushing for at least Python 3.7 instead.

@rincebrain
Copy link
Contributor

Much as I'm sure Python upstream would love for everyone to require 3.10 in all their projects, dropping pre-3.6 already breaks a number of the CI targets; dropping 3.6 would break Ubuntu 18.04 as well, for example.

Dropping Python 2 I can get behind, because at this point everyone's shipped Python3 in their distros. Dropping Python 3 (and some CI targets with it) seems like a heavier step.

@szubersk
Copy link
Contributor Author

This change is very much immature and needs re-work. I'll address comments on particulars later on. The main reason I posted it is to fish for feedback about migrating to Python 3. I am very thankful for such a quick (and positive so far :)) one.

While Python 3.6 was just deprecated this Christmas, I'm afraid we cannot go above Python 3.6 without breaking --enable-pyzfs builds on CentOS/RHEL 7. Those distros are going to be supported for 2+ more years and losing audience there doesn't seem like a good idea.

@szubersk szubersk changed the title Removed Python 2 support. Python 3.6+ is required Removed Python 2 and Python 3.5- support Jan 1, 2022
@szubersk szubersk marked this pull request as ready for review January 1, 2022 15:55
@szubersk
Copy link
Contributor Author

szubersk commented Jan 1, 2022

Looks like the PR is ready for a review.

We still have 2 failing checks though: Debian 8 arm and Ubuntu 16.04 i386, both due to Python 3.5 installed there

checking for python3... python3
checking for python version... 3.5
checking for python platform... linux
checking for python script directory... ${prefix}/lib/python3.5/site-packages
checking for python extension module directory... ${exec_prefix}/lib/python3.5/site-packages
configure: error: "Python >= 3.6 is required"

A cheap workaround would be to explicitly pass --disable-pyzfs to configure on those systems or avoid installing Python 3 at all (e. g. like in Debian 8 ppc64). A better solution would be to revise if we should still support Debian 8 and Ubuntu 16.04, both of them are deprecated. From my PoV it would make sense to support following OS-es:

  • Debian 10 and 11 (Debian 9 will be soon deprecated)
  • Ubuntu 18.04 and 20.04
  • CentOS 7 and 8
  • Rocky Linux 8
  • FreeBSD 13 and current
  • Fedora 34 and 35 (2 most recent releases)

@nabijaczleweli
Copy link
Contributor

Yeah, wasn't the last time I? or rincebrain? made one of these the consensus that it can't pass because it'd break the oldest CentOS we still support?

@behlendorf
Copy link
Contributor

Just for reference, our policy has always been to retain support for RHEL and Ubuntu LTS releases until they go EOL (excluding extended support). Applying that criteria we need to continue supporting RHEL 7, but can drop support for Ubuntu 16.04 which went EOL in 2021. So I agree with @szubersk's list, although I'd add FreeBSD 12 since it's supported until Jun 2024.

Which is all long way of saying I'm on board with this change since everything on that list provides Python 3.6 or newer.

As for the failing builders let me see about upgrading the Ubuntu 16.04 i386 builder to 18.04 which still has 32-bit support. As for the ppc64 system we can look in to if there's a newer python available to install. If not --disable-pyzfs may be an option, we don't actually care about supporting Debian 8. What we do care about is having a ppc64 system to verify the build on.

@behlendorf behlendorf added the Status: Code Review Needed Ready for review and testing label Jan 5, 2022
@rincebrain
Copy link
Contributor

rincebrain commented Jan 5, 2022 via email

@nabijaczleweli
Copy link
Contributor

nabijaczleweli commented Jan 5, 2022

Hey, I'd be extatic to run some sort of big endian system, but, like, they all went extinct, POWER 8+ is prohibitively expensive (and <8 is landfill), and QEMU, even on a relatively powerful amd64 machine, is so brain-numbingly slow that any experimentation, or, indeed, building ZFS is impossible.

Ad rem: even if you do strap a pure ppc64 system from ports, you're still running a ppc64le kernel (not unlike x32 with its amd64 kernel, I guess) – this has the added benefit of the "and the kernel and userspace must agree." comment in lib/libspl/os/linux/gethostid.c not holding, so any actual use beyond building (which the current buildd seems to do) is likely to explode a bit.

@rincebrain
Copy link
Contributor

Are you sure FBSD's shipping a ppc64le kernel in the PPC64 image? Because they seem to claim it'll run on pure BE systems, and an LE kernel seems to contradict that statement...

(As I've mentioned elsewhere, in addition to a collection of qemu VMs, which are, indeed, quite slow and sad, I've got a couple of actual sparc64 systems sitting on and under my couch.)

@behlendorf
Copy link
Contributor

Wait, actually the Debian 8 ppc64 build appears to be passing because python isn't installed and it disables support entirely. It's the Debian 8 arm system which is causing the failure, we can probably just do the same thing there for now.

Migrating to something newer for these Debian 8 installs will need to happen at some point. They're still using a 3.16 kernel and we'll likely want to drop support for that when CentOS 7 goes EOL in a few years.

Adding a real big endian ppc64 system and running the test suite on it would be great, but it's something we'd end up wanting to do in a VM since crashing the node is always a possibility. This is why we only compile test today, which is better than nothing.

@nabijaczleweli
Copy link
Contributor

That was Re: debian-ports, I assume FreeBSD ppc64 is uniform, as you said. I guess it also won't expose that bug since it only appears on heterogenous systems.
(And, oh, what a dream!)

@rincebrain
Copy link
Contributor

I can't find any evidence that what you're claiming about Debian's ppc64 kernels is accurate?

They don't use the same packages or binaries, the ppc64 wiki article claims they run on systems that don't do ppc64le, they differ in actual size by about 50%, everything on boot on my VM reports ppc64 not ppc64{le,el}...

Where are you getting this conclusion?

@nabijaczleweli
Copy link
Contributor

nabijaczleweli commented Jan 6, 2022

ugh, you're right, of course, soz; I seem to've extrapolated from the packages.d.o src:linux binaries list (which I could've sworn used to list binaries for ports arches in src: pages before, too? the ports.d.o/d-ports pool doesn't lie, however)

@behlendorf
Copy link
Contributor

@szubersk would you mind force updating this PR to kick off a new CI run. The builders which were failing have been updated to new releases with a modern enough version of python.

Debian 8 arm -> Ubuntu 20.04 arm
Ubuntu 16.04 i386 -> Ubuntu 18.04 i386 (the last 32-bit Ubuntu release)

@szubersk
Copy link
Contributor Author

szubersk commented Jan 7, 2022

@behlendorf Seems like the VMs were either changed manually or provisioned with different version of buildbot scripts. Indeed, OpenZFS built OK on older versions of Linux because of complete lack of Python 3.

No FreeBSD (TEST) check run zfs-tests, merely built the project. Not sure if that was intentional.

When it comes to distro choices:

  • choosing Ubuntu as buildbot worker seems like re-work since Ubuntu is already covered by GitHub workflows (18.04 and 20.04). I think Debian would be a better choice since quite a few distros are derived from it
  • FreeBSD started OpenZFS support from release 13.0 onwards, feels like processing power of FreeBSD 12.0 VM could go elsewhere
  • is i386 support still relevant nowadays?
  • I'd suggest marking FreeBSD as required (at least (BUILD) flavor), clang is used there (vs. gcc in Linux) and we had PRs addressing compilation warnings. It would be a pity to have them come back.

I must say I'm very happy to see you and the contributors being so open to suggestions. It's a pleasure to work with you guys.

@behlendorf
Copy link
Contributor

@szubersk

choosing Ubuntu as buildbot worker seems like re-work since Ubuntu is already covered by GitHub workflows (18.04 and 20.04).

It is in some ways, but the main purpose of most of the BUILD builders is just to verify a clean build of several interesting architectures (amd64, i386, ppc64, arm, and aarch64). What distribution that happens with isn't really so important, Ubuntu just happened to be convenient and low maintenance.

FreeBSD started OpenZFS support from release 13.0 onwards.

We could drop this, it was critical during the initial port and we opted to keep it for now since it hasn't been too much of a burden to maintain.

is i386 support still relevant nowadays?

Not for quite a while now, but retaining the builder does ensure we at least don't accidentally break the 32-bit build. That's useful for distributions which integrate OpenZFS and still provide a i386 iso (FreeBSD for example).

I'd suggest marking FreeBSD as required (at least (BUILD) flavor), clang is used there (vs. gcc in Linux) and we had PRs addressing compilation warnings. It would be a pity to have them come back.

Done.

@behlendorf behlendorf requested a review from jwk404 January 7, 2022 18:22
Deprecation of Python versions below 3.6 gives opportunity to unify the
build and install requirements for OpenZFS packages. The minimal
supported Python version is 3.6 as this is the most recent Python
package CentOS/RHEL 7 users can get.

Signed-off-by: szubersk <[email protected]>
@szubersk szubersk requested a review from rincebrain January 13, 2022 08:38
Copy link
Contributor

@rincebrain rincebrain left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just hope this doesn't become something a bunch of distros revert...

@szubersk
Copy link
Contributor Author

I just hope this doesn't become something a bunch of distros revert...

Looks like:

Seems we're safe.

@jwk404 jwk404 merged commit 8a7c4ef into openzfs:master Jan 13, 2022
@szubersk szubersk deleted the szubersk-python2 branch January 14, 2022 10:17
@behlendorf
Copy link
Contributor

Seems we're safe.

@szubersk well almost safe. We appear not to have noticed that with this change the FreeBSD tests were skipped but a pass was reported by the CI. Would you mind looking taking a look?

@szubersk
Copy link
Contributor Author

szubersk commented Jan 21, 2022

@behlendorf
I would tell more once I reproduce the issue in my VM. Would you mind sharing /etc/buildslave from FreeBSD main?

@rincebrain
Copy link
Contributor

rincebrain commented Jan 21, 2022 via email

@szubersk
Copy link
Contributor Author

szubersk commented Jan 23, 2022

@behlendorf
Rectified by #12990 and #12998, thanks to @rincebrain and @nabijaczleweli.

I'd still insist on publishing /etc/buildslave from buildslaves to make future environment provisioning by developers easier.

@behlendorf
Copy link
Contributor

@szubersk thanks. Rather than publishing the /etc/buildslave file, I think what you really want to know is what ec2 AMIs are used for the testing. Those can be found here in the buildbot config. We could add some more information for the non-ec2 BUILD builders if that'd be helpful.

@szubersk
Copy link
Contributor Author

@behlendorf, the thing is, bb-*.sh scripts rely on BB_* variables injected from that file

[dev@vm ~]$ grep -hEo 'BB_[^[:space:]"=/]+' bb-*.sh | sort -u
BB_DIR
BB_MODE
BB_NAME
BB_SHUTDOWN

I had to guess those values to be able to reconstruct the environment (by fetching and executing bb-*.sh). Knowing AMIs is not that important (although helpful), e. g. for FreeBSD-current I used vanilla installation ISO.

tonyhutter pushed a commit to tonyhutter/zfs that referenced this pull request Feb 10, 2022
Deprecation of Python versions below 3.6 gives opportunity to unify the
build and install requirements for OpenZFS packages. The minimal
supported Python version is 3.6 as this is the most recent Python
package CentOS/RHEL 7 users can get.

Reviewed-by: Brian Behlendorf <[email protected]>
Reviewed-by: Rich Ercolani <[email protected]>
Reviewed-by: John Kennedy <[email protected]>
Signed-off-by: szubersk <[email protected]>
Closes openzfs#12925
nicman23 pushed a commit to nicman23/zfs that referenced this pull request Aug 22, 2022
Deprecation of Python versions below 3.6 gives opportunity to unify the
build and install requirements for OpenZFS packages. The minimal
supported Python version is 3.6 as this is the most recent Python
package CentOS/RHEL 7 users can get.

Reviewed-by: Brian Behlendorf <[email protected]>
Reviewed-by: Rich Ercolani <[email protected]>
Reviewed-by: John Kennedy <[email protected]>
Signed-off-by: szubersk <[email protected]>
Closes openzfs#12925
nicman23 pushed a commit to nicman23/zfs that referenced this pull request Aug 22, 2022
Deprecation of Python versions below 3.6 gives opportunity to unify the
build and install requirements for OpenZFS packages. The minimal
supported Python version is 3.6 as this is the most recent Python
package CentOS/RHEL 7 users can get.

Reviewed-by: Brian Behlendorf <[email protected]>
Reviewed-by: Rich Ercolani <[email protected]>
Reviewed-by: John Kennedy <[email protected]>
Signed-off-by: szubersk <[email protected]>
Closes openzfs#12925
usaleem-ix pushed a commit to truenas/zfs that referenced this pull request Dec 16, 2022
Deprecation of Python versions below 3.6 gives opportunity to unify the
build and install requirements for OpenZFS packages. The minimal
supported Python version is 3.6 as this is the most recent Python
package CentOS/RHEL 7 users can get.

Reviewed-by: Brian Behlendorf <[email protected]>
Reviewed-by: Rich Ercolani <[email protected]>
Reviewed-by: John Kennedy <[email protected]>
Signed-off-by: szubersk <[email protected]>
Closes openzfs#12925
tonyhutter pushed a commit to tonyhutter/zfs that referenced this pull request Apr 13, 2023
Deprecation of Python versions below 3.6 gives opportunity to unify the
build and install requirements for OpenZFS packages. The minimal
supported Python version is 3.6 as this is the most recent Python
package CentOS/RHEL 7 users can get.

Reviewed-by: Brian Behlendorf <[email protected]>
Reviewed-by: Rich Ercolani <[email protected]>
Reviewed-by: John Kennedy <[email protected]>
Signed-off-by: szubersk <[email protected]>
Closes openzfs#12925
nkeor added a commit to nkeor/archzfs that referenced this pull request Apr 15, 2023
Drop python2 from the test requirements, see openzfs/zfs#12925
minextu pushed a commit to archzfs/archzfs that referenced this pull request Apr 17, 2023
Drop python2 from the test requirements, see openzfs/zfs#12925
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status: Code Review Needed Ready for review and testing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants