[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:
-
ArtifactsUploaderCommandcreates a stream for archiving a list of files. -
GitLabClientcreates a read-write pipe and then a Goroutine that reads from that archive stream and writes to the pipe. -
GitLabClientthen sends the pipe's readio.ReadCloserto thehttp.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)