Refactor the Reason struct and how tags for lookups are generated (#2087)

## Description

Solidify the interface between BackupOp and KopiaWrapper by making Reason the de facto way to pass/generate tags for things. The Reason struct now includes a function to generate tags for that instance. KopiaWrapper also now hides the fact that it prefixes tags from other components

## Does this PR need a docs update or release note?

- [ ]  Yes, it's included
- [ ] 🕐 Yes, but in a later PR
- [x]  No 

## Type of change

- [ ] 🌻 Feature
- [ ] 🐛 Bugfix
- [ ] 🗺️ Documentation
- [ ] 🤖 Test
- [ ] 💻 CI/Deployment
- [x] 🧹 Tech Debt/Cleanup

## Issue(s)

* #1916 

## Test Plan

- [ ] 💪 Manual
- [x]  Unit test
- [x] 💚 E2E
This commit is contained in:
ashmrtn 2023-01-10 12:32:17 -08:00 committed by GitHub
parent 778a60774a
commit e88553d073
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
7 changed files with 101 additions and 51 deletions

View File

@ -32,6 +32,13 @@ type Reason struct {
Category path.CategoryType Category path.CategoryType
} }
func (r Reason) TagKeys() []string {
return []string{
r.ResourceOwner,
serviceCatString(r.Service, r.Category),
}
}
type ManifestEntry struct { type ManifestEntry struct {
*snapshot.Manifest *snapshot.Manifest
// Reason contains the ResourceOwners and Service/Categories that caused this // Reason contains the ResourceOwners and Service/Categories that caused this
@ -44,6 +51,13 @@ type ManifestEntry struct {
Reasons []Reason Reasons []Reason
} }
func (me ManifestEntry) GetTag(key string) (string, bool) {
k, _ := makeTagKV(key)
v, ok := me.Tags[k]
return v, ok
}
type snapshotManager interface { type snapshotManager interface {
FindManifests( FindManifests(
ctx context.Context, ctx context.Context,
@ -68,6 +82,10 @@ func MakeServiceCat(s path.ServiceType, c path.CategoryType) (string, ServiceCat
return serviceCatString(s, c), ServiceCat{s, c} return serviceCatString(s, c), ServiceCat{s, c}
} }
// TODO(ashmrtn): Remove in a future PR.
//
//nolint:unused
//lint:ignore U1000 will be removed in future PR.
func serviceCatTag(p path.Path) string { func serviceCatTag(p path.Path) string {
return serviceCatString(p.Service(), p.Category()) return serviceCatString(p.Service(), p.Category())
} }
@ -82,7 +100,7 @@ func serviceCatString(s path.ServiceType, c path.CategoryType) string {
// Returns the normalized Key plus a default value. If you're embedding a // Returns the normalized Key plus a default value. If you're embedding a
// key-only tag, the returned default value msut be used instead of an // key-only tag, the returned default value msut be used instead of an
// empty string. // empty string.
func MakeTagKV(k string) (string, string) { func makeTagKV(k string) (string, string) {
return userTagPrefix + k, defaultTagValue return userTagPrefix + k, defaultTagValue
} }
@ -101,12 +119,12 @@ func tagsFromStrings(oc *OwnersCats) map[string]string {
res := make(map[string]string, len(oc.ServiceCats)+len(oc.ResourceOwners)) res := make(map[string]string, len(oc.ServiceCats)+len(oc.ResourceOwners))
for k := range oc.ServiceCats { for k := range oc.ServiceCats {
tk, tv := MakeTagKV(k) tk, tv := makeTagKV(k)
res[tk] = tv res[tk] = tv
} }
for k := range oc.ResourceOwners { for k := range oc.ResourceOwners {
tk, tv := MakeTagKV(k) tk, tv := makeTagKV(k)
res[tk] = tv res[tk] = tv
} }
@ -195,14 +213,14 @@ func fetchPrevManifests(
reason Reason, reason Reason,
tags map[string]string, tags map[string]string,
) ([]*ManifestEntry, error) { ) ([]*ManifestEntry, error) {
tags = normalizeTagKVs(tags) allTags := map[string]string{}
serviceCatKey, _ := MakeServiceCat(reason.Service, reason.Category)
allTags := normalizeTagKVs(map[string]string{ for _, k := range reason.TagKeys() {
serviceCatKey: "", allTags[k] = ""
reason.ResourceOwner: "", }
})
maps.Copy(allTags, tags) maps.Copy(allTags, tags)
allTags = normalizeTagKVs(allTags)
metas, err := sm.FindManifests(ctx, allTags) metas, err := sm.FindManifests(ctx, allTags)
if err != nil { if err != nil {
@ -335,7 +353,7 @@ func normalizeTagKVs(tags map[string]string) map[string]string {
t2 := make(map[string]string, len(tags)) t2 := make(map[string]string, len(tags))
for k, v := range tags { for k, v := range tags {
mk, mv := MakeTagKV(k) mk, mv := makeTagKV(k)
if len(v) == 0 { if len(v) == 0 {
v = mv v = mv

View File

@ -29,16 +29,9 @@ var (
testID2 = manifest.ID("snap2") testID2 = manifest.ID("snap2")
testID3 = manifest.ID("snap3") testID3 = manifest.ID("snap3")
testMail = path.ExchangeService.String() + path.EmailCategory.String() testMail = path.ExchangeService.String() + path.EmailCategory.String()
testMailServiceCat = ServiceCat{ testEvents = path.ExchangeService.String() + path.EventsCategory.String()
Service: path.ExchangeService,
Category: path.EmailCategory,
}
testEvents = path.ExchangeService.String() + path.EventsCategory.String()
testEventsServiceCat = ServiceCat{
Service: path.ExchangeService,
Category: path.EventsCategory,
}
testUser1 = "user1" testUser1 = "user1"
testUser2 = "user2" testUser2 = "user2"
testUser3 = "user3" testUser3 = "user3"
@ -115,7 +108,7 @@ func newManifestInfo(
structTags := make(map[string]struct{}, len(tags)) structTags := make(map[string]struct{}, len(tags))
for _, t := range tags { for _, t := range tags {
tk, _ := MakeTagKV(t) tk, _ := makeTagKV(t)
structTags[tk] = struct{}{} structTags[tk] = struct{}{}
} }

View File

@ -230,7 +230,7 @@ func (w Wrapper) makeSnapshotWithRoot(
man.Tags = map[string]string{} man.Tags = map[string]string{}
for k, v := range addlTags { for k, v := range addlTags {
mk, mv := MakeTagKV(k) mk, mv := makeTagKV(k)
if len(v) == 0 { if len(v) == 0 {
v = mv v = mv

View File

@ -212,11 +212,25 @@ func (suite *KopiaIntegrationSuite) TestBackupCollections() {
tags := map[string]string{ tags := map[string]string{
"fnords": "smarf", "fnords": "smarf",
"brunhilda": "", "brunhilda": "",
}
suite.testPath1.ResourceOwner(): "", reasons := []Reason{
suite.testPath2.ResourceOwner(): "", {
serviceCatTag(suite.testPath1): "", ResourceOwner: suite.testPath1.ResourceOwner(),
serviceCatTag(suite.testPath2): "", Service: suite.testPath1.Service(),
Category: suite.testPath1.Category(),
},
{
ResourceOwner: suite.testPath2.ResourceOwner(),
Service: suite.testPath2.Service(),
Category: suite.testPath2.Category(),
},
}
for _, r := range reasons {
for _, k := range r.TagKeys() {
tags[k] = ""
}
} }
expectedTags := map[string]string{} expectedTags := map[string]string{}
@ -305,9 +319,15 @@ func (suite *KopiaIntegrationSuite) TestRestoreAfterCompressionChange() {
w := &Wrapper{k} w := &Wrapper{k}
tags := map[string]string{ tags := map[string]string{}
testUser: "", reason := Reason{
serviceCatString(path.ExchangeService, path.EmailCategory): "", ResourceOwner: testUser,
Service: path.ExchangeService,
Category: path.EmailCategory,
}
for _, k := range reason.TagKeys() {
tags[k] = ""
} }
dc1 := mockconnector.NewMockExchangeCollection(suite.testPath1, 1) dc1 := mockconnector.NewMockExchangeCollection(suite.testPath1, 1)
@ -353,9 +373,15 @@ func (suite *KopiaIntegrationSuite) TestRestoreAfterCompressionChange() {
func (suite *KopiaIntegrationSuite) TestBackupCollections_ReaderError() { func (suite *KopiaIntegrationSuite) TestBackupCollections_ReaderError() {
t := suite.T() t := suite.T()
tags := map[string]string{ tags := map[string]string{}
testUser: "", reason := Reason{
serviceCatString(path.ExchangeService, path.EmailCategory): "", ResourceOwner: testUser,
Service: path.ExchangeService,
Category: path.EmailCategory,
}
for _, k := range reason.TagKeys() {
tags[k] = ""
} }
collections := []data.Collection{ collections := []data.Collection{
@ -586,9 +612,15 @@ func (suite *KopiaSimpleRepoIntegrationSuite) SetupTest() {
collections = append(collections, collection) collections = append(collections, collection)
} }
tags := map[string]string{ tags := map[string]string{}
testUser: "", reason := Reason{
serviceCatString(path.ExchangeService, path.EmailCategory): "", ResourceOwner: testUser,
Service: path.ExchangeService,
Category: path.EmailCategory,
}
for _, k := range reason.TagKeys() {
tags[k] = ""
} }
stats, deets, _, err := suite.w.BackupCollections( stats, deets, _, err := suite.w.BackupCollections(

View File

@ -344,10 +344,8 @@ func produceManifestsAndMetadata(
continue continue
} }
tk, _ := kopia.MakeTagKV(kopia.TagBackupID) bID, ok := man.GetTag(kopia.TagBackupID)
if !ok {
bID := man.Tags[tk]
if len(bID) == 0 {
return nil, nil, false, errors.New("snapshot manifest missing backup ID") return nil, nil, false, errors.New("snapshot manifest missing backup ID")
} }
@ -502,12 +500,9 @@ func consumeBackupDataCollections(
} }
for _, reason := range reasons { for _, reason := range reasons {
tags[reason.ResourceOwner] = "" for _, k := range reason.TagKeys() {
tags[k] = ""
// TODO(ashmrtn): Create a separate helper function to go from service/cat }
// to a tag.
serviceCat, _ := kopia.MakeServiceCat(reason.Service, reason.Category)
tags[serviceCat] = ""
} }
bases := make([]kopia.IncrementalBase, 0, len(mans)) bases := make([]kopia.IncrementalBase, 0, len(mans))
@ -568,8 +563,10 @@ func mergeDetails(
continue continue
} }
k, _ := kopia.MakeTagKV(kopia.TagBackupID) bID, ok := man.GetTag(kopia.TagBackupID)
bID := man.Tags[k] if !ok {
return errors.Errorf("no backup ID in snapshot manifest with ID %s", man.ID)
}
_, baseDeets, err := getBackupAndDetailsFromID( _, baseDeets, err := getBackupAndDetailsFromID(
ctx, ctx,

View File

@ -191,8 +191,12 @@ func checkBackupIsInManifests(
require.NoError(t, err) require.NoError(t, err)
for _, man := range mans { for _, man := range mans {
tk, _ := kopia.MakeTagKV(kopia.TagBackupID) bID, ok := man.GetTag(kopia.TagBackupID)
if man.Tags[tk] == string(bo.Results.BackupID) { if !assert.Truef(t, ok, "snapshot manifest %s missing backup ID tag", man.ID) {
continue
}
if bID == string(bo.Results.BackupID) {
found = true found = true
break break
} }

View File

@ -276,14 +276,20 @@ func makeDetailsEntry(
return res return res
} }
// TODO(ashmrtn): This should belong to some code that lives in the kopia
// package that is only compiled when running tests.
func makeKopiaTagKey(k string) string {
return "tag:" + k
}
func makeManifest(t *testing.T, backupID model.StableID, incompleteReason string) *snapshot.Manifest { func makeManifest(t *testing.T, backupID model.StableID, incompleteReason string) *snapshot.Manifest {
t.Helper() t.Helper()
backupIDTagKey, _ := kopia.MakeTagKV(kopia.TagBackupID) tagKey := makeKopiaTagKey(kopia.TagBackupID)
return &snapshot.Manifest{ return &snapshot.Manifest{
Tags: map[string]string{ Tags: map[string]string{
backupIDTagKey: string(backupID), tagKey: string(backupID),
}, },
IncompleteReason: incompleteReason, IncompleteReason: incompleteReason,
} }