#753 - set up an order system to let eyeo employees file order requests

modules/trac/manifests/init.pp

Index: modules/trac/manifests/init.pp
===================================================================
--- a/modules/trac/manifests/init.pp
+++ b/modules/trac/manifests/init.pp
@@ -2,19 +2,29 @@
     $domain,
     $certificate,
     $private_key,
+    $fcgi_config_dir = '/etc/nginx/trac.d',

[Wladimir Palant, 2014/07/31 09:46:53]

This path is hardcoded in the site.conf, no point having it as a configurable
parameter then.

[mathias, 2014/08/01 13:59:46]

Well, since the site.conf is just one line, I removed it in favor of
$fcgi_config_dir ;-) - As soon as we may need more directives in there we can
convert it into a template.

     $is_default = false) inherits private::trac {
+
   package {['python-mysqldb','python-pip','subversion', 'tofrodos', 'graphviz']:
     ensure => present
   }
 
   include nginx, spawn-fcgi
 
+  file {$fcgi_config_dir:
+    ensure => directory,
+    owner => 'root',
+    mode => 755,
+    require => Package['nginx'],
+  }
+
   nginx::hostconfig {$domain:
     source => 'puppet:///modules/trac/site.conf',
     is_default => $is_default,
     certificate => $certificate,
     private_key => $private_key,
-    log => 'access_log_trac'
+    log => 'access_log_trac',
+    require => File[$fcgi_config_dir],
   }
 
   user {'trac':
@@ -25,17 +35,7 @@
   }
 
   class {'mysql::server':
-    root_password => $database_root_password
-  }
-
-  mysql::db {'trac':
-    user => 'trac',
-    password => $database_password,
-    host => 'localhost',
-    grant => ['all'],
-    charset => 'utf8',
-    collate => 'utf8_bin',
-    require => Class['mysql::server']
+    root_password => $database_root_password,
   }
 
   Exec {
@@ -48,16 +48,6 @@
     unless => "python -c 'import trac,sys;sys.exit(0 if trac.__version__ == \"1.0.1\" else 1)'",
   }
 
-  exec { 'trac_env':
-    command => "trac-admin /home/trac/environment initenv \"Adblock Plus issue tracker\" mysql://trac:${database_password}@localhost:3306/trac",
-    require => [
-      Exec['install_trac'],
-      Mysql_grant['trac@localhost/trac.*']
-    ],
-    user => trac,
-    unless => "test -d /home/trac/environment"
-  }
-
   exec { 'install_BlackMagicTicketTweaks':
     command => "pip install svn+http://trac-hacks.org/svn/blackmagictickettweaksplugin/0.12/",
     require => Package['subversion', 'python-pip'],
@@ -118,72 +108,154 @@
     unless => "python -c 'import tracspamfilter'",
   }
 
-  file {"/home/trac/environment/conf/trac.ini":
-    ensure => present,
-    content => template('trac/trac.ini.erb'),
-    owner => trac,
-    require => Exec['trac_env']
-  }
-
   exec { 'install_Tractags':
     command => "pip install svn+http://trac-hacks.org/svn/tagsplugin/tags/0.7/",
     require => Package['python-pip'],
     unless => "python -c 'import tractags'",
   }
 
-  file {"/home/trac/htdocs/htdocs/common/adblockplus_logo.png":
-    ensure => present,
-    source => 'puppet:///modules/trac/adblockplus_logo.png',
-    owner => trac,
-    require => Exec['deploy']
+  exec { 'install_PrivateTickets':
+    command => "pip install svn+http://trac-hacks.org/svn/privateticketsplugin/tags/2.0.2/",
+    require => Package['subversion', 'python-pip'],
+    unless => "python -c 'import privateticketsplugin'",
   }
 
-  file {"/home/trac/environment/htdocs/theme.css":
+  file { '/home/trac/trac.ini':
     ensure => present,
-    source => 'puppet:///modules/trac/theme.css',
-    owner => trac,
-    require => Exec['trac_env']
+    source => 'puppet:///modules/trac/trac.ini',
+    owner => 'trac',
+    mode => 644,
   }
 
-  exec {"update_env":
-    command => "trac-admin /home/trac/environment upgrade",
-    user => trac,
-    require => [
-      File['/home/trac/environment/conf/trac.ini'],
-      Exec['install_SensitiveTickets'],
-      Exec['install_BlackMagicTicketTweaks'],
-      Exec['install_AccountManager'],
-      Exec['install_AutocompleteUsers'],
-      Exec['install_TicketTemplate'],
-      Exec['install_NeverNotifyUpdater'],
-      Exec['install_MasterTickets'],
-      Exec['install_ThemeEngine'],
-      Exec['install_Tractags'],
-      Exec['install_TracSpamFilter']]
+  define instance (
+      $config = 'trac/trac.ini.erb',
+      $description = 'Issue Tracker',
+      $environment = 'environment',

[Wladimir Palant, 2014/07/31 09:46:53]

It's probably better to just set environment based on name and not have it
configurable, same as with deployment directories:

  $environment = "environment-$name"

[mathias, 2014/08/01 13:59:46]

Sure, but there's another but: This will cause the trac-admin/initenv to become
a major source of error, requiring a hack (see issueserver.pp). Both versions
are somehow ugly, but mine/the former one does not introduce any quirks in the
file system.

+      $location = '/',
+      $logo = 'eyeo_logo.png',
+      $database = 'trac',
+      $permissions = 'puppet:///modules/trac/permissions.csv',
+      $theme = 'puppet:///modules/trac/theme.css') {
+
+    $database_password = $private::trac::database_password
+ 
+    mysql::db {$database:
+      user => 'trac',
+      password => $database_password,
+      host => 'localhost',
+      grant => ['all'],
+      charset => 'utf8',
+      collate => 'utf8_bin',
+      require => Class['mysql::server'],
+    }
+
+    $location_base = $location ? {
+        '/' => '',
+        default => $location,
+    }

[Wladimir Palant, 2014/07/31 09:46:53]

I think what you really want here is removing trailing slashes:

  $location_base = regsubst($location, '/+$', '')

[mathias, 2014/08/01 13:59:46]

Makes sense, done.

+
+    file {"${trac::fcgi_config_dir}/${name}.conf":
+      ensure => file,
+      owner => 'root',
+      mode => 644,
+      content => "
+        location $location_base/
+        {
+          fastcgi_pass unix:/tmp/${name}-fastcgi.sock;
+          fastcgi_split_path_info ^(${location_base})(.*)\$;
+          include /etc/nginx/fastcgi_params;
+          fastcgi_param SCRIPT_NAME \$fastcgi_script_name;
+          fastcgi_param PATH_INFO \$fastcgi_path_info;
+          client_max_body_size 20m;
+        }
+        location $location_base/chrome/
+        {
+          alias /home/trac/htdocs-$name/htdocs/;
+        }",

[Wladimir Palant, 2014/07/31 09:46:53]

I'd rather go with a template here, having nginx configuration files inline is
ugly (also because of the escaping required).

[mathias, 2014/08/01 13:59:46]

Done.

+      require => File[$trac::fcgi_config_dir],
+    }

[Wladimir Palant, 2014/07/31 09:46:53]

This needs:

  notify => Service['nginx']

Otherwise nginx might not pick up the changes to configuration files.

[mathias, 2014/08/01 13:59:46]

Done.

+  
+    exec {"trac_env_${name}":
+      command => shellquote(
+        'trac-admin', "/home/trac/$environment", 'initenv', $description,
+        "mysql://trac:${database_password}@localhost:3306/$database"),
+      path => '/usr/bin:/usr/sbin:/bin:/usr/local/bin',
+      require => [
+        Exec['install_trac'],
+        Mysql_grant["trac@localhost/${database}.*"]],
+      user => trac,
+      unless => "test -d /home/trac/$environment"
+    }
+
+    file {"/home/trac/${environment}/conf/permissions.csv":
+      ensure => present,
+      owner => trac,
+      source => $permissions,
+      require => Exec["trac_env_$name"],
+    }
+  
+    file {"/home/trac/$environment/conf/trac.ini":
+      ensure => present,
+      content => template($config),
+      owner => trac,
+      require => Exec["trac_env_$name"]
+    }
+  
+    file {"/home/trac/$environment/htdocs/theme.css":
+      ensure => present,
+      source => $theme,
+      owner => trac,
+      require => Exec["trac_env_$name"],
+    }
+  
+    exec {"update_env_$name":
+      command => "trac-admin /home/trac/$environment upgrade",
+      user => trac,
+      path => '/usr/bin:/usr/sbin:/bin:/usr/local/bin',
+      require => [
+        File["/home/trac/$environment/conf/trac.ini"],
+        Exec['install_SensitiveTickets'],
+        Exec['install_BlackMagicTicketTweaks'],
+        Exec['install_AccountManager'],
+        Exec['install_AutocompleteUsers'],
+        Exec['install_TicketTemplate'],
+        Exec['install_NeverNotifyUpdater'],
+        Exec['install_MasterTickets'],
+        Exec['install_ThemeEngine'],
+        Exec['install_Tractags'],
+        Exec['install_TracSpamFilter'],
+        Exec['install_PrivateTickets']],
+    }
+  
+    exec {"deploy_$name":
+      command => "trac-admin /home/trac/$environment \
+        deploy /home/trac/htdocs-$name \
+        && fromdos /home/trac/htdocs-$name/cgi-bin/trac.fcgi \
+        && chmod 755 /home/trac/htdocs-$name/cgi-bin/trac.fcgi",
+      user => trac,
+      path => '/usr/bin:/usr/sbin:/bin:/usr/local/bin',
+      require => [
+        Exec["update_env_$name"],
+        Package["tofrodos"]],
+    }
+  
+    file {"/home/trac/htdocs-$name/htdocs/common/$logo":

[Wladimir Palant, 2014/07/31 09:46:53]

Feel free to always name that file logo.png (naming conflict has been resolved
by separating deployment directories), regardless of the source file name. Then
this entry in trac.ini will be static and you will be able to use an absolute
path for the $logo parameter, consistent with $theme.

[mathias, 2014/08/01 13:59:46]

Done.

+      ensure => present,
+      source => "puppet:///modules/trac/$logo",
+      owner => trac,
+      require => Exec["deploy_$name"],
+    }
+
+    spawn-fcgi::pool {"${name}d":

[Wladimir Palant, 2014/07/31 09:46:53]

I think "tracd_$name" was a better name for the service...

[mathias, 2014/08/01 13:59:46]

Sure, I think so too. But I've had a hard time trying to find a way to tell the
spawn-fcgi module that the former "tracd" is gone and replaced by two new
instances. It always failed to do anything right (when migrating from the
current production setup, a plain installation worked of course), so I figured
it may be better not to waste any more time on that (with the current version
the old name remains in use). However, after your review I've applied a hack to
issueserver.pp - it works now with more verbose pool names, but is still ugly.

+      ensure => present,
+      fcgi_app => "/home/trac/htdocs-$name/cgi-bin/trac.fcgi",
+      socket => "/tmp/${name}-fastcgi.sock",
+      mode => "0666",
+      user => trac,
+      children => 1,
+      require => Exec["deploy_$name"],
+      notify => Service['nginx'],

[Wladimir Palant, 2014/07/31 09:46:53]

This one is pointless - nginx doesn't care whether the FCGI process is running,
it will simply produce "502 Bad Gateway" if it isn't.

[mathias, 2014/08/01 13:59:46]

Not quite correct, I believe. It's the file descriptors to the sockets that
nginx(8) needs to re-open if it still has a handle on any of them. Otherwise one
gets gateway errors although the processes are running. At least that's what
we've encountered during the tests.

[Wladimir Palant, 2014/08/01 14:11:13]

We had FCGI processes crash often enough that I know - nginx doesn't need to be
reloaded if the process is restared.

[mathias, 2014/08/01 16:25:03]

Indeed, Nginx will re-open the descriptor - after it realizes that the handle is
invalid, at least. However, since it's only during the computation of the first
page view after the provision, I'll remove it.

+    }
   }
+}
 
-  exec {"deploy":
-    command => "trac-admin /home/trac/environment deploy /home/trac/htdocs && fromdos /home/trac/htdocs/cgi-bin/trac.fcgi && chmod 755 /home/trac/htdocs/cgi-bin/trac.fcgi",
-    user => trac,
-    require => [
-      Exec["update_env"],
-      Package["tofrodos"]]
-  }
-
-  spawn-fcgi::pool {"tracd":
-    ensure => present,
-    fcgi_app => "/home/trac/htdocs/cgi-bin/trac.fcgi",
-    socket => "/tmp/trac-fastcgi.sock",
-    mode => "0666",
-    user => trac,
-    children => 1,
-    require => Exec['deploy'],
-  }
-
-  file {"/home/trac/permissions.csv":
-    ensure => present,
-    owner => trac,
-    source => 'puppet:///modules/trac/permissions.csv'
-  }
-
-}