Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion src/roles/postgresql/defaults/main.yml
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
---
postgresql_container_image: quay.io/sclorg/postgresql-13-c9s
postgresql_container_image: quay.io/sclorg/postgresql-16-c10s
postgresql_container_tag: "latest"
postgresql_container_name: postgresql
postgresql_network: host
Expand All @@ -12,3 +12,5 @@ postgresql_admin_password: "{{ undef(hint='Set a secure database password') }}"
postgresql_max_connections: 500
postgresql_shared_buffers: 512MB
postgresql_effective_cache_size: 1GB

postgresql_upgrade: "hardlink"
15 changes: 3 additions & 12 deletions src/roles/postgresql/tasks/main.yml
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,9 @@
notify:
- Restart postgresql

- name: Upgrade PostgreSQL database
ansible.builtin.include_tasks: upgrade.yaml

- name: Deploy PostgreSQL container
containers.podman.podman_container:
name: "{{ postgresql_container_name }}"
Expand Down Expand Up @@ -107,18 +110,6 @@
name: "{{ postgresql_container_name }}"
state: started

# SCRAM-SHA-256 is default for PostgreSQL 14+,
# after the upgrade, we can drop this task.
- name: Use scram-sha-256 for password encryption
community.postgresql.postgresql_set:
login_user: postgres
login_password: "{{ postgresql_admin_password }}"
login_host: localhost
name: password_encryption
value: "scram-sha-256"
notify:
- Restart postgresql

- name: Create PostgreSQL users
community.postgresql.postgresql_user:
name: "{{ item.name }}"
Expand Down
79 changes: 79 additions & 0 deletions src/roles/postgresql/tasks/upgrade.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,79 @@
---
- name: Check if PG_VERSION file exists
ansible.builtin.stat:
path: "{{ postgresql_data_dir }}/userdata/PG_VERSION"
register: postgresql_version_file

- name: Set postgresql_version version fact
when: postgresql_version_file.stat.exists
block:
- name: Read PG_VERSION file
ansible.builtin.slurp:
src: "{{ postgresql_data_dir }}/userdata/PG_VERSION"
register: postgresql_version_encoded

- name: Register PG version fact
ansible.builtin.set_fact:
postgresql_version: "{{ postgresql_version_encoded.content | b64decode | trim }}"

- name: Upgrade PostgreSQL 13 to 16
when:
- "postgresql_version is defined"
- "postgresql_version == '13'"
block:
- name: Stop foreman target
ansible.builtin.systemd:
name: foreman.target
state: stopped

- name: Wait for port 5432 to be completely freed by Podman
ansible.builtin.wait_for:
port: 5432
state: stopped
timeout: 30

- name: Deploy PostgreSQL upgrade container
Comment thread
ekohl marked this conversation as resolved.
containers.podman.podman_container:
name: "{{ postgresql_container_name }}-upgrade"
image: postgresql.image
state: quadlet

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Does it need to be a quadlet? In favor of it means you get logs in the journal, but it's also fragile: if Ansible somehow starts, it can leave the system in an unpredictable state.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

if Ansible somehow starts, it can leave the system in an unpredictable state.

I don't follow this logic. However, I assume you are asking the question of a quadlet which generates a systemd service vs. just running the container directly with the equivalent of a podman run?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Does it need to be a quadlet?

I had the same question. I decided to go on with a quadlet to reuse the postgres.image container. If I spin it up as a separate container, it means I cannot reuse the image definition and any drop-ins that will be defined there. Originally it was a regular container.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

And I messed up: I intended to write "Ansible somehow stops in the middle". Not start.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I don't think it is disastrous:

  • if the playbook stops before defining the service - no harm done
  • if it stops after the service is defined, but before it executes the service - it will be executed at first reboot
  • if it stops after the execution - it may leave some leftovers, but they would be harmless

There are options to add the upgrade as a dependency for the postgres.container, so it will pull the service before the execution and then push the condition into the service startup, so it will just silently finish with noop in case the DB was already migrated.

I think it's too complex for a one time only step.

sdnotify: false
volumes:
- "{{ postgresql_data_dir }}:/var/lib/pgsql/data:rw,Z"
secrets:
- 'postgresql-admin-password,target=POSTGRESQL_ADMIN_PASSWORD,type=env'
env:
POSTGRESQL_MAX_CONNECTIONS: "{{ postgresql_max_connections }}"
POSTGRESQL_SHARED_BUFFERS: "{{ postgresql_shared_buffers }}"
POSTGRESQL_EFFECTIVE_CACHE_SIZE: "{{ postgresql_effective_cache_size }}"
# explicitly set the previous version to avoid the upgrade failure
# see https://github.com/sclorg/postgresql-container/issues/661
POSTGRESQL_PREV_VERSION: "{{ postgresql_version }}"
POSTGRESQL_UPGRADE: "{{ postgresql_upgrade }}"
command: "run-postgresql -V"
quadlet_options:
- |
[Unit]
Description=PostgreSQL Database Upgrade from 13 to 16

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Perhaps make sure that systemd knows it's 1 or the other? Or will this introduce too much uncertainty?

Suggested change
Description=PostgreSQL Database Upgrade from 13 to 16
Description=PostgreSQL Database Upgrade from 13 to 16
Conflicts=postgresql.service

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

According to Gemini, the directive is symmetric, so it won't protect us from someone that tries to start the server. Actually it will just cause the upgrade container to stop (even more problematic in the middle of an upgrade).

[Service]
Type=oneshot
RemainAfterExit=yes

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This means the service will remain as active. Is that useful?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

It will mark the service as active(exited) when it will stop the execution. In the happy path it won't matter, since we remove the service completely, but if it fails, we will have indication whether it was run or not.

TimeoutStartSec=30m
- name: Run daemon reload to make Quadlet create the upgrade service file
ansible.builtin.systemd:
daemon_reload: true

- name: Upgrade the database from 13 to 16 but don't start the server
ansible.builtin.systemd:
name: "{{ postgresql_container_name }}-upgrade.service"
state: restarted

- name: Remove the quadlet file
ansible.builtin.file:
path: "/etc/containers/systemd/{{ postgresql_container_name }}-upgrade.container"
state: absent

- name: Reload systemd
ansible.builtin.systemd_service:
daemon_reload: true
11 changes: 11 additions & 0 deletions src/roles/pre_install/tasks/main.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,17 @@
ansible.builtin.include_role:
name: debug_tools

- name: 'Enable postgresql:16 module'

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

@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?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Would, sure. I just had none that I could integrate easily into the various workflows where we need Postgres tooling on the host.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

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.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@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.

ansible.builtin.command: dnf module enable -y postgresql:16
# can't use the `dnf` module for modules without a default stream
# https://github.com/ansible/ansible/issues/56504
# https://github.com/ansible/ansible/issues/64852
args:
creates: /etc/dnf/modules.d/postgresql.module
when:
- ansible_facts['os_family'] == 'RedHat'
- ansible_facts['distribution_major_version'] == '9'

- name: Install podman and utilities
ansible.builtin.package:
name:
Expand Down
2 changes: 1 addition & 1 deletion src/vars/images.yml
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ foreman_container_tag: "{{ container_tag_stream }}"
foreman_proxy_container_image: "quay.io/foreman/foreman-proxy"
foreman_proxy_container_tag: "{{ container_tag_stream }}"

postgresql_container_image: quay.io/sclorg/postgresql-13-c9s
postgresql_container_image: quay.io/sclorg/postgresql-16-c10s
postgresql_container_tag: "latest"
pulp_container_image: quay.io/foreman/pulp
pulp_container_tag: "foreman-{{ container_tag_stream }}"
Expand Down