Skip to content
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

storage: Expose Stratis virtual filesystem sizes #20611

Merged
merged 1 commit into from
Sep 6, 2024

Conversation

mvollmer
Copy link
Member

@mvollmer mvollmer commented Jun 17, 2024

Instead of the "Managed filesystem sizes" concept. This makes Cockpit
easier to discover when you know the real Stratis concepts and exposes
its full capabilities.

Demo: https://youtu.be/E1zqhAowus8
Longer exploration: https://youtu.be/1W6QPNPwT7Q


Storage: Support for managing virtual Stratis filesystem sizes

Cockpit now directly exposes the --size and --size-limit options of the stratis filesystem create command as well as the stratis filesystem set-size-limit and stratis filesystem unset-size-limit commands.

@mvollmer mvollmer added the blocked Don't land until something else happens first (see task list) label Jun 17, 2024
@mvollmer mvollmer marked this pull request as draft June 17, 2024 13:57
@mvollmer mvollmer removed the blocked Don't land until something else happens first (see task list) label Jun 18, 2024
@mvollmer mvollmer force-pushed the storage-raw-stratis branch 2 times, most recently from a62d313 to 19e362f Compare June 18, 2024 12:06
@mvollmer mvollmer marked this pull request as ready for review June 18, 2024 12:06
test/verify/check-storage-stratis Fixed Show fixed Hide fixed
test/verify/check-storage-stratis Fixed Show fixed Hide fixed
@mvollmer mvollmer marked this pull request as draft June 19, 2024 12:30
@mvollmer mvollmer force-pushed the storage-raw-stratis branch 2 times, most recently from 8db80eb to 4204c65 Compare June 20, 2024 11:25
@mvollmer mvollmer requested a review from jelly September 5, 2024 11:49
@mvollmer mvollmer marked this pull request as ready for review September 5, 2024 12:00
@mvollmer
Copy link
Member Author

mvollmer commented Sep 5, 2024

Let's get this in. The UI here assumes that people are experts in Stratis and know exactly what they are doing and don't need any help or explanations from Cockpit. If that's not good enough, then we have to kill this. I am not going to work on this much more.

Copy link
Member

@jelly jelly left a comment

Choose a reason for hiding this comment

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

No strong argument against or for. There is one tiny codeql warning however which seems nice to fix.

@mvollmer
Copy link
Member Author

mvollmer commented Sep 6, 2024

No strong argument against or for.

Yeah, it's meh all around.

There is one tiny codeql warning however which seems nice to fix.

There's lots of linting to be done, but I think the Variable m is not used. warning has been fixed.

@mvollmer
Copy link
Member Author

mvollmer commented Sep 6, 2024

but I think the Variable m is not used. warning has been fixed.

Ohh: there were two "unused m" warnings, and I only fixed one, but the comments here show the wrong one as fixed! I am always suprised at how fragile these shiny web things actually are.

Instead of the "Managed filesystem sizes" concept. This makes Cockpit
easier to discover when you know the real Stratis concepts and exposes
its full capabilities.
visible: vals => vals.size_options.custom_limit,
value: fsys.SizeLimit[0] && Number(fsys.SizeLimit[1]),
min: Number(fsys.Size),
max: pool.Overprovisioning ? stats.pool_total : stats.pool_free + Number(fsys.Size),
Copy link
Contributor

Choose a reason for hiding this comment

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

This added line is not executed by any test.

@mvollmer mvollmer merged commit 236ffd3 into cockpit-project:main Sep 6, 2024
84 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants