summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorKamil Trzcinski <ayufan@ayufan.eu>2016-08-11 20:54:02 +0200
committerKamil Trzcinski <ayufan@ayufan.eu>2016-08-11 21:32:16 +0200
commit6a6a69f4afbe0107a75df018b662f02b5ec0166a (patch)
treeb7a49ab9b0913d90251245648a885de9442708c9
parentd983c5bd4671d989edf3741d0db0a54dcef9c3b6 (diff)
downloadgitlab-ce-6a6a69f4afbe0107a75df018b662f02b5ec0166a.tar.gz
Use state machine for pipeline event processing
-rw-r--r--app/models/ci/pipeline.rb61
-rw-r--r--app/models/commit_status.rb8
-rw-r--r--app/services/ci/create_pipeline_service.rb5
-rw-r--r--features/steps/shared/builds.rb2
-rw-r--r--spec/features/pipelines_spec.rb4
-rw-r--r--spec/lib/ci/charts_spec.rb1
-rw-r--r--spec/models/ci/pipeline_spec.rb38
-rw-r--r--spec/requests/api/builds_spec.rb4
-rw-r--r--spec/services/ci/image_for_build_service_spec.rb2
9 files changed, 54 insertions, 71 deletions
diff --git a/app/models/ci/pipeline.rb b/app/models/ci/pipeline.rb
index 8de799d6088..7a0430f277a 100644
--- a/app/models/ci/pipeline.rb
+++ b/app/models/ci/pipeline.rb
@@ -19,6 +19,37 @@ module Ci
after_save :keep_around_commits
+ state_machine :status, initial: :created do
+ event :skip do
+ transition any => :skipped
+ end
+
+ event :drop do
+ transition any => :failed
+ end
+
+ event :update_status do
+ transition any => :pending, if: ->(pipeline) { pipeline.can_transition_to?('pending') }
+ transition any => :running, if: ->(pipeline) { pipeline.can_transition_to?('running') }
+ transition any => :failed, if: ->(pipeline) { pipeline.can_transition_to?('failed') }
+ transition any => :success, if: ->(pipeline) { pipeline.can_transition_to?('success') }
+ transition any => :canceled, if: ->(pipeline) { pipeline.can_transition_to?('canceled') }
+ transition any => :skipped, if: ->(pipeline) { pipeline.can_transition_to?('skipped') }
+ end
+
+ after_transition [:created, :pending] => :running do |pipeline|
+ pipeline.update(started_at: Time.now)
+ end
+
+ after_transition any => [:success, :failed, :canceled] do |pipeline|
+ pipeline.update(finished_at: Time.now)
+ end
+
+ after_transition do |pipeline|
+ pipeline.update_duration
+ end
+ end
+
# ref can't be HEAD or SHA, can only be branch/tag name
scope :latest_successful_for, ->(ref = default_branch) do
where(ref: ref).success.order(id: :desc).limit(1)
@@ -89,16 +120,12 @@ module Ci
def cancel_running
builds.running_or_pending.each(&:cancel)
-
- reload_status!
end
def retry_failed(user)
builds.latest.failed.select(&:retryable?).each do |build|
Ci::Build.retry(build, user)
end
-
- reload_status!
end
def latest?
@@ -185,8 +212,6 @@ module Ci
def process!
Ci::ProcessPipelineService.new(project, user).execute(self)
-
- reload_status!
end
def predefined_variables
@@ -195,22 +220,22 @@ module Ci
]
end
- def reload_status!
- reload
- self.status =
- if yaml_errors.blank?
- statuses.latest.status || 'skipped'
- else
- 'failed'
- end
- self.started_at = statuses.started_at
- self.finished_at = statuses.finished_at
- self.duration = statuses.latest.duration
- save
+ def can_transition_to?(expected_status)
+ latest_status == expected_status
+ end
+
+ def update_duration
+ update(duration: statuses.latest.duration)
end
private
+ def latest_status
+ return 'failed' unless yaml_errors.blank?
+
+ statuses.latest.status || 'skipped'
+ end
+
def keep_around_commits
return unless project
diff --git a/app/models/commit_status.rb b/app/models/commit_status.rb
index 3ab44461179..64ce5431d63 100644
--- a/app/models/commit_status.rb
+++ b/app/models/commit_status.rb
@@ -74,13 +74,13 @@ class CommitStatus < ActiveRecord::Base
around_transition any => [:success, :failed, :canceled] do |commit_status, block|
block.call
- commit_status.pipeline.process! if commit_status.pipeline
+ commit_status.pipeline.try(:process!)
end
- around_transition any => [:pending, :running] do |commit_status, block|
- block.call
+ # Try to update the pipeline status
- commit_status.pipeline.reload_status! if commit_status.pipeline
+ after_transition do |commit_status, transition|
+ commit_status.pipeline.try(:update_status) unless transition.loopback?
end
end
diff --git a/app/services/ci/create_pipeline_service.rb b/app/services/ci/create_pipeline_service.rb
index 7398fd8e10a..cde856b0186 100644
--- a/app/services/ci/create_pipeline_service.rb
+++ b/app/services/ci/create_pipeline_service.rb
@@ -37,7 +37,8 @@ module Ci
end
if !ignore_skip_ci && skip_ci?
- return error('Creation of pipeline is skipped', save: save_on_errors)
+ pipeline.skip if save_on_errors
+ return pipeline
end
unless pipeline.config_builds_attributes.present?
@@ -93,7 +94,7 @@ module Ci
def error(message, save: false)
pipeline.errors.add(:base, message)
- pipeline.reload_status! if save
+ pipeline.drop if save
pipeline
end
end
diff --git a/features/steps/shared/builds.rb b/features/steps/shared/builds.rb
index c7f61da05fa..5ed5cdb759f 100644
--- a/features/steps/shared/builds.rb
+++ b/features/steps/shared/builds.rb
@@ -12,7 +12,6 @@ module SharedBuilds
step 'project has a recent build' do
@pipeline = create(:ci_empty_pipeline, project: @project, sha: @project.commit.sha, ref: 'master')
@build = create(:ci_build_with_coverage, pipeline: @pipeline)
- @pipeline.reload_status!
end
step 'recent build is successful' do
@@ -25,7 +24,6 @@ module SharedBuilds
step 'project has another build that is running' do
create(:ci_build, pipeline: @pipeline, name: 'second build', status: 'running')
- @pipeline.reload_status!
end
step 'I visit recent build details page' do
diff --git a/spec/features/pipelines_spec.rb b/spec/features/pipelines_spec.rb
index f88b8f8e60b..248e44a93aa 100644
--- a/spec/features/pipelines_spec.rb
+++ b/spec/features/pipelines_spec.rb
@@ -34,7 +34,6 @@ describe "Pipelines" do
let!(:running) { create(:ci_build, :running, pipeline: pipeline, stage: 'test', commands: 'test') }
before do
- pipeline.reload_status!
visit namespace_project_pipelines_path(project.namespace, project)
end
@@ -53,7 +52,6 @@ describe "Pipelines" do
let!(:failed) { create(:ci_build, :failed, pipeline: pipeline, stage: 'test', commands: 'test') }
before do
- pipeline.reload_status!
visit namespace_project_pipelines_path(project.namespace, project)
end
@@ -87,7 +85,6 @@ describe "Pipelines" do
let!(:running) { create(:generic_commit_status, status: 'running', pipeline: pipeline, stage: 'test') }
before do
- pipeline.reload_status!
visit namespace_project_pipelines_path(project.namespace, project)
end
@@ -104,7 +101,6 @@ describe "Pipelines" do
let!(:failed) { create(:generic_commit_status, status: 'failed', pipeline: pipeline, stage: 'test') }
before do
- pipeline.reload_status!
visit namespace_project_pipelines_path(project.namespace, project)
end
diff --git a/spec/lib/ci/charts_spec.rb b/spec/lib/ci/charts_spec.rb
index 2cd6b00dad6..034ea098193 100644
--- a/spec/lib/ci/charts_spec.rb
+++ b/spec/lib/ci/charts_spec.rb
@@ -5,7 +5,6 @@ describe Ci::Charts, lib: true do
before do
@pipeline = FactoryGirl.create(:ci_pipeline)
FactoryGirl.create(:ci_build, pipeline: @pipeline)
- @pipeline.reload_status!
end
it 'returns build times in minutes' do
diff --git a/spec/models/ci/pipeline_spec.rb b/spec/models/ci/pipeline_spec.rb
index 9a9720cfbfc..eb762276cbe 100644
--- a/spec/models/ci/pipeline_spec.rb
+++ b/spec/models/ci/pipeline_spec.rb
@@ -2,7 +2,7 @@ require 'spec_helper'
describe Ci::Pipeline, models: true do
let(:project) { FactoryGirl.create :empty_project }
- let(:pipeline) { FactoryGirl.create :ci_pipeline, project: project }
+ let(:pipeline) { FactoryGirl.create :ci_empty_pipeline, project: project }
it { is_expected.to belong_to(:project) }
it { is_expected.to belong_to(:user) }
@@ -51,25 +51,6 @@ describe Ci::Pipeline, models: true do
end
end
- describe "#finished_at" do
- let(:pipeline) { FactoryGirl.create :ci_pipeline }
-
- it "returns finished_at of latest build" do
- build = FactoryGirl.create :ci_build, pipeline: pipeline, finished_at: Time.now - 60
- FactoryGirl.create :ci_build, pipeline: pipeline, finished_at: Time.now - 120
- pipeline.reload_status!
-
- expect(pipeline.finished_at.to_i).to eq(build.finished_at.to_i)
- end
-
- it "returns nil if there is no finished build" do
- FactoryGirl.create :ci_not_started_build, pipeline: pipeline
- pipeline.reload_status!
-
- expect(pipeline.finished_at).to be_nil
- end
- end
-
describe "coverage" do
let(:project) { FactoryGirl.create :empty_project, build_coverage_regex: "/.*/" }
let(:pipeline) { FactoryGirl.create :ci_empty_pipeline, project: project }
@@ -139,31 +120,20 @@ describe Ci::Pipeline, models: true do
end
end
- describe '#reload_status!' do
+ describe '#update_counters' do
let(:pipeline) { create :ci_empty_pipeline, project: project }
- context 'dependent objects' do
- let(:commit_status) { create :commit_status, :pending, pipeline: pipeline }
-
- it 'executes reload_status! after succeeding dependent object' do
- expect(pipeline).to receive(:reload_status!).and_return(true)
-
- commit_status.success
- end
- end
-
context 'updates' do
let(:current) { Time.now.change(usec: 0) }
let(:build) { FactoryGirl.create :ci_build, pipeline: pipeline, started_at: current - 120, finished_at: current - 60 }
before do
- build
- pipeline.reload_status!
+ build.skip
end
[:status, :started_at, :finished_at, :duration].each do |param|
it "#{param}" do
- expect(pipeline.send(param)).to eq(build.send(param))
+ expect(pipeline.reload.send(param)).to eq(build.send(param))
end
end
end
diff --git a/spec/requests/api/builds_spec.rb b/spec/requests/api/builds_spec.rb
index a4cdd8f3140..966d302dfd3 100644
--- a/spec/requests/api/builds_spec.rb
+++ b/spec/requests/api/builds_spec.rb
@@ -238,10 +238,6 @@ describe API::API, api: true do
it { expect(response.headers).to include(download_headers) }
end
- before do
- pipeline.reload_status!
- end
-
context 'with regular branch' do
before do
pipeline.update(ref: 'master',
diff --git a/spec/services/ci/image_for_build_service_spec.rb b/spec/services/ci/image_for_build_service_spec.rb
index 259062406c7..c931c3e4829 100644
--- a/spec/services/ci/image_for_build_service_spec.rb
+++ b/spec/services/ci/image_for_build_service_spec.rb
@@ -14,7 +14,6 @@ module Ci
context 'branch name' do
before { allow(project).to receive(:commit).and_return(OpenStruct.new(sha: commit_sha)) }
before { build.run! }
- before { pipeline.reload_status! }
let(:image) { service.execute(project, ref: 'master') }
it { expect(image).to be_kind_of(OpenStruct) }
@@ -32,7 +31,6 @@ module Ci
context 'commit sha' do
before { build.run! }
- before { pipeline.reload_status! }
let(:image) { service.execute(project, sha: build.sha) }
it { expect(image).to be_kind_of(OpenStruct) }