-
Notifications
You must be signed in to change notification settings - Fork 72
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
don't fail when chown to postgres fails #888
Conversation
evgeni
commented
Jun 25, 2024
- add prepare directory tests
- move chown postgres inside unless preserve
- only chown to postgres group for online backups with local DB
- don't fail when chown to postgres fails
all others do not need the postgres user to be able to read the backup for restore
# foreman-maintain backup online /mnt/online-new-1
Starting backup: 2024-06-25 09:11:30 +0000
…
--------------------------------------------------------------------------------
Prepare backup Directory:
Creating backup folder /mnt/online-new-1/katello-backup-2024-06-25-09-11-30
[WARNING]
/mnt/online-new-1/katello-backup-2024-06-25-09-11-30 could not be made readable by the 'postgres' user.
This won't affect the backup procedure, but you have to ensure that
the 'postgres' user can read the data during restore.
--------------------------------------------------------------------------------
…
--------------------------------------------------------------------------------
Scenario [Backup] failed.
The following steps ended up in warning state:
[backup-prepare-directory]
The steps in warning state itself might not mean there is an error,
but it should be reviewed to ensure the behavior is expected
Done with backup: 2024-06-25 09:11:39 +0000
**** BACKUP Complete, contents can be found in: /mnt/online-new-1/katello-backup-2024-06-25-09-11-30 ****
# echo $?
78 |
MSG | ||
set_status(:warning, warn_msg) | ||
end | ||
end | ||
end | ||
|
||
FileUtils.rm(Dir.glob(File.join(@backup_dir, '.*.snar'))) if @preserve_dir |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this be in the else
part of unless @preserve_dir
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe. But honestly I didn't want to touch too much of the code, just the parts that I actually wanted to change.
Also, I am not really sure why this is guarded by if @preserve_dir
anyway.
As when it's false
, the @backup_dir
will be fresh and empty and the .glob
will just return []
.
puts "Creating backup folder #{@backup_dir}" | ||
|
||
unless @preserve_dir |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this would actually make a lot more sense, but I can understand why you don't want to touch those too much
puts "Creating backup folder #{@backup_dir}" | |
unless @preserve_dir | |
if @preserve_dir | |
FileUtils.rm(Dir.glob(File.join(@backup_dir, '.*.snar'))) | |
else | |
puts "Creating backup folder #{@backup_dir}" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(moved the puts, but not the rm
)
I think what my brain tries to fight is the fact that "not preserve_dir" is the primary mode of operation of this code, so it should come first vs the else branch that is, well, "else" ;-)