From 77a417b1cea1597bdac0a0682f6357a78076f782 Mon Sep 17 00:00:00 2001 From: ashmrtn <3891298+ashmrtn@users.noreply.github.com> Date: Wed, 1 Nov 2023 09:30:06 -0700 Subject: [PATCH] Exclude preview backups when looking for bases (#4593) Add a comment and test ensuring preview backups are excluded during base selection as expected. It's still not possible to make a backup tagged with the preview tag right now --- #### Does this PR need a docs update or release note? - [ ] :white_check_mark: Yes, it's included - [ ] :clock1: Yes, but in a later PR - [x] :no_entry: No #### Type of change - [x] :sunflower: Feature - [ ] :bug: Bugfix - [ ] :world_map: Documentation - [ ] :robot: Supportability/Tests - [ ] :computer: CI/Deployment - [ ] :broom: Tech Debt/Cleanup #### Test Plan - [ ] :muscle: Manual - [x] :zap: Unit test - [ ] :green_heart: E2E --- src/internal/kopia/base_finder.go | 4 ++++ src/internal/kopia/base_finder_test.go | 22 ++++++++++++++++++++ src/internal/model/model.go | 28 +++++++++++++++++++------- 3 files changed, 47 insertions(+), 7 deletions(-) diff --git a/src/internal/kopia/base_finder.go b/src/internal/kopia/base_finder.go index e65722278..c058524cf 100644 --- a/src/internal/kopia/base_finder.go +++ b/src/internal/kopia/base_finder.go @@ -237,6 +237,10 @@ func (b *baseFinder) findBasesInSet( Reasons: []identity.Reasoner{reason}, } + case model.PreviewBackup: + // Preview backups are listed here for clarity though they use the same + // handling as the default case because they can't be used as bases. + fallthrough default: logger.Ctx(ictx).Infow( "skipping backup with empty or invalid type for incremental backups", diff --git a/src/internal/kopia/base_finder_test.go b/src/internal/kopia/base_finder_test.go index ed4d6f294..0b6f359d9 100644 --- a/src/internal/kopia/base_finder_test.go +++ b/src/internal/kopia/base_finder_test.go @@ -698,6 +698,28 @@ func (suite *BaseFinderUnitSuite) TestGetBases() { 0: testUser1Mail, }, }, + { + name: "Ignore Preview Base", + input: testUser1Mail, + // Ordered by time from newest to oldest. + data: []baseInfo{ + newBaseInfoBuilder(3, testT3, testUser1Mail...). + setBackupType(model.PreviewBackup). + build(), + newBaseInfoBuilder(2, testT2, testUser1Mail...). + setBackupType(model.AssistBackup). + build(), + newBaseInfoBuilder(1, testT1, testUser1Mail...). + setBackupType(model.MergeBackup). + build(), + }, + expectedMergeReasons: map[int][]identity.Reasoner{ + 2: testUser1Mail, + }, + expectedAssistReasons: map[int][]identity.Reasoner{ + 1: testUser1Mail, + }, + }, } for _, test := range table { diff --git a/src/internal/model/model.go b/src/internal/model/model.go index 0bb73d321..784888aa5 100644 --- a/src/internal/model/model.go +++ b/src/internal/model/model.go @@ -34,25 +34,39 @@ const ( // common tags for filtering const ( - ServiceTag = "service" + ServiceTag = "service" + // BackupTypeTag is the key used to store the resulting type of backup from a + // backup operation. The type of the backup is determined by a combination of + // input options and if errors were encountered during the backup. When making + // an incremental backup, previous backups' types are inspected to determine + // if they can be used as a base. + // + // The backup type associated with this key should only be used for + // determining if a backup is a valid base. Once the bases for a backup + // operation have been found, structs like kopia.BackupBases should be used to + // track the type of each base. BackupTypeTag = "backup-type" // AssistBackup denotes that this backup should only be used for kopia // assisted incrementals since it doesn't contain the complete set of data // being backed up. // - // This tag should only be used for finding backups during a - // manifest search. It shouldn't be used to differentiate between backups once - // the manifest search completes. + // See comment on BackupTypeTag for more information. AssistBackup = "assist-backup" // MergeBackup denotes that this backup can be used as a merge base during an // incremental backup. It contains a complete snapshot of the data in the // external service. Merge bases can also be used as assist bases during an // incremental backup or demoted to being only an assist base. // - // This tag should only be used for finding backups during a - // manifest search. It shouldn't be used to differentiate between backups once - // the manifest search completes. + // See comment on BackupTypeTag for more information. MergeBackup = "merge-backup" + // PreviewBackup denotes that this backup contains a subset of information for + // the protected resource. PreviewBackups are used to demonstrate value but + // are not safe to use as merge bases for incremental backups. It's possible + // they could be used as assist bases since the only difference from a regular + // backup is the amount of data they contain. + // + // See comment on BackupTypeTag for more information. + PreviewBackup = "preview-backup" ) // Valid returns true if the ModelType value fits within the const range.