fix(base-cluster/ingress-nginx): set a couple of timeouts in the loadbalancer to the maximum value#1571
Conversation
…balancer to the maximum value
WalkthroughThe change updates the ingress-nginx HelmRelease configuration by adding four new OpenStack load balancer timeout annotations to the service annotations section. These annotations specify large timeout values for client data, member connect, member data, and TCP inspect. No other configuration or logic is altered. Changes
Poem
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
charts/base-cluster/templates/ingress/nginx.yaml (1)
54-57: Avoid hard-coding the 24-day “magic number” & gate the annotation to OpenStack clustersEmbedding the maximum timeout (
"2073600000"≈ 24 days) four times makes the template harder to maintain and exposes OpenStack-specific configuration in every cluster, even where it is irrelevant.Consider:
• Move the value to a single Helm input (e.g.
values.ingress.openstackTimeout) or at least a local template variable to eliminate duplication and document why 24 days was chosen.
• Wrap the block with a provider check (if eq .Values.cloud.provider "openstack") so other providers aren’t saddled with unused annotations.Example patch:
- loadbalancer.openstack.org/timeout-client-data: "2073600000" - loadbalancer.openstack.org/timeout-member-connect: "2073600000" - loadbalancer.openstack.org/timeout-member-data: "2073600000" - loadbalancer.openstack.org/timeout-tcp-inspect: "2073600000" + {{- if eq $.Values.cloud.provider "openstack" }} + {{- $maxTimeout := default "2073600000" $.Values.ingress.openstackTimeout }} + loadbalancer.openstack.org/timeout-client-data: {{ $maxTimeout | quote }} + loadbalancer.openstack.org/timeout-member-connect: {{ $maxTimeout | quote }} + loadbalancer.openstack.org/timeout-member-data: {{ $maxTimeout | quote }} + loadbalancer.openstack.org/timeout-tcp-inspect: {{ $maxTimeout | quote }} + {{- end }}This keeps intent clear, eases future tuning, and prevents vendor-specific noise outside OpenStack environments.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
charts/base-cluster/templates/ingress/nginx.yaml(1 hunks)
🔇 Additional comments (1)
charts/base-cluster/templates/ingress/nginx.yaml (1)
54-57: Verify unit correctness (ms vs s) to avoid 65-year timeoutsOctavia interprets these timeout values in milliseconds.
2073600000 ms≈ 24 days (the documented maximum). If someone later assumes seconds, the load balancer would be configured for > 65 years. Please add an inline comment or variable name that clarifies the unit to prevent misconfiguration.
🤖 I have diffed this beep boop"/$namespace/$kind/$name.yaml" for normal resources
|
🤖 I have created a release *beep* *boop* --- ## [9.2.0](base-cluster-v9.1.1...base-cluster-v9.2.0) (2025-08-14) ### Features * **base-cluster/cert-manager:** enableCertificateOwnerRef ([#1653](#1653)) ([283d86f](283d86f)) * **base-cluster/cert-manager:** use oci repository ([#1650](#1650)) ([ef6382d](ef6382d)) * **base-cluster/kyverno:** enable policyExceptions for kyverno ([#1655](#1655)) ([2029bcb](2029bcb)) ### Bug Fixes * **base-cluster/certificates:** certificate for `baseDomain` is not used ([#1644](#1644)) ([6a3ccae](6a3ccae)) * **base-cluster/dns:** only deploy external-dns HelmRepository if needed ([#1645](#1645)) ([7d313f2](7d313f2)) * **base-cluster/ingress-nginx:** set a couple of timeouts in the loadbalancer to the maximum value ([#1571](#1571)) ([bc6fe78](bc6fe78)) * **base-cluster/monitoring:** remove versions from datasources so they always take precedence ([#1651](#1651)) ([6821ed8](6821ed8)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - New Features - Cert-manager: option to set certificate owner references. - Cert-manager: support for pulling from an OCI repository. - Kyverno: ability to enable policy exceptions. - Bug Fixes - Corrected use of the base domain certificate. - External DNS repository now created only when required. - Ingress load balancer timeouts set to maximum to prevent premature terminations. - Monitoring datasources prioritized by removing version pinning. - Chores - Bumped base-cluster chart to 9.2.0 and updated release notes. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: github-actions <41898282+github-actions[bot]@users.noreply.github.com>
Summary by CodeRabbit