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.

Sign up for GitHub

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

Jump to bottom

Backup of / is not cron friendly #181

Open
prontog opened this issue Nov 11, 2019 · 6 comments · May be fixed by #261
Open

Backup of / is not cron friendly #181

prontog opened this issue Nov 11, 2019 · 6 comments · May be fixed by #261

Comments

@prontog
Copy link

prontog commented Nov 11, 2019

Hello and thanks for this very useful tool!

When you backup /, the SRC_FOLDER var is stripped of the last / (line 378), and the df call (line 411) writes "df: '': No such file or directory" to STDERR. The script continues with the backup but it is not cron friendly since it always writes to STDERR and you cannot do:

MAILTO=panos@example.com
0 */1 * * * rsync_tmbackup.sh / /mnt/backup >/dev/null

and only expect emails when something unexpected happens. It will always send an email due to the df error.

Best regards,

Panos
@kapitainsky
Copy link
Contributor

definitely a bug introduced by #170

@prontog
Copy link
Author

prontog commented Nov 12, 2019

@kapitainsky, thanks for the quick response!

After adding a check so that it only strips when [[ $SRC_PATH != / ]], I stumbled on another cron unfriendliness :)

rsync_tmbackup.sh panos@example.com:/home/panos /mnt/backup_drive >/dev/null

will always write "Warning: Permanently added 'blah blah blah" (RSA) to the list of known hosts."

This is most likely because of the -o UserKnownHostsFile=/dev/null in lines 549, 551.

Wouldn't it be easier and more flexible to leave the ssh config to the user (with the -e option of rsync)?

The same goes for the necessary user part when parsing the SRC_FOLDERS and DEST_FOLDER in lines 177 and 188. I bet many users will set the user in their ~/.ssh/config file. I think simply splitting on the : is enough.

Should I make a PR? I could also add tests.

Best regards,

Panos

@kapitainsky
Copy link
Contributor

You should definitely create PR and explain all details. I am sure @laurent22 will consider including it.

@prontog prontog mentioned this issue Nov 12, 2019
Open
omer-musa-battal added a commit to omer-musa-battal/rsync-time-backup that referenced this issue Nov 14, 2019
@omer-musa-battal
macOS compatibility for filesystem type checks
c95cc96 
updates laurent22#170, by introducing cases for different OSes. Also includes fixes for laurent22#172 and part of laurent22#181.
@omer-musa-battal omer-musa-battal mentioned this issue Nov 14, 2019
Open
@reactive-firewall
Copy link

reactive-firewall commented Mar 19, 2020 •
edited
Loading

FYI this issue is caused by

rsync-time-backup/rsync_tmbackup.sh

Line 359 in 88db869

DEST_FOLDER="${DEST_FOLDER%/}"

and

rsync-time-backup/rsync_tmbackup.sh

Line 378 in 88db869

SRC_FOLDER="${SRC_FOLDER%/}"

@reactive-firewall
Copy link

#184 looks to be a possible fix for the / issue discussed here, but I wonder if fixing the issue when parsing args might be safer and easier to maintain 🤔 ... thoughts?

@prontog
Copy link
Author

prontog commented Mar 19, 2020

This sounds like the best approach. Normalize the arguments at the start.

This was referenced Mar 19, 2020
Open
Open
@normana10 normana10 linked a pull request Sep 14, 2023 that will close this issue
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Add flag to enable secure SSH features normana10/rsync-time-backup
3 participants
@prontog @kapitainsky @reactive-firewall