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

GraphQL: Add STALE value to CiRunnerStatus enum

See merge request gitlab-org/gitlab!74619

app/graphql/resolvers/ci/runner_status_resolver.rb

+# frozen_string_literal: true
+
+module Resolvers
+  module Ci
+    # NOTE: This class was introduced to allow modifying the meaning of certain values in RunnerStatusEnum
+    # while preserving backward compatibility. It can be removed in 15.0 once the API has stabilized.
+    class RunnerStatusResolver < BaseResolver
+      type Types::Ci::RunnerStatusEnum, null: false
+
+      alias_method :runner, :object
+
+      argument :legacy_mode,
+               type: GraphQL::Types::String,
+               default_value: '14.5',
+               required: false,
+               description: 'Compatibility mode. A null value turns off compatibility mode.',
+               deprecated: { reason: 'Will be removed in 15.0. From that release onward, the field will behave as if legacyMode is null', milestone: '14.6' }
+
+      def resolve(legacy_mode:, **args)
+        runner.status(legacy_mode)
+      end
+    end
+  end
+end

app/graphql/types/ci/runner_status_enum.rb

@@ -5,24 +5,33 @@ module Types
     class RunnerStatusEnum < BaseEnum
       graphql_name 'CiRunnerStatus'
 
-      ::Ci::Runner::AVAILABLE_STATUSES.each do |status|
-        description = case status
-                      when 'active'
-                        "A runner that is not paused."
-                      when 'online'
-                        "A runner that contacted this instance within the last #{::Ci::Runner::ONLINE_CONTACT_TIMEOUT.inspect}."
-                      when 'offline'
-                        "A runner that has not contacted this instance within the last #{::Ci::Runner::ONLINE_CONTACT_TIMEOUT.inspect}."
-                      when 'not_connected'
-                        "A runner that has never contacted this instance."
-                      else
-                        "A runner that is #{status.to_s.tr('_', ' ')}."
-                      end
+      value 'ACTIVE',
+            description: 'Runner that is not paused.',
+            deprecated: { reason: 'Use CiRunnerType.active instead', milestone: '14.6' },
+            value: :active
 
-        value status.to_s.upcase,
-              description: description,
-              value: status.to_sym
-      end
+      value 'PAUSED',
+            description: 'Runner that is paused.',
+            deprecated: { reason: 'Use CiRunnerType.active instead', milestone: '14.6' },
+            value: :paused
+
+      value 'ONLINE',
+            description: "Runner that contacted this instance within the last #{::Ci::Runner::ONLINE_CONTACT_TIMEOUT.inspect}.",
+            value: :online
+
+      value 'OFFLINE',
+            description: "Runner that has not contacted this instance within the last #{::Ci::Runner::ONLINE_CONTACT_TIMEOUT.inspect}.",
+            deprecated: { reason: 'This field will have a slightly different scope starting in 15.0, with STALE being returned after a certain period offline', milestone: '14.6' },
+            value: :offline
+
+      value 'STALE',
+            description: "Runner that has not contacted this instance within the last #{::Ci::Runner::STALE_TIMEOUT.inspect}. Only available if legacyMode is null. Will be a possible return value starting in 15.0",
+            value: :stale
+
+      value 'NOT_CONNECTED',
+            description: 'Runner that has never contacted this instance.',
+            deprecated: { reason: 'This field will have a slightly different scope starting in 15.0, with STALE being returned after a certain period of no contact', milestone: '14.6' },
+            value: :not_connected
     end
   end
 end

app/graphql/types/ci/runner_type.rb

@@ -27,8 +27,11 @@ module Types
             description: 'Access level of the runner.'
       field :active, GraphQL::Types::Boolean, null: false,
             description: 'Indicates the runner is allowed to receive jobs.'
-      field :status, ::Types::Ci::RunnerStatusEnum, null: false,
-            description: 'Status of the runner.'
+      field :status,
+            Types::Ci::RunnerStatusEnum,
+            null: false,
+            description: 'Status of the runner.',
+            resolver: ::Resolvers::Ci::RunnerStatusResolver
       field :version, GraphQL::Types::String, null: true,
             description: 'Version of the runner.'
       field :short_sha, GraphQL::Types::String, null: true,
@@ -50,7 +53,7 @@ module Types
       field :job_count, GraphQL::Types::Int, null: true,
             description: "Number of jobs processed by the runner (limited to #{JOB_COUNT_LIMIT}, plus one to indicate that more items exist)."
       field :admin_url, GraphQL::Types::String, null: true,
-            description: 'Admin URL of the runner. Only available for adminstrators.'
+            description: 'Admin URL of the runner. Only available for administrators.'
 
       def job_count
         # We limit to 1 above the JOB_COUNT_LIMIT to indicate that more items exist after JOB_COUNT_LIMIT

app/models/ci/runner.rb

@@ -44,7 +44,7 @@ module Ci
 
     AVAILABLE_TYPES_LEGACY = %w[specific shared].freeze
     AVAILABLE_TYPES = runner_types.keys.freeze
-    AVAILABLE_STATUSES = %w[active paused online offline not_connected].freeze
+    AVAILABLE_STATUSES = %w[active paused online offline not_connected stale].freeze
     AVAILABLE_SCOPES = (AVAILABLE_TYPES_LEGACY + AVAILABLE_TYPES + AVAILABLE_STATUSES).freeze
 
     FORM_EDITABLE = %i[description tag_list active run_untagged locked access_level maximum_timeout_human_readable].freeze
@@ -287,10 +287,15 @@ module Ci
     end
 
     def stale?
+      return false unless created_at
+
       [created_at, contacted_at].compact.max < self.class.stale_deadline
     end
 
-    def status
+    def status(legacy_mode = nil)
+      return deprecated_rest_status if legacy_mode == '14.5'
+
+      return :stale if stale?
       return :not_connected unless contacted_at
 
       online? ? :online : :offline

doc/api/graphql/reference/index.md

@@ -8750,7 +8750,7 @@ Represents the total number of issues and their weights for a particular day.
 | ---- | ---- | ----------- |
 | <a id="cirunneraccesslevel"></a>`accessLevel` | [`CiRunnerAccessLevel!`](#cirunneraccesslevel) | Access level of the runner. |
 | <a id="cirunneractive"></a>`active` | [`Boolean!`](#boolean) | Indicates the runner is allowed to receive jobs. |
-| <a id="cirunneradminurl"></a>`adminUrl` | [`String`](#string) | Admin URL of the runner. Only available for adminstrators. |
+| <a id="cirunneradminurl"></a>`adminUrl` | [`String`](#string) | Admin URL of the runner. Only available for administrators. |
 | <a id="cirunnercontactedat"></a>`contactedAt` | [`Time`](#time) | Last contact from the runner. |
 | <a id="cirunnerdescription"></a>`description` | [`String`](#string) | Description of the runner. |
 | <a id="cirunnerid"></a>`id` | [`CiRunnerID!`](#cirunnerid) | ID of the runner. |
@@ -8765,11 +8765,24 @@ Represents the total number of issues and their weights for a particular day.
 | <a id="cirunnerrununtagged"></a>`runUntagged` | [`Boolean!`](#boolean) | Indicates the runner is able to run untagged jobs. |
 | <a id="cirunnerrunnertype"></a>`runnerType` | [`CiRunnerType!`](#cirunnertype) | Type of the runner. |
 | <a id="cirunnershortsha"></a>`shortSha` | [`String`](#string) | First eight characters of the runner's token used to authenticate new job requests. Used as the runner's unique ID. |
-| <a id="cirunnerstatus"></a>`status` | [`CiRunnerStatus!`](#cirunnerstatus) | Status of the runner. |
 | <a id="cirunnertaglist"></a>`tagList` | [`[String!]`](#string) | Tags associated with the runner. |
 | <a id="cirunneruserpermissions"></a>`userPermissions` | [`RunnerPermissions!`](#runnerpermissions) | Permissions for the current user on the resource. |
 | <a id="cirunnerversion"></a>`version` | [`String`](#string) | Version of the runner. |
 
+#### Fields with arguments
+
+##### `CiRunner.status`
+
+Status of the runner.
+
+Returns [`CiRunnerStatus!`](#cirunnerstatus).
+
+###### Arguments
+
+| Name | Type | Description |
+| ---- | ---- | ----------- |
+| <a id="cirunnerstatuslegacymode"></a>`legacyMode` **{warning-solid}** | [`String`](#string) | **Deprecated** in 14.6. Will be removed in 15.0. From that release onward, the field will behave as if legacyMode is null. |
+
 ### `CiStage`
 
 #### Fields
@@ -15956,11 +15969,12 @@ Values for sorting runners.
 
 | Value | Description |
 | ----- | ----------- |
-| <a id="cirunnerstatusactive"></a>`ACTIVE` | A runner that is not paused. |
-| <a id="cirunnerstatusnot_connected"></a>`NOT_CONNECTED` | A runner that has never contacted this instance. |
-| <a id="cirunnerstatusoffline"></a>`OFFLINE` | A runner that has not contacted this instance within the last 2 hours. |
-| <a id="cirunnerstatusonline"></a>`ONLINE` | A runner that contacted this instance within the last 2 hours. |
-| <a id="cirunnerstatuspaused"></a>`PAUSED` | A runner that is paused. |
+| <a id="cirunnerstatusactive"></a>`ACTIVE` **{warning-solid}** | **Deprecated** in 14.6. Use CiRunnerType.active instead. |
+| <a id="cirunnerstatusnot_connected"></a>`NOT_CONNECTED` **{warning-solid}** | **Deprecated** in 14.6. This field will have a slightly different scope starting in 15.0, with STALE being returned after a certain period of no contact. |
+| <a id="cirunnerstatusoffline"></a>`OFFLINE` **{warning-solid}** | **Deprecated** in 14.6. This field will have a slightly different scope starting in 15.0, with STALE being returned after a certain period offline. |
+| <a id="cirunnerstatusonline"></a>`ONLINE` | Runner that contacted this instance within the last 2 hours. |
+| <a id="cirunnerstatuspaused"></a>`PAUSED` **{warning-solid}** | **Deprecated** in 14.6. Use CiRunnerType.active instead. |
+| <a id="cirunnerstatusstale"></a>`STALE` | Runner that has not contacted this instance within the last 3 months. Only available if legacyMode is null. Will be a possible return value starting in 15.0. |
 
 ### `CiRunnerType`
 

spec/graphql/resolvers/ci/runner_status_resolver_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Resolvers::Ci::RunnerStatusResolver do
+  include GraphqlHelpers
+
+  describe '#resolve' do
+    let(:user) { build(:user) }
+    let(:runner) { build(:ci_runner) }
+
+    subject(:resolve_subject) { resolve(described_class, ctx: { current_user: user }, obj: runner, args: args) }
+
+    context 'with legacy_mode' do
+      context 'set to 14.5' do
+        let(:args) do
+          { legacy_mode: '14.5' }
+        end
+
+        it 'calls runner.status with specified legacy_mode' do
+          expect(runner).to receive(:status).with('14.5').once.and_return(:online)
+
+          expect(resolve_subject).to eq(:online)
+        end
+      end
+
+      context 'set to nil' do
+        let(:args) do
+          { legacy_mode: nil }
+        end
+
+        it 'calls runner.status with specified legacy_mode' do
+          expect(runner).to receive(:status).with(nil).once.and_return(:stale)
+
+          expect(resolve_subject).to eq(:stale)
+        end
+      end
+    end
+  end
+end

spec/models/ci/runner_spec.rb

@@ -342,6 +342,7 @@ RSpec.describe Ci::Runner do
       using RSpec::Parameterized::TableSyntax
 
       where(:created_at, :contacted_at, :expected_stale?) do
+        nil                       | nil                          | false
         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
@@ -376,6 +377,8 @@ RSpec.describe Ci::Runner do
         end
 
         def stub_redis_runner_contacted_at(value)
+          return unless created_at
+
           Gitlab::Redis::Cache.with do |redis|
             cache_key = runner.send(:cache_attribute_key)
             expect(redis).to receive(:get).with(cache_key)
@@ -419,7 +422,7 @@ RSpec.describe Ci::Runner do
         it { is_expected.to be_falsey }
       end
 
-      context 'contacted long time ago time' do
+      context 'contacted long time ago' do
         before do
           runner.contacted_at = 1.year.ago
         end
@@ -437,7 +440,7 @@ RSpec.describe Ci::Runner do
     end
 
     context 'with cache value' do
-      context 'contacted long time ago time' do
+      context 'contacted long time ago' do
         before do
           runner.contacted_at = 1.year.ago
           stub_redis_runner_contacted_at(1.year.ago.to_s)
@@ -699,16 +702,33 @@ RSpec.describe Ci::Runner do
   end
 
   describe '#status' do
-    let(:runner) { build(:ci_runner, :instance) }
+    let(:runner) { build(:ci_runner, :instance, created_at: 4.months.ago) }
+    let(:legacy_mode) { }
 
-    subject { runner.status }
+    subject { runner.status(legacy_mode) }
 
     context 'never connected' do
       before do
         runner.contacted_at = nil
       end
 
-      it { is_expected.to eq(:not_connected) }
+      context 'with legacy_mode enabled' do
+        let(:legacy_mode) { '14.5' }
+
+        it { is_expected.to eq(:not_connected) }
+      end
+
+      context 'with legacy_mode disabled' do
+        it { is_expected.to eq(:stale) }
+      end
+
+      context 'created recently' do
+        before do
+          runner.created_at = 1.day.ago
+        end
+
+        it { is_expected.to eq(:not_connected) }
+      end
     end
 
     context 'inactive but online' do
@@ -717,7 +737,15 @@ RSpec.describe Ci::Runner do
         runner.active = false
       end
 
-      it { is_expected.to eq(:online) }
+      context 'with legacy_mode enabled' do
+        let(:legacy_mode) { '14.5' }
+
+        it { is_expected.to eq(:paused) }
+      end
+
+      context 'with legacy_mode disabled' do
+        it { is_expected.to eq(:online) }
+      end
     end
 
     context 'contacted 1s ago' do
@@ -728,13 +756,29 @@ RSpec.describe Ci::Runner do
       it { is_expected.to eq(:online) }
     end
 
-    context 'contacted long time ago' do
+    context 'contacted recently' do
       before do
-        runner.contacted_at = 1.year.ago
+        runner.contacted_at = (3.months - 1.hour).ago
       end
 
       it { is_expected.to eq(:offline) }
     end
+
+    context 'contacted long time ago' do
+      before do
+        runner.contacted_at = (3.months + 1.second).ago
+      end
+
+      context 'with legacy_mode enabled' do
+        let(:legacy_mode) { '14.5' }
+
+        it { is_expected.to eq(:offline) }
+      end
+
+      context 'with legacy_mode disabled' do
+        it { is_expected.to eq(:stale) }
+      end
+    end
   end
 
   describe '#deprecated_rest_status' do

spec/requests/api/graphql/ci/runner_spec.rb

@@ -63,7 +63,7 @@ RSpec.describe 'Query.runner(id)' do
         'revision' => runner.revision,
         'locked' => false,
         'active' => runner.active,
-        'status' => runner.status.to_s.upcase,
+        'status' => runner.status('14.5').to_s.upcase,
         'maximumTimeout' => runner.maximum_timeout,
         'accessLevel' => runner.access_level.to_s.upcase,
         'runUntagged' => runner.run_untagged,
@@ -221,6 +221,45 @@ RSpec.describe 'Query.runner(id)' do
     end
   end
 
+  describe 'for runner with status' do
+    let_it_be(:stale_runner) { create(:ci_runner, description: 'Stale runner 1', created_at: 3.months.ago) }
+
+    let(:query) do
+      %(
+        query {
+          staleRunner: runner(id: "#{stale_runner.to_global_id}") {
+            status
+            legacyStatusWithExplicitVersion: status(legacyMode: "14.5")
+            newStatus: status(legacyMode: null)
+          }
+          pausedRunner: runner(id: "#{inactive_instance_runner.to_global_id}") {
+            status
+            legacyStatusWithExplicitVersion: status(legacyMode: "14.5")
+            newStatus: status(legacyMode: null)
+          }
+        }
+      )
+    end
+
+    it 'retrieves status fields with expected values' do
+      post_graphql(query, current_user: user)
+
+      stale_runner_data = graphql_data_at(:stale_runner)
+      expect(stale_runner_data).to match a_hash_including(
+        'status' => 'NOT_CONNECTED',
+        'legacyStatusWithExplicitVersion' => 'NOT_CONNECTED',
+        'newStatus' => 'STALE'
+      )
+
+      paused_runner_data = graphql_data_at(:paused_runner)
+      expect(paused_runner_data).to match a_hash_including(
+        'status' => 'PAUSED',
+        'legacyStatusWithExplicitVersion' => 'PAUSED',
+        'newStatus' => 'OFFLINE'
+      )
+    end
+  end
+
   describe 'for multiple runners' do
     let_it_be(:project1) { create(:project, :test_repo) }
     let_it_be(:project2) { create(:project, :test_repo) }