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

deps: update postgres docker tag to v16 #72

Merged
merged 2 commits into from
Jun 18, 2024

Conversation

eclipse-apoapsis-bot
Copy link
Contributor

@eclipse-apoapsis-bot eclipse-apoapsis-bot commented Apr 10, 2024

This PR contains the following updates:

Package Update Change
postgres major 14 -> 16

Configuration

📅 Schedule: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined).

🚦 Automerge: Disabled by config. Please merge this manually once you are satisfied.

Rebasing: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox.

🔕 Ignore: Close this PR and you won't be reminded about this update again.


  • If you want to rebase/retry this PR, check this box

This PR has been generated by Renovate Bot.

@eclipse-apoapsis-bot eclipse-apoapsis-bot added the dependencies Dependency updates. label Apr 19, 2024
@eclipse-apoapsis-bot eclipse-apoapsis-bot force-pushed the renovate-action/postgres-16.x branch from 07c5e3c to c76d926 Compare April 19, 2024 15:06
@eclipse-apoapsis-bot eclipse-apoapsis-bot force-pushed the renovate-action/postgres-16.x branch from c76d926 to ebf3539 Compare May 3, 2024 08:08
@eclipse-apoapsis-bot eclipse-apoapsis-bot force-pushed the renovate-action/postgres-16.x branch 2 times, most recently from 8dbb660 to 49fa1ec Compare May 29, 2024 11:50
Copy link
Contributor

@mnonnenmacher mnonnenmacher left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will break any existing local databases as PostgreSQL cannot read the data directory of previous major versions. I want to add documentation for manual migration of the database before merging this, so I'm blocking it for now.

@mmurto
Copy link
Contributor

mmurto commented Jun 14, 2024

This will break any existing local databases as PostgreSQL cannot read the data directory of previous major versions. I want to add documentation for manual migration of the database before merging this, so I'm blocking it for now.

I think a good solution to this migration problem would be to rather create a set of test data and include the necessary scripts to automatically seed the compose database with this. It would also help in E2E testing, if we want to go there at some point.

@mnonnenmacher
Copy link
Contributor

This will break any existing local databases as PostgreSQL cannot read the data directory of previous major versions. I want to add documentation for manual migration of the database before merging this, so I'm blocking it for now.

I think a good solution to this migration problem would be to rather create a set of test data and include the necessary scripts to automatically seed the compose database with this. It would also help in E2E testing, if we want to go there at some point.

That would be useful but I have lots of scan results for custom tests in my local database that I do not want lose and it's probably the same for others.

@mmurto
Copy link
Contributor

mmurto commented Jun 14, 2024

This will break any existing local databases as PostgreSQL cannot read the data directory of previous major versions. I want to add documentation for manual migration of the database before merging this, so I'm blocking it for now.

I think a good solution to this migration problem would be to rather create a set of test data and include the necessary scripts to automatically seed the compose database with this. It would also help in E2E testing, if we want to go there at some point.

That would be useful but I have lots of scan results for custom tests in my local database that I do not want lose and it's probably the same for others.

It works mostly like this, but I'm having some troubles with the admin user not having the admin functionality in the UI after the migration. For this kind of problems (and to make the dev environment generally cleaner) we should probably use a separate database for Keycloak, so don't store all the Keycloak tables in the ort_server database. Feel free to debug, I may come back to this some time.

Dump the database from Postgres 14:

pg_dump -Fc -v -d postgres://postgres:postgres@localhost:5433/ort_server -f mydumpfile.bak

Bring the Compose environment down and destroy the volumes to reset the database:

docker compose down -v

Update the postgres image to 16 in docker-compose.yml and restart the environment:

docker compose up -d

Restore the database from the dump to the updated Postgres:

pg_restore --clean -v -d postgres://postgres:postgres@localhost:5433/ort_server mydumpfile.bak

@mmurto
Copy link
Contributor

mmurto commented Jun 14, 2024

Keycloak seems to use public schema and ORT Server the ort_server schema, so this could work by only dumping the ort_server schema and otherwise going through the motions above. So use this for the first command:

pg_dump -Fc -v -d -n ort_server postgres://postgres:postgres@localhost:5433/ort_server -f mydumpfile.bak

@mmurto
Copy link
Contributor

mmurto commented Jun 14, 2024

Keycloak seems to use public schema and ORT Server the ort_server schema, so this could work by only dumping the ort_server schema and otherwise going through the motions above. So use this for the first command:

pg_dump -Fc -v -d -n ort_server postgres://postgres:postgres@localhost:5433/ort_server -f mydumpfile.bak

...though then the permissions etc won't migrate, so it doesn't work that way.

@mnonnenmacher mnonnenmacher force-pushed the renovate-action/postgres-16.x branch from 49fa1ec to 0df4bc6 Compare June 18, 2024 15:03
@mnonnenmacher mnonnenmacher dismissed their stale review June 18, 2024 15:05

Added the migration guide to this PR.

@eclipse-apoapsis-bot
Copy link
Contributor Author

Edited/Blocked Notification

Renovate will not automatically rebase this PR, because it does not recognize the last commit author and assumes somebody else may have edited the PR.

You can manually request rebase by checking the rebase/retry box above.

⚠️ Warning: custom changes will be lost.

@mnonnenmacher
Copy link
Contributor

mnonnenmacher commented Jun 18, 2024

@mmurto I have added a commit with a migration guide using basically the commands you provided, I have only changed them to run pg_dump and pg_restore inside the container so that the commands do not have to be installed on the host system.

@mnonnenmacher mnonnenmacher enabled auto-merge June 18, 2024 15:09
README.md Outdated Show resolved Hide resolved
Comment on lines +148 to +149
When upgrading the PostgreSQL version in the Docker Compose setup, the database data directory must be manually
migrated by following these steps:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't this be a single line according to conventions?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This file was not yet migrated to use one sentence per line so I wanted to keep it consistent.

README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated
- Make a backup of the ORT Server and Keycloak databases:

```shell
docker compose exec postgres pg_dump -Fc -U postgres -d ort_server -n public > keycloak.dump
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Side remark: IMO it's a bit weird that Keycloak does not use a more speaking schema than "public".

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I noticed that as well but didn't want to change it as part of this PR.

README.md Outdated Show resolved Hide resolved
Add a migration guide for PostgreSQL major version updates in the Docker
Compose environment.

Signed-off-by: Martin Nonnenmacher <martin.nonnenmacher@bosch.com>
@mnonnenmacher mnonnenmacher force-pushed the renovate-action/postgres-16.x branch from 0df4bc6 to dc09be6 Compare June 18, 2024 15:49
@mnonnenmacher mnonnenmacher requested a review from sschuberth June 18, 2024 15:50
@mmurto
Copy link
Contributor

mmurto commented Jun 18, 2024

@mmurto I have added a commit with a migration guide using basically the commands you provided, I have only changed them to run pg_dump and pg_restore inside the container so that the commands do not have to be installed on the host system.

Should we create a DEVELOPMENT.md or something similar? Probably not urgent, but longer term that's probably not information that we need to have prominently visible in readme, as it probably only happens once a year at most and is only relevant for a small group of people.

@mnonnenmacher mnonnenmacher added this pull request to the merge queue Jun 18, 2024
Merged via the queue into main with commit e7d9508 Jun 18, 2024
12 checks passed
@mnonnenmacher mnonnenmacher deleted the renovate-action/postgres-16.x branch June 18, 2024 16:16
@mnonnenmacher
Copy link
Contributor

@mmurto I have added a commit with a migration guide using basically the commands you provided, I have only changed them to run pg_dump and pg_restore inside the container so that the commands do not have to be installed on the host system.

Should we create a DEVELOPMENT.md or something similar? Probably not urgent, but longer term that's probably not information that we need to have prominently visible in readme, as it probably only happens once a year at most and is only relevant for a small group of people.

Restructuring of the documentation will happen as part of rendering it as a website, similar to how it's done for ORT. I want to start working on that soon, in the meantime you could provide feedback to #256 if you want.

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

Successfully merging this pull request may close these issues.

4 participants