| Age | Commit message (Collapse) | Author |
|---|
|
This reverts merge request !2482
|
|
Detect Git executable getting resolved via PATH
See merge request gitlab-org/gitaly!2482
|
|
When configuring Gitaly setups, admins can choose which Git executable
they want Gitaly to use for all Git invocations by specifing the
`config.git_path` key. Because of this, Gitaly is never allowed to
lookup Git via the PATH environment, but always needs to consult the
configured value. While most of our code (except for tests which have
been fixed with preceding commits) always honored this, there were two
locations which didn't, which wen't silently under the radar waiting for
trouble.
This commit introduces an improved test setup which will always catch
such bugs: we simply set up a Git executable in a separate directory
which writes an error message and immediately aborts with a strange
error value and then adjust our PATH variable to have the executable's
directory as first item. As a result, whenever Git is being executed
with an unqualified path, we'll pick this binary instead of the real Git
and produce an error. Tests would quickly catch this, e.g. like the
following:
```
--- FAIL: TestSuccessfulIsAncestorRequestWithAltGitObjectDirs (0.04s)
commit.go:80: stdout: , stderr: Git executable from $PATH was picked up. Please fix code to use `command.GitPath()` instead.
```
The real Git binary is injected via the `GITALY_TESTING_GIT_BINARY`
environment variable, which is being picked up by our configuration
code.
|
|
In order to allow easy testing of Gitaly against custom Git binaries and
different versions thereof, this commit introduces a new environment
variable `GITALY_TESTING_GIT_BINARY`. If this variable is set, then
we'll use its value as the Git binary for testing.
|
|
With PATH now containing a broken version of Git by design, we need to
adjust all callers of Git to use the real executable instead of the
broken one. The script `check-mod-tidy` is one of those which needs
adjusting.
Given it's such a tiny script, let's just inline it into our Makefile
and have it use the `${GIT}` variable.
|
|
When executing git-linguist, one of the first things it'll do is verify
whether it's running in a Git directory by executing `git rev-parse
--git-dir`. git-linguist doesn't allow specifying which Git executable
is executed here, so it'll always use the first one it finds in PATH,
which isn't necessarily the one configured in Gitaly's configuration.
Fix the issue by always prepending the configured Git executable's
directory to PATH previous to executing git-linguist. As our own command
interface will overwrite and PATH environment variables passed to it, we
need to use a hack here and specify the PATH variable by executing
git-linguist with `env PATH=$GITDIR:$PATH`.
|
|
When reading the index in `internal/git/packfile`, we're directly
executing the "git" executable which is thus going to be resolved via
the PATH environment variable.
Fix this to instead use the configured Git command.
|
|
Two of our helpers in the rspec tests use the system Git instead of the
configured one. While this isn't much of a problem by itself as it's
only tests, we're going to change test execution so that we can verify
only the configured Git executable is ever executed. Executing Git via
the PATH variable is always going to fail with this setup.
Prepare for this by honoring the GITALY_TESTING_GIT_BINARY environment
variable like our Go tests do. Convert callers to use the configured
value. As there's now a dependency cycle between the spec helper and the
test repo helper because the test repo helper depends on the overrides
defined in the spec helper, this commit also moves those overrides into
the test repo helper.
|
|
In various tests we're setting up hooks in order to verify certain
operations behave as we intend them to. Some of those hooks we have
invoke Git again, but they'll use the PATH variable to do so. As a
result, they may use the wrong Git executable instead of the configured
one.
Fix this by generating hook scripts with the configured and resolved Git
path.
|
|
Remove some unused code
See merge request gitlab-org/gitaly!2480
|
|
In order to test our support for Git protocol v2, we have set up a
wrapper script around Git which captures its environment and writes it
to a file. This allows us to check whether required options are set up
correctly for protocol v2 to be enabled. The wrapper script we currently
use doesn't invoke the correct Git executable, though, as it will simply
use the one present in `$PATH`.
Fix the issue by writing a temporary script containing the correct path
configured via `config.Config.Git.Binary`. All the other alternatives
like injecting another environment variable would've been non-trivial to
implement, as we'd need to manually inject them at the right spots.
Note that there's one peculiarity here: as we're about to migrate to use
`command.GitPath()` everywhere, it will cause us to invoke the wrapper
multiple times. To fix this, this commit starts appending to the
environment file instead of overwriting it on each execution, deleting
the file after the test is done.
|
|
While production code uses the Git DSL to execute Git commands in most
places which always respects the configured Git binary, many of our
tests don't. This commit fixes them by modifying `MustRunCommand()` to
automatically adjust invocations of `"git"` and fixing up any other
locations which used `exec.Cmd` directly.
|
|
|
|
|
|
Pass CORRELATION_ID env variable to spawned git subprocesses
See merge request gitlab-org/gitaly!2478
|
|
|
|
|
|
Log cumulative per-request rusage ("command stats")
See merge request gitlab-org/gitaly!2368
|
|
[ci skip]
|
|
[ci skip]
|
|
[ci skip]
|
|
Documentation on tracking the state of a physical storage
See merge request gitlab-org/gitaly!2468
|
|
Adds documentation about how Praefect tracks state of physical
storages within virtual storages.
|
|
Feature flag distributed_reads enabled by default
Closes #2951
See merge request gitlab-org/gitaly!2470
|
|
[ci skip]
|
|
[ci skip]
|
|
[ci skip]
|
|
Refactor setup of transaction metrics
See merge request gitlab-org/gitaly!2474
|
|
|
|
Use repository generations to determine the best leader to elect
Closes #3008
See merge request gitlab-org/gitaly!2459
|
|
My first gitaly commit!
|
|
Right now, setup of metrics used in the coordinator is split across
multiple locations. This makes the process of adding new metrics more
involved than it needs to be and is a source of bugs in case any of
those locations is not updated.
Improve the situation by moving setup of metrics into the coordinator.
Metrics are exposed by implementing the Collector interface and
registering the coordinator itself as a metric.
|
|
Right now, setup of metrics used in the transaction manager is split
across multiple locations. This makes the process of adding new metrics
more involved than it needs to be and is a source of bugs in case any of
those locations is not updated.
Improve the situation by moving setup of metrics into the transaction
manager. Metrics are exposed by implementing the Collector interface and
registering the transaction manager itself as a metric.
|
|
We should take into account repositories that are not yet exist
on the storage, so we should treat them as completely out of date.
With this we won't have situation when storage will be chosen as
primary because it has only one repository that is up to date, but
missing all other repositories that exist on other storages.
Closes: https://gitlab.com/gitlab-org/gitaly/-/issues/3008
|
|
Gitaly team members are maintainers by default, and as such should be
listed in the documentation. Futher, the user name is added to a Ruby
array that Danger uses to automatically apply labels.
|
|
Prune objects older than 24h on garbage collection
Closes #2919
See merge request gitlab-org/gitaly!2469
|
|
Praefect: replication processing should omit unhealthy nodes
Closes #2922
See merge request gitlab-org/gitaly!2464
|
|
After enabling feature flag and verification of it on prod env
it shows expected results. The next step is to enable it by
default and allow a user to explicitly disable it in case of
some errors/performance problems.
Relates to: https://gitlab.com/gitlab-org/gitaly/-/issues/2944
|
|
By default the dangling objects remain in the object storage
for 2 weeks. This could be an issue if there are large files
or sensitive data being removed, but not fully deleted from
storage. The new configuration parameter `prune` for
`GarbageCollect` RPC results to execution of `gc` with additional
configuration option for `prune` command executed as part of
the garbage collection process. It removes all dangling objects
from the storage that are older then 24h.
Closes: https://gitlab.com/gitlab-org/gitaly/-/issues/2919
|
|
Fix post-receive hooks with reference transactions
See merge request gitlab-org/gitaly!2471
|
|
The Ruby hook logic already honors the primary-flag of transaction
information as it should, but the Go-implementation which is hidden
behind a feature flag doesn't. This would lead us to execute hooks on
secondaries if the Go implementation was enabled.
Fix this by conditionally executing the hook logic in Go code.
|
|
When using transactions and streaming commands to multiple nodes at
once, we need to avoid running Git hooks on all nodes taking part in the
transaction. To do so, the transactions contains a "primary" flag to
tell the given node that it is considerd the primary and should thus
execute the hook. While we already check for this in the post-receive
hook, we don't actually inject the transaction information. As a result,
each node won't find any transaction information, assume there is on
transaction and ultimately execute hooks even on nodes considered a
secondary.
Fix this by injecting tranasction information into the hook's
environment.
|
|
Fix accessors mislabeled as mutators
See merge request gitlab-org/gitaly!2466
|
|
Fix registration of Gitaly metrics dependent on config
See merge request gitlab-org/gitaly!2467
|
|
Log transaction state when cancelling them
See merge request gitlab-org/gitaly!2465
|
|
Fix transaction voting delay metric for pre-receive hook
See merge request gitlab-org/gitaly!2458
|
|
Update mime-types for Ruby 2.7
See merge request gitlab-org/gitaly!2456
|
|
Replication should consider the state of the target nodes and
omit dequeueing and processing of replication events for unhealthy
nodes. Processing of replication events for unhealthy nodes could
lead to outdated repositories as all attempts to process the
event would be failed and event will be marked 'dead'. And the
repository will remain outdated until the next replication event
happen to it. By checking the health state before processing
the event we have more chances to apply them without issues, so
reduce potential data loss.
Closes: https://gitlab.com/gitlab-org/gitaly/-/issues/2922
|
|
We've recently grown a metric in Gitaly which is dependent on the
configuration in order to set up its buckets. This metric is registered
as part of the global variables, but this is wrong as the global
configuration will not yet have been read at that point in time.
Ideally, we'd do the same as we do for Praefect, where we register and
inject metrics in Gitaly's main package. But given that this would
require quite some refactoring to do, this is deemed infeasible in the
context of this small change.
Let's instead fix this by lazily registering metrics via `sync.Once`.
|
|
In order to make a transaction's outcome more visible, add a new log
message that is written when cancelling a transaction. Next to the
actual notification that it's got wrapped up, the log message will also
include the number of total nodes registered as part of the transaction,
how many committed and the number of subtransactions created. This
improves visibility and makes it a lot easier to reason about what the
actual result of a transaction is.
|
