lxc / incus

Powerful system container and virtual machine manager
https://linuxcontainers.org/incus
Apache License 2.0
2.26k stars 173 forks source link

Allow pool-specifc disk limits in projects #906

Open stgraber opened 1 month ago

stgraber commented 1 month ago

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.

awalvie commented 1 month ago

I'd like to work on this issue!

stgraber commented 1 month ago

Cool, I assigned it to you!

awalvie commented 1 month ago

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

https://github.com/lxc/incus/blob/7031065b2a3d352136ce6e014dcdac05a34feede/cmd/incusd/api_project.go#L1293

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:

https://github.com/lxc/incus/blob/7031065b2a3d352136ce6e014dcdac05a34feede/cmd/incusd/api_project.go#L302

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

stgraber commented 1 month ago

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 commented 1 month ago

@awalvie any luck with this one?

awalvie commented 3 weeks ago

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 commented 3 weeks ago

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

awalvie commented 5 days ago

Sorry about the leave of absence again, I was out of town again, attending a conference. I'll be working on this, this week!

stgraber commented 5 days ago

sounds great, thanks!