Major cleanup of the common module

* improve documentation on all defines
* rename modules_dir to module_dir and modules_file to module_file
* create $module_dir_path to achieve DRY-ness
* silence the lsb_release stuff
* improve concatenated_file to use an intermediate file, improving reliability
  and decoupling the updateing from notifying our peers.
* remove serveral functions that were moved to puppet proper:
  - sha1 => sha1
  - gsub,substitute,slash_escape => regsubst

9 files changed, 139 insertions, 67 deletions

diff --git a/manifests/classes/lsb_release.pp b/manifests/classes/lsb_release.pp
index 8455a56..bf9baeb 100644
--- a/manifests/classes/lsb_release.pp
+++ b/manifests/classes/lsb_release.pp
@@ -14,12 +14,15 @@
 # 
 # To fail individual resources on a missing lsbdistcodename, require
 # Exec[assert_lsbdistcodename] on the specific resource
+#
+# This is just one example of how you could avoid evaluation of parts of the
+# manifest, before a bootstrapping class has enabled all the necessary goodies.
 class assert_lsbdistcodename {
 
 	case $lsbdistcodename {
 		'': {
 			err("Please install lsb_release or set facter_lsbdistcodename in the environment of $fqdn")
-			exec { "false # assert_lsbdistcodename": alias => assert_lsbdistcodename }
+			exec { "false # assert_lsbdistcodename": alias => assert_lsbdistcodename, loglevel => err }
 		}
 		'n/a': {
 			case $operatingsystem {
@@ -30,11 +33,11 @@ class assert_lsbdistcodename {
 					err("lsb_release was unable to report your distcodename; please set facter_lsbdistcodename in the environment of $fqdn")
 				}
 			}
-			exec { "false # assert_lsbdistcodename": alias => assert_lsbdistcodename }
+			exec { "false # assert_lsbdistcodename": alias => assert_lsbdistcodename, loglevel => err }
 		}
 		default: {
-			exec { "true # assert_lsbdistcodename": alias => assert_lsbdistcodename }
-			exec { "true # require_lsbdistcodename": alias => require_lsbdistcodename }
+			exec { "true # assert_lsbdistcodename": alias => assert_lsbdistcodename, loglevel => debug }
+			exec { "true # require_lsbdistcodename": alias => require_lsbdistcodename, loglevel => debug }
 		}
 	}
 
@@ -42,5 +45,5 @@ class assert_lsbdistcodename {
 
 # To fail the complete compilation, include this class
 class require_lsbdistcodename inherits assert_lsbdistcodename {
-	exec { "false # require_lsbdistcodename": require => Exec[require_lsbdistcodename], }
+	exec { "false # require_lsbdistcodename": require => Exec[require_lsbdistcodename], loglevel => err }
 }
diff --git a/manifests/defines/concatenated_file.pp b/manifests/defines/concatenated_file.pp
index 5a5144e..3741eea 100644
--- a/manifests/defines/concatenated_file.pp
+++ b/manifests/defines/concatenated_file.pp
@@ -4,14 +4,31 @@
 # Copyright (C) 2007 David Schmitt <david@schmitt.edv-bus.at>
 # See LICENSE for the full license granted to you.
 
+module_dir { "common/cf": }
+
 # TODO:
 # * create the directory in _part too
 
+# This resource collects file snippets from a directory ($dir) and concatenates
+# them in lexical order of their names into a new file ($name). This can be
+# used to collect information from disparate sources, when the target file
+# format doesn't allow includes.
+#
+# concatenated_file_part can be used to easily configure content for this.
+#
+# If no $dir is specified, the target name with '.d' appended will be used.
+#
+# The $dir is purged by puppet and will only contain explicitely configured
+# files. This can be overridden by defining the directory before the
+# concatenated_file.
+#
+# Depend on File[$name] to change if and only if its contents change. Notify
+# Exec["concat_${name}"] if you want to force an update.
+# 
 # Usage:
 # concatenated_file { "/etc/some.conf":
 # 	dir => "/etc/some.conf.d",
 # }
-# Use Exec["concat_$name"] as Semaphor
 define concatenated_file (
 	# where the snippets are located
 	$dir = '',
@@ -19,12 +36,16 @@ define concatenated_file (
 	$header = '',
 	# a file with content to append
 	$footer = '',
+	# default permissions for the target file
 	$mode = 0644, $owner = root, $group = 0
 	)
 {
 
 	$dir_real = $dir ? { '' => "${name}.d", default => $dir }
 
+	$tmp_file_name = regsubst($dir_real, '/', '_', 'G')
+	$tmp_file = "${module_dir_path}/${tmp_file_name}"
+
 	if defined(File[$dir_real]) {
 		debug("${dir_real} already defined")
 	} else {
@@ -40,9 +61,18 @@ define concatenated_file (
 	}
 
 	file {
-		$name:
+		$tmp_file:
 			ensure => present, checksum => md5,
 			mode => $mode, owner => $owner, group => $group;
+		# decouple the actual file from the generation process by using a
+		# temporary file and puppet's source mechanism. This ensures that events
+		# for notify/subscribe will only be generated when there is an actual
+		# change.
+		$name:
+			ensure => present, checksum => md5,
+			source => $tmp_file,
+			mode => $mode, owner => $owner, group => $group,
+			require => File[$tmp_file];
 	}
 
 	# if there is a header or footer file, add it
@@ -59,21 +89,12 @@ define concatenated_file (
 
 	# use >| to force clobbering the target file
 	exec { "concat_${name}":
-		command => "/usr/bin/find ${dir_real} -maxdepth 1 -type f ! -name '*puppettmp' -print0 | sort -z | xargs -0 cat ${additional_cmd} >| ${name}",
+		command => "/usr/bin/find ${dir_real} -maxdepth 1 -type f ! -name '*puppettmp' -print0 | sort -z | xargs -0 cat ${additional_cmd} >| ${tmp_file}",
 		refreshonly => true,
 		subscribe => [ File[$dir_real] ],
-		before => File[$name],
+		before => File[$tmp_file],
 		alias => [ "concat_${dir_real}"] ,
 	}
-	#case $header {
-		#'': {}
-		#default: { Exec["concat_${name}"] { subscribe +> File[$header] } }
-	#}
-
-	#case $footer {
-		#'': {}
-		#default: { Exec["concat_${name}"] { subscribe +> File[$footer] } }
-	#}
 
 }
 
diff --git a/manifests/defines/config_file.pp b/manifests/defines/config_file.pp
index cd6ccc3..8d93e0a 100644
--- a/manifests/defines/config_file.pp
+++ b/manifests/defines/config_file.pp
@@ -2,6 +2,8 @@
 # Copyright (C) 2007 David Schmitt <david@schmitt.edv-bus.at>
 # See LICENSE for the full license granted to you.
 
+# A simple wrapper to give all configuration files common defaults.
+# 
 # Usage:
 # config_file { filename:
 # 	content => "....\n",
@@ -27,7 +29,11 @@
 #              notify => Exec["reload-apache2"]
 # }
 
-define config_file ($content = '', $source = '', $ensure = 'present') {
+define config_file (
+	$content = '',
+	$source = '',
+	$ensure = 'present')
+{
 	file { $name:
 		ensure => $ensure,
 		# keep old versions on the server
diff --git a/manifests/defines/line.pp b/manifests/defines/line.pp
index c055f5b..fe2124d 100644
--- a/manifests/defines/line.pp
+++ b/manifests/defines/line.pp
@@ -2,6 +2,16 @@
 # Copyright (C) 2007 David Schmitt <david@schmitt.edv-bus.at>
 # See LICENSE for the full license granted to you.
 
+# Ensures that a specific line is present or absent in a file. This can be very
+# brittle, since even small changes can throw this off.
+#
+# If the line is not present yet, it will be appended to the file.
+# 
+# The name of the define is not used. Just keep it (globally) unique and
+# descriptive.
+#
+# Use this only for very trivial stuff.
+#
 # Usage:
 # line { description:
 # 	file => "filename",
@@ -22,8 +32,11 @@
 #       require => Package[munin-node],
 # }
 #
-#
-define line($file, $line, $ensure = 'present') {
+define line(
+	$file,
+	$line,
+	$ensure = 'present'
+) {
 	case $ensure {
 		default : { err ( "unknown ensure value '${ensure}'" ) }
 		present: {
diff --git a/manifests/defines/module_dir.pp b/manifests/defines/module_dir.pp
new file mode 100644
index 0000000..712e611
--- /dev/null
+++ b/manifests/defines/module_dir.pp
@@ -0,0 +1,43 @@
+# common/manifests/defines/module_dir.pp -- create a default directory
+# for storing module specific information
+#
+# Copyright (C) 2007 David Schmitt <david@schmitt.edv-bus.at>
+# See LICENSE for the full license granted to you.
+
+# Use this variable to reference the base path. Thus you are safe from any
+# changes.
+$module_dir_path = '/var/lib/puppet/modules'
+
+# A module_dir is a storage place for all the stuff a module might want to
+# store. According to the FHS, this should go to /var/lib. Since this is a part
+# of puppet, the full path is /var/lib/puppet/modules/${name}. Every module
+# should # prefix its module_dirs with its name.
+# 
+# By default, the module_dir is loaded from "puppet:///${name}/module_dir". If
+# that doesn't exist an empty directory is taken as template. The directory is
+# purged so that modules do not have to worry about removing cruft.
+# 
+# Usage:
+# module_dir { ["common", "common/dir1", "common/dir2" ]: }
+define module_dir (
+		$mode = 0644,
+		$owner = root,
+		$group = 0
+	)
+{
+	$dir = "${module_dir_path}/${name}"
+	if defined(File[$dir]) {
+		debug("${dir} already defined")
+	} else {
+		file {
+			$dir:
+				source => [ "puppet:///${name}/module_dir", "puppet:///common/empty"],
+				checksum => mtime,
+				# ignore the placeholder
+				ignore => '\.ignore', 
+				recurse => true, purge => true, force => true,
+				mode => $mode, owner => $owner, group => $group;
+		}
+	}
+}
+
diff --git a/manifests/defines/modules_file.pp b/manifests/defines/module_file.pp
index 6a37242..6082eff 100644
--- a/manifests/defines/modules_file.pp
+++ b/manifests/defines/module_file.pp
@@ -1,23 +1,23 @@
-# common/manifests/defines/modules_file.pp -- use a modules_dir to store module
-# specific files
+# common/manifests/defines/module_file.pp -- use an already defined module_dir
+# to store module specific files
 #
 # Copyright (C) 2007 David Schmitt <david@schmitt.edv-bus.at>
 # See LICENSE for the full license granted to you.
 
 # Usage:
-# modules_file { "module/file":
+# module_file { "module/file":
 # 		source => "puppet://..",
 # 		mode   => 644,   # default
 # 		owner  => root,  # default
 #		group  => root,  # default
 # }
-define modules_file (
+define module_file (
 		$source,
-		$mode = 0644, $owner = root, $group = root
+		$mode = 0644, $owner = root, $group = 0
 	)
 {
 	file {
-		"/var/lib/puppet/modules/${name}":
+		"${module_dir_path}/${name}":
 			source => $source,
 			mode => $mode, owner => $owner, group => $group;
 	}
diff --git a/manifests/defines/modules_dir.pp b/manifests/defines/modules_dir.pp
deleted file mode 100644
index 797de2d..0000000
--- a/manifests/defines/modules_dir.pp
+++ /dev/null
@@ -1,27 +0,0 @@
-# common/manifests/defines/modules_dir.pp -- create a default directory
-# for storing module specific information
-#
-# Copyright (C) 2007 David Schmitt <david@schmitt.edv-bus.at>
-# See LICENSE for the full license granted to you.
-
-# Usage:
-# modules_dir { ["common", "common/dir1", "common/dir2" ]: }
-define modules_dir (
-		$mode = 0644, $owner = root, $group = 0
-	)
-{
-	$dir = "/var/lib/puppet/modules/${name}"
-	if defined(File[$dir]) {
-		debug("${dir} already defined")
-	} else {
-		file {
-			"/var/lib/puppet/modules/${name}":
-				source => [ "puppet:///${name}/modules_dir", "puppet:///common/empty"],
-				checksum => mtime,
-				# ignore the placeholder
-				ignore => '\.ignore', 
-				recurse => true, purge => true, force => true,
-				mode => $mode, owner => $owner, group => $group;
-		}
-	}
-}
diff --git a/manifests/defines/replace.pp b/manifests/defines/replace.pp
index c13325e..a7a59b8 100644
--- a/manifests/defines/replace.pp
+++ b/manifests/defines/replace.pp
@@ -2,6 +2,13 @@
 # Copyright (C) 2007 David Schmitt <david@schmitt.edv-bus.at>
 # See LICENSE for the full license granted to you.
 
+# A hack to replace all ocurrances of a regular expression in a file with a
+# specified string. Sometimes it can be less effort to replace only a single
+# value in a huge config file instead of creating a template out of it. Still,
+# creating a template is often better than this hack.
+#
+# This define uses perl regular expressions.
+# 
 # Usage:
 #
 # replace { description: 
@@ -20,8 +27,9 @@
 # }  
 
 define replace($file, $pattern, $replacement) {
-	$pattern_no_slashes = slash_escape($pattern)
-	$replacement_no_slashes = slash_escape($replacement)
+	$pattern_no_slashes = regsubst($pattern, '/', '\\/', 'G', 'U')
+	$replacement_no_slashes = regsubst($replacement, '/', '\\/', 'G', 'U')
+
 	exec { "replace_${pattern}_${file}":
 		command => "/usr/bin/perl -pi -e 's/${pattern_no_slashes}/${replacement_no_slashes}/' '${file}'",
 		onlyif => "/usr/bin/perl -ne 'BEGIN { \$ret = 1; } \$ret = 0 if /${pattern_no_slashes}/ && ! /\\Q${replacement_no_slashes}\\E/; END { exit \$ret; }' '${file}'",
diff --git a/manifests/init.pp b/manifests/init.pp
index e0feaaf..f7298e3 100644
--- a/manifests/init.pp
+++ b/manifests/init.pp
@@ -2,19 +2,24 @@
 # Copyright (C) 2007 David Schmitt <david@schmitt.edv-bus.at>
 # See LICENSE for the full license granted to you.
 
-file {
-	# Module programmers can use /var/lib/puppet/modules/$modulename to save
-	# module-local data, e.g. for constructing config files
-	"/var/lib/puppet/modules":
-		ensure => directory,
-		source => "puppet:///common/modules/",
-		ignore => ".ignore",
-		recurse => true, purge => true, force => true,
-		mode => 0755, owner => root, group => 0;
-}
-
 import "defines/*.pp"
 import "classes/*.pp"
 
+module_dir { [ 'common' ]: }
+
+file {
+		 # Module programmers can use /var/lib/puppet/modules/$modulename to save
+		 # module-local data, e.g. for constructing config files. See module_dir
+		 # for details
+       "/var/lib/puppet/modules":
+               ensure => directory,
+               source => "puppet:///common/modules",
+               ignore => ".ignore",
+               recurse => true, purge => true, force => true,
+               mode => 0755, owner => root, group => 0;
+}
+
 # common packages
 class pkg::openssl { package { openssl: ensure => installed } }
+class pkg::rsync { package { rsync: ensure => installed } }
+