Skip to content

K8s: Add new annotation as new rule in ingress-nginx controller #2891

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Jul 9, 2025

Conversation

VietND96
Copy link
Member

@VietND96 VietND96 commented Jul 9, 2025

User description

Thanks for contributing to the Docker-Selenium project!
A PR well described will help maintainers to quickly review and merge it

Before submitting your PR, please check our contributing guidelines, applied for this repository.
Avoid large PRs, help reviewers by making them as simple and short as possible.

Description

After sub-chart ingress-nginx upgrade to 4.13.0, a new rule was added in the configuration.
Since our values override buffer size, so need to align with new rule.

  Error: exit status 1
  2025/07/09 13:20:11 [emerg] 22#22: "proxy_busy_buffers_size" must be equal to or greater than the maximum of the value of "proxy_buffer_size" and one of the "proxy_buffers" in /tmp/nginx/nginx-cfg3915377558:1235
  nginx: [emerg] "proxy_busy_buffers_size" must be equal to or greater than the maximum of the value of "proxy_buffer_size" and one of the "proxy_buffers" in /tmp/nginx/nginx-cfg3915377558:1235
  nginx: configuration file /tmp/nginx/nginx-cfg3915377558 test failed

Related to kubernetes/ingress-nginx#13598

Motivation and Context

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist

  • I have read the contributing document.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

PR Type

Bug fix


Description

  • Add missing proxy-busy-buffers-size annotation to ingress-nginx configuration

  • Fix nginx configuration validation error after ingress-nginx upgrade to 4.13.0

  • Align buffer size annotations with new ingress controller requirements


Changes diagram

flowchart LR
  A["ingress-nginx 4.13.0"] --> B["New buffer validation rule"]
  B --> C["Add proxy-busy-buffers-size annotation"]
  C --> D["Fix nginx configuration error"]
Loading

Changes walkthrough 📝

Relevant files
Bug fix
_helpers.tpl
Add missing proxy-busy-buffers-size annotation                     

charts/selenium-grid/templates/_helpers.tpl

  • Add nginx.ingress.kubernetes.io/proxy-busy-buffers-size annotation
  • Set annotation value to match existing buffer size configuration
  • Fix nginx configuration validation error
  • +1/-0     

    Need help?
  • Type /help how to ... in the comments thread for any questions about Qodo Merge usage.
  • Check out the documentation for more information.
  • Signed-off-by: Viet Nguyen Duc <nguyenducviet4496@gmail.com>
    Copy link
    Contributor

    qodo-merge-pro bot commented Jul 9, 2025

    PR Reviewer Guide 🔍

    Here are some key observations to aid the review process:

    🎫 Ticket compliance analysis ❌

    22 - Not compliant

    Non-compliant requirements:

    • Provide additional containers of browsers with XVFB support
    • Enable testing of Flash applications and other non-headless scenarios

    ⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
    🧪 No relevant tests
    🔒 No security concerns identified
    ⚡ Recommended focus areas for review

    Misaligned PR

    This PR addresses nginx ingress configuration issues but is completely unrelated to the linked ticket about XVFB browser containers. The changes don't implement any XVFB functionality.

    nginx.ingress.kubernetes.io/proxy-busy-buffers-size: {{ . | quote }}
    nginx.ingress.kubernetes.io/client-body-buffer-size: {{ . | quote }}

    Copy link
    Contributor

    qodo-merge-pro bot commented Jul 9, 2025

    PR Code Suggestions ✨

    Explore these optional code suggestions:

    CategorySuggestion                                                                                                                                    Impact
    General
    Optimize busy buffer sizing

    The proxy-busy-buffers-size should typically be smaller than proxy-buffer-size
    for optimal nginx performance. Consider using a fraction of the buffer size or a
    separate configuration value instead of reusing the same size value.

    charts/selenium-grid/templates/_helpers.tpl [117]

    -nginx.ingress.kubernetes.io/proxy-busy-buffers-size: {{ . | quote }}
    +nginx.ingress.kubernetes.io/proxy-busy-buffers-size: {{ div . 2 | quote }}
    • Apply / Chat
    Suggestion importance[1-10]: 7

    __

    Why: The suggestion correctly identifies that setting proxy-busy-buffers-size to the same value as proxy-buffer-size can be suboptimal for NGINX performance, and proposes a valid improvement.

    Medium
    • Update

    [skip ci]
    
    Signed-off-by: Viet Nguyen Duc <nguyenducviet4496@gmail.com>
    @VietND96 VietND96 merged commit c462c9c into trunk Jul 9, 2025
    1 check passed
    @VietND96 VietND96 deleted the ingress-chart branch July 9, 2025 14:10
    VietND96 added a commit to NDViet/docker-selenium that referenced this pull request Jul 25, 2025
    …niumHQ#2891)
    
    Signed-off-by: Viet Nguyen Duc <nguyenducviet4496@gmail.com>
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Projects
    None yet
    Development

    Successfully merging this pull request may close these issues.

    1 participant