summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
Diffstat
-rw-r--r--app/controllers/projects/git_http_controller.rb2
-rw-r--r--app/services/ci/process_pipeline_service.rb15
-rw-r--r--changelogs/unreleased/fix-gb-remove-deprecated-pipeline-processing-code.yml4
-rw-r--r--changelogs/unreleased/namespace-race-condition.yml4
-rw-r--r--config/gitlab.yml.example2
-rw-r--r--config/initializers/8_gitaly.rb2
-rw-r--r--lib/gitlab/gitaly_client.rb4
-rw-r--r--lib/gitlab/shell.rb5
-rw-r--r--lib/gitlab/workhorse.rb16
-rw-r--r--spec/lib/gitlab/gitaly_client_spec.rb26
-rw-r--r--spec/lib/gitlab/workhorse_spec.rb58
-rw-r--r--spec/services/ci/process_pipeline_service_spec.rb59
12 files changed, 109 insertions, 88 deletions
diff --git a/app/controllers/projects/git_http_controller.rb b/app/controllers/projects/git_http_controller.rb
index 278098fcc58..37f6f637ff0 100644
--- a/app/controllers/projects/git_http_controller.rb
+++ b/app/controllers/projects/git_http_controller.rb
@@ -57,7 +57,7 @@ class Projects::GitHttpController < Projects::GitHttpClientController
def render_ok
set_workhorse_internal_api_content_type
- render json: Gitlab::Workhorse.git_http_ok(repository, user)
+ render json: Gitlab::Workhorse.git_http_ok(repository, user, action_name)
end
def render_http_not_allowed
diff --git a/app/services/ci/process_pipeline_service.rb b/app/services/ci/process_pipeline_service.rb
index 2935d00c075..33edcd60944 100644
--- a/app/services/ci/process_pipeline_service.rb
+++ b/app/services/ci/process_pipeline_service.rb
@@ -5,8 +5,6 @@ module Ci
def execute(pipeline)
@pipeline = pipeline
- ensure_created_builds! # TODO, remove me in 9.0
-
new_builds =
stage_indexes_of_created_builds.map do |index|
process_stage(index)
@@ -73,18 +71,5 @@ module Ci
def created_builds
pipeline.builds.created
end
-
- # This method is DEPRECATED and should be removed in 9.0.
- #
- # We need it to maintain backwards compatibility with previous versions
- # when builds were not created within one transaction with the pipeline.
- #
- def ensure_created_builds!
- return if created_builds.any?
-
- Ci::CreatePipelineBuildsService
- .new(project, current_user)
- .execute(pipeline)
- end
end
end
diff --git a/changelogs/unreleased/fix-gb-remove-deprecated-pipeline-processing-code.yml b/changelogs/unreleased/fix-gb-remove-deprecated-pipeline-processing-code.yml
new file mode 100644
index 00000000000..32862b527fd
--- /dev/null
+++ b/changelogs/unreleased/fix-gb-remove-deprecated-pipeline-processing-code.yml
@@ -0,0 +1,4 @@
+---
+title: Drop support for correctly processing legacy pipelines
+merge_request: 10266
+author:
diff --git a/changelogs/unreleased/namespace-race-condition.yml b/changelogs/unreleased/namespace-race-condition.yml
new file mode 100644
index 00000000000..2a76b6c74e8
--- /dev/null
+++ b/changelogs/unreleased/namespace-race-condition.yml
@@ -0,0 +1,4 @@
+---
+title: Fix project creation failure due to race condition in namespace directory creation
+merge_request: 10268
+author: Robin Bobbitt
diff --git a/config/gitlab.yml.example b/config/gitlab.yml.example
index bd27f01c872..4314e902564 100644
--- a/config/gitlab.yml.example
+++ b/config/gitlab.yml.example
@@ -461,7 +461,7 @@ production: &base
storages: # You must have at least a `default` storage path.
default:
path: /home/git/repositories/
- gitaly_address: unix:/home/git/gitlab/tmp/sockets/private/gitaly.socket
+ gitaly_address: unix:/home/git/gitlab/tmp/sockets/private/gitaly.socket # TCP connections are supported too (e.g. tcp://host:port)
## Backup settings
backup:
diff --git a/config/initializers/8_gitaly.rb b/config/initializers/8_gitaly.rb
index 69c0a91d6f0..c7f27c78535 100644
--- a/config/initializers/8_gitaly.rb
+++ b/config/initializers/8_gitaly.rb
@@ -9,7 +9,7 @@ if Gitlab.config.gitaly.enabled || Rails.env.test?
raise "storage #{name.inspect} is missing a gitaly_address"
end
- unless URI(address).scheme == 'unix'
+ unless URI(address).scheme.in?(%w(tcp unix))
raise "Unsupported Gitaly address: #{address.inspect}"
end
diff --git a/lib/gitlab/gitaly_client.rb b/lib/gitlab/gitaly_client.rb
index a0dbe0a8c11..fe15fb12adb 100644
--- a/lib/gitlab/gitaly_client.rb
+++ b/lib/gitlab/gitaly_client.rb
@@ -12,9 +12,11 @@ module Gitlab
end
def self.new_channel(address)
- # NOTE: Gitaly currently runs on a Unix socket, so permissions are
+ address = address.sub(%r{^tcp://}, '') if URI(address).scheme == 'tcp'
+ # NOTE: When Gitaly runs on a Unix socket, permissions are
# handled using the file system and no additional authentication is
# required (therefore the :this_channel_is_insecure flag)
+ # TODO: Add authentication support when Gitaly is running on a TCP socket.
GRPC::Core::Channel.new(address, {}, :this_channel_is_insecure)
end
diff --git a/lib/gitlab/shell.rb b/lib/gitlab/shell.rb
index 2744d627ceb..b631ef11ce7 100644
--- a/lib/gitlab/shell.rb
+++ b/lib/gitlab/shell.rb
@@ -194,7 +194,10 @@ module Gitlab
# add_namespace("/path/to/storage", "gitlab")
#
def add_namespace(storage, name)
- FileUtils.mkdir_p(full_path(storage, name), mode: 0770) unless exists?(storage, name)
+ path = full_path(storage, name)
+ FileUtils.mkdir_p(path, mode: 0770) unless exists?(storage, name)
+ rescue Errno::EEXIST => e
+ Rails.logger.warn("Directory exists as a file: #{e} at: #{path}")
end
# Remove directory from repositories storage
diff --git a/lib/gitlab/workhorse.rb b/lib/gitlab/workhorse.rb
index d1131ad65e0..d0637f8b394 100644
--- a/lib/gitlab/workhorse.rb
+++ b/lib/gitlab/workhorse.rb
@@ -16,7 +16,7 @@ module Gitlab
SECRET_LENGTH = 32
class << self
- def git_http_ok(repository, user)
+ def git_http_ok(repository, user, action)
repo_path = repository.path_to_repo
params = {
GL_ID: Gitlab::GlId.gl_id(user),
@@ -26,13 +26,25 @@ module Gitlab
if Gitlab.config.gitaly.enabled
storage = repository.project.repository_storage
address = Gitlab::GitalyClient.get_address(storage)
- params[:GitalySocketPath] = URI(address).path
# TODO: use GitalyClient code to assemble the Repository message
params[:Repository] = Gitaly::Repository.new(
path: repo_path,
storage_name: storage,
relative_path: Gitlab::RepoPath.strip_storage_path(repo_path),
).to_h
+
+ feature_enabled = case action.to_s
+ when 'git_receive_pack'
+ Gitlab::GitalyClient.feature_enabled?(:post_receive_pack)
+ when 'git_upload_pack'
+ Gitlab::GitalyClient.feature_enabled?(:post_upload_pack)
+ when 'info_refs'
+ true
+ else
+ raise "Unsupported action: #{action}"
+ end
+
+ params[:GitalySocketPath] = URI(address).path if feature_enabled
end
params
diff --git a/spec/lib/gitlab/gitaly_client_spec.rb b/spec/lib/gitlab/gitaly_client_spec.rb
new file mode 100644
index 00000000000..55fcf91fb6e
--- /dev/null
+++ b/spec/lib/gitlab/gitaly_client_spec.rb
@@ -0,0 +1,26 @@
+require 'spec_helper'
+
+describe Gitlab::GitalyClient, lib: true do
+ describe '.new_channel' do
+ context 'when passed a UNIX socket address' do
+ it 'passes the address as-is to GRPC::Core::Channel initializer' do
+ address = 'unix:/tmp/gitaly.sock'
+
+ expect(GRPC::Core::Channel).to receive(:new).with(address, any_args)
+
+ described_class.new_channel(address)
+ end
+ end
+
+ context 'when passed a TCP address' do
+ it 'strips tcp:// prefix before passing it to GRPC::Core::Channel initializer' do
+ address = 'localhost:9876'
+ prefixed_address = "tcp://#{address}"
+
+ expect(GRPC::Core::Channel).to receive(:new).with(address, any_args)
+
+ described_class.new_channel(prefixed_address)
+ end
+ end
+ end
+end
diff --git a/spec/lib/gitlab/workhorse_spec.rb b/spec/lib/gitlab/workhorse_spec.rb
index cb7c810124e..3bd2a3238fe 100644
--- a/spec/lib/gitlab/workhorse_spec.rb
+++ b/spec/lib/gitlab/workhorse_spec.rb
@@ -180,24 +180,68 @@ describe Gitlab::Workhorse, lib: true do
describe '.git_http_ok' do
let(:user) { create(:user) }
let(:repo_path) { repository.path_to_repo }
+ let(:action) { 'info_refs' }
- subject { described_class.git_http_ok(repository, user) }
+ subject { described_class.git_http_ok(repository, user, action) }
- it { expect(subject).to eq({ GL_ID: "user-#{user.id}", RepoPath: repo_path }) }
+ it { expect(subject).to include({ GL_ID: "user-#{user.id}", RepoPath: repo_path }) }
context 'when Gitaly is enabled' do
+ let(:gitaly_params) do
+ {
+ GitalySocketPath: URI(Gitlab::GitalyClient.get_address('default')).path,
+ }
+ end
+
before do
allow(Gitlab.config.gitaly).to receive(:enabled).and_return(true)
end
- it 'includes Gitaly params in the returned value' do
- gitaly_socket_path = URI(Gitlab::GitalyClient.get_address('default')).path
- expect(subject).to include({ GitalySocketPath: gitaly_socket_path })
- expect(subject[:Repository]).to include({
+ it 'includes a Repository param' do
+ repo_param = { Repository: {
path: repo_path,
storage_name: 'default',
relative_path: project.full_path + '.git',
- })
+ } }
+
+ expect(subject).to include(repo_param)
+ end
+
+ {
+ git_receive_pack: :post_receive_pack,
+ git_upload_pack: :post_upload_pack
+ }.each do |action_name, feature_flag|
+ context "when #{action_name} action is passed" do
+ let(:action) { action_name }
+
+ context 'when action is enabled by feature flag' do
+ it 'includes Gitaly params in the returned value' do
+ allow(Gitlab::GitalyClient).to receive(:feature_enabled?).with(feature_flag).and_return(true)
+
+ expect(subject).to include(gitaly_params)
+ end
+ end
+
+ context 'when action is not enabled by feature flag' do
+ it 'does not include Gitaly params in the returned value' do
+ allow(Gitlab::GitalyClient).to receive(:feature_enabled?).with(feature_flag).and_return(false)
+
+ expect(subject).not_to include(gitaly_params)
+ end
+ end
+ end
+ end
+
+ context "when info_refs action is passed" do
+ let(:action) { 'info_refs' }
+
+ it { expect(subject).to include(gitaly_params) }
+ end
+
+ context 'when action passed is not supported by Gitaly' do
+ let(:action) { 'download' }
+
+ it { expect { subject }.to raise_exception('Unsupported action: download') }
end
end
end
diff --git a/spec/services/ci/process_pipeline_service_spec.rb b/spec/services/ci/process_pipeline_service_spec.rb
index d93616c4f50..bb98fb37a90 100644
--- a/spec/services/ci/process_pipeline_service_spec.rb
+++ b/spec/services/ci/process_pipeline_service_spec.rb
@@ -418,65 +418,6 @@ describe Ci::ProcessPipelineService, '#execute', :services do
end
end
- context 'when there are builds that are not created yet' do
- let(:pipeline) do
- create(:ci_pipeline, config: config)
- end
-
- let(:config) do
- { rspec: { stage: 'test', script: 'rspec' },
- deploy: { stage: 'deploy', script: 'rsync' } }
- end
-
- before do
- create_build('linux', stage: 'build', stage_idx: 0)
- create_build('mac', stage: 'build', stage_idx: 0)
- end
-
- it 'processes the pipeline' do
- # Currently we have five builds with state created
- #
- expect(builds.count).to eq(0)
- expect(all_builds.count).to eq(2)
-
- # Process builds service will enqueue builds from the first stage.
- #
- process_pipeline
-
- expect(builds.count).to eq(2)
- expect(all_builds.count).to eq(2)
-
- # When builds succeed we will enqueue remaining builds.
- #
- # We will have 2 succeeded, 1 pending (from stage test), total 4 (two
- # additional build from `.gitlab-ci.yml`).
- #
- succeed_pending
- process_pipeline
-
- expect(builds.success.count).to eq(2)
- expect(builds.pending.count).to eq(1)
- expect(all_builds.count).to eq(4)
-
- # When pending merge_when_pipeline_succeeds in stage test, we enqueue deploy stage.
- #
- succeed_pending
- process_pipeline
-
- expect(builds.pending.count).to eq(1)
- expect(builds.success.count).to eq(3)
- expect(all_builds.count).to eq(4)
-
- # When the last one succeeds we have 4 successful builds.
- #
- succeed_pending
- process_pipeline
-
- expect(builds.success.count).to eq(4)
- expect(all_builds.count).to eq(4)
- end
- end
-
def process_pipeline
described_class.new(pipeline.project, user).execute(pipeline)
end
View Lorry Controller Status