Merge branch 'fix-runner-heartbeat' into 'master'

Heartbeat runner only for specific job-context requests

See merge request gitlab-org/gitlab!75022

lib/api/ci/helpers/runner.rb

@@ -52,7 +52,7 @@ module API
 
         # HTTP status codes to terminate the job on GitLab Runner:
         # - 403
-        def authenticate_job!(require_running: true)
+        def authenticate_job!(require_running: true, heartbeat_runner: false)
           job = current_job
 
           # 404 is not returned here because we want to terminate the job if it's
@@ -70,7 +70,17 @@ module API
             job_forbidden!(job, 'Job is not running') unless job.running?
           end
 
-          job.runner&.heartbeat(get_runner_ip)
+          # Only some requests (like updating the job or patching the trace) should trigger
+          # runner heartbeat. Operations like artifacts uploading are executed in context of
+          # the running job and in the job environment, which in many cases will cause the IP
+          # to be updated to not the expected value. And operations like artifacts downloads can
+          # be done even after the job is finished and from totally different runners - while
+          # they would then update the connection status of not the runner that they should.
+          # Runner requests done in context of job authentication should explicitly define when
+          # the heartbeat should be triggered.
+          if heartbeat_runner
+            job.runner&.heartbeat(get_runner_ip)
+          end
 
           job
         end

lib/api/ci/runner.rb

@@ -176,7 +176,7 @@ module API
           optional :exit_code, type: Integer, desc: %q(Job's exit code)
         end
         put '/:id', feature_category: :continuous_integration do
-          job = authenticate_job!
+          job = authenticate_job!(heartbeat_runner: true)
 
           Gitlab::Metrics.add_event(:update_build)
 
@@ -203,7 +203,7 @@ module API
           optional :token, type: String, desc: %q(Job's authentication token)
         end
         patch '/:id/trace', feature_category: :continuous_integration do
-          job = authenticate_job!
+          job = authenticate_job!(heartbeat_runner: true)
 
           error!('400 Missing header Content-Range', 400) unless request.headers.key?('Content-Range')
           content_range = request.headers['Content-Range']

spec/requests/api/ci/runner/jobs_artifacts_spec.rb

@@ -131,8 +131,8 @@ RSpec.describe API::Ci::Runner, :clean_gitlab_redis_shared_state do
               let(:send_request) { subject }
             end
 
-            it 'updates runner info' do
-              expect { subject }.to change { runner.reload.contacted_at }
+            it "doesn't update runner info" do
+              expect { subject }.not_to change { runner.reload.contacted_at }
             end
 
             shared_examples 'authorizes local file' do
@@ -280,8 +280,8 @@ RSpec.describe API::Ci::Runner, :clean_gitlab_redis_shared_state do
           end
         end
 
-        it 'updates runner info' do
-          expect { upload_artifacts(file_upload, headers_with_token) }.to change { runner.reload.contacted_at }
+        it "doesn't update runner info" do
+          expect { upload_artifacts(file_upload, headers_with_token) }.not_to change { runner.reload.contacted_at }
         end
 
         context 'when the artifact is too large' do
@@ -812,8 +812,8 @@ RSpec.describe API::Ci::Runner, :clean_gitlab_redis_shared_state do
           let(:send_request) { download_artifact }
         end
 
-        it 'updates runner info' do
-          expect { download_artifact }.to change { runner.reload.contacted_at }
+        it "doesn't update runner info" do
+          expect { download_artifact }.not_to change { runner.reload.contacted_at }
         end
 
         context 'when job has artifacts' do