Commit 3ca9f3e7 authored by Pedro Pombeiro's avatar Pedro Pombeiro

REST API: Deprecate ‘active’ property for Runners

Changelog: deprecated
parent bf33d5e5
...@@ -11,7 +11,7 @@ ...@@ -11,7 +11,7 @@
`online`, `offline`, or `not_connected`. Status `paused` or `active` will no longer appear. `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 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 stage: Verify
tiers: [Core, Premium, Ultimate] tiers: [Core, Premium, Ultimate]
issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/344648 issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/344648
......
This diff is collapsed.
...@@ -240,7 +240,7 @@ A runner's status will only relate to runner contact status, such as: ...@@ -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. `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 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)** **Planned removal milestone: 15.0 (2022-05-22)**
......
...@@ -18,21 +18,25 @@ module API ...@@ -18,21 +18,25 @@ module API
optional :maintainer_note, type: String, desc: %q(Deprecated: Use :maintenance_note instead. Runner's maintenance notes) 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 :maintenance_note, type: String, desc: %q(Runner's maintenance notes)
optional :info, type: Hash, desc: %q(Runner's metadata) optional :info, type: Hash, desc: %q(Runner's metadata)
optional :active, type: Boolean, desc: 'Should Runner be active' optional :active, type: Boolean, desc: 'Deprecated: Use `:paused` instead. Should runner be active'
optional :locked, type: Boolean, desc: 'Should Runner be locked for current project' 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, optional :access_level, type: String, values: ::Ci::Runner.access_levels.keys,
desc: 'The access_level of the runner' desc: 'The access_level of the runner; `not_protected` or `ref_protected`'
optional :run_untagged, type: Boolean, desc: 'Should Runner handle untagged jobs' 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 :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 end
post '/', feature_category: :runner do 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) .merge(get_runner_details_from_request)
# Pull in deprecated maintainer_note if that's the only note value available # Pull in deprecated maintainer_note if that's the only note value available
deprecated_note = attributes.delete(:maintainer_note) deprecated_note = attributes.delete(:maintainer_note)
attributes[:maintenance_note] ||= deprecated_note if deprecated_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) @runner = ::Ci::RegisterRunnerService.new.execute(params[:token], attributes)
forbidden! unless @runner forbidden! unless @runner
......
...@@ -77,18 +77,21 @@ module API ...@@ -77,18 +77,21 @@ module API
params do params do
requires :id, type: Integer, desc: 'The ID of the runner' requires :id, type: Integer, desc: 'The ID of the runner'
optional :description, type: String, desc: 'The description 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 :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 :locked, type: Boolean, desc: 'Flag indicating the runner is locked'
optional :access_level, type: String, values: ::Ci::Runner.access_levels.keys, optional :access_level, type: String, values: ::Ci::Runner.access_levels.keys,
desc: 'The access_level of the runner' desc: 'The access_level of the runner'
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 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 end
put ':id' do put ':id' do
runner = get_runner(params.delete(:id)) runner = get_runner(params.delete(:id))
authenticate_update_runner!(runner) authenticate_update_runner!(runner)
params[:active] = !params.delete(:paused) if params.include?(:paused)
update_service = ::Ci::UpdateRunnerService.new(runner) update_service = ::Ci::UpdateRunnerService.new(runner)
if update_service.update(declared_params(include_missing: false)) if update_service.update(declared_params(include_missing: false))
......
...@@ -7,7 +7,10 @@ module API ...@@ -7,7 +7,10 @@ module API
expose :id expose :id
expose :description expose :description
expose :ip_address 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 :instance_type?, as: :is_shared
expose :runner_type expose :runner_type
expose :name expose :name
......
...@@ -34,7 +34,7 @@ RSpec.describe API::Ci::Runner, :clean_gitlab_redis_shared_state do ...@@ -34,7 +34,7 @@ RSpec.describe API::Ci::Runner, :clean_gitlab_redis_shared_state do
run_untagged: false, run_untagged: false,
tag_list: 'tag1, tag2', tag_list: 'tag1, tag2',
locked: true, locked: true,
active: true, paused: false,
access_level: 'ref_protected', access_level: 'ref_protected',
maximum_timeout: 9000 maximum_timeout: 9000
} }
...@@ -55,7 +55,7 @@ RSpec.describe API::Ci::Runner, :clean_gitlab_redis_shared_state do ...@@ -55,7 +55,7 @@ RSpec.describe API::Ci::Runner, :clean_gitlab_redis_shared_state do
maximum_timeout: 9000 maximum_timeout: 9000
}.stringify_keys }.stringify_keys
allow(service).to receive(:execute) expect(service).to receive(:execute)
.once .once
.with('valid token', a_hash_including(expected_params)) .with('valid token', a_hash_including(expected_params))
.and_return(new_runner) .and_return(new_runner)
...@@ -108,6 +108,32 @@ RSpec.describe API::Ci::Runner, :clean_gitlab_redis_shared_state do ...@@ -108,6 +108,32 @@ RSpec.describe API::Ci::Runner, :clean_gitlab_redis_shared_state do
end end
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 context 'calling actual register service' do
include StubGitlabCalls include StubGitlabCalls
......
...@@ -109,7 +109,7 @@ RSpec.describe API::Ci::Runners do ...@@ -109,7 +109,7 @@ RSpec.describe API::Ci::Runners do
get api('/runners?tag_list=tag1,tag2', user) get api('/runners?tag_list=tag1,tag2', user)
expect(json_response).to match_array [ 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
end end
...@@ -137,7 +137,7 @@ RSpec.describe API::Ci::Runners do ...@@ -137,7 +137,7 @@ RSpec.describe API::Ci::Runners do
get api('/runners/all', admin) get api('/runners/all', admin)
expect(json_response).to match_array [ 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' => '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 A', 'is_shared' => false, 'runner_type' => 'group_type'),
a_hash_including('description' => 'Group runner B', '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 ...@@ -255,6 +255,8 @@ RSpec.describe API::Ci::Runners do
expect(json_response['description']).to eq(shared_runner.description) expect(json_response['description']).to eq(shared_runner.description)
expect(json_response['maximum_timeout']).to be_nil expect(json_response['maximum_timeout']).to be_nil
expect(json_response['status']).to eq("not_connected") expect(json_response['status']).to eq("not_connected")
expect(json_response['active']).to eq(true)
expect(json_response['paused']).to eq(false)
end end
end end
...@@ -359,6 +361,14 @@ RSpec.describe API::Ci::Runners do ...@@ -359,6 +361,14 @@ RSpec.describe API::Ci::Runners do
expect(shared_runner.reload.active).to eq(!active) expect(shared_runner.reload.active).to eq(!active)
end 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 it 'runner tag list' do
update_runner(shared_runner.id, admin, tag_list: ['ruby2.1', 'pgsql', 'mysql']) update_runner(shared_runner.id, admin, tag_list: ['ruby2.1', 'pgsql', 'mysql'])
...@@ -908,9 +918,9 @@ RSpec.describe API::Ci::Runners do ...@@ -908,9 +918,9 @@ RSpec.describe API::Ci::Runners do
get api("/projects/#{project.id}/runners", user) get api("/projects/#{project.id}/runners", user)
expect(json_response).to match_array [ expect(json_response).to match_array [
a_hash_including('description' => 'Project runner'), a_hash_including('description' => 'Project runner', 'active' => true, 'paused' => false),
a_hash_including('description' => 'Two projects runner'), a_hash_including('description' => 'Two projects runner', 'active' => true, 'paused' => false),
a_hash_including('description' => 'Shared runner') a_hash_including('description' => 'Shared runner', 'active' => true, 'paused' => false)
] ]
end end
...@@ -986,7 +996,7 @@ RSpec.describe API::Ci::Runners do ...@@ -986,7 +996,7 @@ RSpec.describe API::Ci::Runners do
get api("/groups/#{group.id}/runners", user) get api("/groups/#{group.id}/runners", user)
expect(json_response).to match_array([ 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 end
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment