| Age | Commit message (Collapse) | Author |
|---|
|
|
|
Convert CI to use matrix builds
See merge request gitlab-org/gitaly!2496
|
|
|
|
Fix Git hooks when GitLab relative URL path and UNIX socket in use
See merge request gitlab-org/gitaly!2485
|
|
Fix hanging info/refs cache when error occurs
Closes #3087
See merge request gitlab-org/gitaly!2497
|
|
This resolves the issue where the info ref RPC hangs when a failure
occurs while persisting a response stream. The fix discards any
remaining bytes in the tee reader used by the cache when the cache
operation cannot proceed. This unblocks the tee reader so that the
normal non-cache mechanism can proceed to serving the request.
|
|
Introduces a failing test for when the info/ref cache encounters an
error while attempting to persist a response stream. This validates the
problem described in https://gitlab.com/gitlab-org/gitaly/-/issues/3087.
The failing test accomplishes overrides the info ref cache with a mock
implementation that always returns an error. The test asserts that a
normal response is received regardless of the error, and also asserts
that the mocked code was actually called.
|
|
Fix sparse checkout file list for rebase onto remote branch
Closes #2759
See merge request gitlab-org/gitaly!2447
|
|
|
|
Reenable Git2Go integration
See merge request gitlab-org/gitaly!2438
|
|
Implement majority-wins transaction voting strategy
See merge request gitlab-org/gitaly!2476
|
|
Daily maintenance scheduler
See merge request gitlab-org/gitaly!2423
|
|
This moves the integration of the walker and scheduler to the server
factory called by the bootstrap process. This is higher up in the call
stack than the original location.
This also changes the walker to use the client to call the repository
service via internal socket.
|
|
Fix potentially executing linguist with wrong version of bundle
Closes #3085
See merge request gitlab-org/gitaly!2495
|
|
Don't expand filesystem paths of wiki pages
Closes gitlab#207
See merge request gitlab-org/security/gitaly!12
|
|
[ci skip]
|
|
In order to make test results available to the CI, we generate JUnit
test reports. The generated files currently carry the CI_JOB_NAME
variable in their name, mostly to disambiguate them when downloading
multiple reports and extracting them. With migrating to matrix-based CI
jobs, the naming pattern has changed though to be e.g. "test 2/8". While
not being too helpful, it also carries a space and a slash character. As
a result, the report name is not referencing a file anymore, but instead
a directory hierarchy, causing the Makefile to fail.
Fix the issue by explicitly using the Go and Git versions as part of the
generated report name instead.
|
|
Our CI setup uses an implicit matrix to build, assemble and test our
code against different Go, Git and Ruby versions. In order to not repeat
ourselves, we thus used YAML anchors to merge in a common definition,
which already avoids quite some maintenance burden. But the end result
is still a tad hard to read as it's easy to miss the differences between
jobs.
Let's migrate to parallel matrix jobs instead. This new feature which
has been introducen in GitLab v13.3 allow us to do exactly what we want:
given a matrix of variables, run the job with each combination. This
allows us to get rid of some of the YAML anchors and explicitly states
what we're iterating over, improving maintainability.
Note that while the GIT_VERSION variable isn't yet used by anything, it
will get used as soon as we start using a custom-built Git version in
our CI pipelines. But as matrix builds require at least two variables,
they're added as part of this commit already.
|
|
We currently have two ways we set up transactions in the coordinator:
- With the primary-wins feature flag, the primary will always
succeed no matter what the secondaries vote.
- Without the flag, all nodes need to agree.
The first strategy really is only to ensure smoother transitioning
towards using reference transactions and is going away at some point in
time. The second voting strategy is the one that's going to stay, but
right now it's really pessimistic because it will always require all
nodes to agree on the same thing.
Let's refine this strategy and move it to the next level, which is a
majority-wins approach: instead of requiring all nodes to agree, we will
now require a majority of nodes to agree. In this case, we define
majority to:
- Always include the primary node. This is a hard requirement, as
the primary node is the one who's executing hooks. If it failed
and the transaction succeeded, we'd have changes persisted to disk
but none of the hooks were run, which may lead to inconsistent
state.
- Require at least half of the secondaries to agree, rounded up. As
a special case, if there's only a single secondary, then it's not
required to agree with the primary.
This would result in the following typical scenarios:
- 1 primary, 0 secondaries: Primary always wins.
- 1 primary, 1 secondary: Primary always wins.
- 1 primary, 2 secondaries: Primary and a single secondary required.
- 1 primary, 3 secondaries: Primary and two secondaries required.
- 1 primary, 4 secondaries: Primary and two secondaries required.
This commit implements the above by changing the default transaction
setup in case the primary-wins feature flag is disabled and adds/adjusts
some of our tests to match this new strategy.
|
|
Since 489e4eac (linguist: Use configured Git executable, 2020-08-19), we
modify git-linguist's PATH environment variable in order to have it use
the correct Git executable. This is mostly done as git-linguist doesn't
allow us to pick a Git executable, so we need prepend the real Git
executable's directory to PATH.
This hack has caused a regression, though: in case the Git directory
also contains bundle(1), then we now potentially use the wrong bundle
version and thus also the wrong version of Ruby. As a result, execution
will fail.
Fix the issue by resolving bundle to an absolute path and using that one
instead.
|
|
While the `ruby_definition` snippet was initially used to cache Ruby
Gems, only, it's meanwhile been extended to be a general cache for build
artifacts.
Rename it to `cache_definition` in order to avoid any confusion.
|
|
With commit a6091637 (Merge branch 'pks-revert-git2go' into 'master',
2020-07-31), our new Git2Go dependency was reverted again due to several
issues:
- Some distributions didn't have a recent enough version of
CMake available. As a result, building libgit2 failed. This was
fixed by bumping the build images' CMake version.
- Build tags didn't work as expected and thus we ended up not using
them at all. This was fixed by passing build flags via the
Makefile. As this causes e.g. `go test ./...` to fail with a
linking error due to build tags not being specified and the
PKG_CONFIG_PATH not being set up as required. To work around this
issue and not break developer workflows, gitaly-git2go will now
only be built if build tags required for git2go are set.
- Invalidation of libgit2.a didn't work as required. E.g. if the
Makefile changes, it wasn't rebuilt and it was thus impossible to
provide fixes for any broken setups if libgit2.a was built
already. This was fixed by adding a build dependency on the
Makefile.
All in all, this should fix all currently known problems with building
libgit2.
|
|
Rebuild targets only if Makefile content changes
See merge request gitlab-org/gitaly!2492
|
|
Use cache keyed by build instructions
See merge request gitlab-org/gitaly!2493
|
|
Document transaction-related terminology
See merge request gitlab-org/gitaly!2477
|
|
There's quite a lof of different terms which we've started using in the
context of transactions, where some may not be as obvious as others. In
order to stop any confusion, this commit adds them to our documentation.
|
|
|
|
Previously if GitLab were configured to use a relative URL
(e.g. `/gitlab`) and the Gitaly `gitlab.url` configuration used the
http+unix:// scheme, the hooks would not be able to contact the API
server. We add an explicit `relative_url_root` parameter to make it
possible for all connections to go through Workhorse.
This commit depends on changes in
https://gitlab.com/gitlab-org/gitlab-shell/-/merge_requests/406.
NOTE: This only fixes the Git hooks that are implemented in Go. The Ruby
gitlab-shell hooks don't appear to have ever work with GitLab
installations using a relative URL when a UNIX domain socket is
used. Omnibus installations bypass Workhorse and talk directly to the
Web server.
Relates to https://gitlab.com/gitlab-org/gitlab-shell/-/issues/476
|
|
[ci skip]
|
|
CentOS doesn't ship the `shasum` binary, but instead only ships
`sha256sum` and family. Let's use them instead to unblock building on
those distros.
|
|
As changes to our Makefile may require a rebuild of dependencies, e.g.
because of a version change, those targets need to depend on the
Makefile itself. Naturally, this is a heuristic that's going to be wrong
often, but it's better than using stale artifacts for our build.
Unfortunately, this breaks the caching mechanism used in CI as
timestamps of files won't be preserved across differnt jobs. As a
result, the build cache is going to be invalidated all the time.
Let's improve the situation by not depending on filestamps anymore, but
instead on the Makefile's contents. It's quite easy to achieve by
depending on a generated Makefile.sha256 file instead of the real
Makefile. This file is being generated by a rule that's always
executing, but the target will only get updated in case the hash of the
source file actually changed. Like this, only real content changes will
cause the timestamp of that file to get updated.
In order to make use of this in our CI, this commit also puts the
generated Makefile.sha256 into our cache.
|
|
As changes to our Makefile may require a rebuild of dependencies, e.g.
because of a version change, those targets need to depend on the
Makefile itself. Naturally, this is a heuristic that's going to be wrong
often, but it's better than using stale artifacts for our build.
Unfortunately, this breaks the caching mechanism used in CI as
timestamps of files won't be preserved across differnt jobs. As a
result, the build cache is going to be invalidated all the time.
Let's improve the situation by not depending on filestamps anymore, but
instead on the Makefile's contents. It's quite easy to achieve by
depending on a generated Makefile.sha256 file instead of the real
Makefile. This file is being generated by a rule that's always
executing, but the target will only get updated in case the hash of the
source file actually changed. Like this, only real content changes will
cause the timestamp of that file to get updated.
In order to make use of this in our CI, this commit also puts the
generated Makefile.sha256 into our cache.
|
|
In order to support testing against our own version of Git and become
independent of provided container's Git version, we'll modify CI jobs to
use the generic "git" target which is currently in-flight, calling it
with GIT_VERSION set to the specific version we want to test against.
As we currently cannot yet use pre-built Git versions as they're built
against a different set of libraries, we'll instead have to build Git
ourselves every time, introducing overhead.
Let's prepare for this by improving our caching strategy. While we're
currently only caching Ruby Gems, we'll want to extend the cache to the
self-built version of Git and eventually also to the self-built version
of libgit2. This requires us to bust the cache whenever their respective
build instructions change. Luckily, GitLab CI allows us to key the cache
by files, which is in our case both the Makefile and also the Ruby
Gemfile.
|
|
Update rbtrace gem to v0.4.14
See merge request gitlab-org/gitaly!2491
|
|
Relabel smarthttp.InfoRefsReceivePack as accessor
Closes #3007
See merge request gitlab-org/gitaly!2487
|
|
This fixes a compile issue with Clang v12:
https://github.com/tmm1/rbtrace/pull/82
Part of
https://gitlab.com/gitlab-org/gitlab-development-kit/-/issues/1034
|
|
Move fake git path to test target
Closes gitlab#239103
See merge request gitlab-org/gitaly!2490
|
|
This moves a fake git test, needed for testing, to the test target of
the Makefile. This mitigates potential issues where the fake git
script is used instead of the real git executable during various
Makefile tasks.
|
|
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.
|
|
The `smarthttp.InfoRefsReceivePack` RPC is currently labelled as
mutator. Taking a closer look at it, there is no way it can change any
objects in the repo though, as it will always either spawn `git
receive-pack --stateless-rpc --advertise-refs` or `git upload-pack
--stateless-rpc --advertise-refs`. While git-receive-pack(1) may modify
the repo, it won't ever do so with the `--advertise-refs` flag, as it
will cause the command to only advertise references and then exit
immediately afterwards.
As the RPC is called in quick succession with `smarthttp.PostUploadPack`
in case a user performs a push, the first call to InfoRefsReceivePack
would've increased the repository generation and caused replication jobs
to be created. As a result, when PostUploadPack gets invoked it is
likely that secondaries will be treated as out-of-date as replication
jobs usually weren't processed yet. This in turn breaks transactions, as
we will now only add the primary to any transaction for all pushes via
smarthttp.
So let's relabel the RPC as an accessor to avoid replication when it's
called and fix the described issue.
|
|
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.
|
