Commit 1152adc9 authored by Douwe Maan's avatar Douwe Maan

Merge branch '36807-gc-unwanted-refs-after-import-ee' into 'master'

EE:  Remove unwanted refs after importing a project

See merge request !2792
parents 83408f08 82320ee9
......@@ -114,7 +114,7 @@ class Environment < ActiveRecord::Base
end
def ref_path
"refs/environments/#{Shellwords.shellescape(name)}"
"refs/#{Repository::REF_ENVIRONMENTS}/#{Shellwords.shellescape(name)}"
end
def formatted_external_url
......
......@@ -822,7 +822,7 @@ class MergeRequest < ActiveRecord::Base
end
def ref_path
"refs/merge-requests/#{iid}/head"
"refs/#{Repository::REF_MERGE_REQUEST}/#{iid}/head"
end
def ref_fetched?
......
......@@ -371,11 +371,7 @@ class Project < ActiveRecord::Base
if Gitlab::ImportSources.importer_names.include?(project.import_type) && project.repo_exists?
project.run_after_commit do
begin
Projects::HousekeepingService.new(project).execute
rescue Projects::HousekeepingService::LeaseTaken => e
Rails.logger.info("Could not perform housekeeping for project #{project.full_path} (#{project.id}): #{e}")
end
Projects::AfterImportService.new(project).execute
end
end
end
......
......@@ -3,6 +3,18 @@ require 'securerandom'
require 'forwardable'
class Repository
REF_MERGE_REQUEST = 'merge-requests'.freeze
REF_KEEP_AROUND = 'keep-around'.freeze
REF_ENVIRONMENTS = 'environments'.freeze
RESERVED_REFS_NAMES = %W[
heads
tags
#{REF_ENVIRONMENTS}
#{REF_KEEP_AROUND}
#{REF_ENVIRONMENTS}
].freeze
include Gitlab::ShellAdapter
include Elastic::RepositoriesSearch
include RepositoryMirroring
......@@ -241,10 +253,10 @@ class Repository
begin
write_ref(keep_around_ref_name(sha), sha)
rescue Rugged::ReferenceError => ex
Rails.logger.error "Unable to create keep-around reference for repository #{path}: #{ex}"
Rails.logger.error "Unable to create #{REF_KEEP_AROUND} reference for repository #{path}: #{ex}"
rescue Rugged::OSError => ex
raise unless ex.message =~ /Failed to create locked file/ && ex.message =~ /File exists/
Rails.logger.error "Unable to create keep-around reference for repository #{path}: #{ex}"
Rails.logger.error "Unable to create #{REF_KEEP_AROUND} reference for repository #{path}: #{ex}"
end
end
......@@ -1239,7 +1251,7 @@ class Repository
end
def keep_around_ref_name(sha)
"refs/keep-around/#{sha}"
"refs/#{REF_KEEP_AROUND}/#{sha}"
end
def repository_event(event, tags = {})
......@@ -1299,4 +1311,16 @@ class Repository
.commits_by_message(query, revision: ref, path: path, limit: limit, offset: offset)
.map { |c| commit(c) }
end
def with_repo_tmp_commit(start_repository, start_branch_name, sha)
tmp_ref = fetch_ref(
start_repository.path_to_repo,
"#{Gitlab::Git::BRANCH_REF_PREFIX}#{start_branch_name}",
"refs/tmp/#{SecureRandom.hex}/head"
)
yield commit(sha)
ensure
delete_refs(tmp_ref) if tmp_ref
end
end
module Projects
class AfterImportService
RESERVED_REFS_REGEXP =
%r{\Arefs/(?:#{Regexp.union(*Repository::RESERVED_REFS_NAMES)})/}
def initialize(project)
@project = project
end
def execute
Projects::HousekeepingService.new(@project).execute do
repository.delete_refs(*garbage_refs)
end
rescue Projects::HousekeepingService::LeaseTaken => e
Rails.logger.info(
"Could not perform housekeeping for project #{@project.full_path} (#{@project.id}): #{e}")
end
private
def garbage_refs
@garbage_refs ||= repository.all_ref_names_except(RESERVED_REFS_REGEXP)
end
def repository
@repository ||= @project.repository
end
end
end
......@@ -26,6 +26,8 @@ module Projects
lease_uuid = try_obtain_lease
raise LeaseTaken unless lease_uuid.present?
yield if block_given?
execute_gitlab_shell_gc(lease_uuid)
end
......
---
title: Remove unwanted refs after importing a project
merge_request: 13766
author:
type: other
......@@ -17,6 +17,7 @@ module Gitlab
NoRepository = Class.new(StandardError)
InvalidBlobName = Class.new(StandardError)
InvalidRef = Class.new(StandardError)
GitError = Class.new(StandardError)
class << self
# Unlike `new`, `create` takes the storage path, not the storage name
......@@ -246,6 +247,13 @@ module Gitlab
branch_names + tag_names
end
# Returns an Array of all ref names, except when it's matching pattern
#
# regexp - The pattern for ref names we don't want
def all_ref_names_except(regexp)
rugged.references.reject { |ref| ref.name =~ regexp }.map(&:name)
end
# Discovers the default branch based on the repository's available branches
#
# - If no branches are present, returns nil
......@@ -591,6 +599,23 @@ module Gitlab
rugged.branches.delete(branch_name)
end
def delete_refs(*ref_names)
instructions = ref_names.map do |ref|
"delete #{ref}\x00\x00"
end
command = %W[#{Gitlab.config.git.bin_path} update-ref --stdin -z]
message, status = Gitlab::Popen.popen(
command,
path) do |stdin|
stdin.write(instructions.join)
end
unless status.zero?
raise GitError.new("Could not delete refs #{ref_names}: #{message}")
end
end
# Create a new branch named **ref+ based on **stat_point+, HEAD by default
#
# Examples:
......
......@@ -140,7 +140,7 @@ namespace :gitlab do
next unless id > max_iid
project.deployments.find(id).create_ref
rugged.references.delete(ref)
project.repository.delete_refs(ref)
end
end
end
......
......@@ -433,6 +433,40 @@ describe Gitlab::Git::Repository, seed_helper: true do
end
end
describe '#delete_refs' do
before(:all) do
@repo = Gitlab::Git::Repository.new('default', TEST_MUTABLE_REPO_PATH, '')
end
it 'deletes the ref' do
@repo.delete_refs('refs/heads/feature')
expect(@repo.rugged.references['refs/heads/feature']).to be_nil
end
it 'deletes all refs' do
refs = %w[refs/heads/wip refs/tags/v1.1.0]
@repo.delete_refs(*refs)
refs.each do |ref|
expect(@repo.rugged.references[ref]).to be_nil
end
end
it 'raises an error if it failed' do
expect(Gitlab::Popen).to receive(:popen).and_return(['Error', 1])
expect do
@repo.delete_refs('refs/heads/fix')
end.to raise_error(Gitlab::Git::Repository::GitError)
end
after(:all) do
FileUtils.rm_rf(TEST_MUTABLE_REPO_PATH)
ensure_seeds
end
end
describe "#refs_hash" do
let(:refs) { repository.refs_hash }
......
......@@ -1778,10 +1778,18 @@ describe Project do
describe 'project import state transitions' do
context 'state transition: [:started] => [:finished]' do
let(:housekeeping_service) { spy }
let(:after_import_service) { spy(:after_import_service) }
let(:housekeeping_service) { spy(:housekeeping_service) }
before do
allow(Projects::HousekeepingService).to receive(:new) { housekeeping_service }
allow(Projects::AfterImportService)
.to receive(:new) { after_import_service }
allow(after_import_service)
.to receive(:execute) { housekeeping_service.execute }
allow(Projects::HousekeepingService)
.to receive(:new) { housekeeping_service }
end
it 'resets project import_error' do
......@@ -1796,6 +1804,7 @@ describe Project do
project.import_finish
expect(after_import_service).to have_received(:execute)
expect(housekeeping_service).to have_received(:execute)
end
......
require 'spec_helper'
describe Projects::AfterImportService do
subject { described_class.new(project) }
let(:project) { create(:project, :repository) }
let(:repository) { project.repository }
let(:sha) { project.commit.sha }
let(:housekeeping_service) { double(:housekeeping_service) }
describe '#execute' do
before do
allow(Projects::HousekeepingService)
.to receive(:new).with(project).and_return(housekeeping_service)
allow(housekeeping_service)
.to receive(:execute).and_yield
end
it 'performs housekeeping' do
subject.execute
expect(housekeeping_service).to have_received(:execute)
end
context 'with some refs in refs/pull/**/*' do
before do
repository.write_ref('refs/pull/1/head', sha)
repository.write_ref('refs/pull/1/merge', sha)
subject.execute
end
it 'removes refs/pull/**/*' do
expect(repository.rugged.references.map(&:name))
.not_to include(%r{\Arefs/pull/})
end
end
Repository::RESERVED_REFS_NAMES.each do |name|
context "with a ref in refs/#{name}/tmp" do
before do
repository.write_ref("refs/#{name}/tmp", sha)
subject.execute
end
it "does not remove refs/#{name}/tmp" do
expect(repository.rugged.references.map(&:name))
.to include("refs/#{name}/tmp")
end
end
end
end
end
......@@ -23,6 +23,12 @@ describe Projects::HousekeepingService do
expect(project.reload.pushes_since_gc).to eq(0)
end
it 'yields the block if given' do
expect do |block|
subject.execute(&block)
end.to yield_with_no_args
end
context 'when no lease can be obtained' do
before do
expect(subject).to receive(:try_obtain_lease).and_return(false)
......@@ -39,6 +45,13 @@ describe Projects::HousekeepingService do
expect { subject.execute }.to raise_error(Projects::HousekeepingService::LeaseTaken)
end.not_to change { project.pushes_since_gc }
end
it 'does not yield' do
expect do |block|
expect { subject.execute(&block) }
.to raise_error(Projects::HousekeepingService::LeaseTaken)
end.not_to yield_with_no_args
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