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

[ModuleCatalog] Extend Catalog sync to handle both ModuleTemplate and ModuleReleaseMeta CRs #1851

Closed
3 of 12 tasks
jeremyharisch opened this issue Sep 9, 2024 · 1 comment · Fixed by #2025
Closed
3 of 12 tasks
Labels
kind/feature Categorizes issue or PR as related to a new feature.

Comments

@jeremyharisch
Copy link
Contributor

jeremyharisch commented Sep 9, 2024

Note

The original issue was about creating a dedicated sync controller.
For now though, we decided to just add the implementation to the current codebase with necessary refactorings.
I am leaving the original issue unchanged (below) for reference; this will be extracted to a separate issue after the first version of the extended CatalogSync code is implemented and released.

Current Acceptance Criteria:

  • Extend the module catalog sync with support for ModuleReleaseMeta CRs
  • Update the documentation
  • Extend the test "CRDs sync to SKR and annotations updated in KCP kyma" to cover ModuleReleaseMeta CRs

Original Description:

The synchronization process of ModuleTemplate CRs to all SKR clusters is currently embedded within the Kyma reconcile loop. We have decided to expand this functionality to include ModuleReleaseMeta CRs and to relocate these synchronization tasks to a new, dedicated controller. This approach is intended to enhance performance by offloading the sync operations from the main Kyma reconcile loop.

Detailed decision ticket: #1815

Current Implementation:

The existing synchronization of ModuleTemplate CRs is integrated into the Kyma reconcile loop, detailed here.

Objective:

  • Develop a Dedicated Sync Controller:

    • This new controller will handle the synchronization of both ModuleTemplate and ModuleReleaseMeta CRs from the KCP cluster to all SKR clusters.
    • It will manage the lifecycle of these CRs—handling creation, updates, and deletions—and will ensure that all corresponding Kyma CRs are requeued accordingly.
  • Performance Optimization:

    • By isolating the synchronization process, we aim to reduce the overhead in the Kyma reconcile loop, which currently dedicates approximately 20% of its time to sync operations.
  • Testing and Validation:

    • Comprehensive E2E and integration testing will be conducted to ensure the new controller operates effectively and integrates smoothly with existing systems.
    • Adjustments will be made to existing tests that are affected by this change.

Benefits of Separation of Concerns:

  • Enhanced Performance: Removing the sync tasks from the Kyma controller allows for more focused and efficient reconciliation processes.
  • Reduced Complexity in Core Operations: Segregating the synchronization functionality simplifies the main Kyma reconcile loop, making it easier to maintain and scale.

Potential Challenges:

  • System Complexity: Introducing a new controller adds a layer of complexity to the overall architecture.
  • Race Conditions: Separate controllers working on related resources could lead to conflicts, although these are generally manageable within Kubernetes through established patterns.

Acceptance Criteria:

  • Implementation of the New Controller: The controller successfully synchronizes ModuleTemplate and ModuleReleaseMeta CRs across SKR clusters.
    • Make sure code is easy testable, stick to our current implementation patterns.
    • If a ModuleTemplate or ModuleReleaseMeta CR gets created, updated or deleted, all Kyma CRs should be requeued, since all are affected automatically.
    • Name of controller: SyncResources
    • Cover controller with basic metrics
    • Update only affects the .status.condition
  • Performance Improvements Documented: Metrics demonstrating the reduced load on the Kyma reconcile loop.
  • Comprehensive Test Coverage: The new functionality is fully covered by E2E and integration tests.
  • Documentation: Updated documentation reflecting the new controller’s role and functionality within the ecosystem.
@c-pius c-pius changed the title Implementation of a Dedicated Controller for Syncing ModuleTemplate and ModuleReleaseMeta CRs [ModuleCatalog] Implementation of a Dedicated Controller for Syncing ModuleTemplate and ModuleReleaseMeta CRs Sep 10, 2024
@jeremyharisch jeremyharisch added the kind/feature Categorizes issue or PR as related to a new feature. label Sep 11, 2024
@Tomasz-Smelcerz-SAP Tomasz-Smelcerz-SAP self-assigned this Sep 30, 2024
@Tomasz-Smelcerz-SAP
Copy link
Member

I think we need to refine on the AC: "Name of controller: SyncResources". The SyncResources is too generic and vague. I suggest to use just: catalogsync

@Tomasz-Smelcerz-SAP Tomasz-Smelcerz-SAP changed the title [ModuleCatalog] Implementation of a Dedicated Controller for Syncing ModuleTemplate and ModuleReleaseMeta CRs [ModuleCatalog] Extend Catalog sync to handle both ModuleTemplate and ModuleReleaseMeta CRs Oct 31, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/feature Categorizes issue or PR as related to a new feature.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants