From 5ccfca44970611b3a673e11e4677bada4b7fd239 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Romain=20Tarti=C3=A8re?= Date: Mon, 20 Mar 2023 15:58:34 -1000 Subject: [PATCH 1/2] Prefer Hiera data to params.pp This limit the quantity of spaghetti code we add when adding support for another platform. No functional change. --- REFERENCE.md | 34 ++++++++++-------------- data/common.yaml | 11 ++++++++ data/os/AIX.yaml | 7 +++++ data/os/Archlinux.yaml | 4 +++ data/os/RedHat.yaml | 3 +++ data/os/RedHat/CentOS/8.yaml | 2 ++ data/os/RedHat/Fedora.yaml | 2 ++ data/os/RedHat/RedHat/8.yaml | 2 ++ hiera.yaml | 19 +++++++++++++ manifests/init.pp | 12 ++++----- manifests/params.pp | 44 ------------------------------- manifests/pip.pp | 2 +- manifests/pip/bootstrap.pp | 7 ++--- spec/classes/python_spec.rb | 1 - spec/default_module_facts.yml | 3 --- spec/defines/requirements_spec.rb | 1 - spec/spec.opts | 6 ----- 17 files changed, 75 insertions(+), 85 deletions(-) create mode 100644 data/common.yaml create mode 100644 data/os/AIX.yaml create mode 100644 data/os/Archlinux.yaml create mode 100644 data/os/RedHat.yaml create mode 100644 data/os/RedHat/CentOS/8.yaml create mode 100644 data/os/RedHat/Fedora.yaml create mode 100644 data/os/RedHat/RedHat/8.yaml create mode 100644 hiera.yaml delete mode 100644 manifests/params.pp delete mode 100644 spec/default_module_facts.yml delete mode 100644 spec/spec.opts diff --git a/REFERENCE.md b/REFERENCE.md index 2aac08b3..5b1ba50e 100644 --- a/REFERENCE.md +++ b/REFERENCE.md @@ -15,7 +15,6 @@ * `python::config`: Optionally installs the gunicorn service * `python::install`: Installs core python packages -* `python::params`: The python Module default configuration settings. ### Defined types @@ -81,8 +80,8 @@ The following parameters are available in the `python` class: * [`manage_python_package`](#-python--manage_python_package) * [`manage_venv_package`](#-python--manage_venv_package) * [`manage_pip_package`](#-python--manage_pip_package) -* [`venv`](#-python--venv) * [`gunicorn_package_name`](#-python--gunicorn_package_name) +* [`venv`](#-python--venv) * [`python_pips`](#-python--python_pips) * [`python_pyvenvs`](#-python--python_pyvenvs) * [`python_requirements`](#-python--python_requirements) @@ -112,8 +111,6 @@ Allowed values: - 3/3.3/... means you are going to install the python3/python3.3/... package, if available on your osfamily. -Default value: `$facts['os']['family'] ? { 'Archlinux' => 'system', default => '3'` - ##### `pip` Data type: `Python::Package::Ensure` @@ -160,8 +157,6 @@ Data type: `Boolean` to determine if the epel class is used. -Default value: `$python::params::use_epel` - ##### `manage_scl` Data type: `Boolean` @@ -198,31 +193,25 @@ Data type: `Boolean` manage the state for package venv -Default value: `$python::params::manage_venv_package` - ##### `manage_pip_package` Data type: `Boolean` manage the state for package pip -Default value: `$python::params::manage_pip_package` - -##### `venv` - -Data type: `Python::Package::Ensure` +##### `gunicorn_package_name` +Data type: `String[1]` -Default value: `'absent'` -##### `gunicorn_package_name` +##### `venv` -Data type: `String[1]` +Data type: `Python::Package::Ensure` -Default value: `$python::params::gunicorn_package_name` +Default value: `'absent'` ##### `python_pips` @@ -301,6 +290,7 @@ The following parameters are available in the `python::pip::bootstrap` class: * [`version`](#-python--pip--bootstrap--version) * [`manage_python`](#-python--pip--bootstrap--manage_python) * [`http_proxy`](#-python--pip--bootstrap--http_proxy) +* [`pip_lookup_path`](#-python--pip--bootstrap--pip_lookup_path) * [`exec_provider`](#-python--pip--bootstrap--exec_provider) ##### `version` @@ -327,6 +317,12 @@ Proxy server to use for outbound connections. Default value: `undef` +##### `pip_lookup_path` + +Data type: `Array[String[1]]` + + + ##### `exec_provider` Data type: `String[1]` @@ -753,12 +749,10 @@ Default value: `'root'` ##### `group` -Data type: `Optional[String[1]]` +Data type: `String[1]` The group of the virtualenv being manipulated. -Default value: `getvar('python::params::group')` - ##### `index` Data type: `Variant[Boolean,String[1]]` diff --git a/data/common.yaml b/data/common.yaml new file mode 100644 index 00000000..fedfefcc --- /dev/null +++ b/data/common.yaml @@ -0,0 +1,11 @@ +--- +python::version: '3' +python::use_epel: true +python::gunicorn_package_name: 'gunicorn' +python::manage_pip_package: true +python::manage_venv_package: true +python::pip::group: 'root' +python::pip::bootstrap::pip_lookup_path: + - /bin + - /usr/bin + - /usr/local/bin diff --git a/data/os/AIX.yaml b/data/os/AIX.yaml new file mode 100644 index 00000000..b2fa452d --- /dev/null +++ b/data/os/AIX.yaml @@ -0,0 +1,7 @@ +--- +python::pip::group: 'system' +python::pip::bootstrap::pip_lookup_path: + - /bin + - /usr/bin + - /usr/local/bin + - /opt/freeware/bin/ diff --git a/data/os/Archlinux.yaml b/data/os/Archlinux.yaml new file mode 100644 index 00000000..a438e2ae --- /dev/null +++ b/data/os/Archlinux.yaml @@ -0,0 +1,4 @@ +--- +python::version: 'system' +python::manage_pip_package: false +python::manage_venv_package: false diff --git a/data/os/RedHat.yaml b/data/os/RedHat.yaml new file mode 100644 index 00000000..574c2e86 --- /dev/null +++ b/data/os/RedHat.yaml @@ -0,0 +1,3 @@ +--- +python::use_epel: true +python::gunicorn_package_name: 'python-gunicorn' diff --git a/data/os/RedHat/CentOS/8.yaml b/data/os/RedHat/CentOS/8.yaml new file mode 100644 index 00000000..6b282182 --- /dev/null +++ b/data/os/RedHat/CentOS/8.yaml @@ -0,0 +1,2 @@ +--- +python::gunicorn_package_name: 'python3-gunicorn' diff --git a/data/os/RedHat/Fedora.yaml b/data/os/RedHat/Fedora.yaml new file mode 100644 index 00000000..00be9f25 --- /dev/null +++ b/data/os/RedHat/Fedora.yaml @@ -0,0 +1,2 @@ +--- +python::use_epel: false diff --git a/data/os/RedHat/RedHat/8.yaml b/data/os/RedHat/RedHat/8.yaml new file mode 100644 index 00000000..6b282182 --- /dev/null +++ b/data/os/RedHat/RedHat/8.yaml @@ -0,0 +1,2 @@ +--- +python::gunicorn_package_name: 'python3-gunicorn' diff --git a/hiera.yaml b/hiera.yaml new file mode 100644 index 00000000..19bb8d28 --- /dev/null +++ b/hiera.yaml @@ -0,0 +1,19 @@ +--- +version: 5 + +defaults: + datadir: 'data' + data_hash: 'yaml_data' + +hierarchy: + - name: 'family/name/major' + path: 'os/%{facts.os.family}/%{facts.os.name}/%{facts.os.release.major}.yaml' + + - name: 'family/name' + path: 'os/%{facts.os.family}/%{facts.os.name}.yaml' + + - name: 'family' + path: 'os/%{facts.os.family}.yaml' + + - name: 'common' + path: 'common.yaml' diff --git a/manifests/init.pp b/manifests/init.pp index 5b3c9fdf..e6d2b6a7 100644 --- a/manifests/init.pp +++ b/manifests/init.pp @@ -37,29 +37,29 @@ # } # class python ( + Python::Version $version, + Boolean $manage_venv_package, + Boolean $manage_pip_package, + String[1] $gunicorn_package_name, + Boolean $use_epel, Python::Package::Ensure $ensure = 'present', - Python::Version $version = $facts['os']['family'] ? { 'Archlinux' => 'system', default => '3' }, Python::Package::Ensure $pip = 'present', Python::Package::Ensure $dev = 'absent', Python::Package::Ensure $venv = 'absent', Python::Package::Ensure $gunicorn = 'absent', Boolean $manage_gunicorn = true, Boolean $manage_python_package = true, - Boolean $manage_venv_package = $python::params::manage_venv_package, - Boolean $manage_pip_package = $python::params::manage_pip_package, - String[1] $gunicorn_package_name = $python::params::gunicorn_package_name, Optional[Python::Provider] $provider = undef, Hash $python_pips = {}, Hash $python_pyvenvs = {}, Hash $python_requirements = {}, Hash $python_dotfiles = {}, - Boolean $use_epel = $python::params::use_epel, Boolean $rhscl_use_public_repository = true, Stdlib::Httpurl $anaconda_installer_url = 'https://repo.anaconda.com/archive/Anaconda3-5.2.0-Linux-x86_64.sh', Stdlib::Absolutepath $anaconda_install_path = '/opt/python', Boolean $manage_scl = true, Optional[Python::Umask] $umask = undef, -) inherits python::params { +) { $exec_prefix = $provider ? { 'scl' => "/usr/bin/scl enable ${version} -- ", 'rhscl' => "/usr/bin/scl enable ${version} -- ", diff --git a/manifests/params.pp b/manifests/params.pp deleted file mode 100644 index e498ddaf..00000000 --- a/manifests/params.pp +++ /dev/null @@ -1,44 +0,0 @@ -# @api private -# @summary The python Module default configuration settings. -# -# The python Module default configuration settings. -# -class python::params { - # Module compatibility check - unless $facts['os']['family'] in ['AIX', 'Debian', 'FreeBSD', 'Gentoo', 'RedHat', 'Suse', 'Archlinux'] { - fail("Module is not compatible with ${facts['os']['name']}") - } - - if $facts['os']['family'] == 'RedHat' and $facts['os']['name'] != 'Fedora' { - $use_epel = true - } else { - $use_epel = false - } - - $group = $facts['os']['family'] ? { - 'AIX' => 'system', - default => 'root' - } - - $pip_lookup_path = $facts['os']['family'] ? { - 'AIX' => ['/bin', '/usr/bin', '/usr/local/bin', '/opt/freeware/bin/',], - default => ['/bin', '/usr/bin', '/usr/local/bin',] - } - - $gunicorn_package_name = $facts['os']['family'] ? { - 'RedHat' => $facts['os']['release']['major'] ? { - '8' => 'python3-gunicorn', - default => 'python-gunicorn', - }, - default => 'gunicorn', - } - - $manage_pip_package = $facts['os']['family'] ? { - 'Archlinux' => false, - default => true, - } - $manage_venv_package = $facts['os']['family'] ? { - 'Archlinux' => false, - default => true, - } -} diff --git a/manifests/pip.pp b/manifests/pip.pp index 199e7925..a03e59f0 100644 --- a/manifests/pip.pp +++ b/manifests/pip.pp @@ -49,13 +49,13 @@ # } # define python::pip ( + String[1] $group, String[1] $pkgname = $name, Variant[Enum[present, absent, latest], String[1]] $ensure = present, Variant[Enum['system'], Stdlib::Absolutepath] $virtualenv = 'system', String[1] $pip_provider = 'pip', Variant[Boolean, String] $url = false, String[1] $owner = 'root', - Optional[String[1]] $group = getvar('python::params::group'), Optional[Python::Umask] $umask = undef, Variant[Boolean,String[1]] $index = false, Variant[Boolean,String[1]] $extra_index = false, diff --git a/manifests/pip/bootstrap.pp b/manifests/pip/bootstrap.pp index b060a6d0..80627a06 100644 --- a/manifests/pip/bootstrap.pp +++ b/manifests/pip/bootstrap.pp @@ -10,11 +10,12 @@ # } # class python::pip::bootstrap ( + Array[String[1]] $pip_lookup_path, Enum['pip', 'pip3'] $version = 'pip', Variant[Boolean, String] $manage_python = false, Optional[Stdlib::HTTPUrl] $http_proxy = undef, String[1] $exec_provider = 'shell', -) inherits python::params { +) { if $manage_python { include python } else { @@ -36,7 +37,7 @@ command => '/usr/bin/curl https://bootstrap.pypa.io/get-pip.py | python3', environment => $environ, unless => 'which pip3', - path => $python::params::pip_lookup_path, + path => $pip_lookup_path, require => Package['python3'], provider => $exec_provider, } @@ -53,7 +54,7 @@ command => '/usr/bin/curl https://bootstrap.pypa.io/get-pip.py | python', environment => $environ, unless => 'which pip', - path => $python::params::pip_lookup_path, + path => $pip_lookup_path, require => Package['python'], provider => $exec_provider, } diff --git a/spec/classes/python_spec.rb b/spec/classes/python_spec.rb index cca3ef65..40399626 100644 --- a/spec/classes/python_spec.rb +++ b/spec/classes/python_spec.rb @@ -13,7 +13,6 @@ context 'with defaults' do it { is_expected.to compile.with_all_deps } it { is_expected.to contain_class('python::install') } - it { is_expected.to contain_class('python::params') } it { is_expected.to contain_class('python::config') } it { is_expected.to contain_package('python') } diff --git a/spec/default_module_facts.yml b/spec/default_module_facts.yml deleted file mode 100644 index 3f8d54e2..00000000 --- a/spec/default_module_facts.yml +++ /dev/null @@ -1,3 +0,0 @@ ---- -python_version: "2.7" -python3_version: ~ diff --git a/spec/defines/requirements_spec.rb b/spec/defines/requirements_spec.rb index 1b9a75fb..98f17e5e 100644 --- a/spec/defines/requirements_spec.rb +++ b/spec/defines/requirements_spec.rb @@ -47,7 +47,6 @@ it { is_expected.to compile.with_all_deps } it { is_expected.to contain_class('python::config') } it { is_expected.to contain_class('python::install') } - it { is_expected.to contain_class('python::params') } it { is_expected.to contain_class('python') } it { is_expected.to contain_exec('python_requirements/requirements.txt') } diff --git a/spec/spec.opts b/spec/spec.opts deleted file mode 100644 index 91cd6427..00000000 --- a/spec/spec.opts +++ /dev/null @@ -1,6 +0,0 @@ ---format -s ---colour ---loadby -mtime ---backtrace From 3a3ff1d2524db2ccb3767ba78cfc18227b5e4bfb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Romain=20Tarti=C3=A8re?= Date: Mon, 20 Mar 2023 16:32:41 -1000 Subject: [PATCH 2/2] Workaround CI failure with explicit lookup I would expect Automatic Parameter Lookup to do exactly this, but it does not. --- REFERENCE.md | 2 ++ manifests/pip.pp | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/REFERENCE.md b/REFERENCE.md index 5b1ba50e..e705cfde 100644 --- a/REFERENCE.md +++ b/REFERENCE.md @@ -753,6 +753,8 @@ Data type: `String[1]` The group of the virtualenv being manipulated. +Default value: `lookup('python::pip::group')` + ##### `index` Data type: `Variant[Boolean,String[1]]` diff --git a/manifests/pip.pp b/manifests/pip.pp index a03e59f0..e9e3d244 100644 --- a/manifests/pip.pp +++ b/manifests/pip.pp @@ -49,7 +49,7 @@ # } # define python::pip ( - String[1] $group, + String[1] $group = lookup('python::pip::group'), # lint:ignore:lookup_in_parameter String[1] $pkgname = $name, Variant[Enum[present, absent, latest], String[1]] $ensure = present, Variant[Enum['system'], Stdlib::Absolutepath] $virtualenv = 'system',