RFC: Caching action #1563
Draft
RFC: Caching action #1563
+972
−76
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The caching code for all caching Actions is pretty much the same 1) hash the key 2) if the hash is the same as the stored one, return stored resource 3) otherwise render the resource 4) store the key and the resource 5) return the resource Abastract it with Renderer interface with Render() method which takes lower level renderer to make a chain. Make Render() return if it did actual rendering or returned cached value for further accounting on a higher level. Signed-off-by: Yauheni Kaliuta <[email protected]>
Create on level on the raw cacher to render Manifests and Templates which both produce unstructured resources and save them into ReconcilitaionRequest. Signed-off-by: Yauheni Kaliuta <[email protected]>
Signed-off-by: Yauheni Kaliuta <[email protected]>
For demo purposes. Signed-off-by: Yauheni Kaliuta <[email protected]>
Signed-off-by: Yauheni Kaliuta <[email protected]>
|
Skipping CI for Draft Pull Request. |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: 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 |
5 tasks
|
/cc @lburgazzoli |
This PR can't be merged just yet 😢Please run For more info: https://github.com/opendatahub-io/opendatahub-operator/actions/runs/12953054333 |
ugiordan
suggested changes
Jan 27, 2025
| "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/types" | ||
| ) | ||
|
|
||
| type Renderer interface { |
There was a problem hiding this comment.
Suggested change
| type Renderer interface { | |
| // Renderer defines an interface for rendering resources. | |
| type Renderer interface { |
| Render(r Renderer, rr *types.ReconciliationRequest) (any, bool, error) | ||
| } | ||
|
|
||
| type CachingKeyFn func(rr *types.ReconciliationRequest) ([]byte, error) |
There was a problem hiding this comment.
Suggested change
| type CachingKeyFn func(rr *types.ReconciliationRequest) ([]byte, error) | |
| // CachingKeyFn is a function type used to generate a caching key based on the reconciliation request. | |
| type CachingKeyFn func(rr *types.ReconciliationRequest) ([]byte, error) |
|
|
||
| type CachingKeyFn func(rr *types.ReconciliationRequest) ([]byte, error) | ||
|
|
||
| type Cacher struct { |
There was a problem hiding this comment.
Suggested change
| type Cacher struct { | |
| // Cacher provides caching capabilities for rendered resources. | |
| type Cacher struct { |
| s.cachingKeyFn = key | ||
| } | ||
|
|
||
| func (s *Cacher) InvalidateCache() { |
There was a problem hiding this comment.
Suggested change
| func (s *Cacher) InvalidateCache() { | |
| // InvalidateCache clears the cached key and resources, forcing a re-render on the next invocation. | |
| func (s *Cacher) InvalidateCache() { |
|
|
||
| type CacherOpts func(*Cacher) | ||
|
|
||
| func (s *Cacher) SetKey(key CachingKeyFn) { |
There was a problem hiding this comment.
Suggested change
| func (s *Cacher) SetKey(key CachingKeyFn) { | |
| // SetKey sets the caching key function for the Cacher. | |
| func (s *Cacher) SetKey(key CachingKeyFn) { |
| resList := resources.UnstructuredList(resUnstructured) | ||
|
|
||
| if acted { | ||
| controllerName := strings.ToLower(rr.Instance.GetObjectKind().GroupVersionKind().Kind) |
There was a problem hiding this comment.
Suggested change
| controllerName := strings.ToLower(rr.Instance.GetObjectKind().GroupVersionKind().Kind) | |
| // Track the number of rendered resources for monitoring | |
| controllerName := strings.ToLower(rr.Instance.GetObjectKind().GroupVersionKind().Kind) |
| controllerName := strings.ToLower(rr.Instance.GetObjectKind().GroupVersionKind().Kind) | ||
| render.RenderedResourcesTotal.WithLabelValues(controllerName, s.Name()).Add(float64(len(resList))) | ||
|
|
||
| rr.Generated = true |
There was a problem hiding this comment.
Suggested change
| rr.Generated = true | |
| // Mark the reconciliation request as having generated resources | |
| rr.Generated = true |
Comment on lines
+54
to
+55
| // deep copy object so changes done in the pipelines won't | ||
| // alter them |
There was a problem hiding this comment.
Suggested change
| // deep copy object so changes done in the pipelines won't | |
| // alter them | |
| // Append cloned resources to the reconciliation request, | |
| // ensuring a deep copy so changes in the pipeline don't alter the original objects. |
| rr.Resources = append(rr.Resources, result.Clone()...) | ||
|
|
||
| return nil | ||
| return res, true, nil | ||
| } | ||
|
|
||
| func (a *Action) render(rr *types.ReconciliationRequest) ([]unstructured.Unstructured, error) { |
There was a problem hiding this comment.
Suggested change
| func (a *Action) render(rr *types.ReconciliationRequest) ([]unstructured.Unstructured, error) { | |
| // render handles the rendering of resources using the Kustomize engine. | |
| func (a *Action) render(rr *types.ReconciliationRequest) ([]unstructured.Unstructured, error) { |
|
|
||
| rr.Generated = true | ||
| func (a *Action) Render(_ cacher.Renderer, rr *types.ReconciliationRequest) (any, bool, error) { |
There was a problem hiding this comment.
Suggested change
| func (a *Action) Render(_ cacher.Renderer, rr *types.ReconciliationRequest) (any, bool, error) { | |
| // Render is a wrapper for rendering the reconciliation request using Kustomize. | |
| // It returns the rendered resources and indicates if any resources were acted upon. | |
| func (a *Action) Render(_ cacher.Renderer, rr *types.ReconciliationRequest) (any, bool, error) { |
|
@ykaliuta, I believe the same caching abstraction can be applied to the |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Abstract caching functionality for actions.
Description
How Has This Been Tested?
Screenshot or short clip
Merge criteria