Skip to content

[WIP]chore(backend): migrate GORM v1 to v2 #11929

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

Open
wants to merge 16 commits into
base: master
Choose a base branch
from

Conversation

kaikaila
Copy link
Contributor

@kaikaila kaikaila commented May 25, 2025

Summary

This PR migrates the entire ORM layer of the Kubeflow Pipelines backend from GORM v1 (github.com/jinzhu/gorm) to GORM v2 (gorm.io/gorm), covering:

  • ✅ All core model definitions in backend/src/apiserver/model
  • ✅ Full migration of the Execution Cache system under backend/src/cache/...

Migration Details

Models Migration (GORM v1 → v2)

  • Replaced all gorm:"primary_key" with gorm:"primaryKey"
  • Ensured sql.Null* fields are handled consistently (where applicable)
  • Switched from deprecated NewRecord() to .Create().Error-based flow
  • Updated all AutoMigrate() calls to conform to GORM v2 APIs
  • Removed all legacy GORM v1 imports

Execution Cache Migration

  • Updated NewFakeDB, ClientManager, and associated test helpers to GORM v2
  • Migrated store logic in execution_cache_store.go to use db.Create(...).Scan(...) patterns
  • Removed deprecated NewRecord() usage and added structured error wrapping
  • Restored non-unique index on ExecutionCacheKey to match v1 behavior (tests rely on repeated keys + latest record retrieval)

Client Manager Migration

Changes
• Remove manual index creation block in InitDBClient (client_manager.go).
• Add type:varchar(191) + index:...,priority:... tags in:
• model.Run.ExperimentId and model.Run.Namespace
• model.RunDetails.Conditions and model.RunDetails.FinishedAtInSec
• model.RunDetails.CreatedAtInSec
• Add uniqueIndex:name_namespace_index,priority:... tags in model.Pipeline.Name and model.Pipeline.Namespace
• Retain original index names for backward compatibility and least-surprise.


Notes

This PR supersedes any previous model-level-only migration PRs (e.g., chore/orm-migrate-resource-reference) by consolidating all ORM transitions into a unified update.
It also ensures that GORM v1 is no longer referenced anywhere in the backend codebase.


Checklist

  • All tests pass locally (go test ./backend/...)
  • go.mod and imports updated to use GORM v2
  • No GORM v1 usage remains
  • Migration is backward-compatible with existing data schema where possible

Copy link

[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 assign james-jwu for approval. For more information see the Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

Copy link

Hi @kaikaila. 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.

Copy link

🚫 This command cannot be processed. Only organization members or owners can use the commands.

Copy link

@kaikaila: Cannot trigger testing until a trusted user reviews the PR and leaves an /ok-to-test message.

In response to this:

/retest

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.

Copy link

🚫 This command cannot be processed. Only organization members or owners can use the commands.

@zazulam
Copy link
Contributor

zazulam commented May 26, 2025

/ok-to-test

@kaikaila kaikaila force-pushed the chore/orm-migrate-cache branch from 8b74759 to a8f55db Compare May 28, 2025 04:37
@google-oss-prow google-oss-prow bot added size/L and removed size/XL labels May 29, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants