| Commit message (Collapse) | Author | Age | Files | Lines |
|---|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
What
---
Make the retryableHTTP client introduced in
https://gitlab.com/gitlab-org/gitlab-shell/-/merge_requests/703 the
default HTTP client.
Why
---
In
https://gitlab.com/gitlab-com/gl-infra/production/-/issues/7979#note_1254964426
we've seen a 99% error reduction on `git` commands from `gitlab-shell`
when the retryableHTTP client is used.
This has been running in production for over 2 weeks in `us-east1-b` and
5 days fleet-wide so we should be confident that this client works as
expected.
Reference: https://gitlab.com/gitlab-com/gl-infra/production/-/issues/7979
Signed-off-by: Steve Azzopardi <sazzopardi@gitlab.com>
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
What
---
- Update the `client.HttpClient` fields to have `http.Client` and
`retryablehttp.Client`, one of them will be `nil` depending on the
feature flag toggle.
- Create new method `newRetryableRequest` which will create a
`retryablehttp.Request` and use that if the
`FF_GITLAB_SHELL_RETRYABLE_HTTP` feature flag is turned on.
- Add checks for `FF_GITLAB_SHELL_RETRYABLE_HTTP` everywhere we use the
http client to use the `retryablehttp.Client` or the default
`http.Client`
- New job `tests-integration-retryableHttp` to run the integraiton tests
with the new retryablehttp client. We didn't update go tests because
some assertions are different and will break table driven tests.
Why
---
As discussed in
https://gitlab.com/gitlab-org/gitlab-shell/-/merge_requests/703#note_1229645097
we want to put the client behind a feature flag, not just the retry
logic. This does bring extra risk for accessing a `nil` field but there
should be checks everytime we access `RetryableHTTP` and `HTTPClient`.
Reference: https://gitlab.com/gitlab-com/gl-infra/production/-/issues/7979
Signed-off-by: Steve Azzopardi <sazzopardi@gitlab.com>
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
What
---
Change the default `HTTP.Client` to
`github.com/hashicorp/go-retryablehttp.Client` to get automatic retries
and exponential backoff.
We retry the request 2 times resulting in 3 attempts of sending the
request, the min retry wait is 1 second, and the maximum is 15
seconds.
Hide the retry logic behind a temporary feature flag
`FF_GITLAB_SHELL_RETRYABLE_HTTP` to easily roll this out in GitLab.com.
When we verify that this works as expected we will remove
`FF_GITLAB_SHELL_RETRYABLE_HTTP` and have the retry logic as the default
logic.
Why
---
In https://gitlab.com/gitlab-com/gl-infra/production/-/issues/7979 users
end up seeing the following errors when trying to `git-clone(1)` a
repository locally on in CI.
```shell
remote: ===============================
remote:
remote: ERROR: Internal API unreachable
remote:
remote: ================================
```
When we look at the application logs we see the following error:
```json
{ "err": "http://gitlab-webservice-git.gitlab.svc:8181/api/v4/internal/allowed":
dial tcp 10.69.184.120:8181: connect: connection refused", "msg":
"Internal API unreachable"}
```
In
https://gitlab.com/gitlab-com/gl-infra/production/-/issues/7979#note_1222670120
we've correlated these `connection refused` errors with infrastructure
events that remove the git pods that are hosting
`gitlab-webservice-git` service. We could try to make the underlying
infrastructure more reactive to these changes as suggested in
https://gitlab.com/gitlab-com/gl-infra/production/-/issues/7979#note_1225164944
but we can still end up serving bad requests.
Implementing retry logic for 5xx or other errors would allow users to
still be able to `git-clone(1)` reposirories, although it being slower.
This is espically important during CI runs so users don't have to retry
jobs themselves.
Reference: https://gitlab.com/gitlab-com/gl-infra/production/-/issues/7979
Closes: https://gitlab.com/gitlab-org/gitlab-shell/-/issues/604
Signed-off-by: Steve Azzopardi <sazzopardi@gitlab.com>
|
| |
|
|
|
|
|
|
|
|
|
|
|
| |
While gitlab-shell currently has a major version of v14, the module path
it exposes is not using that major version like it is required by the Go
standard. This makes it impossible for dependents to import gitlab-shell
as a dependency without using a commit as version.
Fix this by changing the module path of gitlab-shell to instead be
`gitlab.com/gitlab-org/gitlab-shell/v14` and adjust all imports
accordingly.
Changelog: fixed
|
| |
|
|
|
|
| |
Contributes to https://gitlab.com/gitlab-org/gitlab-shell/-/issues/541
Changelog: removed
|
| | |
|
| | |
|
| |
|
|
|
| |
In this case we don't need to propagate cleanup
function. It simplifies the code.
|
| |
|
|
|
|
|
|
|
| |
Testify features sub packages `assert` and `require`. The difference is
subtle, and lost on novice Golang developers that don't read the docs.
To create a more consistent code base `assert` will no longer be used.
This change was generated by a running a sed command on all `_test.go`
files, followed by `goimports -w`.
|
| |
|
|
|
|
|
|
|
|
| |
The user agent for requests to the internal API endpoints used the
default Go provided user agent. This change updates that to always set
something else, by default `GitLab-Shell`.
Than for others importing the package, there's a new API to set it to
something else. This has been done with new method, a setter, to
maintain backwards compatibility in the API.
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Previously, gitlab-shell did not pass a context through the application.
Correlation IDs were generated down the call stack instead of passed
around from the start execution.
This has several potential downsides:
1. It's easier for programming mistakes to be made in future that lead
to multiple correlation IDs being generated for a single request.
2. Correlation IDs cannot be passed in from upstream requests
3. Other advantages of context passing, such as distributed tracing is
not possible.
This commit changes the behavior:
1. Extract the correlation ID from the environment at the start of
the application.
2. If no correlation ID exists, generate a random one.
3. Pass the correlation ID to the GitLabNet API requests.
This change also enables other clients of GitLabNet (e.g. Gitaly) to
pass along the correlation ID in the internal API requests
(https://gitlab.com/gitlab-org/gitaly/-/issues/2725).
Fixes https://gitlab.com/gitlab-org/gitlab-shell/-/issues/474
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
From
https://gitlab.com/gitlab-org/omnibus-gitlab/-/merge_requests/4498#note_397401883,
if you specify a relative path such as:
```
external_url 'http://gitlab.example.com/gitlab'
```
gitlab-shell doesn't have a way to pass the `/gitlab` to the host. For example, let's say we have:
```
gitlab_url: "http+unix://%2Fvar%2Fopt%2Fgitlab%2Fgitlab-workhorse%2Fsocket"
```
If we have `/gitlab` as the relative path, how do we specify what is the
UNIX socket path and what is the relative path? If we specify:
```
gitlab_url: "http+unix:///var/opt/gitlab/gitlab-workhorse.socket/gitlab
```
This is ambiguous. Is the socket in
`/var/opt/gitlab/gitlab-workhorse.socket/gitlab` or in
`/var/opt/gitlab/gitlab-workhorse.socket`?
To fix this, this merge request adds an optional
`gitlab_relative_url_root` config parameter:
```
gitlab_url: "http+unix://%2Fvar%2Fopt%2Fgitlab%2Fgitlab-workhorse%2Fsocket"
gitlab_relative_url_root: /gitlab
```
This is only used with UNIX domain sockets to disambiguate the socket
and base URL path. If `gitlab_url` uses `http://` or `https://`, then
`gitlab_relative_url_root` is ignored.
Relates to https://gitlab.com/gitlab-org/gitlab-shell/-/issues/476
|
| |
|
