salt 3000 and master don't work with default pip (9.0.1) on windows

[bryceml]

Description of Issue

pip.installed is not available on salt 3000 or the head of master on older versions of pip on windows

https://gitlab.com/saltstack/employees/sre/bryceml/ec2-amis/-/jobs/473544805
fails with salt 3000 and default pip (9.0.1).
https://gitlab.com/saltstack/employees/sre/bryceml/ec2-amis/-/jobs/473602961
succeeds with salt 2019.2.3 and default pip (9.0.1).

{%- set get_python_dir = salt.temp.dir(prefix='get-python-') %}
{%- set get_python_path = (get_python_dir | path_join('python-3.6.8-amd64.exe')).replace('\\', '\\\\') %}

download-get-python:
  file.managed:
    - name: {{ get_python_path }}
    - source: https://www.python.org/ftp/python/3.6.8/python-3.6.8-amd64.exe
    - skip_verify: true

python3:
  cmd.run:
    - name: {{ get_python_path }} /quiet InstallAllUsers=1 TargetDir=C:\\Python36 Include_doc=0 Include_tcltk=0 Include_test=0 Include_launcher=1 PrependPath=1 Shortcuts=0

{%- set get_pip_dir = salt.temp.dir(prefix='get-pip-') %}
{%- set get_pip_path = (get_pip_dir | path_join('get-pip.py')).replace('\\', '\\\\') %}
{%- set python3 = 'c:\\\\Python36\\\\python.exe' %}
{%- set get_pip3 = '{} {}'.format(python3, get_pip_path) %}
{%- set pip3 = 'pip3' %}

download-get-pip:
  file.managed:
    - name: {{ get_pip_path }}
    - source: https://raw.githubusercontent.com/pypa/get-pip/69941dc3ee6a2562d873f8616a4df31ba401b2a9/get-pip.py
    - skip_verify: true

pip3-install:
  cmd.run:
    # -c <() because of https://github.com/pypa/get-pip/issues/37
    - name: '{{ get_pip3 }} "pip<=20.0.2"'
    - cwd: /
    - reload_modules: True

upgrade-installed-pip3:
  pip.installed:
    - name: pip <=20.0.2
    - upgrade: True
    - onlyif:
      - 'if (py.exe -3 -c "import sys; print(sys.executable)") { exit 0 } else { exit 1 }'
      - 'if (get-command pip3) { exit 0 } else { exit 1 }'
    - require:
      - cmd: pip3-install

are the states I used

If I use salt 3000 or the head of master and upgrade to pip 20.0.2 it works.

When it doesn't work, I get this as a traceback:

==> amazon-ebs: [ERROR   ] Error encountered during module reload. Modules were not reloaded.
==> amazon-ebs: [ERROR   ] Failed to import states pip_state, this is due most likely to a syntax error:
==> amazon-ebs: Traceback (most recent call last):
==> amazon-ebs:   File "C:\salt\bin\lib\site-packages\salt-3000-py2.7.egg\salt\loader.py", line 1607, in _load_module
==> amazon-ebs:     mod = imp.load_module(mod_namespace, fn_, fpath, desc)
==> amazon-ebs:   File "C:\salt\bin\lib\site-packages\salt-3000-py2.7.egg\salt\states\pip_state.py", line 95, in <module>
==> amazon-ebs:     import pip
==> amazon-ebs:   File "C:\salt\bin\lib\site-packages\pip\__init__.py", line 26, in <module>
==> amazon-ebs:     from pip.utils import get_installed_distributions, get_prog
==> amazon-ebs:   File "C:\salt\bin\lib\site-packages\pip\utils\__init__.py", line 23, in <module>
==> amazon-ebs:     from pip.locations import (
==> amazon-ebs:   File "C:\salt\bin\lib\site-packages\pip\locations.py", line 88, in <module>
==> amazon-ebs:     bin_user = os.path.join(user_site, 'Scripts')
==> amazon-ebs:   File "C:\salt\bin\lib\ntpath.py", line 65, in join
==> amazon-ebs:     result_drive, result_path = splitdrive(path)
==> amazon-ebs:   File "C:\salt\bin\lib\ntpath.py", line 115, in splitdrive
==> amazon-ebs:     if len(p) > 1:
==> amazon-ebs: TypeError: object of type 'NoneType' has no len()
==> amazon-ebs: [ERROR   ] State 'pip.installed' was not found in SLS 'dependencies.windows_pip'

[s0undt3ch]

To clarify, the 0.9.1 version of pip is the oldest we try to support, not the default pip version that salt requires to work.

[s0undt3ch]

I know, doesn't impact the issue, it's just wording. :)

[bryceml]

I meant it's the default version of pip that the windows installer comes with.

[Ch3LL]

i can confirm that this is technically an upstream issue here pypa/pip#4437 that has been fixed with newer versions of pip. What is happening is site.USER_SITE is None. This pip fix ensures we handle this situation.

Now what i'm still trying discover is why in our salt packages 3000 site.USER_SITE is now None, where as in 2019.2.3 it returns the user directories path. My current guess is it is possibly related to changes we made in setup.py.

If you take a look at the site-packages dir between both package versions you can see a difference:

PS C:\salt> ls .\bin\Lib\site-packages\salt*    
Directory: C:\salt\bin\Lib\site-packages
Mode                LastWriteTime         Length Name
----                -------------         ------ ----
d-----        3/17/2020   8:07 PM                salt-3000-py2.7.egg

PS C:\salt> ls .\bin\Lib\site-packages\salt*    
Directory: C:\salt\bin\Lib\site-packages
Mode                LastWriteTime         Length Name
----                -------------         ------ ----
d-----        3/17/2020   8:47 PM                salt
-a----         1/7/2020  11:14 PM           3847 salt-2019.2.3-py2.7.egg-info

in salt 3000 the salt dir is here C:\salt\bin\Lib\site-packages\salt-3000-py2.7.egg\site whereas in 2019.2.3 its here: C:\salt\bin\Lib\site-packages\salt. i'm wondering if our changes to use setuptools in setup.py is impacting this and site.USER_SITE.

Either way i do not think we need to block a release since its technically an upstream issue that needs to handle situations when site.USER_SITE is None. Will continue to do some investigation to figure out why it is None in this release.

[Ch3LL]

okay i've figured this one out. I was able to bisect this issue to a399d7f which is PR #55817

The problem line is here https://github.com/saltstack/salt/pull/55817/files#diff-d27e77cec6710027bb120a9dc4f1b959R45

There is a known open issue here: https://bugs.python.org/issue31798

When site.py runs abs__file__ a TypeError returns due to this clr import, and thats how USER_SITE is getting set to None. This pr python/cpython#6731 does add the exception handling around TypeError. Which is fixed in python versions 3.8 and above. I tested this on python 2.7.15 except (AttributeError, OSError, TypeError): and it got things working as well so that PR should fix this.

I was also able to get this use case down to a smaller state file:

test:
  cmd.run:
    - name: 'echo test'
    - reload_modules: True

upgrade-installed-pip3:
  pip.installed:
    - name: requests

To note the line that is actually causing this issue is reload_modules: True. If you remove that everything works just fine. And you can see in the log:

[DEBUG   ] Refreshing modules...
[ERROR   ] Error encountered during module reload. Modules were not reloaded.

its the module refresh that is causing this issue.

So we have two upstream issues that are causing issues. Sure we could upgrade the pip version to ensure we handle the USER_SITE being none, but that will still cause the module refresh to fail.

There is also this issue pythonnet/pythonnet#559 to track this on pythonnets/clr's end.

[arizvisa]

Just to clarify, is this the case for both Python2 and Python3 on windows? And in the meantime is the proper workaround to not reload_modules, and maybe just restart the minion?

[bryceml]

marked needs-triage since we don't know if this is fixed or not on master.

[garethgreenaway]

This is believed to be fixed in Python 3.8 which will be shipped with the Silicon release. Will revisit this issue once Silicon has been released.

[twangboy]

This is fixed on master. I was able to use pip.installed to install pip 9.0.1 and then again to install pip 20.0.2 with python 3.8 on master branch.