(PUP-12061) Handle changing splay in puppet.conf

Previously, splay could not be enabled or disabled in puppet.conf if the
daemonized agent was already running.

If the daemon was started with splay disabled, then enabling it would cause a
NoMethodError, when trying to call `splay_limit` on a regular Job.

If the daemon was started with splay enabled, then disabling it would have no
effect, since we never recalculated the splay limit.

To handle situations where `splay` may be enabled or disabled in puppet.conf,
always create a SplayJob for the agent_run job and set its `splay_limit` to
either the limit or 0, respectively. Note setting a `splay_limit` to 0 causes
the splay offset to also be set to 0 because `rand(1)` always returns 0.

Author: joshcooper

lib/puppet/daemon.rb

@@ -157,7 +157,9 @@ def remove_pidfile
 
   # Loop forever running events - or, at least, until we exit.
   def run_event_loop
-    agent_run = Puppet::Scheduler.create_job(Puppet[:runinterval], Puppet[:splay], Puppet[:splaylimit]) do |job|
+    splaylimit = Puppet[:splay] ? Puppet[:splaylimit] : 0
+
+    agent_run = Puppet::Scheduler.create_job(Puppet[:runinterval], true, splaylimit) do |job|
       Puppet.info "Running agent #{job}"
 
       # Splay for the daemon is handled in the scheduler
@@ -167,7 +169,7 @@ def run_event_loop
     reparse_run = Puppet::Scheduler.create_job(Puppet[:filetimeout]) do
       Puppet.settings.reparse_config_files
       agent_run.run_interval = Puppet[:runinterval]
-      agent_run.splay_limit = Puppet[:splaylimit] if Puppet[:splay]
+      agent_run.splay_limit = Puppet[:splay] ? Puppet[:splaylimit] : 0
       if Puppet[:filetimeout] == 0
         reparse_run.disable
       else

spec/unit/daemon_spec.rb

@@ -84,7 +84,7 @@ def run_loop(jobs)
     it "does not splay the agent run by default" do
       daemon.start
       agent_run = scheduler.jobs[1]
-      expect(agent_run).to be_an_instance_of(Puppet::Scheduler::Job)
+      expect(agent_run.splay).to eq(0)
     end
 
     it "recalculates splay if splaylimit changes" do
@@ -124,6 +124,42 @@ def run_loop(jobs)
 
       expect(agent_run.splay).to eq(init_splay)
     end
+
+    it "recalculates splay if splay is enabled later" do
+      # Set file timeout so the daemon reparses
+      Puppet[:filetimeout] = 1
+      Puppet[:splay] = false
+      daemon.start
+
+      # enable splay
+      Puppet[:splay] = true
+
+      agent_run = scheduler.jobs[1]
+      allow(agent_run).to receive(:rand).and_return(999)
+
+      # run the reparse job
+      reparse_run = scheduler.jobs[0]
+      reparse_run.run(Time.now)
+
+      expect(agent_run.splay).to eq(999)
+    end
+
+    it "sets splay to 0 if splay is disabled" do
+      # Set file timeout so the daemon reparses
+      Puppet[:filetimeout] = 1
+      Puppet[:splay] = true
+      daemon.start
+
+      # disable splay
+      Puppet[:splay] = false
+
+      # run the reparse job
+      reparse_run = scheduler.jobs[0]
+      reparse_run.run(Time.now)
+
+      agent_run = scheduler.jobs[1]
+      expect(agent_run.splay).to eq(0)
+    end
   end
 
   describe "when stopping" do