Commit 40403c78 authored by Robert Speicher's avatar Robert Speicher

Merge branch 'fix-ci-runners-version-update' into 'master'

Fix CI runner version not being properly updated when asked for a build

Due to broken implementation of attribute_for_keys the runner information was not updated correctly.

This MR adds test to check that such scenario will never happen again.

See merge request !2618
parents 56538f9c 95d2f0fb
......@@ -17,6 +17,7 @@ v 8.4.2 (unreleased)
- Bump required gitlab-workhorse version to bring in a fix for missing
artifacts in the build artifacts browser
- Get rid of those ugly borders on the file tree view
- Fix updating the runner information when asking for builds
- Bump gitlab_git version to 7.2.24 in order to bring in a performance
improvement when checking if a repository was empty
- Add instrumentation for Gitlab::Git::Repository instance methods so we can
......
......@@ -153,10 +153,11 @@ module API
end
def attributes_for_keys(keys, custom_params = nil)
params_hash = custom_params || params
attrs = {}
keys.each do |key|
if params[key].present? or (params.has_key?(key) and params[key] == false)
attrs[key] = params[key]
if params_hash[key].present? or (params_hash.has_key?(key) and params_hash[key] == false)
attrs[key] = params_hash[key]
end
end
ActionController::Parameters.new(attrs).permit!
......
......@@ -13,13 +13,13 @@ module Ci
post "register" do
authenticate_runner!
update_runner_last_contact
update_runner_info
required_attributes! [:token]
not_found! unless current_runner.active?
build = Ci::RegisterBuildService.new.execute(current_runner)
if build
update_runner_info
present build, with: Entities::BuildDetails
else
not_found!
......
......@@ -34,10 +34,14 @@ module Ci
@runner ||= Runner.find_by_token(params[:token].to_s)
end
def update_runner_info
def get_runner_version_from_params
return unless params["info"].present?
info = attributes_for_keys(["name", "version", "revision", "platform", "architecture"], params["info"])
current_runner.update(info)
attributes_for_keys(["name", "version", "revision", "platform", "architecture"], params["info"])
end
def update_runner_info
current_runner.assign_attributes(get_runner_version_from_params)
current_runner.save if current_runner.changed?
end
def max_artifacts_size
......
......@@ -47,6 +47,7 @@ module Ci
return forbidden! unless runner
if runner.id
runner.update(get_runner_version_from_params)
present runner, with: Entities::Runner
else
not_found!
......
......@@ -113,6 +113,21 @@ describe Ci::API::API do
expect(json_response["depends_on_builds"].count).to eq(2)
expect(json_response["depends_on_builds"][0]["name"]).to eq("rspec")
end
%w(name version revision platform architecture).each do |param|
context "updates runner #{param}" do
let(:value) { "#{param}_value" }
subject { runner.read_attribute(param.to_sym) }
it do
post ci_api("/builds/register"), token: runner.token, info: { param => value }
expect(response.status).to eq(404)
runner.reload
is_expected.to eq(value)
end
end
end
end
describe "PUT /builds/:id" do
......
......@@ -51,6 +51,20 @@ describe Ci::API::API do
expect(response.status).to eq(400)
end
%w(name version revision platform architecture).each do |param|
context "creates runner with #{param} saved" do
let(:value) { "#{param}_value" }
subject { Ci::Runner.first.read_attribute(param.to_sym) }
it do
post ci_api("/runners/register"), token: registration_token, info: { param => value }
expect(response.status).to eq(201)
is_expected.to eq(value)
end
end
end
end
describe "DELETE /runners/delete" do
......
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