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

Fix bionicbuild image so we don't bundle two different pip version #104

Merged
merged 6 commits into from
Mar 5, 2021

Conversation

Kami
Copy link
Member

@Kami Kami commented Mar 4, 2021

This pull request fixes weird build issues I saw here - StackStorm/st2-packages#687 (comment).

The issue is that Bionic bundles two pip binaries before this change - system one installed via apt which is the old one and one we install using get-pip which is the correct / new one (20.0.2).

And during st2-packages build process, dh-virtualenv picks system one which won't work with new wheels, etc.

This pull request removes system one to ensure only a single pip (aka ltest 20.0.2 is available).

Now the whole Dockerfile also has left cruft and is more in line with Xenial one (which works fine since we don't install two pip versions tehre).

one which we install using get-pip.

This should fix st2-packages dh-virtualenv step to use correct pip
version.
@Kami

This comment has been minimized.

This way it won't pull pip from system site packages and it will use the
correct version.
@Kami
Copy link
Member Author

Kami commented Mar 5, 2021

OK, I managed to track down the root cause.

The issue is that even though we install pip 20.0.2 everywhere, when dh virtualenv is creating virtualenv, it will use pip which comes with system Python distribution (./usr/local/lib/python3.6/dist-packages/virtualenv_support/pip-19.1.1-py2.py3-none-any.whl) and that's an old version.

To fix this issue and unblock me on my various performance improvements PR, I will make a change to our dh-virtualenv fork, to do the following:

  1. Use --no-pip flag with virtualenv
  2. Copy over pip we manually install to dh virtualenv

I believe that with #98 may not be needed anymore since virtualenv is also upgraded there which means it should use pip which ships with that version of venv - but as mentioned above, for now I just want to unblock myself and fix the build (and #98 is a slightly larger change).

@Kami
Copy link
Member Author

Kami commented Mar 5, 2021

OK, I finally got it to work with dh-virtualenv changes from StackStorm/dh-virtualenv@1cf92b6 and also other changes from st2-packages PR in StackStorm/st2-packages#687.

The whole thing is a bit of a madness. Even though we pin pip in various places (and specify --no-download), it turns out incorrect version was used for actual created dh-virtualenv (19.1.1 was ended up being used on Bionic and 21.0.1 on Xenial).

I tested it locally and I can now build Bionic packages without issue and they use a correct pip version.

[package: st2] dh-virualenv versions info
[package: st2] Using Python: Python 3.6.9
[package: st2] Using pip: pip 20.0.2 from /tmp/st2-20210305-8-5bf8me/st2/debian/st2/opt/stackstorm/st2/lib/python3.6/site-packages/pip (python 3.6)

I also added some logging to that dh-virtualenv change. Without it, it was actually hard / impossible during build to see and assert that correct version was used.

@Kami
Copy link
Member Author

Kami commented Mar 5, 2021

@armab Could I get access to last pass so I can see what is going on with Xenial Docker hub job? (I started failing even before I made any changes to Xenial Dockerfile).

Copy link
Contributor

@amanda11 amanda11 left a comment

Choose a reason for hiding this comment

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

LGTM - just a query, but happy for move of install of python-virtualenv


# We use our dh-virtualenv version, since it fixes shebangd lines rewrites
RUN apt-get -y install \
python-setuptools python-mock && \
python-virtualenv python-setuptools python-mock && \
Copy link
Contributor

Choose a reason for hiding this comment

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

Curious - why the move of python-virtualenv to here from earlier?

Copy link
Member Author

Choose a reason for hiding this comment

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

That was done to make Bionic Dockerfile more similar to the Xenial one.

In fact, first I made more changes so those Dockerfiles were almost the same (also added ppa for Python so we install same versions - right now on Xenial we install Python 3.6.12 and on Bionic we install Python 3.6.9, etc.), but then I decided to remove those changes to keep the diff smaller.

If people feel opinionated about it, I can add them back (they work fine as well).

@Kami
Copy link
Member Author

Kami commented Mar 5, 2021

Dockerhub build issue is temporary apt mirror issue:

�[91mFailed to fetch http://archive.ubuntu.com/ubuntu/pool/universe/p/python-virtualenv/python-virtualenv_15.0.1+ds-3ubuntu1_all.deb 404 Not Found [IP: 91.189.88.142 80]

Work cast, to get myself unblocked I may just pin it to a fixed apt mirror in Dockerfile.

Copy link
Member

@cognifloyd cognifloyd left a comment

Choose a reason for hiding this comment

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

LGTM

@Kami Kami merged commit a70cf56 into master Mar 5, 2021
@Kami Kami deleted the more_upgrade_pip branch March 5, 2021 22:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants