Made system virtualenv more robust and ensured that pip logging would occur in an appropriate directory upon failure to avoid errors

fgimian · fgimian · commit 1aaef848f4a5 · 2013-06-03T23:13:50.000+10:00
diff --git a/manifests/pip.pp b/manifests/pip.pp
@@ -28,7 +28,7 @@
 # Sergey Stankevich
 #
 define python::pip (
-  $virtualenv,
+  $virtualenv = 'system',
   $ensure     = present,
   $url        = false,
   $owner      = 'root',
@@ -42,6 +42,20 @@
     fail('python::pip: virtualenv parameter must not be empty')
   }
 
+  if $virtualenv == 'system' and ($owner != 'root' or $group != 'root') {
+    fail('python::pip: root user must be used when virtualenv is system')
+  }
+
+  $cwd = $virtualenv ? {
+    'system' => '/',
+    default  => "${virtualenv}",
+  }
+
+  $pip_env = $virtualenv ? {
+    'system' => '`which pip`',
+    default  => "${virtualenv}/bin/pip",
+  }
+
   $proxy_flag = $proxy ? {
     false    => '',
     default  => "--proxy=${proxy}",
@@ -60,17 +74,17 @@
   case $ensure {
     present: {
       exec { "pip_install_${name}":
-        command     => "${virtualenv}/bin/pip install ${proxy_flag} ${source}",
-        unless      => "${virtualenv}/bin/pip freeze | grep -i -e ${grep_regex}",
+        command     => "$pip_env --log-file ${cwd}/pip.log install ${proxy_flag} ${source}",
+        unless      => "$pip_env freeze | grep -i -e ${grep_regex}",
         user        => $owner,
         environment => $environment,
       }
     }
 
     default: {
       exec { "pip_uninstall_${name}":
-        command     => "echo y | ${virtualenv}/bin/pip uninstall ${proxy_flag} ${name}",
-        onlyif      => "${virtualenv}/bin/pip freeze | grep -i -e ${grep_regex}",
+        command     => "echo y | $pip_env uninstall ${proxy_flag} ${name}",
+        onlyif      => "$pip_env freeze | grep -i -e ${grep_regex}",
         user        => $owner,
         environment => $environment,
       }
diff --git a/manifests/requirements.pp b/manifests/requirements.pp
@@ -33,9 +33,13 @@
   $group        = 'root'
 ) {
 
+  if $virtualenv == 'system' and ($owner != 'root' or $group != 'root') {
+    fail('python::pip: root user must be used when virtualenv is system')
+  }
+
   $cwd = $virtualenv ? {
     'system' => '/',
-    default  => "${virtualenv}/bin/",
+    default  => "${virtualenv}",
   }
 
   $pip_env = $virtualenv ? {
@@ -64,8 +68,7 @@
 
   exec { "python_requirements${name}":
     provider    => shell,
-    command     => "${pip_env} install ${proxy_flag} -r ${requirements}",
-    cwd         => $cwd,
+    command     => "${pip_env} --log-file ${cwd}/pip.log install ${proxy_flag} -r ${requirements}",
     refreshonly => true,
     timeout     => 1800,
     user        => $owner,
diff --git a/manifests/virtualenv.pp b/manifests/virtualenv.pp
@@ -85,15 +85,15 @@
     }
 
     exec { "python_virtualenv_${venv_dir}":
-      command => "mkdir -p ${venv_dir} ${proxy_command} && virtualenv -p `which ${python}` ${system_pkgs_flag} ${venv_dir} && ${venv_dir}/bin/pip install ${pypi_index} ${proxy_flag} --upgrade ${distribute_pkg} pip",
+      command => "mkdir -p ${venv_dir} ${proxy_command} && virtualenv -p `which ${python}` ${system_pkgs_flag} ${venv_dir} && ${venv_dir}/bin/pip --log-file ${venv_dir}/pip.log install ${pypi_index} ${proxy_flag} --upgrade ${distribute_pkg} pip",
       user    => $owner,
       creates => "${venv_dir}/bin/activate",
       path    => [ '/bin', '/usr/bin', '/usr/sbin' ],
     }
 
     if $requirements {
       exec { "python_requirements_initial_install_${requirements}_${venv_dir}":
-        command     => "${venv_dir}/bin/pip install ${pypi_index} ${proxy_flag} --requirement ${requirements}",
+        command     => "${venv_dir}/bin/pip --log-file ${venv_dir}/pip.log install ${pypi_index} ${proxy_flag} --requirement ${requirements}",
         refreshonly => true,
         timeout     => 1800,
         user        => $owner,
