Improve layout configuration part of the user guide
Pull Request Details:
-
Type: Enhancement
-
Impact: Low
-
Reference to related issue (URL): related to #2997
-
How was this pull request tested? Not tested.
-
Description of the changes in this pull request: Improvements of the layout configuration part of the user guide to address #2997
- Describe
EXCLUDE_RECREATEbetter. Instead of copying a snippet of default.conf, describe in prose what the variable does and what is the syntax. - Update whitespace after comment in layout examples. Commit 4d6fac780f9234b55dbb0ca7fa55b56e4fbc7157 changed the formatting of commented entries in layout: "Change commenting in layout file to have no space after the #." Adapt the examples that had been generated before the change to match.
- Do not copy almost identical layout file in the documentation multiple times. Show only the parts that have changed after changing something.
- Move autoexcludes description. I believe that "This behavior is controlled by the
AUTOEXCLUDE_DISKS=yparameter" must refer to the exclusion of the unused backup disk. At the current location (disk layout when the backup disk is NOT unused anymore) it does not make sense. Move the whole part about autoexcludes just after the section where it is mentioned that a disk was automatically excluded. - Don't copy EXCLUDE_BACKUP/RESTORE from default.conf.
EXCLUDE_RESTOREandEXCLUDE_BACKUPare irrelevant for layout file and disk layout recreation. They merely influence file backup or restore (for supported backup methods). This guide is about the recreation of the disk layout. Therefore, omit the variables from here. They are not used in the doc anywhere anyway after showing a copy of their description originating from default.conf. So, why to show it at all? - Do not refer to "restore list". This term is never defined or used anywhere. Let's talk about the layout file instead.
- Describe
Open question: where to describe variables that control only the file backup and restoration (for supported backup methods)? Like EXCLUDE_RESTORE and EXCLUDE_BACKUP and BACKUP_PROG_EXCLUDE. They don't belong here as this chapter is only about storage layout recreation. I think it would be the best to introduce chapter 7 with this information.
@danboid can you please have a look whether this helps with your issue #2997 ?
diff gets confused by the move of a large piece of text, so the diff of the whole PR does not make much sense. I suggest to look at per-commit diffs (https://github.com/rear/rear/pull/3125/commits) when reviewing the PR, each of them looks reasonable.
Thanks for working on the rear docs!
I've had a look at these commits but I have nothing to add really.
I might be able to comment better once committed and I can read it as a whole but even then, I don't know rear well enough to be correcting one of the devs.
@danboid FYI: you can see the changes via https://github.com/rear/rear/pull/3125/files (it even works when one is not logged in at GitHub) and therein at the top right corner of the sub-window that shows the changes for a file click on the three dots '...' which shows a sub-menu where you can select "View file" so you can read the changed file as a whole.
Hi @jsmeix @schlomo thank you for looking, I made some changes according to your comments.
Any opinion on
Open question: where to describe variables that control only the file backup and restoration (for supported backup methods)? Like EXCLUDE_RESTORE and EXCLUDE_BACKUP and BACKUP_PROG_EXCLUDE. They don't belong here as this chapter is only about storage layout recreation. I think it would be the best to introduce chapter 7 with this information.
?
I think a new chapter that only describes variables that control only the file backup and restore is not possible in practice because of the various interdependencies with the variables that control the disk layout.
Right now I see that we have already a chapter 7 https://github.com/rear/rear/blob/master/doc/user-guide/07-tips-and-tricks.adoc which I had never read before.
Yes, unfortunately the chapters are numbered, so to insert a new one we would have to renumber the following ones.
@jsmeix do you think the description of file backup-related variables should be added also to this chapter? This would make it less focused, though.
@pcahyna offhandedly I think disk layout and files backup should be kept separated primarily because
Relax-and-Recover (ReaR) complements backup and restore
of files but ReaR is neither a backup software
nor a backup management software
and it is not meant to be one.
In general backup and restore of the files
is external functionality for ReaR.
https://en.opensuse.org/SDB:Disaster_Recovery#Relax-and-Recover_versus_backup_and_restore
At first glance I think https://github.com/rear/rear/blob/master/doc/user-guide/07-tips-and-tricks.adoc is not really useful - perhaps even bad in some cases, for example:
- I think GRUB_RESCUE can be useful but at the same time it is a bad idea to let ReaR modify the bootloader of the original system
- Why it is a "tip" or a "trick" that it automatically detects and enables serial console support?
- Preserving SSH keys can be useful but at the same time it is a bad idea to leak out secret SSH keys from the original system into the (possibly "public" accessible) recovery system (initrd/ISO/...)
- During recovery one can NOT at any stage re-run "rear recover". In my experience in most cases re-running "rear recover" fails or would only work with too complicated manual things so that in most cases it works better in practice to reboot the recovery system, do needed adaptions, and then run "rear recover" again.
So from my current point of view https://github.com/rear/rear/blob/master/doc/user-guide/07-tips-and-tricks.adoc is mostly questionable / misleading / wrong / ...
- Preserving SSH keys can be useful but at the same time it is a bad idea to leak out secret SSH keys from the original system into the (possibly "public" accessible) recovery system (initrd/ISO/...)
The document does not suggest to leak secrets. I believe that what is intended is to mention the preservation of root's .ssh/authorized_keys, which allows you to login remotely. This file is not especially sensitive.
- During recovery one can NOT at any stage re-run "rear recover". In my experience in most cases re-running "rear recover" fails or would only work with too complicated manual things so that in most cases it works better in practice to reboot the recovery system, do needed adaptions, and then run "rear recover" again.
Maybe. But the whole layout recreation is implemented with this in mind, with all the todo and done in disktodo.conf. This is also the point of the sentence just above https://github.com/rear/rear/blob/master/doc/user-guide/06-layout-configuration.adoc#planning-in-advance :
Notice how Relax-and-Recover detected that it had already created quite a few components and did not try to recreate them anymore.
I must say I don't like the whole section https://github.com/rear/rear/blob/master/doc/user-guide/06-layout-configuration.adoc#restore-to-different-hardware , WDYT?
@danboid can you please have a look at the resulting document https://github.com/rear/rear/blob/16d6be5664a24ce5f30e5f8b9cb727e89a15c37f/doc/user-guide/06-layout-configuration.adoc ? Not to check correctness, but to see whether it is better understandable than the previous version and addresses https://github.com/rear/rear/issues/2997 . (Note that the backup options EXCLUDE_BACKUP etc. are deliberately omitted, as they don't refer to the layout configuration.)
@pcahyna the section https://github.com/rear/rear/blob/master/doc/user-guide/06-layout-configuration.adoc#restore-to-different-hardware is rather old, in particular I never updated it.
I did not pay attention to this section because at SUSE we do not support migration to different hardware.
In general I find migration to different hardware not at all something that is "simple" with ReaR.
I think what that section basically tells is that migration to different hardware does not "just work".
So the basic information is still true and users can clearly see that with an example where the technical details are somewhat outdated which is a minor issue here because nobody has exactly such a system so the details should not matter in practice for this section.
@jsmeix I especially dislike the wording
There are two ways to deal with different hardware. One is being lazy and dealing with problems when you encounter them. The second option is to plan in advance. Both are valid approaches.
as it sounds like one can leave figuring out the details to the moment when disaster happens (although this may not have been the original intent of the text).
@pcahyna according to
# git log --follow -p doc/user-guide/06-layout-configuration.adoc
that part about "being lazy" is unchanged since it was introduced at the beginning as "a first draft of a layout engine usage guide" via https://github.com/rear/rear/commit/259b66cd59a0cde9cf0070c2fa3048c7ddb13664
I think what it tries to tell is that
- the lazy approach is meant when the target hardware is unknown in advance and then one must have good knowledge of one's system - actually the lazy approach is the only possible way in practice when one does not already have replacement hardware to verify it in advance (e.g. for individual end user workstations and laptops)
- the plan in advance approach is meant ("is preferable") when the target hardware is known in advance (in particular for business/enterprise environments)
At SUSE we officially support only the latter case, cf. https://en.opensuse.org/SDB:Disaster_Recovery and https://documentation.suse.com/sle-ha/15-SP5/html/SLE-HA-all/cha-ha-rear.html
@pcahyna is it good enough so that it can be merged (even if some parts are not yet perfectly well) or are further improvements required in this pull request?
@jsmeix I have been hoping that my professional doc writer colleague would review the PR and possibly even help with the numerous formal/style problems that we identified, that's why I have kept the PR open. Does it conflict with your work?
@pcahyna no problem, take your time. There is no conflict with my work. I was only wondering why it did not proceed.
Stale pull request message
@pcahyna @jsmeix Moved the milestone to "3.0" - hope that is ok for you?
@gdha @pcahyna Rear 3.0 milestone is fine with me. I would prefer to have it merged as is over not having this improvement in ReaR 3.0 because it is already a good improvement regardless of some still existing issues.
@pcahyna @rear/contributors I would like to merge it this week on Thursday afternoon (or sooner if @pcahyna agrees) unless there are objections.
@pcahyna I would like to merge it today afternoon, cf. https://github.com/rear/rear/pull/3125#issuecomment-2172462514 unless there are objections.