Commit 6e5f6667 authored by Shinya Maeda's avatar Shinya Maeda

Merge branch 'remove-git-ref-on-env-removal' into 'master'

Remove deployment refs on environment destruction

See merge request gitlab-org/gitlab!27482
parents 0c10584d 5be29637
......@@ -7,6 +7,7 @@ class Deployment < ApplicationRecord
include UpdatedAtFilterable
include Importable
include Gitlab::Utils::StrongMemoize
include FastDestroyAll
belongs_to :project, required: true
belongs_to :environment, required: true
......@@ -113,6 +114,26 @@ class Deployment < ApplicationRecord
success.find_by!(iid: iid)
end
class << self
##
# FastDestroyAll concerns
def begin_fast_destroy
preload(:project).find_each.map do |deployment|
[deployment.project, deployment.ref_path]
end
end
##
# FastDestroyAll concerns
def finalize_fast_destroy(params)
by_project = params.group_by(&:shift)
by_project.each do |project, ref_paths|
project.repository.delete_refs(*ref_paths.flatten)
end
end
end
def commit
project.commit(sha)
end
......@@ -280,12 +301,12 @@ class Deployment < ApplicationRecord
errors.add(:ref, _('The branch or tag does not exist'))
end
private
def ref_path
File.join(environment.ref_path, 'deployments', iid.to_s)
end
private
def legacy_finished_at
self.created_at if success? && !read_attribute(:finished_at)
end
......
......@@ -3,6 +3,7 @@
class Environment < ApplicationRecord
include Gitlab::Utils::StrongMemoize
include ReactiveCaching
include FastDestroyAll::Helpers
self.reactive_cache_refresh_interval = 1.minute
self.reactive_cache_lifetime = 55.seconds
......@@ -10,7 +11,10 @@ class Environment < ApplicationRecord
belongs_to :project, required: true
has_many :deployments, -> { visible }, dependent: :destroy # rubocop:disable Cop/ActiveRecordDependent
use_fast_destroy :all_deployments
has_many :all_deployments, class_name: 'Deployment'
has_many :deployments, -> { visible }
has_many :successful_deployments, -> { success }, class_name: 'Deployment'
has_many :active_deployments, -> { active }, class_name: 'Deployment'
has_many :prometheus_alerts, inverse_of: :environment
......
......@@ -623,4 +623,26 @@ describe Deployment do
expect(deploy.errors[:ref]).not_to be_empty
end
end
describe '.fast_destroy_all' do
it 'cleans path_refs for destroyed environments' do
project = create(:project, :repository)
environment = create(:environment, project: project)
destroyed_deployments = create_list(:deployment, 2, :success, environment: environment, project: project)
other_deployments = create_list(:deployment, 2, :success, environment: environment, project: project)
(destroyed_deployments + other_deployments).each(&:create_ref)
described_class.where(id: destroyed_deployments.map(&:id)).fast_destroy_all
destroyed_deployments.each do |deployment|
expect(project.commit(deployment.ref_path)).to be_nil
end
other_deployments.each do |deployment|
expect(project.commit(deployment.ref_path)).not_to be_nil
end
end
end
end
......@@ -1301,4 +1301,13 @@ describe Environment, :use_clean_rails_memory_store_caching do
end
end
end
describe '#destroy' do
it 'remove the deployment refs from gitaly' do
deployment = create(:deployment, :success, environment: environment, project: project)
deployment.create_ref
expect { environment.destroy }.to change { project.commit(deployment.ref_path) }.to(nil)
end
end
end
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