Refactor bucket configuration flags for Object Store #500

jojohappy · 2018-08-31T09:58:16Z

Changes

This PR try to make some unified bucket flags for all provider. User should set the --provider.type to represent which provider they use. And all flags will be reused for all provider now. This aim is to reduce the number of flags when new provider appended if it can reuse the flags have been exists.

Verification

User can run the thanos bucket ls to verify this PR

jojohappy · 2018-08-31T09:58:40Z

@Bplotka Sorry for later. I'm so busy with my work this two weeks. But it is prepare to review now.

vglafirov · 2018-08-31T13:39:53Z

docs/components/bucket.md

+                              no trace will be sent periodically, unless forced
+                              by baggage item. See `pkg/tracing/tracing.go` for
+                              details.
+      --provider.type=<provider>  


All these parameters are S3 specific. :( I think best approach would be to specify provider config in external JSON file where you can specify type and all additional provider options, like Premium/Standard storage, access policies etc.. Each provider can have arbitrary options to pass there.

Agree... We need some way to not exponentially increase number of flags, but also keep documentation clear, how to configure obj stores. I am generally against moving to a config file, instead of flags. This is basically harder to operate. There is no clear winner here..

Some quick idea:

Put to flags only things that are common and add objstore.extra for file content and allow different providers parse it dynamically to internal structs?

Anything smarter? @brancz you are after inspiring Gophercon, maybe you have some idea (:

I have mixed feelings for anything that goes into the provider direction but still baked into the binary. It's going to be a pain no matter how we spin it and there's always going to be that next provider that someone desperately needs. I think similar to peer discovery we should find an interface that someone can implement and create a shim between Thanos and the storage provider they want to use. Maybe we could extend the store component to expose an additional "upload" service so all object storage functionality is central to that component and people can then just implement their own store binaries, as the code is really not complex, and we could even expose everything except the actual bucket implementation as a library.

I agree. I feel like we need another proposal for this as we need to consider various issues like performance and operating of yet another sidecar/service.

The problem is that this is quite critical part, but some gRPC S3 like API could make sense. It is not time critical though, as we have some safety buffer. I think I like this idea more & more.

But clearly this change will not happen from day to day. I feel like we need something simple for time being...

bwplotka

Some initial review. Overall - very good direction. We need exactly that! Thanks for this!

And good timing, I was just about to implement this (:

bwplotka · 2018-08-31T14:14:17Z

pkg/objstore/config.go

+func NewBucketConfig(cmd *kingpin.CmdClause) *BucketConfig {
+	var bucketConfig BucketConfig
+
+	cmd.Flag("provider.type", "Specify the provider for object store. If empty or unsupport provider, Thanos won't read and store any block to the object store. Now supported GCS / S3.").


s/unsupport/unsupported

bwplotka · 2018-08-31T14:15:18Z

pkg/objstore/config.go

+	var bucketConfig BucketConfig
+
+	cmd.Flag("provider.type", "Specify the provider for object store. If empty or unsupport provider, Thanos won't read and store any block to the object store. Now supported GCS / S3.").
+		PlaceHolder("<provider>").StringVar(&bucketConfig.Provider)


Can we use EnumVar type here (instead of String var)?

bwplotka · 2018-08-31T14:16:11Z

pkg/objstore/config.go

+}
+
+// NewBucketConfig return the configuration of object store
+// TODO(jojohappy) should it support multiple bucket?


Good question. I think yes. We need some way of being able to use NewBucketConfig more than once, with some certain prefix maybe for both flag names and envvars names?

One use case is in bucket verify subcommand, where you can specify backup bucket. (: Not sure if we need this now though. Let's leave TODO and add that in next PRs

bwplotka · 2018-08-31T15:59:49Z

pkg/objstore/config.go

+	Unsupported objProvider = "UNSUPPORTED"
+)
+
+var ErrNotFound = errors.New("no valid provider configuration supplied")


All of this should move to client package. It is confusing that ErrNotFound indicates not found provided and not, "not found bucket" or "not found object"

bwplotka · 2018-08-31T16:00:12Z

pkg/objstore/config.go

+)
+
+var ErrNotFound = errors.New("no valid provider configuration supplied")
+var ErrMissingGCSBucket = errors.New("missing Google Cloud Storage bucket name for stored blocks")


objstore is meant to be generic, no provider details should be here

bwplotka · 2018-08-31T16:36:17Z

pkg/objstore/config.go

+// BucketConfig encapsulates the necessary config values to instantiate an object store client.
+// Use Provider to represent the Object Stores
+type BucketConfig struct {
+	Provider      string


objProvider instead of string?

bwplotka · 2018-08-31T16:37:37Z

pkg/objstore/config.go

+	cmd.Flag("provider.access-key", "Access key for an object store API. Support S3-Compatible API").
+		PlaceHolder("<key>").Envar("PROVIDER_ACCESS_KEY").StringVar(&bucketConfig.AccessKey)
+
+	// TODO(jojohappy): should it be encrypted?


I don't think so. How this improve things, if you would need encryption password/key passed somehow anyway?

No, users should provide the original password or key, but the BucketConfig will encrypt that and return it decrypted. Because the s3.Config did not export the secretKey, I didn't know why did that. In the new BucketConfig, I still doesn't export this field, so I think should it be encrypted?

Not sure for that, if it is not necessary, I will remove that.

bwplotka · 2018-08-31T16:38:39Z

pkg/objstore/config.go

+func NewBucketConfig(cmd *kingpin.CmdClause) *BucketConfig {
+	var bucketConfig BucketConfig
+
+	cmd.Flag("provider.type", "Specify the provider for object store. If empty or unsupport provider, Thanos won't read and store any block to the object store. Now supported GCS / S3.").


I am also not sure about provider name. Provider does not suggest for what things provider we mean. Maybe objstore instead?

yea, it will make sense.

bwplotka · 2018-08-31T18:51:35Z

docs/components/bucket.md

+                              no trace will be sent periodically, unless forced
+                              by baggage item. See `pkg/tracing/tracing.go` for
+                              details.
+      --provider.type=<provider>  


Agree... We need some way to not exponentially increase number of flags, but also keep documentation clear, how to configure obj stores. I am generally against moving to a config file, instead of flags. This is basically harder to operate. There is no clear winner here..

Some quick idea:

Put to flags only things that are common and add objstore.extra for file content and allow different providers parse it dynamically to internal structs?

Anything smarter? @brancz you are after inspiring Gophercon, maybe you have some idea (:

bwplotka · 2018-08-31T18:52:29Z

pkg/objstore/config.go

+}
+
+// NewBackupBucketConfig return the configuration of backup object store
+func NewBackupBucketConfig(cmd *kingpin.CmdClause) *BucketConfig {


yea, maybe we want just NewBucketConfig(cmd, suffix) ^^

bwplotka · 2018-09-03T17:50:15Z

Please rebase this for green CI (Landed fix #504) (:

bwplotka

Thanks for addressing all comments! Some extra one, but overall good direction.

There is blocker comment we need to make some decision on before merging, which is this discussion: #500 (comment)

In current state it still requires custom flags that does not make any sense for different providers. I feel like creating 2 flags:
objstore.type
objstore.config
and config being some custom yaml, marhsalled differently for each provider makes the most sense now.

bwplotka · 2018-09-03T18:44:46Z

cmd/thanos/store.go

@@ -31,7 +31,7 @@ func registerStore(m map[string]setupFunc, app *kingpin.Application, name string
 	dataDir := cmd.Flag("tsdb.path", "Data directory of TSDB.").
 		Default("./data").String()

-	bucketConf := objstore.NewBucketConfig(cmd)
+	bucketConf := objstore.NewBucketConfig(cmd, "")


you could potentiall add NewBucketConfigWithPrefix method to avoid this (:

bwplotka · 2018-09-03T18:46:42Z

pkg/objstore/config.go

@@ -34,104 +43,52 @@ type BucketConfig struct {
 }

 // NewBucketConfig return the configuration of object store
-// TODO(jojohappy) should it support multiple bucket?
-func NewBucketConfig(cmd *kingpin.CmdClause) *BucketConfig {
+func NewBucketConfig(cmd *kingpin.CmdClause, suffix string) *BucketConfig {


Why this has to be here and not in client package?

(and all changes in this file)

bwplotka · 2018-09-03T19:05:56Z

Added #505 that relates to this, but in mean time we should have something working now - objstore.type and objstore.config sounds like some idea for now. Any thoughts?

@jojohappy feel free to ping me on slack, I explain more.

bwplotka

NIce. This will help a lot I think. But this is major change so I will share it widely for review, OK?

I am mostly ok, just some suggestions.

bwplotka · 2018-09-07T15:51:24Z

cmd/thanos/bucket.go

-		PlaceHolder("<bucket>").String()
-
-	s3Config := s3.RegisterS3Params(cmd)
+	bucketConf := cmd.Flag("objstore.config", "The yaml format configuration of bucket for stored blocks.").


Can we improve this description by saying The object store configuration in yaml format.

bwplotka · 2018-09-07T15:51:48Z

cmd/thanos/bucket.go

-
-	s3Config := s3.RegisterS3Params(cmd)
+	bucketConf := cmd.Flag("objstore.config", "The yaml format configuration of bucket for stored blocks.").
+		PlaceHolder("<bucket.config>").String()


it is .Required() here I believe.

bwplotka · 2018-09-07T15:53:47Z

pkg/objstore/client/factory.go

+
+type BucketConfig struct {
+	Type    objProvider `yaml:"type"`
+	Content interface{} `yaml:"content"`


Maybe just config instead of content?

bwplotka · 2018-09-07T15:54:34Z

pkg/objstore/client/factory.go

+	if config == "" {
+		return nil, ErrNotFound
+	}
+	err = yaml.Unmarshal([]byte(config), &bucketConf)


we can use if err := ... ; err != nil { idiom here (:

bwplotka · 2018-09-07T15:57:28Z

pkg/objstore/s3/s3.go

+	Bucket        string `yaml:"bucket"`
+	Endpoint      string `yaml:"endpoint"`
+	AccessKey     string `yaml:"access-key"`
+	SecretKey     string `yaml:"secret-key"`


And now is that real question.. should it be available here or just through envvar (: I remember people saying it is not good idea to specify them via config file or flags

OK, I will rollback it to just use envvar (:

bwplotka · 2018-09-07T16:01:34Z

Can you @jojohappy rebase?

bwplotka · 2018-09-07T16:10:02Z

As a next step we can do in next PR: Some thanos command to describe the schemas of the config files for each objstore provider. It is not enough to just maintain doc - it needs to be tied to binary the config will be used against. But that is something we need to figure out later

vglafirov

Overall, looks good, need some minor tweaks.

vglafirov · 2018-09-07T18:47:37Z

cmd/thanos/downsample.go

-
-	s3Config := s3.RegisterS3Params(cmd)
+	bucketConf := cmd.Flag("objstore.config", "The yaml format configuration of bucket for stored blocks.").
+		PlaceHolder("<bucket.config>").String()


.Required() I guess

cmd/thanos/rule.go

cmd/thanos/bucket.go

vglafirov · 2018-09-07T18:52:43Z

cmd/thanos/store.go

@@ -24,17 +23,15 @@ import (

 // registerStore registers a store command.
 func registerStore(m map[string]setupFunc, app *kingpin.Application, name string) {
-	cmd := app.Command(name, "store node giving access to blocks in a GCS bucket")
+	cmd := app.Command(name, "store node giving access to blocks in a supported bucket provider")


I would print out provider signature here.

vglafirov · 2018-09-07T18:54:25Z

docs/components/bucket.md

+                        How often we send traces (1/<sample-factor>). If 0 no
+                        trace will be sent periodically, unless forced by
+                        baggage item. See `pkg/tracing/tracing.go` for details.
+      --objstore.config=<bucket.config>  


It's not clear from the description that it should be yaml file

e.. It is not a yaml file, just content of configuration in yaml format. But we should update the example.

Sorry for the noise. I know your opinion.

vglafirov · 2018-09-07T19:13:55Z

pkg/objstore/client/factory.go

+
+type BucketConfig struct {
+	Type    objProvider `yaml:"type"`
+	Content interface{} `yaml:"content"`


bwplotka · 2018-09-08T11:24:54Z

let's update CHANGELOG please (:

jojohappy · 2018-09-08T14:32:20Z

@Bplotka @vglafirov Fixed review and CHANGELOG. Meanwhile update the documents.

bwplotka

Nice, awesome work. Just super tiny nits, except maybe error handling in client.Bucket. Let's fix those and we are good to go 💪 Thanks!

bwplotka · 2018-09-10T13:13:35Z

cmd/thanos/bucket.go

-
-	s3Config := s3.RegisterS3Params(cmd)
+	bucketConf := cmd.Flag("objstore.config", "The object store configuration in yaml format.").
+		PlaceHolder("<bucket.config.yaml>").Required().String()

 	// Verify command.
 	verify := cmd.Command("verify", "verify all blocks in the bucket against specified issues")
 	verifyRepair := verify.Flag("repair", "attempt to repair blocks for which issues were detected").
 		Short('r').Default("false").Bool()
 	// NOTE(bplotka): Currently we support backup buckets only in the same project.


Not relevant anymore, we can kill this

cmd/thanos/bucket.go

cmd/thanos/rule.go

bwplotka · 2018-09-10T13:20:32Z

pkg/objstore/client/factory.go

+	case S3:
+		bucket, err = s3.NewBucket(logger, config, reg, component)
+	default:
+		return nil, ErrNotFound


It should return different error than not found. It's not known type we should error on.

bwplotka · 2018-09-10T13:20:47Z

pkg/objstore/gcs/gcs.go

@@ -32,29 +37,56 @@ const (
 // DirDelim is the delimiter used to model a directory structure in an object store bucket.
 const DirDelim = "/"

+// gcsConfig stores the configuration for gcs bucket


missing trailing period

bwplotka · 2018-09-10T13:21:08Z

pkg/objstore/gcs/gcs.go

-func NewBucket(name string, cl *storage.Client, reg prometheus.Registerer) *Bucket {
+func NewBucket(logger log.Logger, ctx context.Context, conf []byte, reg prometheus.Registerer, component string) (*Bucket, error) {
+	var gc gcsConfig
+	err := yaml.Unmarshal(conf, &gc)


we can if err := (..); err != nil { .. } idiom

bwplotka · 2018-09-10T13:22:05Z

pkg/objstore/s3/s3.go

+		return nil, err
+	}
+	config.secretKey = os.Getenv("S3_SECRET_KEY")
+	err = Validate(config)


if err := (...); err != nil {} idiom (:

bwplotka · 2018-09-10T13:22:33Z

pkg/objstore/s3/s3.go

+	if err != nil {
+		return nil, err
+	}
+	config.secretKey = os.Getenv("S3_SECRET_KEY")


bwplotka · 2018-09-10T13:22:58Z

pkg/objstore/s3/s3.go

@@ -185,6 +147,31 @@ func NewBucket(logger log.Logger, conf *Config, reg prometheus.Registerer, compo
 	return bkt, nil
 }

+// GetBucket return the bucket name for s3


returns instead of return and missing trailing period (:

bwplotka · 2018-09-10T13:27:08Z

CHANGELOG.md

@@ -11,6 +11,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.
 - Add Thanos compact --retention.default flag, for configuring storage bucket retention period.
 - Removes support for multiple units in duration. For example: 1m0s won't work, while 1m will work.
 - Adds support for y,w,d time units 
+- Use the configuration of bucket in yaml format.


Can we move to something like:

Removes support of those flags

--gcs-bucket=<bucket> --s3.bucket=<bucket> --s3.endpoint=<api-url> --s3.access-key=<key> --s3.insecure --s3.signature-version2 --s3.encrypt-sse --gcs-backup-bucket=<bucket> --s3-backup-bucket=<bucket>

instead added --objstore.config

bwplotka

I would remove this special case, otherwise rebase + LGTM

bwplotka · 2018-09-10T15:10:44Z

pkg/objstore/client/factory.go

+		bucket, err = gcs.NewBucket(logger, context.Background(), config, reg, component)
+	case S3:
+		bucket, err = s3.NewBucket(logger, config, reg, component)
+	case "":


This one is not required. It's clearly an error if someone puts config with type: "" (:

bwplotka · 2018-09-10T15:10:57Z

pkg/objstore/client/factory_test.go

+	testutil.Assert(t, err != ErrNotFound, "it should not error with not found")
+}
+
+func TestBlankBucketConfig(t *testing.T) {


not required case

bwplotka · 2018-09-10T16:51:24Z

@vglafirov you requested some changes. Is it ok now? I would like to merge this to unblock you and @jojohappy COS client

xjewer · 2018-09-11T12:37:07Z

Isn't it better to pass link to yaml file to the --objstore.config instead of yaml data, is it?

domgreen · 2018-09-11T12:48:15Z

@Bplotka I tend to agree with @xjewer I think passing a path to the yaml file will be much cleaner than actually passing the yaml in a string.

domgreen

Mainly nits ... can we wrap errors that we are returning to give more context and updating a few function signatures.

domgreen · 2018-09-11T12:54:54Z

cmd/thanos/compact.go

-
-	s3config := s3.RegisterS3Params(cmd)
+	bucketConf := cmd.Flag("objstore.config", "The object store configuration in yaml format.").
+		PlaceHolder("<bucket.config.yaml>").String()


should this not be required for compact? We will not be able to do anything without a bucket.

yea, It should be required.

domgreen · 2018-09-11T12:56:55Z

pkg/objstore/client/factory.go

-		return objstore.BucketWithMetrics(s3Config.Bucket, b, reg), nil
+	config, err := yaml.Marshal(bucketConf.Config)
+	if err != nil {
+		return nil, err


can we wrap this error 👍

pkg/objstore/client/factory.go

domgreen · 2018-09-11T13:02:24Z

cmd/thanos/store.go

@@ -83,7 +78,7 @@ func runStore(
 	verbose bool,
 ) error {
 	{
-		bkt, err := client.NewBucket(logger, &gcsBucket, *s3Config, reg, component)
+		bkt, err := client.NewBucket(logger, bucketConf, reg, component)
 		if err != nil {
 			return err


mind wrapping this whilst were here

domgreen · 2018-09-11T13:04:21Z

pkg/objstore/gcs/gcs.go


 	closer io.Closer
 }

 // NewBucket returns a new Bucket against the given bucket handle.
-func NewBucket(name string, cl *storage.Client, reg prometheus.Registerer) *Bucket {
+func NewBucket(logger log.Logger, ctx context.Context, conf []byte, reg prometheus.Registerer, component string) (*Bucket, error) {


i think its convention to have ctx as the first variable in the method. See conclusion here https://blog.golang.org/context

domgreen · 2018-09-11T13:08:57Z

pkg/objstore/gcs/gcs.go

 	bkt      *storage.BucketHandle
 	opsTotal *prometheus.CounterVec
+	bucket   string


domgreen · 2018-09-11T13:09:08Z

pkg/objstore/objstore.go

@@ -26,6 +26,9 @@ type Bucket interface {

 	// Delete removes the object with the given name.
 	Delete(ctx context.Context, name string) error
+
+	// GetBucket returns the bucket name for the provider.
+	GetBucket() string


Can we update this function name ... Name()? We tend not to have Getters in Go and although its returning string on first glance I would assume it returns the bucket.

Signed-off-by: jojohappy <[email protected]>

bwplotka

Some comment after changing to file loading.

bwplotka · 2018-09-18T23:49:30Z

cmd/thanos/bucket.go

-		PlaceHolder("<bucket>").String()
-	verifyBackupS3Bucket := cmd.Flag("s3-backup-bucket", "S3 bucket name to backup blocks on repair operations.").
-		PlaceHolder("<bucket>").String()
+	backupBucketConfFile := verify.Flag("objstore-backup.config", "The backup object store configuration in yaml format.").


config-file for consistency (:

It's my fault. Will be updated

bwplotka · 2018-09-18T23:50:53Z

pkg/objstore/client/factory.go

+func loadFile(confFile string) (*BucketConfig, error) {
+	content, err := ioutil.ReadFile(confFile)
+	if err != nil {
+		return nil, err


wrap with confFile maybe?

bwplotka · 2018-09-18T23:51:03Z

pkg/objstore/client/factory.go

+	}
+
+	bucketConf := &BucketConfig{}
+	if err := yaml.UnmarshalStrict(content, bucketConf); err != nil {


Why Strict?

According to the gopkg.in/yaml.v2 documents, using UnmarshalStrict can throw the error in the data that mapping keys that are duplicates or other unknown errors. The opinion comes from this issue.

bwplotka · 2018-09-18T23:51:47Z

pkg/objstore/client/factory.go

+
+	bucketConf := &BucketConfig{}
+	if err := yaml.UnmarshalStrict(content, bucketConf); err != nil {
+		return nil, fmt.Errorf("parsing YAML file %s: %v", confFile, err)


we use errors.Wrapf(err, msg, ...) from github/pkg/errors package.

Signed-off-by: jojohappy <[email protected]>

bwplotka

🎉 Thanks for you work @jojohappy

Let's merge it!

Signed-off-by: jojohappy <[email protected]>

xjewer · 2018-09-26T15:35:59Z

Just noticed, interesting moment, we deleted support env S3_SECRET_KEY, so what's the way to keep it in secrets in k8s and use configmap for the rest? @jojohappy

xjewer · 2018-09-26T16:08:37Z

the only way to use it in k8s seams to put the whole bucket.yaml config file into the secret https://kubernetes.io/docs/concepts/configuration/secret/#use-case-dotfiles-in-secret-volume

bwplotka · 2018-09-26T16:26:30Z

Very good point! And it's major one, let's figure it out. For me we should get S3_SECRET_KEY back working again. śr., 26 wrz 2018 o 17:16 użytkownik xjewer <[email protected]> napisał:

…

the only way to use it in k8s seams to put the whole bucket.yaml config file into the secret https://kubernetes.io/docs/concepts/configuration/secret/#use-case-dotfiles-in-secret-volume — You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHub <#500 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AGoNu3Ht2-vsVEm8X9zkKWlQj_glgRdNks5ue6higaJpZM4WU8Cz> .

jojohappy · 2018-09-27T04:56:53Z

Well, thanks @xjewer !
We could get the secret variable as Envvar such as S3_SECRET_KEY back with higher priority than the variable in configuration file.
IMO, I don't like put the whole configuration file into the secret.

Open issue: #539

Signed-off-by: jojohappy <[email protected]>

* Feature: support tencent cloud object storage Signed-off-by: jojohappy <[email protected]> * To set Content-Length for *os.File Signed-off-by: jojohappy <[email protected]> * Fixed circleci test Signed-off-by: jojohappy <[email protected]> * Rebase master to migrate PR #500 Signed-off-by: jojohappy <[email protected]> * Fixed code style Signed-off-by: jojohappy <[email protected]> * Update inconsistent constant variable Signed-off-by: jojohappy <[email protected]> * Update storage document Signed-off-by: jojohappy <[email protected]> * Fixed snake_case & some nits Signed-off-by: jojohappy <[email protected]> * Remove unnecessary store metrics Signed-off-by: jojohappy <[email protected]> * Remove evaluating type of block object in production code Signed-off-by: jojohappy <[email protected]> * Remove bucket metrics Signed-off-by: jojohappy <[email protected]> * Address review comments Signed-off-by: jojohappy <[email protected]> * To wrap reader closer as a nop to avoid to close it after uploading in COS client Signed-off-by: jojohappy <[email protected]> * Rebase & Add client struct config to bucketcfggen Signed-off-by: jojohappy <[email protected]> * Update storage docs using bucketcfggen Signed-off-by: jojohappy <[email protected]> * Address review comments Signed-off-by: jojohappy <[email protected]> * Address review comments Signed-off-by: jojohappy <[email protected]> * Update version of COS provider && Remove getting the length of reader Signed-off-by: jojohappy <[email protected]> * Missing the comment of object storage testing env THANOS_SKIP_SWIFT_TESTS Signed-off-by: jojohappy <[email protected]> * Address review comments Signed-off-by: jojohappy <[email protected]> * Typo error && Remove unused function argument Signed-off-by: jojohappy <[email protected]>

bwplotka mentioned this pull request Aug 31, 2018

Object Storage: Support tencent cloud COS provider #467

Merged

bwplotka requested review from domgreen and bwplotka August 31, 2018 13:04

vglafirov reviewed Aug 31, 2018

View reviewed changes

bwplotka reviewed Aug 31, 2018

View reviewed changes

bwplotka reviewed Sep 3, 2018

View reviewed changes

bwplotka mentioned this pull request Sep 3, 2018

Move object store client implementations out of sidecar binary #505

Closed

jojohappy force-pushed the set_unified_flag_for_store branch from 5b6673b to 8764690 Compare September 4, 2018 01:49

This was referenced Sep 6, 2018

downsample: Fix for Issue 494 #496

Closed

Azure Storage support #502

Merged

jojohappy force-pushed the set_unified_flag_for_store branch from b25496d to 8057c44 Compare September 7, 2018 08:41

bwplotka approved these changes Sep 7, 2018

View reviewed changes

vglafirov suggested changes Sep 7, 2018

View reviewed changes

jojohappy force-pushed the set_unified_flag_for_store branch from 8057c44 to 7d5dffa Compare September 8, 2018 13:53

bwplotka approved these changes Sep 10, 2018

View reviewed changes

bwplotka reviewed Sep 10, 2018

View reviewed changes

bwplotka approved these changes Sep 10, 2018

View reviewed changes

jojohappy force-pushed the set_unified_flag_for_store branch from 4b001d5 to aa29f10 Compare September 10, 2018 15:14

domgreen reviewed Sep 11, 2018

View reviewed changes

jojohappy force-pushed the set_unified_flag_for_store branch from c0ae0e6 to a1ec2b3 Compare September 12, 2018 14:05

jojohappy added 6 commits September 19, 2018 07:31

Do not get the secret-key from envvar

cb5f75d

Update documents

8c2dedd

Update CHANGELOG

9ecc7f9

Signed-off-by: jojohappy <[email protected]>

Update test case for package objstore client

2b64ce0

Signed-off-by: jojohappy <[email protected]>

Rename flag objstore.config.file to objstore.config-file

248034e

Signed-off-by: jojohappy <[email protected]>

Update CHANGELOG

4d665f1

jojohappy force-pushed the set_unified_flag_for_store branch from cce5eba to 4d665f1 Compare September 18, 2018 23:34

bwplotka requested changes Sep 18, 2018

View reviewed changes

jojohappy added 2 commits September 19, 2018 14:16

To wrap errors for loading and parsing bucket configuration file

33d1559

Signed-off-by: jojohappy <[email protected]>

Rename objstore flag for consistency

5bbb652

Signed-off-by: jojohappy <[email protected]>

bwplotka approved these changes Sep 19, 2018

View reviewed changes

bwplotka merged commit 984f42e into thanos-io:master Sep 19, 2018

jojohappy deleted the set_unified_flag_for_store branch September 20, 2018 02:34

jojohappy added a commit to jojohappy/thanos that referenced this pull request Sep 22, 2018

Rebase master to migrate PR thanos-io#500

1a68217

Signed-off-by: jojohappy <[email protected]>

jojohappy mentioned this pull request Sep 22, 2018

downsample: Downsample requires gcs_bucket even when using S3 #494

Closed

jojohappy mentioned this pull request Sep 27, 2018

A way to keep some secret variables for Objstore in safe #539

Closed

jojohappy added a commit to jojohappy/thanos that referenced this pull request Oct 11, 2018

Rebase master to migrate PR thanos-io#500

8d4e59a

Signed-off-by: jojohappy <[email protected]>

jojohappy added a commit to jojohappy/thanos that referenced this pull request Oct 30, 2018

Rebase master to migrate PR thanos-io#500

2c23b33

Signed-off-by: jojohappy <[email protected]>

jojohappy added a commit to jojohappy/thanos that referenced this pull request Oct 30, 2018

Rebase master to migrate PR thanos-io#500

d882f05

Signed-off-by: jojohappy <[email protected]>

jojohappy added a commit to jojohappy/thanos that referenced this pull request Nov 6, 2018

Rebase master to migrate PR thanos-io#500

79415f2

Signed-off-by: jojohappy <[email protected]>

jojohappy added a commit to jojohappy/thanos that referenced this pull request Nov 14, 2018

Rebase master to migrate PR thanos-io#500

49402bc

Signed-off-by: jojohappy <[email protected]>

jojohappy added a commit to jojohappy/thanos that referenced this pull request Nov 28, 2018

Rebase master to migrate PR thanos-io#500

75f4b60

Signed-off-by: jojohappy <[email protected]>

jojohappy added a commit to jojohappy/thanos that referenced this pull request Nov 28, 2018

Rebase master to migrate PR thanos-io#500

a6f0dc7

Signed-off-by: jojohappy <[email protected]>

jojohappy added a commit to jojohappy/thanos that referenced this pull request Dec 4, 2018

Rebase master to migrate PR thanos-io#500

3384a6d

Signed-off-by: jojohappy <[email protected]>

jojohappy added a commit to jojohappy/thanos that referenced this pull request Jan 2, 2019

Rebase master to migrate PR thanos-io#500

4581178

Signed-off-by: jojohappy <[email protected]>

Refactor bucket configuration flags for Object Store #500

Refactor bucket configuration flags for Object Store #500

Conversation

jojohappy commented Aug 31, 2018

Changes

Verification

jojohappy commented Aug 31, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bwplotka left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jojohappy Sep 1, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bwplotka commented Sep 3, 2018

bwplotka left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bwplotka commented Sep 3, 2018

bwplotka left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bwplotka commented Sep 7, 2018

bwplotka commented Sep 7, 2018

vglafirov left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bwplotka commented Sep 8, 2018

jojohappy commented Sep 8, 2018

bwplotka left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bwplotka left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bwplotka commented Sep 10, 2018

xjewer commented Sep 11, 2018

domgreen commented Sep 11, 2018

domgreen left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jojohappy commented Aug 31, 2018 •

edited

Loading

jojohappy Sep 1, 2018 •

edited

Loading

jojohappy Sep 19, 2018 •

edited

Loading