search

SeleniumHQ / docker-selenium

Provides a simple way to run Selenium Grid with Chrome, Firefox, and Edge using Docker, making it easier to perform browser automation
http://www.selenium.dev/docker-selenium/
Other
7.86k stars 2.51k forks source link

[🚀 Feature]: Selenium Grid Helm Chart: Do Not Patch Keda If Disabled #2268

Closed AndreasPetersen closed 2 months ago

AndreasPetersen commented 3 months ago

Feature and motivation

We have disabled autoscaling on our Selenium Grid:

autoscaling:
  enabled: false

But when we update the grid, Keda is still patched, because

https://github.com/SeleniumHQ/docker-selenium/blob/trunk/charts/selenium-grid/templates/patch-keda/patch-keda-objects-job.yaml#L1

does not check if autoscaling is also enabled.

This unexpectedly caused some issues for us, as we were not expecting Keda to patch. Specifically we need to fetch our Docker images through a corporate Artifactory, so the default kubectlImage was being blocked, causing upgrade failure.

I propose that patching of Keda is disabled if autoscaling is disabled.

Usage example

Setting

autoscaling:
  enabled: false

in values.yaml will disable Keda patching.

github-actions[bot] commented 3 months ago

@AndreasPetersen, thank you for creating this issue. We will troubleshoot it as soon as we can.

Info for maintainers

Triage this issue by using labels.

If information is missing, add a helpful comment and then I-issue-template label.

If the issue is a question, add the I-question label.

If the issue is valid but there is no time to troubleshoot it, consider adding the help wanted label.

If the issue requires changes or fixes from an external project (e.g., ChromeDriver, GeckoDriver, MSEdgeDriver, W3C), add the applicable G-* label, and it will provide the correct link and auto-close the issue.

After troubleshooting the issue, please add the R-awaiting answer label.

Thank you!

VietND96 commented 3 months ago

Hi, for the reason the patch job is running even autoscaling.enabled: false to ensure cleanup in case upgrade an existing deployment with enabled autoscaling to disabled autoscaling. Of course, I agree that it will cause annoying for common usage. Will update another mechanism for this. In the meantime, you can use config autoscaling.patchObjectFinalizers.enabled to disable it separately.

AndreasPetersen commented 3 months ago

Thanks for the quick reply @VietND96 .

Like you said, I specifically disabled the patcher, and upgrading the grid then works as expected.

github-actions[bot] commented 1 month ago

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.