oamg / leapp-repository

Leapp repositories containing actors for the Leapp framework (https://github.com/oamg/leapp). Currently provides leapp repositories for in-place upgrades of RHEL systems.
Apache License 2.0
52 stars 146 forks source link

Update pam_userdb database backend for RHEL10 #1289

Closed ikerexxe closed 4 weeks ago

ikerexxe commented 2 months ago

pam_userdb module changed its backend database technology from lidb to gdbm for RHEL10. This requires a set of leapp actors to perform the database migration automatically when upgrading to RHEL10.

Three actors were created: PAM service folder scan to detect the location of the database, database location reporting and conversion to the new format.

The changes include unit and component tests to.

Jira: SSSD-7379

github-actions[bot] commented 2 months ago

Thank you for contributing to the Leapp project!

Please note that every PR needs to comply with the Leapp Guidelines and must pass all tests in order to be mergeable. If you want to request a review or rebuild a package in copr, you can use following commands as a comment:

Packit will automatically schedule regression tests for this PR's build and latest upstream leapp build. However, here are additional useful commands for packit:

Note that first time contributors cannot run tests automatically - they need to be started by a reviewer.

It is possible to schedule specific on-demand tests as well. Currently 2 test sets are supported, beaker-minimal and kernel-rt, both can be used to be run on all upgrade paths or just a couple of specific ones. To launch on-demand tests with packit:

See other labels for particular jobs defined in the .packit.yaml file.

Please open ticket in case you experience technical problem with the CI. (RH internal only)

Note: In case there are problems with tests not being triggered automatically on new PR/commit or pending for a long time, please contact leapp-infra.

ikerexxe commented 2 months ago

@matejmatuska the code has been updated taking into account your feedback. Looking forward to your next round of reviews

ikerexxe commented 1 month ago

Added a couple more comments. Also since this has grown to 4 actors, can you move them into a subdirectory in .../el9toel10/actors/? It could be named pamuserdb or maybe even just pam.

I prefer to use pamuserdb as we don't know if we'll need to add additional actors for other modules.

matejmatuska commented 1 month ago

/packit test

matejmatuska commented 1 month ago

/packit build

matejmatuska commented 1 month ago

@ikerexxe Linters report wrong imports order, can you fix that? We have make lint_fix that should take care of it.

I am now running packit builds and will run packit test again then (first time they failed because there were no builds)

ikerexxe commented 1 month ago

@ikerexxe Linters report wrong imports order, can you fix that? We have make lint_fix that should take care of it.

Running make lint* gets stuck in my computer. I've run isort manually and that should fix those problems. Is there any other linter I should be aware of?

matejmatuska commented 1 month ago

/packit build

pirat89 commented 1 month ago

/packit copr-build

matejmatuska commented 1 month ago

I tested the "success case": The DB is properly found:

Risk Factor: info
Title: pam_userdb databases will be converted to GDBM
Summary: On RHEL 10, GDMB is used by pam_userdb as it's backend database, replacing BerkeleyDB. Existing pam_userdb databases will be converted to GDBM. The following databases will be converted:
    - /root/userpass
Key: 85d0ac0988c109032b7c364bdd252f1cb543744b

Converted to GDBM:

Sep 09 12:24:26 localhost upgrade[831]: 2024-09-09 12:24:26.344 INFO     PID: 1 leapp.workflow.Preparation: Executing actor convert_pam_user_db
Sep 09 12:24:26 localhost upgrade[1526]: 2024-09-09 12:24:26.362 DEBUG    PID: 688 leapp.workflow.Preparation.convert_pam_user_db: External command has started: ['db_converter', '--src', '/root/userpass.db', '--dest', '/root/userpass.gdbm']  
Sep 09 12:24:26 localhost upgrade[1526]: 2024-09-09 12:24:26.392 DEBUG    PID: 688 leapp.workflow.Preparation.convert_pam_user_db: External command has finished: ['db_converter', '--src', '/root/userpass.db', '--dest', '/root/userpass.gdbm']

And the original DB is properly cleaned up:

Sep 09 12:28:24 localhost upgrade[40008]: 2024-09-09 12:28:24.696 INFO     PID: 1 leapp.workflow.Applications: Executing actor remove_old_pam_user_db
Sep 09 12:28:24 localhost upgrade[40741]: 2024-09-09 12:28:24.812 DEBUG    PID: 726 leapp.workflow.Applications.remove_old_pam_user_db: External command has started: ['rm', '-f', '/root/userpass.db']                                           
Sep 09 12:28:24 localhost upgrade[40741]: 2024-09-09 12:28:24.821 DEBUG    PID: 726 leapp.workflow.Applications.remove_old_pam_user_db: External command has finished: ['rm', '-f', '/root/userpass.db']
ikerexxe commented 1 month ago

I think the PR is ready for proper review and merging, so I'm moving it.

matejmatuska commented 1 month ago

/packit copr-build

matejmatuska commented 1 month ago

Tested the error case with an updated RPM containing the patch in db_convertor (libdb-utils-5.3.28-55.el9.x86_64.rpm).

The upgrade is stopped with an error in PreparationPhase as expected:

============================================================
                           ERRORS
============================================================
2024-09-27 13:04:32.824713 [ERROR] Actor: convert_pam_user_db
Message: Cannot convert pam_userdb database.
Summary:
    Details: Command ['db_converter', '--src', '/root/userdb.db', '--dest', '/root/userdb.gdbm'] failed with exit code 1.: BDB0004 fop_read_meta: /root/userdb.db: unexpected file type or format
             Can't open database - 22
============================================================
                       END OF ERRORS
============================================================
Debug output written to /var/log/leapp/leapp-upgrade.log
...
matejmatuska commented 4 weeks ago

/packit retest-failed

matejmatuska commented 4 weeks ago

LGTM, please rebase and squash the commits Some 8->9 tests in TF are failing but it seems like a problem with the tests.

ikerexxe commented 4 weeks ago

LGTM, please rebase and squash the commits Some 8->9 tests in TF are failing but it seems like a problem with the tests.

Done!

matejmatuska commented 4 weeks ago

Merged, thanks again for the contribution!