Add incremental backup and restore#609
Conversation
Implements offline backup restore functionality with the following features: Validation: - Metadata-driven validation for different flavors - Verifies backup directory structure and required files - Validates database dumps and foremanctl state archive exist - Supports dry-run mode to validate without restoring Restore Process: - Stops all Foreman services cleanly - Restores databases using pg_restore with postgres user - Fixes database ownership after restore - Restores Pulp content and encryption keys - Restores foremanctl state (OAuth keys, passwords, configuration) - Integrates deploy roles to regenerate configuration - Verifies services after deployment Error Handling: - Rescue block stops services on failure - Leaves system in safe stopped state for investigation - Provides clear recovery instructions Implementation Details: - Derives expected files from backup metadata instead of hardcoding - Uses regex append for file list generation - Removes state tracking variables for simplicity - No debug messages per codebase patterns - Explicit PostgreSQL stop to handle systemd dependencies Documentation: - Comprehensive user guide at docs/user/restore.md - Covers usage, prerequisites, troubleshooting, and best practices
Chyenne8
force-pushed
the
incremental-backup-restore
branch
from
5ebe24f to
2ecaa0d
Compare
Implements full incremental backup/restore capability using GNU tar's --listed-incremental mechanism. Reduces backup time and storage for large Pulp deployments by only backing up changed files. Backup features: Add --incremental CLI parameter: foremanctl backup /backups --incremental /backups/foreman-backup-TIMESTAMP Implementation: - Modified tar commands to use --listed-incremental with .snar snapshot files - .config.snar tracks foremanctl state changes - .pulp.snar tracks Pulp content changes - Replaced community.general.archive with raw tar for --listed-incremental support - Copy .snar files from previous backup before tar runs - Validate previous backup exists and contains .snar files - Enhanced error messages suggest alternative backups when .snar missing - Metadata records: is_incremental, base_backup_dir, base_backup_timestamp Error handling automation: - Scans parent directory for backups with .snar support - Lists available alternatives with type and timestamp - Provides ready-to-use command with suggested backup Restore features: Auto-detect incremental backups from metadata: - Parse metadata.yml to identify backup type - Display full vs incremental in validation output Backup chain validation: - Verify base backup directory exists - Confirm base backup metadata matches expected timestamp - Check if base backup already restored via .last_restore_timestamp - Fail with clear instructions if chain incomplete Add --restore-chain for automatic chain restore: foremanctl restore /backups/foreman-backup-TIMESTAMP --restore-chain Chain restore automation: - Recursively walks metadata to build full dependency chain - Restores backups in chronological order (full -> inc1 -> inc2) - Databases restored only from full backup - Incrementals apply file changes using --listed-incremental=/dev/null - Eliminates manual multi-step restore process Handle .snar files during restore: - Detect .snar presence to identify incremental archives - Use tar --listed-incremental=/dev/null for extraction - Fall back to unarchive module for full backups Usage examples: Create full backup (generates .snar files): foremanctl backup /backups Create incremental backup: foremanctl backup /backups --incremental /backups/foreman-backup-20260629T120000 Restore with auto-chain (recommended): foremanctl restore /backups/foreman-backup-20260701T080000 --restore-chain Manual chain restore: foremanctl restore /backups/foreman-backup-20260629T120000 foremanctl restore /backups/foreman-backup-20260630T080000 --force foremanctl restore /backups/foreman-backup-20260701T080000 --force Validate incremental backup: foremanctl restore /backups/foreman-backup-TIMESTAMP --validate Testing: Backup: - Full backup creates .config.snar and .pulp.snar files - Incremental backup significantly smaller than full - Error with suggestions when .snar files missing - Metadata correctly tracks incremental status Restore: - Incremental detected from metadata - Chain validation prevents incomplete restores - --restore-chain automatically handles dependencies - Clear errors for missing base backups Files modified: Backup: - src/playbooks/backup/metadata.obsah.yaml - src/roles/backup/tasks/main.yaml - src/roles/backup/tasks/pulp_content.yaml - src/roles/backup/tasks/metadata.yaml - docs/user/backup.md Restore: - src/playbooks/restore/metadata.obsah.yaml - src/roles/restore/tasks/validate.yaml - src/roles/restore/tasks/main.yaml - src/roles/restore/tasks/restore_pulp_content.yaml - src/roles/restore/tasks/restore_foremanctl_state.yaml - src/roles/restore/tasks/restore_databases.yaml - src/roles/restore/tasks/restore_chain.yaml (new) - src/roles/restore/tasks/restore_chain_walk.yaml (new) - src/roles/restore/tasks/restore_single.yaml (new) - docs/user/restore.md
Chyenne8
force-pushed
the
incremental-backup-restore
branch
from
2ecaa0d to
bd5f415
Compare
|
I like the idea here: |
|
One note about: There is some difference between command structure with foreman-maintain for incremental backups here: https://docs.theforeman.org/nightly/Administering_Project/index-katello.html#Performing_an_Incremental_Backup_admin We'd like to stay close to the foreman-maintain command. |
|
@coderabbitai review |
✅ Action performedReview finished.
|
WalkthroughThis PR adds incremental backup support to the offline backup playbook (via a new Estimated code review effort: 4 (Complex) | ~75 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
🧹 Nitpick comments (4)
src/roles/restore/tasks/validate.yaml (1)
56-60: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicated, convoluted incremental-detection logic.
The same fallback expression (
incremental.is_incremental | default(incremental | default(false))) is duplicated verbatim inrestore_chain_walk.yaml(lines 11-14). Consider extracting this into a small sharedinclude_tasks(e.g.parse_backup_metadata.yaml) that both files call, to avoid the two copies drifting if the metadata schema changes.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/roles/restore/tasks/validate.yaml` around lines 56 - 60, The incremental backup detection logic in the restore metadata parsing is duplicated and should be centralized. Move the repeated fallback expression used by the restore task’s set_fact block into a shared include_tasks helper such as parse_backup_metadata.yaml, then have both validate.yaml and restore_chain_walk.yaml call that shared task. Keep the logic for restore_is_incremental, restore_base_backup_dir, and restore_base_backup_timestamp in one place so schema changes only need to be updated once.src/roles/restore/tasks/restore_chain_walk.yaml (2)
16-34: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winNo cycle/depth guard on recursive chain walk.
If backup metadata is corrupted (e.g. a base pointing back into a cycle, or an extremely long chain), this recursive
include_taskshas no visited-set or max-depth check, risking unbounded recursion/growth ofrestore_backup_chain. Consider tracking visited directories and failing fast on a repeat.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/roles/restore/tasks/restore_chain_walk.yaml` around lines 16 - 34, The recursive chain walk in restore_chain_walk.yaml has no protection against cycles or excessively deep chains, so add a guard before the recursive include in the restore chain task block. Update the logic around restore_chain_is_incremental, restore_chain_base_stat, and the include_tasks call to track visited base directories (or a max depth) and fail fast if a directory repeats or the chain grows too long. Use the existing restore_backup_chain and restore_chain_current facts to implement the check, and ensure the recursive restore_chain_walk.yaml invocation only proceeds when the next base backup has not already been seen.
19-24: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winGeneric stat failure vs. the friendly chain-error messages elsewhere.
validate.yamlgives detailed "INCOMPLETE BACKUP CHAIN" guidance when a base backup is missing (one level deep). Here, a missing base deeper in the chain fails with a bare stat error, which is less helpful and inconsistent with the PR's stated goal of clearer error messages for missing chain members.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/roles/restore/tasks/restore_chain_walk.yaml` around lines 19 - 24, The base-backup existence check in the restore chain walk currently fails with a generic stat error instead of the friendly chain guidance used elsewhere. Update the task in restore_chain_walk.yaml that uses ansible.builtin.stat and failed_when so it emits the same “INCOMPLETE BACKUP CHAIN” style message as validate.yaml when restore_chain_base_stat.stat.exists is false, keeping the missing-base behavior consistent for deeper chain members.src/roles/restore/tasks/restore_pulp_content.yaml (1)
54-59: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winRecursive file count could be slow on large Pulp media trees.
findwithrecurse: truewalks the entire Pulp media directory just to report a count. For the large Pulp deployments this PR is optimizing for, this can add significant time to every restore.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/roles/restore/tasks/restore_pulp_content.yaml` around lines 54 - 59, The Count restored media files task in restore_pulp_content.yaml is doing an expensive recursive walk of the entire media tree via ansible.builtin.find with recurse:true. Replace that count-only lookup with a cheaper approach that avoids traversing every file in pulp_storage_path/media, using the restore_pulp_media_files task as the place to update the logic while still producing the restored media count needed by the role.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/roles/backup/tasks/main.yaml`:
- Around line 113-224: The incremental backup validation currently always
requires both .config.snar and .pulp.snar, which breaks valid runs when
skip_pulp_content is enabled. Update the .snar check in the backup task flow to
conditionally exclude .pulp.snar whenever the current run skips pulp content,
and make the downstream missing-file/suggestion logic use that filtered list.
Use the existing backup_snar_files_check, skip_pulp_content, and the fail blocks
in main.yaml to keep the behavior aligned with what pulp_content.yaml actually
writes.
- Around line 141-190: The backup suggestion build in
backup_alternatives_with_snar/backup_alternatives_metadata is misaligning paths
and metadata because backup_suggestions filters out failed slurp entries but the
later loop still indexes into the unfiltered path list. Fix this by pairing each
metadata result with its source path in a single pass, or by filtering and
mapping path/content together before building backup_suggestions_formatted, so
the combine step in the set_fact task never relies on idx from a filtered list.
Use the existing tasks around Build suggestions list with backup info and
Combine paths with metadata to keep each metadata.yml matched to the correct
backup directory.
In `@src/roles/restore/tasks/restore_databases.yaml`:
- Around line 2-7: The PostgreSQL admin secret creation task in the restore
workflow is non-idempotent and currently hides real failures. Update the task
around the podman secret creation step so it checks whether
postgresql-admin-password already exists before creating it, or
deletes/recreates it safely when needed; remove the blanket failure suppression
so genuine errors from the secret management step are surfaced. Use the existing
restore_databases task and the secret name postgresql-admin-password to locate
the change.
- Around line 136-151: The restore task in restore_databases.yaml is using
pg_restore with -U item.owner but still setting PGPASSWORD to
postgresql_admin_password, so authentication can fail if the owner password
differs. Update the Restore databases from dump files task to use the correct
password source for each database owner, using the same per-database secret
pattern captured in restore_foremanctl_state.yaml or otherwise ensuring the
password matches item.owner. Also remove the blanket failed_when: false from
this task, or replace it with a condition that only tolerates expected cases, so
failures in ansible.builtin.command are surfaced instead of being silently
reported as success.
In `@src/roles/restore/tasks/restore_foremanctl_state.yaml`:
- Around line 2-6: The restore task currently deletes the target state directory
in a way that breaks chain restores across incremental layers. Update the
restore flow so `restore_foremanctl_state` only clears `obsah_state_path` for
the initial/base restore path, and ensure `restore_single` does not remove the
directory before applying each incremental archive; keep later tar incrementals
applied cumulatively onto the same tree.
In `@src/roles/restore/tasks/restore_single.yaml`:
- Around line 24-27: Chain restore is restoring the wrong database snapshot
because `restore_single_is_first` only runs `restore_databases.yaml` for the
base backup. Update `restore_single.yaml` and the `restore_databases.yaml` flow
so PostgreSQL bootstrap/start-stop still happens once at the beginning of the
chain, but the actual `pg_restore` for the database runs on the last backup
instead of the first. Use the `restore_single_is_first` condition (or a new
end-of-chain check) to ensure later backup DB dumps are applied and the final
restored state matches the latest backup.
In `@src/roles/restore/tasks/validate.yaml`:
- Line 222: The shell command in the restore validation task interpolates
backup_dir directly, which can break or be abused when the path contains spaces
or shell metacharacters. Update the ansible.builtin.shell usage in the validate
task to safely quote backup_dir with the existing du/awk pipeline, or switch to
a safer module if possible. Use the backup_dir variable in this shell step only
through quoting so the restore size check remains functional under become: true.
- Around line 240-251: The Pulp disk-space validation is unreachable because Get
Pulp content archive size uses restore_pulp_archive in its own when condition
before the register value exists. Remove that self-referential guard and gate
the stat task on the appropriate restore/archive prerequisite instead, so
restore_pulp_archive is actually populated; then keep Warn if insufficient Pulp
disk space dependent on restore_pulp_archive.stat.exists and restore_pulp_space
as intended.
- Around line 2-25: The existing-deployment guard in the restore validation task
is too broad and will fail on later members of a restore chain after the first
`restore_single` creates `/var/lib/foremanctl`. Update the `validate.yaml` logic
around `restore_existing_deployment` and the `ansible.builtin.fail` step so this
check only applies to the first backup in a chain (or otherwise skips subsequent
chain members), while still preserving the `force` and `validate` conditions.
---
Nitpick comments:
In `@src/roles/restore/tasks/restore_chain_walk.yaml`:
- Around line 16-34: The recursive chain walk in restore_chain_walk.yaml has no
protection against cycles or excessively deep chains, so add a guard before the
recursive include in the restore chain task block. Update the logic around
restore_chain_is_incremental, restore_chain_base_stat, and the include_tasks
call to track visited base directories (or a max depth) and fail fast if a
directory repeats or the chain grows too long. Use the existing
restore_backup_chain and restore_chain_current facts to implement the check, and
ensure the recursive restore_chain_walk.yaml invocation only proceeds when the
next base backup has not already been seen.
- Around line 19-24: The base-backup existence check in the restore chain walk
currently fails with a generic stat error instead of the friendly chain guidance
used elsewhere. Update the task in restore_chain_walk.yaml that uses
ansible.builtin.stat and failed_when so it emits the same “INCOMPLETE BACKUP
CHAIN” style message as validate.yaml when restore_chain_base_stat.stat.exists
is false, keeping the missing-base behavior consistent for deeper chain members.
In `@src/roles/restore/tasks/restore_pulp_content.yaml`:
- Around line 54-59: The Count restored media files task in
restore_pulp_content.yaml is doing an expensive recursive walk of the entire
media tree via ansible.builtin.find with recurse:true. Replace that count-only
lookup with a cheaper approach that avoids traversing every file in
pulp_storage_path/media, using the restore_pulp_media_files task as the place to
update the logic while still producing the restored media count needed by the
role.
In `@src/roles/restore/tasks/validate.yaml`:
- Around line 56-60: The incremental backup detection logic in the restore
metadata parsing is duplicated and should be centralized. Move the repeated
fallback expression used by the restore task’s set_fact block into a shared
include_tasks helper such as parse_backup_metadata.yaml, then have both
validate.yaml and restore_chain_walk.yaml call that shared task. Keep the logic
for restore_is_incremental, restore_base_backup_dir, and
restore_base_backup_timestamp in one place so schema changes only need to be
updated once.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 90c6b150-79b6-4003-aba5-5b21db4f9dd7
📒 Files selected for processing (19)
docs/user/backup.mddocs/user/restore.mdsrc/playbooks/backup/metadata.obsah.yamlsrc/playbooks/restore/metadata.obsah.yamlsrc/playbooks/restore/restore.yamlsrc/roles/backup/tasks/database_dumps.yamlsrc/roles/backup/tasks/main.yamlsrc/roles/backup/tasks/metadata.yamlsrc/roles/backup/tasks/pulp_content.yamlsrc/roles/restore/defaults/main.yamlsrc/roles/restore/tasks/main.yamlsrc/roles/restore/tasks/prepare_system.yamlsrc/roles/restore/tasks/restore_chain.yamlsrc/roles/restore/tasks/restore_chain_walk.yamlsrc/roles/restore/tasks/restore_databases.yamlsrc/roles/restore/tasks/restore_foremanctl_state.yamlsrc/roles/restore/tasks/restore_pulp_content.yamlsrc/roles/restore/tasks/restore_single.yamlsrc/roles/restore/tasks/validate.yaml
| - name: Check for required .snar files in previous backup | ||
| ansible.builtin.stat: | ||
| path: "{{ incremental }}/{{ item }}" | ||
| register: backup_snar_files_check | ||
| loop: | ||
| - ".config.snar" | ||
| - ".pulp.snar" | ||
| failed_when: false | ||
|
|
||
| - name: Find alternative backups with .snar files if current is missing | ||
| when: backup_snar_files_check.results | selectattr('stat.exists', 'equalto', false) | list | length > 0 | ||
| block: | ||
| - name: Find all backup directories in parent directory | ||
| ansible.builtin.find: | ||
| paths: "{{ incremental | dirname }}" | ||
| file_type: directory | ||
| patterns: "foreman-backup-*" | ||
| register: backup_alternative_dirs | ||
| failed_when: false | ||
|
|
||
| - name: Check for .snar files in alternative backups | ||
| ansible.builtin.stat: | ||
| path: "{{ item.path }}/.config.snar" | ||
| register: backup_alternative_snar_check | ||
| loop: "{{ backup_alternative_dirs.files | default([]) }}" | ||
| failed_when: false | ||
| when: backup_alternative_dirs.matched | default(0) > 0 | ||
|
|
||
| - name: Build list of backups with .snar support | ||
| ansible.builtin.set_fact: | ||
| backup_alternatives_with_snar: >- | ||
| {{ | ||
| backup_alternative_snar_check.results | default([]) | ||
| | selectattr('stat.exists', 'defined') | ||
| | selectattr('stat.exists', 'equalto', true) | ||
| | map(attribute='item') | ||
| | map(attribute='path') | ||
| | list | ||
| }} | ||
| when: backup_alternative_snar_check.results | default([]) | length > 0 | ||
|
|
||
| - name: Read metadata for alternative backups | ||
| ansible.builtin.slurp: | ||
| path: "{{ item }}/metadata.yml" | ||
| register: backup_alternatives_metadata | ||
| loop: "{{ backup_alternatives_with_snar | default([]) }}" | ||
| failed_when: false | ||
| when: backup_alternatives_with_snar | default([]) | length > 0 | ||
|
|
||
| - name: Build suggestions list with backup info | ||
| ansible.builtin.set_fact: | ||
| backup_suggestions: >- | ||
| {{ | ||
| backup_alternatives_metadata.results | default([]) | ||
| | selectattr('content', 'defined') | ||
| | map(attribute='content') | ||
| | map('b64decode') | ||
| | map('from_yaml') | ||
| | map('combine', {'path': ''}) | ||
| | list | ||
| }} | ||
| when: backup_alternatives_metadata.results | default([]) | length > 0 | ||
|
|
||
| - name: Combine paths with metadata | ||
| ansible.builtin.set_fact: | ||
| backup_suggestions_formatted: >- | ||
| {{ | ||
| backup_suggestions_formatted | default([]) + | ||
| [{ | ||
| 'path': backup_alternatives_with_snar[idx], | ||
| 'timestamp': item.timestamp, | ||
| 'type': 'Full' if not (item.incremental.is_incremental | default(item.incremental | default(false)) | bool) else 'Incremental' | ||
| }] | ||
| }} | ||
| loop: "{{ backup_suggestions | default([]) }}" | ||
| loop_control: | ||
| index_var: idx | ||
| when: backup_suggestions | default([]) | length > 0 | ||
|
|
||
| - name: Fail with suggestions if alternatives exist | ||
| ansible.builtin.fail: | ||
| msg: | | ||
| Incremental backup requested, but previous backup is missing .snar files. | ||
| Previous backup: {{ incremental }} | ||
| Missing files: {{ backup_snar_files_check.results | selectattr('stat.exists', 'equalto', false) | map(attribute='item') | list }} | ||
|
|
||
| This usually means the previous backup was not created with incremental support. | ||
| You must use a backup created with foremanctl >= 2.x as the base for incremental backups. | ||
|
|
||
| Available backups with incremental support: | ||
| {% for suggestion in backup_suggestions_formatted | default([]) | sort(attribute='timestamp', reverse=true) %} | ||
| - {{ suggestion.path }} ({{ suggestion.type }}, {{ suggestion.timestamp }}) | ||
| {% endfor %} | ||
|
|
||
| Retry with one of the above backups, for example: | ||
| foremanctl backup {{ backup_dir }} --incremental {{ (backup_suggestions_formatted | default([]) | sort(attribute='timestamp', reverse=true) | first).path }} | ||
| when: backup_suggestions_formatted | default([]) | length > 0 | ||
|
|
||
| - name: Fail without suggestions if no alternatives | ||
| ansible.builtin.fail: | ||
| msg: | | ||
| Incremental backup requested, but previous backup is missing .snar files. | ||
| Previous backup: {{ incremental }} | ||
| Missing files: {{ backup_snar_files_check.results | selectattr('stat.exists', 'equalto', false) | map(attribute='item') | list }} | ||
|
|
||
| This usually means the previous backup was not created with incremental support. | ||
| You must use a backup created with foremanctl >= 2.x as the base for incremental backups. | ||
|
|
||
| No other backups with incremental support were found in {{ incremental | dirname }}. | ||
| Create a new full backup first: | ||
| foremanctl backup {{ backup_dir }} | ||
| when: backup_suggestions_formatted | default([]) | length == 0 |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
.snar validation ignores skip_pulp_content, causing spurious incremental-backup failures.
The check at Lines 113-120 always verifies both .config.snar and .pulp.snar exist in the previous backup, regardless of whether the current run also passes --skip-pulp-content. Since pulp_content.yaml only writes .pulp.snar when: not skip_pulp_content (main.yaml Line 250), any prior backup that used --skip-pulp-content will never have a .pulp.snar. If a user consistently runs --skip-pulp-content --incremental PREV_DIR, this legitimate combination will trigger the "find alternative backups" workflow and can hard-fail the backup (Lines 192-224) even though .pulp.snar is irrelevant for this run.
🛠️ Proposed fix: skip the `.pulp.snar` check when pulp content is skipped
- name: Check for required .snar files in previous backup
ansible.builtin.stat:
path: "{{ incremental }}/{{ item }}"
register: backup_snar_files_check
- loop:
- - ".config.snar"
- - ".pulp.snar"
+ loop: >-
+ {{
+ ['.config.snar'] +
+ ([] if (skip_pulp_content | default(false)) else ['.pulp.snar'])
+ }}
failed_when: false📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - name: Check for required .snar files in previous backup | |
| ansible.builtin.stat: | |
| path: "{{ incremental }}/{{ item }}" | |
| register: backup_snar_files_check | |
| loop: | |
| - ".config.snar" | |
| - ".pulp.snar" | |
| failed_when: false | |
| - name: Find alternative backups with .snar files if current is missing | |
| when: backup_snar_files_check.results | selectattr('stat.exists', 'equalto', false) | list | length > 0 | |
| block: | |
| - name: Find all backup directories in parent directory | |
| ansible.builtin.find: | |
| paths: "{{ incremental | dirname }}" | |
| file_type: directory | |
| patterns: "foreman-backup-*" | |
| register: backup_alternative_dirs | |
| failed_when: false | |
| - name: Check for .snar files in alternative backups | |
| ansible.builtin.stat: | |
| path: "{{ item.path }}/.config.snar" | |
| register: backup_alternative_snar_check | |
| loop: "{{ backup_alternative_dirs.files | default([]) }}" | |
| failed_when: false | |
| when: backup_alternative_dirs.matched | default(0) > 0 | |
| - name: Build list of backups with .snar support | |
| ansible.builtin.set_fact: | |
| backup_alternatives_with_snar: >- | |
| {{ | |
| backup_alternative_snar_check.results | default([]) | |
| | selectattr('stat.exists', 'defined') | |
| | selectattr('stat.exists', 'equalto', true) | |
| | map(attribute='item') | |
| | map(attribute='path') | |
| | list | |
| }} | |
| when: backup_alternative_snar_check.results | default([]) | length > 0 | |
| - name: Read metadata for alternative backups | |
| ansible.builtin.slurp: | |
| path: "{{ item }}/metadata.yml" | |
| register: backup_alternatives_metadata | |
| loop: "{{ backup_alternatives_with_snar | default([]) }}" | |
| failed_when: false | |
| when: backup_alternatives_with_snar | default([]) | length > 0 | |
| - name: Build suggestions list with backup info | |
| ansible.builtin.set_fact: | |
| backup_suggestions: >- | |
| {{ | |
| backup_alternatives_metadata.results | default([]) | |
| | selectattr('content', 'defined') | |
| | map(attribute='content') | |
| | map('b64decode') | |
| | map('from_yaml') | |
| | map('combine', {'path': ''}) | |
| | list | |
| }} | |
| when: backup_alternatives_metadata.results | default([]) | length > 0 | |
| - name: Combine paths with metadata | |
| ansible.builtin.set_fact: | |
| backup_suggestions_formatted: >- | |
| {{ | |
| backup_suggestions_formatted | default([]) + | |
| [{ | |
| 'path': backup_alternatives_with_snar[idx], | |
| 'timestamp': item.timestamp, | |
| 'type': 'Full' if not (item.incremental.is_incremental | default(item.incremental | default(false)) | bool) else 'Incremental' | |
| }] | |
| }} | |
| loop: "{{ backup_suggestions | default([]) }}" | |
| loop_control: | |
| index_var: idx | |
| when: backup_suggestions | default([]) | length > 0 | |
| - name: Fail with suggestions if alternatives exist | |
| ansible.builtin.fail: | |
| msg: | | |
| Incremental backup requested, but previous backup is missing .snar files. | |
| Previous backup: {{ incremental }} | |
| Missing files: {{ backup_snar_files_check.results | selectattr('stat.exists', 'equalto', false) | map(attribute='item') | list }} | |
| This usually means the previous backup was not created with incremental support. | |
| You must use a backup created with foremanctl >= 2.x as the base for incremental backups. | |
| Available backups with incremental support: | |
| {% for suggestion in backup_suggestions_formatted | default([]) | sort(attribute='timestamp', reverse=true) %} | |
| - {{ suggestion.path }} ({{ suggestion.type }}, {{ suggestion.timestamp }}) | |
| {% endfor %} | |
| Retry with one of the above backups, for example: | |
| foremanctl backup {{ backup_dir }} --incremental {{ (backup_suggestions_formatted | default([]) | sort(attribute='timestamp', reverse=true) | first).path }} | |
| when: backup_suggestions_formatted | default([]) | length > 0 | |
| - name: Fail without suggestions if no alternatives | |
| ansible.builtin.fail: | |
| msg: | | |
| Incremental backup requested, but previous backup is missing .snar files. | |
| Previous backup: {{ incremental }} | |
| Missing files: {{ backup_snar_files_check.results | selectattr('stat.exists', 'equalto', false) | map(attribute='item') | list }} | |
| This usually means the previous backup was not created with incremental support. | |
| You must use a backup created with foremanctl >= 2.x as the base for incremental backups. | |
| No other backups with incremental support were found in {{ incremental | dirname }}. | |
| Create a new full backup first: | |
| foremanctl backup {{ backup_dir }} | |
| when: backup_suggestions_formatted | default([]) | length == 0 | |
| - name: Check for required .snar files in previous backup | |
| ansible.builtin.stat: | |
| path: "{{ incremental }}/{{ item }}" | |
| register: backup_snar_files_check | |
| loop: >- | |
| {{ | |
| ['.config.snar'] + | |
| ([] if (skip_pulp_content | default(false)) else ['.pulp.snar']) | |
| }} | |
| failed_when: false | |
| - name: Find alternative backups with .snar files if current is missing | |
| when: backup_snar_files_check.results | selectattr('stat.exists', 'equalto', false) | list | length > 0 | |
| block: | |
| - name: Find all backup directories in parent directory | |
| ansible.builtin.find: | |
| paths: "{{ incremental | dirname }}" | |
| file_type: directory | |
| patterns: "foreman-backup-*" | |
| register: backup_alternative_dirs | |
| failed_when: false | |
| - name: Check for .snar files in alternative backups | |
| ansible.builtin.stat: | |
| path: "{{ item.path }}/.config.snar" | |
| register: backup_alternative_snar_check | |
| loop: "{{ backup_alternative_dirs.files | default([]) }}" | |
| failed_when: false | |
| when: backup_alternative_dirs.matched | default(0) > 0 | |
| - name: Build list of backups with .snar support | |
| ansible.builtin.set_fact: | |
| backup_alternatives_with_snar: >- | |
| {{ | |
| backup_alternative_snar_check.results | default([]) | |
| | selectattr('stat.exists', 'defined') | |
| | selectattr('stat.exists', 'equalto', true) | |
| | map(attribute='item') | |
| | map(attribute='path') | |
| | list | |
| }} | |
| when: backup_alternative_snar_check.results | default([]) | length > 0 | |
| - name: Read metadata for alternative backups | |
| ansible.builtin.slurp: | |
| path: "{{ item }}/metadata.yml" | |
| register: backup_alternatives_metadata | |
| loop: "{{ backup_alternatives_with_snar | default([]) }}" | |
| failed_when: false | |
| when: backup_alternatives_with_snar | default([]) | length > 0 | |
| - name: Build suggestions list with backup info | |
| ansible.builtin.set_fact: | |
| backup_suggestions: >- | |
| {{ | |
| backup_alternatives_metadata.results | default([]) | |
| | selectattr('content', 'defined') | |
| | map(attribute='content') | |
| | map('b64decode') | |
| | map('from_yaml') | |
| | map('combine', {'path': ''}) | |
| | list | |
| }} | |
| when: backup_alternatives_metadata.results | default([]) | length > 0 | |
| - name: Combine paths with metadata | |
| ansible.builtin.set_fact: | |
| backup_suggestions_formatted: >- | |
| {{ | |
| backup_suggestions_formatted | default([]) + | |
| [{ | |
| 'path': backup_alternatives_with_snar[idx], | |
| 'timestamp': item.timestamp, | |
| 'type': 'Full' if not (item.incremental.is_incremental | default(item.incremental | default(false)) | bool) else 'Incremental' | |
| }] | |
| }} | |
| loop: "{{ backup_suggestions | default([]) }}" | |
| loop_control: | |
| index_var: idx | |
| when: backup_suggestions | default([]) | length > 0 | |
| - name: Fail with suggestions if alternatives exist | |
| ansible.builtin.fail: | |
| msg: | | |
| Incremental backup requested, but previous backup is missing .snar files. | |
| Previous backup: {{ incremental }} | |
| Missing files: {{ backup_snar_files_check.results | selectattr('stat.exists', 'equalto', false) | map(attribute='item') | list }} | |
| This usually means the previous backup was not created with incremental support. | |
| You must use a backup created with foremanctl >= 2.x as the base for incremental backups. | |
| Available backups with incremental support: | |
| {% for suggestion in backup_suggestions_formatted | default([]) | sort(attribute='timestamp', reverse=true) %} | |
| - {{ suggestion.path }} ({{ suggestion.type }}, {{ suggestion.timestamp }}) | |
| {% endfor %} | |
| Retry with one of the above backups, for example: | |
| foremanctl backup {{ backup_dir }} --incremental {{ (backup_suggestions_formatted | default([]) | sort(attribute='timestamp', reverse=true) | first).path }} | |
| when: backup_suggestions_formatted | default([]) | length > 0 | |
| - name: Fail without suggestions if no alternatives | |
| ansible.builtin.fail: | |
| msg: | | |
| Incremental backup requested, but previous backup is missing .snar files. | |
| Previous backup: {{ incremental }} | |
| Missing files: {{ backup_snar_files_check.results | selectattr('stat.exists', 'equalto', false) | map(attribute='item') | list }} | |
| This usually means the previous backup was not created with incremental support. | |
| You must use a backup created with foremanctl >= 2.x as the base for incremental backups. | |
| No other backups with incremental support were found in {{ incremental | dirname }}. | |
| Create a new full backup first: | |
| foremanctl backup {{ backup_dir }} | |
| when: backup_suggestions_formatted | default([]) | length == 0 |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/roles/backup/tasks/main.yaml` around lines 113 - 224, The incremental
backup validation currently always requires both .config.snar and .pulp.snar,
which breaks valid runs when skip_pulp_content is enabled. Update the .snar
check in the backup task flow to conditionally exclude .pulp.snar whenever the
current run skips pulp content, and make the downstream missing-file/suggestion
logic use that filtered list. Use the existing backup_snar_files_check,
skip_pulp_content, and the fail blocks in main.yaml to keep the behavior aligned
with what pulp_content.yaml actually writes.
| - name: Build list of backups with .snar support | ||
| ansible.builtin.set_fact: | ||
| backup_alternatives_with_snar: >- | ||
| {{ | ||
| backup_alternative_snar_check.results | default([]) | ||
| | selectattr('stat.exists', 'defined') | ||
| | selectattr('stat.exists', 'equalto', true) | ||
| | map(attribute='item') | ||
| | map(attribute='path') | ||
| | list | ||
| }} | ||
| when: backup_alternative_snar_check.results | default([]) | length > 0 | ||
|
|
||
| - name: Read metadata for alternative backups | ||
| ansible.builtin.slurp: | ||
| path: "{{ item }}/metadata.yml" | ||
| register: backup_alternatives_metadata | ||
| loop: "{{ backup_alternatives_with_snar | default([]) }}" | ||
| failed_when: false | ||
| when: backup_alternatives_with_snar | default([]) | length > 0 | ||
|
|
||
| - name: Build suggestions list with backup info | ||
| ansible.builtin.set_fact: | ||
| backup_suggestions: >- | ||
| {{ | ||
| backup_alternatives_metadata.results | default([]) | ||
| | selectattr('content', 'defined') | ||
| | map(attribute='content') | ||
| | map('b64decode') | ||
| | map('from_yaml') | ||
| | map('combine', {'path': ''}) | ||
| | list | ||
| }} | ||
| when: backup_alternatives_metadata.results | default([]) | length > 0 | ||
|
|
||
| - name: Combine paths with metadata | ||
| ansible.builtin.set_fact: | ||
| backup_suggestions_formatted: >- | ||
| {{ | ||
| backup_suggestions_formatted | default([]) + | ||
| [{ | ||
| 'path': backup_alternatives_with_snar[idx], | ||
| 'timestamp': item.timestamp, | ||
| 'type': 'Full' if not (item.incremental.is_incremental | default(item.incremental | default(false)) | bool) else 'Incremental' | ||
| }] | ||
| }} | ||
| loop: "{{ backup_suggestions | default([]) }}" | ||
| loop_control: | ||
| index_var: idx | ||
| when: backup_suggestions | default([]) | length > 0 |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Index misalignment when building alternative-backup suggestions.
backup_suggestions (Lines 162-174) is built by filtering backup_alternatives_metadata.results with selectattr('content', 'defined'), which drops entries whose metadata.yml slurp failed. The subsequent loop at Lines 176-190 then indexes into backup_alternatives_with_snar[idx] using the filtered list's position (idx), but backup_alternatives_with_snar is the unfiltered list of paths. Whenever any alternative directory lacks a valid metadata.yml, every suggestion after that point gets paired with the wrong path, producing a misleading "Available backups with incremental support" message and a bogus suggested retry command (Line 208).
🛠️ Proposed fix: pair path and content in a single pass, avoiding index lookup
- - name: Build suggestions list with backup info
- ansible.builtin.set_fact:
- backup_suggestions: >-
- {{
- backup_alternatives_metadata.results | default([])
- | selectattr('content', 'defined')
- | map(attribute='content')
- | map('b64decode')
- | map('from_yaml')
- | map('combine', {'path': ''})
- | list
- }}
- when: backup_alternatives_metadata.results | default([]) | length > 0
-
- - name: Combine paths with metadata
- ansible.builtin.set_fact:
- backup_suggestions_formatted: >-
- {{
- backup_suggestions_formatted | default([]) +
- [{
- 'path': backup_alternatives_with_snar[idx],
- 'timestamp': item.timestamp,
- 'type': 'Full' if not (item.incremental.is_incremental | default(item.incremental | default(false)) | bool) else 'Incremental'
- }]
- }}
- loop: "{{ backup_suggestions | default([]) }}"
- loop_control:
- index_var: idx
- when: backup_suggestions | default([]) | length > 0
+ - name: Combine paths with metadata
+ ansible.builtin.set_fact:
+ backup_suggestions_formatted: >-
+ {{
+ backup_suggestions_formatted | default([]) +
+ [{
+ 'path': item.item,
+ 'timestamp': (item.content | b64decode | from_yaml).timestamp,
+ 'type': 'Full' if not ((item.content | b64decode | from_yaml).incremental.is_incremental | default((item.content | b64decode | from_yaml).incremental | default(false)) | bool) else 'Incremental'
+ }]
+ }}
+ loop: "{{ backup_alternatives_metadata.results | default([]) | selectattr('content', 'defined') | list }}"
+ when: backup_alternatives_metadata.results | default([]) | length > 0📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - name: Build list of backups with .snar support | |
| ansible.builtin.set_fact: | |
| backup_alternatives_with_snar: >- | |
| {{ | |
| backup_alternative_snar_check.results | default([]) | |
| | selectattr('stat.exists', 'defined') | |
| | selectattr('stat.exists', 'equalto', true) | |
| | map(attribute='item') | |
| | map(attribute='path') | |
| | list | |
| }} | |
| when: backup_alternative_snar_check.results | default([]) | length > 0 | |
| - name: Read metadata for alternative backups | |
| ansible.builtin.slurp: | |
| path: "{{ item }}/metadata.yml" | |
| register: backup_alternatives_metadata | |
| loop: "{{ backup_alternatives_with_snar | default([]) }}" | |
| failed_when: false | |
| when: backup_alternatives_with_snar | default([]) | length > 0 | |
| - name: Build suggestions list with backup info | |
| ansible.builtin.set_fact: | |
| backup_suggestions: >- | |
| {{ | |
| backup_alternatives_metadata.results | default([]) | |
| | selectattr('content', 'defined') | |
| | map(attribute='content') | |
| | map('b64decode') | |
| | map('from_yaml') | |
| | map('combine', {'path': ''}) | |
| | list | |
| }} | |
| when: backup_alternatives_metadata.results | default([]) | length > 0 | |
| - name: Combine paths with metadata | |
| ansible.builtin.set_fact: | |
| backup_suggestions_formatted: >- | |
| {{ | |
| backup_suggestions_formatted | default([]) + | |
| [{ | |
| 'path': backup_alternatives_with_snar[idx], | |
| 'timestamp': item.timestamp, | |
| 'type': 'Full' if not (item.incremental.is_incremental | default(item.incremental | default(false)) | bool) else 'Incremental' | |
| }] | |
| }} | |
| loop: "{{ backup_suggestions | default([]) }}" | |
| loop_control: | |
| index_var: idx | |
| when: backup_suggestions | default([]) | length > 0 | |
| - name: Build list of backups with .snar support | |
| ansible.builtin.set_fact: | |
| backup_alternatives_with_snar: >- | |
| {{ | |
| backup_alternative_snar_check.results | default([]) | |
| | selectattr('stat.exists', 'defined') | |
| | selectattr('stat.exists', 'equalto', true) | |
| | map(attribute='item') | |
| | map(attribute='path') | |
| | list | |
| }} | |
| when: backup_alternative_snar_check.results | default([]) | length > 0 | |
| - name: Read metadata for alternative backups | |
| ansible.builtin.slurp: | |
| path: "{{ item }}/metadata.yml" | |
| register: backup_alternatives_metadata | |
| loop: "{{ backup_alternatives_with_snar | default([]) }}" | |
| failed_when: false | |
| when: backup_alternatives_with_snar | default([]) | length > 0 | |
| - name: Combine paths with metadata | |
| ansible.builtin.set_fact: | |
| backup_suggestions_formatted: >- | |
| {{ | |
| backup_suggestions_formatted | default([]) + | |
| [{ | |
| 'path': item.item, | |
| 'timestamp': (item.content | b64decode | from_yaml).timestamp, | |
| 'type': 'Full' if not ((item.content | b64decode | from_yaml).incremental.is_incremental | default((item.content | b64decode | from_yaml).incremental | default(false)) | bool) else 'Incremental' | |
| }] | |
| }} | |
| loop: "{{ backup_alternatives_metadata.results | default([]) | selectattr('content', 'defined') | list }}" | |
| when: backup_alternatives_metadata.results | default([]) | length > 0 |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/roles/backup/tasks/main.yaml` around lines 141 - 190, The backup
suggestion build in backup_alternatives_with_snar/backup_alternatives_metadata
is misaligning paths and metadata because backup_suggestions filters out failed
slurp entries but the later loop still indexes into the unfiltered path list.
Fix this by pairing each metadata result with its source path in a single pass,
or by filtering and mapping path/content together before building
backup_suggestions_formatted, so the combine step in the set_fact task never
relies on idx from a filtered list. Use the existing tasks around Build
suggestions list with backup info and Combine paths with metadata to keep each
metadata.yml matched to the correct backup directory.
| - name: Create PostgreSQL admin password secret | ||
| ansible.builtin.shell: | | ||
| cat {{ obsah_state_path }}/postgresql-admin-password | podman secret create postgresql-admin-password - | ||
| when: database_mode == 'internal' | ||
| changed_when: true | ||
| failed_when: false |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Podman secret creation isn't idempotent and failures are fully hidden.
If postgresql-admin-password secret already exists from a prior run, podman secret create fails, and failed_when: false suppresses this without any remediation — PostgreSQL could then start with a stale/mismatched admin secret.
🔧 Suggested fix
+- name: Remove existing PostgreSQL admin password secret
+ ansible.builtin.command:
+ cmd: podman secret rm postgresql-admin-password
+ when: database_mode == 'internal'
+ changed_when: true
+ failed_when: false
+
- name: Create PostgreSQL admin password secret
ansible.builtin.shell: |
cat {{ obsah_state_path }}/postgresql-admin-password | podman secret create postgresql-admin-password -
when: database_mode == 'internal'
changed_when: true
- failed_when: false📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - name: Create PostgreSQL admin password secret | |
| ansible.builtin.shell: | | |
| cat {{ obsah_state_path }}/postgresql-admin-password | podman secret create postgresql-admin-password - | |
| when: database_mode == 'internal' | |
| changed_when: true | |
| failed_when: false | |
| - name: Remove existing PostgreSQL admin password secret | |
| ansible.builtin.command: | |
| cmd: podman secret rm postgresql-admin-password | |
| when: database_mode == 'internal' | |
| changed_when: true | |
| failed_when: false | |
| - name: Create PostgreSQL admin password secret | |
| ansible.builtin.shell: | | |
| cat {{ obsah_state_path }}/postgresql-admin-password | podman secret create postgresql-admin-password - | |
| when: database_mode == 'internal' | |
| changed_when: true |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/roles/restore/tasks/restore_databases.yaml` around lines 2 - 7, The
PostgreSQL admin secret creation task in the restore workflow is non-idempotent
and currently hides real failures. Update the task around the podman secret
creation step so it checks whether postgresql-admin-password already exists
before creating it, or deletes/recreates it safely when needed; remove the
blanket failure suppression so genuine errors from the secret management step
are surfaced. Use the existing restore_databases task and the secret name
postgresql-admin-password to locate the change.
| - name: Restore databases from dump files | ||
| ansible.builtin.command: | ||
| cmd: > | ||
| pg_restore | ||
| -h {{ database_host }} | ||
| -p {{ database_port }} | ||
| -U {{ item.owner }} | ||
| -d {{ item.database_name }} | ||
| {{ backup_dir }}/{{ item.dump_file }} | ||
| environment: | ||
| PGPASSWORD: "{{ postgresql_admin_password }}" | ||
| loop: "{{ restore_databases_to_restore }}" | ||
| loop_control: | ||
| label: "{{ item.dump_file }} -> {{ item.database_name }}" | ||
| changed_when: true | ||
| failed_when: false |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🔴 Critical | ⚡ Quick win
pg_restore likely fails auth and the failure is silently swallowed.
pg_restore connects as -U {{ item.owner }} while PGPASSWORD is set to {{ postgresql_admin_password }} (the admin password), not the owner-specific password (captured separately per DB, e.g. <db>-db-password, in restore_foremanctl_state.yaml). Unless the owner role's password happens to equal the admin password, this authentication will fail. Compounding this, failed_when: false means the task always reports success even when the restore never actually ran, leaving the "Restore completed successfully!" message misleading.
🔧 Suggested fix
- name: Restore databases from dump files
ansible.builtin.command:
cmd: >
pg_restore
-h {{ database_host }}
-p {{ database_port }}
- -U {{ item.owner }}
+ -U postgres
-d {{ item.database_name }}
{{ backup_dir }}/{{ item.dump_file }}
environment:
PGPASSWORD: "{{ postgresql_admin_password }}"
loop: "{{ restore_databases_to_restore }}"
loop_control:
label: "{{ item.dump_file }} -> {{ item.database_name }}"
changed_when: true
- failed_when: false
+ register: restore_pg_restore_result
+ failed_when: restore_pg_restore_result.rc != 0 and 'already exists' not in restore_pg_restore_result.stderr📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - name: Restore databases from dump files | |
| ansible.builtin.command: | |
| cmd: > | |
| pg_restore | |
| -h {{ database_host }} | |
| -p {{ database_port }} | |
| -U {{ item.owner }} | |
| -d {{ item.database_name }} | |
| {{ backup_dir }}/{{ item.dump_file }} | |
| environment: | |
| PGPASSWORD: "{{ postgresql_admin_password }}" | |
| loop: "{{ restore_databases_to_restore }}" | |
| loop_control: | |
| label: "{{ item.dump_file }} -> {{ item.database_name }}" | |
| changed_when: true | |
| failed_when: false | |
| - name: Restore databases from dump files | |
| ansible.builtin.command: | |
| cmd: > | |
| pg_restore | |
| -h {{ database_host }} | |
| -p {{ database_port }} | |
| -U postgres | |
| -d {{ item.database_name }} | |
| {{ backup_dir }}/{{ item.dump_file }} | |
| environment: | |
| PGPASSWORD: "{{ postgresql_admin_password }}" | |
| loop: "{{ restore_databases_to_restore }}" | |
| loop_control: | |
| label: "{{ item.dump_file }} -> {{ item.database_name }}" | |
| changed_when: true | |
| register: restore_pg_restore_result | |
| failed_when: restore_pg_restore_result.rc != 0 and 'already exists' not in restore_pg_restore_result.stderr |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/roles/restore/tasks/restore_databases.yaml` around lines 136 - 151, The
restore task in restore_databases.yaml is using pg_restore with -U item.owner
but still setting PGPASSWORD to postgresql_admin_password, so authentication can
fail if the owner password differs. Update the Restore databases from dump files
task to use the correct password source for each database owner, using the same
per-database secret pattern captured in restore_foremanctl_state.yaml or
otherwise ensuring the password matches item.owner. Also remove the blanket
failed_when: false from this task, or replace it with a condition that only
tolerates expected cases, so failures in ansible.builtin.command are surfaced
instead of being silently reported as success.
| - name: Remove existing foremanctl state directory | ||
| ansible.builtin.file: | ||
| path: "{{ obsah_state_path }}" | ||
| state: absent | ||
|
|
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🔴 Critical | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '== files ==\n'
git ls-files 'src/roles/restore/tasks/*' 'src/roles/restore/*' | sed -n '1,200p'
printf '\n== restore_single references ==\n'
rg -n --no-heading 'restore_single|restore_foremanctl_state|obsah_state_path|is_first' src/roles/restore -S
printf '\n== relevant task files (line-numbered excerpts) ==\n'
for f in \
src/roles/restore/tasks/restore_single.yaml \
src/roles/restore/tasks/restore_foremanctl_state.yaml
do
echo "--- $f ---"
wc -l "$f"
sed -n '1,220p' "$f" | cat -n
doneRepository: theforeman/foremanctl
Length of output: 2032
🌐 Web query:
GNU tar listed incremental restore extract archives sequence onto same tree do not delete target directory between archives
💡 Result:
When restoring from a sequence of GNU tar incremental archives, it is a fundamental aspect of the restoration process that GNU tar attempts to replicate the exact state of the filesystem as it existed at the time each specific archive was created [1]. By design, during an incremental extraction, GNU tar will delete files from the target directory that were not present in the archive [1]. This behavior is how it achieves the state-restoration required for incremental backups, as it must remove files that were deleted on the source system between backup levels [1]. There is no standard GNU tar option to simply disable this deletion behavior while maintaining the incremental restoration logic for files that were present [1]. To perform a correct incremental restore: 1. You must restore the full (level 0) backup first, followed by each subsequent incremental backup in the exact chronological order in which they were created [1][2]. 2. During the extraction of each archive, use the --listed-incremental=/dev/null option (or simply --incremental / -G) [1][3][4]. This tells tar that the archive contains incremental metadata, allowing it to correctly process the file list without needing an external snapshot file [1]. If you wish to avoid the risk of data loss or "unexpected" deletions during this process, the standard practice is to restore the data into an empty, dedicated target directory rather than an active production tree [1][5]. If you must perform this operation on an existing tree and need to avoid deletions, you would typically need to manually inspect the archive contents or use file system snapshots (such as LVM, ZFS, or Btrfs) to restore to a clean state first [1][2]. The provided GNU tar restoration scripts are designed to handle this orchestration automatically, and it is highly recommended to use them if your backup strategy utilizes them [6][7][8].
Citations:
- 1: https://www.manpagez.com/info/tar/tar-1.19/tar_88.php
- 2: https://unix.stackexchange.com/questions/598301/tar-incremental-backups-skip-restoring-deleted-files
- 3: https://www.zeuthen.desy.de/unix/unixguide/infohtml/tar/Incremental-Dumps.html
- 4: http://www.gnu.org/software/tar/manual/html_node/Option-Summary.html
- 5: http://odysseus.fi.muni.cz/pub/gnu/old-gnu/Manuals/tar/html_chapter/tar_5.html
- 6: https://www.gnu.org/software/tar/manual/html_node/Scripted-Restoration.html
- 7: https://www.gnu.org/software/tar/manual/html_section/Backup-Levels.html
- 8: https://www.gnu.org/software/tar/manual/html_section/Backup-Parameters.html
🌐 Web query:
site:github.com theforeman/foremanctl src/roles/restore/tasks/restore_single.yaml restore_foremanctl_state.yaml
💡 Result:
The files src/roles/restore/tasks/restore_single.yaml and src/roles/restore/tasks/restore_foremanctl_state.yaml are Ansible task files within the theforeman/foremanctl repository, which provides tooling for deploying Foreman and Katello using Podman quadlets and Ansible [1][2][3]. These files are components of the foremanctl restore functionality, introduced to enable restoring Foreman instances from backups [3][4]. 1. src/roles/restore/tasks/restore_single.yaml: This task file was added to handle the restoration of a single backup point [3]. It is used as part of the restore process, particularly when dealing with chains of backups (such as those involving incremental backups) [3][5]. 2. src/roles/restore/tasks/restore_foremanctl_state.yaml: This task file manages the restoration of the foremanctl internal state [3]. It ensures that the state configuration, which is tracked via snapshot files (such as.config.snar in incremental backups), is correctly restored during the process [3][5]. Together, these roles support the foremanctl restore command, which validates backups, restores databases, pulp content, and encryption keys, and redeploys the environment to ensure a healthy state [4]. Incremental backups and the associated restore chain logic further utilize these files to manage full and partial state restorations efficiently [5].
Citations:
- 1: https://github.com/theforeman/foremanctl
- 2: https://github.com/theforeman/foremanctl/blob/master/README.md
- 3: 2ecaa0d
- 4: Add foremanctl restore command - Complete offline backup restore #549
- 5: Add incremental backup and restore #609
🌐 Web query:
"src/roles/restore/tasks/restore_single.yaml" "include_tasks" restore_foremanctl_state.yaml
💡 Result:
The file path src/roles/restore/tasks/restore_single.yaml is part of the foremanctl project [1], a command-line tool used for managing Foreman and Katello as containerized services [2]. The task file src/roles/restore/tasks/restore_single.yaml utilizes the include_tasks directive to call restore_foremanctl_state.yaml [1], which is responsible for restoring specific state components during the foremanctl restore process [1][3]. Foremanctl implements backup and restore functionality for these containerized deployments [3][2]. The restore_foremanctl_state.yaml task specifically handles restoring necessary configuration and state information (such as secrets or specific metadata) required to bring the Foreman instance back to a consistent state [3][4]. This workflow is part of a larger, modular restore process that can handle both full and incremental backups [1][4]. In recent versions of foremanctl, the restore command has been expanded to support: - Automated chain restoration (using the --chain flag) [4]. - Incremental backups enabled by GNU tar's --listed-incremental mechanism [4]. - Validation steps to ensure backup integrity before applying changes to an existing deployment [3][4]. The foremanctl tool is distinct from the older foreman-maintain project, although both serve the purpose of system maintenance [5][2]. Foremanctl is specifically architected for containerized environments using Podman Quadlets [2].
Citations:
- 1: 2ecaa0d
- 2: https://github.com/theforeman/foremanctl
- 3: Add foremanctl restore command - Complete offline backup restore #549
- 4: Add incremental backup and restore #609
- 5: https://github.com/theforeman/foreman_maintain/blob/187dfd11235438fc40eb2ea192cf91f37a8828a1/lib/foreman_maintain/utils/backup.rb
🌐 Web query:
site:github.com/theforeman/foremanctl "restore_foremanctl_state.yaml" "obsah_state_path" "state: absent"
💡 Result:
The file restore_foremanctl_state.yaml is an Ansible task file within the foremanctl project [1], a tool used to manage Foreman and Katello deployments via Podman Quadlets [2]. The OBSAH_STATE_PATH (or the environment variable OBSAH_STATE) refers to the directory path where foremanctl maintains its internal state, configuration, and data, typically located at /var/lib/foremanctl on production installations [3][4]. Within the context of restore_foremanctl_state.yaml, the configuration state: absent is a standard Ansible module parameter used to ensure that a specified file, directory, or state resource is removed or does not exist on the target system during the restoration process [1][5]. This is common in deployment and restoration playbooks to clear existing, potentially conflicting state files before applying the state defined in the backup archive [5][3]. In summary, this task is part of the automated restoration workflow [1] that ensures the local foremanctl environment is cleaned (state: absent) [1] before it is overwritten with the data preserved from a previous backup [5].
Citations:
- 1: 2ecaa0d
- 2: https://github.com/theforeman/foremanctl
- 3: Add tests for backup #588
- 4: Add incremental backup and restore #609
- 5: Add foremanctl restore command - Complete offline backup restore #549
src/roles/restore/tasks/restore_foremanctl_state.yaml:2-6 — Only clear obsah_state_path before the base restore, not before every incremental layer.
restore_single.yaml participates in the chain-restore flow, and GNU tar incremental archives need to be applied cumulatively onto the same tree. Removing the directory on each layer drops files restored by earlier archives and leaves later incrementals incomplete.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/roles/restore/tasks/restore_foremanctl_state.yaml` around lines 2 - 6,
The restore task currently deletes the target state directory in a way that
breaks chain restores across incremental layers. Update the restore flow so
`restore_foremanctl_state` only clears `obsah_state_path` for the initial/base
restore path, and ensure `restore_single` does not remove the directory before
applying each incremental archive; keep later tar incrementals applied
cumulatively onto the same tree.
| - name: Restore databases from first backup in chain | ||
| ansible.builtin.include_tasks: | ||
| file: restore_databases.yaml | ||
| when: restore_single_is_first |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🔴 Critical | 🏗️ Heavy lift
Chain restore uses the stale, base-backup database dump instead of the latest one.
Database dumps are full dumps in every backup (only foremanctl state/Pulp content use --listed-incremental). Gating restore_databases.yaml on restore_single_is_first means chain restore always restores the database from the oldest (base) backup, discarding all DB changes captured by later backups in the chain — final state has fresh Pulp/state content but a stale database.
💡 Suggested direction
- - name: Restore databases from first backup in chain
- ansible.builtin.include_tasks:
- file: restore_databases.yaml
- when: restore_single_is_first
+ - name: Restore databases from latest backup in chain
+ ansible.builtin.include_tasks:
+ file: restore_databases.yaml
+ when: restore_single_index | int == restore_single_total | intThis likely requires moving the PostgreSQL start/stop bootstrapping out of restore_databases.yaml so it still runs once at the start of the chain, while the actual pg_restore step only fires on the last backup.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/roles/restore/tasks/restore_single.yaml` around lines 24 - 27, Chain
restore is restoring the wrong database snapshot because
`restore_single_is_first` only runs `restore_databases.yaml` for the base
backup. Update `restore_single.yaml` and the `restore_databases.yaml` flow so
PostgreSQL bootstrap/start-stop still happens once at the beginning of the
chain, but the actual `pg_restore` for the database runs on the last backup
instead of the first. Use the `restore_single_is_first` condition (or a new
end-of-chain check) to ensure later backup DB dumps are applied and the final
restored state matches the latest backup.
| - name: Check if foremanctl is already deployed | ||
| ansible.builtin.stat: | ||
| path: /var/lib/foremanctl | ||
| register: restore_existing_deployment | ||
|
|
||
| - name: Fail if restoring over existing system without --force | ||
| ansible.builtin.fail: | ||
| msg: | | ||
| DESTRUCTIVE OPERATION WARNING: | ||
|
|
||
| An existing Foreman deployment was detected on this system. | ||
| Restore will PERMANENTLY DELETE all current data including: | ||
| - All databases (Foreman, Candlepin, Pulp, etc.) | ||
| - All Pulp content | ||
| - All configuration | ||
|
|
||
| There is NO UNDO operation. | ||
|
|
||
| If you are certain you want to proceed, re-run with --force: | ||
| foremanctl restore {{ backup_dir }} --force | ||
| when: | ||
| - restore_existing_deployment.stat.exists | ||
| - not force | default(false) | ||
| - not validate | default(false) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n## Files\n'
git ls-files 'src/roles/restore/tasks/*' 'docs/user/restore.md' | sed 's#^`#-` #'
printf '\n## Search for chain-related gates\n'
rg -n --no-heading 'restore_single_is_first|restore_chain|validate\.yaml|force \| default|validate:|--force' src docs/user/restore.md
printf '\n## Outline relevant task files\n'
for f in src/roles/restore/tasks/validate.yaml src/roles/restore/tasks/restore_chain.yaml src/roles/restore/tasks/restore_single.yaml; do
echo "\n### $f"
ast-grep outline "$f" --view expanded || true
done
printf '\n## Read relevant file slices\n'
for f in src/roles/restore/tasks/validate.yaml src/roles/restore/tasks/restore_chain.yaml src/roles/restore/tasks/restore_single.yaml docs/user/restore.md; do
echo "\n===== $f ====="
nl -ba "$f" | sed -n '1,220p'
doneRepository: theforeman/foremanctl
Length of output: 2032
🌐 Web query:
theforeman foremanctl src/roles/restore/tasks/restore_chain.yaml restore_single.yaml validate.yaml docs/user/restore.md
💡 Result:
The files you referenced are part of the internal implementation of the restore functionality for foremanctl, a CLI tool for managing containerized Foreman and Katello deployments [1]. These specific Ansible roles and tasks handle the automated restoration of data from backups [2][3]. 1. src/roles/restore/tasks/restore_chain.yaml: This role implements support for restoring backup chains, particularly for incremental backups [2][3]. It allows foremanctl to automatically detect and restore a full backup followed by its dependent incremental backups when the --chain flag is used [3]. 2. src/roles/restore/tasks/restore_single.yaml: This role is used to perform a restore of a specific, single backup archive rather than a chain [2]. 3. src/roles/restore/tasks/validate.yaml: This task file is responsible for pre-restore validation [2]. It ensures the system environment and the backup itself are suitable for restoration [4]. This includes checks for disk space, backup integrity, system requirements, and configuration consistency (e.g., verifying that the backup hostname matches the current system) [4][5]. 4. docs/user/restore.md: This documentation file provides user-facing instructions on how to use the restore command [2][4]. It covers workflows such as running a validation-only restore, executing a full restore (which requires the --force flag on existing deployments), and utilizing the --chain flag for incremental backup restoration [3][4]. In summary, these components form the backend of the foremanctl restore command, which orchestrates the safe recovery of databases, Pulp content, and various system secrets, followed by a redeployment of containerized services [4]. Documentation for these procedures is maintained in the repository to guide users through the process [2][4].
Citations:
- 1: https://github.com/theforeman/foremanctl
- 2: 2ecaa0d
- 3: Add incremental backup and restore #609
- 4: Add foremanctl restore command - Complete offline backup restore #549
- 5: https://community.theforeman.org/t/foreman-maintain-functionality-report-for-foremanctl-port/46389
🌐 Web query:
site:github.com/theforeman/foremanctl "restore_single_is_first" foremanctl
💡 Result:
The term restore_single_is_first appears to be an internal Ansible variable or task flag associated with the restore logic in the foremanctl tool, rather than a user-facing command line parameter [1]. Foremanctl is a CLI tool for managing Foreman and Katello deployments using Podman and Ansible [2]. Based on the codebase for foremanctl, restore_single.yaml is a dedicated task file used during the restore process [1]. The restore_single_is_first indicator is likely a logic control mechanism used within these internal Ansible roles to manage the sequence of operations when performing a single (non-chained) restoration, ensuring that necessary prerequisites are handled correctly before the core restore tasks are executed [1]. For standard user operations, restoration is typically handled using the foremanctl restore command, with options such as --chain or --restore-chain (in newer versions) for handling incremental backups [3][1]. Users should interact with these high-level CLI flags rather than internal task variables [3][1].
Citations:
🌐 Web query:
site:raw.githubusercontent.com theforeman foremanctl "src/roles/restore/tasks/validate.yaml"
💡 Result:
The specific file path src/roles/restore/tasks/validate.yaml does not appear in the authoritative documentation or source code repositories for either foremanctl or the official Foreman infrastructure maintenance tools [1][2]. Foremanctl is a tool designed to install Foreman using containers deployed as podman quadlets [1]. Separate maintenance tasks, such as backups and restores, are typically handled by the foreman-maintain utility, which is a specialized tool for keeping Foreman and Satellite infrastructure running [2]. If you are encountering a validation task within an Ansible role related to restoration, it is likely part of a custom implementation or an unofficial Ansible collection rather than a core component of the Foreman project. Official Foreman maintenance, including backups and restores, relies on the foreman-maintain utility [2], and troubleshooting or configuration for Foreman components is generally managed through standard configuration files located in /etc/foreman or /etc/foreman-proxy [3].
Citations:
- 1: https://raw.githubusercontent.com/theforeman/foreman-packaging/rpm/develop/packages/foreman/foremanctl/foremanctl.spec
- 2: https://raw.githubusercontent.com/theforeman/foreman-packaging/rpm/develop/packages/foreman/rubygem-foreman_maintain/rubygem-foreman_maintain.spec
- 3: https://raw.githubusercontent.com/NETWAYS/foreman-training/gh-pages/advanced/06_troubleshooting.md
🌐 Web query:
site:raw.githubusercontent.com/theforeman/foremanctl 2ecaa0d7264d0a5ddbe8a3f80ca948e96fa9c966 src/roles/restore/tasks/restore_chain.yaml
💡 Result:
Found it. The file src/roles/restore/tasks/restore_chain.yaml at commit 2ecaa0d7264d0a5ddbe8a3f80ca948e96fa9c966 builds an incremental restore chain: it initializes the chain with the current backup, walks backward to find the full backup, reverses the chain, prints the detected order, and then restores each backup in chronological order. (raw.githubusercontent.com)
Citations:
🌐 Web query:
site:raw.githubusercontent.com/theforeman/foremanctl 2ecaa0d7264d0a5ddbe8a3f80ca948e96fa9c966 src/roles/restore/tasks/restore_single.yaml
💡 Result:
Found it. The file at that commit is a single Ansible task list that:
- displays restore progress
- sets
backup_dir - validates the backup
- restores foremanctl state
- prepares the system on the first backup in the chain
- restores databases on the first backup in the chain
- restores Pulp content
- records the successful restore timestamp [1]
[1] https://raw.githubusercontent.com/theforeman/foremanctl/2ecaa0d7264d0a5ddbe8a3f80ca948e96fa9c966/src/roles/restore/tasks/restore_single.yaml
🌐 Web query:
site:raw.githubusercontent.com/theforeman/foremanctl 2ecaa0d7264d0a5ddbe8a3f80ca948e96fa9c966 src/roles/restore/tasks/validate.yaml
💡 Result:
src/roles/restore/tasks/validate.yaml is the restore preflight for foremanctl. It checks for an existing deployment and blocks restore unless --force/validation bypass is used, verifies the backup directory and metadata.yml, parses backup metadata, validates incremental-backup chains, confirms required dump/archive files exist, and warns about disk space and database-mode mismatches. [1]
[1] https://raw.githubusercontent.com/theforeman/foremanctl/2ecaa0d7264d0a5ddbe8a3f80ca948e96fa9c966/src/roles/restore/tasks/validate.yaml
🌐 Web query:
site:github.com/theforeman/foremanctl/pull/609 restore_chain validate restore_single
💡 Result:
The terms restore_chain, validate, and restore_single are associated with backup and restore operations for the Foreman project's management tool, foremanctl, specifically regarding incremental backup support [1]. In the context of foremanctl, these options are used to manage the integrity and sequencing of incremental backup chains: 1. --chain: This flag is used with the restore command to enable automatic backup chain handling, allowing the tool to process a series of incremental backups [1]. 2. --validate: This flag is used in conjunction with the restore and --chain commands to verify the completeness and integrity of a backup chain before the restore operation proceeds [1]. 3. restore_single: While not explicitly defined as a standalone command flag in the documentation snippet, the workflow implies that manual restoration of individual backups can be performed, often requiring the --force flag to override the default chain validation logic when restoring steps out of sequence or without the full chain [1]. For standard operations, it is recommended to use the auto-chain feature, which can be validated as follows [1]: foremanctl restore /backups/foreman-backup-* --chain --validate If manual restoration is required, you would typically execute the restoration for each backup file in the chain sequence, often utilizing the --force flag for subsequent steps [1].
Citations:
Skip the existing-deployment guard for later chain members restore_chain runs restore_single for each backup, so once the first member restores /var/lib/foremanctl, this fail will trip on the next member unless the whole chain is forced. That blocks the documented automatic chain restore path; gate this check to the first backup in the chain.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/roles/restore/tasks/validate.yaml` around lines 2 - 25, The
existing-deployment guard in the restore validation task is too broad and will
fail on later members of a restore chain after the first `restore_single`
creates `/var/lib/foremanctl`. Update the `validate.yaml` logic around
`restore_existing_deployment` and the `ansible.builtin.fail` step so this check
only applies to the first backup in a chain (or otherwise skips subsequent chain
members), while still preserving the `force` and `validate` conditions.
| when: database_mode == 'internal' | ||
|
|
||
| - name: Get total size of database dumps | ||
| ansible.builtin.shell: du -sB1 {{ backup_dir }}/*.dump 2>/dev/null | awk '{sum+=$1} END {print int(sum/1024/1024/1024)+1}' |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win
Unquoted backup_dir interpolated into shell command.
backup_dir is inserted raw into a shell command run under become: true. Special characters (spaces, ;, `, $()) in this admin-supplied path would break or manipulate the command. Recommend quoting or using a safer module.
🛡️ Proposed fix
- name: Get total size of database dumps
ansible.builtin.shell: du -sB1 {{ backup_dir }}/*.dump 2>/dev/null | awk '{sum+=$1} END {print int(sum/1024/1024/1024)+1}'
+ args:
+ executable: /bin/bashOr better: quote the variable, e.g. du -sB1 {{ backup_dir | quote }}/*.dump ....
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/roles/restore/tasks/validate.yaml` at line 222, The shell command in the
restore validation task interpolates backup_dir directly, which can break or be
abused when the path contains spaces or shell metacharacters. Update the
ansible.builtin.shell usage in the validate task to safely quote backup_dir with
the existing du/awk pipeline, or switch to a safer module if possible. Use the
backup_dir variable in this shell step only through quoting so the restore size
check remains functional under become: true.
| - name: Get Pulp content archive size | ||
| ansible.builtin.stat: | ||
| path: "{{ backup_dir }}/pulp-content.tar.gz" | ||
| register: restore_pulp_archive | ||
| when: restore_pulp_archive is defined | ||
|
|
||
| - name: Warn if insufficient Pulp disk space | ||
| ansible.builtin.debug: | ||
| msg: "WARNING: Available space ({{ restore_pulp_space.stdout }}GB) may be insufficient for Pulp content ({{ (restore_pulp_archive.stat.size / 1024 / 1024 / 1024) | int + 1 }}GB needed)" | ||
| when: | ||
| - restore_pulp_archive.stat.exists | default(false) | ||
| - restore_pulp_space.stdout | int < (restore_pulp_archive.stat.size / 1024 / 1024 / 1024) | int + 1 |
There was a problem hiding this comment.
🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win
Dead code: Pulp disk-space check never executes.
when: restore_pulp_archive is defined (line 244) references the same task's own register variable before it has ever been set. On first evaluation this is always Undefined, so the stat task is always skipped, restore_pulp_archive stays undefined, and the follow-up warning at lines 246-251 (restore_pulp_archive.stat.exists | default(false)) silently defaults to false — the insufficient-space warning can never fire.
🐛 Proposed fix
- name: Get Pulp content archive size
ansible.builtin.stat:
path: "{{ backup_dir }}/pulp-content.tar.gz"
register: restore_pulp_archive
- when: restore_pulp_archive is defined📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - name: Get Pulp content archive size | |
| ansible.builtin.stat: | |
| path: "{{ backup_dir }}/pulp-content.tar.gz" | |
| register: restore_pulp_archive | |
| when: restore_pulp_archive is defined | |
| - name: Warn if insufficient Pulp disk space | |
| ansible.builtin.debug: | |
| msg: "WARNING: Available space ({{ restore_pulp_space.stdout }}GB) may be insufficient for Pulp content ({{ (restore_pulp_archive.stat.size / 1024 / 1024 / 1024) | int + 1 }}GB needed)" | |
| when: | |
| - restore_pulp_archive.stat.exists | default(false) | |
| - restore_pulp_space.stdout | int < (restore_pulp_archive.stat.size / 1024 / 1024 / 1024) | int + 1 | |
| - name: Get Pulp content archive size | |
| ansible.builtin.stat: | |
| path: "{{ backup_dir }}/pulp-content.tar.gz" | |
| register: restore_pulp_archive | |
| - name: Warn if insufficient Pulp disk space | |
| ansible.builtin.debug: | |
| msg: "WARNING: Available space ({{ restore_pulp_space.stdout }}GB) may be insufficient for Pulp content ({{ (restore_pulp_archive.stat.size / 1024 / 1024 / 1024) | int + 1 }}GB needed)" | |
| when: | |
| - restore_pulp_archive.stat.exists | default(false) | |
| - restore_pulp_space.stdout | int < (restore_pulp_archive.stat.size / 1024 / 1024 / 1024) | int + 1 |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/roles/restore/tasks/validate.yaml` around lines 240 - 251, The Pulp
disk-space validation is unreachable because Get Pulp content archive size uses
restore_pulp_archive in its own when condition before the register value exists.
Remove that self-referential guard and gate the stat task on the appropriate
restore/archive prerequisite instead, so restore_pulp_archive is actually
populated; then keep Warn if insufficient Pulp disk space dependent on
restore_pulp_archive.stat.exists and restore_pulp_space as intended.
2 participants
Summary
Implements full incremental backup/restore capability using GNU tar's --listed-incremental mechanism. Reduces backup time and storage for large Pulp deployments by only backing up changed files.
Depends on: #549 (restore command)
Changes
--incrementalparameter to backup command--listed-incrementalwith .snar snapshot files--chainflag to restore command for automatic backup chain handlingUsage
Create full backup (generates .snar files):
Create incremental backup:
Restore with auto-chain (recommended):
Manual chain restore:
How to test this pull request
Get a foremanctl box with normal deploy. On this box, clone foremanctl repo and checkout this branch:
Test incremental backup:
Test incremental restore with --chain: