From 3fd1c2de7adb2e8b79297c9f238828347723b4e4 Mon Sep 17 00:00:00 2001
From: Thorsten Eckel <te@znuny.com>
Date: Mon, 12 Feb 2018 12:22:16 +0100
Subject: [PATCH] Added monitoring check for stuck ImportJob backends.

---
 app/controllers/monitoring_controller.rb      | 25 +++++++++++-----
 app/models/import_job.rb                      | 22 ++++++++++++++
 spec/models/import_job_spec.rb                | 29 +++++++++++++++++++
 .../controllers/monitoring_controller_test.rb | 19 ++++++++++++
 4 files changed, 87 insertions(+), 8 deletions(-)

diff --git a/app/controllers/monitoring_controller.rb b/app/controllers/monitoring_controller.rb
index 63188bad3..c407c7a83 100644
--- a/app/controllers/monitoring_controller.rb
+++ b/app/controllers/monitoring_controller.rb
@@ -87,15 +87,10 @@ curl http://localhost/api/v1/monitoring/health_check?token=XXX
       actions.add(:restart_failed_jobs)
     end
 
-    Setting.get('import_backends')&.each do |backend|
+    import_backends = ImportJob.backends
 
-      if !ImportJob.backend_valid?(backend)
-        logger.error "Invalid import backend '#{backend}'"
-        next
-      end
-
-      # skip deactivated backends
-      next if !backend.constantize.active?
+    # failed import jobs
+    import_backends.each do |backend|
 
       job = ImportJob.where(
         name:    backend,
@@ -111,6 +106,20 @@ curl http://localhost/api/v1/monitoring/health_check?token=XXX
       issues.push "Failed to run import backend '#{backend}'. Cause: #{error_message}"
     end
 
+    # stuck import jobs
+    import_backends.each do |backend|
+
+      job = ImportJob.where(
+        name:        backend,
+        dry_run:     false,
+        finished_at: nil,
+      ).where('updated_at <= ?', 5.minutes.ago).limit(1).first
+
+      next if job.blank?
+
+      issues.push "Stuck import backend '#{backend}' detected. Last update: #{job.updated_at}"
+    end
+
     token = Setting.get('monitoring_token')
 
     if issues.blank?
diff --git a/app/models/import_job.rb b/app/models/import_job.rb
index 34b24ace7..16bae41c4 100644
--- a/app/models/import_job.rb
+++ b/app/models/import_job.rb
@@ -143,4 +143,26 @@ class ImportJob < ApplicationModel
   rescue NameError
     false
   end
+
+  # Returns a list of valid import backends.
+  #
+  # @example
+  #  ImportJob.backends
+  #  # => ['Import::Ldap', 'Import::Exchange', ...]
+  #
+  # return [Boolean]
+  def self.backends
+    Setting.get('import_backends')&.select do |backend|
+
+      if !backend_valid?(backend)
+        logger.error "Invalid import backend '#{backend}'"
+        next
+      end
+
+      # skip deactivated backends
+      next if !backend.constantize.active?
+
+      true
+    end || []
+  end
 end
diff --git a/spec/models/import_job_spec.rb b/spec/models/import_job_spec.rb
index 2a5670cc1..0393cf416 100644
--- a/spec/models/import_job_spec.rb
+++ b/spec/models/import_job_spec.rb
@@ -176,6 +176,35 @@ RSpec.describe ImportJob do
     end
   end
 
+  describe '#backends' do
+
+    it 'returns list of backend namespaces' do
+      expect(Setting).to receive(:get).with('import_backends').and_return(['Import::Ldap'])
+      expect(Import::Ldap).to receive(:active?).and_return(true)
+
+      backends = described_class.backends
+
+      expect(backends).to be_a(Array)
+      expect(backends).not_to be_blank
+    end
+
+    it 'returns blank array if none are found' do
+      expect(Setting).to receive(:get).with('import_backends')
+      expect(described_class.backends).to eq([])
+    end
+
+    it "doesn't return invalid backends" do
+      expect(Setting).to receive(:get).with('import_backends').and_return(['Import::InvalidBackend'])
+      expect(described_class.backends).to eq([])
+    end
+
+    it "doesn't return inactive backends" do
+      expect(Setting).to receive(:get).with('import_backends').and_return(['Import::Ldap'])
+      expect(Import::Ldap).to receive(:active?).and_return(false)
+      expect(described_class.backends).to eq([])
+    end
+  end
+
   describe '.start' do
 
     it 'runs import backend and updates started_at and finished_at' do
diff --git a/test/controllers/monitoring_controller_test.rb b/test/controllers/monitoring_controller_test.rb
index 3c656db2f..e4a721f8e 100644
--- a/test/controllers/monitoring_controller_test.rb
+++ b/test/controllers/monitoring_controller_test.rb
@@ -411,6 +411,25 @@ class MonitoringControllerTest < ActionDispatch::IntegrationTest
     assert_equal(false, result['healthy'])
     assert_equal("Channel: Email::Notification out  ;unprocessable mails: 1;scheduler not running;Failed to run import backend 'Import::Ldap'. Cause: Some bad error", result['message'])
 
+    stuck_updated_at_timestamp = 15.minutes.ago
+    ImportJob.create(
+      name:        'Import::Ldap',
+      started_at:  Time.zone.now,
+      finished_at: nil,
+      updated_at:  stuck_updated_at_timestamp,
+    )
+
+    # health_check
+    get "/api/v1/monitoring/health_check?token=#{@token}", params: {}, headers: @headers
+    assert_response(200)
+
+    result = JSON.parse(@response.body)
+    assert_equal(Hash, result.class)
+    assert(result['message'])
+    assert(result['issues'])
+    assert_equal(false, result['healthy'])
+    assert_equal("Channel: Email::Notification out  ;unprocessable mails: 1;scheduler not running;Failed to run import backend 'Import::Ldap'. Cause: Some bad error;Stuck import backend 'Import::Ldap' detected. Last update: #{15.minutes.ago}", result['message'])
+
     Setting.set('ldap_integration', false)
   end