fix(preflight): check the system default NFS version in mount config #129
Conversation
WalkthroughThe pull request includes updates to the documentation files for the Changes
Assessment against linked issues
Possibly related PRs
Suggested reviewers
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
Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (1)
pkg/local/preflight/checker.go (1)
421-464: LGTM with a minor suggestion for error handling.The kernel config check implementation is thorough, checking both built-in and module-based NFS v4 support. Consider adding error logging when module check fails to help with troubleshooting.
moduleLoaded, err := utils.IsModuleLoaded(option[0]) if err != nil { + logrus.Debugf("Failed to check if module %s is loaded: %v", option[0], err) continue }
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (17)
docs/longhornctl.md(1 hunks)docs/longhornctl_check.md(1 hunks)docs/longhornctl_check_preflight.md(1 hunks)docs/longhornctl_doc.md(1 hunks)docs/longhornctl_export.md(1 hunks)docs/longhornctl_export_replica.md(1 hunks)docs/longhornctl_export_replica_stop.md(1 hunks)docs/longhornctl_get.md(1 hunks)docs/longhornctl_get_replica.md(1 hunks)docs/longhornctl_global-options.md(1 hunks)docs/longhornctl_install.md(1 hunks)docs/longhornctl_install_preflight.md(1 hunks)docs/longhornctl_install_preflight_stop.md(1 hunks)docs/longhornctl_trim.md(1 hunks)docs/longhornctl_trim_volume.md(1 hunks)docs/longhornctl_version.md(1 hunks)pkg/local/preflight/checker.go(1 hunks)
🧰 Additional context used
🪛 LanguageTool
docs/longhornctl.md
[uncategorized] ~29-~29: It appears that a hyphen is missing (if ‘auto’ is not used in the context of ‘cars’).
Context: ...d) - Print longhornctl version ###### Auto generated by spf13/cobra on 25-Nov-2024
(AUTO_HYPHEN)
docs/longhornctl_check.md
[uncategorized] ~19-~19: It appears that a hyphen is missing (if ‘auto’ is not used in the context of ‘cars’).
Context: ...md) - Check Longhorn preflight ###### Auto generated by spf13/cobra on 25-Nov-2024
(AUTO_HYPHEN)
docs/longhornctl_check_preflight.md
[uncategorized] ~60-~60: It appears that a hyphen is missing (if ‘auto’ is not used in the context of ‘cars’).
Context: ... - Longhorn checking operations ###### Auto generated by spf13/cobra on 25-Nov-2024
(AUTO_HYPHEN)
docs/longhornctl_doc.md
[uncategorized] ~31-~31: It appears that a hyphen is missing (if ‘auto’ is not used in the context of ‘cars’).
Context: ...Longhorn commandline interface. ###### Auto generated by spf13/cobra on 25-Nov-2024
(AUTO_HYPHEN)
docs/longhornctl_export.md
[uncategorized] ~19-~19: It appears that a hyphen is missing (if ‘auto’ is not used in the context of ‘cars’).
Context: ....md) - Export Longhorn replica ###### Auto generated by spf13/cobra on 25-Nov-2024
(AUTO_HYPHEN)
docs/longhornctl_export_replica.md
[uncategorized] ~65-~65: It appears that a hyphen is missing (if ‘auto’ is not used in the context of ‘cars’).
Context: ...Stop exporting Longhorn replica ###### Auto generated by spf13/cobra on 25-Nov-2024
(AUTO_HYPHEN)
docs/longhornctl_export_replica_stop.md
[uncategorized] ~34-~34: It appears that a hyphen is missing (if ‘auto’ is not used in the context of ‘cars’).
Context: ....md) - Export Longhorn replica ###### Auto generated by spf13/cobra on 25-Nov-2024
(AUTO_HYPHEN)
docs/longhornctl_get.md
[uncategorized] ~19-~19: It appears that a hyphen is missing (if ‘auto’ is not used in the context of ‘cars’).
Context: ...et Longhorn replica information ###### Auto generated by spf13/cobra on 25-Nov-2024
(AUTO_HYPHEN)
docs/longhornctl_get_replica.md
[uncategorized] ~63-~63: It appears that a hyphen is missing (if ‘auto’ is not used in the context of ‘cars’).
Context: ...nformation gathering operations ###### Auto generated by spf13/cobra on 25-Nov-2024
(AUTO_HYPHEN)
docs/longhornctl_global-options.md
[uncategorized] ~31-~31: It appears that a hyphen is missing (if ‘auto’ is not used in the context of ‘cars’).
Context: ...Longhorn commandline interface. ###### Auto generated by spf13/cobra on 25-Nov-2024
(AUTO_HYPHEN)
docs/longhornctl_install.md
[uncategorized] ~19-~19: It appears that a hyphen is missing (if ‘auto’ is not used in the context of ‘cars’).
Context: ...) - Install Longhorn preflight ###### Auto generated by spf13/cobra on 25-Nov-2024
(AUTO_HYPHEN)
docs/longhornctl_install_preflight.md
[uncategorized] ~48-~48: It appears that a hyphen is missing (if ‘auto’ is not used in the context of ‘cars’).
Context: ...op Longhorn preflight installer ###### Auto generated by spf13/cobra on 25-Nov-2024
(AUTO_HYPHEN)
docs/longhornctl_install_preflight_stop.md
[uncategorized] ~35-~35: It appears that a hyphen is missing (if ‘auto’ is not used in the context of ‘cars’).
Context: ...) - Install Longhorn preflight ###### Auto generated by spf13/cobra on 25-Nov-2024
(AUTO_HYPHEN)
docs/longhornctl_trim.md
[uncategorized] ~19-~19: It appears that a hyphen is missing (if ‘auto’ is not used in the context of ‘cars’).
Context: ...e.md) - Trim a Longhorn volume ###### Auto generated by spf13/cobra on 25-Nov-2024
(AUTO_HYPHEN)
docs/longhornctl_trim_volume.md
[uncategorized] ~45-~45: It appears that a hyphen is missing (if ‘auto’ is not used in the context of ‘cars’).
Context: ... - Longhorn trimming operations ###### Auto generated by spf13/cobra on 25-Nov-2024
(AUTO_HYPHEN)
docs/longhornctl_version.md
[uncategorized] ~27-~27: It appears that a hyphen is missing (if ‘auto’ is not used in the context of ‘cars’).
Context: ...Longhorn commandline interface. ###### Auto generated by spf13/cobra on 25-Nov-2024
(AUTO_HYPHEN)
🪛 Markdownlint (0.35.0)
docs/longhornctl.md
29-29: Expected: h4; Actual: h6
Heading levels should only increment by one level at a time
(MD001, heading-increment)
docs/longhornctl_check.md
19-19: Expected: h4; Actual: h6
Heading levels should only increment by one level at a time
(MD001, heading-increment)
docs/longhornctl_check_preflight.md
60-60: Expected: h4; Actual: h6
Heading levels should only increment by one level at a time
(MD001, heading-increment)
docs/longhornctl_doc.md
31-31: Expected: h4; Actual: h6
Heading levels should only increment by one level at a time
(MD001, heading-increment)
docs/longhornctl_export.md
19-19: Expected: h4; Actual: h6
Heading levels should only increment by one level at a time
(MD001, heading-increment)
docs/longhornctl_export_replica.md
65-65: Expected: h4; Actual: h6
Heading levels should only increment by one level at a time
(MD001, heading-increment)
docs/longhornctl_export_replica_stop.md
34-34: Expected: h4; Actual: h6
Heading levels should only increment by one level at a time
(MD001, heading-increment)
docs/longhornctl_get.md
19-19: Expected: h4; Actual: h6
Heading levels should only increment by one level at a time
(MD001, heading-increment)
docs/longhornctl_get_replica.md
63-63: Expected: h4; Actual: h6
Heading levels should only increment by one level at a time
(MD001, heading-increment)
docs/longhornctl_global-options.md
31-31: Expected: h4; Actual: h6
Heading levels should only increment by one level at a time
(MD001, heading-increment)
docs/longhornctl_install.md
19-19: Expected: h4; Actual: h6
Heading levels should only increment by one level at a time
(MD001, heading-increment)
docs/longhornctl_install_preflight.md
48-48: Expected: h4; Actual: h6
Heading levels should only increment by one level at a time
(MD001, heading-increment)
docs/longhornctl_install_preflight_stop.md
35-35: Expected: h4; Actual: h6
Heading levels should only increment by one level at a time
(MD001, heading-increment)
docs/longhornctl_trim.md
19-19: Expected: h4; Actual: h6
Heading levels should only increment by one level at a time
(MD001, heading-increment)
docs/longhornctl_trim_volume.md
45-45: Expected: h4; Actual: h6
Heading levels should only increment by one level at a time
(MD001, heading-increment)
docs/longhornctl_version.md
27-27: Expected: h4; Actual: h6
Heading levels should only increment by one level at a time
(MD001, heading-increment)
🔇 Additional comments (3)
pkg/local/preflight/checker.go (3)
465-468: LGTM!
Proper error handling with descriptive error wrapping.
499-505: LGTM!
The logging implementation provides clear and helpful messages, especially the warning that guides users to the manual page when NFS4 isn't the default version.
469-505: Verify behavior with different nfsmount.conf configurations.
Let's verify the implementation handles various nfsmount.conf configurations correctly.
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Outside diff range and nitpick comments (15)
docs/longhornctl_trim.md (1)
19-19: Documentation structure: Consider adjusting the heading levelThe auto-generated section uses h6 (######) but should use h4 (####) to maintain proper heading hierarchy, as headings should only increment by one level at a time.
-###### Auto generated by spf13/cobra on 26-Nov-2024 +#### Auto generated by spf13/cobra on 26-Nov-2024🧰 Tools
🪛 LanguageTool
[uncategorized] ~19-~19: It appears that a hyphen is missing (if ‘auto’ is not used in the context of ‘cars’).
Context: ...e.md) - Trim a Longhorn volume ###### Auto generated by spf13/cobra on 26-Nov-2024(AUTO_HYPHEN)
🪛 Markdownlint (0.35.0)
19-19: Expected: h4; Actual: h6
Heading levels should only increment by one level at a time(MD001, heading-increment)
docs/longhornctl_export.md (1)
19-19: LGTM - Auto-generated documentation updateThis is an automatically generated documentation file, and the timestamp update is expected. No manual changes are needed as this file is managed by spf13/cobra.
Note: While there are some minor markdown formatting inconsistencies (heading levels), these are part of the auto-generation process and should be addressed at the generator level if needed.
🧰 Tools
🪛 LanguageTool
[uncategorized] ~19-~19: It appears that a hyphen is missing (if ‘auto’ is not used in the context of ‘cars’).
Context: ....md) - Export Longhorn replica ###### Auto generated by spf13/cobra on 26-Nov-2024(AUTO_HYPHEN)
🪛 Markdownlint (0.35.0)
19-19: Expected: h4; Actual: h6
Heading levels should only increment by one level at a time(MD001, heading-increment)
docs/longhornctl_get.md (1)
19-19: Consider improving documentation formattingFor better documentation consistency:
- The heading level should increment by only one level (use
####instead of######)- "Auto generated" should be hyphenated as "Auto-generated"
-###### Auto generated by spf13/cobra on 26-Nov-2024 +#### Auto-generated by spf13/cobra on 26-Nov-2024🧰 Tools
🪛 LanguageTool
[uncategorized] ~19-~19: It appears that a hyphen is missing (if ‘auto’ is not used in the context of ‘cars’).
Context: ...et Longhorn replica information ###### Auto generated by spf13/cobra on 26-Nov-2024(AUTO_HYPHEN)
🪛 Markdownlint (0.35.0)
19-19: Expected: h4; Actual: h6
Heading levels should only increment by one level at a time(MD001, heading-increment)
docs/longhornctl_install.md (2)
19-19: Fix auto-generated footer styleThe auto-generated footer has two style issues:
- Incorrect heading level (h6)
- Missing hyphen in "Auto generated"
Apply this diff to fix the style issues:
-###### Auto generated by spf13/cobra on 26-Nov-2024 +#### Auto-generated by spf13/cobra on 26-Nov-2024🧰 Tools
🪛 LanguageTool
[uncategorized] ~19-~19: It appears that a hyphen is missing (if ‘auto’ is not used in the context of ‘cars’).
Context: ...) - Install Longhorn preflight ###### Auto generated by spf13/cobra on 26-Nov-2024(AUTO_HYPHEN)
🪛 Markdownlint (0.35.0)
19-19: Expected: h4; Actual: h6
Heading levels should only increment by one level at a time(MD001, heading-increment)
Documentation needs to be updated to reflect NFS version configuration checks
The codebase shows that NFS version checks have been implemented in
pkg/local/preflight/checker.go, including verification of/etc/nfsmount.confand protocol version requirements. However, this important information is not documented in eitherlonghornctl_install.mdorlonghornctl_check_preflight.md. The preflight documentation should be updated to:
- Mention the NFS version requirements (v4, 4.1, or 4.2)
- Document the
/etc/nfsmount.confconfiguration check- Include the new warning message that appears when NFS version is not properly configured
🔗 Analysis chain
Line range hint
1-18: Documentation updates for NFS version checks are missingThe PR objectives mention updating the documentation to reflect NFS version checking in
/etc/nfsmount.conf, but these changes are not present in this documentation file.Let's check if NFS-related documentation exists elsewhere:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Search for NFS-related documentation files # Expected: Find documentation files that might need updates for NFS version checks # Search for NFS-related markdown files fd -e md -x grep -l "NFS" {} # Search for NFS-related documentation in other text files fd -e txt -e rst -x grep -l "NFS" {}Length of output: 107
Script:
#!/bin/bash # Let's check the content of the NFS-related documentation file cat ./docs/longhornctl_check_preflight.mdLength of output: 2015
Script:
#!/bin/bash # Let's check for any NFS version or mount configuration related changes in the codebase rg -i "nfs.*vers|nfsmount\.conf" -g '!{*.md,*.txt}'Length of output: 489
🧰 Tools
🪛 LanguageTool
[uncategorized] ~19-~19: It appears that a hyphen is missing (if ‘auto’ is not used in the context of ‘cars’).
Context: ...) - Install Longhorn preflight ###### Auto generated by spf13/cobra on 26-Nov-2024(AUTO_HYPHEN)
🪛 Markdownlint (0.35.0)
19-19: Expected: h4; Actual: h6
Heading levels should only increment by one level at a time(MD001, heading-increment)
16-16: Column: 32
Hard tabs(MD010, no-hard-tabs)
17-17: Column: 68
Hard tabs(MD010, no-hard-tabs)
docs/longhornctl_version.md (1)
27-27: Fix documentation formatting issues.There are two minor formatting improvements needed:
- "Auto generated" should be hyphenated as "Auto-generated"
- The heading level should be h4 (####) instead of h6 (######)
Apply this diff:
-###### Auto generated by spf13/cobra on 26-Nov-2024 +#### Auto-generated by spf13/cobra on 26-Nov-2024🧰 Tools
🪛 LanguageTool
[uncategorized] ~27-~27: It appears that a hyphen is missing (if ‘auto’ is not used in the context of ‘cars’).
Context: ...Longhorn commandline interface. ###### Auto generated by spf13/cobra on 26-Nov-2024(AUTO_HYPHEN)
🪛 Markdownlint (0.35.0)
27-27: Expected: h4; Actual: h6
Heading levels should only increment by one level at a time(MD001, heading-increment)
docs/longhornctl_doc.md (1)
31-31: Documentation formatting improvements needed.There are two formatting issues that should be addressed:
- "Auto generated" should be hyphenated as "Auto-generated"
- The heading level (###### h6) violates markdown hierarchy - it should be #### (h4) to maintain proper heading structure
Apply this diff to fix both issues:
-###### Auto generated by spf13/cobra on 26-Nov-2024 +#### Auto-generated by spf13/cobra on 26-Nov-2024🧰 Tools
🪛 LanguageTool
[uncategorized] ~31-~31: It appears that a hyphen is missing (if ‘auto’ is not used in the context of ‘cars’).
Context: ...Longhorn commandline interface. ###### Auto generated by spf13/cobra on 26-Nov-2024(AUTO_HYPHEN)
🪛 Markdownlint (0.35.0)
31-31: Expected: h4; Actual: h6
Heading levels should only increment by one level at a time(MD001, heading-increment)
docs/longhornctl_global-options.md (1)
31-31: Fix markdown formatting issuesThere are two formatting issues to address:
- "Auto generated" should be hyphenated to "Auto-generated"
- The heading level should be h4 (####) instead of h6 (######)
-###### Auto generated by spf13/cobra on 26-Nov-2024 +#### Auto-generated by spf13/cobra on 26-Nov-2024🧰 Tools
🪛 LanguageTool
[uncategorized] ~31-~31: It appears that a hyphen is missing (if ‘auto’ is not used in the context of ‘cars’).
Context: ...Longhorn commandline interface. ###### Auto generated by spf13/cobra on 26-Nov-2024(AUTO_HYPHEN)
🪛 Markdownlint (0.35.0)
31-31: Expected: h4; Actual: h6
Heading levels should only increment by one level at a time(MD001, heading-increment)
docs/longhornctl_export_replica_stop.md (1)
34-34: Note: Auto-generated documentation with minor formatting issuesThis is an auto-generated documentation file. There are some minor formatting issues in the generated content:
- "Auto generated" should be hyphenated as "Auto-generated"
- The heading level jumps from h1 to h6, which violates markdown best practices
Since these issues are in auto-generated content, they should be addressed in the upstream spf13/cobra project rather than fixed manually here.
Consider opening an issue in the spf13/cobra repository to improve the documentation generation formatting.
🧰 Tools
🪛 LanguageTool
[uncategorized] ~34-~34: It appears that a hyphen is missing (if ‘auto’ is not used in the context of ‘cars’).
Context: ....md) - Export Longhorn replica ###### Auto generated by spf13/cobra on 26-Nov-2024(AUTO_HYPHEN)
🪛 Markdownlint (0.35.0)
34-34: Expected: h4; Actual: h6
Heading levels should only increment by one level at a time(MD001, heading-increment)
docs/longhornctl_install_preflight_stop.md (1)
35-35: Fix markdown formatting issuesThe auto-generated timestamp line has some formatting issues:
- The heading level jumps from h3 to h6 (######). It should use h4 (####) for proper hierarchy.
- "Auto generated" should be hyphenated as "Auto-generated".
Apply this diff to fix the formatting:
-###### Auto generated by spf13/cobra on 26-Nov-2024 +#### Auto-generated by spf13/cobra on 26-Nov-2024🧰 Tools
🪛 LanguageTool
[uncategorized] ~35-~35: It appears that a hyphen is missing (if ‘auto’ is not used in the context of ‘cars’).
Context: ...) - Install Longhorn preflight ###### Auto generated by spf13/cobra on 26-Nov-2024(AUTO_HYPHEN)
🪛 Markdownlint (0.35.0)
35-35: Expected: h4; Actual: h6
Heading levels should only increment by one level at a time(MD001, heading-increment)
docs/longhornctl_trim_volume.md (1)
45-45: Consider fixing markdown formatting issues.
- Add a hyphen to "Auto-generated" for consistency
- Use the correct heading level (h4) instead of h6
Apply this diff to fix the formatting:
-###### Auto generated by spf13/cobra on 26-Nov-2024 +#### Auto-generated by spf13/cobra on 26-Nov-2024🧰 Tools
🪛 LanguageTool
[uncategorized] ~45-~45: It appears that a hyphen is missing (if ‘auto’ is not used in the context of ‘cars’).
Context: ... - Longhorn trimming operations ###### Auto generated by spf13/cobra on 26-Nov-2024(AUTO_HYPHEN)
🪛 Markdownlint (0.35.0)
45-45: Expected: h4; Actual: h6
Heading levels should only increment by one level at a time(MD001, heading-increment)
docs/longhornctl_check_preflight.md (1)
Line range hint
1-59: Documentation needs update regarding NFS version checkingAccording to the PR objectives, the documentation should be updated to reflect the changes in NFS version checking functionality, particularly regarding the
/etc/nfsmount.conffile. Consider adding this information to the command's documentation to help users understand how the preflight checks verify NFS version support.Would you like me to help draft the additional documentation content about NFS version checking?
🧰 Tools
🪛 LanguageTool
[uncategorized] ~60-~60: It appears that a hyphen is missing (if ‘auto’ is not used in the context of ‘cars’).
Context: ... - Longhorn checking operations ###### Auto generated by spf13/cobra on 26-Nov-2024(AUTO_HYPHEN)
🪛 Markdownlint (0.35.0)
60-60: Expected: h4; Actual: h6
Heading levels should only increment by one level at a time(MD001, heading-increment)
58-58: Column: 44
Hard tabs(MD010, no-hard-tabs)
docs/longhornctl_install_preflight.md (1)
48-48: Consider fixing the hyphenation in "Auto generated"The phrase "Auto generated" should be hyphenated as it's used as a compound adjective.
-###### Auto generated by spf13/cobra on 26-Nov-2024 +###### Auto-generated by spf13/cobra on 26-Nov-2024However, since this is auto-generated by cobra, you might want to raise this upstream with the cobra project instead.
🧰 Tools
🪛 LanguageTool
[uncategorized] ~48-~48: It appears that a hyphen is missing (if ‘auto’ is not used in the context of ‘cars’).
Context: ...op Longhorn preflight installer ###### Auto generated by spf13/cobra on 26-Nov-2024(AUTO_HYPHEN)
🪛 Markdownlint (0.35.0)
48-48: Expected: h4; Actual: h6
Heading levels should only increment by one level at a time(MD001, heading-increment)
docs/longhornctl_export_replica.md (1)
65-65: Fix hyphenation in the footer textThe term "Auto generated" should be hyphenated as "Auto-generated" to follow proper English conventions.
-###### Auto generated by spf13/cobra on 26-Nov-2024 +###### Auto-generated by spf13/cobra on 26-Nov-2024🧰 Tools
🪛 LanguageTool
[uncategorized] ~65-~65: It appears that a hyphen is missing (if ‘auto’ is not used in the context of ‘cars’).
Context: ...Stop exporting Longhorn replica ###### Auto generated by spf13/cobra on 26-Nov-2024(AUTO_HYPHEN)
🪛 Markdownlint (0.35.0)
65-65: Expected: h4; Actual: h6
Heading levels should only increment by one level at a time(MD001, heading-increment)
pkg/local/preflight/checker.go (1)
481-497: Enhance config parsing robustnessThe current implementation could be improved in several ways:
- Add comment handling (lines starting with #)
- Make section parsing more robust
- Add validation for version format
for nfsMountConfigScanner.Scan() { line := strings.TrimSpace(nfsMountConfigScanner.Text()) + if line == "" || strings.HasPrefix(line, "#") { + continue + } if strings.HasPrefix(line, "[") { - section = strings.TrimSpace(strings.Trim(line, "[]")) + section = strings.Trim(strings.TrimSpace(line), "[]") + if section == "" { + continue + } } else if section == "NFSMount_Global_Options" { if key, val, isParsable := strings.Cut(line, "="); isParsable { + key = strings.TrimSpace(key) + val = strings.TrimSpace(val) switch key { case "Defaultvers": verParts := strings.SplitN(val, ".", 2) + if len(verParts) == 0 { + continue + } isDefaultV4 = verParts[0] == "4" if len(verParts) == 2 && isDefaultV4 { isDefaultV4 = verParts[1] == "0" || verParts[1] == "1" || verParts[1] == "2" } } } } }
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (17)
docs/longhornctl.md(1 hunks)docs/longhornctl_check.md(1 hunks)docs/longhornctl_check_preflight.md(1 hunks)docs/longhornctl_doc.md(1 hunks)docs/longhornctl_export.md(1 hunks)docs/longhornctl_export_replica.md(1 hunks)docs/longhornctl_export_replica_stop.md(1 hunks)docs/longhornctl_get.md(1 hunks)docs/longhornctl_get_replica.md(1 hunks)docs/longhornctl_global-options.md(1 hunks)docs/longhornctl_install.md(1 hunks)docs/longhornctl_install_preflight.md(1 hunks)docs/longhornctl_install_preflight_stop.md(1 hunks)docs/longhornctl_trim.md(1 hunks)docs/longhornctl_trim_volume.md(1 hunks)docs/longhornctl_version.md(1 hunks)pkg/local/preflight/checker.go(1 hunks)
🧰 Additional context used
🪛 LanguageTool
docs/longhornctl.md
[uncategorized] ~29-~29: It appears that a hyphen is missing (if ‘auto’ is not used in the context of ‘cars’).
Context: ...d) - Print longhornctl version ###### Auto generated by spf13/cobra on 26-Nov-2024
(AUTO_HYPHEN)
docs/longhornctl_check.md
[uncategorized] ~19-~19: It appears that a hyphen is missing (if ‘auto’ is not used in the context of ‘cars’).
Context: ...md) - Check Longhorn preflight ###### Auto generated by spf13/cobra on 26-Nov-2024
(AUTO_HYPHEN)
docs/longhornctl_check_preflight.md
[uncategorized] ~60-~60: It appears that a hyphen is missing (if ‘auto’ is not used in the context of ‘cars’).
Context: ... - Longhorn checking operations ###### Auto generated by spf13/cobra on 26-Nov-2024
(AUTO_HYPHEN)
docs/longhornctl_doc.md
[uncategorized] ~31-~31: It appears that a hyphen is missing (if ‘auto’ is not used in the context of ‘cars’).
Context: ...Longhorn commandline interface. ###### Auto generated by spf13/cobra on 26-Nov-2024
(AUTO_HYPHEN)
docs/longhornctl_export.md
[uncategorized] ~19-~19: It appears that a hyphen is missing (if ‘auto’ is not used in the context of ‘cars’).
Context: ....md) - Export Longhorn replica ###### Auto generated by spf13/cobra on 26-Nov-2024
(AUTO_HYPHEN)
docs/longhornctl_export_replica.md
[uncategorized] ~65-~65: It appears that a hyphen is missing (if ‘auto’ is not used in the context of ‘cars’).
Context: ...Stop exporting Longhorn replica ###### Auto generated by spf13/cobra on 26-Nov-2024
(AUTO_HYPHEN)
docs/longhornctl_export_replica_stop.md
[uncategorized] ~34-~34: It appears that a hyphen is missing (if ‘auto’ is not used in the context of ‘cars’).
Context: ....md) - Export Longhorn replica ###### Auto generated by spf13/cobra on 26-Nov-2024
(AUTO_HYPHEN)
docs/longhornctl_get.md
[uncategorized] ~19-~19: It appears that a hyphen is missing (if ‘auto’ is not used in the context of ‘cars’).
Context: ...et Longhorn replica information ###### Auto generated by spf13/cobra on 26-Nov-2024
(AUTO_HYPHEN)
docs/longhornctl_get_replica.md
[uncategorized] ~63-~63: It appears that a hyphen is missing (if ‘auto’ is not used in the context of ‘cars’).
Context: ...nformation gathering operations ###### Auto generated by spf13/cobra on 26-Nov-2024
(AUTO_HYPHEN)
docs/longhornctl_global-options.md
[uncategorized] ~31-~31: It appears that a hyphen is missing (if ‘auto’ is not used in the context of ‘cars’).
Context: ...Longhorn commandline interface. ###### Auto generated by spf13/cobra on 26-Nov-2024
(AUTO_HYPHEN)
docs/longhornctl_install.md
[uncategorized] ~19-~19: It appears that a hyphen is missing (if ‘auto’ is not used in the context of ‘cars’).
Context: ...) - Install Longhorn preflight ###### Auto generated by spf13/cobra on 26-Nov-2024
(AUTO_HYPHEN)
docs/longhornctl_install_preflight.md
[uncategorized] ~48-~48: It appears that a hyphen is missing (if ‘auto’ is not used in the context of ‘cars’).
Context: ...op Longhorn preflight installer ###### Auto generated by spf13/cobra on 26-Nov-2024
(AUTO_HYPHEN)
docs/longhornctl_install_preflight_stop.md
[uncategorized] ~35-~35: It appears that a hyphen is missing (if ‘auto’ is not used in the context of ‘cars’).
Context: ...) - Install Longhorn preflight ###### Auto generated by spf13/cobra on 26-Nov-2024
(AUTO_HYPHEN)
docs/longhornctl_trim.md
[uncategorized] ~19-~19: It appears that a hyphen is missing (if ‘auto’ is not used in the context of ‘cars’).
Context: ...e.md) - Trim a Longhorn volume ###### Auto generated by spf13/cobra on 26-Nov-2024
(AUTO_HYPHEN)
docs/longhornctl_trim_volume.md
[uncategorized] ~45-~45: It appears that a hyphen is missing (if ‘auto’ is not used in the context of ‘cars’).
Context: ... - Longhorn trimming operations ###### Auto generated by spf13/cobra on 26-Nov-2024
(AUTO_HYPHEN)
docs/longhornctl_version.md
[uncategorized] ~27-~27: It appears that a hyphen is missing (if ‘auto’ is not used in the context of ‘cars’).
Context: ...Longhorn commandline interface. ###### Auto generated by spf13/cobra on 26-Nov-2024
(AUTO_HYPHEN)
🪛 Markdownlint (0.35.0)
docs/longhornctl.md
29-29: Expected: h4; Actual: h6
Heading levels should only increment by one level at a time
(MD001, heading-increment)
docs/longhornctl_check.md
19-19: Expected: h4; Actual: h6
Heading levels should only increment by one level at a time
(MD001, heading-increment)
docs/longhornctl_check_preflight.md
60-60: Expected: h4; Actual: h6
Heading levels should only increment by one level at a time
(MD001, heading-increment)
docs/longhornctl_doc.md
31-31: Expected: h4; Actual: h6
Heading levels should only increment by one level at a time
(MD001, heading-increment)
docs/longhornctl_export.md
19-19: Expected: h4; Actual: h6
Heading levels should only increment by one level at a time
(MD001, heading-increment)
docs/longhornctl_export_replica.md
65-65: Expected: h4; Actual: h6
Heading levels should only increment by one level at a time
(MD001, heading-increment)
docs/longhornctl_export_replica_stop.md
34-34: Expected: h4; Actual: h6
Heading levels should only increment by one level at a time
(MD001, heading-increment)
docs/longhornctl_get.md
19-19: Expected: h4; Actual: h6
Heading levels should only increment by one level at a time
(MD001, heading-increment)
docs/longhornctl_get_replica.md
63-63: Expected: h4; Actual: h6
Heading levels should only increment by one level at a time
(MD001, heading-increment)
docs/longhornctl_global-options.md
31-31: Expected: h4; Actual: h6
Heading levels should only increment by one level at a time
(MD001, heading-increment)
docs/longhornctl_install.md
19-19: Expected: h4; Actual: h6
Heading levels should only increment by one level at a time
(MD001, heading-increment)
docs/longhornctl_install_preflight.md
48-48: Expected: h4; Actual: h6
Heading levels should only increment by one level at a time
(MD001, heading-increment)
docs/longhornctl_install_preflight_stop.md
35-35: Expected: h4; Actual: h6
Heading levels should only increment by one level at a time
(MD001, heading-increment)
docs/longhornctl_trim.md
19-19: Expected: h4; Actual: h6
Heading levels should only increment by one level at a time
(MD001, heading-increment)
docs/longhornctl_trim_volume.md
45-45: Expected: h4; Actual: h6
Heading levels should only increment by one level at a time
(MD001, heading-increment)
docs/longhornctl_version.md
27-27: Expected: h4; Actual: h6
Heading levels should only increment by one level at a time
(MD001, heading-increment)
🔇 Additional comments (11)
docs/longhornctl_check.md (1)
19-19: LGTM - Auto-generated documentation update
The timestamp update is correct and expected as part of the auto-generated documentation.
🧰 Tools
🪛 LanguageTool
[uncategorized] ~19-~19: It appears that a hyphen is missing (if ‘auto’ is not used in the context of ‘cars’).
Context: ...md) - Check Longhorn preflight ###### Auto generated by spf13/cobra on 26-Nov-2024
(AUTO_HYPHEN)
🪛 Markdownlint (0.35.0)
19-19: Expected: h4; Actual: h6
Heading levels should only increment by one level at a time
(MD001, heading-increment)
docs/longhornctl_get.md (1)
19-19: LGTM: Auto-generated timestamp update
The timestamp update is an expected automated change from the cobra CLI tool.
🧰 Tools
🪛 LanguageTool
[uncategorized] ~19-~19: It appears that a hyphen is missing (if ‘auto’ is not used in the context of ‘cars’).
Context: ...et Longhorn replica information ###### Auto generated by spf13/cobra on 26-Nov-2024
(AUTO_HYPHEN)
🪛 Markdownlint (0.35.0)
19-19: Expected: h4; Actual: h6
Heading levels should only increment by one level at a time
(MD001, heading-increment)
docs/longhornctl_version.md (1)
27-27: LGTM: Timestamp update is consistent.
The timestamp update aligns with the changes mentioned in the AI summary.
🧰 Tools
🪛 LanguageTool
[uncategorized] ~27-~27: It appears that a hyphen is missing (if ‘auto’ is not used in the context of ‘cars’).
Context: ...Longhorn commandline interface. ###### Auto generated by spf13/cobra on 26-Nov-2024
(AUTO_HYPHEN)
🪛 Markdownlint (0.35.0)
27-27: Expected: h4; Actual: h6
Heading levels should only increment by one level at a time
(MD001, heading-increment)
docs/longhornctl_doc.md (1)
31-31: LGTM: Timestamp update is correct.
The auto-generated timestamp has been updated appropriately.
🧰 Tools
🪛 LanguageTool
[uncategorized] ~31-~31: It appears that a hyphen is missing (if ‘auto’ is not used in the context of ‘cars’).
Context: ...Longhorn commandline interface. ###### Auto generated by spf13/cobra on 26-Nov-2024
(AUTO_HYPHEN)
🪛 Markdownlint (0.35.0)
31-31: Expected: h4; Actual: h6
Heading levels should only increment by one level at a time
(MD001, heading-increment)
docs/longhornctl_global-options.md (1)
Line range hint 1-31: Documentation updates needed for NFS version checking feature
The documentation doesn't reflect the changes mentioned in the PR objectives regarding NFS version checking in mount config. Consider adding information about:
- The importance of checking
/etc/nfsmount.conf - How to verify the NFS version configuration
- Impact on backup-related features
Would you like me to help draft the documentation updates for the NFS version checking feature?
🧰 Tools
🪛 LanguageTool
[uncategorized] ~31-~31: It appears that a hyphen is missing (if ‘auto’ is not used in the context of ‘cars’).
Context: ...Longhorn commandline interface. ###### Auto generated by spf13/cobra on 26-Nov-2024
(AUTO_HYPHEN)
🪛 Markdownlint (0.35.0)
31-31: Expected: h4; Actual: h6
Heading levels should only increment by one level at a time
(MD001, heading-increment)
29-29: Column: 32
Hard tabs
(MD010, no-hard-tabs)
docs/longhornctl.md (1)
29-29: LGTM: Auto-generated timestamp update
The timestamp update is correct and consistent with the documentation regeneration.
🧰 Tools
🪛 LanguageTool
[uncategorized] ~29-~29: It appears that a hyphen is missing (if ‘auto’ is not used in the context of ‘cars’).
Context: ...d) - Print longhornctl version ###### Auto generated by spf13/cobra on 26-Nov-2024
(AUTO_HYPHEN)
🪛 Markdownlint (0.35.0)
29-29: Expected: h4; Actual: h6
Heading levels should only increment by one level at a time
(MD001, heading-increment)
docs/longhornctl_trim_volume.md (1)
45-45: LGTM on the timestamp update.
The auto-generated timestamp has been updated appropriately.
🧰 Tools
🪛 LanguageTool
[uncategorized] ~45-~45: It appears that a hyphen is missing (if ‘auto’ is not used in the context of ‘cars’).
Context: ... - Longhorn trimming operations ###### Auto generated by spf13/cobra on 26-Nov-2024
(AUTO_HYPHEN)
🪛 Markdownlint (0.35.0)
45-45: Expected: h4; Actual: h6
Heading levels should only increment by one level at a time
(MD001, heading-increment)
docs/longhornctl_check_preflight.md (1)
60-60: LGTM!
The auto-generated timestamp update is correct.
🧰 Tools
🪛 LanguageTool
[uncategorized] ~60-~60: It appears that a hyphen is missing (if ‘auto’ is not used in the context of ‘cars’).
Context: ... - Longhorn checking operations ###### Auto generated by spf13/cobra on 26-Nov-2024
(AUTO_HYPHEN)
🪛 Markdownlint (0.35.0)
60-60: Expected: h4; Actual: h6
Heading levels should only increment by one level at a time
(MD001, heading-increment)
docs/longhornctl_get_replica.md (1)
63-63: LGTM!
The timestamp update is a routine change for auto-generated documentation.
🧰 Tools
🪛 LanguageTool
[uncategorized] ~63-~63: It appears that a hyphen is missing (if ‘auto’ is not used in the context of ‘cars’).
Context: ...nformation gathering operations ###### Auto generated by spf13/cobra on 26-Nov-2024
(AUTO_HYPHEN)
🪛 Markdownlint (0.35.0)
63-63: Expected: h4; Actual: h6
Heading levels should only increment by one level at a time
(MD001, heading-increment)
docs/longhornctl_export_replica.md (1)
65-65: LGTM on the timestamp update!
The auto-generated timestamp has been updated as expected.
🧰 Tools
🪛 LanguageTool
[uncategorized] ~65-~65: It appears that a hyphen is missing (if ‘auto’ is not used in the context of ‘cars’).
Context: ...Stop exporting Longhorn replica ###### Auto generated by spf13/cobra on 26-Nov-2024
(AUTO_HYPHEN)
🪛 Markdownlint (0.35.0)
65-65: Expected: h4; Actual: h6
Heading levels should only increment by one level at a time
(MD001, heading-increment)
pkg/local/preflight/checker.go (1)
507-513: LGTM! Clear and informative status messages
The result handling provides clear feedback with appropriate error levels and helpful guidance for users.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (12)
docs/longhornctl_export.md (1)
19-19: Consider standardizing heading hierarchyThe auto-generated heading uses h6 (######) which skips several heading levels. While this is auto-generated content, it would be good to update the template to use a more appropriate heading level (h4) for better document structure.
🧰 Tools
🪛 LanguageTool
[uncategorized] ~19-~19: It appears that a hyphen is missing (if ‘auto’ is not used in the context of ‘cars’).
Context: ....md) - Export Longhorn replica ###### Auto generated by spf13/cobra on 27-Nov-2024(AUTO_HYPHEN)
🪛 Markdownlint (0.35.0)
19-19: Expected: h4; Actual: h6
Heading levels should only increment by one level at a time(MD001, heading-increment)
docs/longhornctl_get.md (1)
19-19: LGTM: Timestamp update is consistentThe auto-generated timestamp has been updated appropriately.
Consider these formatting improvements:
- Use heading level 4 (
####) instead of level 6 (######) for consistency- Add a hyphen to "Auto-generated" for proper formatting
-###### Auto generated by spf13/cobra on 27-Nov-2024 +#### Auto-generated by spf13/cobra on 27-Nov-2024🧰 Tools
🪛 Markdownlint (0.35.0)
19-19: Expected: h4; Actual: h6
Heading levels should only increment by one level at a time(MD001, heading-increment)
🪛 LanguageTool
[uncategorized] ~19-~19: It appears that a hyphen is missing (if ‘auto’ is not used in the context of ‘cars’).
Context: ...et Longhorn replica information ###### Auto generated by spf13/cobra on 27-Nov-2024(AUTO_HYPHEN)
docs/longhornctl_doc.md (1)
31-31: Note: This is an auto-generated fileThe changes look good. However, since this is auto-generated documentation, any improvements should be made to the generator configuration rather than directly editing this file.
Consider updating the spf13/cobra generator configuration to:
- Add a hyphen in "Auto-generated"
- Use consistent heading levels (h4 instead of h6 for the timestamp)
🧰 Tools
🪛 LanguageTool
[uncategorized] ~31-~31: It appears that a hyphen is missing (if ‘auto’ is not used in the context of ‘cars’).
Context: ...Longhorn commandline interface. ###### Auto generated by spf13/cobra on 27-Nov-2024(AUTO_HYPHEN)
🪛 Markdownlint (0.35.0)
31-31: Expected: h4; Actual: h6
Heading levels should only increment by one level at a time(MD001, heading-increment)
docs/longhornctl_global-options.md (1)
31-31: Fix markdown heading level and hyphenation
- The heading level should be h4 (####) instead of h6 (######) to maintain proper hierarchy
- "Auto generated" should be hyphenated as "Auto-generated"
Apply this diff:
-###### Auto generated by spf13/cobra on 27-Nov-2024 +#### Auto-generated by spf13/cobra on 27-Nov-2024🧰 Tools
🪛 Markdownlint (0.35.0)
31-31: Expected: h4; Actual: h6
Heading levels should only increment by one level at a time(MD001, heading-increment)
🪛 LanguageTool
[uncategorized] ~31-~31: It appears that a hyphen is missing (if ‘auto’ is not used in the context of ‘cars’).
Context: ...Longhorn commandline interface. ###### Auto generated by spf13/cobra on 27-Nov-2024(AUTO_HYPHEN)
docs/longhornctl_export_replica_stop.md (1)
34-34: Consider addressing auto-generation formatting in upstream cobra.The auto-generated footer has some formatting issues that should be addressed in the upstream spf13/cobra generator:
- The heading level jumps from h1 to h6
- "Auto generated" should be hyphenated as "Auto-generated"
Since this is auto-generated content, these issues should be fixed in the generator rather than manually.
Would you like me to help create upstream issues for these formatting improvements in the spf13/cobra repository?
🧰 Tools
🪛 Markdownlint (0.35.0)
34-34: Expected: h4; Actual: h6
Heading levels should only increment by one level at a time(MD001, heading-increment)
🪛 LanguageTool
[uncategorized] ~34-~34: It appears that a hyphen is missing (if ‘auto’ is not used in the context of ‘cars’).
Context: ....md) - Export Longhorn replica ###### Auto generated by spf13/cobra on 27-Nov-2024(AUTO_HYPHEN)
docs/longhornctl_trim_volume.md (1)
45-45: Fix markdown heading level and hyphenation
- The heading level jumps from h1 to h6 which violates markdown best practices
- "Auto generated" should be hyphenated as "Auto-generated"
Apply this diff to fix both issues:
-###### Auto generated by spf13/cobra on 27-Nov-2024 +#### Auto-generated by spf13/cobra on 27-Nov-2024🧰 Tools
🪛 Markdownlint (0.35.0)
45-45: Expected: h4; Actual: h6
Heading levels should only increment by one level at a time(MD001, heading-increment)
🪛 LanguageTool
[uncategorized] ~45-~45: It appears that a hyphen is missing (if ‘auto’ is not used in the context of ‘cars’).
Context: ... - Longhorn trimming operations ###### Auto generated by spf13/cobra on 27-Nov-2024(AUTO_HYPHEN)
docs/longhornctl_check_preflight.md (2)
60-60: Fix markdown formatting issues.Please address the following formatting issues:
- Add a hyphen to "Auto-generated" as it's a compound adjective
- Use the correct heading level (h4) instead of h6 for consistency
-###### Auto generated by spf13/cobra on 27-Nov-2024 +#### Auto-generated by spf13/cobra on 27-Nov-2024🧰 Tools
🪛 LanguageTool
[uncategorized] ~60-~60: It appears that a hyphen is missing (if ‘auto’ is not used in the context of ‘cars’).
Context: ... - Longhorn checking operations ###### Auto generated by spf13/cobra on 27-Nov-2024(AUTO_HYPHEN)
🪛 Markdownlint (0.35.0)
60-60: Expected: h4; Actual: h6
Heading levels should only increment by one level at a time(MD001, heading-increment)
Line range hint
1-60: Documentation needs to be updated to reflect the new NFS version checking behavior.The documentation should be updated to reflect the enhanced NFS version checking that considers the system's default NFS version from
/etc/nfsmount.conf. This will help users understand:
- How the NFS version check works
- What configuration settings are checked
- How to interpret the preflight check results
The example output should also be updated to show the more detailed NFS version check results.
Would you like me to help draft the updated documentation that includes these details?
🧰 Tools
🪛 LanguageTool
[uncategorized] ~60-~60: It appears that a hyphen is missing (if ‘auto’ is not used in the context of ‘cars’).
Context: ... - Longhorn checking operations ###### Auto generated by spf13/cobra on 27-Nov-2024(AUTO_HYPHEN)
🪛 Markdownlint (0.35.0)
60-60: Expected: h4; Actual: h6
Heading levels should only increment by one level at a time(MD001, heading-increment)
58-58: Column: 44
Hard tabs(MD010, no-hard-tabs)
docs/longhornctl_install_preflight.md (1)
Documentation update needed for NFS version configuration
The preflight documentation should be updated to include NFS version configuration details since:
- The code checks
/etc/nfsmount.conffor NFS mount settings- Current documentation only shows "NFS4 is supported" in the output but lacks configuration guidance
- No other documentation files contain information about NFS version configuration
Suggested additions to
docs/longhornctl_install_preflight.md:
- Add a note about NFS version configuration under "Synopsis"
- Include information about
/etc/nfsmount.confin the documentation- Explain that NFS4 is the default version when the config file is absent
🔗 Analysis chain
Line range hint
1-48: Consider adding NFS version check documentation.Given that this PR introduces changes to NFS version checking functionality, consider adding documentation about:
- The importance of checking
/etc/nfsmount.conf- How to verify and configure the correct NFS version
- The impact on backup operations
This aligns with the PR objective to "Update the official documentation to reflect the need to check the
/etc/nfsmount.conffile".Let's check if NFS-related documentation exists elsewhere:
Let me check the content of the found files to better understand the NFS documentation coverage:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Search for NFS-related documentation # Expected: Documentation about NFS version checks and configuration # Search for NFS-related markdown files echo "Searching for NFS documentation files..." fd -e md | xargs rg -l 'nfs|NFS' # Search for specific mentions of nfsmount.conf echo "Searching for nfsmount.conf mentions..." rg '/etc/nfsmount.conf'Length of output: 349
Script:
#!/bin/bash # Check content of NFS-related documentation and code echo "Content of longhornctl_check_preflight.md:" cat docs/longhornctl_check_preflight.md echo -e "\nContent of relevant code section:" rg -A 10 -B 10 '/etc/nfsmount.conf' pkg/local/preflight/checker.goLength of output: 2990
🧰 Tools
🪛 Markdownlint (0.35.0)
48-48: Expected: h4; Actual: h6
Heading levels should only increment by one level at a time(MD001, heading-increment)
45-45: Column: 48
Hard tabs(MD010, no-hard-tabs)
46-46: Column: 78
Hard tabs(MD010, no-hard-tabs)
🪛 LanguageTool
[uncategorized] ~48-~48: It appears that a hyphen is missing (if ‘auto’ is not used in the context of ‘cars’).
Context: ...op Longhorn preflight installer ###### Auto generated by spf13/cobra on 27-Nov-2024(AUTO_HYPHEN)
docs/longhornctl_export_replica.md (1)
65-65: Fix documentation formatting issues.Please address the following minor documentation issues:
- The heading level should be h4 (####) instead of h6 (######)
- "Auto generated" should be hyphenated as "Auto-generated"
Apply this diff:
-###### Auto generated by spf13/cobra on 27-Nov-2024 +#### Auto-generated by spf13/cobra on 27-Nov-2024🧰 Tools
🪛 Markdownlint (0.35.0)
65-65: Expected: h4; Actual: h6
Heading levels should only increment by one level at a time(MD001, heading-increment)
🪛 LanguageTool
[uncategorized] ~65-~65: It appears that a hyphen is missing (if ‘auto’ is not used in the context of ‘cars’).
Context: ...Stop exporting Longhorn replica ###### Auto generated by spf13/cobra on 27-Nov-2024(AUTO_HYPHEN)
pkg/local/preflight/checker.go (2)
440-464: Consider enhancing error handling and logging for module checks.While the logic is correct, the error handling for module checks could be improved to provide better diagnostics.
Apply this change to improve error handling:
moduleLoaded, err := utils.IsModuleLoaded(option[0]) if err != nil { + local.collection.Log.Warn = append(local.collection.Log.Warn, + fmt.Sprintf("Failed to check if module %s is loaded: %v", option[0], err)) continue }
473-517: Enhance config parsing robustness.While the version validation is thorough, the section parsing could be more robust:
Apply these changes:
- if key, val, isParsable := strings.Cut(line, "="); isParsable { + key, val, isParsable := strings.Cut(line, "=") + if isParsable { + key = strings.TrimSpace(key) + val = strings.TrimSpace(val) switch key { case "Defaultvers": verParts := strings.SplitN(val, ".", 2)
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (17)
docs/longhornctl.md(1 hunks)docs/longhornctl_check.md(1 hunks)docs/longhornctl_check_preflight.md(1 hunks)docs/longhornctl_doc.md(1 hunks)docs/longhornctl_export.md(1 hunks)docs/longhornctl_export_replica.md(1 hunks)docs/longhornctl_export_replica_stop.md(1 hunks)docs/longhornctl_get.md(1 hunks)docs/longhornctl_get_replica.md(1 hunks)docs/longhornctl_global-options.md(1 hunks)docs/longhornctl_install.md(1 hunks)docs/longhornctl_install_preflight.md(1 hunks)docs/longhornctl_install_preflight_stop.md(1 hunks)docs/longhornctl_trim.md(1 hunks)docs/longhornctl_trim_volume.md(1 hunks)docs/longhornctl_version.md(1 hunks)pkg/local/preflight/checker.go(1 hunks)
🧰 Additional context used
📓 Learnings (1)
pkg/local/preflight/checker.go (1)
Learnt from: COLDTURNIP
PR: longhorn/cli#129
File: pkg/local/preflight/checker.go:421-423
Timestamp: 2024-11-26T10:35:55.842Z
Learning: In `/etc/nfsmount.conf`, the `Defaultvers` parameter is set to `4` by default.
🪛 LanguageTool
docs/longhornctl.md
[uncategorized] ~29-~29: It appears that a hyphen is missing (if ‘auto’ is not used in the context of ‘cars’).
Context: ...d) - Print longhornctl version ###### Auto generated by spf13/cobra on 27-Nov-2024
(AUTO_HYPHEN)
docs/longhornctl_check.md
[uncategorized] ~19-~19: It appears that a hyphen is missing (if ‘auto’ is not used in the context of ‘cars’).
Context: ...md) - Check Longhorn preflight ###### Auto generated by spf13/cobra on 27-Nov-2024
(AUTO_HYPHEN)
docs/longhornctl_check_preflight.md
[uncategorized] ~60-~60: It appears that a hyphen is missing (if ‘auto’ is not used in the context of ‘cars’).
Context: ... - Longhorn checking operations ###### Auto generated by spf13/cobra on 27-Nov-2024
(AUTO_HYPHEN)
docs/longhornctl_doc.md
[uncategorized] ~31-~31: It appears that a hyphen is missing (if ‘auto’ is not used in the context of ‘cars’).
Context: ...Longhorn commandline interface. ###### Auto generated by spf13/cobra on 27-Nov-2024
(AUTO_HYPHEN)
docs/longhornctl_export.md
[uncategorized] ~19-~19: It appears that a hyphen is missing (if ‘auto’ is not used in the context of ‘cars’).
Context: ....md) - Export Longhorn replica ###### Auto generated by spf13/cobra on 27-Nov-2024
(AUTO_HYPHEN)
docs/longhornctl_export_replica.md
[uncategorized] ~65-~65: It appears that a hyphen is missing (if ‘auto’ is not used in the context of ‘cars’).
Context: ...Stop exporting Longhorn replica ###### Auto generated by spf13/cobra on 27-Nov-2024
(AUTO_HYPHEN)
docs/longhornctl_export_replica_stop.md
[uncategorized] ~34-~34: It appears that a hyphen is missing (if ‘auto’ is not used in the context of ‘cars’).
Context: ....md) - Export Longhorn replica ###### Auto generated by spf13/cobra on 27-Nov-2024
(AUTO_HYPHEN)
docs/longhornctl_get.md
[uncategorized] ~19-~19: It appears that a hyphen is missing (if ‘auto’ is not used in the context of ‘cars’).
Context: ...et Longhorn replica information ###### Auto generated by spf13/cobra on 27-Nov-2024
(AUTO_HYPHEN)
docs/longhornctl_get_replica.md
[uncategorized] ~63-~63: It appears that a hyphen is missing (if ‘auto’ is not used in the context of ‘cars’).
Context: ...nformation gathering operations ###### Auto generated by spf13/cobra on 27-Nov-2024
(AUTO_HYPHEN)
docs/longhornctl_global-options.md
[uncategorized] ~31-~31: It appears that a hyphen is missing (if ‘auto’ is not used in the context of ‘cars’).
Context: ...Longhorn commandline interface. ###### Auto generated by spf13/cobra on 27-Nov-2024
(AUTO_HYPHEN)
docs/longhornctl_install.md
[uncategorized] ~19-~19: It appears that a hyphen is missing (if ‘auto’ is not used in the context of ‘cars’).
Context: ...) - Install Longhorn preflight ###### Auto generated by spf13/cobra on 27-Nov-2024
(AUTO_HYPHEN)
docs/longhornctl_install_preflight.md
[uncategorized] ~48-~48: It appears that a hyphen is missing (if ‘auto’ is not used in the context of ‘cars’).
Context: ...op Longhorn preflight installer ###### Auto generated by spf13/cobra on 27-Nov-2024
(AUTO_HYPHEN)
docs/longhornctl_install_preflight_stop.md
[uncategorized] ~35-~35: It appears that a hyphen is missing (if ‘auto’ is not used in the context of ‘cars’).
Context: ...) - Install Longhorn preflight ###### Auto generated by spf13/cobra on 27-Nov-2024
(AUTO_HYPHEN)
docs/longhornctl_trim.md
[uncategorized] ~19-~19: It appears that a hyphen is missing (if ‘auto’ is not used in the context of ‘cars’).
Context: ...e.md) - Trim a Longhorn volume ###### Auto generated by spf13/cobra on 27-Nov-2024
(AUTO_HYPHEN)
docs/longhornctl_trim_volume.md
[uncategorized] ~45-~45: It appears that a hyphen is missing (if ‘auto’ is not used in the context of ‘cars’).
Context: ... - Longhorn trimming operations ###### Auto generated by spf13/cobra on 27-Nov-2024
(AUTO_HYPHEN)
docs/longhornctl_version.md
[uncategorized] ~27-~27: It appears that a hyphen is missing (if ‘auto’ is not used in the context of ‘cars’).
Context: ...Longhorn commandline interface. ###### Auto generated by spf13/cobra on 27-Nov-2024
(AUTO_HYPHEN)
🪛 Markdownlint (0.35.0)
docs/longhornctl.md
29-29: Expected: h4; Actual: h6
Heading levels should only increment by one level at a time
(MD001, heading-increment)
docs/longhornctl_check.md
19-19: Expected: h4; Actual: h6
Heading levels should only increment by one level at a time
(MD001, heading-increment)
docs/longhornctl_check_preflight.md
60-60: Expected: h4; Actual: h6
Heading levels should only increment by one level at a time
(MD001, heading-increment)
docs/longhornctl_doc.md
31-31: Expected: h4; Actual: h6
Heading levels should only increment by one level at a time
(MD001, heading-increment)
docs/longhornctl_export.md
19-19: Expected: h4; Actual: h6
Heading levels should only increment by one level at a time
(MD001, heading-increment)
docs/longhornctl_export_replica.md
65-65: Expected: h4; Actual: h6
Heading levels should only increment by one level at a time
(MD001, heading-increment)
docs/longhornctl_export_replica_stop.md
34-34: Expected: h4; Actual: h6
Heading levels should only increment by one level at a time
(MD001, heading-increment)
docs/longhornctl_get.md
19-19: Expected: h4; Actual: h6
Heading levels should only increment by one level at a time
(MD001, heading-increment)
docs/longhornctl_get_replica.md
63-63: Expected: h4; Actual: h6
Heading levels should only increment by one level at a time
(MD001, heading-increment)
docs/longhornctl_global-options.md
31-31: Expected: h4; Actual: h6
Heading levels should only increment by one level at a time
(MD001, heading-increment)
docs/longhornctl_install.md
19-19: Expected: h4; Actual: h6
Heading levels should only increment by one level at a time
(MD001, heading-increment)
docs/longhornctl_install_preflight.md
48-48: Expected: h4; Actual: h6
Heading levels should only increment by one level at a time
(MD001, heading-increment)
docs/longhornctl_install_preflight_stop.md
35-35: Expected: h4; Actual: h6
Heading levels should only increment by one level at a time
(MD001, heading-increment)
docs/longhornctl_trim.md
19-19: Expected: h4; Actual: h6
Heading levels should only increment by one level at a time
(MD001, heading-increment)
docs/longhornctl_trim_volume.md
45-45: Expected: h4; Actual: h6
Heading levels should only increment by one level at a time
(MD001, heading-increment)
docs/longhornctl_version.md
27-27: Expected: h4; Actual: h6
Heading levels should only increment by one level at a time
(MD001, heading-increment)
🔇 Additional comments (15)
docs/longhornctl_trim.md (1)
19-19:
This change appears unrelated to the PR objectives
This documentation update only changes an auto-generated timestamp and has no relation to the PR's objective of improving NFS version checking functionality. Consider excluding this auto-generated change from the PR to maintain a focused scope.
Let's verify if this is truly an auto-generated change:
🧰 Tools
🪛 Markdownlint (0.35.0)
19-19: Expected: h4; Actual: h6
Heading levels should only increment by one level at a time
(MD001, heading-increment)
🪛 LanguageTool
[uncategorized] ~19-~19: It appears that a hyphen is missing (if ‘auto’ is not used in the context of ‘cars’).
Context: ...e.md) - Trim a Longhorn volume ###### Auto generated by spf13/cobra on 27-Nov-2024
(AUTO_HYPHEN)
docs/longhornctl_export.md (1)
19-19: LGTM: Auto-generated timestamp update
The timestamp update is an expected change from the documentation auto-generation process.
🧰 Tools
🪛 LanguageTool
[uncategorized] ~19-~19: It appears that a hyphen is missing (if ‘auto’ is not used in the context of ‘cars’).
Context: ....md) - Export Longhorn replica ###### Auto generated by spf13/cobra on 27-Nov-2024
(AUTO_HYPHEN)
🪛 Markdownlint (0.35.0)
19-19: Expected: h4; Actual: h6
Heading levels should only increment by one level at a time
(MD001, heading-increment)
docs/longhornctl_check.md (1)
19-19: No review needed for auto-generated changes.
This line is auto-generated by spf13/cobra. While there are minor formatting issues (missing hyphen in "Auto-generated" and incorrect heading level hierarchy), these should be reported upstream to spf13/cobra rather than modified directly in this file.
🧰 Tools
🪛 LanguageTool
[uncategorized] ~19-~19: It appears that a hyphen is missing (if ‘auto’ is not used in the context of ‘cars’).
Context: ...md) - Check Longhorn preflight ###### Auto generated by spf13/cobra on 27-Nov-2024
(AUTO_HYPHEN)
🪛 Markdownlint (0.35.0)
19-19: Expected: h4; Actual: h6
Heading levels should only increment by one level at a time
(MD001, heading-increment)
docs/longhornctl_install.md (1)
Line range hint 1-19: Documentation update needed for NFS version checking
Based on the PR objectives, this documentation should be updated to include information about the NFS version checking functionality, specifically regarding the /etc/nfsmount.conf configuration.
Let's check if there are any existing NFS-related sections in the documentation:
Would you like me to help draft the documentation updates to include:
- Information about NFS version checking
- The importance of
/etc/nfsmount.confconfiguration - How to verify and configure the correct NFS version?
🧰 Tools
🪛 Markdownlint (0.35.0)
19-19: Expected: h4; Actual: h6
Heading levels should only increment by one level at a time
(MD001, heading-increment)
16-16: Column: 32
Hard tabs
(MD010, no-hard-tabs)
17-17: Column: 68
Hard tabs
(MD010, no-hard-tabs)
🪛 LanguageTool
[uncategorized] ~19-~19: It appears that a hyphen is missing (if ‘auto’ is not used in the context of ‘cars’).
Context: ...) - Install Longhorn preflight ###### Auto generated by spf13/cobra on 27-Nov-2024
(AUTO_HYPHEN)
docs/longhornctl_version.md (1)
27-27: Auto-generated changes look good
The updated timestamp is an auto-generated change and doesn't require further review.
🧰 Tools
🪛 Markdownlint (0.35.0)
27-27: Expected: h4; Actual: h6
Heading levels should only increment by one level at a time
(MD001, heading-increment)
🪛 LanguageTool
[uncategorized] ~27-~27: It appears that a hyphen is missing (if ‘auto’ is not used in the context of ‘cars’).
Context: ...Longhorn commandline interface. ###### Auto generated by spf13/cobra on 27-Nov-2024
(AUTO_HYPHEN)
docs/longhornctl_export_replica_stop.md (1)
34-34: LGTM! Documentation timestamp update is as expected.
The updated timestamp reflects the recent documentation regeneration.
🧰 Tools
🪛 Markdownlint (0.35.0)
34-34: Expected: h4; Actual: h6
Heading levels should only increment by one level at a time
(MD001, heading-increment)
🪛 LanguageTool
[uncategorized] ~34-~34: It appears that a hyphen is missing (if ‘auto’ is not used in the context of ‘cars’).
Context: ....md) - Export Longhorn replica ###### Auto generated by spf13/cobra on 27-Nov-2024
(AUTO_HYPHEN)
docs/longhornctl_install_preflight_stop.md (1)
35-35: Documentation changes look good!
The timestamp update is an auto-generated change and doesn't affect the functionality.
🧰 Tools
🪛 Markdownlint (0.35.0)
35-35: Expected: h4; Actual: h6
Heading levels should only increment by one level at a time
(MD001, heading-increment)
🪛 LanguageTool
[uncategorized] ~35-~35: It appears that a hyphen is missing (if ‘auto’ is not used in the context of ‘cars’).
Context: ...) - Install Longhorn preflight ###### Auto generated by spf13/cobra on 27-Nov-2024
(AUTO_HYPHEN)
docs/longhornctl.md (1)
29-29: Consider improving the auto-generated section formatting
The auto-generated section still has formatting inconsistencies that were previously identified:
- "Auto-generated" should be hyphenated
- The heading level (h6) doesn't follow the proper hierarchy
As previously suggested, these formatting issues can be fixed by setting DisableAutoGenTag in the cobra command configuration. This will allow either removing the auto-generated line completely or implementing a consistent format.
🧰 Tools
🪛 LanguageTool
[uncategorized] ~29-~29: It appears that a hyphen is missing (if ‘auto’ is not used in the context of ‘cars’).
Context: ...d) - Print longhornctl version ###### Auto generated by spf13/cobra on 27-Nov-2024
(AUTO_HYPHEN)
🪛 Markdownlint (0.35.0)
29-29: Expected: h4; Actual: h6
Heading levels should only increment by one level at a time
(MD001, heading-increment)
docs/longhornctl_check_preflight.md (1)
60-60: LGTM on the timestamp update!
The auto-generated timestamp has been updated correctly.
🧰 Tools
🪛 LanguageTool
[uncategorized] ~60-~60: It appears that a hyphen is missing (if ‘auto’ is not used in the context of ‘cars’).
Context: ... - Longhorn checking operations ###### Auto generated by spf13/cobra on 27-Nov-2024
(AUTO_HYPHEN)
🪛 Markdownlint (0.35.0)
60-60: Expected: h4; Actual: h6
Heading levels should only increment by one level at a time
(MD001, heading-increment)
docs/longhornctl_install_preflight.md (1)
48-48: LGTM!
The auto-generated timestamp update is expected and correct.
🧰 Tools
🪛 Markdownlint (0.35.0)
48-48: Expected: h4; Actual: h6
Heading levels should only increment by one level at a time
(MD001, heading-increment)
🪛 LanguageTool
[uncategorized] ~48-~48: It appears that a hyphen is missing (if ‘auto’ is not used in the context of ‘cars’).
Context: ...op Longhorn preflight installer ###### Auto generated by spf13/cobra on 27-Nov-2024
(AUTO_HYPHEN)
docs/longhornctl_get_replica.md (1)
63-63: LGTM! Auto-generated timestamp update.
The timestamp update is consistent with the PR's changes.
🧰 Tools
🪛 Markdownlint (0.35.0)
63-63: Expected: h4; Actual: h6
Heading levels should only increment by one level at a time
(MD001, heading-increment)
🪛 LanguageTool
[uncategorized] ~63-~63: It appears that a hyphen is missing (if ‘auto’ is not used in the context of ‘cars’).
Context: ...nformation gathering operations ###### Auto generated by spf13/cobra on 27-Nov-2024
(AUTO_HYPHEN)
docs/longhornctl_export_replica.md (1)
65-65: LGTM on the timestamp update.
The auto-generated timestamp has been updated correctly.
🧰 Tools
🪛 Markdownlint (0.35.0)
65-65: Expected: h4; Actual: h6
Heading levels should only increment by one level at a time
(MD001, heading-increment)
🪛 LanguageTool
[uncategorized] ~65-~65: It appears that a hyphen is missing (if ‘auto’ is not used in the context of ‘cars’).
Context: ...Stop exporting Longhorn replica ###### Auto generated by spf13/cobra on 27-Nov-2024
(AUTO_HYPHEN)
pkg/local/preflight/checker.go (3)
421-438: LGTM! Clean initialization of variables and proper file handling.
The code properly initializes the boolean flags and handles the kernel config file path construction correctly.
465-471: LGTM! Proper error handling and early return pattern.
The code correctly handles scanner errors and provides appropriate feedback when NFSv4 is not supported.
519-521: LGTM! Clear success reporting.
The code correctly reports success only when all checks pass, providing clear feedback to users.
| @@ -418,51 +418,106 @@ func (local *Checker) checkModulesLoaded(spdkDependent bool) error { | |||
| func (local *Checker) checkNFSv4Support() error { | |||
There was a problem hiding this comment.
I think we could remove the error return and instead log all errors to the collection logs. This approach could allow the checker to continue without blocking other checkups.
We should probably have a separate issue to apply this approach to other checkups as well. cc @derekbit
|
@COLDTURNIP if this is ready to review, re-request @c3y1huang to check this PR. |
|
We're extracting some common logic between cli and LH manager. Please hold on for longhorn/go-common-libs#88 . |
COLDTURNIP
force-pushed
the
9830-check_nfs_version_in_nfsmount
branch
3 times, most recently
from
af20e9f to
0b77497
Compare
derekbit
force-pushed
the
9830-check_nfs_version_in_nfsmount
branch
from
0b77497 to
8220485
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (6)
docs/longhornctl_doc.md (1)
31-31: Fix documentation formatting issues.The auto-generated footer has formatting issues:
- The heading level jumps from h2 to h6 (should be h4).
- "Auto generated" should be hyphenated as "Auto-generated".
-###### Auto generated by spf13/cobra on 27-Nov-2024 +#### Auto-generated by spf13/cobra on 27-Nov-2024🧰 Tools
🪛 LanguageTool
[uncategorized] ~31-~31: It appears that a hyphen is missing (if ‘auto’ is not used in the context of ‘cars’).
Context: ...Longhorn commandline interface. ###### Auto generated by spf13/cobra on 27-Nov-2024(AUTO_HYPHEN)
🪛 Markdownlint (0.35.0)
31-31: Expected: h4; Actual: h6
Heading levels should only increment by one level at a time(MD001, heading-increment)
docs/longhornctl_global-options.md (1)
31-31: Fix documentation formatting issues.The auto-generated footer has formatting issues:
- The heading level jumps from h2 to h6 (should be h4).
- "Auto generated" should be hyphenated as "Auto-generated".
-###### Auto generated by spf13/cobra on 27-Nov-2024 +#### Auto-generated by spf13/cobra on 27-Nov-2024🧰 Tools
🪛 LanguageTool
[uncategorized] ~31-~31: It appears that a hyphen is missing (if ‘auto’ is not used in the context of ‘cars’).
Context: ...Longhorn commandline interface. ###### Auto generated by spf13/cobra on 27-Nov-2024(AUTO_HYPHEN)
🪛 Markdownlint (0.35.0)
31-31: Expected: h4; Actual: h6
Heading levels should only increment by one level at a time(MD001, heading-increment)
docs/longhornctl_export_replica_stop.md (1)
34-34: Fix documentation formatting issues.The auto-generated footer has formatting issues:
- The heading level jumps from h2 to h6 (should be h4).
- "Auto generated" should be hyphenated as "Auto-generated".
-###### Auto generated by spf13/cobra on 27-Nov-2024 +#### Auto-generated by spf13/cobra on 27-Nov-2024🧰 Tools
🪛 LanguageTool
[uncategorized] ~34-~34: It appears that a hyphen is missing (if ‘auto’ is not used in the context of ‘cars’).
Context: ....md) - Export Longhorn replica ###### Auto generated by spf13/cobra on 27-Nov-2024(AUTO_HYPHEN)
🪛 Markdownlint (0.35.0)
34-34: Expected: h4; Actual: h6
Heading levels should only increment by one level at a time(MD001, heading-increment)
docs/longhornctl_export_replica.md (1)
65-65: Fix documentation formatting issues.
- Add a hyphen between "Auto" and "generated".
- Use h4 (####) instead of h6 (######) for consistent heading hierarchy.
-###### Auto generated by spf13/cobra on 27-Nov-2024 +#### Auto-generated by spf13/cobra on 27-Nov-2024🧰 Tools
🪛 LanguageTool
[uncategorized] ~65-~65: It appears that a hyphen is missing (if ‘auto’ is not used in the context of ‘cars’).
Context: ...Stop exporting Longhorn replica ###### Auto generated by spf13/cobra on 27-Nov-2024(AUTO_HYPHEN)
🪛 Markdownlint (0.35.0)
65-65: Expected: h4; Actual: h6
Heading levels should only increment by one level at a time(MD001, heading-increment)
pkg/local/preflight/checker.go (1)
423-457: Improve error handling for module loading check.The error from
IsModuleLoadedis silently ignored. Consider logging it to help with diagnostics.moduleLoaded, err := utils.IsModuleLoaded(module) if err != nil { + local.collection.Log.Warn = append(local.collection.Log.Warn, + fmt.Sprintf("Failed to check if module %s is loaded: %v", module, err)) continue }docs/longhornctl_trim.md (1)
19-19: Consider improving the auto-generated documentation format.The auto-generated documentation has some formatting inconsistencies:
- "Auto generated" should be hyphenated as "Auto-generated"
- The heading level (h6) doesn't follow the proper markdown hierarchy (should be h4)
Since these are auto-generated by cobra, consider:
- Opening an upstream issue with spf13/cobra
- Adding a post-processing step to fix these formatting issues
Would you like me to help create an issue for spf13/cobra or implement a post-processing script?
🧰 Tools
🪛 LanguageTool
[uncategorized] ~19-~19: It appears that a hyphen is missing (if ‘auto’ is not used in the context of ‘cars’).
Context: ...e.md) - Trim a Longhorn volume ###### Auto generated by spf13/cobra on 27-Nov-2024(AUTO_HYPHEN)
🪛 Markdownlint (0.35.0)
19-19: Expected: h4; Actual: h6
Heading levels should only increment by one level at a time(MD001, heading-increment)
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
⛔ Files ignored due to path filters (2)
vendor/github.com/longhorn/go-common-libs/nfs/nfs.gois excluded by!vendor/**vendor/modules.txtis excluded by!vendor/**
📒 Files selected for processing (17)
docs/longhornctl.md(1 hunks)docs/longhornctl_check.md(1 hunks)docs/longhornctl_check_preflight.md(1 hunks)docs/longhornctl_doc.md(1 hunks)docs/longhornctl_export.md(1 hunks)docs/longhornctl_export_replica.md(1 hunks)docs/longhornctl_export_replica_stop.md(1 hunks)docs/longhornctl_get.md(1 hunks)docs/longhornctl_get_replica.md(1 hunks)docs/longhornctl_global-options.md(1 hunks)docs/longhornctl_install.md(1 hunks)docs/longhornctl_install_preflight.md(1 hunks)docs/longhornctl_install_preflight_stop.md(1 hunks)docs/longhornctl_trim.md(1 hunks)docs/longhornctl_trim_volume.md(1 hunks)docs/longhornctl_version.md(1 hunks)pkg/local/preflight/checker.go(2 hunks)
🧰 Additional context used
🪛 LanguageTool
docs/longhornctl.md
[uncategorized] ~29-~29: It appears that a hyphen is missing (if ‘auto’ is not used in the context of ‘cars’).
Context: ...d) - Print longhornctl version ###### Auto generated by spf13/cobra on 27-Nov-2024
(AUTO_HYPHEN)
docs/longhornctl_check.md
[uncategorized] ~19-~19: It appears that a hyphen is missing (if ‘auto’ is not used in the context of ‘cars’).
Context: ...md) - Check Longhorn preflight ###### Auto generated by spf13/cobra on 27-Nov-2024
(AUTO_HYPHEN)
docs/longhornctl_check_preflight.md
[uncategorized] ~60-~60: It appears that a hyphen is missing (if ‘auto’ is not used in the context of ‘cars’).
Context: ... - Longhorn checking operations ###### Auto generated by spf13/cobra on 27-Nov-2024
(AUTO_HYPHEN)
docs/longhornctl_doc.md
[uncategorized] ~31-~31: It appears that a hyphen is missing (if ‘auto’ is not used in the context of ‘cars’).
Context: ...Longhorn commandline interface. ###### Auto generated by spf13/cobra on 27-Nov-2024
(AUTO_HYPHEN)
docs/longhornctl_export.md
[uncategorized] ~19-~19: It appears that a hyphen is missing (if ‘auto’ is not used in the context of ‘cars’).
Context: ....md) - Export Longhorn replica ###### Auto generated by spf13/cobra on 27-Nov-2024
(AUTO_HYPHEN)
docs/longhornctl_export_replica.md
[uncategorized] ~65-~65: It appears that a hyphen is missing (if ‘auto’ is not used in the context of ‘cars’).
Context: ...Stop exporting Longhorn replica ###### Auto generated by spf13/cobra on 27-Nov-2024
(AUTO_HYPHEN)
docs/longhornctl_export_replica_stop.md
[uncategorized] ~34-~34: It appears that a hyphen is missing (if ‘auto’ is not used in the context of ‘cars’).
Context: ....md) - Export Longhorn replica ###### Auto generated by spf13/cobra on 27-Nov-2024
(AUTO_HYPHEN)
docs/longhornctl_get.md
[uncategorized] ~19-~19: It appears that a hyphen is missing (if ‘auto’ is not used in the context of ‘cars’).
Context: ...et Longhorn replica information ###### Auto generated by spf13/cobra on 27-Nov-2024
(AUTO_HYPHEN)
docs/longhornctl_get_replica.md
[uncategorized] ~63-~63: It appears that a hyphen is missing (if ‘auto’ is not used in the context of ‘cars’).
Context: ...nformation gathering operations ###### Auto generated by spf13/cobra on 27-Nov-2024
(AUTO_HYPHEN)
docs/longhornctl_global-options.md
[uncategorized] ~31-~31: It appears that a hyphen is missing (if ‘auto’ is not used in the context of ‘cars’).
Context: ...Longhorn commandline interface. ###### Auto generated by spf13/cobra on 27-Nov-2024
(AUTO_HYPHEN)
docs/longhornctl_install.md
[uncategorized] ~19-~19: It appears that a hyphen is missing (if ‘auto’ is not used in the context of ‘cars’).
Context: ...) - Install Longhorn preflight ###### Auto generated by spf13/cobra on 27-Nov-2024
(AUTO_HYPHEN)
docs/longhornctl_install_preflight.md
[uncategorized] ~48-~48: It appears that a hyphen is missing (if ‘auto’ is not used in the context of ‘cars’).
Context: ...op Longhorn preflight installer ###### Auto generated by spf13/cobra on 27-Nov-2024
(AUTO_HYPHEN)
docs/longhornctl_install_preflight_stop.md
[uncategorized] ~35-~35: It appears that a hyphen is missing (if ‘auto’ is not used in the context of ‘cars’).
Context: ...) - Install Longhorn preflight ###### Auto generated by spf13/cobra on 27-Nov-2024
(AUTO_HYPHEN)
docs/longhornctl_trim.md
[uncategorized] ~19-~19: It appears that a hyphen is missing (if ‘auto’ is not used in the context of ‘cars’).
Context: ...e.md) - Trim a Longhorn volume ###### Auto generated by spf13/cobra on 27-Nov-2024
(AUTO_HYPHEN)
docs/longhornctl_trim_volume.md
[uncategorized] ~45-~45: It appears that a hyphen is missing (if ‘auto’ is not used in the context of ‘cars’).
Context: ... - Longhorn trimming operations ###### Auto generated by spf13/cobra on 27-Nov-2024
(AUTO_HYPHEN)
docs/longhornctl_version.md
[uncategorized] ~27-~27: It appears that a hyphen is missing (if ‘auto’ is not used in the context of ‘cars’).
Context: ...Longhorn commandline interface. ###### Auto generated by spf13/cobra on 27-Nov-2024
(AUTO_HYPHEN)
🪛 Markdownlint (0.35.0)
docs/longhornctl.md
29-29: Expected: h4; Actual: h6
Heading levels should only increment by one level at a time
(MD001, heading-increment)
docs/longhornctl_check.md
19-19: Expected: h4; Actual: h6
Heading levels should only increment by one level at a time
(MD001, heading-increment)
docs/longhornctl_check_preflight.md
60-60: Expected: h4; Actual: h6
Heading levels should only increment by one level at a time
(MD001, heading-increment)
docs/longhornctl_doc.md
31-31: Expected: h4; Actual: h6
Heading levels should only increment by one level at a time
(MD001, heading-increment)
docs/longhornctl_export.md
19-19: Expected: h4; Actual: h6
Heading levels should only increment by one level at a time
(MD001, heading-increment)
docs/longhornctl_export_replica.md
65-65: Expected: h4; Actual: h6
Heading levels should only increment by one level at a time
(MD001, heading-increment)
docs/longhornctl_export_replica_stop.md
34-34: Expected: h4; Actual: h6
Heading levels should only increment by one level at a time
(MD001, heading-increment)
docs/longhornctl_get.md
19-19: Expected: h4; Actual: h6
Heading levels should only increment by one level at a time
(MD001, heading-increment)
docs/longhornctl_get_replica.md
63-63: Expected: h4; Actual: h6
Heading levels should only increment by one level at a time
(MD001, heading-increment)
docs/longhornctl_global-options.md
31-31: Expected: h4; Actual: h6
Heading levels should only increment by one level at a time
(MD001, heading-increment)
docs/longhornctl_install.md
19-19: Expected: h4; Actual: h6
Heading levels should only increment by one level at a time
(MD001, heading-increment)
docs/longhornctl_install_preflight.md
48-48: Expected: h4; Actual: h6
Heading levels should only increment by one level at a time
(MD001, heading-increment)
docs/longhornctl_install_preflight_stop.md
35-35: Expected: h4; Actual: h6
Heading levels should only increment by one level at a time
(MD001, heading-increment)
docs/longhornctl_trim.md
19-19: Expected: h4; Actual: h6
Heading levels should only increment by one level at a time
(MD001, heading-increment)
docs/longhornctl_trim_volume.md
45-45: Expected: h4; Actual: h6
Heading levels should only increment by one level at a time
(MD001, heading-increment)
docs/longhornctl_version.md
27-27: Expected: h4; Actual: h6
Heading levels should only increment by one level at a time
(MD001, heading-increment)
🔇 Additional comments (12)
docs/longhornctl_check.md (1)
19-19: Skip review of auto-generated changes and consider upstream improvements.
The timestamp updates across these documentation files are automatically generated by spf13/cobra and don't require review. However, there are some formatting inconsistencies in the auto-generated content that could be improved upstream:
- The text "auto generated" should be hyphenated as "auto-generated"
- The heading level jumps from h2 to h6 (######), violating markdown heading hierarchy
Consider opening issues upstream in spf13/cobra to address these formatting inconsistencies in the auto-generated content. This would improve documentation consistency across all projects using cobra's documentation generator.
🧰 Tools
🪛 LanguageTool
[uncategorized] ~19-~19: It appears that a hyphen is missing (if ‘auto’ is not used in the context of ‘cars’).
Context: ...md) - Check Longhorn preflight ###### Auto generated by spf13/cobra on 27-Nov-2024
(AUTO_HYPHEN)
🪛 Markdownlint (0.35.0)
19-19: Expected: h4; Actual: h6
Heading levels should only increment by one level at a time
(MD001, heading-increment)
docs/longhornctl_get.md (1)
19-19: Documentation timestamp update only.
This appears to be an auto-generated documentation update. However, I notice that the core changes related to NFS version checking mentioned in the PR objectives are not included in this review batch.
#!/bin/bash
# Description: Verify if there are pending changes related to NFS version checking
# Look for relevant files that might contain the NFS-related changes
rg -l "checkNFSv4Support|nfsmount.conf" 🧰 Tools
🪛 LanguageTool
[uncategorized] ~19-~19: It appears that a hyphen is missing (if ‘auto’ is not used in the context of ‘cars’).
Context: ...et Longhorn replica information ###### Auto generated by spf13/cobra on 27-Nov-2024
(AUTO_HYPHEN)
🪛 Markdownlint (0.35.0)
19-19: Expected: h4; Actual: h6
Heading levels should only increment by one level at a time
(MD001, heading-increment)
docs/longhornctl_install.md (1)
19-19: Documentation timestamp update only.
This is an auto-generated documentation update with no functional changes.
🧰 Tools
🪛 LanguageTool
[uncategorized] ~19-~19: It appears that a hyphen is missing (if ‘auto’ is not used in the context of ‘cars’).
Context: ...) - Install Longhorn preflight ###### Auto generated by spf13/cobra on 27-Nov-2024
(AUTO_HYPHEN)
🪛 Markdownlint (0.35.0)
19-19: Expected: h4; Actual: h6
Heading levels should only increment by one level at a time
(MD001, heading-increment)
docs/longhornctl_version.md (1)
27-27: Documentation timestamp update only.
This is an auto-generated documentation update. Note that the core changes related to NFS version checking mentioned in the PR objectives (checking system default NFS version in mount config) are not included in this review batch. Please ensure those changes are included in a subsequent review.
🧰 Tools
🪛 Markdownlint (0.35.0)
27-27: Expected: h4; Actual: h6
Heading levels should only increment by one level at a time
(MD001, heading-increment)
🪛 LanguageTool
[uncategorized] ~27-~27: It appears that a hyphen is missing (if ‘auto’ is not used in the context of ‘cars’).
Context: ...Longhorn commandline interface. ###### Auto generated by spf13/cobra on 27-Nov-2024
(AUTO_HYPHEN)
docs/longhornctl.md (1)
29-29: Auto-generated section formatting can be improved
The auto-generated section has formatting inconsistencies that were previously identified.
🧰 Tools
🪛 LanguageTool
[uncategorized] ~29-~29: It appears that a hyphen is missing (if ‘auto’ is not used in the context of ‘cars’).
Context: ...d) - Print longhornctl version ###### Auto generated by spf13/cobra on 27-Nov-2024
(AUTO_HYPHEN)
🪛 Markdownlint (0.35.0)
29-29: Expected: h4; Actual: h6
Heading levels should only increment by one level at a time
(MD001, heading-increment)
docs/longhornctl_check_preflight.md (1)
60-60: Documentation appears to be auto-generated
This change only updates the auto-generated timestamp, which is expected.
🧰 Tools
🪛 LanguageTool
[uncategorized] ~60-~60: It appears that a hyphen is missing (if ‘auto’ is not used in the context of ‘cars’).
Context: ... - Longhorn checking operations ###### Auto generated by spf13/cobra on 27-Nov-2024
(AUTO_HYPHEN)
🪛 Markdownlint (0.35.0)
60-60: Expected: h4; Actual: h6
Heading levels should only increment by one level at a time
(MD001, heading-increment)
docs/longhornctl_install_preflight.md (1)
48-48: Documentation appears to be auto-generated
This change only updates the auto-generated timestamp, which is expected.
🧰 Tools
🪛 LanguageTool
[uncategorized] ~48-~48: It appears that a hyphen is missing (if ‘auto’ is not used in the context of ‘cars’).
Context: ...op Longhorn preflight installer ###### Auto generated by spf13/cobra on 27-Nov-2024
(AUTO_HYPHEN)
🪛 Markdownlint (0.35.0)
48-48: Expected: h4; Actual: h6
Heading levels should only increment by one level at a time
(MD001, heading-increment)
docs/longhornctl_get_replica.md (2)
63-63: Documentation appears to be auto-generated
This change only updates the auto-generated timestamp, which is expected.
🧰 Tools
🪛 LanguageTool
[uncategorized] ~63-~63: It appears that a hyphen is missing (if ‘auto’ is not used in the context of ‘cars’).
Context: ...nformation gathering operations ###### Auto generated by spf13/cobra on 27-Nov-2024
(AUTO_HYPHEN)
🪛 Markdownlint (0.35.0)
63-63: Expected: h4; Actual: h6
Heading levels should only increment by one level at a time
(MD001, heading-increment)
Line range hint 1-63: Implementation files missing from review
Based on the PR objectives, this change should include modifications to:
- Documentation about checking
/etc/nfsmount.conf - Longhorn manager's node condition checks
- Longhorn CLI updates
However, the current set of files only shows auto-generated documentation timestamp updates. Please include the implementation files for review.
🧰 Tools
🪛 LanguageTool
[uncategorized] ~63-~63: It appears that a hyphen is missing (if ‘auto’ is not used in the context of ‘cars’).
Context: ...nformation gathering operations ###### Auto generated by spf13/cobra on 27-Nov-2024
(AUTO_HYPHEN)
🪛 Markdownlint (0.35.0)
63-63: Expected: h4; Actual: h6
Heading levels should only increment by one level at a time
(MD001, heading-increment)
61-61: Expected: dash; Actual: asterisk
Unordered list style
(MD004, ul-style)
61-61: Column: 40
Hard tabs
(MD010, no-hard-tabs)
docs/longhornctl_export_replica.md (1)
65-65: LGTM!
The auto-generated timestamp update is correct.
🧰 Tools
🪛 LanguageTool
[uncategorized] ~65-~65: It appears that a hyphen is missing (if ‘auto’ is not used in the context of ‘cars’).
Context: ...Stop exporting Longhorn replica ###### Auto generated by spf13/cobra on 27-Nov-2024
(AUTO_HYPHEN)
🪛 Markdownlint (0.35.0)
65-65: Expected: h4; Actual: h6
Heading levels should only increment by one level at a time
(MD001, heading-increment)
pkg/local/preflight/checker.go (1)
18-20: LGTM!
The new imports are correctly added for NFS and system-related functionality.
docs/longhornctl_export.md (1)
19-19: Same formatting issues as in longhornctl_trim.md
This file has the same auto-generated formatting inconsistencies as discussed in the review of longhornctl_trim.md.
🧰 Tools
🪛 LanguageTool
[uncategorized] ~19-~19: It appears that a hyphen is missing (if ‘auto’ is not used in the context of ‘cars’).
Context: ....md) - Export Longhorn replica ###### Auto generated by spf13/cobra on 27-Nov-2024
(AUTO_HYPHEN)
🪛 Markdownlint (0.35.0)
19-19: Expected: h4; Actual: h6
Heading levels should only increment by one level at a time
(MD001, heading-increment)
ref: longhorn/longhorn-9830 Signed-off-by: Raphanus Lo <[email protected]>
COLDTURNIP
force-pushed
the
9830-check_nfs_version_in_nfsmount
branch
from
8220485 to
d33a227
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Outside diff range and nitpick comments (5)
docs/longhornctl_trim.md (1)
19-19: Auto-generated timestamp update looks good.The timestamp update is consistent with other documentation files and is automatically generated by spf13/cobra.
Note: The static analysis tools flag some style issues in the auto-generated content (heading level and hyphenation). Since this is auto-generated, these should be addressed at the generator level if needed.
🧰 Tools
🪛 LanguageTool
[uncategorized] ~19-~19: It appears that a hyphen is missing (if ‘auto’ is not used in the context of ‘cars’).
Context: ...e.md) - Trim a Longhorn volume ###### Auto generated by spf13/cobra on 27-Nov-2024(AUTO_HYPHEN)
🪛 Markdownlint (0.35.0)
19-19: Expected: h4; Actual: h6
Heading levels should only increment by one level at a time(MD001, heading-increment)
docs/longhornctl_export_replica.md (1)
65-65: Minor documentation formatting improvements needed.Please update the following:
- Change the heading level from h6 (
######) to h4 (####) to maintain proper heading hierarchy- Add a hyphen to "Auto-generated" for consistency
Apply this diff:
-###### Auto generated by spf13/cobra on 27-Nov-2024 +#### Auto-generated by spf13/cobra on 27-Nov-2024🧰 Tools
🪛 LanguageTool
[uncategorized] ~65-~65: It appears that a hyphen is missing (if ‘auto’ is not used in the context of ‘cars’).
Context: ...Stop exporting Longhorn replica ###### Auto generated by spf13/cobra on 27-Nov-2024(AUTO_HYPHEN)
🪛 Markdownlint (0.35.0)
65-65: Expected: h4; Actual: h6
Heading levels should only increment by one level at a time(MD001, heading-increment)
pkg/local/preflight/checker.go (2)
423-434: Consider adding debug logging for kernel version detection.While the implementation is correct, adding debug logs would help troubleshoot issues in production environments.
kernelVersion, err := utils.GetKernelVersion() if err != nil { + logrus.Debugf("Failed to get kernel version: %v", err) return err } +logrus.Debugf("Detected kernel version: %s", kernelVersion)
459-478: Consider consolidating version checks into a helper function.The version checking logic could be extracted into a helper function to improve readability and maintainability.
+func isNFSVersionSupported(major, minor int) bool { + return major == 4 && (minor == 0 || minor == 1 || minor == 2) +} // In the main function: -isSupportedNFSVersion = nfsMajor == 4 && (nfsMinor == 0 || nfsMinor == 1 || nfsMinor == 2) +isSupportedNFSVersion = isNFSVersionSupported(nfsMajor, nfsMinor)docs/longhornctl_export_replica_stop.md (1)
34-34: LGTM! Auto-generated timestamp update.The timestamp update is consistent with other documentation files. Note: The static analysis issues regarding heading levels and hyphenation are inherent to the spf13/cobra auto-generation tool and would need to be addressed upstream if desired.
If you'd like to address the markdown linting issues (heading levels and hyphenation) in the future, we could create an issue in the spf13/cobra repository.
🧰 Tools
🪛 LanguageTool
[uncategorized] ~34-~34: It appears that a hyphen is missing (if ‘auto’ is not used in the context of ‘cars’).
Context: ....md) - Export Longhorn replica ###### Auto generated by spf13/cobra on 27-Nov-2024(AUTO_HYPHEN)
🪛 Markdownlint (0.35.0)
34-34: Expected: h4; Actual: h6
Heading levels should only increment by one level at a time(MD001, heading-increment)
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
⛔ Files ignored due to path filters (2)
vendor/github.com/longhorn/go-common-libs/nfs/nfs.gois excluded by!vendor/**vendor/modules.txtis excluded by!vendor/**
📒 Files selected for processing (17)
docs/longhornctl.md(1 hunks)docs/longhornctl_check.md(1 hunks)docs/longhornctl_check_preflight.md(1 hunks)docs/longhornctl_doc.md(1 hunks)docs/longhornctl_export.md(1 hunks)docs/longhornctl_export_replica.md(1 hunks)docs/longhornctl_export_replica_stop.md(1 hunks)docs/longhornctl_get.md(1 hunks)docs/longhornctl_get_replica.md(1 hunks)docs/longhornctl_global-options.md(1 hunks)docs/longhornctl_install.md(1 hunks)docs/longhornctl_install_preflight.md(1 hunks)docs/longhornctl_install_preflight_stop.md(1 hunks)docs/longhornctl_trim.md(1 hunks)docs/longhornctl_trim_volume.md(1 hunks)docs/longhornctl_version.md(1 hunks)pkg/local/preflight/checker.go(2 hunks)
🧰 Additional context used
🪛 LanguageTool
docs/longhornctl.md
[uncategorized] ~29-~29: It appears that a hyphen is missing (if ‘auto’ is not used in the context of ‘cars’).
Context: ...d) - Print longhornctl version ###### Auto generated by spf13/cobra on 27-Nov-2024
(AUTO_HYPHEN)
docs/longhornctl_check.md
[uncategorized] ~19-~19: It appears that a hyphen is missing (if ‘auto’ is not used in the context of ‘cars’).
Context: ...md) - Check Longhorn preflight ###### Auto generated by spf13/cobra on 27-Nov-2024
(AUTO_HYPHEN)
docs/longhornctl_check_preflight.md
[uncategorized] ~60-~60: It appears that a hyphen is missing (if ‘auto’ is not used in the context of ‘cars’).
Context: ... - Longhorn checking operations ###### Auto generated by spf13/cobra on 27-Nov-2024
(AUTO_HYPHEN)
docs/longhornctl_doc.md
[uncategorized] ~31-~31: It appears that a hyphen is missing (if ‘auto’ is not used in the context of ‘cars’).
Context: ...Longhorn commandline interface. ###### Auto generated by spf13/cobra on 27-Nov-2024
(AUTO_HYPHEN)
docs/longhornctl_export.md
[uncategorized] ~19-~19: It appears that a hyphen is missing (if ‘auto’ is not used in the context of ‘cars’).
Context: ....md) - Export Longhorn replica ###### Auto generated by spf13/cobra on 27-Nov-2024
(AUTO_HYPHEN)
docs/longhornctl_export_replica.md
[uncategorized] ~65-~65: It appears that a hyphen is missing (if ‘auto’ is not used in the context of ‘cars’).
Context: ...Stop exporting Longhorn replica ###### Auto generated by spf13/cobra on 27-Nov-2024
(AUTO_HYPHEN)
docs/longhornctl_export_replica_stop.md
[uncategorized] ~34-~34: It appears that a hyphen is missing (if ‘auto’ is not used in the context of ‘cars’).
Context: ....md) - Export Longhorn replica ###### Auto generated by spf13/cobra on 27-Nov-2024
(AUTO_HYPHEN)
docs/longhornctl_get.md
[uncategorized] ~19-~19: It appears that a hyphen is missing (if ‘auto’ is not used in the context of ‘cars’).
Context: ...et Longhorn replica information ###### Auto generated by spf13/cobra on 27-Nov-2024
(AUTO_HYPHEN)
docs/longhornctl_get_replica.md
[uncategorized] ~63-~63: It appears that a hyphen is missing (if ‘auto’ is not used in the context of ‘cars’).
Context: ...nformation gathering operations ###### Auto generated by spf13/cobra on 27-Nov-2024
(AUTO_HYPHEN)
docs/longhornctl_global-options.md
[uncategorized] ~31-~31: It appears that a hyphen is missing (if ‘auto’ is not used in the context of ‘cars’).
Context: ...Longhorn commandline interface. ###### Auto generated by spf13/cobra on 27-Nov-2024
(AUTO_HYPHEN)
docs/longhornctl_install.md
[uncategorized] ~19-~19: It appears that a hyphen is missing (if ‘auto’ is not used in the context of ‘cars’).
Context: ...) - Install Longhorn preflight ###### Auto generated by spf13/cobra on 27-Nov-2024
(AUTO_HYPHEN)
docs/longhornctl_install_preflight.md
[uncategorized] ~48-~48: It appears that a hyphen is missing (if ‘auto’ is not used in the context of ‘cars’).
Context: ...op Longhorn preflight installer ###### Auto generated by spf13/cobra on 27-Nov-2024
(AUTO_HYPHEN)
docs/longhornctl_install_preflight_stop.md
[uncategorized] ~35-~35: It appears that a hyphen is missing (if ‘auto’ is not used in the context of ‘cars’).
Context: ...) - Install Longhorn preflight ###### Auto generated by spf13/cobra on 27-Nov-2024
(AUTO_HYPHEN)
docs/longhornctl_trim.md
[uncategorized] ~19-~19: It appears that a hyphen is missing (if ‘auto’ is not used in the context of ‘cars’).
Context: ...e.md) - Trim a Longhorn volume ###### Auto generated by spf13/cobra on 27-Nov-2024
(AUTO_HYPHEN)
docs/longhornctl_trim_volume.md
[uncategorized] ~45-~45: It appears that a hyphen is missing (if ‘auto’ is not used in the context of ‘cars’).
Context: ... - Longhorn trimming operations ###### Auto generated by spf13/cobra on 27-Nov-2024
(AUTO_HYPHEN)
docs/longhornctl_version.md
[uncategorized] ~27-~27: It appears that a hyphen is missing (if ‘auto’ is not used in the context of ‘cars’).
Context: ...Longhorn commandline interface. ###### Auto generated by spf13/cobra on 27-Nov-2024
(AUTO_HYPHEN)
🪛 Markdownlint (0.35.0)
docs/longhornctl.md
29-29: Expected: h4; Actual: h6
Heading levels should only increment by one level at a time
(MD001, heading-increment)
docs/longhornctl_check.md
19-19: Expected: h4; Actual: h6
Heading levels should only increment by one level at a time
(MD001, heading-increment)
docs/longhornctl_check_preflight.md
60-60: Expected: h4; Actual: h6
Heading levels should only increment by one level at a time
(MD001, heading-increment)
docs/longhornctl_doc.md
31-31: Expected: h4; Actual: h6
Heading levels should only increment by one level at a time
(MD001, heading-increment)
docs/longhornctl_export.md
19-19: Expected: h4; Actual: h6
Heading levels should only increment by one level at a time
(MD001, heading-increment)
docs/longhornctl_export_replica.md
65-65: Expected: h4; Actual: h6
Heading levels should only increment by one level at a time
(MD001, heading-increment)
docs/longhornctl_export_replica_stop.md
34-34: Expected: h4; Actual: h6
Heading levels should only increment by one level at a time
(MD001, heading-increment)
docs/longhornctl_get.md
19-19: Expected: h4; Actual: h6
Heading levels should only increment by one level at a time
(MD001, heading-increment)
docs/longhornctl_get_replica.md
63-63: Expected: h4; Actual: h6
Heading levels should only increment by one level at a time
(MD001, heading-increment)
docs/longhornctl_global-options.md
31-31: Expected: h4; Actual: h6
Heading levels should only increment by one level at a time
(MD001, heading-increment)
docs/longhornctl_install.md
19-19: Expected: h4; Actual: h6
Heading levels should only increment by one level at a time
(MD001, heading-increment)
docs/longhornctl_install_preflight.md
48-48: Expected: h4; Actual: h6
Heading levels should only increment by one level at a time
(MD001, heading-increment)
docs/longhornctl_install_preflight_stop.md
35-35: Expected: h4; Actual: h6
Heading levels should only increment by one level at a time
(MD001, heading-increment)
docs/longhornctl_trim.md
19-19: Expected: h4; Actual: h6
Heading levels should only increment by one level at a time
(MD001, heading-increment)
docs/longhornctl_trim_volume.md
45-45: Expected: h4; Actual: h6
Heading levels should only increment by one level at a time
(MD001, heading-increment)
docs/longhornctl_version.md
27-27: Expected: h4; Actual: h6
Heading levels should only increment by one level at a time
(MD001, heading-increment)
🔇 Additional comments (10)
docs/longhornctl_export.md (1)
19-19: LGTM!
Timestamp update is consistent with other documentation files.
🧰 Tools
🪛 LanguageTool
[uncategorized] ~19-~19: It appears that a hyphen is missing (if ‘auto’ is not used in the context of ‘cars’).
Context: ....md) - Export Longhorn replica ###### Auto generated by spf13/cobra on 27-Nov-2024
(AUTO_HYPHEN)
🪛 Markdownlint (0.35.0)
19-19: Expected: h4; Actual: h6
Heading levels should only increment by one level at a time
(MD001, heading-increment)
docs/longhornctl_version.md (1)
27-27: Auto-generated timestamp update
This change is unrelated to the PR objectives.
🧰 Tools
🪛 LanguageTool
[uncategorized] ~27-~27: It appears that a hyphen is missing (if ‘auto’ is not used in the context of ‘cars’).
Context: ...Longhorn commandline interface. ###### Auto generated by spf13/cobra on 27-Nov-2024
(AUTO_HYPHEN)
🪛 Markdownlint (0.35.0)
27-27: Expected: h4; Actual: h6
Heading levels should only increment by one level at a time
(MD001, heading-increment)
docs/longhornctl_install_preflight_stop.md (1)
35-35: The auto-generated section can be customized through cobra configuration
The formatting issues in the auto-generated section can be fixed by configuring cobra.
🧰 Tools
🪛 LanguageTool
[uncategorized] ~35-~35: It appears that a hyphen is missing (if ‘auto’ is not used in the context of ‘cars’).
Context: ...) - Install Longhorn preflight ###### Auto generated by spf13/cobra on 27-Nov-2024
(AUTO_HYPHEN)
🪛 Markdownlint (0.35.0)
35-35: Expected: h4; Actual: h6
Heading levels should only increment by one level at a time
(MD001, heading-increment)
docs/longhornctl_get_replica.md (1)
63-63: LGTM!
The timestamp update is correct and this file is not related to the NFS version checking changes.
🧰 Tools
🪛 LanguageTool
[uncategorized] ~63-~63: It appears that a hyphen is missing (if ‘auto’ is not used in the context of ‘cars’).
Context: ...nformation gathering operations ###### Auto generated by spf13/cobra on 27-Nov-2024
(AUTO_HYPHEN)
🪛 Markdownlint (0.35.0)
63-63: Expected: h4; Actual: h6
Heading levels should only increment by one level at a time
(MD001, heading-increment)
docs/longhornctl_export_replica.md (1)
65-65: LGTM on the timestamp update!
The auto-generated timestamp has been updated appropriately.
🧰 Tools
🪛 LanguageTool
[uncategorized] ~65-~65: It appears that a hyphen is missing (if ‘auto’ is not used in the context of ‘cars’).
Context: ...Stop exporting Longhorn replica ###### Auto generated by spf13/cobra on 27-Nov-2024
(AUTO_HYPHEN)
🪛 Markdownlint (0.35.0)
65-65: Expected: h4; Actual: h6
Heading levels should only increment by one level at a time
(MD001, heading-increment)
pkg/local/preflight/checker.go (2)
18-20: LGTM: Import changes are appropriate.
The new imports from go-common-libs are necessary for the enhanced NFS version checking functionality.
435-457: 🛠️ Refactor suggestion
Improve error handling in module loading check.
The error from IsModuleLoaded is silently ignored. Consider logging it to help with troubleshooting.
moduleLoaded, err := utils.IsModuleLoaded(module)
if err != nil {
+ local.collection.Log.Warn = append(local.collection.Log.Warn,
+ fmt.Sprintf("Failed to check if module %s is loaded: %v", module, err))
continue
}Likely invalid or redundant comment.
docs/longhornctl_doc.md (1)
31-31: LGTM! Auto-generated timestamp update.
The timestamp update is consistent with the auto-generated nature of these documentation files using spf13/cobra.
🧰 Tools
🪛 LanguageTool
[uncategorized] ~31-~31: It appears that a hyphen is missing (if ‘auto’ is not used in the context of ‘cars’).
Context: ...Longhorn commandline interface. ###### Auto generated by spf13/cobra on 27-Nov-2024
(AUTO_HYPHEN)
🪛 Markdownlint (0.35.0)
31-31: Expected: h4; Actual: h6
Heading levels should only increment by one level at a time
(MD001, heading-increment)
docs/longhornctl_global-options.md (1)
31-31: LGTM! Auto-generated timestamp update.
The timestamp update is consistent with other documentation files.
🧰 Tools
🪛 LanguageTool
[uncategorized] ~31-~31: It appears that a hyphen is missing (if ‘auto’ is not used in the context of ‘cars’).
Context: ...Longhorn commandline interface. ###### Auto generated by spf13/cobra on 27-Nov-2024
(AUTO_HYPHEN)
🪛 Markdownlint (0.35.0)
31-31: Expected: h4; Actual: h6
Heading levels should only increment by one level at a time
(MD001, heading-increment)
docs/longhornctl.md (1)
29-29: The auto-generated section formatting issues persist
The auto-generated section still has formatting inconsistencies that were previously identified:
- "Auto-generated" should be hyphenated
- The heading level (h6) doesn't follow the proper hierarchy
As mentioned in the previous review, this can be fixed by setting DisableAutoGenTag in the cobra command configuration.
🧰 Tools
🪛 LanguageTool
[uncategorized] ~29-~29: It appears that a hyphen is missing (if ‘auto’ is not used in the context of ‘cars’).
Context: ...d) - Print longhornctl version ###### Auto generated by spf13/cobra on 27-Nov-2024
(AUTO_HYPHEN)
🪛 Markdownlint (0.35.0)
29-29: Expected: h4; Actual: h6
Heading levels should only increment by one level at a time
(MD001, heading-increment)
|
@mergify backport v1.7.x |
✅ Backports have been created
|
Which issue(s) this PR fixes:
Issue longhorn/longhorn#9830
What this PR does / why we need it:
Though the node supports NFSv4, the NFS client picks the protocol version by
Defaultversdefined in nfsmount.conf. This may cause problem while using backup-related features.Special notes for your reviewer:
Additional documentation or context