Account for eventual double slahes in the conversion from legacy to poetry paths #2757

[FroggyFlox]

Fixes #2757
@phillxnet, @Hooverdan96: ready for review.

As detailed in #2757, we currently fail to convert legacy paths to poetry paths for our binaries if the legacy path somehow includes double slashes (/opt/rockstor//bin instead of /opt/rockstor/bin).

This pull-request simply updates the pattern used to find the legacy paths so that they are properly converted as they should.

Demonstration

1. Export a Share from the UI
2. Setup and enable a scheduled task of type snapshot
3. Manually edit /etc/cron.d/rockstortab to use legacy path with a double slash (this mimics the error reported in Failure to correct poetry paths after update #2757)
4. Run poetry initrock to trigger the "conversion logic":

buildvm155:/opt/rockstor # poetry run initrock
(...)
2023-12-20 11:38:36,289: ### BEGIN Establishing poetry path to binaries in local files...
2023-12-20 11:38:36,289: samba_config already looks good.
2023-12-20 11:38:36,290: Set rockstor_crontab to mask 0o600
2023-12-20 11:38:36,290: The path to binaries in rockstor_crontab (/etc/cron.d/rockstortab) has been updated.
2023-12-20 11:38:36,290: The replication_crontab (/etc/cron.d/replicationtab) could not be found
2023-12-20 11:38:36,290: ### DONE establishing poetry path to binaries in local files.

The crontab is indeed looking as it should:

buildvm155:/opt/rockstor # cat /etc/cron.d/rockstortab
SHELL=/bin/bash
PATH=/sbin:/bin:/usr/sbin:/usr/bin
MAILTO=root
# These entries are auto generated by Rockstor. Do not edit.
*/5 * * * * root /opt/rockstor/.venv/bin/st-snapshot 2 \*-*-*-*-*-*

and a few minutes later, CRON was detected running the task:

Dec 20 11:40:01 buildvm155 CRON[29845]: (root) CMD (/opt/rockstor/.venv/bin/st-snapshot 2 \*-*-*-*-*-*)
Dec 20 11:40:02 buildvm155 CRON[29844]: (root) CMDEND (/opt/rockstor/.venv/bin/st-snapshot 2 \*-*-*-*-*-*)

and we can indeed see the snapshot in the webUI:

Unit testing

One new test was added to cover system.osi.replace_inline_pattern(), the function responsible to identify and correct these paths. NOTE, however, that the new test requires to manually set the pattern in the test as it is set in initrock.establish_poetry_paths(). This was preferred over a test of the latter directly as it would have required extensive mocking and maybe some refactoring of that function itself. While this is sub-optimal, it still covers how replace_inline_pattern() should behave and avoid extensive effort for a function that won't be needed in the mid- to long-term.

All tests still pass:

buildvm155:/opt/rockstor/src/rockstor # poetry run django-admin test
Found 279 test(s).
Creating test database for alias 'default'...
Creating test database for alias 'smart_manager'...
System check identified no issues (0 silenced).
.......................................................................................................................................................................................................................................................................................
----------------------------------------------------------------------
Ran 279 tests in 12.517s

OK

Note

@phillxnet, note that I left the Black formatting as a separate commit as it reformatted a lot of test_osi.py... All tests still pass so it seems OK but I wanted to point it to you in case it matters.

[Hooverdan96]

@FroggyFlox this looks good to me.

Crazy, how a 3-character change requires 90+ of code lines for a new test 😄.

[phillxnet]

@FroggyFlox Thanks for seeing to this. Much appreciated: and nice find.

Re the new test, this is great: plus we now have an example of the long awaited mock_open.

I've done a test rpmbuild and all looks good re the new test:

Found 279 test(s).
Creating test database for alias 'smart_manager'...
System check identified no issues (0 silenced).
.......................................................................................................................................................................................................................................................................................
----------------------------------------------------------------------
Ran 279 tests in 31.141s
OK

Merging so we can rebase ongoing development.

Thanks for the extensive exposition.

[phillxnet]

@Hooverdan96 Thanks for taking a look at this. As always much appreciated.