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
48 stars 144 forks source link

Fix detection when a bootable partition in on RAID #1260

Closed mscherer closed 1 month ago

mscherer commented 1 month ago

On my server, leapp preupgrade fail with the following error:

Traceback (most recent call last):
  File "/usr/lib64/python2.7/multiprocessing/process.py", line 258, in _bootstrap
    self.run()
  File "/usr/lib64/python2.7/multiprocessing/process.py", line 114, in run
    self._target(*self._args, **self._kwargs)
  File "/usr/lib/python2.7/site-packages/leapp/repository/actor_definition.py", line 74, in _do_run
    actor_instance.run(*args, **kwargs)
  File "/usr/lib/python2.7/site-packages/leapp/actors/__init__.py", line 289, in run
    self.process(*args)
  File "/usr/share/leapp-repository/repositories/system_upgrade/el7toel8/actors/scangrubdevpartitionlayout/actor.py", line 18, in process
    scan_layout_lib.scan_grub_device_partition_layout()
  File "/usr/share/leapp-repository/repositories/system_upgrade/el7toel8/actors/scangrubdevpartitionlayout/libraries/scan_layout.py", line 91, in scan_grub_device_partition_layout
    dev_info = get_partition_layout(device)
  File "/usr/share/leapp-repository/repositories/system_upgrade/el7toel8/actors/scangrubdevpartitionlayout/libraries/scan_layout.py", line 79, in get_partition_layout
    part_start = int(part_info[2]) if len(part_info) == len(part_all_attrs) else int(part_info[1])
ValueError: invalid literal for int() with base 10: '*'

This is caused by the following line:

/dev/sda1   *        2048     1026047      512000   fd  Linux raid autodetect

I have my server on EL7 with / using a Linux RAID so len(part_info) != len(part_all_attrs), hence why it try to convert '*' to int.

github-actions[bot] commented 1 month 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.

fernflower commented 1 month ago

Could you please add a unit test?

mscherer commented 1 month ago

I modified the existing one to deal with that case, so do you prefer a separate test just for that ?

mscherer commented 1 month ago

I found a not too ugly way to write the test (eg, not cut and paste), hope that work.

pirat89 commented 1 month ago

@mscherer unit test is now reproducing original issue. so it seems it's not fixed :) btw, in case you would like to test locally, you can try (in the main dir) TEST_CONTAINER=rhel7 make test_container or you can use test_container_no_lint skip linters. so you do not need to wait for us to retrigger unit-tests execution every time. see make help for more if interested.

mscherer commented 1 month ago

I am trying to find a VM to run the test without bothering you too much (it is just full of small papercuts)

mscherer commented 1 month ago

And now I managed to get a VM, it seems to run fine.

pirat89 commented 1 month ago

/packit copr-build

fernflower commented 1 month ago

Thanks for the unit test!

mscherer commented 1 month ago

Is there something waiting for me ? From a quick look at the CI, the 3 tests fail for different reasons and they do not look like related to the patch itself (unless I am mistaken)

pirat89 commented 1 month ago

Hi @mscherer , actually it's now problem on our side. Most likely we will ping you during the next week, asking for rebase when tests are fixed, so we can see that there is not introduced another regression. the code looks good to me, just want to be sure that it will be final solution and we will not need to investigate similar issue again soon. And we are also little bit busy last week.

Downstream ticket with basically same issue has been reported today:

Thank you for the contribution!!

pirat89 commented 1 month ago

Hi @mscherer, the test execution in master branch should be fixed now. Can you please rebase your branch?

mscherer commented 1 month ago

@pirat89 done

pirat89 commented 1 month ago

/packit copr-build