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

feat: Modulereleasemeta catalog sync #2014

Conversation

Tomasz-Smelcerz-SAP
Copy link
Member

@Tomasz-Smelcerz-SAP Tomasz-Smelcerz-SAP commented Nov 5, 2024

Description

Extend existing remote catalog sync logic with support for ModuleReleaseMeta objects and corresponding CRD.

Note for reviewer: The catalog sync code in the internal/remote package requires refactoring to remove code duplication and improve it's internal structure. This is a work in progress that started already with PR: #2002
For now, to add support for syncing ModuleReleaseMeta, I tried to avoid big refactoring as much as possible.
This package is already complex and adding massive refactoring on top of a new feature means that the PR would be very hard to review (and the risk of introducing errors would be significant)
This is why there's so much code duplication at the moment - it's easier to understand the new sync logic if it just re-uses the already tried approach.
This code will be refactored later in a dedicated refactoring issue: #2015

Changes proposed in this pull request:

  • Extend the catalog sync logic in the internal/remote package with support for ModuleReleaseMeta objects
  • Use the new functionality in the Kyma reconciliation loop.

Related issue(s)
#1851

@Tomasz-Smelcerz-SAP Tomasz-Smelcerz-SAP requested a review from a team as a code owner November 5, 2024 10:37
@kyma-bot kyma-bot added cla: yes Indicates the PR's author has signed the CLA. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Nov 5, 2024
@Tomasz-Smelcerz-SAP Tomasz-Smelcerz-SAP force-pushed the feat/modulereleasemeta-catalog-sync branch from 1516789 to 08010c4 Compare November 5, 2024 10:41
@nesmabadr nesmabadr force-pushed the feat/modulereleasemeta-catalog-sync branch 4 times, most recently from 1b04052 to 61211d5 Compare November 6, 2024 10:23
@nesmabadr nesmabadr force-pushed the feat/modulereleasemeta-catalog-sync branch from 61211d5 to 21fff25 Compare November 6, 2024 10:39
internal/remote/modulereleasemeta_syncer.go Outdated Show resolved Hide resolved
internal/remote/moduletemplate_syncer.go Outdated Show resolved Hide resolved
internal/remote/crd.go Outdated Show resolved Hide resolved
internal/remote/modulereleasemeta_syncer_test.go Outdated Show resolved Hide resolved
@nesmabadr
Copy link
Contributor

LGTM, but we are still missing the documentation here

nesmabadr
nesmabadr previously approved these changes Nov 7, 2024
@kyma-bot kyma-bot added the lgtm Looks good to me! label Nov 7, 2024
@kyma-bot kyma-bot removed the lgtm Looks good to me! label Nov 7, 2024
@kyma-bot kyma-bot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Nov 7, 2024
nesmabadr
nesmabadr previously approved these changes Nov 7, 2024
@kyma-bot kyma-bot added the lgtm Looks good to me! label Nov 7, 2024
@kyma-bot kyma-bot added the lgtm Looks good to me! label Nov 7, 2024
@Tomasz-Smelcerz-SAP Tomasz-Smelcerz-SAP merged commit 43c06f9 into kyma-project:main Nov 7, 2024
63 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes Indicates the PR's author has signed the CLA. lgtm Looks good to me! size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants