snapshot reporting #4138
snapshot reporting #4138
Conversation
| return errors.Wrap(err, "failed to create velero clientset") | ||
| } | ||
|
|
||
| bsl, err := snapshot.FindBackupStoreLocation(context.TODO(), clientset, veleroClient, deployOptions.Namespace) |
There was a problem hiding this comment.
Refactored FindBackupStoreLocation to accept a velero client interface so we can unit test it. A good chunk of the diff is related to this. We could use a similar pattern to increase unit test coverage for snapshots overall.
| headers["X-Replicated-SnapshotProvider"] = reportingInfo.SnapshotProvider | ||
| headers["X-Replicated-SnapshotFullSchedule"] = reportingInfo.SnapshotFullSchedule | ||
| headers["X-Replicated-SnapshotFullTTL"] = reportingInfo.SnapshotFullTTL | ||
| headers["X-Replicated-SnapshotPartialSchedule"] = reportingInfo.SnapshotPartialSchedule | ||
| headers["X-Replicated-SnapshotPartialTTL"] = reportingInfo.SnapshotPartialTTL |
There was a problem hiding this comment.
Generally followed the pattern we have for GitOps reporting, but do we want to omit these headers if, for example, the provider is empty?
There was a problem hiding this comment.
I don't think it matters in this case.
pkg/reporting/app.go
Outdated
| func getSnapshotReport(kotsStore store.Store, clientset kubernetes.Interface, veleroClient veleroclientv1.VeleroV1Interface, appID string, clusterID string) (*SnapshotReport, error) { | ||
| report := &SnapshotReport{} | ||
|
|
||
| bsl, err := snapshot.FindBackupStoreLocation(context.TODO(), clientset, veleroClient, util.PodNamespace) |
There was a problem hiding this comment.
How about calling this outside of the getSnapshotReport function and pass the BSL as a parameter to avoid refactoring lots of the code?
There was a problem hiding this comment.
bsl as a parameter to getSnapshotReport makes sense. it still seems like a worthwhile refactor to open up unit testing of snapshot code paths, but i can revert that if needed
What this PR does / why we need it:
This PR adds reporting for snapshot provider, schedule, and retention policy (ttl)
Which issue(s) this PR fixes:
https://app.shortcut.com/replicated/story/89885/instrument-and-report-back-whether-the-end-user-is-using-snapshots
Special notes for your reviewer:
Steps to reproduce
Does this PR introduce a user-facing change?
Does this PR require documentation?
NONE