I had a working installation but at some point it stopped being able to mount.
When this error occured I tried a reinstall but it still happens:
Traceback (most recent call last):
File "/opt/rockstor/src/rockstor/rest_framework_custom/generic_view.py", line 41, in _handle_exception
yield
File "/opt/rockstor/src/rockstor/storageadmin/views/disk.py", line 418, in post
return self._update_disk_state()
^^^^^^^^^^^^^^^^^^^^^^^^^
File "/usr/lib64/python3.11/contextlib.py", line 81, in inner
return func(*args, **kwds)
^^^^^^^^^^^^^^^^^^^
File "/opt/rockstor/src/rockstor/storageadmin/views/disk.py", line 115, in _update_disk_state
attached_disks = scan_disks(MIN_DISK_SIZE)
^^^^^^^^^^^^^^^^^^^^^^^^^
File "/opt/rockstor/src/rockstor/system/osi.py", line 330, in scan_disks
blk_dev_properties: dict = {
^
File "/opt/rockstor/src/rockstor/system/osi.py", line 336, in <dictcomp>
for key, value in (
^^^^^^^^^^
ValueError: not enough values to unpack (expected 2, got 1)
This is the output of lsblk which osi.py seems to parse:
I’ve tried making scan_disks more robust but I just don’t seem to find what is causing this. I have noticed a couple of weird spaces in some of the output but can’t really figure it out.
On master there is a change to how this works. I’ve tried replacing my current osi.py with the most recent one but that also fails.
The btrfs array is still alive and well and I can mount it without any problems
Might very well be, didn’t realise the image didn’t contain the latest version of rockstor. I had updated to latest version in the previous install.
I did some refactoring of the generation of the blk_dev_properties to separate the line in a manner where spaces don’t really matter and the strip is done differently:
line = [ item.strip() for item in re.split(r'["=]', line) ]
line = [ item for i, item in enumerate(line,-1) if i %3 != 0 ]
# Device information built from each lsblk line in turn.
blk_dev_properties: dict = {
key.lower() if key != "TRAN" else "transport":
value if value != "" else None for key, value in zip(line[::2], line[1::2])
}
But doesn’t seem changes are picked up immediately when editing osi.py. For some reason, rockstor-pre fails to restart after editing, is there a checksum check or something?
@cellisten a belated welcome to the Rockstor community. I assume, @Flox and/or @phillxnet will chime in here, but for now I have actually created an issue on the github repository referencing this thread. If it is not needed we can always close it again:
Thanks @Hooverdan! I have now managed to build my change locally and it seems to be working. I’ll try to send a pull request for the github issue when I have time
There’s also a mention of a battery of tests that you can run through to ensure that at least all the standard test scenarios are not negatively impacted. Thanks!
I’ve now sent a pull request (Refactor lsblk, resolves rockstor/rockstor-core#2907 by cellisten · Pull Request #2909 · rockstor/rockstor-core · GitHub)
The test case has been updated and verified to fail on the current code but work on the new one. I had some issues running poetry run pytest from the root of the repo (it complained about settings not being configured during collection) but I did manage to run the specific test_osi.py successfully.
I might be missing something when it comes to how to run the battery of tests.
I also added .python-version to the .gitignore to be able to run everything in a local pyenv as well.
Are the unit tests expected to be run on a running server? I’m a bit confused
Ah, so most of the tests are system tests rather than unit tests (as they require a working installation to run). I will test on my running server as well
@cellisten Thanks for checking all the test. Nice.
Re:
That is not a know issue. But note entirely impossible. If you can detail exactly what you mean here that would be good. These tests are not intended to run on a production system: they are development tests and are unit tests as I see it. Keep in mind that we require in some test a DB state of sorts: so we need to use the framework and it’s fixtures to provide known DB states. the test_osi.py and some others are simpler however. We also have some API tests etc.
So re the breakage on test runs, we would need an exact reproducer. And from a source install ideally. All good stuff here however and great to have another set of eyes on ‘stuff’. Lots to do and not may fingers between the regular contributors . But we are late testing phase currently so larger, and specifically deeper changes are a worry and so may be postponed (or better approached) once we are in the next testing phase.
I guessed as much, knew I was taking a chance but I wanted to know what would happen. It solved itself after a restart so nothing to really worry about.
I’m used to unit tests being static tests with no dependencies, hence tending to call tests that rely on api endpoints or databases system tests. If those are known as unit tests in this project, no worries, just wanted to know what to expect. test_osi.py does behave as what I am used to unit tests being though but I haven’t been able to run it outside of the actual installed system the way it’s described in the test file: /opt/rockstor/.venv/bin/python -m unittest test_osi.OSITests.test_scan_disks_lsblk_parse_fail
Running poetry run pytest src/rockstor/system/tests/test_osi.py works like a charm though
