Merge pull request #9484 from joshcooper/splaylimit_daemon

(PUP-12061) Allow splay and splaylimit to be modified in puppet.conf

Author: mhashizume

Diff for: lib/puppet/daemon.rb

@@ -157,14 +157,25 @@ 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
+    splaylimit = Puppet[:splay] ? Puppet[:splaylimit] : 0
+
+    agent_run = Puppet::Scheduler.create_job(Puppet[:runinterval], true, splaylimit) do |job|
+      if job.splay != 0
+        Puppet.info "Running agent every #{job.run_interval} seconds with splay #{job.splay} of #{job.splay_limit} seconds"
+      else
+        Puppet.info "Running agent every #{job.run_interval} seconds"
+      end
+
       # Splay for the daemon is handled in the scheduler
       agent.run(:splay => false)
     end
 
     reparse_run = Puppet::Scheduler.create_job(Puppet[:filetimeout]) do
       Puppet.settings.reparse_config_files
       agent_run.run_interval = Puppet[:runinterval]
+      # Puppet[:splaylimit] defaults to Puppet[:runinterval] so if runinterval
+      # changes, but splaylimit doesn't, we'll still recalculate splay
+      agent_run.splay_limit = Puppet[:splay] ? Puppet[:splaylimit] : 0
       if Puppet[:filetimeout] == 0
         reparse_run.disable
       else
@@ -181,6 +192,7 @@ def run_event_loop
 
     reparse_run.disable if Puppet[:filetimeout] == 0
 
+    # these are added in a different order than they are defined
     @scheduler.run_loop([reparse_run, agent_run, signal_loop])
   end
 end

Diff for: lib/puppet/scheduler/splay_job.rb

@@ -2,10 +2,10 @@
 
 module Puppet::Scheduler
   class SplayJob < Job
-    attr_reader :splay
+    attr_reader :splay, :splay_limit
 
     def initialize(run_interval, splay_limit, &block)
-      @splay = calculate_splay(splay_limit)
+      @splay, @splay_limit = calculate_splay(splay_limit)
       super(run_interval, &block)
     end
 
@@ -25,10 +25,21 @@ def ready?(time)
       end
     end
 
+    # Recalculates splay.
+    #
+    # @param splay_limit [Integer] the maximum time (in seconds) to delay before an agent's first run.
+    # @return @splay [Integer] a random integer less than or equal to the splay limit that represents the seconds to
+    # delay before next agent run.
+    def splay_limit=(splay_limit)
+      if @splay_limit != splay_limit
+        @splay, @splay_limit = calculate_splay(splay_limit)
+      end
+    end
+
     private
 
     def calculate_splay(limit)
-      rand(limit + 1)
+      [rand(limit + 1), limit]
     end
   end
 end

Diff for: spec/unit/daemon_spec.rb

@@ -60,6 +60,9 @@ def run_loop(jobs)
   end
 
   describe "when starting" do
+    let(:reparse_run) { scheduler.jobs[0] }
+    let(:agent_run) { scheduler.jobs[1] }
+
     before do
       allow(daemon).to receive(:set_signal_traps)
     end
@@ -77,7 +80,85 @@ def run_loop(jobs)
     it "disables the reparse of configs if the filetimeout is 0" do
       Puppet[:filetimeout] = 0
       daemon.start
-      expect(scheduler.jobs[0]).not_to be_enabled
+      expect(reparse_run).not_to be_enabled
+    end
+
+    it "does not splay the agent run by default" do
+      daemon.start
+      expect(agent_run.splay).to eq(0)
+    end
+
+    describe "and calculating splay" do
+      before do
+        # Set file timeout so the daemon reparses
+        Puppet[:filetimeout] = 1
+        Puppet[:splay] = true
+      end
+
+      it "recalculates when splaylimit changes" do
+        daemon.start
+
+        Puppet[:splaylimit] = 60
+        init_splay = agent_run.splay
+        next_splay = init_splay + 1
+        allow(agent_run).to receive(:rand).and_return(next_splay)
+        reparse_run.run(Time.now)
+
+        expect(agent_run.splay).to eq(next_splay)
+      end
+
+      it "does not change splay if splaylimit is unmodified" do
+        daemon.start
+
+        init_splay = agent_run.splay
+        reparse_run.run(Time.now)
+
+        expect(agent_run.splay).to eq(init_splay)
+      end
+
+      it "recalculates when splay is enabled later" do
+        Puppet[:splay] = false
+        daemon.start
+
+        Puppet[:splay] = true
+        allow(agent_run).to receive(:rand).and_return(999)
+        reparse_run.run(Time.now)
+
+        expect(agent_run.splay).to eq(999)
+      end
+
+      it "sets splay to 0 when splay is disabled" do
+        daemon.start
+
+        Puppet[:splay] = false
+        reparse_run.run(Time.now)
+
+        expect(agent_run.splay).to eq(0)
+      end
+
+      it "recalculates splay when runinterval is decreased" do
+        Puppet[:runinterval] = 60
+        daemon.start
+
+        Puppet[:runinterval] = Puppet[:runinterval] - 30
+        new_splay = agent_run.splay + 1
+        allow(agent_run).to receive(:rand).and_return(new_splay)
+        reparse_run.run(Time.now)
+
+        expect(agent_run.splay).to eq(new_splay)
+      end
+
+      it "recalculates splay when runinterval is increased" do
+        Puppet[:runinterval] = 60
+        daemon.start
+
+        Puppet[:runinterval] = Puppet[:runinterval] + 30
+        new_splay = agent_run.splay - 1
+        allow(agent_run).to receive(:rand).and_return(new_splay)
+        reparse_run.run(Time.now)
+
+        expect(agent_run.splay).to eq(new_splay)
+      end
     end
   end
 

Diff for: spec/unit/scheduler/splay_job_spec.rb

@@ -31,4 +31,21 @@
     expect(job).to receive(:splay).and_return(6)
     expect(job.ready?(start_time)).not_to be
   end
+
+  it "does not apply a splay if the splaylimit is unchanged" do
+    old_splay = job.splay
+    job.splay_limit = splay_limit
+    expect(job.splay).to eq(old_splay)
+  end
+
+  it "applies a splay if the splaylimit is changed" do
+    new_splay = 999
+    allow(job).to receive(:rand).and_return(new_splay)
+    job.splay_limit = splay_limit + 1
+    expect(job.splay).to eq(new_splay)
+  end
+
+  it "returns the splay_limit" do
+    expect(job.splay_limit).to eq(splay_limit)
+  end
 end