| Age | Commit message (Collapse) | Author |
|---|
|
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.
|
|
Convert all callers of CloneRepo to use CreateRepository.
|
|
We're about to add the ability to test with SHA256 hashes. We assume
none of the tests work with this object format.
With this change we add the build constraint to not run any test when
the tag 'gitaly_test_sha256' is set.
|
|
This commit changes the major version in the package name from v14 to
v15
Updating go.mod & go.sum with new module name v15
Update Makefile to bump major version to v15
Update the gitaly package name in the Makefile. Also update
gitaly-git2go-v14 -> gitaly-git2go-v15. We need to keep
gitaly-git2go-v14 for a release however, for zero downtime upgrades.
This pulls directly from a sha that is v14.
Update package name from v14->v15 for auth, client, cmd, internal packages
This commit changes the package name from v14 to v15 in go and proto
files in the internal, auth, client, cmd packages.
proto: Update major package number in package name
tools: Change major version number in package name from v14 to v15
gitaly-git2go: Change the package name from v14 to v15
update module updater for v15
Update the documentation for the module updater to reflect v15
|
|
In commit 8e5cb6419 (git: Convert internal fetches to use sidechannel,
2022-02-16), we have converted internalf teches to use the sidechannel.
With this change, communication exchanged between git-upload-pack(1) and
git-fetch(1) don't use gRPC encoding anymore, but instead use pkt-line
encoding such that we can avoid having to serialize and deserialize lots
of gRPC messages. This avoids the overhead induced by grpc-go and should
thus reduce the load while serving an internal fetch.
This change has been rolled out to production on February 23rd without
any issues observed. Remove the feature flag to unconditionally enable
use of the sidechannel.
Changelog: changed
|
|
With commit b5722938b (Merge branch 'jv-ssh-sidechannel-2' into
'master', 2022-01-18) we have introduced a new RPC which allows us to
serve SSH-based fetches with a sidechannel. Instead of using gRPC as a
means for communicating between client and server we instead use a
simple protocol based on pktlines, which allows us to avoid the overhead
introduced by gRPC message serialization and deserialization.
For now, use of the sidechannel is only enabled for external clients
which come in via gitlab-shell. This commit adds the necessary infra to
also use the sidechannel for internal fetches, which most importantly
includes the RPCs ReplicateRepository, FetchSourceBranch and a subset of
RPCs in the OperationService which know to fetch from a remote
repository.
The new code is guarded by a feature flag.
Changelog: performance
|
|
setupRepositoryService is currently using a TODO context. This commit
propagates the test context down from the tests to the setup helper.
|
|
This commit creates the repositories in tests of repsository service
via API. This enables the tests to work with Praefect without the
metadata creation hack as Praefect can create the metadata when is
proxies the creation calls. The tests converted in this commit are the
ones where it was straighforward replacing, the rest will be done in
more detailed follow up commits.
|
|
testhelper.Context() currently return a cancellation function as a
second return value. Majority of the tests do not need to explicitly
cancel the context but they simply defer its cancellation to clean up
after the test. Given this, we can reduce the test verbosity and make
testhelper.Context easier to compose by removing the unnecessary second
return value. This adds a t.Cleanup function to automatically cancel the
context at the end of the tests and omits the returned cancellation
function.
Tests which simply `defer cancel()` have had the extra call removed. Some
tests explicitly call the cancellation, and these tests have been modified
to add context.WithCancel around the testhelper.Context call. There are a
few loctions where testing.TB was passed down to test helpers that create the
context.
|
|
The `RequireGrpcError()` helper function misleadingly doesn't check the
error for equality, but instead only verifies that the error has the
expected gRPC code.
Rename the function to `RequireGrpcCode()` to clarify this.
|
|
Move the helper to get Gitaly server metadata from the Gitaly config
from the testhelper package into the testcfg package. This is required
to break the dependency of the testhelper on `internal/gitaly/config`,
such that this package can start to use the testhelper package.
|
|
Rename `CloneRepoAtStorage()` to `CloneRepo()`. We always have storages
available, and there are no other gittest functions to clone a repo
anymore, so having the shorter name should suffice.
|
|
`CloneRepoAtStorage()` currently returns a cleanup function. Since Go
has `t.Cleanup()`, this is considered bad style, so let's remove it and
convert all callers.
|
|
Most tests don't really care about the relative path of the repository
that is to be created. Furthermore, manually specifying a relative path
will lead to repository paths outside of our typicaly hashed storage
paths, and as thus they don't really match what we'd see in production.
Put the relative path into an optional `CloneRepoOpts` structure: if
unset, we'll compute random hashed storage-style repository paths. Most
callers are converted to not pass a relative path at all, while a small
bunch of tests which relies on deterministic paths are converted to use
the options struct.
|
|
To speed up execution of the tests we mark them allowed to
run in parallel by calling t.Parallel(). As a first step we
mark for parallel execution only upper-level test and no sub-tests.
|
|
The new "v14" version of the Gitaly module is named to match
the next GitLab release. The module versioning is needed in
order to pull gitaly as a dependency in other projects. The
change updates all imports to include v14 version. The go.mod
file was modified as well after go mod tidy execution. And
the changes in dependency licenses are reflected in the NOTICE
file.
Part of: https://gitlab.com/gitlab-org/gitaly/-/issues/3177
|
|
In order to get rid of the global variable config.Config we change
CloneRepoAtStorage function to accept config.Cfg as an input parameter.
We use locally defined instance of the config.Cfg type to call modified
function, so the usage of it is no longer depend on the config.Config
variable.
Part of: https://gitlab.com/gitlab-org/gitaly/-/issues/2699
|
|
The current interface to write commits is leaving a lot to be desired.
It's inflexible, we have multiple functions which all do some specific
things which others don't, and in general it's not a breeze to use.
As a first step to improve it, this commit refactors `CreateCommit()`:
instead of an options structure it now receives callback-like options
and it is renamed to `WriteCommit()` to bring it in line with the other
object-writing test helpers.
|
|
It's really hard right now to add additional dependencies on the
localrepo constructor given that it's used all over the place. To
address one part of the problem, introduce a new test helper which
constructs a `localrepo.Repo` given only a Gitaly config and a
repository interface -- all other dependencies get created ad-hoc.
Ideally, this would not be hosted in the localrepo package itself but in
the gittest package, but this doesn't work due to cyclic dependencies.
But given both the function name `NewTestRepo()` and that the function
requires a `testing.T` interface as first parameter, it shouldn't really
be possible to use it in production code.
|
|
The old test setup code is refactored to use the new setup functions.
The TestFetchFullServerRequiresAuthentication test removed
as it was just a sanity check for the test-setup code.
Package changed from repository_test to repository, because
circular dependency is broken with new test setup functions.
Part of: https://gitlab.com/gitlab-org/gitaly/-/issues/2699
|
|
In order to break dependency on the global config.Config
variable this change modifies the parameter list of the
functions that use config.Config internally to accept
it from the outside.
|
|
The function gittest.CreateCommit changed to accept
configuration as an input parameter and now uses
Exec and ExecStream functions internally to run git commands.
All dependencies aligned on that change.
Part of: https://gitlab.com/gitlab-org/gitaly/-/issues/2699
|
|
Current project structure sometimes causes us a circular dependency
issues. The main reason of it is RegisterAll function in the
internal/gitaly/service package. It is a package on a higher level
that imports packages from the deeper levels, like
internal/gitaly/service/blob, etc. It is not an issue for the production
code, but it causes troubles in the tests. The solution used so
far was to use a separate package '<package>_test' for the tests.
Other than that the old server setup doesn't use production code for
the initialization. With RunGitalyServer helper we re-use production
code for gRPC server setup and can register only required services on it.
The dependencies services relies on could be overwritten with the optional
parameters. All dependencies assembled into a single struct Dependencies
for that purpose as well as to reduce amount of parameters to pass into
the function call. The access to the dependencies is done via a GetXXX
methods, so it is easy to replace it with interface and whatever implementation
afterwards.
|
|
This commit dependency injects a shared backchannel registry to the
components that need it, namely the GitalyServerFactory and transaction
manager.
|
|
Implementation of the linguist package relies on the
package-private state: the use of the colorMap variable.
It is filled with a hook mechanism after validation of the
gitaly configuration. The problem is that this operation
invokes ruby code to get it done and this call is pretty
expensive. It also doesn't allow us to convert tests to
parallel as it causes race conditions in attempt to fill
the global variable.
The solution to this problem is creation of the struct
instance that encapsulates the state and can be instantiated
only when needed. The change also includes minification in
generating test configuration that does not require to make
a ruby call and uses a stub file for initialisation. The
behaviour could be returned back to the normal with usage of
the WithActualLinguist option for configuration creation.
The change also includes renames of the test functions for
the linguist package. The change speeds up execution of the
tests: before 0m43.907s and after 0m21.659s on my local.
Part of: https://gitlab.com/gitlab-org/gitaly/-/issues/2699
|
|
It's currently not possible to use our git DSL in the testhelper package
because of an import cycle between the testhelper and git package. To
fix this import cycle, we thus move test-related git helpers into the
gittest package.
This commit moves the repository helpers. As we're already touching all
sites which use these helpers anyway, it also aligns functions to have
more consistent naming.
|
|
It's currently not possible to use our git DSL in the testhelper package
because of an import cycle between the testhelper and git package. To
fix this import cycle, we thus move test-related git helpers into the
gittest package.
This commit moves the commit helpers.
|
|
Server factory should not manage gitaly-ruby server internally
as it completely independent entity and should be provided only
as a dependency into other components.
The change also restructure creation of the gRPC servers by the
factory, so that doesn't rely on the specific services. This gives
us un opportunity to re-use setup code in the tests and remove
the helpers that are not complete compared to actual production used.
|
|
Current implementation of the localrepo.Repo creates
*git.ExecCommandFactory internally and use it for
running git commands. This dependency needs to be
provided from the outside instead. The git.CommandFactory
parameter added to the constructor func and all its usages
fixed thor out the code.
|
|
|
|
This commit converts all users of `log.GetCommit()` and
`log.GetCommitWithTrailers()` to instead use the new
`localrepo.ReadCommit()` function.
|
|
As we are abstracting dependencies required to create a git
command we introduced git.CommandFactory interface that is
in duty of creation of the git commands. The catfile package
used across different services and as a first step we provide
factory as an additional constructor parameter.
Part of: https://gitlab.com/gitlab-org/gitaly/-/issues/2699
|
|
Changes done in 052682f51 (git: Convert needless use of `SafeBareCmd`, 2021-01-13)
remove need in having storage.Locator dependency in
catfile package. This commits removes that dependency as
unused.
Part of: https://gitlab.com/gitlab-org/gitaly/-/issues/2699
|
|
Factory extended with additional constructor parameters to
reduce code coupling and make it dependent on abstractions.
Two server constructors combined into one that has a single
additional parameter to control tls usage.
Part of: https://gitlab.com/gitlab-org/gitaly/-/issues/2699
|
|
We'll need the transaction manager in the RepositoryService in order to
enable transactional voting for RPCs which modify the repository state.
So let's inject the manager to make it available.
|
|
The transaction.Manager is currently instantiated by the hook.Manager
directly, which was done in order to make splitting up transaction and
hook logic focussed on the task without doing global changes in the
preceding commit. We do want to dependency inject it from external
though given that hooks aren't going to be the single location where
they're used.
So let's do so and adjust all sites where the hook manager is
instantiated to also pass a transaction manager.
|
|
The GoFetchSourceBranch feature flag has been default-enabled in commit
8443b3333 (featureflag: Enable Go implementation of FetchSourceBranch by
default, 2021-01-13), which was released with v13.8.0. We can thus
remove this feature flag altogether now.
|
|
Instead of accepting an untyped string, this commit refactors the
log interface to accept Revisions instead.
|
|
The `testhelper.GetTemporaryGitalySocketFileName()` function currently
returns an error. Given that all callers use `require.NoError()` on that
error, let's convert it to instead receive a `testing.TB` and not return
an error.
|
|
Currently, `TestServer.Start()` returns an error. Given that it's only
used for tests and never inside of the test's main function, let's
convert it to instead receive a `testing.TB` and use `require`.
|
|
The catfile package directly or indirectly uses config.Config
value. And as we are about to remove config.Config from the
code we need to break this dependency and substitute with
other abstractions.
For now those are storage.Locator and alternates.Env instead
of the old alternates.PathAndEnv that depends on the global var.
It was decided to extend constructor function of the catfile
to accept storage.Locator as a dependency as it is already
injected in most of the "server"s which handle requests.
The other option could be creation of the "factory", but it will
require a new dependency to be injected into all existing
services which is a lot more changes and adds currently unnecessary
abstraction that needs to be managed.
Part of: https://gitlab.com/gitlab-org/gitaly/-/issues/2699
|
|
To remove dependencies on the global config.Config var the Locator
dependency injected into hook manager.
To reduce amount of parameters to pass into GitlabAPI.Allowed method
a new AllowedParams struct defined and used instead.
Part of: https://gitlab.com/gitlab-org/gitaly/-/issues/2699
|
|
The repository tests currently don't execute any hooks. This is about to
change when we globally enable execution of the reference-transaction
hook, but as we don't have the hook service correctly set up this would
cause tests to fail for this service.
Fix the issue by setting up the hook service on Gitaly's internal
socket and installing the gitaly-hooks binary. Ideally, we'd just set up
the internal socket once and for all in `runRepoServer()`. But for
reasons I wasn't able to figure out, this would cause test failures with
Praefect due to the internal listening socket not being closed
somewhere.
|
|
In order to remove dependency on the global config.Config var we
need to break dependencies between Config and it's initialization
and validation functions. Most of them now represented as a methods
on the config.Cfg type. It allows us to setup required Cfg struct
for the tests to run without need to sync changes done on the
shared global Config var. The tests that were heavily dependent
on the Config var now depend on the struct initialized inside
the test instead.
Part of: https://gitlab.com/gitlab-org/gitaly/-/issues/2699
Blocks: https://gitlab.com/gitlab-org/gitaly/-/issues/3166
|
|
Dependency inject connection pool in to the operations server to
share connections between multiple services.
|
|
In order to break dependency on the shared global config.Config
variable the Locator interface is injected into the service.
Calls that eventually lead to global config.Config var were
removed.
Part of: https://gitlab.com/gitlab-org/gitaly/-/issues/2699
|
|
Since the introduction of Praefect, our code layout started to become
confusing: while Praefect code lives in `internal/praefect`,
Gitaly-specific code is all over the place and not neatly singled out.
This makes it hard at times to tell apart Praefect- and Gitaly-specific
from generic code.
To improve the situation, this commit thus moves most of the server
specific code into a new `internal/gitaly` package. Currently, this is
the `internal/config`, `internal/server`, `internal/service` and
`internal/rubyserver` packages, which are all main components of Gitaly.
The move was realized with the following script:
#!/bin/sh
mkdir -p internal/gitaly
git mv internal/{config,server,service,rubyserver} internal/gitaly/
find . -name '*.go' -exec sed -i \
-e 's|gitlab-org/gitaly/internal/rubyserver|gitlab-org/gitaly/internal/gitaly/rubyserver|' \
-e 's|gitlab-org/gitaly/internal/server|gitlab-org/gitaly/internal/gitaly/server|' \
-e 's|gitlab-org/gitaly/internal/service|gitlab-org/gitaly/internal/gitaly/service|' \
-e 's|gitlab-org/gitaly/internal/config|gitlab-org/gitaly/internal/gitaly/config|' {} \;
In addition to that, some minor adjustments were needed for tests which
used relative paths.
|
