mount nginx.conf as a volume to the frontend container #244
Conversation
Signed-off-by: Ishaan Mittal <[email protected]>
Signed-off-by: Ishaan Mittal <[email protected]>
Signed-off-by: Ishaan Mittal <[email protected]>
Signed-off-by: Ishaan Mittal <[email protected]>
mittal-ishaan
force-pushed
the
add_frontend_config_to_helm_chart
branch
from
0e24ebf to
6f7d4aa
Compare
| volumeMounts: {{- toYaml . | nindent 12 }} | ||
| {{- toYaml . | nindent 12 }} | ||
| {{- end }} | ||
| {{- if .Values.opencost.ui.enabled }} |
There was a problem hiding this comment.
This whole section (270 - 323) is already wrapped in a check on if the ui is enabled. If there isn't going to be a unique setting to enable the configmap, this is a redundant check.
| apiVersion: v1 | ||
| kind: ConfigMap | ||
| metadata: | ||
| name: nginx-conf |
There was a problem hiding this comment.
Feels appropriate to include opencost in this configmap name somewhere. "opencost-ui-nginx-conf" maybe?
| @@ -0,0 +1,93 @@ | |||
| {{- if .Values.opencost.ui.enabled }} | |||
There was a problem hiding this comment.
Is this specific configmap always necessary if opencost ui is enabled? My initial reaction is this should be a separate setting, but as long as its not going to be a breaking change for users I'm ok enabling it by default.
There was a problem hiding this comment.
This configmap is added during runtime in opencost-ui image. Which has write permission issues in openshift environment. This change will not break with opencost-ui v1.114.0 in which @kastl-ars added an if condition in the entrypoint script there. We can get this in after bumping opencost-ui image to 1.114.0 . But as it can break for lower versions, should we keep it under a separate setting, considering that we can release it with ui image bump.
mount
nginx.confas a volume to the frontend container. This way we would me moving out of mounting the nginx.conf to the container when pulling the image in the container as that has permission issues in openshift clusters.Discussion: opencost/opencost-ui#49 (comment)
We need to bump the
opencost-uiimage to have this opencost/opencost-ui#49 changeTesting: I was able to install opencost with these changes