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 /etc/products.d (bsc#1219004) #229

Merged
merged 2 commits into from
May 6, 2024

Conversation

rtamalin
Copy link
Collaborator

@rtamalin rtamalin commented Apr 29, 2024

The Distribution Migration System (DMS) implements a non-interactive
fully automated offline migration of systems by booting into a ISO
image that runs through the migration. In some cases, if a migration
from one product stream to another product stream fails, the normal
rollback handling doesn't always restore the products state correctly.

To address this we should include the /etc/products.d contents in the
backup that is created and restored.

Add changelog entry for upcoming v1.9.0 release stream, including the
bug tracking id.

Add unit test for the zypper Backup and Restore routines.

Also fixed two bugs found while developing the unit test:

  • CreateTarball()'s check for potential tar arguments incorrectly
    checked under the local file system hierarchy, rather than under
    the specified root hierarchy, meaning it could skip backing up
    entries that might exist under the root if they didn't exist
    locally.

  • The /var/adm/backup/system-upgrade directory created under the
    specified root hierarchy, and missing intermediary directories,
    were being created with no access permissions, meaning that only
    a privileged superuser could access them. In reality migrations
    are run as root so it didn't impact customer usage, but it did
    cause problems for the unit tests running as a non-privileged
    user.

Fixes: #231, #232

Co-authored-by: Felix Schnizlein [email protected]

@rtamalin
Copy link
Collaborator Author

During a migration I see that the templated repos.sh restore script has been updated to include the etc/products.d:

migration@localhost:~> sudo cat /system-root/var/adm/backup/system-upgrade/repos.sh 
#! /bin/sh
# change root to first parameter or use / as default
# it is needed to allow restore in installation
cd ${1:-/}
rm -rf etc/zypp/repos.d
rm -rf etc/zypp/credentials.d
rm -rf etc/zypp/services.d
rm -rf etc/products.d

tar xvf var/adm/backup/system-upgrade/repos.tar.gz --overwrite
# return back to original dir
cd -

Similarly the restore tarball includes the etc/products.d files:

...
drwxr-xr-x root/root         0 2024-04-29 16:51 etc/products.d/
lrwxrwxrwx root/root         0 2024-03-04 14:21 etc/products.d/baseproduct -> SLES_SAP.prod
-rw-r--r-- root/root      2937 2024-04-29 18:28 etc/products.d/SLES_SAP.prod
-rw-r--r-- root/root      2049 2017-11-23 10:28 etc/products.d/sle-module-public-cloud.prod
-rw-r--r-- root/root      2145 2019-10-18 19:21 etc/products.d/sle-module-legacy.prod
-rw-r--r-- root/root      2254 2017-11-23 10:48 etc/products.d/sle-module-web-scripting.prod
-rw-r--r-- root/root      2054 2017-11-23 10:25 etc/products.d/sle-module-adv-systems-management.prod
-rw-r--r-- root/root      1577 2019-08-29 10:03 etc/products.d/sle-module-containers.prod
-rw-r--r-- root/root      1814 2016-12-07 12:35 etc/products.d/sle-module-toolchain.prod
-rw-r--r-- root/root      2029 2017-04-20 23:14 etc/products.d/sle-module-hpc.prod
-rw-r--r-- root/root      2363 2019-10-18 11:58 etc/products.d/sle-sdk.prod
-rw-r--r-- root/root      1920 2019-09-02 10:54 etc/products.d/PackageHub.prod

@felixsch
Copy link
Contributor

Can you please provide a test case how to test this change? Should be easy I assume but we have our internal ruling to try to provide at least one test case per pull request :)

@rtamalin
Copy link
Collaborator Author

Can you please provide a test case how to test this change? Should be easy I assume but we have our internal ruling to try to provide at least one test case per pull request :)

Do you mean a feature test or a unit test? There don't appear to be existing unit or feature tests for backup or restore, though I may not be grokking things in the Ruby testing stuff properly...

The Distribution Migration System (DMS) implements a non-interactive
fully automated offline migration of systems by booting into a ISO
image that runs through the migration. In some cases, if a migration
from one product stream to another product stream fails, the normal
rollback handling doesn't always restore the products state correctly.

To address this we should include the /etc/products.d contents in the
backup that is created and restored.

Add changelog entry for upcoming v1.9.0 release stream, including the
bug tracking id.

Add unit test for the zypper Backup and Restore routines.

Also fix two bugs found while developing the unit test:

  * CreateTarball()'s check for potential tar arguments incorrectly
    checked under the local file system hierarchy, rather than under
    the specified root hierarchy, meaning it could skip backing up
    entries that might exist under the root if they didn't exist
    locally.

  * The /var/adm/backup/system-upgrade directory created under the
    specified root hierarchy, and missing intermediary directories,
    were being created with no access permissions, meaning that only
    a privileged superuser could access them. In reality migrations
    are run as root so it didn't impact customer usage, but it did
    cause problems for the unit tests running as a non-privileged
    user.

Fixes: #231, #232

Co-authored-by: Felix Schnizlein <[email protected]>
@rtamalin rtamalin force-pushed the products_backup_and_restore branch from 274204b to a75c944 Compare May 1, 2024 20:45
@rtamalin rtamalin requested a review from felixsch May 1, 2024 20:46
Copy link
Contributor

@felixsch felixsch left a comment

Choose a reason for hiding this comment

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

lgtm 👍

How I reviewed this pull request:

  • Ran a migration from 15.3 to 15.5 and checked that both:
    • /var/adm/backup/system-upgrade/repos.sh
    • /var/adm/backup/system-upgrade/repos.tar.gz exist
  • Checked that repos.sh includes products.d directory
  • Checked that repos.tar.gz includes products.d/ directory content
  • Let the migration rollback and check that everything as been reset

As always, if you think I missed something I should test, please let me know!🚀

@felixsch felixsch merged commit 0b005c9 into main May 6, 2024
2 checks passed
@mssola mssola deleted the products_backup_and_restore branch July 4, 2024 09:34
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.

zypper.CreateTarball() should check for path existence inside specified root, not locally
3 participants