MDL-42717 backups: set correct backup status and add logs
Set a correct backup status of ERROR when we fail to copy a file to an external directory given the backup configs demand it. Also a lot of logs are added around this area to let admins easily find the problem if backup fails because of directory permissions.
This commit is contained in:
committed by
Sam Hemelryk
parent
ab3effc0c6
commit
dca3bab337
@@ -396,18 +396,36 @@ abstract class backup_cron_automated_helper {
|
||||
$results = $bc->get_results();
|
||||
$outcome = self::outcome_from_results($results);
|
||||
$file = $results['backup_destination']; // May be empty if file already moved to target location.
|
||||
if (!file_exists($dir) || !is_dir($dir) || !is_writable($dir)) {
|
||||
|
||||
if (empty($dir) && $storage !== 0) {
|
||||
// This is intentionally left as a warning instead of an error because of the current behaviour of backup settings.
|
||||
// See MDL-48266 for details.
|
||||
$bc->log('No directory specified for automated backups',
|
||||
backup::LOG_WARNING);
|
||||
$outcome = self::BACKUP_STATUS_WARNING;
|
||||
} else if (!file_exists($dir) || !is_dir($dir) || !is_writable($dir) && $storage !== 0) {
|
||||
// If we need to copy the backup file to an external dir and it is not writable, change status to error.
|
||||
$bc->log('Specified backup directory is not writable - ',
|
||||
backup::LOG_ERROR, $dir);
|
||||
$dir = null;
|
||||
$outcome = self::BACKUP_STATUS_ERROR;
|
||||
}
|
||||
|
||||
// Copy file only if there was no error.
|
||||
if ($file && !empty($dir) && $storage !== 0 && $outcome != self::BACKUP_STATUS_ERROR) {
|
||||
$filename = backup_plan_dbops::get_default_backup_filename($format, $type, $course->id, $users, $anonymised,
|
||||
!$config->backup_shortname);
|
||||
if (!$file->copy_content_to($dir.'/'.$filename)) {
|
||||
$bc->log('Attempt to copy backup file to the specified directory failed - ',
|
||||
backup::LOG_ERROR, $dir);
|
||||
$outcome = self::BACKUP_STATUS_ERROR;
|
||||
}
|
||||
if ($outcome != self::BACKUP_STATUS_ERROR && $storage === 1) {
|
||||
$file->delete();
|
||||
if (!$file->delete()) {
|
||||
$outcome = self::BACKUP_STATUS_WARNING;
|
||||
$bc->log('Attempt to delete the backup file from course automated backup area failed - ',
|
||||
backup::LOG_WARNING, $file->get_filename());
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -298,6 +298,10 @@ abstract class backup_helper {
|
||||
@chmod($filedest, $CFG->filepermissions); // may fail because the permissions may not make sense outside of dataroot
|
||||
unlink($filepath);
|
||||
return null;
|
||||
} else {
|
||||
$bc = backup_controller::load_controller($backupid);
|
||||
$bc->log('Attempt to copy backup file to the specified directory using filesystem failed - ',
|
||||
backup::LOG_WARNING, $dir);
|
||||
}
|
||||
// bad luck, try to deal with the file the old way - keep backup in file area if we can not copy to ext system
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user