[17.9] Fix HTTP retries not working properly

What does this MR do?

This backports !5409 (merged) to 17-9-stable.

Previously if GitLab rate-limited an HTTP request to the runner API, the runner would attempt to retry but omit the body. The job log would then show a message such as:

couldn't execute PUT against https://gitlab.example.com/api/v4/jobs/123:
Put "https://gitlab.example.com/api/v4/jobs/123": http: ContentLength=972 with Body length 0

This happened because the rate limiter requester would reuse the http.Request, but the Body had already been consumed by the first attempt. This prevented HTTP retries from working altogether. The GitLab server displayed EOF errors when this occurred.

Go supplies a way to handle this: the GetBody function can be set in an http.Request. This function is expected to produce the body.

This commit changes the way that the request body is passed to the network client. If the bytes to send are known and content length is predefined, clients can pass in a BytesProvider. For example, this is used for job log traces and PUT API requests that update the job status. BytesProvideralways produces an io.ReadCloser with the given set of bytes.

Things get a little more complicated for artifacts uploads. This is how artifact uploads work:

  1. ArtifactsUploaderCommand creates a stream for archiving a list of files.
  2. GitLabClient creates a read-write pipe and then a Goroutine that reads from that archive stream and writes to the pipe.
  3. GitLabClient then sends the pipe's read io.ReadCloser to the http.Request, which then reads from that pipe and writes the body to the netowrk.

BytesProvider doesn't work here because the content is not known ahead of time. For this case, this commit introduces a StreamProvider that wraps these pipes and streams. The caller is responsible for returning a factory that produces a io.ReadCloser type.

With both BytesProvider and StreamProvider, the runner now can set the GetBody function. The rate limit requester must call this manually and set Content-Length if known for retries to work.

Note that a side effect of setting this function is that Go will automatically and quietly handle 307 and 308 redirections. Previously the runner manually handled redirects and displayed a message in the job log. With this change, however, users will no longer see an indication that a redirection is happening behind the scenes.

Relates to #38651 (closed)

Edited by Stan Hu

Merge request reports

Loading