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

Add configurable API Timeout #515

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
5 changes: 5 additions & 0 deletions api/bases/keystone.openstack.org_keystoneapis.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,11 @@ spec:
default: admin
description: AdminUser - admin user name
type: string
apiTimeout:
default: 60
description: APITimeout for HAProxy, Apache
minimum: 10
type: integer
containerImage:
description: Keystone Container Image URL (will be set to environmental
default if empty)
Expand Down
10 changes: 10 additions & 0 deletions api/v1beta1/keystoneapi_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,9 @@ const (

// KeystoneAPIContainerImage is the fall-back container image for KeystoneAPI
KeystoneAPIContainerImage = "quay.io/podified-antelope-centos9/openstack-keystone:current-podified"

// APIDefaultTimeout default timeout for HAProxy, Apache
APIDefaultTimeout = 60
)

type KeystoneAPISpec struct {
Expand Down Expand Up @@ -184,6 +187,12 @@ type KeystoneAPISpecCore struct {
// +operator-sdk:csv:customresourcedefinitions:type=spec
// TLS - Parameters related to the TLS
TLS tls.API `json:"tls,omitempty"`

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

// APIOverrideSpec to override the generated manifest of several child resources.
Expand Down Expand Up @@ -298,6 +307,7 @@ func SetupDefaults() {
// Acquire environmental defaults and initialize Keystone defaults with them
keystoneDefaults := KeystoneAPIDefaults{
ContainerImageURL: util.GetEnvVar("RELATED_IMAGE_KEYSTONE_API_IMAGE_URL_DEFAULT", KeystoneAPIContainerImage),
APITimeout: APIDefaultTimeout,
}

SetupKeystoneAPIDefaults(keystoneDefaults)
Expand Down
28 changes: 27 additions & 1 deletion api/v1beta1/keystoneapi_webhook.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ import (
// KeystoneAPIDefaults -
type KeystoneAPIDefaults struct {
ContainerImageURL string
APITimeout int
}

var keystoneAPIDefaults KeystoneAPIDefaults
Expand All @@ -48,6 +49,7 @@ var keystoneapilog = logf.Log.WithName("keystoneapi-resource")
// SetupKeystoneAPIDefaults - initialize KeystoneAPI spec defaults for use with either internal or external webhooks
func SetupKeystoneAPIDefaults(defaults KeystoneAPIDefaults) {
keystoneAPIDefaults = defaults

keystoneapilog.Info("KeystoneAPI defaults initialized", "defaults", defaults)
}

Expand Down Expand Up @@ -81,7 +83,9 @@ func (spec *KeystoneAPISpec) Default() {
// Default - set defaults for this KeystoneAPI core spec
// NOTE: only this version is used by OpenStackOperators webhook
func (spec *KeystoneAPISpecCore) Default() {
// no defaults to set yet
if spec.APITimeout == 0 {
spec.APITimeout = keystoneAPIDefaults.APITimeout
}
}

// TODO(user): change verbs to "verbs=create;update;delete" if you want to enable deletion validation.
Expand Down Expand Up @@ -167,3 +171,25 @@ func (r *KeystoneAPI) ValidateDelete() (admission.Warnings, error) {
// TODO(user): fill in your validation logic upon object deletion.
return nil, nil
}

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

// SetDefaultRouteAnnotations sets HAProxy timeout values of the route
func (spec *KeystoneAPISpecCore) SetDefaultRouteAnnotations(annotations map[string]string) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What calls this function? I don't see it being called explicitly.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 keystoneAnno = "api.keystone.openstack.org/timeout"
valKeystoneAPI, okKeystoneAPI := annotations[keystoneAnno]
valHAProxy, okHAProxy := annotations[haProxyAnno]
// Human operator set the HAProxy timeout manually
if !okKeystoneAPI && okHAProxy {
return
}
// Human operator modified the HAProxy timeout manually without removing the Keystone flag
if okKeystoneAPI && okHAProxy && valKeystoneAPI != valHAProxy {
delete(annotations, keystoneAnno)
return
}
timeout := fmt.Sprintf("%ds", spec.APITimeout)
annotations[keystoneAnno] = timeout
annotations[haProxyAnno] = timeout
}
5 changes: 5 additions & 0 deletions config/crd/bases/keystone.openstack.org_keystoneapis.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,11 @@ spec:
default: admin
description: AdminUser - admin user name
type: string
apiTimeout:
default: 60
description: APITimeout for HAProxy, Apache
minimum: 10
type: integer
containerImage:
description: Keystone Container Image URL (will be set to environmental
default if empty)
Expand Down
1 change: 1 addition & 0 deletions controllers/keystoneapi_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -1214,6 +1214,7 @@ func (r *KeystoneAPIReconciler) generateServiceConfigMaps(
httpdVhostConfig[endpt.String()] = endptConfig
}
templateParameters["VHosts"] = httpdVhostConfig
templateParameters["TimeOut"] = instance.Spec.APITimeout

tmpl := []util.Template{
// Scripts
Expand Down
1 change: 1 addition & 0 deletions templates/keystoneapi/config/httpd.conf
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ CustomLog /dev/stdout proxy env=forwarded
# {{ $endpt }} vhost {{ $vhost.ServerName }} configuration
<VirtualHost *:5000>
ServerName {{ $vhost.ServerName }}
TimeOut {{ $.TimeOut }}

## Vhost docroot
DocumentRoot "/var/www/cgi-bin/keystone"
Expand Down
3 changes: 3 additions & 0 deletions tests/functional/keystoneapi_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -415,6 +415,9 @@ var _ = Describe("Keystone controller", func() {
configData = string(scrt.Data["my.cnf"])
Expect(configData).To(
ContainSubstring("[client]\nssl=0"))
httpdConfData := string(scrt.Data["httpd.conf"])
Expect(httpdConfData).To(
ContainSubstring("TimeOut 60"))
})
It("should create a Secret for fernet keys", func() {
th.GetSecret(types.NamespacedName{
Expand Down
Loading