Age | Commit message (Collapse) | Author |
|
Enforce that RPC definitions must have a comment and add a placeholder
for all instances where such a comment is missing.
|
|
Enforce that services must have a comment and add a placeholder for all
instances where such a comment is missing.
|
|
gitpipe: Fix propagation of context cancellation errors
Closes #4072
See merge request gitlab-org/gitaly!4524
|
|
When the context gets cancelled while we're iterating over results from
the object data pipeline, then the iterator doesn't return the context
cancellation error to the caller when calling `iter.Err()`. It is thus
easy to assume at the calling side that the iterator has just finished
successfully and that there are no more results, while in reality we
only got a partial set of results.
Fix this issue by propagating context cancellation errors to the caller.
This fixes RPCs based on this pipeline to not return `OK` when there
indeed was an error.
Changelog: fixed
|
|
When the context gets cancelled while we're iterating over results from
the object info pipeline, then the iterator doesn't return the context
cancellation error to the caller when calling `iter.Err()`. It is thus
easy to assume at the calling side that the iterator has just finished
successfully and that there are no more results, while in reality we
only got a partial set of results.
Fix this issue by propagating context cancellation errors to the caller.
This fixes RPCs based on this pipeline to not return `OK` when there
indeed was an error.
Changelog: fixed
|
|
When the context gets cancelled while we're iterating over results from
the revisions pipeline, then the iterator doesn't return the context
cancellation error to the caller when calling `iter.Err()`. It is thus
easy to assume at the calling side that the iterator has just finished
successfully and that there are no more results, while in reality we
only got a partial set of results.
Fix this issue by propagating context cancellation errors to the caller.
This fixes RPCs based on this pipeline to not return `OK` when there
indeed was an error.
Changelog: fixed
|
|
We're not bubbling up any errors in case contexts get cancelled while we
were consuming pipeline result. This error makes it easy to think that
the pipeline finished successfully even though the pipeline has been
cancelled and only returned partial results.
Add a set of tests to demonstrate this behaviour.
|
|
When sending events down the gitpipe pipelines, we need to ensure that
we're paying close attention to context cancellation: if the context got
cancelled, then we shouldn't continue driving the pipeline and instead
exit early. While we already do this in all of our pipelines, we're not
treating context cancellation the same everywhere. In some pipelines we
prioritize context cancellation errors over errors that happened while
serving the request, while in others we treat them the same. Treating
these errors the same can result in racy error propagation though, where
we randomly return either the context error, or the error of the process
we just tried to read from but which got killed because of the context
cancellation.
This issue has originally been fixed in 3a65ca3bb (gitpipe: Prioritize
context cancellation, 2021-07-09), but the fix was only a partial fix
that didn't apply to all pipelines. Fix the remaining cases so that we
always prioritize context cancellation to unify the behaviour.
Note that this commit also fixes multiple cases where we just sent an
error down the pipeline, but didn't actually abort the pipeline. This
was very likely an oversight by the original author who has been doing
too much C programming, where `case` statements automatically fall
through.
Changelog: fixed
|
|
repository: Exclude merge-requests, keep-around, pipelines from size
See merge request gitlab-org/gitaly!4532
|
|
Makefile: Fix protoc linting not working
See merge request gitlab-org/gitaly!4515
|
|
In order to generate our `NOTICE` file, we first export all Go sources
referenced by our project into a directory and then walk that directory
to gather any licenses. The result is that the generated `NOTICE` file
contains the licenses of all our dependencies.
Ideally, the `NOTICE` file wouldn't need to contain any licenses that we
have as part of our own sources. These may for example include licenses
of test files, but also licenses of vendored dependencies. But because
we are already bundling their licenses into our sources in the first
place it shouldn't be necessary to also put them into our `NOTICE` file.
In fact, it seems like we already intended to skip soaking up any such
license files in our own tree: we initially determine the module path of
ourselves, and then return early in `filepath.Walk()` when hitting a
directory path that matches. We only return a `nil` pointer though,
which means we only skip that single directory. That doesn't seem to
make a whole lot of sense though, and it feels like the intent was to
instead skip walking that whole directory.
So let's fix this by returning `filepath.SkipDir` instead so that we
never collect licenses part of our own source tree.
|
|
Move the `noticegen` tool into the top-level `tools/` directory so that
all of our custom build tools are in one place. This also makes its
sources discoverable for our formatter.
|
|
Move the `module-updater` tool into the top-level `tools/` directory so
that all of our custom build tools are in one place. This also makes its
sources discoverable for our formatter.
|
|
The Protoc plugins we use are hidden away deep into the `proto/`
directory, which makes it very hard to discover them when one doesn't
already know about their existence. Let's move them into a new top-level
`tools/` directory.
|
|
There is no real reason why the `protoc-gen-gitaly-lint` package
requires another internal package to provide the actual logic.
Furthermore, we want to move this plugin into a top-level `tools`
directory to make it easier to discover.
Absorb the `linter` package to make it easier to move the code around.
|
|
The `protoc-gen-gitaly-lint` plugin requires the Protobuf definitions in
order to work correctly, but it itself is added already when generating
these definitions in the first place. This is a cyclic dependency which
breaks generation of Protobufs when starting from scratch.
Now that functionality of `protoc-gen-gitaly` has been split up into two
different plugins we can easily fix this cyclic dependency by moving use
of the linting part into `lint-proto`. This refactoring also makes sense
on its own.
|
|
The `protoc-gen-gitaly` plugin provides two different functionalities:
- It lints our Protobuf definitions to make sure they conform to our
coding guidelines.
- It generates a `protolist.go` file, which contains a list of all
Protobuf files.
This is conflating two otherwise unrelated concerns, and thus makes the
divide between our `proto` and `lint-proto` Makefile targets a bit
blurry. Furthermore, it is creating a cyclic dependency: the linting
logic requires that the Protobuf files must have already been generated,
but we can only generate them when we have plugin compiled.
Prepare for a fix by splitting up functionality of this plugin into two
separate plugins: `protoc-gen-gitaly-lint` is responsible for linting
our definitions, while `protoc-gen-gitaly-protolist` will generate the
list of Protobuf files.
|
|
With 2a333822f (Use protoc_gen_gitaly plugin when regenerating protobuf
definitions, 2022-04-29), we have merged usage of Gitaly's protoc plugin
into the use of `make proto`. This was done because the plugin doesn't
only lint sources, but it also generates some data structures on its
own.
This process has broken use of the plugin completely though. While we
make the plugin's path known to `protoc` via the `--plugin` option, it
is not getting used at all because we didn't also move `--gitaly_out`.
This option is required though for the plugin to actually be used.
Fix this bug by adding the option back in.
|
|
Update test coverage artifact key
See merge request gitlab-org/gitaly!4533
|
|
The artifact key for the test coverage report was recently changed.
This commit updates the key in our gitlab-ci.yml to the new one.
|
|
cmd/gitaly: Fix log message announcing Gitaly version
See merge request gitlab-org/gitaly!4529
|
|
The RepositorySize RPC is used to determine the disk-usage of the
repository that is under the control of the user. There are certain refs
like refs/keep-around/*, refs/merge-requests/*, refs/pipelines/* that
GitLab uses internally, so these should not contribute to the overall
size of the repository.
Changelog: changed
|
|
There are times when we want to exclude certain refs from being included
in the size calculation for a repository. Add the ability to do so.
Changelog: added
|
|
Revert "Return correct error when repository has no up to date replicas"
See merge request gitlab-org/gitaly!4531
|
|
This reverts commit 7c78dcb8514bba6195993f83706bacebe3b71dd4.
|
|
On startup, Gitaly is printing its version. The message is currently
quite garbled though:
Starting GitalyversionGitaly, version 14.10.0
Fix the message to be less confused:
Starting Gitaly, version 14.10.0
|
|
gitaly: Shorten maximum internal socket path length
See merge request gitlab-org/gitaly!4523
|
|
go.mod: Remove exclude directive
See merge request gitlab-org/gitaly!4525
|
|
With the migration to the new runtime directory, Gitaly has moved its
internal socket directory as well. Unfortunately, this change resulted
in an expansion of the internal socket path length, and because Unix
systems put a limit on Unix socket paths this has caused Gitaly to not
come up on some systems anymore.
This problem is entirely fixable by the administrator by configuring a
different runtime directory path that has a shorter prefix. But we can
at least try to shorten the paths we generate a bit. Right now, Gitaly
creates three different types of internal sockets:
- The internal socket that is used e.g. by `gitaly-hooks` to connect
back to Gitaly. This file is created as `internal_${PID}.sock`.
Including the PID is not required anymore though: the runtime
directory always contains `gitaly-${PID}` anyway. So we can easily
shorten this to just `intern`. This leaves us with a socket name
length of 6.
- The Ruby worker sockets, which are created as `ruby.$N`. `$N` is
the number of any worker, and typically shouldn't be larger than
in the tens. This leaves us with a typical socket name length of
7.
- The internal test socket that is created to verify whether we can
create and connect to internal sockets. This is done as a sanity
check to alert administrators early on in case the socket path
length exceeds the system's limits. Right now it's created as
`test-XXXX.sock`, but given that we're about to change the naming
strategy we must only ensure that it's as long as the longest
socket we're creating. Furthermore, we don't need to create it
with a random part anymore given that the runtime directory is
keyed by PID. We thus use `tsocket`, which has a socket name
length of 7.
With these changes in place we reduce the maximum socket name length
from 19 characters to 7 characters, which gives us administrators 12
characters more room to play with.
|
|
Danger: Exclude generated protobuf files in size
See merge request gitlab-org/gitaly!4526
|
|
limithandler: Return structured errors
Closes #4197
See merge request gitlab-org/gitaly!4507
|
|
tests: Reduce usage of Git worktrees in service tests
See merge request gitlab-org/gitaly!4519
|
|
[ci skip]
|
|
[ci skip]
|
|
This change adds a lightly adjusted version of the changes_size rule.
This version excludes the generated protobuf files *.pb.go and *_pb.rb.
|
|
Remove the exlude directive that prevented us from pulling in a a buggy
version of grpc/grpc-go. We don't use this package anymore, so there's
no reason to keep this directive. Also, newer versions of grpc/grpc-go
has fixed the bug that's mentioned as the reason for the exclude.
Changelog: other
|
|
Return correct error when repository has no up to date replicas
See merge request gitlab-org/gitaly!4518
|
|
There are no tests left which require a worktree for their test setup,
and we don't want to encourage anybody adding such tests back in. After
all, production never uses non-bare repositories, and thus we don't want
to change any assumptions by using them in our tests.
Drop the ability to create worktrees in these tests.
|
|
Rewrite `ListConflictFiles()` tests to not use a worktree. Tests should
instead use `gittest.WriteCommit()` and `gittest.WriteTree()` to set up
the repository state, which both don't require a worktree.
|
|
There are no tests left which require a worktree for their test setup,
and we don't want to encourage anybody adding such tests back in. After
all, production never uses non-bare repositories, and thus we don't want
to change any assumptions by using them in our tests.
Drop the ability to create worktrees in these tests.
|
|
Rewrite `TreeEntries()` tests to not use a worktree. Tests should
instead use `gittest.WriteCommit()` and `gittest.WriteTree()` to set up
the repository state, which both don't require a worktree.
|
|
Rewrite a `ListLastCommitsForTree()` test to not use a worktree. Tests
should instead use `gittest.WriteCommit()` and `gittest.WriteTree()` to
set up the repository state, which both don't require a worktree.
|
|
Two tests in the `CommitService` code use worktrees without really
needing them. Refactor them to not use worktrees.
|
|
While we already have an option that allows writing a commit with a root
tree generated from a set of tree entries, we don't have any to create a
commit with a precomputed tree ID. Add a new option `WithTree()` to
allow this usecase.
|
|
Makefile: Install bundled Git v2.36.0.gl1
Closes #4190
See merge request gitlab-org/gitaly!4516
|
|
In order to alert upstream clients that Gitaly has reached capacity,
return a structured error of gitalypb.LimitError that clients like
workhorse, gitlab-shell, and rails can parse and take action on.
These errors all have the gRPC status Unavailable.
Changelog: changed
|
|
proto: Add CherryPickError type for UserCherryPick structured errors
See merge request gitlab-org/gitaly!4497
|
|
In UserCherryPick, there are some error conditions where instead of
returning an error, we return OK and embed the error in the response.
Instead of doing this, we want to return structured errors.
This change adds the proto definition for this error.
|
|
GetConsistentStorages retrieves the up to date replicas of a repository.
If the repository has no up to date replicas, it returns a repository
not found error. This is incorrect as the repository may still exist
from Praefect's point of view, it's just that the replicas with the
latest changes have been lost. This commit changes GetConsistentStorages
to return a more descriptive error for this scenario instead of a
repository not found error.
Changelog: fixed
|
|
commit: Various fixes for `CheckObjectsExist()`
See merge request gitlab-org/gitaly!4510
|