storage: support GCS backup and restore#102
Conversation
|
|
||
| func newGCSStorage(gcs *backup.GCS) (*gcsStorage, error) { | ||
| var clientOps []option.ClientOption | ||
| clientOps = append(clientOps, option.WithCredentialsJSON([]byte(gcs.GetCredentialsBlob()))) |
There was a problem hiding this comment.
It looks like we are being forced into one particular way of doing credentials here. I am actually not sure what this is doing. But on GCE or GKE with the workload identity feature enabled the SDK will already automatically find credentials in the metadata.
There was a problem hiding this comment.
Since br is not sure that all the TiKV instances are deployed on GCE or GKE either, the simplest way is getting a credentials JSON and sending it to other TiKV instances.
Yes, I think it's a cool thing to use service account from environment on GCE or GKE. Maybe in the future and we have a idea to handle the credentials for TiKV instances.
There was a problem hiding this comment.
It happens automatically if you don't force one particular method like this. So you don't have to do anything to support it (other than not hard-code a particular auth technique).
There was a problem hiding this comment.
If BR is not deployed on google cloud but on someone's IDC, we need this credentials file.
There was a problem hiding this comment.
Right, so I think all you need here is an if statement, something like:
creds := []byte(gcs.GetCredentialsBlob()
if creds != "" {
clientOps = append(clientOps, option.WithCredentialsJSON(creds))
}There was a problem hiding this comment.
I'll handle this on #106 because we need to deal with --send-credentials-to-tikv option here.
Codecov Report
@@ Coverage Diff @@
## master #102 +/- ##
==========================================
- Coverage 70.34% 69.97% -0.38%
==========================================
Files 30 31 +1
Lines 2799 2911 +112
==========================================
+ Hits 1969 2037 +68
- Misses 550 582 +32
- Partials 280 292 +12
Continue to review full report at Codecov.
|
|
Please fix the confliction for merging. |
rebased. |
pkg/storage/gcs.go
Outdated
| "Specify the GCS storage class for objects. "+ | ||
| "If it is not set, objects uploaded are followed by the default storage class of the bucket. "+ | ||
| "See https://cloud.google.com/storage/docs/storage-classes for valid values.") |
There was a problem hiding this comment.
It would be better if you can wrap them within 80 chars.
part of #89
GCS Support
store BR metadata to GCS