Reorgnize PagesWorkerfix/sm/fix_update_pages_service-ce

3 files changed, 35 insertions, 49 deletions

diff --git a/app/services/projects/update_pages_configuration_service.rb b/app/services/projects/update_pages_configuration_service.rb
index 52ff64cc938..36482b04659 100644
--- a/app/services/projects/update_pages_configuration_service.rb
+++ b/app/services/projects/update_pages_configuration_service.rb
@@ -9,9 +9,6 @@ module Projects
     def execute
       update_file(pages_config_file, pages_config.to_json)
       reload_daemon
-      success
-    rescue => e
-      error(e.message)
     end
 
     private
diff --git a/app/services/projects/update_pages_service.rb b/app/services/projects/update_pages_service.rb
index 5c2c7823f86..1f28a21f109 100644
--- a/app/services/projects/update_pages_service.rb
+++ b/app/services/projects/update_pages_service.rb
@@ -30,38 +30,23 @@ module Projects
 
         # Check if we did extract public directory
         archive_public_path = File.join(archive_path, 'public')
-        raise ArtifactNotFoundError, 'pages miss the public folder' unless Dir.exist?(archive_public_path)
+        raise PublicFolderNotFoundError, 'pages miss the public folder' unless Dir.exist?(archive_public_path)
         raise PageOutdateError, 'pages are outdated' unless latest?
 
         deploy_page!(archive_public_path)
-        success
+        @status.success!
+        delete_artifact!
       end
-    rescue => e
-      register_failure
-      error(e)
-    ensure
-      register_attempt
-    end
-
-    private
-
-    def success
-      @status.success!
-      delete_artifact!
-      super
-    end
-
-    def error(e, http_status = nil)
+    rescue ArtifactNotFoundError, PageOutdateError, PublicFolderNotFoundError => e
       log_error("Projects::UpdatePagesService: #{e.message}")
       @status.allow_failure = !latest?
       @status.description = e.message
       @status.drop(:script_failure)
-      if e.instance_of(PublicFolderNotFoundError) || e.instance_of(PageOutdateError)
-        delete_artifact!
-      end
-      super
+      delete_artifact! if e.is_instance_of(PageOutdateError) || e.is_instance_of(PublicFolderNotFoundError)
     end
 
+    private
+
     def create_status
       GenericCommitStatus.new(
         project: project,
@@ -182,21 +167,5 @@ module Projects
     def sha
       build.sha
     end
-
-    def register_attempt
-      pages_deployments_total_counter.increment
-    end
-
-    def register_failure
-      pages_deployments_failed_total_counter.increment
-    end
-
-    def pages_deployments_total_counter
-      @pages_deployments_total_counter ||= Gitlab::Metrics.counter(:pages_deployments_total, "Counter of GitLab Pages deployments triggered")
-    end
-
-    def pages_deployments_failed_total_counter
-      @pages_deployments_failed_total_counter ||= Gitlab::Metrics.counter(:pages_deployments_failed_total, "Counter of GitLab Pages deployments which failed")
-    end
   end
 end
diff --git a/app/workers/pages_worker.rb b/app/workers/pages_worker.rb
index d3b95009364..d867a9ca92e 100644
--- a/app/workers/pages_worker.rb
+++ b/app/workers/pages_worker.rb
@@ -1,24 +1,44 @@
 class PagesWorker
   include ApplicationWorker
 
-  sidekiq_options retry: false
+  sidekiq_options retry: 3
 
   def perform(action, *arg)
     send(action, *arg) # rubocop:disable GitlabSecurity/PublicSend
   end
 
-  def deploy(build_id)
-    build = Ci::Build.find_by(id: build_id)
-    result = Projects::UpdatePagesService.new(build.project, build).execute
-    if result[:status] == :success
-      result = Projects::UpdatePagesConfigurationService.new(build.project).execute
-    end
+  def deploy(job_id)
+    register_attempt
 
-    result
+    Ci::Build.find_by(id: job_id).try do |job|
+      Projects::UpdatePagesService.new(job.project, job).execute
+      Projects::UpdatePagesConfigurationService.new(job.project).execute
+    end
+  rescue => e
+    register_failure
+    raise e
   end
 
   def remove(namespace_path, project_path)
     full_path = File.join(Settings.pages.path, namespace_path, project_path)
     FileUtils.rm_r(full_path, force: true)
   end
+
+  private
+
+  def register_attempt
+    pages_deployments_total_counter.increment
+  end
+
+  def register_failure
+    pages_deployments_failed_total_counter.increment
+  end
+
+  def pages_deployments_total_counter
+    @pages_deployments_total_counter ||= Gitlab::Metrics.counter(:pages_deployments_total, "Counter of GitLab Pages deployments triggered")
+  end
+
+  def pages_deployments_failed_total_counter
+    @pages_deployments_failed_total_counter ||= Gitlab::Metrics.counter(:pages_deployments_failed_total, "Counter of GitLab Pages deployments which failed")
+  end
 end