-
Notifications
You must be signed in to change notification settings - Fork 1
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Updating docs for Lightbits product version #64
base: master
Are you sure you want to change the base?
Conversation
fix examples for helm chart that were missed in original commit. This commit can/should be squashed with initall commit. Signed-off-by: ronen <[email protected]>
…on of K8 verison from v17-v30
WalkthroughThis pull request introduces multiple updates across various files related to the Lightbits CSI plugin. Key changes include the addition of Helm template commands for generating Kubernetes manifests for multiple versions, updates to compatibility information in documentation, version increments for the plugin and Helm charts, and enhancements to toleration configurations in Kubernetes templates. Additionally, dependency versions in the Go module have been updated, and the documentation has been refined to reflect these changes, ensuring consistency and clarity throughout the project. Changes
Possibly related PRs
Suggested reviewers
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
Quality Gate passedIssues Measures |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 4
🧹 Outside diff range and nitpick comments (6)
deploy/helm/lb-csi/templates/lb-csi-controller.yaml (1)
Line range hint
1-145
: Consider architectural improvements for better resilience.The current configuration has some good practices in place (system-cluster-critical priority, RollingUpdate strategy). However, consider the following architectural improvements:
- The StatefulSet has
replicas: 1
, which could be a single point of failure. Consider implementing leader election and running multiple replicas for high availability.- The sidecar container versions are hardcoded. Consider making them configurable through values.yaml for easier upgrades.
- The toleration configuration could be aligned with node affinity rules to ensure proper pod placement in specialized nodes.
docs/src/CHANGELOG/CHANGELOG-1.17.0.md (1)
10-10
: Format bare URLs as markdown linksConvert bare URLs to proper markdown links for better documentation formatting:
-https://github.com/lightbitslabs/los-csi/releases/tag/v1.17.0 +[Release v1.17.0](https://github.com/lightbitslabs/los-csi/releases/tag/v1.17.0) -https://github.com/LightBitsLabs/los-csi/tree/v1.17.0/docs +[Documentation](https://github.com/LightBitsLabs/los-csi/tree/v1.17.0/docs) -https://github.com/LightBitsLabs/los-csi/tree/v1.17.0/docs/upgrade +[Upgrade Guide](https://github.com/LightBitsLabs/los-csi/tree/v1.17.0/docs/upgrade)Also applies to: 23-23, 27-27
🧰 Tools
🪛 Markdownlint (0.35.0)
10-10: null
Bare URL used(MD034, no-bare-urls)
deploy/helm/lb-csi/values.yaml (1)
23-37
: Fix YAML formatting issuesPlease address the following formatting issues:
- Remove trailing spaces from comment lines
- Add a newline at the end of the file
# CSI controller pod tolerations will enable controller-pod deployment to the desired node. # Replace values with desired key-value pairs of tainted nodes. lbControllerTolerations: key: "lb-csi-controller" operator: "Equal" value: "lb-csi-controller-pod" effect: "NoSchedule" # CSI node pod tolerations will enable node-pod deployment to the desired node. # Replace values with desired key-value pairs of tainted nodes. lbNodeTolerations: key: "lb-csi-node" operator: "Equal" value: "lb-csi-node-pod" - effect: "NoSchedule" + effect: "NoSchedule" +🧰 Tools
🪛 yamllint (1.35.1)
[error] 23-23: trailing spaces
(trailing-spaces)
[error] 24-24: trailing spaces
(trailing-spaces)
[error] 29-29: trailing spaces
(trailing-spaces)
[error] 31-31: trailing spaces
(trailing-spaces)
[error] 32-32: trailing spaces
(trailing-spaces)
[error] 37-37: no new line character at the end of file
(new-line-at-end-of-file)
[error] 37-37: trailing spaces
(trailing-spaces)
docs/src/plugin_deployment/plugin_deployment_using_chart_in_bundle.md (1)
24-24
: Fix table formatting: replace tabs with spaces.The helm list output table uses hard tabs which should be replaced with spaces for better markdown compatibility.
-NAME NAMESPACE REVISION UPDATED STATUS CHART APP VERSION +NAME NAMESPACE REVISION UPDATED STATUS CHART APP VERSION🧰 Tools
🪛 Markdownlint (0.35.0)
24-24: Column: 7
Hard tabs(MD010, no-hard-tabs)
24-24: Column: 19
Hard tabs(MD010, no-hard-tabs)
24-24: Column: 44
Hard tabs(MD010, no-hard-tabs)
24-24: Column: 53
Hard tabs(MD010, no-hard-tabs)
24-24: Column: 74
Hard tabs(MD010, no-hard-tabs)
deploy/helm/lb-csi/templates/lb-csi-node.yaml (2)
204-211
: Document the toleration configuration in values.yamlWhile the toleration configuration is correctly implemented, it would be helpful to document the available options and their implications in the values.yaml file.
Add documentation in values.yaml:
lbNodeTolerations: # -- Node tolerations for the CSI plugin DaemonSet # ref: https://kubernetes.io/docs/concepts/scheduling-eviction/taint-and-toleration/ key: "example-key" operator: "Equal" value: "example-value" effect: "NoSchedule"🧰 Tools
🪛 yamllint (1.35.1)
[error] 211-211: no new line character at the end of file
(new-line-at-end-of-file)
211-211
: Add newline at end of fileAdd a newline character at the end of the file to comply with YAML best practices.
effect: {{ .Values.lbNodeTolerations.effect}} {{- end }} +
🧰 Tools
🪛 yamllint (1.35.1)
[error] 211-211: no new line character at the end of file
(new-line-at-end-of-file)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (1)
go.sum
is excluded by!**/*.sum
📒 Files selected for processing (18)
Makefile
(1 hunks)README.md
(1 hunks)VERSION
(1 hunks)deploy/helm/lb-csi-workload-examples/Chart.yaml
(2 hunks)deploy/helm/lb-csi/Chart.yaml
(1 hunks)deploy/helm/lb-csi/templates/lb-csi-controller.yaml
(1 hunks)deploy/helm/lb-csi/templates/lb-csi-node.yaml
(1 hunks)deploy/helm/lb-csi/values.yaml
(2 hunks)docs/metadata.md
(1 hunks)docs/src/CHANGELOG/CHANGELOG-1.17.0.md
(1 hunks)docs/src/CHANGELOG/README.md
(1 hunks)docs/src/SUMMARY.md
(1 hunks)docs/src/introduction.md
(1 hunks)docs/src/plugin_deployment/deployment.md
(3 hunks)docs/src/plugin_deployment/plugin_deployment_using_chart_in_bundle.md
(2 hunks)docs/src/plugin_deployment/plugin_deployment_using_lightbits_helm_repository.md
(1 hunks)go.mod
(1 hunks)pkg/driver/node.go
(4 hunks)
✅ Files skipped from review due to trivial changes (5)
- VERSION
- docs/src/CHANGELOG/README.md
- docs/src/SUMMARY.md
- deploy/helm/lb-csi/Chart.yaml
- deploy/helm/lb-csi-workload-examples/Chart.yaml
🧰 Additional context used
🪛 Markdownlint (0.35.0)
docs/src/CHANGELOG/CHANGELOG-1.17.0.md
10-10: null
Bare URL used
(MD034, no-bare-urls)
23-23: null
Bare URL used
(MD034, no-bare-urls)
27-27: null
Bare URL used
(MD034, no-bare-urls)
docs/src/plugin_deployment/plugin_deployment_using_chart_in_bundle.md
24-24: Column: 7
Hard tabs
(MD010, no-hard-tabs)
24-24: Column: 19
Hard tabs
(MD010, no-hard-tabs)
24-24: Column: 44
Hard tabs
(MD010, no-hard-tabs)
24-24: Column: 53
Hard tabs
(MD010, no-hard-tabs)
24-24: Column: 74
Hard tabs
(MD010, no-hard-tabs)
🪛 yamllint (1.35.1)
deploy/helm/lb-csi/templates/lb-csi-node.yaml
[error] 211-211: no new line character at the end of file
(new-line-at-end-of-file)
deploy/helm/lb-csi/values.yaml
[error] 23-23: trailing spaces
(trailing-spaces)
[error] 24-24: trailing spaces
(trailing-spaces)
[error] 29-29: trailing spaces
(trailing-spaces)
[error] 31-31: trailing spaces
(trailing-spaces)
[error] 32-32: trailing spaces
(trailing-spaces)
[error] 37-37: no new line character at the end of file
(new-line-at-end-of-file)
[error] 37-37: trailing spaces
(trailing-spaces)
🔇 Additional comments (13)
docs/src/CHANGELOG/CHANGELOG-1.17.0.md (1)
32-32
:
Verify Kubernetes version compatibility
The mentioned Kubernetes version "v30.0.x" seems unusually high. Current stable Kubernetes releases are around v1.29.x. Please verify if this should be "v1.30.x" instead.
go.mod (1)
18-25
: LGTM! Dependencies updated appropriately
The dependency updates look good and maintain consistency with the project's version upgrade. The changes include appropriate version bumps for critical dependencies like golang.org/x/sys
, k8s.io/mount-utils
, and k8s.io/utils
.
deploy/helm/lb-csi/values.yaml (2)
33-37
: Review node tolerations configuration
The node tolerations are currently using the same key/value pair as the controller tolerations (lb-csi-controller
/lb-csi-controller-pod
). This might not be the intended configuration. Consider using node-specific values:
lbNodeTolerations:
- key: "lb-csi-controller"
+ key: "lb-csi-node"
operator: "Equal"
- value: "lb-csi-controller-pod"
+ value: "lb-csi-node-pod"
effect: "NoSchedule"
🧰 Tools
🪛 yamllint (1.35.1)
[error] 37-37: no new line character at the end of file
(new-line-at-end-of-file)
[error] 37-37: trailing spaces
(trailing-spaces)
4-4
: LGTM! Version updates look consistent
The image version updates to 1.17.0 for both the CSI plugin and discovery client are consistent with the changelog.
Also applies to: 11-11
docs/src/plugin_deployment/plugin_deployment_using_chart_in_bundle.md (1)
24-24
: LGTM! Version updates are consistent.
The version updates to 1.17.0 in both the helm list output and image tag are consistent with the overall version changes.
Also applies to: 77-77
🧰 Tools
🪛 Markdownlint (0.35.0)
24-24: Column: 7
Hard tabs
(MD010, no-hard-tabs)
24-24: Column: 19
Hard tabs
(MD010, no-hard-tabs)
24-24: Column: 44
Hard tabs
(MD010, no-hard-tabs)
24-24: Column: 53
Hard tabs
(MD010, no-hard-tabs)
24-24: Column: 74
Hard tabs
(MD010, no-hard-tabs)
docs/metadata.md (1)
6-6
: Verify Kubernetes version compatibility up to v1.30
While the version updates for the CSI Plugin (v1.17.0) and LightOS (v3.12.1) align with the PR objectives, the extended Kubernetes version support range (up to v1.30) requires verification.
Also applies to: 9-9, 11-11
✅ Verification successful
Let me run one more verification to check the Helm chart values and templates for version-specific configurations:
Kubernetes v1.30 compatibility is supported
Based on the verification results:
- The Makefile includes deployment configurations for Kubernetes versions up to v1.30
- The Helm charts show version-aware components (e.g., snapshot-controller has different versions for pre and post v1.20)
- No version-specific code restrictions or compatibility checks were found that would prevent operation on newer K8s versions
- The CSI spec implementation appears version-agnostic
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for any version-specific code or compatibility checks in the codebase
# Search for version-specific Kubernetes compatibility checks
rg -A 3 "v1\.(2[5-9]|30)"
# Look for version-specific code paths in templates
ast-grep --pattern 'if $version $op "1.$_"'
Length of output: 6253
Script:
#!/bin/bash
# Check Helm chart for version-specific configurations
fd "values.yaml|_helpers.tpl" deploy/helm -x cat {}
# Check for any version constraints in Chart.yaml
fd "Chart.yaml" deploy/helm -x cat {}
Length of output: 6926
docs/src/introduction.md (1)
16-16
: LGTM: Version updates are consistent
The version updates and upgrade recommendation are consistent with the changes in metadata.md and align with the PR objectives.
Also applies to: 20-20
docs/src/plugin_deployment/deployment.md (1)
339-339
: Version updates look consistent
The image version updates to 1.17.0
align with the PR objective and are consistent across both controller and node deployments.
Also applies to: 357-357
Makefile (1)
270-371
: Extended Kubernetes version support looks good
The additions follow the existing pattern for generating Kubernetes manifests, properly extending support for versions v1.25 through v1.30. Each version correctly includes both standard and discovery client variants with consistent parameters.
pkg/driver/node.go (4)
276-283
: Mount point check refactor looks good
The change from mountutils.IsNotMountPoint
to d.mounter.IsMountPoint
improves consistency with other mount operations in the codebase. Error handling is properly maintained.
434-444
: Improved I/O error handling in NodeUnstageVolume
The updated error handling for I/O errors during unmounting is more robust, with better logging and proper fallback behavior.
630-634
: Consistent mount point check in NodePublishVolume
The mount point checking logic has been updated to match the pattern used in other methods, maintaining consistency across the codebase.
683-693
: Enhanced error handling in NodeUnpublishVolume
The error handling improvements for I/O errors during unmounting follow the same pattern as NodeUnstageVolume, providing consistent behavior across volume operations.
{{- if .Values.lbControllerTolerations }} | ||
tolerations: | ||
- key: {{ .Values.lbControllerTolerations.key}} | ||
operator: {{ .Values.lbControllerTolerations.operator}} | ||
value: {{ .Values.lbControllerTolerations.value}} | ||
effect: {{ .Values.lbControllerTolerations.effect}} | ||
{{- end }} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Enhance the tolerations configuration for better flexibility and safety.
While the toleration configuration is functional, there are several improvements that could be made:
- The current implementation only supports a single toleration. Consider supporting multiple tolerations for more flexibility.
- Values should be quoted to handle special characters safely.
- The indentation should follow Kubernetes standard of 2 spaces.
Here's the suggested improvement:
{{- if .Values.lbControllerTolerations }}
tolerations:
- - key: {{ .Values.lbControllerTolerations.key}}
- operator: {{ .Values.lbControllerTolerations.operator}}
- value: {{ .Values.lbControllerTolerations.value}}
- effect: {{ .Values.lbControllerTolerations.effect}}
+ {{- range .Values.lbControllerTolerations }}
+ - key: {{ .key | quote }}
+ operator: {{ .operator | quote }}
+ value: {{ .value | quote }}
+ effect: {{ .effect | quote }}
+ {{- end }}
{{- end }}
This change:
- Supports multiple tolerations through array syntax in values.yaml
- Properly quotes values to handle special characters
- Follows Kubernetes standard 2-space indentation
- Maintains backward compatibility
Example values.yaml configuration:
lbControllerTolerations:
- key: "key1"
operator: "Equal"
value: "value1"
effect: "NoSchedule"
- key: "key2"
operator: "Exists"
effect: "NoExecute"
|
||
### Upgrading | ||
|
||
https://github.com/LightBitsLabs/los-csi/tree/duros/docs/upgrade |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix upgrade documentation URL
The upgrade documentation URL points to the "duros" branch instead of v1.17.0. Consider updating to:
-https://github.com/LightBitsLabs/los-csi/tree/duros/docs/upgrade
+https://github.com/LightBitsLabs/los-csi/tree/v1.17.0/docs/upgrade
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
https://github.com/LightBitsLabs/los-csi/tree/duros/docs/upgrade | |
https://github.com/LightBitsLabs/los-csi/tree/v1.17.0/docs/upgrade |
🧰 Tools
🪛 Markdownlint (0.35.0)
27-27: null
Bare URL used
(MD034, no-bare-urls)
@@ -10,7 +10,7 @@ | |||
|
|||
The LightOS CSI plugin is a software module that implements management of persistent storage volumes exported by LightOS software for Container Orchestrator (CO) systems like Kubernetes and Mesos. In conjunction with the LightOS disaggregated storage solution, the CSI plugin provides a building block for the easy deployment of stateful containerized applications on CO clusters. | |||
|
|||
The version of the plugin covered by this document implements version 1.2 of the [Container Storage Interface (CSI) Specification](https://github.com/container-storage-interface/spec/blob/v1.2.0/spec.md), and is compatible with LightOS version 2.3.x | |||
The version of the plugin covered by this document implements version 1.2 of the [Container Storage Interface (CSI) Specification](https://github.com/container-storage-interface/spec/blob/v1.2.0/spec.md), and is compatible with LightOS version 3.12.x |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Codebase verification
Version references need to be synchronized across documentation
Found inconsistency in version references:
README.md
mentions "3.12.x"docs/metadata.md
anddocs/src/introduction.md
specify "3.12.1"
These version references should be aligned to avoid confusion.
🔗 Analysis chain
LGTM! Version update aligns with PR objectives.
The update to LightOS version 3.12.x compatibility is consistent with the PR's goal of updating the core product version.
Let's verify version consistency across documentation:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for any inconsistent version references
# Look for any remaining references to old versions and verify new version consistency
rg -i "version.*3\.12"
rg -i "lightos.*version"
Length of output: 3704
lightbits-helm-repo/lb-csi-plugin 0.15.0 1.17.0 Helm Chart for Lightbits CSI Plugin. | ||
lightbits-helm-repo/lb-csi-workload-examples 0.15.0 1.17.0 Helm Chart for Lightbits CSI Workload Examples. | ||
lightbits-helm-repo/snapshot-controller-3 0.15.0 3.0.3 Deploy snapshot-controller for K8s version < v1.20 | ||
lightbits-helm-repo/snapshot-controller-4 0.15.0 4.2.1 Deploy snapshot-controller for K8s version >= v1.20 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Codebase verification
Version inconsistencies found in Chart.yaml files
The helm search output shows version 0.15.0 for all charts, but the actual Chart.yaml files reveal discrepancies:
- Main charts (lb-csi-plugin, lb-csi-workload-examples) correctly show version 0.15.0
- Snapshot controllers show older version 0.11.0
- Workload example subchart dependencies correctly reference 0.15.0
- Individual workload example subcharts show version 0.1.0
🔗 Analysis chain
LGTM! Chart and application versions are consistently updated.
The version updates are consistent across all charts:
- Chart versions updated to 0.15.0
- Application versions updated to 1.17.0 where applicable
- Snapshot controller versions maintained at 3.0.3 and 4.2.1
Let's verify version consistency in chart files:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify version consistency in Helm charts
# Check Chart.yaml files for version alignment
fd Chart.yaml | xargs cat
Length of output: 2888
Update version to latest core version v3.12.1
LBM1-35660
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Version Updates