Skip to content

Commit

Permalink
Add configurable API Timeouts
Browse files Browse the repository at this point in the history
Multiple limitations exist with existing code:

- Human operator cannot configure Apache Timeout, so it's fixed at the
  defaults of 60 seconds.

  This can be problematic for cinder backends that under load can take
  longer on the export and mapping.

- Default HAProxy timeout is 30 seconds, which is different than the
  default in OSP17 (60 seconds)

- Human operators would need to sync values between 3 places: HAProxy,
  Apache, oslo.messaging sync RPC (call).

This patch adds an `apiTimeout` field to the `CinderSpecBase` to allow
human operators to simultaneously configure the timeouts for HAProxy,
Apache, and the `rpc_response_timeout`.

The `apiTimeout` defaults to 60 seconds, to mimic the behavior present
in OSP17.

Having different timeouts for HAProxy, Apache, and
`rpc_response_timeout` is possible setting the HAProxy timeout in the
`apiOverride`, the Apache timeout with `apiTimeout`, and setting
`rpc_response_timeout` in the top level Cinder `customServiceConfig`.

To be able to change the HAProxy value based on the `apiTimeout` with
any update (and not just the first time) the code adds a custom
annotation "api.cinder.openstack.org/timeout" with the value that was
initially set, this way flags it as being set by the cinder-operator.

Jira: https://issues.redhat.com/browse/OSPRH-7393
  • Loading branch information
Akrog committed Jun 6, 2024
1 parent 4afc791 commit bfb7a85
Show file tree
Hide file tree
Showing 7 changed files with 46 additions and 0 deletions.
4 changes: 4 additions & 0 deletions api/bases/cinder.openstack.org_cinders.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,10 @@ spec:
type: object
spec:
properties:
apiTimeout:
default: 60
minimum: 10
type: integer
cinderAPI:
properties:
containerImage:
Expand Down
6 changes: 6 additions & 0 deletions api/v1beta1/cinder_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,12 @@ type CinderSpecBase struct {
// +kubebuilder:validation:Optional
// DBPurge parameters -
DBPurge DBPurge `json:"dbPurge,omitempty"`

// +kubebuilder:validation:Optional
// +kubebuilder:default=60
// +kubebuilder:validation:Minimum=10
// APITimeout for HAProxy, Apache, and rpc_response_timeout
APITimeout int `json:"apiTimeout"`
}

// CinderSpecCore the same as CinderSpec without ContainerImage references
Expand Down
26 changes: 26 additions & 0 deletions api/v1beta1/cinder_webhook.go
Original file line number Diff line number Diff line change
Expand Up @@ -222,3 +222,29 @@ func (r *Cinder) ValidateDelete() (admission.Warnings, error) {
// TODO(user): fill in your validation logic upon object deletion.
return nil, nil
}

// SetDefaultRouteAnnotations sets HAProxy timeout values of the route
func (spec *CinderSpecCore) SetDefaultRouteAnnotations(annotations map[string]string) {
const haProxyAnno = "haproxy.router.openshift.io/timeout"
// Use a custom annotation to flag when the operator has set the default HAProxy timeout
// With the annotation func determines when to overwrite existing HAProxy timeout with the APITimeout
const cinderAnno = "api.cinder.openstack.org/timeout"

valCinder, okCinder := annotations[cinderAnno]
valHAProxy, okHAProxy := annotations[haProxyAnno]

// Human operator set the HAProxy timeout manually
if (!okCinder && okHAProxy) {
return
}

// Human operator modified the HAProxy timeout manually without removing the Cinder flag
if (okCinder && okHAProxy && valCinder != valHAProxy) {
delete(annotations, cinderAnno)
return
}

timeout := fmt.Sprintf("%ds", spec.APITimeout)
annotations[cinderAnno] = timeout
annotations[haProxyAnno] = timeout
}
4 changes: 4 additions & 0 deletions config/crd/bases/cinder.openstack.org_cinders.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,10 @@ spec:
type: object
spec:
properties:
apiTimeout:
default: 60
minimum: 10
type: integer
cinderAPI:
properties:
containerImage:
Expand Down
1 change: 1 addition & 0 deletions controllers/cinder_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -954,6 +954,7 @@ func (r *CinderReconciler) generateServiceConfigs(
instance.Status.DatabaseHostname,
cinder.DatabaseName)
templateParameters["MemcachedServersWithInet"] = memcached.GetMemcachedServerListWithInetString()
templateParameters["TimeOut"] = instance.Spec.APITimeout

// create httpd vhost template parameters
httpdVhostConfig := map[string]interface{}{}
Expand Down
3 changes: 3 additions & 0 deletions templates/cinder/config/00-global-defaults.conf
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,9 @@ osapi_volume_workers = 4
control_exchange = openstack
api_paste_config = /etc/cinder/api-paste.ini

# Keep the RPC call timeout in sync with HAProxy and Apache timeouts
rpc_response_timeout = {{ .TimeOut }}

[barbican]
auth_endpoint = {{ .KeystoneInternalURL }}
barbican_endpoint_type = internal
Expand Down
2 changes: 2 additions & 0 deletions templates/cinder/config/10-cinder_wsgi.conf
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@
Require all granted
</Directory>

Timeout {{ $.TimeOut }}

## Logging
ErrorLog /dev/stdout
ServerSignature Off
Expand Down

0 comments on commit bfb7a85

Please sign in to comment.