actions: refactor to move template and manifest rendering in a helper #1563

ykaliuta · 2025-01-24T15:57:14Z

Jira: https://issues.redhat.com/browse/RHOAIENG-15921

actions: abstract caching functionality into cacher

The caching code for all caching Actions is pretty much the same

hash the key
if the hash is the same as the stored one, return stored resource
otherwise render the resource
store the key and the resource
return the resource

Abstract this logic into Cacher type which is supposed to be
included as a composition in on the higher levels.

The core engine is implemented as Render method which accepts actual
renderer and return boolean value in addition to the resources
indicating if it performed rendering. It supposed to be used in
resource rendering actions for accounting.

Possible usage supposed to be

type Action struct {
        cacher.Cacher
        // action data
}

type ActionOpts func(*Action)
func WithCache() ActionOpts {
        return func(a *Action) {
                a.Cacher.SetKeyFn(types.Hash) // or own
        }
}

func (s *Action) run(ctx context.Context, rr *types.ReconciliationRequest) error {
        //...
        res, acted, err := a.Cacher.Render(ctx, s.render, rr)
        if acted { /* may be do something */ }
        // use resources
}

func (s *Action) render(ctx context.Context, rr *types.ReconciliationRequest) (any, error) {
        result := make([]unstructured.Unstructured)
        // make some resources
        return result, nil
}

func NewAction(opts ...ActionOpts) actions.Fn {
        for _, opt := range opts {
                opt(&action)
        }

        return action.run
}

with reconciler creation

func (s *componentHandler) NewComponentReconciler(ctx context.Context, mgr ctrl.Manager) error {
        _, err := reconciler.ReconcilerFor(
                mgr,
                &componentApi.Type{},
        ).
                // ...
                WithAction(foobar.NewAction(
                        foobar.WithCache(),
                ))
}

Pay attention to signature of render() and call to Cacher.Render()
from run().

The code is taken pretty much from the existing kustomize rendering
Action.

Copy CachingKeyFn type. In future it can be extented to accept
context for possible debug logging.

Signed-off-by: Yauheni Kaliuta ykaliuta@redhat.com

actions: abstract resource caching into resourcecacher

Make an abstract wrapper on the raw cacher.Cacher to work on actions
whose Renderer produces resources ([]unstructured.Unstructured) to
be recorded into ReconciliationRequest.Manifests and later
deployed (deleted) to the cluster.

Performs common tasks:

invalidate caching if devflags enabled;
account resources if they were generated (and skip cached ones);
add resources to the ReconciliationRequest.

!!! The user should define Name() method for proper accounting.

At this moment this is the only part which interested in the origin
of resources (cached or not) and it's supposed to be used in actions
whose final goal is to add resources to the request (which is done
by this helper), so change signature of its Render()
method (comparing to cacher.Cacher) to return only error.

The code is basically copied from manifest/template rendering
actions run() method.

Usage (added Name() and changed run() comparing to cacher.Cacher):

type Action struct {
        resourcecacher.ResourceCacher
        // action data
}

type ActionOpts func(*Action)
func WithCache() ActionOpts {
        return func(a *Action) {
                a.ResourceCacher.SetKeyFn(types.Hash) // or own
        }
}

func (*Action) Name() string {
        return "CoolActionName"
}

func (s *Action) run(ctx context.Context, rr *types.ReconciliationRequest) error {
        //...
        return a.ResourceCacher.Render(ctx, s.render, rr)
}

func (s *Action) render(ctx context.Context, rr *types.ReconciliationRequest) (any, error) {
        result := make([]unstructured.Unstructured)
        // make some resources
        return result, nil
}

func NewAction(opts ...ActionOpts) actions.Fn {
        for _, opt := range opts {
                opt(&action)
        }

        return action.run
}

Signed-off-by: Yauheni Kaliuta ykaliuta@redhat.com

actions: use resourcecacher for manifests rendering

Implement Name() method as it is required by the ResourceCacher.

All the run() functionality now implemented by the ResourceCacher,
so just call its Render() in run().

Amend WithCache() as required by ResourceCacher to set its
CachingKeyFn and remove CachingKeyFn initialization from NewAction.

Change render() signature to return any

Signed-off-by: Yauheni Kaliuta ykaliuta@redhat.com

actions: use resourcecacher for template rendering

The same changes as to manifests rendering.

Signed-off-by: Yauheni Kaliuta ykaliuta@redhat.com

Description

How Has This Been Tested?

Screenshot or short clip

Merge criteria

You have read the contributors guide.
Commit messages are meaningful - have a clear and concise summary and detailed explanation of what was changed and why.
Pull Request contains a description of the solution, a link to the JIRA issue, and to any dependent or related Pull Request.
Testing instructions have been added in the PR body (for PRs involving changes that are not immediately obvious).
The developer has manually tested the changes and verified that the changes work

openshift-ci · 2025-01-24T15:57:19Z

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

ykaliuta · 2025-01-24T15:58:00Z

/cc @lburgazzoli

github-actions · 2025-01-24T15:58:20Z

This PR can't be merged just yet 😢

Please run make generate manifests api-docs and commit the changes.

For more info: https://github.com/opendatahub-io/opendatahub-operator/actions/runs/12953054333

codecov · 2025-01-24T16:00:46Z

Codecov Report

Attention: Patch coverage is 95.00000% with 4 lines in your changes missing coverage. Please review.

Project coverage is 20.09%. Comparing base (8340068) to head (b26704a).

Files with missing lines	Patch %	Lines
...ctions/render/kustomize/action_render_manifests.go	71.42%	2 Missing ⚠️
...actions/render/template/action_render_templates.go	66.66%	2 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1563      +/-   ##
==========================================
+ Coverage   19.94%   20.09%   +0.14%     
==========================================
  Files         158      160       +2     
  Lines       10799    10800       +1     
==========================================
+ Hits         2154     2170      +16     
+ Misses       8415     8407       -8     
+ Partials      230      223       -7

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

ugiordan · 2025-01-27T13:14:03Z