Merge lp:~lynxman/ubuntu/precise/puppet/puppetlabsfixbug12844 into lp:ubuntu/precise/puppet
- Precise (12.04)
- puppetlabsfixbug12844
- Merge into precise
| Status: | Work in progress |
|---|---|
| Proposed branch: | lp:~lynxman/ubuntu/precise/puppet/puppetlabsfixbug12844 |
| Merge into: | lp:ubuntu/precise/puppet |
| Diff against target: |
4139 lines (+3430/-399) 29 files modified
.pc/.quilt_patches (+1/-0) .pc/.quilt_series (+1/-0) .pc/applied-patches (+1/-0) .pc/puppet-12844/lib/puppet/agent.rb (+114/-0) .pc/puppet-12844/lib/puppet/agent/locker.rb (+30/-0) .pc/puppet-12844/lib/puppet/application/agent.rb (+508/-0) .pc/puppet-12844/lib/puppet/util/anonymous_filelock.rb (+36/-0) .pc/puppet-12844/lib/puppet/util/pidlock.rb (+68/-0) .pc/puppet-12844/spec/unit/agent/locker_spec.rb (+87/-0) .pc/puppet-12844/spec/unit/agent_spec.rb (+285/-0) .pc/puppet-12844/spec/unit/application/agent_spec.rb (+631/-0) .pc/puppet-12844/spec/unit/util/anonymous_filelock_spec.rb (+78/-0) .pc/puppet-12844/spec/unit/util/pidlock_spec.rb (+208/-0) debian/changelog (+13/-0) debian/patches/puppet-12844 (+979/-0) debian/patches/series (+1/-0) debian/puppetmaster-passenger.postinst (+7/-0) lib/puppet/agent.rb (+2/-4) lib/puppet/agent/locker.rb (+15/-1) lib/puppet/application/agent.rb (+3/-11) lib/puppet/util/anonymous_filelock.rb (+0/-36) lib/puppet/util/pidlock.rb (+71/-22) spec/unit/agent/locker_spec.rb (+12/-0) spec/unit/agent_backward_compatibility_spec.rb (+152/-0) spec/unit/agent_spec.rb (+0/-6) spec/unit/application/agent_spec.rb (+1/-33) spec/unit/util/anonymous_filelock_spec.rb (+0/-78) spec/unit/util/pidlock_spec.rb (+0/-208) test/util/pidlock.rb (+126/-0) |
| To merge this branch: | bzr merge lp:~lynxman/ubuntu/precise/puppet/puppetlabsfixbug12844 |
| Related bugs: |
| Reviewer | Review Type | Date Requested | Status |
|---|---|---|---|
| James Page | Needs Fixing on 2012-03-14 | ||
| Ubuntu branches | 2012-03-07 | Pending | |
|
Review via email:
|
|||
Commit Message
Description of the Change
This new puppet package adds a patch by puppetlabs
This reverts commit fcac8f7163c9988
was a backwards-
behavior related to puppet agent lockfiles that was introduced in
2.7.10. The fix is being reverted because we've decided to remove
the new lockfile behavior from the 2.7.x series entirely, and push
it out to 3.x.
This patch is needed in order to not break compatibility with the current user base, will be introduced as part of 2.7.12 but since we're shipping 2.7.11 for 12.04 it would be wise to include this reversion in time.
- 62. By Marc Cluet on 2012-03-16
-
* Added patch to solve puppetlabs bug #12844
- Reverts behaviour of agent lockfiles
- Fixes unit test to readmit old behaviour
* debian/puppetmaster- passenger. postinst (LP: #948983)
- Fixed rack directory location
- Added proper enabling of apache2 headers mod
* debian/puppetmaster- passenger. postinst (LP: #950183)
- Make sure we error if puppet config print doesn't work - 63. By Marc Cluet on 2012-03-16
-
* Added patch for puppet 12844
- 64. By Marc Cluet on 2012-03-16
-
* Correct patch DEP3 header format
Unmerged revisions
- 64. By Marc Cluet on 2012-03-16
-
* Correct patch DEP3 header format
- 63. By Marc Cluet on 2012-03-16
-
* Added patch for puppet 12844
- 62. By Marc Cluet on 2012-03-16
-
* Added patch to solve puppetlabs bug #12844
- Reverts behaviour of agent lockfiles
- Fixes unit test to readmit old behaviour
* debian/puppetmaster- passenger. postinst (LP: #948983)
- Fixed rack directory location
- Added proper enabling of apache2 headers mod
* debian/puppetmaster- passenger. postinst (LP: #950183)
- Make sure we error if puppet config print doesn't work
Preview Diff
| 1 | === added file '.pc/.quilt_patches' |
| 2 | --- .pc/.quilt_patches 1970-01-01 00:00:00 +0000 |
| 3 | +++ .pc/.quilt_patches 2012-03-16 16:38:20 +0000 |
| 4 | @@ -0,0 +1,1 @@ |
| 5 | +patches |
| 6 | |
| 7 | === added file '.pc/.quilt_series' |
| 8 | --- .pc/.quilt_series 1970-01-01 00:00:00 +0000 |
| 9 | +++ .pc/.quilt_series 2012-03-16 16:38:20 +0000 |
| 10 | @@ -0,0 +1,1 @@ |
| 11 | +series |
| 12 | |
| 13 | === modified file '.pc/applied-patches' |
| 14 | --- .pc/applied-patches 2012-01-26 11:27:00 +0000 |
| 15 | +++ .pc/applied-patches 2012-03-16 16:38:20 +0000 |
| 16 | @@ -1,2 +1,3 @@ |
| 17 | fix_logcheck |
| 18 | debian-changes |
| 19 | +puppet-12844 |
| 20 | |
| 21 | === added directory '.pc/puppet-12844' |
| 22 | === added file '.pc/puppet-12844/.timestamp' |
| 23 | === added directory '.pc/puppet-12844/lib' |
| 24 | === added directory '.pc/puppet-12844/lib/puppet' |
| 25 | === added directory '.pc/puppet-12844/lib/puppet/agent' |
| 26 | === added file '.pc/puppet-12844/lib/puppet/agent.rb' |
| 27 | --- .pc/puppet-12844/lib/puppet/agent.rb 1970-01-01 00:00:00 +0000 |
| 28 | +++ .pc/puppet-12844/lib/puppet/agent.rb 2012-03-16 16:38:20 +0000 |
| 29 | @@ -0,0 +1,114 @@ |
| 30 | +require 'sync' |
| 31 | +require 'puppet/external/event-loop' |
| 32 | +require 'puppet/application' |
| 33 | + |
| 34 | +# A general class for triggering a run of another |
| 35 | +# class. |
| 36 | +class Puppet::Agent |
| 37 | + require 'puppet/agent/locker' |
| 38 | + include Puppet::Agent::Locker |
| 39 | + |
| 40 | + require 'puppet/agent/disabler' |
| 41 | + include Puppet::Agent::Disabler |
| 42 | + |
| 43 | + attr_reader :client_class, :client, :splayed |
| 44 | + |
| 45 | + # Just so we can specify that we are "the" instance. |
| 46 | + def initialize(client_class) |
| 47 | + @splayed = false |
| 48 | + |
| 49 | + @client_class = client_class |
| 50 | + end |
| 51 | + |
| 52 | + def lockfile_path |
| 53 | + client_class.lockfile_path |
| 54 | + end |
| 55 | + |
| 56 | + def needing_restart? |
| 57 | + Puppet::Application.restart_requested? |
| 58 | + end |
| 59 | + |
| 60 | + # Perform a run with our client. |
| 61 | + def run(*args) |
| 62 | + if running? |
| 63 | + Puppet.notice "Run of #{client_class} already in progress; skipping" |
| 64 | + return |
| 65 | + end |
| 66 | + if disabled? |
| 67 | + Puppet.notice "Skipping run of #{client_class}; administratively disabled: #{disable_message}" |
| 68 | + return |
| 69 | + end |
| 70 | + result = nil |
| 71 | + block_run = Puppet::Application.controlled_run do |
| 72 | + splay |
| 73 | + with_client do |client| |
| 74 | + begin |
| 75 | + sync.synchronize { lock { result = client.run(*args) } } |
| 76 | + rescue SystemExit,NoMemoryError |
| 77 | + raise |
| 78 | + rescue Exception => detail |
| 79 | + puts detail.backtrace if Puppet[:trace] |
| 80 | + Puppet.err "Could not run #{client_class}: #{detail}" |
| 81 | + end |
| 82 | + end |
| 83 | + true |
| 84 | + end |
| 85 | + Puppet.notice "Shutdown/restart in progress; skipping run" unless block_run |
| 86 | + result |
| 87 | + end |
| 88 | + |
| 89 | + def stopping? |
| 90 | + Puppet::Application.stop_requested? |
| 91 | + end |
| 92 | + |
| 93 | + # Have we splayed already? |
| 94 | + def splayed? |
| 95 | + splayed |
| 96 | + end |
| 97 | + |
| 98 | + # Sleep when splay is enabled; else just return. |
| 99 | + def splay |
| 100 | + return unless Puppet[:splay] |
| 101 | + return if splayed? |
| 102 | + |
| 103 | + time = rand(Integer(Puppet[:splaylimit]) + 1) |
| 104 | + Puppet.info "Sleeping for #{time} seconds (splay is enabled)" |
| 105 | + sleep(time) |
| 106 | + @splayed = true |
| 107 | + end |
| 108 | + |
| 109 | + # Start listening for events. We're pretty much just listening for |
| 110 | + # timer events here. |
| 111 | + def start |
| 112 | + # Create our timer. Puppet will handle observing it and such. |
| 113 | + timer = EventLoop::Timer.new(:interval => Puppet[:runinterval], :tolerance => 1, :start? => true) do |
| 114 | + run |
| 115 | + end |
| 116 | + |
| 117 | + # Run once before we start following the timer |
| 118 | + timer.sound_alarm |
| 119 | + end |
| 120 | + |
| 121 | + def sync |
| 122 | + @sync ||= Sync.new |
| 123 | + end |
| 124 | + |
| 125 | + private |
| 126 | + |
| 127 | + # Create and yield a client instance, keeping a reference |
| 128 | + # to it during the yield. |
| 129 | + def with_client |
| 130 | + begin |
| 131 | + @client = client_class.new |
| 132 | + rescue SystemExit,NoMemoryError |
| 133 | + raise |
| 134 | + rescue Exception => detail |
| 135 | + puts detail.backtrace if Puppet[:trace] |
| 136 | + Puppet.err "Could not create instance of #{client_class}: #{detail}" |
| 137 | + return |
| 138 | + end |
| 139 | + yield @client |
| 140 | + ensure |
| 141 | + @client = nil |
| 142 | + end |
| 143 | +end |
| 144 | |
| 145 | === added file '.pc/puppet-12844/lib/puppet/agent/locker.rb' |
| 146 | --- .pc/puppet-12844/lib/puppet/agent/locker.rb 1970-01-01 00:00:00 +0000 |
| 147 | +++ .pc/puppet-12844/lib/puppet/agent/locker.rb 2012-03-16 16:38:20 +0000 |
| 148 | @@ -0,0 +1,30 @@ |
| 149 | +require 'puppet/util/pidlock' |
| 150 | + |
| 151 | +# Break out the code related to locking the agent. This module is just |
| 152 | +# included into the agent, but having it here makes it easier to test. |
| 153 | +module Puppet::Agent::Locker |
| 154 | + # Yield if we get a lock, else do nothing. Return |
| 155 | + # true/false depending on whether we get the lock. |
| 156 | + def lock |
| 157 | + if lockfile.lock |
| 158 | + begin |
| 159 | + yield |
| 160 | + ensure |
| 161 | + lockfile.unlock |
| 162 | + end |
| 163 | + return true |
| 164 | + else |
| 165 | + return false |
| 166 | + end |
| 167 | + end |
| 168 | + |
| 169 | + def lockfile |
| 170 | + @lockfile ||= Puppet::Util::Pidlock.new(lockfile_path) |
| 171 | + |
| 172 | + @lockfile |
| 173 | + end |
| 174 | + |
| 175 | + def running? |
| 176 | + lockfile.locked? |
| 177 | + end |
| 178 | +end |
| 179 | |
| 180 | === added directory '.pc/puppet-12844/lib/puppet/application' |
| 181 | === added file '.pc/puppet-12844/lib/puppet/application/agent.rb' |
| 182 | --- .pc/puppet-12844/lib/puppet/application/agent.rb 1970-01-01 00:00:00 +0000 |
| 183 | +++ .pc/puppet-12844/lib/puppet/application/agent.rb 2012-03-16 16:38:20 +0000 |
| 184 | @@ -0,0 +1,508 @@ |
| 185 | +require 'puppet/application' |
| 186 | + |
| 187 | +class Puppet::Application::Agent < Puppet::Application |
| 188 | + |
| 189 | + should_parse_config |
| 190 | + run_mode :agent |
| 191 | + |
| 192 | + attr_accessor :args, :agent, :daemon, :host |
| 193 | + |
| 194 | + def preinit |
| 195 | + # Do an initial trap, so that cancels don't get a stack trace. |
| 196 | + Signal.trap(:INT) do |
| 197 | + $stderr.puts "Cancelling startup" |
| 198 | + exit(0) |
| 199 | + end |
| 200 | + |
| 201 | + { |
| 202 | + :waitforcert => nil, |
| 203 | + :detailed_exitcodes => false, |
| 204 | + :verbose => false, |
| 205 | + :debug => false, |
| 206 | + :centrallogs => false, |
| 207 | + :setdest => false, |
| 208 | + :enable => false, |
| 209 | + :disable => false, |
| 210 | + :client => true, |
| 211 | + :fqdn => nil, |
| 212 | + :serve => [], |
| 213 | + :digest => :MD5, |
| 214 | + :fingerprint => false, |
| 215 | + }.each do |opt,val| |
| 216 | + options[opt] = val |
| 217 | + end |
| 218 | + |
| 219 | + @args = {} |
| 220 | + require 'puppet/daemon' |
| 221 | + @daemon = Puppet::Daemon.new |
| 222 | + @daemon.argv = ARGV.dup |
| 223 | + end |
| 224 | + |
| 225 | + option("--centrallogging") |
| 226 | + |
| 227 | + option("--disable [MESSAGE]") do |message| |
| 228 | + options[:disable] = true |
| 229 | + options[:disable_message] = message |
| 230 | + end |
| 231 | + |
| 232 | + option("--enable") |
| 233 | + option("--debug","-d") |
| 234 | + option("--fqdn FQDN","-f") |
| 235 | + option("--test","-t") |
| 236 | + option("--verbose","-v") |
| 237 | + |
| 238 | + option("--fingerprint") |
| 239 | + option("--digest DIGEST") |
| 240 | + |
| 241 | + option("--serve HANDLER", "-s") do |arg| |
| 242 | + if Puppet::Network::Handler.handler(arg) |
| 243 | + options[:serve] << arg.to_sym |
| 244 | + else |
| 245 | + raise "Could not find handler for #{arg}" |
| 246 | + end |
| 247 | + end |
| 248 | + |
| 249 | + option("--no-client") do |arg| |
| 250 | + options[:client] = false |
| 251 | + end |
| 252 | + |
| 253 | + option("--detailed-exitcodes") do |arg| |
| 254 | + options[:detailed_exitcodes] = true |
| 255 | + end |
| 256 | + |
| 257 | + option("--logdest DEST", "-l DEST") do |arg| |
| 258 | + begin |
| 259 | + Puppet::Util::Log.newdestination(arg) |
| 260 | + options[:setdest] = true |
| 261 | + rescue => detail |
| 262 | + puts detail.backtrace if Puppet[:debug] |
| 263 | + $stderr.puts detail.to_s |
| 264 | + end |
| 265 | + end |
| 266 | + |
| 267 | + option("--waitforcert WAITFORCERT", "-w") do |arg| |
| 268 | + options[:waitforcert] = arg.to_i |
| 269 | + end |
| 270 | + |
| 271 | + option("--port PORT","-p") do |arg| |
| 272 | + @args[:Port] = arg |
| 273 | + end |
| 274 | + |
| 275 | + def help |
| 276 | + <<-HELP |
| 277 | + |
| 278 | +puppet-agent(8) -- The puppet agent daemon |
| 279 | +======== |
| 280 | + |
| 281 | +SYNOPSIS |
| 282 | +-------- |
| 283 | +Retrieves the client configuration from the puppet master and applies it to |
| 284 | +the local host. |
| 285 | + |
| 286 | +This service may be run as a daemon, run periodically using cron (or something |
| 287 | +similar), or run interactively for testing purposes. |
| 288 | + |
| 289 | + |
| 290 | +USAGE |
| 291 | +----- |
| 292 | +puppet agent [--certname <name>] [-D|--daemonize|--no-daemonize] |
| 293 | + [-d|--debug] [--detailed-exitcodes] [--digest <digest>] [--disable [message]] [--enable] |
| 294 | + [--fingerprint] [-h|--help] [-l|--logdest syslog|<file>|console] |
| 295 | + [--no-client] [--noop] [-o|--onetime] [--serve <handler>] [-t|--test] |
| 296 | + [-v|--verbose] [-V|--version] [-w|--waitforcert <seconds>] |
| 297 | + |
| 298 | + |
| 299 | +DESCRIPTION |
| 300 | +----------- |
| 301 | +This is the main puppet client. Its job is to retrieve the local |
| 302 | +machine's configuration from a remote server and apply it. In order to |
| 303 | +successfully communicate with the remote server, the client must have a |
| 304 | +certificate signed by a certificate authority that the server trusts; |
| 305 | +the recommended method for this, at the moment, is to run a certificate |
| 306 | +authority as part of the puppet server (which is the default). The |
| 307 | +client will connect and request a signed certificate, and will continue |
| 308 | +connecting until it receives one. |
| 309 | + |
| 310 | +Once the client has a signed certificate, it will retrieve its |
| 311 | +configuration and apply it. |
| 312 | + |
| 313 | + |
| 314 | +USAGE NOTES |
| 315 | +----------- |
| 316 | +'puppet agent' does its best to find a compromise between interactive |
| 317 | +use and daemon use. Run with no arguments and no configuration, it will |
| 318 | +go into the background, attempt to get a signed certificate, and retrieve |
| 319 | +and apply its configuration every 30 minutes. |
| 320 | + |
| 321 | +Some flags are meant specifically for interactive use -- in particular, |
| 322 | +'test', 'tags' or 'fingerprint' are useful. 'test' enables verbose |
| 323 | +logging, causes the daemon to stay in the foreground, exits if the |
| 324 | +server's configuration is invalid (this happens if, for instance, you've |
| 325 | +left a syntax error on the server), and exits after running the |
| 326 | +configuration once (rather than hanging around as a long-running |
| 327 | +process). |
| 328 | + |
| 329 | +'tags' allows you to specify what portions of a configuration you want |
| 330 | +to apply. Puppet elements are tagged with all of the class or definition |
| 331 | +names that contain them, and you can use the 'tags' flag to specify one |
| 332 | +of these names, causing only configuration elements contained within |
| 333 | +that class or definition to be applied. This is very useful when you are |
| 334 | +testing new configurations -- for instance, if you are just starting to |
| 335 | +manage 'ntpd', you would put all of the new elements into an 'ntpd' |
| 336 | +class, and call puppet with '--tags ntpd', which would only apply that |
| 337 | +small portion of the configuration during your testing, rather than |
| 338 | +applying the whole thing. |
| 339 | + |
| 340 | +'fingerprint' is a one-time flag. In this mode 'puppet agent' will run |
| 341 | +once and display on the console (and in the log) the current certificate |
| 342 | +(or certificate request) fingerprint. Providing the '--digest' option |
| 343 | +allows to use a different digest algorithm to generate the fingerprint. |
| 344 | +The main use is to verify that before signing a certificate request on |
| 345 | +the master, the certificate request the master received is the same as |
| 346 | +the one the client sent (to prevent against man-in-the-middle attacks |
| 347 | +when signing certificates). |
| 348 | + |
| 349 | + |
| 350 | +OPTIONS |
| 351 | +------- |
| 352 | +Note that any configuration parameter that's valid in the configuration |
| 353 | +file is also a valid long argument. For example, 'server' is a valid |
| 354 | +configuration parameter, so you can specify '--server <servername>' as |
| 355 | +an argument. |
| 356 | + |
| 357 | +See the configuration file documentation at |
| 358 | +http://docs.puppetlabs.com/references/stable/configuration.html for the |
| 359 | +full list of acceptable parameters. A commented list of all |
| 360 | +configuration options can also be generated by running puppet agent with |
| 361 | +'--genconfig'. |
| 362 | + |
| 363 | +* --certname: |
| 364 | + Set the certname (unique ID) of the client. The master reads this |
| 365 | + unique identifying string, which is usually set to the node's |
| 366 | + fully-qualified domain name, to determine which configurations the |
| 367 | + node will receive. Use this option to debug setup problems or |
| 368 | + implement unusual node identification schemes. |
| 369 | + |
| 370 | +* --daemonize: |
| 371 | + Send the process into the background. This is the default. |
| 372 | + |
| 373 | +* --no-daemonize: |
| 374 | + Do not send the process into the background. |
| 375 | + |
| 376 | +* --debug: |
| 377 | + Enable full debugging. |
| 378 | + |
| 379 | +* --detailed-exitcodes: |
| 380 | + Provide transaction information via exit codes. If this is enabled, an exit |
| 381 | + code of '2' means there were changes, an exit code of '4' means there were |
| 382 | + failures during the transaction, and an exit code of '6' means there were both |
| 383 | + changes and failures. |
| 384 | + |
| 385 | +* --digest: |
| 386 | + Change the certificate fingerprinting digest algorithm. The default is |
| 387 | + MD5. Valid values depends on the version of OpenSSL installed, but |
| 388 | + should always at least contain MD5, MD2, SHA1 and SHA256. |
| 389 | + |
| 390 | +* --disable: |
| 391 | + Disable working on the local system. This puts a lock file in place, |
| 392 | + causing 'puppet agent' not to work on the system until the lock file |
| 393 | + is removed. This is useful if you are testing a configuration and do |
| 394 | + not want the central configuration to override the local state until |
| 395 | + everything is tested and committed. |
| 396 | + |
| 397 | + Disable can also take an optional message that will be reported by the |
| 398 | + 'puppet agent' at the next disabled run. |
| 399 | + |
| 400 | + 'puppet agent' uses the same lock file while it is running, so no more |
| 401 | + than one 'puppet agent' process is working at a time. |
| 402 | + |
| 403 | + 'puppet agent' exits after executing this. |
| 404 | + |
| 405 | +* --enable: |
| 406 | + Enable working on the local system. This removes any lock file, |
| 407 | + causing 'puppet agent' to start managing the local system again |
| 408 | + (although it will continue to use its normal scheduling, so it might |
| 409 | + not start for another half hour). |
| 410 | + |
| 411 | + 'puppet agent' exits after executing this. |
| 412 | + |
| 413 | +* --fingerprint: |
| 414 | + Display the current certificate or certificate signing request |
| 415 | + fingerprint and then exit. Use the '--digest' option to change the |
| 416 | + digest algorithm used. |
| 417 | + |
| 418 | +* --help: |
| 419 | + Print this help message |
| 420 | + |
| 421 | +* --logdest: |
| 422 | + Where to send messages. Choose between syslog, the console, and a log |
| 423 | + file. Defaults to sending messages to syslog, or the console if |
| 424 | + debugging or verbosity is enabled. |
| 425 | + |
| 426 | +* --no-client: |
| 427 | + Do not create a config client. This will cause the daemon to run |
| 428 | + without ever checking for its configuration automatically, and only |
| 429 | + makes sense when puppet agent is being run with listen = true in puppet.conf |
| 430 | + or was started with the `--listen` option. |
| 431 | + |
| 432 | +* --noop: |
| 433 | + Use 'noop' mode where the daemon runs in a no-op or dry-run mode. This |
| 434 | + is useful for seeing what changes Puppet will make without actually |
| 435 | + executing the changes. |
| 436 | + |
| 437 | +* --onetime: |
| 438 | + Run the configuration once. Runs a single (normally daemonized) Puppet |
| 439 | + run. Useful for interactively running puppet agent when used in |
| 440 | + conjunction with the --no-daemonize option. |
| 441 | + |
| 442 | +* --serve: |
| 443 | + Start another type of server. By default, 'puppet agent' will start a |
| 444 | + service handler that allows authenticated and authorized remote nodes |
| 445 | + to trigger the configuration to be pulled down and applied. You can |
| 446 | + specify any handler here that does not require configuration, e.g., |
| 447 | + filebucket, ca, or resource. The handlers are in |
| 448 | + 'lib/puppet/network/handler', and the names must match exactly, both |
| 449 | + in the call to 'serve' and in 'namespaceauth.conf'. |
| 450 | + |
| 451 | +* --test: |
| 452 | + Enable the most common options used for testing. These are 'onetime', |
| 453 | + 'verbose', 'ignorecache', 'no-daemonize', 'no-usecacheonfailure', |
| 454 | + 'detailed-exit-codes', 'no-splay', and 'show_diff'. |
| 455 | + |
| 456 | +* --verbose: |
| 457 | + Turn on verbose reporting. |
| 458 | + |
| 459 | +* --version: |
| 460 | + Print the puppet version number and exit. |
| 461 | + |
| 462 | +* --waitforcert: |
| 463 | + This option only matters for daemons that do not yet have certificates |
| 464 | + and it is enabled by default, with a value of 120 (seconds). This |
| 465 | + causes 'puppet agent' to connect to the server every 2 minutes and ask |
| 466 | + it to sign a certificate request. This is useful for the initial setup |
| 467 | + of a puppet client. You can turn off waiting for certificates by |
| 468 | + specifying a time of 0. |
| 469 | + |
| 470 | + |
| 471 | +EXAMPLE |
| 472 | +------- |
| 473 | + $ puppet agent --server puppet.domain.com |
| 474 | + |
| 475 | + |
| 476 | +DIAGNOSTICS |
| 477 | +----------- |
| 478 | + |
| 479 | +Puppet agent accepts the following signals: |
| 480 | + |
| 481 | +* SIGHUP: |
| 482 | + Restart the puppet agent daemon. |
| 483 | +* SIGINT and SIGTERM: |
| 484 | + Shut down the puppet agent daemon. |
| 485 | +* SIGUSR1: |
| 486 | + Immediately retrieve and apply configurations from the puppet master. |
| 487 | + |
| 488 | +AUTHOR |
| 489 | +------ |
| 490 | +Luke Kanies |
| 491 | + |
| 492 | + |
| 493 | +COPYRIGHT |
| 494 | +--------- |
| 495 | +Copyright (c) 2011 Puppet Labs, LLC Licensed under the Apache 2.0 License |
| 496 | + |
| 497 | + HELP |
| 498 | + end |
| 499 | + |
| 500 | + def run_command |
| 501 | + return fingerprint if options[:fingerprint] |
| 502 | + return onetime if Puppet[:onetime] |
| 503 | + main |
| 504 | + end |
| 505 | + |
| 506 | + def fingerprint |
| 507 | + unless cert = host.certificate || host.certificate_request |
| 508 | + $stderr.puts "Fingerprint asked but no certificate nor certificate request have yet been issued" |
| 509 | + exit(1) |
| 510 | + return |
| 511 | + end |
| 512 | + unless fingerprint = cert.fingerprint(options[:digest]) |
| 513 | + raise ArgumentError, "Could not get fingerprint for digest '#{options[:digest]}'" |
| 514 | + end |
| 515 | + puts fingerprint |
| 516 | + end |
| 517 | + |
| 518 | + def onetime |
| 519 | + unless options[:client] |
| 520 | + $stderr.puts "onetime is specified but there is no client" |
| 521 | + exit(43) |
| 522 | + return |
| 523 | + end |
| 524 | + |
| 525 | + @daemon.set_signal_traps |
| 526 | + |
| 527 | + begin |
| 528 | + report = @agent.run |
| 529 | + rescue => detail |
| 530 | + puts detail.backtrace if Puppet[:trace] |
| 531 | + Puppet.err detail.to_s |
| 532 | + end |
| 533 | + |
| 534 | + @daemon.stop(:exit => false) |
| 535 | + |
| 536 | + if not report |
| 537 | + exit(1) |
| 538 | + elsif options[:detailed_exitcodes] then |
| 539 | + exit(report.exit_status) |
| 540 | + else |
| 541 | + exit(0) |
| 542 | + end |
| 543 | + end |
| 544 | + |
| 545 | + def main |
| 546 | + Puppet.notice "Starting Puppet client version #{Puppet.version}" |
| 547 | + |
| 548 | + @daemon.start |
| 549 | + end |
| 550 | + |
| 551 | + # Enable all of the most common test options. |
| 552 | + def setup_test |
| 553 | + Puppet.settings.handlearg("--ignorecache") |
| 554 | + Puppet.settings.handlearg("--no-usecacheonfailure") |
| 555 | + Puppet.settings.handlearg("--no-splay") |
| 556 | + Puppet.settings.handlearg("--show_diff") |
| 557 | + Puppet.settings.handlearg("--no-daemonize") |
| 558 | + options[:verbose] = true |
| 559 | + Puppet[:onetime] = true |
| 560 | + options[:detailed_exitcodes] = true |
| 561 | + end |
| 562 | + |
| 563 | + # Handle the logging settings. |
| 564 | + def setup_logs |
| 565 | + if options[:debug] or options[:verbose] |
| 566 | + Puppet::Util::Log.newdestination(:console) |
| 567 | + if options[:debug] |
| 568 | + Puppet::Util::Log.level = :debug |
| 569 | + else |
| 570 | + Puppet::Util::Log.level = :info |
| 571 | + end |
| 572 | + end |
| 573 | + |
| 574 | + Puppet::Util::Log.newdestination(:syslog) unless options[:setdest] |
| 575 | + end |
| 576 | + |
| 577 | + def enable_disable_client(agent) |
| 578 | + if options[:enable] |
| 579 | + agent.enable |
| 580 | + elsif options[:disable] |
| 581 | + agent.disable(options[:disable_message] || 'reason not specified') |
| 582 | + end |
| 583 | + exit(0) |
| 584 | + end |
| 585 | + |
| 586 | + def setup_listen |
| 587 | + unless FileTest.exists?(Puppet[:rest_authconfig]) |
| 588 | + Puppet.err "Will not start without authorization file #{Puppet[:rest_authconfig]}" |
| 589 | + exit(14) |
| 590 | + end |
| 591 | + |
| 592 | + handlers = nil |
| 593 | + |
| 594 | + if options[:serve].empty? |
| 595 | + handlers = [:Runner] |
| 596 | + else |
| 597 | + handlers = options[:serve] |
| 598 | + end |
| 599 | + |
| 600 | + require 'puppet/network/server' |
| 601 | + # No REST handlers yet. |
| 602 | + server = Puppet::Network::Server.new(:xmlrpc_handlers => handlers, :port => Puppet[:puppetport]) |
| 603 | + |
| 604 | + @daemon.server = server |
| 605 | + end |
| 606 | + |
| 607 | + def setup_host |
| 608 | + @host = Puppet::SSL::Host.new |
| 609 | + waitforcert = options[:waitforcert] || (Puppet[:onetime] ? 0 : 120) |
| 610 | + cert = @host.wait_for_cert(waitforcert) unless options[:fingerprint] |
| 611 | + end |
| 612 | + |
| 613 | + def setup_agent |
| 614 | + # We need tomake the client either way, we just don't start it |
| 615 | + # if --no-client is set. |
| 616 | + require 'puppet/agent' |
| 617 | + require 'puppet/configurer' |
| 618 | + @agent = Puppet::Agent.new(Puppet::Configurer) |
| 619 | + |
| 620 | + enable_disable_client(@agent) if options[:enable] or options[:disable] |
| 621 | + |
| 622 | + @daemon.agent = agent if options[:client] |
| 623 | + |
| 624 | + # It'd be nice to daemonize later, but we have to daemonize before the |
| 625 | + # waitforcert happens. |
| 626 | + @daemon.daemonize if Puppet[:daemonize] |
| 627 | + |
| 628 | + setup_host |
| 629 | + |
| 630 | + @objects = [] |
| 631 | + |
| 632 | + # This has to go after the certs are dealt with. |
| 633 | + if Puppet[:listen] |
| 634 | + unless Puppet[:onetime] |
| 635 | + setup_listen |
| 636 | + else |
| 637 | + Puppet.notice "Ignoring --listen on onetime run" |
| 638 | + end |
| 639 | + end |
| 640 | + end |
| 641 | + |
| 642 | + def setup |
| 643 | + setup_test if options[:test] |
| 644 | + |
| 645 | + setup_logs |
| 646 | + |
| 647 | + exit(Puppet.settings.print_configs ? 0 : 1) if Puppet.settings.print_configs? |
| 648 | + |
| 649 | + args[:Server] = Puppet[:server] |
| 650 | + if options[:fqdn] |
| 651 | + args[:FQDN] = options[:fqdn] |
| 652 | + Puppet[:certname] = options[:fqdn] |
| 653 | + end |
| 654 | + |
| 655 | + if options[:centrallogs] |
| 656 | + logdest = args[:Server] |
| 657 | + |
| 658 | + logdest += ":" + args[:Port] if args.include?(:Port) |
| 659 | + Puppet::Util::Log.newdestination(logdest) |
| 660 | + end |
| 661 | + |
| 662 | + Puppet.settings.use :main, :agent, :ssl |
| 663 | + |
| 664 | + # Always ignoreimport for agent. It really shouldn't even try to import, |
| 665 | + # but this is just a temporary band-aid. |
| 666 | + Puppet[:ignoreimport] = true |
| 667 | + |
| 668 | + # We need to specify a ca location for all of the SSL-related i |
| 669 | + # indirected classes to work; in fingerprint mode we just need |
| 670 | + # access to the local files and we don't need a ca. |
| 671 | + Puppet::SSL::Host.ca_location = options[:fingerprint] ? :none : :remote |
| 672 | + |
| 673 | + Puppet::Transaction::Report.indirection.terminus_class = :rest |
| 674 | + # we want the last report to be persisted locally |
| 675 | + Puppet::Transaction::Report.indirection.cache_class = :yaml |
| 676 | + |
| 677 | + # Override the default; puppetd needs this, usually. |
| 678 | + # You can still override this on the command-line with, e.g., :compiler. |
| 679 | + Puppet[:catalog_terminus] = :rest |
| 680 | + |
| 681 | + # Override the default. |
| 682 | + Puppet[:facts_terminus] = :facter |
| 683 | + |
| 684 | + Puppet::Resource::Catalog.indirection.cache_class = :yaml |
| 685 | + |
| 686 | + unless options[:fingerprint] |
| 687 | + setup_agent |
| 688 | + else |
| 689 | + setup_host |
| 690 | + end |
| 691 | + end |
| 692 | +end |
| 693 | |
| 694 | === added directory '.pc/puppet-12844/lib/puppet/util' |
| 695 | === added file '.pc/puppet-12844/lib/puppet/util/anonymous_filelock.rb' |
| 696 | --- .pc/puppet-12844/lib/puppet/util/anonymous_filelock.rb 1970-01-01 00:00:00 +0000 |
| 697 | +++ .pc/puppet-12844/lib/puppet/util/anonymous_filelock.rb 2012-03-16 16:38:20 +0000 |
| 698 | @@ -0,0 +1,36 @@ |
| 699 | + |
| 700 | +class Puppet::Util::AnonymousFilelock |
| 701 | + attr_reader :lockfile |
| 702 | + |
| 703 | + def initialize(lockfile) |
| 704 | + @lockfile = lockfile |
| 705 | + end |
| 706 | + |
| 707 | + def anonymous? |
| 708 | + true |
| 709 | + end |
| 710 | + |
| 711 | + def lock(msg = '') |
| 712 | + return false if locked? |
| 713 | + |
| 714 | + File.open(@lockfile, 'w') { |fd| fd.print(msg) } |
| 715 | + true |
| 716 | + end |
| 717 | + |
| 718 | + def unlock |
| 719 | + if locked? |
| 720 | + File.unlink(@lockfile) |
| 721 | + true |
| 722 | + else |
| 723 | + false |
| 724 | + end |
| 725 | + end |
| 726 | + |
| 727 | + def locked? |
| 728 | + File.exists? @lockfile |
| 729 | + end |
| 730 | + |
| 731 | + def message |
| 732 | + return File.read(@lockfile) if locked? |
| 733 | + end |
| 734 | +end |
| 735 | \ No newline at end of file |
| 736 | |
| 737 | === added file '.pc/puppet-12844/lib/puppet/util/pidlock.rb' |
| 738 | --- .pc/puppet-12844/lib/puppet/util/pidlock.rb 1970-01-01 00:00:00 +0000 |
| 739 | +++ .pc/puppet-12844/lib/puppet/util/pidlock.rb 2012-03-16 16:38:20 +0000 |
| 740 | @@ -0,0 +1,68 @@ |
| 741 | +require 'fileutils' |
| 742 | +require 'puppet/util/anonymous_filelock' |
| 743 | + |
| 744 | +class Puppet::Util::Pidlock < Puppet::Util::AnonymousFilelock |
| 745 | + |
| 746 | + def locked? |
| 747 | + clear_if_stale |
| 748 | + File.exists? @lockfile |
| 749 | + end |
| 750 | + |
| 751 | + def mine? |
| 752 | + Process.pid == lock_pid |
| 753 | + end |
| 754 | + |
| 755 | + def anonymous? |
| 756 | + false |
| 757 | + end |
| 758 | + |
| 759 | + def lock |
| 760 | + return mine? if locked? |
| 761 | + |
| 762 | + File.open(@lockfile, "w") { |fd| fd.write(Process.pid) } |
| 763 | + true |
| 764 | + end |
| 765 | + |
| 766 | + def unlock(opts = {}) |
| 767 | + if mine? |
| 768 | + begin |
| 769 | + File.unlink(@lockfile) |
| 770 | + rescue Errno::ENOENT |
| 771 | + # Someone deleted it for us ...and so we do nothing. No point whining |
| 772 | + # about a problem that the user can't actually do anything about. |
| 773 | + rescue SystemCallError => e |
| 774 | + # This one is a real failure though. No idea what went wrong, but it |
| 775 | + # is most likely "read only file(system)" or wrong permissions or |
| 776 | + # something like that. |
| 777 | + Puppet.err "Could not remove PID file #{@lockfile}: #{e}" |
| 778 | + puts e.backtrace if Puppet[:trace] |
| 779 | + end |
| 780 | + true |
| 781 | + else |
| 782 | + false |
| 783 | + end |
| 784 | + end |
| 785 | + |
| 786 | + def lock_pid |
| 787 | + if File.exists? @lockfile |
| 788 | + File.read(@lockfile).to_i |
| 789 | + else |
| 790 | + nil |
| 791 | + end |
| 792 | + end |
| 793 | + |
| 794 | + private |
| 795 | + def clear_if_stale |
| 796 | + return if lock_pid.nil? |
| 797 | + |
| 798 | + errors = [Errno::ESRCH] |
| 799 | + # Process::Error can only happen, and is only defined, on Windows |
| 800 | + errors << Process::Error if defined? Process::Error |
| 801 | + |
| 802 | + begin |
| 803 | + Process.kill(0, lock_pid) |
| 804 | + rescue *errors |
| 805 | + File.unlink(@lockfile) |
| 806 | + end |
| 807 | + end |
| 808 | +end |
| 809 | |
| 810 | === added directory '.pc/puppet-12844/spec' |
| 811 | === added directory '.pc/puppet-12844/spec/unit' |
| 812 | === added directory '.pc/puppet-12844/spec/unit/agent' |
| 813 | === added file '.pc/puppet-12844/spec/unit/agent/locker_spec.rb' |
| 814 | --- .pc/puppet-12844/spec/unit/agent/locker_spec.rb 1970-01-01 00:00:00 +0000 |
| 815 | +++ .pc/puppet-12844/spec/unit/agent/locker_spec.rb 2012-03-16 16:38:20 +0000 |
| 816 | @@ -0,0 +1,87 @@ |
| 817 | +#!/usr/bin/env rspec |
| 818 | +require 'spec_helper' |
| 819 | +require 'puppet/agent' |
| 820 | +require 'puppet/agent/locker' |
| 821 | + |
| 822 | +class LockerTester |
| 823 | + include Puppet::Agent::Locker |
| 824 | +end |
| 825 | + |
| 826 | +describe Puppet::Agent::Locker do |
| 827 | + before do |
| 828 | + @locker = LockerTester.new |
| 829 | + @locker.stubs(:lockfile_path).returns "/my/lock" |
| 830 | + end |
| 831 | + |
| 832 | + it "should use a Pidlock instance as its lockfile" do |
| 833 | + @locker.lockfile.should be_instance_of(Puppet::Util::Pidlock) |
| 834 | + end |
| 835 | + |
| 836 | + it "should use 'lockfile_path' to determine its lockfile path" do |
| 837 | + @locker.expects(:lockfile_path).returns "/my/lock" |
| 838 | + lock = Puppet::Util::Pidlock.new("/my/lock") |
| 839 | + Puppet::Util::Pidlock.expects(:new).with("/my/lock").returns lock |
| 840 | + |
| 841 | + @locker.lockfile |
| 842 | + end |
| 843 | + |
| 844 | + it "should reuse the same lock file each time" do |
| 845 | + @locker.lockfile.should equal(@locker.lockfile) |
| 846 | + end |
| 847 | + |
| 848 | + it "should have a method that yields when a lock is attained" do |
| 849 | + @locker.lockfile.expects(:lock).returns true |
| 850 | + |
| 851 | + yielded = false |
| 852 | + @locker.lock do |
| 853 | + yielded = true |
| 854 | + end |
| 855 | + yielded.should be_true |
| 856 | + end |
| 857 | + |
| 858 | + it "should return true when the lock method successfully locked" do |
| 859 | + @locker.lockfile.expects(:lock).returns true |
| 860 | + |
| 861 | + @locker.lock {}.should be_true |
| 862 | + end |
| 863 | + |
| 864 | + it "should return true when the lock method does not receive the lock" do |
| 865 | + @locker.lockfile.expects(:lock).returns false |
| 866 | + |
| 867 | + @locker.lock {}.should be_false |
| 868 | + end |
| 869 | + |
| 870 | + it "should not yield when the lock method does not receive the lock" do |
| 871 | + @locker.lockfile.expects(:lock).returns false |
| 872 | + |
| 873 | + yielded = false |
| 874 | + @locker.lock { yielded = true } |
| 875 | + yielded.should be_false |
| 876 | + end |
| 877 | + |
| 878 | + it "should not unlock when a lock was not received" do |
| 879 | + @locker.lockfile.expects(:lock).returns false |
| 880 | + @locker.lockfile.expects(:unlock).never |
| 881 | + |
| 882 | + @locker.lock {} |
| 883 | + end |
| 884 | + |
| 885 | + it "should unlock after yielding upon obtaining a lock" do |
| 886 | + @locker.lockfile.stubs(:lock).returns true |
| 887 | + @locker.lockfile.expects(:unlock) |
| 888 | + |
| 889 | + @locker.lock {} |
| 890 | + end |
| 891 | + |
| 892 | + it "should unlock after yielding upon obtaining a lock, even if the block throws an exception" do |
| 893 | + @locker.lockfile.stubs(:lock).returns true |
| 894 | + @locker.lockfile.expects(:unlock) |
| 895 | + |
| 896 | + lambda { @locker.lock { raise "foo" } }.should raise_error(RuntimeError) |
| 897 | + end |
| 898 | + |
| 899 | + it "should be considered running if the lockfile is locked" do |
| 900 | + @locker.lockfile.expects(:locked?).returns true |
| 901 | + @locker.should be_running |
| 902 | + end |
| 903 | +end |
| 904 | |
| 905 | === added file '.pc/puppet-12844/spec/unit/agent_backward_compatibility_spec.rb' |
| 906 | === added file '.pc/puppet-12844/spec/unit/agent_spec.rb' |
| 907 | --- .pc/puppet-12844/spec/unit/agent_spec.rb 1970-01-01 00:00:00 +0000 |
| 908 | +++ .pc/puppet-12844/spec/unit/agent_spec.rb 2012-03-16 16:38:20 +0000 |
| 909 | @@ -0,0 +1,285 @@ |
| 910 | +#!/usr/bin/env rspec |
| 911 | +require 'spec_helper' |
| 912 | +require 'puppet/agent' |
| 913 | + |
| 914 | +class AgentTestClient |
| 915 | + def run |
| 916 | + # no-op |
| 917 | + end |
| 918 | + def stop |
| 919 | + # no-op |
| 920 | + end |
| 921 | +end |
| 922 | + |
| 923 | +def without_warnings |
| 924 | + flag = $VERBOSE |
| 925 | + $VERBOSE = nil |
| 926 | + yield |
| 927 | + $VERBOSE = flag |
| 928 | +end |
| 929 | + |
| 930 | +describe Puppet::Agent do |
| 931 | + before do |
| 932 | + @agent = Puppet::Agent.new(AgentTestClient) |
| 933 | + |
| 934 | + # So we don't actually try to hit the filesystem. |
| 935 | + @agent.stubs(:lock).yields |
| 936 | + @agent.stubs(:disabled?).returns(false) |
| 937 | + |
| 938 | + # make Puppet::Application safe for stubbing; restore in an :after block; silence warnings for this. |
| 939 | + without_warnings { Puppet::Application = Class.new(Puppet::Application) } |
| 940 | + Puppet::Application.stubs(:clear?).returns(true) |
| 941 | + Puppet::Application.class_eval do |
| 942 | + class << self |
| 943 | + def controlled_run(&block) |
| 944 | + block.call |
| 945 | + end |
| 946 | + end |
| 947 | + end |
| 948 | + end |
| 949 | + |
| 950 | + after do |
| 951 | + # restore Puppet::Application from stub-safe subclass, and silence warnings |
| 952 | + without_warnings { Puppet::Application = Puppet::Application.superclass } |
| 953 | + end |
| 954 | + |
| 955 | + it "should set its client class at initialization" do |
| 956 | + Puppet::Agent.new("foo").client_class.should == "foo" |
| 957 | + end |
| 958 | + |
| 959 | + it "should include the Locker module" do |
| 960 | + Puppet::Agent.ancestors.should be_include(Puppet::Agent::Locker) |
| 961 | + end |
| 962 | + |
| 963 | + it "should create an instance of its client class and run it when asked to run" do |
| 964 | + client = mock 'client' |
| 965 | + AgentTestClient.expects(:new).returns client |
| 966 | + |
| 967 | + client.expects(:run) |
| 968 | + |
| 969 | + @agent.stubs(:running?).returns false |
| 970 | + @agent.run |
| 971 | + end |
| 972 | + |
| 973 | + it "should determine its lock file path by asking the client class" do |
| 974 | + AgentTestClient.expects(:lockfile_path).returns "/my/lock" |
| 975 | + @agent.lockfile_path.should == "/my/lock" |
| 976 | + end |
| 977 | + |
| 978 | + it "should be considered running if the lock file is locked" do |
| 979 | + lockfile = mock 'lockfile' |
| 980 | + |
| 981 | + @agent.expects(:lockfile).returns lockfile |
| 982 | + lockfile.expects(:locked?).returns true |
| 983 | + |
| 984 | + @agent.should be_running |
| 985 | + end |
| 986 | + |
| 987 | + describe "when being run" do |
| 988 | + before do |
| 989 | + AgentTestClient.stubs(:lockfile_path).returns "/my/lock" |
| 990 | + @agent.stubs(:running?).returns false |
| 991 | + end |
| 992 | + |
| 993 | + it "should splay" do |
| 994 | + @agent.expects(:splay) |
| 995 | + @agent.stubs(:running?).returns false |
| 996 | + |
| 997 | + @agent.run |
| 998 | + end |
| 999 | + |
| 1000 | + it "should do nothing if already running" do |
| 1001 | + @agent.expects(:running?).returns true |
| 1002 | + AgentTestClient.expects(:new).never |
| 1003 | + @agent.run |
| 1004 | + end |
| 1005 | + |
| 1006 | + it "should do nothing if disabled" do |
| 1007 | + @agent.expects(:disabled?).returns(true) |
| 1008 | + AgentTestClient.expects(:new).never |
| 1009 | + @agent.run |
| 1010 | + end |
| 1011 | + |
| 1012 | + it "should use Puppet::Application.controlled_run to manage process state behavior" do |
| 1013 | + calls = sequence('calls') |
| 1014 | + Puppet::Application.expects(:controlled_run).yields.in_sequence(calls) |
| 1015 | + AgentTestClient.expects(:new).once.in_sequence(calls) |
| 1016 | + @agent.run |
| 1017 | + end |
| 1018 | + |
| 1019 | + it "should not fail if a client class instance cannot be created" do |
| 1020 | + AgentTestClient.expects(:new).raises "eh" |
| 1021 | + Puppet.expects(:err) |
| 1022 | + @agent.run |
| 1023 | + end |
| 1024 | + |
| 1025 | + it "should not fail if there is an exception while running its client" do |
| 1026 | + client = AgentTestClient.new |
| 1027 | + AgentTestClient.expects(:new).returns client |
| 1028 | + client.expects(:run).raises "eh" |
| 1029 | + Puppet.expects(:err) |
| 1030 | + @agent.run |
| 1031 | + end |
| 1032 | + |
| 1033 | + it "should use a mutex to restrict multi-threading" do |
| 1034 | + client = AgentTestClient.new |
| 1035 | + AgentTestClient.expects(:new).returns client |
| 1036 | + |
| 1037 | + mutex = mock 'mutex' |
| 1038 | + @agent.expects(:sync).returns mutex |
| 1039 | + |
| 1040 | + mutex.expects(:synchronize) |
| 1041 | + client.expects(:run).never # if it doesn't run, then we know our yield is what triggers it |
| 1042 | + @agent.run |
| 1043 | + end |
| 1044 | + |
| 1045 | + it "should use a filesystem lock to restrict multiple processes running the agent" do |
| 1046 | + client = AgentTestClient.new |
| 1047 | + AgentTestClient.expects(:new).returns client |
| 1048 | + |
| 1049 | + @agent.expects(:lock) |
| 1050 | + |
| 1051 | + client.expects(:run).never # if it doesn't run, then we know our yield is what triggers it |
| 1052 | + @agent.run |
| 1053 | + end |
| 1054 | + |
| 1055 | + it "should make its client instance available while running" do |
| 1056 | + client = AgentTestClient.new |
| 1057 | + AgentTestClient.expects(:new).returns client |
| 1058 | + |
| 1059 | + client.expects(:run).with { @agent.client.should equal(client); true } |
| 1060 | + @agent.run |
| 1061 | + end |
| 1062 | + |
| 1063 | + it "should run the client instance with any arguments passed to it" do |
| 1064 | + client = AgentTestClient.new |
| 1065 | + AgentTestClient.expects(:new).returns client |
| 1066 | + |
| 1067 | + client.expects(:run).with("testargs") |
| 1068 | + @agent.run("testargs") |
| 1069 | + end |
| 1070 | + end |
| 1071 | + |
| 1072 | + describe "when splaying" do |
| 1073 | + before do |
| 1074 | + Puppet.settings.stubs(:value).with(:splay).returns true |
| 1075 | + Puppet.settings.stubs(:value).with(:splaylimit).returns "10" |
| 1076 | + end |
| 1077 | + |
| 1078 | + it "should do nothing if splay is disabled" do |
| 1079 | + Puppet.settings.expects(:value).returns false |
| 1080 | + @agent.expects(:sleep).never |
| 1081 | + @agent.splay |
| 1082 | + end |
| 1083 | + |
| 1084 | + it "should do nothing if it has already splayed" do |
| 1085 | + @agent.expects(:splayed?).returns true |
| 1086 | + @agent.expects(:sleep).never |
| 1087 | + @agent.splay |
| 1088 | + end |
| 1089 | + |
| 1090 | + it "should log that it is splaying" do |
| 1091 | + @agent.stubs :sleep |
| 1092 | + Puppet.expects :info |
| 1093 | + @agent.splay |
| 1094 | + end |
| 1095 | + |
| 1096 | + it "should sleep for a random portion of the splaylimit plus 1" do |
| 1097 | + Puppet.settings.expects(:value).with(:splaylimit).returns "50" |
| 1098 | + @agent.expects(:rand).with(51).returns 10 |
| 1099 | + @agent.expects(:sleep).with(10) |
| 1100 | + @agent.splay |
| 1101 | + end |
| 1102 | + |
| 1103 | + it "should mark that it has splayed" do |
| 1104 | + @agent.stubs(:sleep) |
| 1105 | + @agent.splay |
| 1106 | + @agent.should be_splayed |
| 1107 | + end |
| 1108 | + end |
| 1109 | + |
| 1110 | + describe "when checking execution state" do |
| 1111 | + describe 'with regular run status' do |
| 1112 | + before :each do |
| 1113 | + Puppet::Application.stubs(:restart_requested?).returns(false) |
| 1114 | + Puppet::Application.stubs(:stop_requested?).returns(false) |
| 1115 | + Puppet::Application.stubs(:interrupted?).returns(false) |
| 1116 | + Puppet::Application.stubs(:clear?).returns(true) |
| 1117 | + end |
| 1118 | + |
| 1119 | + it 'should be false for :stopping?' do |
| 1120 | + @agent.stopping?.should be_false |
| 1121 | + end |
| 1122 | + |
| 1123 | + it 'should be false for :needing_restart?' do |
| 1124 | + @agent.needing_restart?.should be_false |
| 1125 | + end |
| 1126 | + end |
| 1127 | + |
| 1128 | + describe 'with a stop requested' do |
| 1129 | + before :each do |
| 1130 | + Puppet::Application.stubs(:clear?).returns(false) |
| 1131 | + Puppet::Application.stubs(:restart_requested?).returns(false) |
| 1132 | + Puppet::Application.stubs(:stop_requested?).returns(true) |
| 1133 | + Puppet::Application.stubs(:interrupted?).returns(true) |
| 1134 | + end |
| 1135 | + |
| 1136 | + it 'should be true for :stopping?' do |
| 1137 | + @agent.stopping?.should be_true |
| 1138 | + end |
| 1139 | + |
| 1140 | + it 'should be false for :needing_restart?' do |
| 1141 | + @agent.needing_restart?.should be_false |
| 1142 | + end |
| 1143 | + end |
| 1144 | + |
| 1145 | + describe 'with a restart requested' do |
| 1146 | + before :each do |
| 1147 | + Puppet::Application.stubs(:clear?).returns(false) |
| 1148 | + Puppet::Application.stubs(:restart_requested?).returns(true) |
| 1149 | + Puppet::Application.stubs(:stop_requested?).returns(false) |
| 1150 | + Puppet::Application.stubs(:interrupted?).returns(true) |
| 1151 | + end |
| 1152 | + |
| 1153 | + it 'should be false for :stopping?' do |
| 1154 | + @agent.stopping?.should be_false |
| 1155 | + end |
| 1156 | + |
| 1157 | + it 'should be true for :needing_restart?' do |
| 1158 | + @agent.needing_restart?.should be_true |
| 1159 | + end |
| 1160 | + end |
| 1161 | + end |
| 1162 | + |
| 1163 | + describe "when starting" do |
| 1164 | + before do |
| 1165 | + @agent.stubs(:observe_signal) |
| 1166 | + end |
| 1167 | + |
| 1168 | + it "should create a timer with the runinterval, a tolerance of 1, and :start? set to true" do |
| 1169 | + Puppet.settings.expects(:value).with(:runinterval).returns 5 |
| 1170 | + timer = stub 'timer', :sound_alarm => nil |
| 1171 | + EventLoop::Timer.expects(:new).with(:interval => 5, :start? => true, :tolerance => 1).returns timer |
| 1172 | + |
| 1173 | + @agent.stubs(:run) |
| 1174 | + @agent.start |
| 1175 | + end |
| 1176 | + |
| 1177 | + it "should run once immediately" do |
| 1178 | + timer = mock 'timer' |
| 1179 | + EventLoop::Timer.expects(:new).returns timer |
| 1180 | + |
| 1181 | + timer.expects(:sound_alarm) |
| 1182 | + |
| 1183 | + @agent.start |
| 1184 | + end |
| 1185 | + |
| 1186 | + it "should run within the block passed to the timer" do |
| 1187 | + timer = stub 'timer', :sound_alarm => nil |
| 1188 | + EventLoop::Timer.expects(:new).returns(timer).yields |
| 1189 | + @agent.expects(:run) |
| 1190 | + |
| 1191 | + @agent.start |
| 1192 | + end |
| 1193 | + end |
| 1194 | +end |
| 1195 | |
| 1196 | === added directory '.pc/puppet-12844/spec/unit/application' |
| 1197 | === added file '.pc/puppet-12844/spec/unit/application/agent_spec.rb' |
| 1198 | --- .pc/puppet-12844/spec/unit/application/agent_spec.rb 1970-01-01 00:00:00 +0000 |
| 1199 | +++ .pc/puppet-12844/spec/unit/application/agent_spec.rb 2012-03-16 16:38:20 +0000 |
| 1200 | @@ -0,0 +1,631 @@ |
| 1201 | +#!/usr/bin/env rspec |
| 1202 | +require 'spec_helper' |
| 1203 | + |
| 1204 | +require 'puppet/agent' |
| 1205 | +require 'puppet/application/agent' |
| 1206 | +require 'puppet/network/server' |
| 1207 | +require 'puppet/daemon' |
| 1208 | +require 'puppet/network/handler' |
| 1209 | + |
| 1210 | +describe Puppet::Application::Agent do |
| 1211 | + before :each do |
| 1212 | + @puppetd = Puppet::Application[:agent] |
| 1213 | + @puppetd.stubs(:puts) |
| 1214 | + @daemon = stub_everything 'daemon' |
| 1215 | + Puppet::Daemon.stubs(:new).returns(@daemon) |
| 1216 | + Puppet[:daemonize] = false |
| 1217 | + @agent = stub_everything 'agent' |
| 1218 | + Puppet::Agent.stubs(:new).returns(@agent) |
| 1219 | + @puppetd.preinit |
| 1220 | + Puppet::Util::Log.stubs(:newdestination) |
| 1221 | + |
| 1222 | + Puppet::Node.indirection.stubs(:terminus_class=) |
| 1223 | + Puppet::Node.indirection.stubs(:cache_class=) |
| 1224 | + Puppet::Node::Facts.indirection.stubs(:terminus_class=) |
| 1225 | + end |
| 1226 | + |
| 1227 | + it "should operate in agent run_mode" do |
| 1228 | + @puppetd.class.run_mode.name.should == :agent |
| 1229 | + end |
| 1230 | + |
| 1231 | + it "should ask Puppet::Application to parse Puppet configuration file" do |
| 1232 | + @puppetd.should_parse_config?.should be_true |
| 1233 | + end |
| 1234 | + |
| 1235 | + it "should declare a main command" do |
| 1236 | + @puppetd.should respond_to(:main) |
| 1237 | + end |
| 1238 | + |
| 1239 | + it "should declare a onetime command" do |
| 1240 | + @puppetd.should respond_to(:onetime) |
| 1241 | + end |
| 1242 | + |
| 1243 | + it "should declare a fingerprint command" do |
| 1244 | + @puppetd.should respond_to(:fingerprint) |
| 1245 | + end |
| 1246 | + |
| 1247 | + it "should declare a preinit block" do |
| 1248 | + @puppetd.should respond_to(:preinit) |
| 1249 | + end |
| 1250 | + |
| 1251 | + describe "in preinit" do |
| 1252 | + it "should catch INT" do |
| 1253 | + Signal.expects(:trap).with { |arg,block| arg == :INT } |
| 1254 | + |
| 1255 | + @puppetd.preinit |
| 1256 | + end |
| 1257 | + |
| 1258 | + it "should init client to true" do |
| 1259 | + @puppetd.preinit |
| 1260 | + |
| 1261 | + @puppetd.options[:client].should be_true |
| 1262 | + end |
| 1263 | + |
| 1264 | + it "should init fqdn to nil" do |
| 1265 | + @puppetd.preinit |
| 1266 | + |
| 1267 | + @puppetd.options[:fqdn].should be_nil |
| 1268 | + end |
| 1269 | + |
| 1270 | + it "should init serve to []" do |
| 1271 | + @puppetd.preinit |
| 1272 | + |
| 1273 | + @puppetd.options[:serve].should == [] |
| 1274 | + end |
| 1275 | + |
| 1276 | + it "should use MD5 as default digest algorithm" do |
| 1277 | + @puppetd.preinit |
| 1278 | + |
| 1279 | + @puppetd.options[:digest].should == :MD5 |
| 1280 | + end |
| 1281 | + |
| 1282 | + it "should not fingerprint by default" do |
| 1283 | + @puppetd.preinit |
| 1284 | + |
| 1285 | + @puppetd.options[:fingerprint].should be_false |
| 1286 | + end |
| 1287 | + end |
| 1288 | + |
| 1289 | + describe "when handling options" do |
| 1290 | + before do |
| 1291 | + @puppetd.command_line.stubs(:args).returns([]) |
| 1292 | + end |
| 1293 | + |
| 1294 | + [:centrallogging, :enable, :debug, :fqdn, :test, :verbose, :digest].each do |option| |
| 1295 | + it "should declare handle_#{option} method" do |
| 1296 | + @puppetd.should respond_to("handle_#{option}".to_sym) |
| 1297 | + end |
| 1298 | + |
| 1299 | + it "should store argument value when calling handle_#{option}" do |
| 1300 | + @puppetd.options.expects(:[]=).with(option, 'arg') |
| 1301 | + @puppetd.send("handle_#{option}".to_sym, 'arg') |
| 1302 | + end |
| 1303 | + end |
| 1304 | + |
| 1305 | + describe "when handling --disable" do |
| 1306 | + it "should declare handle_disable method" do |
| 1307 | + @puppetd.should respond_to(:handle_disable) |
| 1308 | + end |
| 1309 | + |
| 1310 | + it "should set disable to true" do |
| 1311 | + @puppetd.options.stubs(:[]=) |
| 1312 | + @puppetd.options.expects(:[]=).with(:disable, true) |
| 1313 | + @puppetd.handle_disable('') |
| 1314 | + end |
| 1315 | + |
| 1316 | + it "should store disable message" do |
| 1317 | + @puppetd.options.stubs(:[]=) |
| 1318 | + @puppetd.options.expects(:[]=).with(:disable_message, "message") |
| 1319 | + @puppetd.handle_disable('message') |
| 1320 | + end |
| 1321 | + end |
| 1322 | + |
| 1323 | + it "should set an existing handler on server" do |
| 1324 | + Puppet::Network::Handler.stubs(:handler).with("handler").returns(true) |
| 1325 | + |
| 1326 | + @puppetd.handle_serve("handler") |
| 1327 | + @puppetd.options[:serve].should == [ :handler ] |
| 1328 | + end |
| 1329 | + |
| 1330 | + it "should set client to false with --no-client" do |
| 1331 | + @puppetd.handle_no_client(nil) |
| 1332 | + @puppetd.options[:client].should be_false |
| 1333 | + end |
| 1334 | + |
| 1335 | + it "should set waitforcert to 0 with --onetime and if --waitforcert wasn't given" do |
| 1336 | + Puppet[:onetime] = true |
| 1337 | + Puppet::SSL::Host.any_instance.expects(:wait_for_cert).with(0) |
| 1338 | + @puppetd.setup_host |
| 1339 | + end |
| 1340 | + |
| 1341 | + it "should use supplied waitforcert when --onetime is specified" do |
| 1342 | + Puppet[:onetime] = true |
| 1343 | + @puppetd.handle_waitforcert(60) |
| 1344 | + Puppet::SSL::Host.any_instance.expects(:wait_for_cert).with(60) |
| 1345 | + @puppetd.setup_host |
| 1346 | + end |
| 1347 | + |
| 1348 | + it "should use a default value for waitforcert when --onetime and --waitforcert are not specified" do |
| 1349 | + Puppet::SSL::Host.any_instance.expects(:wait_for_cert).with(120) |
| 1350 | + @puppetd.setup_host |
| 1351 | + end |
| 1352 | + |
| 1353 | + it "should set the log destination with --logdest" do |
| 1354 | + @puppetd.options.stubs(:[]=).with { |opt,val| opt == :setdest } |
| 1355 | + Puppet::Log.expects(:newdestination).with("console") |
| 1356 | + |
| 1357 | + @puppetd.handle_logdest("console") |
| 1358 | + end |
| 1359 | + |
| 1360 | + it "should put the setdest options to true" do |
| 1361 | + @puppetd.options.expects(:[]=).with(:setdest,true) |
| 1362 | + |
| 1363 | + @puppetd.handle_logdest("console") |
| 1364 | + end |
| 1365 | + |
| 1366 | + it "should parse the log destination from the command line" do |
| 1367 | + @puppetd.command_line.stubs(:args).returns(%w{--logdest /my/file}) |
| 1368 | + |
| 1369 | + Puppet::Util::Log.expects(:newdestination).with("/my/file") |
| 1370 | + |
| 1371 | + @puppetd.parse_options |
| 1372 | + end |
| 1373 | + |
| 1374 | + it "should store the waitforcert options with --waitforcert" do |
| 1375 | + @puppetd.options.expects(:[]=).with(:waitforcert,42) |
| 1376 | + |
| 1377 | + @puppetd.handle_waitforcert("42") |
| 1378 | + end |
| 1379 | + |
| 1380 | + it "should set args[:Port] with --port" do |
| 1381 | + @puppetd.handle_port("42") |
| 1382 | + @puppetd.args[:Port].should == "42" |
| 1383 | + end |
| 1384 | + |
| 1385 | + end |
| 1386 | + |
| 1387 | + describe "during setup" do |
| 1388 | + before :each do |
| 1389 | + @puppetd.options.stubs(:[]) |
| 1390 | + Puppet.stubs(:info) |
| 1391 | + FileTest.stubs(:exists?).returns(true) |
| 1392 | + Puppet[:libdir] = "/dev/null/lib" |
| 1393 | + Puppet::SSL::Host.stubs(:ca_location=) |
| 1394 | + Puppet::Transaction::Report.indirection.stubs(:terminus_class=) |
| 1395 | + Puppet::Transaction::Report.indirection.stubs(:cache_class=) |
| 1396 | + Puppet::Resource::Catalog.indirection.stubs(:terminus_class=) |
| 1397 | + Puppet::Resource::Catalog.indirection.stubs(:cache_class=) |
| 1398 | + Puppet::Node::Facts.indirection.stubs(:terminus_class=) |
| 1399 | + @host = stub_everything 'host' |
| 1400 | + Puppet::SSL::Host.stubs(:new).returns(@host) |
| 1401 | + Puppet.stubs(:settraps) |
| 1402 | + end |
| 1403 | + |
| 1404 | + describe "with --test" do |
| 1405 | + before :each do |
| 1406 | + #Puppet.settings.stubs(:handlearg) |
| 1407 | + @puppetd.options.stubs(:[]=) |
| 1408 | + end |
| 1409 | + |
| 1410 | + it "should call setup_test" do |
| 1411 | + @puppetd.options.stubs(:[]).with(:test).returns(true) |
| 1412 | + @puppetd.expects(:setup_test) |
| 1413 | + @puppetd.setup |
| 1414 | + end |
| 1415 | + |
| 1416 | + it "should set options[:verbose] to true" do |
| 1417 | + @puppetd.options.expects(:[]=).with(:verbose,true) |
| 1418 | + @puppetd.setup_test |
| 1419 | + end |
| 1420 | + it "should set options[:onetime] to true" do |
| 1421 | + Puppet[:onetime] = false |
| 1422 | + @puppetd.setup_test |
| 1423 | + Puppet[:onetime].should == true |
| 1424 | + end |
| 1425 | + it "should set options[:detailed_exitcodes] to true" do |
| 1426 | + @puppetd.options.expects(:[]=).with(:detailed_exitcodes,true) |
| 1427 | + @puppetd.setup_test |
| 1428 | + end |
| 1429 | + end |
| 1430 | + |
| 1431 | + it "should call setup_logs" do |
| 1432 | + @puppetd.expects(:setup_logs) |
| 1433 | + @puppetd.setup |
| 1434 | + end |
| 1435 | + |
| 1436 | + describe "when setting up logs" do |
| 1437 | + before :each do |
| 1438 | + Puppet::Util::Log.stubs(:newdestination) |
| 1439 | + end |
| 1440 | + |
| 1441 | + it "should set log level to debug if --debug was passed" do |
| 1442 | + @puppetd.options.stubs(:[]).with(:debug).returns(true) |
| 1443 | + @puppetd.setup_logs |
| 1444 | + Puppet::Util::Log.level.should == :debug |
| 1445 | + end |
| 1446 | + |
| 1447 | + it "should set log level to info if --verbose was passed" do |
| 1448 | + @puppetd.options.stubs(:[]).with(:verbose).returns(true) |
| 1449 | + @puppetd.setup_logs |
| 1450 | + Puppet::Util::Log.level.should == :info |
| 1451 | + end |
| 1452 | + |
| 1453 | + [:verbose, :debug].each do |level| |
| 1454 | + it "should set console as the log destination with level #{level}" do |
| 1455 | + @puppetd.options.stubs(:[]).with(level).returns(true) |
| 1456 | + |
| 1457 | + Puppet::Util::Log.expects(:newdestination).with(:console) |
| 1458 | + |
| 1459 | + @puppetd.setup_logs |
| 1460 | + end |
| 1461 | + end |
| 1462 | + |
| 1463 | + it "should set syslog as the log destination if no --logdest" do |
| 1464 | + @puppetd.options.stubs(:[]).with(:setdest).returns(false) |
| 1465 | + |
| 1466 | + Puppet::Util::Log.expects(:newdestination).with(:syslog) |
| 1467 | + |
| 1468 | + @puppetd.setup_logs |
| 1469 | + end |
| 1470 | + |
| 1471 | + end |
| 1472 | + |
| 1473 | + it "should print puppet config if asked to in Puppet config" do |
| 1474 | + Puppet[:configprint] = "pluginsync" |
| 1475 | + Puppet.settings.expects(:print_configs).returns true |
| 1476 | + expect { @puppetd.setup }.to exit_with 0 |
| 1477 | + end |
| 1478 | + |
| 1479 | + it "should exit after printing puppet config if asked to in Puppet config" do |
| 1480 | + Puppet[:modulepath] = '/my/path' |
| 1481 | + Puppet[:configprint] = "modulepath" |
| 1482 | + Puppet::Util::Settings.any_instance.expects(:puts).with('/my/path') |
| 1483 | + expect { @puppetd.setup }.to exit_with 0 |
| 1484 | + end |
| 1485 | + |
| 1486 | + it "should set a central log destination with --centrallogs" do |
| 1487 | + @puppetd.options.stubs(:[]).with(:centrallogs).returns(true) |
| 1488 | + Puppet[:server] = "puppet.reductivelabs.com" |
| 1489 | + Puppet::Util::Log.stubs(:newdestination).with(:syslog) |
| 1490 | + |
| 1491 | + Puppet::Util::Log.expects(:newdestination).with("puppet.reductivelabs.com") |
| 1492 | + |
| 1493 | + @puppetd.setup |
| 1494 | + end |
| 1495 | + |
| 1496 | + it "should use :main, :puppetd, and :ssl" do |
| 1497 | + Puppet.settings.expects(:use).with(:main, :agent, :ssl) |
| 1498 | + |
| 1499 | + @puppetd.setup |
| 1500 | + end |
| 1501 | + |
| 1502 | + it "should install a remote ca location" do |
| 1503 | + Puppet::SSL::Host.expects(:ca_location=).with(:remote) |
| 1504 | + |
| 1505 | + @puppetd.setup |
| 1506 | + end |
| 1507 | + |
| 1508 | + it "should install a none ca location in fingerprint mode" do |
| 1509 | + @puppetd.options.stubs(:[]).with(:fingerprint).returns(true) |
| 1510 | + Puppet::SSL::Host.expects(:ca_location=).with(:none) |
| 1511 | + |
| 1512 | + @puppetd.setup |
| 1513 | + end |
| 1514 | + |
| 1515 | + it "should tell the report handler to use REST" do |
| 1516 | + Puppet::Transaction::Report.indirection.expects(:terminus_class=).with(:rest) |
| 1517 | + |
| 1518 | + @puppetd.setup |
| 1519 | + end |
| 1520 | + |
| 1521 | + it "should tell the report handler to cache locally as yaml" do |
| 1522 | + Puppet::Transaction::Report.indirection.expects(:cache_class=).with(:yaml) |
| 1523 | + |
| 1524 | + @puppetd.setup |
| 1525 | + end |
| 1526 | + |
| 1527 | + it "should change the catalog_terminus setting to 'rest'" do |
| 1528 | + Puppet[:catalog_terminus] = :foo |
| 1529 | + @puppetd.setup |
| 1530 | + Puppet[:catalog_terminus].should == :rest |
| 1531 | + end |
| 1532 | + |
| 1533 | + it "should tell the catalog handler to use cache" do |
| 1534 | + Puppet::Resource::Catalog.indirection.expects(:cache_class=).with(:yaml) |
| 1535 | + |
| 1536 | + @puppetd.setup |
| 1537 | + end |
| 1538 | + |
| 1539 | + it "should change the facts_terminus setting to 'facter'" do |
| 1540 | + Puppet[:facts_terminus] = :foo |
| 1541 | + |
| 1542 | + @puppetd.setup |
| 1543 | + Puppet[:facts_terminus].should == :facter |
| 1544 | + end |
| 1545 | + |
| 1546 | + it "should create an agent" do |
| 1547 | + Puppet::Agent.stubs(:new).with(Puppet::Configurer) |
| 1548 | + |
| 1549 | + @puppetd.setup |
| 1550 | + end |
| 1551 | + |
| 1552 | + [:enable, :disable].each do |action| |
| 1553 | + it "should delegate to enable_disable_client if we #{action} the agent" do |
| 1554 | + @puppetd.options.stubs(:[]).with(action).returns(true) |
| 1555 | + @puppetd.expects(:enable_disable_client).with(@agent) |
| 1556 | + |
| 1557 | + @puppetd.setup |
| 1558 | + end |
| 1559 | + end |
| 1560 | + |
| 1561 | + describe "when enabling or disabling agent" do |
| 1562 | + [:enable, :disable].each do |action| |
| 1563 | + it "should call client.#{action}" do |
| 1564 | + @puppetd.options.stubs(:[]).with(action).returns(true) |
| 1565 | + @agent.expects(action) |
| 1566 | + expect { @puppetd.enable_disable_client(@agent) }.to exit_with 0 |
| 1567 | + end |
| 1568 | + end |
| 1569 | + |
| 1570 | + it "should pass the disable message when disabling" do |
| 1571 | + @puppetd.options.stubs(:[]).with(:disable).returns(true) |
| 1572 | + @puppetd.options.stubs(:[]).with(:disable_message).returns("message") |
| 1573 | + @agent.expects(:disable).with("message") |
| 1574 | + expect { @puppetd.enable_disable_client(@agent) }.to exit_with 0 |
| 1575 | + end |
| 1576 | + |
| 1577 | + it "should pass the default disable message when disabling without a message" do |
| 1578 | + @puppetd.options.stubs(:[]).with(:disable).returns(true) |
| 1579 | + @puppetd.options.stubs(:[]).with(:disable_message).returns(nil) |
| 1580 | + @agent.expects(:disable).with("reason not specified") |
| 1581 | + expect { @puppetd.enable_disable_client(@agent) }.to exit_with 0 |
| 1582 | + end |
| 1583 | + |
| 1584 | + it "should finally exit" do |
| 1585 | + expect { @puppetd.enable_disable_client(@agent) }.to exit_with 0 |
| 1586 | + end |
| 1587 | + end |
| 1588 | + |
| 1589 | + it "should inform the daemon about our agent if :client is set to 'true'" do |
| 1590 | + @puppetd.options.expects(:[]).with(:client).returns true |
| 1591 | + @daemon.expects(:agent=).with(@agent) |
| 1592 | + @puppetd.setup |
| 1593 | + end |
| 1594 | + |
| 1595 | + it "should not inform the daemon about our agent if :client is set to 'false'" do |
| 1596 | + @puppetd.options[:client] = false |
| 1597 | + @daemon.expects(:agent=).never |
| 1598 | + @puppetd.setup |
| 1599 | + end |
| 1600 | + |
| 1601 | + it "should daemonize if needed" do |
| 1602 | + Puppet.features.stubs(:microsoft_windows?).returns false |
| 1603 | + Puppet[:daemonize] = true |
| 1604 | + |
| 1605 | + @daemon.expects(:daemonize) |
| 1606 | + |
| 1607 | + @puppetd.setup |
| 1608 | + end |
| 1609 | + |
| 1610 | + it "should wait for a certificate" do |
| 1611 | + @puppetd.options.stubs(:[]).with(:waitforcert).returns(123) |
| 1612 | + @host.expects(:wait_for_cert).with(123) |
| 1613 | + |
| 1614 | + @puppetd.setup |
| 1615 | + end |
| 1616 | + |
| 1617 | + it "should not wait for a certificate in fingerprint mode" do |
| 1618 | + @puppetd.options.stubs(:[]).with(:fingerprint).returns(true) |
| 1619 | + @puppetd.options.stubs(:[]).with(:waitforcert).returns(123) |
| 1620 | + @host.expects(:wait_for_cert).never |
| 1621 | + |
| 1622 | + @puppetd.setup |
| 1623 | + end |
| 1624 | + |
| 1625 | + it "should setup listen if told to and not onetime" do |
| 1626 | + Puppet[:listen] = true |
| 1627 | + @puppetd.options.stubs(:[]).with(:onetime).returns(false) |
| 1628 | + |
| 1629 | + @puppetd.expects(:setup_listen) |
| 1630 | + |
| 1631 | + @puppetd.setup |
| 1632 | + end |
| 1633 | + |
| 1634 | + describe "when setting up listen" do |
| 1635 | + before :each do |
| 1636 | + Puppet[:authconfig] = 'auth' |
| 1637 | + FileTest.stubs(:exists?).with('auth').returns(true) |
| 1638 | + File.stubs(:exist?).returns(true) |
| 1639 | + @puppetd.options.stubs(:[]).with(:serve).returns([]) |
| 1640 | + @server = stub_everything 'server' |
| 1641 | + Puppet::Network::Server.stubs(:new).returns(@server) |
| 1642 | + end |
| 1643 | + |
| 1644 | + |
| 1645 | + it "should exit if no authorization file" do |
| 1646 | + Puppet.stubs(:err) |
| 1647 | + FileTest.stubs(:exists?).with(Puppet[:rest_authconfig]).returns(false) |
| 1648 | + expect { @puppetd.setup_listen }.to exit_with 14 |
| 1649 | + end |
| 1650 | + |
| 1651 | + it "should create a server to listen on at least the Runner handler" do |
| 1652 | + Puppet::Network::Server.expects(:new).with { |args| args[:xmlrpc_handlers] == [:Runner] } |
| 1653 | + |
| 1654 | + @puppetd.setup_listen |
| 1655 | + end |
| 1656 | + |
| 1657 | + it "should create a server to listen for specific handlers" do |
| 1658 | + @puppetd.options.stubs(:[]).with(:serve).returns([:handler]) |
| 1659 | + Puppet::Network::Server.expects(:new).with { |args| args[:xmlrpc_handlers] == [:handler] } |
| 1660 | + |
| 1661 | + @puppetd.setup_listen |
| 1662 | + end |
| 1663 | + |
| 1664 | + it "should use puppet default port" do |
| 1665 | + Puppet[:puppetport] = 32768 |
| 1666 | + |
| 1667 | + Puppet::Network::Server.expects(:new).with { |args| args[:port] == 32768 } |
| 1668 | + |
| 1669 | + @puppetd.setup_listen |
| 1670 | + end |
| 1671 | + end |
| 1672 | + |
| 1673 | + describe "when setting up for fingerprint" do |
| 1674 | + before(:each) do |
| 1675 | + @puppetd.options.stubs(:[]).with(:fingerprint).returns(true) |
| 1676 | + end |
| 1677 | + |
| 1678 | + it "should not setup as an agent" do |
| 1679 | + @puppetd.expects(:setup_agent).never |
| 1680 | + @puppetd.setup |
| 1681 | + end |
| 1682 | + |
| 1683 | + it "should not create an agent" do |
| 1684 | + Puppet::Agent.stubs(:new).with(Puppet::Configurer).never |
| 1685 | + @puppetd.setup |
| 1686 | + end |
| 1687 | + |
| 1688 | + it "should not daemonize" do |
| 1689 | + @daemon.expects(:daemonize).never |
| 1690 | + @puppetd.setup |
| 1691 | + end |
| 1692 | + |
| 1693 | + it "should setup our certificate host" do |
| 1694 | + @puppetd.expects(:setup_host) |
| 1695 | + @puppetd.setup |
| 1696 | + end |
| 1697 | + end |
| 1698 | + end |
| 1699 | + |
| 1700 | + |
| 1701 | + describe "when running" do |
| 1702 | + before :each do |
| 1703 | + @puppetd.agent = @agent |
| 1704 | + @puppetd.daemon = @daemon |
| 1705 | + @puppetd.options.stubs(:[]).with(:fingerprint).returns(false) |
| 1706 | + end |
| 1707 | + |
| 1708 | + it "should dispatch to fingerprint if --fingerprint is used" do |
| 1709 | + @puppetd.options.stubs(:[]).with(:fingerprint).returns(true) |
| 1710 | + |
| 1711 | + @puppetd.stubs(:fingerprint) |
| 1712 | + @puppetd.run_command |
| 1713 | + end |
| 1714 | + |
| 1715 | + it "should dispatch to onetime if --onetime is used" do |
| 1716 | + @puppetd.options.stubs(:[]).with(:onetime).returns(true) |
| 1717 | + |
| 1718 | + @puppetd.stubs(:onetime) |
| 1719 | + @puppetd.run_command |
| 1720 | + end |
| 1721 | + |
| 1722 | + it "should dispatch to main if --onetime and --fingerprint are not used" do |
| 1723 | + @puppetd.options.stubs(:[]).with(:onetime).returns(false) |
| 1724 | + |
| 1725 | + @puppetd.stubs(:main) |
| 1726 | + @puppetd.run_command |
| 1727 | + end |
| 1728 | + |
| 1729 | + describe "with --onetime" do |
| 1730 | + |
| 1731 | + before :each do |
| 1732 | + @agent.stubs(:run).returns(:report) |
| 1733 | + @puppetd.options.stubs(:[]).with(:client).returns(:client) |
| 1734 | + @puppetd.options.stubs(:[]).with(:detailed_exitcodes).returns(false) |
| 1735 | + Puppet.stubs(:newservice) |
| 1736 | + end |
| 1737 | + |
| 1738 | + it "should exit if no defined --client" do |
| 1739 | + $stderr.stubs(:puts) |
| 1740 | + @puppetd.options.stubs(:[]).with(:client).returns(nil) |
| 1741 | + expect { @puppetd.onetime }.to exit_with 43 |
| 1742 | + end |
| 1743 | + |
| 1744 | + it "should setup traps" do |
| 1745 | + @daemon.expects(:set_signal_traps) |
| 1746 | + expect { @puppetd.onetime }.to exit_with 0 |
| 1747 | + end |
| 1748 | + |
| 1749 | + it "should let the agent run" do |
| 1750 | + @agent.expects(:run).returns(:report) |
| 1751 | + expect { @puppetd.onetime }.to exit_with 0 |
| 1752 | + end |
| 1753 | + |
| 1754 | + it "should finish by exiting with 0 error code" do |
| 1755 | + expect { @puppetd.onetime }.to exit_with 0 |
| 1756 | + end |
| 1757 | + |
| 1758 | + it "should stop the daemon" do |
| 1759 | + @daemon.expects(:stop).with(:exit => false) |
| 1760 | + expect { @puppetd.onetime }.to exit_with 0 |
| 1761 | + end |
| 1762 | + |
| 1763 | + describe "and --detailed-exitcodes" do |
| 1764 | + before :each do |
| 1765 | + @puppetd.options.stubs(:[]).with(:detailed_exitcodes).returns(true) |
| 1766 | + end |
| 1767 | + |
| 1768 | + it "should exit with report's computed exit status" do |
| 1769 | + Puppet[:noop] = false |
| 1770 | + report = stub 'report', :exit_status => 666 |
| 1771 | + @agent.stubs(:run).returns(report) |
| 1772 | + |
| 1773 | + expect { @puppetd.onetime }.to exit_with 666 |
| 1774 | + end |
| 1775 | + |
| 1776 | + it "should exit with the report's computer exit status, even if --noop is set." do |
| 1777 | + Puppet[:noop] = true |
| 1778 | + report = stub 'report', :exit_status => 666 |
| 1779 | + @agent.stubs(:run).returns(report) |
| 1780 | + |
| 1781 | + expect { @puppetd.onetime }.to exit_with 666 |
| 1782 | + end |
| 1783 | + end |
| 1784 | + end |
| 1785 | + |
| 1786 | + describe "with --fingerprint" do |
| 1787 | + before :each do |
| 1788 | + @cert = stub_everything 'cert' |
| 1789 | + @puppetd.options.stubs(:[]).with(:fingerprint).returns(true) |
| 1790 | + @puppetd.options.stubs(:[]).with(:digest).returns(:MD5) |
| 1791 | + @host = stub_everything 'host' |
| 1792 | + @puppetd.stubs(:host).returns(@host) |
| 1793 | + end |
| 1794 | + |
| 1795 | + it "should fingerprint the certificate if it exists" do |
| 1796 | + @host.expects(:certificate).returns(@cert) |
| 1797 | + @cert.expects(:fingerprint).with(:MD5).returns "fingerprint" |
| 1798 | + @puppetd.fingerprint |
| 1799 | + end |
| 1800 | + |
| 1801 | + it "should fingerprint the certificate request if no certificate have been signed" do |
| 1802 | + @host.expects(:certificate).returns(nil) |
| 1803 | + @host.expects(:certificate_request).returns(@cert) |
| 1804 | + @cert.expects(:fingerprint).with(:MD5).returns "fingerprint" |
| 1805 | + @puppetd.fingerprint |
| 1806 | + end |
| 1807 | + |
| 1808 | + it "should display the fingerprint" do |
| 1809 | + @host.stubs(:certificate).returns(@cert) |
| 1810 | + @cert.stubs(:fingerprint).with(:MD5).returns("DIGEST") |
| 1811 | + |
| 1812 | + @puppetd.expects(:puts).with "DIGEST" |
| 1813 | + |
| 1814 | + @puppetd.fingerprint |
| 1815 | + end |
| 1816 | + end |
| 1817 | + |
| 1818 | + describe "without --onetime and --fingerprint" do |
| 1819 | + before :each do |
| 1820 | + Puppet.stubs(:notice) |
| 1821 | + @puppetd.options.stubs(:[]).with(:client) |
| 1822 | + end |
| 1823 | + |
| 1824 | + it "should start our daemon" do |
| 1825 | + @daemon.expects(:start) |
| 1826 | + |
| 1827 | + @puppetd.main |
| 1828 | + end |
| 1829 | + end |
| 1830 | + end |
| 1831 | +end |
| 1832 | |
| 1833 | === added directory '.pc/puppet-12844/spec/unit/util' |
| 1834 | === added file '.pc/puppet-12844/spec/unit/util/anonymous_filelock_spec.rb' |
| 1835 | --- .pc/puppet-12844/spec/unit/util/anonymous_filelock_spec.rb 1970-01-01 00:00:00 +0000 |
| 1836 | +++ .pc/puppet-12844/spec/unit/util/anonymous_filelock_spec.rb 2012-03-16 16:38:20 +0000 |
| 1837 | @@ -0,0 +1,78 @@ |
| 1838 | +#!/usr/bin/env rspec |
| 1839 | +require 'spec_helper' |
| 1840 | + |
| 1841 | +require 'puppet/util/anonymous_filelock' |
| 1842 | + |
| 1843 | +describe Puppet::Util::AnonymousFilelock do |
| 1844 | + require 'puppet_spec/files' |
| 1845 | + include PuppetSpec::Files |
| 1846 | + |
| 1847 | + before(:each) do |
| 1848 | + @lockfile = tmpfile("lock") |
| 1849 | + @lock = Puppet::Util::AnonymousFilelock.new(@lockfile) |
| 1850 | + end |
| 1851 | + |
| 1852 | + it "should be anonymous" do |
| 1853 | + @lock.should be_anonymous |
| 1854 | + end |
| 1855 | + |
| 1856 | + describe "#lock" do |
| 1857 | + it "should return false if already locked" do |
| 1858 | + @lock.stubs(:locked?).returns(true) |
| 1859 | + @lock.lock.should be_false |
| 1860 | + end |
| 1861 | + |
| 1862 | + it "should return true if it successfully locked" do |
| 1863 | + @lock.lock.should be_true |
| 1864 | + end |
| 1865 | + |
| 1866 | + it "should create a lock file" do |
| 1867 | + @lock.lock |
| 1868 | + |
| 1869 | + File.should be_exists(@lockfile) |
| 1870 | + end |
| 1871 | + |
| 1872 | + it "should create a lock file containing a message" do |
| 1873 | + @lock.lock("message") |
| 1874 | + |
| 1875 | + File.read(@lockfile).should == "message" |
| 1876 | + end |
| 1877 | + end |
| 1878 | + |
| 1879 | + describe "#unlock" do |
| 1880 | + it "should return true when unlocking" do |
| 1881 | + @lock.lock |
| 1882 | + @lock.unlock.should be_true |
| 1883 | + end |
| 1884 | + |
| 1885 | + it "should return false when not locked" do |
| 1886 | + @lock.unlock.should be_false |
| 1887 | + end |
| 1888 | + |
| 1889 | + it "should clear the lock file" do |
| 1890 | + File.open(@lockfile, 'w') { |fd| fd.print("locked") } |
| 1891 | + @lock.unlock |
| 1892 | + File.should_not be_exists(@lockfile) |
| 1893 | + end |
| 1894 | + end |
| 1895 | + |
| 1896 | + it "should be locked when locked" do |
| 1897 | + @lock.lock |
| 1898 | + @lock.should be_locked |
| 1899 | + end |
| 1900 | + |
| 1901 | + it "should not be locked when not locked" do |
| 1902 | + @lock.should_not be_locked |
| 1903 | + end |
| 1904 | + |
| 1905 | + it "should not be locked when unlocked" do |
| 1906 | + @lock.lock |
| 1907 | + @lock.unlock |
| 1908 | + @lock.should_not be_locked |
| 1909 | + end |
| 1910 | + |
| 1911 | + it "should return the lock message" do |
| 1912 | + @lock.lock("lock message") |
| 1913 | + @lock.message.should == "lock message" |
| 1914 | + end |
| 1915 | +end |
| 1916 | \ No newline at end of file |
| 1917 | |
| 1918 | === added file '.pc/puppet-12844/spec/unit/util/pidlock_spec.rb' |
| 1919 | --- .pc/puppet-12844/spec/unit/util/pidlock_spec.rb 1970-01-01 00:00:00 +0000 |
| 1920 | +++ .pc/puppet-12844/spec/unit/util/pidlock_spec.rb 2012-03-16 16:38:20 +0000 |
| 1921 | @@ -0,0 +1,208 @@ |
| 1922 | +#!/usr/bin/env rspec |
| 1923 | +require 'spec_helper' |
| 1924 | + |
| 1925 | +require 'puppet/util/pidlock' |
| 1926 | + |
| 1927 | +describe Puppet::Util::Pidlock do |
| 1928 | + require 'puppet_spec/files' |
| 1929 | + include PuppetSpec::Files |
| 1930 | + |
| 1931 | + before(:each) do |
| 1932 | + @lockfile = tmpfile("lock") |
| 1933 | + @lock = Puppet::Util::Pidlock.new(@lockfile) |
| 1934 | + end |
| 1935 | + |
| 1936 | + it "should not be anonymous" do |
| 1937 | + @lock.should_not be_anonymous |
| 1938 | + end |
| 1939 | + |
| 1940 | + describe "#lock" do |
| 1941 | + it "should not be locked at start" do |
| 1942 | + @lock.should_not be_locked |
| 1943 | + end |
| 1944 | + |
| 1945 | + it "should not be mine at start" do |
| 1946 | + @lock.should_not be_mine |
| 1947 | + end |
| 1948 | + |
| 1949 | + it "should become locked" do |
| 1950 | + @lock.lock |
| 1951 | + @lock.should be_locked |
| 1952 | + end |
| 1953 | + |
| 1954 | + it "should become mine" do |
| 1955 | + @lock.lock |
| 1956 | + @lock.should be_mine |
| 1957 | + end |
| 1958 | + |
| 1959 | + it "should be possible to lock multiple times" do |
| 1960 | + @lock.lock |
| 1961 | + lambda { @lock.lock }.should_not raise_error |
| 1962 | + end |
| 1963 | + |
| 1964 | + it "should return true when locking" do |
| 1965 | + @lock.lock.should be_true |
| 1966 | + end |
| 1967 | + |
| 1968 | + it "should return true if locked by me" do |
| 1969 | + @lock.lock |
| 1970 | + @lock.lock.should be_true |
| 1971 | + end |
| 1972 | + |
| 1973 | + it "should return false if locked by someone else" do |
| 1974 | + Process.stubs(:kill) |
| 1975 | + File.open(@lockfile, "w") { |fd| fd.print('0') } |
| 1976 | + |
| 1977 | + @lock.lock.should be_false |
| 1978 | + end |
| 1979 | + |
| 1980 | + it "should create a lock file" do |
| 1981 | + @lock.lock |
| 1982 | + File.should be_exists(@lockfile) |
| 1983 | + end |
| 1984 | + |
| 1985 | + it "should create a lock file containing our pid" do |
| 1986 | + @lock.lock |
| 1987 | + File.read(@lockfile).to_i.should == Process.pid.to_i |
| 1988 | + end |
| 1989 | + end |
| 1990 | + |
| 1991 | + describe "#unlock" do |
| 1992 | + it "should not be locked anymore" do |
| 1993 | + @lock.lock |
| 1994 | + @lock.unlock |
| 1995 | + @lock.should_not be_locked |
| 1996 | + end |
| 1997 | + |
| 1998 | + it "should return false if not locked" do |
| 1999 | + @lock.unlock.should be_false |
| 2000 | + end |
| 2001 | + |
| 2002 | + it "should return true if properly unlocked" do |
| 2003 | + @lock.lock |
| 2004 | + @lock.unlock.should be_true |
| 2005 | + end |
| 2006 | + |
| 2007 | + it "should get rid of the lock file" do |
| 2008 | + @lock.lock |
| 2009 | + @lock.unlock |
| 2010 | + File.should_not be_exists(@lockfile) |
| 2011 | + end |
| 2012 | + |
| 2013 | + it "should not warn if the lockfile was deleted by someone else" do |
| 2014 | + @lock.lock |
| 2015 | + File.unlink(@lockfile) |
| 2016 | + |
| 2017 | + Puppet.expects(:err).never # meh |
| 2018 | + @lock.unlock |
| 2019 | + end |
| 2020 | + |
| 2021 | + it "should warn if the lockfile can't be deleted" do |
| 2022 | + @lock.lock |
| 2023 | + File.expects(:unlink).with(@lockfile).raises(Errno::EIO) |
| 2024 | + Puppet.expects(:err).with do |argument| |
| 2025 | + argument.should =~ /Input\/output error/ |
| 2026 | + end |
| 2027 | + @lock.unlock |
| 2028 | + |
| 2029 | + # This is necessary because our cleanup code uses File.unlink |
| 2030 | + File.unstub(:unlink) |
| 2031 | + @lock.unlock |
| 2032 | + end |
| 2033 | + end |
| 2034 | + |
| 2035 | + describe "#locked?" do |
| 2036 | + it "should return true if locked" do |
| 2037 | + @lock.lock |
| 2038 | + @lock.should be_locked |
| 2039 | + end |
| 2040 | + end |
| 2041 | + |
| 2042 | + describe "with a stale lock" do |
| 2043 | + before(:each) do |
| 2044 | + Process.stubs(:kill).with(0, 6789) |
| 2045 | + Process.stubs(:kill).with(0, 1234).raises(Errno::ESRCH) |
| 2046 | + Process.stubs(:pid).returns(6789) |
| 2047 | + File.open(@lockfile, 'w') { |fd| fd.write("1234") } |
| 2048 | + end |
| 2049 | + |
| 2050 | + it "should not be locked" do |
| 2051 | + @lock.should_not be_locked |
| 2052 | + end |
| 2053 | + |
| 2054 | + describe "#lock" do |
| 2055 | + it "should clear stale locks" do |
| 2056 | + @lock.locked? |
| 2057 | + File.should_not be_exists(@lockfile) |
| 2058 | + end |
| 2059 | + |
| 2060 | + it "should replace with new locks" do |
| 2061 | + @lock.lock |
| 2062 | + File.should be_exists(@lockfile) |
| 2063 | + @lock.lock_pid.should == 6789 |
| 2064 | + @lock.should be_mine |
| 2065 | + @lock.should be_locked |
| 2066 | + end |
| 2067 | + end |
| 2068 | + |
| 2069 | + describe "#unlock" do |
| 2070 | + it "should not be allowed" do |
| 2071 | + @lock.unlock.should be_false |
| 2072 | + end |
| 2073 | + |
| 2074 | + it "should not remove the lock file" do |
| 2075 | + @lock.unlock |
| 2076 | + File.should be_exists(@lockfile) |
| 2077 | + end |
| 2078 | + end |
| 2079 | + end |
| 2080 | + |
| 2081 | + describe "with another process lock" do |
| 2082 | + before(:each) do |
| 2083 | + Process.stubs(:kill).with(0, 6789) |
| 2084 | + Process.stubs(:kill).with(0, 1234) |
| 2085 | + Process.stubs(:pid).returns(6789) |
| 2086 | + File.open(@lockfile, 'w') { |fd| fd.write("1234") } |
| 2087 | + end |
| 2088 | + |
| 2089 | + it "should be locked" do |
| 2090 | + @lock.should be_locked |
| 2091 | + end |
| 2092 | + |
| 2093 | + it "should not be mine" do |
| 2094 | + @lock.should_not be_mine |
| 2095 | + end |
| 2096 | + |
| 2097 | + describe "#lock" do |
| 2098 | + it "should not be possible" do |
| 2099 | + @lock.lock.should be_false |
| 2100 | + end |
| 2101 | + |
| 2102 | + it "should not overwrite the lock" do |
| 2103 | + @lock.lock |
| 2104 | + @lock.should_not be_mine |
| 2105 | + end |
| 2106 | + end |
| 2107 | + |
| 2108 | + describe "#unlock" do |
| 2109 | + it "should not be possible" do |
| 2110 | + @lock.unlock.should be_false |
| 2111 | + end |
| 2112 | + |
| 2113 | + it "should not remove the lock file" do |
| 2114 | + @lock.unlock |
| 2115 | + File.should be_exists(@lockfile) |
| 2116 | + end |
| 2117 | + |
| 2118 | + it "should still not be our lock" do |
| 2119 | + @lock.unlock |
| 2120 | + @lock.should_not be_mine |
| 2121 | + end |
| 2122 | + |
| 2123 | + it "should not warn" do |
| 2124 | + Puppet.expects(:err).never |
| 2125 | + @lock.unlock |
| 2126 | + end |
| 2127 | + end |
| 2128 | + end |
| 2129 | +end |
| 2130 | |
| 2131 | === added directory '.pc/puppet-12844/test' |
| 2132 | === added directory '.pc/puppet-12844/test/util' |
| 2133 | === added file '.pc/puppet-12844/test/util/pidlock.rb' |
| 2134 | === modified file 'debian/changelog' |
| 2135 | --- debian/changelog 2012-02-23 18:24:48 +0000 |
| 2136 | +++ debian/changelog 2012-03-16 16:38:20 +0000 |
| 2137 | @@ -1,3 +1,16 @@ |
| 2138 | +puppet (2.7.11-1ubuntu1) precise; urgency=low |
| 2139 | + |
| 2140 | + * Added patch to solve puppetlabs bug #12844 |
| 2141 | + - Reverts behaviour of agent lockfiles |
| 2142 | + - Fixes unit test to readmit old behaviour |
| 2143 | + * debian/puppetmaster-passenger.postinst (LP: #948983) |
| 2144 | + - Fixed rack directory location |
| 2145 | + - Added proper enabling of apache2 headers mod |
| 2146 | + * debian/puppetmaster-passenger.postinst (LP: #950183) |
| 2147 | + - Make sure we error if puppet config print doesn't work |
| 2148 | + |
| 2149 | + -- Marc Cluet <marc.cluet@ubuntu.com> Fri, 16 Mar 2012 15:36:35 +0000 |
| 2150 | + |
| 2151 | puppet (2.7.11-1) unstable; urgency=high |
| 2152 | |
| 2153 | * New upstream release |
| 2154 | |
| 2155 | === added file 'debian/patches/puppet-12844' |
| 2156 | --- debian/patches/puppet-12844 1970-01-01 00:00:00 +0000 |
| 2157 | +++ debian/patches/puppet-12844 2012-03-16 16:38:20 +0000 |
| 2158 | @@ -0,0 +1,979 @@ |
| 2159 | +Description: Reverts lockfile behaviour to previous model |
| 2160 | + This reverts commit fcac8f7163c99884fc6b75e3851c4a5e16a3ff07, which |
| 2161 | + was a backwards-compatibility workaround intended to handle some new |
| 2162 | + behavior related to puppet agent lockfiles that was introduced in |
| 2163 | + 2.7.10. The fix is being reverted because we've decided to remove |
| 2164 | + the new lockfile behavior from the 2.7.x series entirely, and push |
| 2165 | + it out to 3.x. |
| 2166 | +Author: cprice <chris@puppetlabs.com> |
| 2167 | +Origin: upstream |
| 2168 | +Bug: http://projects.puppetlabs.com/issues/12844 |
| 2169 | +Forwarded: not-needed |
| 2170 | +Reviewed-By: Marc Cluet <marc.cluet@ubuntu.com> |
| 2171 | + |
| 2172 | +Index: puppet-new-bzr/lib/puppet/agent.rb |
| 2173 | +=================================================================== |
| 2174 | +--- puppet-new-bzr.orig/lib/puppet/agent.rb 2012-03-16 15:14:51.672976000 +0000 |
| 2175 | ++++ puppet-new-bzr/lib/puppet/agent.rb 2012-03-16 15:15:47.974498349 +0000 |
| 2176 | +@@ -8,9 +8,6 @@ |
| 2177 | + require 'puppet/agent/locker' |
| 2178 | + include Puppet::Agent::Locker |
| 2179 | + |
| 2180 | +- require 'puppet/agent/disabler' |
| 2181 | +- include Puppet::Agent::Disabler |
| 2182 | +- |
| 2183 | + attr_reader :client_class, :client, :splayed |
| 2184 | + |
| 2185 | + # Just so we can specify that we are "the" instance. |
| 2186 | +@@ -35,9 +32,10 @@ |
| 2187 | + return |
| 2188 | + end |
| 2189 | + if disabled? |
| 2190 | +- Puppet.notice "Skipping run of #{client_class}; administratively disabled: #{disable_message}" |
| 2191 | ++ Puppet.notice "Skipping run of #{client_class}; administratively disabled; use 'puppet #{client_class} --enable' to re-enable." |
| 2192 | + return |
| 2193 | + end |
| 2194 | ++ |
| 2195 | + result = nil |
| 2196 | + block_run = Puppet::Application.controlled_run do |
| 2197 | + splay |
| 2198 | +Index: puppet-new-bzr/lib/puppet/agent/locker.rb |
| 2199 | +=================================================================== |
| 2200 | +--- puppet-new-bzr.orig/lib/puppet/agent/locker.rb 2012-03-16 15:14:51.672976000 +0000 |
| 2201 | ++++ puppet-new-bzr/lib/puppet/agent/locker.rb 2012-03-16 15:15:47.994506350 +0000 |
| 2202 | +@@ -3,6 +3,16 @@ |
| 2203 | + # Break out the code related to locking the agent. This module is just |
| 2204 | + # included into the agent, but having it here makes it easier to test. |
| 2205 | + module Puppet::Agent::Locker |
| 2206 | ++ # Let the daemon run again, freely in the filesystem. |
| 2207 | ++ def enable |
| 2208 | ++ lockfile.unlock(:anonymous => true) |
| 2209 | ++ end |
| 2210 | ++ |
| 2211 | ++ # Stop the daemon from making any catalog runs. |
| 2212 | ++ def disable |
| 2213 | ++ lockfile.lock(:anonymous => true) |
| 2214 | ++ end |
| 2215 | ++ |
| 2216 | + # Yield if we get a lock, else do nothing. Return |
| 2217 | + # true/false depending on whether we get the lock. |
| 2218 | + def lock |
| 2219 | +@@ -25,6 +35,10 @@ |
| 2220 | + end |
| 2221 | + |
| 2222 | + def running? |
| 2223 | +- lockfile.locked? |
| 2224 | ++ lockfile.locked? and !lockfile.anonymous? |
| 2225 | ++ end |
| 2226 | ++ |
| 2227 | ++ def disabled? |
| 2228 | ++ lockfile.locked? and lockfile.anonymous? |
| 2229 | + end |
| 2230 | + end |
| 2231 | +Index: puppet-new-bzr/lib/puppet/application/agent.rb |
| 2232 | +=================================================================== |
| 2233 | +--- puppet-new-bzr.orig/lib/puppet/application/agent.rb 2012-03-16 15:14:51.672976000 +0000 |
| 2234 | ++++ puppet-new-bzr/lib/puppet/application/agent.rb 2012-03-16 15:15:47.998507951 +0000 |
| 2235 | +@@ -39,12 +39,7 @@ |
| 2236 | + end |
| 2237 | + |
| 2238 | + option("--centrallogging") |
| 2239 | +- |
| 2240 | +- option("--disable [MESSAGE]") do |message| |
| 2241 | +- options[:disable] = true |
| 2242 | +- options[:disable_message] = message |
| 2243 | +- end |
| 2244 | +- |
| 2245 | ++ option("--disable") |
| 2246 | + option("--enable") |
| 2247 | + option("--debug","-d") |
| 2248 | + option("--fqdn FQDN","-f") |
| 2249 | +@@ -106,7 +101,7 @@ |
| 2250 | + USAGE |
| 2251 | + ----- |
| 2252 | + puppet agent [--certname <name>] [-D|--daemonize|--no-daemonize] |
| 2253 | +- [-d|--debug] [--detailed-exitcodes] [--digest <digest>] [--disable [message]] [--enable] |
| 2254 | ++ [-d|--debug] [--detailed-exitcodes] [--digest <digest>] [--disable] [--enable] |
| 2255 | + [--fingerprint] [-h|--help] [-l|--logdest syslog|<file>|console] |
| 2256 | + [--no-client] [--noop] [-o|--onetime] [--serve <handler>] [-t|--test] |
| 2257 | + [-v|--verbose] [-V|--version] [-w|--waitforcert <seconds>] |
| 2258 | +@@ -210,9 +205,6 @@ |
| 2259 | + not want the central configuration to override the local state until |
| 2260 | + everything is tested and committed. |
| 2261 | + |
| 2262 | +- Disable can also take an optional message that will be reported by the |
| 2263 | +- 'puppet agent' at the next disabled run. |
| 2264 | +- |
| 2265 | + 'puppet agent' uses the same lock file while it is running, so no more |
| 2266 | + than one 'puppet agent' process is working at a time. |
| 2267 | + |
| 2268 | +@@ -394,7 +386,7 @@ |
| 2269 | + if options[:enable] |
| 2270 | + agent.enable |
| 2271 | + elsif options[:disable] |
| 2272 | +- agent.disable(options[:disable_message] || 'reason not specified') |
| 2273 | ++ agent.disable |
| 2274 | + end |
| 2275 | + exit(0) |
| 2276 | + end |
| 2277 | +Index: puppet-new-bzr/lib/puppet/util/anonymous_filelock.rb |
| 2278 | +=================================================================== |
| 2279 | +--- puppet-new-bzr.orig/lib/puppet/util/anonymous_filelock.rb 2012-03-16 15:14:51.672976000 +0000 |
| 2280 | ++++ /dev/null 1970-01-01 00:00:00.000000000 +0000 |
| 2281 | +@@ -1,36 +0,0 @@ |
| 2282 | +- |
| 2283 | +-class Puppet::Util::AnonymousFilelock |
| 2284 | +- attr_reader :lockfile |
| 2285 | +- |
| 2286 | +- def initialize(lockfile) |
| 2287 | +- @lockfile = lockfile |
| 2288 | +- end |
| 2289 | +- |
| 2290 | +- def anonymous? |
| 2291 | +- true |
| 2292 | +- end |
| 2293 | +- |
| 2294 | +- def lock(msg = '') |
| 2295 | +- return false if locked? |
| 2296 | +- |
| 2297 | +- File.open(@lockfile, 'w') { |fd| fd.print(msg) } |
| 2298 | +- true |
| 2299 | +- end |
| 2300 | +- |
| 2301 | +- def unlock |
| 2302 | +- if locked? |
| 2303 | +- File.unlink(@lockfile) |
| 2304 | +- true |
| 2305 | +- else |
| 2306 | +- false |
| 2307 | +- end |
| 2308 | +- end |
| 2309 | +- |
| 2310 | +- def locked? |
| 2311 | +- File.exists? @lockfile |
| 2312 | +- end |
| 2313 | +- |
| 2314 | +- def message |
| 2315 | +- return File.read(@lockfile) if locked? |
| 2316 | +- end |
| 2317 | +-end |
| 2318 | +\ No newline at end of file |
| 2319 | +Index: puppet-new-bzr/lib/puppet/util/pidlock.rb |
| 2320 | +=================================================================== |
| 2321 | +--- puppet-new-bzr.orig/lib/puppet/util/pidlock.rb 2012-03-16 15:14:51.672976000 +0000 |
| 2322 | ++++ puppet-new-bzr/lib/puppet/util/pidlock.rb 2012-03-16 15:15:47.998507951 +0000 |
| 2323 | +@@ -1,10 +1,20 @@ |
| 2324 | + require 'fileutils' |
| 2325 | +-require 'puppet/util/anonymous_filelock' |
| 2326 | + |
| 2327 | +-class Puppet::Util::Pidlock < Puppet::Util::AnonymousFilelock |
| 2328 | ++class Puppet::Util::Pidlock |
| 2329 | ++ attr_reader :lockfile |
| 2330 | ++ |
| 2331 | ++ def initialize(lockfile) |
| 2332 | ++ @lockfile = lockfile |
| 2333 | ++ end |
| 2334 | + |
| 2335 | + def locked? |
| 2336 | + clear_if_stale |
| 2337 | ++ return true if File.exists? @lockfile |
| 2338 | ++ |
| 2339 | ++ # HACK! There was a temporary change to the lockfile behavior introduced in 2.7.10 and 2.7.11, and reverted |
| 2340 | ++ # in 2.7.12. We need to pull some chicanery to be backwards-compatible with those versions. For more info, |
| 2341 | ++ # see the comments on the method... and this hack should be removed for the 3.x series. |
| 2342 | ++ handle_2_7_10_disabled_lockfile |
| 2343 | + File.exists? @lockfile |
| 2344 | + end |
| 2345 | + |
| 2346 | +@@ -13,36 +23,39 @@ |
| 2347 | + end |
| 2348 | + |
| 2349 | + def anonymous? |
| 2350 | +- false |
| 2351 | ++ return false unless File.exists?(@lockfile) |
| 2352 | ++ File.read(@lockfile) == "" |
| 2353 | + end |
| 2354 | + |
| 2355 | +- def lock |
| 2356 | +- return mine? if locked? |
| 2357 | ++ def lock(opts = {}) |
| 2358 | ++ opts = {:anonymous => false}.merge(opts) |
| 2359 | + |
| 2360 | +- File.open(@lockfile, "w") { |fd| fd.write(Process.pid) } |
| 2361 | +- true |
| 2362 | ++ if locked? |
| 2363 | ++ mine? |
| 2364 | ++ else |
| 2365 | ++ if opts[:anonymous] |
| 2366 | ++ File.open(@lockfile, 'w') { |fd| true } |
| 2367 | ++ else |
| 2368 | ++ File.open(@lockfile, "w") { |fd| fd.write(Process.pid) } |
| 2369 | ++ end |
| 2370 | ++ true |
| 2371 | ++ end |
| 2372 | + end |
| 2373 | + |
| 2374 | + def unlock(opts = {}) |
| 2375 | +- if mine? |
| 2376 | +- begin |
| 2377 | +- File.unlink(@lockfile) |
| 2378 | +- rescue Errno::ENOENT |
| 2379 | +- # Someone deleted it for us ...and so we do nothing. No point whining |
| 2380 | +- # about a problem that the user can't actually do anything about. |
| 2381 | +- rescue SystemCallError => e |
| 2382 | +- # This one is a real failure though. No idea what went wrong, but it |
| 2383 | +- # is most likely "read only file(system)" or wrong permissions or |
| 2384 | +- # something like that. |
| 2385 | +- Puppet.err "Could not remove PID file #{@lockfile}: #{e}" |
| 2386 | +- puts e.backtrace if Puppet[:trace] |
| 2387 | +- end |
| 2388 | ++ return false unless locked? |
| 2389 | ++ |
| 2390 | ++ opts = {:anonymous => false}.merge(opts) |
| 2391 | ++ |
| 2392 | ++ if mine? or (opts[:anonymous] and anonymous?) |
| 2393 | ++ File.unlink(@lockfile) |
| 2394 | + true |
| 2395 | + else |
| 2396 | + false |
| 2397 | + end |
| 2398 | + end |
| 2399 | + |
| 2400 | ++ private |
| 2401 | + def lock_pid |
| 2402 | + if File.exists? @lockfile |
| 2403 | + File.read(@lockfile).to_i |
| 2404 | +@@ -51,7 +64,6 @@ |
| 2405 | + end |
| 2406 | + end |
| 2407 | + |
| 2408 | +- private |
| 2409 | + def clear_if_stale |
| 2410 | + return if lock_pid.nil? |
| 2411 | + |
| 2412 | +@@ -65,4 +77,41 @@ |
| 2413 | + File.unlink(@lockfile) |
| 2414 | + end |
| 2415 | + end |
| 2416 | ++ |
| 2417 | ++ |
| 2418 | ++ ###################################################################################### |
| 2419 | ++ # Backwards compatibility hack |
| 2420 | ++ ###################################################################################### |
| 2421 | ++ # A change to lockfile behavior was introduced in 2.7.10 and 2.7.11; basically, |
| 2422 | ++ # instead of using a single lockfile to indicate both administrative disabling of |
| 2423 | ++ # the agent *and* the case where an agent run is already in progress, we started using |
| 2424 | ++ # two separate lockfiles: the 'normal' one for the "run in progress" case, and a |
| 2425 | ++ # separate one with a ".disabled" extension to indicate administrative disabling. |
| 2426 | ++ # |
| 2427 | ++ # This was determined to cause incompatibilities with mcollective, so the behavior |
| 2428 | ++ # was reverted for 2.7.12. Unfortunately this leaves the possibility that someone |
| 2429 | ++ # may have run "agent --disable" to administratively disable a 2.7.10 or 2.7.11 |
| 2430 | ++ # agent, and then upgraded to a newer version. This method exists only to |
| 2431 | ++ # provide backwards compatibility. Basically, it just recognizes the 2.7.10/2.7.11 |
| 2432 | ++ # ".disabled" lock file, warns, and cleans it up. |
| 2433 | ++ # |
| 2434 | ++ # This should be removed for the 3.x series. |
| 2435 | ++ # |
| 2436 | ++ # For more information, please see tickets #12844, #3757, #4836, and #11057 |
| 2437 | ++ # |
| 2438 | ++ # -- cprice 2012-03-01 |
| 2439 | ++ # |
| 2440 | ++ def handle_2_7_10_disabled_lockfile |
| 2441 | ++ disabled_lockfile_path = @lockfile + ".disabled" |
| 2442 | ++ if (File.exists?(disabled_lockfile_path)) |
| 2443 | ++ Puppet.warning("Found special lockfile '#{disabled_lockfile_path}'; this file was " + |
| 2444 | ++ "generated by a call to 'puppet agent --disable' in puppet 2.7.10 or 2.7.11. " + |
| 2445 | ++ "The expected lockfile path is '#{@lockfile}'; renaming the lock file.") |
| 2446 | ++ File.rename(disabled_lockfile_path, @lockfile) |
| 2447 | ++ end |
| 2448 | ++ end |
| 2449 | ++ private :handle_2_7_10_disabled_lockfile |
| 2450 | ++ ###################################################################################### |
| 2451 | ++ # End backwards compatibility hack |
| 2452 | ++ ###################################################################################### |
| 2453 | + end |
| 2454 | +Index: puppet-new-bzr/spec/unit/agent/locker_spec.rb |
| 2455 | +=================================================================== |
| 2456 | +--- puppet-new-bzr.orig/spec/unit/agent/locker_spec.rb 2012-03-16 15:14:51.672976000 +0000 |
| 2457 | ++++ puppet-new-bzr/spec/unit/agent/locker_spec.rb 2012-03-16 15:15:47.998507951 +0000 |
| 2458 | +@@ -29,6 +29,18 @@ |
| 2459 | + @locker.lockfile.should equal(@locker.lockfile) |
| 2460 | + end |
| 2461 | + |
| 2462 | ++ it "should use the lock file to anonymously lock the process when disabled" do |
| 2463 | ++ @locker.lockfile.expects(:lock).with(:anonymous => true) |
| 2464 | ++ |
| 2465 | ++ @locker.disable |
| 2466 | ++ end |
| 2467 | ++ |
| 2468 | ++ it "should use the lock file to anonymously unlock the process when enabled" do |
| 2469 | ++ @locker.lockfile.expects(:unlock).with(:anonymous => true) |
| 2470 | ++ |
| 2471 | ++ @locker.enable |
| 2472 | ++ end |
| 2473 | ++ |
| 2474 | + it "should have a method that yields when a lock is attained" do |
| 2475 | + @locker.lockfile.expects(:lock).returns true |
| 2476 | + |
| 2477 | +Index: puppet-new-bzr/spec/unit/agent_backward_compatibility_spec.rb |
| 2478 | +=================================================================== |
| 2479 | +--- /dev/null 1970-01-01 00:00:00.000000000 +0000 |
| 2480 | ++++ puppet-new-bzr/spec/unit/agent_backward_compatibility_spec.rb 2012-03-16 15:15:48.002509551 +0000 |
| 2481 | +@@ -0,0 +1,152 @@ |
| 2482 | ++#!/usr/bin/env rspec |
| 2483 | ++require 'spec_helper' |
| 2484 | ++require 'puppet/agent' |
| 2485 | ++ |
| 2486 | ++ |
| 2487 | ++############################################################################ |
| 2488 | ++# NOTE # |
| 2489 | ++############################################################################ |
| 2490 | ++# # |
| 2491 | ++# This entire spec is only here for backwards compatibility from 2.7.12+ # |
| 2492 | ++# with 2.7.10 and 2.7.11. The entire file should be able to be removed # |
| 2493 | ++# for the 3.x series. # |
| 2494 | ++# # |
| 2495 | ++# For more info, see the comments on the #handle_2_7_10_disabled_lockfile # |
| 2496 | ++# method in pidlock.rb # |
| 2497 | ++# # |
| 2498 | ++# --cprice 2012-03-01 # |
| 2499 | ++############################################################################ |
| 2500 | ++ |
| 2501 | ++class AgentTestClient |
| 2502 | ++ def run |
| 2503 | ++ # no-op |
| 2504 | ++ end |
| 2505 | ++ def stop |
| 2506 | ++ # no-op |
| 2507 | ++ end |
| 2508 | ++end |
| 2509 | ++ |
| 2510 | ++describe Puppet::Agent do |
| 2511 | ++ include PuppetSpec::Files |
| 2512 | ++ |
| 2513 | ++ let(:agent) { Puppet::Agent.new(AgentTestClient) } |
| 2514 | ++ |
| 2515 | ++ describe "in order to be backwards-compatibility with versions 2.7.10 and 2.7.11" do |
| 2516 | ++ |
| 2517 | ++ describe "when the 2.7.10/2.7.11 'disabled' lockfile exists" do |
| 2518 | ++ |
| 2519 | ++ # the "normal" lockfile |
| 2520 | ++ let(:lockfile_path) { tmpfile("agent_spec_lockfile") } |
| 2521 | ++ |
| 2522 | ++ # the 2.7.10/2.7.11 "disabled" lockfile |
| 2523 | ++ # (can't use PuppetSpec::Files.tmpfile here because we need the ".disabled" file to have *exactly* the same |
| 2524 | ++ # path/name as the original file, plus the ".disabled" suffix.) |
| 2525 | ++ let(:disabled_lockfile_path) { lockfile_path + ".disabled" } |
| 2526 | ++ |
| 2527 | ++ # some regexes to match log messages |
| 2528 | ++ let(:warning_regex) { /^Found special lockfile '#{disabled_lockfile_path}'.*renaming/ } |
| 2529 | ++ let(:disabled_regex) { /^Skipping run of .*; administratively disabled/ } |
| 2530 | ++ |
| 2531 | ++ before(:each) do |
| 2532 | ++ # create the 2.7.10 "disable" lockfile. |
| 2533 | ++ FileUtils.touch(disabled_lockfile_path) |
| 2534 | ++ |
| 2535 | ++ # stub in our temp lockfile path. |
| 2536 | ++ AgentTestClient.expects(:lockfile_path).returns lockfile_path |
| 2537 | ++ end |
| 2538 | ++ |
| 2539 | ++ after(:each) do |
| 2540 | ++ # manually clean up the files that we didn't create via PuppetSpec::Files.tmpfile |
| 2541 | ++ begin |
| 2542 | ++ File.unlink(disabled_lockfile_path) |
| 2543 | ++ rescue Errno::ENOENT |
| 2544 | ++ # some of the tests expect for the agent code to take care of deleting this file, |
| 2545 | ++ # so it may (validly) not exist. |
| 2546 | ++ end |
| 2547 | ++ end |
| 2548 | ++ |
| 2549 | ++ describe "when the 'regular' lockfile also exists" do |
| 2550 | ++ # the logic here is that if a 'regular' lockfile already exists, then there is some state that the |
| 2551 | ++ # current version of puppet is responsible for dealing with. All of the tests in this block are |
| 2552 | ++ # simply here to make sure that our backwards-compatibility hack does *not* interfere with this. |
| 2553 | ++ # |
| 2554 | ++ # Even if the ".disabled" lockfile exists--it can be dealt with at another time, when puppet is |
| 2555 | ++ # in *exactly* the state that we want it to be in (mostly meaning that the 'regular' lockfile |
| 2556 | ++ # does not exist.) |
| 2557 | ++ |
| 2558 | ++ before(:each) do |
| 2559 | ++ # create the "regular" lockfile |
| 2560 | ++ FileUtils.touch(lockfile_path) |
| 2561 | ++ end |
| 2562 | ++ |
| 2563 | ++ it "should be recognized as 'disabled'" do |
| 2564 | ++ agent.should be_disabled |
| 2565 | ++ end |
| 2566 | ++ |
| 2567 | ++ it "should not try to start a new agent run" do |
| 2568 | ++ AgentTestClient.expects(:new).never |
| 2569 | ++ Puppet.expects(:notice).with(regexp_matches(disabled_regex)) |
| 2570 | ++ |
| 2571 | ++ agent.run |
| 2572 | ++ end |
| 2573 | ++ |
| 2574 | ++ it "should not delete the 2.7.10/2.7.11 lockfile" do |
| 2575 | ++ agent.run |
| 2576 | ++ |
| 2577 | ++ File.exists?(disabled_lockfile_path).should == true |
| 2578 | ++ end |
| 2579 | ++ |
| 2580 | ++ it "should not print the warning message" do |
| 2581 | ++ Puppet.expects(:warning).with(regexp_matches(warning_regex)).never |
| 2582 | ++ |
| 2583 | ++ agent.run |
| 2584 | ++ end |
| 2585 | ++ end |
| 2586 | ++ |
| 2587 | ++ describe "when the 'regular' lockfile does not exist" do |
| 2588 | ++ # this block of tests is for actually testing the backwards compatibility hack. This |
| 2589 | ++ # is where we're in a clean state and we know it's safe(r) to muck with the lockfile |
| 2590 | ++ # situation. |
| 2591 | ++ |
| 2592 | ++ it "should recognize that the agent is disabled" do |
| 2593 | ++ agent.should be_disabled |
| 2594 | ++ end |
| 2595 | ++ |
| 2596 | ++ describe "when an agent run is requested" do |
| 2597 | ++ it "should not try to start a new agent run" do |
| 2598 | ++ AgentTestClient.expects(:new).never |
| 2599 | ++ Puppet.expects(:notice).with(regexp_matches(disabled_regex)) |
| 2600 | ++ |
| 2601 | ++ agent.run |
| 2602 | ++ end |
| 2603 | ++ |
| 2604 | ++ it "should warn, remove the 2.7.10/2.7.11 lockfile, and create the 'normal' lockfile" do |
| 2605 | ++ Puppet.expects(:warning).with(regexp_matches(warning_regex)) |
| 2606 | ++ |
| 2607 | ++ agent.run |
| 2608 | ++ |
| 2609 | ++ File.exists?(disabled_lockfile_path).should == false |
| 2610 | ++ File.exists?(lockfile_path).should == true |
| 2611 | ++ end |
| 2612 | ++ end |
| 2613 | ++ |
| 2614 | ++ describe "when running --enable" do |
| 2615 | ++ it "should recognize that the agent is disabled" do |
| 2616 | ++ agent.should be_disabled |
| 2617 | ++ end |
| 2618 | ++ |
| 2619 | ++ it "should warn and clean up the 2.7.10/2.7.11 lockfile" do |
| 2620 | ++ Puppet.expects(:warning).with(regexp_matches(warning_regex)) |
| 2621 | ++ |
| 2622 | ++ agent.enable |
| 2623 | ++ |
| 2624 | ++ File.exists?(disabled_lockfile_path).should == false |
| 2625 | ++ File.exists?(lockfile_path).should == false |
| 2626 | ++ end |
| 2627 | ++ end |
| 2628 | ++ end |
| 2629 | ++ end |
| 2630 | ++ end |
| 2631 | ++ |
| 2632 | ++ |
| 2633 | ++end |
| 2634 | +Index: puppet-new-bzr/spec/unit/agent_spec.rb |
| 2635 | +=================================================================== |
| 2636 | +--- puppet-new-bzr.orig/spec/unit/agent_spec.rb 2012-03-16 15:14:51.672976000 +0000 |
| 2637 | ++++ puppet-new-bzr/spec/unit/agent_spec.rb 2012-03-16 15:15:48.002509551 +0000 |
| 2638 | +@@ -94,12 +94,6 @@ |
| 2639 | + @agent.run |
| 2640 | + end |
| 2641 | + |
| 2642 | +- it "should do nothing if disabled" do |
| 2643 | +- @agent.expects(:disabled?).returns(true) |
| 2644 | +- AgentTestClient.expects(:new).never |
| 2645 | +- @agent.run |
| 2646 | +- end |
| 2647 | +- |
| 2648 | + it "should use Puppet::Application.controlled_run to manage process state behavior" do |
| 2649 | + calls = sequence('calls') |
| 2650 | + Puppet::Application.expects(:controlled_run).yields.in_sequence(calls) |
| 2651 | +Index: puppet-new-bzr/spec/unit/application/agent_spec.rb |
| 2652 | +=================================================================== |
| 2653 | +--- puppet-new-bzr.orig/spec/unit/application/agent_spec.rb 2012-03-16 15:14:51.672976000 +0000 |
| 2654 | ++++ puppet-new-bzr/spec/unit/application/agent_spec.rb 2012-03-16 15:15:48.002509551 +0000 |
| 2655 | +@@ -91,7 +91,7 @@ |
| 2656 | + @puppetd.command_line.stubs(:args).returns([]) |
| 2657 | + end |
| 2658 | + |
| 2659 | +- [:centrallogging, :enable, :debug, :fqdn, :test, :verbose, :digest].each do |option| |
| 2660 | ++ [:centrallogging, :disable, :enable, :debug, :fqdn, :test, :verbose, :digest].each do |option| |
| 2661 | + it "should declare handle_#{option} method" do |
| 2662 | + @puppetd.should respond_to("handle_#{option}".to_sym) |
| 2663 | + end |
| 2664 | +@@ -102,24 +102,6 @@ |
| 2665 | + end |
| 2666 | + end |
| 2667 | + |
| 2668 | +- describe "when handling --disable" do |
| 2669 | +- it "should declare handle_disable method" do |
| 2670 | +- @puppetd.should respond_to(:handle_disable) |
| 2671 | +- end |
| 2672 | +- |
| 2673 | +- it "should set disable to true" do |
| 2674 | +- @puppetd.options.stubs(:[]=) |
| 2675 | +- @puppetd.options.expects(:[]=).with(:disable, true) |
| 2676 | +- @puppetd.handle_disable('') |
| 2677 | +- end |
| 2678 | +- |
| 2679 | +- it "should store disable message" do |
| 2680 | +- @puppetd.options.stubs(:[]=) |
| 2681 | +- @puppetd.options.expects(:[]=).with(:disable_message, "message") |
| 2682 | +- @puppetd.handle_disable('message') |
| 2683 | +- end |
| 2684 | +- end |
| 2685 | +- |
| 2686 | + it "should set an existing handler on server" do |
| 2687 | + Puppet::Network::Handler.stubs(:handler).with("handler").returns(true) |
| 2688 | + |
| 2689 | +@@ -367,20 +349,6 @@ |
| 2690 | + end |
| 2691 | + end |
| 2692 | + |
| 2693 | +- it "should pass the disable message when disabling" do |
| 2694 | +- @puppetd.options.stubs(:[]).with(:disable).returns(true) |
| 2695 | +- @puppetd.options.stubs(:[]).with(:disable_message).returns("message") |
| 2696 | +- @agent.expects(:disable).with("message") |
| 2697 | +- expect { @puppetd.enable_disable_client(@agent) }.to exit_with 0 |
| 2698 | +- end |
| 2699 | +- |
| 2700 | +- it "should pass the default disable message when disabling without a message" do |
| 2701 | +- @puppetd.options.stubs(:[]).with(:disable).returns(true) |
| 2702 | +- @puppetd.options.stubs(:[]).with(:disable_message).returns(nil) |
| 2703 | +- @agent.expects(:disable).with("reason not specified") |
| 2704 | +- expect { @puppetd.enable_disable_client(@agent) }.to exit_with 0 |
| 2705 | +- end |
| 2706 | +- |
| 2707 | + it "should finally exit" do |
| 2708 | + expect { @puppetd.enable_disable_client(@agent) }.to exit_with 0 |
| 2709 | + end |
| 2710 | +Index: puppet-new-bzr/spec/unit/util/anonymous_filelock_spec.rb |
| 2711 | +=================================================================== |
| 2712 | +--- puppet-new-bzr.orig/spec/unit/util/anonymous_filelock_spec.rb 2012-03-16 15:14:51.672976000 +0000 |
| 2713 | ++++ /dev/null 1970-01-01 00:00:00.000000000 +0000 |
| 2714 | +@@ -1,78 +0,0 @@ |
| 2715 | +-#!/usr/bin/env rspec |
| 2716 | +-require 'spec_helper' |
| 2717 | +- |
| 2718 | +-require 'puppet/util/anonymous_filelock' |
| 2719 | +- |
| 2720 | +-describe Puppet::Util::AnonymousFilelock do |
| 2721 | +- require 'puppet_spec/files' |
| 2722 | +- include PuppetSpec::Files |
| 2723 | +- |
| 2724 | +- before(:each) do |
| 2725 | +- @lockfile = tmpfile("lock") |
| 2726 | +- @lock = Puppet::Util::AnonymousFilelock.new(@lockfile) |
| 2727 | +- end |
| 2728 | +- |
| 2729 | +- it "should be anonymous" do |
| 2730 | +- @lock.should be_anonymous |
| 2731 | +- end |
| 2732 | +- |
| 2733 | +- describe "#lock" do |
| 2734 | +- it "should return false if already locked" do |
| 2735 | +- @lock.stubs(:locked?).returns(true) |
| 2736 | +- @lock.lock.should be_false |
| 2737 | +- end |
| 2738 | +- |
| 2739 | +- it "should return true if it successfully locked" do |
| 2740 | +- @lock.lock.should be_true |
| 2741 | +- end |
| 2742 | +- |
| 2743 | +- it "should create a lock file" do |
| 2744 | +- @lock.lock |
| 2745 | +- |
| 2746 | +- File.should be_exists(@lockfile) |
| 2747 | +- end |
| 2748 | +- |
| 2749 | +- it "should create a lock file containing a message" do |
| 2750 | +- @lock.lock("message") |
| 2751 | +- |
| 2752 | +- File.read(@lockfile).should == "message" |
| 2753 | +- end |
| 2754 | +- end |
| 2755 | +- |
| 2756 | +- describe "#unlock" do |
| 2757 | +- it "should return true when unlocking" do |
| 2758 | +- @lock.lock |
| 2759 | +- @lock.unlock.should be_true |
| 2760 | +- end |
| 2761 | +- |
| 2762 | +- it "should return false when not locked" do |
| 2763 | +- @lock.unlock.should be_false |
| 2764 | +- end |
| 2765 | +- |
| 2766 | +- it "should clear the lock file" do |
| 2767 | +- File.open(@lockfile, 'w') { |fd| fd.print("locked") } |
| 2768 | +- @lock.unlock |
| 2769 | +- File.should_not be_exists(@lockfile) |
| 2770 | +- end |
| 2771 | +- end |
| 2772 | +- |
| 2773 | +- it "should be locked when locked" do |
| 2774 | +- @lock.lock |
| 2775 | +- @lock.should be_locked |
| 2776 | +- end |
| 2777 | +- |
| 2778 | +- it "should not be locked when not locked" do |
| 2779 | +- @lock.should_not be_locked |
| 2780 | +- end |
| 2781 | +- |
| 2782 | +- it "should not be locked when unlocked" do |
| 2783 | +- @lock.lock |
| 2784 | +- @lock.unlock |
| 2785 | +- @lock.should_not be_locked |
| 2786 | +- end |
| 2787 | +- |
| 2788 | +- it "should return the lock message" do |
| 2789 | +- @lock.lock("lock message") |
| 2790 | +- @lock.message.should == "lock message" |
| 2791 | +- end |
| 2792 | +-end |
| 2793 | +\ No newline at end of file |
| 2794 | +Index: puppet-new-bzr/spec/unit/util/pidlock_spec.rb |
| 2795 | +=================================================================== |
| 2796 | +--- puppet-new-bzr.orig/spec/unit/util/pidlock_spec.rb 2012-03-16 15:14:51.672976000 +0000 |
| 2797 | ++++ /dev/null 1970-01-01 00:00:00.000000000 +0000 |
| 2798 | +@@ -1,208 +0,0 @@ |
| 2799 | +-#!/usr/bin/env rspec |
| 2800 | +-require 'spec_helper' |
| 2801 | +- |
| 2802 | +-require 'puppet/util/pidlock' |
| 2803 | +- |
| 2804 | +-describe Puppet::Util::Pidlock do |
| 2805 | +- require 'puppet_spec/files' |
| 2806 | +- include PuppetSpec::Files |
| 2807 | +- |
| 2808 | +- before(:each) do |
| 2809 | +- @lockfile = tmpfile("lock") |
| 2810 | +- @lock = Puppet::Util::Pidlock.new(@lockfile) |
| 2811 | +- end |
| 2812 | +- |
| 2813 | +- it "should not be anonymous" do |
| 2814 | +- @lock.should_not be_anonymous |
| 2815 | +- end |
| 2816 | +- |
| 2817 | +- describe "#lock" do |
| 2818 | +- it "should not be locked at start" do |
| 2819 | +- @lock.should_not be_locked |
| 2820 | +- end |
| 2821 | +- |
| 2822 | +- it "should not be mine at start" do |
| 2823 | +- @lock.should_not be_mine |
| 2824 | +- end |
| 2825 | +- |
| 2826 | +- it "should become locked" do |
| 2827 | +- @lock.lock |
| 2828 | +- @lock.should be_locked |
| 2829 | +- end |
| 2830 | +- |
| 2831 | +- it "should become mine" do |
| 2832 | +- @lock.lock |
| 2833 | +- @lock.should be_mine |
| 2834 | +- end |
| 2835 | +- |
| 2836 | +- it "should be possible to lock multiple times" do |
| 2837 | +- @lock.lock |
| 2838 | +- lambda { @lock.lock }.should_not raise_error |
| 2839 | +- end |
| 2840 | +- |
| 2841 | +- it "should return true when locking" do |
| 2842 | +- @lock.lock.should be_true |
| 2843 | +- end |
| 2844 | +- |
| 2845 | +- it "should return true if locked by me" do |
| 2846 | +- @lock.lock |
| 2847 | +- @lock.lock.should be_true |
| 2848 | +- end |
| 2849 | +- |
| 2850 | +- it "should return false if locked by someone else" do |
| 2851 | +- Process.stubs(:kill) |
| 2852 | +- File.open(@lockfile, "w") { |fd| fd.print('0') } |
| 2853 | +- |
| 2854 | +- @lock.lock.should be_false |
| 2855 | +- end |
| 2856 | +- |
| 2857 | +- it "should create a lock file" do |
| 2858 | +- @lock.lock |
| 2859 | +- File.should be_exists(@lockfile) |
| 2860 | +- end |
| 2861 | +- |
| 2862 | +- it "should create a lock file containing our pid" do |
| 2863 | +- @lock.lock |
| 2864 | +- File.read(@lockfile).to_i.should == Process.pid.to_i |
| 2865 | +- end |
| 2866 | +- end |
| 2867 | +- |
| 2868 | +- describe "#unlock" do |
| 2869 | +- it "should not be locked anymore" do |
| 2870 | +- @lock.lock |
| 2871 | +- @lock.unlock |
| 2872 | +- @lock.should_not be_locked |
| 2873 | +- end |
| 2874 | +- |
| 2875 | +- it "should return false if not locked" do |
| 2876 | +- @lock.unlock.should be_false |
| 2877 | +- end |
| 2878 | +- |
| 2879 | +- it "should return true if properly unlocked" do |
| 2880 | +- @lock.lock |
| 2881 | +- @lock.unlock.should be_true |
| 2882 | +- end |
| 2883 | +- |
| 2884 | +- it "should get rid of the lock file" do |
| 2885 | +- @lock.lock |
| 2886 | +- @lock.unlock |
| 2887 | +- File.should_not be_exists(@lockfile) |
| 2888 | +- end |
| 2889 | +- |
| 2890 | +- it "should not warn if the lockfile was deleted by someone else" do |
| 2891 | +- @lock.lock |
| 2892 | +- File.unlink(@lockfile) |
| 2893 | +- |
| 2894 | +- Puppet.expects(:err).never # meh |
| 2895 | +- @lock.unlock |
| 2896 | +- end |
| 2897 | +- |
| 2898 | +- it "should warn if the lockfile can't be deleted" do |
| 2899 | +- @lock.lock |
| 2900 | +- File.expects(:unlink).with(@lockfile).raises(Errno::EIO) |
| 2901 | +- Puppet.expects(:err).with do |argument| |
| 2902 | +- argument.should =~ /Input\/output error/ |
| 2903 | +- end |
| 2904 | +- @lock.unlock |
| 2905 | +- |
| 2906 | +- # This is necessary because our cleanup code uses File.unlink |
| 2907 | +- File.unstub(:unlink) |
| 2908 | +- @lock.unlock |
| 2909 | +- end |
| 2910 | +- end |
| 2911 | +- |
| 2912 | +- describe "#locked?" do |
| 2913 | +- it "should return true if locked" do |
| 2914 | +- @lock.lock |
| 2915 | +- @lock.should be_locked |
| 2916 | +- end |
| 2917 | +- end |
| 2918 | +- |
| 2919 | +- describe "with a stale lock" do |
| 2920 | +- before(:each) do |
| 2921 | +- Process.stubs(:kill).with(0, 6789) |
| 2922 | +- Process.stubs(:kill).with(0, 1234).raises(Errno::ESRCH) |
| 2923 | +- Process.stubs(:pid).returns(6789) |
| 2924 | +- File.open(@lockfile, 'w') { |fd| fd.write("1234") } |
| 2925 | +- end |
| 2926 | +- |
| 2927 | +- it "should not be locked" do |
| 2928 | +- @lock.should_not be_locked |
| 2929 | +- end |
| 2930 | +- |
| 2931 | +- describe "#lock" do |
| 2932 | +- it "should clear stale locks" do |
| 2933 | +- @lock.locked? |
| 2934 | +- File.should_not be_exists(@lockfile) |
| 2935 | +- end |
| 2936 | +- |
| 2937 | +- it "should replace with new locks" do |
| 2938 | +- @lock.lock |
| 2939 | +- File.should be_exists(@lockfile) |
| 2940 | +- @lock.lock_pid.should == 6789 |
| 2941 | +- @lock.should be_mine |
| 2942 | +- @lock.should be_locked |
| 2943 | +- end |
| 2944 | +- end |
| 2945 | +- |
| 2946 | +- describe "#unlock" do |
| 2947 | +- it "should not be allowed" do |
| 2948 | +- @lock.unlock.should be_false |
| 2949 | +- end |
| 2950 | +- |
| 2951 | +- it "should not remove the lock file" do |
| 2952 | +- @lock.unlock |
| 2953 | +- File.should be_exists(@lockfile) |
| 2954 | +- end |
| 2955 | +- end |
| 2956 | +- end |
| 2957 | +- |
| 2958 | +- describe "with another process lock" do |
| 2959 | +- before(:each) do |
| 2960 | +- Process.stubs(:kill).with(0, 6789) |
| 2961 | +- Process.stubs(:kill).with(0, 1234) |
| 2962 | +- Process.stubs(:pid).returns(6789) |
| 2963 | +- File.open(@lockfile, 'w') { |fd| fd.write("1234") } |
| 2964 | +- end |
| 2965 | +- |
| 2966 | +- it "should be locked" do |
| 2967 | +- @lock.should be_locked |
| 2968 | +- end |
| 2969 | +- |
| 2970 | +- it "should not be mine" do |
| 2971 | +- @lock.should_not be_mine |
| 2972 | +- end |
| 2973 | +- |
| 2974 | +- describe "#lock" do |
| 2975 | +- it "should not be possible" do |
| 2976 | +- @lock.lock.should be_false |
| 2977 | +- end |
| 2978 | +- |
| 2979 | +- it "should not overwrite the lock" do |
| 2980 | +- @lock.lock |
| 2981 | +- @lock.should_not be_mine |
| 2982 | +- end |
| 2983 | +- end |
| 2984 | +- |
| 2985 | +- describe "#unlock" do |
| 2986 | +- it "should not be possible" do |
| 2987 | +- @lock.unlock.should be_false |
| 2988 | +- end |
| 2989 | +- |
| 2990 | +- it "should not remove the lock file" do |
| 2991 | +- @lock.unlock |
| 2992 | +- File.should be_exists(@lockfile) |
| 2993 | +- end |
| 2994 | +- |
| 2995 | +- it "should still not be our lock" do |
| 2996 | +- @lock.unlock |
| 2997 | +- @lock.should_not be_mine |
| 2998 | +- end |
| 2999 | +- |
| 3000 | +- it "should not warn" do |
| 3001 | +- Puppet.expects(:err).never |
| 3002 | +- @lock.unlock |
| 3003 | +- end |
| 3004 | +- end |
| 3005 | +- end |
| 3006 | +-end |
| 3007 | +Index: puppet-new-bzr/test/util/pidlock.rb |
| 3008 | +=================================================================== |
| 3009 | +--- /dev/null 1970-01-01 00:00:00.000000000 +0000 |
| 3010 | ++++ puppet-new-bzr/test/util/pidlock.rb 2012-03-16 15:15:48.002509551 +0000 |
| 3011 | +@@ -0,0 +1,126 @@ |
| 3012 | ++require File.expand_path(File.dirname(__FILE__) + '/../lib/puppettest') |
| 3013 | ++ |
| 3014 | ++require 'puppet/util/pidlock' |
| 3015 | ++require 'fileutils' |
| 3016 | ++ |
| 3017 | ++# This is *fucked* *up* |
| 3018 | ++Puppet.debug = false |
| 3019 | ++ |
| 3020 | ++class TestPuppetUtilPidlock < Test::Unit::TestCase |
| 3021 | ++ include PuppetTest |
| 3022 | ++ |
| 3023 | ++ def setup |
| 3024 | ++ super |
| 3025 | ++ @workdir = tstdir |
| 3026 | ++ end |
| 3027 | ++ |
| 3028 | ++ def teardown |
| 3029 | ++ super |
| 3030 | ++ FileUtils.rm_rf(@workdir) |
| 3031 | ++ end |
| 3032 | ++ |
| 3033 | ++ def test_00_basic_create |
| 3034 | ++ l = nil |
| 3035 | ++ assert_nothing_raised { l = Puppet::Util::Pidlock.new(@workdir + '/nothingmuch') } |
| 3036 | ++ |
| 3037 | ++ assert_equal Puppet::Util::Pidlock, l.class |
| 3038 | ++ |
| 3039 | ++ assert_equal @workdir + '/nothingmuch', l.lockfile |
| 3040 | ++ end |
| 3041 | ++ |
| 3042 | ++ def test_10_uncontended_lock |
| 3043 | ++ l = Puppet::Util::Pidlock.new(@workdir + '/test_lock') |
| 3044 | ++ |
| 3045 | ++ assert !l.locked? |
| 3046 | ++ assert !l.mine? |
| 3047 | ++ assert l.lock |
| 3048 | ++ assert l.locked? |
| 3049 | ++ assert l.mine? |
| 3050 | ++ assert !l.anonymous? |
| 3051 | ++ # It's OK to call lock multiple times |
| 3052 | ++ assert l.lock |
| 3053 | ++ assert l.unlock |
| 3054 | ++ assert !l.locked? |
| 3055 | ++ assert !l.mine? |
| 3056 | ++ end |
| 3057 | ++ |
| 3058 | ++ def test_20_someone_elses_lock |
| 3059 | ++ childpid = nil |
| 3060 | ++ l = Puppet::Util::Pidlock.new(@workdir + '/someone_elses_lock') |
| 3061 | ++ |
| 3062 | ++ # First, we need a PID that's guaranteed to be (a) used, (b) someone |
| 3063 | ++ # else's, and (c) around for the life of this test. |
| 3064 | ++ childpid = fork { loop do; sleep 10; end } |
| 3065 | ++ |
| 3066 | ++ File.open(l.lockfile, 'w') { |fd| fd.write(childpid) } |
| 3067 | ++ |
| 3068 | ++ assert l.locked? |
| 3069 | ++ assert !l.mine? |
| 3070 | ++ assert !l.lock |
| 3071 | ++ assert l.locked? |
| 3072 | ++ assert !l.mine? |
| 3073 | ++ assert !l.unlock |
| 3074 | ++ assert l.locked? |
| 3075 | ++ assert !l.mine? |
| 3076 | ++ ensure |
| 3077 | ++ Process.kill("KILL", childpid) unless childpid.nil? |
| 3078 | ++ end |
| 3079 | ++ |
| 3080 | ++ def test_30_stale_lock |
| 3081 | ++ # This is a bit hard to guarantee, but we need a PID that is definitely |
| 3082 | ++ # unused, and will stay so for the the life of this test. Our best |
| 3083 | ++ # bet is to create a process, get it's PID, let it die, and *then* |
| 3084 | ++ # lock on it. |
| 3085 | ++ childpid = fork { exit } |
| 3086 | ++ |
| 3087 | ++ # Now we can't continue until we're sure that the PID is dead |
| 3088 | ++ Process.wait(childpid) |
| 3089 | ++ |
| 3090 | ++ l = Puppet::Util::Pidlock.new(@workdir + '/stale_lock') |
| 3091 | ++ |
| 3092 | ++ # locked? should clear the lockfile |
| 3093 | ++ File.open(l.lockfile, 'w') { |fd| fd.write(childpid) } |
| 3094 | ++ assert File.exists?(l.lockfile) |
| 3095 | ++ assert !l.locked? |
| 3096 | ++ assert !File.exists?(l.lockfile) |
| 3097 | ++ |
| 3098 | ++ # lock should replace the lockfile with our own |
| 3099 | ++ File.open(l.lockfile, 'w') { |fd| fd.write(childpid) } |
| 3100 | ++ assert File.exists?(l.lockfile) |
| 3101 | ++ assert l.lock |
| 3102 | ++ assert l.locked? |
| 3103 | ++ assert l.mine? |
| 3104 | ++ |
| 3105 | ++ # unlock should fail, and should *not* molest the existing lockfile, |
| 3106 | ++ # despite it being stale |
| 3107 | ++ File.open(l.lockfile, 'w') { |fd| fd.write(childpid) } |
| 3108 | ++ assert File.exists?(l.lockfile) |
| 3109 | ++ assert !l.unlock |
| 3110 | ++ assert File.exists?(l.lockfile) |
| 3111 | ++ end |
| 3112 | ++ |
| 3113 | ++ def test_40_not_locked_at_all |
| 3114 | ++ l = Puppet::Util::Pidlock.new(@workdir + '/not_locked') |
| 3115 | ++ |
| 3116 | ++ assert !l.locked? |
| 3117 | ++ # We can't unlock if we don't hold the lock |
| 3118 | ++ assert !l.unlock |
| 3119 | ++ end |
| 3120 | ++ |
| 3121 | ++ def test_50_anonymous_lock |
| 3122 | ++ l = Puppet::Util::Pidlock.new(@workdir + '/anonymous_lock') |
| 3123 | ++ |
| 3124 | ++ assert !l.locked? |
| 3125 | ++ assert l.lock(:anonymous => true) |
| 3126 | ++ assert l.locked? |
| 3127 | ++ assert l.anonymous? |
| 3128 | ++ assert !l.mine? |
| 3129 | ++ assert "", File.read(l.lockfile) |
| 3130 | ++ assert !l.unlock |
| 3131 | ++ assert l.locked? |
| 3132 | ++ assert l.anonymous? |
| 3133 | ++ assert l.unlock(:anonymous => true) |
| 3134 | ++ assert !File.exists?(l.lockfile) |
| 3135 | ++ end |
| 3136 | ++end |
| 3137 | ++ |
| 3138 | |
| 3139 | === modified file 'debian/patches/series' |
| 3140 | --- debian/patches/series 2012-01-26 11:27:00 +0000 |
| 3141 | +++ debian/patches/series 2012-03-16 16:38:20 +0000 |
| 3142 | @@ -1,2 +1,3 @@ |
| 3143 | fix_logcheck |
| 3144 | debian-changes |
| 3145 | +puppet-12844 |
| 3146 | |
| 3147 | === modified file 'debian/puppetmaster-passenger.postinst' |
| 3148 | --- debian/puppetmaster-passenger.postinst 2011-10-22 14:08:22 +0000 |
| 3149 | +++ debian/puppetmaster-passenger.postinst 2012-03-16 16:38:20 +0000 |
| 3150 | @@ -20,6 +20,11 @@ |
| 3151 | if [ ! -e "$(puppet config print hostcert)" ]; then |
| 3152 | puppet cert generate $(puppet config print certname) |
| 3153 | fi |
| 3154 | + # Check that puppet config print works properly |
| 3155 | + if [ $(puppet config print 2>&1 | grep "Could not parse" | wc -l) != "0" ]; then |
| 3156 | + echo "Puppet config print not working properly, exiting" |
| 3157 | + exit 1 |
| 3158 | + fi |
| 3159 | # Setup apache2 configuration files |
| 3160 | APACHE2_SITE_FILE="/etc/apache2/sites-available/puppetmaster" |
| 3161 | if [ ! -e "${APACHE2_SITE_FILE}" ]; then |
| 3162 | @@ -30,8 +35,10 @@ |
| 3163 | sed -r -i "s|(SSLCACertificateFile\s+).+$|\1$(puppet config print localcacert)|" "${APACHE2_SITE_FILE}" |
| 3164 | sed -r -i "s|(SSLCertificateChainFile\s+).+$|\1$(puppet config print localcacert)|" "${APACHE2_SITE_FILE}" |
| 3165 | sed -r -i "s|(SSLCARevocationFile\s+).+$|\1$(puppet config print cacrl)|" "${APACHE2_SITE_FILE}" |
| 3166 | + sed -r -i "s|/etc/puppet/rack|/usr/share/puppet/rack/puppetmasterd|" "${APACHE2_SITE_FILE}" |
| 3167 | fi |
| 3168 | a2enmod ssl |
| 3169 | + a2enmod headers |
| 3170 | a2ensite puppetmaster |
| 3171 | if [ -x "/etc/init.d/apache2" ]; then |
| 3172 | # Seems that a restart is needed. reload breaks ssl apparently. |
| 3173 | |
| 3174 | === modified file 'lib/puppet/agent.rb' |
| 3175 | --- lib/puppet/agent.rb 2012-01-26 11:27:00 +0000 |
| 3176 | +++ lib/puppet/agent.rb 2012-03-16 16:38:20 +0000 |
| 3177 | @@ -8,9 +8,6 @@ |
| 3178 | require 'puppet/agent/locker' |
| 3179 | include Puppet::Agent::Locker |
| 3180 | |
| 3181 | - require 'puppet/agent/disabler' |
| 3182 | - include Puppet::Agent::Disabler |
| 3183 | - |
| 3184 | attr_reader :client_class, :client, :splayed |
| 3185 | |
| 3186 | # Just so we can specify that we are "the" instance. |
| 3187 | @@ -35,9 +32,10 @@ |
| 3188 | return |
| 3189 | end |
| 3190 | if disabled? |
| 3191 | - Puppet.notice "Skipping run of #{client_class}; administratively disabled: #{disable_message}" |
| 3192 | + Puppet.notice "Skipping run of #{client_class}; administratively disabled; use 'puppet #{client_class} --enable' to re-enable." |
| 3193 | return |
| 3194 | end |
| 3195 | + |
| 3196 | result = nil |
| 3197 | block_run = Puppet::Application.controlled_run do |
| 3198 | splay |
| 3199 | |
| 3200 | === modified file 'lib/puppet/agent/locker.rb' |
| 3201 | --- lib/puppet/agent/locker.rb 2012-01-26 11:27:00 +0000 |
| 3202 | +++ lib/puppet/agent/locker.rb 2012-03-16 16:38:20 +0000 |
| 3203 | @@ -3,6 +3,16 @@ |
| 3204 | # Break out the code related to locking the agent. This module is just |
| 3205 | # included into the agent, but having it here makes it easier to test. |
| 3206 | module Puppet::Agent::Locker |
| 3207 | + # Let the daemon run again, freely in the filesystem. |
| 3208 | + def enable |
| 3209 | + lockfile.unlock(:anonymous => true) |
| 3210 | + end |
| 3211 | + |
| 3212 | + # Stop the daemon from making any catalog runs. |
| 3213 | + def disable |
| 3214 | + lockfile.lock(:anonymous => true) |
| 3215 | + end |
| 3216 | + |
| 3217 | # Yield if we get a lock, else do nothing. Return |
| 3218 | # true/false depending on whether we get the lock. |
| 3219 | def lock |
| 3220 | @@ -25,6 +35,10 @@ |
| 3221 | end |
| 3222 | |
| 3223 | def running? |
| 3224 | - lockfile.locked? |
| 3225 | + lockfile.locked? and !lockfile.anonymous? |
| 3226 | + end |
| 3227 | + |
| 3228 | + def disabled? |
| 3229 | + lockfile.locked? and lockfile.anonymous? |
| 3230 | end |
| 3231 | end |
| 3232 | |
| 3233 | === modified file 'lib/puppet/application/agent.rb' |
| 3234 | --- lib/puppet/application/agent.rb 2012-01-26 11:27:00 +0000 |
| 3235 | +++ lib/puppet/application/agent.rb 2012-03-16 16:38:20 +0000 |
| 3236 | @@ -39,12 +39,7 @@ |
| 3237 | end |
| 3238 | |
| 3239 | option("--centrallogging") |
| 3240 | - |
| 3241 | - option("--disable [MESSAGE]") do |message| |
| 3242 | - options[:disable] = true |
| 3243 | - options[:disable_message] = message |
| 3244 | - end |
| 3245 | - |
| 3246 | + option("--disable") |
| 3247 | option("--enable") |
| 3248 | option("--debug","-d") |
| 3249 | option("--fqdn FQDN","-f") |
| 3250 | @@ -106,7 +101,7 @@ |
| 3251 | USAGE |
| 3252 | ----- |
| 3253 | puppet agent [--certname <name>] [-D|--daemonize|--no-daemonize] |
| 3254 | - [-d|--debug] [--detailed-exitcodes] [--digest <digest>] [--disable [message]] [--enable] |
| 3255 | + [-d|--debug] [--detailed-exitcodes] [--digest <digest>] [--disable] [--enable] |
| 3256 | [--fingerprint] [-h|--help] [-l|--logdest syslog|<file>|console] |
| 3257 | [--no-client] [--noop] [-o|--onetime] [--serve <handler>] [-t|--test] |
| 3258 | [-v|--verbose] [-V|--version] [-w|--waitforcert <seconds>] |
| 3259 | @@ -210,9 +205,6 @@ |
| 3260 | not want the central configuration to override the local state until |
| 3261 | everything is tested and committed. |
| 3262 | |
| 3263 | - Disable can also take an optional message that will be reported by the |
| 3264 | - 'puppet agent' at the next disabled run. |
| 3265 | - |
| 3266 | 'puppet agent' uses the same lock file while it is running, so no more |
| 3267 | than one 'puppet agent' process is working at a time. |
| 3268 | |
| 3269 | @@ -394,7 +386,7 @@ |
| 3270 | if options[:enable] |
| 3271 | agent.enable |
| 3272 | elsif options[:disable] |
| 3273 | - agent.disable(options[:disable_message] || 'reason not specified') |
| 3274 | + agent.disable |
| 3275 | end |
| 3276 | exit(0) |
| 3277 | end |
| 3278 | |
| 3279 | === removed file 'lib/puppet/util/anonymous_filelock.rb' |
| 3280 | --- lib/puppet/util/anonymous_filelock.rb 2012-01-26 11:27:00 +0000 |
| 3281 | +++ lib/puppet/util/anonymous_filelock.rb 1970-01-01 00:00:00 +0000 |
| 3282 | @@ -1,36 +0,0 @@ |
| 3283 | - |
| 3284 | -class Puppet::Util::AnonymousFilelock |
| 3285 | - attr_reader :lockfile |
| 3286 | - |
| 3287 | - def initialize(lockfile) |
| 3288 | - @lockfile = lockfile |
| 3289 | - end |
| 3290 | - |
| 3291 | - def anonymous? |
| 3292 | - true |
| 3293 | - end |
| 3294 | - |
| 3295 | - def lock(msg = '') |
| 3296 | - return false if locked? |
| 3297 | - |
| 3298 | - File.open(@lockfile, 'w') { |fd| fd.print(msg) } |
| 3299 | - true |
| 3300 | - end |
| 3301 | - |
| 3302 | - def unlock |
| 3303 | - if locked? |
| 3304 | - File.unlink(@lockfile) |
| 3305 | - true |
| 3306 | - else |
| 3307 | - false |
| 3308 | - end |
| 3309 | - end |
| 3310 | - |
| 3311 | - def locked? |
| 3312 | - File.exists? @lockfile |
| 3313 | - end |
| 3314 | - |
| 3315 | - def message |
| 3316 | - return File.read(@lockfile) if locked? |
| 3317 | - end |
| 3318 | -end |
| 3319 | \ No newline at end of file |
| 3320 | |
| 3321 | === modified file 'lib/puppet/util/pidlock.rb' |
| 3322 | --- lib/puppet/util/pidlock.rb 2012-02-23 18:24:48 +0000 |
| 3323 | +++ lib/puppet/util/pidlock.rb 2012-03-16 16:38:20 +0000 |
| 3324 | @@ -1,10 +1,20 @@ |
| 3325 | require 'fileutils' |
| 3326 | -require 'puppet/util/anonymous_filelock' |
| 3327 | - |
| 3328 | -class Puppet::Util::Pidlock < Puppet::Util::AnonymousFilelock |
| 3329 | + |
| 3330 | +class Puppet::Util::Pidlock |
| 3331 | + attr_reader :lockfile |
| 3332 | + |
| 3333 | + def initialize(lockfile) |
| 3334 | + @lockfile = lockfile |
| 3335 | + end |
| 3336 | |
| 3337 | def locked? |
| 3338 | clear_if_stale |
| 3339 | + return true if File.exists? @lockfile |
| 3340 | + |
| 3341 | + # HACK! There was a temporary change to the lockfile behavior introduced in 2.7.10 and 2.7.11, and reverted |
| 3342 | + # in 2.7.12. We need to pull some chicanery to be backwards-compatible with those versions. For more info, |
| 3343 | + # see the comments on the method... and this hack should be removed for the 3.x series. |
| 3344 | + handle_2_7_10_disabled_lockfile |
| 3345 | File.exists? @lockfile |
| 3346 | end |
| 3347 | |
| 3348 | @@ -13,36 +23,39 @@ |
| 3349 | end |
| 3350 | |
| 3351 | def anonymous? |
| 3352 | - false |
| 3353 | + return false unless File.exists?(@lockfile) |
| 3354 | + File.read(@lockfile) == "" |
| 3355 | end |
| 3356 | |
| 3357 | - def lock |
| 3358 | - return mine? if locked? |
| 3359 | + def lock(opts = {}) |
| 3360 | + opts = {:anonymous => false}.merge(opts) |
| 3361 | |
| 3362 | - File.open(@lockfile, "w") { |fd| fd.write(Process.pid) } |
| 3363 | - true |
| 3364 | + if locked? |
| 3365 | + mine? |
| 3366 | + else |
| 3367 | + if opts[:anonymous] |
| 3368 | + File.open(@lockfile, 'w') { |fd| true } |
| 3369 | + else |
| 3370 | + File.open(@lockfile, "w") { |fd| fd.write(Process.pid) } |
| 3371 | + end |
| 3372 | + true |
| 3373 | + end |
| 3374 | end |
| 3375 | |
| 3376 | def unlock(opts = {}) |
| 3377 | - if mine? |
| 3378 | - begin |
| 3379 | - File.unlink(@lockfile) |
| 3380 | - rescue Errno::ENOENT |
| 3381 | - # Someone deleted it for us ...and so we do nothing. No point whining |
| 3382 | - # about a problem that the user can't actually do anything about. |
| 3383 | - rescue SystemCallError => e |
| 3384 | - # This one is a real failure though. No idea what went wrong, but it |
| 3385 | - # is most likely "read only file(system)" or wrong permissions or |
| 3386 | - # something like that. |
| 3387 | - Puppet.err "Could not remove PID file #{@lockfile}: #{e}" |
| 3388 | - puts e.backtrace if Puppet[:trace] |
| 3389 | - end |
| 3390 | + return false unless locked? |
| 3391 | + |
| 3392 | + opts = {:anonymous => false}.merge(opts) |
| 3393 | + |
| 3394 | + if mine? or (opts[:anonymous] and anonymous?) |
| 3395 | + File.unlink(@lockfile) |
| 3396 | true |
| 3397 | else |
| 3398 | false |
| 3399 | end |
| 3400 | end |
| 3401 | |
| 3402 | + private |
| 3403 | def lock_pid |
| 3404 | if File.exists? @lockfile |
| 3405 | File.read(@lockfile).to_i |
| 3406 | @@ -51,7 +64,6 @@ |
| 3407 | end |
| 3408 | end |
| 3409 | |
| 3410 | - private |
| 3411 | def clear_if_stale |
| 3412 | return if lock_pid.nil? |
| 3413 | |
| 3414 | @@ -65,4 +77,41 @@ |
| 3415 | File.unlink(@lockfile) |
| 3416 | end |
| 3417 | end |
| 3418 | + |
| 3419 | + |
| 3420 | + ###################################################################################### |
| 3421 | + # Backwards compatibility hack |
| 3422 | + ###################################################################################### |
| 3423 | + # A change to lockfile behavior was introduced in 2.7.10 and 2.7.11; basically, |
| 3424 | + # instead of using a single lockfile to indicate both administrative disabling of |
| 3425 | + # the agent *and* the case where an agent run is already in progress, we started using |
| 3426 | + # two separate lockfiles: the 'normal' one for the "run in progress" case, and a |
| 3427 | + # separate one with a ".disabled" extension to indicate administrative disabling. |
| 3428 | + # |
| 3429 | + # This was determined to cause incompatibilities with mcollective, so the behavior |
| 3430 | + # was reverted for 2.7.12. Unfortunately this leaves the possibility that someone |
| 3431 | + # may have run "agent --disable" to administratively disable a 2.7.10 or 2.7.11 |
| 3432 | + # agent, and then upgraded to a newer version. This method exists only to |
| 3433 | + # provide backwards compatibility. Basically, it just recognizes the 2.7.10/2.7.11 |
| 3434 | + # ".disabled" lock file, warns, and cleans it up. |
| 3435 | + # |
| 3436 | + # This should be removed for the 3.x series. |
| 3437 | + # |
| 3438 | + # For more information, please see tickets #12844, #3757, #4836, and #11057 |
| 3439 | + # |
| 3440 | + # -- cprice 2012-03-01 |
| 3441 | + # |
| 3442 | + def handle_2_7_10_disabled_lockfile |
| 3443 | + disabled_lockfile_path = @lockfile + ".disabled" |
| 3444 | + if (File.exists?(disabled_lockfile_path)) |
| 3445 | + Puppet.warning("Found special lockfile '#{disabled_lockfile_path}'; this file was " + |
| 3446 | + "generated by a call to 'puppet agent --disable' in puppet 2.7.10 or 2.7.11. " + |
| 3447 | + "The expected lockfile path is '#{@lockfile}'; renaming the lock file.") |
| 3448 | + File.rename(disabled_lockfile_path, @lockfile) |
| 3449 | + end |
| 3450 | + end |
| 3451 | + private :handle_2_7_10_disabled_lockfile |
| 3452 | + ###################################################################################### |
| 3453 | + # End backwards compatibility hack |
| 3454 | + ###################################################################################### |
| 3455 | end |
| 3456 | |
| 3457 | === modified file 'spec/unit/agent/locker_spec.rb' |
| 3458 | --- spec/unit/agent/locker_spec.rb 2012-01-26 11:27:00 +0000 |
| 3459 | +++ spec/unit/agent/locker_spec.rb 2012-03-16 16:38:20 +0000 |
| 3460 | @@ -29,6 +29,18 @@ |
| 3461 | @locker.lockfile.should equal(@locker.lockfile) |
| 3462 | end |
| 3463 | |
| 3464 | + it "should use the lock file to anonymously lock the process when disabled" do |
| 3465 | + @locker.lockfile.expects(:lock).with(:anonymous => true) |
| 3466 | + |
| 3467 | + @locker.disable |
| 3468 | + end |
| 3469 | + |
| 3470 | + it "should use the lock file to anonymously unlock the process when enabled" do |
| 3471 | + @locker.lockfile.expects(:unlock).with(:anonymous => true) |
| 3472 | + |
| 3473 | + @locker.enable |
| 3474 | + end |
| 3475 | + |
| 3476 | it "should have a method that yields when a lock is attained" do |
| 3477 | @locker.lockfile.expects(:lock).returns true |
| 3478 | |
| 3479 | |
| 3480 | === added file 'spec/unit/agent_backward_compatibility_spec.rb' |
| 3481 | --- spec/unit/agent_backward_compatibility_spec.rb 1970-01-01 00:00:00 +0000 |
| 3482 | +++ spec/unit/agent_backward_compatibility_spec.rb 2012-03-16 16:38:20 +0000 |
| 3483 | @@ -0,0 +1,152 @@ |
| 3484 | +#!/usr/bin/env rspec |
| 3485 | +require 'spec_helper' |
| 3486 | +require 'puppet/agent' |
| 3487 | + |
| 3488 | + |
| 3489 | +############################################################################ |
| 3490 | +# NOTE # |
| 3491 | +############################################################################ |
| 3492 | +# # |
| 3493 | +# This entire spec is only here for backwards compatibility from 2.7.12+ # |
| 3494 | +# with 2.7.10 and 2.7.11. The entire file should be able to be removed # |
| 3495 | +# for the 3.x series. # |
| 3496 | +# # |
| 3497 | +# For more info, see the comments on the #handle_2_7_10_disabled_lockfile # |
| 3498 | +# method in pidlock.rb # |
| 3499 | +# # |
| 3500 | +# --cprice 2012-03-01 # |
| 3501 | +############################################################################ |
| 3502 | + |
| 3503 | +class AgentTestClient |
| 3504 | + def run |
| 3505 | + # no-op |
| 3506 | + end |
| 3507 | + def stop |
| 3508 | + # no-op |
| 3509 | + end |
| 3510 | +end |
| 3511 | + |
| 3512 | +describe Puppet::Agent do |
| 3513 | + include PuppetSpec::Files |
| 3514 | + |
| 3515 | + let(:agent) { Puppet::Agent.new(AgentTestClient) } |
| 3516 | + |
| 3517 | + describe "in order to be backwards-compatibility with versions 2.7.10 and 2.7.11" do |
| 3518 | + |
| 3519 | + describe "when the 2.7.10/2.7.11 'disabled' lockfile exists" do |
| 3520 | + |
| 3521 | + # the "normal" lockfile |
| 3522 | + let(:lockfile_path) { tmpfile("agent_spec_lockfile") } |
| 3523 | + |
| 3524 | + # the 2.7.10/2.7.11 "disabled" lockfile |
| 3525 | + # (can't use PuppetSpec::Files.tmpfile here because we need the ".disabled" file to have *exactly* the same |
| 3526 | + # path/name as the original file, plus the ".disabled" suffix.) |
| 3527 | + let(:disabled_lockfile_path) { lockfile_path + ".disabled" } |
| 3528 | + |
| 3529 | + # some regexes to match log messages |
| 3530 | + let(:warning_regex) { /^Found special lockfile '#{disabled_lockfile_path}'.*renaming/ } |
| 3531 | + let(:disabled_regex) { /^Skipping run of .*; administratively disabled/ } |
| 3532 | + |
| 3533 | + before(:each) do |
| 3534 | + # create the 2.7.10 "disable" lockfile. |
| 3535 | + FileUtils.touch(disabled_lockfile_path) |
| 3536 | + |
| 3537 | + # stub in our temp lockfile path. |
| 3538 | + AgentTestClient.expects(:lockfile_path).returns lockfile_path |
| 3539 | + end |
| 3540 | + |
| 3541 | + after(:each) do |
| 3542 | + # manually clean up the files that we didn't create via PuppetSpec::Files.tmpfile |
| 3543 | + begin |
| 3544 | + File.unlink(disabled_lockfile_path) |
| 3545 | + rescue Errno::ENOENT |
| 3546 | + # some of the tests expect for the agent code to take care of deleting this file, |
| 3547 | + # so it may (validly) not exist. |
| 3548 | + end |
| 3549 | + end |
| 3550 | + |
| 3551 | + describe "when the 'regular' lockfile also exists" do |
| 3552 | + # the logic here is that if a 'regular' lockfile already exists, then there is some state that the |
| 3553 | + # current version of puppet is responsible for dealing with. All of the tests in this block are |
| 3554 | + # simply here to make sure that our backwards-compatibility hack does *not* interfere with this. |
| 3555 | + # |
| 3556 | + # Even if the ".disabled" lockfile exists--it can be dealt with at another time, when puppet is |
| 3557 | + # in *exactly* the state that we want it to be in (mostly meaning that the 'regular' lockfile |
| 3558 | + # does not exist.) |
| 3559 | + |
| 3560 | + before(:each) do |
| 3561 | + # create the "regular" lockfile |
| 3562 | + FileUtils.touch(lockfile_path) |
| 3563 | + end |
| 3564 | + |
| 3565 | + it "should be recognized as 'disabled'" do |
| 3566 | + agent.should be_disabled |
| 3567 | + end |
| 3568 | + |
| 3569 | + it "should not try to start a new agent run" do |
| 3570 | + AgentTestClient.expects(:new).never |
| 3571 | + Puppet.expects(:notice).with(regexp_matches(disabled_regex)) |
| 3572 | + |
| 3573 | + agent.run |
| 3574 | + end |
| 3575 | + |
| 3576 | + it "should not delete the 2.7.10/2.7.11 lockfile" do |
| 3577 | + agent.run |
| 3578 | + |
| 3579 | + File.exists?(disabled_lockfile_path).should == true |
| 3580 | + end |
| 3581 | + |
| 3582 | + it "should not print the warning message" do |
| 3583 | + Puppet.expects(:warning).with(regexp_matches(warning_regex)).never |
| 3584 | + |
| 3585 | + agent.run |
| 3586 | + end |
| 3587 | + end |
| 3588 | + |
| 3589 | + describe "when the 'regular' lockfile does not exist" do |
| 3590 | + # this block of tests is for actually testing the backwards compatibility hack. This |
| 3591 | + # is where we're in a clean state and we know it's safe(r) to muck with the lockfile |
| 3592 | + # situation. |
| 3593 | + |
| 3594 | + it "should recognize that the agent is disabled" do |
| 3595 | + agent.should be_disabled |
| 3596 | + end |
| 3597 | + |
| 3598 | + describe "when an agent run is requested" do |
| 3599 | + it "should not try to start a new agent run" do |
| 3600 | + AgentTestClient.expects(:new).never |
| 3601 | + Puppet.expects(:notice).with(regexp_matches(disabled_regex)) |
| 3602 | + |
| 3603 | + agent.run |
| 3604 | + end |
| 3605 | + |
| 3606 | + it "should warn, remove the 2.7.10/2.7.11 lockfile, and create the 'normal' lockfile" do |
| 3607 | + Puppet.expects(:warning).with(regexp_matches(warning_regex)) |
| 3608 | + |
| 3609 | + agent.run |
| 3610 | + |
| 3611 | + File.exists?(disabled_lockfile_path).should == false |
| 3612 | + File.exists?(lockfile_path).should == true |
| 3613 | + end |
| 3614 | + end |
| 3615 | + |
| 3616 | + describe "when running --enable" do |
| 3617 | + it "should recognize that the agent is disabled" do |
| 3618 | + agent.should be_disabled |
| 3619 | + end |
| 3620 | + |
| 3621 | + it "should warn and clean up the 2.7.10/2.7.11 lockfile" do |
| 3622 | + Puppet.expects(:warning).with(regexp_matches(warning_regex)) |
| 3623 | + |
| 3624 | + agent.enable |
| 3625 | + |
| 3626 | + File.exists?(disabled_lockfile_path).should == false |
| 3627 | + File.exists?(lockfile_path).should == false |
| 3628 | + end |
| 3629 | + end |
| 3630 | + end |
| 3631 | + end |
| 3632 | + end |
| 3633 | + |
| 3634 | + |
| 3635 | +end |
| 3636 | |
| 3637 | === modified file 'spec/unit/agent_spec.rb' |
| 3638 | --- spec/unit/agent_spec.rb 2012-01-26 11:27:00 +0000 |
| 3639 | +++ spec/unit/agent_spec.rb 2012-03-16 16:38:20 +0000 |
| 3640 | @@ -94,12 +94,6 @@ |
| 3641 | @agent.run |
| 3642 | end |
| 3643 | |
| 3644 | - it "should do nothing if disabled" do |
| 3645 | - @agent.expects(:disabled?).returns(true) |
| 3646 | - AgentTestClient.expects(:new).never |
| 3647 | - @agent.run |
| 3648 | - end |
| 3649 | - |
| 3650 | it "should use Puppet::Application.controlled_run to manage process state behavior" do |
| 3651 | calls = sequence('calls') |
| 3652 | Puppet::Application.expects(:controlled_run).yields.in_sequence(calls) |
| 3653 | |
| 3654 | === modified file 'spec/unit/application/agent_spec.rb' |
| 3655 | --- spec/unit/application/agent_spec.rb 2012-01-26 11:27:00 +0000 |
| 3656 | +++ spec/unit/application/agent_spec.rb 2012-03-16 16:38:20 +0000 |
| 3657 | @@ -91,7 +91,7 @@ |
| 3658 | @puppetd.command_line.stubs(:args).returns([]) |
| 3659 | end |
| 3660 | |
| 3661 | - [:centrallogging, :enable, :debug, :fqdn, :test, :verbose, :digest].each do |option| |
| 3662 | + [:centrallogging, :disable, :enable, :debug, :fqdn, :test, :verbose, :digest].each do |option| |
| 3663 | it "should declare handle_#{option} method" do |
| 3664 | @puppetd.should respond_to("handle_#{option}".to_sym) |
| 3665 | end |
| 3666 | @@ -102,24 +102,6 @@ |
| 3667 | end |
| 3668 | end |
| 3669 | |
| 3670 | - describe "when handling --disable" do |
| 3671 | - it "should declare handle_disable method" do |
| 3672 | - @puppetd.should respond_to(:handle_disable) |
| 3673 | - end |
| 3674 | - |
| 3675 | - it "should set disable to true" do |
| 3676 | - @puppetd.options.stubs(:[]=) |
| 3677 | - @puppetd.options.expects(:[]=).with(:disable, true) |
| 3678 | - @puppetd.handle_disable('') |
| 3679 | - end |
| 3680 | - |
| 3681 | - it "should store disable message" do |
| 3682 | - @puppetd.options.stubs(:[]=) |
| 3683 | - @puppetd.options.expects(:[]=).with(:disable_message, "message") |
| 3684 | - @puppetd.handle_disable('message') |
| 3685 | - end |
| 3686 | - end |
| 3687 | - |
| 3688 | it "should set an existing handler on server" do |
| 3689 | Puppet::Network::Handler.stubs(:handler).with("handler").returns(true) |
| 3690 | |
| 3691 | @@ -367,20 +349,6 @@ |
| 3692 | end |
| 3693 | end |
| 3694 | |
| 3695 | - it "should pass the disable message when disabling" do |
| 3696 | - @puppetd.options.stubs(:[]).with(:disable).returns(true) |
| 3697 | - @puppetd.options.stubs(:[]).with(:disable_message).returns("message") |
| 3698 | - @agent.expects(:disable).with("message") |
| 3699 | - expect { @puppetd.enable_disable_client(@agent) }.to exit_with 0 |
| 3700 | - end |
| 3701 | - |
| 3702 | - it "should pass the default disable message when disabling without a message" do |
| 3703 | - @puppetd.options.stubs(:[]).with(:disable).returns(true) |
| 3704 | - @puppetd.options.stubs(:[]).with(:disable_message).returns(nil) |
| 3705 | - @agent.expects(:disable).with("reason not specified") |
| 3706 | - expect { @puppetd.enable_disable_client(@agent) }.to exit_with 0 |
| 3707 | - end |
| 3708 | - |
| 3709 | it "should finally exit" do |
| 3710 | expect { @puppetd.enable_disable_client(@agent) }.to exit_with 0 |
| 3711 | end |
| 3712 | |
| 3713 | === removed file 'spec/unit/util/anonymous_filelock_spec.rb' |
| 3714 | --- spec/unit/util/anonymous_filelock_spec.rb 2012-01-26 11:27:00 +0000 |
| 3715 | +++ spec/unit/util/anonymous_filelock_spec.rb 1970-01-01 00:00:00 +0000 |
| 3716 | @@ -1,78 +0,0 @@ |
| 3717 | -#!/usr/bin/env rspec |
| 3718 | -require 'spec_helper' |
| 3719 | - |
| 3720 | -require 'puppet/util/anonymous_filelock' |
| 3721 | - |
| 3722 | -describe Puppet::Util::AnonymousFilelock do |
| 3723 | - require 'puppet_spec/files' |
| 3724 | - include PuppetSpec::Files |
| 3725 | - |
| 3726 | - before(:each) do |
| 3727 | - @lockfile = tmpfile("lock") |
| 3728 | - @lock = Puppet::Util::AnonymousFilelock.new(@lockfile) |
| 3729 | - end |
| 3730 | - |
| 3731 | - it "should be anonymous" do |
| 3732 | - @lock.should be_anonymous |
| 3733 | - end |
| 3734 | - |
| 3735 | - describe "#lock" do |
| 3736 | - it "should return false if already locked" do |
| 3737 | - @lock.stubs(:locked?).returns(true) |
| 3738 | - @lock.lock.should be_false |
| 3739 | - end |
| 3740 | - |
| 3741 | - it "should return true if it successfully locked" do |
| 3742 | - @lock.lock.should be_true |
| 3743 | - end |
| 3744 | - |
| 3745 | - it "should create a lock file" do |
| 3746 | - @lock.lock |
| 3747 | - |
| 3748 | - File.should be_exists(@lockfile) |
| 3749 | - end |
| 3750 | - |
| 3751 | - it "should create a lock file containing a message" do |
| 3752 | - @lock.lock("message") |
| 3753 | - |
| 3754 | - File.read(@lockfile).should == "message" |
| 3755 | - end |
| 3756 | - end |
| 3757 | - |
| 3758 | - describe "#unlock" do |
| 3759 | - it "should return true when unlocking" do |
| 3760 | - @lock.lock |
| 3761 | - @lock.unlock.should be_true |
| 3762 | - end |
| 3763 | - |
| 3764 | - it "should return false when not locked" do |
| 3765 | - @lock.unlock.should be_false |
| 3766 | - end |
| 3767 | - |
| 3768 | - it "should clear the lock file" do |
| 3769 | - File.open(@lockfile, 'w') { |fd| fd.print("locked") } |
| 3770 | - @lock.unlock |
| 3771 | - File.should_not be_exists(@lockfile) |
| 3772 | - end |
| 3773 | - end |
| 3774 | - |
| 3775 | - it "should be locked when locked" do |
| 3776 | - @lock.lock |
| 3777 | - @lock.should be_locked |
| 3778 | - end |
| 3779 | - |
| 3780 | - it "should not be locked when not locked" do |
| 3781 | - @lock.should_not be_locked |
| 3782 | - end |
| 3783 | - |
| 3784 | - it "should not be locked when unlocked" do |
| 3785 | - @lock.lock |
| 3786 | - @lock.unlock |
| 3787 | - @lock.should_not be_locked |
| 3788 | - end |
| 3789 | - |
| 3790 | - it "should return the lock message" do |
| 3791 | - @lock.lock("lock message") |
| 3792 | - @lock.message.should == "lock message" |
| 3793 | - end |
| 3794 | -end |
| 3795 | \ No newline at end of file |
| 3796 | |
| 3797 | === removed file 'spec/unit/util/pidlock_spec.rb' |
| 3798 | --- spec/unit/util/pidlock_spec.rb 2012-02-23 18:24:48 +0000 |
| 3799 | +++ spec/unit/util/pidlock_spec.rb 1970-01-01 00:00:00 +0000 |
| 3800 | @@ -1,208 +0,0 @@ |
| 3801 | -#!/usr/bin/env rspec |
| 3802 | -require 'spec_helper' |
| 3803 | - |
| 3804 | -require 'puppet/util/pidlock' |
| 3805 | - |
| 3806 | -describe Puppet::Util::Pidlock do |
| 3807 | - require 'puppet_spec/files' |
| 3808 | - include PuppetSpec::Files |
| 3809 | - |
| 3810 | - before(:each) do |
| 3811 | - @lockfile = tmpfile("lock") |
| 3812 | - @lock = Puppet::Util::Pidlock.new(@lockfile) |
| 3813 | - end |
| 3814 | - |
| 3815 | - it "should not be anonymous" do |
| 3816 | - @lock.should_not be_anonymous |
| 3817 | - end |
| 3818 | - |
| 3819 | - describe "#lock" do |
| 3820 | - it "should not be locked at start" do |
| 3821 | - @lock.should_not be_locked |
| 3822 | - end |
| 3823 | - |
| 3824 | - it "should not be mine at start" do |
| 3825 | - @lock.should_not be_mine |
| 3826 | - end |
| 3827 | - |
| 3828 | - it "should become locked" do |
| 3829 | - @lock.lock |
| 3830 | - @lock.should be_locked |
| 3831 | - end |
| 3832 | - |
| 3833 | - it "should become mine" do |
| 3834 | - @lock.lock |
| 3835 | - @lock.should be_mine |
| 3836 | - end |
| 3837 | - |
| 3838 | - it "should be possible to lock multiple times" do |
| 3839 | - @lock.lock |
| 3840 | - lambda { @lock.lock }.should_not raise_error |
| 3841 | - end |
| 3842 | - |
| 3843 | - it "should return true when locking" do |
| 3844 | - @lock.lock.should be_true |
| 3845 | - end |
| 3846 | - |
| 3847 | - it "should return true if locked by me" do |
| 3848 | - @lock.lock |
| 3849 | - @lock.lock.should be_true |
| 3850 | - end |
| 3851 | - |
| 3852 | - it "should return false if locked by someone else" do |
| 3853 | - Process.stubs(:kill) |
| 3854 | - File.open(@lockfile, "w") { |fd| fd.print('0') } |
| 3855 | - |
| 3856 | - @lock.lock.should be_false |
| 3857 | - end |
| 3858 | - |
| 3859 | - it "should create a lock file" do |
| 3860 | - @lock.lock |
| 3861 | - File.should be_exists(@lockfile) |
| 3862 | - end |
| 3863 | - |
| 3864 | - it "should create a lock file containing our pid" do |
| 3865 | - @lock.lock |
| 3866 | - File.read(@lockfile).to_i.should == Process.pid.to_i |
| 3867 | - end |
| 3868 | - end |
| 3869 | - |
| 3870 | - describe "#unlock" do |
| 3871 | - it "should not be locked anymore" do |
| 3872 | - @lock.lock |
| 3873 | - @lock.unlock |
| 3874 | - @lock.should_not be_locked |
| 3875 | - end |
| 3876 | - |
| 3877 | - it "should return false if not locked" do |
| 3878 | - @lock.unlock.should be_false |
| 3879 | - end |
| 3880 | - |
| 3881 | - it "should return true if properly unlocked" do |
| 3882 | - @lock.lock |
| 3883 | - @lock.unlock.should be_true |
| 3884 | - end |
| 3885 | - |
| 3886 | - it "should get rid of the lock file" do |
| 3887 | - @lock.lock |
| 3888 | - @lock.unlock |
| 3889 | - File.should_not be_exists(@lockfile) |
| 3890 | - end |
| 3891 | - |
| 3892 | - it "should not warn if the lockfile was deleted by someone else" do |
| 3893 | - @lock.lock |
| 3894 | - File.unlink(@lockfile) |
| 3895 | - |
| 3896 | - Puppet.expects(:err).never # meh |
| 3897 | - @lock.unlock |
| 3898 | - end |
| 3899 | - |
| 3900 | - it "should warn if the lockfile can't be deleted" do |
| 3901 | - @lock.lock |
| 3902 | - File.expects(:unlink).with(@lockfile).raises(Errno::EIO) |
| 3903 | - Puppet.expects(:err).with do |argument| |
| 3904 | - argument.should =~ /Input\/output error/ |
| 3905 | - end |
| 3906 | - @lock.unlock |
| 3907 | - |
| 3908 | - # This is necessary because our cleanup code uses File.unlink |
| 3909 | - File.unstub(:unlink) |
| 3910 | - @lock.unlock |
| 3911 | - end |
| 3912 | - end |
| 3913 | - |
| 3914 | - describe "#locked?" do |
| 3915 | - it "should return true if locked" do |
| 3916 | - @lock.lock |
| 3917 | - @lock.should be_locked |
| 3918 | - end |
| 3919 | - end |
| 3920 | - |
| 3921 | - describe "with a stale lock" do |
| 3922 | - before(:each) do |
| 3923 | - Process.stubs(:kill).with(0, 6789) |
| 3924 | - Process.stubs(:kill).with(0, 1234).raises(Errno::ESRCH) |
| 3925 | - Process.stubs(:pid).returns(6789) |
| 3926 | - File.open(@lockfile, 'w') { |fd| fd.write("1234") } |
| 3927 | - end |
| 3928 | - |
| 3929 | - it "should not be locked" do |
| 3930 | - @lock.should_not be_locked |
| 3931 | - end |
| 3932 | - |
| 3933 | - describe "#lock" do |
| 3934 | - it "should clear stale locks" do |
| 3935 | - @lock.locked? |
| 3936 | - File.should_not be_exists(@lockfile) |
| 3937 | - end |
| 3938 | - |
| 3939 | - it "should replace with new locks" do |
| 3940 | - @lock.lock |
| 3941 | - File.should be_exists(@lockfile) |
| 3942 | - @lock.lock_pid.should == 6789 |
| 3943 | - @lock.should be_mine |
| 3944 | - @lock.should be_locked |
| 3945 | - end |
| 3946 | - end |
| 3947 | - |
| 3948 | - describe "#unlock" do |
| 3949 | - it "should not be allowed" do |
| 3950 | - @lock.unlock.should be_false |
| 3951 | - end |
| 3952 | - |
| 3953 | - it "should not remove the lock file" do |
| 3954 | - @lock.unlock |
| 3955 | - File.should be_exists(@lockfile) |
| 3956 | - end |
| 3957 | - end |
| 3958 | - end |
| 3959 | - |
| 3960 | - describe "with another process lock" do |
| 3961 | - before(:each) do |
| 3962 | - Process.stubs(:kill).with(0, 6789) |
| 3963 | - Process.stubs(:kill).with(0, 1234) |
| 3964 | - Process.stubs(:pid).returns(6789) |
| 3965 | - File.open(@lockfile, 'w') { |fd| fd.write("1234") } |
| 3966 | - end |
| 3967 | - |
| 3968 | - it "should be locked" do |
| 3969 | - @lock.should be_locked |
| 3970 | - end |
| 3971 | - |
| 3972 | - it "should not be mine" do |
| 3973 | - @lock.should_not be_mine |
| 3974 | - end |
| 3975 | - |
| 3976 | - describe "#lock" do |
| 3977 | - it "should not be possible" do |
| 3978 | - @lock.lock.should be_false |
| 3979 | - end |
| 3980 | - |
| 3981 | - it "should not overwrite the lock" do |
| 3982 | - @lock.lock |
| 3983 | - @lock.should_not be_mine |
| 3984 | - end |
| 3985 | - end |
| 3986 | - |
| 3987 | - describe "#unlock" do |
| 3988 | - it "should not be possible" do |
| 3989 | - @lock.unlock.should be_false |
| 3990 | - end |
| 3991 | - |
| 3992 | - it "should not remove the lock file" do |
| 3993 | - @lock.unlock |
| 3994 | - File.should be_exists(@lockfile) |
| 3995 | - end |
| 3996 | - |
| 3997 | - it "should still not be our lock" do |
| 3998 | - @lock.unlock |
| 3999 | - @lock.should_not be_mine |
| 4000 | - end |
| 4001 | - |
| 4002 | - it "should not warn" do |
| 4003 | - Puppet.expects(:err).never |
| 4004 | - @lock.unlock |
| 4005 | - end |
| 4006 | - end |
| 4007 | - end |
| 4008 | -end |
| 4009 | |
| 4010 | === added file 'test/util/pidlock.rb' |
| 4011 | --- test/util/pidlock.rb 1970-01-01 00:00:00 +0000 |
| 4012 | +++ test/util/pidlock.rb 2012-03-16 16:38:20 +0000 |
| 4013 | @@ -0,0 +1,126 @@ |
| 4014 | +require File.expand_path(File.dirname(__FILE__) + '/../lib/puppettest') |
| 4015 | + |
| 4016 | +require 'puppet/util/pidlock' |
| 4017 | +require 'fileutils' |
| 4018 | + |
| 4019 | +# This is *fucked* *up* |
| 4020 | +Puppet.debug = false |
| 4021 | + |
| 4022 | +class TestPuppetUtilPidlock < Test::Unit::TestCase |
| 4023 | + include PuppetTest |
| 4024 | + |
| 4025 | + def setup |
| 4026 | + super |
| 4027 | + @workdir = tstdir |
| 4028 | + end |
| 4029 | + |
| 4030 | + def teardown |
| 4031 | + super |
| 4032 | + FileUtils.rm_rf(@workdir) |
| 4033 | + end |
| 4034 | + |
| 4035 | + def test_00_basic_create |
| 4036 | + l = nil |
| 4037 | + assert_nothing_raised { l = Puppet::Util::Pidlock.new(@workdir + '/nothingmuch') } |
| 4038 | + |
| 4039 | + assert_equal Puppet::Util::Pidlock, l.class |
| 4040 | + |
| 4041 | + assert_equal @workdir + '/nothingmuch', l.lockfile |
| 4042 | + end |
| 4043 | + |
| 4044 | + def test_10_uncontended_lock |
| 4045 | + l = Puppet::Util::Pidlock.new(@workdir + '/test_lock') |
| 4046 | + |
| 4047 | + assert !l.locked? |
| 4048 | + assert !l.mine? |
| 4049 | + assert l.lock |
| 4050 | + assert l.locked? |
| 4051 | + assert l.mine? |
| 4052 | + assert !l.anonymous? |
| 4053 | + # It's OK to call lock multiple times |
| 4054 | + assert l.lock |
| 4055 | + assert l.unlock |
| 4056 | + assert !l.locked? |
| 4057 | + assert !l.mine? |
| 4058 | + end |
| 4059 | + |
| 4060 | + def test_20_someone_elses_lock |
| 4061 | + childpid = nil |
| 4062 | + l = Puppet::Util::Pidlock.new(@workdir + '/someone_elses_lock') |
| 4063 | + |
| 4064 | + # First, we need a PID that's guaranteed to be (a) used, (b) someone |
| 4065 | + # else's, and (c) around for the life of this test. |
| 4066 | + childpid = fork { loop do; sleep 10; end } |
| 4067 | + |
| 4068 | + File.open(l.lockfile, 'w') { |fd| fd.write(childpid) } |
| 4069 | + |
| 4070 | + assert l.locked? |
| 4071 | + assert !l.mine? |
| 4072 | + assert !l.lock |
| 4073 | + assert l.locked? |
| 4074 | + assert !l.mine? |
| 4075 | + assert !l.unlock |
| 4076 | + assert l.locked? |
| 4077 | + assert !l.mine? |
| 4078 | + ensure |
| 4079 | + Process.kill("KILL", childpid) unless childpid.nil? |
| 4080 | + end |
| 4081 | + |
| 4082 | + def test_30_stale_lock |
| 4083 | + # This is a bit hard to guarantee, but we need a PID that is definitely |
| 4084 | + # unused, and will stay so for the the life of this test. Our best |
| 4085 | + # bet is to create a process, get it's PID, let it die, and *then* |
| 4086 | + # lock on it. |
| 4087 | + childpid = fork { exit } |
| 4088 | + |
| 4089 | + # Now we can't continue until we're sure that the PID is dead |
| 4090 | + Process.wait(childpid) |
| 4091 | + |
| 4092 | + l = Puppet::Util::Pidlock.new(@workdir + '/stale_lock') |
| 4093 | + |
| 4094 | + # locked? should clear the lockfile |
| 4095 | + File.open(l.lockfile, 'w') { |fd| fd.write(childpid) } |
| 4096 | + assert File.exists?(l.lockfile) |
| 4097 | + assert !l.locked? |
| 4098 | + assert !File.exists?(l.lockfile) |
| 4099 | + |
| 4100 | + # lock should replace the lockfile with our own |
| 4101 | + File.open(l.lockfile, 'w') { |fd| fd.write(childpid) } |
| 4102 | + assert File.exists?(l.lockfile) |
| 4103 | + assert l.lock |
| 4104 | + assert l.locked? |
| 4105 | + assert l.mine? |
| 4106 | + |
| 4107 | + # unlock should fail, and should *not* molest the existing lockfile, |
| 4108 | + # despite it being stale |
| 4109 | + File.open(l.lockfile, 'w') { |fd| fd.write(childpid) } |
| 4110 | + assert File.exists?(l.lockfile) |
| 4111 | + assert !l.unlock |
| 4112 | + assert File.exists?(l.lockfile) |
| 4113 | + end |
| 4114 | + |
| 4115 | + def test_40_not_locked_at_all |
| 4116 | + l = Puppet::Util::Pidlock.new(@workdir + '/not_locked') |
| 4117 | + |
| 4118 | + assert !l.locked? |
| 4119 | + # We can't unlock if we don't hold the lock |
| 4120 | + assert !l.unlock |
| 4121 | + end |
| 4122 | + |
| 4123 | + def test_50_anonymous_lock |
| 4124 | + l = Puppet::Util::Pidlock.new(@workdir + '/anonymous_lock') |
| 4125 | + |
| 4126 | + assert !l.locked? |
| 4127 | + assert l.lock(:anonymous => true) |
| 4128 | + assert l.locked? |
| 4129 | + assert l.anonymous? |
| 4130 | + assert !l.mine? |
| 4131 | + assert "", File.read(l.lockfile) |
| 4132 | + assert !l.unlock |
| 4133 | + assert l.locked? |
| 4134 | + assert l.anonymous? |
| 4135 | + assert l.unlock(:anonymous => true) |
| 4136 | + assert !File.exists?(l.lockfile) |
| 4137 | + end |
| 4138 | +end |
| 4139 | + |


Hi Marc
Some initial feedback:
1) d/changelog: you appear to have jumped one two many ubuntu versions :-)
puppet (2.7.11-1ubuntu2) precise; urgency=low
* Added patch to solve puppetlabs bug #12844 puppetmaster- passenger. postinst (LP: #948983) puppetmaster- passenger. postinst (LP: #950183)
- Reverts behaviour of agent lockfiles
- Fixes unit test to readmit old behaviour
* debian/
- Fixed rack directory location
- Added proper enabling of apache2 headers mod
* debian/
- Make sure we error if puppet config print doesn't work
-- Marc Cluet <email address hidden> Fri, 09 Mar 2012 09:49:23 +0000
puppet (2.7.11-1) unstable; urgency=high
2) d/patches/ revert_ pidlock
I understand that this is the beef of this change; the DEP-3 fields should all be at the top of the patch file and you appear to have some generated content:
--- dep.debian. net/deps/ dep3/ to learn about the format. Here
The information above should follow the Patch Tagging Guidelines, please
checkout http://
are templates for supplementary fields that you might want to add:
It would also be great if you could reference the upstream pull request in the Origin: field - https:/ /github. com/puppetlabs/ puppet/ pull/551 maybe?
You can also drop the Forwarded field - its not required in this case.
The other fixes both look OK.