| Age | Commit message (Collapse) | Author |
|---|
|
We have observed a racy test in production where fetching a hidden ref
via `PostUploadPack` isn't failing in the way we expect it to. While the
error code is there and the RPC call itself has failed, the response
buffer we see is completely empty.
This race can easily be caused by our testing infrastructure though: we
use `makePostUploadPackWithSidechannelRequest()` to set up a sidechannel
and then copy the sidechannel data via a separate Goroutine. In case we
see an error though while handling the sidechannel (which is the case:
we expect to fail fetching the hidden ref), we don't synchronize with
the Goroutine copying the response.
Fix this race by properly synchronizing the Goroutine via a wait group.
|
|
One of our tests is verifying that the `GitConfigOptions` which can be
passed to us via the `PostUploadPackRequest` is applied as expected.
This is done by creating a reference and then injecting config which
tells git-upload-pack(1) to hide this reference. If this config is
applied as expected, then we should see that the server refuses to give
us the object pointed to by the hidden reference.
Refactor this test to be less reliant on the repository state, which is
kind of confusing. The object pointed to by the hidden reference should
in fact already be pointed to by another visible reference, which is
quite puzzling. Furthermore, we also hard-code how many objects we
expect to receive, which is one more thing that depends on this specific
repository.
Convert the test to use an empty repository instead and create two
commits ad-hoc.
|
|
sidechannel: use default yamux max window size
Closes #4132
See merge request gitlab-org/gitaly!4439
|
|
sidechannel: Convert to use runtime directory to store sockets
See merge request gitlab-org/gitaly!4428
|
|
ci: Convert jobs to test with bundled Git only by default
See merge request gitlab-org/gitaly!4391
|
|
This changes the sidechannel client to not create unnecessarily large
buffers for received data. The previous behavior was to lazily buffer
up to 16MB of response data, which can lead to bloat in the client.
Changelog: other
|
|
git: Enable bundled Git v2.35.1.gl1 by default
See merge request gitlab-org/gitaly!4437
|
|
Our non-bundled test targets currently depend on bundled Git. This was
done such that we were able to verify the bundled Git feature flag,
which toggled between both bundled Git and distributed Git. Now that
this feature flag is gone it doesn't make any sense to keep testing with
both Git execution environments installed.
|
|
With the conversion of our CI jobs to use bundled Git by default we're
not testing the minimum required Git version anymore. Add a new job
which tests that we're still compatible with this version to assert that
we're not starting to use new Git features which aren't available in
this version.
|
|
We only have a single CI job right now which tests bundled Git. This Git
execution environment has become our default though, and consequentially
we should also have most of our CI jobs use it.
Convert jobs to use bundled Git by default.
|
|
Whenever the CI configuration changes chances are quite high that the
caches will become stale. Let's thus invalidate them in case the CI
configuration changes.
|
|
The test-boot script doesn't currently support bundled Git, so it will
only be able to bootstrap a server when a Git distribution is available.
Adapt the script to also support bundled Git so that we can use it in
contexts where it is the only Git execution environment available.
|
|
With b547b368c (git: Support bundled Git v2.35.1.gl1, 2022-02-22)
we have introduced support fur bundled Git v2.35.1.gl1. The flag was
subsequently rolled out on 2022-03-23. While there had been initial
issues with repositories which had preexisting corrupted references,
which were uncovered by this upgrade, the rollout went along smoothly
otherwise.
Let's enable this new Git version by default. Note that this will cause
us to also use the new Git version in background jobs, which aren't
covered right now because we don't support feature flags in there.
Changelog: changed
|
|
When setting up a push for our tests, we also have the ability to inject
feature flags by setting the `featureFlags` field. This has a major
downside though: the feature flags injected via the context and the
feature flags executed at a later point when actually running the
command may easily drift apart. This inconsistency is something that may
break assumptions on our side though, where some parts may now evaluate
a flag to `true` while others evaluate it to `false`.
Fix this inconsistency by instead accepting feature flags via a context.
This both simplifies the code and fixes such an upcoming incompatibility
when we're about to default-enable Git v2.35.1.gl1.
|
|
Our SSH tests need to set up a repository with some new objects so that
it can verify that these objects end up on the remote side after the
push. To create those objects, we currently use a worktree and then
execute git-commit(1). This usage pattern is outdated though: we have
a `gittest.WriteCommit()` helper which does not require a worktree.
Refactor tests to use this new helper so that we can get rid of using a
worktree.
|
|
git: Remove feature flag for bundled Git
Closes #4064
See merge request gitlab-org/gitaly!4429
|
|
[ci skip]
|
|
Makefile: Spring cleanup
See merge request gitlab-org/gitaly!4409
|
|
cgroups: Allow stats to be collected in absence of memsw.* entries
See merge request gitlab-org/gitaly!4431
|
|
The `coverage` Makefile target used to not fail in the past. Nowadays it
does though because it executes `run_go_tests` on its own.
|
|
Fix name of the `GOTESTSUM_VERSION` variable, which was accidentally
called `GOSUMTEST_VERSION`.
|
|
Remove the unused `smoke-test` target. It's used in none of our
projects, and it only tests things already covered by other targets.
|
|
The PgBouncer CI job is currently only executing the `test-postgres`
target, which only exercises a subset of our test suite. It's much more
sensible thoguh to instead execute `test-with-praefect` so that we can
assert that a "real" Praefect proxy works alright in combination with
PgBouncer.
Do this conversion and remove the now-unused `test-postgres` target.
Convert the job to run tests as unprivileged user so that we don't
ignore permissions, or otherwise some of our tests would break.
|
|
Move together Protobuf-related targets.
|
|
Simplify the `no-proto-target` to not write a temporary file, but
instead use the `--exit-code` flag of git-diff(1). Like this, any
changes that exist will be printed to stdout directly.
|
|
Upgrade go-licenses to v1.0.0. Most importantly, this release contains a
fix to the way project sources are copied into the target directory so
that we don't end up with files and directories which cannot be modified
[1]. The workaround we have in our codebase which fixes the permissions
can be removed as a consequence.
Note that this causes us to pick up an additional file that's part of
our own sources as licensed file because it's a test verifying that
licenses can be parsed alright, and thus it contains a license body
itself. It's known though that go-licenses behaves weird already though:
it copies the complete sourcecode of projects into the target directory
in case it detects that the license requires it. So adding one more
weird edge case to our NOTICE file doesn't really matter. Together with
the fact that we can remove the workaround we previously had this seems
like an acceptable tradeoff.
[1]: https://github.com/google/go-licenses/issues/11
|
|
As part of the `format` Makefile target we also run `goimports` to
automatically fix up missing or superfluous imports. Because of the way
`goimports` adds these imports, the end result may not conform to the
code style as dictated by `gofumpt`. To fix any formatting differences,
we call `goimports` twice: once before, and once after `gofumpt`.
This doesn't really make a lot of sense though: executing it once before
the formatter runs should be sufficient. For it to not be sufficient the
formatter would have to remove imports again, and this is something I'd
claim doesn't happen.
Remove the second call to `goimports` to speed up the `format` target.
|
|
Upgrade gofumpt to v0.3.1. Besides a very small number of new rules, the
most important benefit is that the new version parallelizes formatting
of files. It is thus much faster, also for our codebase:
$ hyperfine --warmup=1 'make format' 'make format GOFUMPT_VERSION=0.3.0'
Benchmark #1: make format
Time (mean ± σ): 1.238 s ± 0.060 s [User: 1.337 s, System: 0.084 s]
Range (min … max): 1.125 s … 1.312 s 10 runs
Benchmark #2: make format GOFUMPT_VERSION=0.3.0
Time (mean ± σ): 239.5 ms ± 26.0 ms [User: 1.804 s, System: 0.109 s]
Range (min … max): 180.6 ms … 284.7 ms 14 runs
Summary
'make format GOFUMPT_VERSION=0.3.0' ran
5.17 ± 0.61 times faster than 'make format'
Reformat our code with the new version.
|
|
Remove an unused golangci-lint linting exemption.
|
|
There's several locations where we don't verify that the `Kill()`
syscall succeeded. Move the linting exceptions we have in our
golangci-lint configuration inline to clearly indicate that this is
something we may want to fix in the future.
|
|
Both the local and SQL electors don't check errors returned by
`checkNodes()`. While we could convert all sites to properly do this, it
doesn't really feel worth it: both electors only exist due to legacy
reasons and should not be used in production systems at all.
Ignore most of those errors inline so that we can remove the excemptions
from golangci-lint's config file.
|
|
We currently ignore any errors when waiting for the supervised command
fails. Log the error so that we at least have a chance to see that
something may have gone wrong.
|
|
Update golangci-lint to v1.44.2, which currently is the latest available
release.
|
|
Otherwise, we get errors such as
`open /sys/fs/cgroup/memory/gitaly/gitaly-1/memory.memsw.usage_in_bytes:
no such file or directory`.
Changelog: fixed
|
|
GetArchive: log request hash
See merge request gitlab-org/gitaly!4425
|
|
command: Log cgroup path
See merge request gitlab-org/gitaly!4420
|
|
repository: Structured errors for UserRebaseConfirmable
See merge request gitlab-org/gitaly!4419
|
|
We'd like to be able to tell if a command was run in a cgroup based on
the log entry. This change adds a field in the Command struct so
whenever the Cmd finishes, a cgroup field gets logged.
Changelog: added
|
|
[ci skip]
|
|
The sidechannel code is used to create sidechannels that circumvent gRPC
for efficiency's sake. This socket is currently created in the system's
temporary directory as returned by `os.MkdirTemp()`. This has the very
real downside that it easily leaks created files and directories in case
we have a logic bug anywhere.
We have recently introduced a new runtime directory that can help us in
this situation: the runtime directory is created once at a central place
and will be cleaned up when shutting down Gitaly. Consequentially, even
if we leaked and sidechannel files, we'd still remove them on shutdown.
And even if we didn't: the runtime directory is designed so that we can
check whether it's used because it has the current process's PID as part
of its component. So if a runtime directory exists whose PID doesn't
refer to any existing process it's safe to remove. While we don't have
any such logic yet, it can easily be added at a later point and have all
code which started to use the runtime directory benefit at the same
time.
Migrate the sidechannel code to create sockets in a subdirectory within
the runtime directory called "sidechannel.d" if the runtime directory is
set via the hooks payload.
Changelog: changed
|
|
Since 073727abd (git: Add a feature flag to toggle between bundled and
external Git, 2022-01-13) we have now carried a feature flag that
allowed us to ease into the deployment of bundled Git in production.
With this new Git execution environment, Gitaly doesn't need a full
Git distribution anymore but can instead have multiple sets of bundled
Git binaries installed into its own binary directory from which it can
bootstrap a complete Git environment. This allows us to use feature
flagged rollouts of new Git versions in production and fixes issues with
zero-downtime upgrades.
Bundled Git has been rolled out for a full month by now and is default
enabled since 52625870d (git: Enable use of bundled Git by default,
2022-02-28). Let's consider it stable and remove the feature flag
altogether. In the worst case, users can still disable bundled Git via
the Gitaly configuration anyway.
Changelog: changed
|
|
One of our tests for bundled Git verifies that the command factory is
setting up the execution environment as expected. This test implicitly
relies on the state of feature flags though because it assumes that all
bundled Git constructors are usable. This worked alright by pure chance:
the feature flag for this bundled Git constructor is always true because
we use `math.Rand()` to set the flag's value, which isn't ever properly
seeded.
Fix this issue by hard-coding the feature flag value to `true` so that
the test doesn't break when the randomly injected value changes.
|
|
Currently, UserRebaseConfirmable will return gRPC OK even when there is
an error during rebase or if there is an error during PreReceive when it
calls the GitLab API for an access check.
This change modifies the error handling to return a detailed gRPC
structured error in either of these failure modes.
Changelog: changed
|
|
gitaly/config: Introduce runtime directory configuration
Closes #4113
See merge request gitlab-org/gitaly!4415
|
|
housekeeping: Skip prune if there are no old objects
Closes #4121
See merge request gitlab-org/gitaly!4423
|
|
The `OptimizeRepository()` RPC heuristically executes git-prune(1). The
heuristic we currently use counts the number of loose objects which
exist after having repacked the repository, and, if they exceed a given
threshold, we'll try to remove them. This also causes us to try and
prune objects though in case there are unreachable, but recent objects.
But because unreachable objects don't get packed, and because we only
prune objects older than two weeks, this will in many cases cause us to
execute git-prune(1) without any need.
Improve the heuristic to only prune objects in case there are loose
objects which have an mtime older than two weeks. This should in the
general case avoid useless prunes, but still detect the case where there
is work to be done.
Changelog: performance
|
|
operations: Always use structured errors for UserSquash
Closes #4099
See merge request gitlab-org/gitaly!4424
|
|
This will allow us to determine in the future if it is worth adding a
cache for GetArchive.
|
|
Migrate the temporary Git exec path set up by the Git command factory to
be created in the runtime directory.
Changelog: changed
|
|
Migrate the temporary hook directories set up by the Git command factory
to be created in the runtime directory.
Changelog: changed
|
