From d7e33fc898410638024c535f1ddd4d1d2cbbf677 Mon Sep 17 00:00:00 2001 From: Kamil Sambor Date: Fri, 8 Nov 2024 11:06:29 +0100 Subject: [PATCH] Added a parameter 'apiTimeout' to allow customization Added a parameter 'apiTimeout' to allow customization to the Apache timeout. The default is set to 60s which we do set for HAProxy timeouts currently. 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.nova.openstack.org/timeout" with the value that was initially set, this way flags it as being set by the nova-operator. Timeout is global for both api and metadata and they are set from nova lvl There will be follow up patch in openstack-operator to utilize the method 'SetDefaultRouteAnnotations' to set these default route annotations in openstack-operator --- api/bases/nova.openstack.org_nova.yaml | 5 ++++ api/bases/nova.openstack.org_novaapis.yaml | 5 ++++ api/bases/nova.openstack.org_novacells.yaml | 5 ++++ .../nova.openstack.org_novametadata.yaml | 5 ++++ api/v1beta1/nova_types.go | 6 +++++ api/v1beta1/nova_webhook.go | 27 +++++++++++++++++++ api/v1beta1/novaapi_types.go | 6 +++++ api/v1beta1/novacell_types.go | 6 +++++ api/v1beta1/novametadata_types.go | 7 +++++ config/crd/bases/nova.openstack.org_nova.yaml | 5 ++++ .../bases/nova.openstack.org_novaapis.yaml | 5 ++++ .../bases/nova.openstack.org_novacells.yaml | 5 ++++ .../nova.openstack.org_novametadata.yaml | 5 ++++ controllers/nova_controller.go | 3 +++ controllers/novaapi_controller.go | 2 ++ controllers/novametadata_controller.go | 1 + templates/novaapi/config/httpd.conf | 1 + templates/novametadata/config/httpd.conf | 1 + .../nova_metadata_controller_test.go | 2 ++ test/functional/nova_multicell_test.go | 8 ++++++ test/functional/novaapi_controller_test.go | 2 ++ 21 files changed, 112 insertions(+) diff --git a/api/bases/nova.openstack.org_nova.yaml b/api/bases/nova.openstack.org_nova.yaml index 6cffd7e18..0fe752c17 100644 --- a/api/bases/nova.openstack.org_nova.yaml +++ b/api/bases/nova.openstack.org_nova.yaml @@ -343,6 +343,11 @@ spec: type: string type: object type: object + apiTimeout: + default: 60 + description: APITimeout for Route and Apache + minimum: 10 + type: integer cellTemplates: additionalProperties: description: |- diff --git a/api/bases/nova.openstack.org_novaapis.yaml b/api/bases/nova.openstack.org_novaapis.yaml index 928487bcb..9c4229e54 100644 --- a/api/bases/nova.openstack.org_novaapis.yaml +++ b/api/bases/nova.openstack.org_novaapis.yaml @@ -61,6 +61,11 @@ spec: description: APIDatabaseHostname - hostname to use when accessing the API DB type: string + apiTimeout: + default: 60 + description: APITimeout for Route and Apache + minimum: 10 + type: integer cell0DatabaseAccount: default: nova-cell0 description: APIDatabaseAccount - MariaDBAccount to use when accessing diff --git a/api/bases/nova.openstack.org_novacells.yaml b/api/bases/nova.openstack.org_novacells.yaml index 59223fdd2..60229ddde 100644 --- a/api/bases/nova.openstack.org_novacells.yaml +++ b/api/bases/nova.openstack.org_novacells.yaml @@ -51,6 +51,11 @@ spec: cell0. TODO(gibi): Add a webhook to validate cell0 constraint type: string + apiTimeout: + default: 60 + description: APITimeout for Route and Apache + minimum: 10 + type: integer cellDatabaseAccount: default: nova description: CellDatabaseAccount - MariaDBAccount to use when accessing diff --git a/api/bases/nova.openstack.org_novametadata.yaml b/api/bases/nova.openstack.org_novametadata.yaml index ba9e7c6a6..7a26dd3f2 100644 --- a/api/bases/nova.openstack.org_novametadata.yaml +++ b/api/bases/nova.openstack.org_novametadata.yaml @@ -63,6 +63,11 @@ spec: This filed is Required if the CellName is not provided TODO(gibi): Add a webhook to validate the CellName constraint type: string + apiTimeout: + default: 60 + description: APITimeout for Route and Apache + minimum: 10 + type: integer cellDatabaseAccount: default: nova description: CellDatabaseAccount - MariaDBAccount to use when accessing diff --git a/api/v1beta1/nova_types.go b/api/v1beta1/nova_types.go index 2eb9cf645..ff3fde460 100644 --- a/api/v1beta1/nova_types.go +++ b/api/v1beta1/nova_types.go @@ -66,6 +66,12 @@ type NovaSpecCore struct { // APIDatabaseAccount - MariaDBAccount to use when accessing the API DB APIDatabaseAccount string `json:"apiDatabaseAccount"` + // +kubebuilder:validation:Optional + // +kubebuilder:default=60 + // +kubebuilder:validation:Minimum=10 + // APITimeout for Route and Apache + APITimeout int `json:"apiTimeout"` + // +kubebuilder:validation:Required // Secret is the name of the Secret instance containing password // information for nova like the keystone service password and DB passwords diff --git a/api/v1beta1/nova_webhook.go b/api/v1beta1/nova_webhook.go index 7fefc284f..34dfda761 100644 --- a/api/v1beta1/nova_webhook.go +++ b/api/v1beta1/nova_webhook.go @@ -307,6 +307,33 @@ func (r *Nova) ValidateDelete() (admission.Warnings, error) { return nil, nil } +// SetDefaultRouteAnnotations sets HAProxy timeout values of the route +// NOTE: it is used by ctlplane webhook on openstack-operator +func (r *NovaSpec) 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 novaAnno = "api.nova.openstack.org/timeout" + + valNova, okNova := annotations[novaAnno] + valHAProxy, okHAProxy := annotations[haProxyAnno] + + // Human operator set the HAProxy timeout manually + if !okNova && okHAProxy { + return + } + + // Human operator modified the HAProxy timeout manually without removing the Nova flag + if okNova && okHAProxy && valNova != valHAProxy { + delete(annotations, novaAnno) + return + } + + timeout := fmt.Sprintf("%ds", r.APITimeout) + annotations[novaAnno] = timeout + annotations[haProxyAnno] = timeout +} + // Validate the field values func (r *NovaCellDBPurge) Validate(basePath *field.Path) field.ErrorList { var errors field.ErrorList diff --git a/api/v1beta1/novaapi_types.go b/api/v1beta1/novaapi_types.go index a92b65fcf..67d3c633b 100644 --- a/api/v1beta1/novaapi_types.go +++ b/api/v1beta1/novaapi_types.go @@ -87,6 +87,12 @@ type NovaAPISpec struct { // INSERT ADDITIONAL SPEC FIELDS - desired state of cluster // Important: Run "make" to regenerate code after modifying this file + // +kubebuilder:validation:Optional + // +kubebuilder:default=60 + // +kubebuilder:validation:Minimum=10 + // APITimeout for Route and Apache + APITimeout int `json:"apiTimeout"` + // +kubebuilder:validation:Required // Secret is the name of the Secret instance containing password // information for the nova-api service. This secret is expected to be diff --git a/api/v1beta1/novacell_types.go b/api/v1beta1/novacell_types.go index b7fde482c..6c30af035 100644 --- a/api/v1beta1/novacell_types.go +++ b/api/v1beta1/novacell_types.go @@ -107,6 +107,12 @@ type NovaCellSpec struct { // the deployment. CellName string `json:"cellName"` + // +kubebuilder:validation:Optional + // +kubebuilder:default=60 + // +kubebuilder:validation:Minimum=10 + // APITimeout for Route and Apache + APITimeout int `json:"apiTimeout"` + // +kubebuilder:validation:Required // Secret is the name of the Secret instance containing password // information for the nova cell. This secret is expected to be diff --git a/api/v1beta1/novametadata_types.go b/api/v1beta1/novametadata_types.go index 56853d694..d7c96d4b3 100644 --- a/api/v1beta1/novametadata_types.go +++ b/api/v1beta1/novametadata_types.go @@ -103,6 +103,12 @@ type NovaMetadataSpec struct { // If not provided then the metadata serving every cells in the deployment CellName string `json:"cellName,omitempty"` + // +kubebuilder:validation:Optional + // +kubebuilder:default=60 + // +kubebuilder:validation:Minimum=10 + // APITimeout for Route and Apache + APITimeout int `json:"apiTimeout"` + // +kubebuilder:validation:Required // Secret is the name of the Secret instance containing password // information for the nova-conductor service. This secret is expected to @@ -265,6 +271,7 @@ func NewNovaMetadataSpec( TLS: novaCell.MetadataServiceTemplate.TLS, DefaultConfigOverwrite: novaCell.MetadataServiceTemplate.DefaultConfigOverwrite, MemcachedInstance: novaCell.MemcachedInstance, + APITimeout: novaCell.APITimeout, } if metadataSpec.NodeSelector == nil { diff --git a/config/crd/bases/nova.openstack.org_nova.yaml b/config/crd/bases/nova.openstack.org_nova.yaml index 6cffd7e18..0fe752c17 100644 --- a/config/crd/bases/nova.openstack.org_nova.yaml +++ b/config/crd/bases/nova.openstack.org_nova.yaml @@ -343,6 +343,11 @@ spec: type: string type: object type: object + apiTimeout: + default: 60 + description: APITimeout for Route and Apache + minimum: 10 + type: integer cellTemplates: additionalProperties: description: |- diff --git a/config/crd/bases/nova.openstack.org_novaapis.yaml b/config/crd/bases/nova.openstack.org_novaapis.yaml index 928487bcb..9c4229e54 100644 --- a/config/crd/bases/nova.openstack.org_novaapis.yaml +++ b/config/crd/bases/nova.openstack.org_novaapis.yaml @@ -61,6 +61,11 @@ spec: description: APIDatabaseHostname - hostname to use when accessing the API DB type: string + apiTimeout: + default: 60 + description: APITimeout for Route and Apache + minimum: 10 + type: integer cell0DatabaseAccount: default: nova-cell0 description: APIDatabaseAccount - MariaDBAccount to use when accessing diff --git a/config/crd/bases/nova.openstack.org_novacells.yaml b/config/crd/bases/nova.openstack.org_novacells.yaml index 59223fdd2..60229ddde 100644 --- a/config/crd/bases/nova.openstack.org_novacells.yaml +++ b/config/crd/bases/nova.openstack.org_novacells.yaml @@ -51,6 +51,11 @@ spec: cell0. TODO(gibi): Add a webhook to validate cell0 constraint type: string + apiTimeout: + default: 60 + description: APITimeout for Route and Apache + minimum: 10 + type: integer cellDatabaseAccount: default: nova description: CellDatabaseAccount - MariaDBAccount to use when accessing diff --git a/config/crd/bases/nova.openstack.org_novametadata.yaml b/config/crd/bases/nova.openstack.org_novametadata.yaml index ba9e7c6a6..7a26dd3f2 100644 --- a/config/crd/bases/nova.openstack.org_novametadata.yaml +++ b/config/crd/bases/nova.openstack.org_novametadata.yaml @@ -63,6 +63,11 @@ spec: This filed is Required if the CellName is not provided TODO(gibi): Add a webhook to validate the CellName constraint type: string + apiTimeout: + default: 60 + description: APITimeout for Route and Apache + minimum: 10 + type: integer cellDatabaseAccount: default: nova description: CellDatabaseAccount - MariaDBAccount to use when accessing diff --git a/controllers/nova_controller.go b/controllers/nova_controller.go index 575e378d3..ac91fa849 100644 --- a/controllers/nova_controller.go +++ b/controllers/nova_controller.go @@ -1108,6 +1108,7 @@ func (r *NovaReconciler) ensureCell( ServiceUser: instance.Spec.ServiceUser, KeystoneAuthURL: keystoneAuthURL, ServiceAccount: instance.RbacResourceName(), + APITimeout: instance.Spec.APITimeout, // The assumption is that the CA bundle for ironic compute in the cell // and the conductor in the cell always the same as the NovaAPI TLS: instance.Spec.APIServiceTemplate.TLS.Ca, @@ -1280,6 +1281,7 @@ func (r *NovaReconciler) ensureAPI( TLS: instance.Spec.APIServiceTemplate.TLS, DefaultConfigOverwrite: instance.Spec.APIServiceTemplate.DefaultConfigOverwrite, MemcachedInstance: getMemcachedInstance(instance, cell0Template), + APITimeout: instance.Spec.APITimeout, } api := &novav1.NovaAPI{ ObjectMeta: metav1.ObjectMeta{ @@ -1711,6 +1713,7 @@ func (r *NovaReconciler) ensureMetadata( TLS: instance.Spec.MetadataServiceTemplate.TLS, DefaultConfigOverwrite: instance.Spec.MetadataServiceTemplate.DefaultConfigOverwrite, MemcachedInstance: getMemcachedInstance(instance, cell0Template), + APITimeout: instance.Spec.APITimeout, } metadata = &novav1.NovaMetadata{ ObjectMeta: metav1.ObjectMeta{ diff --git a/controllers/novaapi_controller.go b/controllers/novaapi_controller.go index 6af62f339..884c725d5 100644 --- a/controllers/novaapi_controller.go +++ b/controllers/novaapi_controller.go @@ -487,6 +487,7 @@ func (r *NovaAPIReconciler) generateConfigs( endptConfig := map[string]interface{}{} endptConfig["ServerName"] = fmt.Sprintf("nova-%s.%s.svc", endpt.String(), instance.Namespace) endptConfig["tls"] = false // default TLS to false, and set it bellow to true if enabled + endptConfig["TimeOut"] = instance.Spec.APITimeout if instance.Spec.TLS.API.Enabled(endpt) { templateParameters["tls"] = true endptConfig["tls"] = true @@ -494,6 +495,7 @@ func (r *NovaAPIReconciler) generateConfigs( endptConfig["SSLCertificateKeyFile"] = fmt.Sprintf("/etc/pki/tls/private/%s.key", endpt.String()) } httpdVhostConfig[endpt.String()] = endptConfig + } templateParameters["VHosts"] = httpdVhostConfig diff --git a/controllers/novametadata_controller.go b/controllers/novametadata_controller.go index 4dabbfa64..a1f0c9945 100644 --- a/controllers/novametadata_controller.go +++ b/controllers/novametadata_controller.go @@ -476,6 +476,7 @@ func (r *NovaMetadataReconciler) generateConfigs( "MemcachedServers": memcachedInstance.GetMemcachedServerListString(), "MemcachedServersWithInet": memcachedInstance.GetMemcachedServerListWithInetString(), "MemcachedTLS": memcachedInstance.GetMemcachedTLSSupport(), + "TimeOut": instance.Spec.APITimeout, } var db *mariadbv1.Database diff --git a/templates/novaapi/config/httpd.conf b/templates/novaapi/config/httpd.conf index f79ff2ca1..ea29d1665 100644 --- a/templates/novaapi/config/httpd.conf +++ b/templates/novaapi/config/httpd.conf @@ -38,6 +38,7 @@ LogLevel info SetEnvIf X-Forwarded-For "^.*\..*\..*\..*" forwarded ServerName {{ $vhost.ServerName }} + TimeOut {{ $vhost.TimeOut }} ## Vhost docroot DocumentRoot "/var/www/cgi-bin" diff --git a/templates/novametadata/config/httpd.conf b/templates/novametadata/config/httpd.conf index 14bfb5fd5..f35e2d118 100644 --- a/templates/novametadata/config/httpd.conf +++ b/templates/novametadata/config/httpd.conf @@ -36,6 +36,7 @@ LogLevel info SetEnvIf X-Forwarded-For "^.*\..*\..*\..*" forwarded ServerName {{ .ServerName }} + TimeOut {{ .TimeOut }} ErrorLog /dev/stdout CustomLog /dev/stdout combined env=!forwarded diff --git a/test/functional/nova_metadata_controller_test.go b/test/functional/nova_metadata_controller_test.go index 611c21dbe..0221e75ea 100644 --- a/test/functional/nova_metadata_controller_test.go +++ b/test/functional/nova_metadata_controller_test.go @@ -981,6 +981,8 @@ var _ = Describe("NovaMetadata controller", func() { Expect(configData).Should(ContainSubstring("SSLEngine on")) Expect(configData).Should(ContainSubstring("SSLCertificateFile \"/etc/pki/tls/certs/nova-metadata.crt\"")) Expect(configData).Should(ContainSubstring("SSLCertificateKeyFile \"/etc/pki/tls/private/nova-metadata.key\"")) + Expect(configData).Should( + ContainSubstring("TimeOut 60")) configData = string(configDataMap.Data["01-nova.conf"]) Expect(configData).Should( diff --git a/test/functional/nova_multicell_test.go b/test/functional/nova_multicell_test.go index 0f60982ba..5f64b76d1 100644 --- a/test/functional/nova_multicell_test.go +++ b/test/functional/nova_multicell_test.go @@ -949,6 +949,14 @@ var _ = Describe("Nova multi cell", func() { HaveKeyWithValue(controllers.MetadataSecretSelector, []byte("metadata-secret"))) Expect(cell0Secret.Data).NotTo( HaveKeyWithValue(controllers.MetadataSecretSelector, []byte("metadata-secret-cell1"))) + configDataMap := th.GetSecret(cell1.MetadataConfigDataName) + Expect(configDataMap).ShouldNot(BeNil()) + Expect(configDataMap.Data).Should(HaveKey("httpd.conf")) + Expect(configDataMap.Data).Should(HaveKey("ssl.conf")) + configData := string(configDataMap.Data["httpd.conf"]) + Expect(configData).Should( + ContainSubstring("TimeOut 60")) + }) }) }) diff --git a/test/functional/novaapi_controller_test.go b/test/functional/novaapi_controller_test.go index d324c0365..066d4d39b 100644 --- a/test/functional/novaapi_controller_test.go +++ b/test/functional/novaapi_controller_test.go @@ -1037,6 +1037,8 @@ var _ = Describe("NovaAPI controller", func() { Expect(configData).Should(ContainSubstring("SSLCertificateKeyFile \"/etc/pki/tls/private/internal.key\"")) Expect(configData).Should(ContainSubstring("SSLCertificateFile \"/etc/pki/tls/certs/public.crt\"")) Expect(configData).Should(ContainSubstring("SSLCertificateKeyFile \"/etc/pki/tls/private/public.key\"")) + Expect(configData).Should( + ContainSubstring("TimeOut 60")) configData = string(configDataMap.Data["01-nova.conf"]) Expect(configData).Should(