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

Allow pool-specifc disk limits in projects #906

Open
stgraber opened this issue May 31, 2024 · 7 comments
Open

Allow pool-specifc disk limits in projects #906

stgraber opened this issue May 31, 2024 · 7 comments
Assignees
Labels
Easy Good for new contributors Feature New feature, not a bug
Milestone

Comments

@stgraber
Copy link
Member

The current limits.disk resource limit on projects only allows restricting the overall disk footprint, this is fine in many environments but for those of us with clusters having different class of storage, like:

stgraber@castiana:~/data/code/lxc/incus (lxc/main)$ incus storage list s-dcmtl-cluster:
+--------+--------+---------------------------+---------+---------+
|  NAME  | DRIVER |        DESCRIPTION        | USED BY |  STATE  |
+--------+--------+---------------------------+---------+---------+
| hdd    | ceph   | Slow HDD storage          | 9       | CREATED |
+--------+--------+---------------------------+---------+---------+
| local  | zfs    | Local NVME storage        | 63      | CREATED |
+--------+--------+---------------------------+---------+---------+
| shared | cephfs | Shared filesystem storage | 8       | CREATED |
+--------+--------+---------------------------+---------+---------+
| ssd    | ceph   | Fast SSD storage          | 70      | CREATED |
+--------+--------+---------------------------+---------+---------+

Having all storage treated equal isn't ideal and so having per-pool limits would be preferable.

The easiest way would be to not only have limits.disk but also limits.disk.pool.XYZ where XYZ in this case would be one of hdd, local, shared or ssd.

A limit of 0 would prevent the use for a storage pool entirely and should result in its exclusion from incus storage list for that project.

@stgraber stgraber added Feature New feature, not a bug Easy Good for new contributors labels May 31, 2024
@stgraber stgraber modified the milestones: soon, incus-6.3 May 31, 2024
@awalvie
Copy link
Contributor

awalvie commented Jun 7, 2024

I'd like to work on this issue!

@stgraber
Copy link
Member Author

stgraber commented Jun 7, 2024

Cool, I assigned it to you!

@awalvie
Copy link
Contributor

awalvie commented Jun 13, 2024

Sorry about the delay, but from what I understand I'll have to update projectConfigKeys for func projectValidateConfig in api_project.go

func projectValidateConfig(s *state.State, config map[string]string) error {

with an additional field for limits.disk.pool.type but am not sure where the actual limit on the pool gets configured and set after that.

I saw that the function gets called as part of func projectPost:

func projectsPost(d *Daemon, r *http.Request) response.Response {

but that seems to just create an entry in the database. Is there something else that needs to be done outside of that?

@stgraber
Copy link
Member Author

Sorry for the delay here.

Note that the key is really meant to be limits.disk.pool.NAME so not a static config key name as the pool name will be part of it.

You're correct that the main spot where this needs to be added is in projectValidateConfig.
That will allow you to set it through incus project set.

The actual enforcement however is done in internal/server/project/permissions.go where you'll want to look where current limits.disk is checked and add the equivalent per-pool checking.

@stgraber
Copy link
Member Author

@awalvie any luck with this one?

@awalvie
Copy link
Contributor

awalvie commented Jul 1, 2024

Sorry about the delay here @stgraber, was away on vacation the last couple weeks. I'll start working on this shortly. But if its blocking a release please feel free to remove me as the assignee. I'll send a message to get the issue assigned again if no one has picked it up!

@stgraber
Copy link
Member Author

stgraber commented Jul 1, 2024

Nope, no rush, was just wondering. I hope you had a good vacation!

@stgraber stgraber modified the milestones: incus-6.3, incus-6.4 Jul 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Easy Good for new contributors Feature New feature, not a bug
Development

No branches or pull requests

2 participants