PrometheusRule status update after reconcillation from prometheus by yp969803 · Pull Request #8005 · prometheus-operator/prometheus-operator

yp969803 · 2025-10-09T06:10:51Z

Description

PrometheusRule status update after reconcillation from prometheus

Closes: #7923

Type of change

What type of changes does your code introduce to the Prometheus operator? Put an x in the box that apply.

CHANGE (fix or feature that would cause existing functionality to not work as expected)
FEATURE (non-breaking change which adds functionality)
BUGFIX (non-breaking change which fixes an issue)
ENHANCEMENT (non-breaking change which improves existing functionality)
NONE (if none of the other choices apply. Example, tooling, build system, CI, docs, etc.)

Verification

Please check the Prometheus-Operator testing guidelines for recommendations about automated tests.

Changelog entry

Please put a one-line changelog entry below. This will be copied to the changelog file during the release process.

…handling

…tedPrometheusRules parameters

…ator

…configuration

yp969803 · 2025-10-09T08:03:14Z

@simonpasquier PTAL!

simonpasquier · 2025-10-09T08:55:26Z

pkg/prometheus/server/operator.go

 type ControllerOption func(*Operator)

-// selectedConfigResources return the configuration resources (serviceMonitors, podMonitors, probes and scrapeConfig)
+// selectedConfigResources return the configuration resources (serviceMonitors, podMonitors, probes, rules and scrapeConfigs)


Suggested change

// selectedConfigResources return the configuration resources (serviceMonitors, podMonitors, probes, rules and scrapeConfigs)

// selectedConfigResources return the configuration resources (serviceMonitors, podMonitors, probes, prometheusRules and scrapeConfigs)

simonpasquier · 2025-10-09T09:01:07Z

pkg/operator/rules.go

+// Select selects PrometheusRules by Prometheus or ThanosRuler.
 // The second returned value is the number of rejected PrometheusRule objects.
-func (prs *PrometheusRuleSelector) Select(namespaces []string) (map[string]string, int, error) {
+func (prs *PrometheusRuleSelector) Select(namespaces []string) (TypedResourcesSelection[*monitoringv1.PrometheusRule], int, error) {


we don't need to return the rejected number anymore.

simonpasquier · 2025-10-09T09:02:05Z

pkg/operator/rules.go

+		rules[ruleName] = TypedConfigurationResource[*monitoringv1.PrometheusRule]{
+			resource:   promRule,
+			err:        err,
+			reason:     InvalidConfiguration,


hmm? it doesn't look right, reason should be empty if no error.

simonpasquier · 2025-10-09T09:02:58Z

pkg/prometheus/server/rules.go

-func (c *Operator) createOrUpdateRuleConfigMaps(ctx context.Context, p *monitoringv1.Prometheus) ([]string, error) {
-	logger := c.logger.With("prometheus", p.Name, "namespace", p.Namespace)
-
+func (c *Operator) getSelectedPrometheusRules(p *monitoringv1.Prometheus, logger *slog.Logger) (operator.TypedResourcesSelection[*monitoringv1.PrometheusRule], error) {


Suggested change

func (c *Operator) getSelectedPrometheusRules(p *monitoringv1.Prometheus, logger *slog.Logger) (operator.TypedResourcesSelection[*monitoringv1.PrometheusRule], error) {

func (c *Operator) selectPrometheusRules(p *monitoringv1.Prometheus, logger *slog.Logger) (operator.TypedResourcesSelection[*monitoringv1.PrometheusRule], error) {

simonpasquier · 2025-10-09T09:09:17Z

pkg/operator/config_resource.go

 	err        error  // Error encountered during selection or validation (nil if valid).
 	reason     string // Reason for rejection; empty if accepted.
 	generation int64  // Generation of the desired state (spec).
+	content    string // Marshalled content of the resource (only for PrometheusRules).


rather than extending TypedConfigurationResource with a field which is only useful for PrometheusRule what if we create a specific struct for PrometheusRule wrapping a TypedResourcesSelection + a map[string]string?

…s and update related logic

…unt and rename methods

…election logic

yp969803 · 2025-10-09T10:56:15Z

@simonpasquier PTAL

simonpasquier · 2025-10-09T14:20:46Z

pkg/operator/config_resource.go

 // TypedResourcesSelection represents a map of configuration resources selected by Prometheus or PrometheusAgent.
 type TypedResourcesSelection[T ConfigurationResource] map[string]TypedConfigurationResource[T]

+type SelectedPrometheusRules struct {


(nit)

Suggested change

type SelectedPrometheusRules struct {

type PrometheusRuleSelection struct {

simonpasquier · 2025-10-09T14:21:08Z

pkg/operator/config_resource.go

 type TypedResourcesSelection[T ConfigurationResource] map[string]TypedConfigurationResource[T]

+type SelectedPrometheusRules struct {
+	Rules        TypedResourcesSelection[*monitoringv1.PrometheusRule] // PrometheusRules selected.


Let's avoid public fields.

Suggested change

Rules TypedResourcesSelection[*monitoringv1.PrometheusRule] // PrometheusRules selected.

selection TypedResourcesSelection[*monitoringv1.PrometheusRule] // PrometheusRules selected.

simonpasquier · 2025-10-09T14:21:24Z

pkg/operator/config_resource.go


+type SelectedPrometheusRules struct {
+	Rules        TypedResourcesSelection[*monitoringv1.PrometheusRule] // PrometheusRules selected.
+	MarshalRules map[string]string                                     // Map of valid marshalled rules.


Suggested change

MarshalRules map[string]string // Map of valid marshalled rules.

// Map of rule configuration files serialized to the Prometheus format (key=filename).

ruleFiles map[string]string

simonpasquier · 2025-10-09T14:27:38Z

pkg/thanos/rules.go

 	if tKey, ok := o.accessor.MetaNamespaceKey(t); ok {
-		o.metrics.SetSelectedResources(tKey, monitoringv1.PrometheusRuleKind, len(rules))
-		o.metrics.SetRejectedResources(tKey, monitoringv1.PrometheusRuleKind, rejected)
+		o.metrics.SetSelectedResources(tKey, monitoringv1.PrometheusRuleKind, len(rules.MarshalRules))


For other resources, we report the number of selected resources, irrespective of their validation state?
I think that we should the same here.

simonpasquier · 2025-10-09T14:31:50Z

pkg/prometheus/server/rules.go

 	if pKey, ok := c.accessor.MetaNamespaceKey(p); ok {
-		c.metrics.SetSelectedResources(pKey, monitoringv1.PrometheusRuleKind, len(rules))
-		c.metrics.SetRejectedResources(pKey, monitoringv1.PrometheusRuleKind, rejected)
+		c.metrics.SetSelectedResources(pKey, monitoringv1.PrometheusRuleKind, len(rules.MarshalRules))


same remark as below for ThanosRuler

simonpasquier · 2025-10-09T14:33:09Z

pkg/prometheus/server/operator.go

+
+	rules, err := c.selectPrometheusRules(p, logger)
+	if err != nil {
+		return nil, err


(nit) to be consistent

Suggested change

return nil, err

return nil, fmt.Errorf("selecting PrometheusRule failed: %w", err)

simonpasquier · 2025-10-09T14:34:46Z

pkg/prometheus/server/rules.go

+	return rules, nil
+}
+
+func (c *Operator) createOrUpdateRuleConfigMaps(ctx context.Context, p *monitoringv1.Prometheus, rules map[string]string, logger *slog.Logger) ([]string, error) {


(nit) it can receive a operator.SelectedPrometheusRules argument instead of an anonymous map[string]string

…n and update related logic

…eSelection type

yp969803 · 2025-10-09T20:19:45Z

@simonpasquier merge request

simonpasquier · 2025-10-10T08:06:10Z

pkg/operator/rules.go

-// Select selects PrometheusRules and translates them into native
-// Prometheus/Thanos configurations.
+// Select selects PrometheusRules by Prometheus or ThanosRuler.
 // The second returned value is the number of rejected PrometheusRule objects.


Suggested change

// The second returned value is the number of rejected PrometheusRule objects.

simonpasquier · 2025-10-10T08:09:42Z

pkg/operator/config_resource.go

 	return validRes
 }

+type PrometheusRuleSelection struct {


(nit) can we move this new code to rules.go instead?

simonpasquier · 2025-10-10T08:11:53Z

pkg/operator/config_resource.go


+type PrometheusRuleSelection struct {
+	selection TypedResourcesSelection[*monitoringv1.PrometheusRule] // PrometheusRules selected.
+	ruleFiles map[string]string                                     // Map of rule configuration files serialized to the Prometheus format (key=filename).


(suggestion) add a field to store the number of rejected resources so callers don't have to compute it?
the number of selected resources is len(prs.Selected()).

Suggested change

ruleFiles map[string]string // Map of rule configuration files serialized to the Prometheus format (key=filename).

rejected int

added a function to get Rejected numbers

pkg/prometheus/server/rules.go

simonpasquier · 2025-10-10T08:12:29Z

pkg/prometheus/server/rules.go

 	if pKey, ok := c.accessor.MetaNamespaceKey(p); ok {
-		c.metrics.SetSelectedResources(pKey, monitoringv1.PrometheusRuleKind, len(rules))
-		c.metrics.SetRejectedResources(pKey, monitoringv1.PrometheusRuleKind, rejected)
+		c.metrics.SetSelectedResources(pKey, monitoringv1.PrometheusRuleKind, len(rules.RuleFiles()))


Suggested change

c.metrics.SetSelectedResources(pKey, monitoringv1.PrometheusRuleKind, len(rules.RuleFiles()))

c.metrics.SetSelectedResources(pKey, monitoringv1.PrometheusRuleKind, len(rules.Selected()))

…les.go, update selection logic

simonpasquier

LGTM!

yp969803 · 2025-10-10T09:51:55Z

Error Trace:	/home/runner/work/prometheus-operator/prometheus-operator/test/e2e/status_subresource_test.go:222
        	Error:      	Received unexpected error:
        	            	serviceMonitor status gatedfeatures-servicemonitorstatuswithmultipleworkload-acf63506/servicemonitor-status-multiple-workloads failed to reach expected condition: context deadline exceeded: resource "prometheuses" gatedfeatures-servicemonitorstatuswithmultipleworkload-acf63506/server1: client rate limiter Wait returned an error: context deadline exceeded

@simonpasquier this test is failing i dont think this is related to my changes, can u rerun the test, as it failed last to last time also, but last time it passed

feat: add support for PrometheusRule in configuration resources

6fa90c2

yp969803 requested a review from a team as a code owner October 9, 2025 06:10

pull-request-size bot added the size/XS label Oct 9, 2025

refactor: centralize invalid configuration reason in operator package

3f69bb9

pull-request-size bot added size/S and removed size/XS labels Oct 9, 2025

refactor: improve PrometheusRule selection and logging in operator

577c08f

pull-request-size bot added size/M and removed size/S labels Oct 9, 2025

yp969803 added 5 commits October 9, 2025 12:53

feat: add ValidMarshalledResources method and update rule config map …

3d3245f

…handling

refactor: clarify InvalidConfiguration constant and simplify getSelec…

975ee9e

…tedPrometheusRules parameters

feat: add Bindings method to PrometheusRule and update status in oper…

21e4f0e

…ator

feat: add prometheusrules/status resource to cluster role and bundle …

fcef575

…configuration

feat: add prometheusrules/status subresource support and related tests

85c83ca

pull-request-size bot added size/L and removed size/M labels Oct 9, 2025

simonpasquier reviewed Oct 9, 2025

View reviewed changes

yp969803 added 3 commits October 9, 2025 15:49

refactor: rename getSelectedPrometheusRules to selectedPrometheusRule…

c3b5d9d

…s and update related logic

refactor: update PrometheusRule selection logic to remove rejected co…

82f5390

…unt and rename methods

refactor: introduce SelectedPrometheusRules type and update related s…

e3736e7

…election logic

fix: ensure rules are marshaled correctly in PrometheusRuleSelector

3523483

simonpasquier reviewed Oct 9, 2025

View reviewed changes

yp969803 added 2 commits October 10, 2025 00:20

refactor: replace SelectedPrometheusRules with PrometheusRuleSelectio…

3ab06ac

…n and update related logic

refactor: update createOrUpdateRuleConfigMaps to accept PrometheusRul…

512c73f

…eSelection type

simonpasquier reviewed Oct 10, 2025

View reviewed changes

refactor: move PrometheusRuleSelection type and related methods to ru…

a57d69d

…les.go, update selection logic

simonpasquier approved these changes Oct 10, 2025

View reviewed changes

simonpasquier enabled auto-merge (squash) October 10, 2025 09:41

simonpasquier merged commit 4ca908c into prometheus-operator:main Oct 10, 2025
20 of 22 checks passed

heliapb mentioned this pull request Oct 10, 2025

Remove binding from PrometheusRule's status subresource when Prometheus/ThanosRuler gets deleted #7924

Closed

	// selectedConfigResources return the configuration resources (serviceMonitors, podMonitors, probes, rules and scrapeConfigs)
	// selectedConfigResources return the configuration resources (serviceMonitors, podMonitors, probes, prometheusRules and scrapeConfigs)

	func (c Operator) getSelectedPrometheusRules(p monitoringv1.Prometheus, logger slog.Logger) (operator.TypedResourcesSelection[monitoringv1.PrometheusRule], error) {
	func (c Operator) selectPrometheusRules(p monitoringv1.Prometheus, logger slog.Logger) (operator.TypedResourcesSelection[monitoringv1.PrometheusRule], error) {

	type SelectedPrometheusRules struct {
	type PrometheusRuleSelection struct {

	Rules TypedResourcesSelection[*monitoringv1.PrometheusRule] // PrometheusRules selected.
	selection TypedResourcesSelection[*monitoringv1.PrometheusRule] // PrometheusRules selected.

	MarshalRules map[string]string // Map of valid marshalled rules.
	// Map of rule configuration files serialized to the Prometheus format (key=filename).
	ruleFiles map[string]string

	return nil, err
	return nil, fmt.Errorf("selecting PrometheusRule failed: %w", err)

	ruleFiles map[string]string // Map of rule configuration files serialized to the Prometheus format (key=filename).
	rejected int

	c.metrics.SetSelectedResources(pKey, monitoringv1.PrometheusRuleKind, len(rules.RuleFiles()))
	c.metrics.SetSelectedResources(pKey, monitoringv1.PrometheusRuleKind, len(rules.Selected()))

Conversation

yp969803 commented Oct 9, 2025

Description

Type of change

Verification

Changelog entry

Uh oh!

yp969803 commented Oct 9, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

yp969803 commented Oct 9, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

yp969803 commented Oct 9, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

simonpasquier left a comment

Choose a reason for hiding this comment