Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Create static/config.json from template file + ENV if not exist #511

Conversation

muellerst-hg
Copy link
Contributor

@muellerst-hg muellerst-hg commented Jun 1, 2023

Fixes #60

Description

static/config.json can be set either from ENV variables or by mounting the file using a volume. docker-entrypoint.sh takes care of that. Current behavior was that config.json is used as is if the file is mounted , otherwise the file is populated with ENV variables.

In some container environments, mount command is not feasible (e.g. kubernetes 1.27.2 / containerd 1.7.2) and entrypoint.sh fails.

Currently, static/config.json is build into the image. However it just serves as a template, because it requires at least API_BASE_URL to be set. So instead of building an empty static/config.json into the image, rather build the image with config.json.tmpl Then check in docker-entrypoint if config.json file exists. If yes, then just leave it as it is. If not, create the file from config.json.tmpl and populate with ENV variables.

Addressed Issue

fixes #60

Additional Details

Checklist

@nscuro
Copy link
Member

nscuro commented Jun 12, 2023

To support container environments where mount command is not feasible

@muellerst-hg Under what conditions exactly will mount not work? I tried reproducing this in a related issue (#513 (comment)), but failed to do so locally. Any idea what I can do to reproduce this problem?

@muellerst-hg
Copy link
Contributor Author

@nscuro
kubernetes 1.27.2
containerd 1.7.2
runc 1.1.7

mount works well with docker 24.0.2

@nscuro
Copy link
Member

nscuro commented Jun 13, 2023

Thanks @muellerst-hg, I was able to reproduce with

minikube start --kubernetes-version v1.27.2 --container-runtime containerd

So it's a Containerd limitation.

We now have two approaches that aim to fix this:

What would be the better approach? One thing that comes to mind for the touch way, is that folks using Docker Compose more often than not do not specify :ro for their volume mounts. In those cases the file will be mounted, but still writeable.

@Breee Breee mentioned this pull request Jun 13, 2023
2 tasks
@muellerst-hg
Copy link
Contributor Author

muellerst-hg commented Jun 13, 2023

Good catch regarding :rw volume mounts.

I would prefer a different approach:
Currently the image is build with config.json. However the file just serves as a template, because it requires at least API_BASE_URL to be set - either using a volume mount or by overriding it with docker-entrypoint.

Solution: Build the image with config.json.tmpl instead of config.json. Check in docker-entrypoint if config.json file exists. If yes, then just leave it as it is. If not, create the file from config.json.tmpl and populate with ENV variables.

@nscuro
Copy link
Member

nscuro commented Jun 13, 2023

Sounds like a sane approach to me. @Breee, WDYT?

@Breee
Copy link

Breee commented Jun 13, 2023

Sounds like a sane approach to me. @Breee, WDYT?

Yeah I agree. In addition I think that the frontend should not be broken, if there is no config, i.e. be able to handle an empty API_BASE_URL and inform that it is not configured.

@muellerst-hg muellerst-hg force-pushed the muellerst-hg-entrypoint-mount branch from efaf591 to 73ebe9d Compare June 13, 2023 11:22
@muellerst-hg muellerst-hg changed the title Improve test if static/config.json is writeable Create static/config.json from template file + ENV if not exist Jun 13, 2023
@muellerst-hg
Copy link
Contributor Author

@nscuro updated the PR branch, PR title the description accordingly. Happy merge!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

improve entrypoint
3 participants