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

OS vendor and version autodetection fails on Fedora #3149

Open
lzaoral opened this issue Feb 8, 2024 · 19 comments
Open

OS vendor and version autodetection fails on Fedora #3149

lzaoral opened this issue Feb 8, 2024 · 19 comments
Labels
bug The code does not do what it is meant to do cleanup
Milestone

Comments

@lzaoral
Copy link
Contributor

lzaoral commented Feb 8, 2024

  • ReaR version ("/usr/sbin/rear -V"): latest HEAD

  • OS version ("cat /etc/os-release" or "lsb_release -a" or "cat /etc/rear/os.conf"):

$ cat /etc/os-release
NAME="Fedora Linux"
VERSION="39 (Thirty Nine)"
ID=fedora
VERSION_ID=39
VERSION_CODENAME=""
PLATFORM_ID="platform:f39"
PRETTY_NAME="Fedora Linux 39 (Thirty Nine)"
ANSI_COLOR="0;38;2;60;110;180"
LOGO=fedora-logo-icon
CPE_NAME="cpe:/o:fedoraproject:fedora:39"
DEFAULT_HOSTNAME="fedora"
HOME_URL="https://fedoraproject.org/"
DOCUMENTATION_URL="https://docs.fedoraproject.org/en-US/fedora/f39/system-administrators-guide/"
SUPPORT_URL="https://ask.fedoraproject.org/"
BUG_REPORT_URL="https://bugzilla.redhat.com/"
REDHAT_BUGZILLA_PRODUCT="Fedora"
REDHAT_BUGZILLA_PRODUCT_VERSION=39
REDHAT_SUPPORT_PRODUCT="Fedora"
REDHAT_SUPPORT_PRODUCT_VERSION=39
SUPPORT_END=2024-11-12
  • Description of the issue (ideally so that others can reproduce it):

ReaR does not properly detect OS vendor and version on Fedora:

# /usr/sbin/rear dump
...
# System definition:
  ARCH="Linux-arm"
  OS="GNU/Linux"
  OS_MASTER_VENDOR="Fedora"
  OS_MASTER_VERSION="VERSION_ID=39"
  OS_MASTER_VENDOR_ARCH="Fedora/arm"
  OS_MASTER_VENDOR_VERSION="Fedora/VERSION_ID=39"
  OS_MASTER_VENDOR_VERSION_ARCH="Fedora/VERSION_ID=39/arm"
  OS_VENDOR="RedHatEnterpriseServer"
  OS_VERSION="VERSION_ID=39"
  OS_VENDOR_ARCH="RedHatEnterpriseServer/arm"
  OS_VENDOR_VERSION="RedHatEnterpriseServer/VERSION_ID=39"
  OS_VENDOR_VERSION_ARCH="RedHatEnterpriseServer/VERSION_ID=39/arm"
...

The version should be only 39 and not VERSION_ID=39 and the vendor should be Fedora. Therefore, both the OS_VENDOR_* and OS_MASTER_VENDOR_* should be equal.

cc: @pcahyna

@jsmeix
Copy link
Member

jsmeix commented Feb 8, 2024

As far as I know this variables are set by the function
SetOSVendorAndVersion
in lib/config-functions.sh

I think this function is rather fragile
(some time ago I have had "not so much fun" with it).
I assume most of its values are not actually used in ReaR scripts
so perhaps things could be simplified to what is actually used?

@jsmeix
Copy link
Member

jsmeix commented Feb 8, 2024

FYI:

What variables are set by SetOSVendorAndVersion:

# grep -o '[A-Z][A-Z_]*=' usr/share/rear/lib/config-functions.sh | cut -d '=' -f1 | sort -u
OS_MASTER_VENDOR
OS_MASTER_VENDOR_ARCH
OS_MASTER_VENDOR_VERSION
OS_MASTER_VENDOR_VERSION_ARCH
OS_MASTER_VERSION
OS_VENDOR
OS_VENDOR_ARCH
OS_VENDOR_VERSION
OS_VENDOR_VERSION_ARCH
OS_VERSION
VERSION_ID

Where those variables are used in ReaR scripts:
Here I excluded ReaR scripts that use those variables
in a generic way which are

usr/share/rear/lib/config-functions.sh
usr/share/rear/lib/dump-workflow.sh
usr/share/rear/lib/framework-functions.sh
usr/share/rear/lib/validate-workflow.sh
usr/sbin/rear
usr/share/rear/build/default/975_update_os_conf.sh
usr/share/rear/rescue/default/010_merge_skeletons.sh
usr/share/rear/finalize/default/890_finish_checks.sh
usr/share/rear/init/default/005_verify_os_conf.sh

so only those ReaR scripts should be left
where the behaviour of ReaR depends on those variables:

# for v in $( grep -o '[A-Z][A-Z_]*=' usr/share/rear/lib/config-functions.sh | cut -d '=' -f1 | sort -u ) ; \
 do echo $v ; \
    find usr/sbin/rear usr/share/rear -type f | xargs grep "$v" | grep -v ': *#' ; \
    echo ======================== ; \
 done \
 | egrep -v 'usr/share/rear/lib/config-functions.sh|usr/share/rear/lib/dump-workflow.sh|usr/share/rear/lib/framework-functions.sh|usr/share/rear/lib/validate-workflow.sh|usr/sbin/rear|usr/share/rear/build/default/975_update_os_conf.sh|usr/share/rear/rescue/default/010_merge_skeletons.sh|usr/share/rear/finalize/default/890_finish_checks.sh|usr/share/rear/init/default/005_verify_os_conf.sh

OS_MASTER_VENDOR
usr/share/rear/layout/prepare/Linux-s390/205_s390_enable_disk.sh:case $OS_MASTER_VENDOR in
usr/share/rear/layout/prepare/Linux-s390/205_s390_enable_disk.sh:        LogPrintError "No code for DASD disk device enablement on $OS_MASTER_VENDOR"
usr/share/rear/rescue/GNU/Linux/310_network_devices.sh:if [[ "$ARCH" == "Linux-s390" && "$OS_MASTER_VENDOR" != "SUSE_LINUX" ]] ; then
usr/share/rear/lib/layout-functions.sh:            case $OS_MASTER_VENDOR in
========================
OS_MASTER_VENDOR_ARCH
========================
OS_MASTER_VENDOR_VERSION
========================
OS_MASTER_VENDOR_VERSION_ARCH
========================
OS_MASTER_VERSION
usr/share/rear/lib/layout-functions.sh:                    if (( $OS_MASTER_VERSION < 12 )) ; then
usr/share/rear/lib/layout-functions.sh:                        if (( $OS_MASTER_VERSION < 7 )) ; then
========================
OS_VENDOR
usr/share/rear/conf/default.conf:OS_VENDOR=generic
usr/share/rear/rescue/GNU/Linux/990_sysreqs.sh:    echo "${OS_VENDOR} ${OS_VERSION}"
usr/share/rear/lib/bootloader-functions.sh:    echo "echo \"build from host: $HOSTNAME ($OS_VENDOR $OS_VERSION $ARCH)\""
usr/share/rear/finalize/Linux-i386/610_EFISTUB_run_efibootmgr.sh:--label ${OS_VENDOR} --loader "$loader" \
usr/share/rear/finalize/Linux-i386/670_run_efibootmgr.sh:    LogPrint "Creating  EFI Boot Manager entry '$OS_VENDOR $OS_VERSION' for '$bootloader' (UEFI_BOOTLOADER='$UEFI_BOOTLOADER') "
usr/share/rear/finalize/Linux-i386/670_run_efibootmgr.sh:    Log efibootmgr --create --gpt --disk $disk --part $partition_number --write-signature --label \"${OS_VENDOR} ${OS_VERSION}\" --loader \"\\${bootloader}\"
usr/share/rear/finalize/Linux-i386/670_run_efibootmgr.sh:    if efibootmgr --create --gpt --disk $disk --part $partition_number --write-signature --label "${OS_VENDOR} ${OS_VERSION}" --loader "\\${bootloader}" ; then
========================
OS_VENDOR_ARCH
========================
OS_VENDOR_VERSION
========================
OS_VENDOR_VERSION_ARCH
========================
OS_VERSION
usr/share/rear/conf/default.conf:OS_VERSION=none
usr/share/rear/rescue/GNU/Linux/990_sysreqs.sh:    echo "${OS_VENDOR} ${OS_VERSION}"
usr/share/rear/lib/bootloader-functions.sh:    echo "echo \"build from host: $HOSTNAME ($OS_VENDOR $OS_VERSION $ARCH)\""
usr/share/rear/finalize/Linux-i386/670_run_efibootmgr.sh:    LogPrint "Creating  EFI Boot Manager entry '$OS_VENDOR $OS_VERSION' for '$bootloader' (UEFI_BOOTLOADER='$UEFI_BOOTLOADER') "
usr/share/rear/finalize/Linux-i386/670_run_efibootmgr.sh:    Log efibootmgr --create --gpt --disk $disk --part $partition_number --write-signature --label \"${OS_VENDOR} ${OS_VERSION}\" --loader \"\\${bootloader}\"
usr/share/rear/finalize/Linux-i386/670_run_efibootmgr.sh:    if efibootmgr --create --gpt --disk $disk --part $partition_number --write-signature --label "${OS_VENDOR} ${OS_VERSION}" --loader "\\${bootloader}" ; then
========================
VERSION_ID
========================

So it seems (according to that quick and dirty test)
those variables are not actually used in ReaR scripts

OS_MASTER_VENDOR_ARCH
OS_MASTER_VENDOR_VERSION
OS_MASTER_VENDOR_VERSION_ARCH
OS_VENDOR_ARCH
OS_VENDOR_VERSION
OS_VENDOR_VERSION_ARCH
VERSION_ID

@jsmeix
Copy link
Member

jsmeix commented Feb 8, 2024

What directories we have where the value of such variables
is used as directory name (also a quick and dirty test):

I exluded some generic directories
and those where the OUTPUT value

# grep -o 'OUTPUT=[A-Z]*' usr/share/rear/conf/default.conf | sort -u

OUTPUT=ISO
OUTPUT=OBDR
OUTPUT=PXE
OUTPUT=RAMDISK
OUTPUT=RAWDISK
OUTPUT=USB

or the BACKUP value

# grep -o 'BACKUP=[A-Z][A-Z0-9]*' usr/share/rear/conf/default.conf | sort -u

BACKUP=AVA
BACKUP=BACULA
BACKUP=BAREOS
BACKUP=BLOCKCLONE
BACKUP=BORG
BACKUP=CDM
BACKUP=DP
BACKUP=DUPLICITY
BACKUP=EXTERNAL
BACKUP=FDRUPSTREAM
BACKUP=GALAXY
BACKUP=GALAXY10
BACKUP=GALAXY11
BACKUP=GALAXY7
BACKUP=NBKDC
BACKUP=NBU
BACKUP=NETFS
BACKUP=NFS4SERVER
BACKUP=NSR
BACKUP=RBME
BACKUP=REQUESTRESTORE
BACKUP=RSYNC
BACKUP=SESAM
BACKUP=TSM
BACKUP=YUM
BACKUP=ZYPPER

and where GNU and Linux
is the last directory name

# pushd  usr/share/rear/ ; \
 find . \
 | sed -e 's|/[0-9][0-9][0-9].*.sh||g' \
 | sort -u \
 | egrep -v '/conf/examples|/conf/templates|/lib|/skel|/default|readme|README|shellcheckrc' \
 | egrep -v '/ISO$|/OBDR$|/PXE$|/RAMDISK$|/RAWDISK$|/USB$' \
 | egrep -v '/AVA$|/BACULA$|/BAREOS$|/BLOCKCLONE$|/BORG$|/CDM$|/DP$|/DUPLICITY$|/EXTERNAL$|/FDRUPSTREAM$|/GALAXY$|/GALAXY10$|/GALAXY11$|/GALAXY7$|/NBKDC$|/NBU$|/NETFS$|/NFS4SERVER$|/NSR$|/RBME$|/REQUESTRESTORE$|/RSYNC$|/SESAM$|/TSM$|/YUM$|/ZYPPER$' \
 | egrep -v '/GNU$|/Linux$' ; \
 popd

.
./backup
./build
./build/Debian
./build/Debian/i386
./build/OPALPBA
./build/OPALPBA/Linux-i386
./build/SUSE_LINUX
./check
./conf
./conf/Debian
./conf/Debian/ia64.conf
./conf/GNU/Linux.conf
./conf/Linux-i386.conf
./conf/Linux-ia64.conf
./conf/Linux-ppc64.conf
./conf/Linux-ppc64le.conf
./conf/SUSE_LINUX.conf
./conf/Ubuntu
./conf/Ubuntu.conf
./conf/Ubuntu/7.10.conf
./final-mount
./finalize
./finalize/Debian
./finalize/Debian/i386
./finalize/Debian/ppc64le
./finalize/Fedora
./finalize/Fedora/s390
./finalize/Linux-arm
./finalize/Linux-i386
./finalize/Linux-ia64
./finalize/Linux-ppc64
./finalize/Linux-ppc64le
./finalize/SUSE_LINUX
./finalize/SUSE_LINUX/i386
./finalize/SUSE_LINUX/ppc64
./finalize/SUSE_LINUX/ppc64le
./finalize/SUSE_LINUX/s390
./format
./init
./layout
./layout/compare
./layout/do-mount
./layout/precompare
./layout/prep-for-mount
./layout/prep-for-mount/Linux-s390
./layout/prepare
./layout/prepare/Linux-s390
./layout/recreate
./layout/save
./layout/save/FDRUPSTREAM/Linux-s390
./layout/save/Linux-arm
./output
./output/IPL
./output/IPL/Linux-s390
./output/ISO/Linux-i386
./output/ISO/Linux-ia64
./output/ISO/Linux-ppc64
./output/ISO/Linux-ppc64le
./output/OBDR/Linux-i386
./output/OBDR/Linux-ia64
./output/OBDR/Linux-ppc64
./output/OBDR/Linux-ppc64le
./output/RAWDISK/Linux-i386
./output/USB/Linux-i386
./pack
./prep
./prep/ISO/Linux-i386
./prep/ISO/Linux-ia64
./prep/Linux-arm
./prep/Linux-s390
./prep/OBDR/Linux-i386
./prep/OBDR/Linux-ia64
./prep/OPALPBA
./prep/OPALPBA/Linux-i386
./prep/RAWDISK/Linux-i386
./prep/TAPE
./prep/USB/Linux-arm
./prep/USB/Linux-i386
./prep/USB/Linux-ppc64
./prep/USB/Linux-ppc64le
./prep/USB/Linux-s390
./rescue
./restore
./restore/Fedora
./restore/NETFS/Linux-i386
./restore/OPALPBA
./restore/SUSE_LINUX
./setup
./verify
./wrapup

So it seems only those values are actually used
as directory names:

Debian
Fedora
IPL
Linux-arm
Linux-i386
Linux-ia64
Linux-ppc64
Linux-ppc64le
Linux-s390
SUSE_LINUX
Ubuntu
i386
ppc64
ppc64le
s390

By the way:
Currently I do not understand how
usr/share/rear/output/IPL/Linux-s390/800_create_ipl.sh
gets called because I fail to see
where the value IPL is set in ReaR.

@lzaoral
Copy link
Contributor Author

lzaoral commented Feb 9, 2024

I have also found a related bug. If I create a custom /etc/rear/os.conf with correct values for Fedora, the OS_* and OS_MASTER_* variables will match. Therefore, the scripts array in the SourceStage function will contain some Fedora-specific entries twice because duplicates will not be discarded:

local scripts=( $( cd $SHARE_DIR/$stage
ls -d {default,"$ARCH","$OS","$OS_MASTER_VENDOR","$OS_MASTER_VENDOR_ARCH","$OS_MASTER_VENDOR_VERSION","$OS_VENDOR","$OS_VENDOR_ARCH","$OS_VENDOR_VERSION"}/*.sh \
"$BACKUP"/{default,"$ARCH","$OS","$OS_MASTER_VENDOR","$OS_MASTER_VENDOR_ARCH","$OS_MASTER_VENDOR_VERSION","$OS_VENDOR","$OS_VENDOR_ARCH","$OS_VENDOR_VERSION"}/*.sh \
"$OUTPUT"/{default,"$ARCH","$OS","$OS_MASTER_VENDOR","$OS_MASTER_VENDOR_ARCH","$OS_MASTER_VENDOR_VERSION","$OS_VENDOR","$OS_VENDOR_ARCH","$OS_VENDOR_VERSION"}/*.sh \
"$OUTPUT"/"$BACKUP"/{default,"$ARCH","$OS","$OS_MASTER_VENDOR","$OS_MASTER_VENDOR_ARCH","$OS_MASTER_VENDOR_VERSION","$OS_VENDOR","$OS_VENDOR_ARCH","$OS_VENDOR_VERSION"}/*.sh \
| sed -e 's#/\([0-9][0-9][0-9]\)_#/!\1!_#g' | sort -t \! -k 2 | tr -d \! ) )

For example, the initrd regeneration is then executed twice (rear recover log on s390x):

...
2024-02-06 10:56:46.249979866 ======================
2024-02-06 10:56:46.251775722 Running 'restore' stage
2024-02-06 10:56:46.253627052 ======================
2024-02-06 10:56:46.259645721 Including restore/Fedora/050_copy_dev_files.sh
2024-02-06 10:56:46.270044331 Including restore/Fedora/050_copy_dev_files.sh            <-- DUPLICATE
2024-02-06 10:56:46.278551895 Including restore/default/050_remount_async.sh
2024-02-06 10:56:46.281950834 Including restore/NETFS/default/100_mount_NETFS_path.s
...
2024-02-06 10:57:07.404822080 Including finalize/default/520_confirm_finalize.sh
2024-02-06 10:57:07.408345447 Including finalize/Fedora/550_rebuild_initramfs.sh
2024-02-06 10:57:07.411208165 Original OLD_INITRD_MODULES=(  )
2024-02-06 10:57:07.417129059 New INITRD_MODULES=' dasd_eckd_mod dasd_fba_mod dasd_mod'
2024-02-06 10:57:07.425685641 Running dracut...
2024-02-06 10:57:17.506420149 Updated initrd with new drivers for kernel 6.8.0-0.rc0.20240112git70d201a40823.5.fc40.s390x.
2024-02-06 10:57:17.511338220 Including finalize/Fedora/550_rebuild_initramfs.sh        <-- DUPLICATE
2024-02-06 10:57:17.514697050 Original OLD_INITRD_MODULES=(  )
2024-02-06 10:57:17.519743182 New INITRD_MODULES=' dasd_eckd_mod dasd_fba_mod dasd_mod'
2024-02-06 10:57:17.529820643 Running dracut...
2024-02-06 10:57:26.938005410 Updated initrd with new drivers for kernel 6.8.0-0.rc0.20240112git70d201a40823.5.fc40.s390x.
2024-02-06 10:57:26.944130040 Including finalize/Fedora/s390/660_install_zipl.sh
2024-02-06 10:57:26.946353158 Installing boot loader ZIPL...
2024-02-06 10:57:28.017620673 Including finalize/Fedora/s390/660_install_zipl.sh        <-- DUPLICATE
2024-02-06 10:57:28.021901091 Including finalize/default/880_check_for_mount_by_id.sh
...

@lzaoral
Copy link
Contributor Author

lzaoral commented Feb 9, 2024

By the way:
Currently I do not understand how
usr/share/rear/output/IPL/Linux-s390/800_create_ipl.sh
gets called because I fail to see
where the value IPL is set in ReaR.

It is a supported OUTPUT value on s390x. It just seems that the documentation in default.conf is missing the OUTPUT=IPL entry.

@jsmeix
Copy link
Member

jsmeix commented Feb 9, 2024

Regarding scripts get called twice
when different variables have same value:
Ugh!
It seems whenever I look at ReaR code what I see is a mess
that has grown over longer times :-(

@jsmeix
Copy link
Member

jsmeix commented Feb 9, 2024

Regarding missing "OUTPUT=IPL" documentation in default.conf:

@lzaoral
could you please make a pull request with the missing
"OUTPUT=IPL" documentation in default.conf
because you actually use ReaR on s390x so you know better
than me what to tell our users about "OUTPUT=IPL".
I do no longer test ReaR on IBM Z and I never managed to boot the
ReaR recovery system via the initial program loader on IBM Z 'zipl'.
Thank you in advance!

@jsmeix jsmeix added bug The code does not do what it is meant to do cleanup labels Feb 9, 2024
@jsmeix jsmeix added this to the ReaR v2.8 milestone Feb 9, 2024
@lzaoral
Copy link
Contributor Author

lzaoral commented Feb 12, 2024

@jsmeix Hopefully, I'll get to the documentation of the IPL output option this week. Right now, me and @pcahyna are fixing the packaging of ReaR in Fedora (which we finally co-maintain) in the preparation of the initial branching of CentOS Stream 10.

@lzaoral
Copy link
Contributor Author

lzaoral commented Feb 13, 2024

@jsmeix I've found out, that the initial PR with s390x support in ReaR was a bit messy. The IPL option is completely redundant because it does the exact same thing as the already existing and documented RAMDISK option:

# Create the 'initial program' to boot/load the ReaR recovery system
# on IBM Z via IPL (initial program load)
LogPrint "Creating initial program for IPL on IBM Z"
RESULT_FILES+=( $KERNEL_FILE $TMP_DIR/$REAR_INITRD_FILENAME )

vs.

if test $RAMDISK_SUFFIX ; then
kernel_file="$TMP_DIR/kernel-$RAMDISK_SUFFIX"
cp $v -pLf $KERNEL_FILE $kernel_file || Error "Failed to copy KERNEL_FILE '$KERNEL_FILE'"
initrd_file="$TMP_DIR/initramfs-$RAMDISK_SUFFIX.img"
cp $v -pLf $TMP_DIR/$REAR_INITRD_FILENAME $initrd_file || Error "Failed to copy initramfs '$REAR_INITRD_FILENAME'"
fi
DebugPrint "Adding $kernel_file and $initrd_file to RESULT_FILES"
RESULT_FILES+=( $kernel_file $initrd_file )

Therefore, I suggest to just remove the IPL subtree and to add a fallback that will replace OUTPUT=IPL with OUTPUT=RAMDISK during the prep phase to be backwards compatible with existing local.conf files. Unless there are objections, I'll prepare this clean-up PR tomorrow.

@jsmeix
Copy link
Member

jsmeix commented Feb 14, 2024

@lzaoral
thank you for having a look at our s390 code!

That IPL is redundant (same as RAMDISK)
is what I also thought but I was unsure
because I could no longer test ReaR on IBM Z
and because your above
#3149 (comment)
indicated that IPL is actually used.

I fully agree with your proposed clean-up
and I look forward to your PR!

By the way regarding documentation about
ReaR on IBM Z:

Could you write some initial documentation about
"ReaR on IBM Z"?

Only something short so that we have a starting point
which then can get enhanced step by step as needed?

In particular I would be much interested in
some documentation how to use ReaR on IBM Z
because I never tested what @mutable-dan had
implemented.
But I would like to try that out (as time permits)
to get at least an initial basic experience
how ReaR can be used on IBM Z.

In particular my main point where I know basically
nothing is how to make some "IPL-able thingy"
from ReaR's initrd and the kernel and how to
actually "IPL" that on a replacement VM on IBM Z.
I got lots of IBM documenation also about booting
on IBM Z - but that is a so much different world
that I gave up trying to understand all that
(hundreds of pages of IBM Z domain specific language).
So what I would like to get is some real example
how to make some real "IPL-able thingy" and
how to actually "IPL" that on one specific
kind of IBM Z VM.

lzaoral added a commit to lzaoral/rear that referenced this issue Feb 14, 2024
The initial PR with s390x support in ReaR introduced in s390/s390x-only
OUTPUT option.  However, the OUTPUT=IPL option is completely redundant
because it does the exact same thing as the already existing and documented
OUTPUT=RAMDISK option.

This commit removes the whole IPL directory sub-tree and introduces a fallback
that replaces OUTPUT=IPL with OUTPUT=RAMDISK during the prep phase with
a deprecation warning to still be backwards compatible with existing local.conf
files.

Related: rear#3149 (comment)
lzaoral added a commit to lzaoral/rear that referenced this issue Feb 14, 2024
The initial PR with s390 support in ReaR introduced an s390-only OUTPUT=IPL
undocumented option.  However, the OUTPUT=IPL option is completely redundant
because it does the exact same thing as the already existing and documented
OUTPUT=RAMDISK option.

This commit removes the whole IPL directory sub-tree and introduces a fallback
that replaces OUTPUT=IPL with OUTPUT=RAMDISK during the prep phase with
a deprecation warning to still be backwards compatible with existing local.conf
files.

Related: rear#3149 (comment)
lzaoral added a commit to lzaoral/rear that referenced this issue Feb 14, 2024
The initial PR with s390 support in ReaR introduced an s390-only OUTPUT=IPL
undocumented option.  However, the OUTPUT=IPL option is completely redundant
because it does the exact same thing as the already existing and documented
OUTPUT=RAMDISK option.

This commit removes the whole IPL directory sub-tree and introduces a fallback
that replaces OUTPUT=IPL with OUTPUT=RAMDISK during the prep phase with
a deprecation warning to still be backwards compatible with existing local.conf
files.

Related: rear#3149 (comment)
lzaoral added a commit to lzaoral/rear that referenced this issue Feb 19, 2024
The initial PR with s390 support in ReaR introduced an s390-only OUTPUT=IPL
undocumented option.  However, the OUTPUT=IPL option is completely redundant
because it does the exact same thing as the already existing and documented
OUTPUT=RAMDISK option.

This commit removes the whole IPL directory sub-tree and introduces a fallback
that replaces OUTPUT=IPL with OUTPUT=RAMDISK during the prep phase with
a deprecation warning to still be backwards compatible with existing local.conf
files.

Related: rear#3149 (comment)
lzaoral added a commit to lzaoral/rear that referenced this issue Feb 19, 2024
lzaoral added a commit to lzaoral/rear that referenced this issue Feb 19, 2024
lzaoral added a commit to lzaoral/rear that referenced this issue Feb 26, 2024
lzaoral added a commit to lzaoral/rear that referenced this issue Feb 26, 2024
@lzaoral
Copy link
Contributor Author

lzaoral commented Feb 26, 2024

Hmm, the code used to parse the /etc/os-release is quite brittle. Problems I've encountered so far:

  1. The only reason Fedora gets classified as RHEL is this:

    $ grep -i -E '(centos|redhat|scientific|oracle)' /etc/os-release
    BUG_REPORT_URL="https://bugzilla.redhat.com/"
    REDHAT_BUGZILLA_PRODUCT="Fedora"
    REDHAT_BUGZILLA_PRODUCT_VERSION=39
    REDHAT_SUPPORT_PRODUCT="Fedora"
    REDHAT_SUPPORT_PRODUCT_VERSION=39

    which is clearly wrong. The NAME="Fedora Linux" value should have been parsed instead. edit: Using ID=fedora is even better.

  2. The VERSION_ID value is parsed incorrectly because there is no " character on Fedora:

    $ grep "^VERSION_ID=" /etc/os-release | cut -d\" -f2
    VERSION_ID=39

    The expected value is 39. The solution proposed in ReaR in Docker for development & fix package dependencies #2950 (comment) would have fixed that.

  3. The generation of the /etc/rear/os.conf file is permanent. Once it is created, it won't be regenerated unless it is manually removed. I have overridden that in the rear Fedora package but ReaR installs from upstream will contain bogus values due to bugs above and/or will contains incorrect information after major version system upgrades (this applies to any other OS).

    I don't see a reason to create this file at all because it may do more harm than good. I suggest to just get rid of it as also implied/discussed in remove os.conf creation on rear.spec file #1639 (comment) or Better user messages for finalize/default/060_compare_files.sh #2954 (comment)).

edit: reformatted

lzaoral added a commit to lzaoral/rear that referenced this issue Feb 26, 2024
@lzaoral
Copy link
Contributor Author

lzaoral commented Feb 27, 2024

While investigating a possible solution for problems above, I've also found two additional problems:

  • When OS_VENDOR is set to RedHatEnterpriseServer, OS_MASTER_VENDOR is set to Fedora and the OS_MASTER_VERSION is set to the major version for RHEL 5-7 or unchanged for newer releases. This is wrong for at least two reasons:

    1. The following version comparison will fail for newer RHELs, e.g. 8.8:
      (Fedora)
      if is_false "$user_friendly_names" ; then
      # RHEL 7 and above seems to named partitions on multipathed devices with
      # [mpath device UUID/WWID] + p + [part number] when "user_friendly_names"
      # option is FALSE.
      # For example: /dev/mapper/3600507680c82004cf8000000000000d8p1
      part_name="${device_name}p" # append p between main device and partitions
      else
      # RHEL 7 and above seems to named partitions on multipathed devices with
      # [mpath device name] + [part number] like standard disk when "user_friendly_names"
      # option is used (default).
      # For example: /dev/mapper/mpatha1
      # But the scheme in RHEL 6 need a "p" between [mpath device name] and [part number].
      # For exemple: /dev/mapper/mpathap1
      if (( $OS_MASTER_VERSION < 7 )) ; then
      part_name="${device_name}p" # append p between main device and partitions
      else
      part_name="${device_name}"
      fi
      fi

      because:
      $ bash -c '(( "8.8" < 7 ))'
      bash: line 1: ((: 8.8 < 7 : syntax error: invalid arithmetic operator (error token is ".8 < 7 ")
    2. Since OS_MASTER_VENDOR is Fedora, it would be more sensible to use the actual Fedora release that the given RHEL release branched from (e.g. 34 for RHEL 9).
  • For all SUSE-relates systems, OS_MASTER_VENDOR is set to SUSE so the following comparison will always be true:

    if [[ "$ARCH" == "Linux-s390" && "$OS_MASTER_VENDOR" != "SUSE_LINUX" ]] ; then

lzaoral added a commit to lzaoral/rear that referenced this issue Feb 27, 2024
The generation of the /etc/rear/os.conf file is permanent.  Once it is created,
it won't be regenerated unless it is manually removed.  Therefore, it will
contain incorrect values if ReaR detects them incorrectly (like in [1]) and/or
will contain stale information after major version system upgrades.

[1] rear#3149 (comment)

Related: rear#3149 (comment)
lzaoral added a commit to lzaoral/rear that referenced this issue Feb 27, 2024
The original code always assumed that the VERSION_ID value will be enclosed in
quotation marks which is false on Fedora:

```console
$ grep "^VERSION_ID=" /etc/os-release | cut -d\" -f2
VERSION_ID=39
```

Related: rear#3149 (comment)
lzaoral added a commit to lzaoral/rear that referenced this issue Feb 27, 2024
The generation of the /etc/rear/os.conf file is permanent.  Once it is created,
it won't be regenerated unless it is manually removed.  Therefore, it will
contain incorrect values if ReaR detects them incorrectly (like in [1]) and/or
will contain stale information after major version system upgrades.

[1] rear#3149 (comment)

Related: rear#3149 (comment)
lzaoral added a commit to lzaoral/rear that referenced this issue Feb 27, 2024
The original code always assumed that the VERSION_ID value will be enclosed in
quotation marks which is false on Fedora:

```console
$ grep "^VERSION_ID=" /etc/os-release | cut -d\" -f2
VERSION_ID=39
```

Related: rear#3149 (comment)
@jsmeix
Copy link
Member

jsmeix commented Mar 1, 2024

@lzaoral
when you finished all what you need to do
in particular for Fedora and Red Hat
(no rush, step by step, take your time)
and when that also works for SUSE/openSUSE
then
(as a totally separated subsequent major step)
I will have a look
if I could do such a "great cleanup work"
(i.e. simplify it to only DISTRIBUTION and ARCHITECTURE)
with reasonable effort
(provided time permits and provided I have nothing better to do).

lzaoral added a commit to lzaoral/rear that referenced this issue Mar 4, 2024
lzaoral added a commit to lzaoral/rear that referenced this issue Mar 4, 2024
The initial PR with s390 support in ReaR introduced
undocumented OUTPUT=IPL which is redundant because
it does the same thing as the documented OUTPUT=RAMDISK.
This commit removes the IPL directory sub-tree and introduces
a fallback that replaces OUTPUT=IPL with OUTPUT=RAMDISK
during the prep phase with a deprecation warning,
see rear#3153 and the related
rear#3149 (comment)
lzaoral added a commit to lzaoral/rear that referenced this issue Mar 4, 2024
Delete init/default/005_verify_os_conf.sh - reasoning:
The generation of the /etc/rear/os.conf file is permanent.
Once it is created, it won't be regenerated unless it is manually removed.
Therefore, it will contain incorrect values if ReaR detects them incorrectly
like in rear#3149 (comment)
and/or it will contain stale information after major version system upgrades,
see also rear#3149 (comment)
Furthermore in lib/config-functions.sh in SetOSVendorAndVersion()
fix the collection of VERSION_ID from /etc/os-release because
the code had falsely assumed that the VERSION_ID value
is enclosed in quotation marks which is false on Fedora,
see rear#3165 and the related
rear#3149 (comment)
lzaoral added a commit to lzaoral/rear that referenced this issue Mar 5, 2024
The initial PR with s390 support in ReaR introduced
undocumented OUTPUT=IPL which is redundant because
it does the same thing as the documented OUTPUT=RAMDISK.
This commit removes the IPL directory sub-tree and introduces
a fallback that replaces OUTPUT=IPL with OUTPUT=RAMDISK
during the prep phase with a deprecation warning,
see rear#3153 and the related
rear#3149 (comment)
lzaoral added a commit to lzaoral/rear that referenced this issue Mar 5, 2024
Delete init/default/005_verify_os_conf.sh - reasoning:
The generation of the /etc/rear/os.conf file is permanent.
Once it is created, it won't be regenerated unless it is manually removed.
Therefore, it will contain incorrect values if ReaR detects them incorrectly
like in rear#3149 (comment)
and/or it will contain stale information after major version system upgrades,
see also rear#3149 (comment)
Furthermore in lib/config-functions.sh in SetOSVendorAndVersion()
fix the collection of VERSION_ID from /etc/os-release because
the code had falsely assumed that the VERSION_ID value
is enclosed in quotation marks which is false on Fedora,
see rear#3165 and the related
rear#3149 (comment)
lzaoral added a commit to lzaoral/rear that referenced this issue Mar 7, 2024
jsmeix pushed a commit that referenced this issue Mar 8, 2024
In doc/user-guide/04-scenarios.adoc
document booting of ReaR rescue initramfs on s390/s390x, see the related
#3149 (comment)
lzaoral added a commit to lzaoral/rear that referenced this issue Mar 8, 2024
On SUSE, the OS_MASTER_VENDOR variable is set to SUSE and not SUSE_LINUX.
Since the code was still executed on this platform and no problems have been
reported so far, let's assume that it is safe and remove this redundant
condition.

Related: rear#3149 (comment)
lzaoral added a commit to lzaoral/rear that referenced this issue Mar 8, 2024
Before this change, ReaR would just grep /etc/os-release for suitable
string.  This is wrong because, e.g. Fedora was classified as RHEL
because its /etc/os-release contains the 'redhat' string in URL for its
issue tracker.

os-release(5) [1] specifies ID and ID_LIKE fields for reliable identification
of the distribution.  First, use the ID_LIKE field to deal with derivative
Linux distributions (e.g. CentOS and RHEL, or Ubuntu and Linux Mint). Then use
ID to detect distributions that are either not a derivative (e.g. Arch or Fedora)
or ReaR already recognized the derivate separately (e.g. Fedora vs. RHEL or
Debian vs. Ubuntu).

[1] https://www.freedesktop.org/software/systemd/man/latest/os-release.html

Resolves: rear#3149
lzaoral added a commit to lzaoral/rear that referenced this issue Mar 8, 2024
Contrary to its name, the OS_MASTER_VERSION variable was already used for this
purpose for some versions, e.g. RHEL 7.  This fixes version comparison on
RHEL 8 and newer.

Related: rear#3149 (comment)
lzaoral added a commit to lzaoral/rear that referenced this issue Mar 8, 2024
This change ensures that OS_VENDOR and OS_MASTER_VENDOR will not be set to
equal values which subsequently caused some ReaR stages to be sourced more
than once.

Related: rear#3149 (comment)
lzaoral added a commit to lzaoral/rear that referenced this issue Mar 8, 2024
... because the plain 'Arch' could be confused with 'Architecture'.

Related: rear#3149 (comment)
lzaoral added a commit to lzaoral/rear that referenced this issue Mar 13, 2024
On SUSE, the OS_MASTER_VENDOR variable is set to SUSE and not SUSE_LINUX.
Since the code was still executed on this platform and no problems have been
reported so far, let's assume that it is safe and remove this redundant
condition.

Related: rear#3149 (comment)
lzaoral added a commit to lzaoral/rear that referenced this issue Mar 13, 2024
Before this change, ReaR would just grep /etc/os-release for suitable
string.  This is wrong because, e.g. Fedora was classified as RHEL
because its /etc/os-release contains the 'redhat' string in URL for its
issue tracker.

os-release(5) [1] specifies ID and ID_LIKE fields for reliable identification
of the distribution.  First, use the ID_LIKE field to deal with derivative
Linux distributions (e.g. CentOS and RHEL, or Ubuntu and Linux Mint). Then use
ID to detect distributions that are either not a derivative (e.g. Arch or Fedora)
or ReaR already recognized the derivate separately (e.g. Fedora vs. RHEL or
Debian vs. Ubuntu).

[1] https://www.freedesktop.org/software/systemd/man/latest/os-release.html

Resolves: rear#3149
lzaoral added a commit to lzaoral/rear that referenced this issue Mar 13, 2024
Contrary to its name, the OS_MASTER_VERSION variable was already used for this
purpose for some versions, e.g. RHEL 7.  This fixes version comparison on
RHEL 8 and newer.

Related: rear#3149 (comment)
lzaoral added a commit to lzaoral/rear that referenced this issue Mar 13, 2024
This change ensures that OS_VENDOR and OS_MASTER_VENDOR will not be set to
equal values which subsequently caused some ReaR stages to be sourced more
than once.

Related: rear#3149 (comment)
lzaoral added a commit to lzaoral/rear that referenced this issue Mar 13, 2024
... because the plain 'Arch' could be confused with 'Architecture'.

Related: rear#3149 (comment)
lzaoral added a commit to lzaoral/rear that referenced this issue Mar 20, 2024
On SUSE, the OS_MASTER_VENDOR variable is set to SUSE and not SUSE_LINUX.
Since the code was still executed on this platform and no problems have been
reported so far, let's assume that it is safe and remove this redundant
condition.

Related: rear#3149 (comment)
lzaoral added a commit to lzaoral/rear that referenced this issue Mar 20, 2024
Before this change, ReaR would just grep /etc/os-release for suitable
string.  This is wrong because, e.g. Fedora was classified as RHEL
because its /etc/os-release contains the 'redhat' string in URL for its
issue tracker.

os-release(5) [1] specifies ID and ID_LIKE fields for reliable identification
of the distribution.  First, use the ID_LIKE field to deal with derivative
Linux distributions (e.g. CentOS and RHEL, or Ubuntu and Linux Mint). Then use
ID to detect distributions that are either not a derivative (e.g. Arch or Fedora)
or ReaR already recognized the derivate separately (e.g. Fedora vs. RHEL or
Debian vs. Ubuntu).

[1] https://www.freedesktop.org/software/systemd/man/latest/os-release.html

Resolves: rear#3149
lzaoral added a commit to lzaoral/rear that referenced this issue Mar 20, 2024
Contrary to its name, the OS_MASTER_VERSION variable was already used for this
purpose for some versions, e.g. RHEL 7.  This fixes version comparison on
RHEL 8 and newer.

Related: rear#3149 (comment)
lzaoral added a commit to lzaoral/rear that referenced this issue Mar 20, 2024
This change ensures that OS_VENDOR and OS_MASTER_VENDOR will not be set to
equal values which subsequently caused some ReaR stages to be sourced more
than once.

Related: rear#3149 (comment)
lzaoral added a commit to lzaoral/rear that referenced this issue Mar 20, 2024
... because the plain 'Arch' could be confused with 'Architecture'.

Related: rear#3149 (comment)
lzaoral added a commit to lzaoral/rear that referenced this issue Mar 20, 2024
Before this change, ReaR would just grep /etc/os-release for suitable
string.  This is wrong because, e.g. Fedora was classified as RHEL
because its /etc/os-release contains the 'redhat' string in URL for its
issue tracker.

os-release(5) [1] specifies ID and ID_LIKE fields for reliable identification
of the distribution.  First, use the ID_LIKE field to deal with derivative
Linux distributions (e.g. CentOS and RHEL, or Ubuntu and Linux Mint). Then use
ID to detect distributions that are either not a derivative (e.g. Arch or Fedora)
or ReaR already recognized the derivate separately (e.g. Fedora vs. RHEL or
Debian vs. Ubuntu).

[1] https://www.freedesktop.org/software/systemd/man/latest/os-release.html

Resolves: rear#3149
lzaoral added a commit to lzaoral/rear that referenced this issue Mar 20, 2024
Contrary to its name, the OS_MASTER_VERSION variable was already used for this
purpose for some versions, e.g. RHEL 7.  This fixes version comparison on
RHEL 8 and newer.

Related: rear#3149 (comment)
lzaoral added a commit to lzaoral/rear that referenced this issue Mar 20, 2024
This change ensures that OS_VENDOR and OS_MASTER_VENDOR will not be set to
equal values which subsequently caused some ReaR stages to be sourced more
than once.

Related: rear#3149 (comment)
lzaoral added a commit to lzaoral/rear that referenced this issue Mar 20, 2024
... because the plain 'Arch' could be confused with 'Architecture'.

Related: rear#3149 (comment)
Copy link

github-actions bot commented May 1, 2024

Stale issue message

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug The code does not do what it is meant to do cleanup
Projects
None yet
Development

No branches or pull requests

2 participants