Puppet: What_not_ to do?An interactive journey through the ugly side

of Puppet

Walter Heck, Founder of OlinData

2,5 years experience with Puppet in 5+different environments

Experienced Puppet Fundamentals trainer

Had my eyes bleed many times with uglyPuppet code

•

•

•

•

Design mistakesmight not be glaringly obvious or evenwrong at first, but will cause trouble later

Language mistakesPuppet provides functionality thatshouldn't be used, but is there for edge-cases or historical purposes

•

•

Wake up...

Quiz time!

== File: modules/ssh/manifests/ssh.pp

class ssh_install { package { 'ssh': ensure => present }}

class ssh_configure { file { '/etc/ssh/sshd_config': ensure => present }}

== File: modules/ssh/manifests/ssh.pp

class ssh($state = ‘present’ { package { 'ssh': ensure => $state }

file { '/etc/ssh/sshd_config': ensure => $state }}

# problem: classnames won't be autoloaded, classnames shouldn't have verbs in them,classes should be combined, don't put multiple classes in a file

==

schedule { 'maint': range => '2 - 4', period => daily, repeat => 1,}

exec { '/usr/bin/apt-get update': schedule => 'maint',}

==

schedule { 'maint': range => '2 - 4', period => daily, repeat => 1,}exec { '/usr/bin/apt-get update': schedule => 'maint',}# problem: schedule doesn't mean something will execute, a common pitfall. If there is no puppet run between these hours, the apt-get exec will not be run

==$myvar = ‘false’

if ($myvar) { notice(‘this is true’)} else { notice(‘This is false’)}

==$myvar = ‘false’

if ($myvar) { notice(‘this is true’)} else { notice(‘This is false’)}#problem: 'false' evaluates totrue

==

exec { '/etc/init.d/apache start': onlyif => ‘ps aux | grep apache | grep -v grep |wc -l’}

==

exec { '/etc/init.d/apache start': onlyif => ‘ps aux | grep apache | grep -v grep |wc -l’}

# problem: this shouldn't be an exec, but aservice

==

package { 'ssh': ensure => present, name => $::operatingsystem ? { 'Ubuntu' => 'openssh-server', default => 'ssh', },}

== $sshpkgname = $::operatingsystem ? { 'Ubuntu' => 'openssh-server', default => undef,}

if ($sshpkgname == undef) { fail(‘unsupported OS’)} else { package { 'ssh': ensure => present, name => $sshpkgname, }}

#problem: they encourage behaviour that is not scalable, using default options toassume things, etc.

==case $::operatingsystem { 'RedHat', 'CentOS': { file { ‘/etc/httpd/http.conf’: ensure => ‘present’, } } default: { file { ‘/etc/apache2/apache2.conf’: ensure => ‘present’, } } }

==case $::operatingsystem { 'RedHat', 'CentOS': { file { ‘/etc/httpd/http.conf’: ensure => ‘present’, } } default: { file { ‘/etc/apache2/apache2.conf’: ensure => ‘present’, } } }#problem: case without default that fails, instead it assumes

==class wordpress {

$wordpress_archive = 'wordpress-3.4.1.zip'

$apache = $::operatingsystem ? { Ubuntu => apache2, CentOS => httpd, Debian => apache2, default => httpd }

$phpmysql = $::operatingsystem ? { Ubuntu => php5-mysql, CentOS => php-mysql, Debian => php5-mysql, default => php-mysql }

$php = $::operatingsystem ? { Ubuntu => libapache2-mod-php5, CentOS => php, Debian => libapache2-mod-php5, default => php }

package { ['unzip',$apache,$php,$phpmysql]: ensure => latest }}

==class wordpress {

$wordpress_archive == 'wordpress-3.4.1.zip'

$apache == $::operatingsystem ? { Ubuntu => apache2, CentOS => httpd, Debian => apache2, defaultdefault => httpd }

$phpmysql == $::operatingsystem ? { Ubuntu => php5-mysql, CentOS => php-mysql, Debian => php5-mysql, defaultdefault => php-mysql }

$php == $::operatingsystem ? { Ubuntu => libapache2-mod-php5, CentOS => php, Debian => libapache2-mod-php5, defaultdefault => php }

packagepackage { ['unzip',$apache,$php,$phpmysql]: ensure => latest }}#wordpress class shouldn't touch apache, should be a different module

== $files = [ '/etc/mysql', '/var/log/mysql','/var/run/mysql' ]

file { $files: ensure => present, user => mysql, group => mysql, mode => 0755,}

== #arrays of resources are not wrong, but dangerous.

file { '/etc/mysql': ensure => present, user => mysql, group => mysql, mode => 0700, <=== careful with this!}

file { '/var/log/mysql': ensure => present, user => mysql, group => mysql, mode => 0755,}

file { '/var/run/mysql': ensure => present, user => mysql, group => mysql, mode => 0755,}

==

if defined(File['/tmp/foo']) { notify('This configuration includes the /tmp/foo file.')} else { file {'/tmp/foo': ensure => present, }}

==class test {

if defined(File['/tmp/foo']) { notice('This configuration includes the /tmp/foo file.') } else { file {'/tmp/foo': ensure => present, group => root } }

if defined(File['/tmp/foo']) { notice('This configuration includes the /tmp/foo file.') } else { file {'/tmp/foo': ensure => present, group => puppet } }}

include test

defined() is (usually) the wrong solution to a resource defined in two locations. It isdangerous, because it only checks if the resource has been defined elsewhere, not withwhat attributes.

==

class apache2 {

file { '/etc/apache2': ensure => directory, require => Service['apache2'] }

file { '/etc/apache2/apache2.conf': ensure => present, require => File['/etc/apache2'], notify => Service['apache2'],}

package { 'apache2': ensure => present, allowcdrom => true, before => File['/etc/apache2/apache2.conf']}

service { 'apache2': ensure => running, subscribe => File['/etc/apache2/apache2.conf']}}

include apache2

==# dependency loop

class apache2 {

file { '/etc/apache2': ensure => directory, require => Service['apache2'] }

file { '/etc/apache2/apache2.conf': ensure => present, require => File['/etc/apache2'], notify => Service['apache2'], # <=== The notify metaparameter implies before.}

package { 'apache2': ensure => present, allowcdrom => true, before => File['/etc/apache2/apache2.conf']}

service { 'apache2': ensure => running, subscribe => File['/etc/apache2/apache2.conf'] # <=== The subscribe metaparameter impliesrequire.

}

class test {

file { '/tmp/somefile.txt': ensure => 'file', mode => 0600, owner => 'root', group => 'root', source => '/etc/puppet/modules/test/somefile.txt' }

}

include test

==

# use puppet:///modules/ instead of the full path on the puppet master

class test {

file { '/tmp/somefile.txt': ensure => 'file', mode => 0600, owner => 'root', group => 'root', source => 'puppet:///modules/test/somefile.txt' }

}

include test

==class test { file {‘/tmp/large/dir/with/many/subdirs/and/many/files’: ensure => present, owner => root, group => root, recurse => true }}

include test

==

# do not use recurse => true on a dir with over 100+ files

class test {

file {‘/tmp/large/dir/with/many/files’: ensure => present, owner => root, group => root, recurse => true }}

include test

# alternative :’(

class test {

exec {'/bin/chown -R root:root /tmp/large/dir/with/many/files': }}

include test

Questions? Feel free to get in touch!

Walter Heck - OlinDataEmail: walterheck@olindata.comTwitter: @walterheck / @olindata

Web: http://olindata.com