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

NAS-130271 / 24.10 / Check SnapshotCountAlert on SMB shares only #14161

Merged
merged 9 commits into from
Aug 9, 2024

Conversation

creatorcary
Copy link
Contributor

@creatorcary creatorcary commented Aug 7, 2024

This alert should only be checked for relevant datasets: those that are shared over SMB. The limit of 512 snapshots only applies due to Windows File Explorer limiting the number of snapshots shown in the Previous Versions tab.

I have also changed the alert description to more precisely describe the problem.

Relevant documentation

API tests

nas130271-alerts

@creatorcary creatorcary requested a review from anodos325 August 7, 2024 16:00
@bugclerk
Copy link
Contributor

bugclerk commented Aug 7, 2024

@bugclerk bugclerk changed the title Only check SnapshotCountAlert on SMB shares NAS-130271 / 24.10 / Only check SnapshotCountAlert on SMB shares Aug 7, 2024
@anodos325
Copy link
Contributor

Do we still have some sort of alert when users are doing absurd things with snapshots?

@creatorcary
Copy link
Contributor Author

Do we still have some sort of alert when users are doing absurd things with snapshots?

Yeah the alert that triggers when there's over 10,000 total snapshots is still there.

@creatorcary creatorcary changed the title NAS-130271 / 24.10 / Only check SnapshotCountAlert on SMB shares NAS-130271 / 24.10 / Check SnapshotCountAlert on SMB shares only Aug 7, 2024
@creatorcary creatorcary added the WIP label Aug 7, 2024
@creatorcary creatorcary force-pushed the NAS-130271 branch 2 times, most recently from 30d1a73 to 2892e16 Compare August 7, 2024 20:16
@creatorcary creatorcary removed the WIP label Aug 7, 2024
@creatorcary creatorcary requested a review from a team August 7, 2024 20:34
@creatorcary creatorcary requested a review from yocalebo August 8, 2024 14:46
Copy link
Contributor

@yocalebo yocalebo left a comment

Choose a reason for hiding this comment

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

Excellent! Please wait for @anodos325 approval before merge.

Copy link
Contributor

@anodos325 anodos325 left a comment

Choose a reason for hiding this comment

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

Check for path location to ensure it's local to NAS before checking its snapshot count.

@creatorcary creatorcary requested a review from anodos325 August 9, 2024 17:55
@creatorcary creatorcary merged commit fa1cd29 into master Aug 9, 2024
3 checks passed
@creatorcary creatorcary deleted the NAS-130271 branch August 9, 2024 18:52
@bugclerk
Copy link
Contributor

bugclerk commented Aug 9, 2024

This PR has been merged and conversations have been locked.
If you would like to discuss more about this issue please use our forums or raise a Jira ticket.

@truenas truenas locked as resolved and limited conversation to collaborators Aug 9, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants