voxpupuli / puppet-python

Puppet module for installing and managing Python, pip, virtualenvs and Gunicorn virtual hosts.
https://forge.puppetlabs.com/puppet/python
Apache License 2.0
199 stars 370 forks source link

Fix pyvenv on some operating systems #597

Open treydock opened 3 years ago

treydock commented 3 years ago

Pull Request (PR) description

For Ubuntu 18.04 and Debian 10, ensure distutils is installed.

When testing pyvenv I ran into this problem when using Python 3 and noticed if you install python3-venv instead of python3.7-venv it pulls in python3-distutils but rather than changing the behavior of how to install python*-venv just pulling in the dependency works.

Failure with/without system packages

root@debian10:/# python3.7 -m venv --clear --system-site-packages /opt/globus-cli
The virtual environment was not created successfully because ensurepip is not
available.  On Debian/Ubuntu systems, you need to install the python3-venv
package using the following command.

    apt-get install python3-venv

You may need to use sudo with that command.  After installing the python3-venv
package, recreate your virtual environment.

Failing command: ['/opt/globus-cli/bin/python3.7', '-Im', 'ensurepip', '--upgrade', '--default-pip']

root@debian10:/# python3.7 -m venv --clear /opt/globus-cli
The virtual environment was not created successfully because ensurepip is not
available.  On Debian/Ubuntu systems, you need to install the python3-venv
package using the following command.

    apt-get install python3-venv

You may need to use sudo with that command.  After installing the python3-venv
package, recreate your virtual environment.

Failing command: ['/opt/globus-cli/bin/python3.7', '-Im', 'ensurepip', '--upgrade', '--default-pip']

Solution:

root@debian10:/# apt-get install python3.7-distutils
Reading package lists... Done
Building dependency tree       
Reading state information... Done
Note, selecting 'python3-distutils' instead of 'python3.7-distutils'
The following packages were automatically installed and are no longer required:
  libexpat1-dev libpython-all-dev libpython-dev libpython2-dev libpython2.7 libpython2.7-dev python-all
  python2-dev python2.7-dev
Use 'apt autoremove' to remove them.
The following NEW packages will be installed:
  python3-distutils
0 upgraded, 1 newly installed, 0 to remove and 14 not upgraded.
Need to get 142 kB of archives.
After this operation, 715 kB of additional disk space will be used.
Get:1 http://deb.debian.org/debian buster/main amd64 python3-distutils all 3.7.3-1 [142 kB]
Fetched 142 kB in 0s (333 kB/s)       
debconf: delaying package configuration, since apt-utils is not installed
Selecting previously unselected package python3-distutils.
(Reading database ... 42675 files and directories currently installed.)
Preparing to unpack .../python3-distutils_3.7.3-1_all.deb ...
Unpacking python3-distutils (3.7.3-1) ...
Setting up python3-distutils (3.7.3-1) ...

root@debian10:/# python3.7 -m venv --clear --system-site-packages /opt/globus-cli
root@debian10:/# 
treydock commented 3 years ago

Seems installing python3.7-distutils actually installs python3-distutils and causes issues with not being idempotent.

bastelfreak commented 3 years ago

@treydock thanks for the PR! Are you able to add an acceptance test here as well?

treydock commented 3 years ago

@bastelfreak Any suggestions on what kind of acceptance tests to add? There are already numerous tests for this defined type that the catalog applies successfully. The only thing I could think to test is that python3-distutils is actually installed for these select operating systems, since the there are already tests for success of setting up the pyvenv environment.

bastelfreak commented 3 years ago

if I read this correctly, the code on master fails if you set the python version on debian 10 to 3.7? Is that something that can be validated with a test?

treydock commented 3 years ago

I am fairly certain for at least Debian 10, which is one of the OSes where I encountered this, the Python 3 version installed is 3.7 so I just do something like this in my module:

  class { 'python':
    version => '3',
  }
  python::pyvenv { 'globus-cli':
    ensure     => 'present',
    version    => 'system',
    venv_dir   => '/opt/globus-cli',
    systempkgs => true,
  }

That's when I encountered the error for several Debian based operating systems. I was trying to switch my Globus module to Python 3 for all operating systems: https://github.com/treydock/puppet-module-globus/pull/22

That is why I'm not sure why the existing acceptance tests didn't catch this because the code I'm using looks very similar to what's already being done. I think after looking more closely I see why the tests didn't catch this. If I set python::dev: present then everything works so I'm guessing when you install the python dev packages on Debian it pulls in the necessary packages.

treydock commented 3 years ago

So this PR is not necessary if the dev => 'present' is set for python class. I wonder if this is just something that needs to be documented rather than adding this edge case?

treydock commented 3 years ago

I wonder if maybe a warning is the appropriate change. If python::dev is absent, issue a warning in pyvenv defined type?

vox-pupuli-tasks[bot] commented 2 years ago

Dear @treydock, thanks for the PR!

This is Vox Pupuli Tasks, your friendly Vox Pupuli GitHub Bot. I noticed that your pull request contains merge conflict. Can you please rebase?

You can find my sourcecode at voxpupuli/vox-pupuli-tasks