DnfContext: make dnf_context_module_install like dnf module install

[jlebon]

This significantly revamps dnf_context_module_install. It fixes some
issues noticed during review, but also aligns the logic closer to what
dnf module install does.

[jlebon]

I can split this further into separate PRs if desired.

[jlebon]

If the overall approach looks sane, I can work on adding tests.

[jlebon]

Hmm, I'm trying to understand from the integration tests logs what part failed, but the output is quite large. In the tail of the output, I see:

2021-05-07T20:07:01.3167811Z 1 feature passed, 0 failed, 0 skipped
2021-05-07T20:07:01.3168378Z 32 scenarios passed, 0 failed, 0 skipped
2021-05-07T20:07:01.3168975Z 316 steps passed, 0 failed, 0 skipped, 0 undefined
2021-05-07T20:07:01.3169466Z Took 0m46.922s
2021-05-07T20:07:01.3179164Z ##[error]Process completed with exit code 1.

And quickly grepping the results from the other behave tests, they all say 0 failed.

[lukash]

@jlebon it's an occasional "infra" failure (of the overlay FS), you can search the logs for "OCI" to find it. Sorry about the inconvenience, it's hitting us quite often and I just can't seem to figure out what to do about it. I'll try to report to podman, but I don't have a reproducer outside of GitHub Actions. We could try to switch to docker...

Also @martinpitt would you have any idea?

I'll re-run the jobs for you (you can do it by clicking on the button in top right corner yourself), the failing action for future reference is here: https://github.com/rpm-software-management/libdnf/pull/1207/checks?check_run_id=2530538673

[martinpitt]

The re-run failed again with

 Error: executable file `behave` not found in $PATH: No such file or directory: OCI not found

I'm afraid I have no idea what behave is supposed to be. I see that you are already using options: --privileged, which ought to circumvent the faccessat() dealbreaker. You can still try --security-opt=seccomp=unconfined just in case that helps. Other than that, I'm afraid I have no idea 😢

[jlebon]

Ahh yup, OK that error is a recent podman regression I think: containers/podman#10293 (I hit the same thing locally with my pet container after updating to https://bodhi.fedoraproject.org/updates/FEDORA-2021-fc15685f73).

[lukash]

@martinpitt behave is a test framework, in this context it's just an executable on the filesystem, I've seen the same error happen with chown and others. Thank you, I just thought I'd try to ask 🙂

@jlebon I'm quite sure it's not containers/podman#10293, apparently you can get the "OCI not found" error message in multiple various cases, I've observed this (almost) from the beginning of using GitHub Actions, but somehow the frequency seems to be increasing. It's non-deterministic, sometimes it works, sometimes it happens once in the hundreds of behave invocations, sometimes even more.

Re-running again, off to attempt a podman bug report.

[lukash]

Reported here: containers/podman#10321

[jlebon]

Thanks @lukash, CI looks green now! :)

@j-mracek Could you have a look at this now? Happy to fix things up as necessary to drive this forward. Also open to have a higher-bandwidth conversation over video if you'd like.

Thanks!

[j-mracek]

@jlebon I will take a look in next days.

[j-mracek]

I also suggest that module profile installed by C API will be impossible to remove by dnf module remove command because packages reason in C API is user, but expected is group.

[jlebon]

Updated! I squashed some of the commits together to make review easier. I didn't rebase it so you can use the inter-diff to see what changed.

I also suggest that module profile installed by C API will be impossible to remove by dnf module remove command because packages reason in C API is user, but expected is group.

Can you expand on this? I think I understand what you mean, but I couldn't find in the code what marks a module's install reason.

Worth noting that in rpm-ostree at least, we always re-assemble the rootfs from scratch, so there isn't a need to remove modules the same way: we just stop adding it in the new rootfs (we do support removing base packages, but to do that at the modular level would first require fixing #1207 (comment); until then, just removing the modular RPMs directly should work fine). Obviously, I do want this code to be leveraged by dnf module install too in the future though, so happy to fix up things as needed here to help with that!

[j-mracek]

@jlebon I suggest that the PR looks good to me. It enhance behaviour of dnf_context_module_install. I propose fix the problem with dnf module remove in next PR. Is there anything else what you want to include in this PR (tests?)?

I also suggest that module profile installed by C API will be impossible to remove by dnf module remove command because packages reason in C API is user, but expected is group.

Can you expand on this? I think I understand what you mean, but I couldn't find in the code what marks a module's install reason.

The difference is that dnf module install commands shares implementation with dnf group install. The difference is that after installation of packages installed from profiles (not their dependencies) they are marked with reason group. Normally they are installed as user.
Because the context part in libdnf does not support groups, there is not yet implemented the mechanism to mark installed packages with reason group.

To investigate stored reasons you can use dnf repoquery --installed --qf "%{name}.%{arch} %{reason}"

[jlebon]

@jlebon I suggest that the PR looks good to me. It enhance behaviour of dnf_context_module_install. I propose fix the problem with dnf module remove in next PR. Is there anything else what you want to include in this PR (tests?)?

Thanks, I've added some tests to this now so this is ready to go! And doing that uncovered a bug. 🎉 Split that out as a separate commit.