Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rename ServiceMonitors so they don't get deleted by operator #129

Merged
merged 1 commit into from
Nov 17, 2023

Conversation

simu
Copy link
Member

@simu simu commented Nov 16, 2023

We create our own ServiceMonitor resources, since the argocd-operator doesn't support creating ServiceMonitors without also deploying a Prometheus instance.

However, the operator will delete any ServiceMonitor which matches the name that it would generate for its managed ServiceMonitors if the Prometheus component is disabled, cf. https://github.com/argoproj-labs/argocd-operator/blob/17064c9b310785ab145747a367f7deb5507a572e/controllers/argocd/prometheus.go#L129-L136

This causes the component-managed ServiceMonitors to get deleted from time to time, and they get recreated after a while by an ArgoCD resync.

This commit circumvents the issue by renaming the component-managed ServiceMonitors so they don't get deleted by the operator.

Checklist

  • The PR has a meaningful title. It will be used to auto generate the
    changelog.
    The PR has a meaningful description that sums up the change. It will be
    linked in the changelog.
  • PR contains a single logical change (to build a better changelog).
  • Categorize the PR by adding one of the labels:
    bug, enhancement, documentation, change, breaking, dependency
    as they show up in the changelog.

@simu simu added the bug Something isn't working label Nov 16, 2023
@simu simu requested a review from a team November 16, 2023 16:25
@simu simu force-pushed the fix/service-monitors branch 2 times, most recently from 7038de9 to 00d3d15 Compare November 16, 2023 16:31
We create our own ServiceMonitor resources, since the argocd-operator
doesn't support creating ServiceMonitors without also deploying a
Prometheus instance.

However, the operator will delete any ServiceMonitor which matches the
name that it would generate for its managed ServiceMonitors if the
Prometheus component is disabled, cf.
https://github.com/argoproj-labs/argocd-operator/blob/17064c9b310785ab145747a367f7deb5507a572e/controllers/argocd/prometheus.go#L129-L136

This causes the component-managed ServiceMonitors to get deleted from
time to time, and they get recreated after a while by an ArgoCD resync.

This commit circumvents the issue by renaming the component-managed
ServiceMonitors so they don't get deleted by the operator.
@simu simu merged commit b28b5fe into master Nov 17, 2023
10 checks passed
@simu simu deleted the fix/service-monitors branch November 17, 2023 08:08
simu added a commit that referenced this pull request Nov 22, 2023
We renamed the ServiceMonitors in #129 without verifying that the
renamed service monitors still discover any endpoints.

We need to split the ServiceMonitor name from the name used for the
label selector to ensure that we can use an object name which doesn't
collide with the operator-managed name, while still picking up the
endpoints.
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants