Fix(nginx-config): Listen port based on nextcloud.containerPort #278
Conversation
|
Thanks for the submission @Nold360 . can you bump the chart version? |
Signed-off-by: nold <[email protected]>
Nold360
force-pushed
the
feat/nginx-config-containerport
branch
from
c63f352 to
c72539a
Compare
fixed |
|
+1, this seems necessary for OpenShift. For now I've worked around this using the normal apache image, as an image stream and layered a change on top of it to make it listen on another port, but this is a much cleaner solution. I also think the readiness and liveness probes also need to be able to have their ports configured, because that was the next issue I ran into on OpenShift. They seem to hardcode |
| @@ -44,7 +44,7 @@ data: | |||
| } | |||
|
|
|||
| server { | |||
| listen 80; | |||
| listen {{ .Values.nextcloud.containerPort | default 80 }}; | |||
There was a problem hiding this comment.
Could you move the default value into the values file? That would make it easier to reuse for #316
There was a problem hiding this comment.
Sorry, there's 3 PRs for this that have been open for a couple of months (except mine that I opened today). @provokateurin, are you asking that that nextcloud.containerPort in values.yaml be set to 80?
@Nold360 if you don't have time, I can get this done via github suggestion commits.
There was a problem hiding this comment.
I couldn't figure out how to do a suggest on a new file, so I'm modifying #384 to include this change. Will close my PR if the other PRs are moved forward first.
|
This was done here: a215de8 |
Signed-off-by: nold [email protected]
Pull Request
Description of the change
If
nextcloud.containerPortis set, it will configure nginx to listen to the specified port.Benefits
Allows nginx container to run rootless & follows the expected behavior of
nextcloud.containerPort.Possible drawbacks
Unexpected port change on update?!
Applicable issues
Additional information
Checklist
Chart.yamlaccording to semver.