From 1c5876eb7b2deb069d919bd19b51c9f6218e0f41 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Mon, 28 Jan 2013 17:22:45 +0200 Subject: [PATCH 1/4] Do gitolite calls async. Remove satellite with project remove --- Procfile | 2 +- app/contexts/projects/create_context.rb | 12 ++------ app/models/project.rb | 16 +++++++---- app/models/users_project.rb | 2 +- app/observers/project_observer.rb | 1 + app/workers/gitolite_worker.rb | 10 +++++++ app/workers/post_receive.rb | 15 +++++----- lib/gitlab/backend/gitolite.rb | 14 +++++++-- lib/gitlab/backend/gitolite_config.rb | 38 ++++++++++--------------- lib/gitlab/popen.rb | 18 ++++++++++++ lib/gitlab/satellite/satellite.rb | 12 ++++++-- lib/tasks/sidekiq.rake | 2 +- 12 files changed, 90 insertions(+), 52 deletions(-) create mode 100644 app/workers/gitolite_worker.rb create mode 100644 lib/gitlab/popen.rb diff --git a/Procfile b/Procfile index 28a97dda..1a4145cc 100644 --- a/Procfile +++ b/Procfile @@ -1,2 +1,2 @@ web: bundle exec unicorn_rails -p $PORT -worker: bundle exec sidekiq -q post_receive,mailer,system_hook,project_web_hook,common,default +worker: bundle exec sidekiq -q post_receive,mailer,system_hook,project_web_hook,common,default,gitolite diff --git a/app/contexts/projects/create_context.rb b/app/contexts/projects/create_context.rb index e644d89a..915bd8be 100644 --- a/app/contexts/projects/create_context.rb +++ b/app/contexts/projects/create_context.rb @@ -32,16 +32,10 @@ module Projects @project.namespace_id = current_user.namespace_id end - Project.transaction do - @project.creator = current_user - @project.save! + @project.creator = current_user - # Add user as project master - @project.users_projects.create!(project_access: UsersProject::MASTER, user: current_user) - - # when project saved no team member exist so - # project repository should be updated after first user add - @project.update_repository + if @project.save + @project.users_projects.create(project_access: UsersProject::MASTER, user: current_user) end @project diff --git a/app/models/project.rb b/app/models/project.rb index cb6986ce..dde15927 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -299,6 +299,9 @@ class Project < ActiveRecord::Base def trigger_post_receive(oldrev, newrev, ref, user) data = post_receive_data(oldrev, newrev, ref, user) + # Create satellite + self.satellite.create unless self.satellite.exists? + # Create push event self.observe_push(data) @@ -313,9 +316,6 @@ class Project < ActiveRecord::Base self.execute_services(data.dup) end - # Create satellite - self.satellite.create unless self.satellite.exists? - # Discover the default branch, but only if it hasn't already been set to # something else if repository && default_branch.nil? @@ -460,11 +460,17 @@ class Project < ActiveRecord::Base end def update_repository - gitolite.update_repository(self) + GitoliteWorker.perform_async( + :update_repository, + self.id + ) end def destroy_repository - gitolite.remove_repository(self) + GitoliteWorker.perform_async( + :remove_repository, + self.path_with_namespace + ) end def repo_exists? diff --git a/app/models/users_project.rb b/app/models/users_project.rb index ca5048ca..32066004 100644 --- a/app/models/users_project.rb +++ b/app/models/users_project.rb @@ -129,7 +129,7 @@ class UsersProject < ActiveRecord::Base end def update_repository - gitolite.update_repository(project) + project.update_repository end def project_access_human diff --git a/app/observers/project_observer.rb b/app/observers/project_observer.rb index b1c69456..cc454ae3 100644 --- a/app/observers/project_observer.rb +++ b/app/observers/project_observer.rb @@ -10,6 +10,7 @@ class ProjectObserver < ActiveRecord::Observer def after_destroy(project) log_info("Project \"#{project.name}\" was removed") + project.satellite.destroy project.destroy_repository end diff --git a/app/workers/gitolite_worker.rb b/app/workers/gitolite_worker.rb new file mode 100644 index 00000000..d134ea03 --- /dev/null +++ b/app/workers/gitolite_worker.rb @@ -0,0 +1,10 @@ +class GitoliteWorker + include Sidekiq::Worker + include Gitolited + + sidekiq_options queue: :gitolite + + def perform(action, arg) + gitolite.send(action, arg) + end +end diff --git a/app/workers/post_receive.rb b/app/workers/post_receive.rb index e74379a6..a906f78f 100644 --- a/app/workers/post_receive.rb +++ b/app/workers/post_receive.rb @@ -13,13 +13,14 @@ class PostReceive # Ignore push from non-gitlab users user = if identifier.eql? Gitlab.config.gitolite.admin_key - email = project.repository.commit(newrev).author.email rescue nil - User.find_by_email(email) if email - elsif /^[A-Z0-9._%a-z\-]+@(?:[A-Z0-9a-z\-]+\.)+[A-Za-z]{2,4}$/.match(identifier) - User.find_by_email(identifier) - else - Key.find_by_identifier(identifier).try(:user) - end + email = project.repository.commit(newrev).author.email rescue nil + User.find_by_email(email) if email + elsif /^[A-Z0-9._%a-z\-]+@(?:[A-Z0-9a-z\-]+\.)+[A-Za-z]{2,4}$/.match(identifier) + User.find_by_email(identifier) + else + Key.find_by_identifier(identifier).try(:user) + end + return false unless user project.trigger_post_receive(oldrev, newrev, ref, user) diff --git a/lib/gitlab/backend/gitolite.rb b/lib/gitlab/backend/gitolite.rb index 3b8a2090..d026b76f 100644 --- a/lib/gitlab/backend/gitolite.rb +++ b/lib/gitlab/backend/gitolite.rb @@ -22,7 +22,8 @@ module Gitlab end end - def update_repository project + def update_repository project_id + project = Project.find(project_id) config.update_project!(project) end @@ -33,8 +34,15 @@ module Gitlab end end - def remove_repository project - config.destroy_project!(project) + # Remove repository from gitolite + # + # name - project path with namespace + # + # Ex. + # remove_repository("gitlab/gitlab-ci") + # + def remove_repository(name) + config.destroy_project!(name) end def url_to_repo path diff --git a/lib/gitlab/backend/gitolite_config.rb b/lib/gitlab/backend/gitolite_config.rb index e4ebd595..748f9d74 100644 --- a/lib/gitlab/backend/gitolite_config.rb +++ b/lib/gitlab/backend/gitolite_config.rb @@ -4,6 +4,8 @@ require 'fileutils' module Gitlab class GitoliteConfig + include Gitlab::Popen + class PullError < StandardError; end class PushError < StandardError; end class BrokenGitolite < StandardError; end @@ -87,12 +89,14 @@ module Gitlab Gitlab::GitLogger.error(message) end - def destroy_project(project) - # do rm-rf only if repository exists - if project.repository - FileUtils.rm_rf(project.repository.path_to_repo) - end - conf.rm_repo(project.path_with_namespace) + def path_to_repo(name) + File.join(Gitlab.config.gitolite.repos_path, "#{name}.git") + end + + def destroy_project(name) + full_path = path_to_repo(name) + FileUtils.rm_rf(full_path) if File.exists?(full_path) + conf.rm_repo(name) end def clean_repo repo_name @@ -210,14 +214,14 @@ module Gitlab end def push - output, status = popen('git add -A') + output, status = popen('git add -A', tmp_conf_path) raise "Git add failed." unless status.zero? # git commit returns 0 on success, and 1 if there is nothing to commit - output, status = popen('git commit -m "GitLab"') + output, status = popen('git commit -m "GitLab"', tmp_conf_path) raise "Git add failed." unless [0,1].include?(status) - output, status = popen('git push') + output, status = popen('git push', tmp_conf_path) if output =~ /remote\: FATAL/ raise BrokenGitolite, output @@ -230,20 +234,8 @@ module Gitlab end end - def popen(cmd, path = nil) - path ||= File.join(config_tmp_dir,'gitolite') - vars = { "PWD" => path } - options = { :chdir => path } - - @cmd_output = "" - @cmd_status = 0 - Open3.popen3(vars, cmd, options) do |stdin, stdout, stderr, wait_thr| - @cmd_status = wait_thr.value.exitstatus - @cmd_output << stdout.read - @cmd_output << stderr.read - end - - return @cmd_output, @cmd_status + def tmp_conf_path + File.join(config_tmp_dir,'gitolite') end end end diff --git a/lib/gitlab/popen.rb b/lib/gitlab/popen.rb new file mode 100644 index 00000000..f2cfd807 --- /dev/null +++ b/lib/gitlab/popen.rb @@ -0,0 +1,18 @@ +module Gitlab + module Popen + def popen(cmd, path) + vars = { "PWD" => path } + options = { :chdir => path } + + @cmd_output = "" + @cmd_status = 0 + Open3.popen3(vars, cmd, options) do |stdin, stdout, stderr, wait_thr| + @cmd_status = wait_thr.value.exitstatus + @cmd_output << stdout.read + @cmd_output << stderr.read + end + + return @cmd_output, @cmd_status + end + end +end diff --git a/lib/gitlab/satellite/satellite.rb b/lib/gitlab/satellite/satellite.rb index 164af55d..d8e8f589 100644 --- a/lib/gitlab/satellite/satellite.rb +++ b/lib/gitlab/satellite/satellite.rb @@ -3,6 +3,8 @@ module Gitlab module Satellite class Satellite + include Gitlab::Popen + PARKING_BRANCH = "__parking_branch" attr_accessor :project @@ -24,8 +26,10 @@ module Gitlab end def create - create_cmd = "git clone #{project.url_to_repo} #{path}" - if system(create_cmd) + output, status = popen("git clone #{project.url_to_repo} #{path}", + Gitlab.config.satellites.path) + + if status.zero? true else Gitlab::GitLogger.error("Failed to create satellite for #{project.name_with_namespace}") @@ -66,6 +70,10 @@ module Gitlab @repo ||= Grit::Repo.new(path) end + def destroy + FileUtils.rm_rf(path) + end + private # Clear the working directory diff --git a/lib/tasks/sidekiq.rake b/lib/tasks/sidekiq.rake index 0d2ec6f3..e4eb0e67 100644 --- a/lib/tasks/sidekiq.rake +++ b/lib/tasks/sidekiq.rake @@ -6,7 +6,7 @@ namespace :sidekiq do desc "GITLAB | Start sidekiq" task :start do - run "nohup bundle exec sidekiq -q post_receive,mailer,system_hook,project_web_hook,common,default -e #{Rails.env} -P #{pidfile} >> #{Rails.root.join("log", "sidekiq.log")} 2>&1 &" + run "nohup bundle exec sidekiq -q post_receive,mailer,system_hook,project_web_hook,gitolite,common,default -e #{Rails.env} -P #{pidfile} >> #{Rails.root.join("log", "sidekiq.log")} 2>&1 &" end def pidfile From 8b54b7233ef58a2a39da9777dbeab59b4024cc75 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Mon, 28 Jan 2013 17:39:02 +0200 Subject: [PATCH 2/4] Async perform for add/remove team members --- app/models/protected_branch.rb | 2 +- app/models/users_project.rb | 12 ++++++++++-- lib/gitlab/backend/gitolite.rb | 26 +++++++++++++++++++------- spec/lib/gitolite_spec.rb | 2 +- spec/support/stubbed_repository.rb | 4 ++++ 5 files changed, 35 insertions(+), 11 deletions(-) diff --git a/app/models/protected_branch.rb b/app/models/protected_branch.rb index 3308caf3..2e7010ea 100644 --- a/app/models/protected_branch.rb +++ b/app/models/protected_branch.rb @@ -22,7 +22,7 @@ class ProtectedBranch < ActiveRecord::Base after_destroy :update_repository def update_repository - gitolite.update_repository(project) + project.update_repository end def commit diff --git a/app/models/users_project.rb b/app/models/users_project.rb index 32066004..183878cb 100644 --- a/app/models/users_project.rb +++ b/app/models/users_project.rb @@ -82,9 +82,13 @@ class UsersProject < ActiveRecord::Base users_project.save end end - Gitlab::Gitolite.new.update_repositories(Project.where(id: project_ids)) end + GitoliteWorker.perform_async( + :update_repositories, + project_ids + ) + true rescue false @@ -97,9 +101,13 @@ class UsersProject < ActiveRecord::Base users_project.skip_git = true users_project.destroy end - Gitlab::Gitolite.new.update_repositories(Project.where(id: project_ids)) end + GitoliteWorker.perform_async( + :update_repositories, + project_ids + ) + true rescue false diff --git a/lib/gitlab/backend/gitolite.rb b/lib/gitlab/backend/gitolite.rb index d026b76f..1bcf1264 100644 --- a/lib/gitlab/backend/gitolite.rb +++ b/lib/gitlab/backend/gitolite.rb @@ -22,7 +22,12 @@ module Gitlab end end - def update_repository project_id + # Update project config in gitolite by project id + # + # Ex. + # update_repository(23) + # + def update_repository(project_id) project = Project.find(project_id) config.update_project!(project) end @@ -45,6 +50,19 @@ module Gitlab config.destroy_project!(name) end + # Update projects configs in gitolite by project ids + # + # Ex. + # update_repositories([1, 4, 6]) + # + def update_repositories(project_ids) + projects = Project.where(id: project_ids) + + config.apply do |config| + config.update_projects(projects) + end + end + def url_to_repo path Gitlab.config.gitolite.ssh_path_prefix + "#{path}.git" end @@ -53,12 +71,6 @@ module Gitlab config.admin_all_repo! end - def update_repositories projects - config.apply do |config| - config.update_projects(projects) - end - end - alias_method :create_repository, :update_repository end end diff --git a/spec/lib/gitolite_spec.rb b/spec/lib/gitolite_spec.rb index 8075b99e..df6dc368 100644 --- a/spec/lib/gitolite_spec.rb +++ b/spec/lib/gitolite_spec.rb @@ -20,6 +20,6 @@ describe Gitlab::Gitolite do it "should call config update" do gitolite_config.should_receive(:update_project!) - gitolite.update_repository project + gitolite.update_repository(project.id) end end diff --git a/spec/support/stubbed_repository.rb b/spec/support/stubbed_repository.rb index e6e194d7..e092f8a4 100644 --- a/spec/support/stubbed_repository.rb +++ b/spec/support/stubbed_repository.rb @@ -21,6 +21,10 @@ class Project true end + def destroy + true + end + def create true end From 9ad5fbb416b00bf87c5f5ab3e1ee7ac5d5378b64 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Mon, 28 Jan 2013 17:46:24 +0200 Subject: [PATCH 3/4] user factory username over sequence --- spec/factories.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/factories.rb b/spec/factories.rb index 593b8350..0e0c04f9 100644 --- a/spec/factories.rb +++ b/spec/factories.rb @@ -12,7 +12,7 @@ FactoryGirl.define do factory :user, aliases: [:author, :assignee, :owner, :creator] do email { Faker::Internet.email } name - username { Faker::Internet.user_name } + sequence(:username) { |n| "#{Faker::Internet.user_name}#{n}" } password "123456" password_confirmation { password } From f7ade3b682a104dca76b0eb4e9a0fbc62cfd9cf7 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Mon, 28 Jan 2013 17:53:01 +0200 Subject: [PATCH 4/4] fix tests --- spec/lib/gitolite_spec.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/spec/lib/gitolite_spec.rb b/spec/lib/gitolite_spec.rb index df6dc368..7ba4a633 100644 --- a/spec/lib/gitolite_spec.rb +++ b/spec/lib/gitolite_spec.rb @@ -1,12 +1,13 @@ require 'spec_helper' describe Gitlab::Gitolite do - let(:project) { double('Project', path: 'diaspora') } + let(:project) { double('Project', id: 7, path: 'diaspora') } let(:gitolite_config) { double('Gitlab::GitoliteConfig') } let(:gitolite) { Gitlab::Gitolite.new } before do gitolite.stub(config: gitolite_config) + Project.stub(find: project) end it { should respond_to :set_key }