Switch to PostgreSQL 16 image#577
Conversation
|
Extracted out of #395 |
|
Because #395 has both postgres and valkey in the same PR? |
|
I see the tests are failing on password encryption algorithm detection: https://github.com/theforeman/foremanctl/actions/runs/27768839212/job/82162895170?pr=577#step:17:643 Which is weird. |
f9dc89d to
f6f88a0
Compare
It's not weird, as that was the ext DB and that wasn't using our image definition properly. Fixed. |
| ansible.builtin.include_role: | ||
| name: debug_tools | ||
|
|
||
| - name: 'Enable postgresql:16 module' |
There was a problem hiding this comment.
@evgeni, let me ask this question again: wouldn't it be more resilient to run the PG commands from inside a PG container instead of installing the client tooling directly on the host?
There was a problem hiding this comment.
Would, sure. I just had none that I could integrate easily into the various workflows where we need Postgres tooling on the host.
There was a problem hiding this comment.
You'd likely want to write a module that does this to be re-used because there are postgres actions that are run in a lot more places. I am not sure I see the value of it at the moment but it might be worth exploring after we get everything else.
There was a problem hiding this comment.
@ehelms Debian 13 has PostgreSQL 17. That probably means you can't run pg_dump on the PostgreSQL 16 container we run, which breaks backups.
|
The upgrade complains: The fact its supporting 15→16 only is encoded here: Funnily, the container actually contains 13 binaries: I'd argue this is a bug in the image we should ask for a fix. |
| - "../../../src/vars/defaults.yml" | ||
| - "../../../src/vars/flavors/{{ flavor }}.yml" | ||
| - "../../../src/vars/database.yml" | ||
| - "../../../src/vars/images.yml" |
| notify: | ||
| - Restart postgresql | ||
|
|
||
| - name: Check if PG_VERSION file exists |
There was a problem hiding this comment.
This set of upgrade tasks look good. Could they go in a dedicated task file?
| ansible.builtin.systemd: | ||
| name: postgresql.service | ||
| masked: false | ||
| when: |
There was a problem hiding this comment.
As this when pattern is repeated, you can put it in a block.
ekohl
left a comment
There was a problem hiding this comment.
Wouldn't it be easier to unconditionally set POSTGRESQL_UPGRADE so it will always run when needed?
Thinking about the logging aspect: where do the logs of the upgrade go now? Can we somehow debug it when it fails? When it's part of the regular service then you'll have it the journal and sosreport will pick it up already.
| POSTGRESQL_MAX_CONNECTIONS: "{{ postgresql_max_connections }}" | ||
| POSTGRESQL_SHARED_BUFFERS: "{{ postgresql_shared_buffers }}" | ||
| POSTGRESQL_EFFECTIVE_CACHE_SIZE: "{{ postgresql_effective_cache_size }}" | ||
| POSTGRESQL_PREV_VERSION: "{{ postgresql_version }}" |
There was a problem hiding this comment.
Please link why this workaround is needed
| POSTGRESQL_PREV_VERSION: "{{ postgresql_version }}" | |
| # https://github.com/sclorg/postgresql-container/issues/661 | |
| POSTGRESQL_PREV_VERSION: "{{ postgresql_version }}" |
| POSTGRESQL_SHARED_BUFFERS: "{{ postgresql_shared_buffers }}" | ||
| POSTGRESQL_EFFECTIVE_CACHE_SIZE: "{{ postgresql_effective_cache_size }}" | ||
| POSTGRESQL_PREV_VERSION: "{{ postgresql_version }}" | ||
| POSTGRESQL_UPGRADE: "hardlink" |
There was a problem hiding this comment.
Can you please make this a variable so we could easily switch to copy?
| - "postgresql_version == '13'" | ||
| register: postgresql_upgrade_result | ||
|
|
||
| - name: Stop PostgreSQL systemd service (Quadlet) |
There was a problem hiding this comment.
Why is it needed to stop the service again?
Why are you introducing these changes? (Problem description, related links)
As part of being prepared to run on RHEL 10, we need to switch to postgres 16.
What are the changes introduced in this pull request?
How to test this pull request
If the system spins up successfully and works with content - it's good to go.