Age | Commit message (Collapse) | Author |
|
As drift_threshold_millis field is deprecated we use a newly
added drift_threshold field instead. To support backwards compatibility
the old drift_threshold_millis is taken into account if there
is no value provided for the drift_threshold field.
Closes: https://gitlab.com/gitlab-org/gitaly/-/issues/4412
|
|
The existing drift_threshold_millis field uses int64 type, but
instead it should be of type google.protobuf.Duration.
The new field drift_threshold of the proper type added to be
used instead.
Part of: https://gitlab.com/gitlab-org/gitaly/-/issues/4412
|
|
catfile: Support repositories with SHA256 object hashes
See merge request gitlab-org/gitaly!4779
|
|
server: Give clients grace-period for keepalives
Closes #4397
See merge request gitlab-org/gitaly!4772
|
|
localrepo: Don't run case-sensitive test on macOS
See merge request gitlab-org/gitaly!4773
|
|
ref: Fix `Internal` errors in `FindTag()` when tag doesn't exist
See merge request gitlab-org/gitaly!4771
|
|
Update nokogiri gem to v1.13.8
See merge request gitlab-org/gitaly!4776
|
|
macOS uses case-insensitive filesystems (HFS, APFS), so git
default-enables `core.ignorecase`. This causes attempts to create ref
'refs/heads/MASTER' to fail as 'refs/heads/master' already exists, and
makes `TestRepo_GetReferenceWithAmbiguousRefs()` fail on macOS.
Update this test to only execute the 'MASTER' case when not on macOS.
|
|
go: Update gRPC dependencies
See merge request gitlab-org/gitaly!4781
|
|
|
|
Using require.Equal does generally not work with grpc messages. We've
written the ProtoEqual testhelper, which we are now using instead.
|
|
Stop installing packed binaries
See merge request gitlab-org/gitaly!4673
|
|
Now that the catfile package supports SHA256 object hashes we can remove
the build tags that keep the tests from running in SHA256-mode.
|
|
Refactor the catfile tests to be hash-agnostic so that they behave well
both with SHA1 and SHA256 repositories.
|
|
Refactor some of the catfile tests to match modern best practices.
|
|
The catfile package needs to parse object hashes in multiple locations
and thus needs to know about the object hash format used by a specific
repository. Detect the object hash at runtime to support both SHA1 and
SHA256 repositories.
|
|
The `ParseObjectInfo()` function needs to parse the object hash of the
input and thus requires knowledge about the object hash format it ought
to use. Adapt its interface to accept a `git.ObjectHash` as input to
shift the burden of detecting the format to the caller.
|
|
Convert the test setups for the catfile package to create an empty
repository and have tests write their required test data at runtime.
This prepares for testing with the SHA256 object hash.
|
|
With an earlier version of this patch series to start detecting the
object hash format used by a repository some tests performing validation
of requests with invalid repositories have started to fail because we
now wrapped the resulting error. As a consequence, error codes have
changed for those tests to `Internal`, but there wasn't really any
indicator why that was happening.
Refactor these tests that were failing to properly verify the complete
gRPC error returned by the RPCs. Like this, we have much stronger
guarantees about why an RPC call is failing and have more information in
case the test does indeed start to fail in an unexpected way. While not
required anymore for this patch series, this refactoring is still worth
it on its own to improve the test's quality.
|
|
One of our tests for the `GetArchive()` RPC uses an intercepting command
factory to assert that git-archive(1) in fact sees expected environment
variables. This is done by unconditionally intercepting any Git command
spawned and replacing it with a call to env(1), which only works right
now because we only ever spawn any other command in this code path at
the moment. This is about to change though with the introduction of the
object hash format detection.
Refactor the test to only intercept calls to git-archive(1) in order for
the test not to break when we start spawning other Git commands.
|
|
refs: Return structured errors for DeleteRefs
See merge request gitlab-org/gitaly!4554
|
|
This is done to match the changes in
https://gitlab.com/gitlab-org/gitlab/-/merge_requests/93412 and
to ensure we don't duplicate versions of this gem.
Changes: https://nokogiri.org/CHANGELOG.html
Changelog: changed
|
|
In the cases where we get an error trying to update the ref, return a
structured error with the git error embedded in it so the rails side can
interpret the error and raise a GitError.
Changelog: changed
|
|
A common error is when a reference is already locked. Detect this by
parsing the error string and emit a typed error so callers don't need to
parse the error string themselves.
|
|
localrepo: Support testing with SHA256
Closes #4354
See merge request gitlab-org/gitaly!4769
|
|
Currently gRPC clients are configured to send keepalives every 20
seconds, and the servers enforce that as a minimum interval. Requiring
precisely the same interval has caused a number of problems over time.
gGRPC-Core has an issue where keepalives may be sent a few milliseconds
ahead of schedule[0], breaking keepalives from Puma and Sidekiq.
With the introduction of `SSHUploadPackWithSidechannel` in v15.0, the
gRPC connection for Gitlab-Shell and Gitlab-Workhorse is now idle during
lengthy clones, triggering keepalives in this scenario. Previously the
connection was always busy and keepalives were rare.
After upgrading to v15.0 a large customer reported that they have
started to receive intermittent ENHANCE_YOUR_CALM errors on SSH clones,
which indicates that keepalives are being sent to rapidly from the
client. Examining packet captures from their hosts, we found that
Gitlab-Shell was sending keepalive packets roughly 20,001ms apart,
which is correct. However, the Gitaly server was intermittently
receiving these at an interval of 19,998ms, triggering the error. This
appears to be due to the variable connection latency, where typically
there's 50ms of latency between the hosts, but if there's slightly less
latency then the norm the keepalive arrives early from the server's
perspective.
To resolve this, let's give the clients a grace-period of ten seconds by
reducing the minimum keepalive interval to 10 seconds. This way a
keepalive that arrives slightly ahead of schedule is not a critical
error. Given that a Gitaly server will typically have hundreds of
concurrent clients, perhaps thousands on a very busy host, the volume of
keepalives should not add significant load and we can afford to be
forgiving.
[0] https://github.com/grpc/grpc/issues/28830
Changelog: fixed
|
|
The logic of gitSupportsStatusFlushing is useful beyond the updateref
package. Extract this to its own method in the gittest package.
|
|
We can extract which reference caused an error. Add Ref field to
ReferencesLockedError so we can pass this information back to the
caller.
|
|
Enable testing of most parts of our localrepo tests with SHA256 as
object hash. There are two notable exclusions:
- The remote extra tests depend on the `CreateRepository()` RPC that
doesn't yet support creating SHA256 repositories.
- The `ReadCommit()` function depends on the `catfile` package,
which doesn't yet support parsing SHA256 commits.
We thus skip testing those two parts of the package.
|
|
Over time many of the places in our production-facing code will require
to detect object hashes, often even multiple times per RPC call. Because
we need to spawn a Git command to detect the object hash this cost may
add up over time, so ideally we'd run this detection logic infrequently
only.
We implement a new `ObjectHash()` function for `RepositoryExecutor`s
that allows us to at cache the detected object hash on the `localrepo`
level. Assuming that `localrepo`s get constructed once only per RPC call
this at least allows us to limit the frequency to detect the object hash
to once per RPC call.
While we might eventually want to use an LRU cache to do cache recent
detections, this feels quite a lot like premature optimization. We
should first see how things behave and whether this is going to put some
strain on our command factory before we grow more complexity.
|
|
operations: Introduce structured errors for UserCreateTag
Closes #4137
See merge request gitlab-org/gitaly!4743
|
|
Replace hardcoded uses of SHA1 object hashes in our tests to instead use
the default object hash as indicated by the gittest package.
|
|
Similar as the preceding commit this converts remaining tests that use
seeded repositories to stop doing so and instead write their required
test data at runtime.
|
|
The `setupRepo()` helper function used in the localrepo tests is using a
seeded repository. Refactor these tests to instead use an empty repo and
create the test data at runtime so that we can more readily support
SHA256 in those tests.
|
|
We have already disabled the use of CreateRepository with seeded
repositories in 9ef8ff076 (gittest: Disallow use of seeded repositories
with SHA256, 2022-07-21) when SHA256 is in use. This isn't broad enough
though given that this function uses the `CreateRepository()` RPC behind
the scenes which doesn't yet support SHA256 at all, so it doesn't even
work to use this function with unseeded repositories.
Let's add another test failure that is specific to the unseeded-repo
case to avoid accidental use of this function in SHA256 tests.
|
|
The shared repository suite that is used to verify both the localrepo
and remoterepo implementations currently uses a toggle to tell the
implementation-specific setup function whether the repository should be
seeded or not. This doesn't make a whole lot of sense: setup of the test
data really should be handled by the suite itself.
Refactor the code and create the test data at runtime. This also enables
us to easily test against SHA256.
|
|
Lots of our tests verify `GitCommit`s of commits that have been written
via the `WriteCommit()` test helper and thus need to compare whether the
`CommitAuthor` matches the default commit author. This is quite verbose
and duplicates information across lots of our tests.
Provide a global structure as part of the `gittest` package that holds
this information to reduce the duplication. Note that this requires us
to change some locations to use `testhelper.ProtoEqual()` instead of the
previous `require.Equal()` because Protobuf messages cannot necessarily
be directly compared with each other.
|
|
Many of our tests will require the ability to tell how long the current
object hash is supposed to be in its hex-encoded length. Add a new
function `EncodedLen()` that returns this information.
|
|
Use custom image for FIPS jobs
See merge request gitlab-org/gitaly!4687
|
|
Return context error from requestFinalizer
Closes #4391
See merge request gitlab-org/gitaly!4755
|
|
|
|
With the auxiliary binaries packed into the gitaly binary and deployed
as a single unit, we no longer need to maintain backwards compatibility
between them and Gitaly. This commit drops the now unnecessary major
version suffix from the gitaly-git2go binary which was needed due to the
major version upgrades causing the type paths to change and thus breaking
the Gob protocol between the binaries.
|
|
Makefile contains a target for building a gitaly-git2go-v14 executable.
This is an older version of the gitaly-git2go-v15 so the old executable
stays around for backwards compatibility reasons throughout a major
version upgrade. Since we are now deploying the binaries as a single unit
packed inside the main gitaly binary, we no longer need to maintain backwards
compatibility. Remove the unnecessary target.
|
|
Since Gitaly now unpacks the auxiliary binaries from itself into a
temporary directory and runs them from there, we can stop installing
the packed binaries.
|
|
Makefile: Check if $GOCACHE exists
See merge request gitlab-org/gitaly!4774
|
|
cli: Relocated check subcommand to Gitaly binary
Closes #4320
See merge request gitlab-org/gitaly!4763
|
|
With 6641d296(Clean up go build cache if it exceeds a threshold,
2022-07-28) we now check if the build cache is larger than 4.7 GiB with
`du -sk`.
However, if `make clean` has just been executed then $GOCACHE does
not exist and `make` will emit error:
du: /Users/wchandler/devel/gdk/gitaly/_build/cache: No such file or directory
bash: line 1: [: -gt: unary operator expected
To avoid this, check if $GOCACHE is a directory before running `du`.
|
|
ssh: Fix silent errors when SSHReceivePack fails
Closes #4136
See merge request gitlab-org/gitaly!4766
|
|
The `check` subcommand has been relocated from `gitaly-hooks` to the
main `gitaly` binary. References to the subcommand were updated to
reflect this change.
|
|
The gitaly-hooks check subcommand is used by admins to check whether
the internal API of Rails is accessible. Since in a running system
gitaly-hooks itself does not need to call the internal API this check
makes more sense to reside in the Gitaly binary and has been moved
there accordingly. This will validate whether or not Gitaly can
actually call the internal API.
|