Refactor bucket configuration flags for Object Store (#500)

* Add unified flags for bucket configuration Signed-off-by: jojohappy <sarahdj0917@gmail.com> * Use the unified bucket config for components Signed-off-by: jojohappy <sarahdj0917@gmail.com> * Use double quotes instead of single quotes Signed-off-by: jojohappy <sarahdj0917@gmail.com> * Fixed missing flags in man page Signed-off-by: jojohappy <sarahdj0917@gmail.com> * To use value of bucket config instead of pointer to get value from command-line arguments Signed-off-by: jojohappy <sarahdj0917@gmail.com> * Remove useless code Signed-off-by: jojohappy <sarahdj0917@gmail.com> * Update documents Signed-off-by: jojohappy <sarahdj0917@gmail.com> * Update - Rename provider to objstore for flags - To use objProvider instead of string for Provider Type - Get rid of bucket configuration errors Signed-off-by: jojohappy <sarahdj0917@gmail.com> * Change errors package Signed-off-by: jojohappy <sarahdj0917@gmail.com> * To validate the configuration in each provider client Signed-off-by: jojohappy <sarahdj0917@gmail.com> * To support to make bucket configuration flag using suffix Signed-off-by: jojohappy <sarahdj0917@gmail.com> * Update documents Signed-off-by: jojohappy <sarahdj0917@gmail.com> * Refactor: - Remove all flags of objstore - Add flag as objstore.config to pass the configuration for bucket with yaml - To define the configuration for each provider - Add new method to get the name of bucket Signed-off-by: jojohappy <sarahdj0917@gmail.com> * Update documents * Fixed missing method for inmem bucket * Update the describe for objstore.config * To setup bucket flags required for component bucket and downsample * Rename Content to Config for Bucket.Config Signed-off-by: jojohappy <sarahdj0917@gmail.com> * To change error handler idiom Signed-off-by: jojohappy <sarahdj0917@gmail.com> * Update describe for component store Signed-off-by: jojohappy <sarahdj0917@gmail.com> * To setup secret-key just use envvar Signed-off-by: jojohappy <sarahdj0917@gmail.com> * Update the placeholder of flags and documents Signed-off-by: jojohappy <sarahdj0917@gmail.com> * Update example for bucket Signed-off-by: jojohappy <sarahdj0917@gmail.com> * Update documents Signed-off-by: jojohappy <sarahdj0917@gmail.com> * Update CHANGELOG Signed-off-by: jojohappy <sarahdj0917@gmail.com> * Fixed something nits * To distinguish no bucket is configured or not supported Signed-off-by: jojohappy <sarahdj0917@gmail.com> * Update CHANGELOG Signed-off-by: jojohappy <sarahdj0917@gmail.com> * Remove unrequired unit test Signed-off-by: jojohappy <sarahdj0917@gmail.com> * To set bucket flag required for component store and compact Signed-off-by: jojohappy <sarahdj0917@gmail.com> * Rename GetBucket to Name & Set context as first argument Signed-off-by: jojohappy <sarahdj0917@gmail.com> * Wrap error to give more information Signed-off-by: jojohappy <sarahdj0917@gmail.com> * Rename field bucket to name for the struct of Bucket Signed-off-by: jojohappy <sarahdj0917@gmail.com> * Update documents Signed-off-by: jojohappy <sarahdj0917@gmail.com> * Change the bucket configuration flags to reference to YAML file * Do not get the secret-key from envvar * Update documents * Update CHANGELOG Signed-off-by: jojohappy <sarahdj0917@gmail.com> * Update test case for package objstore client Signed-off-by: jojohappy <sarahdj0917@gmail.com> * Rename flag objstore.config.file to objstore.config-file Signed-off-by: jojohappy <sarahdj0917@gmail.com> * Update CHANGELOG * To wrap errors for loading and parsing bucket configuration file Signed-off-by: jojohappy <sarahdj0917@gmail.com> * Rename objstore flag for consistency Signed-off-by: jojohappy <sarahdj0917@gmail.com>
thanos-io · Sep 19, 2018 · 984f42e · 984f42e
1 parent ebb58c2
commit 984f42e
Show file tree

Hide file tree

Showing 22 changed files with 429 additions and 372 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -8,6 +8,25 @@ NOTE: As semantic versioning states all 0.y.z releases can contain breaking chan
 
 ## Unreleased
 
+- Remove support of those flags for bucket
+    - --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\>
+- Remove support of those environment variables for bucket
+    * S3_BUCKET
+    * S3_ENDPOINT
+    * S3_ACCESS_KEY
+    * S3_INSECURE
+    * S3_SIGNATURE_VERSION2
+    * S3_SECRET_KEY
+- Add flag `--objstore.config-file` to reference to the bucket configuration file in yaml format. Note that detailed information in document [storage](docs/storage.md).
+
 ## [v0.1.0](https://github.com/improbable-eng/thanos/releases/tag/v0.1.0) - 2018.09.14
 
 Initial version to have a stable reference before [gossip protocol removal](https://github.com/improbable-eng/thanos/blob/master/docs/proposals/gossip-removal.md).

diff --git a/Makefile b/Makefile
@@ -139,7 +139,7 @@ tarballs-release: $(PROMU)
 # test runs all Thanos golang tests against each supported version of Prometheus.
 .PHONY: test
 test: test-deps
-	@echo ">> running all tests. Do export THANOS_SKIP_GCS_TESTS="true" or/and  export THANOS_SKIP_S3_AWS_TESTS="true" if you want to skip e2e tests against real store buckets"
+	@echo ">> running all tests. Do export THANOS_SKIP_GCS_TESTS='true' or/and  export THANOS_SKIP_S3_AWS_TESTS='true' if you want to skip e2e tests against real store buckets"
 	@for ver in $(SUPPORTED_PROM_VERSIONS); do \
 		THANOS_TEST_PROMETHEUS_PATH="prometheus-$$ver" THANOS_TEST_ALERTMANAGER_PATH="alertmanager-$(ALERTMANAGER_VERSION)" go test $(shell go list ./... | grep -v /vendor/ | grep -v /benchmark/); \
 	done

diff --git a/cmd/thanos/bucket.go b/cmd/thanos/bucket.go
@@ -12,7 +12,6 @@ import (
 	"github.com/go-kit/kit/log"
 	"github.com/improbable-eng/thanos/pkg/block"
 	"github.com/improbable-eng/thanos/pkg/objstore/client"
-	"github.com/improbable-eng/thanos/pkg/objstore/s3"
 	"github.com/improbable-eng/thanos/pkg/runutil"
 	"github.com/improbable-eng/thanos/pkg/verifier"
 	"github.com/oklog/run"
@@ -42,34 +41,27 @@ var (
 func registerBucket(m map[string]setupFunc, app *kingpin.Application, name string) {
 	cmd := app.Command(name, "inspect metric data in an object storage bucket")
 
-	gcsBucket := cmd.Flag("gcs-bucket", "Google Cloud Storage bucket name for stored blocks.").
-		PlaceHolder("<bucket>").String()
-
-	s3Config := s3.RegisterS3Params(cmd)
+	bucketConfFile := cmd.Flag("objstore.config-file", "The object store configuration file path.").
+		PlaceHolder("<bucket.config.path>").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.
-	verifyBackupGCSBucket := cmd.Flag("gcs-backup-bucket", "Google Cloud Storage bucket name to backup blocks on repair operations.").
-		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-file", "The backup object store configuration file path.").
+		PlaceHolder("<bucket-backup.config.path>").String()
 	verifyIssues := verify.Flag("issues", fmt.Sprintf("Issues to verify (and optionally repair). Possible values: %v", allIssues())).
 		Short('i').Default(verifier.IndexIssueID, verifier.OverlappedBlocksIssueID).Strings()
 	verifyIDWhitelist := verify.Flag("id-whitelist", "Block IDs to verify (and optionally repair) only. "+
 		"If none is specified, all blocks will be verified. Repeated field").Strings()
 	m[name+" verify"] = func(g *run.Group, logger log.Logger, reg *prometheus.Registry, _ opentracing.Tracer, _ bool) error {
-		bkt, err := client.NewBucket(logger, gcsBucket, *s3Config, reg, name)
+		bkt, err := client.NewBucket(logger, *bucketConfFile, reg, name)
 		if err != nil {
 			return err
 		}
 		defer runutil.CloseWithLogOnErr(logger, bkt, "bucket client")
 
-		backupS3Config := *s3Config
-		backupS3Config.Bucket = *verifyBackupS3Bucket
-		backupBkt, err := client.NewBucket(logger, verifyBackupGCSBucket, backupS3Config, reg, name)
+		backupBkt, err := client.NewBucket(logger, *backupBucketConfFile, reg, name)
 		if err == client.ErrNotFound {
 			if *verifyRepair {
 				return errors.Wrap(err, "repair is specified, so backup client is required")
@@ -129,7 +121,7 @@ func registerBucket(m map[string]setupFunc, app *kingpin.Application, name strin
 	lsOutput := ls.Flag("output", "Format in which to print each block's information. May be 'json' or custom template.").
 		Short('o').Default("").String()
 	m[name+" ls"] = func(g *run.Group, logger log.Logger, reg *prometheus.Registry, _ opentracing.Tracer, _ bool) error {
-		bkt, err := client.NewBucket(logger, gcsBucket, *s3Config, reg, name)
+		bkt, err := client.NewBucket(logger, *bucketConfFile, reg, name)
 		if err != nil {
 			return err
 		}

diff --git a/cmd/thanos/compact.go b/cmd/thanos/compact.go
@@ -11,7 +11,6 @@ import (
 	"github.com/improbable-eng/thanos/pkg/compact"
 	"github.com/improbable-eng/thanos/pkg/compact/downsample"
 	"github.com/improbable-eng/thanos/pkg/objstore/client"
-	"github.com/improbable-eng/thanos/pkg/objstore/s3"
 	"github.com/improbable-eng/thanos/pkg/runutil"
 	"github.com/oklog/run"
 	"github.com/opentracing/opentracing-go"
@@ -32,10 +31,8 @@ func registerCompact(m map[string]setupFunc, app *kingpin.Application, name stri
 	dataDir := cmd.Flag("data-dir", "Data directory in which to cache blocks and process compactions.").
 		Default("./data").String()
 
-	gcsBucket := cmd.Flag("gcs.bucket", "Google Cloud Storage bucket name for stored blocks.").
-		PlaceHolder("<bucket>").String()
-
-	s3config := s3.RegisterS3Params(cmd)
+	bucketConfFile := cmd.Flag("objstore.config-file", "The object store configuration file path.").
+		PlaceHolder("<bucket.config.path>").Required().String()
 
 	syncDelay := modelDuration(cmd.Flag("sync-delay", "Minimum age of fresh (non-compacted) blocks before they are being processed.").
 		Default("30m"))
@@ -56,8 +53,7 @@ func registerCompact(m map[string]setupFunc, app *kingpin.Application, name stri
 		return runCompact(g, logger, reg,
 			*httpAddr,
 			*dataDir,
-			*gcsBucket,
-			s3config,
+			*bucketConfFile,
 			time.Duration(*syncDelay),
 			*haltOnError,
 			*wait,
@@ -78,8 +74,7 @@ func runCompact(
 	reg *prometheus.Registry,
 	httpBindAddr string,
 	dataDir string,
-	gcsBucket string,
-	s3Config *s3.Config,
+	bucketConfFile string,
 	syncDelay time.Duration,
 	haltOnError bool,
 	wait bool,
@@ -99,7 +94,7 @@ func runCompact(
 
 	reg.MustRegister(halted)
 
-	bkt, err := client.NewBucket(logger, &gcsBucket, *s3Config, reg, component)
+	bkt, err := client.NewBucket(logger, bucketConfFile, reg, component)
 	if err != nil {
 		return err
 	}

diff --git a/cmd/thanos/downsample.go b/cmd/thanos/downsample.go
@@ -16,7 +16,6 @@ import (
 	"github.com/improbable-eng/thanos/pkg/compact/downsample"
 	"github.com/improbable-eng/thanos/pkg/objstore"
 	"github.com/improbable-eng/thanos/pkg/objstore/client"
-	"github.com/improbable-eng/thanos/pkg/objstore/s3"
 	"github.com/improbable-eng/thanos/pkg/runutil"
 	"github.com/oklog/run"
 	"github.com/oklog/ulid"
@@ -33,13 +32,11 @@ func registerDownsample(m map[string]setupFunc, app *kingpin.Application, name s
 	dataDir := cmd.Flag("data-dir", "Data directory in which to cache blocks and process downsamplings.").
 		Default("./data").String()
 
-	gcsBucket := cmd.Flag("gcs.bucket", "Google Cloud Storage bucket name for stored blocks.").
-		PlaceHolder("<bucket>").Required().String()
-
-	s3Config := s3.RegisterS3Params(cmd)
+	bucketConfFile := cmd.Flag("objstore.config-file", "The object store configuration file path.").
+		PlaceHolder("<bucket.config.path>").Required().String()
 
 	m[name] = func(g *run.Group, logger log.Logger, reg *prometheus.Registry, tracer opentracing.Tracer, _ bool) error {
-		return runDownsample(g, logger, reg, *dataDir, *gcsBucket, s3Config, name)
+		return runDownsample(g, logger, reg, *dataDir, *bucketConfFile, name)
 	}
 }
 
@@ -48,12 +45,11 @@ func runDownsample(
 	logger log.Logger,
 	reg *prometheus.Registry,
 	dataDir string,
-	gcsBucket string,
-	s3Config *s3.Config,
+	bucketConfFile string,
 	component string,
 ) error {
 
-	bkt, err := client.NewBucket(logger, &gcsBucket, *s3Config, reg, component)
+	bkt, err := client.NewBucket(logger, bucketConfFile, reg, component)
 	if err != nil {
 		return err
 	}

diff --git a/cmd/thanos/rule.go b/cmd/thanos/rule.go
@@ -25,7 +25,6 @@ import (
 	"github.com/improbable-eng/thanos/pkg/block"
 	"github.com/improbable-eng/thanos/pkg/cluster"
 	"github.com/improbable-eng/thanos/pkg/objstore/client"
-	"github.com/improbable-eng/thanos/pkg/objstore/s3"
 	"github.com/improbable-eng/thanos/pkg/runutil"
 	"github.com/improbable-eng/thanos/pkg/shipper"
 	"github.com/improbable-eng/thanos/pkg/store"
@@ -72,12 +71,10 @@ func registerRule(m map[string]setupFunc, app *kingpin.Application, name string)
 	alertmgrs := cmd.Flag("alertmanagers.url", "Alertmanager URLs to push firing alerts to. The scheme may be prefixed with 'dns+' or 'dnssrv+' to detect Alertmanager IPs through respective DNS lookups. The port defaults to 9093 or the SRV record's value. The URL path is used as a prefix for the regular Alertmanager API path.").
 		Strings()
 
-	gcsBucket := cmd.Flag("gcs.bucket", "Google Cloud Storage bucket name for stored blocks. If empty, ruler won't store any block inside Google Cloud Storage.").
-		PlaceHolder("<bucket>").String()
-
 	alertQueryURL := cmd.Flag("alert.query-url", "The external Thanos Query URL that would be set in all alerts 'Source' field").String()
 
-	s3Config := s3.RegisterS3Params(cmd)
+	bucketConfFile := cmd.Flag("objstore.config-file", "The object store configuration file path.").
+		PlaceHolder("<bucket.config.path>").String()
 
 	m[name] = func(g *run.Group, logger log.Logger, reg *prometheus.Registry, tracer opentracing.Tracer, _ bool) error {
 		lset, err := parseFlagLabels(*labelStrs)
@@ -112,8 +109,7 @@ func registerRule(m map[string]setupFunc, app *kingpin.Application, name string)
 			*dataDir,
 			*ruleFiles,
 			peer,
-			*gcsBucket,
-			s3Config,
+			*bucketConfFile,
 			tsdbOpts,
 			name,
 			alertQueryURL,
@@ -136,8 +132,7 @@ func runRule(
 	dataDir string,
 	ruleFiles []string,
 	peer *cluster.Peer,
-	gcsBucket string,
-	s3Config *s3.Config,
+	bucketConfFile string,
 	tsdbOpts *tsdb.Options,
 	component string,
 	alertQueryURL *url.URL,
@@ -415,13 +410,13 @@ func runRule(
 
 	// The background shipper continuously scans the data directory and uploads
 	// new blocks to Google Cloud Storage or an S3-compatible storage service.
-	bkt, err := client.NewBucket(logger, &gcsBucket, *s3Config, reg, component)
+	bkt, err := client.NewBucket(logger, bucketConfFile, reg, component)
 	if err != nil && err != client.ErrNotFound {
 		return err
 	}
 
 	if err == client.ErrNotFound {
-		level.Info(logger).Log("msg", "No GCS or S3 bucket was configured, uploads will be disabled")
+		level.Info(logger).Log("msg", "No supported bucket was configured, uploads will be disabled")
 		uploads = false
 	}
 

diff --git a/cmd/thanos/sidecar.go b/cmd/thanos/sidecar.go
@@ -16,7 +16,6 @@ import (
 	"github.com/improbable-eng/thanos/pkg/block"
 	"github.com/improbable-eng/thanos/pkg/cluster"
 	"github.com/improbable-eng/thanos/pkg/objstore/client"
-	"github.com/improbable-eng/thanos/pkg/objstore/s3"
 	"github.com/improbable-eng/thanos/pkg/reloader"
 	"github.com/improbable-eng/thanos/pkg/runutil"
 	"github.com/improbable-eng/thanos/pkg/shipper"
@@ -43,11 +42,6 @@ func registerSidecar(m map[string]setupFunc, app *kingpin.Application, name stri
 	dataDir := cmd.Flag("tsdb.path", "Data directory of TSDB.").
 		Default("./data").String()
 
-	gcsBucket := cmd.Flag("gcs.bucket", "Google Cloud Storage bucket name for stored blocks. If empty, sidecar won't store any block inside Google Cloud Storage.").
-		PlaceHolder("<bucket>").String()
-
-	s3Config := s3.RegisterS3Params(cmd)
-
 	reloaderCfgFile := cmd.Flag("reloader.config-file", "Config file watched by the reloader.").
 		Default("").String()
 
@@ -56,6 +50,9 @@ func registerSidecar(m map[string]setupFunc, app *kingpin.Application, name stri
 
 	reloaderRuleDirs := cmd.Flag("reloader.rule-dir", "Rule directories for the reloader to refresh (repeated field).").Strings()
 
+	bucketConfFile := cmd.Flag("objstore.config-file", "The object store configuration file path.").
+		PlaceHolder("<bucket.config.path>").String()
+
 	m[name] = func(g *run.Group, logger log.Logger, reg *prometheus.Registry, tracer opentracing.Tracer, _ bool) error {
 		rl := reloader.New(
 			log.With(logger, "component", "reloader"),
@@ -77,8 +74,7 @@ func registerSidecar(m map[string]setupFunc, app *kingpin.Application, name stri
 			*httpBindAddr,
 			*promURL,
 			*dataDir,
-			*gcsBucket,
-			s3Config,
+			*bucketConfFile,
 			peer,
 			rl,
 			name,
@@ -95,8 +91,7 @@ func runSidecar(
 	httpBindAddr string,
 	promURL *url.URL,
 	dataDir string,
-	gcsBucket string,
-	s3Config *s3.Config,
+	bucketConfFile string,
 	peer *cluster.Peer,
 	reloader *reloader.Reloader,
 	component string,
@@ -224,13 +219,13 @@ func runSidecar(
 
 	// The background shipper continuously scans the data directory and uploads
 	// new blocks to Google Cloud Storage or an S3-compatible storage service.
-	bkt, err := client.NewBucket(logger, &gcsBucket, *s3Config, reg, component)
+	bkt, err := client.NewBucket(logger, bucketConfFile, reg, component)
 	if err != nil && err != client.ErrNotFound {
 		return err
 	}
 
 	if err == client.ErrNotFound {
-		level.Info(logger).Log("msg", "No GCS or S3 bucket was configured, uploads will be disabled")
+		level.Info(logger).Log("msg", "No supported bucket was configured, uploads will be disabled")
 		uploads = false
 	}
 

diff --git a/cmd/thanos/store.go b/cmd/thanos/store.go
@@ -10,7 +10,6 @@ import (
 	"github.com/go-kit/kit/log/level"
 	"github.com/improbable-eng/thanos/pkg/cluster"
 	"github.com/improbable-eng/thanos/pkg/objstore/client"
-	"github.com/improbable-eng/thanos/pkg/objstore/s3"
 	"github.com/improbable-eng/thanos/pkg/runutil"
 	"github.com/improbable-eng/thanos/pkg/store"
 	"github.com/improbable-eng/thanos/pkg/store/storepb"
@@ -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 bucket provider. Now supported GCS / S3.")
 
 	grpcBindAddr, httpBindAddr, newPeerFn := regCommonServerFlags(cmd)
 
 	dataDir := cmd.Flag("data-dir", "Data directory in which to cache remote blocks.").
 		Default("./data").String()
 
-	gcsBucket := cmd.Flag("gcs.bucket", "Google Cloud Storage bucket name for stored blocks. If empty sidecar won't store any block inside Google Cloud Storage.").
-		PlaceHolder("<bucket>").String()
-
-	s3Config := s3.RegisterS3Params(cmd)
+	bucketConfFile := cmd.Flag("objstore.config-file", "The object store configuration file path.").
+		PlaceHolder("<bucket.config.path>").Required().String()
 
 	indexCacheSize := cmd.Flag("index-cache-size", "Maximum size of items held in the index cache.").
 		Default("250MB").Bytes()
@@ -51,8 +48,7 @@ func registerStore(m map[string]setupFunc, app *kingpin.Application, name string
 			logger,
 			reg,
 			tracer,
-			*gcsBucket,
-			s3Config,
+			*bucketConfFile,
 			*dataDir,
 			*grpcBindAddr,
 			*httpBindAddr,
@@ -71,8 +67,7 @@ func runStore(
 	logger log.Logger,
 	reg *prometheus.Registry,
 	tracer opentracing.Tracer,
-	gcsBucket string,
-	s3Config *s3.Config,
+	bucketConfFile string,
 	dataDir string,
 	grpcBindAddr string,
 	httpBindAddr string,
@@ -83,9 +78,9 @@ func runStore(
 	verbose bool,
 ) error {
 	{
-		bkt, err := client.NewBucket(logger, &gcsBucket, *s3Config, reg, component)
+		bkt, err := client.NewBucket(logger, bucketConfFile, reg, component)
 		if err != nil {
-			return err
+			return errors.Wrap(err, "create bucket client")
 		}
 
 		// Ensure we close up everything properly.