Change update entrypoint instead of adding new keep-alive one

3 files changed, 35 insertions, 76 deletions

diff --git a/lib/api/helpers/runner.rb b/lib/api/helpers/runner.rb
index 35ac0b4cbca..61eb88d3331 100644
--- a/lib/api/helpers/runner.rb
+++ b/lib/api/helpers/runner.rb
@@ -59,6 +59,11 @@ module API
       def max_artifacts_size
         Gitlab::CurrentSettings.max_artifacts_size.megabytes.to_i
       end
+
+      def job_forbidden!(job, reason)
+        header 'Job-Status', job.status
+        forbidden!(reason)
+      end
     end
   end
 end
diff --git a/lib/api/runner.rb b/lib/api/runner.rb
index ac62b83ba4a..dc102259ca8 100644
--- a/lib/api/runner.rb
+++ b/lib/api/runner.rb
@@ -125,7 +125,7 @@ module API
       end
       put '/:id' do
         job = authenticate_job!
-        forbidden!('Job is not running') unless job.running?
+        job_forbidden!(job, 'Job is not running') unless job.running?
 
         job.trace.set(params[:trace]) if params[:trace]
 
@@ -133,6 +133,8 @@ module API
                                   project: job.project.full_path)
 
         case params[:state].to_s
+        when 'running'
+          job.touch if job.needs_touch?
         when 'success'
           job.success!
         when 'failed'
@@ -140,22 +142,6 @@ module API
         end
       end
 
-      desc 'Marks job as live' do
-        http_codes [[200, 'Request accepted']]
-      end
-      params do
-        requires :id, type: Integer, desc: %q(Job's ID)
-        optional :token, type: String, desc: %q(Job's authentication token)
-      end
-      post '/:id/keep-alive' do
-        job = authenticate_job!
-
-        job.touch if job.running? && job.needs_touch?
-
-        status 200
-        header 'Job-Status', job.status
-      end
-
       desc 'Appends a patch to the job trace' do
         http_codes [[202, 'Trace was patched'],
                     [400, 'Missing Content-Range header'],
@@ -168,7 +154,7 @@ module API
       end
       patch '/:id/trace' do
         job = authenticate_job!
-        forbidden!('Job is not running') unless job.running?
+        job_forbidden!(job, 'Job is not running') unless job.running?
 
         error!('400 Missing header Content-Range', 400) unless request.headers.key?('Content-Range')
         content_range = request.headers['Content-Range']
diff --git a/spec/requests/api/runner_spec.rb b/spec/requests/api/runner_spec.rb
index c93612d7ada..57d238ff79b 100644
--- a/spec/requests/api/runner_spec.rb
+++ b/spec/requests/api/runner_spec.rb
@@ -816,6 +816,18 @@ describe API::Runner, :clean_gitlab_redis_shared_state do
 
           expect(job.reload.trace.raw).to eq 'BUILD TRACE'
         end
+
+        context 'when running state is sent' do
+          it 'updates update_at value' do
+            expect { update_job_after_time }.to change { job.reload.updated_at }
+          end
+        end
+
+        context 'when other state is sent' do
+          it "doesn't update update_at value" do
+            expect { update_job_after_time(20.minutes, state: 'success') }.not_to change { job.reload.updated_at }
+          end
+        end
       end
 
       context 'when job has been erased' do
@@ -838,6 +850,7 @@ describe API::Runner, :clean_gitlab_redis_shared_state do
           update_job(state: 'success', trace: 'BUILD TRACE UPDATED')
 
           expect(response).to have_gitlab_http_status(403)
+          expect(response.header['Job-Status']).to eq 'failed'
           expect(job.trace.raw).to eq 'Job failed'
           expect(job).to be_failed
         end
@@ -847,66 +860,10 @@ describe API::Runner, :clean_gitlab_redis_shared_state do
         new_params = params.merge(token: token)
         put api("/jobs/#{job.id}"), new_params
       end
-    end
-
-    describe 'POST /api/v4/jobs/:id/keep-alive' do
-      let(:job) { create(:ci_build, :running, :trace_live, runner_id: runner.id, pipeline: pipeline) }
-      let(:headers) { { API::Helpers::Runner::JOB_TOKEN_HEADER => job.token, 'Content-Type' => 'text/plain' } }
-      let(:update_interval) { 30.seconds }
-
-      it 'returns correct response' do
-        keep_alive_job
-
-        expect(response.status).to eq 200
-        expect(response.header).to have_key 'Job-Status'
-      end
-
-      it 'updates updated_at value' do
-        expect { keep_alive_job }.to change { job.updated_at }
-      end
-
-      context 'when project for the build has been deleted' do
-        let(:job) do
-          create(:ci_build, :running, :trace_live, runner_id: runner.id, pipeline: pipeline) do |job|
-            job.project.update(pending_delete: true)
-          end
-        end
-
-        it 'responds with forbidden' do
-          keep_alive_job
-
-          expect(response.status).to eq(403)
-        end
-      end
 
-      context 'when job has been canceled' do
-        before do
-          job.cancel
-        end
-
-        it 'returns job-status=canceled header' do
-          keep_alive_job
-
-          expect(response.status).to eq 200
-          expect(response.header['Job-Status']).to eq('canceled')
-        end
-      end
-
-      context 'when job has been errased' do
-        let(:job) { create(:ci_build, runner_id: runner.id, erased_at: Time.now) }
-
-        it 'rresponds with forbidden' do
-          keep_alive_job
-
-          expect(response.status).to eq 403
-        end
-      end
-
-      def keep_alive_job(token = job.token, **params)
-        new_params = params.merge(token: token)
+      def update_job_after_time(update_interval = 20.minutes, state = 'running')
         Timecop.travel(job.updated_at + update_interval) do
-          post api("/jobs/#{job.id}/keep-alive"), new_params
-          job.reload
+          update_job(job.token, state: state)
         end
       end
     end
@@ -1041,6 +998,17 @@ describe API::Runner, :clean_gitlab_redis_shared_state do
             end
           end
         end
+
+        context 'when the job is canceled' do
+          before do
+            job.cancel
+            patch_the_trace
+          end
+
+          it 'receives status in header' do
+            expect(response.header['Job-Status']).to eq 'canceled'
+          end
+        end
       end
 
       context 'when Runner makes a force-patch' do