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

fix(GrafanaFolder): proper status updates for removed CRs, don't fail on missing folder in onFolderDeleted events #1516

Merged
merged 2 commits into from May 13, 2024

Conversation

weisdd
Copy link
Collaborator

@weisdd weisdd commented May 5, 2024

Two fixes for grafanafolder controller:

  • Previously, onFolderDeleted would treat a missing folder (e.g. deleted through Grafana UI) as an error, which should have never been the case. - We just overlooked a missing ! in error handling logic;
  • When cleaning up status for CRs that had been deleted, syncFolders was supposed to check if a folder still exists in Grafana, but instead of using method for folders, it used one for dashboards.

Two manual test cases to verify the fix:

  • onFolderDeleted:
    • Steps:
      • deploy GrafanaFolder CR (e.g. from examples/folder);
      • Grafana CR should contain a reference to the folder in status.folders;
      • remove the folder from UI;
      • remove the folder CR;
    • Expected results:
      • No errors;
      • No event requeues;
      • Grafana CR should no longer contain a reference to the folder in status.folders;
  • syncFolders:
    • Steps:
      • deploy GrafanaFolder CR (e.g. from examples/folder);
      • Grafana CR should contain a reference to the folder in status.folders;
      • scale down the operator (kubectl scale deployment grafana-operator-controller-manager-v5 --replicas 0);
      • remove the GrafanaFolder CR;
      • scale up the operator (kubectl scale deployment grafana-operator-controller-manager-v5 --replicas 1);
    • Expected results:
      • No errors;
      • Log message "folder no longer exists";
      • Grafana CR should no longer contain a reference to the folder in status.folders.

Fixes: #1512

Copy link
Member

@theSuess theSuess left a comment

Choose a reason for hiding this comment

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

Thanks for catching this!

@theSuess theSuess enabled auto-merge May 13, 2024 09:04
@theSuess theSuess merged commit 3e81660 into master May 13, 2024
10 checks passed
@theSuess theSuess deleted the fix/folder-sync branch May 13, 2024 09:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug] Deleting a Resource (Folder) operator error loop
2 participants