StackStorm / st2packaging-dockerfiles

Dockerfiles used to build and test StackStorm deb/rpm packages
https://github.com/StackStorm/st2-packages
32 stars 33 forks source link

Pin pip to 20.0.2 which is the same version used in StackStorm/st2 repo (support for manylinux2014 wheel format) #103

Closed Kami closed 3 years ago

Kami commented 3 years ago

This pull request pins pip to 20.0.2 which is the same version used in StackStorm/st2 repo.

In https://github.com/StackStorm/st2/pull/5153 I added orjson dependency which utilizes manywheel2014 format which is only supported by pip >= 19.3.

If older version of pip is used, it will try to build dependency from scratch and fail since build tools are not available.

If for some reason we can't use 20.x, using 19.3 should also work for that specific issue.

Kami commented 3 years ago

/cc @armab

arm4b commented 3 years ago

Duplicate of https://github.com/StackStorm/st2packaging-dockerfiles/pull/98

See also https://github.com/StackStorm/st2/pull/5094 https://github.com/StackStorm/st2/pull/5123 https://github.com/StackStorm/st2-packages/pull/684. The short gist is that the pip/virtualenv upgrade is more involved, - it brings a bunch of breaking changes and to avoid last minute surprises the team decided to postpone it and do as a first thing in next v3.5.0 release.

Kami commented 3 years ago

Depending on how long the release takes and how things develop, one option to get that orjson PR passing would also be to upgrade to 19.3 in the mean time (https://pip.pypa.io/en/stable/news/#id303. I believe that version still uses old resolver and should work fine with our existing code.

cognifloyd commented 3 years ago

This is not a duplicate of #98. #98 is a new feature (bump to pip 20.3), whereas this is a bugfix to make the pip version used here consistent with st2 core.

Jumping to pip 20.3 (with the new resolver) involves a lot more changes/thought/scrutiny whereas this is a simple bugfix and I think we should merge it asap.

See also StackStorm-Exchange/ci#102 which does the same for the exchange.

amanda11 commented 3 years ago

Is ST2 using 20.0.2? The reason I ask is that I see it mentioned in st2-run-pack-tests and in the Makefile. But on a live system then the version of pip in /opt/stackstorm/st2/bin and /opt/stackstorm/virtualenvs/st2/bin are 19.1.1? As I think they take the version of pip from virtualenv.

The version of pip in the rpm spec for instance is still 19.1.1 in st2-packages.

(I've tried in the ones that bump up to 20.3.x to make sure that the same version is used throughout - but I won't merge those until after the release freeze).

arm4b commented 3 years ago

Right, the st2 pip version shipped with stackstorm is indeed v19x coming from the virtualenv there: https://github.com/StackStorm/st2/blob/9baae40359c200c003e8c787269ab810b1ac1910/fixed-requirements.txt#L49-L50 and packaging dockerfiles here.

Kami commented 3 years ago

I guess I wasn't explicit enough and there is some confusion which I'll try to clear up.

Yes StackStorm packages indeed ship and use pip 19.1.1 (which is also the version used in those docker files I'm trying to upgrade).

When I was talking agbout pip 20.0.2 I was referring to pip used by StackStorm Makefile targets (tests, etc). and st2-run-pack-tests tool - https://github.com/StackStorm/st2/blob/master/Makefile#L58, https://github.com/StackStorm/st2/blob/master/st2common/bin/st2-run-pack-tests#L201.

In this PR the only version I want and need to update is version used by st2-packages repo when creating virtual environment for packages (I don't want to upgrade pip used inside the created virtual environments itself. if that version also controls that, then we have slightly larger issue, but see my comment below on 19.3).

As @cognifloyd said, 20.x should likely work without issues (IIRC, it doesn't use new resolver yet by default), but if that doesn't work, 19.3.0 should be sufficient for me to get the orjson passing (and I don't think using 19.3 for building packages virtualenv should cause any issue - but we can try it out and see - I assume the fastest and easier way to try that is to merge that and re-run st2-packages build?).

In any case, if there is a chance this will cause issues with the release, I can wait with merging / testing this until the release is out.

cognifloyd commented 3 years ago

This should be part of the 3.4.0 milestone, not 3.5.0.

Shipping one version of pip in production (19.1.1) but using another during testing (20.0.2) is bound to cause installation issues for packages that use newer pip features (such as the manylinux2014 wheel format). And the painful part is that we will ONLY experience these issues in production. We really need to harmonize the pip version BEFORE releasing 3.4.0 or we'll have even more support headaches.

This PR along with these 2 PRs need to be merged ASAP. Waiting for 3.5.0 is not a good option. https://github.com/StackStorm/st2-packages/pull/687 https://github.com/StackStorm-Exchange/ci/pull/102

edit: Looks like we also need to fix the pinned version of virtualenv in st2 core

cognifloyd commented 3 years ago

Right, the st2 pip version shipped with stackstorm is indeed v19x coming from the virtualenv there: https://github.com/StackStorm/st2/blob/9baae40359c200c003e8c787269ab810b1ac1910/fixed-requirements.txt#L49-L50 and packaging dockerfiles here.

Using an older version of virtualenv shouldn't be a problem because in testing we just update the version of pip again after building the virtualenv.

For Exchange CI, we've been using the latest virtualenv with pip 9, so a newer virtualenv should be safe: https://github.com/StackStorm-Exchange/ci/pull/102/files#diff-d9594ce13859fd38321be0359bshouldn't be a prob919781d96d910446d42ef04929c71b837034b1L41

sudo pip install -U "pip>=9.0,<9.1" setuptools virtualenv

That said, if we want to use a version of virtualenv that has the wheel for pip==20.0.2 embedded, we can use virtualenv==20.0.18 (actually, we can use anything from virtualenv>=20.0.0,<=20.0.18 as all of those have the wheel for pip==20.0.2 embedded.)

I suppose this requires another PR on st2 core to update to virtualenv so that pack virtualenvs also get built with the correct version of pip by default. We really do need to make sure we pin things in testing to the same version or version range that we use in production.

Kami commented 3 years ago

Since now I have multiple PRs which depend on orjson opened, I kinda want to merge this change ASAP.

I think of just updating it to use 19.3 for now, since that should work for orjson.

@armab Any objections? Also, once this PR is merged, do build Docker files get automatically build, pushed to Dockerhub and used by the new st2-packages builds?

arm4b commented 3 years ago

Right, the deployment process is automatic here and will affect the current build environments. Considering the code freeze and start of the pre-release testing (https://github.com/StackStorm/discussions/issues/66), we have to wait with any changes.

arm4b commented 3 years ago

Thie pip version bump is not enough here. Virtualenv version should also be updated to the point that ships that specific pip version. So those 2 need to be aligned and consistent across the repos. That'll save us from the random issues.

Similar to this place https://github.com/StackStorm/st2/blob/27a06f6ae8d4cb31c888cd1a15038f9bea87b5f0/fixed-requirements.txt#L49-L50

Kami commented 3 years ago

@armab Yeah, I'm aware of it, but as part of this change, I just want to unblock orjson / wheel2014 related work and get pip in sync in more places.

Virtualenv and pip will be upgraded in #98 (but that one is a bit more invasive).

And from spending way too many hours on this in the last couple of days, pinning virtualenv there (+a bunch of other places) is not actually enough (see my dh-virtualenv change).

So as part of #98, I think we should actually re-adjust our approach a bit and start with tests / assertions - everywhere where we use pip and virtualenv (st2, st2-packages, st2packaging-dockfiles, etc.) we should add assertions that correct version of pip and virtualenv is used in all the places - and I really do mean every single places since there are so many places with versions not in sync.

Before this change, I would have actually thought it was using correct versions inside dh-virtualenv for st2-packages, but it turned out it was using 19.0.1 on Bionic and 21 on Xenial.

Anything less than that is simply not robust enough and error prone.