Skip to content

Conversation

@addyess
Copy link
Member

@addyess addyess commented Oct 24, 2025

Overview

The cluster default storage class can be applied by this charm (perhaps a bad UX really since this is a cluster wide setting).

However, let's address the "allowed values" comment to make it more clear this can match 1 of the storage classes this charm creates and apply the annotation in the event of a match.

Copilot AI review requested due to automatic review settings December 18, 2025 20:30
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR improves the documentation for the default-storage configuration option to better explain its behavior and implications. The updated description clarifies that this config controls cluster-wide default storage class annotation and warns users about the consequences of having multiple default storage classes.

  • Expands the description to explain that the annotation is applied when charm-created storage class names match the configured value
  • Adds a warning about cluster-wide implications of having multiple default storage classes
  • Documents that leaving the value empty prevents the charm from setting any default storage class

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

Comment on lines +50 to +60
description: |
Default storage class to use.
If the charm created storage class matches this value, it will be
annotated as the default storage class in the kubernetes cluster.
Note: a cluster should only annotate one storage class as the default.
If multiple storage classes are annotated as default, the behavior
is undefined and may lead to unexpected results.
Leave empty to not set any storage class default via this charm.
Copy link

Copilot AI Dec 18, 2025

Choose a reason for hiding this comment

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

The updated description removes the list of allowed values that was present in the original description ('ceph-ext4', 'ceph-xfs', 'cephfs'). While the new explanation of the behavior is clearer, users still need guidance on what values to specify here.

Consider adding a line that mentions the default storage class names or references the storage-class-name-formatter configuration options. For example: "By default, the charm creates storage classes named 'ceph-xfs', 'ceph-ext4', and 'cephfs' (these names can be customized via the respective storage-class-name-formatter options)."

Without this information, users may not know what value to set for this configuration option.

Copilot uses AI. Check for mistakes.
Copy link
Contributor

Choose a reason for hiding this comment

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

Does copilot have a point about including the accepted types?

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

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants