Merge branch 'fix-update-templates-rake' into 'master'

Fix broken update_project_templates rake task

See merge request gitlab-org/gitlab-ce!31174

lib/gitlab/project_template.rb

@@ -13,11 +13,15 @@ module Gitlab
     end
     def archive_path
-      Rails.root.join("vendor/project_templates/#{name}.tar.gz")
+      self.class.archive_directory.join(archive_filename)
+    end
+
+    def archive_filename
+      "#{name}.tar.gz"
     end
     def clone_url
-      "https://gitlab.com/gitlab-org/project-templates/#{name}.git"
+      "#{preview}.git"
     end
     def ==(other)
@@ -54,7 +58,7 @@ module Gitlab
       end
       def archive_directory
-        Rails.root.join("vendor_directory/project_templates")
+        Rails.root.join("vendor/project_templates")
       end
     end
   end

lib/tasks/gitlab/update_templates.rake

@@ -5,25 +5,43 @@ namespace :gitlab do
   end
   desc "GitLab | Update project templates"
-  task :update_project_templates do
-    include Gitlab::ImportExport::CommandLineUtil
+  task :update_project_templates, [] => :environment do |_task, args|
+    # we need an instance method from Gitlab::ImportExport::CommandLineUtil and don't
+    # want to include it in the task, as this would affect subsequent tasks as well
+    downloader = Class.new do
+      extend Gitlab::ImportExport::CommandLineUtil
+
+      def self.call(uploader, upload_path)
+        download_or_copy_upload(uploader, upload_path)
+      end
+    end
+
+    template_names = args.extras.to_set
     if Rails.env.production?
-      puts "This rake task is not meant fo production instances".red
-      exit(1)
+      raise "This rake task is not meant for production instances"
     end
     admin = User.find_by(admin: true)
     unless admin
-      puts "No admin user could be found".red
-      exit(1)
+      raise "No admin user could be found"
     end
-    Gitlab::ProjectTemplate.all.each do |template|
+    tmp_namespace_path = "tmp-project-import-#{Time.now.to_i}"
+    puts "Creating temporary namespace #{tmp_namespace_path}"
+    tmp_namespace = Namespace.create!(owner: admin, name: tmp_namespace_path, path: tmp_namespace_path)
+
+    templates = if template_names.empty?
+                  Gitlab::ProjectTemplate.all
+                else
+                  Gitlab::ProjectTemplate.all.select { |template| template_names.include?(template.name) }
+                end
+
+    templates.each do |template|
       params = {
         import_url: template.clone_url,
-        namespace_id: admin.namespace.id,
+        namespace_id: tmp_namespace.id,
         path: template.name,
         skip_wiki: true
       }
@@ -32,19 +50,17 @@ namespace :gitlab do
       project = Projects::CreateService.new(admin, params).execute
       unless project.persisted?
-        puts project.errors.messages
-        exit(1)
+        raise "Failed to create project: #{project.errors.messages}"
       end
       loop do
-        if project.finished?
+        if project.import_finished?
           puts "Import finished for #{template.name}"
           break
         end
-        if project.failed?
-          puts "Failed to import from #{project_params[:import_url]}".red
-          exit(1)
+        if project.import_failed?
+          raise "Failed to import from #{project_params[:import_url]}"
         end
         puts "Waiting for the import to finish"
@@ -54,11 +70,23 @@ namespace :gitlab do
       end
       Projects::ImportExport::ExportService.new(project, admin).execute
-      download_or_copy_upload(project.export_file, template.archive_path)
-      Projects::DestroyService.new(admin, project).execute
+      downloader.call(project.export_file, template.archive_path)
+
+      unless Projects::DestroyService.new(project, admin).execute
+        puts "Failed to destroy project #{template.name} (but namespace will be cleaned up later)"
+      end
+
       puts "Exported #{template.name}".green
     end
-    puts "Done".green
+
+    success = true
+  ensure
+    if tmp_namespace
+      puts "Destroying temporary namespace #{tmp_namespace_path}"
+      tmp_namespace.destroy
+    end
+
+    puts "Done".green if success
   end
   def update(template)

spec/lib/gitlab/project_template_spec.rb

@@ -44,6 +44,12 @@ describe Gitlab::ProjectTemplate do
     end
   end
+  describe '.archive_directory' do
+    subject { described_class.archive_directory }
+
+    it { is_expected.to be_a Pathname }
+  end
+
   describe 'instance methods' do
     subject { described_class.new('phoenix', 'Phoenix Framework', 'Phoenix description', 'link-to-template') }

spec/tasks/gitlab/update_templates_rake_spec.rb

+# frozen_string_literal: true
+
+require 'rake_helper'
+
+describe 'gitlab:update_project_templates rake task' do
+  let!(:tmpdir) { Dir.mktmpdir }
+
+  before do
+    Rake.application.rake_require 'tasks/gitlab/update_templates'
+    create(:admin)
+    allow(Gitlab::ProjectTemplate)
+      .to receive(:archive_directory)
+        .and_return(Pathname.new(tmpdir))
+  end
+
+  after do
+    FileUtils.rm_rf(tmpdir)
+  end
+
+  it 'updates valid project templates' do
+    expect { run_rake_task('gitlab:update_project_templates', ['rails']) }
+      .to change { Dir.entries(tmpdir) }
+        .by(['rails.tar.gz'])
+  end
+end