Faidon Liambotis has uploaded a new change for review.
https://gerrit.wikimedia.org/r/179082
Change subject: Run "apt-get update" outside of/before puppet
......................................................................
Run "apt-get update" outside of/before puppet
Currently, we run "apt-get update" on every puppet run, in a separate
"first" Stage, with the effect of running it before everything else that
may need up-to-date package lists (= Package resources for the most
part).
This has the following problems:
- Transient apt-get failures (e.g. mirror issues) produce puppet
failures and we get an alert for every single host
- Puppet runs consume more time, something that's especially annoying
during manual puppet runs
- Changing apt's configuration (with apt::repository etc.) doesn't
trigger an additional apt-get update, as you can't run the same
resource twice (and because of Stages, it's tricky to be implemented
correctly because of dependency loops, see e.g. a14a2e3)
The above aren't unfixable but instead of taking a stab at fixing them,
move the "apt-get update" invocation to the cronjob we have for running
puppet. The script runs with -e, so a failed apt-get update would stop
puppet from running (similar to today), but would not warn us until
sufficient time has passed and the "hasn't run puppet for X seconds"
check fires off.
Moreover, instead of dropping the Exec, move it to the main apt class
(and to the main Stage), make it a refreshonly and add notifies from all
of the apt definitions.
Change-Id: I4ae2b64d096d38b3eeeda2791949274fb1f69ffa
---
M manifests/stages.pp
M modules/apt/manifests/conf.pp
M modules/apt/manifests/init.pp
M modules/apt/manifests/pin.pp
M modules/apt/manifests/repository.pp
D modules/apt/manifests/update.pp
A modules/base/files/puppet/puppet-run
M modules/base/manifests/init.pp
M modules/base/manifests/puppet.pp
M modules/base/templates/puppet.cron.erb
10 files changed, 53 insertions(+), 28 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/operations/puppet
refs/changes/82/179082/1
diff --git a/manifests/stages.pp b/manifests/stages.pp
index 758472d..826d665 100644
--- a/manifests/stages.pp
+++ b/manifests/stages.pp
@@ -1,6 +1,2 @@
stage { 'first': before => Stage[main] }
stage { 'last': require => Stage[main] }
-
-class {
- 'apt::update': stage => first;
-}
diff --git a/modules/apt/manifests/conf.pp b/modules/apt/manifests/conf.pp
index 4df12e9..c19a900 100644
--- a/modules/apt/manifests/conf.pp
+++ b/modules/apt/manifests/conf.pp
@@ -10,5 +10,6 @@
group => 'root',
mode => '0444',
content => "${key} \"${value}\";\n",
+ notify => Exec['apt-get update'],
}
}
diff --git a/modules/apt/manifests/init.pp b/modules/apt/manifests/init.pp
index d6622da..fa47f91 100644
--- a/modules/apt/manifests/init.pp
+++ b/modules/apt/manifests/init.pp
@@ -1,4 +1,11 @@
class apt {
+ exec { 'apt-get update':
+ path => '/usr/bin',
+ timeout => 240,
+ returns => [ 0, 100 ],
+ refreshonly => true,
+ }
+
# Directory to hold the repository signing keys
file { '/var/lib/apt/keys':
ensure => directory,
@@ -56,6 +63,7 @@
# no longer needed after the installation is over
file { '/etc/apt/apt.conf':
ensure => absent,
+ notify => Exec['apt-get update'],
}
if $::lsbdistid == 'Debian' {
diff --git a/modules/apt/manifests/pin.pp b/modules/apt/manifests/pin.pp
index 492f3a5..5f9c173 100644
--- a/modules/apt/manifests/pin.pp
+++ b/modules/apt/manifests/pin.pp
@@ -10,5 +10,6 @@
group => 'root',
mode => '0444',
content => "Package: ${package}\nPin: ${pin}\nPin-Priority:
${priority}\n",
+ notify => Exec['apt-get update'],
}
}
diff --git a/modules/apt/manifests/repository.pp
b/modules/apt/manifests/repository.pp
index b6aa2c9..9f22b3c 100644
--- a/modules/apt/manifests/repository.pp
+++ b/modules/apt/manifests/repository.pp
@@ -25,6 +25,7 @@
group => 'root',
mode => '0444',
content => "${binline}${srcline}${pinline}",
+ notify => Exec['apt-get update'],
}
if $comment_old {
@@ -54,13 +55,5 @@
subscribe => File["/var/lib/apt/keys/${name}.gpg"],
refreshonly => true,
}
- }
-
- exec { "apt-update-for-${name}":
- command => '/usr/bin/apt-get update',
- timeout => 240,
- returns => [ 0, 100 ],
- subscribe => File["/etc/apt/sources.list.d/${name}.list"],
- refreshonly => true,
}
}
diff --git a/modules/apt/manifests/update.pp b/modules/apt/manifests/update.pp
deleted file mode 100644
index 0424147..0000000
--- a/modules/apt/manifests/update.pp
+++ /dev/null
@@ -1,6 +0,0 @@
-class apt::update {
- exec { '/usr/bin/apt-get update':
- timeout => 240,
- returns => [ 0, 100 ],
- }
-}
diff --git a/modules/base/files/puppet/puppet-run
b/modules/base/files/puppet/puppet-run
new file mode 100644
index 0000000..6c165f1
--- /dev/null
+++ b/modules/base/files/puppet/puppet-run
@@ -0,0 +1,24 @@
+#!/bin/bash
+
+# older versions of timeout (before coreutils) didn't support -k
+# remove the coditional after the last lucid host goes
+if timeout -k 10 10 true >/dev/null 2>&1; then
+ KILL_TIMEOUT="-k 300"
+fi
+
+set -e
+
+timeout $KILL_TIMEOUT 1800 apt-get update
+
+# We pass show-diff, show the log may be sensitive,
+# so make sure it's sufficiently protected
+umask 077
+
+timeout $KILL_TIMEOUT puppet agent \
+ --onetime \
+ --no-daemonize \
+ --verbose \
+ --show_diff \
+ --splay \
+ --splaylimit 59 \
+ >> /var/log/puppet.log 2>&1
diff --git a/modules/base/manifests/init.pp b/modules/base/manifests/init.pp
index 415d085..84da93d 100644
--- a/modules/base/manifests/init.pp
+++ b/modules/base/manifests/init.pp
@@ -125,7 +125,6 @@
class base {
include apt
- include apt::update
if ($::realm == 'labs') {
include apt::unattendedupgrades,
diff --git a/modules/base/manifests/puppet.pp b/modules/base/manifests/puppet.pp
index 2923361..bea83d8 100644
--- a/modules/base/manifests/puppet.pp
+++ b/modules/base/manifests/puppet.pp
@@ -12,12 +12,6 @@
require => Apt::Puppet['base']
}
- if os_version('ubuntu <= lucid') {
- package {'timeout':
- ensure => latest,
- }
- }
-
file { '/etc/default/puppet':
owner => 'root',
group => 'root',
@@ -91,12 +85,27 @@
enable => false,
}
+ if os_version('ubuntu <= lucid') {
+ # folded into coreutils in newer distros
+ package {'timeout':
+ ensure => present,
+ before => File['/usr/local/sbin/puppet-run'],
+ }
+ }
+
+ file { '/usr/local/sbin/puppet-run':
+ mode => '0555',
+ owner => 'root',
+ group => 'root',
+ source => 'puppet:///modules/base/puppet/puppet-run',
+ }
+
file { '/etc/cron.d/puppet':
- require => File['/etc/default/puppet'],
mode => '0444',
owner => 'root',
group => 'root',
content => template('base/puppet.cron.erb'),
+ require => File['usr/local/sbin/puppet-run'],
}
file { '/etc/logrotate.d/puppet':
diff --git a/modules/base/templates/puppet.cron.erb
b/modules/base/templates/puppet.cron.erb
index ff22e10..2702627 100644
--- a/modules/base/templates/puppet.cron.erb
+++ b/modules/base/templates/puppet.cron.erb
@@ -11,5 +11,5 @@
tmp = tmp.sort()
times = tmp.join(',')
-%>
-<%= times %> * * * * root timeout <% if
scope.function_versioncmp([@lsbdistrelease, "12.04"]) >= 0 %> -k 300<% end %>
1800 puppet agent --onetime --verbose --no-daemonize --splay --splaylimit 59
--show_diff >> /var/log/puppet.log 2>&1
-@reboot root timeout <% if
scope.function_versioncmp([@lsbdistrelease, "12.04"]) >= 0 %> -k 300<% end %>
1800 puppet agent --onetime --verbose --no-daemonize --no-splay --show_diff >>
/var/log/puppet.log 2>&1
+<%= times %> * * * * root puppet-run
+@reboot root puppet-run
--
To view, visit https://gerrit.wikimedia.org/r/179082
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I4ae2b64d096d38b3eeeda2791949274fb1f69ffa
Gerrit-PatchSet: 1
Gerrit-Project: operations/puppet
Gerrit-Branch: production
Gerrit-Owner: Faidon Liambotis <[email protected]>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
