Using unique service db names

[josep-tecnativa]

CC @Tecnativa TT54598

When multiple Odoo projects are deployed within the same Docker network, there is a risk that an instance of Odoo may inadvertently connect to the database of another project due to identical service names. This PR addresses the issue by ensuring unique service names, preventing unintended cross-project database connections and ensuring proper project isolation.

[josep-tecnativa]

@ap-wtioit How does this look to you?

[ap-wtioit]

When multiple Odoo projects are deployed within the same Docker network, there is a risk that an instance of Odoo may inadvertently connect to the database of another project due to identical service names.

This to me seems not just a problem with db service but with all services (e.g. smtp). I'm not sure if it is wise to run multiple Odoo projects/instances in the same network. (Or are you talking about the inverseproxy_shared network only?)

What i get from the changes:

• DB_HOST is used to make sure that the odoo instance is connecting to a postgres service available under {{_key}}-db

When using this please make sure the value of _key is a valid hostname otherwise some key values might cause connection issues. E.g. _ is sometimes used in service names (and can be resolved by docker dns accessible on 127.0.0.11 inside a container within a bridge network) but a correct DNS resolvable hostname would not contain an _. Otherwise i fear that this might lead to issues later (if odoo, postgres or another tool begin to validate db_name / PG_HOST other than resolving it)

We are using a modified prod.yaml with and additional .docker/db-host.env file where PGHOST is defined and on deployment the file is created by symlinking to something like /path/to/postgres/containers/postgres_16.env. I think this should still be compatible with the proposed change.

To prevent multiple odoo (we do not have the problem with db as we are using one postgres container per DB_VERSION) from resolving to the same name in shared networks we usually use aliases and do not change the service name in docker compose:

services:
  odoo:
    ...
    networks:
      default: {}
      globalwhitelist_shared: {}
      inverseproxy_shared:
        aliases:
          - "${DB_NAME:-odoo}-${DOODBA_ENVIRONMENT:-prod}"
...

We use this for nginx providing access to Odoo from the web (instead of using traeffik).

So to sum it up, it looks ok. Only if someone already added PGHOST in db-access.env (or in our case new db-host.env) this could be a breaking change if environment: inside docker-compose.yaml (prod.yaml) wins over env files.

Edit: expected to need longer to find this. But indeed enviroment: overwrites env_file:: https://docs.docker.com/compose/how-tos/environment-variables/envvars-precedence/#simple-example

[josep-tecnativa]

When multiple Odoo projects are deployed within the same Docker network, there is a risk that an instance of Odoo may inadvertently connect to the database of another project due to identical service names.

This to me seems not just a problem with db service but with all services (e.g. smtp). I'm not sure if it is wise to run multiple Odoo projects/instances in the same network. (Or are you talking about the inverseproxy_shared network only?)

What i get from the changes:

* DB_HOST is used to make sure that the odoo instance is connecting to a postgres service available under `{{_key}}-db`

When using this please make sure the value of _key is a valid hostname otherwise some key values might cause connection issues. E.g. _ is sometimes used in service names (and can be resolved by docker dns accessible on 127.0.0.11 inside a container within a bridge network) but a correct DNS resolvable hostname would not contain an _. Otherwise i fear that this might lead to issues later (if odoo, postgres or another tool begin to validate db_name / PG_HOST other than resolving it)

We are using a modified prod.yaml with and additional .docker/db-host.env file where PGHOST is defined and on deployment the file is created by symlinking to something like /path/to/postgres/containers/postgres_16.env. I think this should still be compatible with the proposed change.

To prevent multiple odoo (we do not have the problem with db as we are using one postgres container per DB_VERSION) from resolving to the same name in shared networks we usually use aliases and do not change the service name in docker compose:

services:
  odoo:
    ...
    networks:
      default: {}
      globalwhitelist_shared: {}
      inverseproxy_shared:
        aliases:
          - "${DB_NAME:-odoo}-${DOODBA_ENVIRONMENT:-prod}"
...

We use this for nginx providing access to Odoo from the web (instead of using traeffik).

So to sum it up, it looks ok. Only if someone already added PGHOST in db-access.env (or in our case new db-host.env) this could be a breaking change if environment: inside docker-compose.yaml (prod.yaml) wins over env files.

Edit: expected to need longer to find this. But indeed enviroment: overwrites env_file:: https://docs.docker.com/compose/how-tos/environment-variables/envvars-precedence/#simple-example

Hi Andreas,

First of all, thank you for your detailed feedback and suggestions! 😊

To clarify, yes, this issue occurs specifically with the inverseproxy_shared network. It's a very odd problem because it only happens on a couple of servers, while on the rest everything works just fine.

Also, regarding your setup, we actually use one PostgreSQL container per Odoo instance, rather than having one container per PostgreSQL version. This approach helps us avoid conflicts between instances, but the shared network issue seems to still persist in those specific cases (I think it cloud be a kind of bug on Docker Compose).

Our PGHOST is currently defined here. However, I need it to change dynamically based on the unique db service name, which is the reason behind my changes.

[josep-tecnativa]

Could you please, check this out @pedrobaeza ? I think it's ready to go.

[PabloEForgeFlow]

Hey @josep-tecnativa, it seems I'm a bit late to the party, but I just noticed this change after we ran an update on one of our projects and saw that scripts were failing due to the db service not existing anymore.

I personally believe that compose service names (together with volume and network names) should be treated as part of the template's "API" and should remain stable throughout (perhaps with the exception of major releases).

I've tried to replicate in various ways the issue this PR address and haven't been able to. You mentioned the issue only appears in a couple of servers, and just like @ap-wtioit said this does not appear to be a problem exclusive to the database service. Perhaps you could provide additional information on the specific setups that cause the issue so that we could address the compose issue instead?

In the meantime, do you think this same fix could be achieved by using network aliases instead of outright changing the default name of all database services? That way the fix would keep working while preventing potential disruptions to user scripts depending on the name of the database service.

[Tardo]

For me, these changes break databases... :(

[pedrobaeza]

For the record, this change doesn't break any database. If you do copier update on your scaffolding, your deployment tools may be affected, but nothing more.

The problem we faced was real, and reproduced already in 2 servers, having docker compose mixing the DBs from other versions of the same scaffolding (demo and prod for example). The conditions to get it are not clear though.

The options you have (apart from forking or freeze the copier updates, which I think are not adequate) are:

• Adapt your tooling to this change. We needed to do it, but they have been very few changes.
• Propose an alternative for having unique names for not having that mix. The network thing may work, but it seems everything gets more convoluted.

[Tardo]

If I'm the only one with these problems... it must be because of how we update... What happened to me is that I got a PANIC: could not locate a valid checkpoint record.
I had to restore a copy, since I have not been able to repair the checkpoint.

[pedrobaeza]

This is something only at docker compose level and how the DB service is called, so the checkpoint you are talking about should be another thing, and more if you didn't even update the scaffolding.

[Tardo]

I have to investigate more what has happened to us... :/
It happened with the latest changes, but since we did not "attack" this repository, I was able to revert these changes and update again.

If I find anything that I think may be relevant to here I will comment :)