Commit 7f7359b1 authored by Stan Hu's avatar Stan Hu

Use Redis Ruby client instead of shelling out to redis-cli

Closes gitlab-org/gitlab-ce#17329
parent f0f1bbec
......@@ -3,6 +3,7 @@ v3.0.0
- Remove create-branch and rm-branch commands (Robert Schilling)
- Update PostReceive worker so it logs a unique JID in Sidekiq
- Remove update-head command
- Use Redis Ruby client instead of shelling out to redis-cli
v2.7.2
- Do not prune objects during 'git gc'
......
source "http://rubygems.org"
gem 'redis', '~> 3.3'
group :development, :test do
gem 'coveralls', require: false
gem 'rspec', '~> 2.14.0'
......
......@@ -37,6 +37,7 @@ GEM
method_source (~> 0.8)
slop (~> 3.3.1)
rainbow (2.0.0)
redis (3.3.0)
rest-client (1.7.2)
mime-types (>= 1.16, < 3.0)
netrc (~> 0.7)
......@@ -77,7 +78,11 @@ DEPENDENCIES
coveralls
guard
guard-rspec
redis (~> 3.3)
rspec (~> 2.14.0)
rubocop (= 0.28.0)
vcr
webmock
BUNDLED WITH
1.11.2
......@@ -36,8 +36,5 @@ dirs.each do |dir|
puts "\n"
end
print "Test redis-cli executable: "
abort('FAILED') unless system(*config.redis_command, '--version')
print "Send ping to redis server: "
abort unless system(*config.redis_command, 'ping')
abort unless GitlabNet.new.redis_client.ping
......@@ -54,26 +54,4 @@ class GitlabConfig
def git_annex_enabled?
@config['git_annex_enabled'] ||= false
end
# Build redis command to write update event in gitlab queue
def redis_command
if redis.empty?
# Default to old method of connecting to redis
# for users that haven't updated their configuration
%W(env -i redis-cli)
else
redis['database'] ||= 0
redis['host'] ||= '127.0.0.1'
redis['port'] ||= '6379'
if redis.has_key?("socket")
%W(#{redis['bin']} -s #{redis['socket']} -n #{redis['database']})
else
if redis.has_key?("pass")
%W(#{redis['bin']} -h #{redis['host']} -p #{redis['port']} -n #{redis['database']} -a #{redis['pass']})
else
%W(#{redis['bin']} -h #{redis['host']} -p #{redis['port']} -n #{redis['database']})
end
end
end
end
end
require 'net/http'
require 'openssl'
require 'json'
require 'redis'
require_relative 'gitlab_config'
require_relative 'gitlab_logger'
......@@ -63,6 +64,24 @@ class GitlabNet
nil
end
def redis_client
redis_config = config.redis
database = redis_config['database'] || 0
params = {
host: redis_config['host'] || '127.0.0.1',
port: redis_config['port'] || 6379,
db: database
}
if redis_config.has_key?("socket")
params = { path: redis_config['socket'], db: database }
elsif redis_config.has_key?("pass")
params[:password] = redis_config['pass']
end
Redis.new(params)
end
protected
def config
......
......@@ -2,6 +2,7 @@ require_relative 'gitlab_init'
require_relative 'gitlab_net'
require 'json'
require 'base64'
require 'redis'
require 'securerandom'
class GitlabPostReceive
......@@ -74,11 +75,12 @@ class GitlabPostReceive
queue = "#{config.redis_namespace}:queue:post_receive"
msg = JSON.dump({ 'class' => 'PostReceive', 'args' => [@repo_path, @actor, changes], 'jid' => @jid })
if system(*config.redis_command, 'rpush', queue, msg,
err: '/dev/null', out: '/dev/null')
begin
GitlabNet.new.redis_client.rpush(queue, msg)
return true
else
puts "GitLab: An unexpected error occurred (redis-cli returned #{$?.exitstatus})."
rescue => e
puts "GitLab: An unexpected error occurred in writing to Redis: #{e}"
return false
end
end
......
......@@ -48,40 +48,4 @@ eos
it("returns false by default") { should eq(false) }
end
describe :redis_command do
subject { config.redis_command }
context "with empty redis config" do
before do
config.stub(:redis) { {} }
end
it { should be_an(Array) }
it { should include('redis-cli') }
end
context "with host and port" do
before do
config.stub(:redis) { {'host' => 'localhost', 'port' => 1123, 'bin' => '/usr/bin/redis-cli'} }
end
it { should be_an(Array) }
it { should include(config.redis['host']) }
it { should include(config.redis['bin']) }
it { should include(config.redis['port'].to_s) }
end
context "with redis socket" do
let(:socket) { '/tmp/redis.socket' }
before do
config.stub(:redis) { {'bin' => '', 'socket' => socket } }
end
it { should be_an(Array) }
it { should include(socket) }
it { should_not include('-p') }
it { should_not include('-h') }
end
end
end
......@@ -229,4 +229,44 @@ describe GitlabNet, vcr: true do
store.should_receive(:add_path).with('test_path')
end
end
describe '#redis_client' do
let(:config) { double('config') }
context "with empty redis config" do
it 'returns default parameters' do
allow(gitlab_net).to receive(:config).and_return(config)
allow(config).to receive(:redis).and_return( {} )
expect_any_instance_of(Redis).to receive(:initialize).with({ host: '127.0.0.1',
port: 6379,
db: 0 }).and_call_original
gitlab_net.redis_client
end
end
context "with host and port" do
it 'uses the specified host and port' do
allow(gitlab_net).to receive(:config).and_return(config)
allow(config).to receive(:redis).and_return( { 'host' => 'localhost', 'port' => 1123 } )
expect_any_instance_of(Redis).to receive(:initialize).with({ host: 'localhost',
port: 1123,
db: 0 }).and_call_original
gitlab_net.redis_client
end
end
context "with redis socket" do
let(:socket) { '/tmp/redis.socket' }
it 'uses the socket' do
allow(gitlab_net).to receive(:config).and_return(config)
allow(config).to receive(:redis).and_return( { 'socket' => socket })
expect_any_instance_of(Redis).to receive(:initialize).with({ path: socket, db: 0 }).and_call_original
gitlab_net.redis_client
end
end
end
end
# coding: utf-8
require 'spec_helper'
require 'gitlab_post_receive'
......@@ -11,6 +12,7 @@ describe GitlabPostReceive do
let(:repo_path) { File.join(repository_path, repo_name) + ".git" }
let(:gitlab_post_receive) { GitlabPostReceive.new(repo_path, actor, wrongly_encoded_changes) }
let(:message) { "test " * 10 + "message " * 10 }
let(:redis_client) { double('redis_client') }
before do
GitlabConfig.any_instance.stub(repos_path: repository_path)
......@@ -20,11 +22,11 @@ describe GitlabPostReceive do
describe "#exec" do
before do
GitlabConfig.any_instance.stub(redis_command: %w(env -i redis-cli))
allow(gitlab_post_receive).to receive(:system).and_return(true)
allow_any_instance_of(GitlabNet).to receive(:redis_client).and_return(redis_client)
end
it "prints the broadcast message" do
expect(redis_client).to receive(:rpush)
expect(gitlab_post_receive).to receive(:puts).ordered
expect(gitlab_post_receive).to receive(:puts).with(
"========================================================================"
......@@ -47,12 +49,9 @@ describe GitlabPostReceive do
end
it "pushes a Sidekiq job onto the queue" do
expect(gitlab_post_receive).to receive(:system).with(
*[
*%w(env -i redis-cli rpush resque:gitlab:queue:post_receive),
%Q/{"class":"PostReceive","args":["#{repo_path}","#{actor}",#{base64_changes.inspect}],"jid":"#{gitlab_post_receive.jid}"}/,
{ err: "/dev/null", out: "/dev/null" }
]
expect(redis_client).to receive(:rpush).with(
'resque:gitlab:queue:post_receive',
%Q/{"class":"PostReceive","args":["#{repo_path}","#{actor}",#{base64_changes.inspect}],"jid":"#{gitlab_post_receive.jid}"}/
).and_return(true)
gitlab_post_receive.exec
......@@ -61,7 +60,7 @@ describe GitlabPostReceive do
context "when the redis command succeeds" do
before do
allow(gitlab_post_receive).to receive(:system).and_return(true)
allow(redis_client).to receive(:rpush).and_return(true)
end
it "returns true" do
......@@ -72,8 +71,7 @@ describe GitlabPostReceive do
context "when the redis command fails" do
before do
allow(gitlab_post_receive).to receive(:system).and_return(false)
allow($?).to receive(:exitstatus).and_return(nil)
allow(redis_client).to receive(:rpush).and_raise('Fail')
end
it "returns false" do
......
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