Support S3 storage class

[ArpStorm1]

Is your proposal related to a problem?

Hello!!
We need support to use other storage classes than STANDARD in S3.
The default storage class in S3 is STANDARD, and it is also the mose expensive one.
We would like to get a configuration option to use STANDARD_IA storage class or even GLACIER if needed.

Describe the solution you'd like

An option in the config file to choose the required storage class.

Describe alternatives you've considered

Using STANDARD is expensive.

Additional context

MinIO client is supporting S3 storage class, so it would be nice to use this feature in Thanos as well

[juanrh]

I'm interesting on working on this

[juanrh]

I've been investigating this a bit. I first though I could pass config.put_user_metadata:X-Amz-Storage-Class: STANDARD_IA to setup STANDARD_IA storage class, but that was failing with

level=warn ts=2022-09-01T09:19:09.362133Z caller=sidecar.go:344 err="upload 01G95BH5DQDC8SKB97JBG79V9T: upload chunks: upload file /Users/jrodrig/systems/prometheus/latest/data/thanos/upload/01G95BH5DQDC8SKB97JBG79V9T/chunks/000001 as 01G95BH5DQDC8SKB97JBG79V9T/chunks/000001: upload s3 object: X-Amz-Storage-Class unsupported user defined metadata name" uploaded=0

which is a validation done in https://github.com/minio/minio-go/blob/master/api-put-object.go#L188. It looks like in Minio the storage class has to be specified in PutObjectOptions.StorageClass. I did a test hardcoding that as follows:

gijrodrig-mac:thanos jrodrig$ git diff
diff --git a/pkg/objstore/s3/s3.go b/pkg/objstore/s3/s3.go
index 1e0ae3fe..11e28668 100644
--- a/pkg/objstore/s3/s3.go
+++ b/pkg/objstore/s3/s3.go
@@ -541,6 +541,7 @@ func (b *Bucket) Upload(ctx context.Context, name string, r io.Reader) error {
                        PartSize:             partSize,
                        ServerSideEncryption: sse,
                        UserMetadata:         b.putUserMetadata,
+                       StorageClass: "STANDARD_IA",
                },
        ); err != nil {
                return errors.Wrap(err, "upload s3 object")
jrodrig-mac:thanos jrodrig$ 

and when I test it against S3 as follows I see objects with Storage class Standard-IA uploaded by the sidecar in S3

rm ${PROM_ROOT}/data/thanos.shipper.json # force reupload
~/go/bin/thanos sidecar \
    --tsdb.path            ${PROM_ROOT}/data \
    --prometheus.url       "http://localhost:9090" \
    --objstore.config='type: S3
prefix: thanos-test-standard-ia
config:
  endpoint: s3.us-east-1.amazonaws.com
  region: us-east-1
  bucket: MY_BUCKET
  trace:
    enable: true'

We could pass the storage class in the config so it is available in type Bucket struct in pkg/objstore/s3/s3.go. One option is adding a new key, but another one is allowing to specify it in config.put_user_metadata:X-Amz-Storage-Class: STANDARD_IA (as X-Amz-Storage-Class is already a documented key https://docs.aws.amazon.com/AmazonS3/latest/API/API_PutObject.html#API_PutObject_RequestSyntax and with a constant in vendor/github.com/minio/minio-go/v7/constants.go), and in Bucket.Upload we could move it from minio.PutObjectOptions.UserMetadata and put in in minio.PutObjectOptions.StorageClass.
So I'd like advice on the following to go ahead with this task:

• Do you prefer using config.put_user_metadata:X-Amz-Storage-Class: STANDARD_IA to specify the storage class (by moving X-Amz-Storage-Class to minio.PutObjectOptions.StorageClass in Bucket.Upload as described above), or a new key for https://thanos.io/tip/thanos/storage.md/#s3?
• Is there any other method besides func (b *Bucket) Upload from pkg/objstore/s3/s3.go that should be modified?

Thanks

[juanrh]

I was working using https://github.com/openshift/thanos master as base branch, which is very different to https://github.com/thanos-io/thanos main. So the approach above (https://github.com/juanrh/thanos/commits/5663) won't work. I'll investigate how to solve this for the correct base branch

[juanrh]

I sent a draft PR thanos-io/objstore#25 for this, please let me know what you think about the approach

I've also seen the following references to the object store configuration for s3 in this repo, but I'm not sure if this is relevant, please advice. Below is a potential change that is currently not required in the early manual testing I did in thanos-io/objstore#25

$ git diff internal/
diff --git a/internal/cortex/storage/bucket/s3/bucket_client.go b/internal/cortex/storage/bucket/s3/bucket_client.go
index 0d49690c..24249387 100644
--- a/internal/cortex/storage/bucket/s3/bucket_client.go
+++ b/internal/cortex/storage/bucket/s3/bucket_client.go
@@ -42,6 +42,7 @@ func newS3Config(cfg Config) (s3.Config, error) {
                Region:    cfg.Region,
                AccessKey: cfg.AccessKeyID,
                SecretKey: cfg.SecretAccessKey.Value,
+               PutUserMetadata: cfg.PutUserMetadata,
                Insecure:  cfg.Insecure,
                SSEConfig: sseCfg,
                HTTPConfig: s3.HTTPConfig{
diff --git a/internal/cortex/storage/bucket/s3/config.go b/internal/cortex/storage/bucket/s3/config.go
index bf5450b3..192ef5d2 100644
--- a/internal/cortex/storage/bucket/s3/config.go
+++ b/internal/cortex/storage/bucket/s3/config.go
@@ -60,6 +60,7 @@ type Config struct {
        BucketName       string         `yaml:"bucket_name"`
        SecretAccessKey  flagext.Secret `yaml:"secret_access_key"`
        AccessKeyID      string         `yaml:"access_key_id"`
+       PutUserMetadata  map[string]string  `yaml:"put_user_metadata"`
        Insecure         bool           `yaml:"insecure"`
        SignatureVersion string         `yaml:"signature_version"`

[amitsetty]

This looks great!

[juanrh]

@yeya24 thanos-io/objstore#25 is ready for review when you have some time. Thanks

[juanrh]

@yeya24 can you please take a look to thanos-io/objstore#25 when you have some time? Thanks

[matej-g]

We'll need to bump objstore but the change is in via thanos-io/objstore#25 already 👍

[juanrh]

Thanks for taking a look @matej-g !