Removes http_status from ServiceResponse

* Updates one more instance of RetryBuild

app/controllers/projects/jobs_controller.rb

@@ -78,10 +78,13 @@ def trace
   end
   def retry
-    return respond_422 unless @build.retryable?
-    build = Ci::Build.retry(@build, current_user)
-    redirect_to build_path(build)
+    response = Ci::RetryJobService.new(project, current_user).execute(@build)
+    if response.success?
+      redirect_to build_path(response[:job])
+    else
+      respond_422
+    end
   end
   def play

app/controllers/projects/web_ide_terminals_controller.rb

@@ -57,11 +57,13 @@ def cancel
   end
   def retry
-    return respond_422 unless build.retryable?
-    new_build = Ci::Build.retry(build, current_user)
-
-    render_terminal(new_build)
+    response = Ci::RetryJobService.new(build.project, current_user).execute(build)
+    if response.success?
+      render_terminal(response[:job])
+    else
+      respond_422
+    end
   end
   private

app/graphql/mutations/ci/job/retry.rb

@@ -17,11 +17,19 @@ def resolve(id:)
           job = authorized_find!(id: id)
           project = job.project
-          ::Ci::RetryBuildService.new(project, current_user).execute(job)
-          {
-            job: job,
-            errors: errors_on_object(job)
-          }
+          response = ::Ci::RetryJobService.new(project, current_user).execute(job)
+
+          if response.success?
+            {
+              job: response[:job],
+              errors: []
+            }
+          else
+            {
+              job: nil,
+              errors: [response.message]
+            }
+          end
         end
       end
     end

app/models/ci/bridge.rb

@@ -57,10 +57,6 @@ class Bridge < Ci::Processable
       end
     end
-    def self.retry(bridge, current_user)
-      raise NotImplementedError
-    end
-
     def self.with_preloads
       preload(
         :metadata,
@@ -69,6 +65,10 @@ def self.with_preloads
       )
     end
+    def retryable?
+      false
+    end
+
     def inherit_status_from_downstream!(pipeline)
       case pipeline.status
       when 'success'

app/models/ci/build.rb

@@ -218,17 +218,21 @@ def first_pending
         pending.unstarted.order('created_at ASC').first
       end
-      def retry(build, current_user)
-        # rubocop: disable CodeReuse/ServiceClass
-        Ci::RetryBuildService
-          .new(build.project, current_user)
-          .execute(build)
-        # rubocop: enable CodeReuse/ServiceClass
-      end
-
       def with_preloads
         preload(:job_artifacts_archive, :job_artifacts, :tags, project: [:namespace])
       end
+
+      def extra_accessors
+        []
+      end
+
+      def clone_accessors
+        %i[pipeline project ref tag options name
+           allow_failure stage stage_id stage_idx trigger_request
+           yaml_variables when environment coverage_regex
+           description tag_list protected needs_attributes
+           job_variables_attributes resource_group scheduling_type].freeze
+      end
     end
     state_machine :status do
@@ -351,7 +355,9 @@ def with_preloads
         if build.auto_retry_allowed?
           begin
-            Ci::Build.retry(build, build.user)
+            # rubocop: disable CodeReuse/ServiceClass
+            Ci::RetryJobService.new(build.project, build.user).execute(build)
+            # rubocop: enable CodeReuse/ServiceClass
           rescue Gitlab::Access::AccessDeniedError => ex
             Gitlab::AppLogger.error "Unable to auto-retry job #{build.id}: #{ex}"
           end
@@ -472,12 +478,6 @@ def cancelable?
       active? || created?
     end
-    def retryable?
-      return false if retried? || archived? || deployment_rejected?
-
-      success? || failed? || canceled?
-    end
-
     def retries_count
       pipeline.builds.retried.where(name: self.name).count
     end

app/models/ci/processable.rb

@@ -101,6 +101,12 @@ def self.populate_scheduling_type!
       :merge_train_pipeline?,
       to: :pipeline
+    def retryable?
+      return false if retried? || archived? || deployment_rejected?
+
+      success? || failed? || canceled?
+    end
+
     def aggregated_needs_names
       read_attribute(:aggregated_needs_names)
     end

app/services/ci/play_build_service.rb

@@ -14,10 +14,7 @@ def execute(build, job_variables_attributes = nil)
           AfterRequeueJobService.new(project, current_user).execute(build)
         end
       else
-        # Retrying in Ci::PlayBuildService is a legacy process that should be removed.
-        # Instead, callers should explicitly execute Ci::RetryBuildService.
-        # See https://gitlab.com/gitlab-org/gitlab/-/issues/347493.
-        build.retryable? ? Ci::Build.retry(build, current_user) : build
+        Ci::RetryJobService.new(project, current_user).execute(build)[:job]
       end
     end

app/services/ci/retry_build_service.rb → app/services/ci/retry_job_service.rb

 # frozen_string_literal: true
 module Ci
-  class RetryBuildService < ::BaseService
+  class RetryJobService < ::BaseService
     include Gitlab::Utils::StrongMemoize
-    def self.clone_accessors
-      %i[pipeline project ref tag options name
-         allow_failure stage stage_id stage_idx trigger_request
-         yaml_variables when environment coverage_regex
-         description tag_list protected needs_attributes
-         job_variables_attributes resource_group scheduling_type].freeze
-    end
-
-    def self.extra_accessors
-      []
-    end
-
-    def execute(build)
-      build.ensure_scheduling_type!
-
-      clone!(build).tap do |new_build|
-        check_assignable_runners!(new_build)
-        next if new_build.failed?
-
-        Gitlab::OptimisticLocking.retry_lock(new_build, name: 'retry_build', &:enqueue)
-        AfterRequeueJobService.new(project, current_user).execute(build)
+    def execute(job)
+      if job.retryable?
+        job.ensure_scheduling_type!
+        new_job = retry_job(job)
+
+        ServiceResponse.success(payload: { job: new_job })
+      else
+        ServiceResponse.error(
+          message: 'Job cannot be retried',
+          payload: { job: job, reason: :not_retryable }
+        )
       end
     end
     # rubocop: disable CodeReuse/ActiveRecord
-    def clone!(build)
-      # Cloning a build requires a strict type check to ensure
+    def clone!(job)
+      # Cloning a job requires a strict type check to ensure
       # the attributes being used for the clone are taken straight
       # from the model and not overridden by other abstractions.
-      raise TypeError unless build.instance_of?(Ci::Build)
-      check_access!(build)
-      new_build = clone_build(build)
-      new_build.run_after_commit do
-        ::Ci::CopyCrossDatabaseAssociationsService.new.execute(build, new_build)
-        ::Deployments::CreateForBuildService.new.execute(new_build)
+      raise TypeError unless job.instance_of?(Ci::Build)
+      check_access!(job)
+      new_job = clone_job(job)
+      new_job.run_after_commit do
+        ::Ci::CopyCrossDatabaseAssociationsService.new.execute(job, new_job)
+        ::Deployments::CreateForBuildService.new.execute(new_job)
         ::MergeRequests::AddTodoWhenBuildFailsService
           .new(project: project)
-          .close(new_build)
+          .close(new_job)
       end
-      ::Ci::Pipelines::AddJobService.new(build.pipeline).execute!(new_build) do |job|
+      ::Ci::Pipelines::AddJobService.new(job.pipeline).execute!(new_job) do |processable|
         BulkInsertableAssociations.with_bulk_insert do
-          job.save!
+          processable.save!
         end
       end
-      build.reset # refresh the data to get new values of `retried` and `processed`.
-      new_build
+      job.reset # refresh the data to get new values of `retried` and `processed`.
+      new_job
     end
     # rubocop: enable CodeReuse/ActiveRecord
     private
-    def check_access!(build)
-      unless can?(current_user, :update_build, build)
+    def retry_job(job)
+      clone!(job).tap do |new_job|
+        check_assignable_runners!(new_job)
+        next if new_job.failed?
+
+        Gitlab::OptimisticLocking.retry_lock(new_job, name: 'retry_build', &:enqueue)
+        AfterRequeueJobService.new(project, current_user).execute(job)
+      end
+    end
+
+    def check_access!(job)
+      unless can?(current_user, :update_build, job)
         raise Gitlab::Access::AccessDeniedError, '403 Forbidden'
       end
     end
-    def check_assignable_runners!(build); end
-    def clone_build(build)
-      project.builds.new(build_attributes(build))
+    def check_assignable_runners!(job); end
+    def clone_job(job)
+      project.builds.new(job_attributes(job))
     end
-    def build_attributes(build)
-      attributes = self.class.clone_accessors.to_h do |attribute|
-        [attribute, build.public_send(attribute)] # rubocop:disable GitlabSecurity/PublicSend
+    def job_attributes(job)
+      attributes = job.class.clone_accessors.to_h do |attribute|
+        [attribute, job.public_send(attribute)] # rubocop:disable GitlabSecurity/PublicSend
       end
-      if build.persisted_environment.present?
+      if job.persisted_environment.present?
         attributes[:metadata_attributes] ||= {}
-        attributes[:metadata_attributes][:expanded_environment_name] = build.expanded_environment_name
+        attributes[:metadata_attributes][:expanded_environment_name] = job.expanded_environment_name
       end
       attributes[:user] = current_user
@@ -91,4 +91,4 @@ def build_attributes(build)
   end
 end
-Ci::RetryBuildService.prepend_mod_with('Ci::RetryBuildService')
+Ci::RetryJobService.prepend_mod_with('Ci::RetryJobService')

app/services/ci/retry_pipeline_service.rb

@@ -13,7 +13,7 @@ def execute(pipeline)
       builds_relation(pipeline).find_each do |build|
         next unless can_be_retried?(build)
-        Ci::RetryBuildService.new(project, current_user).clone!(build)
+        Ci::RetryJobService.new(project, current_user).clone!(build)
       end
       pipeline.processables.latest.skipped.find_each do |skipped|

ee/app/models/ee/ci/build.rb

@@ -58,6 +58,19 @@ module Build
         end
       end
+      class_methods do
+        extend ::Gitlab::Utils::Override
+
+        override :clone_accessors
+        def clone_accessors
+          (super + extra_accessors).freeze
+        end
+
+        def extra_accessors
+          (super + %i[secrets]).freeze
+        end
+      end
+
       override :variables
       def variables
         strong_memoize(:variables) do

ee/app/services/deployments/auto_rollback_service.rb

@@ -9,8 +9,13 @@ def execute(environment)
       deployment = find_rollback_target(environment)
       return error('Failed to find a rollback target.') unless deployment
-      new_deployment = rollback_to(deployment)
-      success(deployment: new_deployment)
+      response = rollback_to(deployment)
+
+      if response.success?
+        success(deployment: response[:job].deployment)
+      else
+        error(response.message)
+      end
     end
     private
@@ -48,7 +53,7 @@ def find_rollback_target(environment)
     end
     def rollback_to(deployment)
-      Ci::Build.retry(deployment.deployable, deployment.deployed_by).deployment
+      Ci::RetryJobService.new(deployment.deployable.project, deployment.deployed_by).execute(deployment.deployable)
     end
   end
 end

ee/app/services/ee/ci/retry_build_service.rb → ee/app/services/ee/ci/retry_job_service.rb

@@ -2,24 +2,10 @@
 module EE
   module Ci
-    module RetryBuildService
+    module RetryJobService
       extend ActiveSupport::Concern
       extend ::Gitlab::Utils::Override
-      class_methods do
-        extend ::Gitlab::Utils::Override
-
-        override :clone_accessors
-        def clone_accessors
-          (super + extra_accessors).freeze
-        end
-
-        override :extra_accessors
-        def extra_accessors
-          %i[secrets].freeze
-        end
-      end
-
       private
       override :check_access!

ee/spec/models/ci/build_spec.rb

@@ -40,6 +40,12 @@
     end
   end
+  describe 'extra_accessors' do
+    it 'includes the cloneable extra accessors' do
+      expect(::Ci::Build.extra_accessors).to eq([:secrets])
+    end
+  end
+
   describe 'associations' do
     it { is_expected.to have_many(:security_scans).class_name('Security::Scan') }
     it { is_expected.to have_one(:dast_site_profiles_build).class_name('Dast::SiteProfilesBuild').with_foreign_key(:ci_build_id) }

ee/spec/services/ci/play_build_service_spec.rb

@@ -3,7 +3,9 @@
 require 'spec_helper'
 RSpec.describe Ci::PlayBuildService, '#execute' do
-  it_behaves_like 'restricts access to protected environments'
+  it_behaves_like 'restricts access to protected environments' do
+    subject { service.execute(build) }
+  end
   it_behaves_like 'prevents playing job when credit card is required' do
     let(:user) { create(:user, maintainer_projects: [project]) }

ee/spec/services/ci/retry_build_service_spec.rb → ee/spec/services/ci/retry_job_service_spec.rb

 # frozen_string_literal: true
 require 'spec_helper'
-RSpec.describe Ci::RetryBuildService do
+RSpec.describe Ci::RetryJobService do
   let_it_be(:user) { create(:user) }
   let(:pipeline) { create(:ci_pipeline, project: project) }
-  let(:build) { create(:ci_build, pipeline: pipeline) }
+  let(:build) { create(:ci_build, :success, pipeline: pipeline) }
   subject(:service) { described_class.new(project, user) }
@@ -15,14 +15,16 @@
     project.add_developer(user)
   end
-  it_behaves_like 'restricts access to protected environments'
+  it_behaves_like 'restricts access to protected environments' do
+    subject { service.execute(build)[:job] }
+  end
   describe '#clone!' do
     context 'when user has ability to execute build' do
       let_it_be(:namespace) { create(:namespace) }
       let_it_be(:project) { create(:project, namespace: namespace, creator: user) }
-      let(:new_build) { service.clone!(build)}
+      let(:new_build) { service.clone!(build) }
       context 'dast' do
         let_it_be(:dast_site_profile) { create(:dast_site_profile, project: project) }
@@ -127,7 +129,7 @@
   end
   describe '#execute' do
-    let(:new_build) { service.execute(build) }
+    let(:new_build) { service.execute(build)[:job] }
     context 'when the CI quota is exceeded' do
       let_it_be(:namespace) { create(:namespace, :with_used_build_minutes_limit) }

ee/spec/services/deployments/auto_rollback_service_spec.rb

@@ -30,13 +30,25 @@
       commits.reverse_each { |commit| create_deployment(commit.id) }
     end
-    it 'successfully roll back a deployment' do
+    it 'successfully rolls back a deployment' do
       expect { subject }.to change { Deployment.count }.by(1)
       expect(subject[:status]).to eq(:success)
       expect(subject[:deployment].sha).to eq(commits[1].id)
     end
+    context 'when RetryJobService fails to retry the deployable' do
+      before do
+        allow_next_instance_of(::Ci::RetryJobService) do |service|
+          allow(service).to receive(:execute).and_return(ServiceResponse.error(message: message))
+        end
+      end
+
+      it_behaves_like 'rollback failure' do
+        let(:message) { 'Job cannot be retried.' }
+      end
+    end
+
     context 'when auto_rollback checkbox is disabled on the project' do
       before do
         environment.project.auto_rollback_enabled = false

ee/spec/support/shared_examples/services/build_execute_shared_examples.rb

@@ -6,7 +6,7 @@
     let(:project) { create(:project, :repository) }
     let(:pipeline) { create(:ci_pipeline, project: project) }
     let(:environment) { create(:environment, project: project, name: 'production') }
-    let(:build) { create(:ci_build, :created, pipeline: pipeline, environment: environment.name, project: project) }
+    let(:build) { create(:ci_build, :success, pipeline: pipeline, environment: environment.name, project: project) }
     let(:protected_environment) { create(:protected_environment, name: environment.name, project: project) }
     let(:service) { described_class.new(project, user) }
@@ -19,8 +19,7 @@
     context 'when user does not have access to the environment' do
       it 'raises Gitlab::Access::DeniedError' do
-        expect { service.execute(build) }
-          .to raise_error Gitlab::Access::AccessDeniedError
+        expect { subject }.to raise_error Gitlab::Access::AccessDeniedError
       end
     end
@@ -30,9 +29,7 @@
       end
       it 'enqueues the build' do
-        build_enqueued = service.execute(build)
-
-        expect(build_enqueued).to be_pending
+        is_expected.to be_pending
       end
     end
   end

lib/api/ci/jobs.rb

@@ -114,11 +114,14 @@ class Jobs < ::API::Base
           build = find_build!(params[:job_id])
           authorize!(:update_build, build)
-          break forbidden!('Job is not retryable') unless build.retryable?
-          build = ::Ci::Build.retry(build, current_user)
-          present build, with: Entities::Ci::Job
+          response = ::Ci::RetryJobService.new(@project, current_user).execute(build)
+          if response.success?
+            present response[:job], with: Entities::Ci::Job
+          else
+            forbidden!('Job is not retryable')
+          end
         end
         desc 'Erase job (remove artifacts and the trace)' do

spec/controllers/projects/jobs_controller_spec.rb

@@ -796,7 +796,7 @@ def get_trace
           retried_build = Ci::Build.last
-          Ci::RetryBuildService.clone_accessors.each do |accessor|
+          Ci::Build.clone_accessors.each do |accessor|
             expect(job.read_attribute(accessor))
               .to eq(retried_build.read_attribute(accessor)),
               "Mismatched attribute on \"#{accessor}\". " \

spec/models/ci/bridge_spec.rb

@@ -30,6 +30,12 @@
     expect(bridge).to have_one(:downstream_pipeline)
   end
+  describe '#retryable?' do
+    it 'returns false' do
+      expect(bridge.retryable?).to eq(false)
+    end
+  end
+
   describe '#tags' do
     it 'only has a bridge tag' do
       expect(bridge.tags).to eq [:bridge]