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
200 stars 374 forks source link

pyvenv cannot install until $facts['python3_version'] exists (or is overidden) on RedHat style OSes #635

Open decibelhertz opened 2 years ago

decibelhertz commented 2 years ago

Affected Puppet, Ruby, OS and module versions/distributions

How to reproduce (e.g Puppet code you use)

Use this module to install a python role with a pyvenv declared and absolutely no python3 installed (minimal OEL 8 install)

What are you seeing

Failure to compile catalog

What behaviour did you expect instead

Compiled catalog

Output log

puppet agent -t --verbose
Info: Using environment 'pe2019'
Info: Retrieving pluginfacts
Info: Retrieving plugin
Info: Retrieving locales
Info: Loading facts
Error: Could not retrieve catalog from remote server: Error 500 on SERVER: Server Error: Evaluation Error: Error while evaluating a Resource Statement, Evaluation Error: Error while evaluating a Function Call, 'split' parameter 'str' expects a String value, got Undef (file: /etc/puppetlabs/code/environments/pe2019/modules/python/manifests/pyvenv.pp, line: 44, column: 34) (file: /etc/puppetlabs/code/environments/pe2019/modules/python/manifests/init.pp, line: 76) on node example.com
Warning: Not using cache on failed catalog
Error: Could not retrieve catalog; skipping run

Any additional information you'd like to impart

A cheap workaround would be to wrap https://github.com/voxpupuli/puppet-python/blob/master/manifests/init.pp#L80-L84 in a conditional EG

# Conditionally allow hiera configuration of python resources once Puppet facts exist
# Will take multiple runs to converge
if $facts['python3_version'] {
  create_resources('python::pip', $python_pips)
  create_resources('python::pyvenv', $python_pyvenvs)
  create_resources('python::requirements', $python_requirements)
  create_resources('python::dotfile', $python_dotfiles)
}

...but that depends on if you're supporting python2 still. Perhaps better logic for converting 'system' into a real Python version is needed.

saz commented 2 years ago

python::pyvenv relies on $facts['python3_version'] if python_version is set to system. That's why it's failing here and I'd say, adding a version check seems to be a good fix:

if $facts['python_version'] or $facts['python2_version'] or $facts['python3_version'] {
  create_resources('python::pip', $python_pips)
  create_resources('python::pyvenv', $python_pyvenvs)
  create_resources('python::requirements', $python_requirements)
  create_resources('python::dotfile', $python_dotfiles)
}

Maybe we should drop the python_version and python2_version fact, as it's not used within the module (but might be used by others)

Kritzefitz commented 11 months ago

I think the proposed solution would only solve the problem for sub-resources that have been declared through parameters of ::python, but the problem would still occur when ::python::pyvenv is declared as its own ressource.