From c2f588fc2b1a598d87e51b8438864931efd97b5f Mon Sep 17 00:00:00 2001 From: Joao Marcal Date: Fri, 15 Nov 2024 10:05:40 +0000 Subject: [PATCH] address review comments and fix merge result --- docs/sources/shared/configuration.md | 65 ++++++++++++++++++- pkg/storage/bucket/client.go | 2 +- pkg/storage/bucket/http/config.go | 26 ++++---- pkg/storage/bucket/s3/config.go | 2 +- pkg/storage/bucket/swift/bucket_client.go | 2 - .../client/openstack/swift_object_client.go | 2 +- .../openstack/swift_thanos_object_client.go | 44 ------------- pkg/storage/factory.go | 3 - 8 files changed, 78 insertions(+), 68 deletions(-) delete mode 100644 pkg/storage/chunk/client/openstack/swift_thanos_object_client.go diff --git a/docs/sources/shared/configuration.md b/docs/sources/shared/configuration.md index 64d2a815ab410..9bbc98f77527d 100644 --- a/docs/sources/shared/configuration.md +++ b/docs/sources/shared/configuration.md @@ -5846,6 +5846,18 @@ The `swift_storage_config` block configures the connection to OpenStack Object S # CLI flag: -.swift.internal [internal: | default = false] +# OpenStack Swift application credential id +# CLI flag: -.swift.application-credential-id +[application_credential_id: | default = ""] + +# OpenStack Swift application credential name +# CLI flag: -.swift.application-credential-name +[application_credential_name: | default = ""] + +# OpenStack Swift application credential secret +# CLI flag: -.swift.application-credential-secret +[application_credential_secret: | default = ""] + # OpenStack Swift authentication API version. 0 to autodetect. # CLI flag: -.swift.auth-version [auth_version: | default = 0] @@ -5922,11 +5934,62 @@ The `swift_storage_config` block configures the connection to OpenStack Object S # CLI flag: -.swift.request-timeout [request_timeout: | default = 5s] -http_config: +http: + # The time an idle connection will remain idle before closing. + # CLI flag: -.swift.idle-conn-timeout + [idle_conn_timeout: | default = 1m30s] + + # The amount of time the client will wait for a servers response headers. + # CLI flag: -.swift.response-header-timeout + [response_header_timeout: | default = 2m] + + # If the client connects via HTTPS and this option is enabled, the client will + # accept any certificate and hostname. + # CLI flag: -.swift.insecure-skip-verify + [insecure_skip_verify: | default = false] + + # Maximum time to wait for a TLS handshake. 0 means no limit. + # CLI flag: -.swift.tls-handshake-timeout + [tls_handshake_timeout: | default = 10s] + + # The time to wait for a server's first response headers after fully writing + # the request headers if the request has an Expect header. 0 to send the + # request body immediately. + # CLI flag: -.swift.expect-continue-timeout + [expect_continue_timeout: | default = 1s] + + # Maximum number of idle (keep-alive) connections across all hosts. 0 means no + # limit. + # CLI flag: -.swift.max-idle-connections + [max_idle_connections: | default = 100] + + # Maximum number of idle (keep-alive) connections to keep per-host. If 0, a + # built-in default value is used. + # CLI flag: -.swift.max-idle-connections-per-host + [max_idle_connections_per_host: | default = 100] + + # Maximum number of connections per host. 0 means no limit. + # CLI flag: -.swift.max-connections-per-host + [max_connections_per_host: | default = 0] + # Path to the CA certificates to validate server certificate against. If not # set, the host's root CA certificates are used. # CLI flag: -.swift.http.tls-ca-path [tls_ca_path: | default = ""] + + # Path to the client certificate, which will be used for authenticating with + # the server. Also requires the key path to be configured. + # CLI flag: -.swift.http.tls-cert-path + [tls_cert_path: | default = ""] + + # Path to the key for the client certificate. Also requires the client + # certificate to be configured. + # CLI flag: -.swift.http.tls-key-path + [tls_key_path: | default = ""] + + # Override the expected name on the server certificate. + # CLI flag: -.swift.http.tls-server-name + [tls_server_name: | default = ""] ``` ### table_manager diff --git a/pkg/storage/bucket/client.go b/pkg/storage/bucket/client.go index 64338e8f02a18..d47332182fb4e 100644 --- a/pkg/storage/bucket/client.go +++ b/pkg/storage/bucket/client.go @@ -156,7 +156,7 @@ func (cfg *Config) configureTransport(backend string, rt http.RoundTripper) erro case Azure: cfg.Azure.Transport = rt case Swift: - cfg.Swift.Transport = rt + cfg.Swift.HTTP.Transport = rt case Filesystem: // do nothing default: diff --git a/pkg/storage/bucket/http/config.go b/pkg/storage/bucket/http/config.go index 02f69c1574d22..50f362dab01a9 100644 --- a/pkg/storage/bucket/http/config.go +++ b/pkg/storage/bucket/http/config.go @@ -6,21 +6,17 @@ import ( "time" ) -// NOTE some of the fields are hidden in the documentation due to this struct -// being by the old Swift storage backend. The hidden fields can be unhidden -// when we deprecate the old clients. - // Config stores the http.Client configuration for the storage clients. type Config struct { - IdleConnTimeout time.Duration `yaml:"idle_conn_timeout" doc:"hidden"` - ResponseHeaderTimeout time.Duration `yaml:"response_header_timeout" doc:"hidden"` - InsecureSkipVerify bool `yaml:"insecure_skip_verify" doc:"hidden"` + IdleConnTimeout time.Duration `yaml:"idle_conn_timeout"` + ResponseHeaderTimeout time.Duration `yaml:"response_header_timeout"` + InsecureSkipVerify bool `yaml:"insecure_skip_verify"` - TLSHandshakeTimeout time.Duration `yaml:"tls_handshake_timeout" doc:"hidden"` - ExpectContinueTimeout time.Duration `yaml:"expect_continue_timeout" doc:"hidden"` - MaxIdleConns int `yaml:"max_idle_connections" doc:"hidden"` - MaxIdleConnsPerHost int `yaml:"max_idle_connections_per_host" doc:"hidden"` - MaxConnsPerHost int `yaml:"max_connections_per_host" doc:"hidden"` + TLSHandshakeTimeout time.Duration `yaml:"tls_handshake_timeout"` + ExpectContinueTimeout time.Duration `yaml:"expect_continue_timeout"` + MaxIdleConns int `yaml:"max_idle_connections"` + MaxIdleConnsPerHost int `yaml:"max_idle_connections_per_host"` + MaxConnsPerHost int `yaml:"max_connections_per_host"` // Allow upstream callers to inject a round tripper Transport http.RoundTripper `yaml:"-"` @@ -31,9 +27,9 @@ type Config struct { // TLSConfig configures the options for TLS connections. type TLSConfig struct { CAPath string `yaml:"tls_ca_path" category:"advanced"` - CertPath string `yaml:"tls_cert_path" category:"advanced" doc:"hidden"` - KeyPath string `yaml:"tls_key_path" category:"advanced" doc:"hidden"` - ServerName string `yaml:"tls_server_name" category:"advanced" doc:"hidden"` + CertPath string `yaml:"tls_cert_path" category:"advanced"` + KeyPath string `yaml:"tls_key_path" category:"advanced"` + ServerName string `yaml:"tls_server_name" category:"advanced"` } // RegisterFlags registers the flags for the storage HTTP client. diff --git a/pkg/storage/bucket/s3/config.go b/pkg/storage/bucket/s3/config.go index 564227009d63f..fe00c19c92a34 100644 --- a/pkg/storage/bucket/s3/config.go +++ b/pkg/storage/bucket/s3/config.go @@ -73,7 +73,7 @@ type Config struct { MaxRetries int `yaml:"max_retries"` SSE SSEConfig `yaml:"sse"` - HTTP http.Config `yaml:"http_config"` + HTTP http.Config `yaml:"http"` TraceConfig TraceConfig `yaml:"trace"` } diff --git a/pkg/storage/bucket/swift/bucket_client.go b/pkg/storage/bucket/swift/bucket_client.go index 712390d2de84c..106a656fda2d2 100644 --- a/pkg/storage/bucket/swift/bucket_client.go +++ b/pkg/storage/bucket/swift/bucket_client.go @@ -53,9 +53,7 @@ func NewBucketClient(cfg Config, _ string, logger log.Logger) (objstore.Bucket, // Hard-coded defaults. ChunkSize: swift.DefaultConfig.ChunkSize, UseDynamicLargeObjects: false, - HTTPConfig: exthttp.DefaultHTTPConfig, } - bucketConfig.HTTPConfig.Transport = cfg.Transport return swift.NewContainerFromConfig(logger, &bucketConfig, false, nil) } diff --git a/pkg/storage/chunk/client/openstack/swift_object_client.go b/pkg/storage/chunk/client/openstack/swift_object_client.go index 017682c5e2921..3c3c6fc03406d 100644 --- a/pkg/storage/chunk/client/openstack/swift_object_client.go +++ b/pkg/storage/chunk/client/openstack/swift_object_client.go @@ -47,7 +47,7 @@ func defaultTransport(config bucket_http.Config) (http.RoundTripper, error) { MaxIdleConns: 200, MaxIdleConnsPerHost: 200, ExpectContinueTimeout: 5 * time.Second, - TLSClientConfig: tlsConfig, + TLSClientConfig: tlsConfig, }, nil } diff --git a/pkg/storage/chunk/client/openstack/swift_thanos_object_client.go b/pkg/storage/chunk/client/openstack/swift_thanos_object_client.go deleted file mode 100644 index 4630e89c0f9a6..0000000000000 --- a/pkg/storage/chunk/client/openstack/swift_thanos_object_client.go +++ /dev/null @@ -1,44 +0,0 @@ -package openstack - -import ( - "context" - - "github.com/go-kit/log" - "github.com/prometheus/client_golang/prometheus" - "github.com/thanos-io/objstore" - - "github.com/grafana/loki/v3/pkg/storage/bucket" - "github.com/grafana/loki/v3/pkg/storage/chunk/client" - "github.com/grafana/loki/v3/pkg/storage/chunk/client/hedging" -) - -func NewSwiftThanosObjectClient(ctx context.Context, cfg bucket.Config, component string, logger log.Logger, hedgingCfg hedging.Config) (client.ObjectClient, error) { - b, err := newSwiftThanosObjectClient(ctx, cfg, component, logger, false, hedgingCfg) - if err != nil { - return nil, err - } - - var hedged objstore.Bucket - if hedgingCfg.At != 0 { - hedged, err = newSwiftThanosObjectClient(ctx, cfg, component, logger, true, hedgingCfg) - if err != nil { - return nil, err - } - } - - o := bucket.NewObjectClientAdapter(b, hedged, logger, bucket.WithRetryableErrFunc(IsRetryableErr)) - return o, nil -} - -func newSwiftThanosObjectClient(ctx context.Context, cfg bucket.Config, component string, logger log.Logger, hedging bool, hedgingCfg hedging.Config) (objstore.Bucket, error) { - if hedging { - hedgedTrasport, err := hedgingCfg.RoundTripperWithRegisterer(nil, prometheus.WrapRegistererWithPrefix("loki_", prometheus.DefaultRegisterer)) - if err != nil { - return nil, err - } - - cfg.Swift.HTTP.Transport = hedgedTrasport - } - - return bucket.NewClient(ctx, bucket.Swift, cfg, component, logger) -} diff --git a/pkg/storage/factory.go b/pkg/storage/factory.go index 77d704284b162..bc2257a64a876 100644 --- a/pkg/storage/factory.go +++ b/pkg/storage/factory.go @@ -714,9 +714,6 @@ func internalNewObjectClient(storeName, component string, cfg Config, clientMetr } swiftCfg = (openstack.SwiftConfig)(nsCfg) } - if cfg.UseThanosObjstore { - return openstack.NewSwiftThanosObjectClient(context.Background(), cfg.ObjectStore, component, util_log.Logger, cfg.Hedging) - } return openstack.NewSwiftObjectClient(swiftCfg, cfg.Hedging) case types.StorageTypeFileSystem: