1304176 - Use virtualenv/pip from source checkout, if available

Status: RESOLVED FIXED

(Release Engineering :: Applications: MozharnessCore, defect)

Description

[Gregory Szorc [:gps]]

I've got some scope bloat I wrote as part of bug 1286900 related to virtualenv/pip management foo. It makes virtualenv/pip foo faster and more robust.

Comment 1

[Gregory Szorc [:gps]]

Attachment: Bug 1304176 - Set BaseScript.topsrcdir if we have a source checkout;

We're going to start executing more mozharness scripts from a source
checkout. Rather than add config options to specify the location of
a source checkout - something that must be added to every mozharness
invocation - we teach BaseScript.__init__ to recognize when we're
running from a source checkout and set self.topsrcdir accordingly.

This will allow any script or class to check for self.topsrcdir
and change behavior accordingly.

Review commit: https://reviewboard.mozilla.org/r/79858/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/79858/

Comment 2

[Gregory Szorc [:gps]]

Attachment: Bug 1304176 - Use vendored tooltool.py if available;

Now that we can detect when we're running from a source checkout,
we can start using things from source checkouts instead of relying
on host machine state or grabbing files from another server.

We start by using the vendored tooltool.py if available. This
avoids non-determinism. It avoids a possible 3rd party hosting
dependency on github.com. It avoids a possible MitM attack vector.
Wins all around.

Review commit: https://reviewboard.mozilla.org/r/79860/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/79860/

Comment 3

[Gregory Szorc [:gps]]

Attachment: Bug 1304176 - Use vendored virtualenv if available;

If mozharness is running from a source checkout, it has access to a
modern virtualenv+pip/setuptools vendored as part of the source
checkout.

This commit changes the virtualenv creation code to use the vendored
virtualenv when it is available.

A side effect of this change is that a modern version of pip will
now be used by mozharness when a source checkout is available. This
has a number of consequences.

First, modern versions of pip automatically create and cache wheels
when building packages. This should make automation faster since it can
now reuse cached wheels instead of having to download and rebuild
packages all the time.

Second, modern versions of pip support pinning package hashes. This
opens the door to use having more secure package downloads and more
determinism in our test environment.

Third, modern versions of pip require connections to package servers
be secure by default. Plaintext connections are disallowed by
default. A --trusted-host option or environment variable can be used
to override this behavior.

Since upgrading pip resulted in some jobs failing due to disallowed
connections to insecure servers, I've added code to sniff the pip
version and add --trusted-host where it is needed/supported. This
retains the existing behavior. This is insecure. But fixing that is
for another bug.

Review commit: https://reviewboard.mozilla.org/r/79862/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/79862/

Comment 4

[Gregory Szorc [:gps]]

Attachment: Bug 1304176 - Remove PIP_TRUSTED_HOST and trust-host pip.conf option;

The Python code is now intelligent enough to add this flag on the
command line if supported. Eliminate the copy pasta and help prevent
cargo culting.

Review commit: https://reviewboard.mozilla.org/r/79882/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/79882/

Comment 5

[Gregory Szorc [:gps]]

Attachment: Bug 1304176 - Remove --venv-path as an alias to --virtualenv-path;

Not sure why we support this. The code goes all the way back to the
import of mozharness 0.4 into the old mozharness Mercurial repo in
bug 651974.

Having fewer variations makes it easier to search for usage. So nuke
the variation.

Review commit: https://reviewboard.mozilla.org/r/79884/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/79884/

Comment 6

[Gregory Szorc [:gps]]

Attachment: Bug 1304176 - Document and refactor query_virtualenv_path;

We don't need a variable to hold the result. Just use return.

The "virtualenv_path" option has a default value, so it should always
be set. Add code confirming that. And refactor the code to use
less indentation. And remove a branch that can never occur since
the virtualenv path is guaranteed to be defined.

Review commit: https://reviewboard.mozilla.org/r/79886/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/79886/

Comment 7

[Gregory Szorc [:gps]]

Attachment: Bug 1304176 - Remove path to virtualenv.py in mozharness configs;

For mozharness scripts running out of a source checkout, we now use
the virtualenv vendored in the source checkout. So there's no need to
define the path to virtualenv.py in mozharness configs.

There are probably more mozharness configs that could have the
virtualenv path removed. These are the ones that didn't require me
think very hard.

Review commit: https://reviewboard.mozilla.org/r/79888/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/79888/

Comment 8

[Gregory Szorc [:gps]]

Comment on attachment 8793049 [details]
Bug 1304176 - Set BaseScript.topsrcdir if we have a source checkout;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/79858/diff/1-2/

Comment 9

[Gregory Szorc [:gps]]

Comment on attachment 8793050 [details]
Bug 1304176 - Use vendored tooltool.py if available;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/79860/diff/1-2/

Comment 10

[Gregory Szorc [:gps]]

Comment on attachment 8793051 [details]
Bug 1304176 - Use vendored virtualenv if available;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/79862/diff/1-2/

Comment 11

[Gregory Szorc [:gps]]

Comment on attachment 8793109 [details]
Bug 1304176 - Remove PIP_TRUSTED_HOST and trust-host pip.conf option;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/79882/diff/1-2/

Comment 12

[Gregory Szorc [:gps]]

Comment on attachment 8793110 [details]
Bug 1304176 - Remove --venv-path as an alias to --virtualenv-path;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/79884/diff/1-2/

Comment 13

[Gregory Szorc [:gps]]

Comment on attachment 8793111 [details]
Bug 1304176 - Document and refactor query_virtualenv_path;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/79886/diff/1-2/

Comment 14

[Gregory Szorc [:gps]]

Comment on attachment 8793051 [details]
Bug 1304176 - Use vendored virtualenv if available;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/79862/diff/2-3/

Comment 15

[Gregory Szorc [:gps]]

Comment on attachment 8793109 [details]
Bug 1304176 - Remove PIP_TRUSTED_HOST and trust-host pip.conf option;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/79882/diff/2-3/

Comment 16

[Gregory Szorc [:gps]]

Comment on attachment 8793110 [details]
Bug 1304176 - Remove --venv-path as an alias to --virtualenv-path;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/79884/diff/2-3/

Comment 17

[Gregory Szorc [:gps]]

Comment on attachment 8793111 [details]
Bug 1304176 - Document and refactor query_virtualenv_path;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/79886/diff/2-3/

Comment 18

[Gregory Szorc [:gps]]

Attachment: Bug 1304176 - Move pip find-links defaults out of mozharness configs;

Nearly every job installing things from pip defines a custom find_links
config option listing the same 2 URLs to Mozilla's pip mirrors. There
are a few problems with this.

First, it is excessive typing and config noise. If we always have a
config, it should be the default.

Second, recent changes to use modern pip and always define --trusted-host
when needed cause a regression under certain circumstances. When the
private pypi host is not available, pip wastes several seconds
attempting to connect and reconnect to it before moving on to the
public host. This caused pip operations to slow down. As part of moving
the links URLs to the Python, we put the public server first.

Review commit: https://reviewboard.mozilla.org/r/80158/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/80158/

Comment 19

[Gregory Szorc [:gps]]

Comment on attachment 8793049 [details]
Bug 1304176 - Set BaseScript.topsrcdir if we have a source checkout;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/79858/diff/2-3/

Comment 20

[Gregory Szorc [:gps]]

Comment on attachment 8793050 [details]
Bug 1304176 - Use vendored tooltool.py if available;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/79860/diff/2-3/

Comment 21

[Gregory Szorc [:gps]]

Comment on attachment 8793051 [details]
Bug 1304176 - Use vendored virtualenv if available;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/79862/diff/3-4/

Comment 22

[Gregory Szorc [:gps]]

Comment on attachment 8793109 [details]
Bug 1304176 - Remove PIP_TRUSTED_HOST and trust-host pip.conf option;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/79882/diff/3-4/

Comment 23

[Gregory Szorc [:gps]]

Comment on attachment 8793110 [details]
Bug 1304176 - Remove --venv-path as an alias to --virtualenv-path;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/79884/diff/3-4/

Comment 24

[Gregory Szorc [:gps]]

Comment on attachment 8793111 [details]
Bug 1304176 - Document and refactor query_virtualenv_path;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/79886/diff/3-4/

Comment 25

[Gregory Szorc [:gps]]

Attachment: Bug 1304176 - Install psutil and mozsystemmonitor from source checkout;

Automation spends time downloading psutil and mozsystemmonitor from an
external host. These packages are vendored in the source repo.

This commit changes mozharness to use the vendored copy if available.

Review commit: https://reviewboard.mozilla.org/r/80176/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/80176/

Comment 26

[Gregory Szorc [:gps]]

Comment on attachment 8793427 [details]
Bug 1304176 - Move pip find-links defaults out of mozharness configs;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/80158/diff/1-2/

Comment 28

[(not currently active) Ted Mielczarek]

Comment on attachment 8793049 [details]
Bug 1304176 - Set BaseScript.topsrcdir if we have a source checkout;

https://reviewboard.mozilla.org/r/79858/#review79434

::: testing/mozharness/mozharness/base/script.py:1789
(Diff revision 3)
>          self.script_obj = self
>  
> +        # Indicate we're a source checkout if VCS directory is present at the
> +        # appropriate place. This assumes we're at
> +        # testing/mozharness/mozharness/base/script.py.
> +        here = os.path.normpath(os.path.dirname(__file__))

It'd be good to `assert(here.endswith('testing/mozharness/mozharness/base/script.py'))` to make sure this doesn't get broken in a file move.

Comment 29

[(not currently active) Ted Mielczarek]

Comment on attachment 8793050 [details]
Bug 1304176 - Use vendored tooltool.py if available;

https://reviewboard.mozilla.org/r/79860/#review79440

::: testing/mozharness/mozharness/mozilla/tooltool.py:49
(Diff revision 3)
>          if os.path.exists(fn):
>              return fn
>  
>      def tooltool_fetch(self, manifest,
>                         output_dir=None, privileged=False, cache=None):
>          """docstring for tooltool_fetch"""

LOL

Comment 30

[(not currently active) Ted Mielczarek]

Comment on attachment 8793050 [details]
Bug 1304176 - Use vendored tooltool.py if available;

https://reviewboard.mozilla.org/r/79860/#review79442

Comment 31

[(not currently active) Ted Mielczarek]

Comment on attachment 8793051 [details]
Bug 1304176 - Use vendored virtualenv if available;

https://reviewboard.mozilla.org/r/79862/#review79444

::: testing/mozharness/mozharness/base/python.py:376
(Diff revision 4)
> +            ]
> +            virtualenv_options = c.get('virtualenv_options', [])
> +            # Don't create symlinks. If we don't do this, permissions issues may
> +            # hinder virtualenv creation or operation. Ideally we should do this
> +            # below when using the system virtualenv. However, this is a newer
> +            # feature and isn't guaranteed to be supported.

Is this because we have bad permissions on some of our vendored packages or what?

Comment 32

[(not currently active) Ted Mielczarek]

Comment on attachment 8793109 [details]
Bug 1304176 - Remove PIP_TRUSTED_HOST and trust-host pip.conf option;

https://reviewboard.mozilla.org/r/79882/#review79446

Comment 33

[(not currently active) Ted Mielczarek]

Comment on attachment 8793110 [details]
Bug 1304176 - Remove --venv-path as an alias to --virtualenv-path;

https://reviewboard.mozilla.org/r/79884/#review79450

Comment 34

[(not currently active) Ted Mielczarek]

Comment on attachment 8793111 [details]
Bug 1304176 - Document and refactor query_virtualenv_path;

https://reviewboard.mozilla.org/r/79886/#review79452

Comment 35

[Gregory Szorc [:gps]]

Comment on attachment 8793051 [details]
Bug 1304176 - Use vendored virtualenv if available;

https://reviewboard.mozilla.org/r/79862/#review79444

> Is this because we have bad permissions on some of our vendored packages or what?

I have a suspicion it is a bug in virtualenv. I think it is creating a symlink in the virtualenv to the system-managed site-packages directory where distutils is installed. It then attempts to overwrite that file, yielding a permissions failure.

Comment 36

[Gregory Szorc [:gps]]

Comment on attachment 8793049 [details]
Bug 1304176 - Set BaseScript.topsrcdir if we have a source checkout;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/79858/diff/3-4/

Comment 37

[Gregory Szorc [:gps]]

Comment on attachment 8793050 [details]
Bug 1304176 - Use vendored tooltool.py if available;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/79860/diff/3-4/

Comment 38

[Gregory Szorc [:gps]]

Comment on attachment 8793051 [details]
Bug 1304176 - Use vendored virtualenv if available;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/79862/diff/4-5/

Comment 39

[Gregory Szorc [:gps]]

Comment on attachment 8793109 [details]
Bug 1304176 - Remove PIP_TRUSTED_HOST and trust-host pip.conf option;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/79882/diff/4-5/

Comment 40

[Gregory Szorc [:gps]]

Comment on attachment 8793110 [details]
Bug 1304176 - Remove --venv-path as an alias to --virtualenv-path;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/79884/diff/4-5/

Comment 41

[Gregory Szorc [:gps]]

Comment on attachment 8793111 [details]
Bug 1304176 - Document and refactor query_virtualenv_path;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/79886/diff/4-5/

Comment 42

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/2e07640f74cf
Set BaseScript.topsrcdir if we have a source checkout; r=ted
https://hg.mozilla.org/integration/autoland/rev/b64330d3db8d
Use vendored tooltool.py if available; r=ted
https://hg.mozilla.org/integration/autoland/rev/3fd83c9c0548
Use vendored virtualenv if available; r=ted
https://hg.mozilla.org/integration/autoland/rev/8dc87cef62fb
Remove PIP_TRUSTED_HOST and trust-host pip.conf option; r=ted
https://hg.mozilla.org/integration/autoland/rev/1c0787d40b97
Remove --venv-path as an alias to --virtualenv-path; r=ted
https://hg.mozilla.org/integration/autoland/rev/d5355738ce1e
Document and refactor query_virtualenv_path; r=ted

Comment 43

[Iris Hsiao [:ihsiao]]

https://hg.mozilla.org/mozilla-central/rev/2e07640f74cf
https://hg.mozilla.org/mozilla-central/rev/b64330d3db8d
https://hg.mozilla.org/mozilla-central/rev/3fd83c9c0548
https://hg.mozilla.org/mozilla-central/rev/8dc87cef62fb
https://hg.mozilla.org/mozilla-central/rev/1c0787d40b97
https://hg.mozilla.org/mozilla-central/rev/d5355738ce1e

Comment 44

[Geoff Brown [:gbrown]]

The attempt to use --trusted-host breaks our in-progress work on TC Windows tests:

https://public-artifacts.taskcluster.net/Q36z7Ot9Sg-klAeWDrQr9g/0/public/logs/live_backing.log

17:49:02     INFO - Running command: ['build/venv/Scripts/easy_install.exe', '--find-links', 'http://pypi.pub.build.mozilla.org/pub', '--trusted-host', 'pypi.pub.build.mozilla.org', 'pywin32'] in Z:\task_1475256254\build
...
17:49:04     INFO -  usage: easy_install [options] requirement_or_url ...
17:49:04     INFO -     or: easy_install --help
17:49:04     INFO -  error: option --trusted-host not recognized
17:49:04  WARNING - Return code: 1
17:49:04     INFO - retry: Failed, sleeping 60 seconds before retrying


Do you have a suggestion for dealing with that?

Comment 45

[Gregory Szorc [:gps]]

easy_install should not be used in 2016. I'll fix mozharness.