-
Notifications
You must be signed in to change notification settings - Fork 152
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Tagging support #1146
Tagging support #1146
Conversation
Merge branch 'master' of https://github.com/kanisterio/kanister into vsphere_snapshot_tagging
pkg/blockstorage/vmware/vmware.go
Outdated
Cns: cnsCli, | ||
Gom: gom, | ||
TagManager: tm, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Expose the tagging client and hide your interface.
pkg/blockstorage/vmware/vmware.go
Outdated
Cns *cns.Client | ||
Gom *vslm.GlobalObjectManager | ||
Cns *cns.Client | ||
TagManager tagManager |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Expose the tagging service client too - the FcdProvider
type is used in CBT which could need it over time.
Hide your internal interface variable - it is really tacky anyway to expose a variable to an internal interface.
pkg/blockstorage/vmware/vmware.go
Outdated
// GetOrCreateCategory takes a category name and attempts to get or create the category | ||
// it returns the category ID. | ||
func (p *FcdProvider) GetOrCreateCategory(ctx context.Context, categoryName string) (string, error) { | ||
cat, err := p.TagManager.GetCategory(ctx, categoryName) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not use p.GetCategoryID
here?
pkg/blockstorage/vmware/vmware.go
Outdated
value string | ||
} | ||
|
||
func (t *snapshotTag) print() string { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This signature should be String() string
.
pkg/blockstorage/vmware/vmware.go
Outdated
} | ||
|
||
// setTagsSnapshot sets tags for a snapshot | ||
func (p *FcdProvider) setTagsSnapshot(ctx context.Context, snapshot *blockstorage.Snapshot, tags map[string]string) error { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
setSnapshotTags
pkg/blockstorage/vmware/vmware.go
Outdated
return nil | ||
} | ||
|
||
func (p *FcdProvider) deleteTagsSnapshot(ctx context.Context, snapshot *blockstorage.Snapshot) error { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
deleteSnapshotTags
pkg/blockstorage/vmware/vmware.go
Outdated
return fmt.Sprintf("%s:%s:%s:%s", volid, snapid, key, value) | ||
} | ||
|
||
func (p *FcdProvider) parseTag(tag string) (*snapshotTag, error) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be
func (t *snapshotTag) Parse(string) error
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See comments.
Change Overview
The following PR introduces the snapshot tagging for vsphere
Pull request type
Please check the type of change your PR introduces:
Issues
Test Plan