-
Notifications
You must be signed in to change notification settings - Fork 54
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
doc: add manual test plan for delete backup asynchronously #2223
doc: add manual test plan for delete backup asynchronously #2223
Conversation
WalkthroughThe pull request introduces a new documentation file detailing test procedures for asynchronous backup deletion in a Longhorn cluster. The document comprehensively covers three test scenarios: a normal backup deletion process, an error case involving configuration file permissions, and a scenario simulating controller crashes during backup deletion. Changes
Possibly related PRs
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
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.
Actionable comments posted: 0
🧹 Nitpick comments (5)
docs/content/manual/release-specific/v1.8.0/test-delete-backup-asynchronously.md (5)
7-11
: Format URLs as proper markdown linksConvert bare URLs to proper markdown links for better readability and to follow markdown best practices.
-https://github.com/longhorn/longhorn/issues/8746 +[Issue #8746](https://github.com/longhorn/longhorn/issues/8746) -https://github.com/longhorn/longhorn/pull/9152 +[LEP PR #9152](https://github.com/longhorn/longhorn/pull/9152)🧰 Tools
🪛 Markdownlint (0.37.0)
7-7: null
Bare URL used(MD034, no-bare-urls)
11-11: null
Bare URL used(MD034, no-bare-urls)
27-31
: Improve grammar and clarity in test stepsAdd missing articles and improve sentence structure for better readability.
-Delete the first backup and create second backup from the snapshot at the same time. +Delete the first backup and create a second backup from the snapshot at the same time. -The second backup will be in `Pending` state with message including `waiting for backup to be deleted`. +The second backup will be in a `Pending` state with a message including `waiting for backup to be deleted`.🧰 Tools
🪛 LanguageTool
[uncategorized] ~27-~27: Possible missing article found.
Context: ...en** Delete the first backup and create second backup from the snapshot at the same ti...(AI_HYDRA_LEO_MISSING_A)
48-49
: Add safety checks to file manipulation commandsThe
chattr
command should include checks to ensure the file exists and the command succeeds.-$ chattr +i backups/backup_backup-*.cfg +# Ensure the backup config file exists and set it as immutable +backup_cfg=$(find backups -name "backup_backup-*.cfg") +if [ -n "$backup_cfg" ]; then + chattr +i "$backup_cfg" || echo "Failed to make backup config immutable" +else + echo "Backup config file not found" +fi🧰 Tools
🪛 LanguageTool
[style] ~48-~48: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ... volume. And Create a snapshot. And Connect to the backupstore pod and ma...(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
75-75
: Improve clarity of pod deletion stepThe current wording is verbose and can be made more concise.
-When the backup is in `Deleting` state, delete the longhorn-manager pod which is the owner of the backup. (by checking `Backup.Status.OwnerID`) +When the backup is in `Deleting` state, delete the longhorn-manager pod that owns the backup (identified by `Backup.Status.OwnerID`).🧰 Tools
🪛 LanguageTool
[style] ~75-~75: You can shorten this phrase to avoid wordiness.
Context: ..., delete the longhorn-manager pod which is the owner of the backup. (by checking `Backup.Status...(BE_THE_MAKER_OF_WORDINESS)
1-81
: Well-structured test plan with clear scenariosThe test plan effectively covers the key scenarios for asynchronous backup deletion. The use of Given/And/Then/Verify format makes the steps clear and easy to follow. Consider varying some of the "And" statements to improve readability while maintaining the BDD style.
Consider adding these sections to make the test plan more comprehensive:
- Prerequisites (e.g., required Longhorn version, cluster setup)
- Cleanup steps after each test scenario
- Estimated time for test execution
- Troubleshooting guidelines for common issues
🧰 Tools
🪛 LanguageTool
[style] ~21-~21: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...nd** Write large data to the volume. And Create the first backup from the volu...(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~23-~23: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...te the first backup from the volume. And Write small data to the volume. **An...(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~25-~25: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...nd** Write small data to the volume. And Create a snapshot. Then Delete t...(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[uncategorized] ~27-~27: Possible missing article found.
Context: ...en** Delete the first backup and create second backup from the snapshot at the same ti...(AI_HYDRA_LEO_MISSING_A)
[style] ~42-~42: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...nd** Write large data to the volume. And Create the first backup from the volu...(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~44-~44: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...te the first backup from the volume. And Write small data to the volume. **An...(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~46-~46: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...nd** Write small data to the volume. And Create a snapshot. And Connect t...(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~48-~48: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ... volume. And Create a snapshot. And Connect to the backupstore pod and ma...(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[uncategorized] ~50-~50: Possible missing article found.
Context: ...en** Delete the first backup and create second backup from the snapshot at the same ti...(AI_HYDRA_LEO_MISSING_A)
[style] ~67-~67: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...nd** Write large data to the volume. And Create the first backup from the volu...(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~69-~69: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...te the first backup from the volume. And Connect to the backupstore pod and ma...(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~75-~75: You can shorten this phrase to avoid wordiness.
Context: ..., delete the longhorn-manager pod which is the owner of the backup. (by checking `Backup.Status...(BE_THE_MAKER_OF_WORDINESS)
🪛 Markdownlint (0.37.0)
7-7: null
Bare URL used(MD034, no-bare-urls)
11-11: null
Bare URL used(MD034, no-bare-urls)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
docs/content/manual/release-specific/v1.8.0/test-delete-backup-asynchronously.md
(1 hunks)
🧰 Additional context used
🪛 LanguageTool
docs/content/manual/release-specific/v1.8.0/test-delete-backup-asynchronously.md
[style] ~21-~21: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...nd** Write large data to the volume. And Create the first backup from the volu...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~23-~23: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...te the first backup from the volume. And Write small data to the volume. **An...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~25-~25: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...nd** Write small data to the volume. And Create a snapshot. Then Delete t...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[uncategorized] ~27-~27: Possible missing article found.
Context: ...en** Delete the first backup and create second backup from the snapshot at the same ti...
(AI_HYDRA_LEO_MISSING_A)
[style] ~42-~42: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...nd** Write large data to the volume. And Create the first backup from the volu...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~44-~44: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...te the first backup from the volume. And Write small data to the volume. **An...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~46-~46: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...nd** Write small data to the volume. And Create a snapshot. And Connect t...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~48-~48: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ... volume. And Create a snapshot. And Connect to the backupstore pod and ma...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[uncategorized] ~50-~50: Possible missing article found.
Context: ...en** Delete the first backup and create second backup from the snapshot at the same ti...
(AI_HYDRA_LEO_MISSING_A)
[style] ~67-~67: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...nd** Write large data to the volume. And Create the first backup from the volu...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~69-~69: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...te the first backup from the volume. And Connect to the backupstore pod and ma...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~75-~75: You can shorten this phrase to avoid wordiness.
Context: ..., delete the longhorn-manager pod which is the owner of the backup. (by checking `Backup.Status...
(BE_THE_MAKER_OF_WORDINESS)
🪛 Markdownlint (0.37.0)
docs/content/manual/release-specific/v1.8.0/test-delete-backup-asynchronously.md
7-7: null
Bare URL used
(MD034, no-bare-urls)
11-11: null
Bare URL used
(MD034, no-bare-urls)
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
ref: longhorn/longhorn 8746 Signed-off-by: Jack Lin <[email protected]>
4884a06
to
f0f2d25
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.
Actionable comments posted: 0
🧹 Nitpick comments (6)
docs/content/manual/release-specific/v1.8.0/test-delete-backup-asynchronously.md (6)
7-11
: Format bare URLs as proper markdown linksConvert the bare URLs to proper markdown links for better readability and maintainability.
-- https://github.com/longhorn/longhorn/issues/8746 +- [Longhorn Issue #8746](https://github.com/longhorn/longhorn/issues/8746) -- https://github.com/longhorn/longhorn/pull/9152 +- [Longhorn LEP PR #9152](https://github.com/longhorn/longhorn/pull/9152)🧰 Tools
🪛 Markdownlint (0.37.0)
7-7: null
Bare URL used(MD034, no-bare-urls)
11-11: null
Bare URL used(MD034, no-bare-urls)
27-31
: Add missing articles for clarityAdd the article "a" before "second backup" for better readability.
-Then** Delete the first backup and create second backup from the snapshot at the same time. +Then** Delete the first backup and create a second backup from the snapshot at the same time. -Verify** After the first backup is deleted, second backup should be in progress and complete in the end. +Verify** After the first backup is deleted, the second backup should be in progress and complete in the end.🧰 Tools
🪛 LanguageTool
[uncategorized] ~27-~27: Possible missing article found.
Context: ...en** Delete the first backup and create second backup from the snapshot at the same ti...(AI_HYDRA_LEO_MISSING_A)
[uncategorized] ~31-~31: Possible missing article found.
Context: ...fy** After the first backup is deleted, second backup should be in progress and comple...(AI_HYDRA_LEO_MISSING_A)
48-48
: Format system command as code blockConsider formatting the command in a separate code block for better visibility and copy-paste functionality.
-And** Connect to the backupstore pod and make the backup.cfg file immutable by running the command `$ chattr +i backups/backup_backup-*.cfg`. +And** Connect to the backupstore pod and make the backup.cfg file immutable by running the command: +```bash +$ chattr +i backups/backup_backup-*.cfg +```🧰 Tools
🪛 LanguageTool
[style] ~48-~48: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ... volume. And Create a snapshot. And Connect to the backupstore pod and ma...(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
50-50
: Add missing articleAdd the article "a" before "second backup" for better readability.
-Then** Delete the first backup and create second backup from the snapshot at the same time. +Then** Delete the first backup and create a second backup from the snapshot at the same time.🧰 Tools
🪛 LanguageTool
[uncategorized] ~50-~50: Possible missing article found.
Context: ...en** Delete the first backup and create second backup from the snapshot at the same ti...(AI_HYDRA_LEO_MISSING_A)
75-75
: Enhance technical details about owner identificationThe step about checking the owner ID could be more detailed to help users locate the correct pod.
-Then** When the backup is in `Deleting` state, delete the longhorn-manager pod which is the owner of the backup. (by checking `Backup.Status.OwnerID`) +Then** When the backup is in `Deleting` state, identify and delete the longhorn-manager pod that owns the backup: +1. Get the backup's owner ID: +```bash +$ kubectl get backup <backup-name> -o jsonpath='{.status.ownerID}' +``` +2. Delete the longhorn-manager pod with the matching node ID🧰 Tools
🪛 LanguageTool
[style] ~75-~75: You can shorten this phrase to avoid wordiness.
Context: ..., delete the longhorn-manager pod which is the owner of the backup. (by checking `Backup.Status...(BE_THE_MAKER_OF_WORDINESS)
69-69
: Format system commands consistentlyFormat the commands in separate code blocks, consistent with earlier sections.
-And** Connect to the backupstore pod and make the backup.cfg file immutable by running the command `$ chattr +i backups/backup_backup-*.cfg`. +And** Connect to the backupstore pod and make the backup.cfg file immutable by running the command: +```bash +$ chattr +i backups/backup_backup-*.cfg +``` -Then** Make the config mutable by running the command `$ chattr -i backups/backup_backup-*.cfg`. +Then** Make the config mutable by running the command: +```bash +$ chattr -i backups/backup_backup-*.cfg +```Also applies to: 79-79
🧰 Tools
🪛 LanguageTool
[style] ~69-~69: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...te the first backup from the volume. And Connect to the backupstore pod and ma...(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
docs/content/manual/release-specific/v1.8.0/test-delete-backup-asynchronously.md
(1 hunks)
🧰 Additional context used
🪛 LanguageTool
docs/content/manual/release-specific/v1.8.0/test-delete-backup-asynchronously.md
[style] ~21-~21: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...nd** Write large data to the volume. And Create the first backup from the volu...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~23-~23: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...te the first backup from the volume. And Write small data to the volume. **An...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~25-~25: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...nd** Write small data to the volume. And Create a snapshot. Then Delete t...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[uncategorized] ~27-~27: Possible missing article found.
Context: ...en** Delete the first backup and create second backup from the snapshot at the same ti...
(AI_HYDRA_LEO_MISSING_A)
[uncategorized] ~31-~31: Possible missing article found.
Context: ...fy** After the first backup is deleted, second backup should be in progress and comple...
(AI_HYDRA_LEO_MISSING_A)
[style] ~42-~42: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...nd** Write large data to the volume. And Create the first backup from the volu...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~44-~44: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...te the first backup from the volume. And Write small data to the volume. **An...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~46-~46: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...nd** Write small data to the volume. And Create a snapshot. And Connect t...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~48-~48: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ... volume. And Create a snapshot. And Connect to the backupstore pod and ma...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[uncategorized] ~50-~50: Possible missing article found.
Context: ...en** Delete the first backup and create second backup from the snapshot at the same ti...
(AI_HYDRA_LEO_MISSING_A)
[style] ~67-~67: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...nd** Write large data to the volume. And Create the first backup from the volu...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~69-~69: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...te the first backup from the volume. And Connect to the backupstore pod and ma...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~75-~75: You can shorten this phrase to avoid wordiness.
Context: ..., delete the longhorn-manager pod which is the owner of the backup. (by checking `Backup.Status...
(BE_THE_MAKER_OF_WORDINESS)
🪛 Markdownlint (0.37.0)
docs/content/manual/release-specific/v1.8.0/test-delete-backup-asynchronously.md
7-7: null
Bare URL used
(MD034, no-bare-urls)
11-11: null
Bare URL used
(MD034, no-bare-urls)
🔇 Additional comments (1)
docs/content/manual/release-specific/v1.8.0/test-delete-backup-asynchronously.md (1)
1-81
: Well-structured test plan with comprehensive coverage
The test plan effectively covers the key scenarios for testing asynchronous backup deletion:
- Normal case
- Error handling with immutable config
- Controller crash recovery
The Gherkin-style format makes the steps clear and easy to follow. The suggested formatting improvements will enhance readability, but the core content and structure are solid.
🧰 Tools
🪛 LanguageTool
[style] ~21-~21: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...nd** Write large data to the volume. And Create the first backup from the volu...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~23-~23: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...te the first backup from the volume. And Write small data to the volume. **An...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~25-~25: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...nd** Write small data to the volume. And Create a snapshot. Then Delete t...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[uncategorized] ~27-~27: Possible missing article found.
Context: ...en** Delete the first backup and create second backup from the snapshot at the same ti...
(AI_HYDRA_LEO_MISSING_A)
[uncategorized] ~31-~31: Possible missing article found.
Context: ...fy** After the first backup is deleted, second backup should be in progress and comple...
(AI_HYDRA_LEO_MISSING_A)
[style] ~42-~42: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...nd** Write large data to the volume. And Create the first backup from the volu...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~44-~44: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...te the first backup from the volume. And Write small data to the volume. **An...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~46-~46: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...nd** Write small data to the volume. And Create a snapshot. And Connect t...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~48-~48: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ... volume. And Create a snapshot. And Connect to the backupstore pod and ma...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[uncategorized] ~50-~50: Possible missing article found.
Context: ...en** Delete the first backup and create second backup from the snapshot at the same ti...
(AI_HYDRA_LEO_MISSING_A)
[style] ~67-~67: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...nd** Write large data to the volume. And Create the first backup from the volu...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~69-~69: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...te the first backup from the volume. And Connect to the backupstore pod and ma...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~75-~75: You can shorten this phrase to avoid wordiness.
Context: ..., delete the longhorn-manager pod which is the owner of the backup. (by checking `Backup.Status...
(BE_THE_MAKER_OF_WORDINESS)
🪛 Markdownlint (0.37.0)
7-7: null
Bare URL used
(MD034, no-bare-urls)
11-11: null
Bare URL used
(MD034, no-bare-urls)
ref: longhorn/longhorn 8746
Add manual test plan for delete backup asynchronously
Summary by CodeRabbit