test (backend) Added containerization support in testing and replaced SQLite with MySQL in the api-server/run_store_test
Description of your changes:
Motivation:
- While it's lightweight and fast, SQLite is less strict than MySQL in terms of data types, constraints, and SQL dialect compliance. This can lead to false positives — tests passing in SQLite but failing in production when using MySQL. For example, the attached test passes when using SQLite, but fails on MySQL due to stricter enforcement of constraints.
- Introducing Testcontainers simplifies testing when adding PostgreSQL support. Existing tests can be reused for both MySQL and PostgreSQL.
- Another nice thing is that you’ll be able to use tests for debugging the generated queries, since they now use the MySQL dialect — the same one as in production.
First approach implementation, applied only to run_details.
I ignored the test TestListRuns_Pagination_WithSortingOnMetrics because it generates an invalid query from the MySQL perspective (test was false-positive on SQLite). Below are two SQLite queries that will not work using the MySQL dialect query 1 without where by dummymetric contains the problem in the
SELECT
...
rd.dummymetric
FROM (...)
GROUP BY rd.UUID
Here rd.dummymetric is not included in the GROUP BY clause and is not aggregated, but it is used in the SELECT statement. will only work if the sql_mode is lowered for MySQL and the GROUP_BY_FULL mode is removed, because there is no aggregation for the dummymetric field in the query. Without this adjustment, MySQL will throw an error this query was generated here
mysql_and_where.txt doesn't work at all in MySQL. With exception ' Unknown column 'dummymetric' in 'where clause'' the second query was generated here
The SQLite alternatives to the above queries contain the same issues but still work because SQLite performs less strict query validation.
Also, just to double-check, I have reproduced the first query on my own Kubeflow Pipelines setup and it also threw the same error during the tests(dummymetric is just a random alias):
Additionally, since I am creating a container and reusing it for performance reasons, I had to add logic for cleaning the database between tests, which complicated the initialization logic in run_details_test
Hi @ntny. Thanks for your PR.
I'm waiting for a kubeflow member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.
Once the patch is verified, the new status will be reflected by the ok-to-test label.
I understand the commands that are listed here.
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.
🚫 This command cannot be processed. Only organization members or owners can use the commands.
Hello @HumairAK, I found the closed issue with a similar motivation described. There are several merged commits mostly related to preparation for switching from SQLite testing to MySQL.
However, SQLite tests are still being used in api-server/storage, and it seems that #8851 is still relevant and the current PR partially closed it. Could you please take a look and correct me if I'm wrong?
@nsingla - are you able to also take a look at this pr?
[APPROVALNOTIFIER] This PR is NOT APPROVED
This pull-request has been approved by: Once this PR has been reviewed and has the lgtm label, please ask for approval from humairak. For more information see the Kubernetes Code Review Process.
The full list of commands accepted by this bot can be found here.
Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment
Out of scope for this PR: we should try to remove the hard coded values in assertions to use the created objects in the setup/initialize phase