| Age | Commit message (Collapse) | Author |
|---|
|
Since we upgraded our minimum version to 2.37.1, it's time to upgrade CI
to match.
|
|
Default enable Praefect generated replica paths
See merge request gitlab-org/gitaly!4809
|
|
fsck: Update fsck ignore rules configuration
Closes #4265 and #4404
See merge request gitlab-org/gitaly!4777
|
|
Added tests to ensure correct configuration of git-fsck(1) is generated
and applied to the command when executed.
|
|
Currently fsck ignore rules configuration only applies to
git-fetch-pack(1) and git-receive-pack(1) commands. These rules should
also apply to the git-fsck(1) command itself. This change unifies the
configuration.
|
|
Use semantic sort for tags
See merge request gitlab-org/gitaly!4336
|
|
Remove default MR template
See merge request gitlab-org/gitaly!4811
|
|
The default MR template creates unnecessary work since we already take
pains to write a coherent and detailed commit message. Once the MR
template supports automatic importing of commit messages, we can add a
default template once again.
|
|
golangci-lint: Add the `thelper` linter to enforce more-consistent test style
See merge request gitlab-org/gitaly!4806
|
|
The `thelper` linter has a rule that will flag test helper functions
that don't call `t.Helper()`. Unfortunately, this will also flag all
kinds of functions that really shouldn't have this call, which makes the
linter effectively useless for us.
Document why we disable it to not keep folks wondering.
|
|
Contributes to https://gitlab.com/gitlab-org/gitlab/-/issues/17801
**Problem**
Default sort by name for tags often does not produce the expected
result.
```
v1.1.0
v1.10.0
v1.2.0
```
**Solution**
Use semantic sort for tag names
```
v1.1.0
v1.2.0
v1.10.0
```
Changelog: changed
|
|
Enforce consistent naming of `testing.TB` variables, which should be
called `tb`, and adapt tests that violate this rule.
|
|
Enforce that `testing.T` et al must be the first parameter of test
functions. Unfortuntately, the linter explicitly allows for contexts to
precede `testing.T`, which causes us to still not enforce a uniform
style for our test functions.
Fix cases where we violate this rule.
|
|
Add the `thelper` linter, which knows to enforce consistent code styles
around tests:
- `testing.T` should be the first parameter in test-related
functions.
- Functions called by tests should call `t.Helper()`.
- `testing.T`, `testing.B` and testing.TB` variables should be
consistently named as `t`, `b`, and `tb`, respectively.
Right now we still have all rules disabled that would trigger warnings.
|
|
We currently exclude a revive rule that `context.Context` should be the
first parameter for our test sources. This can be handled better though
because golangci-lint allows us to exclude certain types from this rule.
Adapt the rule to allow `testing.T` et al before `context.Context` and
remove the excluded rule. Interestingly, this now surfaces a whole bunch
of `nolint: revive` annotations that aren't needed anymore, so we fix
them in the same commit.
|
|
Update golangci-lint to v1.48.0 and fix two warnings it starts to
surface due to that update.
|
|
gittest: Consolidate functions to create test repositories
See merge request gitlab-org/gitaly!4795
|
|
git: Fix warning on startup about Git binary fallback
See merge request gitlab-org/gitaly!4805
|
|
This commit default enables Praefect generated replica paths feature
flag. The feature flag has been enabled on production since 2022-07-25.
The changes have been exercised on GitLab.com with new repositories that
are mainly forks of GitLab projects that live on the Gitaly Cluster. There
have been no reports of issues and the logs don't surface anything that
seems problematic.
Changelog: changed
|
|
While it makes sense for tests to skip repository creation via the
`CreateRepository()` RPC when they are not testing in a context where
they have a gRPC server available, there should in the general case not
be a reason to skip this in our service-related tests. There still is a
bunch of tests that do this though.
Adjust the tests to not skip the RPC calls to create the repository and
refactor some that didn't make the server address available via the
Gitaly configuration. Note that we need to some touch up some tests
which erroneously created the repository multiple times, which would now
fail with Praefect.
|
|
Consolidate the unused InitRepo and CloneRepo functions. All existing
callers are using CreateRepository with `SkipCreationViaService` set to
`true.
|
|
Convert all callers of CloneRepo to use CreateRepository.
|
|
Convert all callers of InitRepo to use CreateRepository.
|
|
We have two sets of functions to create repositories for testing
purposes:
- CreateRepository creates the repository by calling the respective
RPCs of the RepositoryService. This allows us to properly test
with Praefect as a proxy, which requires the RPC to be called so
that it can set up the repositories in the database.
- InitRepo and CloneRepo, which both will create the repository
without doing an RPC call. These should only be used in contexts
where we don't have a gRPC service available.
It's confusing at times which of both should be used, and we're not
quite consistent.
Unify all functionality to create repositories into CreateRepository and
add a new option `SkipCreationViaService`. If set, this will behave the
same as InitRepo and CloneRepo, which allows us to consolidate the
functions into a single one. Callers that want to skip creation via the
RepositoryService will thus need to explicitly opt-in to this behaviour
and cannot just use InitRepo respectively CloneRepo without knowing
about the ramifications.
|
|
'master'
objectpool: Default-enable pruning of refs to fix reference conflicts
See merge request gitlab-org/gitaly!4807
|
|
limithandler: Remove RateLimiting feature flag
See merge request gitlab-org/gitaly!4796
|
|
'4257-usermergebranch-does-not-verify-it-got-a-user-name-and-email' into 'master'
usermergebranch: add validation for user.{email, name}
Closes #4257
See merge request gitlab-org/gitaly!4800
|
|
operations: Convert test hooks to use Bash instead of Ruby
See merge request gitlab-org/gitaly!4803
|
|
In 018958fb1 (objectpool: Fix conflicting references when fetching into
pools, 2022-07-27) we have introduced the logic to enable pruning of
references when we update an object pool via FetchIntoObjectPool. This
change was required because we have seen cases where updating object
pools continued to fail due to conflicting references which exist in the
pool repository and the member we have been fetching from. By pruning
any references which don't exist in the member anymore we were able to
resolve these conflicts and thus this bug.
This change has been rolled out to production on August 3rd without any
observed issues. So let's default-enable the feature flag guarding this
code so that the fix will be included in the next release.
Changelog: fixed
|
|
While 'UserMergeBranch' verifies that it got a user as part of the first
request, it doesn't verify that the user actually has a name and email
set. This leads to an Internal error at a later point when trying to
create the merge commit because you cannot create merge commits with
either of them being empty.
This commit improves our validation to check these parameters and return
an InvalidArgument error if unset. We add a new test
'TestUserMergeBranch_failure' to test the validation logic in
'UserMergeBranch'.
Signed-off-by: Karthik Nayak <knayak@gitlab.com>
|
|
Gitaly supports different execution environments with bundled Git and
distributed Git. If no such execution environment was properly set up
though we will fall back to try and resolve Git via PATH as a last
resort. Due to the way our environment constructors are arranged right
now though the fallback will always be constructed, which ultimately
leads to a warning that Git wasn't properly configured even though it in
fact was.
Fix this issue by removing the fallback Git environment from our default
constructors. Instead, we explicitly construct it only in the case where
we didn't find any other available execution environment. Like this, we
can now only print the warning when it actually applies.
Changelog: fixed
|
|
As we're progressing with out conversion of Ruby-based code we're
drawing ever closer to a future where there is no Ruby in Gitaly's code
base anymore. Some of our tests still depend on Ruby though because they
use a set of custom hooks that are implemented in it.
Refactor these scripts to use Bash instead of Ruby. This requires less
boilerplate code and gets rid of the last set of custom hooks in our
tests that use Ruby.
|
|
One of our tests for gitaly-hooks asserts that both the command line
arguments and environment seen by the process match our expectations.
This is done by writing some custom hooks, where one of the hooks is
currently written in Ruby.
Refactor the hook to use Bash instead in order to reduce our reliance on
Ruby. While at it, convert the test to use `gittest.InitRepo()` instead
of `gittest.InitRepo()`: we don't depend on any references or objects
anyway, so there is not much of a point to use a completely seeded
repository.
|
|
server: Introduce drift_threshold with proper type to replace drift_threshold_millis
Closes #4412
See merge request gitlab-org/gitaly!4788
|
|
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
|
|
linguist: Ensure empty files are omitted in totals
Closes #4416
See merge request gitlab-org/gitaly!4791
|
|
In the previous commit we've created a fix in the language stats, but
there still might be faulty values cached. To invalidate those, update
the version so we'll always recalculate stats from scratch unless it was
calculated with the fix in place.
|
|
Empty files should not be taken into account when calculation totals,
because the totals should have no languages that have a count of zero.
The linguist gem did this correctly already, but the newer Go
implementation didn't.
Issue: https://gitlab.com/gitlab-org/gitaly/-/issues/4416
Changelog: fixed
|
|
The ByteCountPerLanguage type is equal to map[string]uint64, so use the
type definition instead.
|
|
To make sure test cases are independent from eachother, convert the
existing test cases to table-based tests that set up a repository from
scratch each run.
|
|
[ci skip]
|
|
praefect: Check of the service readiness with RPC call
See merge request gitlab-org/gitaly!4674
|
|
Don't start of with a pre-existing repo, but instead build the commits
dynamically on demand. This prepares for testing with the SHA256 object
hashes.
|
|
The test coverage of TestInstance_Stats_incremental and
TestInstance_Stats/old_cache is nearly identical, so remove the
duplicate test.
|
|
I noticed when none of both fields is set, the user is presented with
the error saving "entry cannot have both OID and content". And when both
fields are set, no error is given.
Fix this by correctly error out when length of both is greater than
zero.
|
|
Now that this feature has been running in production for a few weeks
without issue, we can remove the feature flag.
|
|
For the praefect binary we have a sub-command to verify
if praefect service can operate without issues. The
verification process checks if migrations were applied,
if gitaly nodes are reachable, if the clock synced, etc.
This check can be done only when you have direct access
to the binary. With introducing of ReadinessCheck RPC
we now can run the same verification process mentioned
above by issuing an RPC call.
The new RPC will be part of the gitlab:gitaly:check task.
It is noop for the gitaly service as of now.
Part of: https://gitlab.com/gitlab-org/gitlab/-/issues/348174
|
|
It is hard to test ReadinessCheck RPC because the set of the
checks that is executed is hard or not possible to mock or
substitute. Those we made check an injectable to provide from
outside. It will help us to write better tests for the RPC.
|
|
To test ReadinessCheck RPC we need to run praefect service.
The runPraefectServer function does what we need, but it is
not exportable. This change moves and renames runPraefectServer
and related code, so it can be re-used in other packages for
testing.
|
