diff --git a/lib/puppet/agent/disabler.rb b/lib/puppet/agent/disabler.rb index a8b2d0fa2..34ab94bbf 100644 --- a/lib/puppet/agent/disabler.rb +++ b/lib/puppet/agent/disabler.rb @@ -1,101 +1,27 @@ require 'puppet/util/anonymous_filelock' -# -# A module that can be mixed in to provide enable/disable support for the agent. -# It would be nice if this module completely encapsulated the details of this -# process; however, it unfortunately relies on the existence of a "lockfile_path" method -# on the class it's being mixed in to. -# module Puppet::Agent::Disabler # Let the daemon run again, freely in the filesystem. def enable disable_lockfile.unlock - - # This is here for backwards compatibility with versions prior to 2.7.10 - # (see ticket #12844 and comments in the method itself). - handle_old_lockfile end - # Stop the daemon from making any catalog runs. def disable(msg='') disable_lockfile.lock(msg) end def disable_lockfile @disable_lockfile ||= Puppet::Util::AnonymousFilelock.new(lockfile_path+".disabled") @disable_lockfile end def disabled? disable_lockfile.locked? end def disable_message disable_lockfile.message end - - - - # - # This method is here for backwards compatibility (see ticket #12844). - # - # In puppet versions prior to 2.7.10, there was a single lock file (puppetdlock) that - # the agent used for two different purposes: - # - # 1. If the file exists and contains a pid, then ostensibly there is an agent process - # currently running on the system. - # 2. If the file exists but is empty, then it was most likely generated by a call to - # "puppet agent --disable", which is the means for administratively disabling the - # agent until further notice. - # - # In puppet 2.7.10, the code was improved to be a little more explicit about this - # distinction; the "--disable/--enable" operations were changed to use a distinct - # lock file (puppetdlock.disabled), to reduce ambiguity relating to the existence - # of the file. - # - # However, it is possible that a user ran "--disable" with an older version of puppet, - # and then upgraded. Therefore we need to try to detect the lock file by the old name. - # If we find it, we need to see if we can tell why it's there, delete it if possible, - # and print a warning. - # - # (cprice 2012-02-28) - # - def handle_old_lockfile - old_disable_lockfile_path = lockfile_path - - begin - contents = File.read(old_disable_lockfile_path) - rescue Errno::ENOENT => err - # The lock file must not exist, so we don't need to do anything - else - case contents - when /^$/ - # The file is empty. This almost certainly means that it was created by an old version of puppet, via - # puppet agent --disable, so we'll warn and delete it. - Puppet.warning("Found an agent lock file at path '#{old_disable_lockfile_path}'. " + - "It appears that this lock file was generated by a previous version of puppet, via a call " + - "to 'puppet agent --disable'. Deleting the empty file so that agents will be enabled.") - File.unlink(old_disable_lockfile_path) - when /^\d{3,10}$/ - # The file appears to contain a pid. This indicates that puppet believes an agent is currently running - # in another process. We'll respect that, but issue a warning just in case something else has happened, - # so that at least the user knows why "--enable" isn't necessarily guaranteed to actually enable the agent. - Puppet.warning("Found an agent lock file at path '#{old_disable_lockfile_path}'. It appears that a puppet " + - "agent process is already running (pid #{contents}). If this is not the case, please remove or rename " + - "the file in order to enable a new agent run.") - else - # The file exists, and it's not empty... but it does not appear to contain just a pid. In other words, - # we have no idea what's going on. So we'll just print a warning. - Puppet.warning("Found an agent lock file at path '#{old_disable_lockfile_path}'; unable to determine " + - "whether an existing agent is running or not. If not, please remove or rename the file in order to " + - "enable a new agent run.") - end - end - - end - private :handle_old_lockfile - - end diff --git a/spec/unit/agent/disabler_spec.rb b/spec/unit/agent/disabler_spec.rb index b6958e20a..5e43cf964 100644 --- a/spec/unit/agent/disabler_spec.rb +++ b/spec/unit/agent/disabler_spec.rb @@ -1,126 +1,60 @@ #!/usr/bin/env rspec require 'spec_helper' require 'puppet/agent' require 'puppet/agent/locker' class LockerTester include Puppet::Agent::Disabler end describe Puppet::Agent::Disabler do - before(:all) do - @lockdir = Dir.mktmpdir("disabler_spec_tmpdir") - @lockfile = File.join(@lockdir, "lock") - end - - after(:all) do - FileUtils.rm_rf(@lockdir) - end - - before(:each) do + before do @locker = LockerTester.new - @locker.stubs(:lockfile_path).returns @lockfile + @locker.stubs(:lockfile_path).returns "/my/lock" end it "should use an AnonymousFilelock instance as its disable_lockfile" do @locker.disable_lockfile.should be_instance_of(Puppet::Util::AnonymousFilelock) end it "should use 'lockfile_path' to determine its disable_lockfile path" do - @locker.expects(:lockfile_path).returns @lockfile - lock = Puppet::Util::AnonymousFilelock.new(@lockfile) - Puppet::Util::AnonymousFilelock.expects(:new).with(@lockfile + ".disabled").returns lock + @locker.expects(:lockfile_path).returns "/my/lock" + lock = Puppet::Util::AnonymousFilelock.new("/my/lock") + Puppet::Util::AnonymousFilelock.expects(:new).with("/my/lock.disabled").returns lock @locker.disable_lockfile end it "should reuse the same lock file each time" do @locker.disable_lockfile.should equal(@locker.disable_lockfile) end it "should lock the anonymous lock when disabled" do @locker.disable_lockfile.expects(:lock) @locker.disable end it "should disable with a message" do @locker.disable_lockfile.expects(:lock).with("disabled because") @locker.disable("disabled because") end it "should unlock the anonymous lock when enabled" do @locker.disable_lockfile.expects(:unlock) @locker.enable end it "should check the lock if it is disabled" do @locker.disable_lockfile.expects(:locked?) @locker.disabled? end it "should report the disable message when disabled" do @locker.disable_lockfile.expects(:message).returns("message") @locker.disable_message.should == "message" end - - describe "when enabling" do - - # this is for backwards compatibility with puppet versions prior to 2.7.10. - # for more detailed information, see the comments in the "#check_for_old_lockfile" method, - # in disabler.rb --cprice 2012-02-28 - describe "when a lockfile with the old filename already exists" do - let(:warning_prefix) { "Found an agent lock file at path '#{@lockfile}'" } - - after(:each) do - File.delete(@lockfile) if File.exists?(@lockfile) - end - - describe "when the lockfile is empty" do - before (:each) do - FileUtils.touch(@lockfile) - end - - it "should assume it was created by --disable in an old version of puppet, print a warning, and remove it" do - Puppet.expects(:warning).with { |msg| msg =~ /^#{warning_prefix}.*Deleting the empty file/ } - - @locker.enable - - File.exists?(@lockfile).should == false - end - end - - describe "when the lockfile contains a pid" do - before (:each) do - File.open(@lockfile, "w") { |f| f.print(12345) } - end - - it "should assume that there may be a running agent process, and print a warning" do - Puppet.expects(:warning).with { |msg| msg =~ /^#{warning_prefix}.*appears that a puppet agent process is already running/ } - - @locker.enable - - File.exists?(@lockfile).should == true - end - end - - describe "when the lockfile contains something other than a pid" do - before (:each) do - File.open(@lockfile, "w") { |f| f.print("Foo\nbar\n\baz") } - end - - it "should admit that it doesn't know what's going on, and print a warning" do - Puppet.expects(:warning).with { |msg| msg =~ /^#{warning_prefix}.*unable to determine whether an existing agent is running or not/ } - - @locker.enable - - File.exists?(@lockfile).should == true - end - end - end - - end end