PHP Memory Limit should be configured properly by default about helm HOT 8 CLOSED

This is a problem of https://github.com/nextcloud/docker though, it should be fixed in the image and not the Helm chart.

from helm.

Agree that the original limit of 128 is from the docker image but still, the need to adjust that setting to different values remains. There are cases where people put it at 1,024.

As such, the second need remains : the need to fix that value from the chart to something different. And once it is an option, the chart will be able to compensate for that problem in the upstream docker image.

So 2nd point would be the reason to do it and 1st point would be an important side benefit of doing it.

from helm.

Agree about the fact that so many php options exist and as such, may need to be adjusted. That rabbit hole can be pretty deep...

If you choose to only document this fix, it would be important to mention that the "zz-" prefix for the file is important and is to ensure the file will be read last, so its value will overwrite whatever other values have been configured previously.

from helm.

@jessebot I agree with you, either fix it upstream or use a config file. Adding extra options for all php options is not helpful.

from helm.

I was actually just looking at the PHP settings in #327 😁 ! So, yeah, as @provokateurin said, that memory limit is actually configured upstream in the docker repo they've linked here:
https://github.com/nextcloud/docker/blob/064069b3060092b64f6fc1e418932fa215d57364/Dockerfile-alpine.template#L82
and also here:
https://github.com/nextcloud/docker/blob/064069b3060092b64f6fc1e418932fa215d57364/Dockerfile-debian.template#L21

You could maybe also override it nextcloud.extraEnv in your values.yaml like this (instead of how you did it above

Update: Scratch that above statement about the env vars. I missed you explaining you tried that prior, and I just tested it and it didn't work. sorry about that!

from helm.

I understand your point, but I think @provokateurin's original comment still stands in that we should actually update it upstream, if its affecting everyone:

https://github.com/nextcloud/docker/blob/65138b6d22bec1ac15e2f0f125426290640bb97a/Dockerfile-alpine.template#L89-L100

We could also have some docs on updating that PHP memory limit in our README, and we'd could review that PR if you or others would like to submit that. I guess alternatively, we could have the php memory limit be an actual option in the chart like:

nextcloud:
  php:
    memory_limit: 512M

That avoids the needs to do:

nextcloud:
  phpConfigs:
    zz-memory_limit.ini: |-
      upload_max_filesize=5G
      memory_limit=512M

But then we do kinda run the risk of scope creep on this one, as there's a lot of PHP options people could need. @provokateurin and @tvories what do yall think? :) Am I overthinking this? (if the answer is yes, that's totally fine!)

from helm.

Okie dokie, submitted #595 :)

from helm.

@Heracles31 since #595 was merged, this was auto-closed, but if you'd to adjust the wording in the docs, please feel free to let us know or submit a new PR. For actually adjusting the default memory limit for php.ini, you'll need to go to the nextcloud/docker repo, but don't worry, they are friendly :)

from helm.