only create /etc/nginx/conf.d/default.nginx.conf if it does not yet exist (fix #4)

[kastl-ars]

What does this PR change?

The docker-entrypoint.sh does not create the /etc/nginx/conf.d/default.nginx.conf file, if it already exist. This way, people can workaround issue #4 more easily by mounting the configuration file from a configMap. Not nice, but working...

Does this PR relate to any other PRs?

No.

How will this PR impact users?

This should have no impact on normal users, other than helping users that want to have readOnlyRootFilesystem set to true or are using OpenShift.

Does this PR address any GitHub or Zendesk issues?

Allows to workaround #4

How was this PR tested?

I cannot test this unfortunately.

Does this PR require changes to documentation?

If this is the only workaround to #4, then this could be documented in the helm chart.

Have you labeled this PR and its corresponding Issue as "next release" if it should be part of the next OpenCost release? If not, why not?

I have not labelled it, as I am not sure if this is a good idea to have this workaround or if this should be fixed properly...

[vercel]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
opencost-ui	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Dec 4, 2024 1:13pm

[mittal-ishaan]

Hi @kastl-ars,

Thank you! Could you help me understand how this change is useful when using OpenShift?

With your workaround mentioned in this comment and without this PR change, I see that the OpenCost deployment succeeds, which is great.

However, I think I might be missing something. In the original issue conversation, you noted:

But then the docker-entrypoint.sh would still try to write things to /etc/nginx/conf.d/default.nginx.conf?

Could you clarify why this is a problem we would want to address? Wouldn't the desired behavior be for the default.nginx.conf to always reflect the latest template? With this change, it seems that if the file already exists, it won't be updated.
Just want to get some more context or understanding I might be missing right now on this.

[kastl-ars]

Thanks for your reply. I'll try to share my thoughts.

With this PR, the entrypoint is fine if it finds that /etc/nginx/conf.d/default.nginx.conf already exists. So, it is enough to just mount a configMap into this place.

Without this PR, the entrypoint will try to write to /etc/nginx/conf.d/default.nginx.conf. Therefore, it needs

1. an emptyDir to make the directory writable (permissions, runAsUser, readOnlyRootFS, ...)
2. AND a configMap mounted to the template file path

In either case, you need to maintain a configMap with the proper contents. But with this PR you would not need the emptyDir.

I agree that this does only make the workaround a little easier, but does not solve the underlying problem.

Maybe it would be better to remove the "create nginx.conf out of a template" and just have the helm chart create a configMap that gets mounted into the container at the right path. However, that would required changes to both the image and the helm chart. And not erroring out if the configuration file already exists would be one of those changes to the image, I guess.

Does this make things a little clearer?

[mittal-ishaan]

thank you @kastl-ars.
One question here: As part of the workaround, do we expect users to create a configMap of the template or the nginx.conf itself?
With this change and the workaround for the openshift environment, for now:

1. Creating configMap of the template: any change to the template will not update the nginx.conf file that should get overwritten when the template is changed.
2. Creating configMap of the nginx.conf file itself: this change benefits us in this case.

Maybe it would be better to remove the "create nginx.conf out of a template" and just have the helm chart create a configMap that gets mounted into the container at the right path. However, that would required changes to both the image and the helm chart.

fully support this ☝️ This will help while installing opencost in openshift environment.

@jessegoodier should we take this up? any thoughts on this?
this will reduce a lot of issues the users are facing in #4

[jessegoodier]

@mittal-ishaan totally support this effort when you have the time.

[kastl-ars]

thank you @kastl-ars. One question here: As part of the workaround, do we expect users to create a configMap of the template or the nginx.conf itself?

Without this PR, the entrypoint always writes to the conf file. So, you need to create the

With this change and the workaround for the openshift environment, for now:

1. Creating configMap of the template: any change to the template will not update the nginx.conf file that should get overwritten when the template is changed.

Without this PR, what you have is an emptyDir shadowing the template, as you need write permissions for the entrypoint script. So, you need to also template into place, so the enytrpoint finds the template.

With this change, you can skip the emptyDir and mount the final config file.

Either way, you need to maintain the template or config file yourself and keep it in sync with the OpenCost development.

2. Creating configMap of the `nginx.conf` file itself: this change benefits us in this case.

Yes, with this change it is enough to maintain the configuration file as a configMap.

Maybe it would be better to remove the "create nginx.conf out of a template" and just have the helm chart create a configMap that gets mounted into the container at the right path. However, that would required changes to both the image and the helm chart.

fully support this ☝️ This will help while installing opencost in openshift environment.

That would be a proper solution and not just a workaround, but involves some more places than just this one script.

@jessegoodier should we take this up? any thoughts on this? this will reduce a lot of issues the users are facing in #4

[mittal-ishaan]

I have tested this that it has no effect on non-OpenShift users. In an OpenShift environment, creating a ConfigMap of either the template or the final nginx.conf file works as expected.

Additionally, when creating a ConfigMap of the configuration file, there is no need to add an extra emptyDir as a volume mount. Simply mounting the final nginx.conf file works as intended with the changes from this PR.

Now the following can also serves as a workaround for #4 skipping the extra emptyDir that needed to be mounted.

    extraVolumeMounts:
      - name: empty-var-www
        mountPath: /var/www
      - name: opencost-ui-nginx-config-volume
        mountPath: /etc/nginx/conf.d/default.nginx.conf
        subPath: default.nginx.conf

extraVolumes:
  - name: empty-var-www
    emptyDir: {}
  - name: opencost-ui-nginx-config-volume
    configMap:
      name: opencost-ui-nginx-config

Thank you @kastl-ars