summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorRobert Speicher <robert@gitlab.com>2016-01-27 20:50:24 +0000
committerRobert Speicher <robert@gitlab.com>2016-01-27 20:50:24 +0000
commit40403c78ef1677a8ce048f4de78d3de32ef81797 (patch)
treeb69dbf3a3bc4a36d24ad5fc09b6316374230ee20
parent56538f9cc144c5149c412b5cf1e41e386c6441e1 (diff)
parent95d2f0fb51de2c5800d41b7c4f285ed533ffe21a (diff)
downloadgitlab-ce-40403c78ef1677a8ce048f4de78d3de32ef81797.tar.gz
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
-rw-r--r--CHANGELOG1
-rw-r--r--lib/api/helpers.rb5
-rw-r--r--lib/ci/api/builds.rb2
-rw-r--r--lib/ci/api/helpers.rb10
-rw-r--r--lib/ci/api/runners.rb1
-rw-r--r--spec/requests/ci/api/builds_spec.rb15
-rw-r--r--spec/requests/ci/api/runners_spec.rb14
7 files changed, 42 insertions, 6 deletions
diff --git a/CHANGELOG b/CHANGELOG
index e6fb2e9f508..e05095887e2 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -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
diff --git a/lib/api/helpers.rb b/lib/api/helpers.rb
index 3f528b9f7c0..9dacf7c1e86 100644
--- a/lib/api/helpers.rb
+++ b/lib/api/helpers.rb
@@ -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!
diff --git a/lib/ci/api/builds.rb b/lib/ci/api/builds.rb
index 690bbf97a89..416b0b5f0b4 100644
--- a/lib/ci/api/builds.rb
+++ b/lib/ci/api/builds.rb
@@ -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!
diff --git a/lib/ci/api/helpers.rb b/lib/ci/api/helpers.rb
index 1c91204e98c..199d62d9b8a 100644
--- a/lib/ci/api/helpers.rb
+++ b/lib/ci/api/helpers.rb
@@ -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
diff --git a/lib/ci/api/runners.rb b/lib/ci/api/runners.rb
index bfc14fe7a6b..192b1d18a51 100644
--- a/lib/ci/api/runners.rb
+++ b/lib/ci/api/runners.rb
@@ -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!
diff --git a/spec/requests/ci/api/builds_spec.rb b/spec/requests/ci/api/builds_spec.rb
index eec927102aa..244947762dd 100644
--- a/spec/requests/ci/api/builds_spec.rb
+++ b/spec/requests/ci/api/builds_spec.rb
@@ -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
diff --git a/spec/requests/ci/api/runners_spec.rb b/spec/requests/ci/api/runners_spec.rb
index 5942aa7a1b5..db8189ffb79 100644
--- a/spec/requests/ci/api/runners_spec.rb
+++ b/spec/requests/ci/api/runners_spec.rb
@@ -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