From 52c3355a7b3892e36f2ce25000616e8ef5cb95cc Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Romain=20Tarti=C3=A8re?= <romain@blogreen.org>
Date: Fri, 15 Sep 2023 10:13:25 -1000
Subject: [PATCH] Install venv packages in python::install::venv

---
 REFERENCE.md                      |  5 +++++
 manifests/install.pp              | 11 +--------
 manifests/install/venv.pp         | 13 +++++++++++
 manifests/params.pp               |  2 ++
 manifests/pyvenv.pp               |  1 +
 spec/classes/install/venv_spec.rb | 37 +++++++++++++++++++++++++++++++
 spec/classes/python_spec.rb       | 12 ++++------
 7 files changed, 63 insertions(+), 18 deletions(-)
 create mode 100644 manifests/install/venv.pp
 create mode 100644 spec/classes/install/venv_spec.rb

diff --git a/REFERENCE.md b/REFERENCE.md
index d46ccdc0..1dd7e63a 100644
--- a/REFERENCE.md
+++ b/REFERENCE.md
@@ -10,6 +10,7 @@
 
 * [`python`](#python): Installs and manages python, python-dev and gunicorn.
 * [`python::install::dev`](#python--install--dev): Installs python development packages
+* [`python::install::venv`](#python--install--venv): Installs python virtualenv packages
 * [`python::pip::bootstrap`](#python--pip--bootstrap): allow to bootstrap pip when python is managed from other module
 
 #### Private Classes
@@ -294,6 +295,10 @@ Default value: `'/opt/python'`
 
 Installs python development packages
 
+### <a name="python--install--venv"></a>`python::install::venv`
+
+Installs python virtualenv packages
+
 ### <a name="python--pip--bootstrap"></a>`python::pip::bootstrap`
 
 allow to bootstrap pip when python is managed from other module
diff --git a/manifests/install.pp b/manifests/install.pp
index d07465d0..348aeda3 100644
--- a/manifests/install.pp
+++ b/manifests/install.pp
@@ -32,16 +32,7 @@
   }
 
   if $python::manage_venv_package {
-    ##
-    ## CentOS has no extra package for venv
-    ##
-    unless $facts['os']['family'] == 'RedHat' {
-      package { 'python-venv':
-        ensure  => $python::venv,
-        name    => "${python}-venv",
-        require => Package['python'],
-      }
-    }
+    contain python::install::venv
   }
 
   case $python::provider {
diff --git a/manifests/install/venv.pp b/manifests/install/venv.pp
new file mode 100644
index 00000000..c0d0abb4
--- /dev/null
+++ b/manifests/install/venv.pp
@@ -0,0 +1,13 @@
+# @summary Installs python virtualenv packages
+class python::install::venv {
+  include python
+
+  # Main python package bundle venv on some operating systems
+  unless $facts['os']['family'] in ['Archlinux', 'FreeBSD', 'RedHat'] {
+    package { 'python-venv':
+      ensure  => $python::venv,
+      name    => "${python::install::python}-venv",
+      require => Package['python'],
+    }
+  }
+}
diff --git a/manifests/params.pp b/manifests/params.pp
index 49bfdeac..c61b1b5d 100644
--- a/manifests/params.pp
+++ b/manifests/params.pp
@@ -40,6 +40,8 @@
   }
   $manage_venv_package = $facts['os']['family'] ? {
     'Archlinux' => false,
+    'FreeBSD'   => false,
+    'RedHat'    => false,
     default     => true,
   }
 }
diff --git a/manifests/pyvenv.pp b/manifests/pyvenv.pp
index 14dcb9fb..dafc66f0 100644
--- a/manifests/pyvenv.pp
+++ b/manifests/pyvenv.pp
@@ -36,6 +36,7 @@
   Python::Venv::PipVersion    $pip_version = 'latest',
 ) {
   include python
+  include python::install::venv
 
   if $ensure == 'present' {
     $python_version = $version ? {
diff --git a/spec/classes/install/venv_spec.rb b/spec/classes/install/venv_spec.rb
new file mode 100644
index 00000000..88ef6b6a
--- /dev/null
+++ b/spec/classes/install/venv_spec.rb
@@ -0,0 +1,37 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe 'python::install::venv' do
+  on_supported_os.each do |os, facts|
+    context "on #{os}" do
+      let :facts do
+        facts
+      end
+
+      context 'with default settings' do
+        if %w[Archlinux FreeBSD RedHat].include?(facts[:os]['family'])
+          it { is_expected.not_to contain_package('python-venv') }
+        else
+          it { is_expected.to contain_package('python-venv').with(ensure: 'absent') }
+        end
+      end
+
+      context 'when ensuring venv is setup' do
+        let(:pre_condition) do
+          <<~PP
+            class { 'python':
+              venv => present,
+            }
+          PP
+        end
+
+        if %w[Archlinux FreeBSD RedHat].include?(facts[:os]['family'])
+          it { is_expected.not_to contain_package('python-venv') }
+        else
+          it { is_expected.to contain_package('python-venv').with(ensure: 'present') }
+        end
+      end
+    end
+  end
+end
diff --git a/spec/classes/python_spec.rb b/spec/classes/python_spec.rb
index 54c47fc7..f6fcf7d9 100644
--- a/spec/classes/python_spec.rb
+++ b/spec/classes/python_spec.rb
@@ -23,10 +23,10 @@
           it { is_expected.to contain_package('pip') }
         end
 
-        if %w[Archlinux RedHat].include?(facts[:os]['family'])
-          it { is_expected.not_to contain_package('python-venv') }
+        if %w[Archlinux FreeBSD RedHat].include?(facts[:os]['family'])
+          it { is_expected.not_to contain_class('python::install::venv') }
         else
-          it { is_expected.to contain_package('python-venv') }
+          it { is_expected.to contain_class('python::install::venv') }
         end
       end
 
@@ -44,23 +44,19 @@
         it { is_expected.not_to contain_package('python') }
         it { is_expected.not_to contain_package('python-dev') }
         it { is_expected.not_to contain_package('pip') }
-        it { is_expected.not_to contain_package('python-venv') }
+        it { is_expected.not_to contain_class('python::install::venv') }
       end
 
       context 'with packages present' do
         let :params do
           {
             manage_pip_package: true,
-            manage_venv_package: true,
             pip: 'present',
-            venv: 'present'
           }
         end
 
         it { is_expected.to compile.with_all_deps }
         it { is_expected.to contain_package('pip').with(ensure: 'present') }
-
-        it { is_expected.to contain_package('python-venv').with(ensure: 'present') } unless facts[:os]['family'] == 'RedHat'
       end
 
       case facts[:os]['family']