Skip to content

Add init container to backup management pod to fix PV permissions#4

Open
ssimpson89 wants to merge 1 commit intoctrliq:develfrom
ssimpson89:fix/backup-restore-pv-permissions
Open

Add init container to backup management pod to fix PV permissions#4
ssimpson89 wants to merge 1 commit intoctrliq:develfrom
ssimpson89:fix/backup-restore-pv-permissions

Conversation

@ssimpson89
Copy link
Copy Markdown

Storage providers like Cinder and Longhorn create PVs with 755 root:root permissions. The backup management pod runs postgres as uid 26, which cannot write to the backup volume. Add an init container that runs as root to chown/chmod the backup mount before the main container starts.

assisted-by Claude

Storage providers like Cinder and Longhorn create PVs with 755 root:root
permissions. The backup management pod runs postgres as uid 26, which
cannot write to the backup volume. Add an init container that runs as
root to chown/chmod the backup mount before the main container starts.

Modeled after the existing postgres StatefulSet init container pattern
(postgres_data_volume_init) and upstream awx-operator PRs #1805/#1998.

Claude assisted-by
Copilot AI review requested due to automatic review settings March 20, 2026 22:35
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Adds a non-OpenShift-only init container to the backup management Pod to adjust backup PV ownership/permissions so the Postgres process (uid 26) can write to /backups on common storage providers that default to root:root 755.

Changes:

  • Add an init container (runs as root) to roles/backup management pod template to chown/chmod the /backups mount before the main container starts (non-OpenShift only).
  • Introduce backup_init_container_commands defaults to control the permission-fix commands.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File Description
roles/backup/templates/management-pod.yml.j2 Adds init container to fix /backups PV permissions on non-OpenShift clusters.
roles/backup/defaults/main.yml Adds default init-container commands for chown/chmod of /backups.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +10 to +16
{% if not is_openshift %}
initContainers:
- name: init
image: "{{ _postgres_image }}"
imagePullPolicy: "{{ image_pull_policy }}"
securityContext:
runAsUser: 0
Copy link

Copilot AI Mar 20, 2026

Choose a reason for hiding this comment

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

initContainers is always added for non-OpenShift clusters and runs as UID 0. This can prevent the pod from being admitted in namespaces enforcing PodSecurity "restricted" (requires non-root) and can also hard-fail on volumes that don’t permit chown/chmod (e.g., root-squash NFS) even when the volume is already writable. Consider gating this init container behind a dedicated boolean (similar to postgres_data_volume_init) and/or only rendering it when explicitly enabled so operators can disable it in restricted environments.

Copilot uses AI. Check for mistakes.
memory: "32Mi"
# Allow additional parameters to be added to the pg_dump backup command
# Init container commands to fix PV permissions for non-OpenShift clusters
backup_init_container_commands: |
Copy link

Copilot AI Mar 20, 2026

Choose a reason for hiding this comment

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

The init container script currently doesn’t fail fast: if chown fails but chmod succeeds, /bin/sh -c will exit 0 and the main container will start with unchanged ownership. Add a fail-fast mode (e.g., start the script with set -e or otherwise ensure failures propagate) so permission problems are surfaced immediately.

Suggested change
backup_init_container_commands: |
backup_init_container_commands: |
set -e

Copilot uses AI. Check for mistakes.
Comment on lines 39 to +44
# Allow additional parameters to be added to the pg_dump backup command
# Init container commands to fix PV permissions for non-OpenShift clusters
backup_init_container_commands: |
chown 26:0 /backups
chmod 700 /backups

Copy link

Copilot AI Mar 20, 2026

Choose a reason for hiding this comment

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

The comment about allowing additional pg_dump parameters is now separated from the pg_dump_suffix setting by the new init-container variable, making it misleading. Move or update the comment so it clearly documents the variable it describes.

Suggested change
# Allow additional parameters to be added to the pg_dump backup command
# Init container commands to fix PV permissions for non-OpenShift clusters
backup_init_container_commands: |
chown 26:0 /backups
chmod 700 /backups
# Init container commands to fix PV permissions for non-OpenShift clusters
backup_init_container_commands: |
chown 26:0 /backups
chmod 700 /backups
# Allow additional parameters to be added to the pg_dump backup command

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants