From 5a79e70d79f65d034546470e50dfc484278b9f97 Mon Sep 17 00:00:00 2001 From: Tiger Kaovilai Date: Mon, 6 Jan 2025 17:16:12 +0700 Subject: [PATCH 01/11] Configurable Kopia Maintenance Interval Signed-off-by: Tiger Kaovilai comment update Signed-off-by: Tiger Kaovilai comment Signed-off-by: Tiger Kaovilai --- changelogs/unreleased/8581-kaovilai | 1 + pkg/repository/udmrepo/kopialib/lib_repo.go | 23 +++++++++++++++ pkg/repository/udmrepo/repo_options.go | 12 ++++++++ .../docs/main/repository-maintenance.md | 28 ++++++++++++++++++- 4 files changed, 63 insertions(+), 1 deletion(-) create mode 100644 changelogs/unreleased/8581-kaovilai diff --git a/changelogs/unreleased/8581-kaovilai b/changelogs/unreleased/8581-kaovilai new file mode 100644 index 000000000..6e7ab78df --- /dev/null +++ b/changelogs/unreleased/8581-kaovilai @@ -0,0 +1 @@ +Configurable Kopia Maintenance Interval. backup-repository-configmap adds an option for configurable`fullMaintenanceInterval` where fastGC (12 hours), and eagerGC (6 hours) allowing for faster removal of deleted velero backups from kopia repo. diff --git a/pkg/repository/udmrepo/kopialib/lib_repo.go b/pkg/repository/udmrepo/kopialib/lib_repo.go index d4e1f8813..7b5016afc 100644 --- a/pkg/repository/udmrepo/kopialib/lib_repo.go +++ b/pkg/repository/udmrepo/kopialib/lib_repo.go @@ -600,6 +600,29 @@ func writeInitParameters(ctx context.Context, repoOption udmrepo.RepoOptions, lo logger.Infof("Quick maintenance interval change from %v to %v", p.QuickCycle.Interval, overwriteQuickMaintainInterval) p.QuickCycle.Interval = overwriteQuickMaintainInterval } + // the repoOption.StorageOptions are set via + // udmrepo.WithStoreOptions -> udmrepo.GetStoreOptions (interface) + // -> pkg/repository/provider.GetStoreOptions(param interface{}) -> pkg/repository/provider.getStorageVariables(..., backupRepoConfig) + // where backupRepoConfig comes from param.(RepoParam).BackupRepo.Spec.RepositoryConfig map[string]string + // where RepositoryConfig comes from pkg/controller/getBackupRepositoryConfig(...) + // where it gets a configMap name from pkg/cmd/server/config/Config.BackupRepoConfig + // which gets set via velero server flag `backup-repository-configmap` "The name of ConfigMap containing backup repository configurations." + // and data stored as json under ConfigMap.Data[repoType] where repoType is BackupRepository.Spec.RepositoryType: either kopia or restic + // repoOption.StorageOptions[udmrepo.StoreOptionKeyFullMaintenanceInterval] would for example look like + // configMapName.data.kopia: {"fullMaintenanceInterval": "eagerGC"} + fullMaintIntervalOption := udmrepo.FullMaintenanceIntervalOptions(repoOption.StorageOptions[udmrepo.StoreOptionKeyFullMaintenanceInterval]) + if fullMaintIntervalOption == udmrepo.FastGC { + logger.Infof("Full maintenance interval change from %v to %v", p.FullCycle.Interval, udmrepo.FastGCInterval) + p.FullCycle.Interval = udmrepo.FastGCInterval + } + if fullMaintIntervalOption == udmrepo.EagerGC { + logger.Infof("Full maintenance interval change from %v to %v", p.FullCycle.Interval, udmrepo.EagerGCInterval) + p.FullCycle.Interval = udmrepo.EagerGCInterval + } + if fullMaintIntervalOption == udmrepo.NormalGC { + logger.Infof("Full maintenance interval change from %v to %v", p.FullCycle.Interval, udmrepo.NormalGCInterval) + p.FullCycle.Interval = udmrepo.NormalGCInterval + } p.Owner = r.ClientOptions().UsernameAtHost() diff --git a/pkg/repository/udmrepo/repo_options.go b/pkg/repository/udmrepo/repo_options.go index ad4c597f9..efddfdcd1 100644 --- a/pkg/repository/udmrepo/repo_options.go +++ b/pkg/repository/udmrepo/repo_options.go @@ -20,6 +20,7 @@ import ( "os" "path/filepath" "strings" + "time" ) const ( @@ -70,8 +71,19 @@ const ( ThrottleOptionListOps = "listOPS" ThrottleOptionUploadBytes = "uploadBytes" ThrottleOptionDownloadBytes = "downloadBytes" + // FullMaintenanceInterval will overwrite kopia maintenance interval + // options are fastGC for 12 hours, eagerGC for 6 hours, normalGC for 24 hours + StoreOptionKeyFullMaintenanceInterval = "fullMaintenanceInterval" + FastGC FullMaintenanceIntervalOptions = "fastGC" + FastGCInterval time.Duration = 12 * time.Hour + EagerGC FullMaintenanceIntervalOptions = "eagerGC" + EagerGCInterval time.Duration = 6 * time.Hour + NormalGC FullMaintenanceIntervalOptions = "normalGC" + NormalGCInterval time.Duration = 24 * time.Hour ) +type FullMaintenanceIntervalOptions string + const ( defaultUsername = "default" defaultDomain = "default" diff --git a/site/content/docs/main/repository-maintenance.md b/site/content/docs/main/repository-maintenance.md index 8c712a9d7..21cff8d00 100644 --- a/site/content/docs/main/repository-maintenance.md +++ b/site/content/docs/main/repository-maintenance.md @@ -130,8 +130,34 @@ velero install --default-repo-maintain-frequency ``` For Kopia the default maintenance frequency is 1 hour, and Restic is 7 * 24 hours. +### Full Maintenance Interval customization +The full maintenance interval defaults to kopia defaults of 24 hours. Velero provide three override options under `fullMaintenanceInterval` configuration using `backup-repository-configmap` ConfigMap provided to velero install commands allowing for faster removal of deleted velero backups from kopia repo. +- normalGC: 24 hours +- fastGC: 12 hours +- eagerGC: 6 hours + +Example of the `backup-repository-configmap` ConfigMap for the above scenario is as below: +```yaml +apiVersion: v1 +kind: ConfigMap +metadata: + name: + namespace: velero +data: + : | + { + "fullMaintenanceInterval": fastGC + } + : | + { + "fullMaintenanceInterval": normalGC + } +``` + +Per kopia [Maintenance Safety](https://kopia.io/docs/advanced/maintenance/#maintenance-safety), it is expected that velero backup deletion will not result in immediate kopia repository data removal. Reducing full maintenance interval using above options should help reduce time taken to remove blobs not in use. + ### Others Maintenance jobs will inherit the labels, annotations, toleration, nodeSelector, service account, image, environment variables, cloud-credentials etc. from Velero deployment. [1]: velero-install.md#usage -[2]: node-agent-concurrency.md \ No newline at end of file +[2]: node-agent-concurrency.md From c153651044498b5a70cdbb1bf4eaa4d4505d2c6c Mon Sep 17 00:00:00 2001 From: Tiger Kaovilai Date: Wed, 5 Feb 2025 10:03:28 -0500 Subject: [PATCH 02/11] Pass all backupRepoConfig keys to storageVariables, and thus RepoOptions. Signed-off-by: Tiger Kaovilai --- pkg/repository/provider/unified_repo.go | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/pkg/repository/provider/unified_repo.go b/pkg/repository/provider/unified_repo.go index bdd909f20..c1ab01321 100644 --- a/pkg/repository/provider/unified_repo.go +++ b/pkg/repository/provider/unified_repo.go @@ -576,9 +576,13 @@ func getStorageVariables(backupLocation *velerov1api.BackupStorageLocation, repo result[udmrepo.StoreOptionOssRegion] = strings.Trim(region, "/") result[udmrepo.StoreOptionFsPath] = config["fspath"] - if backupRepoConfig != nil { - if v, found := backupRepoConfig[udmrepo.StoreOptionCacheLimit]; found { - result[udmrepo.StoreOptionCacheLimit] = v + // Write all backupRepoConfig to results if not exists, otherwise error on conflict + for k, v := range backupRepoConfig { // if nil, this would be skipped + if _, ok := result[k]; !ok { + // TODO: validation of allowed values for each key? + result[k] = v + } else { + return result, errors.Errorf("backupRepoConfig contains key %s that would override storage variables", k) } } From 3bb39d9331587664124d781122077019dde53f00 Mon Sep 17 00:00:00 2001 From: Tiger Kaovilai Date: Tue, 18 Feb 2025 15:36:14 -0600 Subject: [PATCH 03/11] Address https://github.com/vmware-tanzu/velero/pull/8581#pullrequestreview-2622443771 Signed-off-by: Tiger Kaovilai --- pkg/repository/provider/unified_repo.go | 23 ++++++++++++++++------- 1 file changed, 16 insertions(+), 7 deletions(-) diff --git a/pkg/repository/provider/unified_repo.go b/pkg/repository/provider/unified_repo.go index c1ab01321..dd9890485 100644 --- a/pkg/repository/provider/unified_repo.go +++ b/pkg/repository/provider/unified_repo.go @@ -506,6 +506,10 @@ func getStorageCredentials(backupLocation *velerov1api.BackupStorageLocation, cr return result, nil } +// Translates user specified options (backupRepoConfig) to internal parameters +// so we would accept only the options that are well defined in the internal system. +// Users' inputs should not be treated as safe any time. +// We remove the unnecessary parameters and keep the modules/logics below safe func getStorageVariables(backupLocation *velerov1api.BackupStorageLocation, repoBackend string, repoName string, backupRepoConfig map[string]string) (map[string]string, error) { result := make(map[string]string) @@ -576,14 +580,19 @@ func getStorageVariables(backupLocation *velerov1api.BackupStorageLocation, repo result[udmrepo.StoreOptionOssRegion] = strings.Trim(region, "/") result[udmrepo.StoreOptionFsPath] = config["fspath"] - // Write all backupRepoConfig to results if not exists, otherwise error on conflict - for k, v := range backupRepoConfig { // if nil, this would be skipped - if _, ok := result[k]; !ok { - // TODO: validation of allowed values for each key? - result[k] = v - } else { - return result, errors.Errorf("backupRepoConfig contains key %s that would override storage variables", k) + // We remove the unnecessary parameters and keep the modules/logics below safe + if backupRepoConfig != nil { + // range of valid params to keep, everything else will be discarded. + validParams := []string{ + udmrepo.StoreOptionCacheLimit, + udmrepo.StoreOptionKeyFullMaintenanceInterval, } + for _, param := range validParams { + if v, found := backupRepoConfig[param]; found { + result[param] = v + } + } + } return result, nil From 21ae1cbe820457750717a6d6724e59fdb48bfb99 Mon Sep 17 00:00:00 2001 From: Tiger Kaovilai Date: Tue, 18 Feb 2025 15:45:00 -0600 Subject: [PATCH 04/11] Address https://github.com/vmware-tanzu/velero/pull/8581#pullrequestreview-2622445640 Signed-off-by: Tiger Kaovilai --- pkg/repository/udmrepo/kopialib/lib_repo.go | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/pkg/repository/udmrepo/kopialib/lib_repo.go b/pkg/repository/udmrepo/kopialib/lib_repo.go index 7b5016afc..7552eb6ae 100644 --- a/pkg/repository/udmrepo/kopialib/lib_repo.go +++ b/pkg/repository/udmrepo/kopialib/lib_repo.go @@ -611,18 +611,18 @@ func writeInitParameters(ctx context.Context, repoOption udmrepo.RepoOptions, lo // repoOption.StorageOptions[udmrepo.StoreOptionKeyFullMaintenanceInterval] would for example look like // configMapName.data.kopia: {"fullMaintenanceInterval": "eagerGC"} fullMaintIntervalOption := udmrepo.FullMaintenanceIntervalOptions(repoOption.StorageOptions[udmrepo.StoreOptionKeyFullMaintenanceInterval]) - if fullMaintIntervalOption == udmrepo.FastGC { - logger.Infof("Full maintenance interval change from %v to %v", p.FullCycle.Interval, udmrepo.FastGCInterval) + priorMaintInterval := p.FullCycle.Interval + switch fullMaintIntervalOption { + case udmrepo.FastGC: p.FullCycle.Interval = udmrepo.FastGCInterval - } - if fullMaintIntervalOption == udmrepo.EagerGC { - logger.Infof("Full maintenance interval change from %v to %v", p.FullCycle.Interval, udmrepo.EagerGCInterval) + case udmrepo.EagerGC: p.FullCycle.Interval = udmrepo.EagerGCInterval - } - if fullMaintIntervalOption == udmrepo.NormalGC { - logger.Infof("Full maintenance interval change from %v to %v", p.FullCycle.Interval, udmrepo.NormalGCInterval) + case udmrepo.NormalGC: p.FullCycle.Interval = udmrepo.NormalGCInterval + default: + return errors.Errorf("invalid full maintenance interval option %s", fullMaintIntervalOption) } + logger.Infof("Full maintenance interval change from %v to %v", priorMaintInterval, p.FullCycle.Interval) p.Owner = r.ClientOptions().UsernameAtHost() From beb392e0db6dbdfc406e42ee59225a2f0169ed1d Mon Sep 17 00:00:00 2001 From: Tiger Kaovilai Date: Tue, 18 Feb 2025 15:49:46 -0600 Subject: [PATCH 05/11] doc updates Signed-off-by: Tiger Kaovilai --- .../main/backup-repository-configuration.md | 29 ++++++++++++++++++- .../docs/main/repository-maintenance.md | 28 ++---------------- 2 files changed, 31 insertions(+), 26 deletions(-) diff --git a/site/content/docs/main/backup-repository-configuration.md b/site/content/docs/main/backup-repository-configuration.md index 46301c54e..7eb0e5891 100644 --- a/site/content/docs/main/backup-repository-configuration.md +++ b/site/content/docs/main/backup-repository-configuration.md @@ -49,6 +49,33 @@ Below is the supported configurations by Velero and the specific backup reposito ***Kopia repository:*** `cacheLimitMB`: specifies the size limit(in MB) for the local data cache. The more data is cached locally, the less data may be downloaded from the backup storage, so the better performance may be achieved. Practically, you can specify any size that is smaller than the free space so that the disk space won't run out. This parameter is for repository connection, that is, you could change it before connecting to the repository. E.g., before a backup/restore/maintenance. +### Full Maintenance Interval customization +The full maintenance interval defaults to kopia defaults of 24 hours. Velero provide three override options under `fullMaintenanceInterval` configuration using `backup-repository-configmap` ConfigMap provided to velero install commands allowing for faster removal of deleted velero backups from kopia repo. +- normalGC: 24 hours +- fastGC: 12 hours +- eagerGC: 6 hours + +Example of the `backup-repository-configmap` ConfigMap for the above scenario is as below: +```yaml +apiVersion: v1 +kind: ConfigMap +metadata: + name: + namespace: velero +data: + : | + { + "fullMaintenanceInterval": fastGC + } + : | + { + "fullMaintenanceInterval": normalGC + } +``` + +Per kopia [Maintenance Safety](https://kopia.io/docs/advanced/maintenance/#maintenance-safety), it is expected that velero backup deletion will not result in immediate kopia repository data removal. Reducing full maintenance interval using above options should help reduce time taken to remove blobs not in use. + +On the other hand, the not-in-use data will be deleted permanently after the full maintenance, so shorter full maintenance intervals may weaken the data safety if they are used incorrectly. [1]: file-system-backup.md -[2]: csi-snapshot-data-movement.md \ No newline at end of file +[2]: csi-snapshot-data-movement.md diff --git a/site/content/docs/main/repository-maintenance.md b/site/content/docs/main/repository-maintenance.md index 21cff8d00..7a0bb53c6 100644 --- a/site/content/docs/main/repository-maintenance.md +++ b/site/content/docs/main/repository-maintenance.md @@ -51,7 +51,7 @@ For example, the following BackupRepository's key should be `test-default-kopia` You can still customize the maintenance job resource requests and limit when using the [velero install][1] CLI command. -The `LoadAffinity` structure is reused from design [node-agent affinity configuration](2). +The `LoadAffinity` structure is reused from design [node-agent affinity configuration][2]. ### Affinity Example It's possible that the users want to choose nodes that match condition A or condition B to run the job. @@ -131,33 +131,11 @@ velero install --default-repo-maintain-frequency For Kopia the default maintenance frequency is 1 hour, and Restic is 7 * 24 hours. ### Full Maintenance Interval customization -The full maintenance interval defaults to kopia defaults of 24 hours. Velero provide three override options under `fullMaintenanceInterval` configuration using `backup-repository-configmap` ConfigMap provided to velero install commands allowing for faster removal of deleted velero backups from kopia repo. -- normalGC: 24 hours -- fastGC: 12 hours -- eagerGC: 6 hours - -Example of the `backup-repository-configmap` ConfigMap for the above scenario is as below: -```yaml -apiVersion: v1 -kind: ConfigMap -metadata: - name: - namespace: velero -data: - : | - { - "fullMaintenanceInterval": fastGC - } - : | - { - "fullMaintenanceInterval": normalGC - } -``` - -Per kopia [Maintenance Safety](https://kopia.io/docs/advanced/maintenance/#maintenance-safety), it is expected that velero backup deletion will not result in immediate kopia repository data removal. Reducing full maintenance interval using above options should help reduce time taken to remove blobs not in use. +See [backup repository configuration][3] ### Others Maintenance jobs will inherit the labels, annotations, toleration, nodeSelector, service account, image, environment variables, cloud-credentials etc. from Velero deployment. [1]: velero-install.md#usage [2]: node-agent-concurrency.md +[3]: backup-repository-configuration.md#full-maintenance-interval-customization From 271ff180e9e04f35db53133d00047d9307f7a95d Mon Sep 17 00:00:00 2001 From: Tiger Kaovilai Date: Tue, 18 Feb 2025 16:07:21 -0600 Subject: [PATCH 06/11] lint Signed-off-by: Tiger Kaovilai --- pkg/repository/provider/unified_repo.go | 1 - 1 file changed, 1 deletion(-) diff --git a/pkg/repository/provider/unified_repo.go b/pkg/repository/provider/unified_repo.go index dd9890485..251845dac 100644 --- a/pkg/repository/provider/unified_repo.go +++ b/pkg/repository/provider/unified_repo.go @@ -592,7 +592,6 @@ func getStorageVariables(backupLocation *velerov1api.BackupStorageLocation, repo result[param] = v } } - } return result, nil From f93eed56caa25c3e8294c433b4a3bc15e18ce261 Mon Sep 17 00:00:00 2001 From: Tiger Kaovilai Date: Wed, 19 Feb 2025 10:17:54 -0600 Subject: [PATCH 07/11] doc update, move under kopia repo header Signed-off-by: Tiger Kaovilai --- .../main/backup-repository-configuration.md | 24 +++---------------- 1 file changed, 3 insertions(+), 21 deletions(-) diff --git a/site/content/docs/main/backup-repository-configuration.md b/site/content/docs/main/backup-repository-configuration.md index 7eb0e5891..fd6cf0b78 100644 --- a/site/content/docs/main/backup-repository-configuration.md +++ b/site/content/docs/main/backup-repository-configuration.md @@ -30,7 +30,8 @@ metadata: data: : | { - "cacheLimitMB": 2048 + "cacheLimitMB": 2048, + "fullMaintenanceInterval": "fastGC" } : | { @@ -49,30 +50,11 @@ Below is the supported configurations by Velero and the specific backup reposito ***Kopia repository:*** `cacheLimitMB`: specifies the size limit(in MB) for the local data cache. The more data is cached locally, the less data may be downloaded from the backup storage, so the better performance may be achieved. Practically, you can specify any size that is smaller than the free space so that the disk space won't run out. This parameter is for repository connection, that is, you could change it before connecting to the repository. E.g., before a backup/restore/maintenance. -### Full Maintenance Interval customization -The full maintenance interval defaults to kopia defaults of 24 hours. Velero provide three override options under `fullMaintenanceInterval` configuration using `backup-repository-configmap` ConfigMap provided to velero install commands allowing for faster removal of deleted velero backups from kopia repo. +`fullMaintenanceInterval`: The full maintenance interval defaults to kopia defaults of 24 hours. Override options below allows for faster removal of deleted velero backups from kopia repo. - normalGC: 24 hours - fastGC: 12 hours - eagerGC: 6 hours -Example of the `backup-repository-configmap` ConfigMap for the above scenario is as below: -```yaml -apiVersion: v1 -kind: ConfigMap -metadata: - name: - namespace: velero -data: - : | - { - "fullMaintenanceInterval": fastGC - } - : | - { - "fullMaintenanceInterval": normalGC - } -``` - Per kopia [Maintenance Safety](https://kopia.io/docs/advanced/maintenance/#maintenance-safety), it is expected that velero backup deletion will not result in immediate kopia repository data removal. Reducing full maintenance interval using above options should help reduce time taken to remove blobs not in use. On the other hand, the not-in-use data will be deleted permanently after the full maintenance, so shorter full maintenance intervals may weaken the data safety if they are used incorrectly. From 1b7d9014a5909cd2c9823e5fabd814ce338a43f6 Mon Sep 17 00:00:00 2001 From: Tiger Kaovilai Date: Wed, 19 Feb 2025 10:19:03 -0600 Subject: [PATCH 08/11] add to unmarshal test Signed-off-by: Tiger Kaovilai --- pkg/controller/backup_repository_controller_test.go | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/pkg/controller/backup_repository_controller_test.go b/pkg/controller/backup_repository_controller_test.go index e2287f72e..f26a221b2 100644 --- a/pkg/controller/backup_repository_controller_test.go +++ b/pkg/controller/backup_repository_controller_test.go @@ -694,7 +694,7 @@ func TestGetBackupRepositoryConfig(t *testing.T) { Namespace: velerov1api.DefaultNamespace, }, Data: map[string]string{ - "fake-repo-type": "{\"cacheLimitMB\": 1000, \"enableCompression\": true}", + "fake-repo-type": "{\"cacheLimitMB\": 1000, \"enableCompression\": true, \"fullMaintenanceInterval\": \"fastGC\"}", "fake-repo-type-1": "{\"cacheLimitMB\": 1, \"enableCompression\": false}", }, } @@ -744,8 +744,9 @@ func TestGetBackupRepositoryConfig(t *testing.T) { configWithData, }, expectedResult: map[string]string{ - "cacheLimitMB": "1000", - "enableCompression": "true", + "cacheLimitMB": "1000", + "enableCompression": "true", + "fullMaintenanceInterval": "fastGC", }, }, } From 92617d07c57faef89eef936f52d8f62d99e0f2b4 Mon Sep 17 00:00:00 2001 From: Tiger Kaovilai Date: Wed, 19 Feb 2025 10:21:48 -0600 Subject: [PATCH 09/11] log only if not equal Signed-off-by: Tiger Kaovilai --- pkg/repository/udmrepo/kopialib/lib_repo.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/pkg/repository/udmrepo/kopialib/lib_repo.go b/pkg/repository/udmrepo/kopialib/lib_repo.go index 7552eb6ae..6225b1438 100644 --- a/pkg/repository/udmrepo/kopialib/lib_repo.go +++ b/pkg/repository/udmrepo/kopialib/lib_repo.go @@ -622,7 +622,9 @@ func writeInitParameters(ctx context.Context, repoOption udmrepo.RepoOptions, lo default: return errors.Errorf("invalid full maintenance interval option %s", fullMaintIntervalOption) } - logger.Infof("Full maintenance interval change from %v to %v", priorMaintInterval, p.FullCycle.Interval) + if priorMaintInterval != p.FullCycle.Interval { + logger.Infof("Full maintenance interval change from %v to %v", priorMaintInterval, p.FullCycle.Interval) + } p.Owner = r.ClientOptions().UsernameAtHost() From 3fb8c72b6cff48a7b0690d0bfab0be2361ca6815 Mon Sep 17 00:00:00 2001 From: Tiger Kaovilai Date: Thu, 20 Feb 2025 16:31:46 -0600 Subject: [PATCH 10/11] empty string case Signed-off-by: Tiger Kaovilai --- pkg/repository/udmrepo/kopialib/lib_repo.go | 1 + 1 file changed, 1 insertion(+) diff --git a/pkg/repository/udmrepo/kopialib/lib_repo.go b/pkg/repository/udmrepo/kopialib/lib_repo.go index 6225b1438..3937657aa 100644 --- a/pkg/repository/udmrepo/kopialib/lib_repo.go +++ b/pkg/repository/udmrepo/kopialib/lib_repo.go @@ -619,6 +619,7 @@ func writeInitParameters(ctx context.Context, repoOption udmrepo.RepoOptions, lo p.FullCycle.Interval = udmrepo.EagerGCInterval case udmrepo.NormalGC: p.FullCycle.Interval = udmrepo.NormalGCInterval + case "": // do nothing default: return errors.Errorf("invalid full maintenance interval option %s", fullMaintIntervalOption) } From 178b6e3db53129c818ce7c501af56199bb561ab2 Mon Sep 17 00:00:00 2001 From: Tiger Kaovilai Date: Fri, 21 Feb 2025 13:55:15 -0600 Subject: [PATCH 11/11] add more maintenance interval unit tests Signed-off-by: Tiger Kaovilai --- .../udmrepo/kopialib/lib_repo_test.go | 78 ++++++++++++++++++- 1 file changed, 76 insertions(+), 2 deletions(-) diff --git a/pkg/repository/udmrepo/kopialib/lib_repo_test.go b/pkg/repository/udmrepo/kopialib/lib_repo_test.go index a6382dbc8..acdf890b4 100644 --- a/pkg/repository/udmrepo/kopialib/lib_repo_test.go +++ b/pkg/repository/udmrepo/kopialib/lib_repo_test.go @@ -24,6 +24,7 @@ import ( "time" "github.com/kopia/kopia/repo" + "github.com/kopia/kopia/repo/maintenance" "github.com/kopia/kopia/repo/manifest" "github.com/kopia/kopia/repo/object" "github.com/pkg/errors" @@ -264,6 +265,9 @@ func TestMaintain(t *testing.T) { func TestWriteInitParameters(t *testing.T) { var directRpo *repomocks.DirectRepository + assertFullMaintIntervalEqual := func(expected, actual *maintenance.Params) bool { + return assert.Equal(t, expected.FullCycle.Interval, actual.FullCycle.Interval) + } testCases := []struct { name string repoOptions udmrepo.RepoOptions @@ -272,7 +276,11 @@ func TestWriteInitParameters(t *testing.T) { repoOpen func(context.Context, string, string, *repo.Options) (repo.Repository, error) newRepoWriterError error replaceManifestError error - expectedErr string + // expected replacemanifest params to be received by maintenance.SetParams, and therefore writeInitParameters + expectedReplaceManifestsParams *maintenance.Params + // allows for asserting only certain fields are set as expected + assertReplaceManifestsParams func(*maintenance.Params, *maintenance.Params) bool + expectedErr string }{ { name: "repo open fail, repo not exist", @@ -323,6 +331,61 @@ func TestWriteInitParameters(t *testing.T) { replaceManifestError: errors.New("fake-replace-manifest-error"), expectedErr: "error to init write repo parameters: error to set maintenance params: put manifest: fake-replace-manifest-error", }, + { + name: "repo with maintenance interval has expected params", + repoOptions: udmrepo.RepoOptions{ + ConfigFilePath: "/tmp", + StorageOptions: map[string]string{ + udmrepo.StoreOptionKeyFullMaintenanceInterval: string(udmrepo.FastGC), + }, + }, + repoOpen: func(context.Context, string, string, *repo.Options) (repo.Repository, error) { + return directRpo, nil + }, + returnRepo: new(repomocks.DirectRepository), + returnRepoWriter: new(repomocks.DirectRepositoryWriter), + expectedReplaceManifestsParams: &maintenance.Params{ + FullCycle: maintenance.CycleParams{ + Interval: udmrepo.FastGCInterval, + }, + }, + assertReplaceManifestsParams: assertFullMaintIntervalEqual, + }, + { + name: "repo with empty maintenance interval has expected params", + repoOptions: udmrepo.RepoOptions{ + ConfigFilePath: "/tmp", + StorageOptions: map[string]string{ + udmrepo.StoreOptionKeyFullMaintenanceInterval: string(""), + }, + }, + repoOpen: func(context.Context, string, string, *repo.Options) (repo.Repository, error) { + return directRpo, nil + }, + returnRepo: new(repomocks.DirectRepository), + returnRepoWriter: new(repomocks.DirectRepositoryWriter), + expectedReplaceManifestsParams: &maintenance.Params{ + FullCycle: maintenance.CycleParams{ + Interval: udmrepo.NormalGCInterval, + }, + }, + assertReplaceManifestsParams: assertFullMaintIntervalEqual, + }, + { + name: "repo with invalid maintenance interval has expected errors", + repoOptions: udmrepo.RepoOptions{ + ConfigFilePath: "/tmp", + StorageOptions: map[string]string{ + udmrepo.StoreOptionKeyFullMaintenanceInterval: string("foo"), + }, + }, + repoOpen: func(context.Context, string, string, *repo.Options) (repo.Repository, error) { + return directRpo, nil + }, + returnRepo: new(repomocks.DirectRepository), + returnRepoWriter: new(repomocks.DirectRepositoryWriter), + expectedErr: "error to init write repo parameters: invalid full maintenance interval option foo", + }, } for _, tc := range testCases { @@ -346,7 +409,13 @@ func TestWriteInitParameters(t *testing.T) { if tc.returnRepoWriter != nil { tc.returnRepoWriter.On("Close", mock.Anything).Return(nil) - tc.returnRepoWriter.On("ReplaceManifests", mock.Anything, mock.Anything, mock.Anything).Return(manifest.ID(""), tc.replaceManifestError) + if tc.replaceManifestError != nil { + tc.returnRepoWriter.On("ReplaceManifests", mock.Anything, mock.Anything, mock.Anything).Return(manifest.ID(""), tc.replaceManifestError) + } + if tc.expectedReplaceManifestsParams != nil { + tc.returnRepoWriter.On("ReplaceManifests", mock.AnythingOfType("context.backgroundCtx"), mock.AnythingOfType("map[string]string"), mock.AnythingOfType("*maintenance.Params")).Return(manifest.ID(""), nil) + tc.returnRepoWriter.On("Flush", mock.Anything).Return(nil) + } } err := writeInitParameters(ctx, tc.repoOptions, logger) @@ -356,6 +425,11 @@ func TestWriteInitParameters(t *testing.T) { } else { assert.EqualError(t, err, tc.expectedErr) } + if tc.expectedReplaceManifestsParams != nil { + actualReplaceManifestsParams, converted := tc.returnRepoWriter.Calls[0].Arguments.Get(2).(*maintenance.Params) + assert.True(t, converted) + tc.assertReplaceManifestsParams(tc.expectedReplaceManifestsParams, actualReplaceManifestsParams) + } }) } }