✨ Add TransformFuncByObject Option for Informer Cache

[alexzielenski]

This PR is a complement to a recent addition to client-go kubernetes/kubernetes#107507.

Adds two new ForOptions, OwnsOptions, and WatchesOptions to be used with the existing OnlyMetadata called WithoutAnnotations and WithoutManagedFields.

Motivation

We have teams which make extensive use of metadata-only watches (tens of thousands of objects), which have seen sizable reductions in RAM usage when these fields are removed. This new options makes it easy and ergonomic to opt into stripping managedFields and annotations metadata before they are persisted in the cache, to avoid wasting resources in controllers which do not use those fields.

Implementation

Behind the scenes uses SetTransform on SharedIndexInformer, and uses a transformer which respects the builders' elections for each GVK.

Example Usage

bldr := ControllerManagedBy(mgr).
        For(&appsv1.Deployment{}, OnlyMetadata, WithoutAnnotations, WithoutManagedFields)

The above line of code creates a controller whose cache is metadata-only and the annotations/managed fields have been stripped from the metadata.

Comments

I'm open to other ways to expose this new SetTransform API, but would prefer to have it specified near the builder since OnlyMetadata is specified there as well. It seems natural to keep the different options for cache projections together.

Additionally included is a go.mod update to a version of client-go which includes SetTransform. I am unsure if this PR is the best place to put it, or if another PR should be created updating all components in one shot.

It should be noted that in my implementation WithoutAnnotations and WithoutManagedFields only apply to the PartialObjectMeta cache, and not unstructured/structured caches. This was intentional, since I did not see a reason to use it against other cache types. But this point may be worth discussing.

[k8s-ci-robot]

Welcome @alexzielenski!

It looks like this is your first PR to kubernetes-sigs/controller-runtime 🎉. Please refer to our pull request process documentation to help your PR have a smooth ride to approval.

You will be prompted by a bot to use commands during the review process. Do not be afraid to follow the prompts! It is okay to experiment. Here is the bot commands documentation.

You can also check if kubernetes-sigs/controller-runtime has its own contribution guidelines.

You may want to refer to our testing guide if you run into trouble with your tests not passing.

If you are having difficulty getting your pull request seen, please follow the recommended escalation practices. Also, for tips and tricks in the contribution process you may want to read the Kubernetes contributor cheat sheet. We want to make sure your contribution gets all the attention it needs!

Thank you, and welcome to Kubernetes. 😃

[k8s-ci-robot]

Hi @alexzielenski. Thanks for your PR.

I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[FillZpp]

@alexzielenski Thanks. This is the first time I have seen kubernetes/kubernetes#107507 , it is awesome that we can reduce the cache size with TransformFunc. So I'm thinking why not expose that for controller-runtime cache, so that developers could have their transform functions for different GVKs in cache?

Also this might have to wait for k8s v1.24 release to avoid alpha dependency in go.mod.

[alexzielenski]

reviving this PR now that 1.24 is being release

[alexzielenski]

@FillZpp What do you think of the new interface example usage below? It is a bit different from what we talked about on slack, because I still want to give control to only apply transformer to partialMetadata vs full objects.

nodeMetadata := &metav1.PartialObjectMetadata{}
nodeMetadata.SetGroupVersionKind(schema.GroupVersionKind{
						Group:   "",
						Version: "v1",
						Kind:    "Node",
					})
ctrl.Options.NewCache = cache.BuilderWithOptions(cache.Options{
    TransformFuncByObject: cache.TransformFuncByObject{
            &corev1.Node{}: func(val interface{}) interface{} {
                  // Transformer for structured v1.Node
                 ....
             },
             nodeMetadata: func(val interface{}) interface{} {
                  // Transformer for metadata v1.Node
                  ....
             }
    }
})

[FillZpp]

/ok-to-test

[FillZpp]

/retitle ✨ Add TransformFuncByObject Option for Informer Cache

[alvaroaleman]

Two small points, other than that it looks good to me. @FillZpp is there anything left on your end?

[alexzielenski]

/retest

[FillZpp]

@FillZpp is there anything left on your end?

Yeah, I'm not sure if we should define some common transform functions so that most users don't have to search the source code to see how to implement it. (If yes, we should also discuss where to define these functions)
For example:

func TransformStripManagedFields(obj interface{}) (interface{}, error) {
    meta, err := apimeta.Accessor(obj)
    if err != nil {
        return nil, err
    }
    meta.SetManagedFields(nil)
    return obj, nil
}

It can be used for typed, Unstructured and PartialbjectMetadata objects.
WDYT @alvaroaleman

Another question, should we deep copy first or just modify the given object in the transform func? @alexzielenski

[alvaroaleman]

Yeah, I'm not sure if we should define some common transform functions

We don't know how commonly wanted this is. This is definitely an advanced feature that might also serve as a footgun. I'd prefer to not do that unless we are certain that is something ppl are actually interested in.

Another question, should we deep copy first or just modify the given object in the transform func

The whole point is to manipulate the data in the cache directly, so no, no deepcopy.

I am happy with the public api changes and there is decent test coverage, so lets merge this.
/lgtm
/approve

@alexzielenski thanks for making this happen!

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: alexzielenski, alvaroaleman

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• OWNERS [alvaroaleman]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment