fix: data lost caused by Longhorn CSI plugin doing a wrong re-encryption of volume in rare race condition #106
Conversation
…ion of volume in rare race condition see more details at longhorn/longhorn-manager#3566 longhorn-10416 Signed-off-by: Phan Le <phan.le@suse.com>
WalkthroughThe change introduces a new method Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant E as Executor
participant C as Cryptsetup
participant OS as OS/Exec
U->>E: Call IsLuks(devicePath, timeout)
E->>C: Execute "cryptsetup isLuks" with constructed args
C->>OS: Run the command
OS-->>C: Return exit code and error information
C-->>E: Return result based on execution (success, exit code 1, or error)
E-->>U: Return true if successful, false for exit code 1, or error for other issues
✨ Finishing Touches
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
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #106 +/- ##
==========================================
- Coverage 77.25% 76.73% -0.52%
==========================================
Files 37 37
Lines 2071 2085 +14
==========================================
Hits 1600 1600
- Misses 342 356 +14
Partials 129 129
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
Longhorn 10416 Signed-off-by: Derek Su <derek.su@suse.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
ns/crypto.go (1)
54-69: LGTM! Implementation is secure and well-documented.The implementation correctly uses namespace-isolated execution and includes proper error handling with documentation references. The timeout parameter helps prevent hanging, making it robust for production use.
Consider adding parameter validation.
To make the method more robust, consider adding validation for the input parameters:
func (nsexec *Executor) IsLuks(devicePath string, timeout time.Duration) (bool, error) { + if devicePath == "" { + return false, errors.New("device path cannot be empty") + } + if timeout <= 0 { + return false, errors.New("timeout must be positive") + } args := []string{"isLuks", devicePath}
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
ns/crypto.go(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Build binaries
This PR must be merged before the PR longhorn/longhorn-manager#3566
longhorn/longhorn#10416