-
Notifications
You must be signed in to change notification settings - Fork 11.8k
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
Alerting: Prevent cleanup of non-empty folders on migration revert #76439
Conversation
e56e6e7
to
dc6b396
Compare
9cc2691
to
2e86691
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. I agree that it's ok to reuse the folder with the same title because it was created during the the previous migration
var descendantCounts []string | ||
var cntErr error | ||
for kind, cnt := range count { | ||
if cnt > 0 { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe revert?
errs = errors.Join(errs, fmt.Errorf("folder %s: %w", folderUID, err)) | ||
continue | ||
} | ||
var descendantCounts []string |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
could be pre-allocated
2e86691
to
56b4ba6
Compare
…76439) Prevent cleanup of non-empty folders on revert
What is this feature?
Currently, when the migration reverts, it cleans up folders created during the previous migration. If, between when the migration was run and revert, other resources were added to the automatically created folders, the revert will delete these descendants as well.
For example, if a dashboard is added to the folder, the dashboard will also be deleted.
This PR will verify that the folder is empty before deleting it. If it is not empty, a warning will be logged and the delete will be skipped.
Who is this feature for?
Users migrating from Legacy Alerting to Grafana Alerting.
Special notes for your reviewer:
When we attempt the create a folder during the migration, we can no longer fail if the folder already exists. As this possibility will become much more common now. Instead, we will log an informational message and use the existing folder.
This is not a perfect solution, as these existing folders could have different permissions that what would have otherwise been created by the migration. However, the risk of conflict is low because of the strict naming scheme for created folders.
Depends on: #74504
Please check that: