Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixes #36480 - add restore_dir option to backup_local #738

Merged
merged 2 commits into from
Aug 16, 2023

Conversation

martin-schlossarek
Copy link
Contributor

Without a restore_dir option, snapshot backups cannot be restored with "foreman-maintain restore".

Without a restore_dir option, snapshot backups cannot be restored with "foreman-maintain restore".
@theforeman-bot
Copy link
Member

Issues: #36480

@ehelms
Copy link
Member

ehelms commented Jul 31, 2023

@evgeni Could you take a look at this?

@evgeni
Copy link
Member

evgeni commented Aug 15, 2023

I think the patch is correct for EL-based installs, but might break things on Debian where we might have multiple DB paths -- need to test this out.

(Also, I am not sure I like the name of the parameter, restore_dir, as at the time of its use we do not perform a restore but a backup. Maybe something like archive_base_dir or so is more appropriate, but I won't insist on the naming either way, just wanted to point it out)

@evgeni
Copy link
Member

evgeni commented Aug 15, 2023

/packit build

@evgeni
Copy link
Member

evgeni commented Aug 15, 2023

Okay, on CentOS 8 this worked fine, on Debian, this produces a bad backup.

working one:

root@debian11-foreman-nightly:~# tar tvf /var/backup/foreman-backup-2023-08-15-12-12-28/pgsql_data.tar.gz | grep main/PG_VERSION
-rw------- postgres/postgres 3 2023-08-15 12:08 var/lib/postgresql/13/main/PG_VERSION

after this patch:

root@debian11-foreman-nightly:~# tar tvf /var/backup/foreman-backup-2023-08-15-12-14-28/pgsql_data.tar.gz | grep PG_VERSION
-rw------- postgres/postgres 3 2023-08-15 12:08 []PG_VERSION

Going to investigate how we can fix that nicely.

We do not support snapshots on Debian-based systems right now, and the differences in the DB setup would break normal backups on Debian otherwise.
@evgeni evgeni merged commit c8bdc1c into theforeman:master Aug 16, 2023
5 of 7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants