Cleanup robustness tests and `local_fs_test.go`
"Mechanical" changes:
- Use `require` helpers
- Use `testing.T` helpers
Note change in functionality: The use of `require` helpers
stops tests once a check fails. Before, various checks
were using `t.Error`, which fails the test but allows the
test to continue its execution.
* refactor(general): cleanup robustness/snapmeta/kopia_persister_light_test.go
Use `require` helpers
Use `testing.T` helpers
* refactor(general): cleanup local_fs_test.go
* fix import order
* use `os.CreateTemp` in `tempfile.CreateAutoDelete`
* refactor `TestTempFile`: add `VerifyTempfile` test helper
* add test for `createUnixFallback`
* rename function to `tempfile.CreateAutoDelete`
* remove the `dir` parameter to `tempfile.CreateAutoDelete`,
only an empty string was passed, apart from test cases.
* guard against panic in TestShadowCopy
* use uint8 for clarity
* unexport writeContentAsyncAndVerify
* fix typo in test function name
* remove commented interface functions
* use atomic.Int32
* cleanups in socket server activation test
* leverage stdlib's maps and slices packages
replace uses of `golang.org/x/exp/maps`
* nit: leverage `maps.Values`
Added functionality to calculate aggregate statistics when
comparing what's changed between snapshots using kopia diff
Statistics collected during snapshot diff computation includes:
- files added/removed/modified
- dirs added/removed/modified
- files/dirs with metadata changes but same underlying content (OID)
Testing approach:
Added a test for verifying stats collected when comparing two directories with the same objectID but metadata changes across snapshots (dir mode, dir mod time, dir owner, etc), expectation is all the appropriate dir stats fields are updated.
Added another test for verifying stats collected when comparing two directories with similar file contents but the metadata for the files have changed between snapshots but content remains unchanged. Expectation is all the relevant file level stats fields are updated.
Existing tests have been updated due to stats now being printed in addition to previous output.
The kopia server was not uploading any logs to the repository,
because "repodiag" blob uploads would always fail.
The cause was the following: when the (log) repodiag blob
PUT operation was initiated, the `Context` used for this
operation was already canceled.
The context used for blob uploads is passed to
`repodiag.NewLogManager` when opening the repository.
In the case of the kopia server, the repository is asynchronously
opened in `server.Server.InitReposotoryAsync`. The context
passed to `repo.Open` is canceled after the "open repository"
server task completes.
This issue was introduced in #1691
Change:
Use `context.WithoutCancel()` instead of the context passed
when the repo diagnoser is created.
New tests are included to reproduce this failure and verify
the fix.
- test: ensure server logs are uploaded to the repo
- test: honor cancellation in map storage
- test: repodiag context cancellation
Ref:
- #1691
This was caused by the client using key derivation algorithm
from a config file (which did not have it when it was generated
using old version of Kopia).
Fixes#4254
Followups to #3655
* wrap fs.Reader
* nit: remove unnecessary intermediate variable
* nit: rename local variable
* cleanup: move restore.Progress interface to cli pkg
* move cliRestoreProgress to a separate file
* refactor(general): replace switch with if/else for clarity
Removes a tautology for `err == nil`, which was guaranteed
to be true in the second case statement for the switch.
Replacing the switch statement with and if/else block is clearer.
* initialize restoreProgress in restore command
* fix: use error.Wrapf with format string and args
Simplify SetCounters signature:
Pass arguments in a `restore.Stats` struct.
`SetCounters(s restore.Stats)`
Simplifies call sites and implementation.
In this case it makes sense to pass all the values
using the restore.Stats struct as it simplifies
the calls.
However, this pattern should be avoided in general
as it essentially makes all the arguments "optional".
This makes it easy to miss setting a value and simply
passing 0 (the default value), thus it becomes error
prone.
In this particular case, the struct is being passed
through verbatim, thus eliminating the risk of
missing a value, at least in the current state of
the code.
Generalize a couple of functions in the units package using generics.
This allows removing duplicate code and simplifying callers by removing unnecessary integer conversions.
Additional cleanups:
- make "/s" part of the Printf format string ;
- simplify setSizeMBParameter;
- generalize cli.maybeHumanReadable*` helpers;
- remove unneeded receiver in commandRepositorySetParameters helpers.
Cleanups:
- use non-format variants of Log/Print with no additional args;
- fold in Fprintf call with no args into the following one;
- add missing arg placeholder in format strings;
- use require.Positive instead of Greater(..., 0);
- rename function to fillWithZeros to avoid collision with builtin clear;
- define type for context key to avoid collisions.
Addresses failure introduced in #3963
Change: allow `delete-random-snapID` action in the function `tryDeleteAction`.
Test: Manual local run was successful.
The robustness tests set soft limits for content cache size
and metadata cache size. This PR adds the hard limits for
both the caches, thus reducing the memory required to
run the robustness tests for long term.
The soft limits are set for content cache and metadata cache.
This may cause the cache to bloat beyond expectations.
Setting hard limits for the size would keep the cache bloat in
check, thus reducing the memory needed to required to run
the robustness tests over a long time.
---------
Co-authored-by: Julio López <1953782+julio-lopez@users.noreply.github.com>
Ensure repository disconnection at the end of the `server start` CLI command.
This was caught as a result of fixing the test below.
Fix `TestServerStartInsecure`:
Remove `--password=xxx` parameter, which causes a server start failure
due to incorrect repo password, and not for the case being checked,
which is the lack of the `--insecure` parameter.
Update test comments accordingly.
Adds the maintenance action to multi-client robustness jobs.
The robustness jobs do not run a maintenance action until now.
---------
Co-authored-by: Julio López <1953782+julio-lopez@users.noreply.github.com>
The robustness tests perform delete actions on random subdirectories and
files. If the actions encounter `directory not found` error, the framework
returns a `no-op` error.
This change ignores the `no-op` errors specifically for "delete" actions.
This change will reduce the failure frequency when a delete actions on the
source directory result in a no-op.
Use non-formatting logging functions for message without formatting.
For example, `log.Info("message")` instead of `log.Infof("message")`
Configure linter for printf-like functions
The robustness test `TestRandomizedSmall` generate file action gets
picked up four to eight times more frequently than the other actions.
This PR reduces the frequency of the file generation action so that the
test is more representative of the user workflow.
The robustness framework runs do not cover delete actions at the moment.
This change adds the following actions to the robustness jobs
- delete random subdirectories
- delete contents of directories
Code movement and simplification, no functional changes.
Objectives:
- Allow callers specifying the needed key (or hash) size, instead of
hard-coding it in the registered PBK derivers. Conceptually, the caller
needs to specify the key size, since that is a requirement of the
(encryption) algorithm being used in the caller. Now, the code changes
here do not result in any functional changes since the key size is
always 32 bytes.
- Remove a global definition for the default PB key deriver to use.
Instead, each of the 3 use case sets the default value.
Changes:
- `crypto.DeriveKeyFromPassword` now takes a key size.
- Adds new constants for the key sizes at the callers.
- Removes the global `crypto.MasterKeySize` const.
- Removes the global `crypto.DefaultKeyDerivationAlgorithm` const.
- Adds const for the default derivation algorithms for each use case.
- Adds a const for the salt length in the `internal/user` package, to ensure
the same salt length is used in both hash versions.
- Unexports various functions, variables and constants in the `internal/crypto`
& `internal/user` packages.
- Renames various constants for consistency.
- Removes unused functions and symbols.
- Renames files to be consistent and better reflect the structure of the code.
- Adds a couple of tests to ensure the const values are in sync and supported.
- Fixes a couple of typos
Followups to:
- #3725
- #3770
- #3779
- #3799
- #3816
The individual commits show the code transformations to simplify the
review of the changes.
* Fix restoring objects with I prefix
set default of snapshot-time to 'latest' as noted in the help output
* Change test of restore to check it works without a time given
This is because --snapshot-time defaults to "latest" now.
* Implement volume shadow copy support on Windows
* Update go-vss version
* Fix unused variables
* Rename upload_actions*.go files
* Move vss settings to a separate policy section
* Handle existing shadow copy root
* Fix tests
* Fix lint issues
* Add cli policy test
* Add OS snapshot integration test
* Add GitHub Actions VSS test
* Fix "Incorrect function" error for root VSS snapshots
* Rename err to finalErr in createOSSnapshot
* Add OSSnapshotMode test
* Do not modify paths starting with \\?\ on Windows
* Allow warning messages in logfile tests
* Fix ignorefs not wrapping OS snapshot directory
* Retry VSS creation if another op was in progress
---------
Co-authored-by: Jarek Kowalski <jaak@jkowalski.net>
