Ummm I found a strange behaviour... 1. I run a manual backup for my site (web files). 2. It logs an error to the console (as if the copy has not completed) 3. In the backup tab of the site, 16 database backups appears. ?¿? 4. If I made borg list for web repo, I can see the manual backup as done, as well as 6 previous manual backups I performed. If I make a borg list for database repo I see 7 copies (not 16) of my database. Note: My repos are configured to hold 7 daily copies each. Any idea?
Do you do manual backups on the shell or manual backups through ISPConfig GUI by using backup now button?
Yes, this is a common issue with contributions. Often, their authors do not care about fixing, maintaining, and completing the work. They just dump their code, and in the end, someone from the core team must write it again from scratch or work through that code and fix it.
I wouldn't accept anymore such push requests if they are incomplete. I think that code - no matter if by external or by core - contributors should work on all platforms that ISPConfig supports and they should be feature-complete. The only exception I would make here is if they are too complex, but can still be split into multiple PRs in an ongoing development process without affecting the functionality of ISPConfig.
I think the problem could be in line 1997 of backup.inc.php. Lines 1993-1996: Code: $app->system->exec_safe( 'cd ? && ' . $command . ' ' . $command_opts . ' ' . $excludes . ' ? .', $web_path, $backup_repos_path . '::' . $web_backup_archive ); Line 1997: Code: $success = $app->system->last_exec_retcode() == 0; I think that for any reason, the code of last exec is other than 0, although the previous execution success. I don't know what object is $app or where is it documented sorry.
Which would indicate that the backup command failed. The $app object and safe_exec is the ISPConfig core, so that's not part of your issue as such core functions work fine. What the command is doing is this that the ? placeholders get replaced by the function parameters that follow and then the whole command gets executed via exec. you can find the system functions in the file server/lib/classes/system.inc.php So the best way to find out why this fails on your server is to add some echo statements in the file to get all the variables, then assemble the resulting command and execute it to see why it fails on your server.
That's why: https://borgbackup.readthedocs.io/en/stable/usage/general.html#return-codes I think the operation ends with warnings, but ends normaly. So, line 1997 shoud be that? Code: $success = $app->system->last_exec_retcode() > 1;
The code was working fine when it was accepted, and the backup was feature-complete for websites. And that a potential feature does not exist, like adding borg backup for email, is actually not a bug. The problem is that issues often arise later as platforms change and contributors do not fix their code then. The issue we discuss here currently is that a specific part of this code fails on this user's system, which is likely to be caused by specific settings he uses or the way he set up his environment or backup location.
This would mean the system will accept failed backups and show them as working, you will only notice that you can not restore it when you need it. I guess that's not how anybody wants a backup system to work. But you van try to do this modification for your system of course. As I mentioned, add some debug code, assemble the command, run it, to find out why it fails on your system.
Sorry, we crossed our posts . Yes, tomorrow I will debug the exit codes and check what the warning is. But maybe backup.inc.php should be updated to a) consider return code 1 as backup ok, cause in borg documentation says that 1 exit code means normal end with some warnings, or b) add other code to handle exit code 1, maybe another if nested into success code that logs a warning on ispconfig console, kind of "The backup xxxxxx ends with some warnings" Thanks.
If borg says exit code 1 means successful backup, then we can indeed change the code to e.g.: $success = ($app->system->last_exec_retcode() == 0 || $app->system->last_exec_retcode() == 1);
Borg documentation says that 1 exit code is a normal ends with warnings, so the backup should be recoverable. But I agree with you that considering only 0 a normal end is safer and more cautious.
One more question, it is a way to show the output of the last safe_exec? Maybe $app->system->last_exec_out() ?
