Skip to content
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

receiver/compact/store/sidecar: Added a provision in the bucket config to add a prefix #3289

Closed
wants to merge 18 commits into from
Closed
Changes from 13 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 38 additions & 0 deletions docs/storage.md
Original file line number Diff line number Diff line change
@@ -96,6 +96,7 @@ config:
kms_key_id: ""
kms_encryption_context: {}
encryption_key: ""
prefix: ""
```

At a minimum, you will need to provide a value for the `bucket`, `endpoint`, `access_key`, and `secret_key` keys. The rest of the keys are optional.
@@ -244,6 +245,7 @@ type: GCS
config:
bucket: ""
service_account: ""
prefix: ""
```

#### Using GOOGLE_APPLICATION_CREDENTIALS
@@ -328,6 +330,7 @@ config:
container: ""
endpoint: ""
max_retries: 0
prefix: ""
```

### OpenStack Swift
@@ -356,6 +359,7 @@ config:
project_domain_name: ""
region_name: ""
container_name: ""
prefix: ""
```

### Tencent COS
@@ -373,6 +377,7 @@ config:
app_id: ""
secret_key: ""
secret_id: ""
prefix: ""
```

Set the flags `--objstore.config-file` to reference to the configuration file.
@@ -390,6 +395,7 @@ config:
bucket: ""
access_key_id: ""
access_key_secret: ""
prefix: ""
```

Use --objstore.config-file to reference to this configuration file.
@@ -409,4 +415,36 @@ This is mainly useful for testing and demos.
type: FILESYSTEM
config:
directory: ""
prefix: ""
```

## Prefix

Now Thanos also supports multi-tenancy at the object storage using custom prefix. Check [this](https://github.com/thanos-io/thanos/issues/1318) issue. This feature is implemented in such a way that, no client level changes are required. All object storage implementations support this `prefix` key in their object storage config yaml.
dsayan154 marked this conversation as resolved.
Show resolved Hide resolved

Sample object store config:
```yaml
type: S3
config:
bucket: ""
endpoint: ""
region: ""
access_key: ""
insecure: false
signature_version2: false
secret_key: ""
put_user_metadata: {}
http_config:
idle_conn_timeout: 1m30s
response_header_timeout: 2m
insecure_skip_verify: false
trace:
enable: false
part_size: 134217728
sse_config:
type: ""
kms_key_id: ""
kms_encryption_context: {}
encryption_key: ""
prefix: tenant-0
```
3 changes: 2 additions & 1 deletion pkg/objstore/client/factory.go
Original file line number Diff line number Diff line change
@@ -38,6 +38,7 @@ const (
type BucketConfig struct {
Type ObjProvider `yaml:"type"`
Config interface{} `yaml:"config"`
Prefix string `yaml:"prefix" default:""`
dsayan154 marked this conversation as resolved.
Show resolved Hide resolved
}

// NewBucket initializes and returns new object storage clients.
@@ -76,5 +77,5 @@ func NewBucket(logger log.Logger, confContentYaml []byte, reg prometheus.Registe
if err != nil {
return nil, errors.Wrap(err, fmt.Sprintf("create %s client", bucketConf.Type))
}
return objstore.NewTracingBucket(objstore.BucketWithMetrics(bucket.Name(), bucket, reg)), nil
return objstore.NewTracingBucket(objstore.BucketWithMetrics(bucket.Name(), objstore.NewPrefixedBucket(bucket, bucketConf.Prefix), reg)), nil
}
81 changes: 81 additions & 0 deletions pkg/objstore/objstore.go
Original file line number Diff line number Diff line change
@@ -437,6 +437,87 @@ func (b *metricBucket) Name() string {
return b.bkt.Name()
}

type prefixedBucket struct {
dsayan154 marked this conversation as resolved.
Show resolved Hide resolved
bkt Bucket
dsayan154 marked this conversation as resolved.
Show resolved Hide resolved
prefix string
}

// NewPrefixedBucket returns a new prefixedBucket.
func NewPrefixedBucket(bkt Bucket, prefix string) *prefixedBucket {
return &prefixedBucket{
bkt: bkt,
prefix: prefix,
}
}

func (pbkt *prefixedBucket) WithExpectedErrs(expectedFunc IsOpFailureExpectedFunc) Bucket {
if ib, ok := pbkt.bkt.(InstrumentedBucket); ok {
return &prefixedBucket{
bkt: ib.WithExpectedErrs(expectedFunc),
prefix: pbkt.prefix,
}
}
return pbkt
}

func (pbkt *prefixedBucket) ReaderWithExpectedErrs(expectedFunc IsOpFailureExpectedFunc) BucketReader {
return pbkt.WithExpectedErrs(expectedFunc)
}

func (pbkt *prefixedBucket) Name() string {
return pbkt.bkt.Name()
}

func (pbkt *prefixedBucket) Upload(ctx context.Context, name string, r io.Reader) (err error) {
pname := filepath.Join(pbkt.prefix, name)
err = pbkt.bkt.Upload(ctx, pname, r)
return
}

func (pbkt *prefixedBucket) Delete(ctx context.Context, name string) (err error) {
pname := filepath.Join(pbkt.prefix, name)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You should juse path.Join() and not filepath.Join(). The former guarantees paths are joined with /, which is what object stores expect.

Same comment applies across the change set.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Aso the .Join() is used quite a lot. You could add an help prefixedBucket.nameWithPrefix(name string)

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah, might be good to add a helper function but I don't know how will it reduce the redundancy of the code, as I'll have to call the helper function same number of times as I have called filepath.Join().
But I'll update the code with your suggestion. I think it will add consistency to the code.

err = pbkt.bkt.Delete(ctx, pname)
return
}

func (pbkt *prefixedBucket) Close() error {
return pbkt.bkt.Close()
}

func (pbkt *prefixedBucket) Iter(ctx context.Context, dir string, f func(string) error) error {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would be possible to add a unit test on this?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Working on this.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @pracucci ,
I have added few unit tests for the Iter(), can you please review this? the tests are running fine locally.

pdir := filepath.Join(pbkt.prefix, dir)
if pbkt.prefix != "" {
return pbkt.bkt.Iter(ctx, pdir, func(s string) error {
return f(strings.Join(strings.Split(s, pbkt.prefix)[1:], "/"))
dsayan154 marked this conversation as resolved.
Show resolved Hide resolved
})
}
return pbkt.bkt.Iter(ctx, pdir, f)
}

func (pbkt *prefixedBucket) Get(ctx context.Context, name string) (io.ReadCloser, error) {
pname := filepath.Join(pbkt.prefix, name)
return pbkt.bkt.Get(ctx, pname)
}

func (pbkt *prefixedBucket) GetRange(ctx context.Context, name string, off, length int64) (io.ReadCloser, error) {
pname := filepath.Join(pbkt.prefix, name)
return pbkt.bkt.GetRange(ctx, pname, off, length)
}

func (pbkt *prefixedBucket) Exists(ctx context.Context, name string) (bool, error) {
pname := filepath.Join(pbkt.prefix, name)
return pbkt.bkt.Exists(ctx, pname)
}

func (pbkt *prefixedBucket) IsObjNotFoundErr(err error) bool {
return pbkt.bkt.IsObjNotFoundErr(err)
}

func (pbkt *prefixedBucket) Attributes(ctx context.Context, name string) (ObjectAttributes, error) {
pname := filepath.Join(pbkt.prefix, name)
return pbkt.bkt.Attributes(ctx, pname)
}

type timingReadCloser struct {
io.ReadCloser