(#12933) Better error message when agent is administratively disabled

Detect the difference between the cases where an agent run is
aborted due to another agent run already in progress vs. being
aborted due to the agent being administratively disabled via
'--disable', and print a more useful message for the latter case.

lib/puppet/agent.rb

@@ -31,6 +31,11 @@ def run(*args)
       Puppet.notice "Run of #{client_class} already in progress; skipping"
       return
     end
+    if disabled?
+      Puppet.notice "Skipping run of #{client_class}; administratively disabled; use 'puppet #{client_class} --enable' to re-enable."
+      return
+    end
+
     result = nil
     block_run = Puppet::Application.controlled_run do
       splay

lib/puppet/agent/locker.rb

@@ -35,6 +35,10 @@ def lockfile
   end
 
   def running?
-    lockfile.locked?
+    lockfile.locked? and !lockfile.anonymous?
+  end
+
+  def disabled?
+    lockfile.locked? and lockfile.anonymous?
   end
 end

spec/unit/agent_backward_compatibility_spec.rb

@@ -45,7 +45,7 @@ def stop
 
       # some regexes to match log messages
       let(:warning_regex) { /^Found special lockfile '#{disabled_lockfile_path}'.*renaming/ }
-      let(:run_in_progress_regex) { /^Run of .* already in progress; skipping/ }
+      let(:disabled_regex) { /^Skipping run of .*; administratively disabled/ }
 
       before(:each) do
         # create the 2.7.10 "disable" lockfile.
@@ -79,13 +79,13 @@ def stop
           FileUtils.touch(lockfile_path)
         end
 
-        it "should be recognized as 'running'" do
-          agent.should be_running
+        it "should be recognized as 'disabled'" do
+          agent.should be_disabled
         end
 
         it "should not try to start a new agent run" do
           AgentTestClient.expects(:new).never
-          Puppet.expects(:notice).with { |msg| msg =~ run_in_progress_regex  }
+          Puppet.expects(:notice).with(regexp_matches(disabled_regex))
 
           agent.run
         end
@@ -97,7 +97,7 @@ def stop
         end
 
         it "should not print the warning message" do
-          Puppet.expects(:warning).with {|msg| msg =~ warning_regex } .never
+          Puppet.expects(:warning).with(regexp_matches(warning_regex)).never
 
           agent.run
         end
@@ -109,20 +109,19 @@ def stop
         # situation.
 
         it "should recognize that the agent is disabled" do
-          # TODO change this to use "be_disabled" when that is re-introduced.
-          agent.should be_running
+          agent.should be_disabled
         end
 
         describe "when an agent run is requested" do
           it "should not try to start a new agent run" do
             AgentTestClient.expects(:new).never
-            Puppet.expects(:notice).with { |msg| msg =~ run_in_progress_regex  }
+            Puppet.expects(:notice).with(regexp_matches(disabled_regex))
 
             agent.run
           end
 
           it "should warn, remove the 2.7.10/2.7.11 lockfile, and create the 'normal' lockfile" do
-            Puppet.expects(:warning).with { |msg| msg =~ warning_regex }
+            Puppet.expects(:warning).with(regexp_matches(warning_regex))
 
             agent.run
 
@@ -133,12 +132,11 @@ def stop
 
         describe "when running --enable" do
           it "should recognize that the agent is disabled" do
-            # TODO change this to use "be_disabled" when that is re-introduced.
-            agent.should be_running
+            agent.should be_disabled
           end
 
           it "should warn and clean up the 2.7.10/2.7.11 lockfile" do
-            Puppet.expects(:warning).with { |msg| msg =~ warning_regex }
+            Puppet.expects(:warning).with(regexp_matches(warning_regex))
 
             agent.enable
 

spec/unit/agent_spec.rb

@@ -57,6 +57,7 @@ def controlled_run(&block)
     client.expects(:run)
 
     @agent.stubs(:running?).returns false
+    @agent.stubs(:disabled?).returns false
     @agent.run
   end
 
@@ -65,23 +66,34 @@ def controlled_run(&block)
     @agent.lockfile_path.should == "/my/lock"
   end
 
-  it "should be considered running if the lock file is locked" do
+  it "should be considered running if the lock file is locked and not anonymous" do
     lockfile = mock 'lockfile'
 
-    @agent.expects(:lockfile).returns lockfile
+    @agent.expects(:lockfile).returns(lockfile).twice
     lockfile.expects(:locked?).returns true
+    lockfile.expects(:anonymous?).returns false
 
     @agent.should be_running
   end
 
+  it "should be considered disabled if the lock file is locked and anonymous" do
+    lockfile = mock 'lockfile'
+
+    @agent.expects(:lockfile).returns(lockfile).at_least_once
+    lockfile.expects(:locked?).returns(true).at_least_once
+    lockfile.expects(:anonymous?).returns(true).at_least_once
+
+    @agent.should be_disabled
+  end
+
   describe "when being run" do
     before do
       @agent.stubs(:running?).returns false
+      @agent.stubs(:disabled?).returns false
     end
 
     it "should splay" do
       @agent.expects(:splay)
-      @agent.stubs(:running?).returns false
 
       @agent.run
     end
@@ -102,6 +114,12 @@ def controlled_run(&block)
       @agent.run
     end
 
+    it "should display an informative message if the agent is administratively disabled" do
+      @agent.expects(:disabled?).returns true
+      Puppet.expects(:notice).with(regexp_matches(/Skipping run of .*; administratively disabled/))
+      @agent.run
+    end
+
     it "should use Puppet::Application.controlled_run to manage process state behavior" do
       calls = sequence('calls')
       Puppet::Application.expects(:controlled_run).yields.in_sequence(calls)