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

operator: Add support for configuring HTTP server timeouts #9405

Merged
merged 35 commits into from
May 22, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
35 commits
Select commit Hold shift + click to select a range
70d3ea6
operator: Add LokiStack http server limits spec
periklis May 5, 2023
5932c37
Parse server config options from limits spec
periklis May 5, 2023
f5eb750
Add OpenShift route timeout annotation
periklis May 5, 2023
643fc08
Pass server options to loki config
periklis May 5, 2023
e9aaf56
Fix defaults
periklis May 5, 2023
c3a6557
Apply http server and upstream timeouts to the gateway
periklis May 5, 2023
268daaa
Add changelog entry
periklis May 5, 2023
d19eb9d
Add api docs
periklis May 5, 2023
89c7465
Merge branch 'main' into operator-fix-write-timeouts
periklis May 8, 2023
a37e1a2
Add operator-sdk API markers
periklis May 8, 2023
11c531e
Enhance error log message in lokistack handler
periklis May 8, 2023
1c661c5
Merge branch 'main' into operator-fix-write-timeouts
periklis May 8, 2023
08ae251
Merge branch 'main' into operator-fix-write-timeouts
periklis May 9, 2023
9079100
Apply code review suggestions
periklis May 15, 2023
2723949
Merge remote-tracking branch 'upstream' into operator-fix-write-timeouts
periklis May 15, 2023
146a034
Fix merge
periklis May 15, 2023
0bff0e9
Merge remote-tracking branch 'upstream' into operator-fix-write-timeouts
periklis May 17, 2023
10f668e
Regenerate
periklis May 17, 2023
d5e0bbb
Make defaults a global variable, remove pointer
xperimental May 17, 2023
d5d883d
Make defaults more visible in parse function
xperimental May 17, 2023
42e5b49
Unify defaults and simplify NewServerConfig
xperimental May 17, 2023
642dfba
Rename constants
xperimental May 17, 2023
094a59d
Use time.Duration in config template
xperimental May 17, 2023
cd9731d
Reduce indirection in config generator
xperimental May 17, 2023
a732ece
Separate config structs for Loki and gateway
xperimental May 17, 2023
9c4bb0b
Rename configuration struct
xperimental May 17, 2023
47d56cc
Merge remote-tracking branch 'upstream' into operator-fix-write-timeouts
periklis May 17, 2023
12e6080
Rename TestNewServerConfig to TestNewTimeoutConfig
periklis May 17, 2023
c441848
Raise error instead of using defaults
xperimental May 22, 2023
3382fac
Shorten gateway route timeout
xperimental May 22, 2023
4cb456c
Increase default query timeout to match existing default
xperimental May 22, 2023
11d0ebf
Regenerate docs
periklis May 22, 2023
43be4ef
Merge remote-tracking branch 'upstream' into operator-fix-write-timeouts
periklis May 22, 2023
38fc277
Fix failing tests
periklis May 22, 2023
7a2d0d2
Update import formatting to satisfy check
xperimental May 22, 2023
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
Prev Previous commit
Next Next commit
Parse server config options from limits spec
  • Loading branch information
periklis committed May 5, 2023
commit 5932c37869d685ff71782d4716266e959974612b
6 changes: 6 additions & 0 deletions operator/internal/handlers/lokistack_create_or_update.go
Original file line number Diff line number Diff line change
Expand Up @@ -269,6 +269,11 @@ func CreateOrUpdateLokiStack(
certRotationRequiredAt = stack.Annotations[manifests.AnnotationCertRotationRequiredAt]
}

serverCfg, err := manifests.NewServerConfig(stack.Spec.Limits)
if err != nil {
ll.Error(err, "failed to parser server limits")
}

// Here we will translate the lokiv1.LokiStack options into manifest options
opts := manifests.Options{
Name: req.Name,
Expand All @@ -286,6 +291,7 @@ func CreateOrUpdateLokiStack(
Spec: rulerConfig,
Secret: rulerSecret,
},
Server: serverCfg,
Tenants: manifests.Tenants{
Secrets: tenantSecrets,
Configs: tenantConfigs,
Expand Down
1 change: 1 addition & 0 deletions operator/internal/manifests/gateway_tenants.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ func ApplyGatewayDefaultOptions(opts *Options) error {
opts.GatewayBaseDomain,
serviceNameGatewayHTTP(opts.Name),
gatewayHTTPPortName,
opts.Server.HTTP.GatewayWriteTimeout,
ComponentLabels(LabelGatewayComponent, opts.Name),
tenantData,
RulerName(opts.Name),
Expand Down
23 changes: 23 additions & 0 deletions operator/internal/manifests/gateway_tenants_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package manifests
import (
"path"
"testing"
"time"

monitoringv1 "github.com/prometheus-operator/prometheus-operator/pkg/apis/monitoring/v1"
"github.com/stretchr/testify/require"
Expand Down Expand Up @@ -70,6 +71,11 @@ func TestApplyGatewayDefaultsOptions(t *testing.T) {
Mode: lokiv1.OpenshiftLogging,
},
},
Server: ServerConfig{
HTTP: &HTTPConfig{
GatewayWriteTimeout: 1 * time.Minute,
},
},
Tenants: Tenants{
Configs: map[string]TenantConfig{
"application": {
Expand Down Expand Up @@ -99,6 +105,11 @@ func TestApplyGatewayDefaultsOptions(t *testing.T) {
Mode: lokiv1.OpenshiftLogging,
},
},
Server: ServerConfig{
HTTP: &HTTPConfig{
GatewayWriteTimeout: 1 * time.Minute,
},
},
Tenants: Tenants{
Configs: map[string]TenantConfig{
"application": {
Expand All @@ -125,6 +136,7 @@ func TestApplyGatewayDefaultsOptions(t *testing.T) {
GatewayName: "lokistack-ocp-gateway",
GatewaySvcName: "lokistack-ocp-gateway-http",
GatewaySvcTargetPort: "public",
GatewayRouteTimeout: 3 * time.Minute,
RulerName: "lokistack-ocp-ruler",
Labels: ComponentLabels(LabelGatewayComponent, "lokistack-ocp"),
},
Expand Down Expand Up @@ -165,6 +177,11 @@ func TestApplyGatewayDefaultsOptions(t *testing.T) {
Mode: lokiv1.OpenshiftNetwork,
},
},
Server: ServerConfig{
HTTP: &HTTPConfig{
GatewayWriteTimeout: 1 * time.Minute,
},
},
Tenants: Tenants{
Configs: map[string]TenantConfig{
"network": {
Expand All @@ -184,6 +201,11 @@ func TestApplyGatewayDefaultsOptions(t *testing.T) {
Mode: lokiv1.OpenshiftNetwork,
},
},
Server: ServerConfig{
HTTP: &HTTPConfig{
GatewayWriteTimeout: 1 * time.Minute,
},
},
Tenants: Tenants{
Configs: map[string]TenantConfig{
"network": {
Expand All @@ -200,6 +222,7 @@ func TestApplyGatewayDefaultsOptions(t *testing.T) {
GatewayName: "lokistack-ocp-gateway",
GatewaySvcName: "lokistack-ocp-gateway-http",
GatewaySvcTargetPort: "public",
GatewayRouteTimeout: 3 * time.Minute,
RulerName: "lokistack-ocp-ruler",
Labels: ComponentLabels(LabelGatewayComponent, "lokistack-ocp"),
},
Expand Down
23 changes: 20 additions & 3 deletions operator/internal/manifests/openshift/build_test.go
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
package openshift

import (
"fmt"
"testing"
"time"

"github.com/stretchr/testify/require"

Expand All @@ -11,7 +13,7 @@ import (
)

func TestBuildGatewayObjects_ClusterRoleRefMatches(t *testing.T) {
opts := NewOptions(lokiv1.OpenshiftLogging, "abc", "ns", "abc", "example.com", "abc", "abc", map[string]string{}, map[string]TenantData{}, "abc")
opts := NewOptions(lokiv1.OpenshiftLogging, "abc", "ns", "abc", "example.com", "abc", "abc", 1*time.Minute, map[string]string{}, map[string]TenantData{}, "abc")

objs := BuildGatewayObjects(opts)
cr := objs[2].(*rbacv1.ClusterRole)
Expand All @@ -22,7 +24,7 @@ func TestBuildGatewayObjects_ClusterRoleRefMatches(t *testing.T) {
}

func TestBuildGatewayObjects_MonitoringClusterRoleRefMatches(t *testing.T) {
opts := NewOptions(lokiv1.OpenshiftLogging, "abc", "ns", "abc", "example.com", "abc", "abc", map[string]string{}, map[string]TenantData{}, "abc")
opts := NewOptions(lokiv1.OpenshiftLogging, "abc", "ns", "abc", "example.com", "abc", "abc", 1*time.Minute, map[string]string{}, map[string]TenantData{}, "abc")

objs := BuildGatewayObjects(opts)
cr := objs[4].(*rbacv1.Role)
Expand All @@ -32,8 +34,23 @@ func TestBuildGatewayObjects_MonitoringClusterRoleRefMatches(t *testing.T) {
require.Equal(t, cr.Name, rb.RoleRef.Name)
}

func TestBuildGatewayObjets_RouteWithTimeoutAnnotation(t *testing.T) {
gwWriteTimeout := 1 * time.Minute
opts := NewOptions(lokiv1.OpenshiftLogging, "abc", "ns", "abc", "example.com", "abc", "abc", gwWriteTimeout, map[string]string{}, map[string]TenantData{}, "abc")

objs := BuildGatewayObjects(opts)
a := objs[0].GetAnnotations()

got, ok := a[annotationGatewayRouteTimeout]
require.True(t, ok)

routeTimeout := gwWriteTimeout + defaultGatewayRouteWiggleRoom
want := fmt.Sprintf("%.fs", routeTimeout.Seconds())
require.Equal(t, want, got)
}

func TestBuildRulerObjects_ClusterRoleRefMatches(t *testing.T) {
opts := NewOptions(lokiv1.OpenshiftLogging, "abc", "ns", "abc", "example.com", "abc", "abc", map[string]string{}, map[string]TenantData{}, "abc")
opts := NewOptions(lokiv1.OpenshiftLogging, "abc", "ns", "abc", "example.com", "abc", "abc", 1*time.Minute, map[string]string{}, map[string]TenantData{}, "abc")

objs := BuildRulerObjects(opts)
sa := objs[1].(*corev1.ServiceAccount)
Expand Down
4 changes: 4 additions & 0 deletions operator/internal/manifests/openshift/options.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package openshift
import (
"fmt"
"math/rand"
"time"

lokiv1 "github.com/grafana/loki/operator/apis/loki/v1"
)
Expand Down Expand Up @@ -41,6 +42,7 @@ type BuildOptions struct {
GatewayName string
GatewaySvcName string
GatewaySvcTargetPort string
GatewayRouteTimeout time.Duration
RulerName string
Labels map[string]string
AlertManagerEnabled bool
Expand All @@ -57,6 +59,7 @@ func NewOptions(
mode lokiv1.ModeType,
stackName, stackNamespace string,
gwName, gwBaseDomain, gwSvcName, gwPortName string,
gwWriteTimeout time.Duration,
gwLabels map[string]string,
tenantConfigMap map[string]TenantData,
rulerName string,
Expand Down Expand Up @@ -88,6 +91,7 @@ func NewOptions(
GatewayName: gwName,
GatewaySvcName: gwSvcName,
GatewaySvcTargetPort: gwPortName,
GatewayRouteTimeout: gwWriteTimeout + defaultGatewayRouteWiggleRoom,
Labels: gwLabels,
RulerName: rulerName,
},
Expand Down
86 changes: 86 additions & 0 deletions operator/internal/manifests/options.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,14 @@ package manifests

import (
"strings"
"time"

configv1 "github.com/grafana/loki/operator/apis/config/v1"
lokiv1 "github.com/grafana/loki/operator/apis/loki/v1"
"github.com/grafana/loki/operator/internal/manifests/internal"
"github.com/grafana/loki/operator/internal/manifests/openshift"
"github.com/grafana/loki/operator/internal/manifests/storage"
"github.com/imdario/mergo"
)

// Options is a set of configuration values to use when building manifests such as resource sizes, etc.
Expand All @@ -34,11 +36,28 @@ type Options struct {

OpenShiftOptions openshift.Options

Server ServerConfig

Tenants Tenants

TLSProfile TLSProfileSpec
}

// HTTPConfig contains the http server configuration options for all Loki components.
type HTTPConfig struct {
IdleTimeout time.Duration
ReadTimeout time.Duration
WriteTimeout time.Duration

GatewayReadTimeout time.Duration
GatewayWriteTimeout time.Duration
}

// ServerConfig contains the server configuration options for all Loki components
type ServerConfig struct {
HTTP *HTTPConfig
}

// Tenants contains the configuration per tenant and secrets for authn/authz.
// Secrets are required only for modes static and dynamic to reconcile the OIDC provider.
// Configs are required only for all modes to reconcile rules and gateway configuration.
Expand Down Expand Up @@ -105,3 +124,70 @@ type TLSProfileSpec struct {
func (o Options) TLSCipherSuites() string {
return strings.Join(o.TLSProfile.Ciphers, ",")
}

// NewServerConfig transforms the Loki LimitsSpec.Server options
// to a NewServerConfig by applying defaults and parsing durations.
func NewServerConfig(s *lokiv1.LimitsSpec) (ServerConfig, error) {
defaults := ServerConfig{
HTTP: &HTTPConfig{
IdleTimeout: lokiDefaultHTTPIdleTimeout,
ReadTimeout: lokiDefaultHTTPReadTimeout,
WriteTimeout: lokiDefaultHTTPWriteTimeout,
GatewayReadTimeout: gatewayDefaultReadTimeout,
GatewayWriteTimeout: gatewayDefaultWriteTimeout,
},
}

if s == nil || s.Server == nil || s.Server.HTTP == nil {
return defaults, nil
}

customCfg := ServerConfig{}

if s.Server.HTTP.IdleTimeout != "" {
idleTimeout, err := time.ParseDuration(s.Server.HTTP.IdleTimeout)
if err != nil {
return defaults, err
}

if customCfg.HTTP == nil {
customCfg.HTTP = &HTTPConfig{}
}

customCfg.HTTP.IdleTimeout = idleTimeout
}

if s.Server.HTTP.ReadTimeout != "" {
readTimeout, err := time.ParseDuration(s.Server.HTTP.ReadTimeout)
if err != nil {
return defaults, err
}

if customCfg.HTTP == nil {
customCfg.HTTP = &HTTPConfig{}
}

customCfg.HTTP.ReadTimeout = readTimeout
customCfg.HTTP.GatewayReadTimeout = readTimeout + gatewayReadWiggleRoom
}

if s.Server.HTTP.WriteTimeout != "" {
writeTimeout, err := time.ParseDuration(s.Server.HTTP.WriteTimeout)
if err != nil {
return defaults, err
}

if customCfg.HTTP == nil {
customCfg.HTTP = &HTTPConfig{}
}

customCfg.HTTP.WriteTimeout = writeTimeout
customCfg.HTTP.GatewayWriteTimeout = writeTimeout + gatewayWriteWiggleRoom
}

if err := mergo.Merge(&defaults, &customCfg, mergo.WithOverride); err != nil {
return defaults, err
}

return defaults, nil
}
Loading