From ee86f215ee90452290e848ca94cbc8c64da1b6a7 Mon Sep 17 00:00:00 2001 From: Yatin Karel Date: Wed, 30 Oct 2024 12:19:41 -0400 Subject: [PATCH] Make apache timeout configurable 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 neutron-operator. There will be follow up patch in openstack-operator to utilize the method 'SetDefaultRouteAnnotations' to set these default route annotations. Resolves: OSPRH-10843 --- .../neutron.openstack.org_neutronapis.yaml | 5 +++ api/v1beta1/neutronapi_types.go | 10 ++++-- api/v1beta1/neutronapi_webhook.go | 32 ++++++++++++++++--- .../neutron.openstack.org_neutronapis.yaml | 5 +++ controllers/neutronapi_controller.go | 1 + .../neutronapi/httpd/10-neutron-httpd.conf | 2 ++ test/functional/neutronapi_controller_test.go | 23 +++++++++++++ 7 files changed, 72 insertions(+), 6 deletions(-) diff --git a/api/bases/neutron.openstack.org_neutronapis.yaml b/api/bases/neutron.openstack.org_neutronapis.yaml index 33686057..a6f2a725 100644 --- a/api/bases/neutron.openstack.org_neutronapis.yaml +++ b/api/bases/neutron.openstack.org_neutronapis.yaml @@ -48,6 +48,11 @@ spec: spec: description: NeutronAPISpec defines the desired state of NeutronAPI properties: + apiTimeout: + default: 120 + description: APITimeout for HAProxy, Apache + minimum: 1 + type: integer containerImage: description: NeutronAPI Container Image URL (will be set to environmental default if empty) diff --git a/api/v1beta1/neutronapi_types.go b/api/v1beta1/neutronapi_types.go index 49239074..377811ba 100644 --- a/api/v1beta1/neutronapi_types.go +++ b/api/v1beta1/neutronapi_types.go @@ -50,6 +50,12 @@ type NeutronAPISpec struct { // NeutronAPISpecCore - type NeutronAPISpecCore struct { + // +kubebuilder:validation:Optional + // +kubebuilder:default=120 + // +kubebuilder:validation:Minimum=1 + // APITimeout for HAProxy, Apache + APITimeout int `json:"apiTimeout"` + // +kubebuilder:validation:Optional // +kubebuilder:default=neutron // ServiceUser - optional username used for this service to register in neutron @@ -290,8 +296,8 @@ func (instance NeutronAPI) IsOVNEnabled() bool { func SetupDefaults() { // Acquire environmental defaults and initialize Neutron defaults with them neutronDefaults := NeutronAPIDefaults{ - ContainerImageURL: util.GetEnvVar("RELATED_IMAGE_NEUTRON_API_IMAGE_URL_DEFAULT", NeutronAPIContainerImage), - NeutronAPIRouteTimeout: "120s", + ContainerImageURL: util.GetEnvVar("RELATED_IMAGE_NEUTRON_API_IMAGE_URL_DEFAULT", NeutronAPIContainerImage), + APITimeout: 120, } SetupNeutronAPIDefaults(neutronDefaults) diff --git a/api/v1beta1/neutronapi_webhook.go b/api/v1beta1/neutronapi_webhook.go index beb846dd..3bd1e434 100644 --- a/api/v1beta1/neutronapi_webhook.go +++ b/api/v1beta1/neutronapi_webhook.go @@ -37,8 +37,8 @@ import ( // NeutronAPIDefaults - type NeutronAPIDefaults struct { - ContainerImageURL string - NeutronAPIRouteTimeout string + ContainerImageURL string + APITimeout int } var neutronAPIDefaults NeutronAPIDefaults @@ -81,7 +81,9 @@ func (spec *NeutronAPISpec) Default() { // Default - set defaults for this NeutronAPI spec core. This version gets used by OpenStackControlplane func (spec *NeutronAPISpecCore) Default() { - // nothing here yet + if spec.APITimeout == 0 { + spec.APITimeout = neutronAPIDefaults.APITimeout + } } // TODO(user): change verbs to "verbs=create;update;delete" if you want to enable deletion validation. @@ -178,7 +180,7 @@ func (spec *NeutronAPISpec) GetDefaultRouteAnnotations() (annotations map[string func (spec *NeutronAPISpecCore) GetDefaultRouteAnnotations() (annotations map[string]string) { return map[string]string{ - "haproxy.router.openshift.io/timeout": neutronAPIDefaults.NeutronAPIRouteTimeout, + "haproxy.router.openshift.io/timeout": fmt.Sprintf("%ds", neutronAPIDefaults.APITimeout), } } @@ -201,3 +203,25 @@ func ValidateDefaultConfigOverwrite( } return errors } + +// SetDefaultRouteAnnotations sets HAProxy timeout values of the route +func (spec *NeutronAPISpecCore) 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 neutronAnno = "api.neutron.openstack.org/timeout" + valNeutronAPI, okNeutronAPI := annotations[neutronAnno] + valHAProxy, okHAProxy := annotations[haProxyAnno] + // Human operator set the HAProxy timeout manually + if (!okNeutronAPI && okHAProxy) { + return + } + // Human operator modified the HAProxy timeout manually without removing the NeutronAPI flag + if (okNeutronAPI && okHAProxy && valNeutronAPI != valHAProxy) { + delete(annotations, neutronAnno) + return + } + timeout := fmt.Sprintf("%ds", spec.APITimeout) + annotations[neutronAnno] = timeout + annotations[haProxyAnno] = timeout +} diff --git a/config/crd/bases/neutron.openstack.org_neutronapis.yaml b/config/crd/bases/neutron.openstack.org_neutronapis.yaml index 33686057..a6f2a725 100644 --- a/config/crd/bases/neutron.openstack.org_neutronapis.yaml +++ b/config/crd/bases/neutron.openstack.org_neutronapis.yaml @@ -48,6 +48,11 @@ spec: spec: description: NeutronAPISpec defines the desired state of NeutronAPI properties: + apiTimeout: + default: 120 + description: APITimeout for HAProxy, Apache + minimum: 1 + type: integer containerImage: description: NeutronAPI Container Image URL (will be set to environmental default if empty) diff --git a/controllers/neutronapi_controller.go b/controllers/neutronapi_controller.go index 0b32df6b..e19dc992 100644 --- a/controllers/neutronapi_controller.go +++ b/controllers/neutronapi_controller.go @@ -1547,6 +1547,7 @@ func (r *NeutronAPIReconciler) generateServiceSecrets( templateParameters["MemcachedServers"] = mc.GetMemcachedServerListString() templateParameters["MemcachedServersWithInet"] = mc.GetMemcachedServerListWithInetString() templateParameters["MemcachedTLS"] = mc.GetMemcachedTLSSupport() + templateParameters["TimeOut"] = instance.Spec.APITimeout // Other OpenStack services servicePassword := string(ospSecret.Data[instance.Spec.PasswordSelectors.Service]) diff --git a/templates/neutronapi/httpd/10-neutron-httpd.conf b/templates/neutronapi/httpd/10-neutron-httpd.conf index c77386e9..c74511f6 100644 --- a/templates/neutronapi/httpd/10-neutron-httpd.conf +++ b/templates/neutronapi/httpd/10-neutron-httpd.conf @@ -3,6 +3,8 @@ ServerName {{ $vhost.ServerName }} + TimeOut {{ $.TimeOut }} + ## Logging ErrorLog /dev/stdout ServerSignature Off diff --git a/test/functional/neutronapi_controller_test.go b/test/functional/neutronapi_controller_test.go index 0498cf62..89b24faa 100644 --- a/test/functional/neutronapi_controller_test.go +++ b/test/functional/neutronapi_controller_test.go @@ -511,6 +511,29 @@ func getNeutronAPIControllerSuite(ml2MechanismDrivers []string) func() { ) }) + It("should create a Secret for 10-neutron-httpd.conf with Timeout set", func() { + if isOVNEnabled { + DeferCleanup(DeleteOVNDBClusters, CreateOVNDBClusters(namespace)) + } + keystoneAPI := keystone.CreateKeystoneAPI(namespace) + DeferCleanup(keystone.DeleteKeystoneAPI, keystoneAPI) + + secret := types.NamespacedName{ + Namespace: neutronAPIName.Namespace, + Name: fmt.Sprintf("%s-%s", neutronAPIName.Name, "httpd-config"), + } + + Eventually(func() corev1.Secret { + return th.GetSecret(secret) + }, timeout, interval).ShouldNot(BeNil()) + + configData := th.GetSecret(secret) + Expect(configData).ShouldNot(BeNil()) + conf := string(configData.Data["10-neutron-httpd.conf"]) + Expect(conf).Should( + ContainSubstring("TimeOut 120")) + }) + It("should create secret with OwnerReferences set", func() { if isOVNEnabled { dbs := CreateOVNDBClusters(namespace)