chore: remove default CSI component images #3256
Conversation
longhorn/longhorn-9580 Signed-off-by: Chin-Ya Huang <[email protected]>
longhorn/longhorn-9580 Signed-off-by: Chin-Ya Huang <[email protected]>
WalkthroughThe changes in this pull request introduce a new function, Changes
Assessment against linked issues
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 review |
✅ Actions performedReview triggered.
|
![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
🧹 Outside diff range and nitpick comments (1)
app/driver.go (1)
146-163: Consider enhancing the validation function.The implementation is solid and achieves the goal of preventing deployment with missing configurations. However, consider these improvements for better maintainability and robustness:
- Group related flags into categories (e.g., core flags, CSI component flags)
- Add function documentation explaining its purpose and validation rules
- Consider additional validation beyond emptiness (e.g., image format validation)
Here's a suggested enhancement:
+// validateFlags ensures all required flags are provided and valid. +// It checks both core manager flags and CSI component image flags. func validateFlags(c *cli.Context) error { + // Core manager flags + coreFlags := []string{ + FlagManagerImage, + FlagManagerURL, + } + + // CSI component image flags + csiFlags := []string{ + FlagCSIAttacherImage, + FlagCSIProvisionerImage, + FlagCSIResizerImage, + FlagCSISnapshotterImage, + FlagCSINodeDriverRegistrarImage, + FlagCSILivenessProbeImage, + } + + // Validate core flags + for _, flag := range coreFlags { + if c.String(flag) == "" { + return fmt.Errorf("core flag %q cannot be empty", flag) + } + } + + // Validate CSI component flags + for _, flag := range csiFlags { + if c.String(flag) == "" { + return fmt.Errorf("CSI component flag %q cannot be empty", flag) + } + } + + return nil }
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (2)
app/driver.go(1 hunks)csi/deployment.go(0 hunks)
💤 Files with no reviewable changes (1)
- csi/deployment.go
🔇 Additional comments (1)
app/driver.go (1)
135-137: LGTM! Validation check is properly placed.
The validation check is correctly positioned before the driver deployment, with appropriate error handling that follows the existing pattern.
Which issue(s) this PR fixes:
Issue longhorn/longhorn#9580
What this PR does / why we need it:
Special notes for your reviewer:
NoneAdditional documentation or context
None