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

Neon: name 'pip' is not defined #53570

Closed
max-arnold opened this issue Jun 24, 2019 · 19 comments
Closed

Neon: name 'pip' is not defined #53570

max-arnold opened this issue Jun 24, 2019 · 19 comments
Labels
Bug broken, incorrect, or confusing behavior P4 Priority 4 ZRELEASED - Neon retired label
Milestone

Comments

@max-arnold
Copy link
Contributor

Description of Issue

I've been testing some stuff on Neon and got the following traceback:

2019-06-24 03:25:16,909 [salt.state       :1880][INFO    ][9032] Running state [/tmp/file.txt] at time 03:25:16.909899
2019-06-24 03:25:16,910 [salt.state       :1913][INFO    ][9032] Executing state file.managed for [/tmp/file.txt]
2019-06-24 03:25:17,341 [salt.loaded.int.module.cmdmod:399 ][INFO    ][9032] Executing command 'pkg_info -Q LOCALBASE pkgin' in directory '/root'
2019-06-24 03:25:18,018 [salt.loaded.int.module.cmdmod:399 ][INFO    ][9032] Executing command [u'git', u'--version'] in directory '/root'
2019-06-24 03:25:18,225 [salt.loader      :1701][ERROR   ][9032] Failed to import states pip_state, this is due most likely to a syntax error:
Traceback (most recent call last):
  File "/usr/lib/python2.7/dist-packages/salt/loader.py", line 1684, in _load_module
    mod = imp.load_module(mod_namespace, fn_, fpath, desc)
  File "/usr/lib/python2.7/dist-packages/salt/states/pip_state.py", line 58, in <module>
    del pip
NameError: name 'pip' is not defined

Immediately after the traceback, I get huge amount of other exceptions:

Collapsed...
2019-06-24 03:44:50,031 [salt.loader      :1701][ERROR   ][8530] Failed to import states pkg, this is due most likely to a syntax error:
Traceback (most recent call last):
  File "/usr/lib/python2.7/dist-packages/salt/loader.py", line 1684, in _load_module
    mod = imp.load_module(mod_namespace, fn_, fpath, desc)
  File "/usr/lib/python2.7/dist-packages/salt/states/pkg.py", line 79, in <module>
    import logging
  File "/usr/lib/python2.7/logging/__init__.py", line 26, in <module>
    import sys, os, time, cStringIO, traceback, warnings, weakref, collections
  File "/usr/lib/python2.7/os.py", line 119, in <module>
    sys.modules['os.path'] = path
AttributeError: 'module' object has no attribute 'modules'
2019-06-24 03:44:50,034 [salt.loader      :1701][ERROR   ][8530] Failed to import states pkgbuild, this is due most likely to a syntax error:
Traceback (most recent call last):
  File "/usr/lib/python2.7/dist-packages/salt/loader.py", line 1684, in _load_module
    mod = imp.load_module(mod_namespace, fn_, fpath, desc)
  File "/usr/lib/python2.7/dist-packages/salt/states/pkgbuild.py", line 48, in <module>
    import logging
  File "/usr/lib/python2.7/logging/__init__.py", line 26, in <module>
    import sys, os, time, cStringIO, traceback, warnings, weakref, collections
  File "/usr/lib/python2.7/os.py", line 119, in <module>
    sys.modules['os.path'] = path
AttributeError: 'module' object has no attribute 'modules'
2019-06-24 03:44:50,046 [salt.loader      :1701][ERROR   ][8530] Failed to import states pkgrepo, this is due most likely to a syntax error:
Traceback (most recent call last):
  File "/usr/lib/python2.7/dist-packages/salt/loader.py", line 1684, in _load_module
    mod = imp.load_module(mod_namespace, fn_, fpath, desc)
  File "/usr/lib/python2.7/dist-packages/salt/states/pkgrepo.py", line 93, in <module>
    from salt.exceptions import CommandExecutionError, SaltInvocationError
  File "/usr/lib/python2.7/dist-packages/salt/__init__.py", line 100, in <module>
    __define_global_system_encoding_variable__()
  File "/usr/lib/python2.7/dist-packages/salt/__init__.py", line 58, in __define_global_system_encoding_variable__
    encoding = locale.getdefaultlocale()[-1]
  File "/usr/lib/python2.7/locale.py", line 535, in getdefaultlocale
    import os
  File "/usr/lib/python2.7/os.py", line 119, in <module>
    sys.modules['os.path'] = path
AttributeError: 'module' object has no attribute 'modules'

...

Setup

  • Ubuntu 18.04 in Vagrant (with apt-get dist-upgrade applied)
  • Salt 967918f installed via bootstrap-salt.sh -P -f -g https://github.com/saltstack/salt.git -F -c /tmp git neon

Steps to Reproduce Issue

Apply the following (simplified) state using salt XXX state.apply teststate

# teststate.sls
/tmp/file.txt:
  file.managed:
    - contents: |
        Content
    - unless:
      - 'grep "Content" /tmp/file.txt'

The same state works just fine on Salt 2019.2.0.

Versions Report

sudo salt-minion --versions-report
Salt Version:
           Salt: 2019.2.0-n/a-967918f

Dependency Versions:
           cffi: Not Installed
       cherrypy: Not Installed
       dateutil: Not Installed
      docker-py: Not Installed
          gitdb: Not Installed
      gitpython: Not Installed
         Jinja2: 2.10
        libgit2: Not Installed
       M2Crypto: Not Installed
           Mako: Not Installed
   msgpack-pure: Not Installed
 msgpack-python: 0.5.6
   mysql-python: Not Installed
      pycparser: Not Installed
       pycrypto: 2.6.1
   pycryptodome: Not Installed
         pygit2: Not Installed
         Python: 3.6.8 (default, Jan 14 2019, 11:02:34)
   python-gnupg: Not Installed
         PyYAML: 3.12
          PyZMQ: 16.0.2
          smmap: Not Installed
        timelib: Not Installed
        Tornado: 4.5.3
            ZMQ: 4.2.5

System Versions:
           dist: Ubuntu 18.04 bionic
         locale: UTF-8
        machine: x86_64
        release: 4.15.0-29-generic
         system: Linux
        version: Ubuntu 18.04 bionic
@Akm0d
Copy link
Contributor

Akm0d commented Jul 2, 2019

The versions report shows Python 3.6.8 but the tracebacks refer to python 2.7. What are the outputs of pip2.7 --version and pip3.6 --version?

@Akm0d Akm0d added Bug broken, incorrect, or confusing behavior ZRELEASED - Neon retired label P4 Priority 4 Pending-Discussion The issue or pull request needs more discussion before it can be closed or merged labels Jul 2, 2019
@Akm0d Akm0d added this to the Blocked milestone Jul 2, 2019
@max-arnold
Copy link
Contributor Author

@Akm0d

I think I was able to reproduce it on both versions of Python, that is why I posted mixed versions. Just did another test:

sudo salt minion1 state.apply teststate:

Minion log: ModuleNotFoundError: No module named 'pip'
2019-07-02 16:49:52,194 [salt.loader      :1701][ERROR   ][8563] Failed to import states pip_state, this is due most likely to a syntax error:
Traceback (most recent call last):
  File "/usr/lib/python3/dist-packages/salt/states/pip_state.py", line 43, in <module>
    import pip
ModuleNotFoundError: No module named 'pip'

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/usr/lib/python3/dist-packages/salt/loader.py", line 1677, in _load_module
    mod = spec.loader.load_module()
  File "<frozen importlib._bootstrap_external>", line 399, in _check_name_wrapper
  File "<frozen importlib._bootstrap_external>", line 823, in load_module
  File "<frozen importlib._bootstrap_external>", line 682, in load_module
  File "<frozen importlib._bootstrap>", line 265, in _load_module_shim
  File "<frozen importlib._bootstrap>", line 684, in _load
  File "<frozen importlib._bootstrap>", line 665, in _load_unlocked
  File "<frozen importlib._bootstrap_external>", line 678, in exec_module
  File "<frozen importlib._bootstrap>", line 219, in _call_with_frames_removed
  File "/usr/lib/python3/dist-packages/salt/states/pip_state.py", line 58, in <module>
    del pip
NameError: name 'pip' is not defined
2019-07-02 16:49:52,200 [salt.loader      :1701][ERROR   ][8563] Failed to import states pkg, this is due most likely to a syntax error:
Traceback (most recent call last):
  File "/usr/lib/python3/dist-packages/salt/loader.py", line 1677, in _load_module
    mod = spec.loader.load_module()
  File "<frozen importlib._bootstrap_external>", line 399, in _check_name_wrapper
  File "<frozen importlib._bootstrap_external>", line 823, in load_module
  File "<frozen importlib._bootstrap_external>", line 682, in load_module
  File "<frozen importlib._bootstrap>", line 265, in _load_module_shim
  File "<frozen importlib._bootstrap>", line 684, in _load
  File "<frozen importlib._bootstrap>", line 665, in _load_unlocked
  File "<frozen importlib._bootstrap_external>", line 678, in exec_module
  File "<frozen importlib._bootstrap>", line 219, in _call_with_frames_removed
  File "/usr/lib/python3/dist-packages/salt/states/pkg.py", line 78, in <module>
    import fnmatch
  File "/usr/lib/python3.6/fnmatch.py", line 12, in <module>
    import os
  File "/usr/lib/python3.6/os.py", line 91, in <module>
    sys.modules['os.path'] = path
AttributeError: module 'sys' has no attribute 'modules'
2019-07-02 16:49:52,204 [salt.loader      :1701][ERROR   ][8563] Failed to import states pkgbuild, this is due most likely to a syntax error:
Traceback (most recent call last):
  File "/usr/lib/python3/dist-packages/salt/loader.py", line 1677, in _load_module
    mod = spec.loader.load_module()
  File "<frozen importlib._bootstrap_external>", line 399, in _check_name_wrapper
  File "<frozen importlib._bootstrap_external>", line 823, in load_module
  File "<frozen importlib._bootstrap_external>", line 682, in load_module
  File "<frozen importlib._bootstrap>", line 265, in _load_module_shim
  File "<frozen importlib._bootstrap>", line 684, in _load
  File "<frozen importlib._bootstrap>", line 665, in _load_unlocked
  File "<frozen importlib._bootstrap_external>", line 678, in exec_module
  File "<frozen importlib._bootstrap>", line 219, in _call_with_frames_removed
  File "/usr/lib/python3/dist-packages/salt/states/pkgbuild.py", line 48, in <module>
    import logging
  File "/usr/lib/python3.6/logging/__init__.py", line 26, in <module>
    import sys, os, time, io, traceback, warnings, weakref, collections
  File "/usr/lib/python3.6/os.py", line 91, in <module>
    sys.modules['os.path'] = path
AttributeError: module 'sys' has no attribute 'modules'
salt --versions-report
Salt Version:
           Salt: 2019.2.0-n/a-a3ad595

Dependency Versions:
           cffi: Not Installed
       cherrypy: Not Installed
       dateutil: Not Installed
      docker-py: Not Installed
          gitdb: Not Installed
      gitpython: Not Installed
         Jinja2: 2.10
        libgit2: Not Installed
       M2Crypto: Not Installed
           Mako: Not Installed
   msgpack-pure: Not Installed
 msgpack-python: 0.5.6
   mysql-python: Not Installed
      pycparser: Not Installed
       pycrypto: 2.6.1
   pycryptodome: Not Installed
         pygit2: Not Installed
         Python: 3.6.8 (default, Jan 14 2019, 11:02:34)
   python-gnupg: Not Installed
         PyYAML: 3.12
          PyZMQ: 16.0.2
          smmap: Not Installed
        timelib: Not Installed
        Tornado: 4.5.3
            ZMQ: 4.2.5

System Versions:
           dist: Ubuntu 18.04 bionic
         locale: UTF-8
        machine: x86_64
        release: 4.15.0-29-generic
         system: Linux
        version: Ubuntu 18.04 bionic
salt minion1 test.versions_report
minion1:
    Salt Version:
               Salt: 2019.2.0-n/a-a3ad595

    Dependency Versions:
               cffi: Not Installed
           cherrypy: Not Installed
           dateutil: Not Installed
          docker-py: Not Installed
              gitdb: Not Installed
          gitpython: Not Installed
             Jinja2: 2.10
            libgit2: Not Installed
           M2Crypto: Not Installed
               Mako: Not Installed
       msgpack-pure: Not Installed
     msgpack-python: 0.5.6
       mysql-python: Not Installed
          pycparser: Not Installed
           pycrypto: 2.6.1
       pycryptodome: Not Installed
             pygit2: Not Installed
             Python: 3.6.8 (default, Jan 14 2019, 11:02:34)
       python-gnupg: Not Installed
             PyYAML: 3.12
              PyZMQ: 16.0.2
              smmap: Not Installed
            timelib: Not Installed
            Tornado: 4.5.3
                ZMQ: 4.2.5

    System Versions:
               dist: Ubuntu 18.04 bionic
             locale: UTF-8
            machine: x86_64
            release: 4.15.0-29-generic
             system: Linux
            version: Ubuntu 18.04 bionic

pip executable does not exist on both master and minion (probably wasn't installed by salt-bootstrap.sh):

bootstrap-salt.sh -P -x python3 -f -g https://github.com/saltstack/salt.git -F -c /tmp -k /tmp/minion-seed-keys -M -N git neon

@max-arnold
Copy link
Contributor Author

When I bootstrap the 2019.2 branch in the same way (2019.2.0-n/a-797c69e), it works.

@Akm0d Akm0d removed the Pending-Discussion The issue or pull request needs more discussion before it can be closed or merged label Jul 2, 2019
@Akm0d Akm0d modified the milestones: Blocked, Approved Jul 2, 2019
@Akm0d
Copy link
Contributor

Akm0d commented Jul 2, 2019

The -P option should make sure pip and it's dependencies are installed. Maybe this is an issue for the salt-boostrap repo?

@max-arnold
Copy link
Contributor Author

I agree that it could be an issue with salt-bootstrap. But why it is not reproducible with 2019.2 branch? Is there something in salt-bootstrap that checks for specific salt version/branch?

@max-arnold
Copy link
Contributor Author

max-arnold commented Jul 5, 2019

A commit that introduced this issue: fbe648c (#52728)

@max-arnold
Copy link
Contributor Author

max-arnold commented Jul 5, 2019

Existing unit tests weren't able to detect it because the testing evironment has a lot of preinstalled dependencies, including pip.

One possible solution is to have an additional testing environment with no extra deps installed, and run a subset of tests (or a couple of special high-level tests) to ensure that some basic Salt operations can actually work. This would also help to detect (accidentally introduced) new hard dependencies.

CC: @s0undt3ch

@s0undt3ch
Copy link
Member

Unfortunately, we rely on pip to set things up to even run the tests, we could create a separate virtual environment just to test this scenario, but virtualenv will install pip.
Anyway, this is an issue in salt, an easy one to fix, we should be setting the pip variable to None prior to trying to delete it.
This way, if it wasn't imported because it wasn't available at the time, salt would still carry on it's duty.

@s0undt3ch
Copy link
Member

Actually, the fix is likely something like:

diff --git a/salt/states/pip_state.py b/salt/states/pip_state.py
index b8223e3b9b..eb625ad544 100644
--- a/salt/states/pip_state.py
+++ b/salt/states/pip_state.py
@@ -44,6 +44,19 @@ try:
     HAS_PIP = True
 except ImportError:
     HAS_PIP = False
+
+if HAS_PIP is True:
+    if salt.utils.versions.compare(ver1=pip.__version__,
+                                   oper='>=',
+                                   ver2='18.1'):
+        from pip._internal.exceptions import InstallationError  # pylint: disable=E0611,E0401
+    elif salt.utils.versions.compare(ver1=pip.__version__,
+                                     oper='>=',
+                                     ver2='1.0'):
+        from pip.exceptions import InstallationError  # pylint: disable=E0611,E0401
+    else:
+        InstallationError = ValueError
+
     # Remove references to the loaded pip module above so reloading works
     import sys
     pip_related_entries = [
@@ -60,18 +73,6 @@ except ImportError:
     if sys_modules_pip is not None:
         del sys_modules_pip
 
-if HAS_PIP is True:
-    if salt.utils.versions.compare(ver1=pip.__version__,
-                                   oper='>=',
-                                   ver2='18.1'):
-        from pip._internal.exceptions import InstallationError  # pylint: disable=E0611,E0401
-    elif salt.utils.versions.compare(ver1=pip.__version__,
-                                     oper='>=',
-                                     ver2='1.0'):
-        from pip.exceptions import InstallationError  # pylint: disable=E0611,E0401
-    else:
-        InstallationError = ValueError
-
 
 # pylint: enable=import-error

Would you agree @dwoz?

A way to test would be to create a virtualenv, uninstall pip, and try to use the pip state againt that virtualenv and see if we didn't get a traceback, of course, the pip state would be unavailable because pip is missing, but it would test this particular issue.

@swinkels
Copy link

With the release of the packaged 2019.2.1, I am bitten by this bug too. Is there a reason why this shouldn't be fixed as described by #53570 (comment)

@max-arnold
Copy link
Contributor Author

max-arnold commented Sep 26, 2019

It is easy to fix this specific issue (and I have no idea why they didn't fix it, and why it was not considered while backporting the change, and why it wasn't discovered during the 2019.2.1 QA phase).

The problem with this approach is that these kinds of issues (new hard dependencies that are invisible to the existing test suite) could appear again. Besides manual QA, I think it is possible to have an automated set of smoke tests that could run in a clean environment without any extra deps installed.

@swinkels
Copy link

OK, thanks for the quick reply and explanation. As a workaround I will use the 2019.2.0 release for now.

@max-arnold
Copy link
Contributor Author

@swinkels Setting the proper fix aside, there is a simple workaround. Just install the pip (or pip3 for Python 3 based Salt) system package, before installing Salt. On Ubuntu that would be:

apt-get -y install python-pip

or

apt-get -y install python3-pip

@swinkels
Copy link

Thanks for the workaround, but it is not so easy in our case. We have a fully automated deployment using Foreman and Salt that installs and fully configures a "bare metal" machine using custom Debian and CentOS repositories. To cut a long story short, it is a bit more work than just install python-pip before we install Salt.

@d--j
Copy link
Contributor

d--j commented Sep 27, 2019

my solution for now:

echo '# disabled for now, because of https://github.com/saltstack/salt/issues/53570' > /srv/salt/_states/pip_state.py

Of course only applicable when you do not use the pip state.

@micdobro
Copy link

micdobro commented Nov 29, 2019

Bitten by this bug on Ubuntu 16.04 LTS and 2019.2.1. Actually it first happened when provisioning a new machine today (did not experience it a few days ago). Workaround with installing pip on the target machine works, although that breaks completely the automation and magic that comes with a highstate.

I have been searching now actively for a solution - apparently everybody claims "it's not their problem". But pip is not being installed during bootstrap - and it's apparently needed. Or am I completely off here?

@max-arnold
Copy link
Contributor Author

@micdobro The bug is fixed in 2019.2.2

@micdobro
Copy link

@max-arnold Thanks a lot. We are then scheduling an upgrade.

@max-arnold
Copy link
Contributor Author

I guess this could be closed, because Neon is going to be ported on top of the master branch

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug broken, incorrect, or confusing behavior P4 Priority 4 ZRELEASED - Neon retired label
Projects
None yet
Development

No branches or pull requests

6 participants