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

nfs_server as new restore method #2973

Merged
merged 9 commits into from May 3, 2023
Merged

Conversation

codefritzel
Copy link
Contributor

Relax-and-Recover (ReaR) Pull Request Template

Please fill in the following items before submitting a new pull request:

Pull Request Details:
  • Type: New Feature

  • Impact: Low

  • Reference to related issue (URL):

  • How was this pull request tested?
    Manually on Ubu22.04 and OL8.7

  • Brief description of the changes in this pull request:
    This pull request adds NFS_SERVER as a new restore method. In the rescue stage rear starts an NFS server and exports all mount points. With NFS version 4 the rootfs to be restored can be mounted with the following command:
    mount -t nfs -o nfsvers=4 <ip>:/ /mnt
    Another server with working backup software can mount this share and restore all data to it.
    Rear waits until the file 'rear_finished.txt' is created and all connections to the nfs server are disconnected. After that Rear continues with the restore process

@jsmeix jsmeix added the enhancement Adaptions and new features label Apr 25, 2023
@jsmeix jsmeix added this to the ReaR v2.8 milestone Apr 25, 2023
@jsmeix
Copy link
Member

jsmeix commented Apr 25, 2023

@codefritzel
thank you for your contribution to enhance ReaR!

Copy link
Member

@jsmeix jsmeix left a comment

Choose a reason for hiding this comment

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

The changes in usr/share/rear/conf/default.conf indicate
that this pull request is currently based on an older code state
because the current changes in usr/share/rear/conf/default.conf
revert things that were meanwhile changed in our current ReaR master code.
In the end this means the changes in this pull request should be
rebased on the state of our current ReaR master code.

@jsmeix
Copy link
Member

jsmeix commented Apr 25, 2023

All new added files are shown in
https://github.com/rear/rear/pull/2973/files
with a "No newline at end of file" mark
which means the last line does not end with a newline character.
As far as I know a newline character at the end of every line
(including the last line) is required at least by some programs
that read input line by line where only the newline character
marks the end of a line so that a line without terminating
newline character will not be read by such programs.

@schlomo schlomo self-assigned this Apr 26, 2023
@schlomo schlomo added the sponsored This issue will get solved on a paid base by ReaR upstream. label Apr 26, 2023
Copy link
Member

@schlomo schlomo left a comment

Choose a reason for hiding this comment

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

Thanks for the contribution! Please first of all rebase to current master.

Besides the detailed feedback I'd like to suggest one major change: Let's call this NFS4SERVER and limit the code to NFSv4 only, instead of trying to cover NFS 3 and 4 at the same time.

This change will still cover your use case but significantly reduce the testing efforts and make this code here smaller and simpler.

REQUIRED_PROGS_NFS_SERVER_V3=("rpc.statd" "rpc.idmapd" "rpc.gssd")
NFS_SERVER_EXPORT_OPTS="rw,no_root_squash,async,no_subtree_check"
NFS_SERVER_TRUSTED="*"
NFS_SERVER_MODULES=("nfs" "nfsd")
Copy link
Member

Choose a reason for hiding this comment

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

You'll have to check also if the modules are actually available

Copy link
Member

Choose a reason for hiding this comment

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

Main thought is that if rear mkrescue works, then there is a high chance that the configured features will also work. At the moment this is more on the level of "I hope that it will work"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

What is the best method to check if the modules are available?

Copy link
Member

@jsmeix jsmeix May 3, 2023

Choose a reason for hiding this comment

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

Regarding a check if kernel modules are available:
I assume what is meant is "available as files"
(i.e. no builtin kernel "modules").
You may have a look at the code and the comments of the

function modinfo_filename ...

in usr/share/rear/build/GNU/Linux/400_copy_modules.sh
which is in our current GitHub master code at
https://github.com/rear/rear/blob/master/usr/share/rear/build/GNU/Linux/400_copy_modules.sh#L26

In that script at its end we add all what there is
in MODULES_LOAD to "$ROOTFS_DIR/etc/modules" which is used in
skel/default/etc/scripts/system-setup.d/40-start-udev-or-load-modules.sh
that gets installed in the ReaR recovery system as
/etc/scripts/system-setup.d/40-start-udev-or-load-modules.sh
which basically calls modprobe -v $module for all
what there is in MODULES_LOAD so

MODULES_LOAD+=( QQQ )

results that modprobe -v QQQ will be called during
ReaR recovery system startupt which does not cause harm
but only an error message

# modprobe -v QQQ
modprobe: FATAL: Module QQQ not found in directory /lib/modules/...

which seems to be OK from my current point of view.

@gdha
Copy link
Member

gdha commented Apr 26, 2023

@codefritzel Could you please add some clear clarification why you need a NFS_SERVER workflow? Is NFS client not enough?

@schlomo
Copy link
Member

schlomo commented Apr 26, 2023

I can add some more context for this, I see three main scenarios for restore via NFS server:

  1. In some cases there is a problem running the backup client in our rescue system, and then it might be "cheaper" to use an indirect restore scheme instead of spending more hours on enabling our rescue system to support that piece of commercial software
  2. From a security perspective, it is always better to connect from a high-security ("bastion") system to the periphery. For backups this means that one could implement an NFS-based backup solution with pulling the date onto a central server. For restore we then need to start an NFS server in ReaR to push the data back. This could be part of a self-written solution or even use a commercial backup tool that is installed only on the central backup server for licensing or other reasons.
  3. Implement asymmetric solutions, using one tool for backup and another tool for restore, in this case via NFS

In all those cases adding NFS server capabilities to the ReaR rescue system is IMHO a useful addition. I'd also consider modifying it to be an optional add-on feature instead of a stand-alone backup method. I also envision introducing a general split of BACKUP and RESTORE, if somebody needs this.

@gdha
Copy link
Member

gdha commented Apr 26, 2023

@schlomo Thanks for the heads up as it makes it clear to me why it might be useful.
@codefritzel That being said please add some more comments from your side of the need of this new workflow, not just that it could be useful. And, above all, without proper documentation it will be a workflow only used by a happy few (amongst yourself I'm afraid).

@@ -0,0 +1,35 @@
# 300_start_nfs_server.sh

# same options works for mountd
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
# same options works for mountd
# same options works for rpc.nfsd and rpc.mountd


# or look at /var/lib/nfs/rmtab
nfs_connections=1
while [ ! -f "$check_file" ] || [ "$nfs_connections" -gt 0 ]; do
Copy link
Member

Choose a reason for hiding this comment

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

Just a thought: This while loop can potentially run forever. If you'd implement the df monitoring suggested below then you could also error out if the amount of restored data doesn't change within a configurable certain time, e.g. 5 mins or so.

@schlomo
Copy link
Member

schlomo commented May 2, 2023

Thanks a lot @codefritzel for the rework, I added you a bunch of minor fixes/changes and then I think we are good to go.

@pcahyna
Copy link
Member

pcahyna commented May 3, 2023

Hi, looks useful, but I have a question: will the various extended attributes/ACLs get restored properly? SELinux contexts are probably handled in usr/share/rear/restore/default/500_selinux_autorelabel.sh, but there are other extended attributes that matter. See:

$ getfattr -m- -nsecurity.capability - /usr/sbin/* /usr/bin/* 2>/dev/null
# file: usr/sbin/arping
security.capability=0sAAAAAgAgAAAAAAAAAAAAAAAAAAA=

# file: usr/sbin/clockdiff
security.capability=0sAAAAAgAgAAAAAAAAAAAAAAAAAAA=

# file: usr/bin/newgidmap
security.capability=0sAQAAAkAAAAAAAAAAAAAAAAAAAAA=

# file: usr/bin/newuidmap
security.capability=0sAQAAAoAAAAAAAAAAAAAAAAAAAAA=

AFAIK if one uses NFSv4.2, then one is able to use such attributes over the NFS mount, but I have not tried it. -V 4.2 needs to be passed to rpc.nfsd and mount option v4.2 (or nfsvers=4.2?) should be specified.

# List of share options for the recovery system, see 'General Options' in exports(5)
NFS4SERVER_EXPORT_OPTS=("rw" "no_root_squash" "async" "no_subtree_check")
# List of trusted systems that can mount the recovery system, see 'Machine Name Formats' in exports(5)
# e.g. NFS4SERVER_TRUSTED_CLIENTS=( myserver 172.16.76.0/24 )
Copy link
Member

Choose a reason for hiding this comment

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

I am not a NFS expert so I may confuse something but

NFS4SERVER_TRUSTED_CLIENTS=( myserver ...)

looks contradicting i.e. a trusted client called 'myserver'?

Copy link
Member

Choose a reason for hiding this comment

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

With NFS, a "client" connects to a "server" who authorises the "client" during connection. Therefore the term "allowed clients" or "permitted clients" or even just "clients" would probably be most correct.

Copy link
Member

Choose a reason for hiding this comment

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

My point was the possibly misleading word 'myserver' so

# e.g. NFS4SERVER_TRUSTED_CLIENTS=( myclient 172.16.76.0/24 )

seems to be better wording for this example.

Copy link
Member

@schlomo schlomo left a comment

Choose a reason for hiding this comment

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

Very nice, I think primarily the verify check for clients and maybe renaming the variable needs to be done.

Everything else, like a df based progress indicator, can also come later.

usr/share/rear/conf/default.conf Show resolved Hide resolved
usr/share/rear/conf/default.conf Outdated Show resolved Hide resolved
@codefritzel codefritzel requested a review from schlomo May 3, 2023 13:44
MODULES_LOAD+=("${NFS4SERVER_MODULES[@]}")

# Check if at least one trusted client was specified
(( "${#NFS4SERVER_TRUSTED_CLIENTS[@]}" > 0 )) || Error "You must have defined at least one client in NFS4SERVER_TRUSTED_CLIENTS."
Copy link
Member

Choose a reason for hiding this comment

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

You could omit this check from this file and add a symlink pointing to usr/share/rear/verify/NFS4SERVER/default/400_verify_nfs_server.sh to this directory instead to reduce code duplication a bit.

Copy link
Member

Choose a reason for hiding this comment

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

I think better to not mess with the MODULES_LOAD variable during verify because there no code cares about it and I don't like code that is meaningless but appears to be important. It might confuse other people who come to work on ReaR.

Copy link
Member

Choose a reason for hiding this comment

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

@schlomo oh, I meant only the last line, not the stuff above.


exportfs $v -ra || Error "exportfs failed!"

rpc.nfsd --debug "$nfs_threads" "${nfsd_opts[@]}" || Error "rpc.nfsd failed!"
Copy link
Member

Choose a reason for hiding this comment

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

Is it intended to turn on debugging unconditionally?

Copy link
Member

Choose a reason for hiding this comment

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

Yes, in case of error we'll see a bit more useful stuff, otherwise it doesn't matter. As we capture stdout the user isn't impacted.

Debug "nfsd startet with $nfs_threads threads."

if [ -z "$(pidof rpc.mountd)" ]; then
rpc.mountd --debug all "${nfsd_opts[@]}" || Error "rpc.mountd failed!"
Copy link
Member

Choose a reason for hiding this comment

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

ditto about --debug

@pcahyna
Copy link
Member

pcahyna commented May 3, 2023

Another question about NFS. Is NFSv4 required because of its ability to automatically export and access filesystems mounted under the exported filesystem, i.e. to cross mountpoints? Seems pretty crucial for use in ReaR.

@codefritzel
Copy link
Contributor Author

codefritzel commented May 3, 2023

Another question about NFS. Is NFSv4 required because of its ability to automatically export and access filesystems mounted under the exported filesystem, i.e. to cross mountpoints? Seems pretty crucial for use in ReaR.

@pcahyna that is one reason and it is much easier to support only v4 than v2, v3 and v4 together.

usr/share/rear/conf/default.conf Outdated Show resolved Hide resolved
@schlomo
Copy link
Member

schlomo commented May 3, 2023

Thank you everybody for reviewing this. I'm going forward and merging it now, we'll continue to improve this code as it undergoes more testing in production.

@schlomo schlomo merged commit bbda45d into rear:master May 3, 2023
6 of 14 checks passed
codefritzel added a commit to codefritzel/rear that referenced this pull request May 3, 2023
added new backup method NFS4SERVER for restore via NFSv4 export (restore only)
---------

Co-authored-by: Schlomo Schapiro <schlomo@schapiro.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Adaptions and new features fixed / solved / done sponsored This issue will get solved on a paid base by ReaR upstream.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants