summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
Diffstat
-rw-r--r--app/models/ci/pipeline.rb2
-rw-r--r--app/models/concerns/internal_id2.rb32
-rw-r--r--app/services/ci/create_pipeline_service.rb7
-rw-r--r--db/migrate/20170829121123_add_iid_to_ci_pipelines.rb9
-rw-r--r--db/migrate/20170829121143_add_index_to_ci_pipelines_iid.rb15
-rw-r--r--db/schema.rb2
-rw-r--r--spec/lib/gitlab/import_export/safe_model_attributes.yml1
-rw-r--r--spec/models/ci/build_spec.rb1
-rw-r--r--spec/models/ci/pipeline_spec.rb29
-rw-r--r--spec/services/ci/create_pipeline_service_spec.rb13
10 files changed, 110 insertions, 1 deletions
diff --git a/app/models/ci/pipeline.rb b/app/models/ci/pipeline.rb
index ca65e81f27a..233a43c6f3e 100644
--- a/app/models/ci/pipeline.rb
+++ b/app/models/ci/pipeline.rb
@@ -5,6 +5,7 @@ module Ci
include Importable
include AfterCommitQueue
include Presentable
+ include InternalId2
include Gitlab::OptimisticLocking
belongs_to :project
@@ -433,6 +434,7 @@ module Ci
def predefined_variables
[
{ key: 'CI_PIPELINE_ID', value: id.to_s, public: true },
+ { key: 'CI_PIPELINE_IID', value: iid.to_s, public: true },
{ key: 'CI_CONFIG_PATH', value: ci_yaml_file_path, public: true },
{ key: 'CI_PIPELINE_SOURCE', value: source.to_s, public: true }
]
diff --git a/app/models/concerns/internal_id2.rb b/app/models/concerns/internal_id2.rb
new file mode 100644
index 00000000000..b64c831adc0
--- /dev/null
+++ b/app/models/concerns/internal_id2.rb
@@ -0,0 +1,32 @@
+module InternalId2
+ extend ActiveSupport::Concern
+
+ FailedToSaveInternalIdError = Class.new(StandardError)
+
+ included do
+ after_commit :set_iid, on: :create
+ end
+
+ def set_iid
+ parent = project || group
+ records = parent.public_send(table_name) # rubocop:disable GitlabSecurity/PublicSend
+ records = records.with_deleted if self.paranoid?
+
+ begin
+ retries ||= 0
+ max_iid = records.maximum(:iid) || -1
+ update_columns(iid: max_iid.to_i + 1) # Avoid infinite loop
+ rescue ActiveRecord::RecordNotUnique
+ if (retries += 1) < 3
+ retry
+ else
+ raise FailedToSaveInternalIdError
+ end
+ end
+ end
+
+ def table_name
+ self.class.name.deconstantize.split("::").map(&:underscore).join('_')
+ + self.class.name.demodulize.tableize
+ end
+end
diff --git a/app/services/ci/create_pipeline_service.rb b/app/services/ci/create_pipeline_service.rb
index 31a712ccc1b..9af2e527364 100644
--- a/app/services/ci/create_pipeline_service.rb
+++ b/app/services/ci/create_pipeline_service.rb
@@ -41,6 +41,13 @@ module Ci
pipeline.process!
end
end
+
+ rescue ActiveRecord::RecordInvalid => e
+ return error("Failed to persist the pipeline: #{e}")
+ rescue InternalId2::FailedToSaveInternalIdError
+ # TODO: We need to roolback!!!!!!
+ return error("Failed to persist the pipeline: #{e}")
+ end
end
private
diff --git a/db/migrate/20170829121123_add_iid_to_ci_pipelines.rb b/db/migrate/20170829121123_add_iid_to_ci_pipelines.rb
new file mode 100644
index 00000000000..e200f04a1a9
--- /dev/null
+++ b/db/migrate/20170829121123_add_iid_to_ci_pipelines.rb
@@ -0,0 +1,9 @@
+class AddIidToCiPipelines < ActiveRecord::Migration
+ include Gitlab::Database::MigrationHelpers
+
+ DOWNTIME = false
+
+ def change
+ add_column :ci_pipelines, :iid, :integer
+ end
+end
diff --git a/db/migrate/20170829121143_add_index_to_ci_pipelines_iid.rb b/db/migrate/20170829121143_add_index_to_ci_pipelines_iid.rb
new file mode 100644
index 00000000000..8bf39ad4900
--- /dev/null
+++ b/db/migrate/20170829121143_add_index_to_ci_pipelines_iid.rb
@@ -0,0 +1,15 @@
+class AddIndexToCiPipelinesIid < ActiveRecord::Migration
+ include Gitlab::Database::MigrationHelpers
+
+ DOWNTIME = false
+
+ disable_ddl_transaction!
+
+ def up
+ add_concurrent_index(:ci_pipelines, [:project_id, :iid], unique: true)
+ end
+
+ def down
+ remove_concurrent_index(:ci_pipelines, [:project_id, :iid]) if index_exists?(:ci_pipelines, [:project_id, :iid])
+ end
+end
diff --git a/db/schema.rb b/db/schema.rb
index 80d8ff92d6e..4bc43e202a6 100644
--- a/db/schema.rb
+++ b/db/schema.rb
@@ -369,10 +369,12 @@ ActiveRecord::Schema.define(version: 20171026082505) do
t.integer "config_source"
t.boolean "protected"
t.integer "failure_reason"
+ t.integer "iid"
end
add_index "ci_pipelines", ["auto_canceled_by_id"], name: "index_ci_pipelines_on_auto_canceled_by_id", using: :btree
add_index "ci_pipelines", ["pipeline_schedule_id"], name: "index_ci_pipelines_on_pipeline_schedule_id", using: :btree
+ add_index "ci_pipelines", ["project_id", "iid"], name: "index_ci_pipelines_on_project_id_and_iid", unique: true, using: :btree
add_index "ci_pipelines", ["project_id", "ref", "status"], name: "index_ci_pipelines_on_project_id_and_ref_and_status", using: :btree
add_index "ci_pipelines", ["project_id", "sha"], name: "index_ci_pipelines_on_project_id_and_sha", using: :btree
add_index "ci_pipelines", ["project_id"], name: "index_ci_pipelines_on_project_id", using: :btree
diff --git a/spec/lib/gitlab/import_export/safe_model_attributes.yml b/spec/lib/gitlab/import_export/safe_model_attributes.yml
index 89d30407077..da8202ca668 100644
--- a/spec/lib/gitlab/import_export/safe_model_attributes.yml
+++ b/spec/lib/gitlab/import_export/safe_model_attributes.yml
@@ -206,6 +206,7 @@ MergeRequestDiffFile:
- binary
Ci::Pipeline:
- id
+- iid
- project_id
- source
- ref
diff --git a/spec/models/ci/build_spec.rb b/spec/models/ci/build_spec.rb
index 5ed2e1ca99a..c50adfb3d22 100644
--- a/spec/models/ci/build_spec.rb
+++ b/spec/models/ci/build_spec.rb
@@ -1273,6 +1273,7 @@ describe Ci::Build do
{ key: 'CI_PROJECT_URL', value: project.web_url, public: true },
{ key: 'CI_PROJECT_VISIBILITY', value: 'private', public: true },
{ key: 'CI_PIPELINE_ID', value: pipeline.id.to_s, public: true },
+ { key: 'CI_PIPELINE_IID', value: pipeline.iid.to_s, public: true },
{ key: 'CI_CONFIG_PATH', value: pipeline.ci_yaml_file_path, public: true },
{ key: 'CI_REGISTRY_USER', value: 'gitlab-ci-token', public: true },
{ key: 'CI_REGISTRY_PASSWORD', value: build.token, public: false },
diff --git a/spec/models/ci/pipeline_spec.rb b/spec/models/ci/pipeline_spec.rb
index 2c9e7013b77..d8a04676e64 100644
--- a/spec/models/ci/pipeline_spec.rb
+++ b/spec/models/ci/pipeline_spec.rb
@@ -28,6 +28,22 @@ describe Ci::Pipeline, :mailer do
it { is_expected.to respond_to :short_sha }
it { is_expected.to delegate_method(:full_path).to(:project).with_prefix }
+ describe 'IID' do
+ it 'creates sequential iid in the same project' do
+ p1 = create(:ci_empty_pipeline, project: project)
+ p2 = create(:ci_empty_pipeline, project: project)
+ expect(p1.iid).to eq(0)
+ expect(p2.iid).to eq(1)
+ end
+
+ it 'creates the same iid in the different project' do
+ p1 = create(:ci_empty_pipeline, project: project)
+ p2 = create(:ci_empty_pipeline, project: create(:project))
+ expect(p1.iid).to eq(0)
+ expect(p2.iid).to eq(0)
+ end
+ end
+
describe '#source' do
context 'when creating new pipeline' do
let(:pipeline) do
@@ -168,7 +184,18 @@ describe Ci::Pipeline, :mailer do
it 'includes the defined keys' do
keys = subject.map { |v| v[:key] }
- expect(keys).to include('CI_PIPELINE_ID', 'CI_CONFIG_PATH', 'CI_PIPELINE_SOURCE')
+ expect(keys).to eq(%w[CI_PIPELINE_ID CI_PIPELINE_IID CI_CONFIG_PATH CI_PIPELINE_SOURCE])
+ end
+
+ context 'when iid is nil' do
+ before do
+ pipeline.update_columns(iid: nil)
+ end
+
+ it 'returns empty iid' do
+ value = subject.select { |s| s[:key] == 'CI_PIPELINE_IID' } .first[:value]
+ expect(value).to be_empty
+ end
end
end
diff --git a/spec/services/ci/create_pipeline_service_spec.rb b/spec/services/ci/create_pipeline_service_spec.rb
index 08847183bf4..ad92ba768f7 100644
--- a/spec/services/ci/create_pipeline_service_spec.rb
+++ b/spec/services/ci/create_pipeline_service_spec.rb
@@ -45,6 +45,7 @@ describe Ci::CreatePipelineService do
expect(pipeline).to have_attributes(user: user)
expect(pipeline).to have_attributes(status: 'pending')
expect(pipeline.builds.first).to be_kind_of(Ci::Build)
+ expect(pipeline.iid).to eq(0)
end
it 'increments the prometheus counter' do
@@ -498,5 +499,17 @@ describe Ci::CreatePipelineService do
end
end
end
+
+ context 'when failed to set_iid' do
+ before do
+ allow_any_instance_of(Ci::Pipeline).to receive(:set_iid).and_raise(InternalId2::FailedToSaveInternalIdError)
+ end
+
+ let(:pipeline) { execute_service }
+
+ it 'does not create a new pipeline' do
+ expect(pipeline).not_to be_persisted
+ end
+ end
end
end
View Lorry Controller Status