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

add sessionsPerNode and setSessionsFromHub parameters to selenium-grid scaler #6055

Merged

Conversation

rubroboletus
Copy link
Contributor

Added two new parameters for selenium-grid scaler, allowing to scale up based on real browser node sizing. When sesSessionsFromHub is set, KEDA will look for existing browser nodes with browserName, browserVersion and platformName same as scaledObject / scaledJob, get maxSessions and calculate how much nodes it needs. When no existing browser node matches, it uses sessionsPerNode value for calculation.

Checklist

Fixes #

Relates to #

@SpiritZhou
Copy link
Contributor

Could you open an issue and provide a description of your improvement for better tracking?

CHANGELOG.md Outdated Show resolved Hide resolved
Copy link
Member

@JorTurFer JorTurFer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking good ❤️ @Wolfe1 WDYT?

CHANGELOG.md Outdated Show resolved Hide resolved
@JorTurFer
Copy link
Member

JorTurFer commented Aug 19, 2024

/run-e2e selenium
Update: You can check the progress here

@JorTurFer
Copy link
Member

@rubroboletus , there are 2 checks failing:

Could you take a look please?

rubroboletus and others added 12 commits August 20, 2024 07:09
…d scaler, add tests and changelog

Signed-off-by: Hanzlik, Robert (MONETA) <[email protected]>
Signed-off-by: Hanzlik, Robert (MONETA) <[email protected]>
Signed-off-by: Hanzlik, Robert (MONETA) <[email protected]>
Signed-off-by: Hanzlik, Robert (MONETA) <[email protected]>
Signed-off-by: Hanzlik, Robert (MONETA) <[email protected]>
Signed-off-by: Hanzlik, Robert (MONETA) <[email protected]>
Signed-off-by: Hanzlik, Robert (MONETA) <[email protected]>
Signed-off-by: Hanzlik, Robert (MONETA) <[email protected]>
Signed-off-by: Hanzlik, Robert (MONETA) <[email protected]>
Signed-off-by: Hanzlik, Robert (MONETA) <[email protected]>
Signed-off-by: Hanzlik, Robert (MONETA) <[email protected]>
Co-authored-by: Jorge Turrado Ferrero <[email protected]>
Signed-off-by: Robert Hanzlík <[email protected]>
Signed-off-by: Hanzlik, Robert (MONETA) <[email protected]>
@rubroboletus rubroboletus force-pushed the feature/selenium-grid-stereotypes branch from 55c5325 to 4e22dea Compare August 20, 2024 07:10
Signed-off-by: Hanzlik, Robert (MONETA) <[email protected]>
@rubroboletus
Copy link
Contributor Author

@rubroboletus , there are 2 checks failing:

Could you take a look please?

@JorTurFer Fixed.

@JorTurFer
Copy link
Member

JorTurFer commented Sep 2, 2024

/run-e2e selenium
Update: You can check the progress here

@VietND96
Copy link
Contributor

@rubroboletus, can you review this PR #6169 to see it can solve your expectation.
In the PR, there is no need for new params. We extract Node stereotypes, node sessions against requests capabilities.
Expect that a Node with different max sessions is set, scaler will look at the slots available and return correct scale needed.

@rubroboletus
Copy link
Contributor Author

@rubroboletus, can you review this PR #6169 to see it can solve your expectation. In the PR, there is no need for new params. We extract Node stereotypes, node sessions against requests capabilities. Expect that a Node with different max sessions is set, scaler will look at the slots available and return correct scale needed.

Problem is, that you can obtain info from the Grid only in case, that there is at least one node of such type running. When there is no node of such type running, you know nothing about its settings.

@VietND96
Copy link
Contributor

Ok, I can understand now, the problem for calculation when minReplica=0.
Let me update my PR and add a test to prove it works.

@VietND96
Copy link
Contributor

I updated one more scaler param nodeMaxSessions (the naming aligns with CLI option --max-sessions - https://www.selenium.dev/documentation/grid/configuration/cli_options/#node)
Few tests for that param could be seen in https://github.com/kedacore/keda/pull/6169/files#diff-59589fb3c981e73ac59806307eb5a26330cafb0260a88777cba9124621748a8cR1410

@JorTurFer JorTurFer merged commit a103204 into kedacore:main Oct 16, 2024
14 of 18 checks passed
rickbrouwer pushed a commit to rickbrouwer/keda that referenced this pull request Oct 21, 2024
…d scaler (kedacore#6055)

* add sessionsPerNode and setSessionsFromHub parameters to selenium-grid scaler, add tests and changelog

Signed-off-by: Hanzlik, Robert (MONETA) <[email protected]>

* fix tests

Signed-off-by: Hanzlik, Robert (MONETA) <[email protected]>

* fix json

Signed-off-by: Hanzlik, Robert (MONETA) <[email protected]>

* fix json

Signed-off-by: Hanzlik, Robert (MONETA) <[email protected]>

* fix tests

Signed-off-by: Hanzlik, Robert (MONETA) <[email protected]>

* fix metadata tests

Signed-off-by: Hanzlik, Robert (MONETA) <[email protected]>

* fix syntax

Signed-off-by: Hanzlik, Robert (MONETA) <[email protected]>

* link Issue to Changelog

Signed-off-by: Hanzlik, Robert (MONETA) <[email protected]>

* fix link to issue

Signed-off-by: Hanzlik, Robert (MONETA) <[email protected]>

* add sessionBrowserVersion and tests

Signed-off-by: Hanzlik, Robert (MONETA) <[email protected]>

* add metadata tests

Signed-off-by: Hanzlik, Robert (MONETA) <[email protected]>

* Update CHANGELOG.md

Co-authored-by: Jorge Turrado Ferrero <[email protected]>
Signed-off-by: Robert Hanzlík <[email protected]>
Signed-off-by: Hanzlik, Robert (MONETA) <[email protected]>

* make cyclomatic complexity smaller

Signed-off-by: Hanzlik, Robert (MONETA) <[email protected]>

---------

Signed-off-by: Hanzlik, Robert (MONETA) <[email protected]>
Signed-off-by: Robert Hanzlík <[email protected]>
Signed-off-by: Jorge Turrado Ferrero <[email protected]>
Co-authored-by: Jorge Turrado Ferrero <[email protected]>
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.

5 participants