set-up-and-migrate-database.sh uses hard exec into container

[puittenbroek]

Self-Hosted Version

24.2.0

CPU Architecture

x86_64

Docker Version

25.0.3

Docker Compose Version

v2.23.3

Steps to Reproduce

Two ways this will not work:

1. Use an 'off site' postgres configuring DATABASES in sentry.conf.py
2. Use different user then postgres in postgres (also via sentry.conf.py and POSTGRES_USER env)

For the 'off site postgres' you could argue it's not a 'supported feature'; but without recent changes it worked fine.

To limit resource usage we also disabled the postgres service by using a docker-compose.override.yml:

services:
  postgres:
    restart: ''
    image: alpine
    command: sh -c "while true; do sleep 60; done"
    entrypoint: [] 
    healthcheck:
      test: 'echo "Running"'
      start_period: 0s

In sentry.conf.py:

DATABASES = {
    "default": {
        "ENGINE": "sentry.db.postgres",
        "NAME": "sentry",
        "USER": "sentry",
        "PASSWORD": "superseret",
        "HOST": "somehost.ofours.com",
        "PORT": "1337",
    }
}

install.sh wlll fail because it cannot exe into the postgres service here;

self-hosted/install/set-up-and-migrate-database.sh

Lines 11 to 12 in 8d2da22

	$dc exec postgres psql -qAt -U postgres -c "ALTER TABLE IF EXISTS sentry_groupedmessage DROP CONSTRAINT IF EXISTS sentry_groupedmessage_project_id_id_515aaa7e_uniq;"
	$dc exec postgres psql -qAt -U postgres -c "DROP INDEX IF EXISTS sentry_groupedmessage_project_id_id_515aaa7e_uniq;"

Expected Result

• Using Django to execute database changes
• Not using 'hard coded' exec like this.

I understand this is due to a Django bug and is (being?) fixed. So I wonder if recent changes in install/set-up-and-migrate-database.sh are still needed.

Actual Result

▶ Setting up / migrating database ...
 Container sentry-self-hosted-postgres-1  Creating
 Container sentry-self-hosted-postgres-1  Created
 Container sentry-self-hosted-postgres-1  Starting
 Container sentry-self-hosted-postgres-1  Started
Waiting for postgres server, 5 remaining attempts...
Waiting for postgres server, 4 remaining attempts...
Waiting for postgres server, 3 remaining attempts...
Waiting for postgres server, 2 remaining attempts...
Waiting for postgres server, 1 remaining attempts...
OCI runtime exec failed: exec failed: unable to start container process: exec: "psql": executable file not found in $PATH: unknown
Error in install/set-up-and-migrate-database.sh:11.
'$dc exec postgres psql -qAt -U postgres -c "ALTER TABLE IF EXISTS sentry_groupedmessage DROP CONSTRAINT IF EXISTS sentry_groupedmessage_project_id_id_515aaa7e_uniq;"' exited with status 126
-> ./install.sh:main:35
--> install/set-up-and-migrate-database.sh:source:11

Event ID

No response

[roock]

Same issue also affected us because we use a non-default database name (sentry instead of psql).

[aldy505]

The argument from the employees would be "if you're using custom configuration, you should be able to modify those to your needs". But I totally get what you're experiencing.

One thing that we can do is either provide a wrapper script that decides an execution of some scripts based on a configuration that indicate whether those things that interact with "on-site" dependency (Kafka, Zookeeper, ClickHouse, Memcached, Redis, Postgres) would be executed directly -- as per the normal docker-compose.yml file, or not -- meaning users have to execute them manually.

The configuration might resides on the .env file, perhaps something like DISABLE_DIRECT_DEPENDENCY_EXECUTION=true. The install scripts can then read that variable, and decides whether it'll execute any scripts directly.

@puittenbroek What do you think about this?

@hubertdeng123 @azaslavsky Need your feedback on this too, is something like this acceptable?

[puittenbroek]

Such a wrapper script does sounds promising, but having a boolean value for "all" dependencies won't cover it.
For example for our situation, we only have a off-site Postgres, everything else is "on-site" like Kafka.

Don't think you want to do this with a string value either, since that makes checking it in (bash) scripts a PITA.
Perhaps a boolean per dependency?
Makes it more explicit and easier to manage:

DISABLE_DIRECT_DEPENDENCY_EXECUTION_POSTGRES=true
DISABLE_DIRECT_DEPENDENCY_EXECUTION_REDIS=false
etc..

Does mean that if you customize, you have to be VERY alert on execution steps you have to do manually.
This time the install script failed quite hard, which pointed me in the right direction.
But with such a wrapper script it will simply decide not to try and execute the DROP constrains and continue.
Possibly causing Django migrations to fail or in worst case; break at a later stage.

But let's not over-complicate things haha. IMHO:

• boolean per dependency to avoid execution
• wrapper script(s) of some kind to decide on execution or skipping
• if skipping: perhaps a warning "skipping execution on dependency XXXX; DO IT MANUALLY"