Add configurable API Timeouts #396
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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
Akrog
added a commit
to Akrog/openstack-operator
that referenced
this pull request
Jun 6, 2024
This patch allows Cinder to set its route annotations. Depends-On: openstack-k8s-operators/cinder-operator#396 Jira: https://issues.redhat.com/browse/OSPRH-7393
This was referenced Jun 6, 2024
softwarefactory-project-zuul bot
added a commit
to openstack-k8s-operators/architecture
that referenced
this pull request
Jun 6, 2024
Set HAProxy timeout to 60 seconds for storage This patch sets the HAProxy timeout to 60 seconds for storage services (Cinder, Glance, and Manila). This way it's in sync with the Apache default timeouts and matches what we have in OSP17. This will be fixed in the operators themselves to have these same defaults, and this patch is compatible with that change. When the operators code is merged and is run by the CI jobs the defaults in the operators will be ignored in favor of the defaults defined in this patch, so we'll want to revert this patch then. These are the PRs that once their code is run on the CI we can revert this PR: openstack-k8s-operators/cinder-operator#396 openstack-k8s-operators/glance-operator#550 openstack-k8s-operators/manila-operator#282 openstack-k8s-operators/openstack-operator#830 Jira: https://issues.redhat.com/browse/OSPRH-7393 Jira: https://issues.redhat.com/browse/OSPRH-7415 Reviewed-by: Andrew Bays <andrew.bays@gmail.com>
|
looks good but I'll let @ASBishop approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: Akrog, fmount The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
openshift-merge-bot
bot
merged commit 9a88752
into
openstack-k8s-operators:main
7 checks passed
Akrog
added a commit
to Akrog/openstack-operator
that referenced
this pull request
Jun 7, 2024
This patch allows Cinder to set its route annotations. Depends-On: openstack-k8s-operators/cinder-operator#396 Jira: https://issues.redhat.com/browse/OSPRH-7393
Akrog
added a commit
to Akrog/openstack-operator
that referenced
this pull request
Jun 7, 2024
This patch allows Cinder to set its route annotations. Depends-On: openstack-k8s-operators/cinder-operator#396 Jira: https://issues.redhat.com/browse/OSPRH-7393
Akrog
added a commit
to Akrog/openstack-operator
that referenced
this pull request
Jun 7, 2024
This patch allows Cinder to set its route annotations. Depends-On: openstack-k8s-operators/cinder-operator#396 Jira: https://issues.redhat.com/browse/OSPRH-7393
abays
pushed a commit
to Akrog/openstack-operator
that referenced
this pull request
Jun 7, 2024
This patch allows Cinder to set its route annotations. Depends-On: openstack-k8s-operators/cinder-operator#396 Jira: https://issues.redhat.com/browse/OSPRH-7393
abays
pushed a commit
to Akrog/openstack-operator
that referenced
this pull request
Jun 10, 2024
This patch allows Cinder to set its route annotations. Depends-On: openstack-k8s-operators/cinder-operator#396 Jira: https://issues.redhat.com/browse/OSPRH-7393
Akrog
added a commit
to Akrog/openstack-operator
that referenced
this pull request
Jun 10, 2024
This patch allows Cinder to set its route annotations. Depends-On: openstack-k8s-operators/cinder-operator#396 Jira: https://issues.redhat.com/browse/OSPRH-7393
abays
pushed a commit
to Akrog/openstack-operator
that referenced
this pull request
Jun 10, 2024
This patch allows Cinder to set its route annotations. Depends-On: openstack-k8s-operators/cinder-operator#396 Jira: https://issues.redhat.com/browse/OSPRH-7393
abays
pushed a commit
to Akrog/openstack-operator
that referenced
this pull request
Jun 10, 2024
This patch allows Cinder to set its route annotations. Depends-On: openstack-k8s-operators/cinder-operator#396 Jira: https://issues.redhat.com/browse/OSPRH-7393
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
apiTimeoutfield to theCinderSpecBaseto allow human operators to simultaneously configure the timeouts for HAProxy, Apache, and therpc_response_timeout.The
apiTimeoutdefaults to 60 seconds, to mimic the behavior present in OSP17.Having different timeouts for HAProxy, Apache, and
rpc_response_timeoutis possible setting the HAProxy timeout in theapiOverride, the Apache timeout withapiTimeout, and settingrpc_response_timeoutin the top level CindercustomServiceConfig.To be able to change the HAProxy value based on the
apiTimeoutwith 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