Skip to content

change maxLogSize units to base 10 (remove the 'i')#492

Open
cookew wants to merge 2 commits into
ceph:mainfrom
cookew:fix-helm-chart-maxLogSize
Open

change maxLogSize units to base 10 (remove the 'i')#492
cookew wants to merge 2 commits into
ceph:mainfrom
cookew:fix-helm-chart-maxLogSize

Conversation

@cookew

@cookew cookew commented Jun 7, 2026

Copy link
Copy Markdown
  • when using base 2 (Gi) there are logging errors: found error in /csi-logs/*.log , skipping /logrotate-config/csi:8 unknown unit 'i'

Describe what this PR does

When using base 2 (Gi) there are logging errors from the log rotater:

found error in /csi-logs/*.log , skipping /logrotate-config/csi:8 unknown unit 'i'

This changes maxLogSize units to base 10 (remove the 'i') in the ceph-csi-drivers values.yaml file and updates the driver-chart.md file.

Is there anything that requires special attention

Do you have any questions? No

Is the change backward compatible? No

Are there concerns around backward compatibility? No

Provide any external context for the change, if any.

The found error in /csi-logs/*.log , skipping /logrotate-config/csi:8 unknown unit 'i' error messages no longer appeared after changing these in my own values.yaml file.

Related issues

Fixes: #491

Future concerns

None.

Checklist:

  • Commit Message Formatting: Commit titles and messages follow
    guidelines in the developer
    guide
    .
  • Reviewed the developer guide on Submitting a Pull
    Request
  • Pending release
    notes

    updated with breaking and/or notable changes for the next major release.
  • Documentation has been updated, if necessary.
  • Unit tests have been added, if necessary.
  • Integration tests have been added, if necessary.

* when using base 2 (Gi) there are logging errors: found error in
  /csi-logs/*.log , skipping /logrotate-config/csi:8 unknown unit 'i'

Signed-off-by: William Cooke <cookewe@gmail.com>
Copilot AI review requested due to automatic review settings June 7, 2026 14:50

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

Updates the default maxLogSize used for log rotation across the ceph-csi-drivers Helm chart, and aligns the user-facing chart documentation with the new default.

Changes:

  • Changed log rotation maxLogSize default from "10Gi" to "10G" in values.yaml.
  • Updated the drivers chart documentation to reflect the new "10G" default for all drivers and operator defaults.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
docs/helm-charts/drivers-chart.md Updates the documented default maxLogSize value for log rotation parameters.
deploy/charts/ceph-csi-drivers/values.yaml Changes the chart’s actual default maxLogSize values used by operator defaults and each driver.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread deploy/charts/ceph-csi-drivers/values.yaml Outdated
Comment thread deploy/charts/ceph-csi-drivers/values.yaml
Signed-off-by: William Cooke <cookewe@gmail.com>
@Madhu-1

Madhu-1 commented Jun 8, 2026

Copy link
Copy Markdown
Collaborator

@cookew Thanks for the fix, can you please also add kubebuilder validation for

MaxLogSize resource.Quantity `json:"maxLogSize,omitempty"`
so that on one will get into this problem in future.

@cookew

cookew commented Jun 8, 2026

Copy link
Copy Markdown
Author

@Madhu-1 I will be honest, I am not a go programmer and this might be a little beyond me. After looking around at some documentation, would this be the best way to implement this? Adding this line in right before line 45? I am not sure of all the valid characters that can be accepted to prevent the error.

//+kubebuilder:validation:Pattern=`^[0-9]+(K|M|G|T)?$`

@Madhu-1

Madhu-1 commented Jun 11, 2026

Copy link
Copy Markdown
Collaborator

After looking around at some documentation, would this be the best way to implement this? Adding this line in right before line 45? I am not sure of all the valid characters that can be accepted to prevent the error.

// +kubebuilder:validation:Pattern=^[0-9]+[KMG]?$`` see if this work
Yes thats correct a validation like above on top of the function, and verify that when CR is created its failing when someone pass exta prefix

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Incorrect default maxLogSize units in the ceph-csi-drivers helm chart

3 participants