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

Backup and restore scripts for Linux/MacOS #507

Merged
merged 1 commit into from
Sep 24, 2017

Conversation

BClark09
Copy link
Member

@BClark09 BClark09 commented Aug 4, 2017

Related to #499

Signed-off-by: Ben Clark ben@benjyc.uk

@BClark09
Copy link
Member Author

BClark09 commented Aug 4, 2017

That's my best stab at it for now, would appreciate reviews, suggestions and testing @ThomDietrich and @whopperg :)

@BClark09
Copy link
Member Author

BClark09 commented Aug 4, 2017

image

@thopiekar
Copy link
Member

Like it! I previously also added something like a backup mechanism for QNAP NASs. As soon as this is ready I'll adopt it.

@BClark09
Copy link
Member Author

BClark09 commented Aug 5, 2017

Thanks @thopiekar, looking at the folder structure it for QNAP NASs it should work fine, have you already tested it for it?

@kaikreuzer, could you test these scripts on MacOS?

@ThomDietrich
Copy link
Member

ThomDietrich commented Aug 5, 2017

Hey @BClark09, I went through the code thoroughly and everything looks great.

The only thing I still don't quite like is "Usage: ./runtime/bin/backup [filename]". Feels unnatural to demand a relative path for execution of a script/program. As a user I'd like to run the script with an absolute path or right from inside the bin folder.

Furthermore I'm wondering if we should add symlinks to these scripts to /usr/bin and /usr/sbin in the https://github.com/openhab/openhab-linuxpkg project.

@thopiekar
Copy link
Member

@BClark09 No, I haven't but implemented a simple function to my service script to make backups of the whole distribution (only working when openHAB is offline, of course.).
So yeah, nice to have a commonized script/function now!

@BClark09
Copy link
Member Author

BClark09 commented Aug 5, 2017

@ThomDietrich ,

The only thing I still don't quite like is "Usage: ./runtime/bin/backup [filename]". Feels unnatural to demand a relative path for execution of a script/program

This is only necessary if the OPENHAB_* environment paths are unknown. It'll work anywhere otherwise. I'd rather not try to work out where the appropriate path is from the file location, since this starts to break down with symlinks and I can't think of a platform independent way of doing it.

Furthermore I'm wondering if we should add symlinks to these scripts to /usr/bin and /usr/sbin in the https://github.com/openhab/openhab-linuxpkg project.

Actually, I was considering adding /usr/bin/openhab soon to add the commands:

openhab console
openhab backup $@
openhab restore $@
openhab showlogs

thoughts? Perhaps for another issue.

@thopiekar
Copy link
Member

I agree with @ThomDietrich. It would be nice, if the filename would be optional. This way people can decide whether they want to create a archive inside the distro or eg. outside.

@kaikreuzer
Copy link
Member

@kaikreuzer, could you test these scripts on MacOS?

Tested, not working. I get the message (both on backup and restore)

cp: the -R and -r options may not be specified together.
cp: the -R and -r options may not be specified together.

and the backup only contains empty conf and userdata folders.

Regarding the message on restore:

Any existing files with the same name will be replaced, but this will NOT delete existing files that have no replacement.

Why not? If I restore a backup, I would want the setup of that moment and not some mix of current stuff and restored stuff. Or do I misinterpret the notice?

@thopiekar
Copy link
Member

@kaikreuzer That's why I make a full copy of the whole distro and then if something goes wrong I remove everything and restore all from the backup.
Just like I would expect it from a 'snapshot' of a known working state.

@BClark09
Copy link
Member Author

BClark09 commented Aug 5, 2017

Alright will rework. Not sure about that -R, -r error though. It only uses one.

@kaikreuzer
Copy link
Member

Checking the man page it says:

     -a    Same as -pPR options. Preserves structure and attributes of files but not directory structure.

so by specifying -a you implicitly have a -R as it seems.

@kaikreuzer
Copy link
Member

... and indeed, when I remove the r, the backup script seems to work nicely 👍

@BClark09
Copy link
Member Author

BClark09 commented Aug 5, 2017

Alright, made a couple of changes that should mean that the script can be called from anywhere, and now should work on MacOS (Thanks @kaikreuzer!)

I would want the setup of that moment and not some mix of current stuff and restored stuff. Or do I misinterpret the notice?

You didn't misinterpret, but I understand this is probably the desired choice. How best to proceed with that then? Delete all but the specific userdata/etc/ files and then the backup on top of the somewhat empty directory?

@BClark09
Copy link
Member Author

BClark09 commented Aug 6, 2017

Hi all, I've made some adjustments so that it completely replaces userdata and conf, getting rid of anything that wasn't in the backup.

Please could you check and test these new changes, as we need to eliminate anything that could be potentially dangerous.

@kaikreuzer, will these new changes work on a mac?

@bdleedy
Copy link
Contributor

bdleedy commented Aug 7, 2017

I'm writing the Windows version of this and I'm noticing that you put the backup in runtime\bin\backups. If you would then use upgrade that backup would then be deleted. Is this intended? Is there a plan to modify upgrade to handle retaining the backup?

@BClark09
Copy link
Member Author

BClark09 commented Aug 7, 2017

Hi @bdleedy, the backup should be stored in openhab_home/backups by default. Is that not the case here? The line:

 WorkingDir="$(cd "$(dirname "$0")" && cd ../.. && pwd -P)"

should point two directories up from where the backup script is.

@bdleedy
Copy link
Contributor

bdleedy commented Aug 7, 2017

NM. Duh. Apparently too much code floating around in my head yesterday. Thanks.

@BClark09 BClark09 force-pushed the linux-backup branch 3 times, most recently from 2acb05c to 89bd9c0 Compare August 8, 2017 00:18
@BClark09
Copy link
Member Author

BClark09 commented Aug 8, 2017

Alright, I think I've finished with this. @ThomDietrich, @kaikreuzer and @thopiekar, I've taken all comments on board. Please let me know how you get on with testing and reviewing the two scripts. I've tested on a debian virtual machine, but please try to break the scripts as much as possible.


## Restore configuration
echo "Restoring openHAB with backup configuration..."
command cp -af "$TempDir/conf/"* "${OPENHAB_CONF:?}/" || {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what's the command needed for?

Copy link
Member Author

@BClark09 BClark09 Aug 10, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some distributions use an alias for cp which includes the -i flag. There are a ton of files here, so that might not be the best of things to include.

https://serverfault.com/questions/119869/turning-off-cp-copy-commands-interactive-mode-cp-overwrite
https://superuser.com/questions/643388/force-copy-when-i-is-used-in-bash-alias

@kaikreuzer
Copy link
Member

@BClark09 I have tested it on Mac, seems to run smoothly! 👍

@kaikreuzer
Copy link
Member

Any further comments/tests by the others? Or do you think we are good to merge?

@ThomDietrich
Copy link
Member

I did not get around doing any tests (my time is a bit limited atm) but I went through the whole code and it LGTM.

@openhab-bot
Copy link
Collaborator

This pull request has been mentioned on openHAB Community. There might be relevant details there:

https://community.openhab.org/t/recommended-way-to-backup-restore-oh2-configurations-and-things/7193/69

Signed-off-by: Ben Clark <ben@benjyc.uk>
@BClark09
Copy link
Member Author

Hi @ThomDietrich and @kaikreuzer, would you be able to test this once more? I cleaned up and used the scripts on my own home today so these should be fine to use. I've tried to harden the script as much as possible but please try to break it if you can :)

@ThomDietrich
Copy link
Member

LGTM. Great work @BClark09 !

@whopperg
Copy link

TOP!!! @BClark09

@kaikreuzer
Copy link
Member

Thanks @BClark09, successfully tested it on Mac as well!

@kaikreuzer kaikreuzer merged commit 334f234 into openhab:master Sep 24, 2017
@BClark09
Copy link
Member Author

Awesome, thanks everyone! Shall I put a post up on the community forums about this?

@kaikreuzer kaikreuzer added this to the 2.2.0 milestone Dec 15, 2017
@kaikreuzer kaikreuzer added the enhancement An enhancement or new feature label Dec 15, 2017
@kaikreuzer kaikreuzer changed the title Add backup and restore scripts for Linux/MacOS Backup and restore scripts for Linux/MacOS Dec 15, 2017
@kaikreuzer kaikreuzer modified the milestone: 2.2 Dec 15, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement An enhancement or new feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants