Merge branch 'pedropombeiro/347211/api-return-runner-paused-property' into 'master'

REST API: Deprecate 'active' property for Runners

See merge request gitlab-org/gitlab!79244

data/deprecations/14-5-runner-api-status-does-contain-paused.yml

@@ -11,7 +11,7 @@
     `online`, `offline`, or `not_connected`. Status `paused` or `active` will no longer appear.
 
     When checking if a runner is `paused`, API users are advised to check the boolean attribute
-    `active` to be `false` instead. When checking if a runner is `active`, check if `active` is `true`.
+    `paused` to be `true` instead. When checking if a runner is `active`, check if `paused` is `false`.
   stage: Verify
   tiers: [Core, Premium, Ultimate]
   issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/344648

doc/update/deprecations.md

@@ -240,7 +240,7 @@ A runner's status will only relate to runner contact status, such as:
 `online`, `offline`, or `not_connected`. Status `paused` or `active` will no longer appear.
 
 When checking if a runner is `paused`, API users are advised to check the boolean attribute
-`active` to be `false` instead. When checking if a runner is `active`, check if `active` is `true`.
+`paused` to be `true` instead. When checking if a runner is `active`, check if `paused` is `false`.
 
 **Planned removal milestone: 15.0 (2022-05-22)**
 

lib/api/ci/runner.rb

@@ -18,21 +18,25 @@ module API
           optional :maintainer_note, type: String, desc: %q(Deprecated: Use :maintenance_note instead. Runner's maintenance notes)
           optional :maintenance_note, type: String, desc: %q(Runner's maintenance notes)
           optional :info, type: Hash, desc: %q(Runner's metadata)
-          optional :active, type: Boolean, desc: 'Should Runner be active'
-          optional :locked, type: Boolean, desc: 'Should Runner be locked for current project'
+          optional :active, type: Boolean, desc: 'Deprecated: Use `:paused` instead. Should runner be active'
+          optional :paused, type: Boolean, desc: 'Whether the runner should ignore new jobs'
+          optional :locked, type: Boolean, desc: 'Whether the runner should be locked for current project'
           optional :access_level, type: String, values: ::Ci::Runner.access_levels.keys,
-                                  desc: 'The access_level of the runner'
-          optional :run_untagged, type: Boolean, desc: 'Should Runner handle untagged jobs'
+                                  desc: 'The access_level of the runner; `not_protected` or `ref_protected`'
+          optional :run_untagged, type: Boolean, desc: 'Whether the runner should handle untagged jobs'
           optional :tag_list, type: Array[String], coerce_with: ::API::Validations::Types::CommaSeparatedToArray.coerce, desc: %q(List of Runner's tags)
-          optional :maximum_timeout, type: Integer, desc: 'Maximum timeout set when this Runner will handle the job'
+          optional :maximum_timeout, type: Integer, desc: 'Maximum timeout set when this runner handles the job'
+          mutually_exclusive :maintainer_note, :maintainer_note
+          mutually_exclusive :active, :paused
         end
         post '/', feature_category: :runner do
-          attributes = attributes_for_keys(%i[description maintainer_note maintenance_note active locked run_untagged tag_list access_level maximum_timeout])
+          attributes = attributes_for_keys(%i[description maintainer_note maintenance_note active paused locked run_untagged tag_list access_level maximum_timeout])
             .merge(get_runner_details_from_request)
 
           # Pull in deprecated maintainer_note if that's the only note value available
           deprecated_note = attributes.delete(:maintainer_note)
           attributes[:maintenance_note] ||= deprecated_note if deprecated_note
+          attributes[:active] = !attributes.delete(:paused) if attributes.include?(:paused)
 
           @runner = ::Ci::RegisterRunnerService.new.execute(params[:token], attributes)
           forbidden! unless @runner

lib/api/ci/runners.rb

@@ -77,18 +77,21 @@ module API
         params do
           requires :id, type: Integer, desc: 'The ID of the runner'
           optional :description, type: String, desc: 'The description of the runner'
-          optional :active, type: Boolean, desc: 'The state of a runner'
+          optional :active, type: Boolean, desc: 'Deprecated: Use `:paused` instead. Flag indicating whether the runner is allowed to receive jobs'
+          optional :paused, type: Boolean, desc: 'Flag indicating whether the runner should ignore new jobs'
           optional :tag_list, type: Array[String], coerce_with: ::API::Validations::Types::CommaSeparatedToArray.coerce, desc: 'The list of tags for a runner'
-          optional :run_untagged, type: Boolean, desc: 'Flag indicating the runner can execute untagged jobs'
+          optional :run_untagged, type: Boolean, desc: 'Flag indicating whether the runner can execute untagged jobs'
           optional :locked, type: Boolean, desc: 'Flag indicating the runner is locked'
           optional :access_level, type: String, values: ::Ci::Runner.access_levels.keys,
                                   desc: 'The access_level of the runner'
           optional :maximum_timeout, type: Integer, desc: 'Maximum timeout set when this Runner will handle the job'
-          at_least_one_of :description, :active, :tag_list, :run_untagged, :locked, :access_level, :maximum_timeout
+          at_least_one_of :description, :active, :paused, :tag_list, :run_untagged, :locked, :access_level, :maximum_timeout
+          mutually_exclusive :active, :paused
         end
         put ':id' do
           runner = get_runner(params.delete(:id))
           authenticate_update_runner!(runner)
+          params[:active] = !params.delete(:paused) if params.include?(:paused)
           update_service = ::Ci::UpdateRunnerService.new(runner)
 
           if update_service.update(declared_params(include_missing: false))

lib/api/entities/ci/runner.rb

@@ -7,7 +7,10 @@ module API
         expose :id
         expose :description
         expose :ip_address
-        expose :active
+        expose :active # TODO Remove in %15.0 in favor of `paused` for REST calls, see https://gitlab.com/gitlab-org/gitlab/-/issues/351109
+        expose :paused do |runner|
+          !runner.active
+        end
         expose :instance_type?, as: :is_shared
         expose :runner_type
         expose :name

spec/requests/api/ci/runner/runners_post_spec.rb

@@ -34,7 +34,7 @@ RSpec.describe API::Ci::Runner, :clean_gitlab_redis_shared_state do
             run_untagged: false,
             tag_list: 'tag1, tag2',
             locked: true,
-            active: true,
+            paused: false,
             access_level: 'ref_protected',
             maximum_timeout: 9000
           }
@@ -55,7 +55,7 @@ RSpec.describe API::Ci::Runner, :clean_gitlab_redis_shared_state do
               maximum_timeout: 9000
             }.stringify_keys
 
-            allow(service).to receive(:execute)
+            expect(service).to receive(:execute)
               .once
               .with('valid token', a_hash_including(expected_params))
               .and_return(new_runner)
@@ -108,6 +108,32 @@ RSpec.describe API::Ci::Runner, :clean_gitlab_redis_shared_state do
         end
       end
 
+      context 'when deprecated active parameter is provided' do
+        def request
+          post api('/runners'), params: {
+            token: 'valid token',
+            active: false
+          }
+        end
+
+        let_it_be(:new_runner) { create(:ci_runner) }
+
+        it 'uses active value in registration' do
+          expect_next_instance_of(::Ci::RegisterRunnerService) do |service|
+            expected_params = { active: false }.stringify_keys
+
+            expect(service).to receive(:execute)
+              .once
+              .with('valid token', a_hash_including(expected_params))
+              .and_return(new_runner)
+          end
+
+          request
+
+          expect(response).to have_gitlab_http_status(:created)
+        end
+      end
+
       context 'calling actual register service' do
         include StubGitlabCalls
 

spec/requests/api/ci/runners_spec.rb

@@ -109,7 +109,7 @@ RSpec.describe API::Ci::Runners do
         get api('/runners?tag_list=tag1,tag2', user)
 
         expect(json_response).to match_array [
-          a_hash_including('description' => 'Runner tagged with tag1 and tag2')
+          a_hash_including('description' => 'Runner tagged with tag1 and tag2', 'active' => true, 'paused' => false)
         ]
       end
     end
@@ -137,7 +137,7 @@ RSpec.describe API::Ci::Runners do
           get api('/runners/all', admin)
 
           expect(json_response).to match_array [
-            a_hash_including('description' => 'Project runner', 'is_shared' => false, 'runner_type' => 'project_type'),
+            a_hash_including('description' => 'Project runner', 'is_shared' => false, 'active' => true, 'paused' => false, 'runner_type' => 'project_type'),
             a_hash_including('description' => 'Two projects runner', 'is_shared' => false, 'runner_type' => 'project_type'),
             a_hash_including('description' => 'Group runner A', 'is_shared' => false, 'runner_type' => 'group_type'),
             a_hash_including('description' => 'Group runner B', 'is_shared' => false, 'runner_type' => 'group_type'),
@@ -255,6 +255,8 @@ RSpec.describe API::Ci::Runners do
           expect(json_response['description']).to eq(shared_runner.description)
           expect(json_response['maximum_timeout']).to be_nil
           expect(json_response['status']).to eq("not_connected")
+          expect(json_response['active']).to eq(true)
+          expect(json_response['paused']).to eq(false)
         end
       end
 
@@ -359,6 +361,14 @@ RSpec.describe API::Ci::Runners do
           expect(shared_runner.reload.active).to eq(!active)
         end
 
+        it 'runner paused state' do
+          active = shared_runner.active
+          update_runner(shared_runner.id, admin, paused: active)
+
+          expect(response).to have_gitlab_http_status(:ok)
+          expect(shared_runner.reload.active).to eq(!active)
+        end
+
         it 'runner tag list' do
           update_runner(shared_runner.id, admin, tag_list: ['ruby2.1', 'pgsql', 'mysql'])
 
@@ -908,9 +918,9 @@ RSpec.describe API::Ci::Runners do
         get api("/projects/#{project.id}/runners", user)
 
         expect(json_response).to match_array [
-          a_hash_including('description' => 'Project runner'),
-          a_hash_including('description' => 'Two projects runner'),
-          a_hash_including('description' => 'Shared runner')
+          a_hash_including('description' => 'Project runner', 'active' => true, 'paused' => false),
+          a_hash_including('description' => 'Two projects runner', 'active' => true, 'paused' => false),
+          a_hash_including('description' => 'Shared runner', 'active' => true, 'paused' => false)
         ]
       end
 
@@ -986,7 +996,7 @@ RSpec.describe API::Ci::Runners do
         get api("/groups/#{group.id}/runners", user)
 
         expect(json_response).to match_array([
-          a_hash_including('description' => 'Group runner A')
+          a_hash_including('description' => 'Group runner A', 'active' => true, 'paused' => false)
         ])
       end