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; https://github.com/getsentry/self-hosted/blob/8d2da229a2264e31790f736f31d2f6c3a31d0c50/install/set-up-and-migrate-database.sh#L11-L12

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"

[BYK]

Why can we not run the web image separately with shell which will give us a Django shell and run a script there?

[puittenbroek]

Why can we not run the web image separately with shell which will give us a Django shell and run a script there?

That would keep it more agnostic for sure. It would require the 'script' to be python. Which it currently isn't. Not sure how difficult it is to write alter table/drop index in Django shell.

[BYK]

Looks pretty straightforward to me: https://docs.djangoproject.com/en/5.0/topics/db/sql/#executing-custom-sql-directly

[aldy505]

Burak's suggestion sounds even better 😄

[milkomeda]

Same problem here and currently stuck on 24.1.1.

[roock]

Same problem here and currently stuck on 24.1.1.

@milkomeda You can execute the two commands manually in your sentry postgresql database and the upgrade should run fine.

[SeanSith]

Make sure you also run the migrations in the web container later in that script, too.

On Sat, Feb 24, 2024 at 5:38 PM Roman Pertl @.***> wrote:

Same problem here and currently stuck on 24.1.1.

@milkomeda https://github.com/milkomeda You can execute the two commands manually in your sentry postgresql database and the upgrade should run fine.

— Reply to this email directly, view it on GitHub https://github.com/getsentry/self-hosted/issues/2804#issuecomment-1962748170, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABGVHAQHJEXRGVTSXA3THNLYVJTU5AVCNFSM6AAAAABDPPZ752VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSNRSG42DQMJXGA . You are receiving this because you are subscribed to this thread.Message ID: @.***>

[milkomeda]

I renamed our database from sentry to postgres. Afterwards update to 24.2.0 was successful.

I don't want to search for any manual commands yet or in the future. :upside_down_face:

[hubertdeng123]

Good suggestion, I've put up a PR to use the django orm to perform the DROP sql commands.

https://github.com/getsentry/self-hosted/pull/2827

[puittenbroek]

Seeing https://github.com/getsentry/self-hosted/pull/2827 is merged, I consider my issue resolved :) Much appreciated @BYK @hubertdeng123 !