truenas / apps

GNU Lesser General Public License v3.0
74 stars 22 forks source link

Possibility to disable app limits #792

Open vovagorodok opened 1 week ago

vovagorodok commented 1 week ago

In newer version of apps there no possibility to disable app limits: Screenshot from 2024-11-04 10-30-52 Will it be added in the future?

stavros-k commented 1 week ago

Hello, even on previous apps there was only on custom app the ability to disable those. What is the use case that you want to disable those?

You can set your cpu count and memory to the max of what the system has and achieve the same. But with limits it ensures that a misbehaving app won't starve your system

vovagorodok commented 1 week ago

What is the use case that you want to disable those?

I will use TrueNAS only for one NextCloud app. That's why will be nice to give this app as match resources as is possible?

stavros-k commented 1 week ago

You can change the limits to the max of what your system has.

vovagorodok commented 1 week ago

I'm wondering about one more case. What if we change/add RAM or CPU? How app limits will behave in cases when limits are enabled and disabled? Disabled limits means that them are set to max during app installation (fetched from TrueNAS host system)?

stavros-k commented 1 week ago

You can change the values anytime.

vovagorodok commented 1 week ago

You can change the values anytime.

You're right. However, I wonder if the option of setting it to max and not thinking about it in the future wouldn't be better in these cases.

As I correctly understand it will set resources attribute that is optional: https://docs.docker.com/reference/compose-file/deploy/#resources ?

stavros-k commented 1 week ago

Yes it sets this attribute.

Unfortunately to change that now, it will require a migration for every app. And I don't think at the moment it will happen as middleware does not have migration support at all (besides the k8s -> docker).

You current options is to manually set it to the desired max value, or use custom app.

vovagorodok commented 1 week ago

Unfortunately to change that now, it will require a migration for every app. And I don't think at the moment it will happen as middleware does not have migration support at all (besides the k8s -> docker).

Sure. Maybe we can consider it on the following releases? Should we create somewhere additional issue in order to discuss this case at middleware side?

stavros-k commented 1 week ago

Migrations are planned to be added, so there isn't something to discuss there. But even with support added, this is not a priority, as it will need some time to do the migrations. And to be honest, I'm not sure that it's worth the time spent.

vovagorodok commented 1 week ago

Migrations are planned to be added, so there isn't something to discuss there.

Just out of curiosity, does this have a dedicated PR or Issue?

But even with support added, this is not a priority, as it will need some time to do the migrations.

Obviously, there are topics with higher priority, especially after the migration to docker compose

And to be honest, I'm not sure that it's worth the time spent.

From my PoV it makes sense to allow the user not to limit resources for installed apps. Especially for users who have 1-2 apps