Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
add tests + nits
  • Loading branch information
ashwanthgoli committed Oct 30, 2024
commit da20d380cbef3f304e9c0a7fa2de6e92eea0eb77
8 changes: 2 additions & 6 deletions pkg/storage/bucket/named_stores.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package bucket

import (
"fmt"
"slices"

"github.com/grafana/loki/v3/pkg/storage/bucket/azure"
"github.com/grafana/loki/v3/pkg/storage/bucket/filesystem"
Expand Down Expand Up @@ -38,8 +39,7 @@ func (ns *NamedStores) populateStoreType() error {
ns.storeType = make(map[string]string)

checkForDuplicates := func(name string) error {
switch name {
case S3, GCS, Azure, Swift, Filesystem:
if slices.Contains(SupportedBackends, name) {
return fmt.Errorf("named store %q should not match with the name of a predefined storage type", name)
}

Expand Down Expand Up @@ -89,10 +89,6 @@ func (ns *NamedStores) populateStoreType() error {
}

func (ns *NamedStores) LookupStoreType(name string) (string, bool) {
if ns == nil {
return "", false
}

st, ok := ns.storeType[name]
return st, ok
}
Expand Down
92 changes: 92 additions & 0 deletions pkg/storage/bucket/named_stores_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,92 @@
package bucket

import (
"testing"

"github.com/stretchr/testify/require"

"github.com/grafana/loki/v3/pkg/storage/bucket/gcs"
)

func TestNamedStores_populateStoreType(t *testing.T) {
t.Run("found duplicates", func(t *testing.T) {
ns := NamedStores{
S3: map[string]NamedS3StorageConfig{
"store-1": {},
"store-2": {},
},
GCS: map[string]NamedGCSStorageConfig{
"store-1": {},
},
}

err := ns.populateStoreType()
require.ErrorContains(t, err, `named store "store-1" is already defined under`)

})

t.Run("illegal store name", func(t *testing.T) {
ns := NamedStores{
GCS: map[string]NamedGCSStorageConfig{
"s3": {},
},
}

err := ns.populateStoreType()
require.ErrorContains(t, err, `named store "s3" should not match with the name of a predefined storage type`)

})

t.Run("lookup populated entries", func(t *testing.T) {
ns := NamedStores{
S3: map[string]NamedS3StorageConfig{
"store-1": {},
"store-2": {},
},
GCS: map[string]NamedGCSStorageConfig{
"store-3": {},
},
}

err := ns.populateStoreType()
require.NoError(t, err)

storeType, ok := ns.LookupStoreType("store-1")
require.True(t, ok)
require.Equal(t, S3, storeType)

storeType, ok = ns.LookupStoreType("store-2")
require.True(t, ok)
require.Equal(t, S3, storeType)

storeType, ok = ns.LookupStoreType("store-3")
require.True(t, ok)
require.Equal(t, GCS, storeType)

_, ok = ns.LookupStoreType("store-4")
require.False(t, ok)
})
}

func TestNamedStores_OverrideConfig(t *testing.T) {
namedStoreCfg := NamedStores{
GCS: map[string]NamedGCSStorageConfig{
"store-1": {
BucketName: "bar",
ChunkBufferSize: 100,
},
"store-2": {
BucketName: "baz",
},
},
}

storeCfg := Config{
GCS: gcs.Config{
BucketName: "foo",
},
}
namedStoreCfg.OverrideConfig(&storeCfg, "store-1", GCS)

Check failure on line 89 in pkg/storage/bucket/named_stores_test.go

View workflow job for this annotation

GitHub Actions / check / golangciLint

Error return value of `namedStoreCfg.OverrideConfig` is not checked (errcheck)
require.Equal(t, "bar", storeCfg.GCS.BucketName)
require.Equal(t, 100, storeCfg.GCS.ChunkBufferSize)
}
8 changes: 4 additions & 4 deletions pkg/storage/bucket/object_client_adapter.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,26 +38,26 @@ func NewObjectClient(ctx context.Context, backend string, cfg ConfigWithNamedSto
}
}

b, err := NewClient(ctx, storeType, storeCfg, component, logger, nil)
bucket, err := NewClient(ctx, storeType, storeCfg, component, logger, nil)
if err != nil {
return nil, fmt.Errorf("create bucket: %w", err)
}

hedgedBucket := b
hedgedBucket := bucket
if hedgingCfg.At != 0 {
hedgedTrasport, err := hedgingCfg.RoundTripperWithRegisterer(nil, prometheus.WrapRegistererWithPrefix("loki_", prometheus.DefaultRegisterer))
if err != nil {
return nil, fmt.Errorf("create hedged transport: %w", err)
}

b, err = NewClient(ctx, storeType, storeCfg, component, logger, hedgedTrasport)
bucket, err = NewClient(ctx, storeType, storeCfg, component, logger, hedgedTrasport)
if err != nil {
return nil, fmt.Errorf("create hedged bucket: %w", err)
}
}

o := &ObjectClientAdapter{
bucket: b,
bucket: bucket,
hedgedBucket: hedgedBucket,
logger: log.With(logger, "component", "bucket_to_object_client_adapter"),
// default to no retryable errors. Override with WithRetryableErrFunc
Expand Down
Loading