| Age | Commit message (Collapse) | Author |
|---|
|
Signed-off-by: Rémy Coutable <remy@rymai.me>
|
|
This rule was introduced in
https://gitlab.com/gitlab-org/ruby/gems/gitlab-dangerfiles/-/releases/v2.1.0
Signed-off-by: Rémy Coutable <remy@rymai.me>
|
|
Signed-off-by: Rémy Coutable <remy@rymai.me>
|
|
The rule was introduced in
https://gitlab.com/gitlab-org/ruby/gems/gitlab-dangerfiles/-/releases/v2.9.0
Signed-off-by: Rémy Coutable <remy@rymai.me>
|
|
Use features introduced in
https://gitlab.com/gitlab-org/ruby/gems/gitlab-dangerfiles/-/releases/v2.6.0
Signed-off-by: Rémy Coutable <remy@rymai.me>
|
|
Signed-off-by: Rémy Coutable <remy@rymai.me>
|
|
housekeeping: Don't prune recent objects
See merge request gitlab-org/gitaly!4410
|
|
When running git-gc(1), Git automatically executes git-prune(1) with a
grace period of two weeks: only when objects are older than that grace
period will they be pruned. This is really important to fix a race
condition with concurrent commands, which all will first write new
objects into the repository before making them reachable via a ref
update. This means that there is a brief period between writing the new
objects and making them reachable. If objects would be pruned without a
grace period, it could kick in right before updating the references and
thus cause repository corruption. The two-weeks grace period bridges
this gap by giving enough head room to finish the transaction.
With the recent introduction of the heuristics-based OptimizeRepository
RPC we have also started to use git-prune(1). The assumption was that
the two-weeks grace period was the default of git-prune(1), but as it
turns out it is only the default of git-gc(1). So because we are now
running git-prune(1) without any parameters, we accidentally started to
delete all unreachable objects, even if they had just been created.
Curiously, this bug only started to surface as we were migrating Rails
to use OptimizeRepository. Seemingly, because we previously only were
executing the RPC as part of our nightly job, the logic didn't trigger
in the right moment and thus never caused any (known) problems. Still,
this is a serious bug.
Fix this issue by passing `--expire=two.weeks.ago` to git-prune(1).
Changelog: fixed
|
|
repository: Fix indeterministic voting when creating new repos
Closes #4100
See merge request gitlab-org/gitaly!4402
|
|
into 'master'
operations: Fix wrong error code when UserSquash conflicts
See merge request gitlab-org/gitaly!4403
|
|
When creating repositories we use transactional voting to determine that
the repositories have been created the same on all nodes part of the
transaction. This voting happens after we have seeded the repository,
and the vote is computed by walking through the repository's directory
and hashing all its files. We need to be careful though to skip files
which we know to be indeterministic:
- FETCH_HEAD may contain URLs which are different for each of the
nodes.
- Object packfiles contained in the object database are not
deterministic, mostly because it may use multiple threads to
compute deltas.
Luckily, we do not have to rely on either of both types of files in
order to ensure that the user-visible state of the repository is the
same, so we can indeed just skip them.
While we already have the logic to skip these files, this logic didn't
work alright because we embarassingly forgot to actually return
`fs.SkipDir` in case we see the object directory. So even though we
thought we skipped these files, in reality we didn't.
This bug has been manifesting in production in form of CreateFork, which
regularly fails to reach quorum at random on a subset of nodes. The root
cause here is that we use git-clone(1) to seed repository contents of
the fork, which triggers exactly the case of indeterministic packfiles
noted above. So any successful CreateFork RPC call really only succeeded
by pure luck.
Fix this issue by correctly skipping over "object" directories. While at
it, fix how we skip over FETCH_HEAD by returning `nil`: it's a file and
not a directory, so it doesn't make much sense to return `fs.SkipDir`.
Changelog: fixed
|
|
repository: Add updateHeadFromBundle in CreateRepositoryFromBundle
Closes #4086
See merge request gitlab-org/gitaly!4401
|
|
|
|
[ci skip]
|
|
Extend invalid metadata deletion logic to repos existin on target
Closes #4083
See merge request gitlab-org/gitaly!4396
|
|
With the new improved error hadnling in UserSquash we're now returning
errors in some cases where we previously didn't. One of those cases is
when the rebase performed during the squash results in a merge conflict.
While it is correct to return an error in this case, we're using an
Internal error code for this case, which indicates that Gitaly is to
blame instead of the parameters which have been passed by the user.
Fix the error code to instead be FailedPrecondition. This error code is
special-cased by our monitoring infrastructure to not raise any alerts.
Note that this change is only fixing issues with monitoring: Rails
handles the error alright by inspecting the error details instead of the
error code.
Changelog: fixed
|
|
[ci skip]
|
|
proto: Add structured error types for UserRebaseCofirmable
See merge request gitlab-org/gitaly!4382
|
|
This commit introduces these changes by creating a new
UserRebaseConfirmableError Protobuf message which contains all potential
structured errors we want to return from the UserSquash RPC.
Changelog: added
|
|
Disable implicit pool creation on link behind a feature flag
See merge request gitlab-org/gitaly!4397
|
|
[ci skip]
|
|
doc: Document supported Git execution envinronments
See merge request gitlab-org/gitaly!4392
|
|
operations: Implement structured errors for UserSquash
See merge request gitlab-org/gitaly!4374
|
|
Document the different ways to access Git installations supported by
Gitaly. Most importantly, this also documents the way our new bundled
Git binaries work and why they were introduced.
|
|
[ci skip]
|
|
Add squash parameter to git2go merge
See merge request gitlab-org/gitaly!4241
|
|
cgroups: Remove paths field
See merge request gitlab-org/gitaly!4398
|
|
repository: allow CreateRepository to take default_branch
See merge request gitlab-org/gitaly!4385
|
|
The paths field on the CgroupV1Manager was being accessed concurrently,
leading to a panic. However, this field is not actually used by
anything. Fix this issue by removing the field.
Changelog: fixed
|
|
This commit adds a feature flag to disable the implicit pool creation
when joining a repository to a non-existent object pool. It looks like
this behavior is not needed given Rails should create the object pool
prior to considering it ready for joining. Disabling this behavior makes
it easier to handle pools in Praefect as Praefect needs to create metadata
records for all of the repositories. This implicit behavior might create
a repository without Praefect's knowledge. Special handling would be
required for this particular RPC only, so let's see if we can remove the
behavior rather than implement it in Praefect.
|
|
This change adds support for squashing in gitaly-git2go merge.
This will allow developing new RPC for squash that would be
more optimized, and wouldn't have to use rebase before
performing merge.
We simply use the same strategy as for merge, the only difference
is that we set only one parent to the merge commit.
Changelog: added
|
|
Squash commits in GitLab use different committer and author.
Committer is the user that merges, and author is the user
that created the MR.
As we want to add squashing to this command, we have to make
sure that we support Committer* arguments as well.
Committer variables are optional, author data is
used when they are empty.
That way this change is backward compatible.
However, if at least one of Committer* variables are set,
all of them have to be.
|
|
In Git, "ours" and "theirs" are named from the perspective of the
maintainer accepting the changes. Ours is the main target ref that
we merge into, and theirs is the source ref.
Based on orders of parents in call to CreateCommitFromIds
(ours, theirs) that is how we use those as well, only this comment
was wrong.
This is probably related to
https://gitlab.com/gitlab-org/gitaly/-/merge_requests/2868
and it was fixed in code, but the mistaken comment was left.
This is important to iron out before we add squashing,
as the wrong parent for squashed commit could be catastrophic.
|
|
Praefect contains logic to delete metadata record of a non-existent
source repository during replication. Praefect checks for the
ErrInvalidSourceRepository error, and if it receives it, it deletes
the metadata record of the source repository. Right now, the error
is only returned from ReplicateRepository if the source repository
didn't exist when the target is creating the repository for the
first time. This means Praefect's clean up logic doesn't run if the
target already has the repository on the disk but the source repository
doesn't exist. This can lead to reconciliation loops where Praefect
keeps trying to replicate from the non-existent source repository
over and over again as all of the replications fail. This commit
changes ReplicateRepository to return the error when the source
doesn't exist but the target repository does.
This logic was initially implemented to break the reonciliation loops
that occurred due to Praefect creating invalid metadata records when
a repository creation failed. As DeleteObjectPool RPC was not handled
as a repository deleting RPC in Praefect, the metadata records were
left in place which is currently causing reconciliation loops. Changing
the error return allows for Praefect to clean up those invalid metadata
records. In the long run, Praefect's background metadata verifier would
capture such issues and this logic could be removed.
Changelog: fixed
|
|
In GitLab rails, there is code to ensure that the default branch on a
new repository is set. This is currently done with an extra WriteRef
call. This is done in a racy way, which causes problems with transaction
voting. To fix this, we can simplify the logic altogether and avoid
adding another roundtrip, add a parameter in CreateRepositoryRequest
to pass a default_branch.
Changelog: changed
|
|
proto: Deprecate PreReceiveError fields
See merge request gitlab-org/gitaly!4393
|
|
With e542a7d87 (operations: Implement rich errors for UserMergeBranch
access checks, 2021-08-30) we have introduced the ode to return proper
errors when access checks failed via gRPC's rich error model. The
feature flag has been remove with a23b34894 (operations: Enable use of
detailed errors for UserMergeBranch, 2022-02-16) , which means that we
don't ever set the `PreReceiveError` field of the
UserMergeBranchResponse message anymore.
Deprecate the field such that we can eventually remove it.
Changelog: deprecated
|
|
The PreReceiveError field in the UserMergeToRefResponse isn't ever set
because the RPC call doesn't in fact perform any access checks at all.
Digging back through history this doesn't seem to be an oversight in the
Go port, but was the case in the Ruby implementation already. While the
initial implementation of UserMergeToRef in 8fbc337b8 (Allow merging to
a ref without changing the target branch, 2019-01-25) still used to call
hooks, this was explicitly changed with 19fdfa12a (Skip hooks for
UserMergeToRef RPC, 2019-06-14) to not do so anymore. The field thus
hasn't been set anymore for more than two years by now.
Deprecate the PreReceiveError field so that we can eventually remove it.
Changelog: deprecated
|
|
Rewrite relative path in RepositoryReplicas conditionally
See merge request gitlab-org/gitaly!4390
|
|
Rails is failing on RepositoryReplicas tests as it now checks the
replica path. However, the Rails tests do not configure a database
so Praefect can't get the replica path from there. It's not possible
to mock out the function with the interface provided either. To fix
the problem, this commit adds a config check which enables the
relative path rewriting only if repository-specific primaries are
enabled, which also indicates that a database must be configured.
|
|
Bump danger to 8.4.4
See merge request gitlab-org/gitaly!4388
|
|
|
|
Rewrite relative path in RepositoryReplicas
See merge request gitlab-org/gitaly!4384
|
|
git: Add support for bundled Git v2.35.1.gl0
Closes #4067 and #4068
See merge request gitlab-org/gitaly!4352
|
|
RepositoryReplicas RPC is currently not accounting for relative path
rewriting when sending the CalculateChecksum RPCs. This causes it to
fail when Praefect generates unique paths for the repositories. This
commit fixes the issue by sending the requests to the Gitalys with
the appropriate replica paths.
|
|
Praefect's test for RepositoryReplicas is not creating the repository
through the API. As such, it doesn't exercise the relative path
rewriting code and doesn't catch problems there. The QA tests caught
the fact that RepositoryReplicas is not doing relative path rewriting
yet. This commit updates the test first to create the repository through
the API and to account for the relative path rewriting when creating
a test commit in one of the replicas.
|
|
[ci skip]
|
|
Mark RefTransactionService as handled by Praefect
See merge request gitlab-org/gitaly!4383
|
|
This commit marks the RefTransactionService with the intercepted
option to denote it is handled by Praefect. Such services do not
need operation type annotations on the RPCs as they do not go through
the proxy. This should have no behavioral changes and mostly just
cleans up the code a bit.
|
|
We now have support in place for different bundled Git execution
environments, and a preceding commit has already started to build and
install bundled Git v2.35.1.gl1.
Add support for this new bundled Git execution environment behind a
feature flag.
Changelog: added
|
