search

nextcloud / helm

A community maintained helm chart for deploying Nextcloud on Kubernetes.
GNU Affero General Public License v3.0
295 stars 258 forks source link

nginx containerPort confusion #545

Open Zalfsten opened 3 months ago

Zalfsten commented 3 months ago

Hello,

I'm trying to setup nextcloud with root-less nginx. So the default nginx port 80 won't work. Thus I've set nextcloud.nginx.containerPort to 8080. This results in probe failures, because the nginx probes still refer to port 80.

Having a look at the chart templates, I notice that nextcloud.nginx.containerPort is only used for the nginx config, but not when settings up the container in the deployment. The nginx container make use of nextcloud.nextcloud.containerPort instead. I would consider this to be a bug.

On the other hand, when enabling nginx, nextcloud.nextcloud.containerPort is used only for the nginx container, not at all for the nextcloud container. So the simple workaround is to set nextcloud.nginx.containerPort and nextcloud.nextcloud.containerPort simultaneaously.

Still, I find this confusing. I would either join both variables into one or keep them completely separate -- i.e. the nginx container only uses nextcloud.nginx.containerPort, never the nextcloud.nextcloud.containerPort.

Regards, Carsten

provokateurin commented 3 months ago

I agree, the situation is not good. I'd say the proper fix is to use the right variables in the right places, but not to merge them (could still be useful in edge cases). Would you be up for creating a PR?

Zalfsten commented 3 months ago

Ok, I will try...

suyash-811 commented 1 month ago

Is the issue still open? I could work on this.

jessebot commented 1 month ago

I believe this is still available for work. I don't think I've seen a PR for this.