Skip to content

G
gitlab-ce
Commit deedfe19 authored by Shinya Maeda's avatar Shinya Maeda
Browse files
Options

Separate deployments creation from pipeline job creation 

This commit cleans up the feature flag as it's deemed stable.

Changelog: other
parent 91fb24fc
Hide whitespace changes
Inline Side-by-side
Showing with 6 additions and 298 deletions
app/models/ci/pipeline.rb
View file @ deedfe19
......@@ -1284,12 +1284,6 @@ module Ci
end
end
def create_deployment_in_separate_transaction?
strong_memoize(:create_deployment_in_separate_transaction) do
::Feature.enabled?(:create_deployment_in_separate_transaction, project, default_enabled: :yaml)
end
end
def use_variables_builder_definitions?
strong_memoize(:use_variables_builder_definitions) do
::Feature.enabled?(:ci_use_variables_builder_definitions, project, default_enabled: :yaml)
......
app/services/ci/retry_build_service.rb
View file @ deedfe19
......@@ -40,17 +40,13 @@ module Ci
new_build = clone_build(build)
new_build.run_after_commit do
::Deployments::CreateForBuildService.new.execute(new_build)
::MergeRequests::AddTodoWhenBuildFailsService
.new(project: project)
.close(new_build)
end
if create_deployment_in_separate_transaction?
new_build.run_after_commit do |new_build|
::Deployments::CreateForBuildService.new.execute(new_build)
end
end
::Ci::Pipelines::AddJobService.new(build.pipeline).execute!(new_build) do |job|
BulkInsertableAssociations.with_bulk_insert do
job.save!
......@@ -74,11 +70,7 @@ module Ci
def check_assignable_runners!(build); end
def clone_build(build)
project.builds.new(build_attributes(build)).tap do |new_build|
unless create_deployment_in_separate_transaction?
new_build.assign_attributes(deployment_attributes_for(new_build, build))
end
end
project.builds.new(build_attributes(build))
end
def build_attributes(build)
......@@ -86,7 +78,7 @@ module Ci
[attribute, build.public_send(attribute)] # rubocop:disable GitlabSecurity/PublicSend
end
if create_deployment_in_separate_transaction? && build.persisted_environment.present?
if build.persisted_environment.present?
attributes[:metadata_attributes] ||= {}
attributes[:metadata_attributes][:expanded_environment_name] = build.expanded_environment_name
end
......@@ -94,17 +86,6 @@ module Ci
attributes[:user] = current_user
attributes
end
def deployment_attributes_for(new_build, old_build)
::Gitlab::Ci::Pipeline::Seed::Build
.deployment_attributes_for(new_build, old_build.persisted_environment)
end
def create_deployment_in_separate_transaction?
strong_memoize(:create_deployment_in_separate_transaction) do
::Feature.enabled?(:create_deployment_in_separate_transaction, project, default_enabled: :yaml)
end
end
end
end
......
config/feature_flags/development/create_deployment_in_separate_transaction.yml deleted 100644 → 0
View file @ 91fb24fc
---
name: create_deployment_in_separate_transaction
introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/75604
rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/346879
milestone: '14.6'
type: development
group: group::release
default_enabled: true
lib/gitlab/ci/pipeline/chain/build.rb
View file @ deedfe19
......@@ -21,10 +21,6 @@ module Gitlab
merge_request: @command.merge_request,
external_pull_request: @command.external_pull_request,
locked: @command.project.default_pipeline_lock)
# Initialize the feature flag at the beginning of the pipeline creation process
# so that the flag references in the latter chains return the same value.
@pipeline.create_deployment_in_separate_transaction?
end
def break?
......
lib/gitlab/ci/pipeline/chain/create_deployments.rb
View file @ deedfe19
......@@ -6,8 +6,6 @@ module Gitlab
module Chain
class CreateDeployments < Chain::Base
def perform!
return unless pipeline.create_deployment_in_separate_transaction?
create_deployments!
end
......
lib/gitlab/ci/pipeline/chain/ensure_environments.rb
View file @ deedfe19
......@@ -6,8 +6,6 @@ module Gitlab
module Chain
class EnsureEnvironments < Chain::Base
def perform!
return unless pipeline.create_deployment_in_separate_transaction?
pipeline.stages.map(&:statuses).flatten.each(&method(:ensure_environment))
end
......
lib/gitlab/ci/pipeline/chain/ensure_resource_groups.rb
View file @ deedfe19
......@@ -6,8 +6,6 @@ module Gitlab
module Chain
class EnsureResourceGroups < Chain::Base
def perform!
return unless pipeline.create_deployment_in_separate_transaction?
pipeline.stages.map(&:statuses).flatten.each(&method(:ensure_resource_group))
end
......
lib/gitlab/ci/pipeline/seed/build.rb
View file @ deedfe19
......@@ -79,9 +79,7 @@ module Gitlab
def to_resource
strong_memoize(:resource) do
processable = initialize_processable
assign_resource_group(processable) unless @pipeline.create_deployment_in_separate_transaction?
processable
initialize_processable
end
end
......@@ -89,39 +87,10 @@ module Gitlab
if bridge?
::Ci::Bridge.new(attributes)
else
::Ci::Build.new(attributes).tap do |build|
unless @pipeline.create_deployment_in_separate_transaction?
build.assign_attributes(self.class.deployment_attributes_for(build))
end
end
::Ci::Build.new(attributes)
end
end
def assign_resource_group(processable)
processable.resource_group =
Seed::Processable::ResourceGroup.new(processable, @resource_group_key)
.to_resource
end
def self.deployment_attributes_for(build, environment = nil)
return {} unless build.has_environment?
environment = Seed::Environment.new(build).to_resource if environment.nil?
unless environment.persisted?
return { status: :failed, failure_reason: :environment_creation_failure }
end
build.persisted_environment = environment
{
deployment: Seed::Deployment.new(build, environment).to_resource,
metadata_attributes: {
expanded_environment_name: environment.name
}
}
end
private
delegate :logger, to: :@context
......
spec/lib/gitlab/ci/pipeline/chain/create_deployments_spec.rb
View file @ deedfe19
......@@ -47,18 +47,6 @@ RSpec.describe Gitlab::Ci::Pipeline::Chain::CreateDeployments do
expect(job.deployment).to be_nil
end
end
context 'when create_deployment_in_separate_transaction feature flag is disabled' do
before do
stub_feature_flags(create_deployment_in_separate_transaction: false)
end
it 'does not create a deployment record' do
expect { subject }.not_to change { Deployment.count }
expect(job.deployment).to be_nil
end
end
end
context 'when a pipeline contains a teardown job' do
......
spec/lib/gitlab/ci/pipeline/chain/ensure_environments_spec.rb
View file @ deedfe19
......@@ -57,18 +57,6 @@ RSpec.describe Gitlab::Ci::Pipeline::Chain::EnsureEnvironments do
expect(job.persisted_environment).to be_nil
end
end
context 'when create_deployment_in_separate_transaction feature flag is disabled' do
before do
stub_feature_flags(create_deployment_in_separate_transaction: false)
end
it 'does not create any environments' do
expect { subject }.not_to change { Environment.count }
expect(job.persisted_environment).to be_nil
end
end
end
context 'when a pipeline contains a teardown job' do
......
spec/lib/gitlab/ci/pipeline/chain/ensure_resource_groups_spec.rb
View file @ deedfe19
......@@ -60,18 +60,6 @@ RSpec.describe Gitlab::Ci::Pipeline::Chain::EnsureResourceGroups do
expect(job.resource_group).to be_nil
end
end
context 'when create_deployment_in_separate_transaction feature flag is disabled' do
before do
stub_feature_flags(create_deployment_in_separate_transaction: false)
end
it 'does not create any resource groups' do
expect { subject }.not_to change { Ci::ResourceGroup.count }
expect(job.resource_group).to be_nil
end
end
end
context 'when a pipeline does not contain a job that requires a resource group' do
......
spec/lib/gitlab/ci/pipeline/seed/build_spec.rb
View file @ deedfe19
......@@ -411,171 +411,6 @@ RSpec.describe Gitlab::Ci::Pipeline::Seed::Build do
describe '#to_resource' do
subject { seed_build.to_resource }
before do
stub_feature_flags(create_deployment_in_separate_transaction: false)
end
context 'when job is Ci::Build' do
it { is_expected.to be_a(::Ci::Build) }
it { is_expected.to be_valid }
shared_examples_for 'deployment job' do
it 'returns a job with deployment' do
expect { subject }.to change { Environment.count }.by(1)
expect(subject.deployment).not_to be_nil
expect(subject.deployment.deployable).to eq(subject)
expect(subject.deployment.environment.name).to eq(expected_environment_name)
end
end
shared_examples_for 'non-deployment job' do
it 'returns a job without deployment' do
expect(subject.deployment).to be_nil
end
end
shared_examples_for 'ensures environment existence' do
it 'has environment' do
expect { subject }.to change { Environment.count }.by(1)
expect(subject).to be_has_environment
expect(subject.environment).to eq(environment_name)
expect(subject.metadata.expanded_environment_name).to eq(expected_environment_name)
expect(Environment.exists?(name: expected_environment_name)).to eq(true)
end
end
shared_examples_for 'ensures environment inexistence' do
it 'does not have environment' do
expect { subject }.not_to change { Environment.count }
expect(subject).not_to be_has_environment
expect(subject.environment).to be_nil
expect(subject.metadata&.expanded_environment_name).to be_nil
expect(Environment.exists?(name: expected_environment_name)).to eq(false)
end
end
context 'when job deploys to production' do
let(:environment_name) { 'production' }
let(:expected_environment_name) { 'production' }
let(:attributes) { { name: 'deploy', ref: 'master', environment: 'production' } }
it_behaves_like 'deployment job'
it_behaves_like 'ensures environment existence'
context 'when create_deployment_in_separate_transaction feature flag is enabled' do
before do
stub_feature_flags(create_deployment_in_separate_transaction: true)
end
it 'does not create any deployments nor environments' do
expect(subject.deployment).to be_nil
expect(Environment.count).to eq(0)
expect(Deployment.count).to eq(0)
end
end
context 'when the environment name is invalid' do
let(:attributes) { { name: 'deploy', ref: 'master', environment: '!!!' } }
it 'fails the job with a failure reason and does not create an environment' do
expect(subject).to be_failed
expect(subject).to be_environment_creation_failure
expect(subject.metadata.expanded_environment_name).to be_nil
expect(Environment.exists?(name: expected_environment_name)).to eq(false)
end
end
end
context 'when job starts a review app' do
let(:environment_name) { 'review/$CI_COMMIT_REF_NAME' }
let(:expected_environment_name) { "review/#{pipeline.ref}" }
let(:attributes) do
{
name: 'deploy', ref: 'master', environment: environment_name,
options: { environment: { name: environment_name } }
}
end
it_behaves_like 'deployment job'
it_behaves_like 'ensures environment existence'
end
context 'when job stops a review app' do
let(:environment_name) { 'review/$CI_COMMIT_REF_NAME' }
let(:expected_environment_name) { "review/#{pipeline.ref}" }
let(:attributes) do
{
name: 'deploy', ref: 'master', environment: environment_name,
options: { environment: { name: environment_name, action: 'stop' } }
}
end
it 'returns a job without deployment' do
expect(subject.deployment).to be_nil
end
it_behaves_like 'non-deployment job'
it_behaves_like 'ensures environment existence'
end
context 'when job belongs to a resource group' do
let(:resource_group) { 'iOS' }
let(:attributes) { { name: 'rspec', ref: 'master', resource_group_key: resource_group, environment: 'production' }}
it 'returns a job with resource group' do
expect(subject.resource_group).not_to be_nil
expect(subject.resource_group.key).to eq('iOS')
expect(Ci::ResourceGroup.count).to eq(1)
end
context 'when create_deployment_in_separate_transaction feature flag is enabled' do
before do
stub_feature_flags(create_deployment_in_separate_transaction: true)
end
it 'does not create any resource groups' do
expect(subject.resource_group).to be_nil
expect(Ci::ResourceGroup.count).to eq(0)
end
end
context 'when resource group has $CI_ENVIRONMENT_NAME in it' do
let(:resource_group) { 'test/$CI_ENVIRONMENT_NAME' }
it 'expands environment name' do
expect(subject.resource_group.key).to eq('test/production')
end
end
end
end
context 'when job is a bridge' do
let(:base_attributes) do
{
name: 'rspec', ref: 'master', options: { trigger: 'my/project' }, scheduling_type: :stage
}
end
let(:attributes) { base_attributes }
it { is_expected.to be_a(::Ci::Bridge) }
it { is_expected.to be_valid }
context 'when job belongs to a resource group' do
let(:attributes) { base_attributes.merge(resource_group_key: 'iOS') }
it 'returns a job with resource group' do
expect(subject.resource_group).not_to be_nil
expect(subject.resource_group.key).to eq('iOS')
end
end
end
it 'memoizes a resource object' do
expect(subject.object_id).to eq seed_build.to_resource.object_id
end
......
spec/services/ci/retry_build_service_spec.rb
View file @ deedfe19
......@@ -370,23 +370,6 @@ RSpec.describe Ci::RetryBuildService do
it_behaves_like 'when build with deployment is retried'
it_behaves_like 'when build with dynamic environment is retried'
context 'when create_deployment_in_separate_transaction feature flag is disabled' do
let(:new_build) do
travel_to(1.second.from_now) do
::Gitlab::Database::QueryAnalyzers::PreventCrossDatabaseModification.allow_cross_database_modification_within_transaction(url: 'https://gitlab.com/gitlab-org/gitlab/-/issues/345668') do
service.clone!(build)
end
end
end
before do
stub_feature_flags(create_deployment_in_separate_transaction: false)
end
it_behaves_like 'when build with deployment is retried'
it_behaves_like 'when build with dynamic environment is retried'
end
context 'when build has needs' do
before do
create(:ci_build_need, build: build, name: 'build1')
......
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
GitLab Nexedi Edition | About GitLab | About Nexedi | 沪ICP备2021021310号-2 | 沪ICP备2021021310号-7