| Age | Commit message (Collapse) | Author |
|---|
|
We have finally been able to roll out Git v2.37.1.gl1 to production
without any issues observed so far. So let's default-enable this new Git
version so that background jobs will also start to use it.
Changelog: changed
|
|
Stop installing packed binaries
See merge request gitlab-org/gitaly!4673
|
|
refs: Return structured errors for DeleteRefs
See merge request gitlab-org/gitaly!4554
|
|
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
|
|
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.
|
|
[ci skip]
|
|
[ci skip]
|
|
proto: Fix definition of FindTagError
See merge request gitlab-org/gitaly!4770
|
|
git: Remove feature flag to hide refs in git-upload-pack(1)
Closes #4390
See merge request gitlab-org/gitaly!4768
|
|
The definition of the FindTagError we have just introduced in c6e1eb148
(proto: Introduce FindTagError to distinguish non-existent tags,
2022-07-29) was mistakenly missing the `oneof` definition. The error is
not yet used by anything, so let's correct it before we do grow any
callers.
|
|
objectpool: Fix conflicting references when fetching into pools
Closes #4373
See merge request gitlab-org/gitaly!4745
|
|
[ci skip]
|
|
With commit 4a789524c (git: Don't advertise internal references via
git-upload-pack(1), 2022-07-19) we have started to hide a subset of
internal references in git-upload-pack(1) so that these aren't
advertised at all anymore. When this change was rolled out to canary
initially we observed a short CPU burst and increase in writes on that
Gitaly node. This was totally unexpected given that the change was
supposed to only mirror what we have already configured in Omnibus
anyway, so it shouldn't have caused any visible change in behaviour. We
thus decided to retroactively introduce a feature flag for the change
via bc34b47ac (git: Introduce feature flag for hiding refs in
git-upload-pack(1), 2022-07-26).
As it turns out though, the observed behaviour was completely unrelated
to the change at hand, and we have subsequently rolled out the flag to
production without any observed weirdnesses as initially expected. So
let's remove the feature flag again.
|
|
go: Update module github.com/sirupsen/logrus to v1.9.0
See merge request gitlab-org/gitaly!4752
|
|
proto: Introduce FindTagError to distinguish non-existent tags
See merge request gitlab-org/gitaly!4767
|
|
Our tests that exercise pushing objects to SSHReceivePack are using the
helper funciton `sshPushCommand()` to construct a git-push(1) process.
As part of this setup we also inject various environment variables that
are required for the `gitaly-ssh` auxiliary binary so that it knows how
to connect back to the Gitaly server. The setup of those environment
variables is accidentally overriding the complete environment that was
returned by `gittest.NewCommand()` though, which also means that any
setup done by the Git command factory is lost.
Part of this setup also includes the `GIT_EXEC_PATH` environment
variable that tells Git where to find its binaries. This essentially
means that when running tests with bundled Git, we have by accident been
picking up the wrong Git executables. While this has worked just fine
until now, this breaks with the introduction of FIPS-enabled UBI images
which don't have Git installed in `/usr`, but in `/usr/local`. As a
consequence, we fail to find the correct Git executable and thus fail
the test.
Fix this bug by appending to the environment instead of overriding it
completely.
|
|
Signed-off-by: Balasankar "Balu" C <balasankar@gitlab.com>
|
|
Recently we've fixed cases where fetches which are cancelled by the user
will be logged as `Internal` errors and thus easily impact our SLOs.
This shortcoming has been fixed has been fixed via a2fee3dd5 (ssh:
Improve errors for fetches canceled by the user, 2022-07-20), but now
that we started to report errors in SSHReceivePack we are likely to hit
the same issue there.
Let's apply the same fix to SSHReceivePack and special-case this
specific error.
|
|
When git-receive-pack(1) returns an error, then we send that error code
to the client via an `SSHUploadPackResponse`. So while the client will
be notified of the actual error condition if they know to be on the
lookout for the error code, we will not make that error visible outside
of that particular exchange. This has two important consequences:
1. We are effectively blind to errors in SSHUploadPack given that
they also won't be visible in Prometheus. It's thus not possible
to put SLOs on pushes.
2. Praefect will not see any errors either and assume that the
request was indeed successful. When the request has failed under
the hood though it notices that there aren't any transactional
votes and thus it will schedule a replication job as a safety
guard.
This is in fact the same error as we have fixed in 9deaf47f1 (ssh: Log
error on git command failure, 2021-12-03), only that back then we fixed
it for SSHUploadPack.
Fix this by both sending the error code to the client and returning an
error from the RPC call is case git-receive-pack(1) fails.
Changelog: fixed
|
|
We're currently missing tests for SSHReceivePack which exercise error
conditions that may happen when the underlying communitcation between
git-send-pack(1) and git-receive-pack(1) fails. Add some tests to verify
our current behaviour.
Note that this also exposes an error where we're not reporting errors to
the gRPC client at all even though the upload has failed.
|
|
Simplify the `newSSHClient()` function so that it doesn't have to return
the gRPC connection by using `t.Cleanup()` to close it on test end.
|
|
Get rid of some trivial cases where we can get rid of seeded repos in
our SSHReceivePack tests.
|
|
Refacor tests for transactional voting in SSHReceivePack to be
hash-agnostic instead of assuming that we use SHA1 as an object hash.
|
|
While we verify the error code returned by SSHReceivePack in our tests
for argument validation, we don't verify that validation indeed failed
due to the reason we expect it to.
Refactor the tests to also verify the error message of returned errors.
This surfaces that in one of our tests for invalid arguments we failed
to build `gitaly-ssh` and thus tests have failed because of the missing
executable.
While at it, stop using a seeded repository: we're not hitting the repo
anyway.
|
|
Update test names for ReceivePack tests to match our modern best
practices.
|
