| Age | Commit message (Collapse) | Author |
|---|
|
Currently in many places of our codebase, we have:
switch err {
case ErrMissingLeaseFile:
c.errTotal.WithLabelValues("ErrMissingLeaseFile").Inc()
case ErrPendingExists:
c.errTotal.WithLabelValues("ErrPendingExists").Inc()
}
while this works, its not compatible with wrapped errors. So let's fix
this by using `errors.Is()` instead.
|
|
The cache walker and repository counters log their duration upon
completion. However, they do so in nanoseconds, while our other logged
durations are in milliseconds.
Convert these items to log in milliseconds
Changelog: changed
|
|
The ctxlogrus package is going away with the replacement being log
fields extracted from the context via `log.DebugContext()` et al.
Refactor the code to stop using ctxlogrus by injecting a logger and
using the new context-based logging methods.
|
|
Remove the `Errorf()` function from the `Logger` interface. Please refer
to the preceding commit that removes `Debugf()` for the motivation
behind this change.
|
|
Remove the `Infof()` function from the `Logger` interface. Please refer
to the preceding commit that removes `Debugf()` for the motivation
behind this change.
|
|
The functions `Debug()`, `Info()` and related accept multiple arguments.
The first hunch would be that the logger would format those arguments
via a call to `fmt.Sprintf()`, and that's seemingly also the assumption
that many of our callsites had. But in fact, the logrus logger will call
`fmt.Sprint()`, which joins the arguments by spaces when neither of the
arguments is a string.
While not being obvious, the current definition also conflicts with the
`slog` package. While the respective functions there also accept a
variable number of arguments, they will instead be interpreted as key
value pairs of structured data.
Let's simplify our calling conventions such that the functions only
accept a single message parameter and adjust callsites that misused
these functions accordingly. This fixes a source of misformatted log
messages and prepares us for the conversion to the `slog` package.
|
|
The `ctxlogrus` package is about to go away. Let's encapsulate it so
that the transition will become easier for us at a later point.
|
|
Introduce a wrapper for the logrus logging infrastructure. This is a
first step towards abstracting away the actual implementation of our
logs behind a type that we can iterate on and will thus pave the way
towards adoption of the `slog` package.
Note that this is only one step of this transition. The interface does
not yet roundtrip to itself, e.g. calling `log.Logger.WithError()` will
result in a `logrus.Entry`, not in a `log.Logger`. This will be handled
at a later point.
|
|
This commit wires the tests to call SharedLogger instead of NewLogger
so they'll use the test case's shared logger. This way the log output
is printed to the same logger and is ordered.
|
|
Gitaly now has a helper for recording log output and outputting it
only in tests. Replace the usage of the discarding logger with the
recording logger. These changes are search and replace so these call
sites didn't even need a log entry to begin with but rather use a
FieldLogger.
NewDiscardingLogEntry is removed as it is no longer used.
|
|
We're using the global logging instance provided by `log.Default()`,
which is about to go away soon. Refactor the code to instead pass in a
proprely configured logging instance.
|
|
We're using the global logging instance provided by the logrus package,
which is discouraged. Inject a logger such that we can use a properly
configured logger instead.
|
|
The feature flag logic is currently hosted inside of
`internal/metadata/featureflag`. And while they do have something to do
with metadata, they really are a standalone concept.
Move the package to `internal/featureflag` to reflect this.
|
|
We're about to release Gitaly v16.0. As we've landed a bunch of
previously announced removals it's thus time to bump our Go module
version from v15 to v16.
|
|
Avoid giving the executable permission to normal files. Prefer using
PublicFile and rely on umask restricting permissions further.
|
|
Bulk update all file and executable permissions to use the new perm
package.
Strictly speaking changing os.ModePerm to perm.PublicFile is a
permission change (it goes from executable to normal), but since
ModePerm is only used in tests this should be safe.
|
|
655 is executable by group and other but not user. This doesn't
make a lot of sense. So we change it here to non-executable-shared.
|
|
Convert tests to work with the SHA256 object format by removing the need
for seed repositories.
|
|
There should be no actual permissions changes here. This is a
like-for-like replacement.
|
|
A few weeks ago we have decided that `testing.TB` arguments should be
first, `context.Context` second. This is rather a matter of perssnal
taste instead of correctness, but we should strive for consistency.
Reorder all arguments to match the agreed-upon style.
|
|
Go 1.15 has deprecated the `ioutil` package and made replacements
available in the `io` or `os` packages. While we have already migrated
most callsites away, one notable omission was `ioutil.ReadDir()` as this
function doesn't have an exact replacement: the new `os.ReadDir()`
function does not stat all directory entries anymore.
Refactor the cleanup of stale cached files in the `cache` package to use
the new function.
|
|
Go 1.15 has deprecated the `ioutil` package and made replacements
available in the `io` or `os` packages. While we have already migrated
most callsites away, one notable omission was `ioutil.ReadDir()` as this
function doesn't have an exact replacement: the new `os.ReadDir()`
function does not stat all directory entries anymore.
Refactor the cleanup of stale leases in the `cache` package to use the
new function.
|
|
Check error codes when closing a `safe.LockingFileWriter` or
`safe.Writer` and drop the corresponding exclude from our linting rules.
|
|
Enforce consistent naming of `testing.TB` variables, which should be
called `tb`, and adapt tests that violate this rule.
|
|
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.
|
|
To extract feature flags from the current context you can call the
`AllFlags()` function. Just from looking at that function's name it's
not clear though what it returns. As it turns out, it returns the flag
as plain strings in the format of "${flag_name}:${flag_value}". This
format is quite inaccessible to callers given that they'd have to
manually split name and value and then also know to parse the value.
This makes for an easy-to-misuse interface.
Replace `AllFlags()` with a new type-safe `FromContext()` function that
returns a map of `FeatureFlag`s mapped to their respective value. Like
this it's immediately clear what you obtain and enjoy all the comfort of
using the various functions on the feature flag itself.
Refactor callers to use this new function.
|
|
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
|
|
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.
|
|
We're about to disallow use of "normal" contexts created via the
`context` package given that they do not perform sanity checks for
feature flags. Instead, contexts should be created via the testhelper
package.
Convert tests to use the testhelper context.
|
|
The keyer tests are all dependent on each other, and furthermore they're
written in terms of one long test. This makes it hard to see the
different parts we're trying to test, and furthermore it makes it harder
than necessary to refactor those tests.
Split up these tests into several subtests and have each of them use a
separate cache such that there can be no interdependencies.
|
|
Refactor the code to avoid timeouts. Unfortunately, there is no way
right now to verify that the cache walker did its thing, so we still
have to use a timeout there. We hopefully avoid any flakiness though by
setting a high timeout value.
|
|
Consolidate the feature flags context APIs such that we have the same
set of functions for creating feature flags in incoming and outgoing
contexts.
|
|
The disk cache spawns a set of walkers which regularly walk the cache to
check for cache members which are older than a given threshold. These
walkers currently cannot be stopped at all, leading to a Goroutine
leakage in our tests.
Implement a new `StopWalkers()` function which stops them and use it in
our tests.
|
|
The dontpanic package provides a `GoForever()` function which allows the
caller to run a function forever even if it panics. While this is a
useful thing to have, the current design also causes leakage of
Goroutines because we do not have a way to stop these functions even in
the context of tests.
Introduce a new `Forever` structure which now hosts the `Go()` function.
Like this, we can also easily introduce a `Cancel()` function which
causes us to abort execution of the function.
Callers are not yet adjusted to make use of the `Cancel()` function.
|
|
Setup of a test package's main function is quite repetitive: we call a
separate `testMain()` function such that we can use `defer` calls, this
function calls `MustHaveNoChildProcess()` and `Configure()`, and then we
return the result of `m.Run()`. This is almost always exactly the same,
with some exceptions where we need to have additional setup code.
Unify this code via a single `testhelper.Run()` function which does all
of this. It allows us greater flexibility in the setup code such that we
can easily perform additional validation after the tests without having
to modify all callsites.
Note that this change removes calls to the goleak package in some
places. These will resurface in a later commit in this patch series,
where we instead move this call into `testhelper.Run()` such that it is
executed by default.
|
|
The "io/ioutil" package has been deprecated in Go 1.16 and thus
shouldn't be used in our codebase anymore. Disallow its usage by adding
the "depguard" linter with a single entry for this specific package.
Note that we still use the package in some places where we used to rely
on `ioutil.ReadDir()`'s old behaviour of stat'ting all directory
entries. Conversion of these is not part of this patch series given that
it would be a change in behaviour compared to all the other changes
which aren't. Those sites are thus annotated with `nolint` directives.
|
|
With Go 1.16, the ioutil package was deprecated. In addition to being
moved into the os package, `ioutil.ReadDir()` was also changed to not
stat(3P) all dir entries anymore. As a result, the caller now has to
do so manually. This is a performance improvement in some cases where
the caller didn't require any of the file information, but really only
wanted to read the directory's entries.
Adapt trivial usecases of `ioutil.ReadDir()` which do not require this
information with usage of `os.ReadDir()`. This leaves a few callsites of
the old `ioutil.ReadDir()` function for future conversion.
|
|
With Go 1.16, the ioutil package was deprecated. Replace our usage of
`ioutil.WriteFile()` with `os.WriteFile()` to adapt accordingly.
|
|
With Go 1.16, the ioutil package was deprecated. Replace our usage of
`ioutil.TempFile()` with `os.CreateTemp()` to adapt accordingly.
|
|
With Go 1.16, the ioutil package was deprecated. Replace our usage of
`ioutil.TempDir()` with `os.MkdirTemp()` to adapt accordingly.
|
|
With Go 1.16, the ioutil package was deprecated. Replace our usage of
`ioutil.ReadFile()` with `os.ReadFile()` to adapt accordingly.
|
|
With Go 1.16, the ioutil package was deprecated. Replace our usage of
`ioutil.ReadAll()` with `io.ReadAll()` to adapt accordingly.
|
|
In Gitaly, constructors for structs are typically called `NewStruct()`.
Rename `CreateFileWriter()` to `NewFileWriter()` to better match our
coding style.
|
|
Reformat sources with gofumpt.
|
|
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.
|
|
The dependency github.com/golang/protobuf is deprecated and should
be replaced with google.golang.org/protobuf.
This change replaces usage of the proto package. The packages have
incompatible API that is why the code changes were done to migrate.
The exclusion rule removed from the golanci-lint tool configuration
file to prevent future use of the deprecated package.
|
|
Convert the cache package to use the locator interface to determine
locations of cache and state directories. This will eventually allow us
to get rid of the old interface.
|
