Merge pull request #15483 from NickLaMuro/fix_heartbeat_to_file_bug

Fix pseudo heartbeating when HB file missing

app/models/miq_server/worker_management/heartbeat.rb

@@ -108,6 +108,6 @@ def workers_last_heartbeat_to_drb(w)
   end
 
   def workers_last_heartbeat_to_file(w)
-    File.exist?(w.heartbeat_file) ? File.mtime(w.heartbeat_file).utc : Time.now.utc
+    File.mtime(w.heartbeat_file).utc if File.exist?(w.heartbeat_file)
   end
 end

spec/models/miq_server/worker_management/heartbeat_spec.rb

@@ -15,5 +15,75 @@
         expect(worker.reload.last_heartbeat).to be_within(1.second).of(t)
       end
     end
+
+    # Iterating by 5 each time to allow enough spacing to be more than 1 second
+    # apart when using be_within(x).of(t)
+    context "when using file based heartbeating" do
+      let!(:first_heartbeat) { Time.now.utc }
+      let!(:heartbeat_file)  { "/path/to/worker.hb" }
+
+      around do |example|
+        ENV["WORKER_HEARTBEAT_METHOD"] = "file"
+        ENV["WORKER_HEARTBEAT_FILE"]   = heartbeat_file
+        example.run
+        ENV.delete("WORKER_HEARTBEAT_METHOD")
+        ENV.delete("WORKER_HEARTBEAT_FILE")
+      end
+
+      context "with an existing heartbeat file" do
+        it "sets initial and subsequent heartbeats" do
+          expect(File).to receive(:exist?).with(heartbeat_file).and_return(true, true)
+          expect(File).to receive(:mtime).with(heartbeat_file).and_return(first_heartbeat, first_heartbeat + 5)
+
+          [0, 5].each do |i|
+            Timecop.freeze(first_heartbeat) do
+              miq_server.worker_heartbeat(pid)
+              miq_server.validate_heartbeat(worker)
+            end
+
+            expect(worker.reload.last_heartbeat).to be_within(1.second).of(first_heartbeat + i)
+          end
+        end
+      end
+
+      context "with a missing heartbeat file" do
+        it "sets initial heartbeat only" do
+          expect(File).to receive(:exist?).with(heartbeat_file).and_return(false).exactly(4).times
+          expect(File).to receive(:mtime).with(heartbeat_file).never
+
+          # This has different results first iteration of the loop compared to
+          # the rest:
+          #   1. Sets the initial heartbeat
+          #   2. Doesn't update the worker's last_heartbeat value after that
+          #
+          # So the result from the database should not change after the first
+          # iteration of the loop
+          [0, 5, 10, 15].each do |i|
+            Timecop.freeze(first_heartbeat + i) do
+              miq_server.worker_heartbeat(pid)
+              miq_server.validate_heartbeat(worker)
+            end
+
+            expect(worker.reload.last_heartbeat).to be_within(1.second).of(first_heartbeat)
+          end
+        end
+      end
+
+      context "with a missing heartbeat file on the first validate" do
+        it "sets initial heartbeat default, and updates the heartbeat from the file second" do
+          expect(File).to receive(:exist?).with(heartbeat_file).and_return(false, true)
+          expect(File).to receive(:mtime).with(heartbeat_file).and_return(first_heartbeat + 5)
+
+          [0, 5].each do |i|
+            Timecop.freeze(first_heartbeat) do
+              miq_server.worker_heartbeat(pid)
+              miq_server.validate_heartbeat(worker)
+            end
+
+            expect(worker.reload.last_heartbeat).to be_within(1.second).of(first_heartbeat + i)
+          end
+        end
+      end
+    end
   end
 end