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

Portable recovery #3206

Merged
merged 1 commit into from Apr 12, 2024
Merged

Portable recovery #3206

merged 1 commit into from Apr 12, 2024

Conversation

schlomo
Copy link
Member

@schlomo schlomo commented Apr 9, 2024

Add OUTPUT=PORTABLE and --portable command line option to faciliate using ReaR in truly portable mode.

The portable archive contains only ReaR, nothing else.

Tested with an OL9 restore via SystemRescueCD

I'll do some more testing both of portable usage and regular usage to ensure that this change doesn't hurt us.

Implements #3190 and should be merged after #3205, where I extracted the unrelated fixes. To review you can simply look at the last commit here.

@schlomo schlomo requested a review from a team April 9, 2024 21:52
@schlomo schlomo self-assigned this Apr 9, 2024
Copy link
Member

@gdha gdha left a comment

Choose a reason for hiding this comment

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

Did I see this change in your other PR (misc stuff) too?

Copy link
Member

@gdha gdha left a comment

Choose a reason for hiding this comment

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

Same comment here - same as in your other PR (#3205)

@gdha
Copy link
Member

gdha commented Apr 10, 2024

I miss some documentation here. Is it possible to add it?

@schlomo
Copy link
Member Author

schlomo commented Apr 10, 2024

@gdha I added a short manual

@jsmeix the help workflow had bad indentation which I of course fixed, besides that I reduced the whitespace changes.

My goal at the moment is to get feedback on this and do some more tests. There will be a second phase of development with further optimisations (for example, I want to see if I can skip the build stage entirely to speed things up).

@schlomo schlomo requested a review from gdha April 10, 2024 10:40
@schlomo
Copy link
Member Author

schlomo commented Apr 10, 2024

Aaargh, now I get also #3168 (comment) because the TMPDIR is set for portable mode. Which means that I need to extend #3168 to not touch the temp dir for portable mode.

About dracut not showing errors in the ReaR log, I could redirect the output to properly capture it.

@jsmeix
Copy link
Member

jsmeix commented Apr 10, 2024

I did not have a look here (no time yet)
so the following is only an offhanded thought:

Regarding
#3206 (comment)
dracut and portable mode:

Currently we have in sbin/rear

test -e "/etc/rear-release" && RECOVERY_MODE='y' || RECOVERY_MODE=''
readonly RECOVERY_MODE

and we do not change TMPDIR in RECOVERY_MODE

    if ! test "$RECOVERY_MODE" ; then
        # We set TMPDIR to ReaR's TMP_DIR only when we are not in RECOVERY_MODE

so I think you need to enhance how RECOVERY_MODE is set
so that RECOVERY_MODE is also set for "rear recover"
in "portable mode"?

@schlomo
Copy link
Member Author

schlomo commented Apr 10, 2024

@jsmeix about the dracut not showing errors problem I figured out what is going on:

In normal (non-debug) ReaR mode stderr goes to $STDOUT_STDERR_FILE and the Error function can then quote from that. However, our code surrounding dracut doesn't use the Error function but instead only does a LogPrint suggesting to look into the $RUNTIME_LOGFILE, and that is the reason that the logfile doesn't contain useful infos.

How should we solve this? Maybe extract the "pull last lines from stderr" into a function that can then be used? Or add a LogPrintError function that will also show last errors if exist?

I'm not solving this here but I think it is important to keep in mind and fix so that users can get an info about the problem for errors without running ReaR again in debug mode.

@jsmeix
Copy link
Member

jsmeix commented Apr 10, 2024

I made the separated
#3207
regarding
#3206 (comment)

@schlomo schlomo force-pushed the portable-recovery branch 2 times, most recently from c0b7a8f to b4492e4 Compare April 10, 2024 16:42
@schlomo
Copy link
Member Author

schlomo commented Apr 11, 2024

I found a bug in this code, it doesn't work when used from a dist install :-(

I'll keep debugging then.

@schlomo
Copy link
Member Author

schlomo commented Apr 12, 2024

I found a bug in this code, it doesn't work when used from a dist install :-(

I'll keep debugging then.

It's the Makefile patching the main script, so I'll improve the Makefile for this, too.

@schlomo schlomo merged commit a9594b0 into master Apr 12, 2024
22 checks passed
@schlomo schlomo deleted the portable-recovery branch April 12, 2024 16:40
@jsmeix
Copy link
Member

jsmeix commented May 21, 2024

@schlomo
I am confused by your changed comment in sbin/rear
therein the part about PORTABLE mode:

# We set TMPDIR to ReaR's TMP_DIR only when we are not in RECOVERY_MODE and also not in PORTABLE mode
# because TMP_DIR does not exist in the recreated/restored system or in the foreign rescue system where
# the portable mode should be able to work without the need to have a TMP_DIR in the rescue system.
# i.e. there is no /mnt/local/var/tmp/rear.XXXXXXXXXXXXXXX/tmp so that
#   chroot /mnt/local COMMAND
# fails when COMMAND uses TMPDIR = TMP_DIR - in particular "chroot /mnt/local dracut" fails
# see https://github.com/rear/rear/pull/3168#issuecomment-1983377528

You added the part about PORTABLE mode because of your above
#3206 (comment)

But when `chroot /mnt/local dracut' fails with

++ chroot /mnt/local /bin/bash -c 'PATH=/sbin:/usr/sbin:/usr/bin:/bin /usr/bin/dracut --force'
realpath: /var/tmp/rear.bClgGxIS12HNrGx/tmp: No such file or directory
dracut: Invalid tmpdir '/var/tmp/rear.bClgGxIS12HNrGx/tmp'.

it fails not because there is no TMP_DIR in the rescue system
because also for 'rear recover' (for all workflows except 'help')
BUILD_DIR="$( mktemp -d -t rear.XXXXXXXXXXXXXXX ) and
TMP_DIR=$BUILD_DIR/tmp are created by sbin/rear
so there is always /var/tmp/rear.XXXXXXXXXXXXXXX/tmp
on the original system and in any rescue system.

It fails because there is no TMP_DIR
in the recreated/restored system mounted under /mnt/local/
i.e. there is no /mnt/local/var/tmp/rear.XXXXXXXXXXXXXXX/tmp

So I think the part about PORTABLE mode in that comment
could be corrected and simplified to:

# We set TMPDIR to ReaR's TMP_DIR only when we are not in RECOVERY_MODE and also not in PORTABLE mode
# because TMP_DIR does not exist in the recreated/restored system
# i.e. there is no /mnt/local/var/tmp/rear.XXXXXXXXXXXXXXX/tmp so that
#   chroot /mnt/local COMMAND
# fails when COMMAND uses TMPDIR = TMP_DIR - in particular "chroot /mnt/local dracut" fails
# see https://github.com/rear/rear/pull/3168#issuecomment-1983377528
# and https://github.com/rear/rear/pull/3206#issuecomment-2047273428

I think the actual root cause is that
RECOVERY_MODE is not set when "rear recover" is run
in PORTABLE mode because I assume /etc/rear-release
does not exist in a foreign rescue system
so the comment could be corrected and enhanced like

# We set TMPDIR to ReaR's TMP_DIR only when we are not in RECOVERY_MODE
# and also not in PORTABLE mode because RECOVERY_MODE is not set in PORTABLE mode
# (because /etc/rear-release does not exist in a foreign rescue system)
# and TMP_DIR does not exist in the recreated/restored system
# i.e. there is no /mnt/local/var/tmp/rear.XXXXXXXXXXXXXXX/tmp so that
#   chroot /mnt/local COMMAND
# fails when COMMAND uses TMPDIR - in particular "chroot /mnt/local dracut" fails
# see https://github.com/rear/rear/pull/3168#issuecomment-1983377528
# and https://github.com/rear/rear/pull/3206#issuecomment-2047273428

@schlomo
I am still wondering why you did not set
RECOVERY_MODE also in PORTABLE mode, cf. my
#3206 (comment)

In sbin/rear we have currently

# Set RECOVERY_MODE when we are running inside the rescue/recovery system.
# RECOVERY_MODE is a boolean variable (cf. conf/default.conf) because we need it below
# to set TMPDIR to ReaR's TMP_DIR only when we are not running inside the recovery system
# but we do not yet have lib/global-functions.sh (which contains is_false) sourced here.
# In the recovery system /etc/rear-release is unique (it does not exist otherwise)
# see build/default/970_add_rear_release.sh that adds it to identify the rescue environment:
test -e "/etc/rear-release" && RECOVERY_MODE='y' || RECOVERY_MODE=''
readonly RECOVERY_MODE

where "the rescue/recovery system" means here
only ReaR's own rescue/recovery system
and not a foreign rescue system in PORTABLE mode.

As far as I understand
https://github.com/rear/rear/blob/master/doc/user-guide/17-Portable-Mode.adoc
the PORTABLE mode is meant to be used only
when 'rear' is run within a foreign rescue system
so it should be exactly right to set RECOVERY_MODE
also in PORTABLE mode because RECOVERY_MODE means
"we run in a rescue system".

Or do I misunderstand something and PORTABLE mode is also
meant to be used to run 'rear' on the original system?

@schlomo
Copy link
Member Author

schlomo commented May 21, 2024

@jsmeix ahhh, good point and good catch. ATM indeed the only use case for portable mode is recovery. I think that I felt that I shouldn't needlessly limit the portable mode to only that, but now that you mention it maybe that was stupid and indeed portable should be limited to recovery only.

@jsmeix
Copy link
Member

jsmeix commented May 21, 2024

I will do a pull request...

jsmeix added a commit that referenced this pull request May 21, 2024
In sbin/rear set RECOVERY_MODE also in PORTABLE mode,
see #3206 (comment)
and #3206 (comment)
@jsmeix
Copy link
Member

jsmeix commented May 21, 2024

@schlomo
could you please review my
#3228
and ideally - as time permits - test if that really
works as intended in PORTABLE mode?

For a quick "smoke test" it should be sufficient
to only let 'rear -dp recover' start up in a
foreign rescue environment and look if its initial

Setting TMPDIR to ...

DebugPrint message shows the intended TMPDIR value.

@jsmeix
Copy link
Member

jsmeix commented May 21, 2024

@schlomo
regarding "portable should be limited to recovery only":

I think for that in
usr/share/rear/init/default/002_check_rear_recover_mode.sh
the current global permission for portable mode

is_true "$PORTABLE" && return

should be enhanced to a more specific test,
in particular to avoid on the original system
things like rear -p recover.

BUT
currently I have no good idea how to find out
if we are in an arbitrary (foreign) rescue system.

Perhaps the assumption that a rescue system runs in RAM
(so '/' on a rescue system is not "normally mounted")
could help, for example:

My homeoffice workstation with a physical disk:

# findmnt -o 'TARGET,SOURCE,FSTYPE' /
TARGET SOURCE              FSTYPE
/      /dev/mapper/cr_root ext4

My ReaR "original system" test VM:

# findmnt -o 'TARGET,SOURCE,FSTYPE' /
TARGET SOURCE                              FSTYPE
/      /dev/sda2[/@/.snapshots/1/snapshot] btrfs

My ReaR recovery system VM
(made on my "original system" test VM):

RESCUE localhost:~ # findmnt -o 'TARGET,SOURCE,FSTYPE' /
TARGET SOURCE FSTYPE
/      none   rootfs

Perhaps when the 'findmnt' SOURCE is 'none'
it sufficiently indicates that the system runs in RAM
which again sufficiently indicates it is a rescue system?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants