Skip to content

Remove binding from the smon status, if the ref workload is deleted#7795

Merged
simonpasquier merged 6 commits intoprometheus-operator:mainfrom
yp969803:issue7794
Sep 5, 2025
Merged

Remove binding from the smon status, if the ref workload is deleted#7795
simonpasquier merged 6 commits intoprometheus-operator:mainfrom
yp969803:issue7794

Conversation

@yp969803
Copy link
Contributor

@yp969803 yp969803 commented Aug 8, 2025

Description

Remove binding from the smon status, if the ref workload is deleted

Closes: #7794

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 August 8, 2025 10:10
@yp969803
Copy link
Contributor Author

yp969803 commented Aug 8, 2025

@simonpasquier review request

@simonpasquier
Copy link
Contributor

For this one, I'd expect that we leverage the FinalizerSyncer. It should be its responsibility to remove the bindings (maybe with help from the controller?) and then remove the finalizer only after everything's cleaned up.

@yp969803
Copy link
Contributor Author

yp969803 commented Aug 13, 2025

RemoveBinding requires many helper functions and structs from controller, also finalizerSyncer responsiblity is to add/remove finalizer, adding/removing status is done by controller sync function

@yp969803
Copy link
Contributor Author

@simonpasquier PTAL!


finalizersChanged, err := c.finalizerSyncer.Sync(ctx, p, logger, c.rr.DeletionInProgress(p))
assetStore := assets.NewStoreBuilder(c.kclient.CoreV1(), c.kclient.CoreV1())
resources, err := c.getSelectedConfigResources(ctx, logger, p, assetStore)
Copy link
Contributor

Choose a reason for hiding this comment

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

wouldn't it be better to iterate over all config resources and remove bindings which refer to the deleted prometheus? Otherwise we're at risk of missing some config resources.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok

}

// configResStatusCleanup removes prometheus bindings from the selected configuration resources (ServiceMonitor, PodMonitor, ScrapeConfig and PodMonitor).
// It returns true if another reconciliation is needed to complete the cleanup and and an error if the cleanup fails.
Copy link
Contributor

Choose a reason for hiding this comment

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

like discussed directly I don't think that we need to throttle the updates for now.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok

@yp969803
Copy link
Contributor Author

yp969803 commented Sep 5, 2025

@simonpasquier PTAL done the 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 3d04de8 into prometheus-operator:main Sep 5, 2025
19 of 21 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Remove smon status bindings if the refrenced workload is deleted

2 participants