From 161af17c1b69e7e00aefcd4f540a55755259ceda Mon Sep 17 00:00:00 2001 From: Kamil Trzcinski Date: Wed, 24 May 2017 15:13:51 +0200 Subject: [PATCH] Introduce source to pipeline entity --- .../projects/pipelines_controller.rb | 2 +- app/models/ci/pipeline.rb | 15 ++++++++--- app/models/project.rb | 5 ---- app/serializers/pipeline_entity.rb | 2 +- app/services/ci/create_pipeline_service.rb | 3 ++- .../ci/create_trigger_request_service.rb | 2 +- app/services/git_push_service.rb | 2 +- app/services/git_tag_push_service.rb | 2 +- app/workers/pipeline_schedule_worker.rb | 2 +- .../introduce-source-to-pipelines.yml | 4 +++ db/fixtures/development/14_pipelines.rb | 2 +- db/fixtures/development/17_cycle_analytics.rb | 2 +- ...0170524125940_add_source_to_ci_pipeline.rb | 9 +++++++ db/schema.rb | 3 ++- features/steps/project/pages.rb | 2 +- lib/api/commit_statuses.rb | 9 ++++++- lib/api/pipelines.rb | 2 +- spec/factories/ci/pipelines.rb | 1 + .../projects/pipelines/pipelines_spec.rb | 2 ++ .../import_export/safe_model_attributes.yml | 1 + spec/models/ci/pipeline_spec.rb | 26 +++++++++++++++++-- spec/requests/api/commit_statuses_spec.rb | 4 +-- spec/requests/api/commits_spec.rb | 4 +-- spec/requests/api/v3/commits_spec.rb | 4 +-- spec/serializers/pipeline_entity_spec.rb | 4 +-- .../ci/create_pipeline_service_spec.rb | 5 ++-- .../ci/create_trigger_request_service_spec.rb | 2 ++ spec/services/git_push_service_spec.rb | 13 ++++++++++ spec/services/git_tag_push_service_spec.rb | 14 ++++++++++ spec/workers/pipeline_schedule_worker_spec.rb | 3 ++- 30 files changed, 116 insertions(+), 35 deletions(-) create mode 100644 changelogs/unreleased/introduce-source-to-pipelines.yml create mode 100644 db/migrate/20170524125940_add_source_to_ci_pipeline.rb diff --git a/app/controllers/projects/pipelines_controller.rb b/app/controllers/projects/pipelines_controller.rb index 602d3dd8c1c..87ec0df257a 100644 --- a/app/controllers/projects/pipelines_controller.rb +++ b/app/controllers/projects/pipelines_controller.rb @@ -58,7 +58,7 @@ class Projects::PipelinesController < Projects::ApplicationController def create @pipeline = Ci::CreatePipelineService .new(project, current_user, create_params) - .execute(ignore_skip_ci: true, save_on_errors: false) + .execute(:web, ignore_skip_ci: true, save_on_errors: false) if @pipeline.persisted? redirect_to namespace_project_pipeline_path(project.namespace, project, @pipeline) diff --git a/app/models/ci/pipeline.rb b/app/models/ci/pipeline.rb index 81c30b0e077..425ca9278eb 100644 --- a/app/models/ci/pipeline.rb +++ b/app/models/ci/pipeline.rb @@ -30,6 +30,7 @@ module Ci delegate :id, to: :project, prefix: true + validates :source, exclusion: { in: %w(unknown), unless: :importing? }, on: :create validates :sha, presence: { unless: :importing? } validates :ref, presence: { unless: :importing? } validates :status, presence: { unless: :importing? } @@ -37,6 +38,16 @@ module Ci after_create :keep_around_commits, unless: :importing? + enum source: { + unknown: nil, + push: 1, + web: 2, + trigger: 3, + schedule: 4, + api: 5, + external: 6 + } + state_machine :status, initial: :created do event :enqueue do transition created: :pending @@ -269,10 +280,6 @@ module Ci commit.sha == sha end - def triggered? - trigger_requests.any? - end - def retried @retried ||= (statuses.order(id: :desc) - statuses.latest) end diff --git a/app/models/project.rb b/app/models/project.rb index a59095cb25c..6d0b722319c 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -1064,11 +1064,6 @@ class Project < ActiveRecord::Base pipelines.order(id: :desc).find_by(sha: sha, ref: ref) end - def ensure_pipeline(ref, sha, current_user = nil) - pipeline_for(ref, sha) || - pipelines.create(sha: sha, ref: ref, user: current_user) - end - def enable_ci project_feature.update_attribute(:builds_access_level, ProjectFeature::ENABLED) end diff --git a/app/serializers/pipeline_entity.rb b/app/serializers/pipeline_entity.rb index ea57cc97a7e..486f8c36fbd 100644 --- a/app/serializers/pipeline_entity.rb +++ b/app/serializers/pipeline_entity.rb @@ -5,6 +5,7 @@ class PipelineEntity < Grape::Entity expose :user, using: UserEntity expose :active?, as: :active expose :coverage + expose :source expose :path do |pipeline| namespace_project_pipeline_path( @@ -24,7 +25,6 @@ class PipelineEntity < Grape::Entity expose :flags do expose :latest?, as: :latest - expose :triggered?, as: :triggered expose :stuck?, as: :stuck expose :has_yaml_errors?, as: :yaml_errors expose :can_retry?, as: :retryable diff --git a/app/services/ci/create_pipeline_service.rb b/app/services/ci/create_pipeline_service.rb index a98b7167765..8227a78a650 100644 --- a/app/services/ci/create_pipeline_service.rb +++ b/app/services/ci/create_pipeline_service.rb @@ -2,8 +2,9 @@ module Ci class CreatePipelineService < BaseService attr_reader :pipeline - def execute(ignore_skip_ci: false, save_on_errors: true, trigger_request: nil, schedule: nil) + def execute(source, ignore_skip_ci: false, save_on_errors: true, trigger_request: nil, schedule: nil) @pipeline = Ci::Pipeline.new( + source: source, project: project, ref: ref, sha: sha, diff --git a/app/services/ci/create_trigger_request_service.rb b/app/services/ci/create_trigger_request_service.rb index 8362f01ddb8..beb27a5a597 100644 --- a/app/services/ci/create_trigger_request_service.rb +++ b/app/services/ci/create_trigger_request_service.rb @@ -4,7 +4,7 @@ module Ci trigger_request = trigger.trigger_requests.create(variables: variables) pipeline = Ci::CreatePipelineService.new(project, trigger.owner, ref: ref). - execute(ignore_skip_ci: true, trigger_request: trigger_request) + execute(:trigger, ignore_skip_ci: true, trigger_request: trigger_request) trigger_request if pipeline.persisted? end diff --git a/app/services/git_push_service.rb b/app/services/git_push_service.rb index d22236b961b..f080e6326a1 100644 --- a/app/services/git_push_service.rb +++ b/app/services/git_push_service.rb @@ -106,7 +106,7 @@ class GitPushService < BaseService EventCreateService.new.push(@project, current_user, build_push_data) @project.execute_hooks(build_push_data.dup, :push_hooks) @project.execute_services(build_push_data.dup, :push_hooks) - Ci::CreatePipelineService.new(@project, current_user, build_push_data).execute + Ci::CreatePipelineService.new(@project, current_user, build_push_data).execute(:push) if push_remove_branch? AfterBranchDeleteService diff --git a/app/services/git_tag_push_service.rb b/app/services/git_tag_push_service.rb index 96432837481..7c424fba428 100644 --- a/app/services/git_tag_push_service.rb +++ b/app/services/git_tag_push_service.rb @@ -11,7 +11,7 @@ class GitTagPushService < BaseService SystemHooksService.new.execute_hooks(build_system_push_data.dup, :tag_push_hooks) project.execute_hooks(@push_data.dup, :tag_push_hooks) project.execute_services(@push_data.dup, :tag_push_hooks) - Ci::CreatePipelineService.new(project, current_user, @push_data).execute + Ci::CreatePipelineService.new(project, current_user, @push_data).execute(:push) ProjectCacheWorker.perform_async(project.id, [], [:commit_count, :repository_size]) true diff --git a/app/workers/pipeline_schedule_worker.rb b/app/workers/pipeline_schedule_worker.rb index 7eb0e84acb2..7b485b3363c 100644 --- a/app/workers/pipeline_schedule_worker.rb +++ b/app/workers/pipeline_schedule_worker.rb @@ -14,7 +14,7 @@ class PipelineScheduleWorker Ci::CreatePipelineService.new(schedule.project, schedule.owner, ref: schedule.ref) - .execute(save_on_errors: false, schedule: schedule) + .execute(:schedule, save_on_errors: false, schedule: schedule) rescue => e Rails.logger.error "#{schedule.id}: Failed to create a scheduled pipeline: #{e.message}" ensure diff --git a/changelogs/unreleased/introduce-source-to-pipelines.yml b/changelogs/unreleased/introduce-source-to-pipelines.yml new file mode 100644 index 00000000000..7898bd31b39 --- /dev/null +++ b/changelogs/unreleased/introduce-source-to-pipelines.yml @@ -0,0 +1,4 @@ +--- +title: Introduce source to Pipeline entity +merge_request: +author: diff --git a/db/fixtures/development/14_pipelines.rb b/db/fixtures/development/14_pipelines.rb index 3c42f7db6d5..68767f0e585 100644 --- a/db/fixtures/development/14_pipelines.rb +++ b/db/fixtures/development/14_pipelines.rb @@ -98,7 +98,7 @@ class Gitlab::Seeder::Pipelines def create_pipeline!(project, ref, commit) - project.pipelines.create(sha: commit.id, ref: ref) + project.pipelines.create(sha: commit.id, ref: ref, source: :push) end def build_create!(pipeline, opts = {}) diff --git a/db/fixtures/development/17_cycle_analytics.rb b/db/fixtures/development/17_cycle_analytics.rb index 0d7eb1a7c93..75457b2d369 100644 --- a/db/fixtures/development/17_cycle_analytics.rb +++ b/db/fixtures/development/17_cycle_analytics.rb @@ -190,7 +190,7 @@ class Gitlab::Seeder::CycleAnalytics service = Ci::CreatePipelineService.new(merge_request.project, @user, ref: "refs/heads/#{merge_request.source_branch}") - pipeline = service.execute(ignore_skip_ci: true, save_on_errors: false) + pipeline = service.execute(:push, ignore_skip_ci: true, save_on_errors: false) pipeline.run! Timecop.travel rand(1..6).hours.from_now diff --git a/db/migrate/20170524125940_add_source_to_ci_pipeline.rb b/db/migrate/20170524125940_add_source_to_ci_pipeline.rb new file mode 100644 index 00000000000..1fa3d48037b --- /dev/null +++ b/db/migrate/20170524125940_add_source_to_ci_pipeline.rb @@ -0,0 +1,9 @@ +class AddSourceToCiPipeline < ActiveRecord::Migration + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + + def change + add_column :ci_pipelines, :source, :integer + end +end diff --git a/db/schema.rb b/db/schema.rb index 4c73f74ef1f..7371ce8847f 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -11,7 +11,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 20170523091700) do +ActiveRecord::Schema.define(version: 20170524125940) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -283,6 +283,7 @@ ActiveRecord::Schema.define(version: 20170523091700) do t.integer "lock_version" t.integer "auto_canceled_by_id" t.integer "pipeline_schedule_id" + t.integer "source" end add_index "ci_pipelines", ["auto_canceled_by_id"], name: "index_ci_pipelines_on_auto_canceled_by_id", using: :btree diff --git a/features/steps/project/pages.rb b/features/steps/project/pages.rb index fea82d9fb57..4e6830f738b 100644 --- a/features/steps/project/pages.rb +++ b/features/steps/project/pages.rb @@ -35,7 +35,7 @@ class Spinach::Features::ProjectPages < Spinach::FeatureSteps end step 'pages are deployed' do - pipeline = @project.ensure_pipeline('HEAD', @project.commit('HEAD').sha) + pipeline = @project.pipelines.create(ref: 'HEAD', sha: @project.commit('HEAD').sha) build = build(:ci_build, project: @project, pipeline: pipeline, diff --git a/lib/api/commit_statuses.rb b/lib/api/commit_statuses.rb index 827a38d33da..10f2d5ef6a3 100644 --- a/lib/api/commit_statuses.rb +++ b/lib/api/commit_statuses.rb @@ -68,7 +68,14 @@ module API name = params[:name] || params[:context] || 'default' - pipeline = @project.ensure_pipeline(ref, commit.sha, current_user) + pipeline = @project.pipeline_for(ref, commit.sha) + unless pipeline + pipeline = @project.pipelines.create!( + source: :external, + sha: commit.sha, + ref: ref, + user: current_user) + end status = GenericCommitStatus.running_or_pending.find_or_initialize_by( project: @project, diff --git a/lib/api/pipelines.rb b/lib/api/pipelines.rb index 9117704aa46..e505cae3992 100644 --- a/lib/api/pipelines.rb +++ b/lib/api/pipelines.rb @@ -47,7 +47,7 @@ module API new_pipeline = Ci::CreatePipelineService.new(user_project, current_user, declared_params(include_missing: false)) - .execute(ignore_skip_ci: true, save_on_errors: false) + .execute(:api, ignore_skip_ci: true, save_on_errors: false) if new_pipeline.persisted? present new_pipeline, with: Entities::Pipeline else diff --git a/spec/factories/ci/pipelines.rb b/spec/factories/ci/pipelines.rb index 361c5b9a49e..03e3c62effe 100644 --- a/spec/factories/ci/pipelines.rb +++ b/spec/factories/ci/pipelines.rb @@ -1,5 +1,6 @@ FactoryGirl.define do factory :ci_empty_pipeline, class: Ci::Pipeline do + source :push ref 'master' sha '97de212e80737a608d939f648d959671fb0a0142' status 'pending' diff --git a/spec/features/projects/pipelines/pipelines_spec.rb b/spec/features/projects/pipelines/pipelines_spec.rb index a97a92aa64f..05c2bf350f1 100644 --- a/spec/features/projects/pipelines/pipelines_spec.rb +++ b/spec/features/projects/pipelines/pipelines_spec.rb @@ -442,6 +442,8 @@ describe 'Pipelines', :feature, :js do it 'creates a new pipeline' do expect { click_on 'Create pipeline' } .to change { Ci::Pipeline.count }.by(1) + + expect(Ci::Pipeline.last).to be_web end end diff --git a/spec/lib/gitlab/import_export/safe_model_attributes.yml b/spec/lib/gitlab/import_export/safe_model_attributes.yml index 96054c996fd..54ce8051f30 100644 --- a/spec/lib/gitlab/import_export/safe_model_attributes.yml +++ b/spec/lib/gitlab/import_export/safe_model_attributes.yml @@ -191,6 +191,7 @@ Ci::Pipeline: - lock_version - auto_canceled_by_id - pipeline_schedule_id +- source CommitStatus: - id - project_id diff --git a/spec/models/ci/pipeline_spec.rb b/spec/models/ci/pipeline_spec.rb index c8023dc13b1..ae1b01b76ab 100644 --- a/spec/models/ci/pipeline_spec.rb +++ b/spec/models/ci/pipeline_spec.rb @@ -21,13 +21,35 @@ describe Ci::Pipeline, models: true do it { is_expected.to have_many(:auto_canceled_pipelines) } it { is_expected.to have_many(:auto_canceled_jobs) } - it { is_expected.to validate_presence_of :sha } - it { is_expected.to validate_presence_of :status } + it { is_expected.to validate_presence_of(:sha) } + it { is_expected.to validate_presence_of(:status) } it { is_expected.to respond_to :git_author_name } it { is_expected.to respond_to :git_author_email } it { is_expected.to respond_to :short_sha } + describe '#source' do + context 'when creating new pipeline' do + let(:pipeline) do + build(:ci_empty_pipeline, status: :created, project: project, source: nil) + end + + it "prevents from creating an object" do + expect(pipeline).not_to be_valid + end + end + + context 'when updating existing pipeline' do + before do + pipeline.update_attribute(:source, nil) + end + + it "object is valid" do + expect(pipeline).to be_valid + end + end + end + describe '#block' do it 'changes pipeline status to manual' do expect(pipeline.block).to be true diff --git a/spec/requests/api/commit_statuses_spec.rb b/spec/requests/api/commit_statuses_spec.rb index 1c163cee152..6b637a03b6f 100644 --- a/spec/requests/api/commit_statuses_spec.rb +++ b/spec/requests/api/commit_statuses_spec.rb @@ -16,8 +16,8 @@ describe API::CommitStatuses do let(:get_url) { "/projects/#{project.id}/repository/commits/#{sha}/statuses" } context 'ci commit exists' do - let!(:master) { project.pipelines.create(sha: commit.id, ref: 'master') } - let!(:develop) { project.pipelines.create(sha: commit.id, ref: 'develop') } + let!(:master) { project.pipelines.create(source: :push, sha: commit.id, ref: 'master') } + let!(:develop) { project.pipelines.create(source: :push, sha: commit.id, ref: 'develop') } context "reporter user" do let(:statuses_id) { json_response.map { |status| status['id'] } } diff --git a/spec/requests/api/commits_spec.rb b/spec/requests/api/commits_spec.rb index b84361d3abd..b0c265b6453 100644 --- a/spec/requests/api/commits_spec.rb +++ b/spec/requests/api/commits_spec.rb @@ -485,7 +485,7 @@ describe API::Commits do end it "returns status for CI" do - pipeline = project.ensure_pipeline('master', project.repository.commit.sha) + pipeline = project.pipelines.create(source: :push, ref: 'master', sha: project.repository.commit.sha) pipeline.update(status: 'success') get api("/projects/#{project.id}/repository/commits/#{project.repository.commit.id}", user) @@ -495,7 +495,7 @@ describe API::Commits do end it "returns status for CI when pipeline is created" do - project.ensure_pipeline('master', project.repository.commit.sha) + project.pipelines.create(source: :push, ref: 'master', sha: project.repository.commit.sha) get api("/projects/#{project.id}/repository/commits/#{project.repository.commit.id}", user) diff --git a/spec/requests/api/v3/commits_spec.rb b/spec/requests/api/v3/commits_spec.rb index 386f60065ad..4a4a5dc5c7c 100644 --- a/spec/requests/api/v3/commits_spec.rb +++ b/spec/requests/api/v3/commits_spec.rb @@ -386,7 +386,7 @@ describe API::V3::Commits do end it "returns status for CI" do - pipeline = project.ensure_pipeline('master', project.repository.commit.sha) + pipeline = project.pipelines.create(source: :push, ref: 'master', sha: project.repository.commit.sha) pipeline.update(status: 'success') get v3_api("/projects/#{project.id}/repository/commits/#{project.repository.commit.id}", user) @@ -396,7 +396,7 @@ describe API::V3::Commits do end it "returns status for CI when pipeline is created" do - project.ensure_pipeline('master', project.repository.commit.sha) + project.pipelines.create(source: :push, ref: 'master', sha: project.repository.commit.sha) get v3_api("/projects/#{project.id}/repository/commits/#{project.repository.commit.id}", user) diff --git a/spec/serializers/pipeline_entity_spec.rb b/spec/serializers/pipeline_entity_spec.rb index d2482ac434b..88ec4ed2952 100644 --- a/spec/serializers/pipeline_entity_spec.rb +++ b/spec/serializers/pipeline_entity_spec.rb @@ -19,7 +19,7 @@ describe PipelineEntity do let(:pipeline) { create(:ci_empty_pipeline) } it 'contains required fields' do - expect(subject).to include :id, :user, :path, :coverage + expect(subject).to include :id, :user, :path, :coverage, :source expect(subject).to include :ref, :commit expect(subject).to include :updated_at, :created_at end @@ -36,7 +36,7 @@ describe PipelineEntity do it 'contains flags' do expect(subject).to include :flags expect(subject[:flags]) - .to include :latest, :triggered, :stuck, + .to include :latest, :stuck, :yaml_errors, :retryable, :cancelable end end diff --git a/spec/services/ci/create_pipeline_service_spec.rb b/spec/services/ci/create_pipeline_service_spec.rb index 030912b9f45..8bf02f56282 100644 --- a/spec/services/ci/create_pipeline_service_spec.rb +++ b/spec/services/ci/create_pipeline_service_spec.rb @@ -9,13 +9,13 @@ describe Ci::CreatePipelineService, services: true do end describe '#execute' do - def execute_service(after: project.commit.id, message: 'Message', ref: 'refs/heads/master') + def execute_service(source: :push, after: project.commit.id, message: 'Message', ref: 'refs/heads/master') params = { ref: ref, before: '00000000', after: after, commits: [{ message: message }] } - described_class.new(project, user, params).execute + described_class.new(project, user, params).execute(source) end context 'valid params' do @@ -30,6 +30,7 @@ describe Ci::CreatePipelineService, services: true do it 'creates a pipeline' do expect(pipeline).to be_kind_of(Ci::Pipeline) expect(pipeline).to be_valid + expect(pipeline).to be_push expect(pipeline).to eq(project.pipelines.last) expect(pipeline).to have_attributes(user: user) expect(pipeline).to have_attributes(status: 'pending') diff --git a/spec/services/ci/create_trigger_request_service_spec.rb b/spec/services/ci/create_trigger_request_service_spec.rb index 5a20102872a..f2956262f4b 100644 --- a/spec/services/ci/create_trigger_request_service_spec.rb +++ b/spec/services/ci/create_trigger_request_service_spec.rb @@ -16,6 +16,7 @@ describe Ci::CreateTriggerRequestService, services: true do context 'without owner' do it { expect(subject).to be_kind_of(Ci::TriggerRequest) } it { expect(subject.pipeline).to be_kind_of(Ci::Pipeline) } + it { expect(subject.pipeline).to be_trigger } it { expect(subject.builds.first).to be_kind_of(Ci::Build) } end @@ -25,6 +26,7 @@ describe Ci::CreateTriggerRequestService, services: true do it { expect(subject).to be_kind_of(Ci::TriggerRequest) } it { expect(subject.pipeline).to be_kind_of(Ci::Pipeline) } + it { expect(subject.pipeline).to be_trigger } it { expect(subject.pipeline.user).to eq(owner) } it { expect(subject.builds.first).to be_kind_of(Ci::Build) } it { expect(subject.builds.first.user).to eq(owner) } diff --git a/spec/services/git_push_service_spec.rb b/spec/services/git_push_service_spec.rb index 9f5a8beac16..bcd1fb64ab9 100644 --- a/spec/services/git_push_service_spec.rb +++ b/spec/services/git_push_service_spec.rb @@ -131,6 +131,19 @@ describe GitPushService, services: true do end end + describe "Pipelines" do + subject { execute_service(project, user, @oldrev, @newrev, @ref) } + + before do + stub_ci_pipeline_to_return_yaml_file + end + + it "creates a new pipeline" do + expect{ subject }.to change{ Ci::Pipeline.count } + expect(Ci::Pipeline.last).to be_push + end + end + describe "Push Event" do before do service = execute_service(project, user, @oldrev, @newrev, @ref ) diff --git a/spec/services/git_tag_push_service_spec.rb b/spec/services/git_tag_push_service_spec.rb index b73beb3f6fc..1fdcb420a8b 100644 --- a/spec/services/git_tag_push_service_spec.rb +++ b/spec/services/git_tag_push_service_spec.rb @@ -30,6 +30,20 @@ describe GitTagPushService, services: true do end end + describe "Pipelines" do + subject { service.execute } + + before do + stub_ci_pipeline_to_return_yaml_file + project.team << [user, :developer] + end + + it "creates a new pipeline" do + expect{ subject }.to change{ Ci::Pipeline.count } + expect(Ci::Pipeline.last).to be_push + end + end + describe "Git Tag Push Data" do subject { @push_data } let(:tag) { project.repository.find_tag(tag_name) } diff --git a/spec/workers/pipeline_schedule_worker_spec.rb b/spec/workers/pipeline_schedule_worker_spec.rb index 9c650354d72..14ed8b7811e 100644 --- a/spec/workers/pipeline_schedule_worker_spec.rb +++ b/spec/workers/pipeline_schedule_worker_spec.rb @@ -23,7 +23,8 @@ describe PipelineScheduleWorker do context 'when there is a scheduled pipeline within next_run_at' do it 'creates a new pipeline' do - expect { subject }.to change { project.pipelines.count }.by(1) + expect{ subject }.to change { project.pipelines.count }.by(1) + expect(Ci::Pipeline.last).to be_schedule end it 'updates the next_run_at field' do