Experimental: Selenium Grid scaler with different nodeMaxSessions per node

[VietND96]

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

Motivation and Context

Before, autoscaling recommends each Node should have 1 session. However, due to user demand, and Node also can configure a different number of max sessions per node via SE_NODE_MAX_SESSIONS
Once Node is set the max sessions greater than 1, the scaler trigger parameter nodeMaxSessions need to be set for calculation can be aware Node capacity will be scaled up. Refer to https://github.com/SeleniumHQ/docker-selenium/blob/trunk/.keda/scalers/selenium-grid-scaler.md

This is still an experimental feature, so it requires KEDA core components image tag with the patch scaler implementation - see https://github.com/SeleniumHQ/docker-selenium/blob/trunk/.keda/README.md

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

Enhancement, Tests, Documentation

---

Description

• Introduced the ability to configure different nodeMaxSessions for each browser node (Chrome, Firefox, Edge) in Selenium Grid.
• Added tests to ensure the correct configuration of nodeMaxSessions and authentication references in scaler triggers.
• Updated documentation to reflect changes in configuration options and updated the kubectl image reference.
• Made formatting corrections and configuration changes across multiple files to support the new feature.

---

Changes walkthrough 📝

	Relevant files
Formatting	1 files video.sh Fix spacing in GraphQL endpoint error message                        Video/video.sh Fixed spacing issue in error message for GraphQL endpoint. +1/-1
Configuration changes	6 files chart_test.sh Add max session configuration for browser nodes                    tests/charts/make/chart_test.sh Added default max session variables for Chrome, Firefox, and Edge. Updated Helm command to set nodeMaxSessions for different browsers. +6/-0      Makefile Configure max sessions for autoscaling tests                          Makefile Added max session configurations for different browsers in autoscaling tests. +2/-1      node-configmap.yaml Remove SE_DRAIN_AFTER_SESSION_COUNT configuration                charts/selenium-grid/templates/node-configmap.yaml Removed SE_DRAIN_AFTER_SESSION_COUNT configuration. +0/-1      values.yaml Add nodeMaxSessions configuration and update kubectl image charts/selenium-grid/values.yaml Added nodeMaxSessions configuration for global and individual nodes. Updated kubectl image to bitnami/kubectl. +9/-1      dummy.yaml Configure nodeMaxSessions in dummy render template              tests/charts/templates/render/dummy.yaml Added nodeMaxSessions configuration for global and individual nodes. +3/-0      dummy_solution.yaml Configure nodeMaxSessions in dummy solution template          tests/charts/templates/render/dummy_solution.yaml Added nodeMaxSessions configuration for global and individual nodes. +3/-0
Tests	1 files test.py Add tests for scaler triggers and nodeMaxSessions                tests/charts/templates/test.py Added tests for scaler triggers authenticationRef name. Added tests for nodeMaxSessions parameter in scaler triggers. +28/-0
Enhancement	1 files _helpers.tpl Add nodeMaxSessions to autoscaling and pod templates          charts/selenium-grid/templates/_helpers.tpl Added nodeMaxSessions to autoscaling template. Set SE_NODE_MAX_SESSIONS environment variable in pod template. +15/-1
Documentation	2 files README.md Update KEDA core Helm chart link                                                  .keda/README.md Updated link to KEDA core Helm chart. +1/-1      CONFIGURATION.md Document nodeMaxSessions and update kubectl image                charts/selenium-grid/CONFIGURATION.md Documented nodeMaxSessions configuration for nodes. Updated kubectl image to bitnami/kubectl. +5/-1

---

💡 PR-Agent usage:
Comment /help on the PR to get a list of all available PR-Agent tools and their descriptions

[codiumai-pr-agent-pro]

PR Reviewer Guide 🔍

⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪

🧪 PR contains tests

🔒 No security concerns identified

⚡ Key issues to review Potential Bug The SE_DRAIN_AFTER_SESSION_COUNT environment variable is set based on the nodeMaxSessions value, but it's not clear if this is the intended behavior for all scaling types. Configuration Change The SE_DRAIN_AFTER_SESSION_COUNT environment variable has been removed from the node configmap. This might affect the behavior of node draining.

[codiumai-pr-agent-pro]

PR Code Suggestions ✨

Category	Suggestion	Score
Maintainability	Add a comment to explain the purpose of an environment variable Consider adding a comment explaining the purpose of the SE_DRAIN_AFTER_SESSION_COUNT environment variable and its relationship with KEDA and autoscaling. charts/selenium-grid/templates/_helpers.tpl [340-341] +# Set SE_DRAIN_AFTER_SESSION_COUNT to nodeMaxSessions if KEDA is used and scaling type is job, otherwise set to 0 - name: SE_DRAIN_AFTER_SESSION_COUNT value: {{ and (eq (include "seleniumGrid.useKEDA" $) "true") (eq .Values.autoscaling.scalingType "job") | ternary $nodeMaxSessions 0 | quote }} Apply this suggestion Suggestion importance[1-10]: 8 Why: Adding a comment to explain the purpose of the environment variable significantly aids in understanding the code, especially for new developers or maintainers.	8
Enhancement	Improve variable naming for better code readability and maintainability Use a more descriptive variable name instead of 'count' to improve code readability and maintainability. tests/charts/templates/test.py [347-358] -count = 0 +matched_resources_count = 0 for resource_name in resources_name.keys(): for doc in LIST_OF_DOCUMENTS: if doc['metadata']['name'] == resource_name and doc['kind'] == 'ScaledObject': logger.info(f"Assert nodeMaxSessions parameter is set in scaler triggers") self.assertTrue(doc['spec']['triggers'][0]['metadata']['nodeMaxSessions'] == str(resources_name[doc['metadata']['name']])) if doc['metadata']['name'] == resource_name and doc['kind'] == 'Deployment': for env in doc['spec']['template']['spec']['containers'][0]['env']: if env['name'] == 'SE_NODE_MAX_SESSIONS': self.assertTrue(env['value'] == str(resources_name[doc['metadata']['name']]), "Value is not matched") - count += 1 -self.assertEqual(count, len(resources_name.keys()), "Expected {0} resources but found {1}".format(len(resources_name.keys()), count)) + matched_resources_count += 1 +self.assertEqual(matched_resources_count, len(resources_name.keys()), "Expected {0} resources but found {1}".format(len(resources_name.keys()), matched_resources_count)) Apply this suggestion Suggestion importance[1-10]: 7 Why: The suggestion to use a more descriptive variable name enhances code readability and maintainability, which is beneficial for understanding and future modifications.	7
	Improve variable naming for better code readability Consider using a more descriptive variable name for clarity, such as nodeMaxSessionsValue instead of $nodeMaxSessions. charts/selenium-grid/templates/_helpers.tpl [210] -{{- $nodeMaxSessions := default $.Values.global.seleniumGrid.nodeMaxSessions .node.nodeMaxSessions | int64 -}} +{{- $nodeMaxSessionsValue := default $.Values.global.seleniumGrid.nodeMaxSessions .node.nodeMaxSessions | int64 -}} Apply this suggestion Suggestion importance[1-10]: 6 Why: A more descriptive variable name can enhance readability, but the existing name is not misleading, so the improvement is minor.	6
Best practice	Use explicit boolean comparison for improved code clarity Consider using a more explicit comparison for boolean values instead of relying on implicit conversion to improve code clarity. charts/selenium-grid/templates/_helpers.tpl [336-339] {{- if gt $nodeMaxSessions 1 }} - name: SE_NODE_OVERRIDE_MAX_SESSIONS - value: "true" + value: {{ gt $nodeMaxSessions 1 | quote }} {{- end }} Apply this suggestion Suggestion importance[1-10]: 5 Why: While using explicit boolean comparison can improve clarity, the current implicit conversion is already clear in context, making this suggestion a minor improvement.	5

💡 Need additional feedback ? start a PR chat