fix monitor recreate#3050
Conversation
This comment has been minimized.
This comment has been minimized.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #3050 +/- ##
==========================================
+ Coverage 42.43% 42.54% +0.10%
==========================================
Files 337 337
Lines 29020 29029 +9
==========================================
+ Hits 12315 12350 +35
+ Misses 15895 15869 -26
Partials 810 810
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 1 file with indirect coverage changes Continue to review full report in Codecov by Sentry.
🚀 New features to boost your workflow:
|
levan-m
left a comment
levan-m
left a comment
There was a problem hiding this comment.
looks good, two minor comments
| // instead of looping on 404. | ||
| logger.V(1).Info("DatadogMonitor manifest has changed") | ||
| shouldUpdate = true | ||
| _, err = r.get(auth, instance, newStatus) |
There was a problem hiding this comment.
nit - this and next if block has become identical and could be merged.
There was a problem hiding this comment.
I can only run on private repositories.
| assert.NoError(t, err) | ||
| rec.client.Get(context.TODO(), types.NamespacedName{Name: resourcesName, Namespace: resourcesNamespace}, dm) | ||
|
|
||
| assert.Equal(t, 0, dm.Status.ID, "status.ID should be cleared after update returns 404 so next reconcile recreates") |
There was a problem hiding this comment.
For completness sake, do we need to assert on next reconcile outcome?
There was a problem hiding this comment.
I can only run on private repositories.
3 participants
What does this PR do?
Fixes the DatadogMonitor controller to recreate a monitor when it has been deleted out-of-band (e.g. from the Datadog UI). The "manifest changed" path now does a GET before the PUT and falls back to create on 404, and the update path catches 404 by clearing status.ID so the next reconcile recreates instead of looping on "Monitor not found".
Motivation
CONS-8333: customer (Grainger) reported the controller getting stuck logging 404s forever when a monitor was deleted in the UI and the CR spec was then edited before the next sync.
Additional Notes
Anything else we should know when reviewing?
Minimum Agent Versions
Are there minimum versions of the Datadog Agent and/or Cluster Agent required?
Describe your test plan
Write there any instructions and details you may have to test your PR.
Checklist
bug,enhancement,refactoring,documentation,tooling, and/ordependenciesqa/skip-qalabel