From 2fb078acc776b700ca9d5277c139af417a2f4dba 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 120s 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.neutron.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 25ef59931..aa3cd46f2 100644 --- a/api/bases/nova.openstack.org_nova.yaml +++ b/api/bases/nova.openstack.org_nova.yaml @@ -355,6 +355,11 @@ spec: type: string type: object type: object + apiTimeout: + default: 600 + description: APITimeout for Route and Apache + minimum: 60 + type: integer cellTemplates: additionalProperties: description: NovaCellTemplate defines the input parameters specified diff --git a/api/bases/nova.openstack.org_novaapis.yaml b/api/bases/nova.openstack.org_novaapis.yaml index 1d1c5cb4e..02bfefc21 100644 --- a/api/bases/nova.openstack.org_novaapis.yaml +++ b/api/bases/nova.openstack.org_novaapis.yaml @@ -57,6 +57,11 @@ spec: description: APIDatabaseHostname - hostname to use when accessing the API DB type: string + apiTimeout: + default: 600 + description: APITimeout for Route and Apache + minimum: 60 + 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 996b48ea3..2761f9163 100644 --- a/api/bases/nova.openstack.org_novacells.yaml +++ b/api/bases/nova.openstack.org_novacells.yaml @@ -46,6 +46,11 @@ spec: filed is Required for cell0. TODO(gibi): Add a webhook to validate cell0 constraint' type: string + apiTimeout: + default: 600 + description: APITimeout for Route and Apache + minimum: 60 + 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 1ac0ce9c4..28f7d6801 100644 --- a/api/bases/nova.openstack.org_novametadata.yaml +++ b/api/bases/nova.openstack.org_novametadata.yaml @@ -58,6 +58,11 @@ spec: the API DB. This filed is Required if the CellName is not provided TODO(gibi): Add a webhook to validate the CellName constraint' type: string + apiTimeout: + default: 600 + description: APITimeout for Route and Apache + minimum: 60 + 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 16037bf61..69e2e7169 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=600 + // +kubebuilder:validation:Minimum=60 + // 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..a58dab4f6 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 Heat 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 10827247b..f8243a7cb 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=600 + // +kubebuilder:validation:Minimum=60 + // 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 48995d1f7..6920407bb 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=600 + // +kubebuilder:validation:Minimum=60 + // 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 404934c21..8607c870c 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=600 + // +kubebuilder:validation:Minimum=60 + // 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, } return metadataSpec } diff --git a/config/crd/bases/nova.openstack.org_nova.yaml b/config/crd/bases/nova.openstack.org_nova.yaml index 25ef59931..aa3cd46f2 100644 --- a/config/crd/bases/nova.openstack.org_nova.yaml +++ b/config/crd/bases/nova.openstack.org_nova.yaml @@ -355,6 +355,11 @@ spec: type: string type: object type: object + apiTimeout: + default: 600 + description: APITimeout for Route and Apache + minimum: 60 + type: integer cellTemplates: additionalProperties: description: NovaCellTemplate defines the input parameters specified diff --git a/config/crd/bases/nova.openstack.org_novaapis.yaml b/config/crd/bases/nova.openstack.org_novaapis.yaml index 1d1c5cb4e..02bfefc21 100644 --- a/config/crd/bases/nova.openstack.org_novaapis.yaml +++ b/config/crd/bases/nova.openstack.org_novaapis.yaml @@ -57,6 +57,11 @@ spec: description: APIDatabaseHostname - hostname to use when accessing the API DB type: string + apiTimeout: + default: 600 + description: APITimeout for Route and Apache + minimum: 60 + 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 996b48ea3..2761f9163 100644 --- a/config/crd/bases/nova.openstack.org_novacells.yaml +++ b/config/crd/bases/nova.openstack.org_novacells.yaml @@ -46,6 +46,11 @@ spec: filed is Required for cell0. TODO(gibi): Add a webhook to validate cell0 constraint' type: string + apiTimeout: + default: 600 + description: APITimeout for Route and Apache + minimum: 60 + 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 1ac0ce9c4..28f7d6801 100644 --- a/config/crd/bases/nova.openstack.org_novametadata.yaml +++ b/config/crd/bases/nova.openstack.org_novametadata.yaml @@ -58,6 +58,11 @@ spec: the API DB. This filed is Required if the CellName is not provided TODO(gibi): Add a webhook to validate the CellName constraint' type: string + apiTimeout: + default: 600 + description: APITimeout for Route and Apache + minimum: 60 + 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 ca0cc987c..c3970f19a 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, @@ -1279,6 +1280,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{ @@ -1708,6 +1710,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 b5a159edf..fdbd7dcc1 100644 --- a/test/functional/nova_metadata_controller_test.go +++ b/test/functional/nova_metadata_controller_test.go @@ -985,6 +985,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 600")) 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 988798449..d5c5841bb 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 600")) + }) }) }) diff --git a/test/functional/novaapi_controller_test.go b/test/functional/novaapi_controller_test.go index 5ae0f1dd1..388bd4344 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 600")) configData = string(configDataMap.Data["01-nova.conf"]) Expect(configData).Should(