-
Notifications
You must be signed in to change notification settings - Fork 400
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat(network,dbus): meta module should check if module exists #2136
Conversation
dracut modules are design to be optional, so meta module should not just assume that all modules are available.
What problem are you trying to solve with this? |
dracut modules are design to be optional, so meta module should not just assume that all modules are available. It has been a recommended practice by dracut to drop unsupported dracut modules for a given vendor/distro/architecture that are not supported. |
@LaszloGombos why do you come to the conclusion that it has been a recommended practice? |
@LaszloGombos why are you editing my message with your reply instead of replying to it? |
By mistake. Sorry. |
Then fix that mistake by revert your edit and reply normally with your response. |
There are many example. Just picking one (not necessary the best example, but wanted to pick an old example to show a sense of trend) from 2013 for RedHat - a5b48ce In this example a dracut module - that is maintained upstream - is deleted when dracut is packaged for RedHat. Another example from today is dracut modules are deleted when arch is not s390x https://github.com/dracutdevs/dracut/blob/master/pkgbuild/dracut.spec#L229 |
Using the spec file is not a good example to reach that ( or any ) conclusion since the sole purpose of the existence of that spec file here upstream was to ease maintainership for ( mostly ) Harald as part of his $dayjob when he worked for RH and is filled with Fedora/RHEL specific things and decisions made by either the Fedora community and or customer cases (there used to be RHEL related branches here upstream and Suse was considering at one point having branches here too ) |
This is a misconception that modules are specifically designed to be optional . Modules themselves can ( and do ) have dependency on each other ( depends() ) and even can be turned into a meta module for example if upstream that the module depends upon introduce a backward incompatible change to it's prior release one way we solve that is by turning an existing module into a meta module which picks the new module with the new changes or the old module with the old changes depending on some condition, which provides a transparent migration path for downstream in the initramfs image as in the vendor/distribution/user does not have to change any configuration if that configuration points to a meta module. So is a module optional well it depends() ;) Anyways back to the PR. What you are trying to achieve here with wont work and requires a large scope to fix due to the complexity involved Using iwd as an example ( same thing is more or less applicable to nm ), it supports Repeat the above with the other networking solution and throw with and without dbus support into that mix as well so checking for existence of said modules wont work since on systemd based system, systemd-networkd module will always exist standalone or along with either iwd, nm or connman or iwd + nm or iwd + connman on non systemd based system either iwd exist standalone or with NM or Connman. So the condition of the existence of those modules can be true in multiple usecases. To solve the issue you are trying to solve, the scope of that change is Long story put short there is no quick fix, hack or a workaround that can be implemented to fix what you are trying to fix and implementing the solution (b) will take a while ( 060+ milestone since we will be dropping wicked in 059 ) |
I appreciate the consideration, the reviews and sharing your knowledge. In the PR I mentioned that I tested it locally and it did work. |
How did you test it locally? |
1./ I changed https://github.com/dracutdevs/dracut/blob/master/test/TEST-20-NFS/test.sh#L403 to
AND 2./ and removed 35network-manager dracut module directory. Confirmed that removing the 35network-manager directory resulted in running the test with network-legacy module and the test passed with the PR. Without the PR the test failed. |
That's very specific and limited testing. |
end yet no regression was caused by this PR and this PR would have provided a solution for more than two linux distributions (Gentoo and Void Linux and likely more). I find your reviews are biased towards rpm based distros. Closing. |
The purpose of the meta-module is to choose one suitable module among many candidates. It would be inappropriate to consider all of the optional candidates hard dependencies. Likewise, conditioning selection of the backend based on a transitive dependency of the backend rather than on the availability and installability of the module is inappropriate. The network module should neither know nor care what executables are available when selecting the backend, only which modules it can add to the initramfs. |
It's one of the purpose, there are other use cases for a meta modules such as providing a migration path for upstream when it introduces backwards incompatible changes between releases etc.
Right it literally makes no sense.
In most cases meta modules should neither know nor care what executable are available ( there are usecases for it, the network module does not fit such a usecase ) and most certainly should not try to solve module "conflicts", "dependency" or be somekind of dumping ground for workarounds and hacks. The problem with the network module is that all the backend modules can exist ( and in the case of sd-networkd it will always exist on a systemd based system ) and can be used with or without each other |
My argument is that tests like [[ -x $dracutsysrootdir/usr/libexec/nm-initrd-generator ]] || [[ -x $dracutsysrootdir/usr/lib/nm-initrd-generator ]] really belong in the There should be (if there is not already) a function exposed at module install time that can test for the existence of a module and, if it exists, run its if check_module "network-manager"; then
network_handler="network-manager"
elif [...] This decouples implementation details of the backends from the meta-module and will do the right thing when distributions remove incompatible modules from their packages. |
In my understanding none of the original bug reporters have sd-networkd installed. #1756 is about picking the one networking module that is expected to function when there is no ambiguity or conflict and there is no need for any kind of conflict resolution or prioritization. The network meta module is failing to pick the backend even if there is only one backend available. The intent should be to try to resolve a regression for the "no ambiguity to find the one functioning backend" before we pull in more complexity that was not in scope. |
This function is called check_module and it already exists and ready to be called - https://github.com/dracutdevs/dracut/blob/master/dracut-init.sh#L985 . This is the function that dracut.sh calls to check if the module can be installed and meta modules should call this function for each sub-module instead of re-implementing partial heuristics about the sub-package dependencies inside the meta module itself. |
dracut modules are design to be optional, so meta module should not just assume that all modules are available.
It has been a recommended practice by dracut to drop unsupported dracut modules for a given vendor/distro/architecture that are not supported - see https://github.com/dracutdevs/dracut/blob/master/pkgbuild/dracut.spec for examples
meta packages should support this practice.
Checklist
Fixes: #1756