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

Make gateway fixture session scope #364

Merged
merged 1 commit into from
Apr 10, 2024
Merged

Conversation

pehala
Copy link
Contributor

@pehala pehala commented Mar 12, 2024

  • Solves problems with limitador test concurrency failures
  • Increases time for running individual tests but doesn't change overall test time even if load balancers take a while
  • Individual tests still can use module scope GW
    • MGC still uses it

Thanks @azgabur for investigating!

PS: copy of #362, which I inexplicably closed

Fixes #324

@pehala pehala marked this pull request as ready for review March 12, 2024 12:21
@pehala pehala requested review from averevki and martinhesko March 12, 2024 12:21
martinhesko
martinhesko previously approved these changes Mar 12, 2024
Copy link
Contributor

@azgabur azgabur left a comment

Choose a reason for hiding this comment

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

I just noticed the make authorino-standalone is erorring out with ScopeMismatch. Not approved yet.

@pehala
Copy link
Contributor Author

pehala commented Mar 15, 2024

I just noticed the make authorino-standalone is erorring out with ScopeMismatch. Not approved yet.

Thanks to you and also @averevki (who also reported the same issue). It should be fixed now

@pehala pehala force-pushed the session_gw branch 2 times, most recently from d736233 to 48e2452 Compare April 8, 2024 12:49
azgabur
azgabur previously approved these changes Apr 9, 2024
Copy link
Contributor

@azgabur azgabur left a comment

Choose a reason for hiding this comment

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

This PR will solve limitador tests concurrency issues. But will also create some new ones:

  • In case the Gateway object will never reconcile due to some error, testsuite will show error only after gw.wait_for_ready() timeout, which is currently 10 minutes
  • The current timeout is enough for our Openstack testing environment, but just barely.
    • Raising number of threads from default 4 to more will cause timeout errors.
    • Running the testsuite in parallel, which can happen if two people run it on the same environment at the same time, will cause timeout errors at least in one of those runs.
    • Running testsuite just after finishing previous run will cause timeout errors because of slowly deleting load balancers.
  • Quickly debugging functionality of single test will now take ~2 minutes on our testing environment even if most of the authorino tests do not need the gateway to reconcile to function properly.

Overall I recommend making the timeout a variable in settings.yaml so it can be adjusted before running testsuite if the testing environment is faster/slower. This can be done in next PR.

I also recommend to watch the changes in workflow of test development, namely longer waits in running tests and see later if it needs to be adjusted or no. Adjustments could be for example making the session scope as an development option in settings.yaml or making the limitador tests the only ones who require the session scope.

@averevki averevki merged commit ee481e4 into Kuadrant:main Apr 10, 2024
3 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.

Fix RateLimit tests failures in parallel
4 participants