KEP-4785: CRDMetrics Controller

[rexagod]

• One-line PR description: This KEP proposes the incorporation of the CRDMetrics controller into the Kubernetes organization, similar to its existing counterpart for native metrics, Kube State Metrics.

• Issue link: https://github.com/kubernetes/enhancements/issues/4785

• Other comments: Please refer to https://github.com/rexagod/crdmetrics for more details.

[sftim]

Is CustomResourceMetricsConfig a better name?

SGTM

[rexagod]

• [ ] TODO: https://github.com/kubernetes/enhancements/pull/4811#discussion_r1834046385

[rexagod]

@mrueg Regarding naming the managed resource CustomResourceMetricsConfig, I agree with the change, however, since a rename will entail renaming the repository and a good amount of refactor (mostly variable names and test data), I want to clarify some concerns before I start with that, these being:

• Can we truncate the prefix CustomResource to CR in accordance with the existing CRDMetricsResources? This will ensure a uniform crmetrics (case-insensitive) rather than crmetrics and customresourcemetrics (for instance, in the package name and variable names).
• Can we drop the suffix Config, as future additions may add fields other than Configuration to the spec, which may or may not influence the metrics generation directly?

To sum it up, would CRMetricsResource make sense? I'm fine with dropping the Resource suffix if there's a better alternative that accommodates for possible changes to the spec in the future.

[mrueg]

CustomResourceMetrics doesn't work because of the plural verb? A few other suggestions: CustomResourceMonitor (similar to ServiceMonitor / PodMonitor from Prometheus-Operator) CustomResourceState (dropping metrics here, because hypothetically in future you would wan to send events on changes)

How many CRDs do we need? I assumed this one is the only one we need to create the configuration for the controller? Is there another one you've been thinking of for CRDs? Can we combine them?

[rexagod]

ACK, I believe there's only one planned for now. Building on the suggestions above, you think CRMetricsMonitor would be a good idea?

cc @sftim

[mrueg]

ACK, I believe there's only one planned for now. Building on the suggestions above, you think CRMetricsMonitor would be a good idea?

cc @sftim

assuming we might extend this to core resources as well. how about ResourceMetricsMonitor ?

[rexagod]

Ah, nice catch! That makes perfect sense.

For the repository name though, I'm thinking something along the lines of resource-metrics-operator?

[mrueg]

Ah, nice catch! That makes perfect sense.

For the repository name though, I'm thinking something along the lines of resource-metrics-operator?

Sounds good or maybe resource-state-metrics (to match kube-state-metrics)?

[rexagod]

+100! I'll make the changes. Thanks a bunch for all the sound suggestions here! 🚀

[sftim]

The repository name? I'd use -controller; it's a controller not an operator.

[rexagod]

Hmm, I believe that can be left out in case of controllers? For instance, this or this?

Correct me if I'm wrong here, but the only controller-suffixed repository I found under the organization was sample-controller, which I believe makes sense to accommodate for the lack of brevity (sample)?

[rexagod]

I've tried to move things accordingly after archiving https://github.com/rexagod/crdmetrics/ in favor of https://github.com/rexagod/resource-state-metrics, but PLMK if I missed something. I believe with the the latest patch, all reviews till now are addressed.

[mrueg]

/approve

This looks good for the first alpha release. Thanks for your work on this @rexagod :rocket:

[rexagod]

/cc @richabanker

[richabanker]

/lgtm looking good for alpha, thanks for pushing this through!

[rexagod]

/cc @wojtek-t

[rexagod]

Thank you for the thorough review, @wojtek-t, the out-of-tree nature of this KEP had me a bit confused. I believe this is good for another round of reviews now 🙇🏼

[rexagod]

cc @wojtek-t for another review here please.

[rexagod]

Friendly ping, @wojtek-t for another look here please.

[rexagod]

Bump.

cc @kubernetes/production-readiness if folks have spare cycles to help get this reviewed and merged.

[wojtek-t]

Friendly ping, @wojtek-t for another look here please.

Sorry for huge delay - this looks good enough for me, especially for the external component.

/approve PRR

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: mrueg, rexagod, wojtek-t

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:

• ~~keps/prod-readiness/OWNERS~~ [wojtek-t]
• ~~keps/sig-instrumentation/OWNERS~~ [rexagod]

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

[dgrisonnet]

/lgtm