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

(Provided ServiceAccount) Follow-up: update dynamic cache to re-use cache.Cache instead of creating a new one every time #1025

Closed
everettraven opened this issue Jul 9, 2024 · 1 comment · Fixed by #1119
Assignees

Comments

@everettraven
Copy link
Contributor

everettraven commented Jul 9, 2024

In the first pass of the dynamic caching layer for ClusterExtension-managed content, we focused on core functionality and left optimizations for the future. This meant that the fastest path to core functionality being implemented was to stop and remove the old controller-runtime cache and create a new one on every call of the Watch() method for a given ClusterExtension.

Identified as an area of improvement, this ticket is meant to track the work necessary to optimize the caching layer by re-using the controller-runtime cache created on an initial Watch() method call.

This work likely involves:

  • Updating the existing logic to check if a cache has been created for a provided ClusterExtension
    • IF a cache HAS NOT been created for the provided ClusterExtension
      • Build a new runtime.Scheme with the provided client.Object slice
      • Store the runtime.Scheme for future modification
      • Keep a set of client.Objects used to create informers
    • IF a cache HAS been created for the provided ClusterExtension
      • Get the set of NEW resources that need to be managed
      • Using the set of NEW resources, update the runtime.Scheme for the ClusterExtension to register the new resources
      • Establish new watches/informers for the NEW resources to be managed
      • Get the set of resources that no longer need to be managed
        • Use the RemoveInformer() method on the cache.Cache type to remove the informers for no longer managed resources
        • Update the set of stored client.Objects to the new set of managed resources

Acceptance Criteria

  • Dynamic caching layer for ClusterExtension managed content is updated to re-use controller-runtime caches as outlined above
  • Unit tests updated as necessary
@everettraven
Copy link
Contributor Author

This likely entails caching additional information in the dynamic cache to update:

  • The runtime.Scheme used by an already created cache
  • The set of resources in which an informer was created

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Ready for Review
2 participants