Add rietveld to infrastructure

modules/rietveld/manifests/init.pp

 class rietveld($domain) inherits private::rietveld {
 
   include nginx
+  $django_home = '/home/rietveld/django-gae2django'
+  $rietveld_home = "${django_home}/examples/rietveld"
+
+  Exec {
+    path => '/usr/bin:/usr/sbin:/bin:/usr/local/bin',
+  }
 
   nginx::hostconfig {$domain:
     content => template('rietveld/site.erb'),
     enabled => true
   }
 
   package {['python-django', 'make', 'patch', 'gunicorn']: ensure => present}
 
   user {'rietveld':
     ensure => present,
     comment => 'User of the rietveld installation',
     home => '/home/rietveld',
     managehome => true
   }
 
-  exec { "get_rietveld":

[Felix Dahlke, 2014/02/07 15:27:36]

No space after {, and ' instead of ". Likewise below.

[christian, 2014/02/07 17:17:21]

Done.

-    command => "hg clone https://code.google.com/p/django-gae2django/ /home/rietveld/django-gae2django/",

[Felix Dahlke, 2014/02/07 15:27:36]

' instead of ", likewise various times below. According to the style guide,
strings that don't contain variables should use single quotes.

[christian, 2014/02/07 17:17:21]

Done.

+  exec {'get_rietveld':
+    command => "hg clone https://code.google.com/p/django-gae2django/ ${django_home}/",
     require => Package['mercurial'],
     user => rietveld,
-    onlyif => "test ! -d /home/rietveld/django-gae2django",

[Felix Dahlke, 2014/02/07 15:27:36]

This path (/home/rietveld/django-gae2django) is being repeated a lot, should be
a variable.

[christian, 2014/02/07 17:17:21]

Done.

-    path => "/usr/bin:/usr/sbin:/bin:/usr/local/bin",

[Felix Dahlke, 2014/02/07 15:27:36]

This is repeated below, you can have this above instead:

Exec {
  path => "…"
}

That'd apply to every exec resource.

[christian, 2014/02/07 17:17:21]

Done.

-    #refreshonly => true,

[Felix Dahlke, 2014/02/07 15:27:36]

Let's remove this if we don't need it. Likewise below.

[christian, 2014/02/07 17:17:21]

Done.

+    onlyif => "test ! -d ${django_home}",
   }
 
-  file {'/home/rietveld/django-gae2django/examples/rietveld/Makefile':

[Felix Dahlke, 2014/02/07 15:27:36]

This path (/home/rietveld/django-gae2django/examples/rietveld) is also being
used a lot and should be a variable IMO.

[christian, 2014/02/07 17:17:21]

Done.

+  file {"${rietveld_home}/Makefile":
     ensure => file,
     owner => rietveld,
     require => Exec['get_rietveld'],
     source => 'puppet:///modules/rietveld/Makefile',
   }
 
-  file {'/home/rietveld/django-gae2django/examples/rietveld/settings.py':
+  file {"${rietveld_home}/settings.py":
     ensure => file,
     owner => rietveld,
     require => Exec['get_rietveld'],
-    source => 'puppet:///modules/rietveld/settings.py',
+    content => template('rietveld/settings.py.erb'),
   }
 
-  exec { "install_rietveld":
+  exec {'install_rietveld':
     command => "make all",

[Felix Dahlke, 2014/02/07 15:27:36]

"all" is not necessary

[christian, 2014/02/07 17:17:21]

 Actual it is,see Makefile

default:
	@echo "Run 'make all' to fetch required sources to run this example."

[Felix Dahlke, 2014/02/07 20:09:45]

Ah. Well that's peculiar :)

-    cwd => "/home/rietveld/django-gae2django/examples/rietveld",
+    cwd => "${rietveld_home}",
     user => rietveld,
     require => [
-        File['/home/rietveld/django-gae2django/examples/rietveld/Makefile'],

[Felix Dahlke, 2014/02/07 15:27:36]

Should be indented just one level deeper than the line above, i.e. 2 spaces.
Likewise below.

[christian, 2014/02/07 17:17:21]

Done.

-        File['/home/rietveld/django-gae2django/examples/rietveld/settings.py']],
-    onlyif => "test ! -f /home/rietveld/django-gae2django/examples/rietveld/dev.db",
-    path => "/usr/bin:/usr/sbin:/bin:/usr/local/bin",
-    #refreshonly => true,
+      File["${rietveld_home}/Makefile"],
+      File["${rietveld_home}/settings.py"]],
+    onlyif => "test ! -f ${$rietveld_home}/dev.db",
   }
 
   file {'/etc/init/rietveld.conf':
     ensure => file,
     owner => root,
     source => 'puppet:///modules/rietveld/rietveld.conf',
     notify => Service['rietveld'],
   }
 
-  #work around puppets limitation towards upstart

[Felix Dahlke, 2014/02/07 15:27:36]

This is not really clear, what particular limitation is there with upstart and
Puppet that causes us to fall back on init scripts here? Any particular bug you
can link to?

[christian, 2014/02/07 17:17:21]

It is simple, puppet is not aware of upstart so it tries to start up via init.d,
so I use this method to not to include maybe a buggy third party method to use
the upstart script. I have not found a particular bug, but some things in their
tracker:
https://tickets.puppetlabs.com/browse/PUP-876?jql=text%20~%20%22upstart%22

[Felix Dahlke, 2014/02/07 20:09:45]

If Puppet can't do Upstart at all, I don't think this deserves a comment at all
actually. It's to be expected then.

[christian, 2014/02/12 14:14:26]

Done.

-  file { '/etc/init.d/rietveld':
+  file {'/etc/init.d/rietveld':
     ensure => link,
     target => '/lib/init/upstart-job',
-    require => File['/etc/init/rietveld.conf']

[Felix Dahlke, 2014/02/07 15:27:36]

Shouldn't this require Exec['install_rietveld']? Not requiring anything would
also be fine, but I think it'd make sense to do this after the installation.
That way you wouldn't have to require Exec['install_rietveld'] below in the
service resource.

[christian, 2014/02/07 17:17:21]

This only makes the symlink, the service it self need the install. It's just
shifting the need for the install from the service to the file. And for me it
was more logical to put it by the service. But if you think it would be more
belong here, I can put the dependency here.

[Felix Dahlke, 2014/02/07 20:09:45]

Well, I think /etc/init.d/rietveld logically depends on both Rietveld and
/etc/init/rietveld.conf. So I'd either require both here or both below with the
service. Don't have a strong opinion on what'd make more sense.

[christian, 2014/02/12 14:14:26]

Done.

+    require => [File['/etc/init/rietveld.conf'], Exec['install_rietveld']]
   }
 
   service {'rietveld':
     ensure => running,
     hasstatus => false,
-    require => [Package['gunicorn'], File['/etc/init.d/rietveld'], Exec['install_rietveld']]
+    require => [Package['gunicorn'], File['/etc/init.d/rietveld']]
   }
 
-  exec { "set_superuser":
-    command => "echo \"from django.db import DEFAULT_DB_ALIAS as database; from django.contrib.auth.models import User; User.objects.db_manager(database).create_superuser('rietveld', 'rietveld@example.org', '${admin_password}')\" | ./manage.py shell",

[Felix Dahlke, 2014/02/07 15:27:36]

I suppose it's fine to just have a single admin account for now, although shared
accounts are generally a bit of a pain. But it should probably be called "admin"
and definitely have "admins@adblockplus.org" as its email address.

Also, can't we do this in settings.py?

[christian, 2014/02/07 17:17:21]

I found it easier to just make this one account and enter the rest of the
accounts via the webinterface, we only need to do this once. And we cannot do
this via settings.py, because we have no oauth in this version, if i remember
correctly.

[Felix Dahlke, 2014/02/07 20:09:45]

Ah, I see. Yeah sure, we only need to create the admin account(s) via Puppet.

-    cwd => "/home/rietveld/django-gae2django/examples/rietveld",
+  exec {'set_superuser':
+    command => "echo \"from django.db import DEFAULT_DB_ALIAS as database; from django.contrib.auth.models import User; User.objects.db_manager(database).create_superuser('admin', 'admins@adblockplus.org', '${admin_password}')\" | ./manage.py shell",
+    cwd => "${rietveld_home}",
     require => Exec['install_rietveld'],
-    path => "/usr/bin:/usr/sbin:/bin:/usr/local/bin",
-    #refreshonly => true,
   }
 }