Skip to content

Cleanup of bindings for PrometheusRules in prometheus#8024

Merged
simonpasquier merged 4 commits intoprometheus-operator:mainfrom
yp969803:issue7924
Oct 17, 2025
Merged

Cleanup of bindings for PrometheusRules in prometheus#8024
simonpasquier merged 4 commits intoprometheus-operator:mainfrom
yp969803:issue7924

Conversation

@yp969803
Copy link
Contributor

@yp969803 yp969803 commented Oct 13, 2025

Description

Cleanup of bindings for PrometheusRules in prometheus

Closes: #7924

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.

@yp969803 yp969803 requested a review from a team as a code owner October 13, 2025 12:57
@yp969803
Copy link
Contributor Author

yp969803 commented Oct 14, 2025 •
edited
Loading

@simonpasquier review and merge!
Also i don't know why the test

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

is failing i dont think it belong to my code changes

@simonpasquier
Copy link
Contributor

simonpasquier commented Oct 14, 2025

see #8029 and #8028 for the failing test.

simonpasquier
simonpasquier reviewed Oct 14, 2025
View reviewed changes
Copy link
Contributor

@simonpasquier simonpasquier left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure to understand why we need a custom CleanupPrometheusRuleBindings() for rules?

@yp969803
Copy link
Contributor Author

yp969803 commented Oct 14, 2025

@simonpasquier we are using different value of key for promRule k := fmt.Sprintf("%v-%v-%v.yaml", promRule.Namespace, promRule.Name, promRule.UID)

@simonpasquier
Copy link
Contributor

simonpasquier commented Oct 15, 2025

Thanks for the clarification. I'd prefer to avoid a special case for PrometheusRule.
Can we change the current logic to index PrometheusRule resources by namespace/name in the selection map and move the name expansion to the rule syncer (ideally in a separate PR)? In theory the "Selector" shouldn't are about the rule filenames (though I understand why it does now).

@yp969803
Copy link
Contributor Author

yp969803 commented Oct 16, 2025

@simonpasquier PTAL

simonpasquier
simonpasquier approved these changes Oct 17, 2025
View reviewed changes
Copy link
Contributor

@simonpasquier simonpasquier left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@simonpasquier simonpasquier merged commit 0ecdf47 into prometheus-operator:main Oct 17, 2025
22 checks passed
tboerger pushed a commit to tboerger/prometheus-operator that referenced this pull request Dec 20, 2025
@yp969803 @tboerger
Cleanup of bindings for PrometheusRules in prometheus (prometheus-ope…
0b985b4 
…rator#8024)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@simonpasquier simonpasquier simonpasquier approved these changes

Assignees

No one assigned

Labels

size/L

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

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

2 participants

@yp969803 @simonpasquier