Issue 2471 - Update our Discourse instance

modules/discourse/manifests/init.pp

 class discourse(
     $domain,
     $certificate,
     $private_key,
     $is_default = false
   ) inherits private::discourse {
 
+  $hg_revision = 'd31d28147381'
+  $git_revision = '8a3a02421a39f53b6adf3ca9a6fdba73f42bc932'
+  $ruby_version = '2.1.2'
+  $postgresql_version = '9.3'
+
   class { 'postgresql::globals':
     manage_package_repo => true,
-    version => '9.3',
+    version => $postgresql_version,
   }->
   class {"postgresql::server":}
 
   class {"postgresql::server::contrib":
     package_ensure => 'present',
   }
 
   postgresql::server::database {'discourse':}
 
   postgresql::server::role {'discourse':
@@ skipping 13 matching lines @@
   $gem_dependencies = ['libpq-dev']
   $image_optim_dependencies = ['advancecomp', 'gifsicle', 'jhead', 'jpegoptim',
       'libjpeg-progs', 'optipng', 'pngcrush']
   $image_sorcery_dependencies = 'imagemagick'
 
   package {[$rvm_dependencies, $discourse_dependencies, $gem_dependencies, $image_optim_dependencies, $image_sorcery_dependencies]:
     ensure => present
   }
 
   Exec <| tag == 'rvm' |> {
-    path => '/bin:/usr/bin:/usr/sbin:/usr/local/bin:/home/discourse/.rvm/bin',

[mathias, 2015/05/09 15:34:19]

Do you know why the directory precedence here is inverted, in contrast to what
one would expect? I do realize that the former version did the same, but there's
no obvious reason for either one - and it looks like it can quickly become an
issue.

[Wladimir Palant, 2015/05/11 15:00:42]

Done.

+    path => '/home/discourse/.rvm/bin:/usr/local/bin:/usr/bin:/bin',
     user => discourse,
     group => www-data,
     environment => ['HOME=/home/discourse'],
   }
 
   exec {'install-rvm-key':
     command => 'gpg --keyserver hkp://keys.gnupg.net --recv-keys 409B6B1796C275462A1703113804BB82D39DC0E3',
     tag => 'rvm',
     unless => 'gpg --list-keys | grep D39DC0E3',
   }
 
-  exec {'install-ruby':
-    command => 'curl -sSL https://get.rvm.io | bash -s stable --ruby=2.1.2',

[mathias, 2015/05/09 15:34:19]

Note that this does not cover upgrading to future Ruby versions, a specific
patch or similar. Also, it introduces yet another major (but avoidable)
dependency on external resources. However, it's fine for now that we want to fix
Discourse logins, but we have to re-visit it afterwards at least.

[Wladimir Palant, 2015/05/11 15:00:42]

Yes, I had RVM and Ruby installation separate initially but dropped that for
reasons of simplicity. I guess we'll want to reintroduce that separation.

I verified that updating Ruby actually works correctly now. However, I didn't
update to Ruby 2.2.2 yet because in my VM it produced out of memory errors.

+  exec {'install-rvm':
+    command => 'curl -sSL https://get.rvm.io | bash -s stable',
     tag => 'rvm',
     creates => '/home/discourse/.rvm',
     timeout => 0,
     logoutput => true,
     require => [Exec['install-rvm-key'], Package[$rvm_dependencies]],
   }
 
+  exec {'install-ruby':
+    command => "rvm install $ruby_version && rvm default do rvm --default use $ruby_version",
+    tag => 'rvm',
+    unless => "rvm list | grep $ruby_version",

[mathias, 2015/05/26 10:02:16]

Please use shellquote() to ensure $ruby_version cannot break anything here --

[Wladimir Palant, 2015/05/26 10:41:17]

Given that $ruby_version is a constant set above, do you consider that
necessary? Also, if we do it for $ruby_version then we should do it for
$hg_revision as well.

[mathias, 2015/05/26 10:54:36]

Indeed, it should be done there as well.

And yes I believe it's necessary. One should always assume the next programmer
to be less careful and wise than oneself (even if it will be oneself most
likely). And such a guy may forget to check for the use of $ruby_version when
migrating it to become a parameter one day (for example, but not even unlikely),
which can also easily be overseen in a code-review.

However, please note that we agreed last year on making use of shell_escape()
wherever it may make sense - just to have an easy but secure guideline that does
not require the same decision ("is it necessary?") over and over again..

+    timeout => 0,
+    logoutput => true,
+    notify => Exec['init-discourse'],
+    require => Exec['install-rvm'],
+  }
+
   exec {'install-bundler':

[mathias, 2015/05/09 15:34:19]

Note that this would be better handled as a Package with provider => gem --
Though our Puppet version may not be compatible with this version of Gem itself.
So gain this may be fine for now, but should become revisited at least.

[Wladimir Palant, 2015/05/11 15:00:42]

We need two different Ruby versions: one for Puppet/Hiera (1.9), another one for
Discourse (2.0+). So we don't touch the system Ruby version and install a
separate version for discourse user only.

Using Package will only install bundler in the system Ruby - that's what we did
before but it won't be usable with the Ruby installation in the discourse
account. So Exec seems to be the only way.

     command => 'rvm default do gem install bundler',
     tag => 'rvm',
-    unless => 'rvm default do gem list | grep "^bundler ")',
+    unless => 'rvm default do gem list | grep "^bundler "',
     require => Exec['install-ruby'],
   }
 
   file {'/opt/discourse':
     ensure => directory,
     mode => 755,
     owner => discourse,
     group => www-data
   }
 
   file {['/opt/discourse/tmp', '/opt/discourse/tmp/pids']:
     ensure => directory,
     mode => 755,
     owner => discourse,
     group => www-data,
     require => Exec['fetch-discourse']
   }
 
   file {'/opt/discourse/config/discourse.conf':
     mode => 600,
     owner => discourse,
     group => www-data,
     content => template('discourse/discourse.conf.erb'),
     notify => Service['discourse'],
-    require => Exec['fetch-discourse']
+    require => Exec['update-discourse']
   }
 
   file {'/usr/local/bin/init-discourse':
     mode => 0755,
     owner => root,
     group => root,
     source => 'puppet:///modules/discourse/init-discourse'
   }
 
   user {'discourse':
     ensure => present,
     comment => 'Discourse user',
     home => '/home/discourse',
     gid => www-data,
     password => '*',
     managehome => true
   }
 
   file {'/etc/sudoers.d/discourse':
     ensure => present,
     owner => root,
     group => root,
     mode => 0440,
     source => 'puppet:///modules/discourse/sudoers',
     require => User['discourse']
   }
 
   exec {'fetch-discourse':
-    command => "hg clone https://hg.adblockplus.org/discourse /opt/discourse",
+    command => 'hg clone --noupdate https://hg.adblockplus.org/discourse /opt/discourse',
     path => ["/usr/bin/", "/bin/"],
     user => discourse,
     group => www-data,
     timeout => 0,
     require => [Package['mercurial'], File['/opt/discourse']],
+    onlyif => "test ! -d /opt/discourse/.hg"
+  }
+
+  exec {'update-discourse':
+    command => "hg update -R /opt/discourse --clean -r $hg_revision",
+    unless => "hg id -R /opt/discourse | grep $hg_revision",
+    path => ["/usr/bin/", "/bin/"],
+    user => discourse,
+    group => www-data,
     notify => Exec['init-discourse'],
-    onlyif => "test ! -d /opt/discourse/.hg"

[mathias, 2015/05/09 15:34:19]

(Not part of this patch-set, but a bug that needs to be addressed as well;
cloning into a non-empty directory does not work. Though in practice that should
not happen with the code here, it's still a footgun.)

[Wladimir Palant, 2015/05/11 15:00:42]

Why would it be non-empty? It's created right before cloning - and it's only a
separate step because the discourse user doesn't have permission to write to
/opt.

-  }
-
-  file {'/opt/discourse/config/initializers/airbrake.rb':
-    ensure => absent,
-    before => Exec['init-discourse'],
+    require => Exec['fetch-discourse'],
   }
 
   file {'/opt/discourse/config/version.rb':
     ensure => present,
     owner => discourse,
     group => www-data,
 
     # This is hardcoded here so that Discourse doesn't try to extract it from
     # the repository. Ideally, we should update it when updating Discourse.
-    content => '$git_version = "8a3a02421a39f53b6adf3ca9a6fdba73f42bc932"',
-    require => Exec['fetch-discourse'],
+    content => "\$git_version = '$git_revision'",
+    require => Exec['update-discourse'],
     before => Exec['init-discourse'],
   }
 
   exec {'init-discourse':
     command => 'rvm default do /usr/local/bin/init-discourse',
     tag => 'rvm',
     subscribe => File['/usr/local/bin/init-discourse'],
     refreshonly => true,
     timeout => 0,
     logoutput => true,
     require => [Exec['install-bundler'],
                 Package[$discourse_dependencies, $gem_dependencies],
                 User['discourse'], File['/etc/sudoers.d/discourse'],
-                Exec['fetch-discourse'],
+                Exec['update-discourse'],
                 File['/opt/discourse/config/discourse.conf'],
                 Postgresql::Server::Role['discourse']]
   }
 
   Discourse::Sitesetting <| |> {
     require => Exec['init-discourse']
   }
 
   discourse::sitesetting {'title':
     ensure => present,
@@ skipping 149 matching lines @@
     ensure => present,
     type => 3,
     value => '50',
   }
 
   Discourse::Customservice <| |> {
     user => 'discourse',
     workdir => '/opt/discourse',
     env => ['RAILS_ENV=production', 'RUBY_GC_MALLOC_LIMIT=90000000',
       'UNICORN_WORKERS=2', 'LD_PRELOAD=/usr/lib/libjemalloc.so.1'],
     require => Exec['init-discourse']

[mathias, 2015/05/09 15:34:19]

Wouldn't it make more sense to make it subscribe to all
Discourse::Sitesetting[]s? (Then Exec['init-discourse'] is covered as well.)

[Wladimir Palant, 2015/05/11 15:00:42]

Not sure whether the services should really be considered dependent on the
settings. Given that these settings are non-essential (the essential ones are in
discourse.conf) and can be changed via web UI - I think they shouldn't prevent
the services from starting up.

   }
 
   discourse::customservice {'discourse':
     command => '/home/discourse/.rvm/bin/rvm default do bundle exec config/unicorn_launcher -c config/unicorn.conf.rb',
     require => File['/opt/discourse/tmp/pids'],
   }
 
   discourse::customservice {'sidekiq':
     command => '/home/discourse/.rvm/bin/rvm default do bundle exec sidekiq'
   }
 
   class {'nginx':
     worker_processes => 1,
     worker_connections => 500
   }
 
   nginx::hostconfig{$domain:
     source => 'puppet:///modules/discourse/site.conf',
     global_config => '
       upstream discourse {
         server localhost:3000;
       }',
     is_default => $is_default,
     certificate => $certificate,
     private_key => $private_key,
     log => 'access_log_intraforum'
   }
 }