Skip to content

Limitador storage#727

Merged
fabikova merged 2 commits intoKuadrant:mainfrom
azgabur:limitador_storage
Sep 15, 2025
Merged

Limitador storage#727
fabikova merged 2 commits intoKuadrant:mainfrom
azgabur:limitador_storage

Conversation

@azgabur
Copy link
Copy Markdown
Member

@azgabur azgabur commented Aug 15, 2025

Work on #676

Adds basic durability tests for Limitador storage feature. It contains xfail for a bug Kuadrant/limitador-operator#197 which should be removed in future when the bug is fixed.

Also contains:

  • Refactor deprecated kuadrant sections. These no longer exist and instead were replaced by respective objects.
  • Refactor fetch_service_ip method to be more compatible with fetch_service method

Verification steps

make testsuite/tests/singlecluster/limitador/storage

Work left to do in next PR

@azgabur azgabur requested review from averevki and fabikova August 15, 2025 23:15
@azgabur azgabur mentioned this pull request Jun 3, 2025
6 tasks
@azgabur azgabur force-pushed the limitador_storage branch 2 times, most recently from 0b25743 to 2501ed7 Compare August 15, 2025 23:36
@fabikova
Copy link
Copy Markdown
Contributor

My local test run finished with 2 failed, 9 passed, 3 xfailed. Here's a summary of the durability tests:
test_durability for Disk: PASSED

test_durability for Dragonfly: PASSED

test_durability for RedisCached (redis, dragonfly, valkey): XFAIL (as you expected in bug)

test_durability for Redis and Valkey: FAILED

The interesting part is that the test_durability for plain Redis and Valkey failed for the exact same reason as bug #197 (AssertionError: assert 200 == 429 after the pod restart).

It seems bug #197 affects not just RedisCached, but also the standard Redis and Valkey storages. To make the test suite pass cleanly, maybe we could add an xfail marker to test_redis.py's durability test as well, referencing the same issue?

Comment thread testsuite/kuadrant/limitador.py Outdated
@azgabur azgabur force-pushed the limitador_storage branch from 2501ed7 to 936d943 Compare August 18, 2025 15:38
@azgabur
Copy link
Copy Markdown
Member Author

azgabur commented Aug 18, 2025

The interesting part is that the test_durability for plain Redis and Valkey failed for the exact same reason as bug #197 (AssertionError: assert 200 == 429 after the pod restart).

Yes I noticed these errors too, but they seem random and not as consistent as the bug Kuadrant/limitador-operator#197. After re-running the failed tests, they pass again. But it is possible it is connected. I will mention it in the issue. Additionally I do not think xfail is needed here as the tests pass 90% of the time, they are problematic when running together with the Cached tests

@azgabur azgabur force-pushed the limitador_storage branch from 936d943 to 10c76c3 Compare August 18, 2025 19:46
@azgabur
Copy link
Copy Markdown
Member Author

azgabur commented Aug 18, 2025

^ changed rate_limit fixture as it was not acting as "function" scope

Comment thread testsuite/tests/singlecluster/limitador/storage/conftest.py Outdated
Comment thread testsuite/kuadrant/limitador.py Outdated
Comment thread testsuite/tests/singlecluster/limitador/storage/db/test_redis.py Outdated
@azgabur azgabur force-pushed the limitador_storage branch 3 times, most recently from 460831c to 139ad64 Compare August 19, 2025 16:14
Comment thread testsuite/tests/singlecluster/limitador/storage/db/conftest.py Outdated
Comment thread testsuite/tests/singlecluster/limitador/storage/db/test_redis.py Outdated
Refactor deprecated kuadrant sections
Add rollout for Deployment
Refactor fetch_service_ip method to be more compatible with
fetch_service method

Signed-off-by: Alex Zgabur <azgabur@redhat.com>
Signed-off-by: Alex Zgabur <azgabur@redhat.com>
@averevki
Copy link
Copy Markdown
Contributor

/make disruptive

@github-actions
Copy link
Copy Markdown

Test run has started (make disruptive) and can be found here

@fabikova fabikova merged commit 41677a6 into Kuadrant:main Sep 15, 2025
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.

3 participants