Skip to content
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

feat(chart): Add default annotations for ingress nginx controller #2047

Merged
merged 2 commits into from
Dec 6, 2023

Conversation

VietND96
Copy link
Member

@VietND96 VietND96 commented Dec 6, 2023

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

feat(chart): Add default annotations for ingress nginx controller

Motivation and Context

By default, ingress is enabled without annotations set. If NGINX ingress controller is used, you need to set few annotations to override the default timeout values to avoid 504 errors (see #1808). Since in Selenium Grid the default of SE_NODE_SESSION_TIMEOUT and SE_SESSION_REQUEST_TIMEOUT is 300 seconds.

In order to make user experience better, there are few annotations will be set by default if NGINX ingress controller is used. Mostly relates to timeouts and buffer sizes.

If you are not using NGINX ingress controller, you can disable these default annotations by setting ingress.nginx to nil (aka null) via Helm CLI --set ingress.nginx=null) or via an override-values.yaml as below:

ingress:
  nginx:
  # nginx: null (alternative way)

Similarly, if you want to disable a sub-config of ingress.nginx. For example: --set ingress.nginx.proxyBuffer=null)

You are also able to combine using both default annotations and your own annotations in ingress.annotations. Duplicated keys will be merged strategy overwrite with your own annotations in ingress.annotations take precedence.

ingress:
  nginx:
    proxyTimeout: 360
  annotations:
    nginx.ingress.kubernetes.io/proxy-connect-timeout: "3600" # This key will take 3600 instead of 360

List mapping of chart values and default annotation(s)

# `ingress.nginx.proxyTimeout` pass value to annotation(s)
nginx.ingress.kubernetes.io/proxy-connect-timeout
nginx.ingress.kubernetes.io/proxy-send-timeout
nginx.ingress.kubernetes.io/proxy-read-timeout
nginx.ingress.kubernetes.io/proxy-next-upstream-timeout
nginx.ingress.kubernetes.io/auth-keepalive-timeout

# `ingress.nginx.proxyBuffer` pass value to to annotation(s)
nginx.ingress.kubernetes.io/proxy-request-buffering: "on"
nginx.ingress.kubernetes.io/proxy-buffering: "on"

# `ingress.nginx.proxyBuffer.size` pass value to to annotation(s)
nginx.ingress.kubernetes.io/proxy-buffer-size
nginx.ingress.kubernetes.io/client-body-buffer-size

# `ingress.nginx.proxyBuffer.number` pass value to annotation(s)
nginx.ingress.kubernetes.io/proxy-buffers-number

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.

@VietND96 VietND96 requested a review from diemol December 6, 2023 06:53
@VietND96 VietND96 self-assigned this Dec 6, 2023
@VietND96 VietND96 linked an issue Dec 6, 2023 that may be closed by this pull request
@VietND96 VietND96 merged commit e2843cc into SeleniumHQ:trunk Dec 6, 2023
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[🐛 Bug]: 504 Gateway Time-out for Selenium Grid sessions
1 participant