feat: add support for percentage-based retention - #8728

Can we add CEL rule for the number between 0 and 100?

Done in 14fab58 — added a CEL rule on spec.retentionPercentage:

// +kubebuilder:validation:XValidation:rule="quantity(string(self)).compareTo(quantity('0')) >= 0 && quantity(string(self)).compareTo(quantity('100')) <= 0",message="retentionPercentage must be between 0 and 100"

The field is a *resource.Quantity, so it renders as x-kubernetes-int-or-string. string(self) covers both accepted forms (retentionPercentage: 80 and retentionPercentage: "0.5"), and quantity() gives correct numeric comparison for suffixed values instead of a lexical one.

I checked the compiled rule against apiextensions-apiserver's CEL validator (v0.36.3) rather than eyeballing it — accepted: 0, 80, 100, "0.5", "100m"; rejected: 101, -1, "100.1", "-0.1", "1Ki".

Two things worth your call:

• The quantity CEL library was introduced in Kubernetes 1.28 (k8s.io/apiserver/pkg/cel/environment/base.go), so an apiserver older than that rejects the CRD at compile time. CI runs 1.36 (.github/env), and this is the first quantity() use in the repo — if you would rather keep the CRDs installable on older apiservers I can switch to a library-free formulation.
• I kept the Go-side validateRetentionPercentage() as well, so the operator still returns a clear error where the rule cannot be evaluated.

Regenerated artifacts ride in the same commit (bundle.yaml, both CRD YAMLs, jsonnet); make --always-make format generate is a no-op on top of it, and make check-golang / make check-api are clean.

our documentation currently states that we support Kubernetes >= v1.25.0. I'm not opposed bumping the version requirement but it would need further discussion (1.25 is EOL since 2023-10-28).

https://prometheus-operator.dev/docs/getting-started/compatibility/#kubernetes

Fair point, and it is the same concern I flagged when I added the rule — quantity() landed in Kubernetes 1.28, so on 1.25–1.27 the apiserver rejects the CRD at compile time rather than just ignoring the rule.

For context on how it got here: the rule was added at @dongjiang1989's request in this thread, so the version floor is really a call between the two of you rather than something I should decide on my own.

Two ways out that do not need the floor bumped:

• Drop the XValidation marker. validateRetentionPercentage() in pkg/prometheus/promcfg.go already rejects anything outside 0..100 and is covered by TestRetentionConfigFile (the 101 and -1 cases). The only thing lost is admission-time rejection — a bad value is caught at reconcile instead, with a clear error.
• Keep the bound, express it without the quantity library. The field is a *resource.Quantity so it renders as x-kubernetes-int-or-string, and a library-free rule has to handle both the int and the string form. I can write and test that, but it will be noticeably uglier than the current one-liner.

Tell me which you prefer and I will push it. If you would rather take the 1.25 floor as a separate discussion, I am happy to just drop the marker for now so this PR is not blocked on it.

I don't understand the second option you're suggesting (Keep the bound, express it without the quantity library.). I'd rather remove the CEL rule for now and we can discuss later if/when we want to change the minimum supported Kubernetes version.

Can we add CEL rule for this?

Sorry for the delay — your earlier comment on the same field slipped past me. Answered in the thread above: 14fab58 adds a quantity()-based CEL rule bounding retentionPercentage to 0..100, with the regenerated CRDs in the same commit.

our documentation currently states that we support Kubernetes >= v1.25.0. I'm not opposed bumping the version requirement but it would need further discussion (1.25 is EOL since 2023-10-28).

https://prometheus-operator.dev/docs/getting-started/compatibility/#kubernetes

the user doesn't need the implementation details.

we should use cg.WithMinimumVersion("3.11.0").AppendMapItem(...) which would log a warning if the parameter isn't supported.

That line sits inside the if cg.WithMinimumVersion("3.11.0").IsCompatible() branch, so AppendMapItem's notCompatible path cannot be reached from there — it would fall through to the plain append and never warn.

The warning you are describing is already emitted, from the else branch a few lines down:

} else if retentionPercentage != nil {
	// Percentage-based retention has no command-line equivalent, hence it
	// can't be supported by older Prometheus versions.
	cg.WithMinimumVersion("3.11.0").Warn("retentionPercentage")
}

TestRetentionConfigFile/retention.percentage is not in the configuration file for Prometheus < v3.11.0 covers it, and the line it logs is:

level=WARN msg="ignoring \"retentionPercentage\" not supported by Prometheus" version=v3.10.0 minimum_version=3.11.0

The whole storage.tsdb.retention section is gated as a unit because the config-file form only exists from 3.11.0 — time and size still go out as command-line flags below that — so the version check cannot move down to the individual key.

I tried your version to be sure: swapping in cg.WithMinimumVersion("3.11.0").AppendMapItem(retentionSlice, "percentage", ...) leaves every golden file unchanged. Happy to make that change anyway if you prefer it for consistency, but it would be a no-op.

I would refactor the code to be like

var (
    retentionSlice yaml.MapSlice
    cgRetention = cg.WithMinimumVersion("3.11.0")
}
if cgRetention.IsCompatible() {
  // Starting with 3.11.0, time and size retention settings go to the configuration file instead of CLI arguments.
  ...
}
if retentionPercentage != nil {
    retentionSlice = cgRetention.AppendMapItem(retentionSlice, yaml.MapItem{Key: "percentage", Value: retentionPercentage.AsApproximateFloat64()})
}
if len(retentionSlice) > 0 {
    ...
}