Faidon Liambotis has submitted this change and it was merged. Change subject: varnish: whitespace & lint cleanups ......................................................................
varnish: whitespace & lint cleanups Change-Id: Ib04d9b72a6398c110dcf5c1e0d6a59b46523d132 --- M modules/varnish/manifests/common.pp M modules/varnish/manifests/common/vcl.pp M modules/varnish/manifests/extra_vcl.pp M modules/varnish/manifests/htcppurger.pp M modules/varnish/manifests/instance.pp M modules/varnish/manifests/logging.pp M modules/varnish/manifests/logging/config.pp M modules/varnish/manifests/logging/monitor.pp M modules/varnish/manifests/monitoring/ganglia.pp M modules/varnish/manifests/monitoring/ganglia/vhtcpd.pp M modules/varnish/manifests/netmapper_update.pp M modules/varnish/manifests/packages.pp M modules/varnish/manifests/setup_filesystem.pp 13 files changed, 187 insertions(+), 142 deletions(-) Approvals: Faidon Liambotis: Looks good to me, approved jenkins-bot: Verified diff --git a/modules/varnish/manifests/common.pp b/modules/varnish/manifests/common.pp index 8f3c889..62d6cbf 100644 --- a/modules/varnish/manifests/common.pp +++ b/modules/varnish/manifests/common.pp @@ -7,21 +7,20 @@ # Mount /var/lib/ganglia as tmpfs to avoid Linux flushing mlocked # shm memory to disk - mount { "/var/lib/varnish": - require => Class["varnish::packages"], - device => "tmpfs", - fstype => "tmpfs", - options => "noatime,defaults,size=512M", - pass => 0, - dump => 0, - ensure => mounted; + mount { '/var/lib/varnish': + ensure => mounted, + device => 'tmpfs', + fstype => 'tmpfs', + options => 'noatime,defaults,size=512M', + pass => 0, + dump => 0, + require => Class['varnish::packages'], } - file { - "/usr/share/varnish/reload-vcl": - owner => 'root', - group => 'root', - mode => '0555', - source => "puppet:///modules/${module_name}/reload-vcl", + file { '/usr/share/varnish/reload-vcl': + owner => 'root', + group => 'root', + mode => '0555', + source => "puppet:///modules/${module_name}/reload-vcl", } } diff --git a/modules/varnish/manifests/common/vcl.pp b/modules/varnish/manifests/common/vcl.pp index 877b178..5432c8e 100644 --- a/modules/varnish/manifests/common/vcl.pp +++ b/modules/varnish/manifests/common/vcl.pp @@ -1,12 +1,22 @@ class varnish::common::vcl { - require "varnish::common" + require varnish::common - file { - "/etc/varnish/geoip.inc.vcl": - content => template("varnish/geoip.inc.vcl.erb"); - "/etc/varnish/device-detection.inc.vcl": - content => template("varnish/device-detection.inc.vcl.erb"); - "/etc/varnish/errorpage.inc.vcl": - content => template("varnish/errorpage.inc.vcl.erb"); + file { '/etc/varnish/geoip.inc.vcl': + owner => 'root', + group => 'root', + mode => '0444', + content => template('varnish/geoip.inc.vcl.erb'), + } + file { '/etc/varnish/device-detection.inc.vcl': + owner => 'root', + group => 'root', + mode => '0444', + content => template('varnish/device-detection.inc.vcl.erb'), + } + file { '/etc/varnish/errorpage.inc.vcl': + owner => 'root', + group => 'root', + mode => '0444', + content => template('varnish/errorpage.inc.vcl.erb'), } } diff --git a/modules/varnish/manifests/extra_vcl.pp b/modules/varnish/manifests/extra_vcl.pp index fe65d94..cd26f04 100644 --- a/modules/varnish/manifests/extra_vcl.pp +++ b/modules/varnish/manifests/extra_vcl.pp @@ -1,10 +1,13 @@ define varnish::extra_vcl { $vcl = regsubst($title, '^([^ ]+) .*$', '\1') $filename = "/etc/varnish/${vcl}.inc.vcl" + if !defined(File[$filename]) { file { $filename: + owner => 'root', + group => 'root', + mode => '0444', content => template("varnish/${vcl}.inc.vcl.erb"), - mode => 0444; } } } diff --git a/modules/varnish/manifests/htcppurger.pp b/modules/varnish/manifests/htcppurger.pp index f510579..9dca451 100644 --- a/modules/varnish/manifests/htcppurger.pp +++ b/modules/varnish/manifests/htcppurger.pp @@ -1,17 +1,17 @@ -class varnish::htcppurger($varnish_instances=["localhost:80"]) { +class varnish::htcppurger($varnish_instances=['localhost:80']) { Class[varnish::packages] -> Class[varnish::htcppurger] - package { "vhtcpd": + package { 'vhtcpd': ensure => latest, } - file { "/etc/default/vhtcpd": - owner => root, - group => root, - mode => 0444, - require => Package["vhtcpd"], # if we go first, we get overwritten + file { '/etc/default/vhtcpd': + owner => 'root', + group => 'root', + mode => '0444', + require => Package['vhtcpd'], # if we go first, we get overwritten # TODO: -r ^upload\\.wikimedia\\.org\$ (POSIX ERE, new param for class, quoting/escaping will be tricky...) - content => inline_template('DAEMON_OPTS="-m 239.128.0.112<% varnish_instances.each do |inst| -%> -c <%= inst %><% end -%>"'); + content => inline_template('DAEMON_OPTS="-m 239.128.0.112<% varnish_instances.each do |inst| -%> -c <%= inst %><% end -%>"'), } # Wikimedia used to provide vhtcpd under the name varnishhtcpd with an @@ -21,16 +21,16 @@ ensure => absent, } - service { vhtcpd: - require => Package["vhtcpd"], - subscribe => File["/etc/default/vhtcpd"], - hasstatus => true, + service { 'vhtcpd': + ensure => running, + require => Package['vhtcpd'], + subscribe => File['/etc/default/vhtcpd'], + hasstatus => true, hasrestart => true, - ensure => running; } - nrpe::monitor_service { "vhtcpd": - description => "Varnish HTCP daemon", - nrpe_command => "/usr/lib/nagios/plugins/check_procs -c 1:1 -u vhtcpd -a vhtcpd" + nrpe::monitor_service { 'vhtcpd': + description => 'Varnish HTCP daemon', + nrpe_command => '/usr/lib/nagios/plugins/check_procs -c 1:1 -u vhtcpd -a vhtcpd', } } diff --git a/modules/varnish/manifests/instance.pp b/modules/varnish/manifests/instance.pp index b0a44e5..f944160 100644 --- a/modules/varnish/manifests/instance.pp +++ b/modules/varnish/manifests/instance.pp @@ -1,7 +1,7 @@ define varnish::instance( $name="", $vcl = "", - $extra_vcl = [], + $vcl_config, $port="80", $admin_port="6083", $storage="-s malloc,1G", @@ -10,11 +10,12 @@ $directors={}, $director_type="hash", $director_options={}, - $vcl_config, + $extra_vcl = [], $backend_options, $cluster_options={}, $wikimedia_networks=[], - $xff_sources=[]) { + $xff_sources=[] +) { include varnish::common @@ -39,69 +40,80 @@ # $cluster_option is referenced directly # Install VCL include files shared by all instances - require "varnish::common::vcl" + require varnish::common::vcl $extra_vcl_variable_to_make_puppet_parser_happy = suffix($extra_vcl, " ${instancesuffix}") extra_vcl{ $extra_vcl_variable_to_make_puppet_parser_happy: before => Service["varnish${instancesuffix}"] } - file { - "/etc/init.d/varnish${instancesuffix}": - content => template("${module_name}/varnish.init.erb"), - mode => 0555; - "/etc/default/varnish${instancesuffix}": - content => template("${module_name}/varnish-default.erb"), - mode => 0444; - "/etc/varnish/${vcl}.inc.vcl": - content => template("varnish/${vcl}.inc.vcl.erb"), - notify => Exec["load-new-vcl-file${instancesuffix}"], - mode => 0444; - "/etc/varnish/wikimedia_${vcl}.vcl": - require => File["/etc/varnish/${vcl}.inc.vcl"], - content => template("${module_name}/vcl/wikimedia.vcl.erb"), - mode => 0444; + file { "/etc/init.d/varnish${instancesuffix}": + owner => 'root', + group => 'root', + mode => '0555', + content => template("${module_name}/varnish.init.erb"), + } + file { "/etc/default/varnish${instancesuffix}": + owner => 'root', + group => 'root', + mode => '0444', + content => template("${module_name}/varnish-default.erb"), + } + file { "/etc/varnish/${vcl}.inc.vcl": + owner => 'root', + group => 'root', + mode => '0444', + content => template("varnish/${vcl}.inc.vcl.erb"), + notify => Exec["load-new-vcl-file${instancesuffix}"], + } + file { "/etc/varnish/wikimedia_${vcl}.vcl": + owner => 'root', + group => 'root', + mode => '0444', + require => File["/etc/varnish/${vcl}.inc.vcl"], + content => template("${module_name}/vcl/wikimedia.vcl.erb"), } service { "varnish${instancesuffix}": - require => [ - File[ - "/etc/default/varnish${instancesuffix}", - "/etc/init.d/varnish${instancesuffix}", - "/etc/varnish/${vcl}.inc.vcl", - "/etc/varnish/wikimedia_${vcl}.vcl" - ], - Mount["/var/lib/varnish"] + ensure => running, + require => [ + File["/etc/default/varnish${instancesuffix}"], + File["/etc/init.d/varnish${instancesuffix}"], + File["/etc/varnish/${vcl}.inc.vcl"], + File["/etc/varnish/wikimedia_${vcl}.vcl"], + Mount['/var/lib/varnish'] ], hasstatus => false, - pattern => "/var/run/varnishd${instancesuffix}.pid", - subscribe => Package[varnish], - before => Exec['generate varnish.pyconf'], - ensure => running; + pattern => "/var/run/varnishd${instancesuffix}.pid", + subscribe => Package['varnish'], + before => Exec['generate varnish.pyconf'], } exec { "load-new-vcl-file${instancesuffix}": - require => [ Service["varnish${instancesuffix}"], File["/etc/varnish/wikimedia_${vcl}.vcl"] ], - subscribe => [File["/etc/varnish/wikimedia_${vcl}.vcl"], - Class["varnish::common::vcl"], - File[suffix(prefix($extra_vcl, "/etc/varnish/"), ".inc.vcl")] + require => [ + Service["varnish${instancesuffix}"], + File["/etc/varnish/wikimedia_${vcl}.vcl"] ], - command => "/usr/share/varnish/reload-vcl $extraopts", - path => "/bin:/usr/bin", - refreshonly => true; + subscribe => [ + Class['varnish::common::vcl'], + File[suffix(prefix($extra_vcl, '/etc/varnish/'),".inc.vcl")], + File["/etc/varnish/wikimedia_${vcl}.vcl"] + ], + command => "/usr/share/varnish/reload-vcl ${extraopts}", + path => '/bin:/usr/bin', + refreshonly => true, } monitor_service { "varnish http ${title}": - description => "Varnish HTTP ${title}", + description => "Varnish HTTP ${title}", check_command => "check_http_generic!varnishcheck!${port}" } # Restart gmond if this varnish instance has been (re)started later # than gmond was started exec { "restart gmond for varnish${instancesuffix}": - path => "/bin:/sbin:/usr/bin:/usr/sbin", - command => "true", - onlyif => "test /var/run/varnishd${instancesuffix}.pid -nt /var/run/gmond.pid", - notify => Service['gmond'] + command => '/bin/true', + onlyif => "test /var/run/varnishd${instancesuffix}.pid -nt /var/run/gmond.pid", + notify => Service['gmond'], } } diff --git a/modules/varnish/manifests/logging.pp b/modules/varnish/manifests/logging.pp index d395ce6..3e31cd9 100644 --- a/modules/varnish/manifests/logging.pp +++ b/modules/varnish/manifests/logging.pp @@ -1,33 +1,44 @@ -define varnish::logging($listener_address, $port="8420", $cli_args="", $log_fmt=false, $instance_name="frontend", $monitor=true) { - require varnish::packages, - varnish::logging::config +define varnish::logging( + $listener_address, + $port='8420', + $cli_args='', + $log_fmt=false, + $instance_name='frontend', + $monitor=true +) { + require varnish::packages + require varnish::logging::config + if $monitor { require varnish::logging::monitor } $varnishservice = $instance_name ? { - "" => "varnish", + '' => 'varnish', default => "varnish-${instance_name}" } $shm_name = $instance_name ? { - "" => $::hostname, + '' => $::hostname, default => $instance_name } file { "/etc/init.d/varnishncsa-${name}": content => template("${module_name}/varnishncsa.init.erb"), - owner => root, - group => root, - mode => 0555, - notify => Service["varnishncsa-${name}"]; + owner => 'root', + group => 'root', + mode => '0555', + notify => Service["varnishncsa-${name}"], } service { "varnishncsa-${name}": - require => [ File["/etc/init.d/varnishncsa-${name}"], Service[$varnishservice] ], - subscribe => File["/etc/default/varnishncsa"], - ensure => running, - pattern => "/var/run/varnishncsa/varnishncsa-${name}.pid", - hasstatus => false; + ensure => running, + require => [ + File["/etc/init.d/varnishncsa-${name}"], + Service[$varnishservice] + ], + subscribe => File['/etc/default/varnishncsa'], + pattern => "/var/run/varnishncsa/varnishncsa-${name}.pid", + hasstatus => false, } } diff --git a/modules/varnish/manifests/logging/config.pp b/modules/varnish/manifests/logging/config.pp index 1f8e3ba..da8dba6 100644 --- a/modules/varnish/manifests/logging/config.pp +++ b/modules/varnish/manifests/logging/config.pp @@ -1,8 +1,8 @@ class varnish::logging::config { - file { "/etc/default/varnishncsa": + file { '/etc/default/varnishncsa': source => "puppet:///modules/${module_name}/varnishncsa.default", - owner => root, - group => root, - mode => 0444; + owner => 'root', + group => 'root', + mode => '0444', } } diff --git a/modules/varnish/manifests/logging/monitor.pp b/modules/varnish/manifests/logging/monitor.pp index 8238862..af7a6d0 100644 --- a/modules/varnish/manifests/logging/monitor.pp +++ b/modules/varnish/manifests/logging/monitor.pp @@ -1,6 +1,6 @@ class varnish::logging::monitor { - nrpe::monitor_service { "varnishncsa": - description => "Varnish traffic logger", - nrpe_command => "/usr/lib/nagios/plugins/check_procs -w 2:2 -c 2:4 -C varnishncsa" + nrpe::monitor_service { 'varnishncsa': + description => 'Varnish traffic logger', + nrpe_command => '/usr/lib/nagios/plugins/check_procs -w 2:2 -c 2:4 -C varnishncsa', } } diff --git a/modules/varnish/manifests/monitoring/ganglia.pp b/modules/varnish/manifests/monitoring/ganglia.pp index ec3fb73..f634b9e 100644 --- a/modules/varnish/manifests/monitoring/ganglia.pp +++ b/modules/varnish/manifests/monitoring/ganglia.pp @@ -1,24 +1,29 @@ class varnish::monitoring::ganglia($varnish_instances=['']) { - $instances = join($varnish_instances, ",") + $instances = join($varnish_instances, ',') - file { "/usr/lib/ganglia/python_modules/varnish.py": + file { '/usr/lib/ganglia/python_modules/varnish.py': owner => 'root', group => 'root', mode => '0444', - require => File["/usr/lib/ganglia/python_modules"], - source => "puppet:///modules/${module_name}/ganglia/ganglia-varnish.py"; + source => "puppet:///modules/${module_name}/ganglia/ganglia-varnish.py", + require => File['/usr/lib/ganglia/python_modules'], } - exec { - "generate varnish.pyconf": - require => File["/usr/lib/ganglia/python_modules/varnish.py", "/etc/ganglia/conf.d"], - command => "/usr/bin/python /usr/lib/ganglia/python_modules/varnish.py \"$instances\" > /etc/ganglia/conf.d/varnish.pyconf.new"; - "replace varnish.pyconf": - cwd => "/etc/ganglia/conf.d", - path => "/bin:/usr/bin", - unless => "diff -q varnish.pyconf.new varnish.pyconf && rm varnish.pyconf.new", - command => "mv varnish.pyconf.new varnish.pyconf", - notify => Service['gmond']; + exec { 'generate varnish.pyconf': + require => [ + File['/usr/lib/ganglia/python_modules/varnish.py'], + File['/etc/ganglia/conf.d'], + ], + command => "/usr/bin/python /usr/lib/ganglia/python_modules/varnish.py \"${instances}\" > /etc/ganglia/conf.d/varnish.pyconf.new", } - Exec["generate varnish.pyconf"] -> Exec["replace varnish.pyconf"] + + exec { 'replace varnish.pyconf': + cwd => '/etc/ganglia/conf.d', + path => '/bin:/usr/bin', + unless => 'diff -q varnish.pyconf.new varnish.pyconf && rm varnish.pyconf.new', + command => 'mv varnish.pyconf.new varnish.pyconf', + notify => Service['gmond'], + } + + Exec['generate varnish.pyconf'] -> Exec['replace varnish.pyconf'] } diff --git a/modules/varnish/manifests/monitoring/ganglia/vhtcpd.pp b/modules/varnish/manifests/monitoring/ganglia/vhtcpd.pp index 52969cc..4213430 100644 --- a/modules/varnish/manifests/monitoring/ganglia/vhtcpd.pp +++ b/modules/varnish/manifests/monitoring/ganglia/vhtcpd.pp @@ -1,17 +1,16 @@ class varnish::monitoring::ganglia::vhtcpd { - file { - '/usr/lib/ganglia/python_modules/vhtcpd.py': + file { '/usr/lib/ganglia/python_modules/vhtcpd.py': source => "puppet:///modules/${module_name}/ganglia/ganglia-vhtcpd.py", owner => 'root', group => 'root', mode => '0444', - notify => Service['gmond']; - - '/etc/ganglia/conf.d/vhtcpd.pyconf': + notify => Service['gmond'], + } + file { '/etc/ganglia/conf.d/vhtcpd.pyconf': owner => 'root', group => 'root', mode => '0444', content => template("${module_name}/ganglia/vhtcpd.pyconf.erb"), - notify => Service['gmond']; + notify => Service['gmond'], } } diff --git a/modules/varnish/manifests/netmapper_update.pp b/modules/varnish/manifests/netmapper_update.pp index bfb556c..e9ac789 100644 --- a/modules/varnish/manifests/netmapper_update.pp +++ b/modules/varnish/manifests/netmapper_update.pp @@ -1,5 +1,7 @@ class varnish::netmapper_update_common { - group { 'netmap': ensure => present; } + group { 'netmap': + ensure => present, + } user { 'netmap': home => '/var/netmapper', @@ -7,7 +9,7 @@ system => true, managehome => false, shell => '/bin/false', - require => Group['netmap']; + require => Group['netmap'], } file { '/var/netmapper': @@ -15,7 +17,7 @@ owner => 'netmap', group => 'netmap', require => User['netmap'], - mode => '0755'; + mode => '0755', } file { '/usr/share/varnish/netmapper_update.sh': @@ -34,13 +36,13 @@ exec { "netmapper_update_${name}_initial": user => 'netmap', command => $cmd, - creates => "/var/netmapper/${name}"; + creates => "/var/netmapper/${name}", } cron { "netmapper_update_${name}": user => 'netmap', command => $cmd, hour => $hour, - minute => $minute; + minute => $minute, } } diff --git a/modules/varnish/manifests/packages.pp b/modules/varnish/manifests/packages.pp index 974de47..3062479 100644 --- a/modules/varnish/manifests/packages.pp +++ b/modules/varnish/manifests/packages.pp @@ -1,5 +1,7 @@ -class varnish::packages($version="installed") { - package { [ 'varnish', 'libvarnishapi1', 'varnish-dbg' ]: - ensure => $version; +class varnish::packages($version='installed') { + package { [ 'varnish', + 'libvarnishapi1', + 'varnish-dbg' ]: + ensure => $version } } diff --git a/modules/varnish/manifests/setup_filesystem.pp b/modules/varnish/manifests/setup_filesystem.pp index acb11e2..576dea5 100644 --- a/modules/varnish/manifests/setup_filesystem.pp +++ b/modules/varnish/manifests/setup_filesystem.pp @@ -1,18 +1,20 @@ define varnish::setup_filesystem() { file { "/srv/${title}": - owner => root, - group => root, - ensure => directory + ensure => directory, + owner => 'root', + group => 'root', + } + + $mount_options = $::realm ? { + 'production' => 'noatime,nodiratime,nobarrier,logbufs=8', + 'labs' => 'noatime,nodiratime,nobarrier,comment=cloudconfig', } mount { "/srv/${title}": + ensure => mounted, require => File["/srv/${title}"], - device => "/dev/${title}", - fstype => "auto", - options => $::realm ? { - 'production' => "noatime,nodiratime,nobarrier,logbufs=8", - 'labs' => "noatime,nodiratime,nobarrier,comment=cloudconfig" - }, - ensure => mounted + device => "/dev/${title}", + fstype => 'auto', + options => $mount_options, } } -- To view, visit https://gerrit.wikimedia.org/r/97910 To unsubscribe, visit https://gerrit.wikimedia.org/r/settings Gerrit-MessageType: merged Gerrit-Change-Id: Ib04d9b72a6398c110dcf5c1e0d6a59b46523d132 Gerrit-PatchSet: 9 Gerrit-Project: operations/puppet Gerrit-Branch: production Gerrit-Owner: Matanya <mata...@foss.co.il> Gerrit-Reviewer: Akosiaris <akosia...@wikimedia.org> Gerrit-Reviewer: Andrew Bogott <abog...@wikimedia.org> Gerrit-Reviewer: Faidon Liambotis <fai...@wikimedia.org> Gerrit-Reviewer: jenkins-bot _______________________________________________ MediaWiki-commits mailing list MediaWiki-commits@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits