Merge branch 'pedropombeiro/342799/add-stale-runner-scope' into 'master'

Add stale scope to CI runner

See merge request gitlab-org/gitlab!74545

app/models/ci/runner.rb

@@ -40,6 +40,9 @@ module Ci
     # The `UPDATE_CONTACT_COLUMN_EVERY` defines how often the Runner DB entry can be updated
     UPDATE_CONTACT_COLUMN_EVERY = (40.minutes..55.minutes).freeze
 
+    # The `STALE_TIMEOUT` constant defines the how far past the last contact or creation date a runner will be considered stale
+    STALE_TIMEOUT = 3.months
+
     AVAILABLE_TYPES_LEGACY = %w[specific shared].freeze
     AVAILABLE_TYPES = runner_types.keys.freeze
     AVAILABLE_STATUSES = %w[active paused online offline not_connected].freeze
@@ -61,7 +64,8 @@ module Ci
     scope :active, -> { where(active: true) }
     scope :paused, -> { where(active: false) }
     scope :online, -> { where('contacted_at > ?', online_contact_time_deadline) }
-    scope :recent, -> { where('ci_runners.created_at > :date OR ci_runners.contacted_at > :date', date: 3.months.ago) }
+    scope :recent, -> { where('ci_runners.created_at >= :date OR ci_runners.contacted_at >= :date', date: stale_deadline) }
+    scope :stale, -> { where('ci_runners.created_at < :date AND (ci_runners.contacted_at IS NULL OR ci_runners.contacted_at < :date)', date: stale_deadline) }
     scope :offline, -> { where(arel_table[:contacted_at].lteq(online_contact_time_deadline)) }
     scope :not_connected, -> { where(contacted_at: nil) }
     scope :ordered, -> { order(id: :desc) }
@@ -185,6 +189,10 @@ module Ci
       ONLINE_CONTACT_TIMEOUT.ago
     end
 
+    def self.stale_deadline
+      STALE_TIMEOUT.ago
+    end
+
     def self.recent_queue_deadline
       # we add queue expiry + online
       # - contacted_at can be updated at any time within this interval
@@ -273,6 +281,10 @@ module Ci
       contacted_at && contacted_at > self.class.online_contact_time_deadline
     end
 
+    def stale?
+      [created_at, contacted_at].compact.max < self.class.stale_deadline
+    end
+
     def status
       return :not_connected unless contacted_at
 

spec/models/ci/runner_spec.rb

@@ -298,26 +298,93 @@ RSpec.describe Ci::Runner do
   describe '.recent' do
     subject { described_class.recent }
 
+    let!(:runner1) { create(:ci_runner, :instance, contacted_at: nil, created_at: 2.months.ago) }
+    let!(:runner2) { create(:ci_runner, :instance, contacted_at: nil, created_at: 3.months.ago) }
+    let!(:runner3) { create(:ci_runner, :instance, contacted_at: 1.month.ago, created_at: 2.months.ago) }
+    let!(:runner4) { create(:ci_runner, :instance, contacted_at: 1.month.ago, created_at: 3.months.ago) }
+
+    it { is_expected.to eq([runner1, runner3, runner4])}
+  end
+
+  describe '.stale' do
+    subject { described_class.stale }
+
+    let!(:runner1) { create(:ci_runner, :instance, created_at: 4.months.ago, contacted_at: 3.months.ago + 10.seconds) }
+    let!(:runner2) { create(:ci_runner, :instance, created_at: 4.months.ago, contacted_at: 3.months.ago - 1.second) }
+    let!(:runner3) { create(:ci_runner, :instance, created_at: 3.months.ago - 1.second, contacted_at: nil) }
+    let!(:runner4) { create(:ci_runner, :instance, created_at: 2.months.ago, contacted_at: nil) }
+
+    it 'returns stale runners' do
+      is_expected.to match_array([runner2, runner3])
+    end
+  end
+
+  describe '#stale?', :clean_gitlab_redis_cache do
+    let(:runner) { create(:ci_runner, :instance) }
+
+    subject { runner.stale? }
+
     before do
-      @runner1 = create(:ci_runner, :instance, contacted_at: nil, created_at: 2.months.ago)
-      @runner2 = create(:ci_runner, :instance, contacted_at: nil, created_at: 3.months.ago)
-      @runner3 = create(:ci_runner, :instance, contacted_at: 1.month.ago, created_at: 2.months.ago)
-      @runner4 = create(:ci_runner, :instance, contacted_at: 1.month.ago, created_at: 3.months.ago)
-      @runner5 = create(:ci_runner, :instance, contacted_at: 3.months.ago, created_at: 5.months.ago)
+      allow_any_instance_of(described_class).to receive(:cached_attribute).and_call_original
+      allow_any_instance_of(described_class).to receive(:cached_attribute)
+        .with(:platform).and_return("darwin")
     end
 
-    it { is_expected.to eq([@runner1, @runner3, @runner4])}
+    context 'table tests' do
+      using RSpec::Parameterized::TableSyntax
+
+      where(:created_at, :contacted_at, :expected_stale?) do
+        3.months.ago - 1.second   | 3.months.ago - 0.001.seconds | true
+        3.months.ago - 1.second   | 3.months.ago + 1.hour        | false
+        3.months.ago - 1.second   | nil                          | true
+        3.months.ago + 1.hour     | nil                          | false
+      end
+
+      with_them do
+        before do
+          runner.created_at = created_at
+        end
+
+        context 'no cache value' do
+          before do
+            stub_redis_runner_contacted_at(nil)
+            runner.contacted_at = contacted_at
+          end
+
+          specify do
+            is_expected.to eq(expected_stale?)
+          end
+        end
+
+        context 'with cache value' do
+          before do
+            runner.contacted_at = contacted_at ? contacted_at + 1.week : nil
+            stub_redis_runner_contacted_at(contacted_at.to_s)
+          end
+
+          specify do
+            is_expected.to eq(expected_stale?)
+          end
+        end
+
+        def stub_redis_runner_contacted_at(value)
+          Gitlab::Redis::Cache.with do |redis|
+            cache_key = runner.send(:cache_attribute_key)
+            expect(redis).to receive(:get).with(cache_key)
+              .and_return({ contacted_at: value }.to_json).at_least(:once)
+          end
+        end
+      end
+    end
   end
 
   describe '.online' do
     subject { described_class.online }
 
-    before do
-      @runner1 = create(:ci_runner, :instance, contacted_at: 2.hours.ago)
-      @runner2 = create(:ci_runner, :instance, contacted_at: 1.second.ago)
-    end
+    let!(:runner1) { create(:ci_runner, :instance, contacted_at: 2.hours.ago) }
+    let!(:runner2) { create(:ci_runner, :instance, contacted_at: 1.second.ago) }
 
-    it { is_expected.to eq([@runner2])}
+    it { is_expected.to match_array([runner2]) }
   end
 
   describe '#online?', :clean_gitlab_redis_cache do
@@ -393,12 +460,10 @@ RSpec.describe Ci::Runner do
   describe '.offline' do
     subject { described_class.offline }
 
-    before do
-      @runner1 = create(:ci_runner, :instance, contacted_at: 2.hours.ago)
-      @runner2 = create(:ci_runner, :instance, contacted_at: 1.second.ago)
-    end
+    let!(:runner1) { create(:ci_runner, :instance, contacted_at: 2.hours.ago) }
+    let!(:runner2) { create(:ci_runner, :instance, contacted_at: 1.second.ago) }
 
-    it { is_expected.to eq([@runner1])}
+    it { is_expected.to eq([runner1]) }
   end
 
   describe '#tick_runner_queue' do