Fixing the syntax error in the dracut module script

[Rezney]

edit(pstodulk):
The original dracut module script contained the syntax error so the functionality regarding the /dev/mapper has not been handled as expected. As well, on some systems, this crash lead into immadiate end of the script (by default on rhel 7 this doesn't happen, the script just continue as expected, so not sure what is the difference on some systems regarding the behaviour).

Otherwise we get "[: missing `]" error.

Related PR: #605

[Rezney]

Already covered in #583

[Rezney]

Reopened at @pirat89 request. We aim to merge this sooner rather then later.

[leapp-bot]

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 mergable.

If you want to re-run tests or request review, you can use following commands as a comment:

• leapp-ci build to run unit tests, copr build and e2e tests in OAMG CI
• e2e tests to run unit tests, copr build and end-to-end tests in Murphy CI (OAMG members only) [OLD PIPELINE]
• review please to notify leapp developers of review request

Note: In case there are problems with tests not being triggered automatically on new PR/commit or pending for a long time, please consider rerunning the CI by commenting leapp-ci build (might require several comments). If the problem persists, contact leapp-infra.

[pirat89]

@Rezney after discussion with QE, I am keeping it opened yet for a while.

[AloisMahdal]

Sorry but the description does not provide explanation on how this affects the upgrade.

Otherwise we get "[: missing `]" error.

...is not enough. (If that was the intent then the PR could have just deleted the offending line and not fix it.)

Please don't get confused: the fact that root cause is a syntax error may be obvious, but the the actual effect of fixing the syntax error whole another can of worms and it has not been clarfied.

[AloisMahdal]

The original dracut module script contained the syntax error so the functionality regarding the /dev/mapper has not
been handled as expected.

What functionality? What, under which circumstances does/does not happen?

As well, on some systems, this crash lead into immadiate end of the script (by default on rhel 7 this doesn't
happen, the script just continue as expected, so not sure what is the difference on some systems regarding
the behaviour).

Can you elaborate on what kind of systems are affected and how?

If we are indeed not sure then I suggest postponing the fix or removing the line until it's clarified.

[vinzenz]

we are not sure why it continues even though there was a bash syntax error. The && has been missing and this is it. It's blocking it needs to be fixed that's why I merged it. This PR has been posted over 2 months ago, I added this line originally and accidentally kept a broken PR out after forgetting to copy over all fixed line from the original fixed system.

The idea is that if /dev/mapper exists the systemd-nspawn parameters are extended to include a bind mount for /dev/mapper otherwise some parts of the system don't get mounted if they are on LVM during the execution.

[AloisMahdal]

Thanks for explanation.

That said I believe the explanation -- and test coverage -- should have been provided in the original PR.

We went with untested functionality, and the fact that it was untested was discovered only now and only accidentally (only because it was also visibly-ish broken).

</rant>

[AloisMahdal]

BTW, with regards to the "mystery" I've just looked into POSIX and the thing is that behavior where an argument is passed after ] is not defined, but Bash (rhel7 and fedora33) produces the mentioned error and exit status 2.

The script would exit immediately if -e was applied, so apparently it's not the case. (This is decided by whatever runs the script -- if it's run as executable then shebang is relevant; in our case neither shebang does specify -e.)

[pirat89]

Yep, but on some systems this leads into the same behaviour like the -e is specified. No idea how is that possible. So the fix was neccessary.