Skip to content

Conversation

@kaikaila
Copy link
Contributor

@kaikaila kaikaila commented May 19, 2025

Background

As part of the migration from GORM v1 to GORM v2, this PR updates the ResourceReference model to use the new struct tags and conventions.

Changes

  • Replaced legacy gorm:"primary_key" tags with gorm:"primaryKey"
  • Ensured composite primary key across ResourceUUID, ResourceType, and ReferenceType
  • Preserved column names using gorm:"column:..." for compatibility

Verification

  • AutoMigrate(&ResourceReference{}) successfully tested via local GORM v2 playground
  • ✅ All existing unit tests involving ResourceReference passed, including:
    • TestResourceReferenceStore in resource_reference_store_test.go
    • TestParseResourceReferences in list_test.go
    • Conversion and validation logic in server_test.go, converter_test.go

Notes

This PR is the first in a series of staged model migrations to GORM v2.
Design Doc will be posted in the community Slack on Monday morning.

No runtime logic is changed. This is a safe schema-only update.


Just saw that a couple CI checks failed. I’m currently investigating the errors and will follow up with a fix shortly.

Marking this PR as [WIP] for now to avoid unnecessary reviews.

@google-oss-prow
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.

@github-actions
Copy link

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

@google-oss-prow google-oss-prow bot requested review from HumairAK and gmfrasca May 19, 2025 07:52
@kaikaila kaikaila force-pushed the chore/orm-migrate-resource-reference branch from 473486c to b727103 Compare May 19, 2025 07:57
@kaikaila kaikaila changed the title [WIP]chore(backend): migrate ResourceReference model to GORM v2 chore(backend): migrate ResourceReference model to GORM v2 May 19, 2025
@kaikaila
Copy link
Contributor Author

Hi @mprahl – following your earlier suggestion on Slack, I just opened this PR to begin migrating the ResourceReference model to GORM v2.

It's the first in a staged series. The design doc with full context (breaking change audit, PoC, migration plan) will be posted in Slack on Monday morning, but this PR already aligns with the proposed strategy and passes all tests.

Would love to get your thoughts on structure and next steps 🙏

@kaikaila kaikaila changed the title chore(backend): migrate ResourceReference model to GORM v2 [WIP] chore(backend): migrate ResourceReference model to GORM v2 May 19, 2025
@google-oss-prow google-oss-prow bot added size/M and removed size/L labels May 20, 2025
@kaikaila
Copy link
Contributor Author

Hi @humair,

CI/CD failed previously due to this MySQL 8.4 error in the pod logs:
Can’t find error-message file ‘/usr/share/mysql-8.4/english/errmsg.sys’

I've tested by temporarily downgrading to mysql:8.0.26, which fixed the issue and allowed CI to pass.

This suggests the root cause is in the mysql:8.4 image used in:
manifests/kustomize/third-party/mysql/base/mysql-deployment.yaml

I’m happy to revert it back to 8.4 if there's a known fix or replacement image. Let me know how you’d like to proceed.

@kaikaila kaikaila changed the title [WIP] chore(backend): migrate ResourceReference model to GORM v2 chore(backend): migrate ResourceReference model to GORM v2 May 20, 2025
@kaikaila kaikaila changed the title chore(backend): migrate ResourceReference model to GORM v2 [WIP]chore(backend): migrate ResourceReference model to GORM v2 May 22, 2025
@kaikaila kaikaila force-pushed the chore/orm-migrate-resource-reference branch 2 times, most recently from a95c645 to 6819530 Compare May 24, 2025 21:44
@google-oss-prow google-oss-prow bot added size/S and removed size/M labels May 24, 2025
@google-oss-prow
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 hbelmiro 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

@kaikaila
Copy link
Contributor Author

kaikaila commented May 25, 2025

merged into a larger migration in #11929

@kaikaila kaikaila closed this May 25, 2025
@kaikaila kaikaila deleted the chore/orm-migrate-resource-reference branch June 25, 2025 07:24
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.

1 participant