| Age | Commit message (Collapse) | Author |
|---|
|
repocleaner: Log warning message for repositories not known to praefect
Closes #3719
See merge request gitlab-org/gitaly!3839
|
|
Background task runs with start of the praefect and periodically
and scans gitaly nodes for the repositories. For each repository
that is known to praefect it writes a warning log message.
Part of: https://gitlab.com/gitlab-org/gitaly/-/issues/3719
|
|
Use PerRepositoryElector in test-with-praefect
See merge request gitlab-org/gitaly!3914
|
|
Ensure log entries have time formatted as UTC
Closes #3710
See merge request gitlab-org/gitaly!3912
|
|
Use a formatter with logrus logs so all log entries have the time field
as UTC. Also fixed a few places that weren't using the configured
logger.
Changelog: fixed
|
|
Praefect currently is not waiting for a health check to complete
before it takes over the sockets and starts routing RPCs. This can
be a problem as the RPCs will fail due to not having any healthy nodes
available if they are served prior to performing the first check. This
commit fixes the issues by waiting for the first health check to complete
prior to serving RPCs.
Changelog: fixed
|
|
operations: Always return structured errors from Git2Go merges
Closes #3756
See merge request gitlab-org/gitaly!3896
|
|
In commit 84bd2721e (gitaly-git2go: Return structured conflict error on
merge conflict, 2021-08-26), we have changed gitaly-git2go's merge
subcommand to support structured errors. This allows us to return more
information why a merge failed, e.g. by returning the set of conflicting
files to the caller.
Drop the feature flag which guards this code and thus always enable the
use of structured errors.
|
|
While the "set-config" subcommand can in theory still be called when
doing a zero-downtime upgrade given that its corresponding RPC still
existed in this release, in practice the RPC wasn't ever called anymore.
As a result, we can already drop the subcommand right now without having
to fear any regressions.
|
|
Drop support for Go 1.15
See merge request gitlab-org/gitaly!3879
|
|
With Go 1.16, the ioutil package was deprecated. Replace our usage of
`ioutil.WriteFile()` with `os.WriteFile()` to adapt accordingly.
|
|
With Go 1.16, the ioutil package was deprecated. Replace our usage of
`ioutil.TempFile()` with `os.CreateTemp()` to adapt accordingly.
|
|
With Go 1.16, the ioutil package was deprecated. Replace our usage of
`ioutil.TempDir()` with `os.MkdirTemp()` to adapt accordingly.
|
|
With Go 1.16, the ioutil package was deprecated. Replace our usage of
`ioutil.ReadFile()` with `os.ReadFile()` to adapt accordingly.
|
|
With Go 1.16, the ioutil package was deprecated. Replace our usage of
`ioutil.Discard` with `io.Discard` to adapt accordingly.
|
|
While the helpers `IncomingToOutgoing()` and `OutgoingToIncoming()` both
fall into the metadata categories, they're still part of the generic
"helper" package. Move them over into the "metadata" package to make
them easier to find.
|
|
remove-repository: A new sub-command for the praefect to remove repository
See merge request gitlab-org/gitaly!3767
|
|
Users of the praefect are missing tools to manage state of the cluster.
One of such tools is a repository removal cli. It should be used to
remove any repository from the cluster. The removal covers cleanup of
the database and removal from the gitaly storages. The command tries
to remove as much as possible first by removing from praefect, replication
event queue and each gitaly node configured in the provided config file.
Part of: https://gitlab.com/gitlab-org/gitaly/-/issues/3771
Changelog: added
|
|
|
|
updateref: Partially fix locking semantics
See merge request gitlab-org/gitaly!3829
|
|
Before we have converted the updateref package to always use
transactional semantics in 154dbc1a2 (updateref: Safeguard against
accidentally committing updates, 2020-12-15), all the `Wait()` function
used to do was to call `Wait()` on git-update-ref(1). This simply closed
stdin and then waited for the command to return.
But since above commit, semantics have changed such that we now always
use an explicit transaction, where `Wait()` is now responsible for
committing the transaction by sending "commit" to the process. In this
context, calling the function `Wait()` doesn't really convey its spirit
anymore: yes, we wait for the command, but what we are really doing is
to commit the transaction.
Rename the function to `Commit()` to better highlight its modern use.
|
|
In 773668f55 (hook: Fix prereceive returning AllowedError for generic
errors, 2021-08-13), we have refactored our allowed checks to return a
generic error in case the call to `Allowed()` itself failed. The
assumption here was that because the call returns an `allowed` boolean,
it would not raise an error if the call wasn't allowed.
As it turns out, this assumption is wrong: the GitlabNetClient in
gitlab-shell will always return an error if the HTTP status code is not
between 200 and 399, and access checks do return an error code in case
access was denied. As a result, the error we returned to the Gitaly
client wouldn't have its "GitLab: " prefix anymore and thus GitLab
wouldn't recognize this error as an error that shall be returned to the
user.
Ideally, we'd properly fix this by inspecting errors returned by the
GitLabNetClient. But this isn't possible without parsing the error
messages given that the client will only return generic errors. So this
commit just reverts the state to what we had before, where we simply
treat all errors returned by the client as user-facing errors. This
logic is broken and will reveal messages to the user which aren't
intended for him in the first place, but we cannot help this for now.
Changelog: fixed
|
|
global: Support Go-1.17-style build tags
See merge request gitlab-org/gitaly!3804
|
|
Assign repositories unique IDs in Praefect
See merge request gitlab-org/gitaly!3762
|
|
With Go 1.17, new syntax was introduced for build tags which has the
intent to be much easier to use compared to the previous `+build` ones.
To ease the migration, Go 1.17 supports both old-style and new-style
build tags, where the recommended migration path is to have both as long
as projects support older Go versions which don't yet know about the new
syntax.
Migrate our codebase to use both both styles. While we don't yet support
Go 1.17 officially, it doesn't hurt to be prepared, and furthermore it
fixes linting issues I have been observing on my machine.
|
|
Refactor backup pipeline to use commands
See merge request gitlab-org/gitaly!3796
|
|
When a repository is being created, all of the records need to be connected
via the repository ID. This commit expands CreateRepository to take in the
repository ID Praefect reserved for the repository while routing the creation
request. CreateRepository then links all of the records it creates via the
repository id. While the repository ID is not really used for anything yet, the
newly created records being linked via the ID allow us to run a migration in the
following record to just link all existing records correctly via the repository
ID. Any concurrently created records would already be linked correctly by the code
added in this commit.
|
|
When Praefect is routing a repository creation, it needs to reserve
a repository ID in order to generate later a relative path for the
repository. This commit changes the PerRepositoryRouter to reserve a
repository ID and return in the route. The ID reservation also acts
as a fail fast check to see whether there already exists a repository
with the given virtual storage and relative path.
|
|
Now that it no longer only handles create, we can make the name more
generic.
|
|
Changelog: changed
|
|
In order to implement incremental backups we will be adding more
operations than just Create/Restore but once we add these operations it
would become difficult to also keep concurrency. So instead we
encapsulate the execution of these operations into commands.
|
|
Reformat sources with gofumpt.
|
|
The GitLab mock client will currently always return success for all of
its stubbed functions. In a subsequent patch, we'll want to write tests
though which return various failures instead. Prepare for this by
generalizing the mock client such that callers can pass arbitrary
implementations of its mocked functions.
|
|
The pre-receive hook implementation is responsible for calling GitLab's
`/internal/allowed` API to do access checks. In case this API endpoint
tells us that a change is not allowed, we need to make the error message
available to the original caller, which then knows to return the error
message via the response's `PreReceiveError` field.
Our current implementation of this is wrong though: while we correctly
pass up information via a `NotAllowedError` in case the access check
indicates that the change is not allowed, we also pass up information
when invoking the endpoint itself failed. But this really is an internal
error, and we don't ever want to return such information to the user.
Fix this issue by returning a normal error in such cases.
Changelog: fixed
|
|
We have recently started efforts to return structured error types from
Gitaly RPCs to tell upstream callers why an error was raised, which is
going to replace the current approach where error information is passed
as plain string in a normal RPC response. The first candidate we're
converting is UserMergeBranch.
While it is easy in UserMergeBranch to return error details in case
access checks fail, we also want to return details in case the merge
failed due to conflicts. The most important info we want to return in
this case is which files are conflicting as well as the revisions which
do. While we already have info about the revisions, Gitaly currently
cannot tell which files were conflicting: this information is only known
to gitaly-git2go, and we do not pass up this information in any way.
Now that we have gob serialization in place in gitaly-git2go's merge
subcommand, this is an easy fix though given that we can easily return
"real" errors. This commit thus introduces a new `ConflictingFilesError`
which has a slice of conflicting files as field and returns it in case
gob serialization is in use.
Note that we do not yet use this rich information for now, but this
change only lays the groundwork. As such, no change in behaviour is
expected from this commit.
|
|
The gitaly-git2go merge subcommand has learned to use gob for
serialization of parameters and results. This commit now teaches the
internal package to drive the subcommand with gob.
This change is implemented behind a feature flag because of backwards
compatibility.
|
|
Back when we have started implementing RPCs via gitaly-git2go, the
initial implementations were all using JSON as representation for inputs
and outputs. At a later point we noticed that this makes it hard to
return typed errors, which is why later subcommands have shifted to
using gob encoders.
The merge subcommand is the first command we have written, and thus it
also uses JSON encoding, facing the exact error handling issue mentioned
above. We want to extend the command to return information about
conflicting files though, which would ideally be part of the returned
error type.
Refactor the subcommand to optionally support gob as encoding: if the
command is called with no JSON request, then we instead use gob. While
this seems kind of cheap, it's easier than adding a separate flag and
should always provide expected results: we never call the command
without a request, and the request cannot ever be empty given that it's
a JSON-encoded struct.
|
|
Originally, gitaly-git2go commands would always write to their responses
to stdout themselves. With the introduction of gob for serialization and
deserialization of inputs and outputs, this has changed to pass in a
reader and writer to all commands. The merge subcommand is still
directly writing to stdout though.
Fix this issue by writing to the passed-in writer instead. This is only
a theoretical issue: there are no callers which would ask gitaly-git2go
subcommands to write anywhere else.
|
|
The gitaly-git2go merge code handles deserialization of params,
deserialization of results and the actual merge logic in a single
function. This makes it hard to reason about inputs and outputs of this
function and thus prohibits an easy refactor of them.
Refactor the code and pull out the merge logic into its own function,
where all parameters are obtained as input and output is handled by the
calling function.
No change in behaviour is expected from this refactoring.
|
|
This allows testing of new locator strategies without breaking any
existing backups.
|
|
This change implements the license finding in Go using
github.com/go-enry/go-license-detector.
Issue: https://gitlab.com/gitlab-org/gitaly/-/issues/3078
Changelog: performance
|
|
Upgrade golangci-lint to v1.42.0
See merge request gitlab-org/gitaly!3786
|
|
This RPC will not be used by default, it sits behind a feature flag.
Eventually this will replace PackObjectsHook.
Changelog: other
|
|
The documentation of `signal.Notify()` mentions that the caller must
ensure that the provided channel is sufficiently buffered, but we pass
in an unbuffered channel. Let's fix this by creating the channel with a
buffer size of "1" such that we can receive at least one signal.
|
|
Don't schedule replication jobs in SetAuthoritativeStorage
See merge request gitlab-org/gitaly!3776
|
|
InfoService previously used the replication queue to schedule replication
jobs as part of the manual reconciliation command and when accepting
data loss. The manual reconciler has been removed and accepting data loss
relies on the automatic reconciler. As the InfoService no longer needs to
queue replication jobs, this commit removes the unnecessary field and updates
the call sites not to inject the queue.
|
|
test: Parallel SQL dependent tests
See merge request gitlab-org/gitaly!3778
|
|
As now we create a new database on each call of the
glsql.GetDB the test execution time increased. To mitigate
that problem we make tests dependent on the PostgreSQL database
parallel. It should reduce test execution time and may help us
to find new issues in the code because of the shared global state.
|
|
As now we create a new database on each call of the
glsql.GetDB and do not re-use existing database created
on a first call of the function we rename the function.
NewDB is self-explanatory.
|
|
Previously the wrapper was used to omit repetition of the
same database name in the package. As now we create a new
database on each call of the glsql.GetDB with unique
autogenerated name the wrapper is redundant.
|
