Skip to content

Improve responses for Runner's new job requests

Tomasz Maczukin requested to merge split-conflict-and-validation-errors-from-job-scheduling-error-analyse into master

What does this MR do?

Split 'conflict' and 'validation error' case for job scheduling

When job is being scheduled, GitLab tries to detect concurrent requests that would get the same job. In that case only the first Runner that asked for a job gets it, and all other requests are internally handled again (until there are other jobs in the queue that matches such specific Runner).

When such "conflict" situation was detected and no job was found in the loop repeat, Runner receives a 409 Conflict response, which leaves a track about such situation. It additionally triggers a request back-off on Runner, so in highly concurrent installations it should generate some intervals between concurrent requests from different Runners.

However, one of the symptoms used for finding the "conflict" situation is the StateMachines::InvalidTransition exception, which may be also thrown when some data doesn't pass validation. This is a case that should definitely not produce a 409 Conflict response, but instead a 400 Bad Request one with details about what was wrong should be sent.

This commit adds a needed update to the Ci::RegisterJobService service and the /api/v4/jobs/request endpoint.

Note that this is a GitLab EE change (since here we have additional parts of code to change), that should be ported to GitLab CE.

More details about the problem and why we'fe decided to handle it like that can be found at gitlab-runner#4360 (closed).

GitLab CE port at: https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/31678

Does this MR meet the acceptance criteria?

Conformity

Performance and testing

Security

If this MR contains changes to processing or storing of credentials or tokens, authorization and authentication methods and other items described in the security review guidelines:

  • [-] Label as security and @ mention @gitlab-com/gl-security/appsec
  • [-] The MR includes necessary changes to maintain consistency between UI, API, email, or other methods
  • [-] Security reports checked/validated by a reviewer from the AppSec team

References gitlab-runner#4360 (closed)

Edited by 🤖 GitLab Bot 🤖

Merge request reports