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.

Sign up for GitHub

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

Jump to bottom

Docker image does not follow changes to MYSQL_HOST variable #2086

Open
gabriel-v opened this issue Oct 17, 2023 · 6 comments · May be fixed by #2085
Open

Docker image does not follow changes to MYSQL_HOST variable #2086

gabriel-v opened this issue Oct 17, 2023 · 6 comments · May be fixed by #2085
Labels
docs feature: auto config (environment variables) Auto configuring via environment variables integration: database Integration with any Nextcloud supported database needs review Needs confirmation this is still happening or relevant question upstream

Comments

@gabriel-v
Copy link

gabriel-v commented Oct 17, 2023 •
edited
Loading

Initially opened nextcloud/server/issues/40904, was told it's a docker issue, so here I copy over the relevant sections.

Bug description

In the "Auto configuration via environment variables" section of the Docker Readme, one can use MYSQL_HOST to set the database connection info, and NEXTCLUOD_UPDATE=1 to automate installation.

The problem is when we want to change that MySQL_HOST into something else. We change the environment variable, restart the container, and we get this error: SQLSTATE[HY000] [2006] MySQL server has gone away.

We investigate and find that, while the autoconfig.php reads environment variables, the config.php is hard-coded at installation time with the actual IP/PORTS:

<?php
$CONFIG = array (
...
  'dbhost' => '10.66.60.1:9971',
...

OK, let's try to use the CLI command to edit that hard-coded file, like this:

php occ config:system:set dbhost --value 1.2.3.4

We then get this error again: SQLSTATE[HY000] [2006] MySQL server has gone away

So the config:system:set can't fix the dbhost variable without trying to connect to the old, faulty dbhost!

That means the only option is either to manually edit the config file, to overwrite it with sed, or to overwrite it entirely (which sounds complex because it has to keep all the generated secrets already present in it).

The same issue happens with all other environment variables: db pass, user, database name, https override settings, etc.

Steps to reproduce

Trivial Docker-compose example that renames "db" into "db2": https://gist.github.com/gabriel-v/c6e5a1e18686f39649546c1161fadd64

  1. Install docker container using auto-configuration environment variable for MYSQL
  2. Change the IP/PORT of the MYSQL server
  3. Update the environment variables for auto-configuration and restart the Nextcloud server

Expected behavior

Nextcloud starts successfully with the new DB connection info

Installation method

Community Docker image

Nextcloud Server version

confirmed on 26, 27, latest docker images

Work-Around

https://github.com/nextcloud/helm/tree/main/charts/nextcloud#multiple-configphp-file

Create file config/dbhostoverride.config.php with content:

<?php
$CONFIG = array (
  'dbhost' =>  getenv('MYSQL_HOST'),
);

And add keys to this file for each value expected to change (dbport, passwords, etc.)

This solution is the cleanest, but still requires injecting templates into the image for environment variables that we already set.

Suggestions

This causes a great deal of confusion: one expects the environment variable to be the source of truth for configuring the image. Inspecting the autoconfig.php file also does not help, because it's not obvious the $AUTOCONFIG var is used only at install time, while the $CONFIG var is used at run time.

$CONFIG = array (
  'dbhost' =>  getenv('MYSQL_HOST') || getenv('POSTGRES_HOST') || ...,
  ... => ...
@gabriel-v gabriel-v linked a pull request Oct 17, 2023 that will close this issue
Open
@joshtrichards joshtrichards added docs enhancement feature: auto config (environment variables) Auto configuring via environment variables integration: database Integration with any Nextcloud supported database labels Oct 22, 2023
@manuth manuth mentioned this issue Dec 12, 2023
Closed
@pnpninja
Copy link

pnpninja commented Feb 4, 2024

i noticed this too - this needs to be fixed IMO. whats the point of passing env variables then?

@joshtrichards
Copy link
Member

This only happens with the db variables, and it's because they're injected via autoconfig.php[1] rather than as extra *.config.php files like many of the other image parameters. It does seem like maybe we could have a db.config.php that brings them in as well at runtime.

But a serious drawback comes to mind:

  • backwards compatibility: if we start injecting Compose provided db variables post-install we'll break existing environments (i.e. those that have changed their db setup in their config.php after installation but not updated their Compose environment variables to match... Because it wasn't relevant to do so)

So making the change today may be more complicated than it seems at first glance.

Given this downside - combined with the variables primarily being only for installation anyhow (not changes post-installation) - I'm not sure myself of what the path forward here would even look like in terms of risk versus reward.

[1] https://docs.nextcloud.com/server/latest/admin_manual/configuration_server/automatic_configuration.html

@gabriel-v
Copy link
Author

gabriel-v commented Mar 12, 2024 •
edited
Loading

Thank you for looking into this.

the variables primarily being only for installation anyhow (not changes post-installation)

This is not true, for example in Docker or Kubernetes the database IP changes every reboot. In Docker-Compose setups, database hostnames are changed all the time during normal development. In all setups, database passwords may be rotated.

backwards compatibility

The relevant environment variables could be renamed, the old ones deprecated. For example MYSQL_HOST could be deprecated for a couple of NC versions, and the new variable that works as expected could be called NEXTCLOUD_MYSQL_HOST.

The root cause of the issue in my opinion is lacking documentation - this limitation should be plastered in the readme with bold letters.

@martadinata666
Copy link

Is there something that prevent you to use the container name as db host rather than using IP? Docker container IP change doesn't really matter anyways. As nextcloud config can use container name as db host just fine.

@gabriel-v
Copy link
Author

gabriel-v commented Mar 12, 2024 •
edited
Loading

Is there something that prevent you to fix it

It's certainly not a difficult problem to fix once you understand where it comes from and how to mitigate it. But the realization that this docker container reads the env once but doesn't read it again comes as a shock, especially for configs that shouldn't need to be cached, my docker envs aren't going anywhere.

This only happens when you do non-trivial things (e.g. rotate your passwords, support custom db connections from user-provided strings, change hosting domain, etc). So I understand the reason for seeing this as "low reward" - if we got here all the way to successfully host the app enough time to need to change something and actually see this limitation, we're best suited to fix our problem by attaching more configuration files or overriding docker's DNS or something.

The expectation for docker images is that environment configuration is the source of truth - and when this is not the case, it's usually documented.

for example, this warning here:

https://hub.docker.com/_/postgres

Warning: the Docker specific variables will only have an effect if you start the container with a data directory that is empty; any pre-existing database will be left untouched on container startup.

Copy/paste of that is good enough to close this issue

I think Nextcloud is the only modern & popular app that's packaged on Docker with this complication that feels like it's left over from the early Apache days, all these other apps' Docker containers do follow changes in environment for basic things like the server name/location or DB connection options:

@joshtrichards joshtrichards added the needs review Needs confirmation this is still happening or relevant label Jun 19, 2024
@bean5
Copy link

bean5 commented Jul 18, 2024

It came as a surprise to me as well. Some variables can be changed, but others are ignored. I think one I tried broke my installation even though I reverted it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
docs feature: auto config (environment variables) Auto configuring via environment variables integration: database Integration with any Nextcloud supported database needs review Needs confirmation this is still happening or relevant question upstream
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

README.md - specify envs are not read after installation gabriel-v/docker
5 participants
@joshtrichards @bean5 @martadinata666 @pnpninja @gabriel-v