RFE-2195: expose ROUTER_METRICS_HAPROXY_SERVER_THRESHOLD value via IngressController API

[himank98]

Summary

ROUTER_METRICS_HAPROXY_SERVER_THRESHOLD controls the maximum number of HAProxy backends (servers) per shard for which per-server Prometheus metrics are collected. When the number of active backends exceeds this threshold,
per-server metrics are suppressed to prevent high-cardinality metric explosions in the monitoring stack.

Previously this variable was hardcoded to a default of 500 inside the router image with no way to tune it via the IngressController API. This PR exposes it as spec.tuningOptions.haproxyServerThreshold so cluster administrators can adjust the value — for example, raising it to 1000 to match the typical number of routes per shard and improve metrics coverage in large clusters.

[openshift-ci-robot]

@himank98: This pull request references RFE-2195 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the feature request to target the "5.0.0" version, but no target version was set.

Details

In response to this:

Summary

ROUTER_METRICS_HAPROXY_SERVER_THRESHOLD controls the maximum number of HAProxy backends (servers) per shard for which per-server Prometheus metrics are collected. When the number of active backends exceeds this threshold,
per-server metrics are suppressed to prevent high-cardinality metric explosions in the monitoring stack.

Previously this variable was hardcoded to a default of 500 inside the router image with no way to tune it via the IngressController API. This PR exposes it as spec.tuningOptions.haproxyServerThreshold so cluster administrators can adjust the value — for example, raising it to 1000 to match the typical number of routes per shard and improve metrics coverage in large clusters.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository.

[coderabbitai]

📝 Walkthrough

Walkthrough

This PR adds support for configuring the HAProxy server Prometheus metrics threshold through a new environment variable. The change introduces an exported constant RouterMetricsHAProxyServerThreshold in the deployment module, updates the desiredRouterDeployment function to conditionally append this env var to the router container when ci.Spec.TuningOptions.HaproxyServerThreshold is greater than zero, and includes a table-driven test validating correct presence/absence and stringification of the threshold value across zero and positive values.

🚥 Pre-merge checks | ✅ 11 | ❌ 1

❌ Failed checks (1 inconclusive)

Check name	Status	Explanation	Resolution
Test Structure And Quality	❓ Inconclusive	Custom check specifies reviewing Ginkgo test code, but the test in this PR is a standard Go test using testing.T, not Ginkgo (no onsi/ginkgo imports or Describe/It blocks).	Clarify if check applies to standard Go tests or only Ginkgo tests. If standard Go tests: PASS - test meets all quality requirements per codebase patterns.

✅ Passed checks (11 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately summarizes the main change: exposing ROUTER_METRICS_HAPROXY_SERVER_THRESHOLD via the IngressController API.
Description check	✅ Passed	The description provides relevant context about what ROUTER_METRICS_HAPROXY_SERVER_THRESHOLD controls, why it's being exposed, and how it benefits cluster administrators.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Stable And Deterministic Test Names	✅ Passed	Test names in Test_HaproxyServerThreshold are all static literal strings without dynamic content like pod names, timestamps, UUIDs, or generated identifiers.
Microshift Test Compatibility	✅ Passed	No Ginkgo e2e tests were added in this PR. The test added (Test_HaproxyServerThreshold) is a standard Go unit test in deployment_test.go, not a Ginkgo e2e test in test/e2e/.
Single Node Openshift (Sno) Test Compatibility	✅ Passed	PR adds only a standard Go unit test (Test_HaproxyServerThreshold), not Ginkgo e2e tests. SNO compatibility check applies only to Ginkgo e2e tests with It(), Describe(), Context(), When() patterns.
Topology-Aware Scheduling Compatibility	✅ Passed	PR adds only an environment variable constant and configuration logic with no changes to scheduling, affinity, topology spread, node selectors, or topology handling.
Ote Binary Stdout Contract	✅ Passed	PR adds a string constant and conditional environment variable logic with no process-level code, stdout writes, or module-level side effects violating OTE Binary Stdout Contract.
Ipv6 And Disconnected Network Test Compatibility	✅ Passed	New test is a standard Go unit test (using testing.T), not a Ginkgo e2e test. No Ginkgo patterns detected; test has no IPv4 assumptions or external connectivity requirements.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[openshift-ci]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign rfredette for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Details

Needs approval from an approver in each of these files:

• OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[openshift-ci]

Hi @himank98. Thanks for your PR.

I'm waiting for a openshift member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work.

Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

[coderabbitai]

🧹 Nitpick comments (1)

pkg/operator/controller/ingress/deployment_test.go (1)

3110-3122: ⚡ Quick win

Use testify/assert in the new subtest assertions.

Please switch the direct t.Fatalf/t.Errorf checks in this new test block to assert (or require) to align with the test guideline for this repo.

As per coding guidelines, **/*_test.go: “Use table-driven tests with subtests using t.Run() with descriptive names for nested tests, and use github.com/stretchr/testify/assert for assertions”.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@pkg/operator/controller/ingress/deployment_test.go` around lines 3110 - 3122,
Replace the raw t.Fatalf/t.Errorf in this subtest with testify assertions:
import "github.com/stretchr/testify/require" and
"github.com/stretchr/testify/assert" (or just one as needed), then use
require.NoError(t, err, "failed to generate desired router Deployment") after
calling the function that returns err (the block that generates desired router
Deployment) to fail fast on error, and use assert.NoError(t, checkErr,
"environment variable check failed") (where checkErr is the error returned by
checkDeploymentEnvironment) instead of t.Errorf; keep references to the same
variables/functions (deployment, expectedEnv,
RouterMetricsHAProxyServerThreshold, checkDeploymentEnvironment) so the checks
are exactly swapped to require/assert calls.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@pkg/operator/controller/ingress/deployment_test.go`:
- Around line 3110-3122: Replace the raw t.Fatalf/t.Errorf in this subtest with
testify assertions: import "github.com/stretchr/testify/require" and
"github.com/stretchr/testify/assert" (or just one as needed), then use
require.NoError(t, err, "failed to generate desired router Deployment") after
calling the function that returns err (the block that generates desired router
Deployment) to fail fast on error, and use assert.NoError(t, checkErr,
"environment variable check failed") (where checkErr is the error returned by
checkDeploymentEnvironment) instead of t.Errorf; keep references to the same
variables/functions (deployment, expectedEnv,
RouterMetricsHAProxyServerThreshold, checkDeploymentEnvironment) so the checks
are exactly swapped to require/assert calls.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository YAML (base), Central YAML (inherited)

Review profile: CHILL

Plan: Enterprise

Run ID: cb10f54e-aa55-439d-a76c-44256151a4ea

📥 Commits

Reviewing files that changed from the base of the PR and between 8360b35 and fce29b6.

⛔ Files ignored due to path filters (1)

• vendor/github.com/openshift/api/operator/v1/types_ingress.go is excluded by !**/vendor/**, !vendor/**

📒 Files selected for processing (2)

• pkg/operator/controller/ingress/deployment.go
• pkg/operator/controller/ingress/deployment_test.go