✨ Introduce structured keys in pkg/cache

[MikeSpreitzer]

Summary

Did you know that a controller's workqueue contains any value, not necessarily a string? This PR adds a structured alternative to the common encoded string, relieving queue consumers from the complexity of coping with parsing errors. It is also a useful datatype in many other places.

Related issue(s)

Fixes #

[openshift-ci]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign sttts for approval. For more information see the Kubernetes Code Review Process.

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

Needs approval from an approver in each of these files:

• OWNERS

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

[MikeSpreitzer]

/cc @vincepri

[MikeSpreitzer]

@ezrasilvera

[stevekuznetsov]

This is something we had some informal conversations about a couple years ago, the comments in the Kubernetes codebase seem to suggest that a) there was some thought to making this structured in the past and b) there were also concerns about allocations. It might be good to see if e.g. David Eads or Jordan Liggitt have in their memory some conversations on this topic. Certainly OK to add this as an opt-in option for controller authors, but if there are no real concerns with the approach it might be nice to get it to be the default, too.

[stevekuznetsov]

nit: the convention is to call this a KeyFunc

[MikeSpreitzer]

Yeah, I never understood why the names of these functions have to end in "Func" while all the other functions do not require that. The more common use of the suffix "Func" is in the name of a type, not a function.

[stevekuznetsov]

I agree! I only point it out since it might make it clearer for consumers that are used to that convention.

[MikeSpreitzer]

I am not sure I follow the concern about allocations. I am assuming that both boxing a string as an any and boxing a Key as an any require 1 allocation. Avoiding construction of the composite encoded string saves at least 1 allocation. I am assuming that function parameters and variables of type Key do not imply heap allocations.

I think this PR is written in such a way that it does introduce an "opt in".

[stevekuznetsov]

I'm not entirely clear on the allocation concerns but I think it was moreso the allocation for the Key{} as opposed to the string, not the boxing - but in any case if it's opt-in, I don't think it matters much. This PR LGTM - @sttts any thoughts?

[MikeSpreitzer]

The upstream version of this is kubernetes/kubernetes#116869

[xrstf]

Closing & re-opening the PR to trigger the new CI setup. Please do not be alarmed :)

[kcp-ci-bot]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign sttts for approval. For more information see the Kubernetes Code Review Process.

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

Needs approval from an approver in each of these files:

• OWNERS

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

[kcp-ci-bot]

@MikeSpreitzer: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
pull-apimachinery-lint	f0b0334	link	true	/test pull-apimachinery-lint

Full PR test history

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. I understand the commands that are listed here.