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

Selenium Grid | Fix latest browser version use case scaling #4922

Merged
merged 5 commits into from
Sep 5, 2023

Conversation

Wolfe1
Copy link
Contributor

@Wolfe1 Wolfe1 commented Aug 30, 2023

Pull request #4348 broke scaling for the default use case (using only latest browser version). Items coming into the queue do not have a version number and would get counted correctly. After those items become sessions they are assigned a version number and were no longer counted by KEDA in this scenario. After the initial scale up we would only be monitoring the queue which would lead to stalled scaling or scaling down prematurely. More details in #4858

The purposed solution is to ignore the version number on the sessions when we are using the DefaultBrowserVersion. All existing unit tests still pass and have not been modified so #4348's use case should still be in tact. Added a couple unit tests to cover this default scenario.

Checklist

Fixes #4858

Relates to #4348

@Wolfe1 Wolfe1 requested a review from a team as a code owner August 30, 2023 15:14
@github-actions
Copy link

Thank you for your contribution! 🙏 We will review your PR as soon as possible.

🏖️ Over the summer, the response time will be longer than usual due to maintainers taking time off so please bear with us.

While you are waiting, make sure to:

Learn more about:

Signed-off-by: Brandon Wolfe <Wolfe1@gmail.com>
Signed-off-by: Brandon Wolfe <Wolfe1@gmail.com>
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.

Thanks for the fix!
Could you rebase main to resolve the merge conflict?

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

JorTurFer commented Sep 3, 2023

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

Wolfe1 and others added 2 commits September 5, 2023 09:26
Co-authored-by: Jorge Turrado Ferrero <Jorge_turrado@hotmail.es>
Signed-off-by: Brandon Wolfe <Wolfe1@gmail.com>
Signed-off-by: Brandon Wolfe <Wolfe1@gmail.com>
@JorTurFer
Copy link
Member

/skip-e2e

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.

Thanks for your contributions! ❤️

@JorTurFer JorTurFer merged commit acebbfb into kedacore:main Sep 5, 2023
19 checks passed
Adarsh-verma-14 pushed a commit to Adarsh-verma-14/keda that referenced this pull request Sep 6, 2023
…#4922)

Co-authored-by: Jorge Turrado Ferrero <Jorge_turrado@hotmail.es>
jeevanragula pushed a commit to jeevanragula/keda that referenced this pull request Sep 6, 2023
…#4922)

Co-authored-by: Jorge Turrado Ferrero <Jorge_turrado@hotmail.es>
toniiiik pushed a commit to toniiiik/keda that referenced this pull request Jan 15, 2024
…#4922)

Co-authored-by: Jorge Turrado Ferrero <Jorge_turrado@hotmail.es>
Signed-off-by: anton.lysina <alysina@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Modify scaling behaviour for selenium grid to remove effective cap on node creation
3 participants