Fixes from testing

pkg/storage/bucket/azure/bucket_client.go

@@ -5,39 +5,38 @@ import (
 	"github.com/prometheus/common/model"
 	"github.com/thanos-io/objstore"
 	"github.com/thanos-io/objstore/providers/azure"
-	yaml "gopkg.in/yaml.v2"
 )
 
 func NewBucketClient(cfg Config, name string, logger log.Logger) (objstore.Bucket, error) {
-	bucketConfig := azure.Config{
-		StorageAccountName:      cfg.StorageAccountName,
-		StorageAccountKey:       cfg.StorageAccountKey.String(),
-		StorageConnectionString: cfg.StorageConnectionString.String(),
-		ContainerName:           cfg.ContainerName,
-		Endpoint:                cfg.EndpointSuffix,
-		MaxRetries:              cfg.MaxRetries,
-		UserAssignedID:          cfg.UserAssignedID,
-		PipelineConfig: azure.PipelineConfig{
-			MaxRetryDelay: model.Duration(cfg.MaxRetryDelay),
-		},
-		HTTPConfig: azure.HTTPConfig{
-			IdleConnTimeout:       model.Duration(cfg.HTTP.IdleConnTimeout),
-			ResponseHeaderTimeout: model.Duration(cfg.HTTP.ResponseHeaderTimeout),
-			InsecureSkipVerify:    cfg.HTTP.InsecureSkipVerify,
-			TLSHandshakeTimeout:   model.Duration(cfg.HTTP.TLSHandshakeTimeout),
-			ExpectContinueTimeout: model.Duration(cfg.HTTP.ExpectContinueTimeout),
-			MaxIdleConns:          cfg.HTTP.MaxIdleConns,
-			MaxIdleConnsPerHost:   cfg.HTTP.MaxIdleConnsPerHost,
-			MaxConnsPerHost:       cfg.HTTP.MaxConnsPerHost,
-		},
+	return newBucketClient(cfg, name, logger, azure.NewBucketWithConfig)
+}
+
+func newBucketClient(cfg Config, name string, logger log.Logger, factory func(log.Logger, azure.Config, string) (*azure.Bucket, error)) (objstore.Bucket, error) {
+	// Start with default config to make sure that all parameters are set to sensible values, especially
+	// HTTP Config field.
+	bucketConfig := azure.DefaultConfig
+	bucketConfig.StorageAccountName = cfg.StorageAccountName
+	bucketConfig.StorageAccountKey = cfg.StorageAccountKey.String()
+	bucketConfig.ContainerName = cfg.ContainerName
+	bucketConfig.MaxRetries = cfg.MaxRetries
+	bucketConfig.UserAssignedID = cfg.UserAssignedID
+	bucketConfig.PipelineConfig.MaxRetryDelay = model.Duration(cfg.MaxRetryDelay)
+
+	bucketConfig.HTTPConfig = azure.HTTPConfig{
+		IdleConnTimeout:       model.Duration(cfg.HTTP.IdleConnTimeout),
+		ResponseHeaderTimeout: model.Duration(cfg.HTTP.ResponseHeaderTimeout),
+		InsecureSkipVerify:    cfg.HTTP.InsecureSkipVerify,
+		TLSHandshakeTimeout:   model.Duration(cfg.HTTP.TLSHandshakeTimeout),
+		ExpectContinueTimeout: model.Duration(cfg.HTTP.ExpectContinueTimeout),
+		MaxIdleConns:          cfg.HTTP.MaxIdleConns,
+		MaxIdleConnsPerHost:   cfg.HTTP.MaxIdleConnsPerHost,
+		MaxConnsPerHost:       cfg.HTTP.MaxConnsPerHost,
 	}
 
-	// Thanos currently doesn't support passing the config as is, but expects a YAML,
-	// so we're going to serialize it.
-	serialized, err := yaml.Marshal(bucketConfig)
-	if err != nil {
-		return nil, err
+	if cfg.Endpoint != "" {
+		// azure.DefaultConfig has the default Endpoint, overwrite it only if a different one was explicitly provided.
+		bucketConfig.Endpoint = cfg.Endpoint
 	}
 
-	return azure.NewBucket(logger, serialized, name)
+	return factory(logger, bucketConfig, name)
 }

pkg/storage/bucket/azure/config.go

@@ -24,7 +24,7 @@ type Config struct {
 	StorageAccountKey       flagext.Secret `yaml:"account_key"`
 	StorageConnectionString flagext.Secret `yaml:"connection_string"`
 	ContainerName           string         `yaml:"container_name"`
-	EndpointSuffix          string         `yaml:"endpoint_suffix"`
+	Endpoint                string         `yaml:"endpoint_suffix"`
 	UserAssignedID          string         `yaml:"user_assigned_id"`
 	MaxRetries              int            `yaml:"max_retries"`
 	MaxRetryDelay           time.Duration  `yaml:"max_retry_delay"`
@@ -43,7 +43,7 @@ func (cfg *Config) RegisterFlagsWithPrefix(prefix string, f *flag.FlagSet) {
 	f.Var(&cfg.StorageAccountKey, prefix+"azure.account-key", "Azure storage account key")
 	f.Var(&cfg.StorageConnectionString, prefix+"azure.connection-string", "If `connection-string` is set, the values of `account-name` and `endpoint-suffix` values will not be used. Use this method over `account-key` if you need to authenticate via a SAS token. Or if you use the Azurite emulator.")
 	f.StringVar(&cfg.ContainerName, prefix+"azure.container-name", "loki", "Azure storage container name")
-	f.StringVar(&cfg.EndpointSuffix, prefix+"azure.endpoint-suffix", "", "Azure storage endpoint suffix without schema. The account name will be prefixed to this value to create the FQDN")
+	f.StringVar(&cfg.Endpoint, prefix+"azure.endpoint-suffix", "", "Azure storage endpoint suffix without schema. The account name will be prefixed to this value to create the FQDN")
 	f.StringVar(&cfg.UserAssignedID, prefix+"azure.user-assigned-id", "", "User assigned identity ID to authenticate to the Azure storage account.")
 	f.IntVar(&cfg.MaxRetries, prefix+"azure.max-retries", 20, "Number of retries for recoverable errors")
 	f.DurationVar(&cfg.MaxRetryDelay, prefix+"azure.max-retry-delay", 500*time.Millisecond, "Maximum time to wait before retrying a request.")

pkg/storage/bucket/azure/config_test.go

@@ -15,6 +15,7 @@ import (
 var defaultConfig = Config{
 	ContainerName: "loki",
 	MaxRetries:    20,
+	MaxRetryDelay: 500000000,
 	HTTP: HTTPConfig{
 		Config: http.Config{
 			IdleConnTimeout:       90 * time.Second,
@@ -50,6 +51,7 @@ connection_string: test-connection-string
 container_name: test-container-name
 endpoint_suffix: test-endpoint-suffix
 max_retries: 1
+max_retry_delay: 500000000
 http:
   idle_conn_timeout: 2s
   response_header_timeout: 3s
@@ -65,8 +67,9 @@ http:
 				StorageAccountKey:       flagext.SecretWithValue("test-account-key"),
 				StorageConnectionString: flagext.SecretWithValue("test-connection-string"),
 				ContainerName:           "test-container-name",
-				EndpointSuffix:          "test-endpoint-suffix",
+				Endpoint:                "test-endpoint-suffix",
 				MaxRetries:              1,
+				MaxRetryDelay: 500000000,
 				HTTP: HTTPConfig{
 					Config: http.Config{
 						IdleConnTimeout:       2 * time.Second,

pkg/storage/factory.go

@@ -734,6 +734,7 @@ func internalNewObjectClient(component, name string, cfg Config, clientMetrics C
 			azureCfg = (azure.BlobStorageConfig)(nsCfg)
 		}
 		if cfg.ThanosObjStore {
+			clientMetrics.Unregister()
 			azure.NewBlobStorageThanosObjectClient(context.Background(), cfg.ObjStoreConf, component, util_log.Logger, cfg.Hedging, reg)
 		}
 		return azure.NewBlobStorage(&azureCfg, clientMetrics.AzureMetrics, cfg.Hedging)